Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (2)
ℹ️ 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. |
![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: a9f6323cfa
ℹ️ 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".
a9f6323 to
2941336
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 294133682d
ℹ️ 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".
2941336 to
92dda3a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92dda3af45
ℹ️ 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".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?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: 388fcb8dc7
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3859718c9
ℹ️ 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".
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: 0671152d7d
ℹ️ 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: add4f10caf
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd3090d46
ℹ️ 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".
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: 9cd3090d46
ℹ️ 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe0fc52814
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a193793179
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83c9ff2098
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddaf395b82
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ff4b72820
ℹ️ 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".
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 3d75e8e. Two issues remain: the new 1,835-line test module combines independent service and worker boundaries, and the shared/UnixLocal fallback blocks snapshot cleanup for supported configurations with unrelated read-only grants. The inline comments describe a concrete decomposition and the affected released contract.
All 22 hosted checks passed at this head. Source review included an independent pass and comparison with v0.22.3. I did not run local tests or Docker operations; the new service tests use recording workers and syscall doubles, so passing CI does not establish live execution of that privileged service.
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
[High] Preserve supported cleanup with unrelated read-only grants
The existing current-head cleanup finding is verified. BaseSandboxSession.rm() now rejects every recursive removal whenever any read-only extra grant exists, even when the requested workspace subtree cannot contain that grant. A supported manifest with an unrelated read-only toolchain mount therefore makes clear_workspace_dir_on_resume_pruned() fail on an ordinary nonempty workspace, and the opt-in Docker service does not cover UnixLocal or the other fallback paths.
Suggested fix: retain an atomic, backend-appropriate removal path that proves the selected subtree cannot cross a protected grant, or reject the unsupported backend/grant combination at its capability boundary with an explicit migration. I have not duplicated the existing inline thread.
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
[High] Preserve snapshot cleanup with unrelated read-only grants
src/agents/sandbox/session/base_sandbox_session.py:1146
The existing current-head finding remains independently verified. This unconditional guard rejects every recursive removal whenever any read-only grant exists, even when the target is an unrelated writable workspace child. Snapshot resume clears each existing child through rm(..., recursive=True), so a supported manifest such as a writable workspace plus read-only /opt/toolchain can no longer restore a nonempty snapshot on UnixLocal or ordinary remote backends; the opt-in Docker removal service does not cover those paths.
Suggested fix: Preserve an atomic, verified cleanup path for supported backends when the protected grant is unrelated, while keeping racy check-then-delete behavior disabled.
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
One blocking current-head regression is included inline.
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
One current-head blocking compatibility finding is included inline.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34730ebfcc
ℹ️ 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".
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
[High] Preserve released read-only grant configurations
src/agents/sandbox/workspace_paths.py:302
The current-head compatibility regression remains independently verified. This unconditional capability rejection makes every built-in backend except Docker with an optional live DockerRemovalService reject released SandboxPathGrant(read_only=True) manifests during create or resume, even when callers never request recursive removal. The exact-head Tests workflow fails all Linux and Windows Python lanes, including ordinary read-only bind, path-only grant, external-storage, and privileged-storage coverage; the container security integration also fails because its read-only credential grant cannot start.
Suggested fix: Preserve supported configurations whose read-only boundary is already enforced safely, and provide a race-safe cleanup path for the remaining backends. If a released backend must drop this capability, require and document the compatibility and migration decision at the release gate rather than making secure existing manifests fail on upgrade.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 085fdf09d8
ℹ️ 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".
Guard the Linux-only worker during Windows type checking and release descriptors and workers on failure or confirmed container removal. Traverse deep trees without Python recursion while preserving removal authorization.
f45c4e5 to
2e8bc46
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e8bc46b51
ℹ️ 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".
|
@dpiet-oai Can you review again? |
This branch has not been deployed
3 participants
This pull request fixes recursive sandbox removal bypassing nested read-only path grants. UnixLocal and shared remote sessions reject affected directory removals before deleting any contents, preserving protected files and writable siblings. Remote checks resolve aliases while preserving safe leaf-symlink removal and existing grant precedence.