diff --git a/codex-rs/external-agent-migration/src/detect/mod.rs b/codex-rs/external-agent-migration/src/detect/mod.rs index 1490c68209..15f172c6f7 100644 --- a/codex-rs/external-agent-migration/src/detect/mod.rs +++ b/codex-rs/external-agent-migration/src/detect/mod.rs @@ -327,7 +327,9 @@ impl ExternalAgentConfigService { ); } - if self.source.supports_plugin_migration(settings.as_ref()) { + // Plugin import persists user-global enabled state, so repository-controlled + // settings must never be treated as plugin installation authority. + if scope.is_home() && self.source.supports_plugin_migration(settings.as_ref()) { match ConfigBuilder::default() .codex_home(self.codex_home.clone()) .fallback_cwd(Some(self.codex_home.clone())) diff --git a/codex-rs/external-agent-migration/src/plugins.rs b/codex-rs/external-agent-migration/src/plugins.rs index 463bfcfa82..26932c4278 100644 --- a/codex-rs/external-agent-migration/src/plugins.rs +++ b/codex-rs/external-agent-migration/src/plugins.rs @@ -27,16 +27,18 @@ impl ExternalAgentConfigService { &self, cwd: Option<&Path>, ) -> io::Result> { - let Some(scope) = MigrationScope::from_cwd(cwd)? else { - return Ok(BTreeMap::new()); - }; - let source_root = scope - .repo_root() - .unwrap_or(self.external_agent_home.as_path()); + // Plugin installs are user-global. Only missing or empty `cwd` values + // denote home scope; a project scope must not select executable content. + if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { + return Err(invalid_data_error( + "repository-scoped plugin migration is not allowed".to_string(), + )); + } + let scope = MigrationScope::home(); let source_settings = self.source_settings(&scope); self.source.marketplace_import_sources( self.external_agent_home.as_path(), - source_root, + self.external_agent_home.as_path(), &source_settings, ) } @@ -86,11 +88,13 @@ impl ExternalAgentConfigService { cwd: Option<&Path>, details: Option, ) -> io::Result { + let cwd = cwd.filter(|cwd| !cwd.as_os_str().is_empty()); let Some(MigrationDetails { plugins, .. }) = details else { return Err(invalid_data_error( "plugins migration item is missing details".to_string(), )); }; + let import_sources = self.marketplace_import_sources(cwd)?; let config = ConfigBuilder::default() .codex_home(self.codex_home.clone()) .fallback_cwd(Some( @@ -120,7 +124,6 @@ impl ExternalAgentConfigService { .into_iter() .map(|marketplace| (marketplace.name, marketplace.path)) .collect::>(); - let import_sources = self.marketplace_import_sources(cwd)?; for plugin_group in plugins { let marketplace_name = plugin_group.marketplace_name.clone(); let plugin_names = plugin_group.plugin_names; diff --git a/codex-rs/external-agent-migration/src/service_tests/plugins/basics.rs b/codex-rs/external-agent-migration/src/service_tests/plugins/basics.rs index 4bb109bf3f..3f8295898b 100644 --- a/codex-rs/external-agent-migration/src/service_tests/plugins/basics.rs +++ b/codex-rs/external-agent-migration/src/service_tests/plugins/basics.rs @@ -394,7 +394,7 @@ async fn detect_home_plugins_uses_local_settings_over_project_settings() { } #[tokio::test] -async fn detect_repo_skips_plugins_that_are_already_configured_in_codex() { +async fn detect_repo_skips_plugins_from_remote_marketplace() { let root = TempDir::new().expect("create tempdir"); let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); @@ -435,27 +435,7 @@ enabled = true .await .expect("detect"); - assert_eq!( - items, - vec![ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - description: format!( - "Migrate enabled plugins from {}", - repo_root - .join(EXTERNAL_AGENT_DIR) - .join("settings.json") - .display() - ), - cwd: Some(repo_root), - details: Some(MigrationDetails { - plugins: vec![PluginsMigration { - marketplace_name: "acme-tools".to_string(), - plugin_names: vec!["deployer".to_string()], - }], - ..Default::default() - }), - }] - ); + assert_eq!(items, Vec::::new()); } #[tokio::test] @@ -559,7 +539,7 @@ async fn import_plugins_requires_details() { } #[tokio::test] -async fn detect_repo_does_not_skip_plugins_only_configured_in_project_codex() { +async fn detect_repo_skips_plugins_only_configured_in_project_codex() { let root = TempDir::new().expect("create tempdir"); let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); @@ -600,27 +580,7 @@ enabled = true .await .expect("detect"); - assert_eq!( - items, - vec![ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - description: format!( - "Migrate enabled plugins from {}", - repo_root - .join(EXTERNAL_AGENT_DIR) - .join("settings.json") - .display() - ), - cwd: Some(repo_root), - details: Some(MigrationDetails { - plugins: vec![PluginsMigration { - marketplace_name: "acme-tools".to_string(), - plugin_names: vec!["formatter".to_string()], - }], - ..Default::default() - }), - }] - ); + assert_eq!(items, Vec::::new()); } #[tokio::test] @@ -681,7 +641,7 @@ async fn detect_home_skips_plugins_with_invalid_marketplace_source() { } #[tokio::test] -async fn detect_repo_filters_plugins_against_installed_marketplace() { +async fn detect_repo_skips_plugins_even_with_installed_marketplace() { let root = TempDir::new().expect("create tempdir"); let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); @@ -787,25 +747,5 @@ source = "owner/debug-marketplace" .await .expect("detect"); - assert_eq!( - items, - vec![ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - description: format!( - "Migrate enabled plugins from {}", - repo_root - .join(EXTERNAL_AGENT_DIR) - .join("settings.json") - .display() - ), - cwd: Some(repo_root), - details: Some(MigrationDetails { - plugins: vec![PluginsMigration { - marketplace_name: "debug".to_string(), - plugin_names: vec!["available".to_string()], - }], - ..Default::default() - }), - }] - ); + assert_eq!(items, Vec::::new()); } diff --git a/codex-rs/external-agent-migration/src/service_tests/plugins/marketplaces.rs b/codex-rs/external-agent-migration/src/service_tests/plugins/marketplaces.rs index a8e8699ce5..682000381a 100644 --- a/codex-rs/external-agent-migration/src/service_tests/plugins/marketplaces.rs +++ b/codex-rs/external-agent-migration/src/service_tests/plugins/marketplaces.rs @@ -91,7 +91,7 @@ async fn import_plugins_defers_marketplace_source_validation_to_add_marketplace( } #[tokio::test] -async fn import_plugins_supports_external_agent_plugin_marketplace_layout() { +async fn import_plugins_treats_empty_cwd_as_home_scope() { let (_root, external_agent_home, codex_home) = fixture_paths(); let marketplace_root = external_agent_home.join("my-marketplace"); let plugin_root = marketplace_root.join("plugins").join("cloudflare"); @@ -139,7 +139,7 @@ async fn import_plugins_supports_external_agent_plugin_marketplace_layout() { let outcome = service_for_paths(external_agent_home, codex_home.clone()) .import_plugins( - /*cwd*/ None, + /*cwd*/ Some(std::path::Path::new("")), Some(MigrationDetails { plugins: vec![PluginsMigration { marketplace_name: "my-plugins".to_string(), @@ -529,7 +529,7 @@ async fn import_plugins_infers_external_official_marketplace_when_missing_from_s } #[tokio::test] -async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace_path() { +async fn detect_repo_skips_project_relative_external_agent_plugin_marketplace_path() { let root = TempDir::new().expect("create tempdir"); let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); @@ -588,31 +588,11 @@ async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace .await .expect("detect"); - assert_eq!( - items, - vec![ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - description: format!( - "Migrate enabled plugins from {}", - repo_root - .join(EXTERNAL_AGENT_DIR) - .join("settings.json") - .display() - ), - cwd: Some(repo_root), - details: Some(MigrationDetails { - plugins: vec![PluginsMigration { - marketplace_name: "my-plugins".to_string(), - plugin_names: vec!["cloudflare".to_string()], - }], - ..Default::default() - }), - }] - ); + assert_eq!(items, Vec::::new()); } #[tokio::test] -async fn import_plugins_supports_project_relative_external_agent_plugin_marketplace_path() { +async fn import_rejects_forged_project_relative_external_agent_plugin_item() { let root = TempDir::new().expect("create tempdir"); let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); @@ -663,32 +643,66 @@ async fn import_plugins_supports_project_relative_external_agent_plugin_marketpl .expect("write plugin manifest"); let outcome = service_for_paths(external_agent_home, codex_home.clone()) - .import_plugins( - Some(repo_root.as_path()), - Some(MigrationDetails { + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: String::new(), + cwd: Some(repo_root.clone()), + details: Some(MigrationDetails { plugins: vec![PluginsMigration { marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], ..Default::default() }), - ) - .await - .expect("import plugins"); + }]) + .await; assert_eq!( outcome, - PluginImportOutcome { - succeeded_marketplaces: vec!["my-plugins".to_string()], - succeeded_plugin_ids: vec!["cloudflare@my-plugins".to_string()], - failed_marketplaces: Vec::new(), - failed_plugin_ids: Vec::new(), - raw_errors: Vec::new(), + ExternalAgentConfigImportOutcome { + pending_plugin_imports: Vec::new(), + item_results: vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: String::new(), + cwd: Some(repo_root.clone()), + success_count: 0, + error_count: 1, + successes: Vec::new(), + raw_errors: vec![ExternalAgentConfigImportRawError { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + error_type: None, + sub_error_type: None, + failure_stage: "plugin_import".to_string(), + message: "repository-scoped plugin migration is not allowed".to_string(), + cwd: Some(repo_root), + source: None, + }], + }], } ); - let config = fs::read_to_string(codex_home.join("config.toml")).expect("read config"); - assert!(config.contains(r#"[plugins."cloudflare@my-plugins"]"#)); - assert!(config.contains("enabled = true")); + assert!(!codex_home.join("config.toml").exists()); +} + +#[tokio::test] +async fn import_plugins_rejects_project_cwd_before_config_loading() { + let root = TempDir::new().expect("create tempdir"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); + let codex_home = root.path().join(".codex"); + let repo_root = root.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(&codex_home).expect("create codex home"); + fs::write(codex_home.join("config.toml"), "not valid toml").expect("write invalid config"); + + let error = service_for_paths(external_agent_home, codex_home) + .import_plugins(Some(repo_root.as_path()), Some(github_plugin_details())) + .await + .expect_err("reject project cwd before config loading"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert_eq!( + error.to_string(), + "repository-scoped plugin migration is not allowed" + ); } #[test]