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');