Add Windows sandbox provisioning setup command#24831
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b76531a2d
ℹ️ 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".
| proxy_ports: Vec::new(), | ||
| allow_local_binding: false, |
There was a problem hiding this comment.
Preserve proxy settings during provisioning
When the managed setup is run for a user whose later Codex session uses an enforced loopback proxy (for example via HTTP_PROXY/HTTPS_PROXY or CODEX_NETWORK_ALLOW_LOCAL_BINDING), this always writes a marker/firewall setup with no proxy ports and allow_local_binding=false. require_logon_sandbox_creds compares that marker against the user's actual proxy settings and treats the difference as setup drift, so it calls the elevated setup path again; for the non-admin users this command is meant to support, that reintroduces the UAC/elevation requirement on first launch instead of using the pre-provisioned sandbox.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is a known limitation of the alpha. proxy changes will require future IT intervention.
| ConfigEditsBuilder::new(identity.codex_home.as_path()) | ||
| .set_windows_sandbox_mode("elevated") | ||
| .apply() |
There was a problem hiding this comment.
Clear legacy sandbox keys when provisioning
If the target CODEX_HOME already contains any legacy Windows sandbox feature key (for example from a previous opt-in/opt-out), resolve_windows_sandbox_mode gives those keys precedence or ignores [windows].sandbox, while this command only writes the new key. The normal setup path calls clear_legacy_windows_sandbox_keys() before persisting the mode, so managed provisioning can report success but leave the desktop/TUI still seeing the old legacy setting and prompting or running with the wrong sandbox mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
these users won't have legacy keys. I decided to skip this for simplicity
| @@ -543,31 +552,178 @@ fn run_read_acl_only(payload: &Payload, log: &mut dyn Write) -> Result<()> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| fn run_setup_full(payload: &Payload, log: &mut dyn Write, sbx_dir: &Path) -> Result<()> { | |||
| fn provision_and_hide_sandbox_users( | |||
There was a problem hiding this comment.
these next 4 functions are not new. I just moved them out into helpers so that they can easily be called from the new sandbox setup command
|
I had Codex perform a supervised end-to-end test of this feature on Windows. The clean-home flow worked: both setup forms completed, created the expected sandbox artifacts, and persisted I also tested a target In that case, I'm not sure if this is worth addressing now or in the future, but I wanted to raise this finding. |
7b76531 to
7e49fc5
Compare
Why
Some Windows users do not have local admin access, so they cannot complete the elevated portion of the Windows sandbox setup when Codex first needs it. This adds an alpha provisioning path that an admin or IT deployment script can run ahead of time for the Codex user.
The intended managed-deployment shape is:
--elevatedis treated as the requested sandbox setup level, not as proof that the process is elevated. The Windows sandbox setup orchestration still checks that the caller is actually elevated before launching the helper without a UAC prompt.What changed
codex sandbox setup --elevatedwith explicit user selection via either--current-useror--user ... --codex-home ....cli/src/sandbox_setup.rsinstead of growingcli/src/main.rs.ProvisionOnlyhelper mode that runs the elevation-required provisioning work without requiring a workspace cwd or runtime sandbox policy.windows.sandbox = "elevated"into the targetCODEX_HOMEso the desktop app does not show the initial sandbox setup banner after pre-provisioning succeeds.Validation
cargo fmt -p codex-windows-sandbox -p codex-core -p codex-clicargo test -p codex-cli sandbox_setup --target-dir target\sandbox-setup-checkcargo test -p codex-windows-sandbox payload_accepts_provision_only_mode --target-dir target\sandbox-setup-checkgit diff --checkMandi Lavida): ran the new setup command from an admin shell, verified the target.codexcontents, sandbox marker/secrets, ACLs, firewall rules, and desktop startup without the sandbox setup banner once experimental network proxy requirements were disabled.Notes
This intentionally does not solve later elevated update coordination for IT-managed deployments. The setup command can still apply provisioning updates when run again, but a broader coordination/process story is out of scope for this alpha.