Skip to content

config: express implicit sandbox defaults as permission profiles#25926

Merged
bolinfest merged 1 commit into
mainfrom
pr25926
Jun 2, 2026
Merged

config: express implicit sandbox defaults as permission profiles#25926
bolinfest merged 1 commit into
mainfrom
pr25926

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Jun 2, 2026

Why

PermissionProfile is becoming the default way to represent Codex permissions, but the implicit default behavior should stay the same for now:

  • trusted projects use :workspace
  • untrusted projects also use :workspace
  • roots without a trust decision use :read-only
  • unsandboxed Windows falls back to :read-only

This keeps the existing sandbox semantics while making silent config defaults observable as built-in permission profiles instead of treating the legacy SandboxPolicy projection as the primary shape.

What Changed

  • Refactored legacy sandbox derivation to resolve the configured sandbox mode once, then apply the implicit project fallback only when no sandbox mode was configured.
  • Preserved the existing trust-decision fallback: trusted and untrusted projects default to workspace-write where supported.
  • Added empty-config coverage asserting that an untrusted project resolves to the built-in active permission profile (:workspace outside unsandboxed Windows).

Verification

  • just fmt
  • just test -p codex-core 'config::'
  • just test -p codex-config

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest requested a review from a team as a code owner June 2, 2026 19:30
@bolinfest bolinfest force-pushed the pr25926 branch 2 times, most recently from 964eb85 to 5750eb9 Compare June 2, 2026 21:35
@bolinfest bolinfest changed the base branch from main to pr25943 June 2, 2026 21:35
@bolinfest bolinfest force-pushed the pr25943 branch 2 times, most recently from bcaf711 to 8c38bcf Compare June 2, 2026 21:41
@bolinfest bolinfest force-pushed the pr25926 branch 2 times, most recently from 87a13de to c1485dd Compare June 2, 2026 21:44
bolinfest added a commit that referenced this pull request Jun 2, 2026
## Why

`profile_sandbox_mode` was left over from the old selected legacy
profile path. Production now always derives permissions without that
value, and legacy profile contents are ignored, so keeping a parameter
that is always `None` makes `derive_permission_profile` look like it
still supports a fallback that no longer exists.

## What Changed

- Removed the `profile_sandbox_mode` argument from
`ConfigToml::derive_permission_profile`.
- Updated the production caller and legacy sandbox-policy test helper to
match.
- Dropped the stale unselected legacy-profile sandbox test that only
protected the removed fallback shape.

## Verification

- `just test -p codex-config`
- `just test -p codex-core 'config::'`


---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/25943).
* #25926
* __->__ #25943
Base automatically changed from pr25943 to main June 2, 2026 22:05
@bolinfest bolinfest force-pushed the pr25926 branch 2 times, most recently from 9f2643d to 9ab5f3c Compare June 2, 2026 22:39
@bolinfest bolinfest changed the title config: default untrusted projects to read-only permissions config: express implicit sandbox defaults as permission profiles Jun 2, 2026
@bolinfest bolinfest requested a review from viyatb-oai June 2, 2026 22:56
@bolinfest bolinfest merged commit a28b32a into main Jun 2, 2026
59 of 62 checks passed
@bolinfest bolinfest deleted the pr25926 branch June 2, 2026 23:26
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 2, 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