From f05053351a3a01ae9b667c8e2a01baedfc569d47 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Fri, 7 Aug 2026 13:23:14 -0600 Subject: [PATCH 1/2] fix(parser): reject non-SBOM input instead of silently passing empty report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #21 parse() previously accepted any JSON — a package.json passed by mistake, a truncated export, or garbage — and silently returned an empty CycloneDX SBOM. In a CI gate that read 'nothing changed' (a false negative). - parse() now throws ParseError when input is not a recognized CycloneDX or SPDX document (missing bomFormat/spdxVersion), is invalid JSON, or is not an object (array/null/primitives) - The CLI's loadSbom already wraps this in a clear 'Failed to parse' message; main() exits 1 (verified) - 6 new tests cover the rejection paths + valid-document acceptance 106 tests pass, tsc clean. --- src/__tests__/parser.test.ts | 28 +++++++++++++++++++ src/parser.ts | 54 +++++++++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/src/__tests__/parser.test.ts b/src/__tests__/parser.test.ts index 40b9d68..afbd78f 100644 --- a/src/__tests__/parser.test.ts +++ b/src/__tests__/parser.test.ts @@ -433,3 +433,31 @@ describe('parse (JSON string input)', () => { expect(sbom.components[0].name).toBe('ab'); }); }); + +describe('parse (input validation, issue #21)', () => { + it('throws ParseError on a non-SBOM object (e.g. a package.json)', () => { + const notAnSbom = JSON.stringify({ name: 'my-app', dependencies: { lodash: '^4.17.21' } }); + expect(() => parse(notAnSbom)).toThrow(/not a recognized SBOM/); + }); + + it('throws ParseError on invalid JSON', () => { + expect(() => parse('{not json')).toThrow(/not valid JSON/); + }); + + it('throws ParseError on a JSON array', () => { + expect(() => parse('[1, 2, 3]')).toThrow(/not an SBOM document/); + }); + + it('throws ParseError on null input', () => { + expect(() => parse('null')).toThrow(/not an SBOM document/); + }); + + it('throws ParseError on an object passed directly (not a string)', () => { + expect(() => parse({ name: 'my-app', dependencies: {} })).toThrow(/not a recognized SBOM/); + }); + + it('still accepts valid CycloneDX and SPDX documents', () => { + expect(parse(JSON.stringify(cyclonedxFixture)).format).toBe('cyclonedx'); + expect(parse({ spdxVersion: 'SPDX-2.3', packages: [] }).format).toBe('spdx'); + }); +}); diff --git a/src/parser.ts b/src/parser.ts index b0c20ef..a0ba3e7 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -104,23 +104,57 @@ export function parseSPDX(obj: Record): SBOM { }; } +/** + * Thrown when parse() is given input that is not a recognized SBOM document. + * The message explains what was expected so a wrong-format file (package.json, + * a truncated export, garbage JSON) fails loudly instead of silently passing + * a CI gate with "nothing changed". + */ +export class ParseError extends Error { + constructor(message: string) { + super(message); + this.name = 'ParseError'; + } +} + /** * Parse a JSON string or object into an SBOM, auto-detecting format. + * + * Throws ParseError when the input is not a recognized CycloneDX or SPDX + * document. Silently accepting wrong-format input as an empty SBOM is a + * false-negative: a corrupt export or a package.json passed by mistake would + * sail through a CI gate as if nothing changed (issue #21). */ export function parse(input: string | Record): SBOM { - // Strip a leading UTF-8 byte order mark (U+FEFF) before parsing. Several SBOM - // generators and Windows text tooling emit BOM-prefixed JSON, which is valid - // on disk but makes JSON.parse throw a cryptic "Unexpected token" error. - const obj: Record = - typeof input === 'string' ? JSON.parse(input.replace(/^\uFEFF/, '')) : input; - const format = detectFormat(obj); + let obj: unknown; + if (typeof input === 'string') { + try { + // Strip a leading UTF-8 byte order mark (U+FEFF) before parsing. Several + // SBOM generators and Windows text tooling emit BOM-prefixed JSON, which + // is valid on disk but makes JSON.parse throw a cryptic "Unexpected + // token" error. + obj = JSON.parse(input.replace(/^\uFEFF/, '')); + } catch (e) { + throw new ParseError(`input is not valid JSON: ${(e as Error).message}`); + } + } else { + obj = input; + } + + if (obj === null || typeof obj !== 'object' || Array.isArray(obj)) { + throw new ParseError('input is not an SBOM document: expected a JSON object with bomFormat or spdxVersion'); + } + + const record = obj as Record; + const format = detectFormat(record); switch (format) { - case 'cyclonedx': return parseCycloneDX(obj); - case 'spdx': return parseSPDX(obj); + case 'cyclonedx': return parseCycloneDX(record); + case 'spdx': return parseSPDX(record); default: - // Best-effort: treat as CycloneDX-like - return parseCycloneDX(obj); + throw new ParseError( + 'input is not a recognized SBOM: missing CycloneDX "bomFormat" field and SPDX "spdxVersion" field' + ); } } From 0e23a5a9fb0d8929760319c8bb88b5e3be931aef Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Fri, 7 Aug 2026 13:31:27 -0600 Subject: [PATCH 2/2] =?UTF-8?q?feat:=20detect=20component=20hash=20changes?= =?UTF-8?q?=20=E2=80=94=20supply-chain=20tampering=20signal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #22 Component.hashes was declared but never parsed, compared, or rendered — a dependency re-published under the same name@version (event-stream / xz class attack) produced zero output. - Parser: extract CycloneDX hashes[] (alg/content) and SPDX checksums[] (algorithm/checksumValue) into a normalized {algorithm: digest} map - diff(): report HashChange when a component's version is unchanged but a digest differs; a version bump is already reported as an upgrade so its hash change is not double-counted - Reporter: Hash changes section + summary metric in text and markdown - 4 new tests (parser both formats, diff detection, upgrade-exclusion) 110 tests pass, tsc clean. --- src/__tests__/cli.test.ts | 2 ++ src/__tests__/diff.test.ts | 33 +++++++++++++++++++++++++++++ src/__tests__/parser.test.ts | 30 +++++++++++++++++++++++++++ src/__tests__/reporter.test.ts | 12 +++++++---- src/diff.ts | 21 ++++++++++++++++++- src/parser.ts | 38 ++++++++++++++++++++++++++++++++++ src/reporter.ts | 17 +++++++++++++++ src/types.ts | 20 ++++++++++++++++++ 8 files changed, 168 insertions(+), 5 deletions(-) diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index ebb6ee8..351a78d 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -86,6 +86,7 @@ describe('gateFailures', () => { newCVEs, fixedCVEs: [], severityEscalations: [], + hashChanges: [], summary: { totalAdded: 0, totalRemoved: 0, @@ -95,6 +96,7 @@ describe('gateFailures', () => { totalNewCVEs: newCVEs.length, totalFixedCVEs: 0, totalSeverityEscalations: 0, + totalHashChanges: 0, }, }); diff --git a/src/__tests__/diff.test.ts b/src/__tests__/diff.test.ts index 0d3538e..82a72ee 100644 --- a/src/__tests__/diff.test.ts +++ b/src/__tests__/diff.test.ts @@ -274,4 +274,37 @@ describe('diff ordering', () => { expect(report.severityEscalations).toHaveLength(1); expect(report.severityEscalations[0].toScore).toBe(9.0); }); + + it('detects a hash change on an unchanged version (issue #22)', () => { + const a = makesbom([ + { name: 'event-stream', version: '4.0.1', purl: 'pkg:npm/event-stream@4.0.1', hashes: { sha256: 'aaaa' } }, + { name: 'clean-pkg', version: '1.0.0', purl: 'pkg:npm/clean-pkg@1.0.0', hashes: { sha256: 'bbbb' } }, + ]); + const b = makesbom([ + // Same version, different digest — the tampering signal. + { name: 'event-stream', version: '4.0.1', purl: 'pkg:npm/event-stream@4.0.1', hashes: { sha256: 'cccc' } }, + { name: 'clean-pkg', version: '1.0.0', purl: 'pkg:npm/clean-pkg@1.0.0', hashes: { sha256: 'bbbb' } }, + ]); + const report = diff(a, b); + expect(report.hashChanges).toHaveLength(1); + expect(report.hashChanges[0].component.name).toBe('event-stream'); + expect(report.hashChanges[0].algorithm).toBe('sha256'); + expect(report.hashChanges[0].from).toBe('aaaa'); + expect(report.hashChanges[0].to).toBe('cccc'); + expect(report.summary.totalHashChanges).toBe(1); + // The unchanged component must not appear. + expect(report.upgraded).toHaveLength(0); + }); + + it('does not report a hash change when the version also changed (already an upgrade)', () => { + const a = makesbom([ + { name: 'pkg', version: '1.0.0', purl: 'pkg:npm/pkg@1.0.0', hashes: { sha256: 'aaaa' } }, + ]); + const b = makesbom([ + { name: 'pkg', version: '1.0.1', purl: 'pkg:npm/pkg@1.0.1', hashes: { sha256: 'cccc' } }, + ]); + const report = diff(a, b); + expect(report.upgraded).toHaveLength(1); + expect(report.hashChanges).toHaveLength(0); + }); }); diff --git a/src/__tests__/parser.test.ts b/src/__tests__/parser.test.ts index afbd78f..b92c7a1 100644 --- a/src/__tests__/parser.test.ts +++ b/src/__tests__/parser.test.ts @@ -61,6 +61,20 @@ describe('parse (CycloneDX)', () => { expect(sbom.components[0].ecosystem).toBe('npm'); }); + it('extracts component hashes from the CycloneDX hashes array (issue #22)', () => { + const sbom = parse({ + bomFormat: 'CycloneDX', + specVersion: '1.4', + components: [ + { name: 'lodash', version: '4.17.21', hashes: [{ alg: 'SHA-256', content: 'ABCDEF' }, { alg: 'SHA-1', content: '123456' }] }, + { name: 'no-hashes', version: '1.0.0' }, + ], + }); + // Algorithms and digests are normalized to lowercase for comparison. + expect(sbom.components[0].hashes).toEqual({ 'sha-256': 'abcdef', 'sha-1': '123456' }); + expect(sbom.components[1].hashes).toBeUndefined(); + }); + it('parses vulnerabilities', () => { const sbom = parse(cyclonedxFixture); expect(sbom.vulnerabilities).toHaveLength(1); @@ -286,6 +300,22 @@ describe('parse (SPDX)', () => { expect(sbom.components[0].license).toBe('Apache-2.0'); }); + it('extracts package checksums from the SPDX checksums array (issue #22)', () => { + const sbom = parse({ + spdxVersion: 'SPDX-2.3', + name: 'my-service', + packages: [ + { + name: 'requests', + SPDXID: 'SPDXRef-requests', + versionInfo: '2.28.0', + checksums: [{ algorithm: 'SHA256', checksumValue: 'ABCDEF' }], + }, + ], + }); + expect(sbom.components[0].hashes).toEqual({ sha256: 'abcdef' }); + }); + it('treats NOASSERTION/NONE version and supplier sentinels as undefined', () => { const sbom = parse({ spdxVersion: 'SPDX-2.3', diff --git a/src/__tests__/reporter.test.ts b/src/__tests__/reporter.test.ts index 8d235c4..e047609 100644 --- a/src/__tests__/reporter.test.ts +++ b/src/__tests__/reporter.test.ts @@ -12,7 +12,8 @@ const sampleReport: ChangeReport = { newCVEs: [{ id: 'CVE-2023-1234', affects: 'pkg:npm/foo@1.0.0', severity: 'high' }], fixedCVEs: [{ id: 'CVE-2022-9999', affects: 'pkg:npm/bar@0.9.0' }], severityEscalations: [], - summary: { totalAdded: 1, totalRemoved: 1, totalUpgraded: 1, totalLicenseChanges: 1, totalDowngraded: 0, totalNewCVEs: 1, totalFixedCVEs: 1, totalSeverityEscalations: 0 }, + hashChanges: [], + summary: { totalAdded: 1, totalRemoved: 1, totalUpgraded: 1, totalLicenseChanges: 1, totalDowngraded: 0, totalNewCVEs: 1, totalFixedCVEs: 1, totalSeverityEscalations: 0, totalHashChanges: 0 }, }; describe('renderReport', () => { @@ -69,7 +70,8 @@ it('escapes pipes and newlines in markdown cells so the table stays well-formed' newCVEs: [{ id: 'CVE-2024-0001', affects: 'pkg:npm/a | b', severity: 'high', description: 'line1\nline2' }], fixedCVEs: [], severityEscalations: [], - summary: { totalAdded: 1, totalRemoved: 0, totalUpgraded: 0, totalLicenseChanges: 0, totalDowngraded: 0, totalNewCVEs: 1, totalFixedCVEs: 0, totalSeverityEscalations: 0 }, + hashChanges: [], + summary: { totalAdded: 1, totalRemoved: 0, totalUpgraded: 0, totalLicenseChanges: 0, totalDowngraded: 0, totalNewCVEs: 1, totalFixedCVEs: 0, totalSeverityEscalations: 0, totalHashChanges: 0 }, }; const out = renderReport(report, 'markdown'); @@ -96,7 +98,8 @@ it('escapes pipes and newlines in markdown cells so the table stays well-formed' newCVEs: [], fixedCVEs: [], severityEscalations: [], - summary: { totalAdded: 0, totalRemoved: 0, totalUpgraded: 1, totalLicenseChanges: 0, totalDowngraded: 1, totalNewCVEs: 0, totalFixedCVEs: 0, totalSeverityEscalations: 0 }, + hashChanges: [], + summary: { totalAdded: 0, totalRemoved: 0, totalUpgraded: 1, totalLicenseChanges: 0, totalDowngraded: 1, totalNewCVEs: 0, totalFixedCVEs: 0, totalSeverityEscalations: 0, totalHashChanges: 0 }, }; const out = renderReport(report, 'text'); expect(out).toContain('Downgraded: 1'); @@ -120,7 +123,8 @@ it('escapes pipes and newlines in markdown cells so the table stays well-formed' newCVEs: [], fixedCVEs: [], severityEscalations: [], - summary: { totalAdded: 0, totalRemoved: 0, totalUpgraded: 1, totalLicenseChanges: 0, totalDowngraded: 1, totalNewCVEs: 0, totalFixedCVEs: 0, totalSeverityEscalations: 0 }, + hashChanges: [], + summary: { totalAdded: 0, totalRemoved: 0, totalUpgraded: 1, totalLicenseChanges: 0, totalDowngraded: 1, totalNewCVEs: 0, totalFixedCVEs: 0, totalSeverityEscalations: 0, totalHashChanges: 0 }, }; const out = renderReport(report, 'markdown'); expect(out).toContain('Downgraded Components'); diff --git a/src/diff.ts b/src/diff.ts index 2c816d7..54555a1 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -1,4 +1,4 @@ -import type { SBOM, Component, CVEEntry, ChangeReport, VersionChange, LicenseChange, SBOMIdentity, SeverityEscalation } from './types.js'; +import type { SBOM, Component, CVEEntry, ChangeReport, VersionChange, LicenseChange, SBOMIdentity, SeverityEscalation, HashChange } from './types.js'; /** * Compare two parsed SBOMs and produce a ChangeReport. @@ -17,6 +17,7 @@ export function diff(a: SBOM, b: SBOM): ChangeReport { const licenseChanges: LicenseChange[] = []; // Find added, upgraded, and relicensed + const hashChanges: HashChange[] = []; for (const [key, bComp] of bMap) { const aComp = aMap.get(key); if (!aComp) { @@ -41,6 +42,22 @@ export function diff(a: SBOM, b: SBOM): ChangeReport { if (aComp.license && bComp.license && aComp.license !== bComp.license) { licenseChanges.push({ component: bComp, from: aComp.license, to: bComp.license }); } + + // Hash/integrity check: a component whose version is unchanged but whose + // digest changed is the supply-chain tampering signal (re-published / + // back-doored artifact under the same name@version). Only compare digests + // when the version did NOT change — a version bump legitimately changes + // hashes, and that's already reported as an upgrade. + if (aComp.version === bComp.version) { + const aHashes = aComp.hashes ?? {}; + const bHashes = bComp.hashes ?? {}; + for (const [alg, bHash] of Object.entries(bHashes)) { + const aHash = aHashes[alg]; + if (aHash !== undefined && aHash !== bHash) { + hashChanges.push({ component: bComp, algorithm: alg, from: aHash, to: bHash }); + } + } + } } // Find removed @@ -96,6 +113,7 @@ export function diff(a: SBOM, b: SBOM): ChangeReport { newCVEs, fixedCVEs, severityEscalations, + hashChanges, summary: { totalAdded: added.length, totalRemoved: removed.length, @@ -105,6 +123,7 @@ export function diff(a: SBOM, b: SBOM): ChangeReport { totalNewCVEs: newCVEs.length, totalFixedCVEs: fixedCVEs.length, totalSeverityEscalations: severityEscalations.length, + totalHashChanges: hashChanges.length, }, }; } diff --git a/src/parser.ts b/src/parser.ts index a0ba3e7..f158b8d 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -39,6 +39,7 @@ export function parseCycloneDX(obj: Record): SBOM { license: extractCycloneDXLicense(c), ecosystem: extractEcosystemFromPurl(typeof c.purl === 'string' ? c.purl : ''), supplier: extractCycloneDXSupplier(c), + hashes: extractCycloneDXHashes(c), })); const vulnerabilities: CVEEntry[] = rawVulns.map((v: Record) => { @@ -89,6 +90,7 @@ export function parseSPDX(obj: Record): SBOM { license: extractSPDXLicense(pkg), ecosystem: extractEcosystemFromPurl(purl ?? ''), supplier: normalizeSPDXValue(pkg.supplier), + hashes: extractSPDXChecksums(pkg), }; }); @@ -355,6 +357,42 @@ function normalizeSPDXValue(value: unknown): string | undefined { return trimmed; } +/** + * Extract CycloneDX component hashes (the `hashes` array of {alg, content} + * objects) into a `{ algorithm: value }` map. SHA-256 keys are lowercased to + * the standard form so digests from different generators compare equal. + */ +function extractCycloneDXHashes(c: Record): Record | undefined { + if (!Array.isArray(c.hashes) || c.hashes.length === 0) return undefined; + const hashes: Record = {}; + for (const h of c.hashes) { + if (typeof h !== 'object' || h === null) continue; + const entry = h as Record; + if (typeof entry.alg === 'string' && typeof entry.content === 'string') { + hashes[entry.alg.toLowerCase()] = entry.content.toLowerCase(); + } + } + return Object.keys(hashes).length > 0 ? hashes : undefined; +} + +/** + * Extract SPDX package checksums (the `checksums` array of {algorithm, + * checksumValue} objects) into a `{ algorithm: value }` map, same shape as the + * CycloneDX extraction so diff() can compare them uniformly. + */ +function extractSPDXChecksums(pkg: Record): Record | undefined { + if (!Array.isArray(pkg.checksums) || pkg.checksums.length === 0) return undefined; + const hashes: Record = {}; + for (const cs of pkg.checksums) { + if (typeof cs !== 'object' || cs === null) continue; + const entry = cs as Record; + if (typeof entry.algorithm === 'string' && typeof entry.checksumValue === 'string') { + hashes[entry.algorithm.toLowerCase()] = entry.checksumValue.toLowerCase(); + } + } + return Object.keys(hashes).length > 0 ? hashes : undefined; +} + /** * Collect the SPDXIDs of the package(s) the document describes (its subject). * diff --git a/src/reporter.ts b/src/reporter.ts index 47af4da..34d0294 100644 --- a/src/reporter.ts +++ b/src/reporter.ts @@ -54,6 +54,7 @@ function renderText(r: ChangeReport): string { lines.push(` Licenses: ${r.summary.totalLicenseChanges}`); lines.push(` New CVEs: ${r.summary.totalNewCVEs}`); lines.push(` Fixed CVEs: ${r.summary.totalFixedCVEs}`); + lines.push(` Hash changes: ${r.summary.totalHashChanges}`); lines.push(''); if (r.added.length > 0) { @@ -116,6 +117,13 @@ function renderText(r: ChangeReport): string { } lines.push(''); } + if (r.hashChanges.length > 0) { + lines.push('\u26a0 Hash Changes (potential supply-chain tampering):'); + for (const hc of r.hashChanges) { + lines.push(` \u26a0 ${hc.component.name}@${hc.component.version} [${hc.algorithm}: ${hc.from} \u2192 ${hc.to}]`); + } + lines.push(''); + } return lines.join('\n'); } @@ -158,6 +166,7 @@ function renderMarkdown(r: ChangeReport): string { `| New CVEs | ${r.summary.totalNewCVEs} |`, `| Fixed CVEs | ${r.summary.totalFixedCVEs} |`, `| Severity escalations | ${r.summary.totalSeverityEscalations} |`, + `| Hash changes | ${r.summary.totalHashChanges} |`, '', ]; @@ -226,6 +235,14 @@ lines.push('| CVE ID | Severity | CVSS | Affects |'); lines.push(`| ${escapeCell(e.cve.id)} | ${escapeCell(e.fromSeverity ?? 'none')} | ${escapeCell(e.toSeverity ?? 'none')} | ${escapeCell(e.fromScore !== undefined && e.toScore !== undefined ? `${e.fromScore} \u2192 ${e.toScore}` : undefined)} | ${escapeCell(e.cve.affects)} |`); } } + if (r.hashChanges.length > 0) { + lines.push('## \u26a0\ufe0f Hash Changes (supply-chain tampering)', ''); + lines.push('| Component | Version | Algorithm | From | To |'); + lines.push('|-----------|---------|-----------|------|----|'); + for (const hc of r.hashChanges) { + lines.push(`| ${escapeCell(hc.component.name)} | ${escapeCell(hc.component.version)} | ${escapeCell(hc.algorithm)} | \`${escapeCell(hc.from)}\` | \`${escapeCell(hc.to)}\` |`); + } + } return lines.join('\n'); } diff --git a/src/types.ts b/src/types.ts index e1297e9..485236c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -99,6 +99,22 @@ export interface SeverityEscalation { toScore?: number; } +/** + * A component whose version is unchanged but whose digest changed between the + * two SBOMs — the canonical supply-chain tampering signal (a re-published / + * back-doored artifact under the same name@version; the event-stream / xz + * class of attack). + */ +export interface HashChange { + component: Component; + /** Algorithm whose digest changed (e.g. "sha256") */ + algorithm: string; + /** Digest in the old SBOM */ + from: string; + /** Digest in the new SBOM */ + to: string; +} + /** * Minimal carried-forward identity of one of the two SBOMs in a diff. Lets the * report state which artifacts it was produced from (issue #52). @@ -143,6 +159,8 @@ export interface ChangeReport { * newCVEs and fixedCVEs buckets, so without this they'd be invisible. */ severityEscalations: SeverityEscalation[]; + /** Components whose version is unchanged but whose digest changed (issue #22) */ + hashChanges: HashChange[]; summary: { totalAdded: number; totalRemoved: number; @@ -154,6 +172,8 @@ export interface ChangeReport { totalFixedCVEs: number; /** Number of re-scored CVEs (issue #46) */ totalSeverityEscalations: number; + /** Number of components with changed digests (issue #22) */ + totalHashChanges: number; }; }