Skip to content

Commit 11416c1

Browse files
dev-ecd-dmclaude
dev-ecd-dm
and
claude
committed
fix(scope): scope external-tool findings to the detector's ignore set + HTML log
Engine-orchestrated external tools (Gitleaks/Trivy/...) scan the raw working tree and did NOT honor the scanner's file-discovery exclusions, so a single secret scanner flooded the report with false positives from generated caches. Measured on a real Flutter project: 749 path hits from a Chrome profile under .dart_tool/, 265 from .supabase/, plus build/. The built-in detector never sees those dirs, so the external tools were manufacturing the very false positives this suite exists to cut. - Extract a canonical DEFAULT_IGNORE_DIRS / DEFAULT_IGNORE_PATTERNS + buildScannerIgnoreGlobs() into src/ignore.js (single source of truth), adding the generated caches .dart_tool, .gradle and .supabase. - scanner.js derives its discovery globs from it (DRY; a regression test asserts every legacy exclusion is retained). - index.js layers DEFAULT_IGNORE_PATTERNS BEFORE the user's .csreview-ignore (last-match-wins, so a project can `!`-re-include a default) so the external tools' findings are scoped to first-party source, exactly like the detector. Read-only and fail-open, as before. - Each tool result is annotated with how many findings were filtered; the CLI surfaces "N filtered as generated/cache" and an accurate suppression label. - reports/html.js now logs "Generating HTML report..." + "HTML report saved to" for parity with the Markdown/SARIF reporters (the missing line made the HTML report look absent in the run output even though it was always written). Tests: +6 (ignore defaults/globs, runAnalysis tool-scoping, HTML log parity, normalizer source-attribution contract). 167/167 - lint clean - typecheck 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bc0fdca commit 11416c1

8 files changed

Lines changed: 308 additions & 38 deletions

File tree

