diff --git a/crates/application/src/plugin/mod.rs b/crates/application/src/plugin/mod.rs index a20de204..c4a9cb4f 100644 --- a/crates/application/src/plugin/mod.rs +++ b/crates/application/src/plugin/mod.rs @@ -2372,6 +2372,7 @@ impl InstallPluginFromArchive { self.validator.as_ref(), self.events.as_ref(), self.mcp.as_ref(), + None, self.packages .install_from_archive(&LocalPath::new(path)) .await @@ -2411,12 +2412,27 @@ impl InstallPluginFromDirectory { /// Executes the use case. pub async fn execute(&self, path: String) -> Result { + self.execute_with_expected_plugin_id(path, None).await + } + + /// Executes the use case and optionally requires the staged manifest to carry + /// a specific plugin id. The guard is evaluated before the package is committed. + pub async fn execute_with_expected_plugin_id( + &self, + path: String, + expected_plugin_id: Option, + ) -> Result { + let expected_plugin_id = expected_plugin_id + .map(PluginId::new) + .transpose() + .map_err(|e| AppError::Invalid(e.to_string()))?; install_from_staged( self.packages.as_ref(), self.registry.as_ref(), self.validator.as_ref(), self.events.as_ref(), self.mcp.as_ref(), + expected_plugin_id.as_ref(), self.packages .install_from_directory(&LocalPath::new(path)) .await @@ -2597,10 +2613,20 @@ async fn install_from_staged( validator: &dyn PluginManifestValidator, events: &dyn EventBus, mcp: &dyn PluginMcpSupervisor, + expected_plugin_id: Option<&PluginId>, staged: StagedPluginPackage, ) -> Result { let review = review_staged(packages, validator, &staged).await?; let plugin_id = review.manifest.id.clone(); + if let Some(expected) = expected_plugin_id { + if expected != &plugin_id { + return Err(AppError::Invalid(format!( + "plugin manifest id `{}` does not match expected plugin `{}`", + plugin_id.as_str(), + expected.as_str() + ))); + } + } crate::diag!( "[plugins] install staged reviewed plugin={} version={} hash={} source={}", plugin_id.as_str(), @@ -2620,7 +2646,7 @@ async fn install_from_staged( source: review.source.clone(), content_hash: ContentHash::new(review.content_hash.clone()) .map_err(|e| AppError::Invalid(e.to_string()))?, - restart_required: true, + restart_required: false, error: None, }; if let Err(err) = runtime_plugin_from_entry(packages, validator, entry.clone()).await { @@ -2674,7 +2700,7 @@ async fn install_from_staged( Ok(PluginInstallResult { plugin: admin, review, - restart_required: true, + restart_required: false, }) } @@ -3642,7 +3668,7 @@ mod tests { PluginStoreError, StoreError, }; use domain::remote::RemoteRef; - use domain::{BackgroundTaskState, ProjectPath}; + use domain::{BackgroundTaskState, PluginVersion, ProjectPath}; use std::collections::{HashMap, VecDeque}; use std::sync::Mutex; @@ -4646,6 +4672,68 @@ mod tests { assert_eq!(&*mcp.stops.lock().unwrap(), &["dev.acme.gitgraph"]); } + #[tokio::test] + async fn directory_install_is_soft_refreshable_and_publishes_installed_event() { + let packages = Arc::new(FakePackages::with_manifest(valid_manifest())); + let registry = Arc::new(FakeRegistry::default()); + let events = Arc::new(FakeEvents::default()); + let install = InstallPluginFromDirectory::new( + packages, + registry.clone(), + Arc::new(validator()), + events.clone(), + Arc::new(FakeMcp::default()), + ); + + let result = install.execute("/source/plugin".to_owned()).await.unwrap(); + + assert!(!result.restart_required); + assert!(!result.plugin.restart_required); + assert!( + !registry + .load_registry() + .await + .unwrap() + .find(&plugin_id()) + .unwrap() + .restart_required + ); + assert_eq!( + events.events.lock().unwrap().as_slice(), + &[DomainEvent::PluginInstalled { + plugin_id: plugin_id(), + version: PluginVersion::new("1.2.3").unwrap(), + }] + ); + } + + #[tokio::test] + async fn directory_install_rejects_expected_plugin_id_mismatch_before_commit() { + let registry = Arc::new(FakeRegistry::default()); + let events = Arc::new(FakeEvents::default()); + let install = InstallPluginFromDirectory::new( + Arc::new(FakePackages::with_manifest(valid_manifest())), + registry.clone(), + Arc::new(validator()), + events.clone(), + Arc::new(FakeMcp::default()), + ); + + let error = install + .execute_with_expected_plugin_id( + "/source/plugin".to_owned(), + Some("dev.acme.other".to_owned()), + ) + .await + .unwrap_err(); + + assert!( + matches!(error, AppError::Invalid(message) if message.contains("does not match expected plugin")) + ); + assert!(registry.load_registry().await.unwrap().plugins.is_empty()); + assert!(events.events.lock().unwrap().is_empty()); + } + #[derive(Default)] struct FakeProjectStore { projects: Mutex>, diff --git a/crates/backend/src/lib.rs b/crates/backend/src/lib.rs index 8c92c3ad..38ee3d2f 100644 --- a/crates/backend/src/lib.rs +++ b/crates/backend/src/lib.rs @@ -146,9 +146,17 @@ fn spawn_detached(future: impl std::future::Future + Send + 'static } struct AppPluginToolProvider { + install_from_directory: Arc, reload: Arc, } +#[derive(serde::Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct PluginInstallFromDirectoryToolArgs { + path: String, + expected_plugin_id: Option, +} + #[derive(serde::Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] struct PluginReloadToolArgs { @@ -160,11 +168,32 @@ struct PluginReloadToolArgs { impl PluginToolProvider for AppPluginToolProvider { async fn handle_plugin_tool( &self, - _project: &Project, + project: &Project, _requester: &str, name: &str, arguments: Value, ) -> Result { + if name == "idea_plugin_install_from_directory" { + let args: PluginInstallFromDirectoryToolArgs = serde_json::from_value(arguments) + .map_err(|err| { + PluginToolError::new("INVALID", format!("invalid arguments: {err}")) + })?; + let path = resolve_confined_plugin_directory(project, &args.path)?; + let result = self + .install_from_directory + .execute_with_expected_plugin_id( + path.to_string_lossy().into_owned(), + args.expected_plugin_id, + ) + .await + .map_err(|err| PluginToolError::new(err.code(), err.to_string()))?; + return serde_json::to_value(result).map_err(|err| { + PluginToolError::new( + "INTERNAL", + format!("failed to serialize plugin install result: {err}"), + ) + }); + } if name != "idea_plugin_reload" { return Err(PluginToolError::new( "NOT_FOUND", @@ -190,9 +219,63 @@ impl PluginToolProvider for AppPluginToolProvider { } } +fn resolve_confined_plugin_directory( + project: &Project, + requested: &str, +) -> Result { + if requested.trim().is_empty() { + return Err(PluginToolError::new("INVALID", "path must not be empty")); + } + let root = Path::new(project.root.as_str()) + .canonicalize() + .map_err(|err| { + PluginToolError::new("INVALID", format!("project root cannot be resolved: {err}")) + })?; + let requested = Path::new(requested); + let candidate = if requested.is_absolute() { + requested.to_path_buf() + } else { + root.join(requested) + }; + let canonical = candidate.canonicalize().map_err(|err| { + PluginToolError::new( + "INVALID", + format!("plugin directory cannot be resolved: {err}"), + ) + })?; + if !canonical.starts_with(&root) { + return Err(PluginToolError::new( + "INVALID", + "plugin directory must remain within the requester project root", + )); + } + if !canonical.is_dir() { + return Err(PluginToolError::new( + "INVALID", + "plugin source must be a directory", + )); + } + Ok(canonical) +} + #[cfg(test)] mod backend_core_build_tests { - use super::BackendCore; + use std::path::Path; + + use super::{resolve_confined_plugin_directory, BackendCore}; + use domain::{Project, ProjectId, ProjectPath, RemoteRef}; + use uuid::Uuid; + + fn project(root: &Path) -> Project { + Project::new( + ProjectId::from_uuid(Uuid::new_v4()), + "plugin-tool-test", + ProjectPath::new(root.to_string_lossy().into_owned()).unwrap(), + RemoteRef::local(), + 0, + ) + .unwrap() + } #[test] fn backend_core_builds_without_desktop_runtime() { @@ -204,6 +287,68 @@ mod backend_core_build_tests { let _ = std::fs::remove_dir_all(dir); } + + #[test] + fn plugin_directory_resolution_accepts_relative_directory_under_project_root() { + let root = std::env::temp_dir().join(format!("idea-plugin-root-{}", Uuid::new_v4())); + let plugin = root.join("plugins/example"); + std::fs::create_dir_all(&plugin).unwrap(); + + let resolved = resolve_confined_plugin_directory(&project(&root), "plugins/example") + .expect("relative directory should resolve"); + + assert_eq!(resolved, plugin.canonicalize().unwrap()); + let _ = std::fs::remove_dir_all(root); + } + + #[test] + fn plugin_directory_resolution_rejects_absolute_path_outside_project_root() { + let root = std::env::temp_dir().join(format!("idea-plugin-root-{}", Uuid::new_v4())); + let outside = std::env::temp_dir().join(format!("idea-plugin-outside-{}", Uuid::new_v4())); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + + let error = + resolve_confined_plugin_directory(&project(&root), outside.to_string_lossy().as_ref()) + .unwrap_err(); + + assert_eq!(error.code, "INVALID"); + assert!(error.message.contains("within the requester project root")); + let _ = std::fs::remove_dir_all(root); + let _ = std::fs::remove_dir_all(outside); + } + + #[test] + fn plugin_directory_resolution_rejects_file_sources() { + let root = std::env::temp_dir().join(format!("idea-plugin-root-{}", Uuid::new_v4())); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("plugin.zip"), b"not a directory").unwrap(); + + let error = resolve_confined_plugin_directory(&project(&root), "plugin.zip").unwrap_err(); + + assert_eq!(error.code, "INVALID"); + assert!(error.message.contains("must be a directory")); + let _ = std::fs::remove_dir_all(root); + } + + #[cfg(unix)] + #[test] + fn plugin_directory_resolution_rejects_symlink_escaping_project_root() { + use std::os::unix::fs::symlink; + + let root = std::env::temp_dir().join(format!("idea-plugin-root-{}", Uuid::new_v4())); + let outside = std::env::temp_dir().join(format!("idea-plugin-outside-{}", Uuid::new_v4())); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + symlink(&outside, root.join("escape")).unwrap(); + + let error = resolve_confined_plugin_directory(&project(&root), "escape").unwrap_err(); + + assert_eq!(error.code, "INVALID"); + assert!(error.message.contains("within the requester project root")); + let _ = std::fs::remove_dir_all(root); + let _ = std::fs::remove_dir_all(outside); + } } #[derive(Clone, Default)] @@ -2769,6 +2914,7 @@ impl BackendCore { Arc::clone(&plugin_mcp_supervisor_port), )); let plugin_tool_provider: Arc = Arc::new(AppPluginToolProvider { + install_from_directory: Arc::clone(&install_plugin_from_directory), reload: Arc::clone(&reload_plugin), }); let set_plugin_enabled = Arc::new(SetPluginEnabled::new( diff --git a/crates/infrastructure/src/orchestrator/mcp/plugins.rs b/crates/infrastructure/src/orchestrator/mcp/plugins.rs index 7f84e6a5..74c24ede 100644 --- a/crates/infrastructure/src/orchestrator/mcp/plugins.rs +++ b/crates/infrastructure/src/orchestrator/mcp/plugins.rs @@ -49,23 +49,41 @@ pub trait PluginToolProvider: Send + Sync { /// Returns true when `name` is a public plugin tool. #[must_use] pub fn is_plugin_tool(name: &str) -> bool { - name == "idea_plugin_reload" + matches!( + name, + "idea_plugin_install_from_directory" | "idea_plugin_reload" + ) } /// Public plugin tool definitions advertised by `tools/list`. #[must_use] pub fn catalogue() -> Vec { - vec![ToolDef { - name: "idea_plugin_reload", - description: "Hot-reload an installed directory plugin from its recorded source.", - input_schema: json!({ - "type": "object", - "properties": { - "pluginId": { "type": "string" }, - "expectedContentHash": { "type": "string" } - }, - "required": ["pluginId"], - "additionalProperties": false - }), - }] + vec![ + ToolDef { + name: "idea_plugin_install_from_directory", + description: "Install a plugin from a directory confined to the requester project root; the emitted event lets the UI refresh plugin contributions without restarting IdeA.", + input_schema: json!({ + "type": "object", + "properties": { + "path": { "type": "string" }, + "expectedPluginId": { "type": "string" } + }, + "required": ["path"], + "additionalProperties": false + }), + }, + ToolDef { + name: "idea_plugin_reload", + description: "Hot-reload an installed directory plugin from its recorded source; the emitted event lets the UI refresh plugin contributions.", + input_schema: json!({ + "type": "object", + "properties": { + "pluginId": { "type": "string" }, + "expectedContentHash": { "type": "string" } + }, + "required": ["pluginId"], + "additionalProperties": false + }), + }, + ] } diff --git a/crates/infrastructure/src/orchestrator/mcp/tools.rs b/crates/infrastructure/src/orchestrator/mcp/tools.rs index 455dc4a0..63484070 100644 --- a/crates/infrastructure/src/orchestrator/mcp/tools.rs +++ b/crates/infrastructure/src/orchestrator/mcp/tools.rs @@ -96,6 +96,7 @@ pub const WRITE_ACTION_TOOLS: &[&str] = &[ "idea_managed_temp_allocate", "idea_managed_temp_release", "idea_managed_temp_gc", + "idea_plugin_install_from_directory", "idea_plugin_reload", ]; diff --git a/crates/infrastructure/tests/mcp_server.rs b/crates/infrastructure/tests/mcp_server.rs index 6a9a29f2..aeb02b8a 100644 --- a/crates/infrastructure/tests/mcp_server.rs +++ b/crates/infrastructure/tests/mcp_server.rs @@ -801,6 +801,7 @@ async fn tools_list_advertises_the_idea_tools_with_schemas() { "idea_template_update", "idea_template_delete", // Public plugin administration tools. + "idea_plugin_install_from_directory", "idea_plugin_reload", ] { assert!( @@ -811,8 +812,8 @@ async fn tools_list_advertises_the_idea_tools_with_schemas() { assert!(!names.contains(&"idea_reply")); assert_eq!( tools.len(), - 48, - "exactly the forty-eight exposed idea_* tools; got {names:?}" + 49, + "exactly the forty-nine exposed idea_* tools; got {names:?}" ); // Every tool advertises an object input schema. @@ -925,6 +926,7 @@ async fn requester_with_durable_store_but_no_override_sees_read_only_tools() { assert!(!names.contains(&"idea_template_create")); assert!(!names.contains(&"idea_template_update")); assert!(!names.contains(&"idea_template_delete")); + assert!(!names.contains(&"idea_plugin_install_from_directory")); assert!(!names.contains(&"idea_plugin_reload")); } @@ -939,10 +941,12 @@ async fn general_agent_without_mcp_override_is_read_only_for_tools_call() { let (service, _mailbox, _sessions) = build_service_with_mailbox(contexts); let ticket_tools = Arc::new(FakeTicketTools::default()); let template_tools = Arc::new(FakeTemplateTools::default()); + let plugin_tools = Arc::new(FakePluginTools::default()); let agent = AgentId::from_uuid(Uuid::from_u128(83)); let server = server_with_mcp_permissions(service, ProjectMcpToolPermissions::default()) .with_ticket_tools(ticket_tools.clone()) .with_template_tools(template_tools.clone()) + .with_plugin_tools(plugin_tools.clone()) .for_requester(agent.to_string()); for (id, tool, arguments) in [ @@ -1003,6 +1007,16 @@ async fn general_agent_without_mcp_override_is_read_only_for_tools_call() { "idea_template_delete", json!({ "templateId": Uuid::new_v4() }), ), + ( + 118, + "idea_plugin_install_from_directory", + json!({ "path": "plugins/example" }), + ), + ( + 119, + "idea_plugin_reload", + json!({ "pluginId": "dev.acme.gitgraph" }), + ), ] { let response = server .handle_raw(&tools_call(id, tool, arguments)) @@ -1033,6 +1047,10 @@ async fn general_agent_without_mcp_override_is_read_only_for_tools_call() { "denied template mutations must not reach the provider" ); assert_eq!(template_tools.mutation_attempts(), 0); + assert!( + plugin_tools.calls.lock().unwrap().is_empty(), + "denied plugin writes must not reach the provider" + ); } #[tokio::test] @@ -1072,6 +1090,45 @@ async fn explicitly_allowed_plugin_reload_routes_to_provider_with_camel_case_arg assert_eq!(calls[0].2["expectedContentHash"], json!("abc123")); } +#[tokio::test] +async fn explicitly_allowed_plugin_install_routes_to_provider_with_camel_case_arguments() { + let (service, _s) = build_service(FakeContexts::new()); + let plugin_tools = Arc::new(FakePluginTools::default()); + let agent = AgentId::from_uuid(Uuid::from_u128(279)); + let server = server_with_mcp_permissions( + service, + allow_doc(agent, &["idea_plugin_install_from_directory"]), + ) + .with_plugin_tools(plugin_tools.clone()) + .for_requester(agent.to_string()); + + let response = server + .handle_raw(&tools_call( + 279, + "idea_plugin_install_from_directory", + json!({ + "path": "plugins/example", + "expectedPluginId": "dev.acme.gitgraph" + }), + )) + .await + .expect("reply owed"); + + assert!( + response.error.is_none(), + "transport error: {:?}", + response.error + ); + let result = response.result.expect("tool result"); + assert_eq!(result["isError"], json!(false), "got {result}"); + let calls = plugin_tools.calls.lock().unwrap(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].0, agent.to_string()); + assert_eq!(calls[0].1, "idea_plugin_install_from_directory"); + assert_eq!(calls[0].2["path"], json!("plugins/example")); + assert_eq!(calls[0].2["expectedPluginId"], json!("dev.acme.gitgraph")); +} + #[tokio::test] async fn legacy_mcp_requester_with_durable_store_is_read_only() { let (service, _mailbox, _sessions) = build_service_with_mailbox(FakeContexts::new()); diff --git a/crates/infrastructure/tests/plugin_install_load.rs b/crates/infrastructure/tests/plugin_install_load.rs index 2d79a158..d8659781 100644 --- a/crates/infrastructure/tests/plugin_install_load.rs +++ b/crates/infrastructure/tests/plugin_install_load.rs @@ -107,7 +107,8 @@ async fn installs_reference_fixture_and_loads_runtime_catalog() { assert_eq!(result.plugin.display_name, "Reference Minimal Plugin"); assert_eq!(result.review.contribution_summary.top_level_menus, 1); assert_eq!(result.review.contribution_summary.menu_items, 1); - assert!(result.restart_required); + assert!(!result.restart_required); + assert!(!result.plugin.restart_required); assert!(app_data .join("plugins/installed/dev.idea.fixtures.reference-minimal/idea-plugin.json") .is_file());