fix(test): stop the suite signalling pids it does not own — SIGKILL kills systemd --user, SIGTERM logs you out

[Gunther-Schulz]

Four times in one afternoon, running this suite took my whole desktop down.
The kernel audit subsystem named the sender: a node test-runner process
SIGKILLed systemd --user, and pid 1 then tore down the session cgroup —
shell, editors, browser, terminals, mid-work.

Full write-up, evidence and the audit recipe: #351

The defect

test/proxy-held-port.test.mjs, the holder sweep:

let parent = Number(execFileSync("ps", ["-p", pid, "-o", "ppid="], …).trim());
if (parent > 1) process.kill(parent, "SIGKILL");

parent > 1 is a liveness test wearing an ownership test's clothes. An orphan
does not reparent to pid 1 on a machine running a systemd user manager:
that manager sets PR_SET_CHILD_SUBREAPER, so it inherits the orphan and
ps -o ppid= returns its pid. The sweep reads that as a parent it created.

CI never saw it because a container really does reparent to pid 1 — which is
exactly why the assumption survived review.

The fix

A choke point, not a patch at the site that fired. Every signal to a pid this
process did not spawn now goes through killOurs() in test/proc-helpers.mjs,
beside the existing OURS predicate that listeners() and ours() already
use. It resolves the target's command line and refuses anything that is not one
of this repo's own binaries.

Three outcomes, each proven against real processes before this landed:

target	behaviour
alive, not ours	throws, naming the pid and its command line
already gone	returns false quietly — reaping races stay green
alive and ours	signalled exactly as before

The middle row is what keeps the guard from being the next bug: a guard that
over-fires would have broken the suite instead of the desktop. The first row
throws rather than skipping because a silent skip is what let this run four
times before anyone knew where it came from.

Also routed through the choke point in this change:

• test/stdio-epipe-survival.test.mjs — an lsof -nP -t -iTCP:<port> -sTCP:LISTEN
  sweep with no ownership filter and no @127.0.0.1, so it matched listeners
  on any interface; n > 1 && n !== holder.pid was the only thing between that
  loop and a stranger's pid. (Its local variable is named listeners, which
  reads like the shared helper of that name but is a raw lsof — worth a look
  if you'd prefer it renamed.)
• test/proxy-holder-handover.test.mjs — two double-filtered kill sites.
• The remaining SIGKILL-by-number sites in proxy-held-port.test.mjs.

Deliberately not changed

The two negative-pgid reaps on self-spawned handles
(stdio-epipe-survival.test.mjs, shutdown-exit-code.test.mjs). They signal a
group id this process did not necessarily create — the same class of
assumption — but changing cleanup semantics without being able to run the suite
would be an unverified fix stacked on a verified one. Flagged here rather than
silently touched.

Verification, and its honest limit

• every touched file parses (node --check)
• killOurs() exercised on all three outcomes against real processes, including
  the actual systemd --user pid — it refuses it loudly
• static sweep confirms no unguarded SIGKILL-by-number remains in test/

The suite itself was not run. Running it is what breaks the machine, and it
should not be run on a systemd-user desktop until this is in. The confirmation I
can offer afterwards is the audit rule: a run followed by ausearch -k sigkill9
showing no record whose target is ocomm=systemd. Nothing weaker proves it, and
I'd rather say so than claim a green I don't have. CI will exercise the changed
paths normally, since containers were never affected.

---

🤖 Generated with Claude Code

[Gunther-Schulz]

This PR was incomplete when I opened it. New commit, and the reason is worth stating.

The first version fixed the SIGKILL sites. It ran on my machine tonight and the
desktop went down anyway — with the fix applied. killOurs() never fired,
because the call that did the damage never went through it.

test/proxy-held-port.test.mjs had two more sites doing the same ppid walk and
signalling SIGTERM:

let target = pid;
try { target = Number(execFileSync("ps", ["-o","ppid=","-p",String(pid)], …).trim()) || pid; } catch {}
if (target <= 1) target = pid;
try { process.kill(target, "SIGTERM"); } catch {}

Same defect, worse consequence. From systemd(1):

"systemd user managers will start the exit.target unit when this signal
is received. This is mostly equivalent to systemctl --user start exit.target."

SIGTERM to a user manager is a logout. The journal, five seconds into the run:
Activating special unit Exit the Session..., then Main User Target and GNOME
Session stopping, then gnome-shell faulting inside JSRuntime::destroyRuntime —
its shutdown path, so the segfault is a symptom of the exit, not its cause.

The kernel audit rule I recommended in #351 saw none of it. It watches SIGKILL.

How I missed it, since it is the same shape twice

I swept for SIGKILL. My own commit message says so — "the remaining
SIGKILL-by-number sites". The SIGTERM siblings were three lines below the ones I
changed.

The line above them is the sharper lesson, and it is not mine:

// THROUGH listeners(), which is where the ours-only predicate lives.

listeners() establishes that the listener is ours. The next statement walks
to that listener's parent and signals it, and nothing checks the parent. The
comment is true about what it describes and reads as covering the block it heads.

This commit

Both sites now go through killOurs(target, "SIGTERM"), deliberately not
inside a try/catch — the throw is the mechanism, and catching it restores the
silence that let this run unattributed for four collapses.

Expect this to make the suite red where it was previously green and
destructive, on any host where the walked-to parent is not one of yours. That
is the intended direction.

One sibling I did not touch

proxy-held-port.test.mjs:256 is the same false premise, failing quiet instead of
loud. Its orphan sweep is if (ppid !== 1) continue;, and its comment says why:
"a leaked successor is detached and always reparented to init". Under a subreaper
that is false, so on every systemd-user desktop that sweep matches nothing and
reaps nothing. test/proc-helpers.mjs already documents the consequence without
naming the cause — ten invisible orphans at once, the oldest 788 s, holding ports
and CPU that later files then time out on.

Fixing it changes what gets reaped and I cannot run this suite to find out. An
unverified fix stacked on a verified one is how the first version of this PR
happened. Yours to decide.

Verification, unchanged in kind

node --check passes on the touched file. The suite was not run — running it
is what takes the desktop down. Static sweep of the remaining process.kill
sites in that file: 505/550/2407 signal listeners()/onPort() output
(ours-filtered), 723 signals proxyPid() (filtered on /server\.mjs/ against the
command line), 816 is a signal-0 liveness probe, 256 is the site above.

If you arm an audit rule to confirm this after merging, arm both signals — the
SIGKILL-only rule is exactly what hid this:

sudo auditctl -a always,exit -F arch=b64 -S kill -F a1=9  -k sigkill9
sudo auditctl -a always,exit -F arch=b64 -S kill -F a1=15 -k sigterm15

---

🤖 Generated with Claude Code

— Gunther's Claude Code session

[Gunther-Schulz]

Third commit: the guard for this class was green on the tree that had it

