Snapshot managed proxy child environments#28967
Conversation
…winston/mitm-child-env-snapshots
…winston/mitm-child-env-snapshots
…winston/mitm-child-env-snapshots
…winston/mitm-child-env-snapshots
![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: 9cf8420105
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .unwrap(); | ||
| let mut config = state.current_cfg().await.unwrap(); | ||
| config.network.mitm = true; | ||
| let new_state = build_config_state(config, NetworkProxyConstraints::default()).unwrap(); |
There was a problem hiding this comment.
When this test enables network.mitm, build_config_state and the subsequent reload materialize the managed MITM CA under the shared CODEX_HOME. The existing SOCKS MITM tests serialize that same CA creation because load_or_create_ca uses create-new writes and can fail if another test is between writing ca.key and ca.pem; this new test does not take that lock or use a temporary CODEX_HOME, so codex-network-proxy tests can flake when run in parallel from a clean home.
Useful? React with 👍 / 👎.
| runtime_settings.allow_local_binding, | ||
| runtime_settings.mitm_ca_trust_bundle.as_ref(), | ||
| ); | ||
| self.child_env_snapshot().apply_to_env(env); |
There was a problem hiding this comment.
For sandboxed launches that need both proxy env vars and a MITM CA read grant, this still takes two independent snapshots: build_exec_request applies env via NetworkProxy::apply_to_env, then SandboxManager::transform calls NetworkProxy::managed_mitm_ca_trust_bundle_path later. If managed network config reloads between those calls, a child can inherit CA env from one MITM state while the sandbox grants the path from another (or no path), which is the incoherent launch this change is meant to prevent; callers need to capture child_env_snapshot() once and use it for both env preparation and sandbox path selection.
Useful? React with 👍 / 👎.
| self.http_addr, | ||
| self.socks_addr, | ||
| self.socks_enabled, | ||
| self.runtime_settings(), |
There was a problem hiding this comment.
When the running proxy reloads config through the normal ConfigReloader path (reload_if_needed / force_reload on NetworkProxyState), only the proxy state is replaced; this cached runtime_settings is never refreshed except by NetworkProxy::replace_config_state. After editing config to enable MITM, allow local binding, or change Unix-socket allowances, the proxy begins enforcing the new state while child launches keep receiving the old env/permissions snapshot indefinitely, so MITM can start intercepting without children getting the managed CA env.
Useful? React with 👍 / 👎.
|
Closing as superseded by the focused #29013 and #29014 stack. The replacement isolates the persisted MITM private key and preserves file-backed custom CAs present at Codex startup, with zero |
1 participant
Why
A child launch needs one coherent view of proxy and MITM state even when managed network configuration reloads concurrently. Shell snapshots must also not restore stale CA-directory state over the live managed MITM environment.
What
codex-network-proxySSL_CERT_DIRsemantics when shell snapshots cross MITM state changescodex-network-proxy; the productioncodex-corechange is 12 additions and 22 deletions, plus one focused regression testStack
Validation
just test -p codex-network-proxycodex-coreshell-snapshot regression test