Skip to content

core: derive built-in permission profiles from raw policies#25739

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

core: derive built-in permission profiles from raw policies#25739
bolinfest merged 1 commit into
mainfrom
pr25739

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Jun 2, 2026

Why

Permission profiles that extend a built-in profile should behave like other TOML inheritance: parent entries provide defaults, and child keys override matching fields before the profile is compiled.

That was not true for :workspace. Previously, a profile with extends = ":workspace" seeded the compiled runtime PermissionProfile::workspace_write() policy and then appended child filesystem entries. A child override such as ":tmpdir" = "read" therefore left the inherited ":tmpdir" = "write" entry in the final policy. Since same-target write wins over read during runtime resolution, the child override was ineffective.

This also needs a clear source of truth for the built-in profiles. The protocol-level sandbox policy constructors now define the raw built-in filesystem entries, and both PermissionProfile presets and config-profile inheritance derive from those same values.

What Changed

  • Add a canonical FileSystemSandboxPolicy::read_only() constructor while keeping the read-only and workspace-write raw filesystem entries explicit and independent.
  • Derive PermissionProfile::read_only() from FileSystemSandboxPolicy::read_only(); PermissionProfile::workspace_write() continues to derive from FileSystemSandboxPolicy::workspace_write().
  • Build extensible :read-only and :workspace parent profiles by projecting those canonical sandbox policies into PermissionProfileToml, then merge user overrides at the TOML layer before compilation.
  • Add config parsing support for :slash_tmp so the built-in :workspace parent can be expressed in the same TOML-shaped filesystem table as user profiles.
  • Document that PermissionsToml::resolve_profile() returns an already-merged PermissionProfileToml, and return that profile directly after removing the resolved-profile wrapper.
  • Extend the config test for extends = ":workspace" to assert that inherited ":slash_tmp" = "write" is preserved and that a child ":tmpdir" = "read" entry replaces the inherited write entry.

Verification

  • just test -p codex-config
  • just test -p codex-protocol
  • just test -p codex-core permissions_profiles_resolve_extends_parent_first_with_child_overrides
  • just test -p codex-core default_permissions_profile_can_extend_builtin_workspace
  • just test -p codex-core
    • Result: 2596 passed, 4 failed, 1 timed out.
    • The failures were existing sandbox/environment-sensitive tests unrelated to this permissions change:
      suite::user_shell_cmd::user_shell_command_does_not_set_network_sandbox_env_var,
      suite::user_shell_cmd::user_shell_command_history_is_persisted_and_shared_with_model,
      suite::abort_tasks::interrupt_persists_turn_aborted_marker_in_next_request,
      suite::abort_tasks::interrupt_tool_records_history_entries, and
      thread_manager::tests::start_thread_uses_all_default_environments_from_codex_home.

@bolinfest bolinfest requested a review from a team as a code owner June 2, 2026 01:06
@bolinfest bolinfest changed the title core: merge built-in permission profiles as TOML core: derive built-in permission profiles from raw policies Jun 2, 2026
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.

Looks good overall. Please add an additional regression test for inherited :slash_tmp. The new test covers :tmpdir, but would still pass if the parser branch at codex-rs/core/src/config/permissions.rs:787 vanished, silently dropping /tmp writes from profiles extending :workspace.

@bolinfest bolinfest force-pushed the pr25739 branch 3 times, most recently from d965c89 to e287f53 Compare June 2, 2026 16:04
@bolinfest bolinfest merged commit 593df87 into main Jun 2, 2026
59 of 78 checks passed
@bolinfest bolinfest deleted the pr25739 branch June 2, 2026 17:57
@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