Skip to content

windows-sandbox: accept permission profiles from callers#20455

Closed
bolinfest wants to merge 1 commit into
pr20452from
pr20455
Closed

windows-sandbox: accept permission profiles from callers#20455
bolinfest wants to merge 1 commit into
pr20452from
pr20455

Conversation

@bolinfest

@bolinfest bolinfest commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Why

The TUI had to create legacy SandboxPolicy values before asking the Windows sandbox setup and world-writable scan helpers to run. That kept legacy conversion logic in UI code even though the Windows sandbox backend is the only consumer that still needs the legacy policy JSON.

This continues the permissions migration by making core's Windows sandbox boundary accept PermissionProfile directly and localizing the compatibility projection inside that boundary.

What changed

  • Updated codex_core::windows_sandbox setup/preflight/read-root refresh helpers to take PermissionProfile instead of SandboxPolicy.
  • Added a core wrapper for apply_world_writable_scan_and_denies() that takes a PermissionProfile and derives the legacy policy internally.
  • Removed TUI-side legacy policy construction from Windows elevated setup, legacy setup preflight, and world-writable scan paths.
  • Removed a duplicate compatibility projection from windows_sandbox_read_grants.

Verification

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

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest requested a review from a team as a code owner April 30, 2026 16:23
@bolinfest bolinfest changed the base branch from main to pr20452 April 30, 2026 16:23
This was referenced Apr 30, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Apr 30, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

💡 Codex Review

codex/codex-rs/app-server/src/codex_message_processor.rs

Line 9584 in bb346d3

let sandbox_cwd = cwd.unwrap_or(snapshot.cwd.as_path());

P1 Badge Normalize turn cwd before projecting legacy sandbox policy

When turn/start uses legacy sandboxPolicy plus a relative cwd, this helper passes the raw relative path into from_legacy_sandbox_policy_preserving_deny_entries. That conversion only applies workspace default protected subpaths when cwd is absolute, so relative overrides can skip expected deny carveouts (for example under the workdir) and produce a looser PermissionProfile than before. Prior behavior normalized cwd in core before projecting legacy policy, so this is a regression for relative-cwd callers.

codex/codex-rs/core/src/session/session.rs

Line 196 in bb346d3

if let Some(permission_profile) = updates.permission_profile.clone() {

P2 Badge Preserve legacy sandbox overrides when permission profile is absent

After removing sandbox_policy from Op::UserInputWithTurnContext/OverrideTurnContext, older payloads that still send only sandbox_policy deserialize with permission_profile = None (unknown fields are ignored), and this update path now becomes a no-op because settings changes are applied only when permission_profile is Some. In mixed-version scenarios, sandbox overrides are silently dropped and the previous session permissions remain active.

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

This was referenced Apr 30, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR.

@github-actions github-actions Bot closed this May 15, 2026
@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@bolinfest