Skip to content

session: stop exposing legacy sandbox policy#20450

Open
bolinfest wants to merge 1 commit intopr20449from
pr20450
Open

session: stop exposing legacy sandbox policy#20450
bolinfest wants to merge 1 commit intopr20449from
pr20450

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 30, 2026

Why

SessionConfiguration should expose the canonical permission profile and runtime policies, not a convenience SandboxPolicy projection. The only remaining session-level caller needed a legacy projection as a temporary bridge for cwd-only updates that were originally loaded from legacy-compatible workspace policies.

This keeps that compatibility behavior local to apply() while removing the public-ish session helper that invited new session code to keep depending on SandboxPolicy.

What Changed

  • Removed SessionConfiguration::sandbox_policy().
  • Computes the legacy compatibility projection directly inside SessionConfiguration::apply() for the existing cwd-rebind fallback.
  • Updated session tests to compare against explicit compatibility projections from the active PermissionProfile and runtime filesystem/network policies.

Verification

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

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest requested a review from a team as a code owner April 30, 2026 15:54
@bolinfest bolinfest changed the base branch from main to pr20449 April 30, 2026 15:54
This was referenced Apr 30, 2026
@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

💡 Codex Review

// Current app-server responses include `permissionProfile`. If it is absent,
// keep the TUI on its configured profile rather than rebuilding a lossy
// profile from the legacy `sandbox` compatibility field.
config.permissions.permission_profile()

P1 Badge Use sandbox fallback when permissionProfile is omitted

When thread/start, thread/resume, or thread/fork responses omit permissionProfile (which is still allowed by the v2 protocol’s compatibility defaults), this fallback now returns config.permissions.permission_profile() instead of deriving from the response sandbox. In remote mode, turn_permissions_overrides always sends a sandbox override based on this local profile, so the next turn can silently change a resumed thread’s effective permissions away from what the server reported. This regresses compatibility with servers/clients that still rely on the legacy sandbox field and can cause unexpected approval/sandbox behavior on subsequent turns.

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

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