Skip to content

[security] bash has no kernel-enforced file boundary — the guard is a substring denylist #121

Closed

Description

@yusiwen

Context

bash commands are guarded by DefaultSandbox.CheckCommand — a strings.Contains match against a denylist — plus a prefix/operator scan in plan mode (checkPlanModeWrite).

That is a speed bump, not a boundary. rm -fr, sh -c 'rm -rf /', a path assembled from variables or printf, base64 -d | sh, or invoking a binary by absolute path all defeat substring matching. Nothing stops a command from writing anywhere the process user can write, even though the file tools are confined to the project root. Plan mode's guard is also string matching, so the two modes disagree about where the boundary is: a file tool respects the fence, the shell does not.

What to build

  • A real file-effect boundary for bash: wrap the ['bash','-c',cmd] argv in a launcher that grants read/write only under the roots from the shared helper (see [security] Derive the writable roots once and share them between the file fence and command execution #120), and denies writes elsewhere.
  • On Linux, Landlock is the first candidate (kernel >= 5.13, no privileges, no namespaces); bwrap is the fallback where it is usable. The backend choice belongs to a small, explicit selector with a documented preference order.
  • Probe the backend at startup, cache the verdict, and report which one is active — do not discover at first use that nothing can confine.
  • If no backend can enforce the mode, refuse to run the command and say so (fail closed). Never run a confined mode unconfined.
  • Demote CheckCommand to advisory: keep the obvious-danger warnings, but stop treating the denylist as the enforcement.
  • Report the refusal through a structured channel, not by matching stderr text. The launcher must signal "denied" with a dedicated exit code and/or a control descriptor, so the tool classifies the result without parsing human-readable output and without depending on what the command itself printed. (A denied bash command's stderr is the command's, not the sandbox's — string matching there is unreliable.)

Acceptance

  • With the boundary active: bash -c 'echo x > "$HOME/outside.txt"' is refused by the kernel and the file does not exist, while bash -c 'echo x > $PROJECT/x.txt' succeeds. Both are asserted by a gated test.
  • A denied command's result carries a structured denial fact (mode + denied) and a stable marker; the classification reads the structured channel only — a test proves the result is identical when the command's stderr is empty, and when it is full of the words the sandbox would print.
  • With the backend unavailable (simulated), the command is refused, not executed, and the refusal names the missing backend.
  • The denylist still warns on its patterns, but a test shows a bypass (sh -c 'rm -fr …') now fails at the kernel boundary, not at the string.
  • The probe verdict is cached: a test asserts the backend is not re-probed per call.

Depends on: #120 (shared writable-roots helper).

Verification

Three layers, each catching a different failure.

  • Unit (tool, no PTY). With a stub backend: a write inside a writable root succeeds, the same command's write outside it fails. Classification reads the structured channel only — a test asserts an identical verdict when the command's stderr is empty and when it is filled with the words a sandbox would print. Backend unavailable → the command is refused, proven by a sentinel file that would exist had it run.
  • Integration, gated. The real backend is exercised on Linux; the test skips with a printed reason when no backend is usable, so a silent skip cannot look like a pass.
  • Black-box scenario — the only layer that proves the wiring end to end (config → agent → tool → the real binary on a PTY). New file tui/testdata/scenarios/confine-bash-write.scenario:
    • a new stub mode asks for one bash call that writes one file inside the project root and one outside it;
    • after the binary exits, the wrapper shell prints the verdict the scenario waits for, e.g. CONFINE inside=yes outside=no;
    • the assertion is selective on purpose: if no backend were available and the command were refused wholesale, inside=yes would fail — so a fail-closed refusal cannot masquerade as a boundary;
    • gated like the lsp-* files: a new skip pattern in make test-tui-scenarios, a new make test-tui-confine target, and a CI step, so a host without a backend skips loudly instead of failing.
  • Not assertable on screen: the TUI renders tool calls, not tool result text, so a scenario cannot match the refusal marker itself. The filesystem verdict is the observable.

Landing plan and progress: #127.

Activity

  1. added
    securitySecurity hardening, sandbox or SSRF residual
    on Oct 8, 2026
  2. yusiwen commented on Oct 8, 2026

    @yusiwen
    OwnerAuthor

    Backend and launcher decision (for review)

    Preferred backend: Landlock (Linux). Unprivileged self-restriction since kernel 5.13, no namespaces and nothing to install; it is the only candidate with no external dependency. Reference: Landlock: unprivileged access control.

    Fallback: bwrap, probed and used only when usable. It is an external binary and needs unprivileged user namespaces, so it cannot be the primary.

    Out of scope here: macOS. The existing openat2 + no-follow walk stays the portable check; the command boundary is Linux-only and must say so out loud rather than degrade silently (that reporting is #123).

    Launcher shape: re-exec this binary (tinycode __sandbox-exec -- <cmd>) rather than shipping or depending on an external launcher.

    • one binary, no new dependency and no version pin;
    • the ruleset has to be applied after fork and before exec, which a plain exec.Cmd cannot express; a re-exec entry point can, at the top of main before any argument parsing;
    • the process exit code is then ours to define, which is what makes the structured channel below possible.

    Structured channel. The launcher reports "denied" with a dedicated exit code and, where needed, a control descriptor. Classification must not read the command's stderr: that stderr belongs to the command, and a command that happens to print words resembling a sandbox error must never be classified as one. This is an acceptance item above.

    CI feasibility. The runner image's kernel reports CONFIG_LSM="landlock,lockdown,yama,integrity,apparmor" on kernel 6.17 (actions/runner-images#14783), so a kernel backend should be enforceable on ubuntu-latest. That is a report, not a measurement: the first PR that adds the boundary will print the probe verdict in CI, and only then decide whether the confine-* scenarios run in a default job or behind a skip gate.

    Tracked in #127 (landing plan and progress).

  3. yusiwen commented on Oct 8, 2026

    @yusiwen
    OwnerAuthor

    First half landed in PR #132, rebase-merged onto master as 3e13a3d (mechanism) + 9d8f476 (the syscall fix CI caught). This issue stays open: bash is not wrapped yet.

    What exists now

    • The launcher contract: tinycode __sandbox-exec --mode <m> --allow <root> … -- <command>, intercepted at the top of main before anything else, with a strict parser (nine malformed-invocation cases rejected) so a bad call cannot run the command under whatever boundary happened to parse.
    • The Landlock mechanism on Linux: unprivileged and self-imposed; write-class rights only, so reads stay unrestricted; the mask follows the ABI the kernel reports; /dev/null stays writable; prctl(NO_NEW_PRIVS) then restrict_self then exec.
    • The structured channel: a launcher failure exits 125 and prints tinycode-sandbox: , and SandboxLauncherFailed requires both. Nothing reads a command's stderr, and a command that exits 125 by itself is not read as a sandbox report.
    • !linux reports a launcher failure instead of running unconfined.

    Measurements

    • CI (Build + Lint + Test, Linux): TestLandlockBoundaryIsEnforced passes — it re-execs the test binary as a child, applies the real boundary there, and asserts from the parent that a write inside the granted root happened while a write outside it did not. That is the mechanism working on a real kernel, not a stub.
    • Local: go test ./... -count=1 13 packages ok / 0 failed; -race 13 ok / 0 failed; make staticcheck exit 0; GOOS=linux go vet ./... clean for arm64 and amd64 (which is what compiles the build-tagged test here).
    • CI on PR feat(tool): add the confinement launcher and the Landlock boundary #132: 8/8 green; annotations at baseline.

    The gate caught a real bug, recorded here because it is the kind that only CI can find. The first run failed with allow /dev/null: invalid argument. Cause: landlock_add_rule takes four arguments (ruleset_fd, rule_type, rule_attr, flags) and copies its own packed struct — unlike landlock_create_ruleset, which does take a size. Passing the struct's 12 bytes as a fourth argument landed them in flags, and the kernel rejects any non-zero flags before it looks at the rule. Fixed in 9d8f476 and explained in the code so the size argument is not "restored" later. This is Linux-only code that cannot run on the development host, and the requirement is documented nowhere but the kernel source.

    Still to do before this closes

    1. Wrap bash's argv through the launcher, with the mode mapped from plan/build and the roots from WritableRoots().
    2. Fail closed when the boundary is requested and unavailable, and report the facts in the result.
    3. The confine-* scenario and its CI gate: a stub mode whose command writes one file inside the project and one outside, with the wrapper shell printing the verdict so the assertion is selective — a fail-closed refusal must not be able to masquerade as a boundary.
    4. Decide and document that confinement is opt-in: confining commands to the workspace plus temp would break toolchains that write their own caches (GOCACHE, ~/.npm, …), so turning it on has to be a choice that says what it costs.
  4. yusiwen commented on Oct 8, 2026

    @yusiwen
    OwnerAuthor

    Landed. The command boundary exists end to end: PR #132 (3e13a3d, 9d8f476), PR #133 (ba245db), PR #134 (4212201, 799b173, fa23cfa, dadd391, 4a8d857).

    What it does now

    • tinycode __sandbox-exec --mode <m> --allow <root> … -- <command> is the launcher, intercepted at the top of main, with a strict parser.
    • On Linux it applies Landlock — unprivileged, write-class rights only, mask built from the kernel's ABI, /dev/null writable — then execs the command.
    • sandbox.confine_commands (off by default) routes every shell command through it; plan maps to read-only with no granted path, build to workspace-write over exactly WritableRoots().
    • A launcher failure is reported as "the command did not run" with the launcher's own diagnostic lines, and requires both exit 125 and our prefix, so a command that exits 125 by itself is not read as a sandbox report. A command's stderr is never classified.
    • /sandbox reports the switch and whether the host can honour it; startup warns when confinement is on with no writable root or no mechanism.

    Measurements

    • The assertion on a real runner (job Sandbox confinement (Landlock + PTY), run 37727155330): mechanism check: a granted root is writable, then saw CONFINE inside=yes outside=no, ok: 14 steps. The kernel allowed the write inside the granted root and denied the one outside it.
    • LSM list: lockdown,capability,landlock,yama,apparmor,ima,evm on kernel 6.17 — printed by the job rather than assumed.
    • TestLandlockBoundaryIsEnforced passes with -v on that runner; it skips with a reason where the kernel has no Landlock, so a missing feature cannot look like a pass.
    • Local: 13 packages ok plain and under -race, make staticcheck exit 0, GOOS=linux go vet clean for arm64 and amd64, make test-tui-scenarios 27 scenarios ok with confine-* skipped by name.

    Three real bugs the gate caught, and one I caused

    1. landlock_add_rule takes no size argument — the 4th argument is flags, and passing the struct's 12 bytes there made the kernel reject every rule with EINVAL. Found by the Linux test on its first CI run; documented nowhere but the kernel source.
    2. unix.Exec does no PATH lookup, so the launcher applied the boundary and then failed to start bash. Every confined command would have reported "the boundary could not be applied" — the one message that sends a reader to the wrong place.
    3. Confinement on with no writable root refuses everything, inside the workspace included. The interactive path does not fall back to the working directory, so it is now warned about at startup instead of looking like a broken boundary.
    4. A diagnostic line I added dropped a closing quote and /bin/sh refused the whole scenario payload; the scenario failed one step before anything it tested. sh -n on the extracted payload finds that without a PTY.

    Correction to the record. The PR #132 body said CI verified the boundary. That was true — the test does pass there — but at the time I had asserted it from a green job without confirming the test had not skipped, which is exactly the failure mode the repo's testing notes warn about. The -v step added here is what makes that claim checkable, and it is now checked.

    Still not done, deliberately out of scope

    • Confinement is off by default. Turning it on confines commands to the session's writable roots, which breaks toolchains that write their own caches (GOCACHE, ~/.npm, …). Making it the default means deciding which cache roots a command may write, which is a policy change of its own and is not in this issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity hardening, sandbox or SSRF residual

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions