From 2696e7199bd964aed3811e386235703d4966db27 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Tue, 23 Jun 2026 19:42:13 -0700 Subject: [PATCH] [plugins] Add marketplace source requirements (#29690) ## Why Managed deployments need a mergeable way to declare which marketplace sources Codex may use. An enterprise-keyed TOML table avoids array merge ambiguity and lets every requirements layer use the existing config precedence rules without a marketplace-specific merger. ## Requirements shape ```toml [marketplaces] restrict_to_allowed_sources = true [marketplaces.allowed_sources.company_plugins] source = "git" url = "https://github.com/example/company-plugins.git" ref = "main" [marketplaces.allowed_sources.internal_git] source = "host_pattern" host_pattern = "^git\\.example\\.com$" [marketplaces.allowed_sources.local_plugins] source = "local" path = "/opt/company/codex-plugins" ``` `restrict_to_allowed_sources` follows normal scalar precedence. `allowed_sources` follows normal recursive TOML table merge behavior: distinct keys accumulate and fields under the same key use normal layer precedence. The final `source` value later selects which fields the marketplace admission policy interprets. The raw rule fields remain optional while requirements layers are composed, so a higher-priority layer can override only `ref`, `url`, or another individual field. Source-specific validation and normalization intentionally belong to the marketplace admission layer, not requirements merging. This initial shape includes `git`, `host_pattern`, and `local` sources. It does not add npm or path-pattern rules. ## What changed - Add the marketplace requirements TOML shape to `ConfigRequirementsToml`, `ConfigRequirementsWithSources`, and `ConfigRequirements`. - Carry marketplace requirements through the existing regular requirements merge path. - Keep allowed-source entries as raw partial tables for downstream policy interpretation. - Cover partial same-key overlays, source changes, unknown fields, and unmodified local paths. This PR defines and composes the requirements only. Source admission is implemented by the next PR in the stack. ## Stack This is PR 1 of 3. #29753 adds source admission on top of this PR; draft #29691 will add runtime enforcement after it is rebased later. ## Test plan - `just test -p codex-config marketplace_` --- codex-rs/config/src/config_requirements.rs | 56 +++++++ codex-rs/config/src/lib.rs | 3 + .../config/src/requirements_layers/stack.rs | 2 + .../src/requirements_layers/stack_tests.rs | 152 ++++++++++++++++++ codex-rs/core/src/config/config_tests.rs | 1 + codex-rs/core/src/config/mod.rs | 1 + codex-rs/tui/src/debug_config.rs | 2 + 7 files changed, 217 insertions(+) diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 4ccf4523a..e757c2149 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -11,6 +11,7 @@ use serde::de::value::Error as ValueDeserializerError; use serde::de::value::StrDeserializer; use std::collections::BTreeMap; use std::fmt; +use std::path::PathBuf; use wildmatch::WildMatchPattern; use super::requirements_exec_policy::RequirementsExecPolicy; @@ -155,6 +156,7 @@ pub struct ConfigRequirements { pub managed_hooks: Option>, pub mcp_servers: Option>>, pub plugins: Option>>, + pub marketplaces: Option>, pub exec_policy: Option>, pub enforce_residency: ConstrainedWithSource>, /// Managed network constraints derived from requirements. @@ -196,6 +198,7 @@ impl Default for ConfigRequirements { managed_hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, exec_policy: None, enforce_residency: ConstrainedWithSource::new( Constrained::allow_any(/*initial_value*/ None), @@ -231,6 +234,41 @@ pub struct PluginRequirementsToml { pub mcp_servers: Option>, } +#[derive(Deserialize, Debug, Clone, Default, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct MarketplaceRequirementsToml { + pub restrict_to_allowed_sources: Option, + #[serde(default)] + pub allowed_sources: BTreeMap, +} + +impl MarketplaceRequirementsToml { + pub fn is_empty(&self) -> bool { + self.restrict_to_allowed_sources.is_none() && self.allowed_sources.is_empty() + } +} + +/// Raw marketplace source rule whose active fields are interpreted after +/// requirements composition. +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct MarketplaceAllowedSourceToml { + pub source: Option, + pub url: Option, + #[serde(rename = "ref")] + pub ref_name: Option, + pub host_pattern: Option, + pub path: Option, +} + +#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum MarketplaceAllowedSourceKind { + Git, + HostPattern, + Local, +} + impl PluginRequirementsToml { pub fn is_empty(&self) -> bool { self.mcp_servers.as_ref().is_none_or(BTreeMap::is_empty) @@ -842,6 +880,7 @@ pub struct ConfigRequirementsToml { pub hooks: Option, pub mcp_servers: Option>, pub plugins: Option>, + pub marketplaces: Option, pub apps: Option, pub rules: Option, pub enforce_residency: Option, @@ -896,6 +935,7 @@ pub struct ConfigRequirementsWithSources { pub hooks: Option>, pub mcp_servers: Option>>, pub plugins: Option>>, + pub marketplaces: Option>, pub apps: Option>, pub rules: Option>, pub enforce_residency: Option>, @@ -939,6 +979,7 @@ impl ConfigRequirementsWithSources { hooks: _, mcp_servers: _, plugins: _, + marketplaces: _, apps: _, rules: _, enforce_residency: _, @@ -975,6 +1016,7 @@ impl ConfigRequirementsWithSources { hooks, mcp_servers, plugins, + marketplaces, rules, enforce_residency, network, @@ -1009,6 +1051,7 @@ impl ConfigRequirementsWithSources { hooks, mcp_servers, plugins, + marketplaces, apps, rules, enforce_residency, @@ -1033,6 +1076,7 @@ impl ConfigRequirementsWithSources { hooks: hooks.map(|sourced| sourced.value), mcp_servers: mcp_servers.map(|sourced| sourced.value), plugins: plugins.map(|sourced| sourced.value), + marketplaces: marketplaces.map(|sourced| sourced.value), apps: apps.map(|sourced| sourced.value), rules: rules.map(|sourced| sourced.value), enforce_residency: enforce_residency.map(|sourced| sourced.value), @@ -1138,6 +1182,10 @@ impl ConfigRequirementsToml { .plugins .as_ref() .is_none_or(|plugins| plugins.values().all(PluginRequirementsToml::is_empty)) + && self + .marketplaces + .as_ref() + .is_none_or(MarketplaceRequirementsToml::is_empty) && self .apps .as_ref() @@ -1176,6 +1224,7 @@ impl TryFrom for ConfigRequirements { hooks, mcp_servers, plugins, + marketplaces, apps: _apps, rules, enforce_residency, @@ -1456,6 +1505,7 @@ impl TryFrom for ConfigRequirements { managed_hooks, mcp_servers, plugins, + marketplaces, exec_policy, enforce_residency, network, @@ -1549,6 +1599,7 @@ mod tests { hooks, mcp_servers, plugins, + marketplaces, apps, rules, enforce_residency, @@ -1582,6 +1633,7 @@ mod tests { hooks: hooks.map(|value| Sourced::new(value, RequirementSource::Unknown)), mcp_servers: mcp_servers.map(|value| Sourced::new(value, RequirementSource::Unknown)), plugins: plugins.map(|value| Sourced::new(value, RequirementSource::Unknown)), + marketplaces: marketplaces.map(|value| Sourced::new(value, RequirementSource::Unknown)), apps: apps.map(|value| Sourced::new(value, RequirementSource::Unknown)), rules: rules.map(|value| Sourced::new(value, RequirementSource::Unknown)), enforce_residency: enforce_residency @@ -1785,6 +1837,7 @@ mod tests { hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: Some(enforce_residency), @@ -1834,6 +1887,7 @@ mod tests { hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: Some(Sourced::new(enforce_residency, enforce_source)), @@ -1880,6 +1934,7 @@ mod tests { hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: None, @@ -1934,6 +1989,7 @@ mod tests { hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: None, diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index ed2675771..9423b6ff0 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -61,6 +61,9 @@ pub use config_requirements::ConstrainedWithSource; pub use config_requirements::FeatureRequirementsToml; pub use config_requirements::FilesystemConstraints; pub use config_requirements::FilesystemDenyReadPattern; +pub use config_requirements::MarketplaceAllowedSourceKind; +pub use config_requirements::MarketplaceAllowedSourceToml; +pub use config_requirements::MarketplaceRequirementsToml; pub use config_requirements::McpServerIdentity; pub use config_requirements::McpServerRequirement; pub use config_requirements::NetworkConstraints; diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs index a7d8245ec..3be1d11b5 100644 --- a/codex-rs/config/src/requirements_layers/stack.rs +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -222,6 +222,7 @@ fn populate_merged_regular_fields_with_sources( hooks: _, mcp_servers, plugins, + marketplaces, apps, rules: _, enforce_residency, @@ -250,6 +251,7 @@ fn populate_merged_regular_fields_with_sources( set_sourced!(feature_requirements, &["features", "feature_requirements"]); set_sourced!(mcp_servers, &["mcp_servers"]); set_sourced!(plugins, &["plugins"]); + set_sourced!(marketplaces, &["marketplaces"]); set_sourced!(apps, &["apps"]); set_sourced!(enforce_residency, &["enforce_residency"]); set_sourced!(network, &["experimental_network"]); diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs index fafd0523e..a775f68a8 100644 --- a/codex-rs/config/src/requirements_layers/stack_tests.rs +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -13,6 +13,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::cell::Cell; use std::collections::BTreeMap; +use tempfile::TempDir; fn layer(id: &str, name: &str, contents: &str) -> RequirementsLayerEntry { RequirementsLayerEntry::from_toml( @@ -1026,3 +1027,154 @@ fn parse_error_names_layer() { assert!(err.to_string().contains("Bad layer (req_bad)")); assert!(err.to_string().contains("allowed_approval_policies")); } + +#[test] +fn marketplace_allowed_sources_use_default_toml_merge() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.shared] +source = "git" +url = "https://github.com/example/old.git" +ref = "main" + +[marketplaces.allowed_sources.other] +source = "git" +url = "https://github.com/example/other.git" +"#, + ), + layer( + "req_high", + "High", + r#" +[marketplaces.allowed_sources.shared] +ref = "release" +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.shared] +source = "git" +url = "https://github.com/example/old.git" +ref = "release" + +[marketplaces.allowed_sources.other] +source = "git" +url = "https://github.com/example/other.git" +"#, + ) + ); +} + +#[test] +fn marketplace_source_switch_uses_default_toml_merge() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[marketplaces.allowed_sources.company] +source = "git" +url = "https://github.com/example/plugins.git" +ref = "main" +"#, + ), + layer( + "req_high", + "High", + r#" +[marketplaces.allowed_sources.company] +source = "host_pattern" +host_pattern = '^github\.example\.com$' +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[marketplaces.allowed_sources.company] +source = "host_pattern" +url = "https://github.com/example/plugins.git" +ref = "main" +host_pattern = '^github\.example\.com$' +"#, + ) + ); +} + +#[test] +fn marketplace_allowed_source_rejects_unknown_fields() { + let err = compose(vec![layer( + "req_bad", + "Bad marketplace layer", + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.invalid] +source = "git" +url = "https://github.com/example/plugins.git" +reff = "main" +"#, + )]) + .expect_err("invalid marketplace rule should fail"); + + assert!(err.to_string().contains("Bad marketplace layer (req_bad)")); + assert!(err.to_string().contains("unknown field `reff`")); +} + +#[test] +fn local_marketplace_path_is_not_resolved_during_requirements_merge() { + let base_dir = TempDir::new().expect("create requirements base directory"); + let base_dir = AbsolutePathBuf::try_from(base_dir.path().to_path_buf()) + .expect("absolute requirements base directory"); + let composed = compose(vec![ + layer( + "req_local", + "Local marketplace path", + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.local] +source = "local" +path = "../plugins" +"#, + ) + .with_base_dir(base_dir), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.local] +source = "local" +path = "../plugins" +"#, + ) + ); +} diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index f09808460..97993b59c 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -8679,6 +8679,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 923dd6758..96ba987b9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2940,6 +2940,7 @@ impl Config { managed_hooks: _, mcp_servers, plugins: _, + marketplaces: _, exec_policy: _, enforce_residency, network: network_requirements, diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index faacca458..f8da379fd 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -785,6 +785,7 @@ mod tests { }, )])), plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: Some(ResidencyRequirement::Us), @@ -1142,6 +1143,7 @@ approval_policy = "never" hooks: None, mcp_servers: None, plugins: None, + marketplaces: None, apps: None, rules: None, enforce_residency: None,