From ae03e9e09c6489320d166c6afbc067399f775689 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Fri, 31 Jul 2026 18:20:09 -0700 Subject: [PATCH 1/3] fix: skip design-hook scans for files outside the resolved project root The per-edit and Stop deep passes gated on sensitive paths, generated paths, extension, config ignores, and size, but never on containment. Any file the session touched outside the project (harness scratchpad dirs under the system temp root, sibling checkouts) was scanned and judged against THIS project's config and DESIGN.md palette, producing design-system findings that are wrong by construction. Both loops now check isScanTargetInsideProject() (audit reason: outside-project), matching the gate hook-before-edit.mjs already had. Paths are canonicalized so a symlinked root doesn't split the comparison. The Stop pass re-checks containment itself because caches written by older hook versions can still list out-of-project paths. Umbrella-dir launches (issue #305) are unaffected: their projectCwd resolves to the edited file's own project root, so containment holds. Written with AI assistance (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/hook-lib.mjs | 26 +++++++++ tests/hook.test.mjs | 105 +++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index 20059c747..c56018cc5 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -1332,6 +1332,24 @@ function isInsideProject(filePath, projectCwd) { } } +function canonicalPath(p) { + try { return fs.realpathSync(p); } catch { return path.resolve(p); } +} + +// Containment gate for both scan passes. A session routinely touches files +// that belong to no project or to a different one — harness scratchpad dirs +// under the system temp root, sibling checkouts, one-off throwaway HTML — and +// findings against those are judged with THIS project's config and DESIGN.md +// palette, which is never right. Skip them (audit reason: outside-project). +// Paths are canonicalized first so a symlinked root (macOS /tmp -> +// /private/tmp) doesn't split the comparison; the realpath fallback for +// missing paths is only correct because both scan loops check existence +// before calling this. +export function isScanTargetInsideProject(filePath, projectCwd) { + if (!filePath || !projectCwd) return false; + return isInsideProject(canonicalPath(filePath), canonicalPath(projectCwd)); +} + export function parseStaticStyleImports(content, fromFile, projectCwd) { if (!content || typeof content !== 'string') return []; const dir = path.dirname(fromFile); @@ -1690,6 +1708,10 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = lastSkip = 'file-missing'; continue; } + if (!isScanTargetInsideProject(filePath, projectCwd)) { + lastSkip = 'outside-project'; + continue; + } const maxFileBytes = config.limits?.maxFileBytes ?? DEFAULT_CONFIG.limits.maxFileBytes; if (maxFileBytes > 0) { @@ -2020,6 +2042,10 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no const relForMatch = relativize(filePath, projectCwd); if (matchesAnyGlob(relForMatch, config.ignoreFiles) || matchesAnyGlob(filePath, config.ignoreFiles)) continue; if (!fs.existsSync(filePath)) continue; + // Caches written before this gate existed can still hold out-of-project + // paths, so the Stop pass re-checks containment rather than trusting + // the per-edit pass to have filtered them. + if (!isScanTargetInsideProject(filePath, projectCwd)) continue; scanned += 1; let content = ''; diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 88db7bac5..9d4c330c3 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -60,6 +60,7 @@ import { resolveProjectPlatform, isNativePlatform, normalizeIgnoreValueEntries, + isScanTargetInsideProject, } from '../skill/scripts/hook-lib.mjs'; import { normalizeIgnoreValueEntries as normalizeIgnoreValueEntriesCli } from '../cli/lib/impeccable-config.mjs'; import { detectHtml, detectText } from '../cli/engine/detect-antipatterns.mjs'; @@ -158,6 +159,44 @@ describe('SENSITIVE_PATH / GENERATED_PATH', () => { }); }); +describe('isScanTargetInsideProject()', () => { + let root; + beforeEach(() => { root = mkTmp(); }); + afterEach(() => fs.rmSync(root, { recursive: true, force: true })); + + it('accepts files under the project root and the root itself', () => { + const file = path.join(root, 'src', 'Card.tsx'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, 'noop'); + assert.equal(isScanTargetInsideProject(file, root), true); + assert.equal(isScanTargetInsideProject(root, root), true); + }); + + it('rejects siblings, temp scratchpads, and empty inputs', () => { + const scratch = mkTmp(); + try { + const outside = path.join(scratch, 'landing.html'); + fs.writeFileSync(outside, '

x

'); + assert.equal(isScanTargetInsideProject(outside, root), false); + assert.equal(isScanTargetInsideProject('', root), false); + assert.equal(isScanTargetInsideProject(outside, ''), false); + } finally { + fs.rmSync(scratch, { recursive: true, force: true }); + } + }); + + it('treats symlinked and canonical forms of the same tree as one project', () => { + const real = path.join(root, 'real'); + const link = path.join(root, 'link'); + fs.mkdirSync(path.join(real, 'src'), { recursive: true }); + fs.symlinkSync(real, link); + const file = path.join(real, 'src', 'Card.tsx'); + fs.writeFileSync(file, 'noop'); + assert.equal(isScanTargetInsideProject(file, link), true); + assert.equal(isScanTargetInsideProject(path.join(link, 'src', 'Card.tsx'), real), true); + }); +}); + describe('readConfig()', () => { let cwd; beforeEach(() => { cwd = mkTmp(); }); @@ -1532,6 +1571,49 @@ rounded: assert.equal(r.audit.skipped, 'sensitive'); }); + it('rejects files outside the project, like harness scratchpads', async () => { + // Session cwd is a real project; the touched file is a throwaway HTML in + // a temp dir elsewhere. Findings against it would be judged with this + // project's config and DESIGN.md, so the scan must skip it entirely. + fs.writeFileSync(path.join(cwd, 'package.json'), '{"name":"proj"}'); + const scratch = mkTmp(); + try { + const file = path.join(scratch, 'id-test', 'landing.html'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, '

throwaway

'); + const det = fakeDetector([finding('side-tab', 1)]); + const r = await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd, detector: det }); + assert.equal(r.stdout, ''); + assert.equal(r.audit.skipped, 'outside-project'); + assert.ok(!fs.existsSync(path.join(cwd, '.impeccable')), 'out-of-project edit must not dirty the cache'); + } finally { + fs.rmSync(scratch, { recursive: true, force: true }); + } + }); + + it('still scans a file whose project root is reached through a symlinked cwd', async () => { + // macOS /tmp -> /private/tmp style: the session cwd is a symlink to the + // project while the tool reports the canonical file path. Containment + // compares canonical paths, so this is inside, not outside. + const real = path.join(cwd, 'realproj'); + const link = path.join(cwd, 'proj-link'); + fs.mkdirSync(path.join(real, 'src'), { recursive: true }); + fs.writeFileSync(path.join(real, 'package.json'), '{"name":"proj"}'); + fs.symlinkSync(real, link); + const file = path.join(real, 'src', 'Card.tsx'); + fs.writeFileSync(file, 'noop'); + const event = { + session_id: 'sym-sid', + cwd: link, + hook_event_name: 'PostToolUse', + tool_name: 'Edit', + tool_input: { file_path: file }, + }; + const r = await runHook({ stdinJson: JSON.stringify(event), env: {}, cwd: link, detector: fakeDetector([]) }); + assert.notEqual(r.audit.skipped, 'outside-project'); + assert.match(r.stdout, /No deterministic design-quality issues found/); + }); + it('rejects extensions outside the allowlist', async () => { const file = writeFixture('docs/README.md', 'noop'); const r = await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd }); @@ -3342,6 +3424,29 @@ describe('runStopHook()', () => { assert.equal(r.audit.skipped, 'no-touched-files'); }); + it('skips out-of-project files even when an older cache still lists them', async () => { + // Caches written before the containment gate can hold scratchpad paths. + // The deep pass re-checks containment instead of trusting the per-edit + // pass to have filtered them. + const sid = 'stop-outside'; + const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-scratch-')); + try { + const outside = path.join(scratch, 'landing.html'); + fs.writeFileSync(outside, '

throwaway

'); + persistCache(cwd, { + version: 1, + sessions: { [sid]: { updatedAt: Date.now(), files: { [outside]: { editCount: 1, findings: [] } } } }, + }); + const det = fakeDetector([finding('side-tab', 7)]); + const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det }); + assert.equal(stop.stdout, ''); + assert.equal(stop.audit.skipped, 'stop-clean'); + assert.equal(stop.audit.scannedFiles, 0); + } finally { + fs.rmSync(scratch, { recursive: true, force: true }); + } + }); + it('a second Stop fire is silent: deep-pass findings are remembered', async () => { const sid = 'stop-twice'; const file = write('src/Card.tsx', 'noop'); From febce52e8d2b5901adcab40efb8020790e82c797 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Fri, 31 Jul 2026 18:30:50 -0700 Subject: [PATCH 2/3] refactor: share the containment gate with hook-before-edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hook-before-edit.mjs kept its own string-based isInsideProject; it now uses the shared isScanTargetInsideProject so all three hook passes apply one containment semantic, symlink canonicalization included. Because the before-edit hook gates proposed Writes whose target does not exist yet, canonicalPath now resolves the nearest existing ancestor and re-appends the remainder instead of falling back to the raw resolved path — a new file under a symlinked root compares equal to its canonical project. Written with AI assistance (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/hook-before-edit.mjs | 16 ++++---------- skill/scripts/hook-lib.mjs | 34 +++++++++++++++++++++--------- tests/hook.test.mjs | 13 ++++++++++++ 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/skill/scripts/hook-before-edit.mjs b/skill/scripts/hook-before-edit.mjs index 54e789e8b..1dcde6ef3 100644 --- a/skill/scripts/hook-before-edit.mjs +++ b/skill/scripts/hook-before-edit.mjs @@ -23,6 +23,7 @@ import { designSystemOptions, filterFindings, isNativePlatform, + isScanTargetInsideProject, loadDetector, matchConfiguredExtension, matchesAnyGlob, @@ -161,7 +162,7 @@ function replaceOnce(original, oldString, newString) { } function readExistingProjectFile(filePath, cwd) { - if (!isInsideProject(filePath, cwd)) return null; + if (!isScanTargetInsideProject(filePath, cwd)) return null; if (SENSITIVE_PATH.test(filePath) || GENERATED_PATH.test(filePath)) return null; try { const stat = fs.statSync(filePath); @@ -232,7 +233,7 @@ function shellCopiedFileContent(command, cwd) { const source = shellCopyPaths(command)?.source; if (!source) return ''; const sourcePath = path.isAbsolute(source) ? source : path.resolve(cwd, source); - if (!isInsideProject(sourcePath, cwd)) return ''; + if (!isScanTargetInsideProject(sourcePath, cwd)) return ''; if (SENSITIVE_PATH.test(sourcePath) || GENERATED_PATH.test(sourcePath)) return ''; try { const stat = fs.statSync(sourcePath); @@ -328,15 +329,6 @@ function relativePath(filePath, cwd) { } } -function isInsideProject(filePath, cwd) { - try { - const rel = path.relative(cwd, filePath); - return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)); - } catch { - return false; - } -} - // The static HTML engine reads its input from disk, but preToolUse only has // the proposed content. Stage it in a temp file so html-engine targets get the // same DOM-structural rules pre-write that runHook applies post-edit. @@ -414,7 +406,7 @@ async function main() { }; if (!filePath) return allow({ ...audit, skipped: 'no-file-path', durationMs: Date.now() - started }); - if (!isInsideProject(filePath, cwd)) return allow({ ...audit, skipped: 'outside-project', durationMs: Date.now() - started }); + if (!isScanTargetInsideProject(filePath, cwd)) return allow({ ...audit, skipped: 'outside-project', durationMs: Date.now() - started }); if (SENSITIVE_PATH.test(filePath)) return allow({ ...audit, skipped: 'sensitive', durationMs: Date.now() - started }); if (GENERATED_PATH.test(filePath)) return allow({ ...audit, skipped: 'generated', durationMs: Date.now() - started }); diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index c56018cc5..5967b4052 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -1332,19 +1332,33 @@ function isInsideProject(filePath, projectCwd) { } } +// Resolve a path to its canonical (symlink-free) form. When the path does +// not exist yet — the before-edit hook gates proposed Writes — canonicalize +// the nearest existing ancestor and re-append the remainder, so a new file +// under a symlinked root still compares equal to its canonical project. function canonicalPath(p) { - try { return fs.realpathSync(p); } catch { return path.resolve(p); } + const resolved = path.resolve(p); + let dir = resolved; + const tail = []; + while (true) { + try { + return tail.length ? path.join(fs.realpathSync(dir), ...tail) : fs.realpathSync(dir); + } catch { /* keep climbing */ } + const parent = path.dirname(dir); + if (parent === dir) return resolved; + tail.unshift(path.basename(dir)); + dir = parent; + } } -// Containment gate for both scan passes. A session routinely touches files -// that belong to no project or to a different one — harness scratchpad dirs -// under the system temp root, sibling checkouts, one-off throwaway HTML — and -// findings against those are judged with THIS project's config and DESIGN.md -// palette, which is never right. Skip them (audit reason: outside-project). -// Paths are canonicalized first so a symlinked root (macOS /tmp -> -// /private/tmp) doesn't split the comparison; the realpath fallback for -// missing paths is only correct because both scan loops check existence -// before calling this. +// Containment gate shared by the before-edit hook and both scan passes. A +// session routinely touches files that belong to no project or to a +// different one — harness scratchpad dirs under the system temp root, +// sibling checkouts, one-off throwaway HTML — and findings against those are +// judged with THIS project's config and DESIGN.md palette, which is never +// right. Skip them (audit reason: outside-project). Paths are canonicalized +// first so a symlinked root (macOS /tmp -> /private/tmp) doesn't split the +// comparison. export function isScanTargetInsideProject(filePath, projectCwd) { if (!filePath || !projectCwd) return false; return isInsideProject(canonicalPath(filePath), canonicalPath(projectCwd)); diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 9d4c330c3..b773db2b3 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -195,6 +195,19 @@ describe('isScanTargetInsideProject()', () => { assert.equal(isScanTargetInsideProject(file, link), true); assert.equal(isScanTargetInsideProject(path.join(link, 'src', 'Card.tsx'), real), true); }); + + it('classifies not-yet-written files by their nearest existing ancestor', () => { + // The before-edit hook gates proposed Writes, so the target often does + // not exist. Canonicalization must climb to an existing ancestor rather + // than bail, or a new file under a symlinked root would read as outside. + const real = path.join(root, 'real'); + const link = path.join(root, 'link'); + fs.mkdirSync(real, { recursive: true }); + fs.symlinkSync(real, link); + assert.equal(isScanTargetInsideProject(path.join(link, 'src', 'New.tsx'), real), true); + assert.equal(isScanTargetInsideProject(path.join(real, 'deep', 'New.tsx'), link), true); + assert.equal(isScanTargetInsideProject(path.join(root, 'elsewhere', 'New.tsx'), real), false); + }); }); describe('readConfig()', () => { From 62a2026afcd8223bed178b0348fc84d9c03690f2 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sun, 2 Aug 2026 19:27:13 -0700 Subject: [PATCH 3/3] perf: memoize canonicalPath so scan loops resolve the project root once The containment gate re-canonicalized projectCwd for every target file in the per-edit and Stop loops. The hook runs as a fresh process per tool event, so a module-level memo makes it once-per-event work; the size cap only matters to long-lived importers like the test runner. Addresses Copilot review feedback on PR #471. Written with AI assistance (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/hook-lib.mjs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index 5967b4052..40645069d 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -1336,19 +1336,32 @@ function isInsideProject(filePath, projectCwd) { // not exist yet — the before-edit hook gates proposed Writes — canonicalize // the nearest existing ancestor and re-append the remainder, so a new file // under a symlinked root still compares equal to its canonical project. +// Memoized: the hook runs as a fresh process per tool event, so the cache +// amounts to once-per-event work — the scan loops re-check the same project +// root for every target file. The cap only matters to long-lived importers +// like the test runner. +const canonicalPathCache = new Map(); +const CANONICAL_PATH_CACHE_MAX = 1024; + function canonicalPath(p) { const resolved = path.resolve(p); + if (canonicalPathCache.has(resolved)) return canonicalPathCache.get(resolved); + let canonical = resolved; let dir = resolved; const tail = []; while (true) { try { - return tail.length ? path.join(fs.realpathSync(dir), ...tail) : fs.realpathSync(dir); + canonical = tail.length ? path.join(fs.realpathSync(dir), ...tail) : fs.realpathSync(dir); + break; } catch { /* keep climbing */ } const parent = path.dirname(dir); - if (parent === dir) return resolved; + if (parent === dir) break; tail.unshift(path.basename(dir)); dir = parent; } + if (canonicalPathCache.size >= CANONICAL_PATH_CACHE_MAX) canonicalPathCache.clear(); + canonicalPathCache.set(resolved, canonical); + return canonical; } // Containment gate shared by the before-edit hook and both scan passes. A