From 2554403c52028d227bc79ca2102829dadf24e730 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Wed, 17 Jun 2026 10:44:42 -0700 Subject: [PATCH] Isolate marketplace Git transport config --- codex-rs/core-plugins/src/git_transport.rs | 141 ++++++++++++++++++ codex-rs/core-plugins/src/lib.rs | 1 + codex-rs/core-plugins/src/loader.rs | 4 + .../src/marketplace_add/install.rs | 7 + .../src/marketplace_upgrade/git.rs | 42 ++++-- codex-rs/core-plugins/src/startup_sync.rs | 4 + 6 files changed, 187 insertions(+), 12 deletions(-) create mode 100644 codex-rs/core-plugins/src/git_transport.rs diff --git a/codex-rs/core-plugins/src/git_transport.rs b/codex-rs/core-plugins/src/git_transport.rs new file mode 100644 index 0000000000..b1fbd43676 --- /dev/null +++ b/codex-rs/core-plugins/src/git_transport.rs @@ -0,0 +1,141 @@ +use std::process::Command; +use tempfile::TempDir; + +/// Runs transport commands from a clean, minimal repository so Git cannot +/// discover repository-local configuration from the directory where Codex was +/// launched. User-level Git configuration remains available. +pub(crate) struct NeutralGitCwd { + directory: TempDir, +} + +impl NeutralGitCwd { + pub(crate) fn new() -> std::io::Result { + let directory = tempfile::tempdir()?; + initialize_empty_repository(directory.path())?; + Ok(Self { directory }) + } + + pub(crate) fn configure(&self, command: &mut Command) { + command + .current_dir(self.directory.path()) + .env_remove("GIT_DIR") + .env_remove("GIT_WORK_TREE") + .env("GIT_CEILING_DIRECTORIES", self.directory.path()); + } +} + +fn initialize_empty_repository(root: &std::path::Path) -> std::io::Result<()> { + let git_dir = root.join(".git"); + std::fs::create_dir(&git_dir)?; + std::fs::create_dir(git_dir.join("objects"))?; + std::fs::create_dir_all(git_dir.join("refs/heads"))?; + std::fs::write(git_dir.join("HEAD"), "ref: refs/heads/main\n")?; + std::fs::write( + git_dir.join("config"), + "[core]\n\trepositoryformatversion = 0\n\tbare = false\n", + ) +} + +#[cfg(test)] +mod tests { + use super::NeutralGitCwd; + use super::initialize_empty_repository; + use pretty_assertions::assert_eq; + use std::ffi::OsStr; + use std::fs; + use std::process::Command; + + #[test] + fn configures_an_isolated_repository_discovery_boundary() { + let neutral_cwd = NeutralGitCwd::new().expect("create neutral Git working directory"); + let mut command = Command::new("git"); + neutral_cwd.configure(&mut command); + + assert_eq!( + command.get_current_dir(), + Some(neutral_cwd.directory.path()) + ); + assert_eq!( + command + .get_envs() + .find(|(key, _)| *key == OsStr::new("GIT_CEILING_DIRECTORIES")) + .and_then(|(_, value)| value), + Some(neutral_cwd.directory.path().as_os_str()) + ); + assert_eq!( + command + .get_envs() + .find(|(key, _)| *key == OsStr::new("GIT_DIR")) + .map(|(_, value)| value), + Some(None) + ); + assert_eq!( + command + .get_envs() + .find(|(key, _)| *key == OsStr::new("GIT_WORK_TREE")) + .map(|(_, value)| value), + Some(None) + ); + } + + #[cfg(unix)] + #[test] + fn nested_neutral_directory_does_not_load_parent_repository_config() { + let root = tempfile::tempdir().expect("create test root"); + let source = root.path().join("source"); + fs::create_dir(&source).expect("create source repository"); + run_git(&source, &["init"]); + run_git(&source, &["config", "user.email", "codex-test@example.com"]); + run_git(&source, &["config", "user.name", "Codex Test"]); + fs::write(source.join("README.md"), "safe source\n").expect("write source file"); + run_git(&source, &["add", "README.md"]); + run_git(&source, &["commit", "-m", "initial"]); + + let hostile_repo = root.path().join("hostile"); + fs::create_dir(&hostile_repo).expect("create hostile repository"); + run_git(&hostile_repo, &["init"]); + run_git( + &hostile_repo, + &[ + "config", + "url.file:///definitely-not-a-real-codex-test-repo.insteadOf", + source.to_string_lossy().as_ref(), + ], + ); + + let directory = tempfile::Builder::new() + .prefix("neutral-") + .tempdir_in(&hostile_repo) + .expect("create nested neutral directory"); + initialize_empty_repository(directory.path()).expect("initialize neutral repository"); + let neutral_cwd = NeutralGitCwd { directory }; + let mut command = Command::new("git"); + neutral_cwd.configure(&mut command); + let output = command + .args(["ls-remote", source.to_string_lossy().as_ref(), "HEAD"]) + .output() + .expect("run git ls-remote"); + + assert!( + output.status.success(), + "ls-remote should ignore the parent repository's URL rewrite: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(!output.stdout.is_empty()); + } + + #[cfg(unix)] + fn run_git(cwd: &std::path::Path, args: &[&str]) { + let output = Command::new("git") + .current_dir(cwd) + .args(args) + .output() + .expect("run git"); + assert!( + output.status.success(), + "git {} failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr) + ); + } +} diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index df30305c38..9afab4d6b6 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -1,5 +1,6 @@ mod app_mcp_routing; mod discoverable; +mod git_transport; pub mod installed_marketplaces; pub mod loader; mod manager; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 2b5c4782e8..43e27a1bc9 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,5 +1,6 @@ use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::app_mcp_routing::apps_route_available; +use crate::git_transport::NeutralGitCwd; use crate::is_openai_curated_marketplace_name; use crate::manifest::PluginManifest; use crate::manifest::PluginManifestHooks; @@ -1493,9 +1494,12 @@ fn clone_git_plugin_source( } fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), String> { + let neutral_cwd = NeutralGitCwd::new() + .map_err(|err| format!("failed to create neutral Git working directory: {err}"))?; let mut command = Command::new("git"); command.args(args); command.env("GIT_TERMINAL_PROMPT", "0"); + neutral_cwd.configure(&mut command); if let Some(cwd) = cwd { command.current_dir(cwd); } diff --git a/codex-rs/core-plugins/src/marketplace_add/install.rs b/codex-rs/core-plugins/src/marketplace_add/install.rs index 1ecfa050d3..ccbac0aeb9 100644 --- a/codex-rs/core-plugins/src/marketplace_add/install.rs +++ b/codex-rs/core-plugins/src/marketplace_add/install.rs @@ -1,4 +1,5 @@ use super::MarketplaceAddError; +use crate::git_transport::NeutralGitCwd; use std::fs; use std::path::Path; use std::path::PathBuf; @@ -111,9 +112,15 @@ pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { } fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), MarketplaceAddError> { + let neutral_cwd = NeutralGitCwd::new().map_err(|err| { + MarketplaceAddError::Internal(format!( + "failed to create neutral Git working directory: {err}" + )) + })?; let mut command = Command::new("git"); command.args(args); command.env("GIT_TERMINAL_PROMPT", "0"); + neutral_cwd.configure(&mut command); if let Some(cwd) = cwd { command.current_dir(cwd); } diff --git a/codex-rs/core-plugins/src/marketplace_upgrade/git.rs b/codex-rs/core-plugins/src/marketplace_upgrade/git.rs index 9a46465dbc..ffa54966ad 100644 --- a/codex-rs/core-plugins/src/marketplace_upgrade/git.rs +++ b/codex-rs/core-plugins/src/marketplace_upgrade/git.rs @@ -1,3 +1,4 @@ +use crate::git_transport::NeutralGitCwd; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -17,8 +18,13 @@ pub(super) fn git_remote_revision( } let ref_name = ref_name.unwrap_or("HEAD"); + let neutral_cwd = NeutralGitCwd::new() + .map_err(|err| format!("failed to create neutral Git working directory: {err}"))?; let output = run_git_command_with_timeout( - git_command().arg("ls-remote").arg(source).arg(ref_name), + git_command(&neutral_cwd) + .arg("ls-remote") + .arg(source) + .arg(ref_name), "git ls-remote marketplace source", timeout, )?; @@ -47,17 +53,22 @@ pub(super) fn clone_git_source( destination: &Path, timeout: Duration, ) -> Result { + let neutral_cwd = NeutralGitCwd::new() + .map_err(|err| format!("failed to create neutral Git working directory: {err}"))?; let git_destination = git_path_arg(destination); if sparse_paths.is_empty() { let output = run_git_command_with_timeout( - git_command().arg("clone").arg(source).arg(&git_destination), + git_command(&neutral_cwd) + .arg("clone") + .arg(source) + .arg(&git_destination), "git clone marketplace source", timeout, )?; ensure_git_success(&output, "git clone marketplace source")?; if let Some(ref_name) = ref_name { let output = run_git_command_with_timeout( - git_command() + git_command(&neutral_cwd) .arg("-C") .arg(&git_destination) .arg("checkout") @@ -67,11 +78,11 @@ pub(super) fn clone_git_source( )?; ensure_git_success(&output, "git checkout marketplace ref")?; } - return git_worktree_revision(&git_destination, timeout); + return git_worktree_revision(&git_destination, timeout, &neutral_cwd); } let output = run_git_command_with_timeout( - git_command() + git_command(&neutral_cwd) .arg("clone") .arg("--filter=blob:none") .arg("--no-checkout") @@ -82,7 +93,7 @@ pub(super) fn clone_git_source( )?; ensure_git_success(&output, "git clone marketplace source")?; - let mut sparse_checkout = git_command(); + let mut sparse_checkout = git_command(&neutral_cwd); sparse_checkout .arg("-C") .arg(&git_destination) @@ -97,7 +108,7 @@ pub(super) fn clone_git_source( ensure_git_success(&output, "git sparse-checkout marketplace source")?; let output = run_git_command_with_timeout( - git_command() + git_command(&neutral_cwd) .arg("-C") .arg(&git_destination) .arg("checkout") @@ -106,12 +117,16 @@ pub(super) fn clone_git_source( timeout, )?; ensure_git_success(&output, "git checkout marketplace ref")?; - git_worktree_revision(&git_destination, timeout) + git_worktree_revision(&git_destination, timeout, &neutral_cwd) } -fn git_worktree_revision(destination: &Path, timeout: Duration) -> Result { +fn git_worktree_revision( + destination: &Path, + timeout: Duration, + neutral_cwd: &NeutralGitCwd, +) -> Result { let output = run_git_command_with_timeout( - git_command() + git_command(neutral_cwd) .arg("-C") .arg(destination) .arg("rev-parse") @@ -133,11 +148,12 @@ fn is_full_git_sha(value: &str) -> bool { value.len() == 40 && value.chars().all(|ch| ch.is_ascii_hexdigit()) } -fn git_command() -> Command { +fn git_command(neutral_cwd: &NeutralGitCwd) -> Command { let mut command = Command::new("git"); command .env("GIT_OPTIONAL_LOCKS", "0") .env("GIT_TERMINAL_PROMPT", "0"); + neutral_cwd.configure(&mut command); command } @@ -226,6 +242,7 @@ mod tests { use super::git_command; use super::is_full_git_sha; use super::strip_windows_verbatim_path_prefix; + use crate::git_transport::NeutralGitCwd; use pretty_assertions::assert_eq; use std::ffi::OsStr; @@ -238,7 +255,8 @@ mod tests { #[test] fn git_command_uses_path_lookup_with_stable_noninteractive_env() { - let command = git_command(); + let neutral_cwd = NeutralGitCwd::new().expect("create neutral Git working directory"); + let command = git_command(&neutral_cwd); assert_eq!(command.get_program(), OsStr::new("git")); assert_eq!( diff --git a/codex-rs/core-plugins/src/startup_sync.rs b/codex-rs/core-plugins/src/startup_sync.rs index eb7813f35e..90d5603592 100644 --- a/codex-rs/core-plugins/src/startup_sync.rs +++ b/codex-rs/core-plugins/src/startup_sync.rs @@ -1,3 +1,4 @@ +use crate::git_transport::NeutralGitCwd; use std::fs::File; use std::path::Path; use std::path::PathBuf; @@ -595,11 +596,14 @@ fn read_local_git_or_sha_file( } fn git_ls_remote_head_sha(git_binary: &str) -> Result { + let neutral_cwd = NeutralGitCwd::new() + .map_err(|err| format!("failed to create neutral Git working directory: {err}"))?; let mut command = git_command(git_binary); command .arg("ls-remote") .arg("https://github.com/openai/plugins.git") .arg("HEAD"); + neutral_cwd.configure(&mut command); let output = run_git_command_with_timeout( &mut command, "git ls-remote curated plugins repo",