fix(git): pin trusted executables across scan hosts - #467
Conversation
|
@codex review |
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.
Honor explicit Git disable bindings
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. |
Summary
Use one trusted Git executable consistently across SDK scans, the bundled workbench, and MCP hosts.
Fixes #129. Extends the still-open proposal in #140.
Changes
PATHis omitted without broadening an explicitly empty or rejected path.PATHaliases consistently.0.1.22so cached hosts refresh the helper changes.Testing
Current head
743e32f0:bun test --timeout 30000 tests-ts/runtime.test.ts --test-name-pattern 'stages only package-owned native ripgrep and cleans failed copies|resolves the exact npm Codex executable|uses an explicit Codex executable override'— three passed.bun test --timeout 30000 tests-ts/api.test.ts --test-name-pattern 'binds staged bundled ripgrep only when a trusted host tool is unavailable|cleans the bootstrap workspace when credential-home cleanup fails'— two passed. New package, filesystem, and platform cases use isolated mocks.bun test --timeout 30000 tests-ts/workbench-tool-environment.test.ts tests-ts/workbench-scan-history.test.ts— ten tests passed on Python 3.12.12 and again on Python 3.10.19 withtomli2.3.0.bun --no-env-file test --randomize --seed 12345 --timeout 30000 ./tests-ts— 1,316 passed, 11 skipped, zero failures, and 9,286 assertions with Bun 1.3.14. Live integration and machine-policy opt-ins were disabled.pnpm run typesandpnpm run format— passed.ruff check --no-cache _bundled_plugin/scripts/workbench_target.py _bundled_plugin/scripts/generate_in_scope_files.py _bundled_plugin/scripts/generate_rank_input.py— passed. These modules andworkbench_scan_historyalso imported successfully under Python 3.10.19.pnpm run build,pnpm pack,pnpm run check:package, and the separatepnpm run test:packageon the resultingopenai-codex-security-0.1.14.tgz— passed. The package validator checked 218 archive entries and 106 bundled-plugin files. Both installed checks ran the actual nested worker with its original timeout.git diff --check— passed.The source head and tree stayed clean and unchanged through all local gates. Standard and cross-platform remote CI remain pending for the published head.
Risk and rollout
Git remains optional, safe user settings remain available, and the existing snapshot and scan-completion contracts are unchanged. The packaged-ripgrep fallback uses the installed application's native-package provenance and existing runtime cleanup; it does not relax the Python repository exclusion or add a persistent executable cache. Explicit disable bindings remain effective on POSIX and Windows. The missing-
PATHbehavior follows the documented Node command-lookup defaults.Integrate #449 first and retain its streamed Git command support, committed-object immutability, and offline-read policy. Preserve its shared-runner routing and immutable blob-read flags. Reconcile the shared inventory and containment changes in #320 and #472 when they land. This branch uses paired plugin version
0.1.22; the recorded #449, #462, and #472 heads carried competing0.1.21updates. Before merging, update both version fields together to the next monotonic version after the preceding plugin changes, then rerun the integration checks on the resulting head.Public disclosure review
Newly authored material was reviewed for public disclosure. Existing automated review comments contain access-restricted report references; maintainer cleanup remains outstanding.