Skip to content

feat(sandbox): bind /proc in bwrap on Lakebox hosts#3258

Open
mxatone wants to merge 1 commit into
mainfrom
mxatone/lakeboxsandbox
Open

feat(sandbox): bind /proc in bwrap on Lakebox hosts#3258
mxatone wants to merge 1 commit into
mainfrom
mxatone/lakeboxsandbox

Conversation

@mxatone

@mxatone mxatone commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Related issue

N/A

Summary

The linux_bwrap sandbox mounts a fresh procfs under --unshare-pid. A
Databricks Lakebox microVM masks /proc, so that mount is rejected with
bwrap: Can't mount proc on /newroot/proc: Operation not permitted and the
sandbox never starts. That blocked linux_bwrap — and the L7 egress
management built on top of it — on the Lakebox backend.

  • Bind the existing /proc instead of mounting a fresh one, but only on
    outer sandbox backends known to be safe
    (allow-list: lakebox).
    wrap_launcher_argv emits --bind /proc /proc in that case and
    --proc /proc everywhere else.
  • Resolve the outer backend from OMNIGENT_HOST_SANDBOX_BACKEND when a
    launcher sets it, otherwise autodetect Lakebox via the /run/lakebox
    marker. Detection runs in the host process, never inside the sandboxed
    agent, so it cannot be forged from within the sandbox.
  • Impact on non-Lakebox hosts: none. They keep the fresh-proc mount and
    its fail-closed behavior. Adding a backend later is one entry in
    _PROC_BIND_HOST_BACKENDS.

ELI5. bwrap normally gives the agent a brand-new /proc that shows only
its own processes. Lakebox does not allow creating that new /proc, so the
agent could not start at all. On Lakebox we hand the agent the existing
/proc instead. That reveals the list of other process names on the box,
but the user namespace still hides the sensitive parts (each process's
environment, memory, and open files), so it is a safe trade on a
single-tenant dev VM.

flowchart TD
    A[bwrap builds sandbox argv] --> B{host backend on the<br/>proc-bind allow-list?}
    B -- "lakebox (env var or /run/lakebox)" --> C["--bind /proc /proc<br/>reuse host proc"]
    B -- "anything else" --> D["--proc /proc<br/>fresh procfs, fail-closed"]
Loading

Security trade-off (documented inline in bwrap_sandbox.py): binding
/proc exposes the outer process list and the world-readable per-process
files (cmdline / comm / stat / status). It does not expose the
ptrace-gated files (environ / mem / maps / fd): the retained user
namespace keeps those blocked even for same-uid targets and even as
namespaced root, and --unshare-pid still contains signalling. Because the
bind is a security downgrade, it is gated to an explicit allow-list rather
than a blind "fresh-proc failed, retry with a bind" fallback.

Test Plan

Unit tests and lint (local):

  • uv run pytest tests/inner/test_bwrap_sandbox.py -q -k "proc or host_backend"7 passed.
  • uv run ruff check and ruff format --check on both files → clean;
    pre-commit on both files → all hooks passed.

Live end-to-end on a Lakebox microVM (e2-dogfood, omnigent-egress-test):

  • Detection inside the VM (no env var set): detect = lakebox | should_bind_proc = True.
  • linux_bwrap shell now starts and runs: id and uname -a return
    rc=0; cat /proc/1/cmdline returns the host sandbox-daemon, proving
    /proc is the bound host proc (a fresh procfs would show the helper as
    PID 1).
  • Negative control OMNIGENT_HOST_SANDBOX_BACKEND=bareshould_bind_proc = False
    → reproduces the original failure bwrap: Can't mount proc on /newroot/proc: Operation not permitted, confirming the gate is precisely scoped.
  • linux_bwrap + egress_rules: ["* pypi.org/**"] (default-deny): allowed
    pypi.orgHTTP=200 exit=0; denied example.comHTTP=000 exit=56
    (blocked by the egress proxy).

New coverage (unit):

  • _detect_host_sandbox_backend — none without signals, env declaration
    normalised and authoritative, non-lakebox env keeps fresh proc even with
    the marker present, /run/lakebox marker autodetects lakebox.
  • wrap_launcher_argv — emits --proc /proc by default and
    --bind /proc /proc on lakebox (with /dev and /tmp unchanged).

Pre-existing, unrelated: the four resolve() tests in
tests/inner/test_bwrap_sandbox.py fail on macOS with "only available on
Linux". I confirmed via git stash that they fail identically without this
change (the Linux gate is in the base file), so they are not a regression;
they pass on Linux/CI.

Demo

N/A (non-visual change).

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

Manual verification was run on a live Lakebox microVM on e2-dogfood: the
branch's bwrap_sandbox.py was loaded in the VM and exercised via
create_os_environment().shell() for the positive (linux_bwrap starts,
/proc bound), negative-control (forced non-lakebox backend reproduces the
mount EPERM), and egress (pypi.org allowed, example.com denied) cases.
The VM's file was restored to its original state afterward. The proc-mode
selection logic itself is covered by the new unit tests above.

Changelog

