permissions: support workspace roots in profiles by bolinfest · Pull Request #22610 · openai/codex

bolinfest · 2026-05-14T05:01:48Z

Why

This is the configuration/model half of the alternative permissions migration we discussed as a comparison point for #22401 and #22402.

The old workspace-write model mixes three concerns that we want to keep separate:

reusable profile rules that should stay immutable once selected
user/runtime workspace roots from cwd, --add-dir, and legacy workspace-write config
internal Codex writable roots such as memories, which should not be shown as user workspace roots

This PR gives permission profiles first-class workspace_roots so users can opt multiple repositories into the same :workspace_roots rules without using broad absolute-path write grants. It also starts separating the raw selected profile from the effective runtime profile by making Permissions expose explicit accessors instead of public mutable fields.

A representative config.toml looks like this:

default_permissions = "dev"

[permissions.dev.workspace_roots]
"~/code/openai" = true
"~/code/developers-website" = true

[permissions.dev.filesystem.":workspace_roots"]
"." = "write"
".codex" = "read"
".git" = "read"
".vscode" = "read"

If Codex starts in ~/code/codex with that profile selected, the effective workspace-root set becomes:

~/code/codex from the runtime cwd
~/code/openai from the profile
~/code/developers-website from the profile

The :workspace_roots rules are materialized across each root, so .git, .codex, and .vscode stay scoped the same way everywhere. Runtime additions such as --add-dir can still layer on later stack entries without mutating the selected profile.

Stack Shape

This PR intentionally stops before the profile-identity cleanup in #22683 so the base review stays focused on config loading, workspace-root materialization, and compatibility with legacy workspace-write.

The representation in this PR is therefore transitional: Permissions carries enough state to distinguish the raw constrained profile from the effective runtime profile, and there are still call sites that must keep the active profile identity and constrained profile value in sync. The follow-up PR replaces that with a single resolved profile state (ResolvedPermissionProfile / PermissionProfileState) that keeps the profile id, immutable PermissionProfile, and profile-declared workspace roots together. That follow-up removes APIs such as set_constrained_permission_profile_with_active_profile() where separate arguments could drift out of sync.

Downstream PRs then build on this base to switch app-server turn updates to profile ids plus runtime workspace roots and to finish the user-visible summary behavior. Reviewers should judge this PR as the workspace-roots foundation, not as the final in-memory shape of selected permission profiles.

Review Guide

Suggested review order:

Start with codex-rs/core/src/config/mod.rs.
This is the main shape change in the base slice. Permissions now stores a private raw Constrained<PermissionProfile> plus runtime workspace_roots. Callers use permission_profile() when they need the raw constrained value and effective_permission_profile() when they need a materialized runtime profile. As noted above, #22683 replaces this transitional shape with a resolved profile state that keeps identity and profile data together.
Review codex-rs/config/src/permissions_toml.rs and codex-rs/core/src/config/permissions.rs.
These add [permissions.<id>.workspace_roots], resolve enabled entries relative to the policy cwd, and keep :workspace_roots deny-read glob patterns symbolic until the actual roots are known.
Review codex-rs/protocol/src/permissions.rs and codex-rs/protocol/src/models.rs.
These add the policy/profile materialization helpers that expand exact :workspace_roots entries and scoped deny-read globs over every workspace root. This is also where ActivePermissionProfileModification is removed from the core model.
Review the legacy bridge in Config::load_from_base_config_with_overrides and Config::set_legacy_sandbox_policy.
This is where legacy workspace-write roots become runtime workspace roots, while Codex internal writable roots stay internal and do not appear as user-facing workspace roots.
Then skim downstream call sites.
The interesting pattern is raw-vs-effective access: state/proxy/bwrap paths keep the raw constrained profile, while execution, summaries, and user-visible status use the effective profile and workspace-root list.

What Changed

added [permissions.<id>.workspace_roots] to the config model and schema
added runtime workspace_roots state to Config/Permissions and ConfigOverrides
made Permissions profile fields private and replaced direct mutation with accessors/setters
added PermissionProfile and FileSystemSandboxPolicy helpers for materializing :workspace_roots exact paths and deny-read globs across all roots
moved legacy additional writable roots into runtime workspace-root state instead of active profile modifications
removed ActivePermissionProfileModification and its app-server protocol/schema export
updated sandbox/status summary paths so internal writable roots are not reported as user workspace roots

