signals: a signal to the tree is the payload's to answer, and snug is the only author of the run's death (closes #595)

[vyskocilm]

Closes #595.

Red-team round is DONE: no sandbox escape, no boundary crossing. Two findings, both answered on the branch — one fixed, one kept and pinned. Definition of done is green: make gate, make verify (6 passed, 0 skipped, 0 failed), make integration with SNUG_REQUIRE_SANDBOX=1 (sandbox 555.2s, signals 68.3s).

What the round held against

sigseal on both arms (launcher with INT/HUP SIG_IGN and USR1 + an RT signal blocked -> payload reads SigBlk 0, SigIgn 0); no SIG_IGN leak into a live podman engine; the relay cannot reach host pids (payload sees pids 1-5, host-range kill -> ESRCH); grace is single and bounded (second signal cuts at 0.21s, ignoring payload capped at 1.02s); a setsid+disown daemon spawned DURING the grace dies with the pidns collapse; localhost still refused from @net after pasta's Setpgid; unshare -Urpf inside the staged sandbox -> EPERM; terminalWillDeliver is evaluated pre-fork so a payload sharing the pty cannot influence it with tcsetpgrp.

Finding 1 — the relay reached the payload but not its foreground child (FIXED, 52008b5)

trap "echo TRAPPED; exit 7" TERM; sleep 30   +   kill -TERM <snug>, no tty
  before   trap never fired, burned the whole 1.018s, then swept
  after    trap fired at 11-12ms, 3/3

A POSIX shell defers a trap until its foreground child returns; signalling the shell and not the sleep leaves the handler unrun. The relay now names the sandbox's process group — what a terminal does. Safe because bwrap's init calls setsid() and LEADS that group, so the group id is the init's own pid, already pidfd-pinned. Both conditions checked, never assumed.

My own measurements missed it because every payload used sleep 0.05 in a loop — short enough for the shell to reach its trap inside the budget. The 4/4 below was true and proved less than it looked: the exact error this PR accuses #595's reproduction of making. TestAHandlerBehindALongBlockingChildStillRuns is the missing case and FAILS against the old relay (verified by disabling the group path).

This also flipped TestGroupDeliveredSIGINTTearsDownTheWholeSandboxWithoutForwarding, whose payload is that same blocking-child shape: the trap now fires and snug exits 7, which is what the stage commit's by-hand transcript expected before #105's guard flipped it as a side effect. Renamed TestAGroupDeliveredSIGINTLetsThePayloadAnswerItFirst; its header keeps the old name, assertions and false #13 justification verbatim.

Finding 2 — exit-code substitution (KEPT, pinned)

A payload that traps the signal and exits 0 makes a signalled run report 0, so CI gating on $? alone can be shown success for a run it force-stopped. Honest trade rather than a hole — nothing crosses the boundary and the payload already picks the code on every ordinary exit — but the new part is that "the operator killed me" can hide behind a zero. A harness that cares should treat "I sent a kill" as failure independently of $?. Named in grace's comment and pinned by TestASignalledRunCanReportThePayloadsOwnZero.

#595 asks the wrong question, measured

Its own reproduction does not reproduce. trap 'echo yes > f' INT is 10/10 on main with #101's nesting confirmed on (--dry-run TOPOLOGY: pid nesting depth 2). It measures whether the handler STARTED. Split apart — trap INT, write h-start, do N work, write h-done:

arm	handler work	h-start	h-done
offline	none	8/8	3/8
offline	spin 100 (~1.5ms)	8/8	0/8
offline	sleep 0.05	4/4	0/4
staged @net	none	4/4	3/4
staged @net	sleep 0.05	4/4	0/4
offline, snug SIGSTOPped first (control)	none	4/4	4/4

Handler always ENTERS, never finishes past ~1.5ms of shell work, on BOTH arms — and staged @net has no intermediate namespace, so #101 is not the cause.

#13 is a STARTUP leak. SIGKILL of snug 1.0s into a running payload: survived 0/4 offline, 0/4 staged, heartbeat frozen at 0.4s and 1.4s after snug died, no Go code involved. teardown.go's own table: 0 leaks at 86-94ms, 8/8 at 110-160ms, ~206ms payload start. A grace gated on a named init cannot touch it.

The real question is who answers a signal delivered to the tree. It was decided by the scheduler and differed by topology, by host and by launcher.

Three independent authors of the staged tree's death

Nothing calls setpgid, so a terminal's ^C reaches every group member. One member at a time, @net:

SIGINT to the stage alone        run ends, snug exits 69
SIGINT to the outer bwrap alone  run ends, snug exits 255
SIGINT to the offline NP bwrap   IGNORED, payload keeps running, tree intact

The offline arm has ONE author because its bwrap is pid 1 of #101's namespace. So #101's nesting — which #595 blames — is what makes a grace possible, and the arm lacking it is the one that could not have one.

Signal state leaked into the payload, differently per arm

bash -c 'snug -p @net … &'      staged payload SigIgn 0x2   offline 0x0
python blocks USR1, execs snug  BOTH arms      SigBlk 0x200

SigIgn 0x2 is SIGINT, so trap … INT was a silent no-op; SigBlk 0x200 is SIGUSR1, so trap … USR1 never fired anywhere. execve resets CAUGHT handlers and preserves IGNORED ones and the mask — those two are the whole leak. The asymmetry was an accident of ordering: Go installs no handler for a signal already SIG_IGN at startup and os/exec only resets what it handles, so the offline arm escaped because armTeardown precedes cmd.Start() while the stage is forked before the guard.

What is built

• internal/sigseal — clears the mask, forces ignored dispositions to default, both exec verbs. Arch-independent (signal.Notify over the SIG_IGN, execve clears it): no rt_sigaction, no cgo, no arm64 struct layout to get wrong.
• internal/nestproc — the procfs an intermediate pid namespace needs, one author for both verbs.
• Stage stops being an author. MainServe catches and drops INT/TERM/HUP/QUIT (Notify, never Ignore — SIG_IGN inherits into every child). runOneSandbox forks bwrap into CLONE_NEWPID|CLONE_NEWNS; no CLONE_NEWUSER, the stage already holds CAP_SYS_ADMIN in U, so it costs no nesting level. info.InitPID translated once.
• The grace. INT/TERM/HUP, once the init is named: 1s to handle and exit, payload's own exit code reported, second signal cuts. Relay ONLY where the terminal could not have delivered (ttydelivery.go) — on a shared session the tty already delivered and relaying would make one keypress read as the two that mean force-quit to compose, npm, pytest, vite. Budget is snug's number: no flag, no profile key.
• pasta gets Setpgid so a group ^C cannot kill the network out from under a payload still inside its window.

Measured

Real pty, handler doing sleep 0.5, 4 trials/cell. Every cell was 0/4 before:

delivery	offline	staged @net
^C	4/4	4/4
kill -INT snug, no tty	4/4	4/4
kill -INT snug, snug is fg tty job	0/4	0/4

The no-tty row reached the payload on NO host before. These payloads all loop on sleep 0.05; see finding 1 for the long-blocking-child shape they did not cover, which is now its own test. The last row is the deliberate residual — snug cannot tell it from a ^C (os/signal carries no si_code), so it relays nothing rather than double-delivering every real keypress; stated in ttydelivery.go.

Negatives, both arms: payload ignoring the signal exits 130 at 1.01-1.06s with 0 leftovers; second ^C cuts at 0.27s; payload sees exactly one SIGINT per keypress; $? is the payload's own 7; SIGKILL at steady state still leaves nothing.

Abuse sentence

A hostile payload can keep running inside the sandbox, with what the policy already granted it, for up to 1s after the operator asked it to stop — and can make that second happen every time by ignoring the signal.

Bounded by the constant, by a second signal, and by confirmTeardown running afterwards unchanged. No new path, no new visibility, no host reach.

Two hazards found in self-review, fixed here

• The init is pidfd-pinned. The relay targets the init's CHILDREN with a CATCHABLE signal, so a reaped-and-recycled init pid would have snug SIGTERMing a stranger's children. A pidfd holds the kernel's struct pid, so the number cannot be reused. Named regression test reads the pin back out of /proc/self/fdinfo.
• The payload is COUNTED even where it is not relayed to, or a terminal-delivered signal spent the whole budget on a container run whose payload was still PARKED on --block-fd and never forked.

What this forced

#594's container stop now runs on the signalled path, because a payload exiting inside its grace exits NORMALLY and st.Wait returns an ordinary status. Holding ^C on a container run can cost both budgets, ~2s. Two tests encoded the old guarantee; renamed and re-conditioned to assert the surviving negative (payload ignores -> no stop).

A test pinned the old behaviour with a FALSE reason. TestGroupDeliveredSIGINTTearsDownTheWholeSandboxWithoutForwarding said "this is the correct, safety-motivated behaviour to pin — a future change that made snug wait for the payload's own handler would reopen that window", citing #13. Refuted above. History: the stage commit's by-hand transcript (2026-08-13) expected trap … exit 7 to fire and snug to exit 7; #105's guard (2026-08-17) flipped it as a side effect; #592 (2026-09-20) turned the stale transcript into a test with the wrong reason. Nobody decided the current behaviour. The transcript's expectation is restored and now asserted.

Prose corrected where it was measurably false

• exec.go's unconditional "Ctrl-C reaches the payload" — depends on --new-session, which policy.NewSession() emits on two independent conditions.
• INDEX.md's "~40 ms before bwrap arms --die-with-parent" — teardown.go already says in capitals that number is wrong — and its "Ctrl-C reaches every stage", where reaching the stage is what killed it.
• dryrun.go's claim that confirmTeardown SIGKILLs the stage on a catchable signal — on a pty the stage died of the tty's own SIGINT first.
• stage.Wait's "check dmesg for an OOM kill" — the stage no longer exits on its own.

Tests

internal/sigseal with its unsealed arm as the positive control; test/integration/payloadgrace_test.go (5 tests — 4 fail against a build with grace() neutered, and the two upper bounds say in their own comments that they cannot stand alone); the pidfd-pin regression; and scripts/0040 for the real-pty ^C path CI structurally cannot run (with no terminal every Go test takes the relay path instead).

One CI failure, diagnosed twice

TestSealClearsWhatExecveWouldOtherwiseCarry/plain failed on CI, passed locally: SigBlk SIGUSR1 bit = 0x0, want 0x200 — the unsealed CONTROL, the arm whose job is to prove the test can fail.

ubuntu-latest's /bin/sh is dash, and dash clears the inherited signal mask at startup. The child exec'd /bin/sh -c 'grep …/proc/self/status', so the program asked to report the mask rewrote it first. One ubuntu:24.04 container, same binary, same blocked SIGUSR1: dash SigBlk 0, bash 0x200, grep with no shell 0x200, cat 0x200. SigIgn 0x2 survived all four — a disposition is not a mask, which is why it looked selective. Reporter is now cat /proc/self/status, parsed in Go; control both ways in that container, the old binary failing with CI's line byte for byte.

1e5866e diagnosed the same failure as a per-thread race and added runtime.LockOSThread(); the failure came back unchanged at 52008b5 with the lock in place. Lock stays (Seal() has always locked), its false cause comment does not. Product unaffected either way — a shell clearing MORE than Seal() cannot make a payload inherit anything.

Performance, attributed rather than assumed

A/B on the nesting alone, same two engine tests, -count=1: 17.941s disabled vs 17.748s enabled — it costs nothing. The sandbox suite's wall-clock spread is podman-tier variance; it once hit the 8m -timeout, which is the sizing the Makefile already documents as wrong for a host where the engine tier runs. Not changed here.

🤖 Generated with Claude Code