Skip to content

tui: sync session permission profiles#18284

Merged
viyatb-oai merged 1 commit intomainfrom
pr18284
Apr 23, 2026
Merged

tui: sync session permission profiles#18284
viyatb-oai merged 1 commit intomainfrom
pr18284

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 17, 2026

Why

Once SessionConfigured carries the active PermissionProfile, the TUI must treat that as authoritative session state. Otherwise the widget can keep stale local permission details after a session is configured or resumed.

The TUI also keeps a local Config copy used for later operations, so session-sourced profiles and subsequent local sandbox changes need to keep the derived split runtime permissions in sync. Because this PR may land before the follow-up user-turn profile plumbing, embedded app-server turns also need a standalone path for carrying local runtime sandbox overrides.

What changed

  • Sync the chat widget runtime filesystem/network permissions from SessionConfigured.permission_profile, with the legacy sandbox_policy as the fallback.
  • Recompute split runtime permissions whenever the TUI applies or carries forward a local sandbox-policy override.
  • Mark feature-driven Auto-review sandbox changes as runtime sandbox overrides so the standalone embedded turn-start profile path is used even without the follow-up user-turn profile PR.
  • Send a turn-start permissionProfile for embedded, non-ExternalSandbox turns when the TUI has a runtime sandbox override; remote and ExternalSandbox turns keep using the legacy sandbox field.
  • Extend coverage for profile sync, local sandbox changes, ExternalSandbox fallback, feature-driven sandbox overrides, and turn-start permission override selection.

Verification

  • cargo test -p codex-tui update_feature_flags_enabling_guardian_selects_auto_review
  • cargo test -p codex-tui turn_start_permission_overrides_send_profiles_only_for_embedded_runtime_overrides
  • cargo test -p codex-tui permission_settings_sync
  • cargo test -p codex-tui session_configured_external_sandbox_keeps_external_runtime_policy
  • cargo test -p codex-tui session_configured_syncs_widget_config_permissions_and_cwd
  • just fix -p codex-tui

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest force-pushed the pr18284 branch 2 times, most recently from dc15de8 to ffac8a6 Compare April 17, 2026 17:08
@bolinfest bolinfest force-pushed the pr18283 branch 2 times, most recently from 8b4d2a2 to e5f2e57 Compare April 17, 2026 19:33
bolinfest added a commit that referenced this pull request Apr 22, 2026
## Why

`Permissions` should not store a separate `PermissionProfile` that can
drift from the constrained `SandboxPolicy` and network settings. The
active profile needs to be derived from the same constrained values that
already honor `requirements.toml`.

## What changed

This adds derivation of the active `PermissionProfile` from the
constrained runtime permission settings and exposes that derived value
through config snapshots and thread state. The app-server can then
report the active profile without introducing a second source of truth.

## Verification

- `cargo test -p codex-core --test all permissions_messages --
--nocapture`
- `cargo test -p codex-core --test all request_permissions --
--nocapture`



























---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18277).
* #18288
* #18287
* #18286
* #18285
* #18284
* #18283
* #18282
* #18281
* #18280
* #18279
* #18278
* __->__ #18277
@bolinfest bolinfest force-pushed the pr18284 branch 2 times, most recently from f8c3ebf to c673808 Compare April 22, 2026 06:53
@bolinfest bolinfest force-pushed the pr18283 branch 2 times, most recently from 37a49c3 to d9bfa96 Compare April 22, 2026 17:53
@bolinfest bolinfest requested a review from a team as a code owner April 22, 2026 18:10
bolinfest added a commit that referenced this pull request Apr 22, 2026
## Why

`PermissionProfile` is becoming the canonical permissions shape shared
by core and app-server. After app-server responses expose the active
profile, clients need to be able to send that same shape back when
starting, resuming, forking, or overriding a turn instead of translating
through the legacy `sandbox`/`sandboxPolicy` shorthands.

This still needs to preserve the existing requirements/platform
enforcement model. A profile-shaped request can be downgraded or
rejected by constraints, but the server should keep the user's
elevated-access intent for project trust decisions. Turn-level profile
overrides also need to retain existing read protections, including
deny-read entries and bounded glob-scan metadata, so a permission
override cannot accidentally drop configured protections such as
`**/*.env = deny`.

## What changed

- Adds optional `permissionProfile` request fields to `thread/start`,
`thread/resume`, `thread/fork`, and `turn/start`.
- Rejects ambiguous requests that specify both `permissionProfile` and
the legacy `sandbox`/`sandboxPolicy` fields, including running-thread
resume requests.
- Converts profile-shaped overrides into core runtime filesystem/network
permissions while continuing to derive the constrained legacy sandbox
projection used by existing execution paths.
- Preserves project-trust intent for profile overrides that are
equivalent to workspace-write or full-access sandbox requests.
- Preserves existing deny-read entries and `globScanMaxDepth` when
applying turn-level `permissionProfile` overrides.
- Updates app-server docs plus generated JSON/TypeScript schema fixtures
and regression coverage.

## Verification

- `cargo test -p codex-app-server-protocol schema_fixtures`
- `cargo test -p codex-core
session_configuration_apply_permission_profile_preserves_existing_deny_read_entries`







---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18279).
* #18288
* #18287
* #18286
* #18285
* #18284
* #18283
* #18282
* #18281
* #18280
* __->__ #18279
@bolinfest bolinfest force-pushed the pr18284 branch 2 times, most recently from 47f27ce to 50fffd0 Compare April 22, 2026 21:49
Comment thread codex-rs/tui/src/chatwidget.rs Outdated
@viyatb-oai
Copy link
Copy Markdown
Collaborator

viyatb-oai commented Apr 23, 2026

[P1] Preserve runtime sandbox changes on embedded turns

This issue still appears on the rebased stack, but codex-rs/tui/src/app_server_session.rs is no longer in this PR diff, so I am leaving it as a PR-level note instead of an inline comment. Embedded turn/start still omits both sandboxPolicy and permissionProfile for every non-ExternalSandbox turn, and OverrideTurnContext is still swallowed on the app-server path. That means TUI runtime sandbox changes such as /sandbox or approval-preset updates only change local/chat config; the next embedded turn does not carry the updated policy to app-server core, so a thread can keep running under the previous sandbox after switching read-only/workspace-write. Please distinguish “no permission change” from “new runtime sandbox override”, or send the corresponding permission profile when the sandbox policy has changed.

Suggested small fix: when /sandbox or an approval preset updates the legacy sandbox, also rebuild the split permissions in the affected config copies, then send a profile only while the TUI has a runtime sandbox override.

// config/chat sandbox setters, after the legacy sandbox_policy.set(...)
config.permissions.file_system_sandbox_policy =
    FileSystemSandboxPolicy::from_legacy_sandbox_policy(
        config.permissions.sandbox_policy.get(),
        &config.cwd,
    );
config.permissions.network_sandbox_policy =
    NetworkSandboxPolicy::from(config.permissions.sandbox_policy.get());

Then thread permission_profile: Option<PermissionProfile> through AppServerSession::turn_start and pass it from try_submit_active_thread_op_via_app_server only for embedded, non-ExternalSandbox runtime overrides:

let permission_profile = if !app_server.is_remote()
    && self.runtime_sandbox_policy_override.is_some()
    && !matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. })
{
    Some(self.config.permissions.permission_profile())
} else {
    None
};

turn_start can keep sending the legacy sandboxPolicy for remote/ExternalSandbox, but for this embedded runtime-change case it should set permissionProfile: permission_profile.map(Into::into) instead of None. That keeps ordinary turns from re-projecting permissions, while still applying real runtime sandbox changes to the core session.

@bolinfest
Copy link
Copy Markdown
Collaborator Author

[codex] Addressed on the current head (9afc4e9eec80): /sandbox and Auto-review sandbox changes now rebuild split runtime permissions, mark runtime_sandbox_policy_override, and embedded non-ExternalSandbox turn/start sends a permissionProfile only for that runtime-override case. Remote and ExternalSandbox turns still use the legacy sandboxPolicy fallback.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants