diff() silently drops components that share a key: buildComponentMap collapses coexisting entries (last-write-wins), under-reporting added/removed packages

[dmchaledev]

Summary

buildComponentMap() stores components in a Map keyed by purl ?? name and uses map.set(key, comp) — last-write-wins (src/diff.ts:63-71). When a single SBOM contains two or more components that share that key, every entry but the last is silently discarded before the diff runs. The dropped component then never appears in added / removed / upgraded, so the report under-reports the change set — the one outcome a supply-chain diff must never produce.

This collapse is invisible: there is no warning, no count, no trace. A newly-introduced package can be completely absent from the "Added" list, and a removed one absent from "Removed."

Two real-world trigger cases

The shared key arises whenever two distinct components map to the same purl ?? name:

1. Purl-less components with the same name. Common in OS-package and container SBOMs, vendored/relocated copies, or multi-arch builds — e.g. two zlib entries from different suppliers/versions, neither carrying a purl. Both key to "zlib".
2. The same package present at two versions. npm dependency trees routinely ship two majors of one library (e.g. lodash@3 and lodash@4). Today they key by their (version-qualified) purl and so coexist — but the in-flight keying PRs (fix: detect version upgrades by purl and repair default CLI invocation #10/fix(diff): detect upgrades for version-qualified purls #20/fix(diff): detect version upgrades for version-qualified purls #31/fix(diff): detect upgrades when the purl carries the version #37/fix(diff): detect upgrades when purls embed the version #42/fix(diff): detect version upgrades when purls include the version #44/fix(diff): detect upgrades by stripping version from purl key #47) deliberately strip the version from the purl key to fix upgrade detection. Once any of those lands, pkg:npm/lodash@3.x and pkg:npm/lodash@4.x both collapse to pkg:npm/lodash, and one silently vanishes. This bug gets worse the moment the upgrade-detection fix merges, so it should be sequenced alongside that work.

Reproduction (current main)

import { diff } from '@hailbytes/sbom-diff';

// Two DISTINCT components that legitimately share a name but have no purl.
const a = { format: 'cyclonedx', components: [], vulnerabilities: [] };
const b = { format: 'cyclonedx', components: [
  { name: 'zlib', version: '1.2.11', supplier: 'distro-x' },
  { name: 'zlib', version: '1.3.1',  supplier: 'distro-y' },
], vulnerabilities: [] };

const r = diff(a, b);
console.log(r.added.length);                       // -> 1   (expected 2)
console.log(r.added.map(c => `${c.name}@${c.version}`)); // -> [ 'zlib@1.3.1' ]  — 1.2.11 is gone

One of two genuinely-added packages is missing from the report. totalAdded reads 1, so a --fail-on/audit consumer sees an under-count with no indication anything was dropped.

Evidence in source

src/diff.ts:63-71 — the collapse:

function buildComponentMap(components: Component[]): Map<string, Component> {
  const map = new Map<string, Component>();
  for (const comp of components) {
    const key = comp.purl ?? comp.name;   // shared by coexisting entries
    map.set(key, comp);                   // last-write-wins: earlier entries dropped
  }
  return map;
}

The three diff loops (src/diff.ts:19-38) then iterate these already-lossy maps, so added / removed / upgraded are computed over a set that is missing entries. summary.totalAdded etc. inherit the under-count.

Why this is distinct from everything in flight

• The keying PRs (fix: detect version upgrades by purl and repair default CLI invocation #10/fix(diff): detect upgrades for version-qualified purls #20/fix(diff): detect version upgrades for version-qualified purls #31/fix(diff): detect upgrades when the purl carries the version #37/fix(diff): detect upgrades when purls embed the version #42/fix(diff): detect version upgrades when purls include the version #44/fix(diff): detect upgrades by stripping version from purl key #47) change what the key is (strip the version so upgrades match). None change the last-write-wins collision behavior — they keep a Map<string, Component> and .set(). As noted above, they amplify this bug rather than fix it.
• fix(parser): flatten nested CycloneDX components so they are diffed #29 flattens nested components[].components; that adds more entries to the same lossy map — orthogonal, and if anything increases collision odds.
• Issues CVE blast radius is truncated: parser keeps only the first affected component (affects[0]), hiding every other package a CVE hits #30 (CVE affects[0] truncation) and Detect CVE severity escalations: a re-scored CVE (e.g. medium → critical) that persists in both SBOMs produces an empty, green report #46 (CVE severity escalation) are about the vulnerability path (v.id keying), not the component map.

No open issue or PR addresses component-key collisions.

Proposed change

Make the component map multiplicity-aware and pair entries up during the diff, instead of collapsing them.

1. Build lists, not single entries (src/diff.ts)

function buildComponentMap(components: Component[]): Map<string, Component[]> {
  const map = new Map<string, Component[]>();
  for (const comp of components) {
    const key = identityKey(comp);              // version-independent (see below)
    (map.get(key) ?? map.set(key, []).get(key)!).push(comp);
  }
  return map;
}

identityKey(comp) should be version-independent so upgrade detection keeps working — i.e. purl-with-version-stripped, else name. (This is the same key the in-flight upgrade PRs are converging on; this issue only asks that the bucket hold a list.)

2. Pair up per key in the diff (src/diff.ts:19-38)

For each identity key:

• present only in B → every B entry is added; only in A → every A entry is removed.
• present in both → match entries by exact version first; a leftover 1-vs-1 with differing versions is an upgrade (preserving today's name-based upgrade behavior — see diff.test.ts:55-63); any remaining unmatched A entries are removed, unmatched B entries added.

For the overwhelmingly common 1:1 case this reduces exactly to current behavior; it only changes the multiplicity case, where it stops dropping data.

3. Tests (src/__tests__/diff.test.ts)

• Two purl-less zlib entries added ⇒ added.length === 2 (the reproduction above).
• Same package at two versions coexisting in both SBOMs ⇒ no phantom add/remove; each is preserved.
• Single name upgrade 4.17.20 → 4.17.21 ⇒ still one upgraded (regression guard for the existing name-match test).
• Identical SBOM with duplicate-keyed entries self-diffs to all-zero.

Why this is high-leverage

• Protects the tool's core promise. "See what changed — added/removed/upgraded packages" is the headline feature; silently dropping a component means an injected or removed dependency can go completely unreported. That is squarely the supply-chain-security remit.
• Backward compatible for the common case — 1:1 keys behave exactly as today; only the currently-lossy multiplicity case changes.
• No new dependencies.
• Time-sensitive: it should be resolved with the upgrade-detection keying rework, because stripping the version from the key (the fix those 7 PRs implement) turns an occasional collision into a routine one.

Happy to open a focused PR (multiplicity-aware map + pairing + tests) once the direction is confirmed and whichever keying PR the maintainer prefers has landed, to keep diff.ts conflicts minimal.