Make independent verification output and timeouts reliable

docs/reviews/2026-10-06-verifier.md

@@ -0,0 +1,11 @@
+# Independent verifier review
+
+Baseline: 59ceacb634afd2d840cfed432e5bf2b4d5d05f18.
+
+Improvements: ignore blank/non-string package scripts; redact complete captured output before shortening its tail; preserve UTF-8 characters across stream chunks; kill the verifier's own POSIX process group on timeout; discard an over-limit stream with an explicit diagnostic rather than retain a potentially incomplete secret; reject invalid timeout/output limits before launching anything.
+
+The initial six regression tests all failed against the original implementation. The final suite has 73 passing tests, including nine new verifier checks. TypeScript compilation passes. Existing orchestration tests use a local fake OpenCode HTTP server. No OpenCode CLI, model provider, paid agent, real user workspace, live service or credentials were invoked. Live smoke and MCP end-to-end scripts were deliberately not run because they require a real provider.
+
+The 200,000-character cap applies independently to stdout and stderr. Overflow intentionally omits that stream, losing diagnostics to avoid exposing a partially captured secret. Timeout process-group cleanup covers ordinary POSIX child processes; commands that deliberately detach into another session and native Windows behavior are not certified. Regex guards/redaction are heuristic, not a sandbox or comprehensive secret detector. Existing arbitrary shell-command execution semantics remain unchanged.
+
+Run `npm ci --ignore-scripts` then `npm test`. Draft PR only; no merge, installation or deployment.

src/verify.ts

@@ -50,7 +50,7 @@ export function discoverCommands(dir: string): Candidate[] {
     ["test", "test"], ["typecheck", "typecheck"], ["type-check", "typecheck"],
     ["lint", "lint"], ["build", "build"], ["check", "lint"],
   ] as const) {
-    if (scripts[name]) add(`${runner} ${name}`, kind, "package.json scripts");
+    if (typeof scripts[name] === "string" && scripts[name].trim()) add(`${runner} ${name}`, kind, "package.json scripts");
   }
 
   // Python
