fix(sandbox): Landlock FS bound plus workspace_only defense-in-depth

[adriannoes]

Summary

Local rewrite of the sandbox FS cluster that Cloud tried to land as #87 and #88. One PR against development. Operator merges.

Landlock is the kernel host-FS bound. workspace_only stays defense-in-depth. The #87 encoding cat-and-mouse (\x2f, identity PW\D=, quote-split file:, …) is frozen — that loop does not converge and is why Cloud never merged.

Allowlist DiD (original #66, 68a, #82)

• Walk the first existing ancestor when realpath fails (workspace/../../tmp/stolen).
• Collapse . / .. lexically so a missing directory before .. cannot pin the walk at the workspace; do not cancel .. across a symlink.
• Scan quoted/embedded / ~ and relative ../ on the full command.
• Extract file: URLs (including percent-decoding). https:// stays allowed.
• Expand $HOME / $PWD on the full command, including glued $IFS and mid-token $HOME. Other $ forms fail closed.
• Fail closed on in-command HOME= / PWD= assignment, export, and unset.
• Resolve bare relative tokens (cat leak) against the workspace.
• strdup OOM is fail-closed.

Kernel bound (original #74, #76) plus #88 Must Fix

• Probe Landlock ABI; ABI-1 attr size so older kernels are not E2BIG.
• RW under the workspace; RO/exec for /bin /usr /lib*; writable /dev/null /dev/zero /dev/urandom /dev/tty.
• Privileged unshare(CLONE_NEWNS|NEWNET|NEWPID) first; on failure enter user ns then unshare again. Still failing → isolation error (fail-closed).
• Fork after CLONE_NEWPID so the command is PID 1 (/bin/echo A; /bin/echo B can still fork).
• Isolation failure uses a control pipe, not sh exit 122/123.
• MS_REC|MS_PRIVATE after CLONE_NEWNS. PR_SET_NO_NEW_PRIVS fails closed.
• No .github/workflows/ci.yml AppArmor sysctl hack. Tests skip or treat deny as success when the runner cannot apply userns/Landlock.

Jetson GPU (/dev/nvhost, /dev/nvgpu, /dev/nvmap) and Argus (/tmp/argus_socket) substring blocklist entries on development are preserved. Landlock does not grant /dev/nv*.

Cloud failure (do not repeat)

• fix(sandbox): harden workspace_only host-FS allowlist #87: Security Agent kept finding new encodings; the scanner was treated as the host-FS gate. Approval never approved (Security pending). Co-authored-by Cloud.
• fix(sandbox): Landlock FS bound plus workspace_only defense-in-depth #88: Personal review CHANGES_REQUESTED — CLONE_NEWPID without fork(), exit 122/123 colliding with isolation sentinels. Cloud-only apparmor_restrict_unprivileged_userns=0 in CI. Same authorship issue.

Test plan

• CI=true make clean && CI=true make test
• make static
• make test_allowlist / make test_sandbox (macOS: Landlock/userns tests skip; allowlist string tests run)
• Linux CI: Landlock + netns + PID-ns multi-command + exit 122/123 are not isolation failures

Residuals

• Allowlist is not a language interpreter: chr(47)+ has no path character (Landlock is the bound).
• Encoded-slash / identity-escape / quote-split file: / printf -v / nameref cat-and-mouse is frozen.
• Conservative scanner FPs: awk '/foo/', echo HOME=foo.
• Empty workspace_path skips Landlock (namespaces still apply).
• No pivot_root / minimal /dev.
• cgroup setup remains non-fatal when the hierarchy is not writable.

Supersedes

Supersedes #87 #88.

Original Bugbot / split PRs covered here: #66, 68a (quoted-path half of #68; 68b bind gateway.host already on development), #82, #74, #76.

Does not mix file/cron/config clusters.

Operator merges.

[cursor]

Summary

This is a focused sandbox rewrite that does what the title claims: Landlock is the kernel host-FS bound, and workspace_only is defense-in-depth. It closes the #87 glued $IFS scanner hole and the #88 isolation bugs (control pipe instead of exit 122/123, fork so the command is PID 1, writable /dev/null). The extra fork after CLONE_NEWPID breaks the timeout contract: the parent SIGKILLs the isolator, and the command (PID 1 in the new namespace) is reparented to host init and keeps running. test_timeout_kills_process still passes because it only looks at the output string.

Repo is a C99 edge agent (namespaces + cgroups + tools/channels/gateway). This PR stays in src/sandbox/ plus tests and SECURITY/ARCHITECTURE notes.

Must Fix

1. Timeout does not kill the sandboxed command (src/sandbox/sandbox.c around the second fork, and reap_child). unshare(CLONE_NEWPID) does not move the caller; the new child is PID 1. On timeout the parent kills the isolator only. Reproduced here: after sandbox_exec("sleep 60", …, 300, NULL), sh -c sleep 60 was still running with NSpid … 1 and PPid 1. Same leak from test_timeout_kills_process itself. Fix with prctl(PR_SET_PDEATHSIG, SIGKILL) in the command child before execl, or clone(CLONE_NEWPID|SIGCHLD) so the pid the parent tracks is PID 1 (killing it tears down the namespace). The regression must assert the process is gone, not only that the output contains timed out.

Should Fix

• Whole /proc grant without remounting procfs (src/sandbox/sandbox_landlock.c RO_PATHS). ls /proc showed host PIDs; cat /proc/<host-pid>/cmdline succeeded under Landlock. /proc/1/root/etc/passwd stayed EACCES, so this is a process leak, not a full FS bypass. Remount proc after PID 1, then grant /proc or /proc/self.
• docs/SECURITY.md GPU section still says sandbox_exec() does not call mount(). The child now does mount(…, MS_REC|MS_PRIVATE, …).
• rw_file for /dev/null omits LANDLOCK_ACCESS_FS_TRUNCATE while ABI 3+ handles that bit. echo hi >/dev/null worked on this ABI 6 kernel (char device), but the UAPI says O_TRUNC needs TRUNCATE.
• landlock_handled_fs stops at ABI 3 FS bits. This kernel reports ABI 6, so later rights (including IOCTL_DEV) stay unhandled and therefore allowed. Probe and handle them even if the distro headers lack the define.
• No closefrom after fork. /proc/self/fd showed inherited sockets. Landlock does not restrict already-open fds.
• read(errpipe[0]) has no timeout; a stuck unshare hangs the caller.
• test_shadow_not_accessible uses cfg == NULL (no Landlock) and only checks strlen(out) > 0.

Nice to Have

• awk '{print $1}' fail-closes on $1 (same conservative class as documented awk '/foo/').
• An https URL whose path contains file: can hit the file: scanner (https://example.com/file:foo).
• Freeze further allowlist encoding growth, as the PR body already says.

Positive Highlights

• Control pipe instead of sh exit 122/123, with tests for those exits.
• /bin/echo A; /bin/echo B runs (the #88 Cannot fork bug).
• Glued $IFS, mid-token $HOME, percent-decoded file:, missing-dir .., and relative symlink cat leak tests are real containment checks, not coverage padding.
• Split allowlist_path.c / sandbox_landlock.c keeps allowlist.c at 686 lines (under the 1000-line rule).
• Fail-closed namespaces with user-ns fallback, and no AppArmor sysctl CI hack.

Local evidence: CI=true gcc -Werror test_allowlist and test_sandbox passed (linked -lm; Makefile still passes -lcurl). ASan test_sandbox passed aside from the leftover sleep. Landlock ABI 6. GitHub static, test, release was still pending at review time.

  

_{Sent by Cursor Automation: Adrianno’s personal code review}

[cursor]

Stale comment

[cursor]

Stale comment

Security review of the Landlock + PID-1 sandbox rewrite found two medium issues: cgroup limits still attach to the isolator rather than the command process, and the Landlock RO grant of /etc/ssl re-opens a host-FS subtree that the string scanner cannot see. Prior threads on timeout reaping, /proc, and inherited FDs look addressed on this head.

  

_{Sent by Cursor Security Agent: Security Reviewer}

[cursor]

Security review of this head found one remaining medium issue: reserved runtime-state filenames are no longer whole-command substring-blocked on the sandbox-on path, so wrapper commands can still read them when they live in the workspace.

  

_{Sent by Cursor Security Agent: Security Reviewer}

Dismissing the stale personal review on outdated SHA 88b3686. All review threads are resolved; current head is ec9ef6b.