openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 17:28:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Harden marketplace Git environment isolation

Browse Source
This commit is contained in:
Chris Bookholt
2026-06-29 15:26:21 -07:00
parent ad2da8c522
commit 798210c6f7
3 changed files with 221 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core-plugins/src/git_transport.rs
+210 -43
View File
@@ -1,6 +1,44 @@
use std::process::Command;
use tempfile::TempDir;
/// Environment variables that can redirect Git away from the repository
/// selected by `current_dir` or `-C`, or inject command-scoped configuration.
///
/// This follows `git rev-parse --local-env-vars` and also clears discovery and
/// namespace controls that affect repository selection. Indexed
/// `GIT_CONFIG_KEY_*` / `GIT_CONFIG_VALUE_*` entries are inert once both
/// command-scope entry points are removed.
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",
];
/// Make an internal Git command honor its explicitly selected repository
/// instead of ambient repository state inherited by the Codex process.
///
/// User-global and system configuration, authentication helpers, and transport
/// configuration remain available.
pub(crate) fn sanitize_repository_environment(command: &mut Command) {
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES {
command.env_remove(name);
}
}
/// 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.
@@ -16,10 +54,9 @@ impl NeutralGitCwd {
}
pub(crate) fn configure(&self, command: &mut Command) {
sanitize_repository_environment(command);
command
.current_dir(self.directory.path())
.env_remove("GIT_DIR")
.env_remove("GIT_WORK_TREE")
.env("GIT_CEILING_DIRECTORIES", self.directory.path());
}
}
@@ -39,6 +76,7 @@ fn initialize_empty_repository(root: &std::path::Path) -> std::io::Result<()> {
#[cfg(test)]
mod tests {
use super::NeutralGitCwd;
use super::REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES;
#[cfg(unix)]
use super::initialize_empty_repository;
use pretty_assertions::assert_eq;
@@ -47,12 +85,19 @@ mod tests {
use std::fs;
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
#[cfg(unix)]
use std::path::Path;
#[cfg(unix)]
use std::path::PathBuf;
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");
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES {
command.env(name, "hostile");
}
neutral_cwd.configure(&mut command);
assert_eq!(
@@ -66,27 +111,166 @@ mod tests {
.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)
);
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES
.iter()
.filter(|name| **name != "GIT_CEILING_DIRECTORIES")
{
assert_eq!(
command
.get_envs()
.find(|(key, _)| *key == OsStr::new(name))
.map(|(_, value)| value),
Some(None),
"{name} should be removed"
);
}
}
#[test]
fn preserves_trusted_global_system_and_auth_environment() {
let neutral_cwd = NeutralGitCwd::new().expect("create neutral Git working directory");
let trusted = [
("GIT_CONFIG_GLOBAL", "/trusted/global.gitconfig"),
("GIT_CONFIG_SYSTEM", "/trusted/system.gitconfig"),
("GIT_SSH_COMMAND", "/trusted/ssh-wrapper"),
("HOME", "/trusted/home"),
("XDG_CONFIG_HOME", "/trusted/config"),
];
let mut command = Command::new("git");
for (name, value) in trusted {
command.env(name, value);
}
neutral_cwd.configure(&mut command);
for (name, value) in trusted {
assert_eq!(
command
.get_envs()
.find(|(key, _)| *key == OsStr::new(name))
.and_then(|(_, value)| value),
Some(OsStr::new(value)),
"{name} should be preserved"
);
}
}
#[cfg(unix)]
#[test]
fn nested_neutral_directory_does_not_run_parent_repository_transport_helper() {
let root = tempfile::tempdir().expect("create test root");
let source = root.path().join("source");
let fixture = create_transport_fixture(root.path());
let directory = tempfile::Builder::new()
.prefix("neutral-")
.tempdir_in(&fixture.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",
fixture.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 transport rewrite: {}",
String::from_utf8_lossy(&output.stderr)
);
assert!(!output.stdout.is_empty());
assert!(
!fixture.marker.exists(),
"repository-selected transport helper must not run"
);
}
#[cfg(unix)]
#[test]
fn neutral_directory_ignores_inherited_git_common_dir() {
let root = tempfile::tempdir().expect("create test root");
let fixture = create_transport_fixture(root.path());
let neutral_cwd = NeutralGitCwd::new().expect("create neutral Git working directory");
let mut command = Command::new("git");
command.env("GIT_COMMON_DIR", fixture.hostile_repo.join(".git"));
neutral_cwd.configure(&mut command);
let output = command
.args([
"ls-remote",
fixture.source.to_string_lossy().as_ref(),
"HEAD",
])
.output()
.expect("run git ls-remote");
assert!(
output.status.success(),
"ls-remote should ignore inherited GIT_COMMON_DIR: {}",
String::from_utf8_lossy(&output.stderr)
);
assert!(!output.stdout.is_empty());
assert!(
!fixture.marker.exists(),
"inherited common-dir transport helper must not run"
);
}
#[cfg(unix)]
#[test]
fn neutral_directory_ignores_inherited_command_scope_config() {
let root = tempfile::tempdir().expect("create test root");
let fixture = create_transport_fixture(root.path());
let neutral_cwd = NeutralGitCwd::new().expect("create neutral Git working directory");
let rewrite_key = format!("url.ext::{}.insteadOf", fixture.helper.display());
let mut command = Command::new("git");
command
.env("GIT_CONFIG_COUNT", "2")
.env("GIT_CONFIG_KEY_0", "protocol.ext.allow")
.env("GIT_CONFIG_VALUE_0", "always")
.env("GIT_CONFIG_KEY_1", rewrite_key)
.env(
"GIT_CONFIG_VALUE_1",
fixture.source.to_string_lossy().as_ref(),
);
neutral_cwd.configure(&mut command);
let output = command
.args([
"ls-remote",
fixture.source.to_string_lossy().as_ref(),
"HEAD",
])
.output()
.expect("run git ls-remote");
assert!(
output.status.success(),
"ls-remote should ignore inherited command-scope config: {}",
String::from_utf8_lossy(&output.stderr)
);
assert!(!output.stdout.is_empty());
assert!(
!fixture.marker.exists(),
"command-scope transport helper must not run"
);
}
#[cfg(unix)]
struct TransportFixture {
source: PathBuf,
hostile_repo: PathBuf,
helper: PathBuf,
marker: PathBuf,
}
#[cfg(unix)]
fn create_transport_fixture(root: &Path) -> TransportFixture {
let source = root.join("source");
fs::create_dir(&source).expect("create source repository");
run_git(&source, &["init"]);
run_git(&source, &["config", "user.email", "codex-test@example.com"]);
@@ -95,11 +279,11 @@ mod tests {
run_git(&source, &["add", "README.md"]);
run_git(&source, &["commit", "-m", "initial"]);
let hostile_repo = root.path().join("hostile");
let hostile_repo = root.join("hostile");
fs::create_dir(&hostile_repo).expect("create hostile repository");
run_git(&hostile_repo, &["init"]);
let marker = root.path().join("transport-ran");
let helper = root.path().join("transport-helper.sh");
let marker = root.join("transport-ran");
let helper = root.join("transport-helper.sh");
fs::write(
&helper,
format!("#!/bin/sh\nprintf ran > \"{}\"\nexit 1\n", marker.display()),
@@ -117,29 +301,12 @@ mod tests {
&["config", &rewrite_key, 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 transport rewrite: {}",
String::from_utf8_lossy(&output.stderr)
);
assert!(!output.stdout.is_empty());
assert!(
!marker.exists(),
"repository-selected transport helper must not run"
);
TransportFixture {
source,
hostile_repo,
helper,
marker,
}
}
#[cfg(unix)]
codex-rs/core-plugins/src/startup_sync.rs
+2 -24
View File
@@ -1,4 +1,5 @@
use crate::git_transport::NeutralGitCwd;
use crate::git_transport::sanitize_repository_environment;
use std::fs::File;
use std::path::Path;
use std::path::PathBuf;
@@ -35,27 +36,6 @@ 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 {
@@ -654,9 +634,7 @@ fn git_head_sha(repo_path: &Path, git_binary: &str) -> Result<String, String> {
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);
}
sanitize_repository_environment(&mut command);
command
}
codex-rs/core-plugins/src/startup_sync_tests.rs
+9 -3
View File
@@ -20,15 +20,21 @@ 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 {
for name in [
"GIT_COMMON_DIR",
"GIT_CONFIG_COUNT",
"GIT_CONFIG_PARAMETERS",
"GIT_DIR",
"GIT_OBJECT_DIRECTORY",
"GIT_WORK_TREE",
] {
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"
"{name} should be removed from startup-sync Git commands"
);
}
}