Verification Strategy

The targeted tests cover the behavior at the layers where regressions are most likely:

codex-rs/core/src/config/config_tests.rs verifies config loading, legacy workspace-root seeding, effective profile materialization, and memory-root handling.
codex-rs/core/src/config/permissions_tests.rs verifies profile workspace_roots parsing and :workspace_roots scoped/glob compilation.
codex-rs/protocol/src/permissions.rs unit tests verify exact and glob materialization over multiple workspace roots.
codex-rs/tui/src/status/tests.rs and codex-rs/utils/sandbox-summary/src/sandbox_summary.rs verify the user-facing summaries show effective workspace roots and hide internal writes.

I also ran cargo check --tests locally after the latest stack refresh to catch cross-crate API breakage from the private-field/accessor changes.

Stack created with Sapling. Best reviewed with ReviewStack.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c3262458d

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

chatgpt-codex-connector · 2026-05-14T05:11:12Z

                approval_policy: constrained_approval_policy.value,
                permission_profile: constrained_permission_profile.value,
                active_permission_profile,
+                workspace_roots,


Carry runtime workspace roots into sessions

When a caller supplies runtime workspace roots (--add-dir / ConfigOverrides.workspace_roots), they are only stored in this new Permissions.workspace_roots side field. The session path later copies config.permissions.permission_profile.clone() into SessionConfiguration and does not carry this field, so actual turn sandboxes materialize :project_roots only against the cwd and drop the extra roots; for example codex --add-dir ../backend can show the backend root in config/status while shell/apply_patch turns cannot write it, and profile deny globs for that runtime root are not enforced. Materialize these roots into the stored profile before session creation or pass the workspace-root list through the session/turn configuration.

Useful? React with 👍 / 👎.

Addressed in the current head. Session creation now stores the effective permission profile after materializing runtime workspace roots, so --add-dir / runtime roots are carried into turn sandboxes instead of only appearing in config/status. The focused history-replay coverage for this path passes on the current PR head.

chatgpt-codex-connector · 2026-05-14T05:11:12Z

    /// inheritance. This is always `None` until that config feature exists.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    #[ts(optional)]
    pub extends: Option<String>,


Restore active profile modifications

Removing the modifications member here leaves existing callers broken: codex-rs/app-server-protocol/src/protocol/v2/permissions.rs still converts CoreActivePermissionProfileModification, and both codex-rs/exec/src/lib.rs and codex-rs/tui/src/app_server_session.rs still read active.modifications when round-tripping additional writable roots. Any build that includes those crates now fails, and even after fixing imports this would drop additionalWritableRoot selections from the active-profile API unless all conversions are updated together.

Useful? React with 👍 / 👎.

Addressed in the current head. The app-server protocol conversions and embedded client call sites were updated together: core no longer carries the old active-profile modification type, and the embedded protocol still emits bounded AdditionalWritableRoot modifications where that compatibility path needs them.

chatgpt-codex-connector · 2026-05-14T05:11:12Z

 "codex-otel",
 "codex-protocol",
 "codex-state",
+ "codex-utils-absolute-path",


Remove spurious lockfile dependencies

This lockfile entry now claims codex-rollout depends on codex-utils-absolute-path, but codex-rs/rollout/Cargo.toml was not changed and does not declare that dependency; the same unrelated addition appears for codex-thread-store at line 3711. With the lockfile no longer matching the workspace manifests, Cargo --locked/CI will reject or rewrite it, so regenerate Cargo.lock from the actual manifest changes and drop these unrelated dependency entries.

Useful? React with 👍 / 👎.

Addressed in the current head. The spurious codex-utils-absolute-path entries are no longer present for codex-rollout or codex-thread-store in Cargo.lock, and the current PR head is passing CI.

