feat: add managed network proxy feature flag#20147
Conversation
Co-authored-by: Codex [email protected]
viyatb-oai
changed the title
![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: 94f65a149e
ℹ️ 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".
|
|
||
| fn network_proxy_toml_config(features: Option<&FeaturesToml>) -> Option<&NetworkProxyConfigToml> { | ||
| match features?.network_proxy.as_ref()? { | ||
| FeatureToml::Enabled(_) => None, |
There was a problem hiding this comment.
network_proxy = false in profile overrides
network_proxy_toml_config ignores FeatureToml::Enabled(false), so a profile-level disable does not clear base [features.network_proxy] table settings. Because config merging later only forces enabled = true (never false), a root table with enabled = true can still start the managed proxy even when the active profile sets network_proxy = false.
Useful? React with 👍 / 👎.
| if let Some(network_proxy) = network_proxy_toml_config(cfg.features.as_ref()) { | ||
| apply_network_proxy_feature_config(&mut configured_network_proxy_config, network_proxy); | ||
| } |
There was a problem hiding this comment.
Applying [features.network_proxy] config directly onto configured_network_proxy_config lets enabled = false override proxy settings derived from active [permissions] profiles. With --disable network_proxy now writing features.network_proxy.enabled=false, this can turn off a proxy that profile network policy would otherwise start, contradicting the goal to preserve existing managed proxy startup behavior.
Useful? React with 👍 / 👎.
1 participant
Why
The permissions migration is making
permissions.<profile>.network.enabledthe canonical sandbox network bit, but managed proxy startup is a different concern. Enabling direct network access should not implicitly start the proxy, and users who are still on legacy sandbox modes need a separate place to opt into proxy startup and provide proxy-specific settings.This follow-up to #19900 gives the managed proxy its own feature surface instead of overloading permission-profile network semantics.
What changed
network_proxyfeature with a configurable[features.network_proxy]table.features.<name>.enabledrather than replacing the whole table.features.network_proxysettings onto the configured proxy state after permission-profile selection, so the feature can start the managed proxy without changing the activeNetworkSandboxPolicy.[experimental_network]startup behavior independently of the new feature flag.Relevant code:
features/src/lib.rsadds the configurable feature-table plumbing.core/src/config/mod.rsreads the feature bit, and later applies it without widening sandbox network access.Verification
Added focused coverage for:
features.network_proxywithout enabling sandbox network access[experimental_network]startup without the featureRan:
cargo test -p codex-featurescargo test -p codex-core network_proxy_featurecargo test -p codex-core experimental_network_requirements_enable_proxy_without_featurecargo test -p codex-cli feature_toggles_preserve_configurable_feature_tables