From b0f0d639c630138f8f630f266f10f9ae19f767d9 Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 16 Sep 2026 22:06:36 -0400 Subject: [PATCH 1/4] test(runtime): stop the isolation sweep failing toward PASS (RIG-3818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The batched sweep suppressed awk's stderr and had no guard for an unopenable input. gawk treats that as fatal: it aborts the invocation, skips END, and exits 2 — so one unreadable file loses the other 199 in the batch, and the result is indistinguishable from genuine confinement. On a security assertion that is a failure toward PASS. Measured with the two awkProg strings taken verbatim from main and from this change, over a batch whose middle file is unreadable and whose last file holds the needle: main exits 2, finds nothing, and emits a fatal: diagnostic the old 2>/dev/null hid; this version exits 0 and finds it. The [[ -f && -r ]] guard filters the statically-unreadable case, so the reachable exposure is its TOCTOU window — checked during accumulation, opened up to 200 files later on a box with two live microVMs writing. Also fixes a test that did not exercise the property it names. Bash globs lexicographically, so over unpadded f1..f425 the "in the final batch" row landed at glob index 362, inside a mid-loop flush; neither row ever reached the trailing flush. Zero-padding to f%03d puts it at index 424, in the trailing flush, as the row claims. And moves two Linux-only quota probes out of the unix-tagged file. Off Linux they reach the refusal stub, so TestReadVolumeQuotaOnRealPath fails outright and TestReadVolumeQuotaAbsentPath passes for the wrong reason — its error comes from the stub, not the absent path. Latent today: no CI lane runs Go unit tests on darwin. RIG-3818 --- .../runtime/microvm_isolation_microvm_test.go | 29 +++++++++--- .../runtime/microvm_quota_linux_test.go | 47 ++++++++++++++++++- go/internal/runtime/microvm_quota_test.go | 42 ----------------- 3 files changed, 69 insertions(+), 49 deletions(-) diff --git a/go/internal/runtime/microvm_isolation_microvm_test.go b/go/internal/runtime/microvm_isolation_microvm_test.go index e24ec5011..6b6062b3e 100644 --- a/go/internal/runtime/microvm_isolation_microvm_test.go +++ b/go/internal/runtime/microvm_isolation_microvm_test.go @@ -167,14 +167,25 @@ func sweepScript(needle, roots string) string { // The awk program: scan every FILENAME handed to this invocation, print // `:` per match, and exit non-zero when the batch had none — so // the caller's `found` accumulator keeps grep's semantics across batches. - const awkProg = `index($0, ENVIRON["SWEEP_NEEDLE"]) { print FILENAME ":" $0; hit=1 } END { exit !hit }` + // + // BEGINFILE/ERRNO is load-bearing, not defensive: gawk treats an unopenable + // input as FATAL, so without it one unreadable file aborts the invocation, + // skips END, and exits 2 — losing the other 199 paths in the batch and + // rendering the result indistinguishable from genuine confinement. The + // guest's /bin/awk is gawk (guest-image/default.nix), so the extension is + // available even though the program is invoked as `awk`. + const awkProg = `BEGINFILE { if (ERRNO) nextfile } ` + + `index($0, ENVIRON["SWEEP_NEEDLE"]) { print FILENAME ":" $0; hit=1 } END { exit !hit }` return "export SWEEP_NEEDLE='" + needle + "'; " + "shopt -s globstar nullglob dotglob; found=1; batch=(); " + // scan() runs one awk over the accumulated batch and clears it. Guarded // on a non-empty batch so a trailing flush with nothing pending does not // invoke awk on zero files (which would read stdin and hang). + // + // stderr is NOT suppressed: it carries the one signal that separates a + // real negative from a dead probe. "scan() { ((${#batch[@]})) || return 0; " + - "if awk '" + awkProg + "' \"${batch[@]}\" 2>/dev/null; then found=0; fi; batch=(); }; " + + "if awk '" + awkProg + "' \"${batch[@]}\"; then found=0; fi; batch=(); }; " + "for root in " + roots + "; do " + "for f in \"$root\"/**/*; do " + // Collapse repeated slashes before matching: a "/" root globs to @@ -186,7 +197,10 @@ func sweepScript(needle, roots string) string { "[[ -f $f && -r $f ]] || continue; " + "batch+=(\"$f\"); " + "((${#batch[@]} >= " + strconv.Itoa(sweepBatchSize) + ")) && scan; " + - "done; done 2>/dev/null; scan; exit $found" + // One `done` closes the per-file loop, the next the per-root loop. + "done; " + + "done; " + + "scan; exit $found" } // TestMicroVMSweepScriptFindsItsNeedle is the non-vacuity control for @@ -239,8 +253,11 @@ func TestMicroVMSweepScriptFindsANeedleAcrossBatches(t *testing.T) { // Comfortably more than two full batches, so at least two mid-loop flushes // happen before the trailing one. fileCount := sweepBatchSize*2 + 25 + // Zero-padded so the glob's LEXICOGRAPHIC order matches numeric order. + // Unpadded, f425.txt sorts to glob index 362 — inside a mid-loop batch — so + // the "final batch" row below never reached the trailing flush it names. plant := "mkdir -p /workspace/many && for i in $(seq 1 " + strconv.Itoa(fileCount) + "); do " + - "printf 'filler line %s\\n' \"$i\" > /workspace/many/f$i.txt; done && ls /workspace/many | wc -l" + "printf 'filler line %s\\n' \"$i\" > \"$(printf '/workspace/many/f%03d.txt' \"$i\")\"; done && ls /workspace/many | wc -l" out, code := guestSh(t, m, id, plant) if code != 0 { t.Fatalf("planting %d filler files: exit %d, %q", fileCount, code, truncate(out)) @@ -251,10 +268,10 @@ func TestMicroVMSweepScriptFindsANeedleAcrossBatches(t *testing.T) { for _, tt := range []struct{ name, file, needle string }{ // Last file: only the TRAILING flush can find it. - {"in the final batch", "/workspace/many/f" + strconv.Itoa(fileCount) + ".txt", "SWEEP-BATCH-LAST-6c1e8f30"}, + {"in the final batch", fmt.Sprintf("/workspace/many/f%03d.txt", fileCount), "SWEEP-BATCH-LAST-6c1e8f30"}, // First file: found by a MID-LOOP flush, so `found` must survive every // later batch that matched nothing. - {"in the first batch", "/workspace/many/f1.txt", "SWEEP-BATCH-FIRST-91ad47b2"}, + {"in the first batch", "/workspace/many/f001.txt", "SWEEP-BATCH-FIRST-91ad47b2"}, } { t.Run(tt.name, func(t *testing.T) { if out, code := guestSh(t, m, id, diff --git a/go/internal/runtime/microvm_quota_linux_test.go b/go/internal/runtime/microvm_quota_linux_test.go index a75edaa7f..b24ee4ae5 100644 --- a/go/internal/runtime/microvm_quota_linux_test.go +++ b/go/internal/runtime/microvm_quota_linux_test.go @@ -8,9 +8,12 @@ package runtime // // These are separate from microvm_quota_test.go because mountRoot and deviceOf // only exist under //go:build linux — the pure decision they feed is covered -// there, on every GOOS. +// there, on every GOOS. The two readVolumeQuota probes at the end are here for +// the same reason: off Linux they reach the refusal stub, so one fails outright +// and the other passes for the wrong reason. import ( + "math" "os" "path/filepath" "strings" @@ -208,3 +211,45 @@ func TestReadVolumeQuotaPropagatesInconclusiveMountRoot(t *testing.T) { "required-quota startup fails closed with the real cause", volume, reading) } } + +// TestReadVolumeQuotaOnRealPath exercises the PRODUCTION statfs probe against a +// real directory. What it can honestly assert without root is bounded but real: +// the probe succeeds, reports a plausible filesystem, resolves a mount root, and +// — since no test box's temp dir carries a project quota — reads as NOT active. +// That negative is load-bearing: it is what proves the detection does not +// false-positive and pass an unbounded volume off as quota'd. +func TestReadVolumeQuotaOnRealPath(t *testing.T) { + dir := t.TempDir() + reading, err := readVolumeQuota(dir) + if err != nil { + t.Fatalf("readVolumeQuota(%q) = %v, want a successful rootless read", dir, err) + } + if reading.LimitBytes <= 0 { + t.Fatalf("reading %s has no block total; statfs must report the filesystem size", reading) + } + if reading.MountRoot == "" { + t.Fatalf("reading %s resolved no mount root", reading) + } + if reading.UsedBytes < 0 || reading.UsedBytes > reading.LimitBytes { + t.Fatalf("reading %s has nonsensical usage", reading) + } + if reading.Active() { + t.Fatalf("reading %s reports an active project quota on a plain temp dir; "+ + "the detection must not false-positive (that would pass an unbounded volume as quota'd)", reading) + } + // The utilization the preflight logs must be finite and in range even with + // no quota — V7 meters this value. + if ratio := reading.UsedRatio(); ratio < 0 || ratio > 1 || math.IsNaN(ratio) { + t.Fatalf("UsedRatio() = %v on reading %s, want a finite ratio in [0,1]", ratio, reading) + } +} + +// TestReadVolumeQuotaAbsentPath: a path that does not exist is a probe ERROR, +// not a silent "no quota". Under QuotaRequired that difference decides whether +// startup fails with the real cause (an unreachable volume) or with a misleading +// missing-quota message. +func TestReadVolumeQuotaAbsentPath(t *testing.T) { + if _, err := readVolumeQuota(t.TempDir() + "/does-not-exist"); err == nil { + t.Fatal("readVolumeQuota on an absent path = nil error, want a failure naming the path") + } +} diff --git a/go/internal/runtime/microvm_quota_test.go b/go/internal/runtime/microvm_quota_test.go index e01726c88..57a6b7d86 100644 --- a/go/internal/runtime/microvm_quota_test.go +++ b/go/internal/runtime/microvm_quota_test.go @@ -310,45 +310,3 @@ func TestVerifyVolumeQuotaNeverAssigns(t *testing.T) { t.Fatalf("returned reading Path = %q, want the verified path", got.Path) } } - -// TestReadVolumeQuotaOnRealPath exercises the PRODUCTION statfs probe against a -// real directory. What it can honestly assert without root is bounded but real: -// the probe succeeds, reports a plausible filesystem, resolves a mount root, and -// — since no test box's temp dir carries a project quota — reads as NOT active. -// That negative is load-bearing: it is what proves the detection does not -// false-positive and pass an unbounded volume off as quota'd. -func TestReadVolumeQuotaOnRealPath(t *testing.T) { - dir := t.TempDir() - reading, err := readVolumeQuota(dir) - if err != nil { - t.Fatalf("readVolumeQuota(%q) = %v, want a successful rootless read", dir, err) - } - if reading.LimitBytes <= 0 { - t.Fatalf("reading %s has no block total; statfs must report the filesystem size", reading) - } - if reading.MountRoot == "" { - t.Fatalf("reading %s resolved no mount root", reading) - } - if reading.UsedBytes < 0 || reading.UsedBytes > reading.LimitBytes { - t.Fatalf("reading %s has nonsensical usage", reading) - } - if reading.Active() { - t.Fatalf("reading %s reports an active project quota on a plain temp dir; "+ - "the detection must not false-positive (that would pass an unbounded volume as quota'd)", reading) - } - // The utilization the preflight logs must be finite and in range even with - // no quota — V7 meters this value. - if ratio := reading.UsedRatio(); ratio < 0 || ratio > 1 || math.IsNaN(ratio) { - t.Fatalf("UsedRatio() = %v on reading %s, want a finite ratio in [0,1]", ratio, reading) - } -} - -// TestReadVolumeQuotaAbsentPath: a path that does not exist is a probe ERROR, -// not a silent "no quota". Under QuotaRequired that difference decides whether -// startup fails with the real cause (an unreachable volume) or with a misleading -// missing-quota message. -func TestReadVolumeQuotaAbsentPath(t *testing.T) { - if _, err := readVolumeQuota(t.TempDir() + "/does-not-exist"); err == nil { - t.Fatal("readVolumeQuota on an absent path = nil error, want a failure naming the path") - } -} From 284321ed3a46d65e520de6b6bb4488a06735f92d Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 16 Sep 2026 22:43:41 -0400 Subject: [PATCH 2/4] docs(runtime): say what the sweep guard covers, not what I wanted it to (RIG-3818) Three comment corrections from #1265 review. No code change: stripping comments from both revisions leaves both files byte-identical. The BEGINFILE claim was wrong twice. "losing the other 199 paths" is the best case stated as the rule -- gawk aborts AT the bad input, so the loss is positional (bad file at 1/100/200 of a 200-file batch leaves 0/99/199 scanned). And the guard covers OPEN errors only: a file that opens and then fails to READ still aborts at rc=2 with the needle unfound, so the comment implied a hole was closed that is not. Filed as RIG-3862 with the residue and the accumulator bug behind it -- a fatal rc=2 reads as a clean negative to scan(), which also drops a hit already printed before the abort. The padding comment cited glob index 362 as absolute; it is locale-specific (en_US.UTF-8 gives 360, and moves f1.txt from 0 to 110). That is why two measurements of mine disagreed: both were right in different collations. The comment now states the invariant -- padded ranks are 1 and 425 in ANY collation -- with the guest C-locale number as an aside. And microvm_quota_test.go still promised coverage for the two probes this PR moved out of it: its only remaining readVolumeQuota mention was that promise. Replaced with a pointer to their new home. RIG-3818 --- .../runtime/microvm_isolation_microvm_test.go | 17 ++++++++--------- go/internal/runtime/microvm_quota_test.go | 7 ++----- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/go/internal/runtime/microvm_isolation_microvm_test.go b/go/internal/runtime/microvm_isolation_microvm_test.go index 6b6062b3e..f874a2e5a 100644 --- a/go/internal/runtime/microvm_isolation_microvm_test.go +++ b/go/internal/runtime/microvm_isolation_microvm_test.go @@ -168,12 +168,11 @@ func sweepScript(needle, roots string) string { // `:` per match, and exit non-zero when the batch had none — so // the caller's `found` accumulator keeps grep's semantics across batches. // - // BEGINFILE/ERRNO is load-bearing, not defensive: gawk treats an unopenable - // input as FATAL, so without it one unreadable file aborts the invocation, - // skips END, and exits 2 — losing the other 199 paths in the batch and - // rendering the result indistinguishable from genuine confinement. The - // guest's /bin/awk is gawk (guest-image/default.nix), so the extension is - // available even though the program is invoked as `awk`. + // BEGINFILE/ERRNO is load-bearing, not defensive: gawk makes an unopenable + // input FATAL, so one bad path aborts the invocation there, skips END, exits + // 2, and drops every path after it plus any hit already printed. It covers + // OPEN errors only — a read error still aborts, the other reason stderr + // stays visible. The guest's /bin/awk is gawk (guest-image/default.nix). const awkProg = `BEGINFILE { if (ERRNO) nextfile } ` + `index($0, ENVIRON["SWEEP_NEEDLE"]) { print FILENAME ":" $0; hit=1 } END { exit !hit }` return "export SWEEP_NEEDLE='" + needle + "'; " + @@ -253,9 +252,9 @@ func TestMicroVMSweepScriptFindsANeedleAcrossBatches(t *testing.T) { // Comfortably more than two full batches, so at least two mid-loop flushes // happen before the trailing one. fileCount := sweepBatchSize*2 + 25 - // Zero-padded so the glob's LEXICOGRAPHIC order matches numeric order. - // Unpadded, f425.txt sorts to glob index 362 — inside a mid-loop batch — so - // the "final batch" row below never reached the trailing flush it names. + // Zero-padded so the glob's lexicographic order matches numeric order in ANY + // collation. Unpadded, f425.txt sorts mid-run (index 362 in the guest's C + // locale), so the "final batch" row never reached the trailing flush it names. plant := "mkdir -p /workspace/many && for i in $(seq 1 " + strconv.Itoa(fileCount) + "); do " + "printf 'filler line %s\\n' \"$i\" > \"$(printf '/workspace/many/f%03d.txt' \"$i\")\"; done && ls /workspace/many | wc -l" out, code := guestSh(t, m, id, plant) diff --git a/go/internal/runtime/microvm_quota_test.go b/go/internal/runtime/microvm_quota_test.go index 57a6b7d86..3359bf542 100644 --- a/go/internal/runtime/microvm_quota_test.go +++ b/go/internal/runtime/microvm_quota_test.go @@ -12,11 +12,8 @@ package runtime // quotaReadFn precisely so it is covered here rather than left to a leg that // skips. // -// The real statfs probe (readVolumeQuota) is exercised too, but only for what is -// honestly assertable without a quota'd filesystem: that it reads a real path, -// and that an unquota'd tree correctly reads as NOT active. A green here does -// not claim quota enforcement was proven — the guest-side ENOSPC/EDQUOT proof is -// the root-gated leg in microvm_isolation_microvm_test.go. +// The real statfs probe is Linux-only; its two probes live in +// microvm_quota_linux_test.go. import ( "errors" From 5f3cf5ed5c0a55542c768cb625dfd6129afade74 Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 16 Sep 2026 23:05:52 -0400 Subject: [PATCH 3/4] docs(runtime): name the channel the abort actually loses (RIG-3818) Round-2 review fold. Comment text only: AST-stripping both revisions of both files (go/parser, dropping f.Comments and every Doc/Comment field, re-printed from the AST) leaves them byte-identical at 20812B and 7392B, with the build constraints unchanged. "drops ... any hit already printed" was false, and backwards in the direction that matters. gawk flushes stdout on its fatal path, so the match line survives the abort -- and guestSh returns stdout, which is what the cross-tenant row's strings.Contains discriminator reads. The hit text is the one channel that still works; what is lost is the batch's contribution to the exit status, because awk exits 2, the if-awk-then-found=0 never fires, and exit $found reports 1. Measured through the real accumulator: needle at file 1, read-fail mid-batch -> stdout carries the hit, FINAL-EXIT=1. The correction also brings the inline comment back to the 4-line cap, and "the second reason the scan below" now resolves its own direction rather than pointing at a comment the reader has not met. The quota header dropped its count: four tests exercise readVolumeQuota in the linux file, not the two this PR moved, so a bare pointer cannot go stale. It also regains the enforcement thread the trim lost, with the gate stated accurately -- requireQuotaFS wants an operator-provided quota'd filesystem, not root. RIG-3818 --- go/internal/runtime/microvm_isolation_microvm_test.go | 7 +++---- go/internal/runtime/microvm_quota_test.go | 6 ++++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/go/internal/runtime/microvm_isolation_microvm_test.go b/go/internal/runtime/microvm_isolation_microvm_test.go index f874a2e5a..e25b2f1d9 100644 --- a/go/internal/runtime/microvm_isolation_microvm_test.go +++ b/go/internal/runtime/microvm_isolation_microvm_test.go @@ -169,10 +169,9 @@ func sweepScript(needle, roots string) string { // the caller's `found` accumulator keeps grep's semantics across batches. // // BEGINFILE/ERRNO is load-bearing, not defensive: gawk makes an unopenable - // input FATAL, so one bad path aborts the invocation there, skips END, exits - // 2, and drops every path after it plus any hit already printed. It covers - // OPEN errors only — a read error still aborts, the other reason stderr - // stays visible. The guest's /bin/awk is gawk (guest-image/default.nix). + // input FATAL, aborting there and dropping every later path plus the batch's + // contribution to the exit status. It covers OPEN errors only — a read error + // still aborts, the second reason the scan below leaves stderr unsuppressed. const awkProg = `BEGINFILE { if (ERRNO) nextfile } ` + `index($0, ENVIRON["SWEEP_NEEDLE"]) { print FILENAME ":" $0; hit=1 } END { exit !hit }` return "export SWEEP_NEEDLE='" + needle + "'; " + diff --git a/go/internal/runtime/microvm_quota_test.go b/go/internal/runtime/microvm_quota_test.go index 3359bf542..e5e087d0c 100644 --- a/go/internal/runtime/microvm_quota_test.go +++ b/go/internal/runtime/microvm_quota_test.go @@ -12,8 +12,10 @@ package runtime // quotaReadFn precisely so it is covered here rather than left to a leg that // skips. // -// The real statfs probe is Linux-only; its two probes live in -// microvm_quota_linux_test.go. +// The real statfs probe is Linux-only; it is exercised in +// microvm_quota_linux_test.go. Enforcement itself is proven by the +// ENOSPC/EDQUOT leg in microvm_isolation_microvm_test.go, which needs an +// operator-provided quota'd filesystem. import ( "errors" From fdfbba4dd8a5238ce5be6b7a045a9efb1b10bb14 Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 16 Sep 2026 23:45:02 -0400 Subject: [PATCH 4/4] docs(runtime): cite where the guest gawk actually comes from (RIG-3818) Round-3 review fold; the round returned all-clear at the gating floor, these are its two LOWs. Comment text only. The guard is gawk-specific -- BEGINFILE is a GNU extension and silently no-ops elsewhere -- and the round-2 trim had dropped the citation grounding that. Restoring it turned up a worse problem: BOTH the earlier version and the reviewer cited guest-image/default.nix, and that file contains no gawk at all. It only ASSERTS bin/awk exists in the assembled tree (its userlandContract). The awk is shipped by the agent image, agent-image/toolchain.nix, for the egress arm. Neither of us had checked the citation against main. Also names the gate that actually binds on the enforcement pointer: the microvm build tag stops that leg long before the quota gate does, so a reader was told a quota gate was sufficient to run it. RIG-3818 --- go/internal/runtime/microvm_isolation_microvm_test.go | 8 ++++---- go/internal/runtime/microvm_quota_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go/internal/runtime/microvm_isolation_microvm_test.go b/go/internal/runtime/microvm_isolation_microvm_test.go index e25b2f1d9..7b42e0e4d 100644 --- a/go/internal/runtime/microvm_isolation_microvm_test.go +++ b/go/internal/runtime/microvm_isolation_microvm_test.go @@ -168,10 +168,10 @@ func sweepScript(needle, roots string) string { // `:` per match, and exit non-zero when the batch had none — so // the caller's `found` accumulator keeps grep's semantics across batches. // - // BEGINFILE/ERRNO is load-bearing, not defensive: gawk makes an unopenable - // input FATAL, aborting there and dropping every later path plus the batch's - // contribution to the exit status. It covers OPEN errors only — a read error - // still aborts, the second reason the scan below leaves stderr unsuppressed. + // BEGINFILE/ERRNO is load-bearing, not defensive: gawk (agent-image ships + // pkgs.gawk) makes an unopenable input FATAL, aborting there and dropping + // later paths plus the batch's exit-status contribution. It covers OPEN + // errors only — a read error still aborts, why the scan below shows stderr. const awkProg = `BEGINFILE { if (ERRNO) nextfile } ` + `index($0, ENVIRON["SWEEP_NEEDLE"]) { print FILENAME ":" $0; hit=1 } END { exit !hit }` return "export SWEEP_NEEDLE='" + needle + "'; " + diff --git a/go/internal/runtime/microvm_quota_test.go b/go/internal/runtime/microvm_quota_test.go index e5e087d0c..b9f93b9cf 100644 --- a/go/internal/runtime/microvm_quota_test.go +++ b/go/internal/runtime/microvm_quota_test.go @@ -14,8 +14,8 @@ package runtime // // The real statfs probe is Linux-only; it is exercised in // microvm_quota_linux_test.go. Enforcement itself is proven by the -// ENOSPC/EDQUOT leg in microvm_isolation_microvm_test.go, which needs an -// operator-provided quota'd filesystem. +// ENOSPC/EDQUOT leg in microvm_isolation_microvm_test.go, which needs the +// microvm tag and an operator-provided quota'd filesystem. import ( "errors"