Skip to content

permissions: remove legacy read-only access modes#19449

Merged
bolinfest merged 1 commit into
mainfrom
pr19449
Apr 25, 2026
Merged

permissions: remove legacy read-only access modes#19449
bolinfest merged 1 commit into
mainfrom
pr19449

Conversation

@bolinfest

@bolinfest bolinfest commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Why

ReadOnlyAccess was a transitional legacy shape on SandboxPolicy: FullAccess meant the historical read-only/workspace-write modes could read the full filesystem, while Restricted tried to carry partial readable roots. The partial-read model now belongs in FileSystemSandboxPolicy and PermissionProfile, so keeping it on SandboxPolicy makes every legacy projection reintroduce lossy read-root bookkeeping and creates unnecessary noise in the rest of the permissions migration.

This PR makes the legacy policy model narrower and explicit: SandboxPolicy::ReadOnly and SandboxPolicy::WorkspaceWrite represent the old full-read sandbox modes only. Split readable roots, deny-read globs, and platform-default/minimal read behavior stay in the runtime permissions model.

What changed

  • Removes ReadOnlyAccess from codex_protocol::protocol::SandboxPolicy, including the generated access and readOnlyAccess API fields.
  • Updates legacy policy/profile conversions so restricted filesystem reads are represented only by FileSystemSandboxPolicy / PermissionProfile entries.
  • Keeps app-server v2 compatible with legacy fullAccess read-access payloads by accepting and ignoring that no-op shape, while rejecting legacy restricted read-access payloads instead of silently widening them to full-read legacy policies.
  • Carries Windows sandbox platform-default read behavior with an explicit override flag instead of depending on ReadOnlyAccess::Restricted.
  • Refreshes generated app-server schema/types and updates tests/docs for the simplified legacy policy shape.

Verification

  • cargo check -p codex-app-server-protocol --tests
  • cargo check -p codex-windows-sandbox --tests
  • cargo test -p codex-app-server-protocol sandbox_policy_

Stack created with Sapling. Best reviewed with ReviewStack.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bbc6c1beb8

ℹ️ 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".

Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs
Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs Outdated
@bolinfest bolinfest merged commit 789f387 into main Apr 25, 2026
39 of 50 checks passed
@bolinfest bolinfest deleted the pr19449 branch April 25, 2026 00:17
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants