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 65931f9e6..b874985a6 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -1335,6 +1335,51 @@ 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. +// 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 { + canonical = tail.length ? path.join(fs.realpathSync(dir), ...tail) : fs.realpathSync(dir); + break; + } catch { /* keep climbing */ } + const parent = path.dirname(dir); + 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 +// 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)); +} + export function parseStaticStyleImports(content, fromFile, projectCwd) { if (!content || typeof content !== 'string') return []; const dir = path.dirname(fromFile); @@ -1693,6 +1738,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) { @@ -2023,6 +2072,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 08ca6d80e..895b4f0f0 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,57 @@ 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); + }); + + 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()', () => { let cwd; beforeEach(() => { cwd = mkTmp(); }); @@ -1538,6 +1590,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 }); @@ -3348,6 +3443,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');