diff --git a/CHANGELOG.md b/CHANGELOG.md index 034675d..b7ab0ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,17 @@ backward compatible. ### Fixed +- **SBOM component order no longer follows scan or filesystem order.** + `vg sbom export` and CycloneDX/SPDX graph export sort a component by its + Package URL when one is written, otherwise by package name, then by version. + Rows that share a Package URL and a version are ordered by name. CycloneDX + `components`, SPDX `packages`, and the package entries in CycloneDX + `dependencies` share that order, and `SPDXRef-Package-N` follows it. The + same artifact and the same lockfiles produce the same document on every run. + Reordering projects still changes which project's metadata is kept, and + therefore the document id, but it does not change component order or SPDX + IDs. (#286) + - **`vg report --format` rejects values other than `md`, `text`, and `json`.** An unknown value, including `html`, exits `5` with a usage error that names the value and lists the valid ones. Stdout is empty. `text` stays the diff --git a/DOCS.md b/DOCS.md index f39554a..b6b753c 100644 --- a/DOCS.md +++ b/DOCS.md @@ -1624,7 +1624,7 @@ Use this to treat SBOMs as operational intelligence instead of static compliance `vg sbom export` writes one JSON document. `--format` accepts `cyclonedx` or `spdx`, compared without regard to case. The default is `cyclonedx`. Any other value prints `Invalid SBOM format. Use cyclonedx or spdx.` and the process exits 1. -Both formats are built from the same scan artifact and the same lockfiles under `--root`. The component list and its order are the same. Each specification then places those facts in its own fields. +Both formats are built from the same scan artifact and the same lockfiles under `--root`. The component list and its order are the same: Package URL when the component has one, otherwise the package name, then the version. Each specification then places those facts in its own fields. ```bash vg scan --offline @@ -1642,7 +1642,7 @@ vg sbom export --format spdx --out sbom.spdx.json | Created | `metadata.timestamp` is the artifact's `timestamp` | `creationInfo.created` is that same timestamp | | Scan root | `metadata.component` has `type` `application`, `bom-ref` `vibgrate-root`, and `name` set to the scan root. That component is not copied into `components` | There is no package for the scan root | | Package identity | `components[].purl`. When a purl is written, `bom-ref` is that purl | `packages[].externalRefs[]` with `referenceCategory` `PACKAGE-MANAGER`, `referenceType` `purl`, and `referenceLocator` set to the purl. `SPDXID` is `SPDXRef-Package-N` | -| Dependency graph | `dependencies` is an array of `{ ref, dependsOn }`. `ref` is a `bom-ref`. The first entry is `vibgrate-root`. Every component is an entry. `dependsOn` lists child `bom-ref` values, or is `[]` when none were recorded for that component | `relationships` is an array of `{ spdxElementId, relatedSpdxElementId, relationshipType }`. `relationshipType` is `DEPENDS_ON`. Edges from the document use `SPDXRef-DOCUMENT`. A row is written only for a recorded edge | +| Dependency graph | `dependencies` is an array of `{ ref, dependsOn }`. `ref` is a `bom-ref`. The first entry is `vibgrate-root`. Every component is an entry after that, in the component order above. `dependsOn` lists child `bom-ref` values, or is `[]` when none were recorded for that component | `relationships` is an array of `{ spdxElementId, relatedSpdxElementId, relationshipType }`. `relationshipType` is `DEPENDS_ON`. Edges from the document use `SPDXRef-DOCUMENT`. A row is written only for a recorded edge. `SPDXID` values follow the component order above | | Licenses | `licenses` is present when the declared license can be written, and absent when it cannot | `licenseDeclared` is set on every package. `licenseConcluded` is `NOASSERTION` on every package. `hasExtractedLicensingInfos` is present when a `LicenseRef-…` is used. Every package has `downloadLocation` `NOASSERTION` and `filesAnalyzed` `false` | `dataLicense` `CC0-1.0` is the SPDX license for the document data. Package licenses stay on `licenseDeclared`. @@ -1763,9 +1763,9 @@ Two other strings in the same file are easy to misread as package digests. `vcs. | `uv.lock` | `hash` | | `go.sum` | `h1:` | -**Several digests, one component.** A lockfile can list more than one digest for one package. The export still writes one component for that ecosystem, name, and version. It does not add a row per digest. `hashes` and `checksums` are omitted, so there is no digest array and no digest order to keep stable. Component order stays the order in [Several versions of one package](#several-versions-of-one-package): direct rows follow the scan artifact, then lockfile-only rows sorted by package name, then version, then ecosystem. Digest text is not part of that sort, and it is not part of the document id. Exporting the same scan artifact again, after a lockfile edit that changes only those digest strings and leaves names, versions, and edges alone, writes the same JSON, including `serialNumber` and `documentNamespace`. +**Several digests, one component.** A lockfile can list more than one digest for one package. The export still writes one component for that ecosystem, name, and version. It does not add a row per digest. `hashes` and `checksums` are omitted, so there is no digest array and no digest order to keep stable. Component order stays the order in [Several versions of one package](#several-versions-of-one-package): Package URL when the component has one, otherwise the package name, then the version. Digest text is not part of that sort, and it is not part of the document id. Exporting the same scan artifact again, after a lockfile edit that changes only those digest strings and leaves names, versions, and edges alone, writes the same JSON, including `serialNumber` and `documentNamespace`. -`go.sum` lists a module twice: ` h1:…` and ` /go.mod h1:…`. The `/go.mod` line is not a second component. Both `h1:` values are dropped. A `uv.lock` package block can carry more than one `hash`. Those values are dropped, and the block stays one component, sorted with the others by name and version. An npm `integrity` string and a pnpm `resolution.integrity` string are not read, including when the string names more than one algorithm. +`go.sum` lists a module twice: ` h1:…` and ` /go.mod h1:…`. The `/go.mod` line is not a second component. Both `h1:` values are dropped. A `uv.lock` package block can carry more than one `hash`. Those values are dropped, and the block stays one component, in the same order as the other rows. An npm `integrity` string and a pnpm `resolution.integrity` string are not read, including when the string names more than one algorithm. ```bash vg scan --offline --no-graph --format json --out scan.json @@ -1882,13 +1882,17 @@ absent, and so is the dependency graph. A consumer that filters to `vibgrate:scope=direct` sees the same gap: other installed versions of that package are still in the full document, marked `transitive`. -**Order.** Direct rows follow `projects` on the scan artifact, and within a -project they follow that project's `dependencies` array. The npm scanner sorts -each project's array by drift, then by package name, before it writes the -artifact. Lockfile-only rows are appended after the direct rows, sorted by -package name, then by version, then by ecosystem. That combined list is the order of CycloneDX -`components`, CycloneDX `dependencies`, and SPDX `packages`. `dependsOn` entries -and the names inside one lockfile edge are sorted on their own. +**Order.** CycloneDX `components`, SPDX `packages`, and the package entries in +CycloneDX `dependencies` share one order. A component with a Package URL sorts +by that purl. A component without one sorts by package name. Version is the +next key. When two components share a purl and a version, package name orders +them (`Flask` before `flask`). `dependencies` starts with `vibgrate-root`, then +those components. The order is the same on every run of the same artifact and +the same lockfiles. It is independent of project order in the scan artifact, +directory walk order, and lockfile map order. `dependsOn` entries and the names +inside one lockfile edge are sorted on their own. SPDX `SPDXID` values are +`SPDXRef-Package-N` for that order, so they stay put when only discovery order +changes. The document id stays a content-derived UUID. **Same inputs, same document.** For one scan artifact and the lockfiles under `--root`, `vg sbom export` writes the same JSON on every run, including the @@ -1903,13 +1907,14 @@ are in [Package digests](#package-digests). **Known limitations:** -- Direct-row order, and which project's metadata is kept for a shared - identity, follow the scan artifact. Reordering projects changes - `vibgrate:project` on that row, reassigns SPDX `SPDXID` values to match the - new positions, retargets SPDX `DEPENDS_ON` relationships (they point at - SPDX IDs), and changes the document serial number and namespace. CycloneDX `bom-ref` stays on the purl, - so a scanner that stored the purl still matches. `vibgrate:projects` stays - the sorted set of contributing projects. +- Which project's metadata is kept for a shared identity follows the scan + artifact. Reordering projects changes `vibgrate:project` on that row and + changes the document serial number and namespace, because the kept project + is part of the document id. Component order and SPDX `SPDXID` values stay + on the Package URL, or on the name and version when there is no purl, so + those identifiers do not move when only project order changes. CycloneDX + `bom-ref` stays on the purl, so a scanner that stored the purl still + matches. `vibgrate:projects` stays the sorted set of contributing projects. - npm `package-lock.json` v2/v3 collapses two install paths of the same `name@version` into one component. When those paths declare different dependencies, the edge list is the path that appears last in the lockfile diff --git a/docs/sbom-dependency-scope.md b/docs/sbom-dependency-scope.md index 625275c..a912e3b 100644 --- a/docs/sbom-dependency-scope.md +++ b/docs/sbom-dependency-scope.md @@ -78,16 +78,16 @@ scope-fixture └── dev-optional-pkg@1.2.3 lockfile flags dev, optional, and devOptional ``` -Component order follows the scan's dependency array (drift, then package name) and then lockfile-only rows (package name, then version). With every row at drift `unknown`, the names sort alphabetically inside each group. +Component order is the Package URL, then the version. Every row in this tree has a purl, so the names sort alphabetically and then by version. CycloneDX `dependencies` still starts with `vibgrate-root`. | Package | `vibgrate:scope` | CycloneDX component `scope` | Root `dependsOn` | SPDX annotation | SPDX relationship | | --- | --- | --- | --- | --- | --- | -| `fsevents@2.3.3` | `direct` | omitted | yes (`pkg:npm/fsevents@2.3.3`) | `scope=direct` on `SPDXRef-Package-1` | `DEPENDS_ON` from `SPDXRef-DOCUMENT` | -| `left-pad@1.3.0` | `direct` | omitted | yes | `scope=direct` on `SPDXRef-Package-2` | `DEPENDS_ON` from `SPDXRef-DOCUMENT`. This package `DEPENDS_ON` `SPDXRef-Package-6` (`nested-prod`) | -| `typescript@5.4.5` | `direct` | omitted | no | `scope=direct` on `SPDXRef-Package-3` | No document relationship. This package `DEPENDS_ON` `SPDXRef-Package-5` (`nested-dev`) | -| `dev-optional-pkg@1.2.3` | `transitive` | omitted | no | `scope=transitive` on `SPDXRef-Package-4` | none | -| `nested-dev@1.0.0` | `transitive` | omitted | no | `scope=transitive` on `SPDXRef-Package-5` | target of `typescript` only | -| `nested-prod@1.0.0` | `transitive` | omitted | no | `scope=transitive` on `SPDXRef-Package-6` | target of `left-pad` only | +| `dev-optional-pkg@1.2.3` | `transitive` | omitted | no | `scope=transitive` on `SPDXRef-Package-1` | none | +| `fsevents@2.3.3` | `direct` | omitted | yes (`pkg:npm/fsevents@2.3.3`) | `scope=direct` on `SPDXRef-Package-2` | `DEPENDS_ON` from `SPDXRef-DOCUMENT` | +| `left-pad@1.3.0` | `direct` | omitted | yes | `scope=direct` on `SPDXRef-Package-3` | `DEPENDS_ON` from `SPDXRef-DOCUMENT`. This package `DEPENDS_ON` `SPDXRef-Package-5` (`nested-prod`) | +| `nested-dev@1.0.0` | `transitive` | omitted | no | `scope=transitive` on `SPDXRef-Package-4` | target of `typescript` only | +| `nested-prod@1.0.0` | `transitive` | omitted | no | `scope=transitive` on `SPDXRef-Package-5` | target of `left-pad` only | +| `typescript@5.4.5` | `direct` | omitted | no | `scope=direct` on `SPDXRef-Package-6` | No document relationship. This package `DEPENDS_ON` `SPDXRef-Package-4` (`nested-dev`) | A CycloneDX component for `typescript` has `type`, `bom-ref`, `name`, `version`, `purl`, and `properties`. The component `scope` member is absent. `properties` includes `vibgrate:scope` = `direct`. The root `dependencies` entry (`bom-ref` `vibgrate-root`) lists `fsevents` and `left-pad`. `typescript` has its own `dependsOn` entry pointing at `nested-dev`. diff --git a/src/engine/export.test.ts b/src/engine/export.test.ts index 499dba5..f6fed50 100644 --- a/src/engine/export.test.ts +++ b/src/engine/export.test.ts @@ -148,4 +148,37 @@ describe('cyclonedx export purl', () => { // Non-npm rows still carry no guessed npm purl. expect(bom.components.find((c) => c.name === 'requests')?.purl).toBeUndefined(); }); + + it('two runs with reversed inputs emit components in purl, then name, then version order', () => { + const base = ctx(makeGraph(false)); + const deps = [ + { name: 'zzz', ecosystem: 'npm' as const, declared: '1.0.0', installed: '1.0.0' }, + { name: 'foo bar', ecosystem: 'npm' as const, declared: '1.0.0', installed: '1.0.0' }, + { name: 'aaa', ecosystem: 'npm' as const, declared: '2.0.0', installed: '2.0.0' }, + { name: 'requests', ecosystem: 'pypi' as const, declared: '2.31.0', installed: '2.31.0' }, + ]; + const models = [ + { runtime: 'ollama' as const, name: 'llama3:latest', path: '/x' }, + { runtime: 'ollama' as const, name: 'aaa-model', path: '/y' }, + ]; + const first = exportGraph('cyclonedx', { ...base, deps, models }); + const second = exportGraph('cyclonedx', { + ...base, + deps: [...deps].reverse(), + models: [...models].reverse(), + }); + expect(second).toBe(first); + const bom = JSON.parse(first) as { components: Array<{ name: string; purl?: string }> }; + expect(bom.components.map((c) => c.name)).toEqual(['aaa-model', 'foo bar', 'llama3:latest', 'aaa', 'zzz', 'requests']); + expect(bom.components.find((c) => c.name === 'aaa')?.purl).toBe('pkg:npm/aaa@2.0.0'); + expect(bom.components.find((c) => c.name === 'foo bar')?.purl).toBeUndefined(); + + const spdxFirst = exportGraph('spdx', { ...base, deps }); + const spdxSecond = exportGraph('spdx', { ...base, deps: [...deps].reverse() }); + expect(spdxSecond).toBe(spdxFirst); + const spdx = JSON.parse(spdxFirst) as { packages: Array<{ name: string; SPDXID: string }> }; + expect(spdx.packages.map((p) => p.name)).toEqual(['foo bar', 'aaa', 'zzz', 'requests']); + expect(spdx.packages.find((p) => p.name === 'aaa')?.SPDXID).toBe('SPDXRef-Package-aaa'); + expect(spdx.packages.find((p) => p.name === 'zzz')?.SPDXID).toBe('SPDXRef-Package-zzz'); + }); }); diff --git a/src/engine/export.ts b/src/engine/export.ts index f6ac8d5..ac9cd0c 100644 --- a/src/engine/export.ts +++ b/src/engine/export.ts @@ -2,7 +2,7 @@ import { serializeGraph, slimGraphForExport } from './serialize.js'; import { renderReport } from './report.js'; import { renderHtml } from './html.js'; import type { DepRecord } from './drift.js'; -import { resolvePurl } from '../reporting/commands/sbom.js'; +import { compareSbomOrder, resolvePurl } from '../reporting/commands/sbom.js'; import type { LocalModel } from './models.js'; import type { VgGraph } from '../schema.js'; @@ -242,11 +242,20 @@ function sqlBool(v: boolean | null | undefined): string { return v == null ? 'NULL' : v ? '1' : '0'; } +interface SbomLibraryComponent { + type: string; + name: string; + version?: string; + purl?: string; + properties?: Array<{ name: string; value: string | null }>; +} + function cyclonedx(ctx: ExportContext): string { // CycloneDX 1.6 JSON — dependencies as library components + local models as - // machine-learning-model components (AI-BOM). Deterministic ordering; no + // machine-learning-model components (AI-BOM). Component order is the Package + // URL when one is emitted, otherwise the name, then the version. No // timestamps beyond the pinned generatedAt. - const components: unknown[] = []; + const components: SbomLibraryComponent[] = []; for (const d of ctx.deps ?? []) { const version = d.installed ?? d.declared; if (d.ecosystem !== 'npm') { @@ -273,6 +282,7 @@ function cyclonedx(ctx: ExportContext): string { for (const m of ctx.models ?? []) { components.push({ type: 'machine-learning-model', name: m.name, properties: [{ name: 'vg:runtime', value: m.runtime }] }); } + components.sort((a, b) => compareSbomOrder({ purl: a.purl, name: a.name, version: a.version }, { purl: b.purl, name: b.name, version: b.version })); const bom = { bomFormat: 'CycloneDX', specVersion: '1.6', @@ -282,13 +292,26 @@ function cyclonedx(ctx: ExportContext): string { return JSON.stringify(bom, null, 2) + '\n'; } +/** Purl this exporter actually writes for a dependency. Non-npm rows omit it. */ +function emittedExportPurl(d: DepRecord): string | null { + if (d.ecosystem !== 'npm') return null; + return resolvePurl('npm', d.name, d.installed ?? '').purl; +} + function spdx(ctx: ExportContext): string { - const packages = (ctx.deps ?? []).map((d) => ({ - SPDXID: `SPDXRef-Package-${cypherLabel(d.name)}`, - name: d.name, - versionInfo: d.installed ?? d.declared, - downloadLocation: 'NOASSERTION', - })); + const packages = [...(ctx.deps ?? [])] + .sort((a, b) => + compareSbomOrder( + { purl: emittedExportPurl(a), name: a.name, version: a.installed ?? a.declared }, + { purl: emittedExportPurl(b), name: b.name, version: b.installed ?? b.declared }, + ), + ) + .map((d) => ({ + SPDXID: `SPDXRef-Package-${cypherLabel(d.name)}`, + name: d.name, + versionInfo: d.installed ?? d.declared, + downloadLocation: 'NOASSERTION', + })); const doc = { spdxVersion: 'SPDX-2.3', dataLicense: 'CC0-1.0', diff --git a/src/reporting/commands/sbom.test.ts b/src/reporting/commands/sbom.test.ts index 5e18bbe..4fd6a59 100644 --- a/src/reporting/commands/sbom.test.ts +++ b/src/reporting/commands/sbom.test.ts @@ -90,12 +90,12 @@ describe('sbom helpers', () => { expect(cdx.metadata.component.type).toBe('application'); expect(cdx.metadata.component.version).toBeUndefined(); expect(cdx.components.map((c) => ({ type: c.type, name: c.name }))).toEqual([ - { type: 'library', name: 'chalk' }, { type: 'library', name: 'library/alpine' }, + { type: 'library', name: 'chalk' }, ]); const spdx = toSpdx(artifact) as { packages: Array> }; - expect(spdx.packages.map((pkg) => pkg.name)).toEqual(['chalk', 'library/alpine']); + expect(spdx.packages.map((pkg) => pkg.name)).toEqual(['library/alpine', 'chalk']); for (const pkg of spdx.packages) { expect(pkg).not.toHaveProperty('primaryPackagePurpose'); } @@ -122,7 +122,7 @@ describe('sbom helpers', () => { const sbom = toSpdx(makeArtifact('5.3.0', 90), componentsOnlyGraph([{ package: 'ansi-styles', version: '6.2.1' }])) as { packages: Array<{ name: string }>; }; - expect(sbom.packages.map((p) => p.name)).toEqual(['chalk', 'ansi-styles']); + expect(sbom.packages.map((p) => p.name)).toEqual(['ansi-styles', 'chalk']); }); it('gives every CycloneDX component and the root a purl-based bom-ref, and a matching purl field', () => { @@ -158,8 +158,8 @@ describe('sbom helpers', () => { }; expect(sbom.dependencies).toEqual([ { ref: 'vibgrate-root', dependsOn: ['pkg:npm/chalk@5.3.0'] }, - { ref: 'pkg:npm/chalk@5.3.0', dependsOn: ['pkg:npm/ansi-styles@6.2.1'] }, { ref: 'pkg:npm/ansi-styles@6.2.1', dependsOn: [] }, + { ref: 'pkg:npm/chalk@5.3.0', dependsOn: ['pkg:npm/ansi-styles@6.2.1'] }, ]); }); @@ -176,8 +176,8 @@ describe('sbom helpers', () => { relationships: Array<{ spdxElementId: string; relatedSpdxElementId: string; relationshipType: string }>; }; expect(sbom.relationships).toEqual([ - { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-1', relationshipType: 'DEPENDS_ON' }, - { spdxElementId: 'SPDXRef-Package-1', relatedSpdxElementId: 'SPDXRef-Package-2', relationshipType: 'DEPENDS_ON' }, + { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-2', relationshipType: 'DEPENDS_ON' }, + { spdxElementId: 'SPDXRef-Package-2', relatedSpdxElementId: 'SPDXRef-Package-1', relationshipType: 'DEPENDS_ON' }, ]); }); @@ -306,8 +306,8 @@ describe('sbom helpers', () => { expect(again).not.toContain('%40scope/'); expect(again).not.toContain('%C3%A9'); - expect(cyclone.components.map((c) => c.name)).toEqual(['chalk', 'foo bar', '@scope/', 'café']); - expect(cyclone.components[0]!.purl).toBe('pkg:npm/chalk@5.3.0'); + expect(cyclone.components.map((c) => c.name)).toEqual(['@scope/', 'café', 'foo bar', 'chalk']); + expect(cyclone.components.find((c) => c.name === 'chalk')!.purl).toBe('pkg:npm/chalk@5.3.0'); for (const name of ['foo bar', '@scope/', 'café']) { const row = cyclone.components.find((c) => c.name === name)!; @@ -324,12 +324,12 @@ describe('sbom helpers', () => { const warnings = collectPurlWarnings(artifact); expect(warnings).toEqual([ - describeUnavailablePurl('npm', 'foo bar', '1.0.0'), describeUnavailablePurl('npm', '@scope/', '2.0.0'), describeUnavailablePurl('npm', 'café', '3.0.0'), + describeUnavailablePurl('npm', 'foo bar', '1.0.0'), ]); - expect(warnings[0]).toContain('whitespace or a non-ASCII character'); - expect(warnings[1]).toContain('empty path segment'); + expect(warnings[0]).toContain('empty path segment'); + expect(warnings[1]).toContain('whitespace or a non-ASCII character'); const spdx = toSpdx(artifact) as { packages: Array<{ @@ -342,7 +342,7 @@ describe('sbom helpers', () => { const bad = spdx.packages.find((p) => p.name === 'foo bar')!; expect(bad.externalRefs).toBeUndefined(); expect(bad.annotations[0]!.comment).toContain('purlStatus=unavailable'); - expect(bad.annotations[1]!.comment).toBe(warnings[0]); + expect(bad.annotations[1]!.comment).toBe(describeUnavailablePurl('npm', 'foo bar', '1.0.0')); expect(spdx.packages.find((p) => p.name === 'chalk')!.externalRefs?.[0]?.referenceLocator).toBe('pkg:npm/chalk@5.3.0'); }); @@ -366,8 +366,8 @@ describe('sbom helpers', () => { expect(badRef).toBe('vibgrate:npm:foo bar@1.0.0'); expect(sbom.dependencies).toEqual([ { ref: 'vibgrate-root', dependsOn: ['pkg:npm/chalk@5.3.0', badRef] }, - { ref: 'pkg:npm/chalk@5.3.0', dependsOn: [badRef] }, { ref: badRef, dependsOn: [] }, + { ref: 'pkg:npm/chalk@5.3.0', dependsOn: [badRef] }, ]); expect(JSON.stringify(sbom.dependencies)).not.toContain('pkg:npm/foo'); }); diff --git a/src/reporting/commands/sbom.ts b/src/reporting/commands/sbom.ts index 9c112ea..156a2c8 100644 --- a/src/reporting/commands/sbom.ts +++ b/src/reporting/commands/sbom.ts @@ -303,6 +303,37 @@ function componentBomRef(ecosystem: Ecosystem, name: string, version: string): s return purlFor(ecosystem, name, version) ?? `vibgrate:${ecosystem}:${name}@${version}`; } +/** Code-unit order, so the result does not depend on the process locale. */ +function cmpText(a: string, b: string): number { + if (a < b) return -1; + if (a > b) return 1; + return 0; +} + +/** + * Order for SBOM component and dependency rows. Primary key is the Package URL + * when one was emitted, otherwise the package name. Version is next. The name + * is the last key so two rows that share a purl and a version (PyPI `Flask` + * and `flask`) stay in a fixed order instead of discovery order. + */ +export function compareSbomOrder( + a: { purl?: string | null; name: string; version?: string | null }, + b: { purl?: string | null; name: string; version?: string | null }, +): number { + const aPrimary = a.purl ? a.purl : a.name; + const bPrimary = b.purl ? b.purl : b.name; + return cmpText(aPrimary, bPrimary) || cmpText(a.version ?? '', b.version ?? '') || cmpText(a.name, b.name); +} + +function compareFlattenedDependency(a: FlattenedDependency, b: FlattenedDependency): number { + return ( + compareSbomOrder( + { purl: purlFor(a.ecosystem, a.package, a.version), name: a.package, version: a.version }, + { purl: purlFor(b.ecosystem, b.package, b.version), name: b.package, version: b.version }, + ) || cmpText(a.ecosystem, b.ecosystem) + ); +} + function splitDependencyKey(key: string): { name: string; version: string } { const at = key.lastIndexOf('@'); return { name: key.slice(0, at), version: key.slice(at + 1) }; @@ -608,10 +639,9 @@ export function flattenDependencies( index.set(key, row); lockfileOnly.push(row); } - lockfileOnly.sort( - (a, b) => a.package.localeCompare(b.package) || a.version.localeCompare(b.version) || a.ecosystem.localeCompare(b.ecosystem), - ); - return [...rows, ...lockfileOnly]; + // Sort before any serializer, warning list, or document-id seed reads this + // array. Discovery order (project walk, lockfile map) must not leak. + return [...rows, ...lockfileOnly].sort(compareFlattenedDependency); } interface LockfileMergeEntry { diff --git a/test/sbom-duplicate-versions.test.ts b/test/sbom-duplicate-versions.test.ts index 2768be7..f3c5eac 100644 --- a/test/sbom-duplicate-versions.test.ts +++ b/test/sbom-duplicate-versions.test.ts @@ -145,12 +145,12 @@ describe('sbom export: several versions of one package', () => { project: scopeOf(c.properties, 'vibgrate:project'), })); expect(rows).toEqual([ - { name: 'left-pad', version: '1.3.0', bom: 'pkg:npm/left-pad@1.3.0', purl: 'pkg:npm/left-pad@1.3.0', scope: 'direct', project: 'shared-root' }, - { name: 'widget', version: '1.0.0', bom: 'pkg:npm/widget@1.0.0', purl: 'pkg:npm/widget@1.0.0', scope: 'direct', project: 'shared-root' }, - { name: 'once', version: '1.4.0', bom: 'pkg:npm/once@1.4.0', purl: 'pkg:npm/once@1.4.0', scope: 'direct', project: 'shared-extra' }, { name: 'left-pad', version: '1.2.0', bom: 'pkg:npm/left-pad@1.2.0', purl: 'pkg:npm/left-pad@1.2.0', scope: 'transitive', project: 'shared-root' }, + { name: 'left-pad', version: '1.3.0', bom: 'pkg:npm/left-pad@1.3.0', purl: 'pkg:npm/left-pad@1.3.0', scope: 'direct', project: 'shared-root' }, { name: 'ms', version: '2.1.3', bom: 'pkg:npm/ms@2.1.3', purl: 'pkg:npm/ms@2.1.3', scope: 'transitive', project: 'shared-extra' }, { name: 'once', version: '1.3.0', bom: 'pkg:npm/once@1.3.0', purl: 'pkg:npm/once@1.3.0', scope: 'transitive', project: 'shared-root' }, + { name: 'once', version: '1.4.0', bom: 'pkg:npm/once@1.4.0', purl: 'pkg:npm/once@1.4.0', scope: 'direct', project: 'shared-extra' }, + { name: 'widget', version: '1.0.0', bom: 'pkg:npm/widget@1.0.0', purl: 'pkg:npm/widget@1.0.0', scope: 'direct', project: 'shared-root' }, ]); expect(new Set(rows.map((r) => r.bom)).size).toBe(rows.length); @@ -158,12 +158,12 @@ describe('sbom export: several versions of one package', () => { // ms exists only in the nested lockfile, so the root graph leaves it with no edges. expect(cdx.dependencies).toEqual([ { ref: 'vibgrate-root', dependsOn: ['pkg:npm/left-pad@1.3.0', 'pkg:npm/widget@1.0.0'] }, - { ref: 'pkg:npm/left-pad@1.3.0', dependsOn: [] }, - { ref: 'pkg:npm/widget@1.0.0', dependsOn: ['pkg:npm/left-pad@1.2.0'] }, - { ref: 'pkg:npm/once@1.4.0', dependsOn: [] }, { ref: 'pkg:npm/left-pad@1.2.0', dependsOn: ['pkg:npm/once@1.3.0'] }, + { ref: 'pkg:npm/left-pad@1.3.0', dependsOn: [] }, { ref: 'pkg:npm/ms@2.1.3', dependsOn: [] }, { ref: 'pkg:npm/once@1.3.0', dependsOn: [] }, + { ref: 'pkg:npm/once@1.4.0', dependsOn: [] }, + { ref: 'pkg:npm/widget@1.0.0', dependsOn: ['pkg:npm/left-pad@1.2.0'] }, ]); const spdx = toSpdx(scan, graph) as { @@ -177,20 +177,20 @@ describe('sbom export: several versions of one package', () => { relationships: Array<{ spdxElementId: string; relatedSpdxElementId: string; relationshipType: string }>; }; expect(spdx.packages.map((p) => [p.SPDXID, p.name, p.versionInfo, p.externalRefs[0]!.referenceLocator])).toEqual([ - ['SPDXRef-Package-1', 'left-pad', '1.3.0', 'pkg:npm/left-pad@1.3.0'], - ['SPDXRef-Package-2', 'widget', '1.0.0', 'pkg:npm/widget@1.0.0'], - ['SPDXRef-Package-3', 'once', '1.4.0', 'pkg:npm/once@1.4.0'], - ['SPDXRef-Package-4', 'left-pad', '1.2.0', 'pkg:npm/left-pad@1.2.0'], - ['SPDXRef-Package-5', 'ms', '2.1.3', 'pkg:npm/ms@2.1.3'], - ['SPDXRef-Package-6', 'once', '1.3.0', 'pkg:npm/once@1.3.0'], + ['SPDXRef-Package-1', 'left-pad', '1.2.0', 'pkg:npm/left-pad@1.2.0'], + ['SPDXRef-Package-2', 'left-pad', '1.3.0', 'pkg:npm/left-pad@1.3.0'], + ['SPDXRef-Package-3', 'ms', '2.1.3', 'pkg:npm/ms@2.1.3'], + ['SPDXRef-Package-4', 'once', '1.3.0', 'pkg:npm/once@1.3.0'], + ['SPDXRef-Package-5', 'once', '1.4.0', 'pkg:npm/once@1.4.0'], + ['SPDXRef-Package-6', 'widget', '1.0.0', 'pkg:npm/widget@1.0.0'], ]); - expect(spdx.packages[0]!.annotations[0]!.comment).toContain('scope=direct'); - expect(spdx.packages[3]!.annotations[0]!.comment).toContain('scope=transitive'); + expect(spdx.packages[1]!.annotations[0]!.comment).toContain('scope=direct'); + expect(spdx.packages[0]!.annotations[0]!.comment).toContain('scope=transitive'); expect(spdx.relationships).toEqual([ - { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-1', relationshipType: 'DEPENDS_ON' }, { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-2', relationshipType: 'DEPENDS_ON' }, - { spdxElementId: 'SPDXRef-Package-2', relatedSpdxElementId: 'SPDXRef-Package-4', relationshipType: 'DEPENDS_ON' }, - { spdxElementId: 'SPDXRef-Package-4', relatedSpdxElementId: 'SPDXRef-Package-6', relationshipType: 'DEPENDS_ON' }, + { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-6', relationshipType: 'DEPENDS_ON' }, + { spdxElementId: 'SPDXRef-Package-1', relatedSpdxElementId: 'SPDXRef-Package-4', relationshipType: 'DEPENDS_ON' }, + { spdxElementId: 'SPDXRef-Package-6', relatedSpdxElementId: 'SPDXRef-Package-1', relationshipType: 'DEPENDS_ON' }, ]); }); @@ -212,7 +212,7 @@ describe('sbom export: several versions of one package', () => { expect(second.serialNumber).not.toBe(first.serialNumber); }); - it('follows artifact project order for attribution and SPDX IDs, and keeps purls', () => { + it('keeps attribution from artifact project order and assigns SPDX IDs from the stable component order', () => { writeRootLock('other-last'); const forward = scanned(); const reversed = artifact([...forward.projects].reverse()); @@ -231,8 +231,9 @@ describe('sbom export: several versions of one package', () => { const spdxA = toSpdx(forward, graph) as { packages: Array<{ SPDXID: string; name: string }> }; const spdxB = toSpdx(reversed, graph) as { packages: Array<{ SPDXID: string; name: string }> }; - expect(spdxA.packages.find((p) => p.name === 'widget')!.SPDXID).toBe('SPDXRef-Package-2'); - expect(spdxB.packages.find((p) => p.name === 'widget')!.SPDXID).toBe('SPDXRef-Package-3'); + expect(spdxA.packages.map((p) => p.SPDXID)).toEqual(spdxB.packages.map((p) => p.SPDXID)); + expect(spdxA.packages.find((p) => p.name === 'widget')!.SPDXID).toBe('SPDXRef-Package-6'); + expect(spdxB.packages.find((p) => p.name === 'widget')!.SPDXID).toBe(spdxA.packages.find((p) => p.name === 'widget')!.SPDXID); }); it('omits lockfile-only versions when the lockfile graph is absent (--no-transitive)', () => { @@ -241,11 +242,11 @@ describe('sbom export: several versions of one package', () => { components: Array<{ name: string; version: string; properties: Array<{ name: string; value: string }> }>; dependencies?: unknown; }; - expect(cdx.components.map((c) => `${c.name}@${c.version}`)).toEqual(['left-pad@1.3.0', 'widget@1.0.0', 'once@1.4.0']); + expect(cdx.components.map((c) => `${c.name}@${c.version}`)).toEqual(['left-pad@1.3.0', 'once@1.4.0', 'widget@1.0.0']); expect(cdx.components.map((c) => scopeOf(c.properties, 'vibgrate:scope'))).toEqual(['direct', 'direct', 'direct']); expect(cdx.dependencies).toBeUndefined(); const spdx = toSpdx(scan) as { packages: Array<{ name: string; versionInfo: string }>; relationships?: unknown }; - expect(spdx.packages.map((p) => `${p.name}@${p.versionInfo}`)).toEqual(['left-pad@1.3.0', 'widget@1.0.0', 'once@1.4.0']); + expect(spdx.packages.map((p) => `${p.name}@${p.versionInfo}`)).toEqual(['left-pad@1.3.0', 'once@1.4.0', 'widget@1.0.0']); expect(spdx.relationships).toBeUndefined(); }); diff --git a/test/sbom-lockfile-merge.test.ts b/test/sbom-lockfile-merge.test.ts index 7733d3c..0099a52 100644 --- a/test/sbom-lockfile-merge.test.ts +++ b/test/sbom-lockfile-merge.test.ts @@ -113,9 +113,9 @@ describe('sbom export: multi-project lockfile merge', () => { }; expect(cdx.components.map((c) => `${c.purl}`)).toEqual([ 'pkg:npm/left-pad@1.3.0', - 'pkg:npm/widget@1.0.0', 'pkg:npm/once@1.3.0', 'pkg:npm/once@1.4.0', + 'pkg:npm/widget@1.0.0', 'pkg:npm/widget@2.0.0', ]); expect(new Set(cdx.components.map((c) => c['bom-ref'])).size).toBe(cdx.components.length); @@ -141,9 +141,9 @@ describe('sbom export: multi-project lockfile merge', () => { expect(cdx.dependencies).toEqual([ { ref: 'vibgrate-root', dependsOn: ['pkg:npm/left-pad@1.3.0', 'pkg:npm/widget@2.0.0'] }, { ref: 'pkg:npm/left-pad@1.3.0', dependsOn: ['pkg:npm/once@1.3.0'] }, - { ref: 'pkg:npm/widget@1.0.0', dependsOn: [] }, { ref: 'pkg:npm/once@1.3.0', dependsOn: [] }, { ref: 'pkg:npm/once@1.4.0', dependsOn: [] }, + { ref: 'pkg:npm/widget@1.0.0', dependsOn: [] }, { ref: 'pkg:npm/widget@2.0.0', dependsOn: [] }, ]); @@ -161,7 +161,7 @@ describe('sbom export: multi-project lockfile merge', () => { expect(spdx.relationships).toEqual([ { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-1', relationshipType: 'DEPENDS_ON' }, { spdxElementId: 'SPDXRef-DOCUMENT', relatedSpdxElementId: 'SPDXRef-Package-5', relationshipType: 'DEPENDS_ON' }, - { spdxElementId: 'SPDXRef-Package-1', relatedSpdxElementId: 'SPDXRef-Package-3', relationshipType: 'DEPENDS_ON' }, + { spdxElementId: 'SPDXRef-Package-1', relatedSpdxElementId: 'SPDXRef-Package-2', relationshipType: 'DEPENDS_ON' }, ]); expect(collectMergeWarnings(scan, graph)).toEqual([LOSSY_EDGE_WARNING]); }); @@ -331,4 +331,40 @@ describe('sbom export: multi-project lockfile merge', () => { expect(stderr.join('\n')).toContain(`warning [VG_WARN_SBOM_LOSSY_EDGES]: ${LOSSY_EDGE_WARNING}`); expect(stderr.join('\n')).not.toContain('http'); }); + + it('two runs on a fixture emit the same component and dependency order', () => { + const base = scanned(); + const graph = collectLockfileGraph(base, root); + expect(graph?.edges).toBeDefined(); + const place = (where: 'start' | 'end') => { + const scan = scanned(); + const row = dep('foo bar', '1.0.0'); + const deps = scan.projects[0]!.dependencies; + if (where === 'start') deps.unshift(row); + else deps.push(row); + return scan; + }; + const reversed = { + components: [...graph!.components].reverse(), + edges: new Map([...graph!.edges!.entries()].reverse()), + rootDependsOn: [...graph!.rootDependsOn].reverse(), + ecosystem: graph!.ecosystem, + }; + const first = toCycloneDx(place('start'), graph); + const second = toCycloneDx(place('end'), reversed); + expect(JSON.stringify(second)).toBe(JSON.stringify(first)); + const doc = first as { + serialNumber: string; + components: Array<{ name: string; purl?: string; 'bom-ref': string }>; + dependencies: Array<{ ref: string }>; + }; + const rerun = toCycloneDx(place('start'), collectLockfileGraph(base, root)) as { serialNumber: string }; + expect(rerun.serialNumber).toBe(doc.serialNumber); + expect(doc.serialNumber).toMatch(/^urn:uuid:/); + expect(doc.components.map((c) => c.name)).toEqual(['foo bar', 'left-pad', 'once', 'once', 'widget', 'widget']); + expect(doc.components[0]!.purl).toBeUndefined(); + expect(doc.dependencies[0]!.ref).toBe('vibgrate-root'); + expect(doc.dependencies.slice(1).map((d) => d.ref)).toEqual(doc.components.map((c) => c['bom-ref'])); + expect(JSON.stringify(toSpdx(place('start'), graph))).toBe(JSON.stringify(toSpdx(place('end'), reversed))); + }); });