diff --git a/codex-rs/core-plugins/src/startup_sync.rs b/codex-rs/core-plugins/src/startup_sync.rs index 44fa95794..eb7813f35 100644 --- a/codex-rs/core-plugins/src/startup_sync.rs +++ b/codex-rs/core-plugins/src/startup_sync.rs @@ -34,6 +34,27 @@ const CURATED_PLUGINS_HTTP_TIMEOUT: Duration = Duration::from_secs(30); const CURATED_PLUGINS_BACKUP_ARCHIVE_TIMEOUT: Duration = Duration::from_secs(30); // Keep this comfortably above a normal sync attempt so we do not race another Codex process. const CURATED_PLUGINS_STALE_TEMP_DIR_MAX_AGE: Duration = Duration::from_secs(10 * 60); +// These variables can redirect Git away from the repository selected by `-C`, +// or inject command-scoped configuration into the sync commands. +const REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES: &[&str] = &[ + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_CEILING_DIRECTORIES", + "GIT_COMMON_DIR", + "GIT_CONFIG", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_PARAMETERS", + "GIT_DIR", + "GIT_DISCOVERY_ACROSS_FILESYSTEM", + "GIT_GRAFT_FILE", + "GIT_IMPLICIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_NAMESPACE", + "GIT_OBJECT_DIRECTORY", + "GIT_PREFIX", + "GIT_REPLACE_REF_BASE", + "GIT_SHALLOW_FILE", + "GIT_WORK_TREE", +]; #[derive(Debug, Deserialize)] struct GitHubRepositorySummary { @@ -242,17 +263,14 @@ fn fetch_curated_plugins_commit_from( context: &str, ) -> Result<(), String> { let fetch_refspec = format!("+{source_revision}:{CURATED_PLUGINS_FETCH_REF}"); - let output = run_git_command_with_timeout( - Command::new(git_binary) - .env("GIT_OPTIONAL_LOCKS", "0") - .arg("-C") - .arg(repo_path) - .args(["fetch", "--depth", "1", "--no-tags"]) - .arg(source) - .arg(fetch_refspec), - context, - CURATED_PLUGINS_GIT_TIMEOUT, - )?; + let mut command = git_command(git_binary); + command + .arg("-C") + .arg(repo_path) + .args(["fetch", "--depth", "1", "--no-tags"]) + .arg(source) + .arg(fetch_refspec); + let output = run_git_command_with_timeout(&mut command, context, CURATED_PLUGINS_GIT_TIMEOUT)?; ensure_git_success(&output, context) } @@ -277,15 +295,9 @@ fn run_git_in_repo( args: &[&str], context: &str, ) -> Result<(), String> { - let output = run_git_command_with_timeout( - Command::new(git_binary) - .env("GIT_OPTIONAL_LOCKS", "0") - .arg("-C") - .arg(repo_path) - .args(args), - context, - CURATED_PLUGINS_GIT_TIMEOUT, - )?; + let mut command = git_command(git_binary); + command.arg("-C").arg(repo_path).args(args); + let output = run_git_command_with_timeout(&mut command, context, CURATED_PLUGINS_GIT_TIMEOUT)?; ensure_git_success(&output, context) } @@ -583,12 +595,13 @@ fn read_local_git_or_sha_file( } fn git_ls_remote_head_sha(git_binary: &str) -> Result { + let mut command = git_command(git_binary); + command + .arg("ls-remote") + .arg("https://github.com/openai/plugins.git") + .arg("HEAD"); let output = run_git_command_with_timeout( - Command::new(git_binary) - .env("GIT_OPTIONAL_LOCKS", "0") - .arg("ls-remote") - .arg("https://github.com/openai/plugins.git") - .arg("HEAD"), + &mut command, "git ls-remote curated plugins repo", CURATED_PLUGINS_GIT_TIMEOUT, )?; @@ -610,8 +623,7 @@ fn git_ls_remote_head_sha(git_binary: &str) -> Result { } fn git_head_sha(repo_path: &Path, git_binary: &str) -> Result { - let output = Command::new(git_binary) - .env("GIT_OPTIONAL_LOCKS", "0") + let output = git_command(git_binary) .arg("-C") .arg(repo_path) .arg("rev-parse") @@ -635,6 +647,15 @@ fn git_head_sha(repo_path: &Path, git_binary: &str) -> Result { Ok(sha) } +fn git_command(git_binary: &str) -> Command { + let mut command = Command::new(git_binary); + command.env("GIT_OPTIONAL_LOCKS", "0"); + for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES { + command.env_remove(name); + } + command +} + fn run_git_command_with_timeout( command: &mut Command, context: &str, diff --git a/codex-rs/core-plugins/src/startup_sync_tests.rs b/codex-rs/core-plugins/src/startup_sync_tests.rs index f73e4ad9d..fcedf3084 100644 --- a/codex-rs/core-plugins/src/startup_sync_tests.rs +++ b/codex-rs/core-plugins/src/startup_sync_tests.rs @@ -1,5 +1,6 @@ use super::*; use pretty_assertions::assert_eq; +use std::ffi::OsStr; use std::io::Write; use std::path::Path; use std::path::PathBuf; @@ -16,6 +17,22 @@ use zip::write::SimpleFileOptions; const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; +#[test] +fn git_command_sanitizes_ambient_repository_environment() { + let command = git_command("git"); + + for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES { + assert_eq!( + command + .get_envs() + .find(|(key, _)| *key == OsStr::new(name)) + .map(|(_, value)| value), + Some(None), + "{name} should be removed from startup sync Git commands" + ); + } +} + fn write_file(path: &Path, contents: &str) { std::fs::create_dir_all(path.parent().expect("file should have a parent")).unwrap(); std::fs::write(path, contents).unwrap();