Prevent marketplace identity spoofing (#39165)

## Why

Repository manifests and user-configured sources must not be able to claim names reserved for managed or remote marketplaces.

## What changed

- Reject reserved marketplace names from unmanaged sources during discovery, installation, configuration projection, and configured marketplace upgrades, even when source restrictions are disabled.
- Recognize managed marketplaces only at their expected paths. Preserve the required macOS and Windows path equivalences without allowing symlink aliases to inherit managed provenance.

## Testing

- Cover reserved-name rejection for plugin list and install RPCs, configured upgrades, unrestricted policy, and remote marketplace names.
- Cover symlink spoofing and Windows managed-path normalization.

GitOrigin-RevId: a610bdc382857f306dd3e244f0992de2ec279673
This commit is contained in:
Eric Traut
2026-08-18 07:56:41 +00:00
committed by copyberry
parent e2eea07140
commit 0acf302db5
7 changed files with 262 additions and 65 deletions
@@ -19,6 +19,7 @@ use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::LoginAccountResponse;
use codex_app_server_protocol::PluginAuthPolicy;
use codex_app_server_protocol::PluginDisabledReason;
use codex_app_server_protocol::PluginInstallParams;
use codex_app_server_protocol::PluginInstallPolicy;
use codex_app_server_protocol::PluginInstallPolicySource;
use codex_app_server_protocol::PluginInstalledParams;
@@ -158,6 +159,68 @@ async fn plugin_list_skips_invalid_marketplace_file_and_reports_error() -> Resul
Ok(())
}
#[tokio::test]
async fn plugin_rpcs_reject_repository_spoofing_openai_curated() -> Result<()> {
let codex_home = TempDir::new()?;
let repository = TempDir::new()?;
std::fs::create_dir_all(repository.path().join(".git"))?;
std::fs::create_dir_all(repository.path().join(".agents/plugins"))?;
std::fs::create_dir_all(repository.path().join("attacker/.codex-plugin"))?;
std::fs::write(
repository.path().join("attacker/.codex-plugin/plugin.json"),
r#"{"name":"attacker"}"#,
)?;
let marketplace_path =
AbsolutePathBuf::try_from(repository.path().join(".agents/plugins/marketplace.json"))?;
std::fs::write(
marketplace_path.as_path(),
r#"{"name":"openai-curated","plugins":[{"name":"attacker","source":{"source":"local","path":"./attacker"}}]}"#,
)?;
write_plugins_enabled_config(codex_home.path())?;
let original_config = std::fs::read(codex_home.path().join("config.toml"))?;
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
.build_initialized_with_timeout(DEFAULT_TIMEOUT)
.await?;
let request_id = mcp
.send_plugin_list_request(PluginListParams {
cwds: Some(vec![AbsolutePathBuf::try_from(repository.path())?]),
marketplace_kinds: None,
force_refetch: false,
})
.await?;
let response: PluginListResponse =
timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??;
assert!(
response
.marketplaces
.iter()
.all(|marketplace| marketplace.path.as_ref() != Some(&marketplace_path))
);
let request_id = mcp
.send_plugin_install_request(PluginInstallParams {
marketplace_path: Some(marketplace_path),
remote_marketplace_name: None,
install_attempt_id: None,
plugin_name: "attacker".to_string(),
})
.await?;
let error = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
assert!(error.error.message.contains("reserved"));
assert!(!codex_home.path().join("plugins/cache").exists());
assert_eq!(
std::fs::read(codex_home.path().join("config.toml"))?,
original_config
);
Ok(())
}
#[tokio::test]
async fn plugin_installed_includes_installed_plugins_and_explicit_install_suggestions() -> Result<()>
{
@@ -959,18 +959,11 @@ async fn plugin_read_rejects_invalid_remote_plugin_name() -> Result<()> {
#[tokio::test]
async fn plugin_read_returns_canonical_openai_curated_marketplace_name() -> Result<()> {
let codex_home = TempDir::new()?;
let repo_root = TempDir::new()?;
write_plugin_marketplace(
repo_root.path(),
"openai-curated",
"demo-plugin",
"./demo-plugin",
)?;
std::fs::create_dir_all(repo_root.path().join("demo-plugin/.codex-plugin"))?;
let repo_root = codex_home.path().join(".tmp/plugins");
write_plugin_marketplace(&repo_root, "openai-curated", "demo-plugin", "./demo-plugin")?;
std::fs::create_dir_all(repo_root.join("demo-plugin/.codex-plugin"))?;
std::fs::write(
repo_root
.path()
.join("demo-plugin/.codex-plugin/plugin.json"),
repo_root.join("demo-plugin/.codex-plugin/plugin.json"),
r#"{
"name": "demo-plugin",
"description": "OpenAI curated plugin"
@@ -994,7 +987,7 @@ enabled = true
.await?;
let marketplace_path =
AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?;
AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json"))?;
let request_id = mcp
.send_plugin_read_request(PluginReadParams {
marketplace_path: Some(marketplace_path.clone()),
@@ -156,9 +156,9 @@ async fn omits_openai_curated_but_keeps_configured_marketplaces_for_remote_codex
write_openai_curated_marketplace(&curated_root, &["slack"]);
let bundled_marketplace_name = OPENAI_BUNDLED_MARKETPLACE_NAME;
let bundled_marketplace_root = codex_home
.path()
.join(format!(".tmp/marketplaces/{bundled_marketplace_name}"));
let bundled_marketplace_root = codex_home.path().join(format!(
".tmp/bundled-marketplaces/{bundled_marketplace_name}"
));
write_file(
&bundled_marketplace_root.join(".agents/plugins/marketplace.json"),
&format!(
@@ -179,8 +179,8 @@ async fn omits_openai_curated_but_keeps_configured_marketplaces_for_remote_codex
plugins = true
[marketplaces.{bundled_marketplace_name}]
source_type = "git"
source = "/tmp/{bundled_marketplace_name}"
source_type = "local"
source = {bundled_marketplace_root:?}
"#
),
);
@@ -240,7 +240,7 @@ async fn deduplicates_and_reprojects_cached_configured_marketplace_plugin() {
let plugin_id = format!("{plugin_name}@{marketplace_name}");
let marketplace_root = codex_home
.path()
.join(format!(".tmp/marketplaces/{marketplace_name}"));
.join(format!(".tmp/bundled-marketplaces/{marketplace_name}"));
write_file(
&marketplace_root.join(".agents/plugins/marketplace.json"),
&format!(
@@ -267,8 +267,8 @@ async fn deduplicates_and_reprojects_cached_configured_marketplace_plugin() {
plugins = true
[marketplaces.{marketplace_name}]
source_type = "git"
source = "/tmp/{marketplace_name}"
source_type = "local"
source = {marketplace_root:?}
"#
),
);
+8 -10
View File
@@ -40,7 +40,6 @@ 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;
@@ -2921,16 +2920,15 @@ impl PluginsManager {
) -> Result<MarketplaceListOutcome, MarketplaceError> {
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)
policy
.validate_install(
&config.config_layer_stack,
self.codex_home.as_path(),
&marketplace.path,
&marketplace.name,
)
.is_ok()
});
Ok(outcome)
}
+74 -26
View File
@@ -9,6 +9,7 @@ use crate::is_openai_curated_marketplace_name;
use crate::marketplace::marketplace_root_dir;
use crate::marketplace_add::MarketplaceSource;
use crate::marketplace_add::parse_marketplace_source;
use crate::remote::RemotePluginScope;
use crate::startup_sync::curated_plugins_api_marketplace_path;
use crate::startup_sync::curated_plugins_repo_path;
use codex_config::ConfigLayerStack;
@@ -107,14 +108,18 @@ impl MarketplacePolicy {
marketplace_path: &AbsolutePathBuf,
marketplace_name: &str,
) -> Result<(), String> {
if !self.is_restricted() {
return Ok(());
}
let root = marketplace_root_dir(marketplace_path).map_err(|err| err.to_string())?;
if let Some(expected_name) = managed_marketplace_name(codex_home, marketplace_path, &root) {
return validate_expected_marketplace_name(expected_name, marketplace_name);
}
if is_reserved_marketplace_name(marketplace_name) {
return Err(format!(
"marketplace `{marketplace_name}` is reserved and cannot be loaded from this source"
));
}
if !self.is_restricted() {
return Ok(());
}
let user_config = config_layer_stack.effective_user_config().ok_or_else(|| {
format!(
@@ -208,9 +213,6 @@ pub(crate) fn project_effective_user_config(
) -> Option<toml::Value> {
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
@@ -232,11 +234,13 @@ pub(crate) fn project_effective_user_config(
{
plugins.retain(|plugin_key, _| {
let Ok(plugin_id) = PluginId::parse(plugin_key) else {
return false;
return !policy.is_restricted();
};
(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)
let marketplace_name = &plugin_id.marketplace_name;
if configured_marketplace_names.contains(marketplace_name) {
return allowed_marketplace_names.contains(marketplace_name);
}
is_openai_curated_marketplace_name(marketplace_name) || !policy.is_restricted()
});
}
Some(user_config)
@@ -264,14 +268,13 @@ fn allowed_configured_marketplace_names_with_policy(
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 if is_reserved_marketplace_name(marketplace_name) => false,
None if !policy.is_restricted() => true,
None => policy
.validate_configured_marketplace(marketplace_name, marketplace)
.is_ok(),
@@ -306,14 +309,14 @@ pub(crate) fn validate_marketplace_source_for_add(
source: &MarketplaceSource,
) -> Result<Option<&'static str>, String> {
let policy = MarketplacePolicy::from_requirements(requirements);
if !policy.is_restricted() {
return Ok(None);
}
if let MarketplaceSource::Local { path } = source
&& let Some(expected_name) = managed_local_marketplace_name(codex_home, path)
{
return Ok(Some(expected_name));
}
if !policy.is_restricted() {
return Ok(None);
}
policy.validate_source(source)?;
Ok(None)
}
@@ -325,7 +328,7 @@ pub(crate) fn validate_marketplace_name_for_add(
if let Some(expected_name) = expected_name {
return validate_expected_marketplace_name(expected_name, marketplace_name);
}
if is_openai_curated_marketplace_name(marketplace_name) {
if is_reserved_marketplace_name(marketplace_name) {
return Err(format!(
"marketplace `{marketplace_name}` is reserved and cannot be added from this source"
));
@@ -443,18 +446,30 @@ fn validate_expected_marketplace_name(
})
}
fn is_reserved_marketplace_name(marketplace_name: &str) -> bool {
matches!(
marketplace_name,
OPENAI_CURATED_MARKETPLACE_NAME
| OPENAI_API_CURATED_MARKETPLACE_NAME
| OPENAI_BUNDLED_MARKETPLACE_NAME
| OPENAI_BUNDLED_ALPHA_MARKETPLACE_NAME
| OPENAI_PRIMARY_RUNTIME_MARKETPLACE_NAME
) || RemotePluginScope::from_marketplace_name(marketplace_name).is_some()
}
fn managed_marketplace_name(
codex_home: &Path,
marketplace_path: &AbsolutePathBuf,
root: &AbsolutePathBuf,
) -> Option<&'static str> {
if paths_match_after_normalization(
if is_expected_managed_path(
marketplace_path.as_path(),
curated_plugins_api_marketplace_path(codex_home),
) {
&curated_plugins_api_marketplace_path(codex_home),
) && is_expected_managed_path(root.as_path(), &curated_plugins_repo_path(codex_home))
{
return Some(OPENAI_API_CURATED_MARKETPLACE_NAME);
}
if paths_match_after_normalization(root.as_path(), curated_plugins_repo_path(codex_home)) {
if is_expected_managed_path(root.as_path(), &curated_plugins_repo_path(codex_home)) {
return Some(OPENAI_CURATED_MARKETPLACE_NAME);
}
managed_local_marketplace_name(codex_home, root.as_path())
@@ -483,14 +498,42 @@ fn managed_local_marketplace_name(codex_home: &Path, root: &Path) -> Option<&'st
let expected_root = codex_home
.join(".tmp/bundled-marketplaces")
.join(marketplace_name);
if paths_match_after_normalization(root, &expected_root) {
if is_expected_managed_path(root, &expected_root) {
return Some(marketplace_name);
}
}
let runtime_root = primary_runtime_marketplace_root()?;
paths_match_after_normalization(root, &runtime_root)
.then_some(OPENAI_PRIMARY_RUNTIME_MARKETPLACE_NAME)
is_expected_managed_path(root, &runtime_root).then_some(OPENAI_PRIMARY_RUNTIME_MARKETPLACE_NAME)
}
fn is_expected_managed_path(path: &Path, expected: &Path) -> bool {
if path == expected {
return true;
}
#[cfg(target_os = "macos")]
{
path.strip_prefix("/private/var")
.is_ok_and(|suffix| Path::new("/var").join(suffix) == expected)
|| expected
.strip_prefix("/private/var")
.is_ok_and(|suffix| Path::new("/var").join(suffix) == path)
}
#[cfg(windows)]
{
let path = path.to_string_lossy();
let expected = expected.to_string_lossy();
let path = codex_utils_absolute_path::normalize_windows_device_path(&path)
.unwrap_or_else(|| path.into_owned());
let expected = codex_utils_absolute_path::normalize_windows_device_path(&expected)
.unwrap_or_else(|| expected.into_owned());
path.replace('/', "\\")
.eq_ignore_ascii_case(&expected.replace('/', "\\"))
}
#[cfg(not(any(target_os = "macos", windows)))]
{
false
}
}
pub(crate) fn primary_runtime_marketplace_root() -> Option<PathBuf> {
@@ -511,7 +554,12 @@ fn primary_runtime_cache_dir_from_user_profile(user_profile: Option<PathBuf>) ->
user_profile.map(|profile| profile.join(".cache"))
}
#[cfg(not(target_os = "windows"))]
#[cfg(target_os = "macos")]
fn primary_runtime_cache_dir() -> Option<PathBuf> {
dirs::home_dir().map(|home| home.join(".cache"))
}
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
fn primary_runtime_cache_dir() -> Option<PathBuf> {
dirs::cache_dir()
}
@@ -22,6 +22,10 @@ fn primary_runtime_cache_uses_user_profile_on_windows() {
primary_runtime_cache_dir_from_user_profile(Some(PathBuf::from(r"C:\Users\user"))),
Some(PathBuf::from(r"C:\Users\user\.cache"))
);
assert!(is_expected_managed_path(
Path::new(r"\\?\c:\users\USER\.cache"),
Path::new(r"C:\Users/user\.cache"),
));
}
fn config_layer_stack_with_user_config(
@@ -363,6 +367,81 @@ restrict_to_allowed_sources = true
)
.is_err()
);
let repository_manifest = AbsolutePathBuf::try_from(
codex_home
.path()
.join("repository/.agents/plugins/marketplace.json"),
)
.expect("absolute repository manifest");
let unrestricted = ConfigLayerStack::default();
for config in [&stack, &unrestricted] {
for marketplace in [
OPENAI_CURATED_MARKETPLACE_NAME,
OPENAI_API_CURATED_MARKETPLACE_NAME,
OPENAI_BUNDLED_MARKETPLACE_NAME,
OPENAI_BUNDLED_ALPHA_MARKETPLACE_NAME,
OPENAI_PRIMARY_RUNTIME_MARKETPLACE_NAME,
crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME,
crate::remote::REMOTE_CREATED_BY_ME_MARKETPLACE_NAME,
crate::remote::REMOTE_WORKSPACE_MARKETPLACE_NAME,
crate::remote::REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME,
crate::remote::REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME,
crate::remote::REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME,
] {
assert!(
MarketplacePolicy::from_requirements(config.requirements())
.validate_install(config, codex_home.path(), &repository_manifest, marketplace)
.is_err()
);
}
}
}
#[cfg(unix)]
#[test]
fn symlinked_marketplaces_cannot_borrow_managed_provenance() {
let codex_home = TempDir::new().expect("create Codex home");
let official_root = curated_plugins_repo_path(codex_home.path());
let manifest = official_root.join(".agents/plugins/marketplace.json");
fs::create_dir_all(manifest.parent().expect("manifest directory"))
.expect("create manifest directory");
fs::write(&manifest, "{}").expect("write official manifest");
let root_alias = codex_home.path().join("repository");
std::os::unix::fs::symlink(&official_root, &root_alias).expect("symlink official root");
let parent_alias = codex_home.path().join("parent-alias");
std::os::unix::fs::symlink(
official_root.parent().expect("official root parent"),
&parent_alias,
)
.expect("symlink official root parent");
let manifest_alias = codex_home
.path()
.join("manifest-alias/.agents/plugins/api_marketplace.json");
fs::create_dir_all(manifest_alias.parent().expect("manifest alias parent"))
.expect("create manifest alias directory");
std::os::unix::fs::symlink(&manifest, &manifest_alias).expect("symlink official manifest");
let stack = ConfigLayerStack::default();
for (path, marketplace) in [
(
root_alias.join(".agents/plugins/marketplace.json"),
OPENAI_CURATED_MARKETPLACE_NAME,
),
(
parent_alias.join("plugins/.agents/plugins/marketplace.json"),
OPENAI_CURATED_MARKETPLACE_NAME,
),
(manifest_alias, OPENAI_API_CURATED_MARKETPLACE_NAME),
] {
let manifest = AbsolutePathBuf::try_from(path).expect("absolute repository manifest");
assert!(
MarketplacePolicy::from_requirements(stack.requirements())
.validate_install(&stack, codex_home.path(), &manifest, marketplace)
.is_err()
);
}
}
#[test]
@@ -518,7 +597,7 @@ enabled = true
}
#[test]
fn allowlisted_config_names_are_not_globally_reserved() {
fn allowlisted_sources_cannot_claim_reserved_marketplace_names() {
let codex_home = TempDir::new().expect("create Codex home");
let source_root = TempDir::new().expect("create marketplace root");
let source_root = source_root
@@ -569,7 +648,7 @@ enabled = true
.keys()
.cloned()
.collect::<Vec<_>>(),
vec!["openai-bundled".to_string(), "openai-curated".to_string()]
Vec::<String>::new()
);
assert_eq!(
projected["plugins"]
@@ -578,15 +657,12 @@ enabled = true
.keys()
.cloned()
.collect::<Vec<_>>(),
vec![
"sample@openai-bundled".to_string(),
"sample@openai-curated".to_string()
]
Vec::<String>::new()
);
}
#[test]
fn blocked_upgrade_is_rejected_before_marketplace_installation() {
fn blocked_or_reserved_upgrade_is_rejected_before_marketplace_installation() {
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");
@@ -601,7 +677,7 @@ restrict_to_allowed_sources = true
source_type = "git"
source = "https://github.com/example/blocked.git"
"#,
config_file,
config_file.clone(),
)),
);
@@ -616,6 +692,21 @@ source = "https://github.com/example/blocked.git"
.contains("is not allowed by requirements")
);
assert!(!marketplace_install_root(codex_home.path()).exists());
let stack = config_layer_stack_with_user_config(
"[marketplaces]\nrestrict_to_allowed_sources = false\n",
Some((
"[marketplaces.openai-bundled]\nsource_type = \"git\"\nsource = \"https://github.com/example/blocked.git\"\n",
config_file,
)),
);
let outcome = upgrade_configured_git_marketplaces(
codex_home.path(),
&stack,
Some(crate::OPENAI_BUNDLED_MARKETPLACE_NAME),
);
assert!(outcome.errors[0].message.contains("is reserved"));
assert!(!marketplace_install_root(codex_home.path()).exists());
}
#[test]
@@ -10,6 +10,7 @@ use crate::installed_marketplaces::marketplace_install_root;
use crate::marketplace::validate_marketplace_root;
use crate::marketplace_add::MarketplaceSource;
use crate::marketplace_policy::MarketplacePolicy;
use crate::marketplace_policy::validate_marketplace_name_for_add;
use codex_config::CONFIG_TOML_FILE;
use codex_config::ConfigLayerStack;
use codex_config::MarketplaceConfigUpdate;
@@ -99,7 +100,10 @@ pub fn upgrade_configured_git_marketplaces(
let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements());
for marketplace in marketplaces {
let normalized_source =
match policy.validate_git_source(&marketplace.source, marketplace.ref_name.clone()) {
match validate_marketplace_name_for_add(/*expected_name*/ None, &marketplace.name)
.and_then(|()| {
policy.validate_git_source(&marketplace.source, marketplace.ref_name.clone())
}) {
Ok(normalized_source) => normalized_source,
Err(message) => {
errors.push(ConfiguredMarketplaceUpgradeError {