A fix without the check that would have caught it just waits to be re-made, and
this repo already had that check — no test file asks lsof who holds a port. It
matched the literal execFileSync(" + lsof. The one file that had gone around
listeners() spells it spawn( + lsof, so the guard reported clean on a tree
carrying exactly the defect it was written for.

Its own comment is where the hazard is described, in the past tense:

"Four files did exactly that once, and two of them then walked UP to the
listener's parent and sent SIGTERM; a stranger's parent is this runner."

…followed by "the only thing left to police is a NEW inline call." The hazard was
known and written down. The predicate covered one spelling of it.

Discriminating pair, same input both times — the frozen upstream/main tree,
which still carries that raw call:

	result
old guard	17 pass, 0 fail — the violation is invisible
new guard	16 pass, 1 fail — "these files ask lsof directly and never reach killOurs()"

What changed:

1. Every call form, not one spelling of one — execFileSync, execSync,
   spawnSync, spawn, exec, with optional whitespace and any quote style,
   assembled so the file (itself swept) cannot match its own source.
2. Re-pointed at what actually protects the machine. A raw call is legitimate
   where a deadline is wanted — the shared helper's execFileSync cannot give
   one, and the file in question needs it for a reason it documents. What must
   never be skipped is the filter. So the rule is now "asks lsof and never
   reaches killOurs()", not "asks lsof".
3. A second arm: the call must be loopback-scoped. -iTCP:<port> matches any
   interface, so the answer can name a process merely holding the same port
   number on another address. Fixed in stdio-epipe-survival.test.mjs too — the
   fixture binds 127.0.0.1, so the narrow query was always the intended one.

Both arms are proven to fire alone, so neither rides the other: dropping only
the scope from an otherwise-filtered call goes red naming @127.0.0.1 while the
filter arm stays quiet, and the upstream tree goes red naming killOurs with the
scope arm quiet. Baseline before each mutation: 17 pass, 0 fail.

suite-collection.test.mjs is static — it reads sources and never spawns — so
this one I could actually run. It is the only part of this PR that has been
executed rather than reasoned about, and I would rather say which is which.

---

🤖 Generated with Claude Code

— Gunther's Claude Code session

[Gunther-Schulz]

CI has been red on this branch since it opened, and the cause was the branch's own guard. Fixed in cdda567; all three Node versions are green on that head.

What was failing

Five failures in run 32409699496, one cause. Every one of them was killOurs() refusing to signal a fake proxy the case had spawned itself moments earlier:

refusing to SIGKILL pid 3599: it is alive and its command line is not one of ours.
  command: node /tmp/ccf-fake-proxy-3290-3-PDRL3r/scratch-fake-server-3290-3.mjs
  ours:    /\/(?:bin|proxy)\/[\w.-]+\.mjs\b/

That is this branch's guard firing on legitimate work, and it is not a safe direction to fail in: it is precisely what trains the catch {} around killOurs() that the guard's own message forbids. The message says "filter the pid at its source" — and the call site had already done exactly that, twice, taking only children of a launcher it spawned and then only those whose command line names its own stand-in. The predicate overrode that with a weaker, path-shaped test.

The two stand-in families differed only by accident of location. The launcher copy is written into bin/, so it matched OURS; its server is written into a fresh temp dir, so it did not.

The fix

Ownership is widened with evidence that is stronger than a path segment, which is what makes this a widening rather than a softening. Stand-in names are built from ${process.pid}-${++fakeSeq}, so a command line carrying our pid inside a filename we generated cannot belong to a stranger. A bare scratch-fake-server- match could, and is deliberately not what this does.

• rebuilt per call, so a forked runner cannot inherit its parent's claim;
• reaches killOurs() only — listeners() and ours() answer "which proxy holds this port", where a stand-in is not wanted.

The predicate now has a test

It had none, which is why this shipped red. Arms come in pairs because both directions are load-bearing: a wrong NO is the failure above, a wrong YES is the outage this branch exists to stop. The heaviest arm is a different runner's stand-in — identical shape, must still be refused — because without it a bare name match passes everything else. The first case is the real command line from the failing run.

Red-first, staged so the arms are observable rather than lost behind a missing export: the old predicate re-exported under the new name gives 1 pass / 2 fail, the failure naming that exact command line; with the fix, 3 of 3. The "OURS itself is unchanged" arm is green in both, so it is not riding on the change.

What I did not do

I did not run the suite locally. Running it on a host with a systemd user manager is what this branch exists to prevent, and the fix has not yet been exercised on such a host — so CI is the verifier here, not a local green I could not honestly claim. Prior CI on this head: 1941 pass / 5 fail. Now: test (18), test (20), test (22) all pass.

Worth stating for sequencing, since it affects more than this PR: until this lands, no contributor on such a host can run the suite for any branch cut from main — which is why our other upstream-facing work is currently verifiable only in CI.

🤖 Generated with Claude Code

— Gunther's Claude Code session