Skip to content

tests: isolate approval fixtures from host rules#18288

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

tests: isolate approval fixtures from host rules#18288
bolinfest merged 1 commit intomainfrom
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants