Skip to content

app-server: accept command permission profiles#18283

Merged
bolinfest merged 1 commit intomainfrom
pr18283
Apr 23, 2026
Merged

app-server: accept command permission profiles#18283
bolinfest merged 1 commit intomainfrom
pr18283

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 17, 2026

Why

command/exec is another app-server entry point that can run under caller-provided permissions. It needs to accept PermissionProfile directly so command execution is not left behind on SandboxPolicy while thread APIs move forward.

Command-level profiles also need to preserve the semantics clients expect from profile-relative paths. :cwd and cwd-relative deny globs should be anchored to the resolved command cwd for a command-specific profile, while configured deny-read restrictions such as **/*.env = none still need to be enforced because they can come from config or requirements rather than the command override itself.

What Changed

This adds permissionProfile to CommandExecParams, rejects requests that combine it with sandboxPolicy, and converts accepted profiles into the runtime filesystem/network permissions used for command execution.

When a command supplies a profile, the app-server resolves that profile against the command cwd instead of the thread/server cwd. It also preserves configured deny-read entries and globScanMaxDepth on the effective filesystem policy so one-off command overrides cannot drop those read protections. The PR also updates app-server docs/schema fixtures and adds command-exec coverage for accepted, rejected, cwd-scoped, and deny-read-preserving profile paths.

Verification

  • cargo test -p codex-app-server command_exec_permission_profile_cwd_uses_command_cwd
  • cargo test -p codex-app-server command_profile_preserves_configured_deny_read_restrictions
  • cargo test -p codex-app-server command_exec_accepts_permission_profile
  • cargo test -p codex-app-server command_exec_rejects_sandbox_policy_with_permission_profile
  • just fix -p codex-app-server

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest force-pushed the pr18282 branch 2 times, most recently from bc7e463 to 41ebc4d 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 bolinfest force-pushed the pr18283 branch 2 times, most recently from a9a5407 to e600010 Compare April 20, 2026 18:35
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 pr18283 branch 2 times, most recently from 134b830 to f16904c Compare April 22, 2026 06:02
@bolinfest bolinfest force-pushed the pr18282 branch 2 times, most recently from bbabb69 to ab39876 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 force-pushed the pr18282 branch 2 times, most recently from cf96acd to 99782a9 Compare April 22, 2026 19:56
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 pr18283 branch 2 times, most recently from 939a7f0 to e4813ba Compare April 22, 2026 21:23
@bolinfest bolinfest force-pushed the pr18282 branch 2 times, most recently from 94ec857 to 33269ed Compare April 22, 2026 21:49
) = if let Some(permission_profile) = permission_profile {
let permission_profile =
codex_protocol::models::PermissionProfile::from(permission_profile);
let sandbox_policy = match permission_profile.to_legacy_sandbox_policy(&sandbox_cwd) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Resolve command permission profiles against the command cwd

command/exec resolves the requested process cwd above, but this new permissionProfile path still materializes profile entries against self.config.cwd via sandbox_cwd. The command API docs show permissionProfile using current_working_directory next to the request cwd, and profiles can also carry cwd-relative deny globs. With the current code, a command running in cwd: "subdir" with :cwd write grants the server root instead of the command cwd, while relative deny globs constrain the wrong tree. That can deny the intended command or grant broader writes than the client requested. Use the resolved command cwd as the policy cwd for command-specific permission profiles, or require/materialize only cwd-independent profile entries before accepting them.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested minimal patch:

-        let sandbox_cwd = self.config.cwd.clone();
+        let sandbox_cwd = if permission_profile.is_some() {
+            cwd.clone()
+        } else {
+            self.config.cwd.clone()
+        };

That keeps the legacy/default command/exec paths resolving policy-relative entries against the configured server cwd, while making the new command-specific permissionProfile path use the resolved process cwd for both to_legacy_sandbox_policy(...) and the later build_exec_request(...) call.

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