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}`); + } +});