linux_bwrap sandboxes and their egress rules now run on the Databricks Lakebox backend.

@github-actions github-actions Bot added the size/L Pull request size: L label Jul 25, 2026
The linux_bwrap sandbox mounts a fresh procfs under --unshare-pid, but a
Lakebox microVM masks /proc so that mount returns EPERM and the sandbox
fails to start. That blocked linux_bwrap — and the L7 egress management
built on top of it — on the Lakebox backend.

Bind the existing /proc instead of mounting a fresh one, but only on
outer sandbox backends known to be safe for it (allow-list: lakebox).
The backend is read from OMNIGENT_HOST_SANDBOX_BACKEND when set, else
autodetected via the /run/lakebox marker. Everywhere else the fresh-proc
mount and its fail-closed behavior stay unchanged.

Binding /proc exposes the outer process list and world-readable per-proc
files (cmdline/comm/stat/status). The retained user namespace still
blocks ptrace-gated files (environ/mem/maps/fd) and --unshare-pid still
contains signalling, so the leak is acceptable on a single-tenant
Lakebox microVM.

Signed-off-by: Thomas Garnier <6202935+mxatone@users.noreply.github.com>
@mxatone
mxatone force-pushed the mxatone/lakeboxsandbox branch from fc25813 to f2b8935 Compare July 25, 2026 01:13
try:
if _LAKEBOX_MARKER.is_dir():
return "lakebox"
except OSError:
@omnigent-ci

omnigent-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The change is correct, narrowly scoped, and the new branch in wrap_launcher_argv cleanly swaps --proc /proc for --bind /proc /proc only when _should_bind_host_proc() is true. The default path (fresh procfs, fail-closed) is preserved for every non-allow-listed host, so ordinary hosts are unchanged.

Security vulnerabilities

None introduced. The security posture holds up under scrutiny:

  • Detection is host-side and unforgeable. _detect_host_sandbox_backend() reads os.environ[OMNIGENT_HOST_SANDBOX_BACKEND] and /run/lakebox in the parent process while building the argv, before the sandbox exists. The sandboxed agent cannot influence either signal, so it cannot coerce the /proc downgrade. Confirmed the callsite is inside wrap_launcher_argv (host path), not inside the launched helper.
  • Downgrade is gated by an explicit allow-list, not a blind fallback. _PROC_BIND_HOST_BACKENDS = {"lakebox"} means an unknown/unset backend keeps the fresh-proc mount. A declared-but-not-allow-listed backend (modal) correctly stays on fresh proc even when the /run/lakebox marker is present — the env declaration is authoritative and the test asserts this precise precedence.
  • The documented leak boundary is accurate. Binding the host /proc exposes the world-readable per-process files and the outer process list, but environ/mem/maps/fd remain ptrace-gated under the retained user namespace, and --unshare-pid still contains signalling. Gating this to a single-tenant dev microVM is a reasonable, well-argued trade-off, and the inline comment captures it for future readers.

Non-blocking notes

  • Autodetect widens the trust surface slightly. /run/lakebox existing is treated as sufficient to trigger the downgrade with no env declaration. This is fine for the intended single-tenant Lakebox VM, but it means any future host that happens to create /run/lakebox inherits the proc bind silently. The env-var path is the safer signal; consider whether marker-only autodetect should log at info/debug when it flips the mode, to make the downgrade observable in host logs. (The PR notes the env var may be pruned by spawn_env_allowlist, which is why the marker fallback exists — so this is a deliberate tradeoff, just worth a log line.)
  • --bind /proc /proc vs --dev-bind semantics. The fresh-proc mount and the bind end at the same mountpoint; the test correctly asserts the two are mutually exclusive (--bind present, --proc absent) so there's no double-mount conflict. Good coverage.
  • Test suite mirrors every branch of the new logic (none/env-normalised/env-non-lakebox/marker) plus both argv outcomes, and isolates the host machine via _clear_host_backend_signals so it won't misbehave when CI itself runs on a Lakebox-like host.

No visual demonstration is needed — this is a backend sandbox change with no user-visible surface.

Summary

A tight, well-reasoned fix that unblocks linux_bwrap (and the L7 egress path) on Lakebox microVMs by binding the existing /proc where a fresh procfs mount is rejected. The security downgrade is real but correctly bounded to an explicit allow-list, detected host-side so it can't be forged from inside the sandbox, and left entirely off for every other host. Test coverage is thorough and the trade-off is documented inline. Approve; the only optional follow-up is a log line when marker-based autodetect flips into bind mode.


Automated review by Polly · workflow run

@github-actions

Copy link
Copy Markdown
Contributor

@mxatone This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder.

These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:

  • A screenshot or screen recording of the change, or
  • A link to a hosted video or GIF showing the new behaviour.

Use N/A only when the change has no user-visible effect whatsoever (e.g. a pure refactor or test-only change). If that's the case, uncheck the relevant type box and check Refactor / chore or Test / CI instead.

@github-actions github-actions Bot added the needs-demo PR needs a demo screenshot or recording label Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-demo PR needs a demo screenshot or recording size/L Pull request size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant