Skip to content

permissions: add built-in default profiles#19900

Open
bolinfest wants to merge 1 commit intopr19899from
pr19900
Open

permissions: add built-in default profiles#19900
bolinfest wants to merge 1 commit intopr19899from
pr19900

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 28, 2026

Why

The migration away from SandboxPolicy needs new configs to start from permissions profiles instead of deriving profiles from legacy sandbox modes. At the same time, existing users may have empty config.toml files, and we should not rewrite user-owned config files that may live in shared repositories.

This PR introduces built-in profile names so an empty config can resolve to a canonical PermissionProfile, while explicit named [permissions] profiles still behave predictably.

What changed

  • Adds built-in default_permissions profile names:
    • :read-only maps to PermissionProfile::read_only().
    • :workspace maps to the workspace-write profile, including project-root metadata carveouts.
    • :danger-no-sandbox maps to PermissionProfile::Disabled, preserving the distinction between no sandbox and a broad managed sandbox.
  • Reserves the : prefix for built-in profiles so user-defined [permissions] profiles cannot collide with future built-ins.
  • Allows default_permissions to reference a built-in profile without requiring a [permissions] table.
  • Makes an otherwise empty config choose a built-in profile by trust/platform context: trusted or untrusted project roots use :workspace when the platform supports that sandbox, while roots without a trust decision use :read-only.
  • Leaves legacy sandbox_mode configs on the legacy path, and still rejects user-defined [permissions] profiles that omit default_permissions so we do not silently guess among custom profiles.

Verification

  • cargo test -p codex-core builtin --lib

Documentation

Public Codex config docs should mention these built-in names when the [permissions] config format is ready to document as stable.


Stack created with Sapling. Best reviewed with ReviewStack.

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: 393a21dcd6

ℹ️ 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 on lines 1874 to +1877
let profiles_are_active = matches!(
permission_config_syntax,
Some(PermissionConfigSyntax::Profiles)
) || (permission_config_syntax.is_none()
&& has_permission_profiles);
) || permission_config_syntax.is_none();
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.

P1 Badge Preserve implicit workspace-write overrides in default syntax

Treating permission_config_syntax.is_none() as profiles_are_active routes configs with no explicit permission syntax through built-in profiles, which bypasses ConfigToml::derive_permission_profile and therefore ignores [sandbox_workspace_write] knobs. A trusted/untrusted project that previously relied on implicit workspace-write plus sandbox_workspace_write (for network_access or extra writable_roots) will silently lose those settings after this change, changing runtime permissions for existing configs.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant