mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
git-utils: fix Windows repository authority checks
This commit is contained in:
@@ -26,6 +26,7 @@ pub(crate) struct GitRunner {
|
||||
/// Canonical executable target pinned at selection time. Never execute the
|
||||
/// mutable PATH spelling after validation.
|
||||
executable: PathBuf,
|
||||
#[cfg(any(unix, test))]
|
||||
argv0: PathBuf,
|
||||
safe_path: std::ffi::OsString,
|
||||
authority: RepositoryAuthority,
|
||||
@@ -116,6 +117,7 @@ impl GitRunner {
|
||||
let selected = select_git_executable(&authority, search_path)?;
|
||||
Ok(Self {
|
||||
executable: selected.executable,
|
||||
#[cfg(any(unix, test))]
|
||||
argv0: selected.argv0,
|
||||
safe_path: selected.safe_path,
|
||||
authority,
|
||||
|
||||
@@ -128,6 +128,29 @@ fn path_text(path: &Path) -> &str {
|
||||
path.to_str().expect("UTF-8 fixture path")
|
||||
}
|
||||
|
||||
fn tempdir_for_native_git() -> tempfile::TempDir {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// Git for Windows rejects the `\\?\` spelling returned when the
|
||||
// process temp directory is canonicalized.
|
||||
tempfile::tempdir().expect("fixture")
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let temp_base = std::fs::canonicalize(std::env::temp_dir()).expect("canonical temp dir");
|
||||
tempfile::tempdir_in(temp_base).expect("fixture")
|
||||
}
|
||||
}
|
||||
|
||||
fn git_config_path(path: &Path) -> String {
|
||||
let path = path.to_string_lossy();
|
||||
#[cfg(windows)]
|
||||
let path = path.replace('\\', "/");
|
||||
#[cfg(not(windows))]
|
||||
let path = path.into_owned();
|
||||
format!("\"{}\"", path.replace('\\', "\\\\").replace('"', "\\\""))
|
||||
}
|
||||
|
||||
struct OverlappingRegisteredRoute {
|
||||
main: PathBuf,
|
||||
nested: PathBuf,
|
||||
@@ -1056,6 +1079,34 @@ fn command_cwd_is_bound_before_worktree_junction_retarget() {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn command_for_cwd_executes_from_a_canonical_windows_path() {
|
||||
let fixture = tempfile::tempdir().expect("fixture");
|
||||
let root = fixture.path().join("repo");
|
||||
std::fs::create_dir_all(&root).expect("create repository");
|
||||
run_git(&root, &["init", "-q"]);
|
||||
let canonical_root = std::fs::canonicalize(&root).expect("canonical repository");
|
||||
|
||||
let runner = GitRunner::for_cwd(&root).expect("runner for canonical cwd");
|
||||
let mut command = runner
|
||||
.command_for_cwd(&canonical_root)
|
||||
.expect("command for canonical cwd");
|
||||
command.args(["rev-parse", "--show-toplevel"]);
|
||||
let output = runner.output(command).expect("run Git from canonical cwd");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"Git rejected canonical cwd {}: {}",
|
||||
canonical_root.display(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::canonicalize(String::from_utf8_lossy(&output.stdout).trim())
|
||||
.expect("canonical Git root"),
|
||||
canonical_root
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gitdir_terminal_worktree_root_is_never_promoted_to_metadata() {
|
||||
let fixture = tempfile::tempdir().expect("fixture");
|
||||
@@ -1406,8 +1457,7 @@ fn linked_worktree_rejects_git_from_main_and_linked_worktrees() {
|
||||
|
||||
#[test]
|
||||
fn registered_sibling_remains_untrusted_without_its_worktree_marker_or_root() {
|
||||
let temp_base = std::fs::canonicalize(std::env::temp_dir()).expect("canonical temp dir");
|
||||
let fixture = tempfile::tempdir_in(temp_base).expect("fixture");
|
||||
let fixture = tempdir_for_native_git();
|
||||
let main = fixture.path().join("main");
|
||||
let sibling = fixture.path().join("sibling");
|
||||
let sibling_bin = sibling.join("bin");
|
||||
@@ -2218,8 +2268,7 @@ fn nested_repo_inside_linked_outer_denies_unproven_outer_primary_before_path_git
|
||||
|
||||
#[test]
|
||||
fn explicit_absolute_core_worktree_is_recorded_before_path_selection() {
|
||||
let temp_base = std::fs::canonicalize(std::env::temp_dir()).expect("canonical temp dir");
|
||||
let fixture = tempfile::tempdir_in(temp_base).expect("fixture");
|
||||
let fixture = tempdir_for_native_git();
|
||||
let primary = fixture.path().join("primary");
|
||||
let common = fixture.path().join("separate-common");
|
||||
let linked = fixture.path().join("linked");
|
||||
@@ -2249,7 +2298,10 @@ fn explicit_absolute_core_worktree_is_recorded_before_path_selection() {
|
||||
],
|
||||
);
|
||||
let mut config = std::fs::read_to_string(common.join("config")).expect("read common config");
|
||||
config.push_str(&format!("\n[core]\n\tworktree = {}\n", primary.display()));
|
||||
config.push_str(&format!(
|
||||
"\n[core]\n\tworktree = {}\n",
|
||||
git_config_path(&primary)
|
||||
));
|
||||
std::fs::write(common.join("config"), config).expect("write explicit core.worktree");
|
||||
std::fs::remove_file(primary.join(".git")).expect("remove primary reverse marker");
|
||||
write_git_candidate(&primary_bin);
|
||||
@@ -2355,8 +2407,8 @@ fn resolver_rejects_unicode_case_alias_through_repository_junction() {
|
||||
|
||||
let locations = locations_for_root(&repo);
|
||||
assert!(
|
||||
!path_is_untrusted(&verbatim_case_alias, &locations),
|
||||
"fixture must exercise the Unicode alias before canonical ancestry"
|
||||
path_is_untrusted(&verbatim_case_alias, &locations),
|
||||
"route observation missed the Unicode repository alias"
|
||||
);
|
||||
assert!(search_directory_is_untrusted(
|
||||
&verbatim_case_alias,
|
||||
@@ -2407,4 +2459,8 @@ fn resolver_selects_native_git_exe_only() {
|
||||
let path = std::env::join_paths([scripts, native.clone()]).expect("PATH");
|
||||
let runner = GitRunner::from_search_path(locations, &path).expect("native Git");
|
||||
assert_eq!(runner.argv0, native.join("git.exe"));
|
||||
assert_eq!(
|
||||
runner.executable,
|
||||
std::fs::canonicalize(native.join("git.exe")).expect("canonical native Git")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ use std::path::Prefix;
|
||||
|
||||
pub(crate) struct SelectedGitExecutable {
|
||||
pub(crate) executable: PathBuf,
|
||||
#[cfg(any(unix, test))]
|
||||
pub(crate) argv0: PathBuf,
|
||||
pub(crate) safe_path: OsString,
|
||||
}
|
||||
@@ -55,13 +56,21 @@ pub(crate) fn select_git_executable(
|
||||
if let Some(parent) = canonical_candidate.parent() {
|
||||
push_unique_path(&mut safe_directories, parent.to_path_buf());
|
||||
}
|
||||
selected = Some((canonical_candidate, candidate));
|
||||
#[cfg(any(unix, test))]
|
||||
let selected_candidate = (canonical_candidate, candidate);
|
||||
#[cfg(not(any(unix, test)))]
|
||||
let selected_candidate = canonical_candidate;
|
||||
selected = Some(selected_candidate);
|
||||
}
|
||||
#[cfg(any(unix, test))]
|
||||
let (executable, argv0) = selected.ok_or(GitReadError::NoTrustedGit)?;
|
||||
#[cfg(not(any(unix, test)))]
|
||||
let executable = selected.ok_or(GitReadError::NoTrustedGit)?;
|
||||
let safe_path =
|
||||
std::env::join_paths(safe_directories).map_err(|_| GitReadError::NoTrustedGit)?;
|
||||
Ok(SelectedGitExecutable {
|
||||
executable,
|
||||
#[cfg(any(unix, test))]
|
||||
argv0,
|
||||
safe_path,
|
||||
})
|
||||
@@ -203,7 +212,7 @@ pub(crate) fn is_native_executable_file(path: &Path) -> bool {
|
||||
if !metadata.is_file() || metadata.permissions().mode() & 0o111 == 0 {
|
||||
return false;
|
||||
}
|
||||
let Ok(bytes) = read_prefix(path, 4) else {
|
||||
let Ok(bytes) = read_prefix(path, /*length*/ 4) else {
|
||||
return false;
|
||||
};
|
||||
matches!(
|
||||
@@ -228,7 +237,7 @@ pub(crate) fn is_native_executable_file(path: &Path) -> bool {
|
||||
};
|
||||
metadata.is_file()
|
||||
&& metadata.permissions().mode() & 0o111 != 0
|
||||
&& read_prefix(path, 4).is_ok_and(|bytes| bytes == b"\x7fELF")
|
||||
&& read_prefix(path, /*length*/ 4).is_ok_and(|bytes| bytes == b"\x7fELF")
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -257,7 +266,7 @@ pub(crate) fn is_native_executable_file(path: &Path) -> bool {
|
||||
if file.read_exact(&mut dos).is_err() || &dos[..2] != b"MZ" {
|
||||
return false;
|
||||
}
|
||||
let offset = u32::from_le_bytes(dos[60..64].try_into().expect("PE offset bytes")) as u64;
|
||||
let offset = u32::from_le_bytes([dos[60], dos[61], dos[62], dos[63]]) as u64;
|
||||
if offset > 1024 * 1024 || offset + 4 > metadata.len() {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -103,7 +103,7 @@ fn push_unique_spelling(paths: &mut Vec<PathBuf>, path: PathBuf) {
|
||||
fn symlink_route_hops(path: &Path) -> io::Result<Vec<SymlinkRouteHop>> {
|
||||
let mut hops = Vec::new();
|
||||
let mut seen = BTreeSet::new();
|
||||
collect_symlink_route_hops(path, 0, &mut seen, &mut hops)?;
|
||||
collect_symlink_route_hops(path, /*depth*/ 0, &mut seen, &mut hops)?;
|
||||
Ok(hops)
|
||||
}
|
||||
|
||||
@@ -121,8 +121,8 @@ fn collect_symlink_route_hops(
|
||||
}
|
||||
for ancestor in path.ancestors() {
|
||||
match std::fs::symlink_metadata(ancestor) {
|
||||
Ok(_) => match std::fs::read_link(ancestor) {
|
||||
Ok(target) => {
|
||||
Ok(_) => match read_link_if_symlink(ancestor) {
|
||||
Ok(Some(target)) => {
|
||||
let parent = ancestor
|
||||
.parent()
|
||||
.ok_or_else(|| invalid_data("authority symlink has no parent"))?;
|
||||
@@ -143,7 +143,7 @@ fn collect_symlink_route_hops(
|
||||
});
|
||||
collect_symlink_route_hops(&projected, depth + 1, seen, hops)?;
|
||||
}
|
||||
Err(error) if error.kind() == io::ErrorKind::InvalidInput => {}
|
||||
Ok(None) => {}
|
||||
Err(error) => return Err(error),
|
||||
},
|
||||
Err(error)
|
||||
@@ -157,8 +157,30 @@ fn collect_symlink_route_hops(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn read_link_if_symlink(path: &Path) -> io::Result<Option<PathBuf>> {
|
||||
match std::fs::read_link(path) {
|
||||
Ok(target) => Ok(Some(target)),
|
||||
Err(error) if read_link_error_means_not_symlink(&error) => Ok(None),
|
||||
Err(error) => Err(error),
|
||||
}
|
||||
}
|
||||
|
||||
fn read_link_error_means_not_symlink(error: &io::Error) -> bool {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// Win32 reports this for ordinary files and directories passed to
|
||||
// `std::fs::read_link`; it is the Windows equivalent of EINVAL here.
|
||||
const ERROR_NOT_A_REPARSE_POINT: i32 = 4390;
|
||||
error.raw_os_error() == Some(ERROR_NOT_A_REPARSE_POINT)
|
||||
}
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
error.kind() == io::ErrorKind::InvalidInput
|
||||
}
|
||||
}
|
||||
|
||||
fn project_through_longest_existing_ancestor(path: &Path) -> io::Result<PathBuf> {
|
||||
project_path(path, 0)
|
||||
project_path(path, /*symlink_depth*/ 0)
|
||||
}
|
||||
|
||||
fn project_path(path: &Path, symlink_depth: usize) -> io::Result<PathBuf> {
|
||||
@@ -219,3 +241,7 @@ fn resolve_literal_path(path: impl AsRef<Path>, base: &Path) -> PathBuf {
|
||||
pub(super) fn invalid_data(message: &str) -> io::Error {
|
||||
io::Error::new(io::ErrorKind::InvalidData, message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "route_walker_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::read_link_if_symlink;
|
||||
|
||||
#[test]
|
||||
fn ordinary_paths_are_not_treated_as_failed_symlink_reads() {
|
||||
let fixture = tempfile::tempdir().expect("fixture");
|
||||
let directory = fixture.path().join("directory");
|
||||
let file = fixture.path().join("file");
|
||||
std::fs::create_dir(&directory).expect("directory");
|
||||
std::fs::write(&file, b"ordinary file").expect("file");
|
||||
|
||||
assert_eq!(
|
||||
read_link_if_symlink(&directory).expect("ordinary directory"),
|
||||
None
|
||||
);
|
||||
assert_eq!(read_link_if_symlink(&file).expect("ordinary file"), None);
|
||||
}
|
||||
@@ -173,7 +173,7 @@ impl RepositoryAuthority {
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[cfg(all(test, unix))]
|
||||
pub(crate) fn active_git_dir(&self) -> Option<&Path> {
|
||||
self.active_metadata
|
||||
.as_ref()
|
||||
|
||||
@@ -26,6 +26,15 @@ fn write_common_config(body: &[u8]) -> tempfile::TempDir {
|
||||
common
|
||||
}
|
||||
|
||||
fn git_config_path(path: &Path) -> String {
|
||||
let path = path.to_string_lossy();
|
||||
#[cfg(windows)]
|
||||
let path = path.replace('\\', "/");
|
||||
#[cfg(not(windows))]
|
||||
let path = path.into_owned();
|
||||
format!("\"{}\"", path.replace('\\', "\\\\").replace('"', "\\\""))
|
||||
}
|
||||
|
||||
fn native_bare_value(config: &Path, includes: bool) -> io::Result<Option<bool>> {
|
||||
let mut command = std::process::Command::new("git");
|
||||
crate::safe_git::isolate_git_command_environment(&mut command);
|
||||
@@ -116,7 +125,7 @@ fn plain_common_bare_parser_matches_native_boolean_syntax_and_is_stricter_on_dup
|
||||
"authority result for {name}"
|
||||
);
|
||||
assert_eq!(
|
||||
native_bare_value(&common.path().join("config"), false).expect(name),
|
||||
native_bare_value(&common.path().join("config"), /*includes*/ false).expect(name),
|
||||
native,
|
||||
"native result for {name}"
|
||||
);
|
||||
@@ -161,16 +170,18 @@ fn common_bare_proof_rejects_includes_worktree_config_and_relative_worktree() {
|
||||
let common = write_common_config(
|
||||
format!(
|
||||
"[core]\n\tbare = true\n[include]\n\tpath = {}\n",
|
||||
include_target.path().display()
|
||||
git_config_path(include_target.path())
|
||||
)
|
||||
.as_bytes(),
|
||||
);
|
||||
assert_eq!(
|
||||
native_bare_value(&common.path().join("config"), false).expect("direct native bare"),
|
||||
native_bare_value(&common.path().join("config"), /*includes*/ false)
|
||||
.expect("direct native bare"),
|
||||
Some(true)
|
||||
);
|
||||
assert_eq!(
|
||||
native_bare_value(&common.path().join("config"), true).expect("included native bare"),
|
||||
native_bare_value(&common.path().join("config"), /*includes*/ true)
|
||||
.expect("included native bare"),
|
||||
Some(false)
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -203,7 +214,11 @@ fn common_bare_proof_rejects_includes_worktree_config_and_relative_worktree() {
|
||||
fn common_config_absolute_worktree_is_returned_as_authority() {
|
||||
let worktree = tempfile::tempdir().expect("worktree");
|
||||
let common = write_common_config(
|
||||
format!("[core]\n\tworktree = {}\n", worktree.path().display()).as_bytes(),
|
||||
format!(
|
||||
"[core]\n\tworktree = {}\n",
|
||||
git_config_path(worktree.path())
|
||||
)
|
||||
.as_bytes(),
|
||||
);
|
||||
assert_eq!(
|
||||
inspect_plain_common_config_authority(common.path()).expect("worktree authority"),
|
||||
@@ -218,7 +233,7 @@ fn common_config_rejects_contradictory_or_malformed_bare_with_absolute_worktree(
|
||||
let common = write_common_config(
|
||||
format!(
|
||||
"[core]\n\tbare = {bare}\n\tworktree = {}\n",
|
||||
worktree.path().display()
|
||||
git_config_path(worktree.path())
|
||||
)
|
||||
.as_bytes(),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user