diff --git a/app/src-tauri/src/marketplace/catalog.rs b/app/src-tauri/src/marketplace/catalog.rs index 1763c07..9c29e31 100644 --- a/app/src-tauri/src/marketplace/catalog.rs +++ b/app/src-tauri/src/marketplace/catalog.rs @@ -648,45 +648,117 @@ fn parse_folders(tree: &dyn TreeView, kind: ItemKind, folder: &str, out: &mut Ve /// Keys of a plugin's catalog entry or `plugin.json` that make Claude Code /// run something or add commands. const PLUGIN_RUNNABLE_KEYS: &[&str] = &["hooks", "mcpServers", "lspServers", "commands"]; +/// Of those, the keys whose value may instead be a path (or a list of paths) +/// to a JSON file inside the plugin, which is then what runs. +const PLUGIN_PATH_KEYS: &[&str] = &["hooks", "mcpServers", "lspServers"]; +/// Files in a plugin's root folder that declare what it runs. +const PLUGIN_RUNNABLE_FILES: &[&str] = &["hooks/hooks.json", ".mcp.json", ".lsp.json"]; -fn runnable_fields(label: &str, json: &serde_json::Value, out: &mut Vec) { - for key in PLUGIN_RUNNABLE_KEYS { - if let Some(value) = json.get(key) { - out.push(PluginComponent { - label: format!("{}: {}", label, key), - content: truncate_preview(&serde_json::to_string_pretty(value).unwrap_or_default()), - }); +/// A path a plugin gives for one of its own files, as a path relative to the +/// plugin root; refused unless it stays inside the plugin folder. +fn plugin_relative_path(value: &str) -> Result { + let outside = || format!("{:?} points outside the plugin folder", value); + if value.starts_with('/') || value.contains('\\') || value.contains(':') { + return Err(outside()); + } + let mut parts = Vec::new(); + for part in value.split('/') { + match part { + "" | "." => {} + ".." => return Err(outside()), + p => parts.push(p), } } + if parts.is_empty() { + return Err(format!("{:?} does not name a file in the plugin", value)); + } + Ok(parts.join("/")) } -/// What a plugin brings that can run (PR review #4): its catalog entry's -/// and `plugin.json`'s hooks / MCP / LSP servers / commands, and the -/// folder's `hooks/hooks.json`, `.mcp.json` and `commands/`. +/// The component, shown whole: a runnable manifest cut short could hide a +/// hook (round 2), so anything over the manifest cap refuses the plugin. +fn whole_component(label: String, content: String) -> Result { + if content.len() as u64 > MAX_MANIFEST_BYTES { + return Err(format!( + "{} is larger than {} and cannot be shown for review", + label, + describe_size(MAX_MANIFEST_BYTES) + )); + } + Ok(PluginComponent { label, content }) +} + +/// A plugin file, whole; missing or oversized files refuse the plugin. +fn plugin_file(tree: &dyn TreeView, root: &str, rel: &str) -> Result, String> { + read_utf8(tree, &format!("{}/{}", root, rel), MAX_MANIFEST_BYTES) + .map_err(|e| e.replacen(&format!("{}/", root), "", 1)) +} + +fn runnable_fields( + tree: &dyn TreeView, + root: &str, + label: &str, + json: &serde_json::Value, + out: &mut Vec, +) -> Result<(), String> { + for key in PLUGIN_RUNNABLE_KEYS { + let Some(value) = json.get(key) else { + continue; + }; + let paths: Option> = match value { + serde_json::Value::String(p) => Some(vec![p.as_str()]), + serde_json::Value::Array(items) if items.iter().all(|v| v.is_string()) => { + Some(items.iter().filter_map(|v| v.as_str()).collect()) + } + _ => None, + }; + match paths { + Some(paths) if PLUGIN_PATH_KEYS.contains(key) => { + for path in paths { + let rel = plugin_relative_path(path) + .map_err(|e| format!("{}: {} {}", label, key, e))?; + let text = plugin_file(tree, root, &rel)?.ok_or_else(|| { + format!( + "{}: {} names {}, which is not in the plugin", + label, key, rel + ) + })?; + out.push(whole_component( + format!("{}: {} → {}", label, key, rel), + text, + )?); + } + } + _ => out.push(whole_component( + format!("{}: {}", label, key), + serde_json::to_string_pretty(value).unwrap_or_default(), + )?), + } + } + Ok(()) +} + +/// What a plugin brings that can run (PR review #4, round 2): its catalog +/// entry's and `plugin.json`'s hooks / MCP / LSP servers / commands — with +/// path-valued ones resolved inside the plugin and shown as the files they +/// name — the folder's `hooks/hooks.json`, `.mcp.json` and `.lsp.json`, and +/// `commands/`. Everything is shown whole; an `Err` makes the plugin +/// invalid, since it could not be reviewed. fn plugin_components( tree: &dyn TreeView, entry: &serde_json::Value, root: &str, -) -> Vec { +) -> Result, String> { let mut out = Vec::new(); - runnable_fields("marketplace.json entry", entry, &mut out); - let manifest = format!("{}/.claude-plugin/plugin.json", root); - if let Ok(Some(text)) = read_utf8(tree, &manifest, MAX_MANIFEST_BYTES) { - if let Ok(json) = serde_json::from_str::(&text) { - runnable_fields(".claude-plugin/plugin.json", &json, &mut out); - } + runnable_fields(tree, root, "marketplace.json entry", entry, &mut out)?; + if let Some(text) = plugin_file(tree, root, ".claude-plugin/plugin.json")? { + let json: serde_json::Value = serde_json::from_str(&text) + .map_err(|e| format!(".claude-plugin/plugin.json is not valid JSON: {}", e))?; + runnable_fields(tree, root, ".claude-plugin/plugin.json", &json, &mut out)?; } - for file in ["hooks/hooks.json", ".mcp.json"] { - match read_utf8(tree, &format!("{}/{}", root, file), MAX_MANIFEST_BYTES) { - Ok(Some(text)) => out.push(PluginComponent { - label: file.to_string(), - content: truncate_preview(&text), - }), - Ok(None) => {} - Err(e) => out.push(PluginComponent { - label: file.to_string(), - content: e, - }), + for file in PLUGIN_RUNNABLE_FILES { + if let Some(text) = plugin_file(tree, root, file)? { + out.push(whole_component(file.to_string(), text)?); } } if let Ok(Some(children)) = tree.list_dir(&format!("{}/commands", root)) { @@ -699,7 +771,7 @@ fn plugin_components( .join("\n"), }); } - out + Ok(out) } fn parse_plugins(tree: &dyn TreeView, out: &mut Vec) { @@ -745,7 +817,10 @@ fn parse_plugins(tree: &dyn TreeView, out: &mut Vec) { .collect::>() .join("\n"); } - it.plugin_components = plugin_components(tree, &entry, &path); + match plugin_components(tree, &entry, &path) { + Ok(components) => it.plugin_components = components, + Err(e) => it.invalid = Some(e), + } } Err(e) => it.invalid = Some(e), } @@ -1201,6 +1276,112 @@ mod tests { ); } + fn plugin_repo(entry_extra: &str, plugin_json: &str) -> MemTree { + let catalog = format!( + r#"{{"plugins":[{{"name":"p","source":"./p"{}}}]}}"#, + entry_extra + ); + MemTree::new() + .file("plugins/.claude-plugin/marketplace.json", &catalog) + .file("plugins/p/.claude-plugin/plugin.json", plugin_json) + .file("plugins/p/skills/s/SKILL.md", "x") + } + + fn plugin(t: &MemTree) -> CatalogItem { + parse_catalog(t).into_iter().find(|i| i.key == "p").unwrap() + } + + /// Round 2 (#4): a `hooks` / `mcpServers` / `lspServers` value that is a + /// path (or a list of paths) is shown as the referenced file's contents. + #[test] + fn path_valued_plugin_components_show_the_referenced_files() { + let t = plugin_repo( + r#","hooks":"./config/entry-hooks.json""#, + r#"{"name":"p","mcpServers":["./mcp/a.json","mcp/b.json"],"lspServers":"./lsp.json"}"#, + ) + .file( + "plugins/p/config/entry-hooks.json", + r#"{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"entry-hook-cmd"}]}]}}"#, + ) + .file("plugins/p/mcp/a.json", r#"{"a":{"command":"mcp-a-cmd"}}"#) + .file("plugins/p/mcp/b.json", r#"{"b":{"command":"mcp-b-cmd"}}"#) + .file("plugins/p/lsp.json", r#"{"l":{"command":"lsp-cmd"}}"#); + let p = plugin(&t); + assert_eq!(p.invalid, None); + let find = |label: &str| { + p.plugin_components + .iter() + .find(|c| c.label == label) + .unwrap_or_else(|| panic!("{label} missing: {:?}", p.plugin_components)) + .content + .clone() + }; + assert!( + find("marketplace.json entry: hooks → config/entry-hooks.json") + .contains("entry-hook-cmd") + ); + assert!(find(".claude-plugin/plugin.json: mcpServers → mcp/a.json").contains("mcp-a-cmd")); + assert!(find(".claude-plugin/plugin.json: mcpServers → mcp/b.json").contains("mcp-b-cmd")); + assert!(find(".claude-plugin/plugin.json: lspServers → lsp.json").contains("lsp-cmd")); + } + + #[test] + fn a_component_path_outside_the_plugin_or_missing_makes_it_invalid() { + for (value, reason) in [ + (r#""../other/hooks.json""#, "outside the plugin folder"), + (r#""/etc/hooks.json""#, "outside the plugin folder"), + (r#""./nope.json""#, "not in the plugin"), + ] { + let t = plugin_repo("", &format!(r#"{{"name":"p","hooks":{value}}}"#)) + .file("plugins/other/hooks.json", "{}"); + let p = plugin(&t); + let why = p.invalid.unwrap_or_default(); + assert!(why.contains(reason), "{value}: {why}"); + } + } + + #[test] + fn a_plugin_lsp_json_is_listed() { + let t = plugin_repo("", r#"{"name":"p"}"#) + .file("plugins/p/.lsp.json", r#"{"go":{"command":"gopls-cmd"}}"#); + let p = plugin(&t); + let lsp = p + .plugin_components + .iter() + .find(|c| c.label == ".lsp.json") + .unwrap(); + assert!(lsp.content.contains("gopls-cmd")); + } + + /// Round 2 (#2): runnable manifests are shown whole (up to the 1 MiB + /// manifest cap), never cut; one that cannot be shown whole is refused. + #[test] + fn plugin_components_are_shown_whole_or_the_plugin_is_refused() { + let padded = format!( + r#"{{"pad":"{}","hooks":{{"Stop":[{{"hooks":[{{"type":"command","command":"hidden-cmd"}}]}}]}}}}"#, + "x".repeat(200 * 1024) + ); + let t = plugin_repo("", r#"{"name":"p"}"#).file("plugins/p/hooks/hooks.json", &padded); + let p = plugin(&t); + assert_eq!(p.invalid, None); + let hooks = p + .plugin_components + .iter() + .find(|c| c.label == "hooks/hooks.json") + .unwrap(); + assert!(hooks.content.contains("hidden-cmd"), "cut short"); + assert!(!hooks.content.contains("(truncated)")); + + let huge = "x".repeat(MAX_MANIFEST_BYTES as usize + 1); + let t = plugin_repo("", r#"{"name":"p"}"#).file("plugins/p/.mcp.json", &huge); + let why = plugin(&t).invalid.unwrap_or_default(); + assert!(why.contains(".mcp.json is larger than 1 MiB"), "{why}"); + + let t = plugin_repo("", "{ not json"); + let why = plugin(&t).invalid.unwrap_or_default(); + assert!(why.contains("plugin.json is not valid JSON"), "{why}"); + } + #[test] fn plugin_catalog_entry_is_returned_verbatim() { let entry = plugin_catalog_entry(&full_repo(), "example-plugin").unwrap();