Skip to content

windows-sandbox: fail elevated setup when firewall policy is ineffective#22353

Open
iceweasel-oai wants to merge 1 commit into
mainfrom
codex/windows-firewall-policy-effective
Open

windows-sandbox: fail elevated setup when firewall policy is ineffective#22353
iceweasel-oai wants to merge 1 commit into
mainfrom
codex/windows-firewall-policy-effective

Conversation

@iceweasel-oai
Copy link
Copy Markdown
Collaborator

Why

Elevated Windows sandbox setup currently assumes that the firewall rules it writes will take effect. On managed Windows hosts, local firewall policy changes can be ignored or only partially apply across the active profiles, which means setup can appear to succeed without providing the expected network isolation.

What changed

  • Query INetFwPolicy2::LocalPolicyModifyState before configuring the elevated sandbox firewall rules.
  • Fail setup when Windows reports that local firewall policy edits are ineffective or only apply to some current profiles.
  • Surface that condition with a dedicated helper_firewall_policy_ineffective setup error code so support and IT-facing diagnostics can distinguish it from COM access failures.
  • Add focused coverage for effective policy, group-policy override, and partial-profile coverage cases.

Testing

  • cargo test -p codex-windows-sandbox --bin codex-windows-sandbox-setup

@iceweasel-oai iceweasel-oai requested a review from viyatb-oai May 12, 2026 17:17
Copy link
Copy Markdown
Contributor

@evawong-oai evawong-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated on AWS Windows VM against head ae1ef5c.

  1. Fetched PR 22353 on EC2AMAZ 6M6C40J and confirmed HEAD matched.
  2. Ran cargo test for codex windows sandbox setup. Result 7 passed and 0 failed, including the LocalPolicyModifyState cases and firewall COM scope cases.
  3. Built codex cli and codex windows sandbox bins successfully in the prior pass.

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.

3 participants