Materialize child MITM CA bundles#26315
Conversation
…ca-bundle-helpers # Conflicts: # codex-rs/network-proxy/src/certs.rs
|
@codex review |
This reverts commit 7fcc48c.
![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: 7fcc48c628
ℹ️ 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".
This reverts commit 3d8d010.
…ca-bundle-helpers-refresh-20260618
…rs' into dev/winston/mitm-ca-bundle-helpers-refresh-20260618
…ca-bundle-helpers-refresh-20260618
…ca-bundle-helpers-refresh-20260618
…ca-bundle-helpers-refresh-20260618
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23ba4ebbc3
ℹ️ 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".
| Err(err) => { | ||
| warn!( | ||
| ca_bundle_path = %ca_dir_path.display(), | ||
| "failed to read child MITM CA directory; skipping it: {err}" | ||
| ); |
There was a problem hiding this comment.
When SSL_CERT_DIR contains multiple paths and any one directory materializes successfully, this branch logs and skips a later directory that fails policy/bounds/read checks, after which the function can still return Some(trust_bundle) and the caller removes SSL_CERT_DIR. In that scenario roots from the failed directory were still available to OpenSSL-style clients through the original env var, but they are dropped from every materialized bundle. Fresh evidence is this multi-path loop continues after an error and still returns Some if another directory contributed contents.
Useful? React with 👍 / 👎.
| ) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
When a custom CA env var points at a bundle this helper refuses to copy, for example because it exceeds the 4 MiB helper cap, while SSL_CERT_DIR is readable, read_child_ca_bundle_contents returns None but this default turns the failure into an empty bundle; the subsequent directory append then materializes a new path and overwrites the original env var. That drops the original file-backed roots even though leaving the variable alone would still let the child use them, so the failure case needs to stay distinct from the “value is already the managed bundle” no-op case.
Useful? React with 👍 / 👎.
|
|
||
| [target.'cfg(windows)'.dependencies] | ||
| schannel = "0.1" | ||
| windows-sys = { version = "0.52", features = ["Win32_Foundation", "Win32_Storage_FileSystem"] } |
There was a problem hiding this comment.
Adding this direct dependency, along with the new libc dependency above, changes Rust dependency metadata, but the commit only updates Cargo.lock and leaves MODULE.bazel.lock unchanged. The repo’s bazel-lock-check path is explicitly meant to catch that stale lockfile state, so this will need the just bazel-lock-update output committed with the dependency change.
AGENTS.md reference: AGENTS.md:L37-L40
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
Sandboxed children need a bounded, immutable CA bundle assembled only from CA inputs they are allowed to read. The proxy must not expose its private key or shared working directory to achieve that.
What
SSL_CERT_DIRcodex-network-proxyStack
Validation
just test -p codex-network-proxyjust bazel-lock-check