From b2e67d0097cea337df6e7002809a34607cc5f3dd Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 18:05:21 +0000 Subject: [PATCH] fix(design-detect): an engine is a file named impeccable outside the project; DOM dumps scan without inline ignores Second review cycle, security + checklist: - IMPECCABLE_BIN=/bin/sh (or node) was READY, and `detect` with cwd=repoRoot made the interpreter run the repository's own `detect` file. Every engine candidate (env override, PATH entry, cache, sibling) is now judged by the realpath of the FILE and must be named impeccable[.exe]; PATH and cache candidates that resolve into the repository are skipped like the others. "Inside the project" means the repository, or cwd when cwd is a project directory: HOME and its ancestors are exempt, so a URL-mode review launched from HOME still finds the HOME-rooted installs. - A base for --changed that starts with `-` was spliced into git argv (`--output=` made git write a file and report no changes); an option- like or missing base is DETECT_REFUSED (not a ref name), exit 1, and the parser no longer defaults a missing value to main. - DOM dumps are the audited page's bytes, so an in-file `impeccable-disable` comment there is page-controlled: batches under the designs root run with --no-inline-ignores, repository batches keep the project's own ignores. - neutralizeSentinels covers the shapes it missed (bare sentinels such as DETECT_TOP total= and IMPECCABLE_DISABLED, the DETECT_EXIT_CODE= echo, the `[rule-id] impact=` group header) in one precompiled alternation instead of 37 replaceAll passes per field; only kept findings are normalized, and the summary's total stays the engine's count. - The minimal engine environment compares keys case-insensitively on Windows (process.env enumerates Path, SystemRoot there) and passes PATHEXT, COMSPEC, HOMEDRIVE, HOMEPATH, PROGRAMDATA. - Bare 64s move into DETECT_LIMITS; the unused SentinelName type is gone; the header states the directory-target contract (the engine's own walk). Tests: an interpreter as IMPECCABLE_BIN never runs the repo's detect file; a PATH symlink into the repository is never READY; option-like and empty bases are refused with no file written; the designs-root batch carries --no-inline-ignores and the repo batch does not; the identity label is deterministic per binary; the bare-sentinel and header shapes are neutralized; the installed fake engine works without IMPECCABLE_FAKE_OUTPUT (the helper copies the sample beside it); two tests clean up in finally. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-design-detect.ts | 117 +++++++++++++++++-------- lib/design-detect-contract.ts | 32 ++++--- test/design-detect-contract.test.ts | 13 +++ test/gstack-design-detect.test.ts | 129 +++++++++++++++++++++++++--- test/helpers/fake-impeccable.ts | 1 + 5 files changed, 233 insertions(+), 59 deletions(-) diff --git a/bin/gstack-design-detect.ts b/bin/gstack-design-detect.ts index 6f0a99a08..df475f407 100755 --- a/bin/gstack-design-detect.ts +++ b/bin/gstack-design-detect.ts @@ -16,11 +16,11 @@ * * design_detector config ── off ──► IMPECCABLE_DISABLED * │ auto - * $IMPECCABLE_BIN (absolute, realpath outside repo/cwd, executable, not a script) ──► READY + * $IMPECCABLE_BIN (absolute, realpath outside repo/cwd, executable, named impeccable[.exe]) ──► READY * │ - * PATH walk (absolute entries only, none inside repo/cwd; `impeccable[.exe]`) ─┬─ binary ──► READY + * PATH walk (absolute entries; file realpath outside repo/cwd, named impeccable[.exe]) ─┬─ binary ──► READY * │ └─ #! shim ──► launcher-present - * $IMPECCABLE_HOME|~/.impeccable/bin//impeccable[.exe] ──► READY + * $IMPECCABLE_HOME|~/.impeccable/bin//impeccable[.exe] (realpath outside repo/cwd) ──► READY * │ * ~/{.claude,.agents,.cursor,.gemini,.github,.opencode}/skills/impeccable/scripts/ * ├─ bin/-/impeccable[.exe] (engine installed beside the launcher) ──► READY @@ -33,7 +33,11 @@ * checked-out branch can commit `.claude/skills/impeccable/scripts/bin/-/ * impeccable`, a `node_modules/.bin/impeccable`, or a PATH entry under the repo; * none of those is ever READY. Only HOME-rooted installs, the env override, the - * cache, and PATH entries outside the repo qualify, all by realpath. + * cache, and PATH entries outside the repo qualify, all by realpath of the FILE, + * and every engine is named impeccable[.exe]: an env override pointing at an + * interpreter (/bin/sh, node) would otherwise run the repository's own `detect` + * file from cwd. "cwd" means a project directory: when cwd is HOME or above it, + * only the repository rule applies (a URL-mode review can run from anywhere). * * Sentinel contract: lib/design-detect-contract.ts (one owner, imported here and * by the gen-time resolvers). Scan output: stdout is one JSON document @@ -45,7 +49,8 @@ * existing regular file or directory whose realpath lies under the repo root (or * cwd) or under ${GSTACK_HOME:-~/.gstack}/projects//designs/ (where design- * review keeps rendered-DOM dumps); symlinks are never followed out of those roots - * and are skipped when git names them; URLs are refused (the one engine path that + * and are skipped when git names them (a directory target is handed to the engine + * as-is: its own walk decides what inside it is read); URLs are refused (the one engine path that * talks to the network); the engine runs with a minimal environment (PATH, HOME, * TMPDIR, LANG/LC_*, IMPECCABLE_*), stdin ignored (its ">50 files, continue?" * prompt is gated on a TTY), a wall-clock timeout with SIGKILL on the direct @@ -181,12 +186,30 @@ function isExecutableFile(file: string): boolean { * On the PATH walk only: an executable that is not a `#!` script. A node shim * named `impeccable` is launcher-present, never READY (running it downloads). * Explicit install locations (IMPECCABLE_BIN, the ~/.impeccable/bin cache, the - * engine beside a skill install's launcher) accept any executable regular file. + * engine beside a skill install's launcher) accept any executable regular file + * whose realpath is named impeccable[.exe]. */ function isEngineBinary(file: string): boolean { return isExecutableFile(file) && !isScript(file); } +/** The engine's real file is named impeccable[.exe]; an interpreter reached through a symlink or env override is not an engine. */ +function isEngineName(realFile: string): boolean { + const base = WIN ? path.basename(realFile).toLowerCase() : path.basename(realFile); + return base === 'impeccable' || base === 'impeccable.exe'; +} + +/** + * Under the project the agent is reviewing: inside the repository, or inside cwd + * when cwd is itself a project directory. HOME and its ancestors are not projects + * (a URL-mode review can run from HOME, where every HOME-rooted install lives). + */ +function underProject(real: string, repoRoot: string, cwd: string): boolean { + if (isInside(real, repoRoot)) return true; + if (isInside(HOME, cwd)) return false; + return isInside(real, cwd); +} + function semverKey(v: string): number[] | null { const m = v.match(/^v?(\d+)\.(\d+)\.(\d+)(?:[-+].*)?$/); return m ? [Number(m[1]), Number(m[2]), Number(m[3])] : null; @@ -220,7 +243,7 @@ function trustedEnvPath(name: string, repoRoot: string, cwd: string, notes: stri step(`${name}=${raw} does not exist`); return null; } - if (isInside(real, repoRoot) || isInside(real, cwd)) { + if (underProject(real, repoRoot, cwd)) { notes.push(`${SENTINEL.ENV_IGNORED}: ${name} resolves inside the repository`); return null; } @@ -260,7 +283,7 @@ function probe(host: string, verbose = false): Probe { let siblingVersion: string | null = null; let repoLocalLauncher = false; for (const root of roots) { - const rootIsRepo = isInside(root, repoRoot) || isInside(root, cwd); + const rootIsRepo = underProject(root, repoRoot, cwd); for (const sub of SKILL_ROOTS) { const skillDir = path.join(root, sub, 'skills', 'impeccable'); if (fs.existsSync(path.join(skillDir, 'SKILL.md'))) p.skillPresent = true; @@ -268,11 +291,11 @@ function probe(host: string, verbose = false): Probe { if (!fs.existsSync(launcher)) continue; if (rootIsRepo) { repoLocalLauncher = true; continue; } const realLauncher = realpathOrNull(launcher); - if (!realLauncher || isInside(realLauncher, repoRoot) || isInside(realLauncher, cwd)) { repoLocalLauncher = true; continue; } + if (!realLauncher || underProject(realLauncher, repoRoot, cwd)) { repoLocalLauncher = true; continue; } p.launcher ??= launcher; for (const cand of engineSiblings(path.dirname(launcher))) { const real = realpathOrNull(cand); - if (siblingEngine || !real || !isExecutableFile(real) || isInside(real, repoRoot) || isInside(real, cwd)) continue; + if (siblingEngine || !real || !isExecutableFile(real) || !isEngineName(real) || underProject(real, repoRoot, cwd)) continue; siblingEngine = real; try { const v = fs.readFileSync(path.join(path.dirname(launcher), 'VERSION'), 'utf-8').trim(); @@ -325,9 +348,12 @@ function probe(host: string, verbose = false): Probe { // IMPECCABLE_BIN const envBin = trustedEnvPath('IMPECCABLE_BIN', repoRoot, cwd, p.notes, step); - if (envBin && isExecutableFile(envBin)) { + if (envBin && isExecutableFile(envBin) && isEngineName(envBin)) { p.sentinel = `${SENTINEL.READY}: ${envBin}`; p.engine = envBin; + } else if (envBin && isExecutableFile(envBin)) { + // /bin/sh or node as the "engine" would execute the repository's own `detect` file from cwd. + p.notes.push(`${SENTINEL.ENV_IGNORED}: IMPECCABLE_BIN is not named impeccable`); } else if (envBin) { step(`IMPECCABLE_BIN=${envBin} is not an executable file`); } @@ -339,11 +365,13 @@ function probe(host: string, verbose = false): Probe { for (const entry of (ENV.PATH || '').split(path.delimiter)) { if (!entry || !path.isAbsolute(entry)) continue; const real = realpathOrNull(entry); - if (!real || isInside(real, repoRoot) || isInside(real, cwd)) continue; + if (!real || underProject(real, repoRoot, cwd)) continue; for (const ext of exts) { const cand = path.join(real, `impeccable${ext}`); if (!fs.existsSync(cand)) continue; - if (isEngineBinary(cand)) { p.engine = cand; p.sentinel = `${SENTINEL.READY}: ${cand}`; break; } + const realCand = realpathOrNull(cand); + if (!realCand || underProject(realCand, repoRoot, cwd) || !isEngineName(realCand)) { step(`PATH ${cand} resolves to ${realCand ?? 'nothing'}: not an engine`); continue; } + if (isEngineBinary(realCand)) { p.engine = realCand; p.sentinel = `${SENTINEL.READY}: ${realCand}`; break; } launcherOnPath ??= cand; // node shim or .cmd wrapper: launcher present, engine not proven } if (p.engine) break; @@ -359,7 +387,10 @@ function probe(host: string, verbose = false): Probe { const newest = newestSemverDir(binDir); if (newest) { const cand = path.join(binDir, newest, WIN ? 'impeccable.exe' : 'impeccable'); - if (isExecutableFile(cand)) { p.engine = cand; p.engineVersion = newest.replace(/^v/, ''); p.sentinel = `${SENTINEL.READY}: ${cand}`; } + const realCand = realpathOrNull(cand); + if (realCand && isExecutableFile(realCand) && isEngineName(realCand) && !underProject(realCand, repoRoot, cwd)) { + p.engine = realCand; p.engineVersion = newest.replace(/^v/, ''); p.sentinel = `${SENTINEL.READY}: ${realCand}`; + } } step(`cache ${binDir}: newest=${newest ?? 'none'} engine=${p.engine ?? 'none'}`); } @@ -386,7 +417,7 @@ function probe(host: string, verbose = false): Probe { } p.engineVersion ??= `sha256:${engineIdentity(p.engine)}`; } - p.engineVersion = clip(stripControl(p.engineVersion), 64); + p.engineVersion = clip(stripControl(p.engineVersion), DETECT_LIMITS.field.engineVersion); if (!TESTED_ENGINE_VERSIONS.includes(p.engineVersion)) p.notes.push(`${SENTINEL.ENGINE_UNTESTED}: ${p.engineVersion}`); return p; } @@ -447,7 +478,7 @@ function clip(s: string, n: number): string { function sanitizeId(raw: unknown): string | null { if (typeof raw !== 'string') return null; const s = raw.trim().toLowerCase(); - return /^[a-z0-9-]{1,64}$/.test(s) ? s : null; + return new RegExp(`^[a-z0-9-]{1,${DETECT_LIMITS.field.id}}$`).test(s) ? s : null; } function str(v: unknown): string { return typeof v === 'string' ? v : v == null ? '' : String(v); @@ -489,9 +520,10 @@ function resolveTargets(args: ScanArgs, p: Probe): { targets: string[]; refusedB }; for (const t of args.targets) push(t); if (args.changed !== undefined) { - const top = gitTopLevel(p.cwd); - if (!top) { refuse(args.changed, 'not a repository'); return { targets: out, refusedBase: true }; } const base = args.changed; + if (!base || base.startsWith('-')) { refuse(base || '(empty)', 'not a ref name'); return { targets: out, refusedBase: true }; } + const top = gitTopLevel(p.cwd); + if (!top) { refuse(base, 'not a repository'); return { targets: out, refusedBase: true }; } const files = new Set(); const runZ = (argv: string[]): boolean => { const r = spawnSync('git', argv, { cwd: top, encoding: 'buffer', timeout: DETECT_LIMITS.gitTimeoutMs, maxBuffer: DETECT_LIMITS.gitMaxBuffer }); @@ -522,18 +554,25 @@ function resolveTargets(args: ScanArgs, p: Probe): { targets: string[]; refusedB interface EngineRun { exit: number; stdout: string; stderr: string; timedOut: boolean; tooLarge: boolean } +/** Environment the engine may see (Windows keys compared case-insensitively: process.env there enumerates `Path`, `SystemRoot`). */ +const ENGINE_ENV_KEYS = new Set([ + 'PATH', 'HOME', 'TMPDIR', 'TMP', 'TEMP', 'LANG', 'TERM', 'NO_COLOR', + 'SYSTEMROOT', 'USERPROFILE', 'APPDATA', 'LOCALAPPDATA', 'PATHEXT', 'COMSPEC', 'HOMEDRIVE', 'HOMEPATH', 'PROGRAMDATA', +]); + /** The engine sees PATH/HOME/TMPDIR/locale and its own IMPECCABLE_* knobs, never the agent's tokens. */ function engineEnv(): Record { const out: Record = {}; for (const [k, v] of Object.entries(ENV)) { if (v === undefined) continue; - if (['PATH', 'HOME', 'TMPDIR', 'TMP', 'TEMP', 'LANG', 'TERM', 'NO_COLOR', 'SYSTEMROOT', 'USERPROFILE', 'APPDATA', 'LOCALAPPDATA'].includes(k) || k.startsWith('LC_') || k.startsWith('IMPECCABLE_')) out[k] = v; + const key = WIN ? k.toUpperCase() : k; + if (ENGINE_ENV_KEYS.has(key) || key.startsWith('LC_') || key.startsWith('IMPECCABLE_')) out[k] = v; } return out; } -function runEngine(engine: string, batch: string[], cwd: string, timeoutMs: number): EngineRun { - const r = Bun.spawnSync([engine, 'detect', '--json', ...batch], { +function runEngine(engine: string, batch: string[], cwd: string, timeoutMs: number, extra: string[] = []): EngineRun { + const r = Bun.spawnSync([engine, 'detect', '--json', ...extra, ...batch], { cwd, stdin: 'ignore', stdout: 'pipe', stderr: 'pipe', env: engineEnv(), timeout: timeoutMs, killSignal: 'SIGKILL', maxBuffer: DETECT_LIMITS.stdoutBytes + 1024, }); @@ -599,9 +638,16 @@ function scan(args: ScanArgs): number { let diagnosticsTotal = 0; let exit = 0; const started = Date.now(); - for (let i = 0; i < targets.length; i += DETECT_LIMITS.batch) { - const batch = targets.slice(i, i + DETECT_LIMITS.batch); - const run = runEngine(p.engine, batch, p.repoRoot, timeoutMs); + // Repo files honor the project's own inline `impeccable-disable` comments. DOM dumps + // under the designs root are the audited page's bytes: an inline ignore there is + // page-controlled, never a decision the user made, so those batches disable them. + const inProject = (t: string) => isInside(t, p.repoRoot) || isInside(t, p.cwd); + const batches: Array<{ files: string[]; extra: string[] }> = []; + for (const [files, extra] of [[targets.filter(inProject), []], [targets.filter(t => !inProject(t)), ['--no-inline-ignores']]] as Array<[string[], string[]]>) { + for (let i = 0; i < files.length; i += DETECT_LIMITS.batch) batches.push({ files: files.slice(i, i + DETECT_LIMITS.batch), extra }); + } + for (const { files: batch, extra } of batches) { + const run = runEngine(p.engine, batch, p.repoRoot, timeoutMs, extra); for (const line of run.stderr.split('\n')) { if (!line.trim()) continue; diagnosticsTotal++; @@ -626,12 +672,13 @@ function scan(args: ScanArgs): number { if (args.format === 'raw') { process.stdout.write(rawChunks.length === 1 ? rawChunks[0] : JSON.stringify(rawFindings, null, 2) + '\n'); } else { - const all = rawFindings.map(normalize); - const truncated = all.length > DETECT_LIMITS.findings; - const findings = truncated ? all.slice(0, DETECT_LIMITS.findings) : all; + // Only the kept findings are sanitized; at the stdout ceiling the rest would be normalized and discarded. + const truncated = rawFindings.length > DETECT_LIMITS.findings; + const findings = (truncated ? rawFindings.slice(0, DETECT_LIMITS.findings) : rawFindings).map(normalize); + const total = rawFindings.length; const byRule: Record = {}; let advisory = 0, high = 0, medium = 0, polish = 0, slop = 0, quality = 0; - for (const f of all) { + for (const f of findings) { byRule[f.impeccableId] = (byRule[f.impeccableId] ?? 0) + 1; if (f.advisory) { advisory++; continue; } if (f.kind === 'slop') slop++; else if (f.kind === 'quality') quality++; @@ -639,12 +686,12 @@ function scan(args: ScanArgs): number { } const result: ScanResult = { schemaVersion: 1, engine: p.engine, engineVersion: p.engineVersion ?? 'unknown', targets: targets.length, - exit, total: all.length, counted: all.length - advisory, advisory, ignoredRules: p.ignoredRules, byRule, findings, truncated, + exit, total, counted: findings.length - advisory, advisory, ignoredRules: p.ignoredRules, byRule, findings, truncated, diagnostics: diagnosticsTotal > diagnostics.length ? [...diagnostics, `… ${diagnosticsTotal - diagnostics.length} more engine stderr lines not kept`] : diagnostics, }; process.stdout.write(JSON.stringify(result, null, 2) + '\n'); - writeTop(all, truncated); - process.stderr.write(`${SENTINEL.DETECT_SUMMARY}: total=${all.length} slop=${slop} quality=${quality} advisory=${advisory} ignored=${p.ignoredRules.length} high=${high} medium=${medium} polish=${polish}${truncated ? ' truncated=true' : ''}\n`); + writeTop(findings, total, truncated); + process.stderr.write(`${SENTINEL.DETECT_SUMMARY}: total=${total} slop=${slop} quality=${quality} advisory=${advisory} ignored=${p.ignoredRules.length} high=${high} medium=${medium} polish=${polish}${truncated ? ' truncated=true' : ''}\n`); } for (const d of diagnostics.slice(0, DETECT_LIMITS.diagnosticsEchoed)) process.stderr.write(`${SENTINEL.ENGINE_STDERR}: ${d}\n`); process.stderr.write(`${SENTINEL.DETECT_EXIT}: ${exit}\n`); @@ -654,7 +701,7 @@ function scan(args: ScanArgs): number { const IMPACT_ORDER = { high: 0, medium: 1, polish: 2 } as const; -function writeTop(findings: NormalizedFinding[], truncated: boolean) { +function writeTop(findings: NormalizedFinding[], total: number, truncated: boolean) { const groups = new Map(); for (const f of findings) { if (f.advisory) continue; @@ -664,7 +711,7 @@ function writeTop(findings: NormalizedFinding[], truncated: boolean) { } const ordered = [...groups.entries()].sort((a, b) => (IMPACT_ORDER[a[1][0].impact] - IMPACT_ORDER[b[1][0].impact]) || (b[1].length - a[1].length) || a[0].localeCompare(b[0])); - const lines = [UNTRUSTED_BEGIN, `${SENTINEL.DETECT_TOP} total=${findings.length} rules=${groups.size}${truncated ? ' truncated=true' : ''}`]; + const lines = [UNTRUSTED_BEGIN, `${SENTINEL.DETECT_TOP} total=${total} rules=${groups.size}${truncated ? ' truncated=true' : ''}`]; let shown = 0; for (const [id, group] of ordered) { const f0 = group[0]; @@ -675,7 +722,7 @@ function writeTop(findings: NormalizedFinding[], truncated: boolean) { shown++; } } - if (shown >= DETECT_LIMITS.topLocations && findings.length > shown) lines.push(` … ${findings.length - shown} more locations in the JSON`); + if (shown >= DETECT_LIMITS.topLocations && total > shown) lines.push(` … ${total - shown} more locations in the JSON`); lines.push(UNTRUSTED_END); process.stderr.write(lines.join('\n') + '\n'); } @@ -714,7 +761,7 @@ function parse(argv: string[]): { verb: string; host: string; verbose: boolean; if (a === '--host') host = argv[++i] ?? host; else if (a === '--verbose') verbose = true; else if (a === '--format') { const v = argv[++i]; format = v === 'raw' ? 'raw' : 'gstack'; } - else if (a === '--changed') changed = argv[++i] ?? 'main'; + else if (a === '--changed') changed = argv[++i] ?? ''; // an empty base is refused in resolveTargets, never defaulted else if (a === '--') { targets.push(...argv.slice(i + 1)); break; } else if (a.startsWith('--')) process.stderr.write(`ignoring unknown flag ${a}\n`); else targets.push(a); diff --git a/lib/design-detect-contract.ts b/lib/design-detect-contract.ts index eeedf72ea..25e093d4b 100644 --- a/lib/design-detect-contract.ts +++ b/lib/design-detect-contract.ts @@ -60,7 +60,6 @@ export const SENTINEL = { ENGINE_STDERR: 'ENGINE_STDERR', } as const; -export type SentinelName = keyof typeof SENTINEL; /** * Sentinels whose line explains itself after the colon (a path, a version, a @@ -107,21 +106,32 @@ export const DETECT_LIMITS = { /** git subprocess budgets inside the wrapper */ gitTimeoutMs: 30_000, gitMaxBuffer: 64 * 1024 * 1024, - field: { id: 64, message: 120, snippet: 120, value: 200, file: 4096, diagnostic: 400, refusedTarget: 200, parseErrorPreview: 80, internalError: 300 }, + field: { id: 64, engineVersion: 64, message: 120, snippet: 120, value: 200, file: 4096, diagnostic: 400, refusedTarget: 200, parseErrorPreview: 80, internalError: 300 }, } as const; +const escapeRe = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + /** - * Break any sentinel or fence marker that appears INSIDE engine-derived text, - * so page content echoed through a finding cannot close the untrusted envelope - * or forge a probe line. Inserts a zero-width space after the first character - * (the same technique browse/src/content-security.ts uses for its markers). + * One pass over every shape the agent reads as gstack's own voice: the two fence + * markers, any sentinel word (whole word, colon or not: `DETECT_TOP total=` and + * `IMPECCABLE_DISABLED` are printed bare), and the `[rule-id] impact=` group + * header. Longest sentinel first so DETECT_EXIT_CODE is not split at DETECT_EXIT. + */ +const NEUTRALIZE_RE = new RegExp( + [escapeRe(UNTRUSTED_BEGIN), escapeRe(UNTRUSTED_END), + '\\b(?:' + [...new Set(Object.values(SENTINEL))].sort((a, b) => b.length - a.length).map(escapeRe).join('|') + ')\\b', + '\\[(?=[a-z0-9-]+\\] impact=)'].join('|'), 'g'); + +/** + * Break any sentinel, fence marker, or group header that appears INSIDE + * engine-derived text, so page content echoed through a finding cannot close + * the untrusted envelope or forge a probe line. Inserts a zero-width space after + * the first character (the same technique browse/src/content-security.ts uses + * for its markers). One precompiled alternation: this runs on four fields of + * every kept finding. */ export function neutralizeSentinels(s: string): string { - const zw = '\u200b'; - let out = s.replaceAll(UNTRUSTED_BEGIN, UNTRUSTED_BEGIN[0] + zw + UNTRUSTED_BEGIN.slice(1)) - .replaceAll(UNTRUSTED_END, UNTRUSTED_END[0] + zw + UNTRUSTED_END.slice(1)); - for (const v of Object.values(SENTINEL)) out = out.replaceAll(v + ':', v[0] + zw + v.slice(1) + ':'); - return out; + return s.replace(NEUTRALIZE_RE, m => m[0] + '\u200b' + m.slice(1)); } diff --git a/test/design-detect-contract.test.ts b/test/design-detect-contract.test.ts index 9e45356f8..f75964b71 100644 --- a/test/design-detect-contract.test.ts +++ b/test/design-detect-contract.test.ts @@ -69,6 +69,19 @@ describe('contract shape', () => { expect(out.replace(/\u200b/g, '')).toBe(forged); }); + test('neutralizeSentinels also breaks bare sentinels, the exit-code echo, and the [rule-id] impact= header shape', () => { + for (const s of [SENTINEL.NOT_AVAILABLE, SENTINEL.DISABLED, SENTINEL.DETECT_NO_TARGETS, `${SENTINEL.DETECT_TOP} total=0 rules=0`, `${SENTINEL.DETECT_EXIT_CODE}=0`]) { + const out = neutralizeSentinels(`snippet ${s} tail`); + expect(out).not.toContain(s.split(/[ =]/)[0]); + expect(out.replace(/\u200b/g, '')).toBe(`snippet ${s} tail`); + } + // longest sentinel wins: DETECT_EXIT_CODE is broken once, not split at DETECT_EXIT + expect(neutralizeSentinels(`${SENTINEL.DETECT_EXIT_CODE}=0`)).toBe(`${SENTINEL.DETECT_EXIT_CODE[0]}\u200b${SENTINEL.DETECT_EXIT_CODE.slice(1)}=0`); + expect(neutralizeSentinels('[tiny-text] impact=high tier=auto-fix count=1')).toBe('[\u200btiny-text] impact=high tier=auto-fix count=1'); + expect(neutralizeSentinels('[tiny-text] is a rule')).toBe('[tiny-text] is a rule'); + expect(neutralizeSentinels('plain snippet text')).toBe('plain snippet text'); + }); + test('module is pure: no imports, loading prints nothing', () => { const file = path.join(ROOT, 'lib', 'design-detect-contract.ts'); expect(fs.readFileSync(file, 'utf-8')).not.toMatch(/^import /m); diff --git a/test/gstack-design-detect.test.ts b/test/gstack-design-detect.test.ts index 0d180a327..d3309190d 100644 --- a/test/gstack-design-detect.test.ts +++ b/test/gstack-design-detect.test.ts @@ -102,11 +102,14 @@ describe('probe', () => { fs.mkdirSync(path.dirname(inRepo), { recursive: true }); fs.writeFileSync(inRepo, '#!/bin/sh\necho MARKER > marker.txt\n'); fs.chmodSync(inRepo, 0o755); - const r = run(['probe'], { env: { IMPECCABLE_BIN: inRepo } }); - expect(r.out).toContain(`${SENTINEL.ENV_IGNORED}: IMPECCABLE_BIN resolves inside the repository`); - expect(lines(r.out)[0]).toBe(SENTINEL.NOT_AVAILABLE); - expect(fs.existsSync(path.join(REPO, 'marker.txt'))).toBe(false); - fs.rmSync(path.join(REPO, 'tools'), { recursive: true, force: true }); + try { + const r = run(['probe'], { env: { IMPECCABLE_BIN: inRepo } }); + expect(r.out).toContain(`${SENTINEL.ENV_IGNORED}: IMPECCABLE_BIN resolves inside the repository`); + expect(lines(r.out)[0]).toBe(SENTINEL.NOT_AVAILABLE); + expect(fs.existsSync(path.join(REPO, 'marker.txt'))).toBe(false); + } finally { + fs.rmSync(path.join(REPO, 'tools'), { recursive: true, force: true }); + } }); test('a cwd .env naming IMPECCABLE_BIN is never loaded (--no-env-file) and never executed', () => { @@ -346,7 +349,7 @@ describe('scan', () => { expect(r.err).toContain(`${SENTINEL.DETECT_REFUSED}: ${notDesigns}`); const argv = JSON.parse(fs.readFileSync(log, 'utf-8').trim().split('\n')[0]).argv as string[]; expect(argv.slice(0, 2)).toEqual(['detect', '--json']); - expect(argv.slice(2)).toEqual([fs.realpathSync(path.join(designs, 'home.dom.html'))]); + expect(argv.slice(2)).toEqual(['--no-inline-ignores', fs.realpathSync(path.join(designs, 'home.dom.html'))]); // a dump's inline ignores are page-controlled expect(r.code).toBe(2); } finally { fs.rmSync(path.join(GSTACK_HOME, 'projects'), { recursive: true, force: true }); @@ -505,7 +508,7 @@ describe('design-review REPORT_DIR agrees with the allow-list', () => { const s = run(['scan', '--format', 'gstack', dom + '/home.dom.html'], { env: { IMPECCABLE_BIN: FAKE, IMPECCABLE_FAKE_LOG: log } }); expect(s.err).not.toContain(SENTINEL.DETECT_REFUSED); const argv = JSON.parse(fs.readFileSync(log, 'utf-8').trim().split('\n')[0]).argv as string[]; - expect(argv.slice(2)).toEqual([fs.realpathSync(path.join(dom, 'home.dom.html'))]); + expect(argv.slice(2)).toEqual(['--no-inline-ignores', fs.realpathSync(path.join(dom, 'home.dom.html'))]); } finally { fs.rmSync(path.join(GSTACK_HOME, 'projects'), { recursive: true, force: true }); } @@ -748,7 +751,9 @@ describe('coverage: scan security edges', () => { }); test.skipIf(!POSIX)('the engine sees a minimal environment, never the agent tokens', () => { - const envDump = path.join(SANDBOX, 'env-dump.sh'); + const envDumpDir = path.join(SANDBOX, 'env-dump'); + fs.mkdirSync(envDumpDir, { recursive: true }); + const envDump = path.join(envDumpDir, 'impeccable'); // an engine is named impeccable; anything else is refused const out = path.join(SANDBOX, 'env-seen.txt'); fs.writeFileSync(envDump, `#!/bin/sh\nenv > ${JSON.stringify(out)}\necho "[]"\n`); fs.chmodSync(envDump, 0o755); @@ -794,12 +799,15 @@ describe('coverage: scan security edges', () => { }, 120_000); test.skipIf(!POSIX)('a quoted or commented design_detector value still reads as off', () => { - for (const line of ['design_detector: "off"', "design_detector: 'off'", 'design_detector: off # why']) { - fs.writeFileSync(path.join(GSTACK_HOME, 'config.yaml'), line + '\n'); - const r = run(['probe'], { env: { IMPECCABLE_BIN: FAKE } }); - expect(lines(r.out)[0]).toBe(SENTINEL.DISABLED); + try { + for (const line of ['design_detector: "off"', "design_detector: 'off'", 'design_detector: off # why']) { + fs.writeFileSync(path.join(GSTACK_HOME, 'config.yaml'), line + '\n'); + const r = run(['probe'], { env: { IMPECCABLE_BIN: FAKE } }); + expect(lines(r.out)[0]).toBe(SENTINEL.DISABLED); + } + } finally { + fs.rmSync(path.join(GSTACK_HOME, 'config.yaml'), { force: true }); // a failing expect must not leave every later probe DISABLED } - fs.rmSync(path.join(GSTACK_HOME, 'config.yaml')); }); }); @@ -819,3 +827,98 @@ describe('rules', () => { expect(r.err).toContain('usage:'); }); }); + +describe('engine identity: named impeccable, realpath outside the project', () => { + test.skipIf(!POSIX)('IMPECCABLE_BIN pointing at an interpreter is never READY and the repository\'s own detect file never runs', () => { + const marker = path.join(REPO, 'detect-ran.txt'); + fs.writeFileSync(path.join(REPO, 'detect'), `echo ran > ${JSON.stringify(marker)}\n`); + try { + const r = run(['scan', 'src/styles.css'], { env: { IMPECCABLE_BIN: '/bin/sh' } }); + expect(r.out).toContain(`${SENTINEL.ENV_IGNORED}: IMPECCABLE_BIN is not named impeccable`); + expect(lines(r.out)[0]).toBe(SENTINEL.NOT_AVAILABLE); + expect(fs.existsSync(marker)).toBe(false); + } finally { + fs.rmSync(path.join(REPO, 'detect'), { force: true }); + fs.rmSync(marker, { force: true }); + } + }); + + test.skipIf(!POSIX)('a PATH entry named impeccable that resolves into the repository is never READY', () => { + const inRepo = path.join(REPO, 'tools', 'impeccable'); + fs.mkdirSync(path.dirname(inRepo), { recursive: true }); + fs.writeFileSync(inRepo, 'echo MARKER > marker.txt\n'); // no #!: looks like a binary to the sniff + fs.chmodSync(inRepo, 0o755); + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-path-bin-')); + fs.symlinkSync(inRepo, path.join(binDir, 'impeccable')); + try { + const r = run(['probe', '--verbose'], { env: { PATH: `${binDir}${path.delimiter}${process.env.PATH}` } }); + expect(r.out).not.toContain(SENTINEL.READY); + expect(lines(r.out)[0]).toBe(SENTINEL.NOT_AVAILABLE); + expect(fs.existsSync(path.join(REPO, 'marker.txt'))).toBe(false); + } finally { + fs.rmSync(path.join(REPO, 'tools'), { recursive: true, force: true }); + fs.rmSync(binDir, { recursive: true, force: true }); + } + }); + + test.skipIf(!POSIX)('the engine identity label is deterministic per binary and differs between binaries', () => { + const label = (out: string) => out.match(/ENGINE_UNTESTED: (sha256:[0-9a-f]{12})/)?.[1]; + const a = label(run(['probe'], { env: { IMPECCABLE_BIN: FAKE } }).out); + expect(a).toBeDefined(); + expect(label(run(['probe'], { env: { IMPECCABLE_BIN: FAKE } }).out)).toBe(a); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-other-engine-')); + const other = path.join(dir, 'impeccable'); + fs.writeFileSync(other, fs.readFileSync(FAKE, 'utf-8') + '\n// x\n'); + fs.chmodSync(other, 0o755); + try { + expect(label(run(['probe'], { env: { IMPECCABLE_BIN: other } }).out)).not.toBe(a); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test.skipIf(!POSIX)('the installed fake engine prints the sample without IMPECCABLE_FAKE_OUTPUT', () => { + const { dir, bin } = installFakeImpeccable(); + try { + const r = spawnSync(bin, ['detect', '--json', 'x.css'], { encoding: 'utf-8', timeout: 30_000, env: { PATH: process.env.PATH!, HOME: os.homedir() } }); + expect(r.status).toBe(2); + expect(JSON.parse(r.stdout).length).toBeGreaterThan(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('scan: option-like bases and page-controlled inline ignores', () => { + test.skipIf(!POSIX)('--changed with an option-like or missing base is refused (exit 1) and git never writes the file', () => { + const outFile = path.join(SANDBOX, 'git-output-injection.txt'); + const r = run(['scan', '--changed', `--output=${outFile}`], { env: { IMPECCABLE_BIN: FAKE } }); + expect(r.err).toContain(`${SENTINEL.DETECT_REFUSED}: --output=${outFile} (not a ref name)`); + expect(r.code).toBe(1); + expect(fs.existsSync(outFile)).toBe(false); + const r2 = run(['scan', '--changed'], { env: { IMPECCABLE_BIN: FAKE } }); + expect(r2.err).toContain(`${SENTINEL.DETECT_REFUSED}: (empty) (not a ref name)`); + expect(r2.code).toBe(1); + }); + + test.skipIf(!POSIX)('DOM dumps under the designs root scan with --no-inline-ignores; repository files keep their inline ignores', () => { + const designs = path.join(GSTACK_HOME, 'projects', 'x', 'designs', 'design-audit-20260908', 'dom-ignores'); + fs.mkdirSync(designs, { recursive: true }); + fs.writeFileSync(path.join(designs, 'home.dom.html'), ''); + const log = path.join(SANDBOX, 'argv-ignores.log'); + fs.rmSync(log, { force: true }); + try { + const r = run(['scan', '--format', 'gstack', 'src/styles.css', path.join(designs, 'home.dom.html')], { env: { IMPECCABLE_BIN: FAKE, IMPECCABLE_FAKE_LOG: log } }); + expect(r.code).toBe(2); + const calls = fs.readFileSync(log, 'utf-8').trim().split('\n').map(l => JSON.parse(l).argv as string[]); + expect(calls).toHaveLength(2); + const repoCall = calls.find(a => a.some(x => x.endsWith('styles.css')))!; + const domCall = calls.find(a => a.some(x => x.endsWith('home.dom.html')))!; + expect(repoCall).not.toContain('--no-inline-ignores'); + expect(domCall).toContain('--no-inline-ignores'); + expect(domCall.indexOf('--no-inline-ignores')).toBeLessThan(domCall.findIndex(x => x.endsWith('home.dom.html'))); + } finally { + fs.rmSync(designs, { recursive: true, force: true }); + } + }); +}); diff --git a/test/helpers/fake-impeccable.ts b/test/helpers/fake-impeccable.ts index dd27e899a..0ab958f20 100644 --- a/test/helpers/fake-impeccable.ts +++ b/test/helpers/fake-impeccable.ts @@ -15,5 +15,6 @@ export function installFakeImpeccable(prefix = 'gstack-fake-impeccable-'): { dir const bin = path.join(dir, 'impeccable'); fs.copyFileSync(IMPECCABLE_FAKE_SRC, bin); fs.chmodSync(bin, 0o755); + fs.copyFileSync(DETECT_SAMPLE, path.join(dir, 'impeccable-detect-sample.json')); // the shim's documented default output, beside it return { dir, bin }; }