From d85fbccf7a210f623efa80b5ec36d113802b0567 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 18 Aug 2026 08:16:25 -0700 Subject: [PATCH] =?UTF-8?q?fix(test):=20banner-tripwire=20exec=20used=20JS?= =?UTF-8?q?ON.stringify=20as=20shell=20quoting=20=E2=80=94=20vacuous=20pas?= =?UTF-8?q?s=20+=20stray=20artifact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSON escaping is not shell escaping. Interpolating JSON.stringify(script) into `bash -c ${...}` left every JSON "\n" as a literal backslash-n inside shell double quotes, collapsing the extracted release-body tripwire block onto one line: `then\n` parsed as the command word `thenn`, and `>&2\nelse\n` parsed as the redirect `>&2nelsen` — so every full-suite run littered a `2nelsen` file (containing "bash: thenn: command not found") in the repo root, and the test's single not-contains assertion passed VACUOUSLY because all output had been redirected into that file. The "and it actually fires" functional check never verified anything. Fix: pass the script as an argv element (spawnSync array form) and assert both branches for real — ABORT case must print the leak message to stderr, clean case must print "banner tripwire clean" to stdout. Verified: `bun test test/binding-template-drift.test.ts` previously created the artifact deterministically; the full free suite now runs artifact-free. The other shell-interpolation sites (evidence, schema-aware concurrency, empty-find-fallthrough, branch-slug-hygiene) already use correct quoting. Co-Authored-By: Claude Fable 5 --- test/binding-template-drift.test.ts | 42 +++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/test/binding-template-drift.test.ts b/test/binding-template-drift.test.ts index 3fd99115d..08888400c 100644 --- a/test/binding-template-drift.test.ts +++ b/test/binding-template-drift.test.ts @@ -68,21 +68,47 @@ describe('content-binding template drift', () => { // Functional: execute the template's tripwire block against a 0-banner // original and a 1-banner outgoing body — the ABORT branch must fire. + // + // Pass the script as an ARGV element (spawnSync array form), never by + // interpolating JSON.stringify into a shell line: JSON escaping is not + // shell escaping. Inside shell double quotes a JSON "\n" stays a literal + // backslash-n, which collapsed this multi-line script onto one line where + // `then\n` became the command word `thenn` and `>&2\nelse\n` became the + // redirect `>&2nelsen` — silently littering a `2nelsen` file (containing + // "bash: thenn: command not found") in the repo root on every suite run, + // while the old not-contains assertion passed vacuously because ALL + // output had been redirected into that file. const block = body.match(/_ORIG_BANNERS=\$\(grep[\s\S]*?fi\n/); expect(block).not.toBeNull(); const fs = require('fs'); const os = require('os'); const path = require('path'); - const { execSync } = require('child_process'); + const { spawnSync } = require('child_process'); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-banner-')); try { - fs.writeFileSync(path.join(dir, 'orig.md'), 'clean body\n'); - fs.writeFileSync(path.join(dir, 'new.md'), 'body with UNTRUSTED TRACKER CONTENT banner leak\n'); - const script = block![0] - .replaceAll('/tmp/gstack-pr-body-orig-$$.md', path.join(dir, 'orig.md')) - .replaceAll('/tmp/gstack-pr-body-$$.md', path.join(dir, 'new.md')); - const out = execSync(`bash -c ${JSON.stringify(script + '; true')}`, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); - expect(out).not.toContain('banner tripwire clean'); + const scriptFor = (origContent: string, newContent: string) => { + fs.writeFileSync(path.join(dir, 'orig.md'), origContent); + fs.writeFileSync(path.join(dir, 'new.md'), newContent); + return block![0] + .replaceAll('/tmp/gstack-pr-body-orig-$$.md', path.join(dir, 'orig.md')) + .replaceAll('/tmp/gstack-pr-body-$$.md', path.join(dir, 'new.md')); + }; + + // Banner leaked into the outgoing body → the ABORT branch fires, loudly. + const abort = spawnSync('bash', ['-c', scriptFor( + 'clean body\n', + 'body with UNTRUSTED TRACKER CONTENT banner leak\n', + )], { encoding: 'utf-8' }); + expect(abort.stderr).toContain('ABORT: envelope banner leaked'); + expect(abort.stdout).not.toContain('banner tripwire clean'); + + // No banner delta → the clean branch fires. + const clean = spawnSync('bash', ['-c', scriptFor( + 'clean body\n', + 'also clean body\n', + )], { encoding: 'utf-8' }); + expect(clean.stdout).toContain('banner tripwire clean'); + expect(clean.stderr).not.toContain('ABORT'); } finally { fs.rmSync(dir, { recursive: true, force: true }); }