#22624) ## Why This is a small precursor to the larger permissions-migration work. Both the comparison stack in [#22401](#22401) / [#22402](#22402) and the alternate stack in [#22610](#22610) / [#22611](#22611) / [#22612](#22612) are easier to review if the terminology is already settled underneath them. Because `:project_roots` and `:danger-no-sandbox` have not shipped as stable user-facing surface area, carrying them forward as aliases would just add more migration logic to the later stacks. This PR removes that ambiguity now so the follow-on work can rely on one spelling for each built-in concept. ## What Changed - renamed the config-facing special filesystem key from `:project_roots` to `:workspace_roots` - dropped unpublished `:project_roots` parsing support in `core/src/config/permissions.rs`, so new config only recognizes `:workspace_roots` - renamed the built-in full-access permission profile id from `:danger-no-sandbox` to `:danger-full-access` - dropped unpublished `:danger-no-sandbox` support entirely, including the old active-profile canonicalization path, and added explicit rejection coverage for the legacy id - introduced shared built-in permission-profile id constants in `codex-rs/protocol/src/models.rs` - updated `core`, `app-server`, and `tui` call sites that special-case built-in profiles to use the shared constants and canonical ids - updated tests and the Linux sandbox README to use `:workspace_roots` / `:danger-full-access` ## Verification I focused verification on the three places this rename can regress: config parsing, active-profile identity surfaced back out of `core`, and user/server call sites that special-case built-in profiles. Targeted checks: - `config::tests::default_permissions_can_select_builtin_profile_without_permissions_table` - `config::tests::default_permissions_read_only_applies_additional_writable_roots_as_modifications` - `config::tests::default_permissions_can_select_builtin_full_access_profile` - `config::tests::legacy_danger_no_sandbox_is_rejected` - `workspace_root` filtered `codex-core` tests - `request_processors::thread_processor::thread_processor_tests::thread_processor_behavior_tests::requested_permissions_trust_project_uses_permission_profile_intent` - `suite::v2::turn_start::turn_start_rejects_invalid_permission_selection_before_starting_turn` - `status::tests::status_snapshot_shows_auto_review_permissions` - `status::tests::status_permissions_full_disk_managed_with_network_is_danger_full_access` - `app_server_session::tests::embedded_turn_permissions_use_active_profile_selection`

viyatb-oai · 2026-05-14T20:00:07Z

 fn permissions_selection_from_active_profile(
    active: ActivePermissionProfile,
 ) -> PermissionProfileSelectionParams {
-    let modifications = active
-        .modifications
-        .into_iter()
-        .map(|modification| match modification {
-            ActivePermissionProfileModification::AdditionalWritableRoot { path } => {
-                PermissionProfileModificationParams::AdditionalWritableRoot { path }
-            }
-        })
-        .collect::<Vec<_>>();
    PermissionProfileSelectionParams::Profile {
        id: active.id,
-        modifications: (!modifications.is_empty()).then_some(modifications),
+        modifications: None,
    }
 }


This still needs the caller sites to pass cwd plus the runtime root slice, and to restore the removed PermissionProfileModificationParams import, but the profile handoff should keep emitting bounded writable-root modifications instead of dropping them.

Suggested change

fn permissions_selection_from_active_profile(

active: ActivePermissionProfile,

) -> PermissionProfileSelectionParams {

let modifications = active

.modifications

.into_iter()

.map(|modification| match modification {

ActivePermissionProfileModification::AdditionalWritableRoot { path } => {

PermissionProfileModificationParams::AdditionalWritableRoot { path }

}

})

.collect::<Vec<_>>();

PermissionProfileSelectionParams::Profile {

id: active.id,

modifications: (!modifications.is_empty()).then_some(modifications),

modifications: None,

}

}

fn permissions_selection_from_active_profile(

active: ActivePermissionProfile,

cwd: &std::path::Path,

workspace_roots: &[AbsolutePathBuf],

) -> PermissionProfileSelectionParams {

let modifications = workspace_roots

.iter()

.filter(|root| root.as_path() != cwd)

.cloned()

.map(|path| PermissionProfileModificationParams::AdditionalWritableRoot { path })

.collect::<Vec<_>>();

PermissionProfileSelectionParams::Profile {

id: active.id,

modifications: (!modifications.is_empty()).then_some(modifications),

}

}

Addressed in the current head. permissions_selection_from_active_profile now takes cwd and the runtime workspace-root slice, emits AdditionalWritableRoot only for non-cwd roots, and there is embedded-turn coverage for extra workspace roots.

