diff --git a/docs/reviews/2026-10-06-verifier.md b/docs/reviews/2026-10-06-verifier.md new file mode 100644 index 0000000..6fbdc1a --- /dev/null +++ b/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. diff --git a/src/verify.ts b/src/verify.ts index 2d93388..49235f7 100644 --- a/src/verify.ts +++ b/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 { + 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, }); diff --git a/test/verification-regressions.test.ts b/test/verification-regressions.test.ts new file mode 100644 index 0000000..88bf4b3 --- /dev/null +++ b/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); +});