Skip to content

core tests: migrate apply patch turns to profiles#20040

Open
bolinfest wants to merge 1 commit intopr20037from
pr20040
Open

core tests: migrate apply patch turns to profiles#20040
bolinfest wants to merge 1 commit intopr20037from
pr20040

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 28, 2026

Summary

  • Removes SandboxPolicy from the apply-patch CLI test suite.
  • Uses the harness' profile-backed submit helper for danger/no-sandbox turns instead of constructing Op::UserTurn manually with legacy fields.
  • Converts the workspace-write traversal cases to submit PermissionProfile::workspace_write_with(...) directly.

Verification

  • cargo check -p codex-core --tests
  • just fmt

Stack created with Sapling. Best reviewed with ReviewStack.

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

💡 Codex Review

let Ok(sandbox_policy) = permission_profile.to_legacy_sandbox_policy(cwd.as_path())
else {
tx.send(AppEvent::OpenWorldWritableWarningConfirmation {
preset: None,
sample_paths: Vec::new(),
extra_count: 0usize,
failed_scan: true,
});
return;

P2 Badge Use compatibility projection for world-writable scans

On Windows, PermissionProfile::to_legacy_sandbox_policy returns an error for valid managed profiles that write outside the workspace root (for example, custom profiles with non-cwd writable roots). This new early-return path marks the scan as failed and skips apply_world_writable_scan_and_denies, so those sessions always show the fallback warning and never get deny rules applied. Before this change, scan entry points used the legacy compatibility projection, which handled these profiles.

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

bolinfest added a commit that referenced this pull request Apr 28, 2026
## Why

The migration away from `SandboxPolicy` needs new configs to start from
permissions profiles instead of deriving profiles from legacy sandbox
modes. Existing users can have empty `config.toml` files, and we should
not rewrite user-owned config files that may live in shared
repositories.

This PR introduces built-in profile names so an empty config can resolve
to a canonical `PermissionProfile`, while explicit named `[permissions]`
profiles still behave predictably.

## What changed

- Adds built-in `default_permissions` profile names:
  - `:read-only` maps to `PermissionProfile::read_only()`.
- `:workspace` maps to the workspace-write profile, including
project-root metadata carveouts.
- `:danger-no-sandbox` maps to `PermissionProfile::Disabled`, preserving
the distinction between no sandbox and a broad managed sandbox.
- Reserves the `:` prefix for built-in profiles so user-defined
`[permissions]` profiles cannot collide with future built-ins.
- Allows `default_permissions` to reference a built-in profile without
requiring a `[permissions]` table.
- Makes an otherwise empty config choose a built-in profile by
trust/platform context: trusted or untrusted project roots use
`:workspace` when the platform supports that sandbox, while roots
without a trust decision use `:read-only`.
- Keeps legacy `sandbox_mode` configs on the legacy path, and still
rejects user-defined `[permissions]` profiles that omit
`default_permissions` so we do not silently guess among custom profiles.
- Preserves compatibility behavior for implicit defaults: bare
`network.enabled = true` allows runtime network without starting the
managed proxy, explicit profile proxy policy still starts the proxy, and
implicit workspace/add-dir roots keep legacy metadata carveouts.

## Verification

- `cargo test -p codex-core builtin --lib`
- `cargo test -p codex-core profile_network_proxy_config`
- `cargo test -p codex-core
implicit_builtin_workspace_profile_preserves_add_dir_metadata_carveouts`
- `cargo test -p codex-core
permissions_profiles_network_enabled_allows_runtime_network_without_proxy`
- `cargo test -p codex-core
permissions_profiles_proxy_policy_starts_managed_network_proxy`

## Documentation

Public Codex config docs should mention these built-in names when the
`[permissions]` config format is ready to document as stable.









---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19900).
* #20041
* #20040
* #20037
* #20035
* #20034
* #20033
* #20032
* #20030
* #20028
* #20027
* #20026
* #20024
* #20021
* #20018
* #20016
* #20015
* #20013
* #20011
* #20010
* #20008
* __->__ #19900
@bolinfest bolinfest force-pushed the pr20040 branch 2 times, most recently from b1026ee to 737ceca Compare April 28, 2026 20:10
@bolinfest bolinfest force-pushed the pr20040 branch 2 times, most recently from 4c95857 to a9a4664 Compare April 28, 2026 20:18
bolinfest added a commit that referenced this pull request Apr 28, 2026
## Summary
- Move TUI permission state from legacy `SandboxPolicy` values to
canonical `PermissionProfile` values across presets, app events, chat
widget state, app commands, thread routing, and cached thread session
state.
- Keep app-server compatibility boundaries explicit: embedded sessions
send `permissionProfile`, while remote sessions send only a legacy
`sandbox` projection and fall back to read-only when a custom profile
cannot be projected.
- Update status/add-dir UI summaries and snapshots to render the active
permission profile, including workspace profiles selected by the new
built-in defaults.

## Verification
- `rg '\bSandboxPolicy\b' codex-rs/tui -n` returns no matches.
- `cargo test -p codex-tui`
- `cargo check -p codex-tui --tests`
- `cargo test -p codex-tui additional_dirs`
- `just fmt`
- `just fix -p codex-tui`




































---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/20008).
* #20041
* #20040
* #20037
* #20035
* #20034
* #20033
* #20032
* #20030
* #20028
* #20027
* #20026
* #20024
* #20021
* #20018
* #20016
* #20015
* #20013
* #20011
* #20010
* __->__ #20008
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant