clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-29 15:30:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

[codex] Fix Windows sandbox git safe.directory for worktrees (#21409)

Browse Source 
## Why

Windows sandboxed commands run as a sandbox user, while workspace
repositories are usually owned by the real user. The sandbox compensates
by injecting a temporary Git `safe.directory` entry into the child
environment.

That injection was still broken for linked worktrees because the helper
followed the `.git` file's `gitdir:` pointer and injected the internal
`.git/worktrees/...` location. Git's dubious-ownership check expects the
worktree root instead, so sandboxed Git commands still failed in
worktree-based Codex checkouts.

## What changed

- Treat any `.git` marker, directory or file, as the worktree root for
`safe.directory` injection.
- Keep the safe-directory logic in
`windows-sandbox-rs/src/sandbox_utils.rs` and have the one-shot elevated
path reuse it.
- Add regression coverage for both normal `.git` directories and
gitfile-based worktrees.

## Validation

- `cargo test -p codex-windows-sandbox sandbox_utils::tests`
- `cargo test -p codex-windows-sandbox` built and ran; the new
`sandbox_utils` tests passed, while two pre-existing legacy sandbox
tests failed locally with `Access is denied`:
`session::tests::legacy_non_tty_cmd_emits_output` and
`spawn_prep::tests::legacy_spawn_env_applies_offline_network_rewrite`.
This commit is contained in:
iceweasel-oai
2026-05-06 14:08:45 -07:00
committed by GitHub
parent fbdbc6b2fe
commit 123e78b97b
2 changed files with 72 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
codex-rs/windows-sandbox-rs/src/elevated_impl.rs
View File

@@ -37,72 +37,11 @@ mod windows_impl {
use crate::policy::SandboxPolicy;
use crate::policy::parse_policy;
use crate::runner_client::spawn_runner_transport;
use crate::sandbox_utils::ensure_codex_home_exists;
use crate::sandbox_utils::inject_git_safe_directory;
use crate::token::convert_string_sid_to_sid;
use anyhow::Result;
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
/// Ensures the parent directory of a path exists before writing to it.
/// Walks upward from `start` to locate the git worktree root, following gitfile redirects.
fn find_git_root(start: &Path) -> Option<PathBuf> {
let mut cur = dunce::canonicalize(start).ok()?;
loop {
let marker = cur.join(".git");
if marker.is_dir() {
return Some(cur);
}
if marker.is_file() {
if let Ok(txt) = std::fs::read_to_string(&marker)
&& let Some(rest) = txt.trim().strip_prefix("gitdir:")
{
let gitdir = rest.trim();
let resolved = if Path::new(gitdir).is_absolute() {
PathBuf::from(gitdir)
} else {
cur.join(gitdir)
};
return resolved.parent().map(Path::to_path_buf).or(Some(cur));
}
return Some(cur);
}
let parent = cur.parent()?;
if parent == cur {
return None;
}
cur = parent.to_path_buf();
}
}
/// Creates the sandbox user's Codex home directory if it does not already exist.
fn ensure_codex_home_exists(p: &Path) -> Result<()> {
std::fs::create_dir_all(p)?;
Ok(())
}
/// Adds a git safe.directory entry to the environment when running inside a repository.
/// git will not otherwise allow the Sandbox user to run git commands on the repo directory
/// which is owned by the primary user.
fn inject_git_safe_directory(
env_map: &mut HashMap<String, String>,
cwd: &Path,
_logs_base_dir: Option<&Path>,
) {
if let Some(git_root) = find_git_root(cwd) {
let mut cfg_count: usize = env_map
.get("GIT_CONFIG_COUNT")
.and_then(|v| v.parse::<usize>().ok())
.unwrap_or(0);
let git_path = git_root.to_string_lossy().replace("\\\\", "/");
env_map.insert(
format!("GIT_CONFIG_KEY_{cfg_count}"),
"safe.directory".to_string(),
);
env_map.insert(format!("GIT_CONFIG_VALUE_{cfg_count}"), git_path);
cfg_count += 1;
env_map.insert("GIT_CONFIG_COUNT".to_string(), cfg_count.to_string());
}
}
pub use crate::windows_impl::CaptureResult;
@@ -130,7 +69,7 @@ mod windows_impl {
normalize_null_device_env(&mut env_map);
ensure_non_interactive_pager(&mut env_map);
inherit_path_env(&mut env_map);
inject_git_safe_directory(&mut env_map, cwd, None);
inject_git_safe_directory(&mut env_map, cwd);
// Use a temp-based log dir that the sandbox user can write.
let sandbox_base = codex_home.join(".sandbox");
ensure_codex_home_exists(&sandbox_base)?;

89
codex-rs/windows-sandbox-rs/src/sandbox_utils.rs
View File

@@ -8,28 +8,12 @@
use anyhow::Result;
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
/// Walk upward from `start` to locate the git worktree root (supports gitfile redirects).
fn find_git_root(start: &Path) -> Option<PathBuf> {
/// Walk upward from `start` to locate the git worktree root for `safe.directory`.
fn find_git_worktree_root_for_safe_directory(start: &Path) -> Option<std::path::PathBuf> {
let mut cur = dunce::canonicalize(start).ok()?;
loop {
let marker = cur.join(".git");
if marker.is_dir() {
return Some(cur);
}
if marker.is_file() {
if let Ok(txt) = std::fs::read_to_string(&marker)
&& let Some(rest) = txt.trim().strip_prefix("gitdir:")
{
let gitdir = rest.trim();
let resolved = if Path::new(gitdir).is_absolute() {
PathBuf::from(gitdir)
} else {
cur.join(gitdir)
};
return resolved.parent().map(Path::to_path_buf).or(Some(cur));
}
if cur.join(".git").exists() {
return Some(cur);
}
let parent = cur.parent()?;
@@ -50,7 +34,7 @@ pub fn ensure_codex_home_exists(p: &Path) -> Result<()> {
/// git will not otherwise allow the Sandbox user to run git commands on the repo directory
/// which is owned by the primary user.
pub fn inject_git_safe_directory(env_map: &mut HashMap<String, String>, cwd: &Path) {
if let Some(git_root) = find_git_root(cwd) {
if let Some(git_root) = find_git_worktree_root_for_safe_directory(cwd) {
let mut cfg_count: usize = env_map
.get("GIT_CONFIG_COUNT")
.and_then(|v| v.parse::<usize>().ok())
@@ -65,3 +49,68 @@ pub fn inject_git_safe_directory(env_map: &mut HashMap<String, String>, cwd: &Pa
env_map.insert("GIT_CONFIG_COUNT".to_string(), cfg_count.to_string());
}
}
#[cfg(test)]
mod tests {
use super::inject_git_safe_directory;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::fs;
use std::path::Path;
use tempfile::TempDir;
fn safe_directory_value(path: &Path) -> String {
dunce::canonicalize(path)
.expect("canonicalize path")
.to_string_lossy()
.replace("\\\\", "/")
}
#[test]
fn injects_safe_directory_for_git_directory() {
let temp = TempDir::new().expect("tempdir");
let repo = temp.path().join("repo");
let nested = repo.join("nested");
fs::create_dir_all(repo.join(".git")).expect("create .git");
fs::create_dir_all(&nested).expect("create nested dir");
let mut env_map = HashMap::new();
inject_git_safe_directory(&mut env_map, &nested);
let expected = HashMap::from([
("GIT_CONFIG_COUNT".to_string(), "1".to_string()),
("GIT_CONFIG_KEY_0".to_string(), "safe.directory".to_string()),
(
"GIT_CONFIG_VALUE_0".to_string(),
safe_directory_value(&repo),
),
]);
assert_eq!(env_map, expected);
}
#[test]
fn injects_worktree_root_for_gitfile() {
let temp = TempDir::new().expect("tempdir");
let repo = temp.path().join("repo");
let nested = repo.join("nested");
fs::create_dir_all(&nested).expect("create nested dir");
fs::write(
repo.join(".git"),
"gitdir: C:/Users/example/repo/.git/worktrees/codex3\n",
)
.expect("write .git file");
let mut env_map = HashMap::new();
inject_git_safe_directory(&mut env_map, &nested);
let expected = HashMap::from([
("GIT_CONFIG_COUNT".to_string(), "1".to_string()),
("GIT_CONFIG_KEY_0".to_string(), "safe.directory".to_string()),
(
"GIT_CONFIG_VALUE_0".to_string(),
safe_directory_value(&repo),
),
]);
assert_eq!(env_map, expected);
}
}