containment-broker: six remaining findings from the #30 review (one fail-open of the same class as the seccomp bug)

[thegeorgepu]

Remaining findings from the adversarial review of #30 that were not fixed in that PR, because they
sit in pre-existing src/ and session_linux.c code the PR did not touch. Each is tagged with how far
it was actually checked — several review claims turned out to be stale or wrong elsewhere in this
review, so please treat the tags as load-bearing.

1. monotonic_now reports success while never assigning the clock — VERIFIED (read)

native/session_linux.c:68

if (clock_gettime(CLOCK_MONOTONIC, &value) != 0 || value.tv_sec < 0) return -errno;

On the tv_sec < 0 branch clock_gettime SUCCEEDED, so errno is unset or stale and the function
returns -0 == 0 — success — with *now never assigned. remaining_milliseconds (:76) and the
zero-stability loop (:918, :924) then compare an indeterminate value against a deadline.

This is the same defect class as the seccomp fail-open fixed in #30 (trampoline_linux.c:70):
return -errno on a branch where errno was never set. I swept all 25 return -errno sites in
native/ for the pattern; these two are the only instances, and every other site follows a call that
genuinely sets errno. So the class is bounded, and this is the one that remains.

2. Receipt HMAC key not zeroed on four early returns — VERIFIED (read)

native/session_linux.c:906,908,910,913. In vinci_broker_session_finalize_terminal the key is read
into key at :886; attach_key/capture_key/policy_key/ingress_key are zeroed at :903-904,
but the four cgroup.freeze / cgroup.kill failure returns leave key un-zeroed, unlike every other exit
(:899, :941, :955, :956). A failing openat("cgroup.kill") — routine on a host without the kill
controller — leaves the 32-byte signing key in the stack frame.

3. The mutation-definition tests execute no product code — VERIFIED (read)

test/mutation-definitions.test.mjs asserts against test/harnesses.mjs, which contains five
export const frozen arrays and imports nothing from src/ or native/. Three of its four tests
therefore compare literals to literals and cannot fail for any change to the broker, while
package.json exposes them as npm run test:mutations and #30 wired them into CI as coverage. The
fourth test — the source_digests enumeration — IS load-bearing and mutation-tested. Either give the
matrices a real consumer or stop presenting them as a mutation suite.

4. linkat(AT_EMPTY_PATH) may make receipt persistence impossible unprivileged — REPORTED, probe pending

native/session_linux.c:281. persist_atomic publishes the O_TMPFILE receipt with
linkat(fd, "", dirfd, name, AT_EMPTY_PATH), which required CAP_DAC_READ_SEARCH before kernel 6.10.
An unprivileged broker would get EPERM, which is not EEXIST, so -EPERM propagates and every
receipt persist fails: commit_prelaunch drops to RECONCILE_ONLY (:618), begin_closing to stage 2
(:825), finalize_terminal to stage 3 (:947), and no session can ever reach SEALED. The portable
form is linkat(AT_FDCWD, "/proc/self/fd/<fd>", dirfd, name, AT_SYMLINK_FOLLOW). I have a probe
queued on a real unprivileged host and will post the result here.

5. Recovery tombstone is written before fsync — REPORTED, not independently verified

src/journal.mjs:186. recoverToUncontained writes UNCONTAINED.json with {flag:"wx"} and fsyncs
only afterwards (:194-200); a crash between the two leaves a torn file, and every later recovery
attempt hits JSON.parse at :186 and throws a raw SyntaxError instead of the intended
JournalError("...", "RECOVERY_TOMBSTONE_INVALID"). Same line also reads without O_NOFOLLOW,
inconsistent with the openSync(..., O_NOFOLLOW) two lines below.

6. recordDomainZero has no duplicate-call guard — REPORTED, not independently verified

src/capture.mjs:72, unlike closeIngress (:61, CAPTURE_CLOSE_DUPLICATE) and seal (:98,
CAPTURE_SEAL_DUPLICATE). A second call with a different proof passes every check and silently
overwrites this.zeroProof — the evidence the terminal receipt is built from — with no record that a
first proof existed.

---

Two other findings from the same review are filed separately as #34 (canonicalization collision) and
#35 (bare TypeError from a refusal path), both reproduced. One further claim — that the prctl
varargs UB was harmless at current call sites — was WRONG in the other direction: there was a short
call site, and it is fixed in #30.

[thegeorgepu]

Item 4 is now CONFIRMED on a real unprivileged Linux host, not just reported. Probe run on an
unprivileged container (kernel 6.8.0-88-generic, CapEff all zeros), against an ext filesystem —
not an overlay, to rule that out:

===== filesystem of the persistent volume =====
ext2/ext3
===== receipt persist on /home/jovyan =====
AT_EMPTY_PATH linkat FAILED: errno=2 (No such file or directory)
  -> persist_atomic cannot publish a receipt here; no session can reach SEALED
/proc/self/fd form OK — the documented portable alternative works here

Two corrections to the original finding, both of which change the fix:

1. The errno is ENOENT, not EPERM. The review predicted EPERM from the missing
   CAP_DAC_READ_SEARCH. On this kernel the empty path is simply not resolved and the call returns
   ENOENT. Anyone fixing this by special-casing EPERM would miss it entirely. What matters for
   persist_atomic is only that the errno is not EEXIST, so it takes the -saved branch at
   session_linux.c:282 and the failure propagates regardless of which errno it is.

2. The proposed alternative is verified working on the same host in the same run —
   linkat(AT_FDCWD, "/proc/self/fd/<fd>", dirfd, name, AT_SYMLINK_FOLLOW) succeeded where
   AT_EMPTY_PATH failed. So the fix is confirmed available, not just documented.

One further portability note found on the way: O_TMPFILE is unsupported on overlayfs, which is a
very common container root filesystem. The first probe run failed at openat(..., O_TMPFILE) with
EOPNOTSUPP on /tmp before reaching the linkat at all. So persist_atomic has two independent
environment dependencies — the filesystem must support O_TMPFILE, and the link must not rely on
AT_EMPTY_PATH — and a receipt directory placed on an overlay mount fails on the first one.

Separately, on item 4's blast radius: because this is the shared publish path, the practical effect is
that on such a host commit_prelaunch drops to RECONCILE_ONLY, begin_closing to stage 2 and
finalize_terminal to stage 3, so no session can ever reach SEALED — the broker degrades to a
mode where it can never produce terminal evidence, rather than failing loudly at startup.