Skip to content

fix(seatbelt): narrow macOS sandbox write grants under built-in profile#781

Merged
DorianZheng merged 1 commit into
boxlite-ai:mainfrom
G4614:fix/seatbelt-tmpdir-scope
Jun 15, 2026
Merged

fix(seatbelt): narrow macOS sandbox write grants under built-in profile#781
DorianZheng merged 1 commit into
boxlite-ai:mainfrom
G4614:fix/seatbelt-tmpdir-scope

Conversation

@G4614

@G4614 G4614 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Drops the static seatbelt (subpath "/private/var/folders") write grant under the built-in macOS profile.

That grant was vestigial: vmm/controller/spawn.rs::configure_env already redirects the shim's TMPDIR to a box-scoped path whenever jailer_enabled && sandbox_profile.is_none(), and since #742 every box socket binds through /tmp/bl-{uid}/{box_id}/ (added dynamically via BoxSockets::policy_paths()). With both already in place, no per-user temp tree write was actually needed.

Updated TODO in seatbelt_network_policy.sbpl documents why narrowing (allow network-inbound) is non-trivial (port list is runtime data, doesn't fit sandbox-exec -D; network-bind vs network-inbound distinction matters for the gvproxy NAT return path; needs real EXPOSE e2e). Out of scope for this PR.

Test plan (macOS arm64)

  • Two-side verification of new regression test test_file_write_policy_excludes_per_user_temp_tree:
    • Re-added (subpath "/private/var/folders") → test FAILS at the grant-form check
    • Restored → 27/27 seatbelt unit tests pass, including 4 test_seatbelt_runtime_* that spawn real sandbox-exec
  • Live VM happy path: boxlite run --rm alpine:latest sh -c '...' → guest boots (Linux 6.12.76 aarch64), command runs, output returned (uid=0(root)), auto-stopped. No EPERM / sandbox denial.
  • Live VM EXPOSE port forwarding: boxlite run -d -p HOST:8080 alpine:latest sh -c 'nc -l -p 8080' → host curl 127.0.0.1:HOST/ returned guest payload via gvproxy NAT; boxlite stop cleaned up.

Summary by CodeRabbit

  • Bug Fixes

    • Restricted temporary file write access to system-only directories.
  • Tests

    • Added sandbox permission validation tests.

The static seatbelt file-write policy granted `(subpath
"/private/var/folders")` — the entire per-user temp/cache tree (browser
caches, app sqlite, token caches, …). Under the built-in profile this
grant is vestigial:

- `vmm/controller/spawn.rs::configure_env` already redirects the shim's
  TMPDIR/TMP/TEMP to the box-scoped `tmp_dir`
  (`~/.boxlite/boxes/{id}/tmp`) whenever
  `jailer_enabled && sandbox_profile.is_none()`, so libkrun's transient
  `krun-empty-root-*` and anything else honoring TMPDIR no longer lands
  in `/private/var/folders`.
- Since boxlite-ai#742 every box socket (gvproxy `net.sock`, libkrun's derived
  `net.sock-krun.sock`, `box.sock`, `ready.sock`) is bound through
  `/tmp/bl-{uid}/{box_id}/`, added dynamically via
  `BoxSockets::policy_paths()` — also not under `/private/var/folders`.

Network-side narrowing (`(allow network-inbound)` is broader than the
forwarded EXPOSE ports require) is deferred: the port list is runtime
data in `NetworkBackendConfig.final_mappings` that doesn't fit
`sandbox-exec -D` string substitution, needs dynamic policy
construction, careful disambiguation of `network-bind` vs
`network-inbound` for the gvproxy NAT return path, and real EXPOSE e2e
before tightening. The TODO comment in seatbelt_network_policy.sbpl
records those constraints.

Test plan:
- Two-side verification of the new regression test
  `test_file_write_policy_excludes_per_user_temp_tree`:
    1. Temporarily re-added `(subpath "/private/var/folders")` to the
       sbpl → new test FAILED at the grant-form check; confirms the
       test catches the regression, not a tautology.
    2. Restored final sbpl → 27/27 seatbelt unit tests pass on macOS
       arm64, including 4 runtime tests that spawn real `sandbox-exec`
       invocations (allow-write-to-writable-path,
       deny-write-outside-writable-path, allow-exec-from-tmp-dynamic-path,
       deny-read-outside-allowlist).
- Live VM e2e on macOS arm64:
    - `boxlite run --rm alpine:latest sh -c '...'` → guest booted
      (Linux 6.12.76 aarch64), command executed, output returned
      (`uid=0(root)`), auto-stopped. No EPERM / sandbox denial.
    - `boxlite run -d -p HOST:8080 alpine:latest sh -c 'nc -l -p 8080'`
      → `curl http://127.0.0.1:HOST/` returned the guest payload via
      gvproxy NAT, `boxlite stop` cleaned up.
@G4614 G4614 requested a review from a team as a code owner June 15, 2026 08:40
@coderabbitai

coderabbitai Bot commented Jun 15, 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: f74b62ca-a679-46d8-8018-33f4ab78beab

📥 Commits

Reviewing files that changed from the base of the PR and between 6d3c8a7 and a355f30.

📒 Files selected for processing (3)
  • src/boxlite/resources/seatbelt/seatbelt_file_write_policy.sbpl
  • src/boxlite/resources/seatbelt/seatbelt_network_policy.sbpl
  • src/boxlite/src/jailer/sandbox/seatbelt.rs

📝 Walkthrough

Walkthrough

Removes (subpath "/private/var/folders") from the macOS Seatbelt static file-write allowlist, expands policy comments to document the per-user temp tree exclusion and dynamic path scoping, adds a regression test asserting the path stays absent, and inserts a TODO comment in the network policy noting the over-broad network-inbound grant.

Changes

Seatbelt sandbox policy hardening

Layer / File(s) Summary
Remove /private/var/folders from file-write allowlist and add regression test
src/boxlite/resources/seatbelt/seatbelt_file_write_policy.sbpl, src/boxlite/src/jailer/sandbox/seatbelt.rs
Deletes the (subpath "/private/var/folders") directive from the static allow file-write* block, rewrites surrounding comments to document the exclusion intent and list the dynamic paths ({box_dir}/tmp/, socket locations) added by seatbelt.rs, and introduces test_file_write_policy_excludes_per_user_temp_tree to assert neither the exact nor slash-extended variant of the pattern is present.
Document over-broad network-inbound permission
src/boxlite/resources/seatbelt/seatbelt_network_policy.sbpl
Adds a TODO(security) comment block explaining that (allow network-inbound) covers more than gvproxy requires and describing the runtime port-mapping, network-bind vs network-inbound semantics, and macOS e2e validation constraints that must be addressed before narrowing it.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop, hop, away with that folder wide,
/private/var/folders can no longer hide!
Our tmp stays tidy, just /private/tmp in sight,
A TODO reminds us to keep networks tight.
The sandbox grows smaller — and that feels just right! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: narrowing macOS sandbox write grants by removing the /private/var/folders grant from the built-in seatbelt profile.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 unit tests (beta)
  • Create PR with unit tests

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

@DorianZheng DorianZheng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DorianZheng DorianZheng enabled auto-merge June 15, 2026 08:49
@DorianZheng DorianZheng disabled auto-merge June 15, 2026 08:51
@DorianZheng DorianZheng merged commit 3386730 into boxlite-ai:main Jun 15, 2026
26 checks passed
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