From 641f65a858f14e9d72ffbb529648e6bb74d210c5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 6 May 2026 18:38:54 -0700 Subject: [PATCH 1/2] feat: enforce managed plugin allowlists Co-authored-by: Codex noreply@openai.com --- codex-rs/core-plugins/src/loader.rs | 6 +- codex-rs/core-plugins/src/manager.rs | 168 ++++++++++++++- codex-rs/core-plugins/src/manager_tests.rs | 225 +++++++++++++++++++++ codex-rs/core-plugins/src/marketplace.rs | 3 + 4 files changed, 387 insertions(+), 15 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index f348a3414df1..81703aaeb57f 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,4 +1,5 @@ use crate::OPENAI_CURATED_MARKETPLACE_NAME; +use crate::manager::configured_plugins_for_stack; use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; @@ -378,10 +379,7 @@ fn refresh_non_curated_plugin_cache_with_mode( fn configured_plugins_from_stack( config_layer_stack: &ConfigLayerStack, ) -> HashMap { - let Some(user_layer) = config_layer_stack.get_user_layer() else { - return HashMap::new(); - }; - configured_plugins_from_user_config_value(&user_layer.config) + configured_plugins_for_stack(config_layer_stack) } fn is_full_git_sha(value: &str) -> bool { diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 2c974ef9662c..c19752cdfe02 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -48,6 +48,7 @@ use crate::store::PluginStoreError; use codex_analytics::AnalyticsEventsClient; use codex_config::ConfigLayerStack; use codex_config::PluginConfigEdit; +use codex_config::PluginMarketplaceRequirementsToml; use codex_config::apply_user_plugin_config_edits; use codex_config::clear_user_plugin; use codex_config::set_user_plugin_enabled; @@ -401,6 +402,7 @@ pub struct PluginsManager { struct CachedPluginLoadOutcome { config_version: String, plugin_hooks_enabled: bool, + plugin_marketplace_requirements: Option, outcome: PluginLoadOutcome, } @@ -473,9 +475,13 @@ impl PluginsManager { let plugin_hooks_enabled = config.plugin_hooks_enabled; let config_version = version_for_toml(&config.config_layer_stack.effective_config()); + let plugin_marketplace_requirements = plugin_marketplace_requirements(config); if !force_reload - && let Some(outcome) = - self.cached_enabled_outcome(&config_version, plugin_hooks_enabled) + && let Some(outcome) = self.cached_enabled_outcome( + &config_version, + plugin_hooks_enabled, + plugin_marketplace_requirements.as_ref(), + ) { return outcome; } @@ -496,6 +502,7 @@ impl PluginsManager { *cache = Some(CachedPluginLoadOutcome { config_version, plugin_hooks_enabled, + plugin_marketplace_requirements, outcome: outcome.clone(), }); outcome @@ -553,6 +560,7 @@ impl PluginsManager { &self, config_version: &str, plugin_hooks_enabled: bool, + plugin_marketplace_requirements: Option<&PluginMarketplaceRequirementsToml>, ) -> Option { match self.cached_enabled_outcome.read() { Ok(cache) => cache @@ -560,6 +568,8 @@ impl PluginsManager { .filter(|cached| { cached.config_version == config_version && cached.plugin_hooks_enabled == plugin_hooks_enabled + && cached.plugin_marketplace_requirements.as_ref() + == plugin_marketplace_requirements }) .map(|cached| cached.outcome.clone()), Err(err) => err @@ -568,6 +578,8 @@ impl PluginsManager { .filter(|cached| { cached.config_version == config_version && cached.plugin_hooks_enabled == plugin_hooks_enabled + && cached.plugin_marketplace_requirements.as_ref() + == plugin_marketplace_requirements }) .map(|cached| cached.outcome.clone()), } @@ -589,7 +601,16 @@ impl PluginsManager { return HashMap::new(); }; - remote_installed_plugins_to_config(plugins, &self.store) + if plugin_marketplace_requirements(config).is_none() { + return remote_installed_plugins_to_config(plugins, &self.store); + } + + let plugins = plugins + .iter() + .filter(|plugin| marketplace_is_allowed(config, &plugin.marketplace_name)) + .cloned() + .collect::>(); + remote_installed_plugins_to_config(&plugins, &self.store) } fn write_remote_installed_plugins_cache(&self, plugins: Vec) -> bool { @@ -801,6 +822,25 @@ impl PluginsManager { self.install_resolved_plugin(resolved).await } + pub async fn install_plugin_for_config( + &self, + config: &PluginsConfigInput, + request: PluginInstallRequest, + ) -> Result { + let resolved = find_installable_marketplace_plugin( + &request.marketplace_path, + &request.plugin_name, + self.restriction_product, + )?; + if !marketplace_is_allowed(config, &resolved.plugin_id.marketplace_name) { + return Err(MarketplaceError::MarketplaceBlocked { + marketplace_name: resolved.plugin_id.marketplace_name, + } + .into()); + } + self.install_resolved_plugin(resolved).await + } + pub async fn install_plugin_with_remote_sync( &self, config: &PluginsConfigInput, @@ -952,6 +992,9 @@ impl PluginsManager { if !config.plugins_enabled { return Ok(RemotePluginSyncResult::default()); } + if !marketplace_is_allowed(config, OPENAI_CURATED_MARKETPLACE_NAME) { + return Ok(RemotePluginSyncResult::default()); + } info!("starting remote plugin sync"); let remote_plugins = crate::remote_legacy::fetch_remote_plugin_status( @@ -960,7 +1003,7 @@ impl PluginsManager { ) .await .map_err(PluginRemoteSyncError::from)?; - let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); + let configured_plugins = configured_plugins_for_config(config); let curated_marketplace_root = curated_plugins_repo_path(self.codex_home.as_path()); let curated_marketplace_path = AbsolutePathBuf::try_from( curated_marketplace_root.join(".agents/plugins/marketplace.json"), @@ -1174,6 +1217,9 @@ impl PluginsManager { .marketplaces .into_iter() .filter_map(|marketplace| { + if !marketplace_is_allowed(config, &marketplace.name) { + return None; + } let marketplace_name = marketplace.name.clone(); let plugins = marketplace .plugins @@ -1228,6 +1274,11 @@ impl PluginsManager { } let plugin = find_marketplace_plugin(&request.marketplace_path, &request.plugin_name)?; + if !marketplace_is_allowed(config, &plugin.plugin_id.marketplace_name) { + return Err(MarketplaceError::MarketplaceBlocked { + marketplace_name: plugin.plugin_id.marketplace_name, + }); + } if !self.restriction_product_matches(plugin.policy.products.as_deref()) { return Err(MarketplaceError::PluginNotFound { plugin_name: plugin.plugin_id.plugin_name, @@ -1491,8 +1542,10 @@ impl PluginsManager { config: &PluginsConfigInput, marketplace_name: Option<&str>, ) -> Result { + let configured_marketplace_names = + configured_git_marketplace_names(&config.config_layer_stack); if let Some(marketplace_name) = marketplace_name - && !configured_git_marketplace_names(&config.config_layer_stack) + && !configured_marketplace_names .iter() .any(|name| name == marketplace_name) { @@ -1501,11 +1554,49 @@ impl PluginsManager { )); } - let mut outcome = upgrade_configured_git_marketplaces( - self.codex_home.as_path(), - &config.config_layer_stack, - marketplace_name, - ); + let mut outcome = if config + .config_layer_stack + .requirements() + .plugin_marketplaces + .is_none() + { + upgrade_configured_git_marketplaces( + self.codex_home.as_path(), + &config.config_layer_stack, + marketplace_name, + ) + } else { + let selected_marketplace_names = match marketplace_name { + Some(marketplace_name) => { + if !marketplace_is_allowed(config, marketplace_name) { + return Err(format!( + "marketplace `{marketplace_name}` is not allowed by managed requirements" + )); + } + vec![marketplace_name.to_string()] + } + None => configured_marketplace_names + .into_iter() + .filter(|marketplace_name| marketplace_is_allowed(config, marketplace_name)) + .collect::>(), + }; + let mut outcome = ConfiguredMarketplaceUpgradeOutcome::default(); + for marketplace_name in selected_marketplace_names { + let mut marketplace_outcome = upgrade_configured_git_marketplaces( + self.codex_home.as_path(), + &config.config_layer_stack, + Some(&marketplace_name), + ); + outcome + .selected_marketplaces + .append(&mut marketplace_outcome.selected_marketplaces); + outcome + .upgraded_roots + .append(&mut marketplace_outcome.upgraded_roots); + outcome.errors.append(&mut marketplace_outcome.errors); + } + outcome + }; if !outcome.upgraded_roots.is_empty() { match refresh_non_curated_plugin_cache_force_reinstall( self.codex_home.as_path(), @@ -1813,7 +1904,7 @@ impl PluginsManager { &self, config: &PluginsConfigInput, ) -> (HashSet, HashSet) { - let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); + let configured_plugins = configured_plugins_for_config(config); let installed_plugins = configured_plugins .keys() .filter(|plugin_key| { @@ -1913,6 +2004,7 @@ impl PluginInstallError { | MarketplaceError::InvalidMarketplaceFile { .. } | MarketplaceError::PluginNotFound { .. } | MarketplaceError::PluginNotAvailable { .. } + | MarketplaceError::MarketplaceBlocked { .. } | MarketplaceError::InvalidPlugin(_) ) | Self::Store(PluginStoreError::Invalid(_)) ) @@ -1957,6 +2049,60 @@ pub(crate) fn configured_plugins_from_stack( configured_plugins_from_user_config_value(&user_layer.config) } +pub(crate) fn configured_plugins_for_stack( + config_layer_stack: &ConfigLayerStack, +) -> HashMap { + let configured_plugins = configured_plugins_from_stack(config_layer_stack); + if config_layer_stack + .requirements() + .plugin_marketplaces + .is_none() + { + return configured_plugins; + } + + configured_plugins + .into_iter() + .filter(|(plugin_key, _)| plugin_key_is_allowed(config_layer_stack, plugin_key)) + .collect() +} + +fn marketplace_is_allowed(config: &PluginsConfigInput, marketplace_name: &str) -> bool { + marketplace_is_allowed_in_stack(&config.config_layer_stack, marketplace_name) +} + +fn plugin_marketplace_requirements( + config: &PluginsConfigInput, +) -> Option { + config + .config_layer_stack + .requirements() + .plugin_marketplaces + .as_ref() + .map(|requirements| requirements.value.clone()) +} + +pub(crate) fn marketplace_is_allowed_in_stack( + config_layer_stack: &ConfigLayerStack, + marketplace_name: &str, +) -> bool { + config_layer_stack + .requirements() + .plugin_marketplaces + .as_ref() + .is_none_or(|requirements| requirements.value.allows_marketplace(marketplace_name)) +} + +fn plugin_key_is_allowed(config_layer_stack: &ConfigLayerStack, plugin_key: &str) -> bool { + PluginId::parse(plugin_key).ok().is_some_and(|plugin_id| { + marketplace_is_allowed_in_stack(config_layer_stack, &plugin_id.marketplace_name) + }) +} + +fn configured_plugins_for_config(config: &PluginsConfigInput) -> HashMap { + configured_plugins_for_stack(&config.config_layer_stack) +} + fn configured_plugins_from_user_config_value( user_config: &toml::Value, ) -> HashMap { diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 50d0d4625838..938cb8eb4d6c 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -23,6 +23,9 @@ use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::McpServerConfig; use codex_config::McpServerToolConfig; +use codex_config::PluginMarketplaceRequirementsToml; +use codex_config::RequirementSource; +use codex_config::Sourced; use codex_config::types::McpServerTransportConfig; use codex_login::CodexAuth; use codex_protocol::protocol::Product; @@ -141,6 +144,32 @@ async fn load_plugins_from_config(config_toml: &str, codex_home: &Path) -> Plugi .await } +fn plugins_config_with_requirements( + codex_home: &Path, + user_config: Value, + requirements: ConfigRequirements, +) -> PluginsConfigInput { + let config_path = + codex_utils_absolute_path::AbsolutePathBuf::try_from(codex_home.join(CONFIG_TOML_FILE)) + .expect("config path should be absolute"); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + user_config, + )], + requirements, + ConfigRequirementsToml::default(), + ) + .expect("valid config layer stack"); + PluginsConfigInput::new( + config_layer_stack, + /*plugins_enabled*/ true, + /*remote_plugin_enabled*/ false, + /*plugin_hooks_enabled*/ false, + "https://chatgpt.com/backend-api/".to_string(), + ) +} + async fn load_config(codex_home: &Path, cwd: &Path) -> PluginsConfigInput { load_plugins_config_input(codex_home, cwd).await } @@ -1576,6 +1605,143 @@ enabled = false ); } +#[tokio::test] +async fn managed_marketplace_allowlist_hides_and_disables_unapproved_plugins() { + 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( + &tmp.path().join("plugins/cache/debug"), + "sample/local", + "sample", + ); + fs::write( + repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample", + "source": { + "source": "local", + "path": "./sample" + } + } + ] +}"#, + ) + .unwrap(); + let config = plugins_config_with_requirements( + tmp.path(), + toml::toml! { + [plugins."sample@debug"] + enabled = true + } + .into(), + ConfigRequirements { + plugin_marketplaces: Some(Sourced::new( + PluginMarketplaceRequirementsToml { + allowed_names: Some(vec!["openai-curated".to_string()]), + allow_user_additions: Some(false), + }, + RequirementSource::Unknown, + )), + ..Default::default() + }, + ); + let manager = PluginsManager::new(tmp.path().to_path_buf()); + + let outcome = manager.plugins_for_config(&config).await; + assert_eq!(outcome.plugins(), &[]); + + let marketplaces = manager + .list_marketplaces_for_config(&config, &[AbsolutePathBuf::try_from(repo_root).unwrap()]) + .unwrap() + .marketplaces; + assert_eq!(marketplaces, Vec::new()); + + let err = manager + .read_plugin_for_config( + &config, + &PluginReadRequest { + plugin_name: "sample".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + tmp.path().join("repo/.agents/plugins/marketplace.json"), + ) + .unwrap(), + }, + ) + .await + .expect_err("disallowed marketplace should not be readable"); + assert!(matches!( + err, + MarketplaceError::MarketplaceBlocked { marketplace_name } + if marketplace_name == "debug" + )); + + let err = manager + .install_plugin_for_config( + &config, + PluginInstallRequest { + plugin_name: "sample".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + tmp.path().join("repo/.agents/plugins/marketplace.json"), + ) + .unwrap(), + }, + ) + .await + .expect_err("disallowed marketplace should not be installable"); + assert!(matches!( + err, + PluginInstallError::Marketplace(MarketplaceError::MarketplaceBlocked { + marketplace_name + }) if marketplace_name == "debug" + )); +} + +#[tokio::test] +async fn plugins_for_config_separates_cache_entries_by_managed_marketplace_allowlist() { + let tmp = tempfile::tempdir().unwrap(); + write_plugin( + &tmp.path().join("plugins/cache/debug"), + "sample/local", + "sample", + ); + let user_config: Value = toml::toml! { + [plugins."sample@debug"] + enabled = true + } + .into(); + let unrestricted = plugins_config_with_requirements( + tmp.path(), + user_config.clone(), + ConfigRequirements::default(), + ); + let restricted = plugins_config_with_requirements( + tmp.path(), + user_config, + ConfigRequirements { + plugin_marketplaces: Some(Sourced::new( + PluginMarketplaceRequirementsToml { + allowed_names: Some(vec!["openai-curated".to_string()]), + allow_user_additions: Some(false), + }, + RequirementSource::Unknown, + )), + ..Default::default() + }, + ); + let manager = PluginsManager::new(tmp.path().to_path_buf()); + + let unrestricted = manager.plugins_for_config(&unrestricted).await; + assert_eq!(unrestricted.plugins().len(), 1); + + let restricted = manager.plugins_for_config(&restricted).await; + assert_eq!(restricted.plugins(), &[]); +} + #[tokio::test] async fn list_marketplaces_returns_empty_when_feature_disabled() { let tmp = tempfile::tempdir().unwrap(); @@ -2827,6 +2993,65 @@ enabled = false assert!(config.contains("enabled = false")); } +#[tokio::test] +async fn sync_plugins_from_remote_skips_disallowed_curated_marketplace() { + let tmp = tempfile::tempdir().unwrap(); + let curated_root = curated_plugins_repo_path(tmp.path()); + write_openai_curated_marketplace(&curated_root, &["gmail"]); + write_curated_plugin_sha(tmp.path(), TEST_CURATED_PLUGIN_SHA); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + + let mut config = load_config(tmp.path(), tmp.path()).await; + config.config_layer_stack = ConfigLayerStack::new( + config + .config_layer_stack + .get_layers( + codex_config::ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .cloned() + .collect(), + ConfigRequirements { + plugin_marketplaces: Some(Sourced::new( + PluginMarketplaceRequirementsToml { + allowed_names: Some(vec!["arm-internal".to_string()]), + allow_user_additions: Some(false), + }, + RequirementSource::Unknown, + )), + ..Default::default() + }, + ConfigRequirementsToml::default(), + ) + .unwrap(); + let manager = PluginsManager::new(tmp.path().to_path_buf()); + let result = manager + .sync_plugins_from_remote( + &config, + Some(&CodexAuth::create_dummy_chatgpt_auth_for_testing()), + /*additive_only*/ false, + ) + .await + .unwrap(); + + assert_eq!(result, RemotePluginSyncResult::default()); + assert!( + !tmp.path() + .join("plugins/cache/openai-curated/gmail") + .exists() + ); + assert_eq!( + fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(), + "[features]\nplugins = true\n" + ); +} + #[tokio::test] async fn sync_plugins_from_remote_uses_first_duplicate_local_plugin_entry() { let tmp = tempfile::tempdir().unwrap(); diff --git a/codex-rs/core-plugins/src/marketplace.rs b/codex-rs/core-plugins/src/marketplace.rs index f66b5d1b227c..16b47df652d1 100644 --- a/codex-rs/core-plugins/src/marketplace.rs +++ b/codex-rs/core-plugins/src/marketplace.rs @@ -158,6 +158,9 @@ pub enum MarketplaceError { #[error("plugins feature is disabled")] PluginsDisabled, + #[error("marketplace `{marketplace_name}` is not allowed by managed requirements")] + MarketplaceBlocked { marketplace_name: String }, + #[error("{0}")] InvalidPlugin(String), } From 106296cb8db19e61d6364e2f13b07c2bffb69bca Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 6 May 2026 19:57:38 -0700 Subject: [PATCH 2/2] fix: keep plugin allowlist PR independently buildable Co-authored-by: Codex noreply@openai.com --- codex-rs/app-server/src/request_processors/plugins.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index bb1ebb611155..c32829079fb7 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1275,6 +1275,7 @@ impl PluginRequestProcessor { | MarketplaceError::PluginNotFound { .. } | MarketplaceError::PluginNotAvailable { .. } | MarketplaceError::PluginsDisabled + | MarketplaceError::MarketplaceBlocked { .. } | MarketplaceError::InvalidPlugin(_) => invalid_request(err.to_string()), MarketplaceError::Io { .. } => internal_error(format!("failed to {action}: {err}")), }