From f1ef5d3612df43d2400547324cfd4d5aa4bddea1 Mon Sep 17 00:00:00 2001 From: dev-ecd-dm Date: Thu, 4 Jun 2026 01:34:54 -0300 Subject: [PATCH] fix(engine): exclude build outputs (.output) from detector + Semgrep; tighten DEFAULT_CREDENTIALS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driven by a real monorepo comparison (DeckMidia, Nuxt): the engine report had 218 findings / score 49 dominated by FALSE POSITIVES from build artifacts under .output/ (prototype pollution in _nitro.mjs, JWTs in bundles) — ~47 critical FPs. Root causes: - .output (Nuxt/Nitro build output) was missing from DEFAULT_IGNORE_DIRS (only .next/.nuxt were covered), so the heuristic detector scanned it. - runSemgrep passed only --exclude node_modules --exclude csreview-reports, so Semgrep scanned .output/dist/build/.nuxt and reported FPs from compiled bundles. - DEFAULT_CREDENTIALS matched a bare admin|root|default|test key, flagging UI role-label maps like { admin: 'Admin' } as CRITICAL default credentials. Fixes: - Add JS/TS build outputs to DEFAULT_IGNORE_DIRS: .output, out, .vercel, .netlify, .svelte-kit, .angular, .turbo, .parcel-cache. Flows to BOTH the scanner discovery globs and the external-tool finding suppression. - New semgrepExcludeArgs(): Semgrep now excludes the SAME dirs as the detector (single source of truth = DEFAULT_IGNORE_DIRS), not just node_modules. - DEFAULT_CREDENTIALS now requires a credential-context key (password/pwd/secret/ credential) set to a weak value; keeps real default creds, drops the label FP. Tests: +2 + extended ignore/credential guards. 183/183 - lint clean - typecheck 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- csreview/src/detector.js | 6 +++++- csreview/src/ignore.js | 12 +++++++++++ csreview/src/index.js | 24 ++++++++++++++++++++-- csreview/test/detector-calibration.test.js | 19 +++++++++++++++++ csreview/test/ignore.test.js | 5 +++++ csreview/test/tool-env.test.js | 16 ++++++++++++++- 6 files changed, 78 insertions(+), 4 deletions(-) diff --git a/csreview/src/detector.js b/csreview/src/detector.js index 473c11d..dae3a22 100644 --- a/csreview/src/detector.js +++ b/csreview/src/detector.js @@ -851,7 +851,11 @@ const VULNERABILITY_PATTERNS = [ category: 'Configuration', name: 'Default Credentials', description: 'Default admin/password credentials.', - regex: /(?:admin|root|default|test)['"]?\s*[:=]\s*['"](?:admin|password|123456|root|default|changeme|test)['"]/gi, + // Requires a credential-context KEY (password/pwd/secret/credential) set to a + // weak/default value — not a bare admin|root|default|test key, which matched + // UI role-label maps like `{ admin: 'Admin' }` (a CRITICAL false positive). + regex: + /(?:pass(?:word|wd)?|pwd|secret|credential)['"]?\s*[:=]\s*['"](?:admin|password|123456|root|default|changeme|test|pass|secret|letmein|qwerty|admin123|password1|password123)['"]/gi, cwe: 'CWE-798', owasp: 'A07:2021-Identification and Authentication Failures', fix: 'Change all default credentials.', diff --git a/csreview/src/ignore.js b/csreview/src/ignore.js index 5d7b15b..b520220 100644 --- a/csreview/src/ignore.js +++ b/csreview/src/ignore.js @@ -219,6 +219,18 @@ export const DEFAULT_IGNORE_DIRS = [ '.dart_tool', // Flutter/Dart build cache '.gradle', // Gradle/Android cache '.supabase', // Supabase CLI local runtime state (distinct from the `supabase/` source tree) + // JS/TS framework build outputs & deploy/cache dirs — compiled/bundled code, + // never first-party source. `.output` (Nuxt/Nitro) was the dominant false- + // positive source on a real monorepo: prototype-pollution in `_nitro.mjs`, + // JWTs in bundles, etc. (`.next`/`.nuxt` were already covered above.) + '.output', // Nuxt/Nitro build output + 'out', // Next.js static export + '.vercel', // Vercel build output + '.netlify', // Netlify build output + '.svelte-kit', // SvelteKit build output + '.angular', // Angular cache/build + '.turbo', // Turborepo cache + '.parcel-cache', // Parcel cache ]; /** Built-in default file globs (minified/generated artifacts and prior reports). */ diff --git a/csreview/src/index.js b/csreview/src/index.js index 601f997..7201234 100644 --- a/csreview/src/index.js +++ b/csreview/src/index.js @@ -10,7 +10,13 @@ import { generateMarkdownReport } from './reports/markdown.js'; import { generateSarifReport } from './reports/sarif.js'; import { calculateSecurityScore } from './score.js'; import { normalizeLocalPath, safeResolveInside } from './pathSafety.js'; -import { loadIgnore, applyIgnore, compileIgnorePatterns, DEFAULT_IGNORE_PATTERNS } from './ignore.js'; +import { + loadIgnore, + applyIgnore, + compileIgnorePatterns, + DEFAULT_IGNORE_PATTERNS, + DEFAULT_IGNORE_DIRS, +} from './ignore.js'; import { loadBaseline, applyBaseline, writeBaseline } from './baseline.js'; /** @@ -865,12 +871,26 @@ export function normalizeOsvScannerFindings(osvJson = {}, rootDir = process.cwd( return findings; } +/** + * Build Semgrep `--exclude ` argument pairs from the canonical ignore-dir + * list so Semgrep skips the SAME build outputs / vendored dirs as the heuristic + * detector. Without this Semgrep scanned `.output`, `dist`, `.nuxt`, etc. and + * produced critical false positives from compiled bundles (e.g. prototype + * pollution in `_nitro.mjs`). + * + * @param {string[]} [dirs] + * @returns {string[]} + */ +export function semgrepExcludeArgs(dirs = DEFAULT_IGNORE_DIRS) { + return dirs.flatMap((dir) => ['--exclude', dir]); +} + async function runSemgrep(rootDir) { try { const versionResult = await execTool('semgrep', ['--version'], { timeout: VERSION_CHECK_TIMEOUT_MS }); const scanResult = await execTool( 'semgrep', - ['--config', 'auto', '--json', '--quiet', '--exclude', 'node_modules', '--exclude', 'csreview-reports', rootDir], + ['--config', 'auto', '--json', '--quiet', ...semgrepExcludeArgs(), rootDir], { cwd: rootDir, timeout: 120000, diff --git a/csreview/test/detector-calibration.test.js b/csreview/test/detector-calibration.test.js index 02ee99d..1bfe3ab 100644 --- a/csreview/test/detector-calibration.test.js +++ b/csreview/test/detector-calibration.test.js @@ -87,6 +87,25 @@ test('findings in non-source paths (test/fixtures) are downgraded; real source k assert.equal(inTest.confidence, 'LOW'); }); +test('DEFAULT_CREDENTIALS does not fire on UI role-label maps but still catches weak password defaults', () => { + // FP (DeckMidia): a role->label map like { admin: 'Admin' } was flagged as + // CRITICAL Default Credentials because the rule matched admin: 'Admin'. + const fp = scan([ + { path: 'src/labels.js', code: "export const roleLabels = { owner: 'Owner', admin: 'Admin', test: 'Test' };\n" }, + ]); + assert.equal( + fp.filter((f) => String(f.id).startsWith('DEFAULT_CREDENTIALS')).length, + 0, + `unexpected DEFAULT_CREDENTIALS FPs: ${fp.map((f) => f.file).join(', ')}`, + ); + // TP: a credential field set to a weak/default value must still be caught. + const tp = scan([{ path: 'src/cfg.js', code: "export const cfg = { password: 'admin', pwd: 'changeme' };\n" }]); + assert.ok( + tp.some((f) => String(f.id).startsWith('DEFAULT_CREDENTIALS')), + 'expected DEFAULT_CREDENTIALS to catch password: "admin"', + ); +}); + test('the shipped .csreview-ignore keeps a csreview self-audit free of rule-definition meta-FPs', async () => { // A scanner must not flag its own rulebook: detector.js (regexes/descriptions/ // exploitation strings) and dumpGuide.js (sample connection strings) match the diff --git a/csreview/test/ignore.test.js b/csreview/test/ignore.test.js index f894ddb..5567ccd 100644 --- a/csreview/test/ignore.test.js +++ b/csreview/test/ignore.test.js @@ -129,6 +129,8 @@ test('DEFAULT_IGNORE_DIRS covers vendored deps, build output, and generated cach '.dart_tool', // Flutter/Dart cache (the dominant external-tool noise source) '.gradle', // Gradle/Android cache '.supabase', // Supabase CLI local runtime state + '.output', // Nuxt/Nitro build output (the DeckMidia .output false-positive source) + 'out', // Next.js static export 'csreview-reports', '.csreview', ]) { @@ -142,6 +144,9 @@ test('DEFAULT_IGNORE_PATTERNS are gitignore-syntax and scope external-tool findi assert.ok(isIgnored('flutter/apps/operator/.dart_tool/chrome-device/Default/Preferences', compiled)); assert.ok(isIgnored('supabase-stuff/.supabase/postgres/data/x', compiled)); assert.ok(isIgnored('android/app/.gradle/cache/x', compiled)); + // Nuxt/Nitro build output — the DeckMidia false positives (prototype pollution + // in _nitro.mjs, JWT in bundles) all came from here. + assert.ok(isIgnored('apps/web/.output/server/chunks/_nitro.mjs', compiled)); assert.ok(isIgnored('a/node_modules/pkg/index.js', compiled)); assert.ok(isIgnored('packages/web/dist/bundle.min.js', compiled)); // root-level cache dirs (no parent component) must also match — guards against diff --git a/csreview/test/tool-env.test.js b/csreview/test/tool-env.test.js index 1e6e51d..4d6a0a6 100644 --- a/csreview/test/tool-env.test.js +++ b/csreview/test/tool-env.test.js @@ -1,7 +1,7 @@ // @ts-check import test from 'node:test'; import assert from 'node:assert/strict'; -import { withToolEnv } from '../src/index.js'; +import { withToolEnv, semgrepExcludeArgs } from '../src/index.js'; // Semgrep's update/version "phone home" check can hang the process (notably on // Linux): `semgrep --version` prints the version then blocks on the check. @@ -32,3 +32,17 @@ test('withToolEnv is a no-op for non-semgrep tools (returns options unchanged)', assert.equal(opts, original); assert.equal(opts.env, undefined); }); + +test('semgrepExcludeArgs excludes build outputs and vendored dirs (the DeckMidia .output noise)', () => { + const args = semgrepExcludeArgs(); + // shape: alternating --exclude + assert.ok(args.length >= 2 && args.length % 2 === 0); + const excluded = new Set(); + for (let i = 0; i < args.length; i += 2) { + assert.equal(args[i], '--exclude'); + excluded.add(args[i + 1]); + } + for (const dir of ['node_modules', 'dist', 'build', '.nuxt', '.output', 'csreview-reports', '.git']) { + assert.ok(excluded.has(dir), `semgrep must exclude ${dir}`); + } +});