feat: introducing a network sandbox proxy#8442
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
viyatb-oai
changed the title
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
![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: d2042b92b6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef2c2d3131
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c394c2dbd
ℹ️ 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".
| .iter() | ||
| .map(|s| s.to_ascii_lowercase()) | ||
| .collect(); |
There was a problem hiding this comment.
The managed constraint check lowercases unix socket paths before comparing them. On case-sensitive macOS filesystems, this lets an untrusted layer satisfy the constraint with a different-cased path (e.g., managed allows /tmp/Sock, user config uses /tmp/sock), which can refer to a different socket while still passing validation. Socket paths should be compared exactly (or canonicalized) rather than case-folded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f6e54016
ℹ️ 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".
| if cfg.policy.allow_unix_sockets.is_empty() { | ||
| return (http_addr, admin_addr); | ||
| } |
There was a problem hiding this comment.
clamp_bind_addrs forces both listeners onto 127.0.0.1 whenever allow_unix_sockets is non-empty, but unix-socket proxying is macOS-only (unix_socket_permissions_supported() returns false elsewhere). In a shared config that enables unix sockets for macOS, this will silently clamp Linux/Windows binds to loopback even when dangerously_allow_non_loopback_* is true, breaking non-loopback deployments despite the feature being unsupported. Consider guarding this clamping behavior behind the platform support check.
Useful? React with 👍 / 👎.
| let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; | ||
| let cli_overrides = Vec::new(); | ||
| let overrides = LoaderOverrides::default(); | ||
| let config_layer_stack = load_config_layers_state(&codex_home, None, &cli_overrides, overrides) |
There was a problem hiding this comment.
Let's look at this in a follow-up PR, as I'd like to have a stronger abstraction for detecting changes in the stack.
There was a problem hiding this comment.
What's here is pretty good, but I want to take a deeper look when I have more time. This isn't blocking.
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
Really amazing work here! Thanks for being so attentive to all of the feedback: I know it was a lot!!!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8daa525777
ℹ️ 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".
| let path = cfg_path.display(); | ||
| warn!("failed to reload config from {path}: {err}; keeping previous config"); | ||
| Err(err) | ||
| } |
There was a problem hiding this comment.
When build_config_state() returns an error (e.g., a transiently invalid config.toml), this branch returns Err(err) even though the previous config is explicitly kept. Because reload_if_needed() compares mtimes against the old layer_mtimes, the reload will be retried on every request and will keep returning an error, effectively taking the proxy down until the config is fixed. Consider swallowing the reload error (and/or updating mtimes) so the last known-good config continues to serve requests.
Useful? React with 👍 / 👎.
4 participants
This add a new crate,
codex-network-proxy, a local network proxy service used by Codex to enforce fine-grained network policy (domain allow/deny) and to surface blocked network events for interactive approvals.codex-rs/network-proxy/(codex-network-proxybinary + library)Will follow up integration with codex in subsequent PRs.
Testing
cd codex-rs && cargo build -p codex-network-proxycd codex-rs && cargo run -p codex-network-proxy -- proxy