fix(llm-security): v8 Phase 1 — Berry lockfile, nested-v1 recursion, per-occurrence strip attribution
Three TDD-first fixes surviving the B8 roadmap bucket (v8.0.0-plan.local.md Phase 1, items 1-3; item 4 JAR hardening scoped out at review): - supply-chain-recheck.mjs parseYarnLock: ported the hook's per-entry parser (pre-install-supply-chain.mjs) so Berry's `version: x` format (unquoted) is recognized alongside Classic's `version "x"` — Berry lockfiles previously yielded zero deps, silently missing pinned compromised packages. - supply-chain-recheck.mjs parsePackageLock: lockfileVersion-1 fallback now recurses nested `dependencies`, mirroring the hook's walk() — a transitive, non-hoisted compromised copy below the top level was previously invisible. - content-extractor.mjs stripInjection: attribution moved from a global `Set<label>` to `Set<label::lineIndex>`. The old check silenced the unstripped flag for ANY occurrence of a label once ANY occurrence had been line-redacted, so a second, cross-line-only encoded occurrence of the same label survived into sanitized output without being flagged. Full suite 2019/2019 (one known-flaky timing test confirmed green in isolation).
This commit is contained in:
parent
b929ddc2bb
commit
ff4d8e8a31
5 changed files with 149 additions and 33 deletions
|
|
@ -137,7 +137,11 @@ function stripInjection(text, file) {
|
||||||
// Runs first so that line indices still line up with the original text.
|
// Runs first so that line indices still line up with the original text.
|
||||||
const lines = text.split('\n');
|
const lines = text.split('\n');
|
||||||
const normalizedLines = isDifferent ? lines.map(l => normalizeForScan(l)) : [];
|
const normalizedLines = isDifferent ? lines.map(l => normalizeForScan(l)) : [];
|
||||||
const attributed = new Set();
|
// Keyed by `${label}::${lineIndex}`, not just label — a label redacted on
|
||||||
|
// one line must not silence the unstripped check for a DIFFERENT
|
||||||
|
// occurrence of the same label elsewhere (e.g. a second, cross-line-only
|
||||||
|
// encoding Pass 1 couldn't catch). Attribution is judged per occurrence.
|
||||||
|
const attributedLines = new Set();
|
||||||
|
|
||||||
if (isDifferent) {
|
if (isDifferent) {
|
||||||
for (const { pattern, label } of allPatterns) {
|
for (const { pattern, label } of allPatterns) {
|
||||||
|
|
@ -147,7 +151,7 @@ function stripInjection(text, file) {
|
||||||
if (lines[i].includes(STRIP_MARKER)) continue;
|
if (lines[i].includes(STRIP_MARKER)) continue;
|
||||||
if (toGlobal(pattern).test(normalizedLines[i])) {
|
if (toGlobal(pattern).test(normalizedLines[i])) {
|
||||||
lines[i] = `[${STRIP_MARKER}: ${label}]`;
|
lines[i] = `[${STRIP_MARKER}: ${label}]`;
|
||||||
attributed.add(label);
|
attributedLines.add(`${label}::${i}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -164,8 +168,10 @@ function stripInjection(text, file) {
|
||||||
const finding = { file, line, label, severity };
|
const finding = { file, line, label, severity };
|
||||||
const before = sanitized;
|
const before = sanitized;
|
||||||
sanitized = sanitized.replace(match[0], `[${STRIP_MARKER}: ${label}]`);
|
sanitized = sanitized.replace(match[0], `[${STRIP_MARKER}: ${label}]`);
|
||||||
// Neither a literal replace nor a line redaction removed this one.
|
// Neither a literal replace nor a line redaction removed THIS occurrence.
|
||||||
if (sanitized === before && !attributed.has(label)) finding.unstripped = true;
|
if (sanitized === before && !attributedLines.has(`${label}::${line - 1}`)) {
|
||||||
|
finding.unstripped = true;
|
||||||
|
}
|
||||||
findings.push(finding);
|
findings.push(finding);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -84,21 +84,32 @@ async function parsePackageLock(filePath) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// v1 fallback: dependencies object
|
// v1 fallback: dependencies is a nested tree — recurse so a non-hoisted
|
||||||
|
// nested copy of a package is still found, mirroring the hook's walk()
|
||||||
|
// (pre-install-supply-chain.mjs scanNpmLockfile).
|
||||||
if (deps.length === 0 && lock.dependencies) {
|
if (deps.length === 0 && lock.dependencies) {
|
||||||
for (const [name, info] of Object.entries(lock.dependencies)) {
|
const walk = (dependencies) => {
|
||||||
if (info.version) {
|
for (const [name, info] of Object.entries(dependencies)) {
|
||||||
deps.push({ name, version: info.version, ecosystem: 'npm' });
|
if (info && info.version) {
|
||||||
|
deps.push({ name, version: info.version, ecosystem: 'npm' });
|
||||||
|
}
|
||||||
|
if (info && info.dependencies) walk(info.dependencies);
|
||||||
}
|
}
|
||||||
}
|
};
|
||||||
|
walk(lock.dependencies);
|
||||||
}
|
}
|
||||||
} catch { /* parse error — skip */ }
|
} catch { /* parse error — skip */ }
|
||||||
return deps;
|
return deps;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Parse yarn.lock (v1 format).
|
* Parse yarn.lock — both Classic (v1) and Berry (v2+) formats.
|
||||||
* Extracts package name and resolved version from each entry.
|
* Parses per entry: an entry starts at column 0 with one or more specs
|
||||||
|
* ending in ':' ("pkg@range" / Berry '"pkg@npm:range"'), followed by an
|
||||||
|
* indented body carrying `version "x"` (Classic) or `version: x` (Berry).
|
||||||
|
* The version is associated with ITS OWN entry (no unanchored substring
|
||||||
|
* matching across entries), mirroring the hook's parser
|
||||||
|
* (pre-install-supply-chain.mjs scanNpmLockfile).
|
||||||
* @param {string} filePath - Absolute path to yarn.lock
|
* @param {string} filePath - Absolute path to yarn.lock
|
||||||
* @returns {Promise<{ name: string, version: string, ecosystem: string }[]>}
|
* @returns {Promise<{ name: string, version: string, ecosystem: string }[]>}
|
||||||
*/
|
*/
|
||||||
|
|
@ -106,28 +117,21 @@ async function parseYarnLock(filePath) {
|
||||||
const deps = [];
|
const deps = [];
|
||||||
try {
|
try {
|
||||||
const raw = await readFile(filePath, 'utf8');
|
const raw = await readFile(filePath, 'utf8');
|
||||||
const lines = raw.split('\n');
|
for (const entry of raw.split(/\n(?=\S)/)) {
|
||||||
let currentPkg = null;
|
const nl = entry.indexOf('\n');
|
||||||
|
if (nl === -1) continue;
|
||||||
for (const line of lines) {
|
const header = entry.slice(0, nl).trim();
|
||||||
// Package header: "pkg@^1.0.0", "pkg@1.0.0:" or "@scope/pkg@^1.0.0":
|
if (!header.endsWith(':') || header.startsWith('#')) continue;
|
||||||
if (!line.startsWith(' ') && !line.startsWith('#') && line.includes('@')) {
|
const vMatch = entry.match(/^\s+version:?\s+"?([^"\s]+)"?\s*$/m);
|
||||||
const trimmed = line.replace(/[":]/g, '').trim();
|
const entryVersion = vMatch ? vMatch[1] : null;
|
||||||
if (trimmed.startsWith('@')) {
|
if (!entryVersion) continue;
|
||||||
// Scoped: @scope/pkg@version
|
const names = new Set(header.slice(0, -1).split(',').map(part => {
|
||||||
const rest = trimmed.slice(1);
|
const spec = part.trim().replace(/^"|"$/g, '');
|
||||||
const atIdx = rest.indexOf('@');
|
const at = spec.indexOf('@', spec.startsWith('@') ? 1 : 0);
|
||||||
if (atIdx > 0) currentPkg = '@' + rest.slice(0, atIdx);
|
return at > 0 ? spec.slice(0, at) : spec;
|
||||||
} else {
|
}));
|
||||||
const atIdx = trimmed.indexOf('@');
|
for (const name of names) {
|
||||||
if (atIdx > 0) currentPkg = trimmed.slice(0, atIdx);
|
deps.push({ name, version: entryVersion, ecosystem: 'npm' });
|
||||||
}
|
|
||||||
}
|
|
||||||
// Version line: " version "1.2.3""
|
|
||||||
const versionMatch = line.match(/^\s+version\s+"([^"]+)"/);
|
|
||||||
if (versionMatch && currentPkg) {
|
|
||||||
deps.push({ name: currentPkg, version: versionMatch[1], ecosystem: 'npm' });
|
|
||||||
currentPkg = null;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch { /* parse error — skip */ }
|
} catch { /* parse error — skip */ }
|
||||||
|
|
|
||||||
20
tests/fixtures/supply-chain/yarn-berry-compromised.lock
vendored
Normal file
20
tests/fixtures/supply-chain/yarn-berry-compromised.lock
vendored
Normal file
|
|
@ -0,0 +1,20 @@
|
||||||
|
# This file is generated by running "yarn install" inside your project.
|
||||||
|
# yarn lockfile v6
|
||||||
|
|
||||||
|
__metadata:
|
||||||
|
version: 8
|
||||||
|
cacheKey: 10
|
||||||
|
|
||||||
|
"event-stream@npm:^3.3.6":
|
||||||
|
version: 3.3.6
|
||||||
|
resolution: "event-stream@npm:3.3.6"
|
||||||
|
checksum: 0123456789abcdef
|
||||||
|
languageName: node
|
||||||
|
linkType: hard
|
||||||
|
|
||||||
|
"lodash@npm:^4.17.21":
|
||||||
|
version: 4.17.21
|
||||||
|
resolution: "lodash@npm:4.17.21"
|
||||||
|
checksum: fedcba9876543210
|
||||||
|
languageName: node
|
||||||
|
linkType: hard
|
||||||
|
|
@ -74,4 +74,37 @@ describe('content-extractor — stripInjection removes what it reports', () => {
|
||||||
assert.match(sanitized, /keep-after/);
|
assert.match(sanitized, /keep-after/);
|
||||||
assert.ok(!sanitized.includes(encoded));
|
assert.ok(!sanitized.includes(encoded));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('flags a second, cross-line occurrence of an already-stripped label as unstripped', () => {
|
||||||
|
// Two occurrences of the SAME label: the first is single-line encoded
|
||||||
|
// (caught + redacted by the per-line pass), the second is split across
|
||||||
|
// two lines so no individual line's decoded form matches — only the
|
||||||
|
// whole-text normalization does (the documented residual gap). A global
|
||||||
|
// `attributed.has(label)` check wrongly treats the second occurrence as
|
||||||
|
// handled because the FIRST one was; it must be judged on its own line
|
||||||
|
// span instead.
|
||||||
|
const singleLine = htmlEntities('ignore all previous');
|
||||||
|
const crossLineA = htmlEntities('ignore all');
|
||||||
|
const crossLineB = htmlEntities('previous');
|
||||||
|
const text = [
|
||||||
|
'keep-before',
|
||||||
|
singleLine,
|
||||||
|
'keep-middle',
|
||||||
|
crossLineA,
|
||||||
|
crossLineB,
|
||||||
|
'keep-after',
|
||||||
|
'',
|
||||||
|
].join('\n');
|
||||||
|
const { sanitized, findings } = stripInjection(text, 'README.md');
|
||||||
|
|
||||||
|
assert.ok(!sanitized.includes(singleLine), 'the single-line occurrence must be stripped');
|
||||||
|
assert.ok(
|
||||||
|
sanitized.includes(crossLineA) && sanitized.includes(crossLineB),
|
||||||
|
'the cross-line occurrence is expected to survive stripping (documented residual gap)'
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
findings.some(f => f.unstripped === true),
|
||||||
|
'a payload that survives into sanitized output must be flagged unstripped, not silently reported as handled'
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -117,6 +117,39 @@ describe('supply-chain-recheck: npm blocklist', () => {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('detects compromised event-stream@3.3.6 nested under lockfileVersion 1 dependencies', async () => {
|
||||||
|
// lockfileVersion 1's `dependencies` is a nested tree — a transitive,
|
||||||
|
// non-hoisted copy sits under a parent's own `dependencies` object.
|
||||||
|
// parsePackageLock must recurse, mirroring the hook's walk()
|
||||||
|
// (pre-install-supply-chain.mjs #48).
|
||||||
|
if (existsSync(TEMP)) rmSync(TEMP, { recursive: true });
|
||||||
|
mkdirSync(TEMP, { recursive: true });
|
||||||
|
writeFileSync(join(TEMP, 'package-lock.json'), JSON.stringify({
|
||||||
|
name: 'sample',
|
||||||
|
lockfileVersion: 1,
|
||||||
|
dependencies: {
|
||||||
|
a: {
|
||||||
|
version: '1.0.0',
|
||||||
|
dependencies: {
|
||||||
|
'event-stream': { version: '3.3.6' },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
try {
|
||||||
|
const result = await scan(TEMP, { files: [] });
|
||||||
|
const compromised = result.findings.filter(
|
||||||
|
f => f.title.includes('Compromised') && f.title.includes('event-stream')
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
compromised.length >= 1,
|
||||||
|
`Expected compromised finding for nested event-stream, got ${result.findings.map(f => f.title).join('; ')}`
|
||||||
|
);
|
||||||
|
} finally {
|
||||||
|
cleanupTemp();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it('does not flag clean packages in package-lock.json', async () => {
|
it('does not flag clean packages in package-lock.json', async () => {
|
||||||
setupTemp({ 'package-lock.json': 'package-lock-clean.json' });
|
setupTemp({ 'package-lock.json': 'package-lock-clean.json' });
|
||||||
try {
|
try {
|
||||||
|
|
@ -140,6 +173,26 @@ describe('supply-chain-recheck: npm blocklist', () => {
|
||||||
cleanupTemp();
|
cleanupTemp();
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('detects compromised event-stream@3.3.6 in a Berry yarn.lock (version: x format)', async () => {
|
||||||
|
// Berry (yarn v2+) writes `version: x` with no quotes, unlike Classic's
|
||||||
|
// `version "x"`. parseYarnLock must recognize both, mirroring the hook's
|
||||||
|
// parser (pre-install-supply-chain.mjs #17).
|
||||||
|
setupTemp({ 'yarn.lock': 'yarn-berry-compromised.lock' });
|
||||||
|
try {
|
||||||
|
const result = await scan(TEMP, { files: [] });
|
||||||
|
const compromised = result.findings.filter(
|
||||||
|
f => f.title.includes('Compromised') && f.title.includes('event-stream')
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
compromised.length >= 1,
|
||||||
|
`Expected compromised finding for event-stream, got ${result.findings.map(f => f.title).join('; ')}`
|
||||||
|
);
|
||||||
|
assert.equal(compromised[0].severity, 'critical');
|
||||||
|
} finally {
|
||||||
|
cleanupTemp();
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue