From 840965fc77083131761e80570d6f8fdd6b99e37d Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 2 Sep 2026 19:01:05 -0700 Subject: [PATCH] Handle unreadable detector targets AI assistance disclosure: Codex implemented and tested this fix under maintainer direction. --- cli/engine/cli/main.mjs | 45 +++++++++++++++++++--------- cli/engine/node/file-system.mjs | 11 +++++-- tests/detect-antipatterns.test.js | 17 +++++++++++ tests/detect-url-launch.test.mjs | 50 +++++++++++++++++++++++++++++-- 4 files changed, 105 insertions(+), 18 deletions(-) diff --git a/cli/engine/cli/main.mjs b/cli/engine/cli/main.mjs index e54887323..de3e71991 100644 --- a/cli/engine/cli/main.mjs +++ b/cli/engine/cli/main.mjs @@ -316,6 +316,10 @@ async function detectCli() { let allFindings = []; let hadOperationalFailure = false; + const reportLocalScanFailure = (target, error) => { + hadOperationalFailure = true; + process.stderr.write(`Error: cannot scan ${target}: ${error.message}\n`); + }; if (!process.stdin.isTTY && targets.length === 0) { allFindings = await handleStdin(scanOptionsFor); @@ -413,7 +417,11 @@ async function detectCli() { } // Build import graph for multi-file awareness - const graph = buildImportGraph(files); + const unreadableFiles = new Set(); + const graph = buildImportGraph(files, (file, error) => { + unreadableFiles.add(file); + reportLocalScanFailure(file, error); + }); // Build reverse map: file -> set of files that import it const importedByMap = new Map(); for (const [importer, imports] of graph) { @@ -424,24 +432,33 @@ async function detectCli() { } for (const file of files) { - // Each file resolves its own project design system (cached by root), - // so a scan spanning sibling projects applies the right rules per file. - const fileOptions = scanOptionsFor(file); - const fileFindings = await detectLocalFile(file, fileOptions); - // Annotate findings with import context - const importers = importedByMap.get(file); - if (importers && importers.size > 0) { - const importerNames = [...importers].map(f => path.basename(f)); - for (const f of fileFindings) { - f.importedBy = importerNames; + if (unreadableFiles.has(file)) continue; + try { + // Each file resolves its own project design system (cached by root), + // so a scan spanning sibling projects applies the right rules per file. + const fileOptions = scanOptionsFor(file); + const fileFindings = await detectLocalFile(file, fileOptions); + // Annotate findings with import context + const importers = importedByMap.get(file); + if (importers && importers.size > 0) { + const importerNames = [...importers].map(f => path.basename(f)); + for (const f of fileFindings) { + f.importedBy = importerNames; + } } + allFindings.push(...fileFindings); + } catch (error) { + reportLocalScanFailure(file, error); } - allFindings.push(...fileFindings); } } else if (stat.isFile()) { if (shouldIgnoreDetectionFile(resolved, process.cwd(), detectionConfig)) continue; - const fileOptions = scanOptionsFor(resolved); - allFindings.push(...await detectLocalFile(resolved, fileOptions)); + try { + const fileOptions = scanOptionsFor(resolved); + allFindings.push(...await detectLocalFile(resolved, fileOptions)); + } catch (error) { + reportLocalScanFailure(target, error); + } } } } finally { diff --git a/cli/engine/node/file-system.mjs b/cli/engine/node/file-system.mjs index 964f6712d..7fe2c866a 100644 --- a/cli/engine/node/file-system.mjs +++ b/cli/engine/node/file-system.mjs @@ -81,12 +81,19 @@ function resolveImport(specifier, fromDir, fileSet) { return null; } -function buildImportGraph(files) { +function buildImportGraph(files, onReadError = null) { const fileSet = new Set(files); const graph = new Map(); for (const file of files) { - const content = fs.readFileSync(file, 'utf-8'); + let content; + try { + content = fs.readFileSync(file, 'utf-8'); + } catch (error) { + if (!onReadError) throw error; + onReadError(file, error); + continue; + } const dir = path.dirname(file); const imports = new Set(); diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js index 30c5e8f18..1f45641eb 100644 --- a/tests/detect-antipatterns.test.js +++ b/tests/detect-antipatterns.test.js @@ -3684,6 +3684,23 @@ describe('buildImportGraph', () => { expect(imp).toContain(MF); } }); + + test('reports unreadable files and continues building the graph', async () => { + await withStaticFixture({ + 'readable.css': '@import "./missing.css";\n', + }, ({ dir }) => { + const readable = path.join(dir, 'readable.css'); + const missing = path.join(dir, 'missing.css'); + const errors = []; + const graph = buildImportGraph([missing, readable], (file, error) => { + errors.push({ file, code: error.code }); + }); + + expect(errors).toEqual([{ file: missing, code: 'ENOENT' }]); + expect(graph.get(readable)).toEqual(new Set([missing])); + expect(graph.has(missing)).toBe(false); + }); + }); }); describe('resolveImport', () => { diff --git a/tests/detect-url-launch.test.mjs b/tests/detect-url-launch.test.mjs index 18119e96c..a3adacc38 100644 --- a/tests/detect-url-launch.test.mjs +++ b/tests/detect-url-launch.test.mjs @@ -45,7 +45,7 @@ function makePuppeteer({ failChannel = false } = {}) { }; } -function runWithoutPuppeteer(args, files = {}) { +function runWithoutPuppeteer(args, files = {}, { nodeArgs = [] } = {}) { const isolatedRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-no-puppeteer-')); try { const cliRoot = path.join(isolatedRoot, 'cli'); @@ -59,7 +59,7 @@ function runWithoutPuppeteer(args, files = {}) { } return spawnSync( 'node', - [path.join(cliRoot, 'engine', 'detect-antipatterns.mjs'), '--json', ...args], + [...nodeArgs, path.join(cliRoot, 'engine', 'detect-antipatterns.mjs'), '--json', ...args], { cwd: isolatedRoot, encoding: 'utf8' }, ); } finally { @@ -67,6 +67,19 @@ function runWithoutPuppeteer(args, files = {}) { } } +const DENY_READ_PRELOAD = ` +const fs = require('node:fs'); +const originalReadFileSync = fs.readFileSync; +fs.readFileSync = function (file, ...args) { + if (String(file).endsWith('unreadable.css')) { + const error = new Error('simulated EACCES'); + error.code = 'EACCES'; + throw error; + } + return originalReadFileSync.call(this, file, ...args); +}; +`; + describe('launchBrowser', () => { test('Windows: prefers system Chrome via channel:chrome', async () => { setPlatform('win32'); @@ -158,6 +171,39 @@ describe('detect CLI browser failures', () => { expect(findings.some(finding => finding.antipattern === 'bounce-easing')).toBe(true); expect(result.stderr).toContain('Warning: cannot access missing.css'); }); + + test('unreadable local file exits 1 with valid empty JSON and no stack trace', () => { + const result = runWithoutPuppeteer( + ['unreadable.css'], + { + 'deny-read.cjs': DENY_READ_PRELOAD, + 'unreadable.css': '.hero { color: red; }\n', + }, + { nodeArgs: ['--require=./deny-read.cjs'] }, + ); + + expect(result.status).toBe(1); + expect(result.stdout).toBe('[]\n'); + expect(result.stderr).toContain('Error: cannot scan unreadable.css: simulated EACCES'); + expect(result.stderr).not.toContain('at detectLocalFile'); + }); + + test('unreadable directory file preserves findings from readable siblings', () => { + const result = runWithoutPuppeteer( + ['styles'], + { + 'deny-read.cjs': DENY_READ_PRELOAD, + 'styles/unreadable.css': '.hero { color: red; }\n', + 'styles/page.css': '.hero { animation: bounce 1s linear infinite; }\n', + }, + { nodeArgs: ['--require=./deny-read.cjs'] }, + ); + const findings = JSON.parse(result.stdout); + + expect(result.status).toBe(1); + expect(findings.some(finding => finding.antipattern === 'bounce-easing')).toBe(true); + expect(result.stderr.match(/simulated EACCES/g)).toHaveLength(1); + }); }); describe('splitScanUrl', () => {