fix(sdk): bind workbench Git to a trusted executable - #467
fix(sdk): bind workbench Git to a trusted executable#467mldangelo-oai wants to merge 37 commits into
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".
|
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".
|
@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".
|
@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". |
mldangelo-oai
changed the title
|
@codex security review Please review exact head |
|
@codex review Please review exact head |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ 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 Review: Didn't find any major issues. Keep them coming! 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". |
kmbroai
left a comment
kmbroai
left a comment
There was a problem hiding this comment.
Critical review
Reviewed head 8f4f32a47b703c4f452e702d63fa708e91d22dbd, including the earlier executable-selection feedback.
Recommendation: keep this narrowed trusted-Git binding. No new blocking defect found in the reviewed changes. Host inspection is ineffective if the Python helper later performs a fresh PATH search inside the scan environment. Carrying the inspected invocation into the helper closes that gap without requiring a general tool-distribution subsystem.
Correctness and earlier comments
trusted_git_executable checks both lexical and canonical containment, preserves the invocation name for wrappers/multicall binaries, and validates native Windows invocation semantics without requiring an extension on the resolved native target. Missing/unusable binding returns the optional-Git failure status without executing the literal fallback string.
The host now retains the sanitized environment even when no executable is found. It derives protected roots from the selected filesystem inputs, not Git's configurable core.worktree, and carries the binding through .mcp.json. Inventory/ranking Git calls use the shared command wrapper with external diff/textconv disabled. This addresses the substantive earlier selection and invocation-path concerns.
The public CODEX_SECURITY_GIT configuration/disable proposal and bundled-ripgrep staging were removed. The current README correctly describes this variable as host-generated runtime data, so the old requests to preserve user overrides do not apply to the narrowed contract.
Simplification and limits
This is a good place to stop the abstraction: one inspected executable/environment pair and Python revalidation of the same binding. Do not reintroduce separate Git/ripgrep installers, broad launch-directory restrictions, or per-tool configuration flags without a demonstrated requirement.
The shared resolver still needs the distinction between a null executable and a sanitized environment; collapsing that back to null would recreate the earlier bug. Do not describe the binding as a cryptographic executable pin or a general sandbox—it establishes the selected trusted path under the repository-input threat model.
Verification
Ran four resolver/launcher/workbench suites: 13 passed, 2 skipped, 0 failed, then seven focused API/runtime Git integration cases: 7 passed, 0 failed. These include actual nested-repository shim rejection and an MCP launch with empty PATH. Linux, Bun 1.3.14 / Node 22.13.1 with cached dependencies. Native Windows alias behavior was source-reviewed but not executed on Windows; no model call occurred.
3 participants
Summary
Keep repository-controlled Git shims out of bundled Python scan helpers. The SDK
selects Git outside the scan and knowledge-base repositories, then passes that
binding to the workbench and MCP process. Python does not search its inherited
PATH for Git.
Changes
repository plus canonical knowledge-base source repositories.
bindings return the existing optional-Git status 127 without a PATH lookup.
hooks, filesystem monitors, external diff drivers, and text conversion disabled.
helper, and avoid an unrelated rewrite of the public environment helper.
0.1.89.CODEX_SECURITY_GITisgenerated runtime data, not a supported user setting.
Testing
Local first-pass verification:
no failures. Tests cover host/Python binding parity, knowledge-base sources,
nested worktrees, unavailable Git, rank/inventory helpers, and Windows rules.
After merging main
01bd062, 295 Git-binding/runtime/API tests passed with17 platform skips. Both real cache upgrades from 0.1.60 and 0.1.63 matched all
118 installed files and preserved credentials. Types/models, formatting,
build, static package validation and full installed-package smoke passed again.
The Git-binding implementation is unchanged by this final version-field merge.
The full suite and native Windows execution were not repeated. New-head CI is
left for the second pass, without delaying the remaining PR updates.
fd98a90): package 0.1.21 includes the MCP launcher-permission fix; feature source and bundled payload are unchanged. Types/model generation, formatting, build, 28 focused package/report/launcher tests, static artifact verification and full installed-package smoke passed, including MCP initialization. CI was not awaited.Risk and rollout
No new CLI flag, supported environment setting, dependency, npm version, or root
package export. The public
pluginExecutionEnvironmentsignature is unchanged.The SDK overwrites the internal Git binding for its children.
Git remains optional for directory scans, while diff scans still require it.
Safe operator Git and SSH settings remain available; repository execution hooks
stay disabled during probes. This remains a draft PR and publishes no release.
Public disclosure review
The current update and public PR surface have been reviewed. Existing automated
comments contain access-restricted report links, so the second attestation
remains unchecked.