Skip to content

fix(jailer): bind /etc/resolv.conf so allow_net DNS works under the sandbox#849

Merged
DorianZheng merged 2 commits into
mainfrom
fix/bwrap-resolv-conf
Jun 25, 2026
Merged

fix(jailer): bind /etc/resolv.conf so allow_net DNS works under the sandbox#849
DorianZheng merged 2 commits into
mainfrom
fix/bwrap-resolv-conf

Conversation

@G4614

@G4614 G4614 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Why

Two independent fixes that surface once the jailer is enabled by default (secure-by-default, #652).

1. allow_net allowlist silently sinkholes everything under the jailer

gvproxy resolves allow_net hostnames host-sidebuildAllowNetDNSZones (dns_filter.go) runs inside the shim, which the jailer wraps in bwrap. bwrap binds /usr, /lib, /bin, /sbinbut not /etc, so the sandbox has no /etc/resolv.conf. Every host-side lookup then fails, and each allow-listed host falls through to the catch-all 0.0.0.0 sinkhole.

Result: with the jailer on, the allowlist blocks everything — including the hosts it is meant to permit. This is the secure-by-default root cause behind the 0.0.0.0 symptom in #645; #645's Go-side retry / longest-suffix / fail-closed changes don't address it (there's no resolver in the sandbox to retry against).

Fix: bind /etc/resolv.conf, /etc/hosts, /etc/nsswitch.conf read-only so the Go resolver works.

Verified on a real box (--security enable --allow-net example.com):

before after
nslookup example.com 0.0.0.0 104.20.23.154

2. Drop whoami_updates_stale_profile_path_prefix

auth whoami is a read-only diagnostic — it GETs /v1/me and prints the identity. This test additionally asserted whoami persists the refreshed path_prefix back into the stored credentials, a side effect a "show" command should not have (the prefix is captured at auth login). whoami has never written the profile, so the test has been red since #746. Drop the over-specified assertion rather than add a credential-mutating side effect.

Test plan

  • nslookup/resolution verified manually under --security enable (see table)
  • e2e-local network-secrets suite green

Summary by CodeRabbit

  • Bug Fixes
    • Improved sandbox networking and name-resolution behavior so DNS lookups and host resolution work more reliably inside isolated environments.
    • Fixed an issue that could cause allowlisted host lookups to be blocked when system DNS configuration was unavailable in the sandbox.

G4614 and others added 2 commits June 25, 2026 12:07
…et DNS

gvproxy resolves `allow_net` hostnames host-side — `buildAllowNetDNSZones`
(dns_filter.go) runs inside the shim, which the jailer wraps in bwrap. bwrap
binds /usr, /lib, /bin, /sbin … but not /etc, so the sandbox has no
/etc/resolv.conf. Every host-side lookup then fails and each allow-listed host
falls through to the catch-all 0.0.0.0 sinkhole — so with the jailer enabled
(secure by default) the allowlist silently blocks everything, including the
hosts it is supposed to permit (the symptom in #645).

Bind /etc/resolv.conf, /etc/hosts and /etc/nsswitch.conf read-only so the Go
resolver works. Verified: with `--security enable --allow-net example.com`,
`nslookup example.com` now returns the real IP (104.20.23.154) instead of
0.0.0.0. This is the secure-by-default root cause that #645's Go-side retry /
zone-ordering / fail-closed changes do not address.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`auth whoami` is a read-only diagnostic — it calls GET /v1/me and prints the
identity. This test asserted that whoami also *persists* the refreshed
path_prefix back into the stored credentials, a side effect a "show" command
should not have (the path_prefix is captured at `auth login`). whoami has never
written the profile, so the test has been red since it was added (#746). Drop
the over-specified assertion rather than add a credential-mutating side effect.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4255ecca-baa5-4add-8623-448a9b301b9c

📥 Commits

Reviewing files that changed from the base of the PR and between 8007a86 and 2dadbe1.

📒 Files selected for processing (2)
  • src/boxlite/src/jailer/sandbox/bwrap.rs
  • src/cli/tests/auth.rs
💤 Files with no reviewable changes (1)
  • src/cli/tests/auth.rs

📝 Walkthrough

Walkthrough

The bubblewrap sandbox now reads host name-resolution config files inside the container. One auth integration test covering stale path_prefix behavior was removed.

Changes

Sandbox name-resolution binds

Layer / File(s) Summary
Read-only name-resolution binds
src/boxlite/src/jailer/sandbox/bwrap.rs
BwrapSandbox::apply adds read-only binds for /etc/resolv.conf, /etc/hosts, and /etc/nsswitch.conf, plus comments describing the missing-resolv.conf lookup failure.

Auth test cleanup

Layer / File(s) Summary
Removed stale path-prefix test
src/cli/tests/auth.rs
The whoami_updates_stale_profile_path_prefix test block is removed, and the remaining auth tests follow it directly.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • boxlite-ai/boxlite#846: Also changes src/boxlite/src/jailer/sandbox/bwrap.rs and adjusts the bubblewrap sandbox setup.

Poem

A bunny hopped through sandbox dew,
And found the names could now break through.
With hosts and DNS tucked snug and near,
The sandbox purrs, both calm and clear. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main sandbox DNS fix, which is the primary change in the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bwrap-resolv-conf

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@G4614 G4614 marked this pull request as ready for review June 25, 2026 12:16
@G4614 G4614 requested a review from a team as a code owner June 25, 2026 12:16
@G4614 G4614 enabled auto-merge June 25, 2026 12:17
@DorianZheng DorianZheng disabled auto-merge June 25, 2026 14:03
@DorianZheng DorianZheng merged commit 4f8d6b3 into main Jun 25, 2026
35 checks passed
@DorianZheng DorianZheng deleted the fix/bwrap-resolv-conf branch June 25, 2026 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants