mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[plugins] Enforce marketplace source admission requirements (#29753)
## Why Managed marketplace source requirements only become effective when every local marketplace mutation path applies the same admission decision. This change centralizes that decision so CLI, app-server, and external-agent migration flows cannot add, install from, or refresh a disallowed source. ## What changed - Match exact normalized Git repository URLs with an optional exact `ref`. - Match Git hosts with managed regular expressions. - Match local marketplaces by exact absolute path. - Preserve the expected path/name boundary for managed OpenAI marketplaces. - Enforce source admission during marketplace add, plugin install, and configured Git marketplace upgrade. - Continue upgrading independent marketplaces when one source is rejected and return a per-marketplace error. - Load the effective requirements stack at CLI, app-server, and external-agent migration entry points. This PR does not filter already configured marketplaces at runtime; that remains in draft follow-up #29691. ## Stack This is PR 2 of 3 and is based on #29690, which introduces the requirements data shape and merge behavior. ## Test plan - Source matcher coverage for Git URL/ref, host-pattern, local-path, and managed marketplace cases. - Marketplace add and plugin install coverage for allowed and rejected sources. - Marketplace upgrade coverage for rejection and per-marketplace continuation.
This commit is contained in:
@@ -35,6 +35,9 @@ use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerOAuthConfig;
|
||||
use codex_config::McpServerToolConfig;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::RequirementsLayerEntry;
|
||||
use codex_config::compose_requirements;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core_skills::PluginSkillSnapshots;
|
||||
use codex_core_skills::SkillsLoadInput;
|
||||
@@ -64,6 +67,39 @@ use wiremock::matchers::query_param;
|
||||
|
||||
const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024;
|
||||
|
||||
fn unrestricted_config_layer_stack() -> ConfigLayerStack {
|
||||
ConfigLayerStack::default()
|
||||
}
|
||||
|
||||
fn config_layer_stack_with_requirements(
|
||||
codex_home: &Path,
|
||||
user_config: &str,
|
||||
requirements: &str,
|
||||
) -> ConfigLayerStack {
|
||||
let with_sources = compose_requirements([RequirementsLayerEntry::from_toml(
|
||||
RequirementSource::Unknown,
|
||||
requirements,
|
||||
)])
|
||||
.expect("compose requirements")
|
||||
.expect("requirements should be present");
|
||||
let requirements_toml = with_sources.clone().into_toml();
|
||||
let requirements = ConfigRequirements::try_from(with_sources).expect("normalize requirements");
|
||||
let config_file =
|
||||
AbsolutePathBuf::try_from(codex_home.join(CONFIG_TOML_FILE)).expect("absolute config path");
|
||||
ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: config_file,
|
||||
profile: None,
|
||||
},
|
||||
toml::from_str(user_config).expect("parse user config"),
|
||||
)],
|
||||
requirements,
|
||||
requirements_toml,
|
||||
)
|
||||
.expect("build config layer stack")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugins_manager_tracks_auth_mode() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
@@ -2402,13 +2438,16 @@ async fn install_plugin_updates_config_with_relative_path_and_plugin_key() {
|
||||
.unwrap();
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.install_plugin(
|
||||
&unrestricted_config_layer_stack(),
|
||||
PluginInstallRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2428,6 +2467,85 @@ async fn install_plugin_updates_config_with_relative_path_and_plugin_key() {
|
||||
assert!(config.contains("enabled = true"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn strict_install_requires_allowed_local_marketplace_to_be_added_first() {
|
||||
let codex_home = TempDir::new().expect("create Codex home");
|
||||
let marketplace_root = codex_home.path().join("company-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 marketplace_root = marketplace_root
|
||||
.canonicalize()
|
||||
.expect("canonical marketplace root");
|
||||
let requirements = format!(
|
||||
r#"
|
||||
[marketplaces]
|
||||
restrict_to_allowed_sources = true
|
||||
|
||||
[marketplaces.allowed_sources.company]
|
||||
source = "local"
|
||||
path = {marketplace_root:?}
|
||||
"#
|
||||
);
|
||||
let config = config_layer_stack_with_requirements(codex_home.path(), "", &requirements);
|
||||
let marketplace_path =
|
||||
AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json"))
|
||||
.expect("absolute marketplace path");
|
||||
let manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
|
||||
let err = manager
|
||||
.install_plugin(
|
||||
&config,
|
||||
PluginInstallRequest {
|
||||
plugin_name: "sample".to_string(),
|
||||
marketplace_path: marketplace_path.clone(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect_err("unconfigured local marketplace should not be installable in strict mode");
|
||||
assert!(matches!(
|
||||
err,
|
||||
PluginInstallError::Marketplace(MarketplaceError::InvalidMarketplaceFile { .. })
|
||||
));
|
||||
assert!(err.to_string().contains("must be added to config"));
|
||||
assert!(!codex_home.path().join(CONFIG_TOML_FILE).exists());
|
||||
|
||||
let user_config = format!(
|
||||
r#"
|
||||
[marketplaces.company]
|
||||
source_type = "local"
|
||||
source = {marketplace_root:?}
|
||||
"#
|
||||
);
|
||||
write_file(&codex_home.path().join(CONFIG_TOML_FILE), &user_config);
|
||||
let config =
|
||||
config_layer_stack_with_requirements(codex_home.path(), &user_config, &requirements);
|
||||
let outcome = manager
|
||||
.install_plugin(
|
||||
&config,
|
||||
PluginInstallRequest {
|
||||
plugin_name: "sample".to_string(),
|
||||
marketplace_path,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("configured allowlisted marketplace should be installable");
|
||||
assert_eq!(
|
||||
outcome.plugin_id,
|
||||
PluginId::new("sample".to_string(), "company".to_string()).expect("plugin id")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_openai_curated_plugin_uses_short_sha_cache_version() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
@@ -2436,13 +2554,16 @@ async fn install_openai_curated_plugin_uses_short_sha_cache_version() {
|
||||
write_curated_plugin_sha(tmp.path(), TEST_CURATED_PLUGIN_SHA);
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "slack".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
curated_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.install_plugin(
|
||||
&unrestricted_config_layer_stack(),
|
||||
PluginInstallRequest {
|
||||
plugin_name: "slack".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
curated_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2494,13 +2615,16 @@ async fn install_plugin_uses_manifest_version_for_non_curated_plugins() {
|
||||
.unwrap();
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.install_plugin(
|
||||
&unrestricted_config_layer_stack(),
|
||||
PluginInstallRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2554,13 +2678,16 @@ async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin
|
||||
.unwrap();
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "quality-review".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.install_plugin(
|
||||
&unrestricted_config_layer_stack(),
|
||||
PluginInstallRequest {
|
||||
plugin_name: "quality-review".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2643,13 +2770,16 @@ async fn install_plugin_supports_git_subdir_marketplace_sources() {
|
||||
.unwrap();
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "toolkit".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.install_plugin(
|
||||
&unrestricted_config_layer_stack(),
|
||||
PluginInstallRequest {
|
||||
plugin_name: "toolkit".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -2694,13 +2824,16 @@ async fn install_plugin_supports_relative_git_subdir_marketplace_sources() {
|
||||
.unwrap();
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "toolkit".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.install_plugin(
|
||||
&unrestricted_config_layer_stack(),
|
||||
PluginInstallRequest {
|
||||
plugin_name: "toolkit".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -4094,7 +4227,7 @@ source = "{remote_repo_url}"
|
||||
run_git(&remote_repo, &["add", "."]);
|
||||
run_git(&remote_repo, &["commit", "-m", "update plugin"]);
|
||||
let upgrade = manager
|
||||
.upgrade_configured_marketplaces_for_config(&config, /*marketplace_name*/ None)
|
||||
.upgrade_configured_marketplaces_for_config(&config, Some("debug"))
|
||||
.expect("marketplace upgrade should succeed");
|
||||
assert_eq!(upgrade.errors, Vec::new());
|
||||
assert_eq!(upgrade.upgraded_roots.len(), 1);
|
||||
|
||||
Reference in New Issue
Block a user