Skip to content

protocol: canonicalize file system permissions#18274

Merged
bolinfest merged 1 commit intomainfrom
pr18274
Apr 20, 2026
Merged

protocol: canonicalize file system permissions#18274
bolinfest merged 1 commit intomainfrom
pr18274

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 17, 2026

Why

PermissionProfile needs stable, canonical file-system semantics before it can become the primary runtime permissions abstraction. Without a canonical form, callers have to keep re-deriving legacy sandbox maps and profile comparisons remain lossy or order-dependent.

What changed

This adds canonicalization helpers for FileSystemPermissions and PermissionProfile, expands special paths into explicit sandbox entries, and updates permission request/conversion paths to consume those canonical entries. It also tightens the legacy bridge so root-wide write profiles with narrower carveouts are not silently projected as full-disk legacy access.

Verification

  • cargo test -p codex-protocol root_write_with_read_only_child_is_not_full_disk_write -- --nocapture
  • cargo test -p codex-sandboxing permission -- --nocapture
  • cargo test -p codex-tui permissions -- --nocapture

Copy link
Copy Markdown
Contributor

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

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: 9015f3683d

ℹ️ 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/tui/src/bottom_pane/approval_overlay.rs Outdated
Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs Outdated
@bolinfest bolinfest force-pushed the pr18274 branch 3 times, most recently from 18fc17a to 577cde3 Compare April 17, 2026 18:23
Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs
Comment thread codex-rs/protocol/src/models.rs
@bolinfest bolinfest force-pushed the pr18274 branch 5 times, most recently from f7496e7 to f3ff42a Compare April 17, 2026 21:36
Comment thread codex-rs/sandboxing/src/policy_transforms.rs
Copy link
Copy Markdown
Collaborator

@viyatb-oai viyatb-oai left a comment

Choose a reason for hiding this comment

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

one remaining finding but otherwise looks in a good shape

@bolinfest bolinfest merged commit dcec516 into main Apr 20, 2026
39 of 50 checks passed
@bolinfest bolinfest deleted the pr18274 branch April 20, 2026 16:57
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 20, 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