‎csreview/src/cli.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,9 @@ try {
277277
console.log(chalk.bold('\n Stack-native security tools:'));
278278
for (const t of result.securityTools) {
279279
if (t.available) {
280+
const filtered = t.suppressed ? chalk.gray(`, ${t.suppressed} filtered as generated/cache`) : '';
280281
console.log(
281-
` ${String(t.tool).padEnd(10)} ${chalk.green('ran')} (${t.source}${t.provisioned ? ', provisioned' : ''}, ${t.rawCount || 0} findings)`,
282+
` ${String(t.tool).padEnd(10)} ${chalk.green('ran')} (${t.source}${t.provisioned ? ', provisioned' : ''}, ${t.rawCount || 0} findings)${filtered}`,
282283
);
283284
} else {
284285
console.log(
@@ -321,7 +322,9 @@ try {
321322
}
322323

323324
if (result.suppressedByIgnore > 0) {
324-
console.log(`\n ${chalk.gray(`Suppressed by .csreview-ignore: ${result.suppressedByIgnore}`)}`);
325+
console.log(
326+
`\n ${chalk.gray(`Suppressed by ignore rules (generated caches, vendored deps, .csreview-ignore): ${result.suppressedByIgnore}`)}`,
327+
);
325328
}
326329
if (result.baseline?.applied) {
327330
console.log(` ${chalk.gray(`Baselined (known) findings hidden: ${result.baseline.baselinedCount}`)}`);

‎csreview/src/ignore.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,4 +173,73 @@ export function loadIgnore(rootDir) {
173173
}
174174
}
175175

176+
/**
177+
* Built-in default suppression — directories and file globs that are never
178+
* first-party source (vendored deps, build output, generated tool caches).
179+
*
180+
* These mirror the scanner's file-discovery exclusions, but live here as a
181+
* single reusable list because the engine-orchestrated external tools
182+
* (Gitleaks/Trivy/...) scan the RAW working tree and do NOT honor the scanner's
183+
* discovery globs. Applying these defaults to the merged finding set scopes the
184+
* tools to the same surface as the built-in detector — without it a secret
185+
* scanner reports thousands of false positives from generated caches (e.g. an
186+
* entire Chrome profile under `.dart_tool/`, Gradle or Supabase local state).
187+
* The user's `.csreview-ignore` is layered AFTER these (last-match-wins), so a
188+
* project can re-include a default with a `!negation`.
189+
*/
190+
export const DEFAULT_IGNORE_DIRS = [
191+
'node_modules',
192+
'.git',
193+
'dist',
194+
'build',
195+
'.next',
196+
'.nuxt',
197+
'coverage',
198+
'__pycache__',
199+
'.venv',
200+
'venv',
201+
'.tox',
202+
'.mypy_cache',
203+
'vendor',
204+
'target',
205+
'bin',
206+
'obj',
207+
'.trae',
208+
'.vscode',
209+
'.idea',
210+
'csreview-reports',
211+
'.csreview',
212+
'.dart_tool', // Flutter/Dart build cache
213+
'.gradle', // Gradle/Android cache
214+
'.supabase', // Supabase CLI local runtime state (distinct from the `supabase/` source tree)
215+
];
216+
217+
/** Built-in default file globs (minified/generated artifacts and prior reports). */
218+
export const DEFAULT_IGNORE_FILES = [
219+
'*.min.js',
220+
'*.min.css',
221+
'security-report.html',
222+
'security-findings.md',
223+
'csreview-report.html',
224+
'csreview-report.md',
225+
'*_security-report.html',
226+
'*_security-findings.md',
227+
];
228+
229+
/**
230+
* Built-in defaults in `.csreview-ignore` (gitignore) syntax, for suppressing
231+
* findings from the merged report set via {@link compileIgnorePatterns}.
232+
* @type {string[]}
233+
*/
234+
export const DEFAULT_IGNORE_PATTERNS = [...DEFAULT_IGNORE_DIRS.map((d) => `${d}/`), ...DEFAULT_IGNORE_FILES];
235+
236+
/**
237+
* The same defaults expressed as recursive globs for the scanner's
238+
* file-discovery `ignore` option (the `glob` library's syntax).
239+
* @returns {string[]}
240+
*/
241+
export function buildScannerIgnoreGlobs() {
242+
return [...DEFAULT_IGNORE_DIRS.map((d) => `**/${d}/**`), ...DEFAULT_IGNORE_FILES.map((f) => `**/${f}`)];
243+
}
244+
176245
export { IGNORE_FILE_NAME };

‎csreview/src/index.js‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { generateMarkdownReport } from './reports/markdown.js';
1010
import { generateSarifReport } from './reports/sarif.js';
1111
import { calculateSecurityScore } from './score.js';
1212
import { normalizeLocalPath, safeResolveInside } from './pathSafety.js';
13-
import { loadIgnore, applyIgnore } from './ignore.js';
13+
import { loadIgnore, applyIgnore, compileIgnorePatterns, DEFAULT_IGNORE_PATTERNS } from './ignore.js';
1414
import { loadBaseline, applyBaseline, writeBaseline } from './baseline.js';
1515

1616
/**
@@ -1214,13 +1214,39 @@ export async function runAnalysis(rootDir, options = {}) {
12141214
strict: Boolean(options.strictPartials),
12151215
});
12161216

1217-
// Report-level suppression: .csreview-ignore (path globs) then --baseline
1218-
// (known-finding fingerprints). Both are read-only and only filter the
1219-
// reported set; subagent reconciliation above runs on the full deduped set.
1217+
// Report-level suppression: built-in defaults + .csreview-ignore (path globs)
1218+
// then --baseline (known-finding fingerprints). All read-only and only filter
1219+
// the reported set; subagent reconciliation above runs on the full deduped set.
1220+
//
1221+
// The built-in DEFAULT_IGNORE_PATTERNS (generated caches, vendored deps) are
1222+
// layered FIRST, then the user's .csreview-ignore (last-match-wins, so a
1223+
// project can `!`-re-include a default). This is what scopes the external
1224+
// security tools' findings to first-party source, exactly like the detector —
1225+
// those tools scan the raw tree and otherwise flood the report with secrets
1226+
// from generated caches (e.g. a Chrome profile under .dart_tool/).
12201227
const ignore = loadIgnore(absRoot);
1221-
const ignoreResult = applyIgnore(findings, ignore.compiled);
1228+
const ignoreCompiled = compileIgnorePatterns([...DEFAULT_IGNORE_PATTERNS, ...ignore.patterns]);
1229+
const ignoreResult = applyIgnore(findings, ignoreCompiled);
12221230
let reportFindings = ignoreResult.kept;
12231231

1232+
// Attribute ignore-suppressed findings back to their originating external tool
1233+
// so the CLI can report an honest "raw vs filtered as generated/cache" count.
1234+
// The join is finding.source === result.tool (locked by a normalizer contract
1235+
// test). Counts are post-dedup: if a finding was reported by multiple tools
1236+
// and deduped to one entry, only the surviving source is counted (rawCount,
1237+
// captured pre-dedup, is unaffected).
1238+
if (securityToolResults.length > 0 && ignoreResult.suppressed.length > 0) {
1239+
/** @type {Record<string, number>} */
1240+
const suppressedBySource = {};
1241+
for (const f of ignoreResult.suppressed) {
1242+
const src = f && f.source;
1243+
if (src) suppressedBySource[src] = (suppressedBySource[src] || 0) + 1;
1244+
}
1245+
securityToolResults = securityToolResults.map((r) =>
1246+
suppressedBySource[r.tool] ? { ...r, suppressed: suppressedBySource[r.tool] } : r,
1247+
);
1248+
}
1249+
12241250
/** @type {{applied: boolean, baselinedCount: number, written: string|null}} */
12251251
const baselineInfo = { applied: false, baselinedCount: 0, written: null };
12261252
if (options.updateBaselinePath) {

‎csreview/src/reports/html.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ function buildFileData(findings) {
279279
}
280280

281281
export function generateHtmlReport(projectInfo, findings, outputPath, metadata = {}) {
282+
console.log('Generating HTML report...');
282283
const score = calculateSecurityScore(findings, projectInfo);
283284
const scoreColor = getScoreColor(score);
284285
const scoreLabel = getScoreLabel(score);
@@ -1637,5 +1638,6 @@ document.addEventListener('keydown', function(e) {
16371638
</html>`;
16381639

16391640
fs.writeFileSync(outputPath, html, 'utf8');
1641+
console.log(`HTML report saved to ${outputPath}`);
16401642
return outputPath;
16411643
}

‎csreview/src/scanner.js‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import fs from 'fs';
33
import path from 'path';
44
import { glob } from 'glob';
55
import { safeResolveInside } from './pathSafety.js';
6+
import { buildScannerIgnoreGlobs } from './ignore.js';
67

78
const SOURCE_EXTENSIONS = [
89
'js',
@@ -66,37 +67,11 @@ const SOURCE_EXTENSIONS = [
6667
'gql',
6768
];
6869

69-
const IGNORE_PATTERNS = [
70-
'**/node_modules/**',
71-
'**/.git/**',
72-
'**/dist/**',
73-
'**/build/**',
74-
'**/.next/**',
75-
'**/.nuxt/**',
76-
'**/coverage/**',
77-
'**/__pycache__/**',
78-
'**/.venv/**',
79-
'**/venv/**',
80-
'**/.tox/**',
81-
'**/.mypy_cache/**',
82-
'**/vendor/**',
83-
'**/target/**',
84-
'**/bin/**',
85-
'**/obj/**',
86-
'**/*.min.js',
87-
'**/*.min.css',
88-
'**/.trae/**',
89-
'**/.vscode/**',
90-
'**/.idea/**',
91-
'**/security-report.html',
92-
'**/security-findings.md',
93-
'**/csreview-report.html',
94-
'**/csreview-report.md',
95-
'**/*_security-report.html',
96-
'**/*_security-findings.md',
97-
'**/csreview-reports/**',
98-
'**/.csreview/**',
99-
];
70+
// File-discovery exclusions derived at module init from the single source of
71+
// truth in ignore.js (shared with the report-level default suppression that
72+
// scopes the external security tools). Adding a generated cache there updates
73+
// both paths. Keep ignore.js side-effect-free so this top-level call is safe.
74+
const IGNORE_PATTERNS = buildScannerIgnoreGlobs();
10075

10176
const EXTENSION_TO_TECH = {
10277
js: 'JavaScript/TypeScript',

‎csreview/test/ignore.test.js‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import {
1111
isIgnored,
1212
applyIgnore,
1313
loadIgnore,
14+
DEFAULT_IGNORE_DIRS,
15+
DEFAULT_IGNORE_FILES,
16+
DEFAULT_IGNORE_PATTERNS,
17+
buildScannerIgnoreGlobs,
1418
} from '../src/ignore.js';
1519
import { runAnalysis } from '../src/index.js';
1620

@@ -112,3 +116,84 @@ test('pathologically wildcard-heavy patterns are refused (fail-open)', () => {
112116
// Fail-open: a refused pattern suppresses nothing.
113117
assert.equal(isIgnored('a'.repeat(101), compileIgnorePatterns(['*a'.repeat(101)])), false);
114118
});
119+
120+
test('DEFAULT_IGNORE_DIRS covers vendored deps, build output, and generated caches', () => {
121+
for (const d of [
122+
'node_modules',
123+
'.git',
124+
'dist',
125+
'build',
126+
'vendor',
127+
'target',
128+
'__pycache__',
129+
'.dart_tool', // Flutter/Dart cache (the dominant external-tool noise source)
130+
'.gradle', // Gradle/Android cache
131+
'.supabase', // Supabase CLI local runtime state
132+
'csreview-reports',
133+
'.csreview',
134+
]) {
135+
assert.ok(DEFAULT_IGNORE_DIRS.includes(d), `expected DEFAULT_IGNORE_DIRS to include ${d}`);
136+
}
137+
});
138+
139+
test('DEFAULT_IGNORE_PATTERNS are gitignore-syntax and scope external-tool findings to first-party source', () => {
140+
const compiled = compileIgnorePatterns(DEFAULT_IGNORE_PATTERNS);
141+
// the real-world noise from CaiuPixOld: a Chrome profile under .dart_tool
142+
assert.ok(isIgnored('flutter/apps/operator/.dart_tool/chrome-device/Default/Preferences', compiled));
143+
assert.ok(isIgnored('supabase-stuff/.supabase/postgres/data/x', compiled));
144+
assert.ok(isIgnored('android/app/.gradle/cache/x', compiled));
145+
assert.ok(isIgnored('a/node_modules/pkg/index.js', compiled));
146+
assert.ok(isIgnored('packages/web/dist/bundle.min.js', compiled));
147+
// root-level cache dirs (no parent component) must also match — guards against
148+
// an anchoring regression in the matcher (Codex M1)
149+
assert.ok(isIgnored('.supabase/local/x', compiled));
150+
assert.ok(isIgnored('.dart_tool/package_config.json', compiled));
151+
assert.ok(isIgnored('node_modules/pkg/index.js', compiled));
152+
// first-party source is never suppressed by the defaults
153+
assert.ok(!isIgnored('src/index.js', compiled));
154+
assert.ok(!isIgnored('lib/feature.dart', compiled));
155+
// the dot-dir default must NOT swallow the real `supabase/` source tree
156+
assert.ok(!isIgnored('supabase/migrations/0001_init.sql', compiled));
157+
// every default file glob is present in the compiled pattern set (export is intentional, Codex N1)
158+
for (const f of DEFAULT_IGNORE_FILES) assert.ok(DEFAULT_IGNORE_PATTERNS.includes(f), `missing default file ${f}`);
159+
});
160+
161+
test('buildScannerIgnoreGlobs retains every legacy scanner exclusion and adds the new caches', () => {
162+
const globs = buildScannerIgnoreGlobs();
163+
for (const legacy of [
164+
'**/node_modules/**',
165+
'**/.git/**',
166+
'**/dist/**',
167+
'**/build/**',
168+
'**/.next/**',
169+
'**/.nuxt/**',
170+
'**/coverage/**',
171+
'**/__pycache__/**',
172+
'**/.venv/**',
173+
'**/venv/**',
174+
'**/.tox/**',
175+
'**/.mypy_cache/**',
176+
'**/vendor/**',
177+
'**/target/**',
178+
'**/bin/**',
179+
'**/obj/**',
180+
'**/*.min.js',
181+
'**/*.min.css',
182+
'**/.trae/**',
183+
'**/.vscode/**',
184+
'**/.idea/**',
185+
'**/security-report.html',
186+
'**/security-findings.md',
187+
'**/csreview-report.html',
188+
'**/csreview-report.md',
189+
'**/*_security-report.html',
190+
'**/*_security-findings.md',
191+
'**/csreview-reports/**',
192+
'**/.csreview/**',
193+
]) {
194+
assert.ok(globs.includes(legacy), `regression: lost legacy scanner glob ${legacy}`);
195+
}
196+
assert.ok(globs.includes('**/.dart_tool/**'));
197+
assert.ok(globs.includes('**/.gradle/**'));
198+
assert.ok(globs.includes('**/.supabase/**'));
199+
});

‎csreview/test/reports.test.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import os from 'node:os';
66
import path from 'node:path';
77
import { generateMarkdownReport, escapeMdInline, mdCodeSpan, fencedCode } from '../src/reports/markdown.js';
88
import { buildSarifLog, generateSarifReport } from '../src/reports/sarif.js';
9+
import { generateHtmlReport } from '../src/reports/html.js';
910

1011
function tmpFile(name) {
1112
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'csreview-reports-'));
@@ -139,6 +140,40 @@ test('buildSarifLog tolerates empty findings', () => {
139140
assert.deepEqual(log.runs[0].tool.driver.rules, []);
140141
});
141142

143+
test('HTML report logs its generation and save (parity with Markdown/SARIF logs)', () => {
144+
const out = tmpFile('demo_security-report.html');
145+
const projectInfo = {
146+
name: 'demo',
147+
files: ['src/app.js'],
148+
configFiles: [],
149+
depFiles: [],
150+
baasFiles: [],
151+
frameworks: [],
152+
techStack: [],
153+
projectType: 'unknown',
154+
};
155+
const logs = [];
156+
const orig = console.log;
157+
console.log = (...args) => logs.push(args.join(' '));
158+
try {
159+
const returned = generateHtmlReport(projectInfo, [baseFinding()], out, {});
160+
assert.equal(returned, out);
161+
} finally {
162+
console.log = orig;
163+
}
164+
// The missing "Generating HTML report..." line is what made the HTML report
165+
// look absent in the run output even though it was always written.
166+
assert.ok(
167+
logs.some((l) => l.includes('Generating HTML report...')),
168+
'expected a "Generating HTML report..." log line',
169+
);
170+
assert.ok(
171+
logs.some((l) => l.includes('HTML report saved to')),
172+
'expected an "HTML report saved to ..." log line',
173+
);
174+
assert.ok(fs.existsSync(out), 'HTML file written');
175+
});
176+
142177
test('Markdown does not allow link injection through a crafted CWE id (M1)', () => {
143178
const out = tmpFile('cwe_security-findings.md');
144179
const finding = baseFinding({ id: 'CWEINJ', cwe: 'x)](http://evil.com) and [pwn](http://evil2.com' });

0 commit comments

Comments
 (0)