refactor: share Codex session execution - #917
mldangelo-oai wants to merge 15 commits into
Conversation
|
@codex review Please review exact head |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsBlocking findings (1)
ℹ️ 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: Didn't find any major issues. 👍 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 review Please review exact head |
|
Codex Review: Didn't find any major issues. 🚀 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". |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 0b43c3f474
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| "model_reasoning_effort", | ||
| "model_reasoning_summary", | ||
| "service_tier", | ||
| "model_providers", |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
When codexOverrides.model_providers contains an embedded credential (including in an unrelated provider), this loop copies the entire table into <CODEX_SECURITY_CONFIG_PATH>.workers.toml. Every scan child receives the base path, and its filesystem profile grants root read, so repository-controlled instructions can read the predictable sibling and return the credential in a model request. This bypasses the existing sanitized preflight projection. Store the worker snapshot outside scan-readable paths or project only the selected provider without exposing secrets.
SECURITY.md reference: SECURITY.md:L94-L100
Dismiss this finding: Reply with @codex security dismiss <reason> [context]. Codex will resolve this conversation automatically; GitHub may require a page refresh to show the result.
Valid reasons: false-positive, duplicate, out-of-scope, compensating-control, risk-accepted, or other. Example: @codex security dismiss duplicate Already flagged by another review
What each reason means
false-positive— Not a vulnerabilityduplicate— Already tracked elsewhereout-of-scope— Outside this review's scopecompensating-control— Mitigated by another controlrisk-accepted— Risk intentionally acceptedother— Another reason; context required
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2877c44477
ℹ️ 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".
| isObject(providers) && | ||
| Object.hasOwn(providers, selected) | ||
| ) { | ||
| result["model_providers"] = { [selected]: providers[selected]! }; |
There was a problem hiding this comment.
When the selected custom provider contains literal credentials such as experimental_bearer_token or an authorization value in http_headers, this assignment preserves them verbatim in the predictable <CODEX_SECURITY_CONFIG_PATH>.workers.toml file. The file is written before both Standard and Deep parent sessions start, and their root-read sandbox plus the exported config path lets repository-controlled instructions read the credential and return it in a model request or scan artifact. Fresh evidence in this head is the new config.test.ts case explicitly asserting that these selected-provider secret fields survive, even though unrelated providers are now removed. Project only non-secret settings, or explicitly deny the snapshot path to scan processes.
AGENTS.md reference: sdk/typescript/AGENTS.md:L8-L12
Useful? React with 👍 / 👎.
1 participant
Summary
Share streamed-turn handling between Standard scans and Deep workers. Preserve each session’s selected provider and authentication settings through concurrent and reconstructed workers.
Changes
Testing
2877c44, SDK types, formatting, stock package build/check and installed/detached controls passed. Package controls cover default Python discovery, Python-only PATH and an explicit executable path containing spaces, plus discovery, reducers, checkpoints, retry, restart and sealed results.e418d41and a distinct source/provenance audit passed without findings.Risk and rollout
This internal change adds no command, setting, result field, dependency, migration or scheduler policy. Standard and Deep retain their intentional permission differences and follow-up behavior. Older SDKs without worker snapshots retain their inherited-home behavior. Linux package controls do not establish native Windows/macOS equivalence.
Public disclosure review