Skip to content

fix: prompt for unsafe commands on Windows#9117

Merged
bolinfest merged 1 commit intomainfrom
pr9117
Jan 13, 2026
Merged

fix: prompt for unsafe commands on Windows#9117
bolinfest merged 1 commit intomainfrom
pr9117

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Jan 12, 2026

This fixes a bug where Codex was allowing unsafe commands to run when SandboxPolicy::ReadOnly was set on Windows (but should not have).

There is some understandable confusion in the codebase, as we currently lack a variant of SandboxPolicy to express the case where "there is no sandbox available, not because the user did --yolo, but because there just isn't one, sorry."

Because many places in our code require a SandboxPolicy as a parameter, rather than pass Option<SandboxPolicy> around everywhere, we simply default to SandboxPolicy::ReadOnly on Windows. On Mac and Linux, we provide an actual read-only sandbox such that we can allow commands to run freely and rely on the sandbox to prevent anything bad from happening. However, the same assumption does not apply on Windows.

Ultimately, the proper fix is to introduce a new variant to SandboxPolicy to accurately represent the current sandbox state, but for now, I ported the requires_initial_appoval() function in is_dangerous_command.rs that contained the existing logic over to render_decision_for_unmatched_command() in exec_policy.rs and cleaned it up. Note that this function was originally introduced in #6380 for unified exec, but things have changed such that it has been repurposed for, and used exclusively by, execpolicy. The migrated function is now private to exec_policy.rs so it is not picked up by anyone else.

To verify this fixes the existing behavior, I added only the verify_approval_requirement_for_unsafe_powershell_command() test from this PR on main and verified that it failed as expected, returning ExecApprovalRequirement::Skip instead of ExecApprovalRequirement::NeedsApproval for pwsh -Command 'echo hi @(calc)' on Windows. With the changes to render_decision_for_unmatched_command(), now it passes.

@bolinfest bolinfest changed the title fix requires_initial_appoval fix: prompt for unsafe commands on Windows Jan 13, 2026
@bolinfest bolinfest merged commit ddae70b into main Jan 13, 2026
63 of 70 checks passed
@bolinfest bolinfest deleted the pr9117 branch January 13, 2026 05:30
@github-actions github-actions bot locked and limited conversation to collaborators Jan 13, 2026
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.

3 participants