Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fe4ec9567
ℹ️ 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-rs/protocol/src/protocol.rs
Outdated
Comment on lines
653
to
654
| let mut roots = read_only_access.get_readable_roots_with_cwd(cwd); | ||
| roots.extend(writable_roots.clone()); |
Contributor
There was a problem hiding this comment.
When WorkspaceWrite uses restricted read access, this code only merges read_only_access roots with the explicitly configured writable_roots, but it skips the implicit writable roots added elsewhere (cwd, /tmp, and optionally TMPDIR). That creates a mismatch where default workspace paths remain writable but are not readable, so commands can fail in restricted-read mode on the Seatbelt backend unless callers manually duplicate those paths in readable_roots.
Useful? React with 👍 / 👎.
bolinfest
force-pushed
the
pr11387
branch
8 times, most recently
from
adca2d9 to
9132f09
Compare
bolinfest
changed the title
ReadOnlyAccess
bolinfest
force-pushed
the
pr11387
branch
8 times, most recently
from
2c0af3f to
1245a70
Compare
## What
This change introduces a new `ReadOnlyAccess` model and threads it through
sandbox policy consumers so read access is explicit instead of implicit.
- Added `ReadOnlyAccess` to protocol:
- `Restricted { include_platform_defaults, readable_roots }`
- `FullAccess`
- Changed `SandboxPolicy` shape:
- `ReadOnly` is now `ReadOnly { access: ReadOnlyAccess }`
- `WorkspaceWrite` now carries `read_only_access: ReadOnlyAccess`
- Kept existing behavior for now by defaulting to `ReadOnlyAccess::FullAccess`
in constructors and current config/app-server mappings.
- Added helper methods to compute effective readable roots (including optional
platform defaults + cwd) and to detect full read access.
- Updated seatbelt policy generation to honor restricted read roots by emitting
scoped `(allow file-read* ...)` entries when full read access is not granted.
- Updated Linux backends (`bwrap`, legacy landlock path) to fail closed with an
explicit `UnsupportedOperation` when restricted read access is requested but
not yet implemented there.
- Updated Windows sandbox backends (standard, elevated, and runner paths) to
fail closed in the same way for restricted read access.
- Updated all call sites/tests/pattern matches for the new structured variants
and regenerated app-server protocol schema/types.
## Why
The previous `SandboxPolicy::ReadOnly` implied full-disk read access and left
no way to express a narrower read surface.
This refactor establishes the policy model needed to support user-configurable
read restrictions in a follow-up without changing current runtime behavior.
It also ensures we do not silently ignore future restricted-read policies on
platform backends that do not support them yet. Failing closed keeps sandbox
semantics predictable and avoids accidental over-permission.
## Compatibility and rollout notes
- Existing behavior is preserved by default (`FullAccess`).
- Existing config/app-server flows continue to serialize/deserialize cleanly.
- New schema artifacts are included to keep generated protocol outputs in sync.
## Validation
- `just fmt`
- `just fix -p codex-protocol -p codex-core -p codex-linux-sandbox -p codex-windows-sandbox -p codex-app-server-protocol`
- `cargo check -p codex-windows-sandbox`
- Targeted crate/test runs were executed during development for protocol/core/
sandbox-related crates.
viyatb-oai
approved these changes
Feb 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SandboxPolicy::ReadOnlypreviously implied broad read access and could not express a narrower read surface.This change introduces an explicit read-access model so we can support user-configurable read restrictions in follow-up work, while preserving current behavior today.
It also ensures unsupported backends fail closed for restricted-read policies instead of silently granting broader access than intended.
What
ReadOnlyAccessin protocol with:Restricted { include_platform_defaults, readable_roots }FullAccessSandboxPolicyto carry read-access configuration:ReadOnly { access: ReadOnlyAccess }WorkspaceWrite { ..., read_only_access: ReadOnlyAccess }ReadOnlyAccess::FullAccess.core,tui,linux-sandbox,windows-sandbox, and related tests.UnsupportedOperation).ReadOnlyAccess.Compatibility / rollout
FullAccess).