fix(comparison): isolate restricted finding comparisons - #432
fix(comparison): isolate restricted finding comparisons#432mldangelo-oai wants to merge 9 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: 882a85ec31
ℹ️ 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. |
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Found a reproducible approval-boundary regression in the restricted finding-comparison session.
…-scan-comparison-approvals
…-scan-comparison-approvals
mldangelo-oai
changed the title
github-actions
Bot
added
skip-release-notes
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e18491ff8
ℹ️ 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 |
mldangelo-oai
changed the title
github-actions
Bot
added
bug
…-scan-comparison-approvals
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4bf8ab869
ℹ️ 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. 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". |
|
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 |
|
Codex Review: Didn't find any major issues. You're on a roll. 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. |
…-scan-comparison-approvals
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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. |
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Found a reproducible credential-exposure path during MCP inspection, before the restricted comparison session is created.
| options.signal, | ||
| ); | ||
| const command = resolveCodexCommand(environment); | ||
| const configuredServers = await runCodexCommand( |
There was a problem hiding this comment.
[P1] Do not contact MCP servers before enforcing comparison isolation
Could we inspect MCP configuration without invoking networked mcp list, or enforce network isolation before this subprocess starts? The pinned Codex CLI performs OAuth discovery for configured HTTP MCP servers while executing mcp list --json, before the comparison's network-disabled sandbox or secret-filtering policy exists. A localhost-only reproduction generated eight requests, and a project-local MCP configuration using env_http_headers = { "X-Provider-Key" = "OPENAI_API_KEY" } sent the inherited API key with those requests. Please add a synthetic HTTP MCP regression proving that inspection performs no outbound requests and cannot disclose environment credentials.
3 participants
Summary
Keep finding-comparison sessions read-only and noninteractive while preventing inherited MCP servers from bypassing their execution boundary.
Changes
approvalPolicy: "never"with its existing read-only sandbox, disabled network, and disabled web search.__proto__MCP servers start without isolation and never start in the restricted comparison.Testing
bun test --timeout 30000 tests-ts/scan-comparison.test.ts— 22 passed, 0 failed.bun test --timeout 30000 tests-ts/scan-comparison.test.ts tests-ts/auth.test.ts— 32 passed, 0 failed.bun test --timeout 30000 ./tests-ts— 1,124 passed, 11 skipped, 0 failed on the combined latest-main integration checkout.pnpm run typespnpm run formatpnpm exec prettier --check ../../SECURITY.mdpnpm run buildRisk and rollout
Comparison sessions retain their strict approval, filesystem, and network settings and additionally disable inherited MCP servers. If the selected executable cannot report its MCP configuration or a server name cannot be represented safely by the pinned SDK, comparison fails closed before creating a model session.
Public disclosure review