feat(linux-sandbox): route DNS through managed proxy#31644
Conversation
a44b6fe to
d7aae07
Compare
d7aae07 to
3a15264
Compare
9f584f0 to
a5ea367
Compare
d7aae07 to
f3c3920
Compare
e0b5dc2 to
83d864e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 816e07cbd9
ℹ️ 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 [logical_path, target_path].into_iter().any(|path| { | ||
| !policy.can_read_path_with_cwd(path, cwd) | ||
| || deny_matcher |
There was a problem hiding this comment.
Honor minimal read defaults when mounting DNS config
When managed DNS is enabled with a :minimal/split filesystem profile, this check rejects /etc/resolv.conf before bwrap starts because can_read_path_with_cwd() does not include the Linux platform-default read roots that bwrap later adds for :minimal (including /etc). Those launches will panic in DNS setup with PermissionDenied even though the sandbox would otherwise mount the resolver path; account for the same platform defaults here while still honoring explicit deny rules.
Useful? React with 👍 / 👎.
| if bound_dns.is_some() { | ||
| drop_and_verify_capabilities().unwrap_or_else(|err| { | ||
| panic!("error dropping Linux DNS setup capabilities: {err}") | ||
| }); |
There was a problem hiding this comment.
Keep route setup capabilities until bridges are active
When network.enable_dns is enabled, this drops every capability before activate_proxy_routes_in_netns() runs. That routing path may need to initialize the fresh bwrap network namespace’s loopback device before binding the local proxy bridge (bind_local_loopback_listener() retries via ensure_loopback_interface_up() on EADDRNOTAVAIL/ENETUNREACH), which requires the namespace’s network-admin capability. On those Linux environments the DNS-enabled managed proxy path will fail during bridge activation, or leave loopback unusable, before the user command starts; bind/start the proxy routes while the setup capabilities are still available and only then drop them before exec.
Useful? React with 👍 / 👎.
| BubblewrapLauncher::System(_) | BubblewrapLauncher::Unavailable => { | ||
| match bundled_bwrap::launcher() { | ||
| Some(launcher) => BubblewrapLauncher::Bundled(launcher), | ||
| None => BubblewrapLauncher::Unavailable, |
There was a problem hiding this comment.
Don’t force bundled bwrap on setuid-only hosts
On hosts where the system bwrap is setuid because unprivileged user namespaces are disabled, this branch treats DNS support as available by falling back to the bundled bwrap whenever the system launcher cannot use --cap-add. The vendored bwrap itself rejects --cap-add in setuid mode for non-root (vendor/bubblewrap/bubblewrap.c lines 2958-2959), but the bundled binary also cannot create the user namespace on those hosts, so enabling DNS turns an otherwise working system-bwrap sandbox into a startup failure. In that environment this should disable the DNS bridge and keep using the working system launcher rather than selecting bundled bwrap solely for --cap-add.
Useful? React with 👍 / 👎.
| let bound_dns = if std::env::var_os(DNS_PROXY_ENV_KEY).is_some() { | ||
| Some( | ||
| bind_netns_dns_stub() | ||
| .unwrap_or_else(|err| panic!("error binding Linux DNS bridge: {err}")), |
There was a problem hiding this comment.
Bring loopback up before binding the DNS stub
When network.enable_dns is enabled in a fresh --unshare-net namespace, this binds 127.0.0.1:53 before any code has initialized the namespace loopback device. The existing proxy bridge path only brings lo up later when its own bind sees EADDRNOTAVAIL/ENETUNREACH, so on those Linux hosts the DNS-enabled sandbox aborts here with error binding Linux DNS bridge before route setup can recover. Initialize loopback before this bind, or defer the DNS bind until after the proxy route setup has done so.
Useful? React with 👍 / 👎.
816e07c to
9ad7318
Compare
Co-authored-by: Codex noreply@openai.com
Add enable_dns to the managed network proxy config so DNS listener reservation and endpoint export are opt-in. Keep the Linux DNS bridge driven solely by the endpoint env, and simplify the Linux integration test by using the DNS wire-format parser instead of hand-rolled packet bytes.\n\nCo-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
9ad7318 to
3a5d83f
Compare
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Summary
Native DNS clients do not honor HTTP or SOCKS proxy variables, so Linux needs a DNS-speaking adapter inside the bubblewrap network namespace when managed DNS is enabled.
enable_dnsmanaged proxy setting; DNS listeners and private DNS endpoint env are not created by default127.0.0.1:53for UDP and TCP when the managed proxy provides a DNS endpoint127.0.0.1and remove the private DNS endpoint variable before the user command runs--cap-addwith setuid system bubblewrap by selecting a capable bundled launcher when available; otherwise DNS routing is disabled while HTTP/SOCKS routing remains unchangedTesting
just test -p codex-network-proxy(passed outside the filesystem sandbox; the sandboxed run could not write test CA files under the local proxy state directory)just test -p codex-linux-sandbox(builds on macOS, then exits withno tests to runbecause the crate tests are Linux-gated)cargo check -q -p codex-network-proxy -p codex-core -p codex-linux-sandboxcargo check -q --target x86_64-unknown-linux-gnu -p codex-linux-sandbox(blocked locally: missingx86_64-linux-gnu-gccand Linux OpenSSL sysroot)just write-config-schemajust fix -p codex-network-proxyjust fix -p codex-linux-sandboxjust fix -p codex-configjust fix -p codex-featuresjust fix -p codex-coreStack: 2 of 2. Previous: #31642.