fix(llm-security): HIGH — one unparseable JetBrains plugin crashed ide-scan

The two manifest parsers disagree on how they signal failure:
parseVSCodeExtension returns a bare null, parseIntelliJPlugin returns a
TRUTHY { manifest: null, warnings }. scanOneExtension guarded only the
bare-null form, so every JetBrains failure path — lib/ missing, lib/ not
a directory, lib/ unreadable, no jars in lib/, no jar extractable — fell
through the guard and dereferenced `manifest.hasSignature`.

The resulting TypeError propagated out of scanOneExtension into
mapConcurrent, which awaited fn() with no per-item try/catch, so
Promise.all rejected and the ENTIRE ide-scan aborted. One malformed
plugin directory on disk was enough to take down the scan of every
other installed extension — including, in an audit context, a plugin
that is malformed precisely because it is hostile.

- scanOneExtension: guard is now `!parsed || !parsed.manifest`, and the
  parser's own warnings are carried into the result so the reason for
  the skip survives instead of being replaced by a generic message.
- unscannableExtension(): the result envelope is extracted so the early
  return and the isolation belt below produce the same shape.
- Belt (defense-in-depth): the scanOneExtension call site catches per
  extension and yields an unscannable result. mapConcurrent stays
  generic — the isolation lives at the call site, not in the helper.

Regression test drives scanOneExtension across all four JetBrains parse
failure paths plus a no-invented-findings assertion.

npm test: 1884/1884 green (1879 + 5 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:
Kjell Tore Guttormsen 2026-07-18 09:14:12 +02:00
commit 4e16ea0e72
2 changed files with 136 additions and 14 deletions

View file

@ -628,6 +628,30 @@ function runJetBrainsChecks(ext, manifest, topList, blocklist, relLocation) {
// Reused-scanner orchestration per extension
// ---------------------------------------------------------------------------
/**
* Result envelope for an extension that could not be scanned at all.
* Shape-compatible with a normal per-extension result so the top-level
* aggregation loop can consume it without special-casing.
* @param {object} ext
* @param {...string} reasons
*/
function unscannableExtension(ext, ...reasons) {
return {
id: ext.id,
version: ext.version,
type: ext.type,
location: ext.location,
publisher: ext.publisher,
source: ext.source,
is_builtin: ext.isBuiltin,
signed: ext.signed,
scanner_results: {},
warnings: reasons.filter(Boolean),
aggregate: { counts: { critical: 0, high: 0, medium: 0, low: 0, info: 0 }, risk_score: 0, risk_band: 'Low', verdict: 'ALLOW' },
duration_ms: 0,
};
}
async function scanOneExtension(ext, options) {
const started = Date.now();
const warnings = [];
@ -636,19 +660,16 @@ async function scanOneExtension(ext, options) {
const parsed = ext.type === 'jetbrains'
? await parseIntelliJPlugin(ext.location)
: await parseVSCodeExtension(ext.location);
if (!parsed) {
// Two "unparseable" shapes reach here: parseVSCodeExtension returns a bare
// null, parseIntelliJPlugin returns a TRUTHY { manifest: null, warnings }.
// Both must short-circuit — otherwise the null manifest is dereferenced below.
if (!parsed || !parsed.manifest) {
return {
id: ext.id,
version: ext.version,
type: ext.type,
location: ext.location,
publisher: ext.publisher,
source: ext.source,
is_builtin: ext.isBuiltin,
signed: ext.signed,
scanner_results: {},
warnings: [`failed to parse manifest for ${ext.id}`],
aggregate: { counts: { critical: 0, high: 0, medium: 0, low: 0, info: 0 }, risk_score: 0, risk_band: 'Low', verdict: 'ALLOW' },
...unscannableExtension(
ext,
`failed to parse manifest for ${ext.id}`,
...(parsed?.warnings || []),
),
duration_ms: Date.now() - started,
};
}
@ -935,8 +956,17 @@ export async function scan(target, options = {}) {
const targetBase = singleTargetPath || (rootsScanned[0] || process.cwd());
const concurrency = Math.max(1, Math.min(options.concurrency || 4, 16));
const perExt = await mapConcurrent(extensions, concurrency, ext =>
scanOneExtension(ext, { targetBase, online: options.online === true }));
// Fault isolation (belt): one unscannable extension must never abort the run.
// scanOneExtension is hardened against the known null-manifest case, but any
// future throw would otherwise reject mapConcurrent's Promise.all and fail the
// entire ide-scan because of a single bad plugin on disk.
const perExt = await mapConcurrent(extensions, concurrency, async ext => {
try {
return await scanOneExtension(ext, { targetBase, online: options.online === true });
} catch (err) {
return unscannableExtension(ext, `scan failed: ${err?.message || err}`);
}
});
// Top-level aggregate
const aggCounts = { critical: 0, high: 0, medium: 0, low: 0, info: 0 };