fix(git): pass native paths to isolated apply

This commit is contained in:
Chris Bookholt
2026-07-03 17:19:26 -07:00
parent 653b79082c
commit 7da7bcc8cd
3 changed files with 467 additions and 16 deletions
+265 -16
View File
@@ -5,6 +5,9 @@ use std::path::PathBuf;
use std::process::Command;
use std::process::Stdio;
#[cfg(windows)]
use same_file::Handle;
use crate::errors::GitReadError;
use crate::git_config_environment::GitConfigEnvironmentSnapshot;
#[cfg(test)]
@@ -247,43 +250,83 @@ impl GitRunner {
isolate_git_command_environment(&mut command.inner);
scrub_repository_and_config_environment(&mut command.inner);
let git_dir = self.authority.active_git_dir().ok_or_else(|| {
let canonical_git_dir = self.authority.active_git_dir().ok_or_else(|| {
io::Error::new(
io::ErrorKind::PermissionDenied,
"active Git directory is unavailable for isolated three-way apply",
)
})?;
let common_dir = self.authority.active_common_dir().ok_or_else(|| {
let canonical_common_dir = self.authority.active_common_dir().ok_or_else(|| {
io::Error::new(
io::ErrorKind::PermissionDenied,
"active Git common directory is unavailable for isolated three-way apply",
)
})?;
let worktree = self.authority.active_worktree_root();
let canonical_worktree = self.authority.active_worktree_root();
// Repository authority retains canonical paths for identity and route
// validation. Git for Windows does not recognize the verbatim
// `\\?\` spellings returned by `std::fs::canonicalize` in these
// environment variables, so simplify only the child-facing spelling
// after proving that it names the same existing directory.
let git_dir = git_child_directory(canonical_git_dir, "active Git directory")?;
let common_dir = git_child_directory(canonical_common_dir, "active Git common directory")?;
let worktree = git_child_directory(canonical_worktree, "active Git worktree")?;
#[cfg(windows)]
let isolated_root_base = std::fs::canonicalize(isolated.root.path())?;
#[cfg(not(windows))]
let isolated_root_base = isolated.root.path().to_path_buf();
let isolated_root =
git_child_directory(&isolated_root_base, "owned isolated Git common directory")?;
let index_file = git_dir.join("index");
let object_directory = common_dir.join("objects");
let system_config = isolated_root.join("system.gitconfig");
let global_config = isolated_root.join("global.gitconfig");
let home = isolated_root.join("home");
let xdg_config_home = isolated_root.join("xdg");
command
.inner
.env("GIT_DIR", git_dir)
.env("GIT_COMMON_DIR", isolated.root.path())
.env("GIT_WORK_TREE", worktree)
.env("GIT_INDEX_FILE", git_dir.join("index"))
.env("GIT_OBJECT_DIRECTORY", common_dir.join("objects"))
.env("GIT_DIR", git_dir.as_path())
.env("GIT_COMMON_DIR", isolated_root.as_path())
.env("GIT_WORK_TREE", worktree.as_path())
.env("GIT_INDEX_FILE", index_file)
.env("GIT_OBJECT_DIRECTORY", object_directory)
.env("GIT_CONFIG_NOSYSTEM", "1")
.env("GIT_CONFIG_SYSTEM", isolated.system_config_path())
.env("GIT_CONFIG_GLOBAL", isolated.global_config_path())
.env("GIT_CONFIG_SYSTEM", system_config)
.env("GIT_CONFIG_GLOBAL", global_config)
.env("GIT_CONFIG_COUNT", "0")
.env("GIT_ATTR_NOSYSTEM", "1")
.env("GIT_NO_REPLACE_OBJECTS", "1")
.env("HOME", isolated.home_path())
.env("XDG_CONFIG_HOME", isolated.xdg_config_home());
.env("HOME", &home)
.env("XDG_CONFIG_HOME", &xdg_config_home);
#[cfg(windows)]
command
.inner
.env("APPDATA", isolated.home_path())
.env("PROGRAMDATA", isolated.home_path())
.env("USERPROFILE", isolated.home_path());
.env("APPDATA", &home)
.env("PROGRAMDATA", &home)
.env("USERPROFILE", &home);
command.inner.envs(crate::local_only_git_env());
harden_git_launch_environment(&mut command.inner, &self.safe_path);
command.inner.output()
self.output_after_isolated_child_validation(
&mut command.inner,
isolated,
[&git_dir, &common_dir, &worktree, &isolated_root],
)
}
fn output_after_isolated_child_validation(
&self,
command: &mut Command,
isolated: &IsolatedGitCommonDir,
child_directories: [&GitChildDirectory; 4],
) -> io::Result<std::process::Output> {
self.revalidate_active_repository_metadata()?;
isolated.validate()?;
let _child_directory_validations = child_directories
.into_iter()
.map(GitChildDirectory::revalidate)
.collect::<io::Result<Vec<_>>>()?;
command.output()
}
fn revalidate_active_repository_metadata(&self) -> io::Result<()> {
@@ -312,6 +355,212 @@ impl GitRunner {
}
}
struct GitChildDirectory {
child_path: PathBuf,
#[cfg(windows)]
canonical_path: PathBuf,
#[cfg(windows)]
canonical_identity: Handle,
#[cfg(windows)]
child_identity: Handle,
#[cfg(windows)]
description: &'static str,
}
struct GitChildDirectoryValidation {
#[cfg(windows)]
_canonical_identity: Handle,
#[cfg(windows)]
_child_identity: Handle,
}
impl GitChildDirectory {
fn as_path(&self) -> &Path {
&self.child_path
}
fn join(&self, path: impl AsRef<Path>) -> PathBuf {
self.child_path.join(path)
}
#[cfg(windows)]
fn new(
canonical_path: PathBuf,
child_path: PathBuf,
description: &'static str,
) -> io::Result<Self> {
let canonical_metadata = std::fs::metadata(&canonical_path)?;
let child_metadata = std::fs::metadata(&child_path)?;
if !canonical_metadata.is_dir() || !child_metadata.is_dir() {
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("{description} is not an existing directory"),
));
}
let canonical_identity = Handle::from_path(&canonical_path)?;
let child_identity = Handle::from_path(&child_path)?;
if canonical_identity != child_identity {
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("Git-compatible spelling changed the identity of {description}"),
));
}
Ok(Self {
child_path,
canonical_path,
canonical_identity,
child_identity,
description,
})
}
#[cfg(windows)]
fn revalidate(&self) -> io::Result<GitChildDirectoryValidation> {
let canonical_metadata = std::fs::metadata(&self.canonical_path)?;
let child_metadata = std::fs::metadata(&self.child_path)?;
if !canonical_metadata.is_dir() || !child_metadata.is_dir() {
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("{} is no longer an existing directory", self.description),
));
}
let canonical_identity = Handle::from_path(&self.canonical_path)?;
let child_identity = Handle::from_path(&self.child_path)?;
if canonical_identity != self.canonical_identity
|| child_identity != self.child_identity
|| canonical_identity != child_identity
{
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!(
"Git-compatible spelling no longer identifies the original {}",
self.description
),
));
}
Ok(GitChildDirectoryValidation {
_canonical_identity: canonical_identity,
_child_identity: child_identity,
})
}
#[cfg(not(windows))]
fn revalidate(&self) -> io::Result<GitChildDirectoryValidation> {
Ok(GitChildDirectoryValidation {})
}
}
#[cfg(windows)]
fn git_child_directory(path: &Path, description: &'static str) -> io::Result<GitChildDirectory> {
let child_path = git_compatible_windows_path(path, description)?;
GitChildDirectory::new(path.to_path_buf(), child_path, description)
}
#[cfg(not(windows))]
fn git_child_directory(path: &Path, _description: &'static str) -> io::Result<GitChildDirectory> {
Ok(GitChildDirectory {
child_path: path.to_path_buf(),
})
}
#[cfg(windows)]
fn git_compatible_windows_path(path: &Path, description: &'static str) -> io::Result<PathBuf> {
use std::ffi::OsString;
use std::os::windows::ffi::OsStrExt;
use std::os::windows::ffi::OsStringExt;
let units = path.as_os_str().encode_wide().collect::<Vec<_>>();
match classify_windows_git_path_units(&units) {
WindowsGitPathUnits::Unchanged => Ok(path.to_path_buf()),
WindowsGitPathUnits::Converted(simplified) => {
Ok(PathBuf::from(OsString::from_wide(&simplified)))
}
WindowsGitPathUnits::Rejected => Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!("refusing unsupported Windows namespace for {description}"),
)),
}
}
#[cfg(any(windows, test))]
#[derive(Debug, Eq, PartialEq)]
enum WindowsGitPathUnits {
Unchanged,
Converted(Vec<u16>),
Rejected,
}
#[cfg(any(windows, test))]
fn classify_windows_git_path_units(path: &[u16]) -> WindowsGitPathUnits {
const BACKSLASH: u16 = b'\\' as u16;
const VERBATIM_PREFIX: [u16; 4] = [BACKSLASH, BACKSLASH, b'?' as u16, BACKSLASH];
const DEVICE_PREFIX: [u16; 4] = [BACKSLASH, BACKSLASH, b'.' as u16, BACKSLASH];
const NT_PREFIX: [u16; 4] = [BACKSLASH, b'?' as u16, b'?' as u16, BACKSLASH];
const FORWARD_VERBATIM_PREFIX: [u16; 4] = [b'/' as u16, b'/' as u16, b'?' as u16, b'/' as u16];
const FORWARD_DEVICE_PREFIX: [u16; 4] = [b'/' as u16, b'/' as u16, b'.' as u16, b'/' as u16];
if path.contains(&0) {
return WindowsGitPathUnits::Rejected;
}
if path.starts_with(&DEVICE_PREFIX)
|| path.starts_with(&NT_PREFIX)
|| path.starts_with(&FORWARD_VERBATIM_PREFIX)
|| path.starts_with(&FORWARD_DEVICE_PREFIX)
{
return WindowsGitPathUnits::Rejected;
}
let Some(path) = path.strip_prefix(&VERBATIM_PREFIX) else {
return WindowsGitPathUnits::Unchanged;
};
if path.len() >= 3
&& ascii_u16_is_alphabetic(path[0])
&& path[1] == b':' as u16
&& path[2] == BACKSLASH
{
return WindowsGitPathUnits::Converted(path.to_vec());
}
if path.len() < 4
|| !ascii_u16_eq_ignore_case(path[0], b'U')
|| !ascii_u16_eq_ignore_case(path[1], b'N')
|| !ascii_u16_eq_ignore_case(path[2], b'C')
|| path[3] != BACKSLASH
{
return WindowsGitPathUnits::Rejected;
}
let unc_path = &path[4..];
let Some(server_end) = unc_path.iter().position(|unit| *unit == BACKSLASH) else {
return WindowsGitPathUnits::Rejected;
};
if server_end == 0 {
return WindowsGitPathUnits::Rejected;
}
let share = &unc_path[server_end + 1..];
let share_end = share
.iter()
.position(|unit| *unit == BACKSLASH)
.unwrap_or(share.len());
if share_end == 0 {
return WindowsGitPathUnits::Rejected;
}
let mut simplified = Vec::with_capacity(unc_path.len() + 2);
simplified.extend([BACKSLASH, BACKSLASH]);
simplified.extend_from_slice(unc_path);
WindowsGitPathUnits::Converted(simplified)
}
#[cfg(any(windows, test))]
fn ascii_u16_is_alphabetic(unit: u16) -> bool {
(b'A' as u16..=b'Z' as u16).contains(&unit) || (b'a' as u16..=b'z' as u16).contains(&unit)
}
#[cfg(any(windows, test))]
fn ascii_u16_eq_ignore_case(unit: u16, ascii: u8) -> bool {
unit == ascii.to_ascii_lowercase() as u16 || unit == ascii.to_ascii_uppercase() as u16
}
fn scrub_repository_and_config_environment(command: &mut Command) {
let mut names = std::env::vars_os()
.map(|(name, _)| name)
+149
View File
@@ -1356,6 +1356,155 @@ fn command_for_cwd_executes_from_a_canonical_windows_path() {
);
}
#[test]
fn windows_verbatim_git_path_conversion_is_narrow_and_non_lossy() {
fn units(value: &str) -> Vec<u16> {
value.encode_utf16().collect()
}
assert_eq!(
classify_windows_git_path_units(&units(r"\\?\C:\Users\alice\repo")),
WindowsGitPathUnits::Converted(units(r"C:\Users\alice\repo"))
);
assert_eq!(
classify_windows_git_path_units(&units(r"\\?\UNC\server\share\repo")),
WindowsGitPathUnits::Converted(units(r"\\server\share\repo"))
);
assert_eq!(
classify_windows_git_path_units(&units(r"\\?\unc\server\share")),
WindowsGitPathUnits::Converted(units(r"\\server\share"))
);
for unchanged in [r"C:\Users\alice\repo", r"\\server\share\repo"] {
assert_eq!(
classify_windows_git_path_units(&units(unchanged)),
WindowsGitPathUnits::Unchanged,
"unexpected conversion for {unchanged:?}"
);
}
for rejected in [
r"\\.\C:\device-path",
r"\??\C:\nt-path",
r"//?/C:/forward-verbatim",
r"//./C:/forward-device",
r"\\?\C:relative",
r"\\?\C:/forward-tail",
r"\\?\GLOBALROOT\Device\HarddiskVolume1",
r"\\?\Volume{12345678-1234-1234-1234-123456789abc}\repo",
r"\\?\UNC\\share",
r"\\?\UNC\server\",
] {
assert_eq!(
classify_windows_git_path_units(&units(rejected)),
WindowsGitPathUnits::Rejected,
"unexpected acceptance for {rejected:?}"
);
}
let mut unpaired_surrogate = units(r"\\?\C:\repo\");
unpaired_surrogate.push(0xD800);
let mut expected = units(r"C:\repo\");
expected.push(0xD800);
assert_eq!(
classify_windows_git_path_units(&unpaired_surrogate),
WindowsGitPathUnits::Converted(expected)
);
assert_eq!(
classify_windows_git_path_units(&units(r"\\?\C:\repo\😀")),
WindowsGitPathUnits::Converted(units(r"C:\repo\😀"))
);
let mut unpaired_low_surrogate = units(r"\\?\C:\repo\");
unpaired_low_surrogate.push(0xDC00);
let mut expected = units(r"C:\repo\");
expected.push(0xDC00);
assert_eq!(
classify_windows_git_path_units(&unpaired_low_surrogate),
WindowsGitPathUnits::Converted(expected)
);
let mut nul = units(r"\\?\C:\repo\");
nul.push(0);
assert_eq!(
classify_windows_git_path_units(&nul),
WindowsGitPathUnits::Rejected
);
}
#[cfg(windows)]
#[test]
fn git_child_directory_simplifies_canonical_path_without_changing_identity() {
use std::os::windows::ffi::OsStrExt;
let directory = tempfile::tempdir().expect("fixture");
let canonical = std::fs::canonicalize(directory.path()).expect("canonical fixture");
let child = git_child_directory(&canonical, "test directory").expect("child spelling");
assert_eq!(
same_file::Handle::from_path(&canonical).expect("canonical identity"),
same_file::Handle::from_path(child.as_path()).expect("child identity")
);
child.revalidate().expect("revalidate child spelling");
let child_units = child
.as_path()
.as_os_str()
.encode_wide()
.collect::<Vec<_>>();
assert!(!child_units.starts_with(&[b'\\' as u16, b'\\' as u16, b'?' as u16, b'\\' as u16,]));
}
#[cfg(windows)]
#[test]
fn isolated_child_final_validation_rejects_retargeted_spelling_before_spawn() {
let fixture = tempfile::tempdir().expect("fixture");
let repo = fixture.path().join("repo");
let original = fixture.path().join("original");
let attacker = fixture.path().join("attacker");
let child_spelling = fixture.path().join("child-spelling");
for directory in [&repo, &original, &attacker] {
std::fs::create_dir_all(directory).expect("create directory");
}
run_git(&repo, &["init", "-q"]);
create_junction(&child_spelling, &original);
let canonical = std::fs::canonicalize(&original).expect("canonical original");
let child = GitChildDirectory::new(
canonical,
child_spelling.clone(),
"test Git child directory",
)
.expect("initial child identity");
let runner = GitRunner::for_cwd(&repo).expect("runner");
let isolated = runner
.create_isolated_common_dir()
.expect("isolated common dir");
let marker = fixture.path().join("child-ran.gitconfig");
let mut command = runner.command_for_cwd(&repo).expect("marker command");
command
.args(["config", "--file"])
.arg(&marker)
.args(["codex.child-ran", "true"]);
std::fs::remove_dir(&child_spelling).expect("remove original junction");
create_junction(&child_spelling, &attacker);
let error = runner
.output_after_isolated_child_validation(
&mut command.inner,
&isolated,
[&child, &child, &child, &child],
)
.expect_err("retargeted child spelling must fail closed");
assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}");
assert!(
error
.to_string()
.contains("no longer identifies the original"),
"{error}"
);
assert!(!marker.exists(), "child command ran despite stale identity");
}
#[test]
fn gitdir_terminal_worktree_root_is_never_promoted_to_metadata() {
let fixture = tempfile::tempdir().expect("fixture");
@@ -264,6 +264,59 @@ fn isolated_three_way_config_preserves_builtin_union_default() {
assert!(!contents.contains("<<<<<<<"), "{contents}");
}
#[test]
fn isolated_three_way_add_add_conflict_preserves_markers_and_index_stages() {
let repo = init_repo();
let root = repo.path();
let target = root.join("conflict.txt");
std::fs::write(&target, "existing\n").expect("write existing file");
assert_eq!(run(root, &["git", "add", "conflict.txt"]).0, 0);
assert_eq!(run(root, &["git", "commit", "-m", "existing"]).0, 0);
let incoming = root.join("incoming.txt");
std::fs::write(&incoming, "incoming\n").expect("write incoming file");
let (hash_code, object_id, hash_error) =
run(root, &["git", "hash-object", "-w", "incoming.txt"]);
assert_eq!(hash_code, 0, "hash incoming blob: {hash_error}");
std::fs::remove_file(incoming).expect("remove incoming fixture");
let object_id = object_id.trim();
let null_object_id = "0".repeat(object_id.len());
let patch = format!(
"diff --git a/conflict.txt b/conflict.txt\nnew file mode 100644\nindex {null_object_id}..{object_id}\n--- /dev/null\n+++ b/conflict.txt\n@@ -0,0 +1 @@\n+incoming\n"
);
let result = apply_git_patch(&ApplyGitRequest {
cwd: root.to_path_buf(),
diff: patch,
revert: false,
preflight: false,
})
.expect("run isolated add/add three-way apply");
assert_ne!(result.exit_code, 0, "add/add apply unexpectedly succeeded");
assert!(
result.cmd_for_log.contains("GIT_COMMON_DIR=<isolated>")
&& result.cmd_for_log.contains("--3way"),
"unexpected final command: {}",
result.cmd_for_log
);
let contents = std::fs::read_to_string(&target).expect("read conflicted file");
assert!(
contents.contains("<<<<<<<")
&& contents.contains("=======")
&& contents.contains(">>>>>>>"),
"three-way add/add apply did not leave conflict markers:\nresult: {result:#?}\ncontents:\n{contents}"
);
let (stage_code, stages, stage_error) =
run(root, &["git", "ls-files", "--stage", "--", "conflict.txt"]);
assert_eq!(stage_code, 0, "read conflict stages: {stage_error}");
let actual_stages = stages
.lines()
.filter_map(|line| line.split_ascii_whitespace().nth(2))
.collect::<Vec<_>>();
assert_eq!(actual_stages, vec!["2", "3"], "{result:#?}");
}
#[test]
fn isolated_three_way_config_preserves_core_whitespace_error_classification() {
let repo = init_repo();