fix(git): pin trusted executables across scan hosts - #467
Conversation
|
@codex review |
![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: 5dc0f1c298
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9be47a60bb
ℹ️ 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".
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49000ab013
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…/codex/portfolio-pr-467-20260815
|
@codex review Please review the current head, |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 743e32f028
ℹ️ 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".
| const git = await inspectTrustedExecutable( | ||
| "git", | ||
| pluginEnvironment, | ||
| protectedGitRoot, | ||
| ); |
There was a problem hiding this comment.
When the SDK environment sets CODEX_SECURITY_GIT="" to disable optional Git, this lookup ignores the binding, discovers Git from PATH, and later replaces the empty value with that executable. Codebase scans therefore use a Git snapshot instead of the requested directory fallback, while diff scans proceed rather than reporting Git as unavailable. Check the binding before lookup, including case-insensitive aliases on Windows, as is already done for ripgrep.
AGENTS.md reference: sdk/typescript/AGENTS.md:L19-L20
Useful? React with 👍 / 👎.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review the current head, |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0142c5739
ℹ️ 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".
| let ripgrep = await inspectTrustedExecutable( | ||
| "rg", | ||
| git.environment, | ||
| protectedGitRoot, | ||
| ); |
There was a problem hiding this comment.
On Windows, when PATH contains an rg.exe symlink targeting an rg.cmd or rg.bat file and no later trusted ripgrep exists, inspectTrustedExecutable treats the .exe invocation as runnable and this call therefore skips the bundled-ripgrep fallback. The Python resolver subsequently rejects the canonical batch target in _is_native_executable, causing inventory generation—and thus the scan—to fail. Reject canonical batch targets during host inspection while continuing to allow extensionless native targets.
AGENTS.md reference: sdk/typescript/AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review the current head, |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex security review Please review the current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76b9e90661
ℹ️ 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".
|
|
||
| const gitRoots = await protectedGitInputRoots( | ||
| [ | ||
| process.cwd(), |
There was a problem hiding this comment.
When multiscan is launched from a directory containing a deliberately configured Git executable—for example, CODEX_SECURITY_GIT=$PWD/tools/git while the inventory and task repositories are elsewhere—adding process.cwd() makes the entire launch worktree protected, so resolveGitCommand() rejects the requested executable and every pending checkout fails. Relative repository entries are resolved against the inventory directory, and the inventory file and local sources are already included separately, so the launch directory should not be protected unconditionally.
AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0552695. Bulk tool selection now excludes the actual inventory, scan, knowledge-base, local-source, and managed-checkout inputs without treating an unrelated launch directory as an input. The regression runs from a separate synthetic repository and intercepts the selected executable before execution. The affected tests pass.
| runtime.bootstrapWorkspace !== undefined | ||
| ) { | ||
| const workspace = await realpath(runtime.bootstrapWorkspace); | ||
| requireOutputOutsideRepository(protectedGitRoot, workspace, "runtime"); |
There was a problem hiding this comment.
When host ripgrep is unavailable and bootstrapWorkspace is beneath another protected input root—for example, TMPDIR points into the repository containing a knowledge-base document—this check validates only the scanned repository, so the SDK stages ripgrep inside that protected input tree. The subsequent inspection uses protectedGitRoots, rejects the staged executable, and leaves inventory generation without ripgrep; validate the workspace against every protected root before staging.
AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0552695. Bundled-ripgrep staging now checks the runtime workspace against every protected input before copying. The regression covers a separate knowledge-base repository with an empty PATH and an explicit CLI path, and confirms that an overlapping workspace fails before staging or launch. The affected tests pass.
|
@codex review Please review exact head 0552695. The additive follow-up fixes the launch-directory exclusion and checks bundled-ripgrep staging against all actual protected inputs. The affected tests, installed-package checks, strict TypeScript consumers, and both full suites pass. The PR remains draft. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex security review Please review the current head |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Summary
Use one trusted Git executable consistently across SDK target validation, scans, the bundled workbench, and MCP hosts.
Fixes #129. Extends the proposal in #140.
Changes
0.1.29for the changed payload.Testing
On
055269586a148c0f0fc1fa08025deeb00ef3f434:0.1.29are unchanged from the published parent.Earlier results remain historical evidence, not validation of this follow-up.
Risk and rollout
Public root exports and runtime behavior are unchanged by the declaration correction. Code that depended on unsupported internal signatures may stop type-checking.
Git remains optional. An explicit disable now applies to early SDK Git operations as well as plugin hosts, so Git-dependent diff targets report that Git is unavailable. Invalid explicit selections fail before scan initialization. The existing snapshot, scan-completion, Python containment, and runtime-cleanup contracts remain in place. No dependency or npm-version change is included.
Integrate #449 first and preserve its streamed Git runner, cancellation behavior, private-spool handling, and immutable blob reads. Reconcile overlapping inventory changes in #320 and #472 when they land. Before merging, update both plugin version fields together to the next monotonic version after preceding plugin changes and rerun integration checks.
Public disclosure review
Newly authored material was reviewed for public disclosure. Existing automated review comments contain access-restricted report references; maintainer cleanup remains outstanding.