viyatb-oai · 2026-05-14T20:00:07Z

        let approval_policy = AskForApproval::from(config.permissions.approval_policy.value());
-        let permission_profile = config.permissions.permission_profile();
+        let permission_profile = config.permissions.effective_permission_profile();
+        let workspace_roots = config.permissions.workspace_roots();
        let mut config_entries = vec![


The status surface needs a workspace-root slice that includes both runtime roots and profile-declared roots. This sketch keeps the summary call honest by making that combined/user-visible root set explicit at the call site.

Suggested change

let approval_policy = AskForApproval::from(config.permissions.approval_policy.value());

let permission_profile = config.permissions.permission_profile();

let permission_profile = config.permissions.effective_permission_profile();

let workspace_roots = config.permissions.workspace_roots();

let mut config_entries = vec![

let approval_policy = AskForApproval::from(config.permissions.approval_policy.value());

let permission_profile = config.permissions.effective_permission_profile();

let workspace_roots = config.permissions.user_visible_workspace_roots();

let mut config_entries = vec![

Addressed in the current head. The status card now summarizes config.permissions.effective_permission_profile() with config.permissions.user_visible_workspace_roots(), so the displayed sandbox includes runtime roots plus profile-declared roots while still hiding internal writable roots.

viyatb-oai · 2026-05-14T20:00:07Z

            summarize_permission_profile(
-                config.permissions.permission_profile.get(),
+                &config.permissions.effective_permission_profile(),
                config.cwd.as_path(),
            ),


Please route headless output through the same summary helper the TUI uses so internal writable roots stay hidden. This also needs the codex-utils-sandbox-summary dependency wired into codex-exec, and the local formatter below can then go away.

Suggested change

summarize_permission_profile(

config.permissions.permission_profile.get(),

&config.permissions.effective_permission_profile(),

config.cwd.as_path(),

),

codex_utils_sandbox_summary::summarize_permission_profile(

&config.permissions.effective_permission_profile(),

&config.cwd,

config.permissions.workspace_roots(),

),

Addressed in the current head. Headless output now uses the shared codex-utils-sandbox-summary helper with the effective permission profile and user_visible_workspace_roots(), and codex-exec has the helper dependency wired in.

bolinfest requested a review from a team as a code owner May 14, 2026 05:01

This was referenced May 14, 2026

tui/exec: show effective workspace roots in summaries #22612

Open

app-server: use permission ids and runtime workspace roots #22611

Open

chatgpt-codex-connector Bot reviewed May 14, 2026

View reviewed changes

bolinfest mentioned this pull request May 14, 2026

permissions: canonicalize workspace_roots and danger-full-access names #22624

Merged

bolinfest force-pushed the pr22610 branch from 6c32624 to d8553d1 Compare May 14, 2026 15:59

bolinfest requested a review from viyatb-oai May 14, 2026 16:07

bolinfest force-pushed the pr22610 branch 8 times, most recently from 6589c91 to 5e3f04d Compare May 14, 2026 18:39

bolinfest mentioned this pull request May 14, 2026

permissions: resolve profile identity with constraints #22683

Merged

viyatb-oai reviewed May 14, 2026

View reviewed changes

bolinfest force-pushed the pr22610 branch 4 times, most recently from 28c51e6 to 797dbb4 Compare May 14, 2026 22:45

bolinfest requested a review from viyatb-oai May 14, 2026 23:18

bolinfest added the code-reviewed label May 14, 2026

bolinfest force-pushed the pr22610 branch from 797dbb4 to 149cf98 Compare May 15, 2026 00:18

permissions: support workspace roots in profiles

4b39a27

bolinfest force-pushed the pr22610 branch from 149cf98 to 4b39a27 Compare May 15, 2026 00:25

viyatb-oai approved these changes May 15, 2026

View reviewed changes

bolinfest merged commit c25d905 into main May 15, 2026
47 of 62 checks passed

bolinfest deleted the pr22610 branch May 15, 2026 01:25

github-actions Bot locked and limited conversation to collaborators May 15, 2026

Conversation

bolinfest commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Stack Shape

Review Guide

What Changed

Verification Strategy

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bolinfest commented May 14, 2026 •

edited

Loading