Require elevated-only Windows policy for network proxies#31437
Require elevated-only Windows policy for network proxies#31437iceweasel-oai wants to merge 5 commits into
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
iceweasel-oai
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1e181047f
ℹ️ 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".
| validate_windows_network_proxy_requirements_for_platform( | ||
| cfg!(target_os = "windows"), |
There was a problem hiding this comment.
In mixed deployments where the app-server runs on Linux/macOS but the selected exec-server is Windows, this cfg!(target_os = "windows") is false while the sandbox context still carries a Windows sandbox level to the remote executor. User/CLI/profile-driven network proxy configs can therefore load without the elevated-only Windows requirement and be sent to a Windows exec-server, bypassing the policy this change is trying to enforce for that executor; the check needs to use the target executor platform or otherwise validate before sending Windows sandbox requests.
AGENTS.md reference: AGENTS.md:L317-L322
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c32b50beb7
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c20a892b53
ℹ️ 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".
| validate_windows_sandbox_network_proxy_compatibility( | ||
| exec_req.windows_sandbox_level, | ||
| exec_req.network.is_some(), | ||
| ); | ||
| ) | ||
| .map_err(CodexErr::Io)?; |
There was a problem hiding this comment.
This compatibility check only protects the legacy build_exec_request path; the unified-exec runtime builds its ExecRequest through SandboxAttempt::env_for, so it never reaches this guard, and the Windows spawn branch in core/src/unified_exec/process_manager.rs still sends WindowsSandboxLevel::RestrictedToken requests to spawn_windows_sandbox_session_legacy where request.network is not enforced. For any unified-exec session or manually constructed TurnContext that still has a network proxy with the restricted-token Windows sandbox, the proxy is silently dropped instead of failing; put this validation in the shared sandbox/ExecRequest preparation path or add the same guard before unified-exec Windows spawning.
Useful? React with 👍 / 👎.
| if strict_config || is_unrecoverable_windows_network_config_error(&err) { | ||
| return Err(err); |
There was a problem hiding this comment.
When network_proxy is enabled in local/user config and the required elevated-only [windows] requirement is supplied by the cloud bundle, the preload above still uses the default empty cloud loader, so it fails before replace_cloud_config_bundle_loader can install the real loader; this new unrecoverable branch then makes the second load return the same missing-requirements error instead of retrying with the cloud requirements. That bricks an otherwise valid cloud-managed Windows config at app-server startup, so defer this hard failure until after the cloud loader is installed or bootstrap auth without enforcing this requirement.
AGENTS.md reference: AGENTS.md:L105-L110
Useful? React with 👍 / 👎.
1 participant
Why
Windows network proxy enforcement previously promoted command execution to the elevated sandbox backend whenever a proxy was present. That could produce an unexpected UAC prompt even when the configured sandbox was unelevated.
Backend selection should follow the configured sandbox level, not silently override it. To keep proxy enforcement valid, any effective Windows configuration that activates the network proxy must explicitly include an administrator-authored requirement that permits only the elevated Windows sandbox.
This supersedes #28591 by validating the policy when effective configuration is resolved instead of allowing ordinary app setup to create an incompatible state.
What changed
[experimental_network]requirements, thenetwork_proxyfeature, legacy network settings, CLI overrides, and active permission profiles.network proxy + unelevated Windows sandboxexecution requests.Validation
cargo test -p codex-config --libcargo test -p codex-core network_proxy --libcargo test -p codex-core active_profile_update_rebuilds_network_proxy_config --libcargo test -p codex-app-server windows_sandbox --libcargo test -p codex-app-server windows_network_config_errors_are_unrecoverable --libcargo clippy -p codex-config -p codex-core -p codex-app-server --lib --no-deps -- -D warningsDocumentation
Update the network proxy documentation to state that Windows deployments must pin
allowed_sandbox_implementationsto["elevated"]in requirements.