Reject implicitly discovered bare Git repositories (#36924)

## Why

A repository can contain a tracked directory that Git implicitly treats as a bare
repository. Its configuration may select helpers such as `core.fsmonitor`, causing
Codex Git operations in that directory to execute repository-controlled code.

## What changed

- Pass `-c safe.bareRepository=explicit` to Codex-managed Git commands so they
  reject implicitly discovered bare repositories.
- Continue to support repositories explicitly selected with `--git-dir` or
  `GIT_DIR`.

## Testing

Add a regression test that clones a repository containing a tracked embedded Git
repository and verifies that guarded Git inspection rejects it without running
its configured filesystem monitor.

GitOrigin-RevId: 344b5bc1e0ffa94f2a1b788488aa653222da10f3
This commit is contained in:
Jeremy Rose
2026-08-04 16:53:33 +00:00
committed by copyberry
parent 02bc1dd796
commit 7ada37a15e
13 changed files with 226 additions and 15 deletions
+1
View File
@@ -194,6 +194,7 @@ async fn git_output(git_path: &Path, cwd: &Path, args: &[&str]) -> Option<String
let mut command = Command::new(git_path);
command
.env("GIT_OPTIONAL_LOCKS", "0")
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(args)
.current_dir(cwd)
.stdin(Stdio::null())
+2
View File
@@ -216,6 +216,7 @@ impl EnvironmentHttp for RouteAwareClientPool {
fn get_git_origins() -> Vec<String> {
// Prefer: git config --get-regexp remote\..*\.url
let out = std::process::Command::new("git")
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(["config", "--get-regexp", "remote\\..*\\.url"])
.output();
if let Ok(ok) = out
@@ -234,6 +235,7 @@ fn get_git_origins() -> Vec<String> {
}
// Fallback: git remote -v
let out = std::process::Command::new("git")
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(["remote", "-v"])
.output();
if let Ok(ok) = out
+3 -1
View File
@@ -1675,7 +1675,9 @@ fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), String> {
fn run_git_output(args: &[&str], cwd: Option<&Path>) -> Result<String, String> {
let mut command = Command::new("git");
command.args(args);
command
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(args);
command.env("GIT_TERMINAL_PROMPT", "0");
if let Some(cwd) = cwd {
command.current_dir(cwd);
@@ -112,7 +112,9 @@ pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf {
fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), MarketplaceAddError> {
let mut command = Command::new("git");
command.args(args);
command
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(args);
command.env("GIT_TERMINAL_PROMPT", "0");
if let Some(cwd) = cwd {
command.current_dir(cwd);
@@ -136,6 +136,7 @@ fn is_full_git_sha(value: &str) -> bool {
fn git_command() -> Command {
let mut command = Command::new("git");
command
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.env("GIT_OPTIONAL_LOCKS", "0")
.env("GIT_TERMINAL_PROMPT", "0");
command
@@ -241,6 +242,13 @@ mod tests {
let command = git_command();
assert_eq!(command.get_program(), OsStr::new("git"));
assert_eq!(
command.get_args().collect::<Vec<_>>(),
[
OsStr::new("-c"),
OsStr::new(codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG),
]
);
assert_eq!(
command_env(&command, "GIT_OPTIONAL_LOCKS"),
Some(Some(OsStr::new("0")))
+3 -1
View File
@@ -684,7 +684,9 @@ fn git_head_sha(repo_path: &Path, git_binary: &Path) -> Result<String, String> {
fn git_command(git_binary: &Path) -> Command {
let mut command = Command::new(git_binary);
command.env("GIT_OPTIONAL_LOCKS", "0");
command
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.env("GIT_OPTIONAL_LOCKS", "0");
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES {
command.env_remove(name);
}
+156 -4
View File
@@ -94,6 +94,14 @@ async fn backup_archive_routes_metadata_and_backend_supplied_download_urls() {
fn git_command_sanitizes_ambient_repository_environment() {
let command = git_command(Path::new("git"));
assert_eq!(
command.get_args().collect::<Vec<_>>(),
[
OsStr::new("-c"),
OsStr::new(codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG),
]
);
for name in REPOSITORY_LOCAL_GIT_ENVIRONMENT_VARIABLES {
assert_eq!(
command
@@ -106,6 +114,147 @@ fn git_command_sanitizes_ambient_repository_environment() {
}
}
#[tokio::test]
async fn ordinary_clone_rejects_tracked_embedded_bare_repository() {
let temp_dir = tempdir().expect("create temporary directory");
let source = temp_dir.path().join("source");
let clone = temp_dir.path().join("clone");
let nested_source = source.join("nested");
std::fs::create_dir_all(nested_source.join("objects")).expect("create nested object directory");
std::fs::create_dir_all(nested_source.join("refs"))
.expect("create nested references directory");
let run_setup_git = |cwd: &Path, args: &[&str]| {
let output = Command::new("git")
.args(args)
.current_dir(cwd)
.output()
.expect("run repository setup Git command");
assert!(
output.status.success(),
"git {args:?} failed: {}",
String::from_utf8_lossy(&output.stderr)
);
};
run_setup_git(&source, &["init", "--quiet"]);
std::fs::write(nested_source.join("HEAD"), "ref: refs/heads/main\n")
.expect("write tracked nested HEAD");
std::fs::write(
nested_source.join("config"),
"[core]\n\trepositoryformatversion = 0\n\tbare = false\n\tworktree = .\n\tfsmonitor = ./payload.sh\n",
)
.expect("write tracked nested Git configuration");
std::fs::write(nested_source.join("objects/.keep"), "").expect("track nested object directory");
std::fs::write(nested_source.join("refs/.keep"), "")
.expect("track nested references directory");
std::fs::write(
nested_source.join("payload.sh"),
"#!/bin/sh\nprintf ran > \"$0.ran\"\n",
)
.expect("write tracked filesystem monitor");
run_setup_git(&source, &["add", "--all"]);
run_setup_git(&source, &["add", "--chmod=+x", "nested/payload.sh"]);
run_setup_git(
&source,
&[
"-c",
"user.name=Codex Tests",
"-c",
"user.email=codex-tests@example.com",
"commit",
"--quiet",
"-m",
"track embedded Git repository",
],
);
let clone_output = Command::new("git")
.arg("clone")
.arg(&source)
.arg(&clone)
.output()
.expect("clone repository normally");
assert!(
clone_output.status.success(),
"ordinary git clone failed: {}",
String::from_utf8_lossy(&clone_output.stderr)
);
let nested = clone.join("nested");
let marker = nested.join("payload.sh.ran");
let vulnerable = Command::new("git")
.args(["status", "--short"])
.current_dir(&nested)
.output()
.expect("run unguarded Git against tracked embedded repository");
assert!(
vulnerable.status.success(),
"unguarded Git should discover the tracked embedded repository: {}",
String::from_utf8_lossy(&vulnerable.stderr)
);
assert!(
marker.exists(),
"unguarded Git should execute the tracked helper"
);
std::fs::remove_file(&marker).expect("remove unguarded execution marker");
let guarded = git_command(Path::new("git"))
.args(["status", "--short"])
.current_dir(&nested)
.output()
.expect("run guarded startup Git against tracked embedded repository");
assert!(
!guarded.status.success(),
"startup Git should reject the repository"
);
assert!(
!marker.exists(),
"startup Git must reject the repository before executing its helper"
);
let apply_error = codex_git_utils::apply_git_patch(&codex_git_utils::ApplyGitRequest {
cwd: nested.clone(),
diff: String::new(),
revert: false,
preflight: true,
})
.expect_err("patch root discovery should reject the tracked embedded repository");
assert!(apply_error.to_string().contains("not a git repository"));
assert!(codex_git_utils::collect_git_info(&nested).await.is_none());
assert!(codex_git_utils::git_diff_to_remote(&nested).await.is_none());
assert!(
!marker.exists(),
"Rust-owned Git inspection must not execute the tracked helper"
);
let explicit_git_dir = git_command(Path::new("git"))
.arg("--git-dir")
.arg(&nested)
.args(["rev-parse", "--git-dir"])
.current_dir(&clone)
.output()
.expect("run Git with an explicitly selected bare repository");
assert!(
explicit_git_dir.status.success(),
"--git-dir must continue to permit an explicitly selected repository: {}",
String::from_utf8_lossy(&explicit_git_dir.stderr)
);
let explicit_environment = Command::new("git")
.args(["-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG])
.args(["rev-parse", "--git-dir"])
.env("GIT_DIR", &nested)
.current_dir(&clone)
.output()
.expect("run Git with explicitly selected GIT_DIR");
assert!(
explicit_environment.status.success(),
"GIT_DIR must continue to permit an explicitly selected repository: {}",
String::from_utf8_lossy(&explicit_environment.stderr)
);
}
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();
@@ -411,6 +560,7 @@ fn concurrent_syncs_serialize_fetches_without_skipping_remote_checks() {
&git_path,
&format!(
r#"#!/bin/sh
if [ "$1" = "-c" ] && [ "$2" = "safe.bareRepository=explicit" ]; then shift 2; fi
printf '%s\n' "$*" >> '{}'
if [ "$1" = "ls-remote" ]; then
sleep 1
@@ -652,7 +802,7 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
.collect::<Vec<_>>();
let curated_repo_path = curated_plugins_repo_path(tmp.path());
assert!(incremental_sync_invocations.iter().any(|invocation| {
invocation.starts_with(&format!("-C {} fetch ", curated_repo_path.display()))
invocation.contains(&format!(" -C {} fetch ", curated_repo_path.display()))
&& invocation.contains(" https://github.com/openai/plugins.git ")
&& invocation.contains(updated_sha.as_str())
&& invocation.ends_with(CURATED_PLUGINS_FETCH_REF)
@@ -675,8 +825,8 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
.any(|invocation| invocation.split_whitespace().any(|arg| arg == "clone"))
);
assert!(!incremental_sync_invocations.iter().any(|invocation| {
invocation.starts_with(&format!("-C {} reset ", curated_repo_path.display()))
|| invocation.starts_with(&format!("-C {} clean ", curated_repo_path.display()))
invocation.contains(&format!(" -C {} reset ", curated_repo_path.display()))
|| invocation.contains(&format!(" -C {} clean ", curated_repo_path.display()))
}));
assert!(!has_plugins_clone_dirs(tmp.path()));
@@ -693,7 +843,7 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
assert!(
unchanged_sync_invocations
.iter()
.any(|invocation| invocation.starts_with("ls-remote "))
.any(|invocation| invocation.contains(" ls-remote "))
);
assert!(
!unchanged_sync_invocations
@@ -825,6 +975,7 @@ fn sync_openai_plugins_repo_via_git_cleans_up_staged_dir_on_fetch_failure() {
&git_path,
&format!(
r#"#!/bin/sh
if [ "$1" = "-c" ] && [ "$2" = "safe.bareRepository=explicit" ]; then shift 2; fi
if [ "$1" = "ls-remote" ]; then
printf '%s\tHEAD\n' "{sha}"
exit 0
@@ -870,6 +1021,7 @@ fn sync_openai_plugins_repo_via_git_preserves_existing_snapshot_on_validation_fa
&git_path,
&format!(
r#"#!/bin/sh
if [ "$1" = "-c" ] && [ "$2" = "safe.bareRepository=explicit" ]; then shift 2; fi
if [ "$1" = "ls-remote" ]; then
printf '%s\tHEAD\n' "{remote_sha}"
exit 0
+3
View File
@@ -125,6 +125,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
fn resolve_git_root(cwd: &Path) -> io::Result<PathBuf> {
let out = std::process::Command::new("git")
.args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG])
.arg("rev-parse")
.arg("--show-toplevel")
.current_dir(cwd)
@@ -153,6 +154,7 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32,
for p in git_cfg {
cmd.arg(p);
}
cmd.args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG]);
for a in args {
cmd.arg(a);
}
@@ -330,6 +332,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
return Ok(());
}
let mut cmd = std::process::Command::new("git");
cmd.args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG]);
cmd.arg("add");
cmd.arg("--");
for p in &existing {
+7 -1
View File
@@ -400,7 +400,10 @@ impl crate::FsmonitorProbeRunner for LocalFsmonitorProbeRunner<'_> {
// Both probes are fast, bounded metadata queries that do not inspect the
// worktree or index, so do not reduce the requested command's timeout.
let mut command = Command::new(self.git);
command.args(args).current_dir(self.cwd);
command
.args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG])
.args(args)
.current_dir(self.cwd);
match run_git_command_with_timeout_output(&mut command, GIT_COMMAND_TIMEOUT).await {
Some(output) if output.status.success() => Some(output.stdout),
_ => None,
@@ -422,6 +425,7 @@ async fn run_git_command_with_timeout_from(
let mut command = Command::new(git);
command
.env("GIT_OPTIONAL_LOCKS", "0")
.args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG])
// Keep internal Git commands independent of repository-selected hooks
// and fsmonitor helpers while preserving built-in fsmonitor acceleration.
.args(["-c", &format!("core.hooksPath={DISABLED_HOOKS_PATH}")])
@@ -1032,6 +1036,7 @@ mod tests {
std::fs::write(
&git,
"#!/bin/sh\n\
if [ \"$1\" = \"-c\" ] && [ \"$2\" = \"safe.bareRepository=explicit\" ]; then shift 2; fi\n\
printf '%s\\n' \"$*\" >>\"$0.log\"\n\
case \"$1\" in\n\
config) printf '/tmp/fsmonitor-helper\\000' ;;\n\
@@ -1097,6 +1102,7 @@ mod tests {
std::fs::write(
&git,
"#!/bin/sh\n\
if [ \"$1\" = \"-c\" ] && [ \"$2\" = \"safe.bareRepository=explicit\" ]; then shift 2; fi\n\
printf '%s\\n' \"$*\" >>\"$0.log\"\n\
case \"$1\" in\n\
config)\n\
+4
View File
@@ -8,6 +8,10 @@ mod info;
mod operations;
mod platform;
/// Git configuration that rejects implicitly discovered bare repositories while
/// preserving repositories selected explicitly through `GIT_DIR` or `--git-dir`.
pub const SAFE_BARE_REPOSITORY_CONFIG: &str = "safe.bareRepository=explicit";
pub use apply::ApplyGitRequest;
pub use apply::ApplyGitResult;
pub use apply::apply_git_patch;
+3 -1
View File
@@ -100,7 +100,9 @@ where
{
let iterator = args.into_iter();
let (lower, upper) = iterator.size_hint();
let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower) + 2);
let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower) + 4);
args_vec.push(OsString::from("-c"));
args_vec.push(OsString::from(crate::SAFE_BARE_REPOSITORY_CONFIG));
// Keep internal Git helper commands independent of configured hook directories.
args_vec.push(OsString::from("-c"));
args_vec.push(OsString::from(format!(
+24 -3
View File
@@ -474,8 +474,10 @@ async fn run_git_command(
cwd: &Path,
args: &[&str],
) -> Result<WorkspaceCommandOutput, crate::workspace_command::WorkspaceCommandError> {
let mut argv = Vec::with_capacity(args.len() + 1);
let mut argv = Vec::with_capacity(args.len() + 3);
argv.push("git".to_string());
argv.push("-c".to_string());
argv.push(codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG.to_string());
argv.extend(args.iter().map(|arg| (*arg).to_string()));
runner
.run(
@@ -672,8 +674,18 @@ mod tests {
}
fn response(argv: &[&str], exit_code: i32, stdout: &str) -> FakeResponse {
let mut argv: Vec<String> = argv.iter().map(|arg| (*arg).to_string()).collect();
if argv.first().map(String::as_str) == Some("git") {
argv.splice(
1..1,
[
"-c".to_string(),
codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG.to_string(),
],
);
}
FakeResponse {
argv: argv.iter().map(|arg| (*arg).to_string()).collect(),
argv,
output: WorkspaceCommandOutput {
exit_code,
stdout: stdout.to_string(),
@@ -701,7 +713,16 @@ mod tests {
}
fn saw(&self, argv: &[&str]) -> bool {
let argv: Vec<String> = argv.iter().map(|arg| (*arg).to_string()).collect();
let mut argv: Vec<String> = argv.iter().map(|arg| (*arg).to_string()).collect();
if argv.first().map(String::as_str) == Some("git") {
argv.splice(
1..1,
[
"-c".to_string(),
codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG.to_string(),
],
);
}
self.seen
.lock()
.expect("seen lock")
+9 -3
View File
@@ -33,7 +33,9 @@ struct WorkspaceFsmonitorProbeRunner<'a> {
impl FsmonitorProbeRunner for WorkspaceFsmonitorProbeRunner<'_> {
async fn run_probe(&mut self, args: &[&str]) -> Option<Vec<u8>> {
let argv = ["git"].into_iter().chain(args.iter().copied());
let argv = ["git", "-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG]
.into_iter()
.chain(args.iter().copied());
let command = WorkspaceCommand::new(argv).cwd(self.cwd.to_path_buf());
match self.runner.run(command).await {
Ok(output) if output.success() => Some(output.stdout.into_bytes()),
@@ -232,6 +234,8 @@ async fn run_git_command(
let argv = [
"git",
"-c",
codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG,
"-c",
fsmonitor.git_config_arg(),
"-c",
DISABLE_HOOKS_CONFIG,
@@ -745,6 +749,8 @@ mod tests {
[
"git",
"-c",
codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG,
"-c",
fsmonitor.git_config_arg(),
"-c",
DISABLE_HOOKS_CONFIG,
@@ -756,7 +762,7 @@ mod tests {
}
fn git_probe_command(args: &[&str]) -> Vec<String> {
["git"]
["git", "-c", codex_git_utils::SAFE_BARE_REPOSITORY_CONFIG]
.into_iter()
.chain(args.iter().copied())
.map(str::to_string)
@@ -830,7 +836,7 @@ mod tests {
for command in commands {
assert_eq!(command.cwd.as_deref(), Some(cwd));
if matches!(
command.argv.get(1).map(String::as_str),
command.argv.get(3).map(String::as_str),
Some("config" | "version")
) {
assert_eq!(command.env, HashMap::new());