Bind Git worktree helpers to a trusted executable#30833
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ca4f55c0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
When adding this new test module, the repo guidance requires putting the test contents in a separate sibling file with a descriptive #[path = "..._tests.rs"] declaration rather than defining the module inline. Keeping these new tests in operations.rs makes this file inconsistent with the adjacent git_command_tests.rs/git_config_tests.rs organization and harder to navigate.
AGENTS.md reference: AGENTS.md:L167-L178
Useful? React with 👍 / 👎.
| if path_is_untrusted(&candidate, untrusted) { | ||
| continue; |
There was a problem hiding this comment.
When a trusted Git binary is outside the repo but the PATH entry is spelled through the repo with .. (for example /work/repo/../tools/bin), this pre-canonical path_is_untrusted check treats the candidate as being inside /work/repo and skips it before the later canonical checks can prove it is outside. In environments where that is the only usable Git on PATH, all of these helpers now fail with NoTrustedGit even though a trusted executable is available.
Useful? React with 👍 / 👎.
| locations | ||
| .common_dir | ||
| .as_deref() | ||
| .is_some_and(|common_dir| path_is_in_worktree_for_common_dir(path, common_dir)) |
There was a problem hiding this comment.
When Codex is run inside a submodule or nested repo and PATH contains an absolute bin directory from the enclosing checkout, the candidate is outside roots and path_is_in_worktree_for_common_dir returns false because the parent repo has a different common dir. That lets a parent-repository-controlled git be selected, leaving the PATH hijack this resolver is meant to prevent open for that common setup.
Useful? React with 👍 / 👎.
…4394-trusted-git-runner
1 participant
Why
Codex's internal worktree helpers used whichever
gitexecutable appeared first in PATH at each call site. A repository-controlled executable or linked worktree could therefore run before the helper reached its normal safety checks.What
Choose one absolute native Git executable outside the active repository, any enclosing repository, its linked worktrees, and its shared Git metadata. Pin that executable and reuse it for patch and repository operations.
How
Immediately before each child starts, remove inherited settings that could redirect the repository, index, path matching, executable lookup, or Git configuration. Reapply the existing local-only transport policy at the same boundary.
On Windows, reject PATH entries whose spelling, aliases, namespaces, or junctions could hide traversal through repository-controlled locations.
This PR is stacked on #29470 and should land after it.
Testing
codex-git-utilstests passed.PWDvariants, and Windows path aliases.63f6aa5da5, including native Windows.Related: PSEC-4394