@@ -105,6 +105,10 @@ export interface ExecResult {
 
 /** Run one command in the project directory. Output is redacted and truncated. */
 export function execCommand(command: string, cwd: string, timeoutMs = 600_000, tailChars = 4000): Promise<ExecResult> {
+  if (!Number.isInteger(timeoutMs) || timeoutMs < 1 || timeoutMs > 2_147_483_647 ||
+      !Number.isInteger(tailChars) || tailChars < 0 || tailChars > 200_000) {
+    return Promise.resolve({ command, exitCode: -1, passed: false, stdout: "", stderr: "Invalid verification limits.", durationMs: 0, timedOut: false });
+  }
   const hits = scanDestructive(command);
   if (hits.length) {
     return Promise.resolve({
@@ -120,26 +124,44 @@ export function execCommand(command: string, cwd: string, timeoutMs = 600_000, t
 
   return new Promise((resolve) => {
     const started = Date.now();
-    const child = spawn("sh", ["-lc", command], { cwd, env: { ...process.env, CI: "1", NO_COLOR: "1" } });
+    const grouped = process.platform !== "win32";
+    const child = spawn("sh", ["-lc", command], { cwd, detached: grouped, env: { ...process.env, CI: "1", NO_COLOR: "1" } });
     let out = "";
     let err = "";
+    let outOverflow = false;
+    let errOverflow = false;
     let timedOut = false;
-    const cap = (s: string, chunk: string) => (s.length > 200_000 ? s : s + chunk);
-    child.stdout.on("data", (d) => (out = cap(out, d.toString())));
-    child.stderr.on("data", (d) => (err = cap(err, d.toString())));
+    child.stdout.setEncoding("utf8");
+    child.stderr.setEncoding("utf8");
+    child.stdout.on("data", (d: string) => {
+      if (outOverflow) return;
+      if (out.length + d.length > 200_000) { out = ""; outOverflow = true; } else out += d;
+    });
+    child.stderr.on("data", (d: string) => {
+      if (errOverflow) return;
+      if (err.length + d.length > 200_000) { err = ""; errOverflow = true; } else err += d;
+    });
     const timer = setTimeout(() => {
       timedOut = true;
-      child.kill("SIGKILL");
+      try {
+        if (grouped && child.pid) process.kill(-child.pid, "SIGKILL");
+        else child.kill("SIGKILL");
+      } catch { child.kill("SIGKILL"); }
     }, timeoutMs);
     child.on("close", (code) => {
       clearTimeout(timer);
-      const tail = (s: string) => redact(s.length > tailChars ? "...\n" + s.slice(-tailChars) : s).text;
+      const tail = (s: string, overflow: boolean) => {
+        if (overflow) return "[Output omitted: capture limit exceeded.]";
+        const safe = redact(s).text;
+        if (tailChars === 0) return "";
+        return safe.length > tailChars ? "...\n" + safe.slice(-tailChars) : safe;
+      };
       resolve({
         command,
         exitCode: code ?? -1,
         passed: code === 0 && !timedOut,
-        stdout: tail(out),
-        stderr: tail(err),
+        stdout: tail(out, outOverflow),
+        stderr: tail(err, errOverflow),
         durationMs: Date.now() - started,
         timedOut,
       });

test/verification-regressions.test.ts

@@ -0,0 +1,70 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, writeFileSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { discoverCommands, execCommand } from "../src/verify.js";
+
+function fixture(t: any) {
+  const dir = mkdtempSync(join(tmpdir(), "massa-verifier-"));
+  t.after(() => rmSync(dir, { recursive: true, force: true }));
+  return dir;
+}
+test("discovery rejects non-string and blank package scripts", (t) => {
+  const dir = fixture(t);
+  writeFileSync(join(dir, "package.json"), JSON.stringify({ scripts: { test: [], build: true, lint: "  ", check: "echo ok" } }));
+  assert.deepEqual(discoverCommands(dir).map(c => c.command), ["npm run check"]);
+});
+test("redaction happens before a tail cuts off a token prefix", async (t) => {
+  const dir = fixture(t);
+  writeFileSync(join(dir, "emit.cjs"), "process.stdout.write('ghp_' + 'Z'.repeat(40))");
+  const r = await execCommand("node emit.cjs", dir, 2000, 25);
+  assert.equal(r.passed, true);
+  assert.ok(!r.stdout.includes("Z".repeat(20)), "secret suffix escaped redaction");
+});
+test("UTF-8 codepoints split across writes are preserved", async (t) => {
+  const dir = fixture(t);
+  writeFileSync(join(dir, "emit.cjs"), "const b=Buffer.from('🌼');process.stdout.write(b.subarray(0,2));setTimeout(()=>process.stdout.write(b.subarray(2)),70)");
+  const r = await execCommand("node emit.cjs", dir, 2000);
+  assert.equal(r.stdout, "🌼");
+});
+test("timeout terminates descendants that hold inherited pipes", async (t) => {
+  const dir = fixture(t);
+  const r = await execCommand("sleep 2 & wait", dir, 80);
+  assert.equal(r.timedOut, true);
+  assert.equal(r.passed, false);
+  assert.ok(r.durationMs < 1500, `timeout took ${r.durationMs}ms`);
+});
+test("oversized output is omitted rather than returning a cut-off secret", async (t) => {
+  const dir = fixture(t);
+  writeFileSync(join(dir, "emit.cjs"), "process.stdout.write('X'.repeat(210000));setTimeout(()=>process.stdout.write('tail'),40)");
+  const r = await execCommand("node emit.cjs", dir, 2000);
+  assert.equal(r.passed, true);
+  assert.match(r.stdout, /omitted.*limit/i);
+  assert.ok(r.stdout.length < 200);
+});
+test("invalid timeout is rejected without running the command", async (t) => {
+  const r = await execCommand("echo should-not-run", fixture(t), NaN);
+  assert.equal(r.passed, false);
+  assert.match(r.stderr, /invalid.*limit/i);
+});
+test("zero tail suppresses normal stdout and stderr", async (t) => {
+  const r = await execCommand("echo hello; echo problem >&2", fixture(t), 2000, 0);
+  assert.equal(r.passed, true);
+  assert.equal(r.stdout, "");
+  assert.equal(r.stderr, "");
+});
+test("stderr redacts before truncating and reports overflow", async (t) => {
+  const dir = fixture(t);
+  writeFileSync(join(dir, "emit.cjs"), "process.stderr.write('ghp_' + 'Z'.repeat(40))");
+  const r = await execCommand("node emit.cjs", dir, 2000, 25);
+  assert.ok(!r.stderr.includes("Z".repeat(20)));
+  writeFileSync(join(dir, "emit.cjs"), "process.stderr.write('Y'.repeat(210000))");
+  const large = await execCommand("node emit.cjs", dir, 2000);
+  assert.match(large.stderr, /omitted.*limit/i);
+});
+test("spawn errors remain failures", async () => {
+  const r = await execCommand("echo ok", "/nonexistent-massa-fixture", 2000);
+  assert.equal(r.passed, false);
+  assert.equal(r.exitCode, -1);
+});