From 9dbdb4e2c08723e8fc9c18f64d7ccad3dadc03a7 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Sat, 27 Jun 2026 15:22:05 -0700 Subject: [PATCH] [plugins] Enforce marketplace source policy at runtime (#29691) ## Summary - project effective marketplace/plugin config through the enterprise source policy so blocked installed plugins become inactive - filter plugin list/read/discovery and CLI marketplace source/snapshot reporting using the same policy - enforce source admission for background marketplace cache refreshes - continue refreshing/upgrading independent marketplaces and plugins when one entry fails, returning per-entry errors - include policy-projected plugin state in cache and refresh keys so requirement changes invalidate stale results ## Stack This is PR 2 of 2 and is based on #29690. Review the admission model and source matcher in #29690 first; this PR contains only runtime enforcement. ## Test plan - `just test -p codex-core-plugins` (287 tests) - `just test -p codex-cli plugin_list_ignores_implicit_system_marketplace_roots_without_manifests` - `cargo check -p codex-cli -p codex-app-server --tests` --- codex-rs/cli/src/marketplace_cmd.rs | 2 +- codex-rs/cli/src/plugin_cmd.rs | 12 +- codex-rs/cli/tests/plugin_cli.rs | 4 +- .../src/installed_marketplaces.rs | 3 +- codex-rs/core-plugins/src/lib.rs | 1 + codex-rs/core-plugins/src/loader.rs | 213 ++++++++++----- codex-rs/core-plugins/src/loader_tests.rs | 2 +- codex-rs/core-plugins/src/manager.rs | 229 +++++++++++----- codex-rs/core-plugins/src/manager_tests.rs | 252 +++++++++++++++++- .../core-plugins/src/marketplace_policy.rs | 119 ++++++++- .../src/marketplace_policy_tests.rs | 191 +++++++++++++ codex-rs/core-plugins/src/store.rs | 13 +- 12 files changed, 897 insertions(+), 144 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index e4e2b6337..16aeb092e 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -341,7 +341,7 @@ fn configured_marketplace_sources_by_root( codex_home: &Path, plugins_input: &PluginsConfigInput, ) -> HashMap { - let marketplace_sources = configured_marketplace_sources(plugins_input); + let marketplace_sources = configured_marketplace_sources(plugins_input, codex_home); let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else { return HashMap::new(); }; diff --git a/codex-rs/cli/src/plugin_cmd.rs b/codex-rs/cli/src/plugin_cmd.rs index 6d1771997..056c256b3 100644 --- a/codex-rs/cli/src/plugin_cmd.rs +++ b/codex-rs/cli/src/plugin_cmd.rs @@ -10,6 +10,7 @@ use codex_core_plugins::PluginInstallOutcome; use codex_core_plugins::PluginInstallRequest; use codex_core_plugins::PluginsConfigInput; use codex_core_plugins::PluginsManager; +use codex_core_plugins::allowed_configured_marketplace_names; use codex_core_plugins::installed_marketplaces::marketplace_install_root; use codex_core_plugins::installed_marketplaces::resolve_configured_marketplace_root; use codex_core_plugins::marketplace::MarketplaceListError; @@ -228,7 +229,7 @@ pub async fn run_plugin_list( .is_none_or(|name| marketplace.name == *name) }) .collect::>(); - let marketplace_sources = configured_marketplace_sources(&plugins_input); + let marketplace_sources = configured_marketplace_sources(&plugins_input, codex_home.as_path()); if args.json { let output = JsonPluginListOutput::from_marketplaces( @@ -480,6 +481,7 @@ pub(crate) struct JsonMarketplaceSource { pub(crate) fn configured_marketplace_sources( plugins_input: &PluginsConfigInput, + codex_home: &Path, ) -> HashMap { let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else { return HashMap::new(); @@ -490,9 +492,12 @@ pub(crate) fn configured_marketplace_sources( else { return HashMap::new(); }; + let allowed_marketplace_names = + allowed_configured_marketplace_names(&plugins_input.config_layer_stack, codex_home); marketplaces .iter() + .filter(|(marketplace_name, _)| allowed_marketplace_names.contains(*marketplace_name)) .filter_map(|(marketplace_name, marketplace)| { let source_type = marketplace .get("source_type") @@ -746,11 +751,16 @@ pub(crate) fn configured_marketplace_snapshot_issues( else { return Vec::new(); }; + let allowed_marketplace_names = + allowed_configured_marketplace_names(&plugins_input.config_layer_stack, codex_home); let default_install_root = marketplace_install_root(codex_home); let mut manifest_paths = Vec::new(); let mut issues = Vec::new(); for (configured_name, marketplace) in configured_marketplaces { + if !allowed_marketplace_names.contains(configured_name) { + continue; + } if marketplace_name.is_some_and(|name| configured_name != name) { continue; } diff --git a/codex-rs/cli/tests/plugin_cli.rs b/codex-rs/cli/tests/plugin_cli.rs index c8bd7b542..f9d73c1ce 100644 --- a/codex-rs/cli/tests/plugin_cli.rs +++ b/codex-rs/cli/tests/plugin_cli.rs @@ -205,6 +205,7 @@ fn setup_local_marketplace_with_implicit_system_roots() -> Result<(TempDir, Temp let cache_home = TempDir::new()?; let runtime_root = cache_home .path() + .join(".cache") .join("codex-runtimes") .join("codex-primary-runtime") .join("plugins") @@ -844,7 +845,8 @@ async fn plugin_list_ignores_implicit_system_marketplace_roots_without_manifests let (codex_home, source, cache_home) = setup_local_marketplace_with_implicit_system_roots()?; codex_command(codex_home.path())? - .env("XDG_CACHE_HOME", cache_home.path()) + .env("HOME", cache_home.path()) + .env("USERPROFILE", cache_home.path()) .args(["plugin", "list"]) .assert() .success() diff --git a/codex-rs/core-plugins/src/installed_marketplaces.rs b/codex-rs/core-plugins/src/installed_marketplaces.rs index 0377fac9e..b5bf3b769 100644 --- a/codex-rs/core-plugins/src/installed_marketplaces.rs +++ b/codex-rs/core-plugins/src/installed_marketplaces.rs @@ -7,6 +7,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use tracing::warn; use crate::marketplace::find_marketplace_manifest_path; +use crate::marketplace_policy::project_effective_user_config; pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; @@ -18,7 +19,7 @@ pub fn installed_marketplace_roots_from_layer_stack( config_layer_stack: &ConfigLayerStack, codex_home: &Path, ) -> Vec { - let Some(user_config) = config_layer_stack.effective_user_config() else { + let Some(user_config) = project_effective_user_config(config_layer_stack, codex_home) else { return Vec::new(); }; let Some(marketplaces_value) = user_config.get("marketplaces") else { diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index d2047afa8..df30305c3 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -55,6 +55,7 @@ pub use manager::PluginUninstallError; pub use manager::PluginsConfigInput; pub use manager::PluginsManager; pub use manager::RecommendedPluginCandidatesInput; +pub use marketplace_policy::allowed_configured_marketplace_names; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome; pub use provider::ExecutorPluginProvider; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 2f406439a..2b5c4782e 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -8,8 +8,9 @@ use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; use crate::marketplace::MarketplacePluginSource; use crate::marketplace::find_marketplace_plugin; -use crate::marketplace::list_marketplaces; +use crate::marketplace::list_marketplaces_with_home; use crate::marketplace::load_marketplace; +use crate::marketplace_policy::configured_plugins_from_stack; use crate::npm_source::materialize_npm_plugin_source; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RemoteInstalledPlugin; @@ -84,6 +85,18 @@ enum NonCuratedCacheRefreshMode { ForceReinstall, } +#[derive(Debug)] +pub(crate) struct NonCuratedCacheRefreshOutcome { + pub(crate) cache_refreshed: bool, + pub(crate) errors: Vec, +} + +#[derive(Debug)] +pub(crate) struct NonCuratedCacheRefreshError { + pub(crate) marketplace_name: String, + pub(crate) message: String, +} + pub(crate) fn log_plugin_load_errors(plugins: &[LoadedPlugin]) { for plugin in plugins.iter().filter(|plugin| plugin.error.is_some()) { if let Some(error) = plugin.error.as_deref() { @@ -129,7 +142,7 @@ async fn load_plugins_from_layer_stack_with_scope( scope: PluginLoadScope<'_>, ) -> Vec> { let configured_plugins = merge_configured_plugins_with_remote_installed( - configured_plugins_from_stack(config_layer_stack), + configured_plugins_from_stack(config_layer_stack, store.codex_home().as_path()), extra_plugins, store, remote_global_catalog_active, @@ -410,24 +423,54 @@ pub fn curated_plugin_cache_version(plugin_version: &str) -> String { } } -pub fn refresh_non_curated_plugin_cache( +#[cfg(test)] +pub(crate) fn refresh_non_curated_plugin_cache( codex_home: &Path, additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], ) -> Result { + collapse_non_curated_cache_refresh(refresh_non_curated_plugin_cache_detailed( + codex_home, + additional_roots, + configured_plugin_keys, + )) +} + +pub(crate) fn refresh_non_curated_plugin_cache_detailed( + codex_home: &Path, + additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], +) -> Result { refresh_non_curated_plugin_cache_with_mode( codex_home, additional_roots, + configured_plugin_keys, NonCuratedCacheRefreshMode::IfVersionChanged, ) } -pub fn refresh_non_curated_plugin_cache_force_reinstall( +#[cfg(test)] +pub(crate) fn refresh_non_curated_plugin_cache_force_reinstall( codex_home: &Path, additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], ) -> Result { + collapse_non_curated_cache_refresh(refresh_non_curated_plugin_cache_force_reinstall_detailed( + codex_home, + additional_roots, + configured_plugin_keys, + )) +} + +pub(crate) fn refresh_non_curated_plugin_cache_force_reinstall_detailed( + codex_home: &Path, + additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], +) -> Result { refresh_non_curated_plugin_cache_with_mode( codex_home, additional_roots, + configured_plugin_keys, NonCuratedCacheRefreshMode::ForceReinstall, ) } @@ -435,16 +478,32 @@ pub fn refresh_non_curated_plugin_cache_force_reinstall( fn refresh_non_curated_plugin_cache_with_mode( codex_home: &Path, additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], mode: NonCuratedCacheRefreshMode, -) -> Result { - let configured_non_curated_plugin_ids = - non_curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( - codex_home, - "failed to read user config while refreshing non-curated plugin cache", - "failed to parse user config while refreshing non-curated plugin cache", - )); +) -> Result { + let mut configured_non_curated_plugin_ids = configured_plugin_keys + .iter() + .filter_map(|plugin_key| match PluginId::parse(plugin_key) { + Ok(plugin_id) if !is_openai_curated_marketplace_name(&plugin_id.marketplace_name) => { + Some(plugin_id) + } + Ok(_) => None, + Err(err) => { + warn!( + plugin_key, + error = %err, + "ignoring invalid plugin key during non-curated cache refresh setup" + ); + None + } + }) + .collect::>(); + configured_non_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); if configured_non_curated_plugin_ids.is_empty() { - return Ok(false); + return Ok(NonCuratedCacheRefreshOutcome { + cache_refreshed: false, + errors: Vec::new(), + }); } let configured_non_curated_plugin_keys = configured_non_curated_plugin_ids .iter() @@ -452,7 +511,7 @@ fn refresh_non_curated_plugin_cache_with_mode( .collect::>(); let store = PluginStore::try_new(codex_home.to_path_buf()).map_err(|err| err.to_string())?; - let marketplace_outcome = list_marketplaces(additional_roots) + let marketplace_outcome = list_marketplaces_with_home(additional_roots, /*home_dir*/ None) .map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?; let mut plugin_sources = HashMap::)>::new(); @@ -462,14 +521,18 @@ fn refresh_non_curated_plugin_cache_with_mode( } for plugin in marketplace.plugins { - let plugin_id = - PluginId::new(plugin.name.clone(), marketplace.name.clone()).map_err(|err| { - match err { - PluginIdError::Invalid(message) => { - format!("failed to prepare non-curated plugin cache refresh: {message}") - } - } - })?; + let plugin_id = match PluginId::new(plugin.name.clone(), marketplace.name.clone()) { + Ok(plugin_id) => plugin_id, + Err(PluginIdError::Invalid(message)) => { + warn!( + plugin = plugin.name, + marketplace = marketplace.name, + error = %message, + "ignoring invalid plugin entry during cache refresh" + ); + continue; + } + }; let plugin_key = plugin_id.as_key(); if !configured_non_curated_plugin_keys.contains(&plugin_key) { continue; @@ -504,6 +567,7 @@ fn refresh_non_curated_plugin_cache_with_mode( } let mut cache_refreshed = false; + let mut refresh_errors = Vec::new(); for plugin_id in configured_non_curated_plugin_ids { let plugin_key = plugin_id.as_key(); let Some((source, manifest_fallback_contents)) = plugin_sources.get(&plugin_key).cloned() @@ -515,49 +579,70 @@ fn refresh_non_curated_plugin_cache_with_mode( ); continue; }; - let materialized = - materialize_marketplace_plugin_source(codex_home, &source).map_err(|err| { - format!("failed to materialize plugin source for {plugin_key}: {err}") - })?; - let source_path = materialized.path.clone(); - let plugin_version = match manifest_fallback_contents.as_deref() { - Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest( - source_path.as_path(), - manifest_contents, - ), - None => plugin_version_for_source(source_path.as_path()), - } - .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; + let refresh_result = (|| -> Result { + let materialized = + materialize_marketplace_plugin_source(codex_home, &source).map_err(|err| { + format!("failed to materialize plugin source for {plugin_key}: {err}") + })?; + let source_path = materialized.path; + let plugin_version = match manifest_fallback_contents.as_deref() { + Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest( + source_path.as_path(), + manifest_contents, + ), + None => plugin_version_for_source(source_path.as_path()), + } + .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; - if mode == NonCuratedCacheRefreshMode::IfVersionChanged - && store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str()) - { - continue; - } + if mode == NonCuratedCacheRefreshMode::IfVersionChanged + && store.active_plugin_version(&plugin_id).as_deref() + == Some(plugin_version.as_str()) + { + return Ok(false); + } - match manifest_fallback_contents.as_deref() { - Some(manifest_contents) => store.install_with_version_and_fallback_manifest( - source_path, - plugin_id.clone(), - plugin_version, - manifest_contents, - ), - None => store.install_with_version(source_path, plugin_id.clone(), plugin_version), + match manifest_fallback_contents.as_deref() { + Some(manifest_contents) => store.install_with_version_and_fallback_manifest( + source_path, + plugin_id.clone(), + plugin_version, + manifest_contents, + ), + None => store.install_with_version(source_path, plugin_id.clone(), plugin_version), + } + .map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?; + Ok(true) + })(); + match refresh_result { + Ok(refreshed) => cache_refreshed |= refreshed, + Err(message) => refresh_errors.push(NonCuratedCacheRefreshError { + marketplace_name: plugin_id.marketplace_name, + message, + }), } - .map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?; - cache_refreshed = true; } - Ok(cache_refreshed) + Ok(NonCuratedCacheRefreshOutcome { + cache_refreshed, + errors: refresh_errors, + }) } -fn configured_plugins_from_stack( - config_layer_stack: &ConfigLayerStack, -) -> HashMap { - let Some(user_config) = config_layer_stack.effective_user_config() else { - return HashMap::new(); - }; - configured_plugins_from_user_config_value(&user_config) +#[cfg(test)] +fn collapse_non_curated_cache_refresh( + outcome: Result, +) -> Result { + let outcome = outcome?; + if outcome.errors.is_empty() { + Ok(outcome.cache_refreshed) + } else { + Err(outcome + .errors + .into_iter() + .map(|error| error.message) + .collect::>() + .join("; ")) + } } fn is_full_git_sha(value: &str) -> bool { @@ -647,20 +732,6 @@ fn curated_plugin_ids_from_config_keys( configured_curated_plugin_ids } -fn non_curated_plugin_ids_from_config_keys( - configured_plugins: HashMap, -) -> Vec { - let mut configured_non_curated_plugin_ids = configured_plugin_ids( - configured_plugins, - "ignoring invalid plugin key during non-curated cache refresh setup", - ) - .into_iter() - .filter(|plugin_id| !is_openai_curated_marketplace_name(&plugin_id.marketplace_name)) - .collect::>(); - configured_non_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); - configured_non_curated_plugin_ids -} - pub fn configured_curated_plugin_ids_from_codex_home(codex_home: &Path) -> Vec { curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( codex_home, diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index d4faaa8e7..8e5a76b4b 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -43,7 +43,7 @@ fn configured_plugins_from_stack_merges_user_layers() { ) .expect("valid config layer stack"); - let plugins = configured_plugins_from_stack(&stack); + let plugins = configured_plugins_from_stack(&stack, temp_dir.path()); assert_eq!( plugins, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 39e5152ab..4d215b61d 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -16,8 +16,8 @@ use crate::loader::log_plugin_load_errors; use crate::loader::materialize_marketplace_plugin_source; use crate::loader::plugin_capability_summary_from_root; use crate::loader::refresh_curated_plugin_cache; -use crate::loader::refresh_non_curated_plugin_cache; -use crate::loader::refresh_non_curated_plugin_cache_force_reinstall; +use crate::loader::refresh_non_curated_plugin_cache_detailed; +use crate::loader::refresh_non_curated_plugin_cache_force_reinstall_detailed; use crate::loader::remote_installed_plugins_to_config; use crate::manifest::PluginManifestInterface; use crate::manifest::load_plugin_manifest; @@ -32,9 +32,12 @@ use crate::marketplace::MarketplacePluginSource; use crate::marketplace::ResolvedMarketplacePlugin; use crate::marketplace::find_installable_marketplace_plugin; use crate::marketplace::find_marketplace_plugin; -use crate::marketplace::list_marketplaces; +use crate::marketplace::home_dir; +use crate::marketplace::list_marketplaces_with_home; use crate::marketplace::plugin_interface_with_marketplace_category; use crate::marketplace_policy::MarketplacePolicy; +use crate::marketplace_policy::allowed_configured_marketplace_names; +use crate::marketplace_policy::configured_plugins_from_stack; use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError; use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; use crate::marketplace_upgrade::upgrade_configured_git_marketplaces; @@ -84,6 +87,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginSkillRoot; use std::collections::HashMap; use std::collections::HashSet; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::RwLock; @@ -195,6 +199,7 @@ pub struct PluginListBackgroundTaskOptions { #[derive(Clone, PartialEq, Eq)] struct NonCuratedCacheRefreshRequest { roots: Vec, + configured_plugin_keys: Vec, mode: NonCuratedCacheRefreshMode, } @@ -388,9 +393,16 @@ struct PluginLoadCacheKey { } impl PluginLoadCacheKey { - fn from_config(config: &PluginsConfigInput, remote_global_catalog_active: bool) -> Self { + fn from_config( + config: &PluginsConfigInput, + codex_home: &Path, + remote_global_catalog_active: bool, + ) -> Self { Self { - configured_plugins: configured_plugins_from_stack(&config.config_layer_stack), + configured_plugins: configured_plugins_from_stack( + &config.config_layer_stack, + codex_home, + ), skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack), remote_global_catalog_active, } @@ -494,8 +506,11 @@ impl PluginsManager { if !config.plugins_enabled { return None; } - let key = - PluginLoadCacheKey::from_config(config, self.remote_global_catalog_active(config)); + let key = PluginLoadCacheKey::from_config( + config, + self.codex_home.as_path(), + self.remote_global_catalog_active(config), + ); self.loaded_plugins_cache .read() .unwrap_or_else(std::sync::PoisonError::into_inner) @@ -525,7 +540,11 @@ impl PluginsManager { } let remote_global_catalog_active = self.remote_global_catalog_active(config); - let cache_key = PluginLoadCacheKey::from_config(config, remote_global_catalog_active); + let cache_key = PluginLoadCacheKey::from_config( + config, + self.codex_home.as_path(), + remote_global_catalog_active, + ); if !force_reload && let Some(plugins) = self.cached_loaded_plugins(&cache_key) { return self.resolve_loaded_plugins_for_auth(plugins); } @@ -1007,7 +1026,7 @@ impl PluginsManager { options: PluginListBackgroundTaskOptions, on_effective_plugins_changed: Option>, ) { - self.maybe_start_non_curated_plugin_cache_refresh(roots); + self.maybe_start_non_curated_plugin_cache_refresh(config, roots); if options.refresh_global_remote_catalog_cache { self.maybe_start_global_remote_catalog_cache_refresh(config, auth.clone()); } @@ -1547,7 +1566,7 @@ impl PluginsManager { let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); let marketplace_roots = self.marketplace_roots(config, additional_roots, include_openai_curated); - let marketplace_outcome = list_marketplaces(&marketplace_roots)?; + let marketplace_outcome = self.list_marketplaces_with_policy(config, &marketplace_roots)?; let mut seen_plugin_keys = HashSet::new(); let marketplaces = marketplace_outcome .marketplaces @@ -1636,11 +1655,12 @@ impl PluginsManager { return Ok(MarketplaceListOutcome::default()); } - list_marketplaces(&self.marketplace_roots( + let marketplace_roots = self.marketplace_roots( config, additional_roots, /*include_openai_curated*/ true, - )) + ); + self.list_marketplaces_with_policy(config, &marketplace_roots) } pub(crate) async fn tool_suggest_metadata_for_marketplace_plugin( @@ -1666,6 +1686,17 @@ impl PluginsManager { } let plugin = find_marketplace_plugin(&request.marketplace_path, &request.plugin_name)?; + MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()) + .validate_install( + &config.config_layer_stack, + self.codex_home.as_path(), + &request.marketplace_path, + &plugin.plugin_id.marketplace_name, + ) + .map_err(|message| MarketplaceError::InvalidMarketplaceFile { + path: request.marketplace_path.to_path_buf(), + message, + })?; if !self.restriction_product_matches(plugin.policy.products.as_deref()) { return Err(MarketplaceError::PluginNotFound { plugin_name: plugin.plugin_id.plugin_name, @@ -2023,12 +2054,30 @@ impl PluginsManager { )); } if !outcome.upgraded_roots.is_empty() { - match refresh_non_curated_plugin_cache_force_reinstall( + let mut configured_plugin_keys = configured_plugins_from_stack( + &config.config_layer_stack, + self.codex_home.as_path(), + ) + .into_keys() + .collect::>(); + configured_plugin_keys.sort_unstable(); + match refresh_non_curated_plugin_cache_force_reinstall_detailed( self.codex_home.as_path(), &outcome.upgraded_roots, + &configured_plugin_keys, ) { - Ok(cache_refreshed) => { - self.clear_caches_after_marketplace_source_refresh(cache_refreshed); + Ok(refresh_outcome) => { + self.clear_caches_after_marketplace_source_refresh( + refresh_outcome.cache_refreshed, + ); + outcome + .errors + .extend(refresh_outcome.errors.into_iter().map(|error| { + ConfiguredMarketplaceUpgradeError { + marketplace_name: error.marketplace_name, + message: error.message, + } + })); } Err(err) => { self.clear_cache(); @@ -2048,9 +2097,11 @@ impl PluginsManager { pub fn maybe_start_non_curated_plugin_cache_refresh( self: &Arc, + config: &PluginsConfigInput, roots: &[AbsolutePathBuf], ) { self.schedule_non_curated_plugin_cache_refresh( + config, roots, NonCuratedCacheRefreshMode::IfVersionChanged, ); @@ -2127,38 +2178,81 @@ impl PluginsManager { fn schedule_non_curated_plugin_cache_refresh( self: &Arc, + config: &PluginsConfigInput, roots: &[AbsolutePathBuf], mode: NonCuratedCacheRefreshMode, ) { - let mut roots = roots.to_vec(); + let marketplace_roots = + self.marketplace_roots(config, roots, /*include_openai_curated*/ false); + let outcome = match self.list_marketplaces_with_policy(config, &marketplace_roots) { + Ok(outcome) => outcome, + Err(err) => { + warn!("failed to prepare non-curated plugin cache refresh: {err}"); + return; + } + }; + let policy = MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()); + let mut roots = outcome + .marketplaces + .into_iter() + .filter(|marketplace| !is_openai_curated_marketplace_name(&marketplace.name)) + .filter_map(|marketplace| { + match policy.validate_install( + &config.config_layer_stack, + self.codex_home.as_path(), + &marketplace.path, + &marketplace.name, + ) { + Ok(()) => Some(marketplace.path), + Err(err) => { + warn!( + marketplace = marketplace.name, + path = %marketplace.path.display(), + error = %err, + "skipping marketplace source during plugin cache refresh" + ); + None + } + } + }) + .collect::>(); roots.sort_unstable(); roots.dedup(); - if roots.is_empty() { + let mut configured_plugin_keys = + configured_plugins_from_stack(&config.config_layer_stack, self.codex_home.as_path()) + .into_keys() + .collect::>(); + configured_plugin_keys.sort_unstable(); + if roots.is_empty() || configured_plugin_keys.is_empty() { return; } - let request = NonCuratedCacheRefreshRequest { roots, mode }; + let mut request = NonCuratedCacheRefreshRequest { + roots, + configured_plugin_keys, + mode, + }; let should_spawn = { let mut state = match self.non_curated_cache_refresh_state.write() { Ok(state) => state, Err(err) => err.into_inner(), }; - // Collapse repeated plugin/list requests onto one worker and only queue another pass - // when the requested roots set actually changes. Forced reinstall requests are not - // deduped against the last completed pass because the same marketplace root path can - // point at newly activated files after an auto-upgrade. - if state.requested.as_ref() == Some(&request) - || (mode == NonCuratedCacheRefreshMode::IfVersionChanged - && !state.in_flight - && state.last_refreshed.as_ref() == Some(&request)) - { - return; - } - if mode == NonCuratedCacheRefreshMode::IfVersionChanged + if request.mode == NonCuratedCacheRefreshMode::IfVersionChanged && state.requested.as_ref().is_some_and(|requested| { requested.mode == NonCuratedCacheRefreshMode::ForceReinstall && requested.roots == request.roots }) + { + request.mode = NonCuratedCacheRefreshMode::ForceReinstall; + } + // Collapse repeated plugin/list requests onto one worker and only queue another pass + // when the roots or configured plugin set changes. Forced reinstall requests are not + // deduped against the last completed pass because the same marketplace root path can + // point at newly activated files after an auto-upgrade. + if state.requested.as_ref() == Some(&request) + || (request.mode == NonCuratedCacheRefreshMode::IfVersionChanged + && !state.in_flight + && state.last_refreshed.as_ref() == Some(&request)) { return; } @@ -2349,21 +2443,33 @@ impl PluginsManager { let refresh_result = match request.mode { NonCuratedCacheRefreshMode::IfVersionChanged => { - refresh_non_curated_plugin_cache(self.codex_home.as_path(), &request.roots) - } - NonCuratedCacheRefreshMode::ForceReinstall => { - refresh_non_curated_plugin_cache_force_reinstall( + refresh_non_curated_plugin_cache_detailed( self.codex_home.as_path(), &request.roots, + &request.configured_plugin_keys, + ) + } + NonCuratedCacheRefreshMode::ForceReinstall => { + refresh_non_curated_plugin_cache_force_reinstall_detailed( + self.codex_home.as_path(), + &request.roots, + &request.configured_plugin_keys, ) } }; let refreshed = match refresh_result { - Ok(cache_refreshed) => { - if cache_refreshed { + Ok(refresh_outcome) => { + if refresh_outcome.cache_refreshed { self.clear_cache(); } - true + for error in &refresh_outcome.errors { + warn!( + marketplace = error.marketplace_name, + error = %error.message, + "failed to refresh configured plugin cache" + ); + } + refresh_outcome.errors.is_empty() } Err(err) => { self.clear_cache(); @@ -2391,7 +2497,8 @@ impl PluginsManager { &self, config: &PluginsConfigInput, ) -> (HashSet, HashSet) { - let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); + let configured_plugins = + configured_plugins_from_stack(&config.config_layer_stack, self.codex_home.as_path()); let installed_plugins = configured_plugins .keys() .filter(|plugin_key| { @@ -2448,6 +2555,27 @@ impl PluginsManager { roots.dedup(); roots } + + fn list_marketplaces_with_policy( + &self, + config: &PluginsConfigInput, + roots: &[AbsolutePathBuf], + ) -> Result { + let mut outcome = list_marketplaces_with_home(roots, home_dir().as_deref())?; + let policy = MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()); + if !policy.is_restricted() { + return Ok(outcome); + } + let allowed_marketplace_names = allowed_configured_marketplace_names( + &config.config_layer_stack, + self.codex_home.as_path(), + ); + outcome.marketplaces.retain(|marketplace| { + is_openai_curated_marketplace_name(&marketplace.name) + || allowed_marketplace_names.contains(&marketplace.name) + }); + Ok(outcome) + } } pub(crate) fn remote_plugin_install_required_description( @@ -2612,31 +2740,6 @@ impl PluginUninstallError { } } -pub(crate) fn configured_plugins_from_stack( - config_layer_stack: &ConfigLayerStack, -) -> HashMap { - // Plugin entries remain persisted user config only. - let Some(user_config) = config_layer_stack.effective_user_config() else { - return HashMap::new(); - }; - configured_plugins_from_user_config_value(&user_config) -} - -fn configured_plugins_from_user_config_value( - user_config: &toml::Value, -) -> HashMap { - let Some(plugins_value) = user_config.get("plugins") else { - return HashMap::new(); - }; - match plugins_value.clone().try_into() { - Ok(plugins) => plugins, - Err(err) => { - warn!("invalid plugins config: {err}"); - HashMap::new() - } - } -} - #[cfg(test)] #[path = "manager_tests.rs"] mod tests; diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index cefca287a..06ebf89a7 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -100,6 +100,19 @@ fn config_layer_stack_with_requirements( .expect("build config layer stack") } +fn plugins_config_input_with_requirements( + codex_home: &Path, + user_config: &str, + requirements: &str, +) -> PluginsConfigInput { + PluginsConfigInput::new( + config_layer_stack_with_requirements(codex_home, user_config, requirements), + /*plugins_enabled*/ true, + /*remote_plugin_enabled*/ false, + String::new(), + ) +} + #[test] fn plugins_manager_tracks_auth_mode() { let tmp = TempDir::new().unwrap(); @@ -122,6 +135,184 @@ fn plugins_manager_tracks_auth_mode() { assert_eq!(manager_with_auth.auth_mode(), Some(AuthMode::Chatgpt)); } +#[tokio::test] +async fn marketplace_policy_projection_disables_installed_plugin_and_invalidates_cache() { + let codex_home = TempDir::new().expect("create Codex home"); + write_plugin( + &codex_home.path().join("plugins/cache/company"), + "sample/local", + "sample", + ); + let user_config = r#" +[marketplaces.company] +source_type = "git" +source = "https://github.com/example/company.git" + +[plugins."sample@company"] +enabled = true +"#; + let allowed = plugins_config_input_with_requirements( + codex_home.path(), + user_config, + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.company] +source = "git" +url = "https://github.com/example/company.git" +"#, + ); + let blocked = plugins_config_input_with_requirements( + codex_home.path(), + user_config, + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.other] +source = "git" +url = "https://github.com/example/other.git" +"#, + ); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + + let allowed_outcome = manager.plugins_for_config(&allowed).await; + assert_eq!(allowed_outcome.plugins().len(), 1); + assert_eq!(allowed_outcome.plugins()[0].config_name, "sample@company"); + + let blocked_outcome = manager.plugins_for_config(&blocked).await; + assert_eq!(blocked_outcome, PluginLoadOutcome::default()); +} + +#[tokio::test] +async fn plugin_read_rejects_marketplace_blocked_by_requirements() { + let codex_home = TempDir::new().expect("create Codex home"); + let marketplace_root = codex_home.path().join("marketplace"); + write_plugin(&marketplace_root, "sample", "sample"); + write_file( + &marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "company", + "plugins": [ + { + "name": "sample", + "source": {"source": "local", "path": "./sample"} + } + ] +}"#, + ); + let config = plugins_config_input_with_requirements( + codex_home.path(), + "", + r#" +[marketplaces] +restrict_to_allowed_sources = true +"#, + ); + let marketplace_path = + AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json")) + .expect("absolute marketplace path"); + + let err = PluginsManager::new(codex_home.path().to_path_buf()) + .read_plugin_for_config( + &config, + &PluginReadRequest { + plugin_name: "sample".to_string(), + marketplace_path, + }, + ) + .await + .expect_err("blocked marketplace should not be readable"); + assert!(matches!( + err, + MarketplaceError::InvalidMarketplaceFile { .. } + )); +} + +#[test] +fn marketplace_policy_filters_discovered_marketplaces_by_configured_name() { + let codex_home = TempDir::new().expect("create Codex home"); + let repo_root = codex_home.path().join("repo"); + let subdirectory = repo_root.join("worktree/subdirectory"); + fs::create_dir_all(&subdirectory).expect("create input subdirectory"); + write_plugin(&repo_root, "sample", "sample"); + write_file( + &repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "company", + "plugins": [ + { + "name": "sample", + "source": {"source": "local", "path": "./sample"} + } + ] +}"#, + ); + init_git_repo(&repo_root); + let repo_root = AbsolutePathBuf::try_from(repo_root).expect("absolute repository root"); + let subdirectory = + AbsolutePathBuf::try_from(subdirectory).expect("absolute input subdirectory"); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let user_config = format!( + r#" +[marketplaces.company] +source_type = "local" +source = {:?} +"#, + repo_root.as_path() + ); + let allowed = plugins_config_input_with_requirements( + codex_home.path(), + &user_config, + &format!( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.company] +source = "local" +path = {:?} +"#, + repo_root.as_path() + ), + ); + let blocked = plugins_config_input_with_requirements( + codex_home.path(), + &user_config, + &format!( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.subdirectory] +source = "local" +path = {:?} +"#, + subdirectory.as_path() + ), + ); + + let allowed_outcome = manager + .list_marketplaces_for_config( + &allowed, + std::slice::from_ref(&subdirectory), + /*include_openai_curated*/ false, + ) + .expect("list allowed marketplace"); + assert_eq!(allowed_outcome.marketplaces.len(), 1); + assert_eq!(allowed_outcome.marketplaces[0].name, "company"); + + let blocked_outcome = manager + .list_marketplaces_for_config( + &blocked, + std::slice::from_ref(&subdirectory), + /*include_openai_curated*/ false, + ) + .expect("list blocked marketplace"); + assert_eq!(blocked_outcome.marketplaces, Vec::new()); +} + fn write_auth_projection_plugin(codex_home: &Path, name: &str, include_app: bool) { let plugin_root = codex_home .join("plugins/cache") @@ -3348,7 +3539,11 @@ plugins = true .unwrap() .marketplaces .into_iter() - .find(|marketplace| marketplace.name == "debug") + .find(|marketplace| { + marketplace.path + == AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")) + .unwrap() + }) .unwrap() .plugins .into_iter() @@ -3785,7 +3980,7 @@ enabled = true let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, - &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()], /*include_openai_curated*/ true, ) .unwrap() @@ -3793,7 +3988,11 @@ enabled = true let marketplace = marketplaces .into_iter() - .find(|marketplace| marketplace.name == "debug") + .find(|marketplace| { + marketplace.path + == AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")) + .unwrap() + }) .expect("debug marketplace should be listed"); let mut plugins = marketplace.plugins; @@ -5256,6 +5455,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should succeed") ); @@ -5308,6 +5508,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should reinstall missing configured plugin") ); @@ -5367,6 +5568,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should materialize configured Git plugin") ); @@ -5420,6 +5622,7 @@ enabled = true !refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should be a no-op when configured plugins are current") ); @@ -5473,6 +5676,7 @@ enabled = true refresh_non_curated_plugin_cache_force_reinstall( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should reinstall unchanged local version") ); @@ -5531,6 +5735,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should ignore unrelated invalid plugin manifests") ); @@ -5542,6 +5747,47 @@ enabled = true ); } +#[test] +fn refresh_non_curated_plugin_cache_continues_after_plugin_error() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + write_plugin_with_version(&repo_root, "z-good", "z-good", Some("1.2.3")); + write_file( + &repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "a-broken", + "source": { + "source": "local", + "path": "./missing" + } + }, + { + "name": "z-good", + "source": { + "source": "local", + "path": "./z-good" + } + } + ] +}"#, + ); + + let err = refresh_non_curated_plugin_cache( + tmp.path(), + &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["a-broken@debug".to_string(), "z-good@debug".to_string()], + ) + .expect_err("broken plugin should be reported after refreshing the remaining plugins"); + + assert!(err.contains("a-broken@debug")); + assert!(tmp.path().join("plugins/cache/debug/z-good/1.2.3").is_dir()); +} + #[tokio::test] async fn load_plugins_ignores_project_config_files() { let codex_home = TempDir::new().unwrap(); diff --git a/codex-rs/core-plugins/src/marketplace_policy.rs b/codex-rs/core-plugins/src/marketplace_policy.rs index 8e41fb893..a4c99a81c 100644 --- a/codex-rs/core-plugins/src/marketplace_policy.rs +++ b/codex-rs/core-plugins/src/marketplace_policy.rs @@ -18,9 +18,13 @@ use codex_config::MarketplaceAllowedSourceToml; use codex_config::RequirementSource; use codex_config::types::MarketplaceConfig; use codex_config::types::MarketplaceSourceType; +use codex_config::types::PluginConfig; +use codex_plugin::PluginId; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path::paths_match_after_normalization; use regex::Regex; +use std::collections::HashMap; +use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; use url::Url; @@ -70,7 +74,7 @@ impl MarketplacePolicy { } } - fn is_restricted(&self) -> bool { + pub(crate) fn is_restricted(&self) -> bool { self.restricted.is_some() } @@ -198,6 +202,104 @@ impl AllowedMarketplaceSource { } } +pub(crate) fn project_effective_user_config( + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> Option { + let mut user_config = config_layer_stack.effective_user_config()?; + let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements()); + if !policy.is_restricted() { + return Some(user_config); + } + let allowed_marketplace_names = + allowed_configured_marketplace_names_with_policy(&user_config, &policy, codex_home); + let configured_marketplace_names = user_config + .get("marketplaces") + .and_then(toml::Value::as_table) + .map(|marketplaces| marketplaces.keys().cloned().collect::>()) + .unwrap_or_default(); + + if let Some(marketplaces) = user_config + .get_mut("marketplaces") + .and_then(toml::Value::as_table_mut) + { + marketplaces + .retain(|marketplace_name, _| allowed_marketplace_names.contains(marketplace_name)); + } + if let Some(plugins) = user_config + .get_mut("plugins") + .and_then(toml::Value::as_table_mut) + { + plugins.retain(|plugin_key, _| { + let Ok(plugin_id) = PluginId::parse(plugin_key) else { + return false; + }; + (is_openai_curated_marketplace_name(&plugin_id.marketplace_name) + && !configured_marketplace_names.contains(&plugin_id.marketplace_name)) + || allowed_marketplace_names.contains(&plugin_id.marketplace_name) + }); + } + Some(user_config) +} + +pub fn allowed_configured_marketplace_names( + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> HashSet { + let Some(user_config) = config_layer_stack.effective_user_config() else { + return HashSet::new(); + }; + let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements()); + allowed_configured_marketplace_names_with_policy(&user_config, &policy, codex_home) +} + +fn allowed_configured_marketplace_names_with_policy( + user_config: &toml::Value, + policy: &MarketplacePolicy, + codex_home: &Path, +) -> HashSet { + let Some(marketplaces) = user_config + .get("marketplaces") + .and_then(toml::Value::as_table) + else { + return HashSet::new(); + }; + if !policy.is_restricted() { + return marketplaces.keys().cloned().collect(); + } + marketplaces + .iter() + .filter_map(|(marketplace_name, marketplace)| { + let allowed = match managed_marketplace_config_name(codex_home, marketplace) { + Some(expected_name) => expected_name == marketplace_name, + None => policy + .validate_configured_marketplace(marketplace_name, marketplace) + .is_ok(), + }; + allowed.then(|| marketplace_name.clone()) + }) + .collect() +} + +pub(crate) fn configured_plugins_from_stack( + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> HashMap { + let Some(user_config) = project_effective_user_config(config_layer_stack, codex_home) else { + return HashMap::new(); + }; + let Some(plugins_value) = user_config.get("plugins") else { + return HashMap::new(); + }; + match plugins_value.clone().try_into() { + Ok(plugins) => plugins, + Err(err) => { + tracing::warn!("invalid plugins config: {err}"); + HashMap::new() + } + } +} + pub(crate) fn validate_marketplace_source_for_add( codex_home: &Path, requirements: &ConfigRequirements, @@ -358,6 +460,21 @@ fn managed_marketplace_name( managed_local_marketplace_name(codex_home, root.as_path()) } +fn managed_marketplace_config_name( + codex_home: &Path, + marketplace: &toml::Value, +) -> Option<&'static str> { + if marketplace.get("source_type").and_then(toml::Value::as_str) != Some("local") { + return None; + } + let path = marketplace + .get("source") + .and_then(toml::Value::as_str) + .map(Path::new) + .filter(|path| path.is_absolute())?; + managed_local_marketplace_name(codex_home, path) +} + fn managed_local_marketplace_name(codex_home: &Path, root: &Path) -> Option<&'static str> { for marketplace_name in [ OPENAI_BUNDLED_MARKETPLACE_NAME, diff --git a/codex-rs/core-plugins/src/marketplace_policy_tests.rs b/codex-rs/core-plugins/src/marketplace_policy_tests.rs index 63518dfb3..75c0a2f17 100644 --- a/codex-rs/core-plugins/src/marketplace_policy_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_policy_tests.rs @@ -383,6 +383,197 @@ restrict_to_allowed_sources = true assert!(validate_marketplace_name_for_add(expected_name, "other").is_err()); } +#[test] +fn projected_user_config_removes_blocked_marketplaces_and_plugins() { + let codex_home = TempDir::new().expect("create Codex home"); + let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml")) + .expect("absolute config path"); + let stack = config_layer_stack_with_user_config( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.company] +source = "git" +url = "https://github.com/example/allowed.git" +"#, + Some(( + r#" +[marketplaces.allowed] +source_type = "git" +source = "https://github.com/example/allowed.git" + +[marketplaces.blocked] +source_type = "git" +source = "https://github.com/example/blocked.git" + +[plugins."sample@allowed"] +enabled = true + +[plugins."sample@blocked"] +enabled = true +"#, + config_file, + )), + ); + + let projected = + project_effective_user_config(&stack, codex_home.path()).expect("project user config"); + assert_eq!( + projected["marketplaces"] + .as_table() + .expect("projected marketplaces") + .keys() + .cloned() + .collect::>(), + vec!["allowed".to_string()] + ); + assert_eq!( + configured_plugins_from_stack(&stack, codex_home.path()) + .into_keys() + .collect::>(), + vec!["sample@allowed".to_string()] + ); + + let raw = stack.effective_user_config().expect("raw user config"); + assert!(raw["marketplaces"]["blocked"].is_table()); + assert!(raw["plugins"]["sample@blocked"].is_table()); +} + +#[test] +fn managed_bundled_config_is_retained_only_at_its_owned_path() { + let codex_home = TempDir::new().expect("create Codex home"); + let bundled_root = codex_home + .path() + .join(".tmp/bundled-marketplaces") + .join(crate::OPENAI_BUNDLED_MARKETPLACE_NAME); + let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml")) + .expect("absolute config path"); + let stack = config_layer_stack_with_user_config( + r#" +[marketplaces] +restrict_to_allowed_sources = true +"#, + Some(( + &format!( + r#" +[marketplaces.openai-bundled] +source_type = "local" +source = {bundled_root:?} + +[marketplaces.openai-bundled-alpha] +source_type = "local" +source = "/tmp/not-managed" + +[marketplaces.evil] +source_type = "local" +source = {bundled_root:?} + +[plugins."sample@openai-bundled"] +enabled = true + +[plugins."sample@openai-bundled-alpha"] +enabled = true + +[plugins."sample@evil"] +enabled = true +"# + ), + config_file, + )), + ); + + let projected = + project_effective_user_config(&stack, codex_home.path()).expect("project user config"); + + assert_eq!( + projected["marketplaces"] + .as_table() + .expect("projected marketplaces") + .keys() + .cloned() + .collect::>(), + vec![crate::OPENAI_BUNDLED_MARKETPLACE_NAME.to_string()] + ); + assert_eq!( + projected["plugins"] + .as_table() + .expect("projected plugins") + .keys() + .cloned() + .collect::>(), + vec![format!("sample@{}", crate::OPENAI_BUNDLED_MARKETPLACE_NAME)] + ); +} + +#[test] +fn allowlisted_config_names_are_not_globally_reserved() { + let codex_home = TempDir::new().expect("create Codex home"); + let source_root = TempDir::new().expect("create marketplace root"); + let source_root = source_root + .path() + .canonicalize() + .expect("canonical marketplace root"); + let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml")) + .expect("absolute config path"); + let stack = config_layer_stack_with_user_config( + &format!( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.local] +source = "local" +path = {source_root:?} +"# + ), + Some(( + &format!( + r#" +[marketplaces.openai-bundled] +source_type = "local" +source = {source_root:?} + +[marketplaces.openai-curated] +source_type = "local" +source = {source_root:?} + +[plugins."sample@openai-bundled"] +enabled = true + +[plugins."sample@openai-curated"] +enabled = true +"# + ), + config_file, + )), + ); + + let projected = + project_effective_user_config(&stack, codex_home.path()).expect("project user config"); + assert_eq!( + projected["marketplaces"] + .as_table() + .expect("projected marketplaces") + .keys() + .cloned() + .collect::>(), + vec!["openai-bundled".to_string(), "openai-curated".to_string()] + ); + assert_eq!( + projected["plugins"] + .as_table() + .expect("projected plugins") + .keys() + .cloned() + .collect::>(), + vec![ + "sample@openai-bundled".to_string(), + "sample@openai-curated".to_string() + ] + ); +} + #[test] fn blocked_upgrade_is_rejected_before_marketplace_installation() { let codex_home = TempDir::new().expect("create Codex home"); diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 5d78fefa9..bb4929ded 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -37,6 +37,7 @@ pub struct PluginInstallResult { #[derive(Debug, Clone)] pub struct PluginStore { + codex_home: AbsolutePathBuf, root: AbsolutePathBuf, data_root: AbsolutePathBuf, } @@ -59,14 +60,24 @@ impl PluginStore { let data_root = AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_DATA_DIR)) .map_err(|err| PluginStoreError::io("failed to resolve plugin data root", err))?; + let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home) + .map_err(|err| PluginStoreError::io("failed to resolve Codex home", err))?; - Ok(Self { root, data_root }) + Ok(Self { + codex_home, + root, + data_root, + }) } pub fn root(&self) -> &AbsolutePathBuf { &self.root } + pub(crate) fn codex_home(&self) -> &AbsolutePathBuf { + &self.codex_home + } + pub fn plugin_base_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf { self.root .join(&plugin_id.marketplace_name)