Skip to content

tests: isolate approval fixtures from host rules#18288

Merged
bolinfest merged 1 commit into
mainfrom
pr18288
Apr 23, 2026
Merged

tests: isolate approval fixtures from host rules#18288
bolinfest merged 1 commit into
mainfrom
pr18288

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 17, 2026

Why

Several approval-focused tests were unintentionally sensitive to host-level rule files. On machines with broader allowed command prefixes, commonly allowed commands such as /bin/date could bypass the approval path these tests were meant to exercise, making the fixtures depend on the developer or CI host configuration.

What changed

  • Pins the approval matrix fixture to the explicit user reviewer so it does not inherit a host reviewer.
  • Changes OTel approval fixtures to request /usr/bin/touch codex-otel-approval-test, avoiding a command that may be pre-approved by local rules.
  • Clears the config layer stack for the permissions-message assertion that needs to compare only the permissions text under test.

Verification

  • env -u CODEX_SANDBOX_NETWORK_DISABLED cargo test -p codex-core --test all approval_matrix_covers_all_modes -- --nocapture
  • env -u CODEX_SANDBOX_NETWORK_DISABLED cargo test -p codex-core --test all permissions_messages -- --nocapture
@bolinfest bolinfest force-pushed the pr18287 branch 2 times, most recently from 752c140 to f40015e Compare April 17, 2026 18:23
@bolinfest bolinfest force-pushed the pr18288 branch 2 times, most recently from 44bf27d to 9afbb4b Compare April 17, 2026 19:33
@bolinfest bolinfest requested a review from a team as a code owner April 20, 2026 17:09
bolinfest added a commit that referenced this pull request Apr 22, 2026
## Why

#18275 anchors session-scoped `:cwd` and `:project_roots` grants to the
request cwd before recording them for reuse. Relative deny glob entries
need the same treatment. Without anchoring, a stored session permission
can keep a pattern such as `**/*.env` relative, then reinterpret that
deny against a later turn cwd. That makes the persisted profile depend
on the cwd at reuse time instead of the cwd that was reviewed and
approved.

## What changed

`intersect_permission_profiles` now materializes retained
`FileSystemPath::GlobPattern` entries against the request cwd, matching
the existing materialization for cwd-sensitive special paths.

Materialized accepted grants are now deduplicated before deny retention
runs. This keeps the sticky-grant preapproval shape stable when a
repeated request is merged with the stored grant and both `:cwd = write`
and the materialized absolute cwd write are present.

The preapproval check compares against the same materialized form, so a
later request for the same cwd-relative deny glob still matches the
stored anchored grant instead of re-prompting or rejecting.

Tests cover both the storage path and the preapproval path: a
session-scoped `:cwd = write` grant with `**/*.env = none` is stored
with both the cwd write and deny glob anchored to the original request
cwd, cannot be reused from a later cwd, and remains preapproved when
re-requested from the original cwd after merging with the stored grant.

## Verification

- `cargo test -p codex-sandboxing policy_transforms`
- `cargo test -p codex-core --lib
relative_deny_glob_grants_remain_preapproved_after_materialization`
- `cargo clippy -p codex-sandboxing --tests -- -D
clippy::redundant_clone`
- `cargo clippy -p codex-core --lib -- -D clippy::redundant_clone`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18867).
* #18288
* #18287
* #18286
* #18285
* #18284
* #18283
* #18282
* #18281
* #18280
* #18279
* #18278
* #18277
* #18276
* __->__ #18867
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 pr18288 branch 2 times, most recently from cf92f0c to 72268ce Compare April 22, 2026 06:53
@bolinfest bolinfest force-pushed the pr18287 branch 2 times, most recently from 8d03956 to 342a6da Compare April 22, 2026 15:22
@bolinfest bolinfest force-pushed the pr18288 branch 2 times, most recently from 6a06087 to 82737b7 Compare April 22, 2026 18:11
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 pr18288 branch 2 times, most recently from 4696169 to 013c20f Compare April 22, 2026 21:23
@bolinfest bolinfest force-pushed the pr18287 branch 2 times, most recently from 5de9a66 to f677a22 Compare April 22, 2026 21:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants