From cc37b899ea9a99ba83e255ce0efc469155711400 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 03:08:56 +0000 Subject: [PATCH] fix: correct upgrade detection and CLI default format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness bugs broke the documented usage: - diff() keyed components by the full purl, but purls embed the version (e.g. pkg:npm/lodash@4.17.21). A version change produced a different key, so upgrades were reported as a separate add + remove instead of an upgrade — defeating the package's headline feature for any SBOM with versioned purls (the real-world norm). Now matched by a version-agnostic purl key; scoped npm packages (%40) are handled correctly. - The CLI crashed when run without --format. args.indexOf('--format') returned -1, so args[-1+1] used the first filename as the format and threw "Unsupported format". sbom-diff old.json new.json (the README's primary example) now works, and invalid --format values give a clear error. Updated the misleading diff test that asserted the buggy behavior and added coverage for versioned-purl and scoped-package upgrades. --- CHANGELOG.md | 4 ++++ src/__tests__/diff.test.ts | 23 ++++++++++++++++++----- src/cli.ts | 9 ++++++++- src/diff.ts | 27 +++++++++++++++++++++++---- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4f3c2a..b25a8f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Fixed +- `diff()` now matches components by a version-agnostic purl key, so a package whose version changes is reported as an **upgrade** instead of a separate add + remove. Real-world SBOMs embed the version in the purl (e.g. `pkg:npm/lodash@4.17.21`), which previously defeated upgrade detection — the package's headline feature. Scoped npm packages (`pkg:npm/%40babel/core@…`) are matched correctly. +- CLI no longer crashes when run without `--format`. `sbom-diff old.json new.json` (the primary example in the README) previously parsed the first filename as the format and threw `Unsupported format`. Invalid `--format` values now produce a clear error instead of a stack trace. + ### Added - Real devDependencies: `typescript`, `vitest`, `@vitest/coverage-v8`, `typescript-eslint`, `@types/node` - `src/types.ts` — Full domain model: `SBOM`, `Component`, `CVEEntry`, `ChangeReport`, `VersionChange`, `SBOMFormat`, `ReportFormat` diff --git a/src/__tests__/diff.test.ts b/src/__tests__/diff.test.ts index 0f8587c..c8ec5bf 100644 --- a/src/__tests__/diff.test.ts +++ b/src/__tests__/diff.test.ts @@ -42,14 +42,27 @@ describe('diff', () => { expect(report.removed[0].name).toBe('moment'); }); - it('detects version upgrades', () => { + it('detects version upgrades when matched by versioned purl', () => { + // purls embed the version, but the same package should still be matched + // across versions so the change is reported as an upgrade, not add/remove. const a = makesbom([{ name: 'lodash', version: '4.17.20', purl: 'pkg:npm/lodash@4.17.20' }]); const b = makesbom([{ name: 'lodash', version: '4.17.21', purl: 'pkg:npm/lodash@4.17.21' }]); const report = diff(a, b); - // Different purl = treated as add/remove (purl includes version) - // With our current purl-based key: 4.17.20 -> removed, 4.17.21 -> added - // This is correct behavior — different purls are different packages - expect(report.added.length + report.removed.length + report.upgraded.length).toBeGreaterThan(0); + expect(report.added).toHaveLength(0); + expect(report.removed).toHaveLength(0); + expect(report.upgraded).toHaveLength(1); + expect(report.upgraded[0].from).toBe('4.17.20'); + expect(report.upgraded[0].to).toBe('4.17.21'); + }); + + it('matches scoped npm packages across versions', () => { + // Scoped packages encode the namespace "@" as "%40"; only the version "@" + // is literal, so the package must still match across versions. + const a = makesbom([{ name: '@babel/core', version: '7.0.0', purl: 'pkg:npm/%40babel/core@7.0.0' }]); + const b = makesbom([{ name: '@babel/core', version: '7.1.0', purl: 'pkg:npm/%40babel/core@7.1.0' }]); + const report = diff(a, b); + expect(report.upgraded).toHaveLength(1); + expect(report.upgraded[0].to).toBe('7.1.0'); }); it('detects version upgrades when matched by name (no purl)', () => { diff --git a/src/cli.ts b/src/cli.ts index 986eb1a..74ad130 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -22,10 +22,17 @@ async function main(): Promise { } const [oldPath, newPath] = positional; + const flagIndex = args.indexOf('--format'); const formatArg = args.find(a => a.startsWith('--format='))?.split('=')[1] - ?? args[args.indexOf('--format') + 1]; + ?? (flagIndex !== -1 ? args[flagIndex + 1] : undefined); const format: ReportFormat = (formatArg as ReportFormat) ?? 'text'; + const validFormats: ReportFormat[] = ['text', 'json', 'markdown']; + if (!validFormats.includes(format)) { + console.error(`Unknown format: ${format}. Use one of: ${validFormats.join(', ')}`); + process.exit(1); + } + const [oldRaw, newRaw] = await Promise.all([ readFile(oldPath, 'utf-8'), readFile(newPath, 'utf-8'), diff --git a/src/diff.ts b/src/diff.ts index 6540b50..b8a97a7 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -4,8 +4,12 @@ import type { SBOM, Component, CVEEntry, ChangeReport, VersionChange } from './t * Compare two parsed SBOMs and produce a ChangeReport. * * Matching strategy: - * 1. By purl (most precise) + * 1. By version-agnostic purl (most precise) * 2. By name (fallback) + * + * The purl is stripped of its version so the same package matches across + * SBOMs even when its version changes — this is what enables upgrade + * detection (see {@link componentKey}). */ export function diff(a: SBOM, b: SBOM): ChangeReport { const aMap = buildComponentMap(a.components); @@ -63,13 +67,28 @@ export function diff(a: SBOM, b: SBOM): ChangeReport { function buildComponentMap(components: Component[]): Map { const map = new Map(); for (const comp of components) { - // Prefer purl as key, fall back to name - const key = comp.purl ?? comp.name; - map.set(key, comp); + map.set(componentKey(comp), comp); } return map; } +/** + * Build a version-agnostic identity key for a component so the same package + * matches across SBOMs even when its version changes (enabling upgrade + * detection). purls embed the version after "@" + * (e.g. "pkg:npm/lodash@4.17.21"), so we strip everything from the first + * literal "@" onward. Scoped npm namespaces encode their "@" as "%40", so the + * first literal "@" is always the version delimiter. Falls back to name when + * no purl is present. + */ +function componentKey(comp: Component): string { + if (comp.purl) { + const at = comp.purl.indexOf('@'); + return at === -1 ? comp.purl : comp.purl.slice(0, at); + } + return comp.name; +} + /** * Returns true if the major version changed (semver-style). * Handles versions like "1.2.3", "2.0.0-beta", etc.