Repository navigation
Confine sandboxed file edits to verified native workspace handles - #10577
PhilipJohnBasile wants to merge 17 commits into
Conversation
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. |
![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: a88b95ecc6
ℹ️ 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".
|
This also rewrites the existing edit_file path in studio/backend/core/inference/tools.py to descriptor-relative O_NOFOLLOW operations, so files reached through a symlink inside a workspace would now be refused where they edit today; could you confirm that case before I review? The direction is right, but the branch bundles the edit confinement, a new Linux process supervisor and fence, and the approval UI changes together, so it would be much easier to take if the edit_file hardening landed on its own first. |
PhilipJohnBasile
changed the title
Sandboxed edit_file binds reads and atomic publication to opened workspace directories, preventing link traversal and rejecting changed targets. This PR now contains only native file-edit confinement and its tests; command supervision, process fences, shutdown cleanup and approval UI have been extracted into a separate follow-up.
POSIX operations use descriptor-relative traversal. Windows verifies handles and metadata, preserves DACLs and basic attributes, and rejects reparse points, hard links, named streams and unsupported replacements. Existing matching, multi-edit behavior, newline/BOM preservation, modes and result strings retain their public contract. Explicit Full access retains its existing unconfined behavior.
Symlink refusal is explicit: tests cover both linked files and linked parent directories, pointing inside and outside the workspace, and assert that target content is untouched. Local macOS validation: 111 passed, six Windows-only skips. The dedicated native CI matrix runs this narrowed file-edit contract on Linux, macOS and Windows; it no longer installs or runs command supervision.
Shared persisted-root authority matches #10576, including the deletion recheck. Source review details and reproduction commands are in studio/SECURE_TOOLS_REVIEW.md. Native filesystem tests are model-free; packaged desktop and live-model qualification remain separate.
Current native qualification: all three Linux/macOS/Windows jobs passed at
a85723e1cc245a6d286fc0f3521eee8a21c869d3in run 34481925917. Local tests passed 111 cases with six Windows-only skips. The upstream merge preserves the confined-edit implementation and incorporates the current Studio source. Edit approval/display remains separately reviewed in #10643.