diff --git a/dev-docs/SECURITY_ASSURANCE.md b/dev-docs/SECURITY_ASSURANCE.md index fcebcb78..fa43e06b 100644 --- a/dev-docs/SECURITY_ASSURANCE.md +++ b/dev-docs/SECURITY_ASSURANCE.md @@ -39,7 +39,7 @@ start a subprocess, enable a plugin, or choose an output path. | Repository manifests, lockfiles, and analyzer source | In-process file reads have a 64 MiB per-file limit that is checked before and during the read. Parsers never receive a partial over-limit document. Discovery is scoped to the selected target, does not follow directory symlinks, and has depth and exclusion controls. Pure parsers have registered fuzz targets and malformed-input tests. | `TestSDKContractReadLimitEnforcesBoundsAndGrowth`, `TestSDKContractReadRepositoryFileEnforces64MiBBound` (local contracts; full suite upstream in `bomly-dev/bomly-sdk` `system/read_test.go`), `test/assurance/REPOSITORY_INPUT_LIMITS.md`, `test/assurance/PARSER_FUZZING.md`, `scripts/run-fuzz.sh`, detector and analyzer package tests | A selected package manager's command output is not truncated because partial output could produce an incomplete graph. A local target may contain an unbounded number of individually bounded files. | | Project discovery and baselines | Recursive discovery does not follow directory symlinks. Automatic baseline selection warns and ignores a symlinked `.bomly` directory or baseline file. Explicit baseline selection may use a symlink because the user chose that path. Baseline JSON is limited to 16 MiB and 10,000 entries, is strict, and validates duplicates in linear time. It can only supply policy-status decisions. | `TestPlanSubprojectsRecursiveDoesNotFollowSymlinkedDirs`, `TestResolversForTargetIgnoresAutomaticSymlinksAndAllowsExplicitSelection`, `TestResolversForTargetAllowsUserSelectedSymlinkAsProjectRoot`, `TestLoadRejectsMalformedAndUnsupportedDocuments`, `TestLoadRejectsOversizedBaseline`, `TestDocumentEntryLimit`, `TestDocumentRejectsIndexedAdvisoryOverlap`, `FuzzLoad` | An explicitly selected baseline path is trusted and may be a symlink. | | SBOM and configuration input | Configuration reads are limited to 4 MiB and SBOM reads to 256 MiB before parsing. Strict configuration parsing rejects unknown keys. SBOM parsers are fuzzed and oversized documents fail clearly. SBOM ingest additionally refuses a document without a single unambiguous reading -- a repeated object member name or invalid UTF-8 (ADR-0039) -- with an error naming the member path or byte offset; the guarantee is exactly those two classes, and the check validates rather than re-decodes so no other parsing behavior changes. Duplicate detection retains an object's member names until that object closes, so the cost is set by every object open at once; the check is bounded by refusing a document that would hold more than 100,000 member names, or more than 16 MiB of them, simultaneously -- failing closed, since skipping the check on the largest inputs would place the gap where a crafted document would aim. The preflight also reads the document in place through a `*bytes.Buffer` rather than a generic reader, which would grow a buffer of its own and materialize each token into it -- a single 150 MiB string value retained 447 MiB that way and nothing at all in place; the name bounds do not reach values, so this is handled by not copying them. Both name bounds were needed and each was found by measurement rather than reasoning: bounding only the widest single object missed nested objects that accumulate (2.5 GiB from a 411 MiB document), and bounding names by count alone missed long names (900 MiB from a 194 MiB document, now 116 MiB and refused). An oversized name is refused on its source span before it is read, because reading one costs a copy of it -- measured at a full extra copy, escaped or not, on top of what the decoder retains; the span gates, the name accounts, and using either for both was wrong in a different direction each time. The strict reader is confined to `internal/sbom` by a guard test, so the trusted plugin wire cannot be tightened by a decoder swap. | `TestSDKContractReadLimitEnforcesBoundsAndGrowth`, `TestLoadFileRejectsOversizedFile`, `TestDetectorResolveGraph_RejectsOversizedSBOM`, `FuzzLoadFile`, `FuzzUnmarshalAutoJSON`, `TestIngestRejectsDuplicateObjectNames`, `TestIngestRejectsNestedDuplicateObjectNames`, `TestIngestRejectsInvalidUTF8`, `TestStrictIngestDoesNotRejectMerelyUnusualDocuments`, `TestBomlyOutputPassesTheStrictGate`, `TestStrictJSONStaysInSBOMIngest`, `TestADocumentTooLargeToCheckIsRefused`, `TestNestedObjectsCannotAccumulatePastTheBound`, `TestSiblingObjectsDoNotAccumulate`, `TestLongMemberNamesAreBoundedByTheirBytes`, `TestClosedObjectsReleaseTheirNameBytes`, `TestOrdinaryNamesDoNotApproachTheByteBound`, `TestStrictPreflightReadsInPlace`, `TestAnOversizedNameIsRefusedBeforeItIsRead`, `TestManyComponentsDoNotTripTheWidthBound`, `TestAmbiguityIsReportedBeforeFormatDetection` | A user-selected file can still consume work up to its limit. Case-variant field matching and other reader-divergence classes are outside the stated guarantee. | -| SBOM assertions re-published on export | An ingested document's component and document-level assertions are re-emitted under Bomly's name, so each clears its SDK publication gate on ingest *and* again on export -- a `GraphEntry` is reachable by any detector or plugin, so a value written straight onto one never passed a decoder. Contacts drop email addresses; unpublishable identities, names, comments, references, CPEs and digests are dropped rather than written. Reference types `distribution` and `vcs` are deliberately not ingested, so a detector's origin assertion cannot re-enter as a document assertion (ADR-0033, ADR-0042). Gates are idempotent, so a value cannot be laundered by another hop. | `FuzzIngestedAssertions`, `FuzzDocumentAssertions`, `TestSourceClaimsAreRegatedOnExport`, `TestOriginIsNotReadBackFromAnIngestedDocument`, `TestSingleSourceExportIsAFixedPoint`, `TestIngestLeavesComponentsEligibleForEnrichment` | A published value is only as safe as the SDK gate that admitted it; a gate defect is an SDK defect (see bomly-dev/bomly-sdk#54). A merged SPDX export cannot yet name its sources (bomly-dev/bomly-sdk#55). | +| SBOM assertions re-published on export | An ingested document's component and document-level assertions are re-emitted under Bomly's name, so each clears its SDK publication gate on ingest *and* again on export -- a `GraphEntry` is reachable by any detector or plugin, so a value written straight onto one never passed a decoder. Contacts drop email addresses; unpublishable identities, names, comments, references, CPEs and digests are dropped rather than written. Reference types `distribution` and `vcs` are deliberately not ingested, so a detector's origin assertion cannot re-enter as a document assertion (ADR-0033, ADR-0042). Gates are idempotent, so a value cannot be laundered by another hop. | `FuzzIngestedAssertions`, `FuzzDocumentAssertions`, `TestSourceClaimsAreRegatedOnExport`, `TestOriginIsNotReadBackFromAnIngestedDocument`, `TestSingleSourceExportIsAFixedPoint`, `TestIngestLeavesComponentsEligibleForEnrichment` | A published value is only as safe as the SDK gate that admitted it; a gate defect is an SDK defect -- bomly-dev/bomly-sdk#54 was one, and the local normalize-until-it-settles workaround it forced is gone since SDK v0.9.5. A merged export now names its sources in both formats, so a source identity and a source checksum are two more published values, each re-gated on export. | | Plugin download and extraction | Direct URL packages require a checksum unless the user explicitly bypasses it. GitHub release metadata is limited to 4 MiB. ZIP and tar extraction reject traversal, links, and special files. Downloads are limited to 256 MiB; archives to 4,096 entries, 256 MiB per expanded file, and 512 MiB total. Partial files are removed. | `TestResolveGitHubReleaseRejectsOversizedMetadata`, `TestExtractZipArchiveRejectsEscapingAndSymlinkEntries`, `TestExtractTarGzArchiveRejectsEscapingLinksAndSpecialFiles`, `TestCopyDownloadWithLimit`, `TestInstallRemoteArchiveRejectsDeclaredDownloadOverLimit`, `TestArchiveExtractionLimitsAtBoundary`, `TestArchiveExtractionRejectsResourceLimits`, `TestWriteArchiveFileRemovesPartialFileAtLimit` | `--insecure-skip-checksum` is an explicit integrity bypass. | | Plugin metadata and lifecycle | Manifests and runtime snapshots are limited to 1 MiB; the installed database is limited to 16 MiB. Plugins are installed disabled. Only enabled plugins register or run. The managed environment is allowlisted. | `TestReadFileWithLimitAcceptsExactBoundary`, `TestReadFileWithLimitRejectsOverBoundary`, `TestPluginJSONReadersRejectOversizedFiles`, `TestInstallDevBinaryVerifyEnableDisableAndUninstall`, `TestPrepareLoadsAndRunsExternalDetector`, `TestProtocolV1DetectorSnapshotDefaultsAbsentOptionalCapabilities`, `TestPluginEnvDoesNotForwardUnrelatedHostEnvironment` | Enabled plugins are trusted native processes with the user's privileges. The protocol is not an OS sandbox. | | Package-manager and detector subprocesses | Debug logs contain executable, sanitized arguments, working directory, and raw subprocess stderr. Credential-like flag values and URL user information are redacted from command arguments. Raw stderr is hidden below debug verbosity and is not added to structured results. Build-tool commands have existing timeouts where their detector contract supplies one. | `TestSDKContractSanitizeArgsRedactsCredentials`, `TestSDKContractCommandStderrHiddenBelowDebug` (local contracts; full suites upstream in `bomly-dev/bomly-sdk` `logkit/command_test.go` and `logkit/stderr_test.go`), `TestNewConsole` (`internal/logging/logger_test.go`), `TestInstallLogsReproducibleCommandAndDebugStderr`, `TestRunGitLogsStderrAtDebug`, Java readiness command tests, representative detector command-log tests | The selected executable, its credential store, registry configuration, network traffic, and filesystem behavior are trusted host concerns. Bomly cannot reliably sanitize arbitrary tool output. Debug stderr may contain credentials or other sensitive values and users must handle debug logs as sensitive data. | diff --git a/dev-docs/adr/0042-a-conversion-restates-its-source-a-merge-links-its-sources.md b/dev-docs/adr/0042-a-conversion-restates-its-source-a-merge-links-its-sources.md index 893cd525..35fafb02 100644 --- a/dev-docs/adr/0042-a-conversion-restates-its-source-a-merge-links-its-sources.md +++ b/dev-docs/adr/0042-a-conversion-restates-its-source-a-merge-links-its-sources.md @@ -80,3 +80,27 @@ written straight onto one never passed a decoder. Everything runs through and asserts idempotence, the property whose failure in the component half surfaced bomly-dev/bomly-sdk#54. The merged case is validated end to end by the SBOM interoperability workflow, through both official validators. + +## Update (2026-09-06): both gaps this ADR left open are closed + +Two consequences above were open questions with SDK issues attached. SDK +v0.9.5 closed both, and this repository consumes them. + +**A merged SPDX document now links its sources.** The blocker was that +`DocumentAssertions` had nowhere to keep a checksum over a source document's +bytes, which SPDX requires on every `externalDocumentRefs` entry +(bomly-dev/bomly-sdk#55). The carrier grew a document version and a source +checksum, and ingest computes that checksum where the original bytes are -- +in the codec entry point, once, for every format including one added later. +The SPDX projection now sits beside the CycloneDX one. A source that reached +a graph entry without passing through ingest has no checksum and is left +unnamed rather than written as an invalid reference. + +**Those links are read back.** `DocumentAssertions.Sources` +(bomly-dev/bomly-sdk#61) gives the documents behind a document a home, so +both codecs read the links on ingest and re-emit them on export. A merged +export converted again still names its inputs. Each source contributes its +own link tuple and the tuples it recorded, which is the SDK's declared merge +class for the set -- inheritance, not a rule re-decided here. The CycloneDX +`bom` reference carries the checksum too, so converting a merged CycloneDX +document to SPDX can still name every source. diff --git a/docs/SBOM.md b/docs/SBOM.md index fc8a4f93..8e1eb2c4 100644 --- a/docs/SBOM.md +++ b/docs/SBOM.md @@ -307,11 +307,22 @@ Every generated document carries a stable identity: stays the scanned project's name. - **Several source documents** (a merge). The output mints its own identity — both formats give a document exactly one, and adopting a source's would - name a document that is not this one — and *links* each source: a - CycloneDX external reference of type `bom`, carrying a BOM-Link - (`urn:cdx:/`) for a CycloneDX source or the namespace URI - for an SPDX one. People and tools credited by any source are credited by + name a document that is not this one — and *links* each source. CycloneDX + writes a document-level external reference of type `bom`, carrying a + BOM-Link (`urn:cdx:/`) for a CycloneDX source or the + namespace URI for an SPDX one. SPDX writes an `externalDocumentRefs` + entry naming the same identity, with a SHA-256 checksum over the source + document's bytes, which the specification requires on every entry; the + CycloneDX reference carries that checksum too, so the link stays usable + if the merged document is later converted to SPDX. The checksum is + computed while the source document is being read, because it cannot be + recovered from the parsed model afterwards — a source that reached Bomly + without those bytes is left unnamed in SPDX rather than written as an + invalid reference. People and tools credited by any source are credited by the merged document too. + - Source links are read back on import, so provenance survives more than one + conversion: converting a merged document again produces a document that + still names the documents behind it, in either format. - The producing tool with its version (CycloneDX `metadata.tools[]`; SPDX `Creator: Tool: bomly-cli-`), plus one tool entry per detector that contributed to the graph. @@ -406,13 +417,25 @@ Some information necessarily becomes less specific during conversion: prefers: a `bomly:scopes` CycloneDX property, and the `scope=` field of the SPDX package comment. A Bomly document therefore round-trips its scope set exactly, and a document from any other producer still yields a usable scope. - Reading the other way, CycloneDX `required` and `optional` both become - runtime and `excluded` becomes development. -- A source document's own scope word is not preserved verbatim. A component - a CycloneDX document marked `optional` imports as runtime and re-exports as - `required`, because Bomly's model has no slot for a source-asserted scope - beside the set it derives (tracked as bomly-dev/bomly-sdk#57). Bomly's own - documents are unaffected: their scope sets survive through the carrier. + Reading the other way, CycloneDX `required` becomes runtime, while + `optional` and `excluded` become development — the specification defines an + optional component as one that is not installed or otherwise reachable, so + it is absent from what runs. +- A scope carrier naming a token this build does not recognize keeps the + scopes it does recognize. The carrier is Bomly's own, so an unreadable token + is almost always one a newer Bomly wrote; refusing the whole value would have + left the component unscoped, which in SPDX — where there is no native scope + field to fall back on — meant losing the scope entirely. The tokens that were + not read are reported as a warning naming the file. +- A source document's own scope word is preserved and written back. A + component a CycloneDX document marked `optional` re-exports as `optional`, + not as Bomly's projection of the set it derives — so `optional` and + `excluded` do not collapse into `required` across a round trip that asserted + neither. The word yields to the projection when Bomly's own scope set stops + meaning what the word meant, for example once the package turns out to be + reachable from a development root as well. SPDX 2.3 has no scope field, so + the word is carried only through a CycloneDX export; an SPDX document still + carries the full set in its package comment. - Package origin is written on export but not read back on ingest: scanning an SBOM produces packages with no origin, so re-exporting that graph emits `NOASSERTION` and no distribution or vcs reference. Origin comes from a @@ -440,17 +463,10 @@ Some information necessarily becomes less specific during conversion: package that is both the document's subject and a component describes itself the same way in both places. -- A merged SPDX document does not link its sources. SPDX names another - document through `externalDocumentRefs`, and every entry there requires a - checksum over that document's bytes, which Bomly's document carrier cannot - yet hold (tracked as bomly-dev/bomly-sdk#55). Merged CycloneDX documents do - link their sources, and merged SPDX documents still preserve every - component assertion. -- Bomly does not read those source links back. A merged CycloneDX document - names the documents it was built from, but converting that document again - produces one that names no sources: Bomly records what a document says about - itself and has no place for the documents behind it (tracked as - bomly-dev/bomly-sdk#61). Keep the merged output if you need its provenance. +- A conversion is a fixed point within a format, not across one. A CycloneDX + serial can hold only a UUID URN, so an SPDX source converted to CycloneDX is + linked rather than adopted, and converting back produces a different + document. See "Document identity" above for how sources are named. - The CycloneDX `group` namespace survives a CycloneDX round trip. SPDX 2.3 has no group field, so an SPDX round trip recovers the namespace only from the PURL. diff --git a/go.mod b/go.mod index b5f8107b..8680a59c 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/bomly-dev/bomly-plugin-pyreach-analyzer v0.2.0 github.com/bomly-dev/bomly-plugin-scorecard-matcher v0.2.0 github.com/bomly-dev/bomly-plugin-syft-detector v0.2.0 - github.com/bomly-dev/bomly-sdk v0.9.6 + github.com/bomly-dev/bomly-sdk v0.9.7 github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 diff --git a/go.sum b/go.sum index f6e74b18..43cff82e 100644 --- a/go.sum +++ b/go.sum @@ -257,8 +257,8 @@ github.com/bomly-dev/bomly-plugin-scorecard-matcher v0.2.0 h1:VOK4+GfVGukbccabjD github.com/bomly-dev/bomly-plugin-scorecard-matcher v0.2.0/go.mod h1:3ux1Su5UCCKeF+wtttQrlw7r+B7sc6UXrK98Ybuq5gw= github.com/bomly-dev/bomly-plugin-syft-detector v0.2.0 h1:UlKwTqp+ZWu25leky9J6NITCKDdePpdgJQl/8dInApg= github.com/bomly-dev/bomly-plugin-syft-detector v0.2.0/go.mod h1:NVVrSMHkjC3VEDPtCI7+1c4tWBzwI1eve8tynO1pRoM= -github.com/bomly-dev/bomly-sdk v0.9.6 h1:igV2PbQBciou8jfzQ8QYSVnvQ8dC3RFUtR7priyrqIY= -github.com/bomly-dev/bomly-sdk v0.9.6/go.mod h1:gAGOEa88BE8gELt2rM/ZrqGIJnOzXiR6ocuuKW6AaJ0= +github.com/bomly-dev/bomly-sdk v0.9.7 h1:kYRSCR5jjzuAM81xyXExKC8yDstaUwxWKR/ml5gAQmQ= +github.com/bomly-dev/bomly-sdk v0.9.7/go.mod h1:gAGOEa88BE8gELt2rM/ZrqGIJnOzXiR6ocuuKW6AaJ0= github.com/bradleyjkemp/cupaloy/v2 v2.8.0 h1:any4BmKE+jGIaMpnU8YgH/I2LPiLBufr6oMMlVBbn9M= github.com/bradleyjkemp/cupaloy/v2 v2.8.0/go.mod h1:bm7JXdkRd4BHJk9HpwqAI8BoAY1lps46Enkdqw6aRX0= github.com/bufbuild/protocompile v0.14.1 h1:iA73zAf/fyljNjQKwYzUHD6AD4R8KMasmwa/FBatYVw= diff --git a/internal/detectors/guards_test.go b/internal/detectors/guards_test.go index f49f6920..9728a846 100644 --- a/internal/detectors/guards_test.go +++ b/internal/detectors/guards_test.go @@ -113,6 +113,35 @@ func TestNoDirectSPDXExpressionUse(t *testing.T) { } } +// guardFiles are the files whose job is to forbid a module, so they are the +// files that have to spell it. Exempting them is not a loophole -- naming a +// module in a rule that bans it is the opposite of reaching for it. +// +// An explicit set of canonical paths, for two reasons learned the hard way. +// Matching a name instead exempted every guards_test.go under internal/, so a +// second guard file anywhere could name a forbidden module unnoticed. And +// exempting only this one file made the guards flag each other the moment a +// second one existed: internal/output grew its own presentation-layer guard, +// which must name packageurl-go to forbid it, and this rule reported it. +// +// Adding a guard therefore costs one line here, on purpose. That is the point: +// a new exemption should be a deliberate edit somebody reviews, not a pattern +// that silently widens. +var guardFiles = map[string]struct{}{ + filepath.Clean(filepath.Join(internalRoot, "detectors", "guards_test.go")): {}, + filepath.Clean(filepath.Join(internalRoot, "output", "registry_lookup_guard_test.go")): {}, +} + +// isGuardFile reports whether a path is one of the guard files above. +// +// By path. Exempting anything named guards_test.go was the earlier bug: it +// meant a second guard file in any package could name a forbidden module and +// no rule would report it. +func isGuardFile(path string) bool { + _, ok := guardFiles[filepath.Clean(path)] + return ok +} + // filesNamingModule returns every Go file under internal/ -- test files // included -- whose text names the module path. Tests count because a test // reaching a library directly proves the hazard is still reachable, and a test @@ -127,13 +156,15 @@ func TestNoDirectSPDXExpressionUse(t *testing.T) { // the right direction to fail. func filesNamingModule(t *testing.T, module string) []string { t.Helper() - self := filepath.Clean(filepath.Join(internalRoot, "detectors", "guards_test.go")) var offenders []string err := filepath.Walk(internalRoot, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - if info.IsDir() || !strings.HasSuffix(path, ".go") || filepath.Clean(path) == self { + if info.IsDir() || !strings.HasSuffix(path, ".go") { + return nil + } + if isGuardFile(path) { return nil } body, err := os.ReadFile(path) @@ -279,3 +310,23 @@ func TestDetectionResultsCarryingGraphsAreAttributed(t *testing.T) { "wrap the result in detectors.Attributed: %v", offenders) } } + +// The exemption is a set of paths, and a file is not exempt for being named +// like a guard. Both halves have been wrong here before: matching the basename +// hid a forbidden import in a second guards_test.go, and exempting only one +// file made two guards report each other. +func TestGuardExemptionIsByPathNotByName(t *testing.T) { + for path := range guardFiles { + if _, err := os.Stat(path); err != nil { + t.Errorf("guard file %q does not exist; a dead exemption is a rule nobody is applying", path) + } + if !isGuardFile(path) { + t.Errorf("guard file %q is not recognized by its own predicate", path) + } + } + // A file named like a guard, in a package that has none, must not be + // exempt -- whether or not it exists today. + if impostor := filepath.Join(internalRoot, "sbom", "guards_test.go"); isGuardFile(impostor) { + t.Errorf("%q is exempt for being named guards_test.go rather than for being a guard", impostor) + } +} diff --git a/internal/detectors/sbom/detector.go b/internal/detectors/sbom/detector.go index a161a2de..dc0a6715 100644 --- a/internal/detectors/sbom/detector.go +++ b/internal/detectors/sbom/detector.go @@ -105,6 +105,19 @@ func (d Detector) ResolveGraph(_ context.Context, req sdk.DetectionRequest) (sdk } } + // A token in Bomly's own scope carrier that this build cannot read is + // almost always one a newer Bomly wrote. The scopes beside it are kept -- + // the SDK reads the carrier leniently -- so the graph is sound and the + // scan continues; what a user needs to know is that this binary is + // reading a document written by a later one, because that is the thing + // they can act on. This logger is the channel the ingest path has: the + // codec has none, and the SDK deliberately does not log. + if len(doc.UnknownScopeTokens) > 0 { + logger.Warn(fmt.Sprintf("sbom: ignored %d unrecognized scope token(s) in %q; a newer Bomly may have written them", + len(doc.UnknownScopeTokens), sbomPath), + zap.Strings("tokens", doc.UnknownScopeTokens)) + } + depsGraph, err := sbom.ToGraph(doc) if err != nil { return sdk.DetectionResult{}, fmt.Errorf("convert sbom %q to graph: %w", sbomPath, err) diff --git a/internal/detectors/sbom/detector_test.go b/internal/detectors/sbom/detector_test.go index d242f12f..d49e3217 100644 --- a/internal/detectors/sbom/detector_test.go +++ b/internal/detectors/sbom/detector_test.go @@ -3,6 +3,7 @@ package sbom import ( "context" "errors" + "fmt" "os" "path/filepath" "strings" @@ -13,6 +14,8 @@ import ( "github.com/bomly-dev/bomly-cli/internal/testnodes" "github.com/bomly-dev/bomly-sdk" "github.com/bomly-dev/bomly-sdk/system" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" ) func TestDetectorResolveGraph_SPDXJSON(t *testing.T) { @@ -249,3 +252,88 @@ func verifyResolvedGraph(t *testing.T, result sdk.DetectionResult, wantDependenc } t.Fatalf("expected graph to contain stable package id %q, got %s", wantDependencyID, g.PrettyString()) } + +// A carrier token this build cannot read reaches the user as a warning, and +// the scopes beside it still reach the graph. +// +// The SDK reads the carrier leniently so an older Bomly does not lose every +// scope to one token a newer one wrote (bomly-dev/bomly-sdk#64). Dropping the +// token silently would leave a user with no way to learn that the document was +// written by a later build; this detector's logger is the channel the ingest +// path has, since the codec has none and the SDK deliberately does not log. +func TestDetectorWarnsAboutUnreadableScopeTokens(t *testing.T) { + raw := `{ + "spdxVersion": "SPDX-2.3", + "dataLicense": "CC0-1.0", + "SPDXID": "SPDXRef-DOCUMENT", + "name": "n", + "documentNamespace": "https://acme.example/spdx/n", + "creationInfo": {"created": "2026-01-02T03:04:05Z", "creators": ["Tool: t"]}, + "packages": [ + { + "SPDXID": "SPDXRef-widget", + "name": "widget", + "versionInfo": "1.0.0", + "comment": "bomly:scope=runtime,future-scope", + "externalRefs": [ + {"referenceCategory": "PACKAGE-MANAGER", "referenceType": "purl", "referenceLocator": "pkg:npm/widget@1.0.0"} + ] + } + ] +}` + path := filepath.Join(t.TempDir(), "carrier.spdx.json") + if err := os.WriteFile(path, []byte(raw), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + core, logs := observer.New(zap.WarnLevel) + detector := Detector{Logger: zap.New(core)} + result, err := detector.ResolveGraph(context.Background(), requestForSBOMPath(path)) + if err != nil { + t.Fatalf("ResolveGraph() error = %v", err) + } + + entries := logs.All() + if len(entries) != 1 { + t.Fatalf("warnings = %+v, want exactly one", entries) + } + if !strings.Contains(entries[0].Message, "scope token") { + t.Errorf("warning = %q, want it to name the unreadable tokens", entries[0].Message) + } + var named bool + for _, field := range entries[0].Context { + if field.Key == "tokens" && strings.Contains(fmt.Sprint(field.Interface), "future-scope") { + named = true + } + } + if !named { + t.Errorf("warning fields = %+v, want the token itself", entries[0].Context) + } + + // The scope this build can read still reached the graph: the warning is + // about what was dropped, not about what was kept. + g, err := result.ConsolidatedGraph() + if err != nil { + t.Fatalf("ConsolidatedGraph() error = %v", err) + } + nodes := g.DependencyNodes() + if len(nodes) != 1 { + t.Fatalf("nodes = %d, want the component", len(nodes)) + } + if len(nodes[0].Scopes) != 1 || nodes[0].Scopes[0] != sdk.ScopeRuntime { + t.Errorf("scopes = %v, want runtime", nodes[0].Scopes) + } +} + +// A document with a carrier this build reads completely warns about nothing. +func TestDetectorStaysQuietForAReadableCarrier(t *testing.T) { + path := writeSBOMFixture(t, sbom.TargetSPDX23JSON) + core, logs := observer.New(zap.WarnLevel) + detector := Detector{Logger: zap.New(core)} + if _, err := detector.ResolveGraph(context.Background(), requestForSBOMPath(path)); err != nil { + t.Fatalf("ResolveGraph() error = %v", err) + } + if entries := logs.All(); len(entries) != 0 { + t.Errorf("warnings = %+v, want none", entries) + } +} diff --git a/internal/sbom/codec.go b/internal/sbom/codec.go index 8feaa5e6..5e4082a4 100644 --- a/internal/sbom/codec.go +++ b/internal/sbom/codec.go @@ -2,6 +2,8 @@ package sbom import ( "bytes" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -54,7 +56,7 @@ func UnmarshalJSON(data []byte, target Target) (*Document, error) { if err := requireUnambiguousJSON(data); err != nil { return nil, err } - return c.decodeJSON(data) + return decodeDocument(c, data) } // unmarshalValidated decodes a document whose bytes the caller has already @@ -64,7 +66,39 @@ func unmarshalValidated(data []byte, target Target) (*Document, error) { if !ok { return nil, fmt.Errorf("%w: %s", ErrUnsupportedTarget, target) } - return c.decodeJSON(data) + return decodeDocument(c, data) +} + +// decodeDocument runs a codec and stamps the document with a checksum over the +// bytes it was decoded from. +// +// Every ingest path goes through here, which is the point: the checksum can +// only be computed while the original bytes are in hand, and it cannot be +// recovered from the parsed model afterwards. An SPDX externalDocumentRef is +// invalid without one, so a merged SPDX export that has to name its sources +// has exactly one chance to capture it -- here, for every format, including +// one added later (ADR-0037). +// +// SHA-256 because both formats define it and both validators accept it; the +// spelling each writes is the SDK's to render, not this package's. +func decodeDocument(c codec, data []byte) (*Document, error) { + doc, err := c.decodeJSON(data) + if err != nil { + return nil, err + } + if doc == nil { + return nil, ErrNilDocument + } + sum := sha256.Sum256(data) + // The gate runs here rather than at the export site, so a checksum that + // could not be published never reaches the model at all. + if checksum, ok := (sdk.Digest{ + Algorithm: sdk.DigestAlgorithmSHA256, + Value: hex.EncodeToString(sum[:]), + }).Normalized(); ok { + doc.Assertions.Checksum = &checksum + } + return doc, nil } // DetectJSONTarget identifies the supported SBOM JSON format represented by data. diff --git a/internal/sbom/codec_fuzz_test.go b/internal/sbom/codec_fuzz_test.go index 21e9b6ef..b234316c 100644 --- a/internal/sbom/codec_fuzz_test.go +++ b/internal/sbom/codec_fuzz_test.go @@ -16,6 +16,13 @@ func FuzzUnmarshalAutoJSON(f *testing.F) { `{"bomFormat":"CycloneDX","specVersion":"1.5","version":1,"components":[]}`, `{"bomFormat":"CycloneDX","specVersion":"1.6","version":1,"components":[]}`, `{"artifacts":[],"artifactRelationships":[],"source":{"type":"directory","target":"."},"descriptor":{"name":"syft","version":"seed"},"schema":{"version":"16.0.34","url":"https://raw.githubusercontent.com/anchore/syft/main/schema/json/schema-16.0.34.json"}}`, + // Documents that name their own sources, so the fuzzer reaches the + // link read-back: an SPDX externalDocumentRef and a CycloneDX + // reference of type "bom", each with and without a usable checksum. + `{"spdxVersion":"SPDX-2.3","SPDXID":"SPDXRef-DOCUMENT","name":"demo","documentNamespace":"https://example.com/spdx/demo","externalDocumentRefs":[{"externalDocumentId":"DocumentRef-a","spdxDocument":"https://example.com/spdx/a","checksum":{"algorithm":"SHA256","checksumValue":"0000000000000000000000000000000000000000000000000000000000000000"}}],"creationInfo":{"created":"2026-01-01T00:00:00Z","creators":["Tool: bomly-fuzz"]},"packages":[]}`, + `{"spdxVersion":"SPDX-2.3","SPDXID":"SPDXRef-DOCUMENT","name":"demo","documentNamespace":"https://example.com/spdx/demo","externalDocumentRefs":[{"externalDocumentId":"DocumentRef-a","spdxDocument":"../../etc/passwd","checksum":{"algorithm":"NOPE","checksumValue":""}}],"packages":[]}`, + `{"bomFormat":"CycloneDX","specVersion":"1.6","version":1,"serialNumber":"urn:uuid:3e671687-395b-41f5-a30f-a58921a69b79","externalReferences":[{"type":"bom","url":"urn:cdx:3e671687-395b-41f5-a30f-a58921a69b79/2","hashes":[{"alg":"SHA-256","content":"0000000000000000000000000000000000000000000000000000000000000000"}]}],"components":[]}`, + `{"bomFormat":"CycloneDX","specVersion":"1.6","version":1,"externalReferences":[{"type":"bom","url":"file:///etc/passwd"}],"components":[{"bom-ref":"a","name":"a","version":"1","purl":"pkg:npm/a@1","scope":"optional","properties":[{"name":"bomly:scopes","value":"runtime,future"}]}]}`, // Malformed inputs: rejection paths must be deterministic, never panic. ``, `{}`, diff --git a/internal/sbom/cyclonedx.go b/internal/sbom/cyclonedx.go index 62740a9b..ad970c8c 100644 --- a/internal/sbom/cyclonedx.go +++ b/internal/sbom/cyclonedx.go @@ -109,18 +109,25 @@ func (c cycloneDXCodec) decodeJSON(data []byte) (*Document, error) { } componentByID := make(map[string]Component) + var unknownScopes []string if bom.Components != nil { for _, comp := range *bom.Components { + unknownScopes = mergeUnknownScopeTokens(unknownScopes, unknownScopeTokens(cycloneDXCarriedScopes(comp.Properties))) component := Component{ - ID: comp.BOMRef, - Name: comp.Name, - Org: comp.Group, - Type: string(comp.Type), - Scopes: sdk.ScopesFromCycloneDXComponent(string(comp.Scope), cycloneDXCarriedScopes(comp.Properties)), - Version: comp.Version, - PURL: comp.PackageURL, - Copyright: comp.Copyright, - Licenses: parseCycloneDXLicenses(comp.Licenses), + ID: comp.BOMRef, + Name: comp.Name, + Org: comp.Group, + Type: string(comp.Type), + Scopes: sdk.ScopesFromCycloneDXComponent(string(comp.Scope), cycloneDXCarriedScopes(comp.Properties)), + // The word beside the set it derives, so an export can say + // what this document said rather than Bomly's projection of + // it. Gated by the SDK, which is also what refuses a value + // that is not a scope word at all. + SourceScope: sdk.NormalizeSourceScope(string(comp.Scope)), + Version: comp.Version, + PURL: comp.PackageURL, + Copyright: comp.Copyright, + Licenses: parseCycloneDXLicenses(comp.Licenses), } applyCycloneDXAssertions(&component, comp) componentByID[comp.BOMRef] = component @@ -161,16 +168,18 @@ func (c cycloneDXCodec) decodeJSON(data []byte) (*Document, error) { if len(componentByID) == 0 && bom.Metadata != nil && bom.Metadata.Component != nil { root := bom.Metadata.Component + unknownScopes = mergeUnknownScopeTokens(unknownScopes, unknownScopeTokens(cycloneDXCarriedScopes(root.Properties))) component := Component{ - ID: root.BOMRef, - Name: root.Name, - Org: root.Group, - Type: string(root.Type), - Scopes: sdk.ScopesFromCycloneDXComponent(string(root.Scope), cycloneDXCarriedScopes(root.Properties)), - Version: root.Version, - PURL: root.PackageURL, - Copyright: root.Copyright, - Licenses: parseCycloneDXLicenses(root.Licenses), + ID: root.BOMRef, + Name: root.Name, + Org: root.Group, + Type: string(root.Type), + Scopes: sdk.ScopesFromCycloneDXComponent(string(root.Scope), cycloneDXCarriedScopes(root.Properties)), + SourceScope: sdk.NormalizeSourceScope(string(root.Scope)), + Version: root.Version, + PURL: root.PackageURL, + Copyright: root.Copyright, + Licenses: parseCycloneDXLicenses(root.Licenses), } // The same assertions the inventory loop applies. A document whose // only component is its primary one is legal, and reading it with @@ -210,15 +219,16 @@ func (c cycloneDXCodec) decodeJSON(data []byte) (*Document, error) { } return &Document{ - Name: defaultDocumentName, - Assertions: cycloneDXDocumentAssertions(bom), - Tool: cycloneDXPrimaryToolName(bom.Metadata), - Tools: cycloneDXToolNames(bom.Metadata), - Created: created, - SerialNumber: bom.SerialNumber, - Components: components, - Dependencies: dependencies, - Roots: roots, + Name: defaultDocumentName, + Assertions: cycloneDXDocumentAssertions(bom), + Tool: cycloneDXPrimaryToolName(bom.Metadata), + Tools: cycloneDXToolNames(bom.Metadata), + Created: created, + SerialNumber: bom.SerialNumber, + Components: components, + Dependencies: dependencies, + Roots: roots, + UnknownScopeTokens: unknownScopes, }, nil } @@ -442,11 +452,16 @@ func toCycloneDXVersion(target Target) cdx.SpecVersion { // the document describes a package the same way wherever it appears. func cycloneDXComponent(comp Component) cdx.Component { component := cdx.Component{ - BOMRef: comp.ID, - Type: cycloneDXComponentType(comp.Type), - Name: comp.NameOrID(), - Group: comp.Org, - Scope: cdx.Scope(sdk.CycloneDXScope(comp.Scopes)), + BOMRef: comp.ID, + Type: cycloneDXComponentType(comp.Type), + Name: comp.NameOrID(), + Group: comp.Org, + // The source document's own word when Bomly's scope set still means + // what that word meant, and the projection of the set otherwise. The + // SDK owns that decision: it is the same mapping that read the word + // in, and only it can say whether the word still describes the set + // (ADR-0037). + Scope: cdx.Scope(sdk.CycloneDXScopeForExport(comp.Scopes, comp.SourceScope)), Version: comp.Version, PackageURL: comp.PURL, Copyright: comp.Copyright, @@ -582,27 +597,23 @@ func cycloneDXHashes(digests []Digest) []cdx.Hash { // cycloneDXHashAlgorithm maps a digest algorithm string onto a CycloneDX hash // algorithm constant. Returns "" when the algorithm is unsupported so the // digest is dropped rather than emitting an invalid BOM. +// cycloneDXHashAlgorithm renders a digest algorithm in CycloneDX's spelling. +// +// The registry is the SDK's, not a list here. A hand-written switch stood in +// this spot and knew eight algorithms against the registry's nineteen, so a +// component carrying BLAKE2b, BLAKE3 or Streebog had that checksum silently +// dropped -- the same defect this PR already fixed on the SPDX side, in the +// same shape, one file away. +// +// An algorithm CycloneDX does not define returns "", which the caller drops. +// That is a real limit of the format: MD2, MD4, MD6 and ADLER32 are SPDX +// spellings with no CycloneDX equivalent. func cycloneDXHashAlgorithm(algorithm string) cdx.HashAlgorithm { - switch strings.ToLower(strings.TrimSpace(algorithm)) { - case "md5": - return cdx.HashAlgoMD5 - case "sha1", "sha-1": - return cdx.HashAlgoSHA1 - case "sha256", "sha-256": - return cdx.HashAlgoSHA256 - case "sha384", "sha-384": - return cdx.HashAlgoSHA384 - case "sha512", "sha-512": - return cdx.HashAlgoSHA512 - case "sha3-256": - return cdx.HashAlgoSHA3_256 - case "sha3-384": - return cdx.HashAlgoSHA3_384 - case "sha3-512": - return cdx.HashAlgoSHA3_512 - default: + parsed, err := sdk.ParseDigestAlgorithm(algorithm) + if err != nil { return "" } + return cdx.HashAlgorithm(parsed.CycloneDXName()) } // The property names carrying end-of-life through a CycloneDX document. They diff --git a/internal/sbom/cyclonedx_assertions.go b/internal/sbom/cyclonedx_assertions.go index 5207bb38..e1c5cd7f 100644 --- a/internal/sbom/cyclonedx_assertions.go +++ b/internal/sbom/cyclonedx_assertions.go @@ -297,8 +297,16 @@ func cycloneDXEmittedHashes(digests []sdk.Digest) *[]cdx.Hash { if !ok { continue } + // Through the same rendering the component hashes use. Casting the + // SDK token straight into CycloneDX's enum wrote "sha256" where the + // schema says "SHA-256" -- invalid for every algorithm, not only the + // ones CycloneDX has no name for. + algorithm := cycloneDXHashAlgorithm(string(normalized.Algorithm)) + if algorithm == "" { + continue + } hashes = append(hashes, cdx.Hash{ - Algorithm: cdx.HashAlgorithm(normalized.Algorithm), + Algorithm: algorithm, Value: normalized.Value, }) } @@ -329,6 +337,14 @@ func cycloneDXDocumentAssertions(bom *cdx.BOM) sdk.DocumentAssertions { if link, err := cdx.NewBOMLink(bom.SerialNumber, version, nil); err == nil { assertions.Identity = link.String() } + // The version the document stated, not the default applied above: zero + // means the source numbered nothing, and a BOM-Link identity's tail says + // so anyway. Writing the default here would put a version onto every + // document that never claimed one. + assertions.Version = bom.Version + // What this document says it was built from, so a second conversion can + // still name them. + assertions.Sources = cycloneDXIngestedSources(bom.ExternalReferences) if bom.Metadata != nil { assertions.Created = bom.Metadata.Timestamp if bom.Metadata.Manufacturer != nil { @@ -493,6 +509,13 @@ func cycloneDXMetadataTools(doc *Document) *cdx.ToolsChoice { // cycloneDXSourceLinks renders the links naming the documents this one was // built from, as external references of type "bom" on the document itself. +// +// The checksum rides along in the reference's hashes. CycloneDX does not +// require one, but SPDX's externalDocumentRef does, and a merged CycloneDX +// document converted to SPDX has nowhere else to recover it from -- so +// dropping it here would make the provenance survive one format and die on the +// next hop. The category-free reference gate is the SDK's; the digest gate +// runs again on the way out for the reason every export re-gates. func cycloneDXSourceLinks(doc *Document) []cdx.ExternalReference { // CycloneDX writes a serial and no namespace, so that is the only identity // a reader of this document can see, and the only one a source link could @@ -506,10 +529,69 @@ func cycloneDXSourceLinks(doc *Document) []cdx.ExternalReference { } refs := make([]cdx.ExternalReference, 0, len(links)) for _, link := range links { - refs = append(refs, cdx.ExternalReference{ - Type: cdx.ExternalReferenceType(link.Type), - URL: link.Locator, - }) + // Category stays unset: this is CycloneDX's axis, and the reference + // is written from the link tuple rather than from a stored reference. + ref, ok := sdk.ExternalReference{ + Type: string(cdx.ERTypeBOM), + Locator: link.Identity, + }.Normalized() + if !ok { + continue + } + emitted := cdx.ExternalReference{ + Type: cdx.ExternalReferenceType(ref.Type), + URL: ref.Locator, + } + if link.Checksum != nil { + emitted.Hashes = cycloneDXEmittedHashes([]sdk.Digest{*link.Checksum}) + } + refs = append(refs, emitted) + } + if len(refs) == 0 { + return nil } return refs } + +// cycloneDXIngestedSources reads the documents a CycloneDX document says it +// was built from: its own external references of type "bom". +// +// These were write-only until now -- the export wrote them and nothing read +// them back, so converting a merged document again produced one that named no +// sources at all (bomly-dev/bomly-sdk#61). The reference type is the +// library's constant, and every field is gated by DocumentSource.Normalized +// when the parent record is normalized, so a reference naming nothing +// publishable drops rather than becoming an empty link. +func cycloneDXIngestedSources(refs *[]cdx.ExternalReference) []sdk.DocumentSource { + if refs == nil { + return nil + } + sources := make([]sdk.DocumentSource, 0, len(*refs)) + for _, ref := range *refs { + if !strings.EqualFold(strings.TrimSpace(string(ref.Type)), string(cdx.ERTypeBOM)) { + continue + } + source := sdk.DocumentSource{Identity: ref.URL} + // The first hash that clears the digest gate. A reference may carry + // several; the record holds one, and the SPDX projection it feeds has + // one slot too. + if ref.Hashes != nil { + for _, hash := range *ref.Hashes { + checksum, ok := (sdk.Digest{ + Algorithm: sdk.DigestAlgorithm(hash.Algorithm), + Value: hash.Value, + }).Normalized() + if !ok { + continue + } + source.Checksum = &checksum + break + } + } + sources = append(sources, source) + } + if len(sources) == 0 { + return nil + } + return sources +} diff --git a/internal/sbom/document_assertions.go b/internal/sbom/document_assertions.go index ccac5f9b..109512f4 100644 --- a/internal/sbom/document_assertions.go +++ b/internal/sbom/document_assertions.go @@ -181,9 +181,9 @@ func (d documentIdentity) names(identity string) bool { return false } -// documentSourceLinks returns the references that name each source document -// this one was built from, for the sources whose identity this document did -// not adopt as the identity it is about to write. +// documentSourceLinks returns the link tuples naming each source document this +// one was built from, for the sources whose identity this document did not +// adopt as the identity it is about to write. // // The comparison is against what the format emits, not against the model's // namespace field. Converting an SPDX source to CycloneDX adopts the source @@ -192,41 +192,48 @@ func (d documentIdentity) names(identity string) bool { // suppressed the link for a document that had not in fact adopted anything, // and the export named its source neither way. // -// Only CycloneDX renders these today. SPDX links documents through -// externalDocumentRefs, whose every entry requires a checksum over the source -// document's bytes -- and DocumentAssertions has nowhere to carry one, so a -// merged SPDX export names no sources. Tracked as bomly-dev/bomly-sdk#55; -// when that field ships, an SPDX projection belongs here beside this one. +// Each source contributes two things: its own link tuple, and the tuples it +// recorded for the documents *it* was built from. That second half is what +// makes provenance survive more than one hop, and it is the SDK's declared +// merge class for the set rather than a rule invented here -- a document +// built from a merged document inherits that document's sources beside its +// own identity. Without it a merged export converted again named nothing: the +// links were write-only, which is what bomly-dev/bomly-sdk#61 recorded. // -// These links are also write-only for now: DocumentAssertions records what a -// document says about itself, with no field for the documents behind it, so -// ingesting a merged export cannot restore them and a second export names no -// sources. Tracked as bomly-dev/bomly-sdk#61. Carrying them through Metadata -// instead is exactly what ADR-0037 closed off, so the gap is documented rather -// than worked around. -func documentSourceLinks(doc *Document, emitted documentIdentity) []sdk.ExternalReference { +// The result is folded, gated, sorted and bounded by the SDK, by handing the +// candidates back to DocumentAssertions.Normalized. Doing it here would be a +// second copy of the set's key, its self-reference rule and its bound -- the +// three things that decide whether the merge stays associative. +// +// Both formats render these now. CycloneDX writes an external reference of +// type "bom"; SPDX writes an externalDocumentRef, which requires a checksum +// over the source document's bytes -- captured at ingest by decodeDocument, so +// a source that arrived without one is skipped there rather than written as an +// invalid reference. +func documentSourceLinks(doc *Document, emitted documentIdentity) []sdk.DocumentSource { if doc == nil || len(doc.Sources) == 0 { return nil } - links := make([]sdk.ExternalReference, 0, len(doc.Sources)) + candidates := make([]sdk.DocumentSource, 0, len(doc.Sources)*2) for _, source := range doc.Sources { - if emitted.names(source.Identity) || source.Identity == "" { - continue - } - // Category stays unknown: this is CycloneDX's axis, and SPDX's - // referenceCategory has no member that means "another document" -- - // SPDX links documents through externalDocumentRefs instead. - ref, ok := sdk.ExternalReference{ - Type: string(cdx.ERTypeBOM), - Locator: source.Identity, - }.Normalized() - if !ok { + candidates = append(candidates, sdk.DocumentSource{ + Identity: source.Identity, + Version: source.Version, + Checksum: source.Checksum, + }) + candidates = append(candidates, source.Sources...) + } + folded, _ := sdk.DocumentAssertions{Sources: candidates}.Normalized() + + links := make([]sdk.DocumentSource, 0, len(folded.Sources)) + for _, source := range folded.Sources { + if emitted.names(source.Identity) { continue } - links = append(links, ref) + links = append(links, source) } if len(links) == 0 { return nil } - return sdk.MergeExternalReferences(nil, links) + return links } diff --git a/internal/sbom/document_assertions_fuzz_test.go b/internal/sbom/document_assertions_fuzz_test.go index abd57198..5313beaa 100644 --- a/internal/sbom/document_assertions_fuzz_test.go +++ b/internal/sbom/document_assertions_fuzz_test.go @@ -44,6 +44,16 @@ func FuzzDocumentAssertions(f *testing.F) { Creators: []sdk.Contact{{Kind: sdk.ContactKindOrganization, Name: creator}}, Tools: []sdk.DocumentTool{{Name: tool}}, Comment: comment, + // The documents this one claims to be built from, asserted just + // as hostilely: they become external references and SPDX + // externalDocumentRefs on export, so an identity that is a local + // path or a digest that is not one must not reach a document. + // The self-reference -- the same identity this record claims -- + // is the cycle the SDK's gate drops. + Sources: []sdk.DocumentSource{ + {Identity: identity}, + {Identity: name, Checksum: &sdk.Digest{Algorithm: sdk.DigestAlgorithm(dataLicense), Value: tool}}, + }, } g := mustFuzzGraph(t) diff --git a/internal/sbom/document_assertions_test.go b/internal/sbom/document_assertions_test.go index bd5ac497..9001db54 100644 --- a/internal/sbom/document_assertions_test.go +++ b/internal/sbom/document_assertions_test.go @@ -250,9 +250,15 @@ func TestMergedExportLinksItsSourcesInsteadOfAdoptingOne(t *testing.T) { if len(links) != 2 { t.Fatalf("links = %+v, want one per source", links) } + // Each link is a full tuple, not a bare identity: the checksum is what + // SPDX's externalDocumentRef requires on every entry, and it can only be + // computed while the source's original bytes are in hand. for _, link := range links { - if link.Type != string(cdx.ERTypeBOM) { - t.Errorf("link type = %q, want %q", link.Type, cdx.ERTypeBOM) + if link.Identity == "" { + t.Errorf("link %+v names no document", link) + } + if link.Checksum == nil { + t.Errorf("link %q carries no checksum, so SPDX cannot name it", link.Identity) } } } @@ -497,6 +503,11 @@ func TestConfiguredProvenanceOutranksAnIngestedOrganization(t *testing.T) { // The record exists for a document that asserted nothing, which is what makes // a merge involving such a document read as a merge. +// +// "Asserted nothing" is about the claims the document made: no identity, no +// name, no creators, no tools. The record is no longer empty even then, +// because ingest stamps a checksum over the bytes it read -- the value SPDX's +// externalDocumentRef requires and that nothing downstream can recompute. func TestDocumentAssertionsForAlwaysRecordsThatADocumentWasRead(t *testing.T) { doc, _, err := UnmarshalAutoJSON([]byte(`{"bomFormat":"CycloneDX","specVersion":"1.5","version":1,"components":[]}`)) if err != nil { @@ -506,8 +517,12 @@ func TestDocumentAssertionsForAlwaysRecordsThatADocumentWasRead(t *testing.T) { if got == nil { t.Fatal("a document that asserted nothing left no record that it was read") } - if !got.IsEmpty() { - t.Errorf("assertions = %+v, want the empty record", *got) + if got.Identity != "" || got.Name != "" || got.DataLicense != "" || got.Comment != "" || + len(got.Creators) != 0 || len(got.Tools) != 0 || len(got.Sources) != 0 { + t.Errorf("assertions = %+v, want no claims of its own", *got) + } + if got.Checksum == nil { + t.Errorf("assertions = %+v, want the ingest-captured checksum", *got) } if DocumentAssertionsFor(nil) != nil { t.Error("no document must mean no record") diff --git a/internal/sbom/document_sources_test.go b/internal/sbom/document_sources_test.go new file mode 100644 index 00000000..ad7be2a6 --- /dev/null +++ b/internal/sbom/document_sources_test.go @@ -0,0 +1,318 @@ +package sbom + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "strings" + "testing" + + cdx "github.com/CycloneDX/cyclonedx-go" + "github.com/bomly-dev/bomly-sdk" +) + +// mergedExport ingests two documents, merges their graphs, and exports the +// result to one target -- the merge case ADR-0042 defines, where the document +// mints its own identity and links its sources. +func mergedExport(t *testing.T, target Target, raws ...string) ([]byte, []sdk.GraphEntry) { + t.Helper() + entries := make([]sdk.GraphEntry, 0, len(raws)) + merged := sdk.New() + for _, raw := range raws { + _, entry := ingestDocument(t, raw) + entries = append(entries, entry) + if err := sdk.MergeGraph(merged, entry.Graph); err != nil { + t.Fatalf("merge: %v", err) + } + } + raw, err := MarshalGraphEntriesJSON(merged, entries, target, BuildOptions{Created: fixedExportTime()}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("export: %v", err) + } + return raw, entries +} + +// spdxExternalDocumentRefs reads the externalDocumentRefs off a rendered SPDX +// document, as a consumer would. +func spdxExternalDocumentRefs(t *testing.T, raw []byte) []struct { + ID string `json:"externalDocumentId"` + URI string `json:"spdxDocument"` + Checksum struct { + Algorithm string `json:"algorithm"` + Value string `json:"checksumValue"` + } `json:"checksum"` +} { + t.Helper() + var doc struct { + Namespace string `json:"documentNamespace"` + Refs []struct { + ID string `json:"externalDocumentId"` + URI string `json:"spdxDocument"` + Checksum struct { + Algorithm string `json:"algorithm"` + Value string `json:"checksumValue"` + } `json:"checksum"` + } `json:"externalDocumentRefs"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + t.Fatalf("decode spdx: %v", err) + } + return doc.Refs +} + +// cycloneDXSourceRefs reads the document-level references of type "bom". +func cycloneDXSourceRefs(t *testing.T, raw []byte) []cdx.ExternalReference { + t.Helper() + var bom cdx.BOM + if err := json.Unmarshal(raw, &bom); err != nil { + t.Fatalf("decode cyclonedx: %v", err) + } + if bom.ExternalReferences == nil { + return nil + } + refs := make([]cdx.ExternalReference, 0, len(*bom.ExternalReferences)) + for _, ref := range *bom.ExternalReferences { + if ref.Type == cdx.ERTypeBOM { + refs = append(refs, ref) + } + } + return refs +} + +// A merged SPDX document names the documents it was built from. +// +// It could not before: SPDX links a document through externalDocumentRefs, +// every entry there requires a checksum over that document's bytes, and the +// document carrier had nowhere to hold one -- so the CycloneDX half of +// ADR-0042 shipped and the SPDX half was left open as +// bomly-dev/bomly-sdk#55. The checksum is captured at ingest, which is the +// only moment those bytes exist. +func TestMergedSPDXExportNamesItsSources(t *testing.T) { + raw, _ := mergedExport(t, TargetSPDX23JSON, documentRichSPDX, serialCycloneDX) + + var doc struct { + Namespace string `json:"documentNamespace"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + t.Fatalf("decode: %v", err) + } + if doc.Namespace == "https://acme.example/spdx/acme-platform-7f3c" { + t.Error("the merged document adopted a source's identity instead of minting its own") + } + + refs := spdxExternalDocumentRefs(t, raw) + if len(refs) != 2 { + t.Fatalf("externalDocumentRefs = %+v, want one per source\n%s", refs, raw) + } + byURI := make(map[string]string, len(refs)) + seenIDs := make(map[string]struct{}, len(refs)) + for _, ref := range refs { + if !strings.HasPrefix(ref.ID, "DocumentRef-") { + t.Errorf("externalDocumentId = %q, want the mandatory DocumentRef- prefix", ref.ID) + } + if _, duplicate := seenIDs[ref.ID]; duplicate { + t.Errorf("externalDocumentId %q is used twice", ref.ID) + } + seenIDs[ref.ID] = struct{}{} + if ref.Checksum.Algorithm != "SHA256" { + t.Errorf("checksum algorithm = %q, want SPDX's own spelling", ref.Checksum.Algorithm) + } + if len(ref.Checksum.Value) != 64 { + t.Errorf("checksum for %q = %q, want a SHA-256 hex digest", ref.URI, ref.Checksum.Value) + } + byURI[ref.URI] = ref.Checksum.Value + } + if _, ok := byURI["https://acme.example/spdx/acme-platform-7f3c"]; !ok { + t.Errorf("the SPDX source is not named: %+v", refs) + } + if _, ok := byURI["urn:cdx:3e671687-395b-41f5-a30f-a58921a69b79/1"]; !ok { + t.Errorf("the CycloneDX source is not named by its BOM-Link: %+v", refs) + } + + // The two sources are different documents, so their checksums differ. A + // single digest reused across every entry would be the shape of a bug + // that hashes the export rather than each source. + values := make(map[string]struct{}, len(byURI)) + for _, value := range byURI { + values[value] = struct{}{} + } + if len(values) != len(byURI) { + t.Errorf("the sources share a checksum: %+v", byURI) + } +} + +// The checksum a source link carries is a digest of that source document's +// own bytes, not of anything else. Asserted against the digest computed here +// so a change that hashes the wrong buffer cannot pass. +func TestSourceLinkChecksumCoversTheSourceBytes(t *testing.T) { + raw, _ := mergedExport(t, TargetSPDX23JSON, documentRichSPDX, serialCycloneDX) + + want := map[string]string{ + "https://acme.example/spdx/acme-platform-7f3c": sha256Hex(documentRichSPDX), + "urn:cdx:3e671687-395b-41f5-a30f-a58921a69b79/1": sha256Hex(serialCycloneDX), + } + for _, ref := range spdxExternalDocumentRefs(t, raw) { + expected, known := want[ref.URI] + if !known { + t.Errorf("unexpected source %q", ref.URI) + continue + } + if ref.Checksum.Value != expected { + t.Errorf("checksum for %q = %q, want a digest of that document's bytes %q", ref.URI, ref.Checksum.Value, expected) + } + } +} + +// Converting a merged document again still names the documents behind it. +// +// The links used to be write-only: an export wrote them and ingest read +// nothing back, so the second conversion produced a document that claimed to +// be built from nothing (bomly-dev/bomly-sdk#61). Provenance now survives more +// than one hop, which is the SDK's declared merge class for the source set. +func TestMergedSourceLinksSurviveASecondConversion(t *testing.T) { + for _, target := range []Target{TargetSPDX23JSON, TargetCycloneDX16JSON} { + t.Run(string(target), func(t *testing.T) { + first, _ := mergedExport(t, target, documentRichSPDX, serialCycloneDX) + + // The merged document, converted a second time. One source now, + // so this is a conversion: it restates that document, and the + // documents that document named are still named. + _, entry := ingestDocument(t, string(first)) + second, err := MarshalGraphEntriesJSON(entry.Graph, []sdk.GraphEntry{entry}, target, + BuildOptions{Created: fixedExportTime()}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("second export: %v", err) + } + + var named []string + switch target { + case TargetSPDX23JSON: + for _, ref := range spdxExternalDocumentRefs(t, second) { + named = append(named, ref.URI) + } + default: + for _, ref := range cycloneDXSourceRefs(t, second) { + named = append(named, ref.URL) + } + } + for _, want := range []string{ + "https://acme.example/spdx/acme-platform-7f3c", + "urn:cdx:3e671687-395b-41f5-a30f-a58921a69b79/1", + } { + if !containsStringValue(named, want) { + t.Errorf("second export names %v, missing %q\n%s", named, want, second) + } + } + }) + } +} + +// A merged CycloneDX document carries each source's checksum on the link, so +// the tuple SPDX needs survives a CycloneDX hop. Without it, converting a +// merged CycloneDX document to SPDX could name no source at all -- the format +// requires the checksum and there would be nowhere left to recover it from. +func TestCycloneDXSourceLinksCarryTheirChecksum(t *testing.T) { + raw, _ := mergedExport(t, TargetCycloneDX16JSON, documentRichSPDX, serialCycloneDX) + refs := cycloneDXSourceRefs(t, raw) + if len(refs) != 2 { + t.Fatalf("bom references = %+v, want one per source\n%s", refs, raw) + } + for _, ref := range refs { + if ref.Hashes == nil || len(*ref.Hashes) == 0 { + t.Fatalf("the link to %q carries no hash\n%s", ref.URL, raw) + } + } + + // And converting that document to SPDX still names both sources, which is + // the whole reason the hash is written. + _, entry := ingestDocument(t, string(raw)) + converted, err := MarshalGraphEntriesJSON(entry.Graph, []sdk.GraphEntry{entry}, TargetSPDX23JSON, + BuildOptions{Created: fixedExportTime()}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("convert to spdx: %v", err) + } + if got := len(spdxExternalDocumentRefs(t, converted)); got != 2 { + t.Errorf("externalDocumentRefs = %d, want both sources\n%s", got, converted) + } +} + +// A native scan names no sources: there were none. +func TestNativeExportNamesNoSources(t *testing.T) { + g := scopedGraph(t, sdk.ScopeRuntime) + for _, target := range []Target{TargetSPDX23JSON, TargetCycloneDX16JSON} { + t.Run(string(target), func(t *testing.T) { + raw, err := MarshalDepGraphJSON(g, target, BuildOptions{Created: fixedExportTime()}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("export: %v", err) + } + switch target { + case TargetSPDX23JSON: + if refs := spdxExternalDocumentRefs(t, raw); len(refs) != 0 { + t.Errorf("externalDocumentRefs = %+v, want none", refs) + } + default: + if refs := cycloneDXSourceRefs(t, raw); len(refs) != 0 { + t.Errorf("bom references = %+v, want none", refs) + } + } + }) + } +} + +// A conversion does not link the document it restates: it *is* that document. +// Only the documents behind it are named. +func TestConversionDoesNotLinkTheDocumentItRestates(t *testing.T) { + _, entry := ingestDocument(t, documentRichSPDX) + raw, err := MarshalGraphEntriesJSON(entry.Graph, []sdk.GraphEntry{entry}, TargetSPDX23JSON, + BuildOptions{Created: fixedExportTime()}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("export: %v", err) + } + if refs := spdxExternalDocumentRefs(t, raw); len(refs) != 0 { + t.Errorf("externalDocumentRefs = %+v, want none: the export adopted that identity", refs) + } +} + +func containsStringValue(values []string, want string) bool { + for _, value := range values { + if value == want { + return true + } + } + return false +} + +// sha256Hex is the digest form the ingest checksum is written in. +func sha256Hex(value string) string { + sum := sha256.Sum256([]byte(value)) + return hex.EncodeToString(sum[:]) +} + +// Two sources whose identities reduce to the same SPDX idstring still get +// distinct reference ids. +// +// The idstring alphabet is narrow -- letters, digits, "." and "-" -- so two +// different URIs can sanitize to one id, and SPDX requires each +// externalDocumentId to be unique within a document. Nothing in tools-golang +// or the SDK mints one, so this package reuses its own package-id rule +// including the collision suffix; this is the case that says so. +func TestCollidingSourceIdentitiesGetDistinctReferenceIDs(t *testing.T) { + checksum := sdk.Digest{ + Algorithm: sdk.DigestAlgorithmSHA256, + Value: "0000000000000000000000000000000000000000000000000000000000000000", + } + doc := &Document{ + Namespace: "https://bomly.dev/spdx/merged", + Sources: []sdk.DocumentAssertions{ + {Identity: "https://acme.example/a_b", Checksum: &checksum}, + {Identity: "https://acme.example/a+b", Checksum: &checksum}, + }, + } + refs := spdxSourceLinks(doc) + if len(refs) != 2 { + t.Fatalf("refs = %+v, want one per source", refs) + } + if refs[0].DocumentRefID == refs[1].DocumentRefID { + t.Errorf("both sources share the id %q; SPDX requires them to be distinct", refs[0].DocumentRefID) + } +} diff --git a/internal/sbom/graph.go b/internal/sbom/graph.go index 01404a42..ebfb16f1 100644 --- a/internal/sbom/graph.go +++ b/internal/sbom/graph.go @@ -41,40 +41,13 @@ func applyIngestedAssertions(pkg *sdk.DependencyNode, component Component) { pkg.Originator = &contact } } - pkg.Description = stableValue(sdk.NormalizeDescription, component.Description) - pkg.Homepage = stableValue(sdk.NormalizeHomepage, component.Homepage) + pkg.Description = sdk.NormalizeDescription(component.Description) + pkg.Homepage = sdk.NormalizeHomepage(component.Homepage) pkg.ExternalReferences = sdk.MergeExternalReferences(nil, component.ExternalReferences) pkg.Digests = ingestedDigests(component.Digests) pkg.CPEs = ingestedCPEs(component.CPEs) } -// stableValue normalizes until the result stops changing, and drops a value -// that will not settle. -// -// A gate that is not idempotent breaks the round trip this whole change -// exists to provide: a description carrying invalid UTF-8 normalizes to three -// bytes per bad byte, which can push it past the length bound that was -// applied before the repair -- so the next pass sees an over-long value and -// returns empty. The field then survives one hop and vanishes on the next. -// Found by FuzzIngestedAssertions within a minute of first running, and filed -// as bomly-dev/bomly-sdk#54. -// -// This does not re-implement any rule: it applies the SDK's own function -// until it reaches its fixed point, which is a policy decision about how much -// to trust a gate rather than a second opinion about what the gate should -// say. Publishing a value that changes on the next hop is worse than -// publishing nothing, so a value that has not settled after one further pass -// is dropped. Remove this once #54 ships; the fuzz target asserts the -// property either way. -func stableValue(normalize func(string) string, value string) string { - first := normalize(value) - second := normalize(first) - if first != second { - return "" - } - return first -} - // ingestedDigests admits the checksums a document stated, each through the // digest gate. // @@ -221,6 +194,9 @@ func ToGraph(doc *Document) (*sdk.Graph, error) { return nil, fmt.Errorf("sbom component %q (%s): %w", component.ID, componentIdentityHint(component), err) } pkg.Scopes = append([]sdk.Scope(nil), component.Scopes...) + // Through the gate, not copied: the field crosses the same trust + // boundary every other ingested assertion does. + pkg.SourceScope = sdk.NormalizeSourceScope(component.SourceScope) pkg.Copyright = component.Copyright applyIngestedAssertions(pkg, component) // The document's own component ID does not survive: the node answers diff --git a/internal/sbom/graph_test.go b/internal/sbom/graph_test.go index f5e80fff..04171e23 100644 --- a/internal/sbom/graph_test.go +++ b/internal/sbom/graph_test.go @@ -149,3 +149,57 @@ func TestExportedDependenciesNameOnlyComponents(t *testing.T) { t.Fatalf("child module dependsOn = %v, want %q", got, leaf.NodeID()) } } + +// The ingest gates are fixed points, so the values they admit survive every +// further hop. +// +// This is the property bomly-dev/bomly-sdk#54 broke and the SDK's v0.9.5 +// NormalizeDescription restored, and it is the reason applyIngestedAssertions +// no longer wraps the gates in a local normalize-until-it-settles loop. The +// input is the shape that found the defect: bytes that are not valid UTF-8, +// short enough to pass the gate's input bound and long enough that repairing +// each of them into U+FFFD -- three bytes apiece -- carries the result past +// that same bound. Under the old gate the first pass returned the repaired, +// over-long value and the second returned "", so a description survived one +// conversion and vanished on the next. +// +// Asserted on the gate itself rather than only through the node, because it +// is the gate's promise; the node assertion below is what the ingest path +// actually depends on. +func TestIngestGatesAreFixedPoints(t *testing.T) { + for _, testCase := range []struct { + name string + value string + }{ + {"invalid utf-8 that triples on repair", strings.Repeat("\xff", 5000)}, + {"invalid utf-8 within the bound", strings.Repeat("\xff", 16)}, + {"control characters", "a\x00b\x07c"}, + {"plain text", "a widget for widgets"}, + } { + t.Run(testCase.name, func(t *testing.T) { + once := sdk.NormalizeDescription(testCase.value) + if twice := sdk.NormalizeDescription(once); twice != once { + t.Fatalf("NormalizeDescription is not idempotent: %d bytes then %d bytes", len(once), len(twice)) + } + + node, err := sdk.NewDependencyNode(sdk.Coordinates{Ecosystem: "npm", Name: "widget", Version: "1.0.0"}) + if err != nil { + t.Fatalf("construct node: %v", err) + } + applyIngestedAssertions(node, Component{Name: "widget", Description: testCase.value}) + if node.Description != once { + t.Fatalf("ingested description = %q, want the gate's own answer %q", node.Description, once) + } + // And a second hop -- export back into a component, ingest again -- + // keeps it, which is what the deleted workaround was protecting. + second, err := sdk.NewDependencyNode(sdk.Coordinates{Ecosystem: "npm", Name: "widget", Version: "1.0.0"}) + if err != nil { + t.Fatalf("construct node: %v", err) + } + applyIngestedAssertions(second, Component{Name: "widget", Description: node.Description}) + if second.Description != node.Description { + t.Fatalf("description changed on the second hop: %q then %q", node.Description, second.Description) + } + }) + } +} diff --git a/internal/sbom/identity.go b/internal/sbom/identity.go index 32ba4ef8..384b02a8 100644 --- a/internal/sbom/identity.go +++ b/internal/sbom/identity.go @@ -29,26 +29,14 @@ func parsePURL(value string) *purlkit.PURL { // it and SPDX rebuilds it from the PURL — guessing here would relabel every // round-tripped Erlang dependency as Elixir, and packageManagerForPURLType // would then call it Mix. Leaving it unknown keeps the ambiguity visible. -// The purl-type -> ecosystem table that lived here is purlkit's -// (EcosystemForType). It was a second copy of the SDK's mapping, and a second -// copy is a mapping that drifts: purl-spec adds a type, one table learns it -// and the other keeps answering unknown. Phase 2.1 deletes the copy. +// The purl-type -> ecosystem question is the SDK's, answered by +// EcosystemForPURLType. It was reassembled here out of purlkit calls plus a +// fallback of this package's own, which is the same drift in a thinner +// disguise: the SDK grew a second lookup for manager-name aliases and this +// copy did not, so "swiftpm" resolved to unknown here and to swift there. +// Delegating picked that up rather than any change being made for it. func ecosystemFromPURLType(purlType string) sdk.Ecosystem { - normalized := strings.ToLower(strings.TrimSpace(purlType)) - if normalized == "" { - return sdk.EcosystemUnknown - } - if name, ok := purlkit.EcosystemForType(normalized); ok { - if ecosystem, err := sdk.ParseEcosystem(name); err == nil { - return ecosystem - } - } - // A type purlkit does not map may still be an ecosystem name Bomly knows. - ecosystem, err := sdk.ParseEcosystem(normalized) - if err != nil { - return sdk.EcosystemUnknown - } - return ecosystem + return sdk.EcosystemForPURLType(purlType) } // ComponentEcosystem resolves the ecosystem a document component belongs to: diff --git a/internal/sbom/license_emission_test.go b/internal/sbom/license_emission_test.go index 32e26e1d..2af8497e 100644 --- a/internal/sbom/license_emission_test.go +++ b/internal/sbom/license_emission_test.go @@ -396,3 +396,75 @@ func TestLicenseExpressionsAreEmittedCanonically(t *testing.T) { }) } } + +// Every algorithm the SDK knows an SPDX spelling for must survive export. +// +// This is a differential test on purpose. Referencing constants makes a rename +// a compile error and does nothing about an addition, which is exactly how a +// hand-written switch here came to know nine algorithms against the registry's +// nineteen -- dropping BLAKE2b, BLAKE3, MD2, MD4, MD6, ADLER32 and Streebog +// checksums on export without a word. Reading the registry means the next +// algorithm added upstream fails this test rather than disappearing. +func TestEverySPDXKnownDigestAlgorithmIsEmitted(t *testing.T) { + var checked int + for _, algorithm := range sdk.DigestAlgorithms() { + spdxName := algorithm.SPDXName() + if spdxName == "" { + continue // SPDX does not define this one; the format's limit, not ours. + } + checked++ + got := spdxChecksumAlgorithm(string(algorithm)) + if string(got) != spdxName { + t.Errorf("%s renders as %q, want SPDX's %q", algorithm, got, spdxName) + } + } + if checked < 10 { + t.Fatalf("only %d algorithms were checked; the registry looks unread", checked) + } +} + +// Every algorithm the SDK knows a CycloneDX spelling for must survive export, +// and must go out in that spelling. +// +// The mirror of TestEverySPDXKnownDigestAlgorithmIsEmitted, and it exists for +// the same reason twice over: a hand-written switch here knew eight algorithms +// against the registry's nineteen, and the external-reference path cast the +// SDK token straight into the enum, writing "sha256" where the schema says +// "SHA-256". Differential on purpose -- a constant reference makes a rename a +// compile error and says nothing about an addition. +func TestEveryCycloneDXKnownDigestAlgorithmIsEmitted(t *testing.T) { + var checked int + for _, algorithm := range sdk.DigestAlgorithms() { + name := algorithm.CycloneDXName() + if name == "" { + continue // CycloneDX does not define this one; the format's limit. + } + checked++ + if got := cycloneDXHashAlgorithm(string(algorithm)); string(got) != name { + t.Errorf("%s renders as %q, want CycloneDX's %q", algorithm, got, name) + } + } + if checked < 10 { + t.Fatalf("only %d algorithms were checked; the registry looks unread", checked) + } +} + +// An algorithm CycloneDX has no name for is omitted rather than written in a +// spelling the schema rejects. +func TestUnmappableDigestIsOmittedFromCycloneDXReferences(t *testing.T) { + hashes := cycloneDXEmittedHashes([]sdk.Digest{ + {Algorithm: sdk.DigestAlgorithmADLER32, Value: "0badf00d"}, + {Algorithm: sdk.DigestAlgorithmSHA256, Value: "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"}, + }) + if hashes == nil { + t.Fatal("the sha-256 hash was dropped along with the unmappable one") + } + for _, h := range *hashes { + if h.Algorithm == "adler32" || h.Algorithm == "ADLER32" { + t.Errorf("an algorithm CycloneDX does not define was emitted: %+v", h) + } + if h.Algorithm == "sha256" { + t.Errorf("the SDK token was emitted instead of CycloneDX's spelling: %+v", h) + } + } +} diff --git a/internal/sbom/model.go b/internal/sbom/model.go index f98003fb..314985ba 100644 --- a/internal/sbom/model.go +++ b/internal/sbom/model.go @@ -120,6 +120,19 @@ type Document struct { // same field as Sources below. Assertions sdk.DocumentAssertions + // UnknownScopeTokens are the scope-carrier tokens an ingested document + // named that this build does not recognize, deduplicated and sorted. + // + // The carrier is Bomly's own, so an unreadable token is most likely one a + // newer Bomly wrote. The scopes beside it are still kept -- that is the + // point of the SDK's lenient read -- but the fact that something was not + // read is worth telling a user, and this package has no logger of its + // own. The SBOM detector is where it becomes a warning. + // + // Empty for a document Bomly wrote and for a document with no carrier at + // all. + UnknownScopeTokens []string + // Sources are the documents this one was built from, one per ingested // SBOM, in entry order. // @@ -171,17 +184,29 @@ type Component struct { // and a second copy here is how the forward and reverse directions came // to disagree in the first place. // - // One clause of ADR-0037 is still not implemented here: a source - // document's own scalar -- CycloneDX's "optional", say -- is not - // preserved for verbatim re-emission. The SDK side of that is done -- - // DependencyNode.SourceScope carries the word and CycloneDXScopeForExport - // decides when to re-emit it -- but ingest here does not populate the - // field, so the word is still lost. "optional" reads as development - // (bomly-dev/bomly-sdk#63) and re-exports as "excluded", collapsing two - // source words the document kept apart. Tracked as - // bomly-dev/bomly-sdk#57. + // A source document's own scalar rides beside the set in SourceScope, so + // the derived set is never the only record of what the document said. Scopes []sdk.Scope + // SourceScope is the scope word the source document used, in that + // document's own vocabulary -- CycloneDX's "optional", say. It is a + // preserved claim and never an input to filtering: Scopes is what the + // pipeline reads, and the SDK derives it from this. + // + // ADR-0037 asks that the word be re-emitted verbatim unless Bomly's own + // scope set changed, so "optional" and "excluded" do not collapse into + // "required" across a round trip that asserted neither. Whether the word + // still means what the set means is sdk.CycloneDXScopeForExport's + // decision, not this package's -- the mapping that read the word in is + // the only thing that can say whether it still describes the set, and a + // second copy of it here is how the two directions came to disagree + // before. + // + // Only CycloneDX has a scalar to hold it. SPDX 2.3 has no scope field at + // all, so an SPDX export carries the set in its comment carrier and the + // source word stops there. + SourceScope string + PURL string Ecosystem string PackageManager string diff --git a/internal/sbom/sbom_test.go b/internal/sbom/sbom_test.go index 78588a53..6bb21b0d 100644 --- a/internal/sbom/sbom_test.go +++ b/internal/sbom/sbom_test.go @@ -866,3 +866,29 @@ func mustDep(t testing.TB, node sdk.GraphNode) *sdk.DependencyNode { } return dep } + +// The purl-type to ecosystem join is the SDK's, not a local reassembly of +// purlkit calls. A CLI copy had already fallen behind: the SDK consults a +// second lookup for manager-name aliases, so "swiftpm" resolved to the swift +// ecosystem there and to unknown here. +func TestEcosystemFromPURLTypeDelegatesToTheSDK(t *testing.T) { + for _, testCase := range []struct { + purlType string + want sdk.Ecosystem + }{ + {"golang", sdk.EcosystemGo}, + {"npm", sdk.EcosystemNPM}, + {"swiftpm", sdk.EcosystemSwift}, + {"", sdk.EcosystemUnknown}, + {"nothing-like-this", sdk.EcosystemUnknown}, + } { + t.Run(testCase.purlType, func(t *testing.T) { + if got := ecosystemFromPURLType(testCase.purlType); got != testCase.want { + t.Errorf("ecosystem = %q, want %q", got, testCase.want) + } + if got, want := ecosystemFromPURLType(testCase.purlType), sdk.EcosystemForPURLType(testCase.purlType); got != want { + t.Errorf("diverged from the SDK: %q vs %q", got, want) + } + }) + } +} diff --git a/internal/sbom/scope_carrier.go b/internal/sbom/scope_carrier.go new file mode 100644 index 00000000..7cb3bd03 --- /dev/null +++ b/internal/sbom/scope_carrier.go @@ -0,0 +1,63 @@ +package sbom + +import ( + "sort" + + "github.com/bomly-dev/bomly-sdk" +) + +// This file reads Bomly's own scope carrier out of an ingested document and +// collects the tokens this build could not read, so they reach a user instead +// of vanishing (ADR-0037). +// +// The carrier is a `bomly:scopes` CycloneDX property and the `scope=` field of +// the SPDX package comment. Both hold the same value, and the SDK owns what it +// means in both directions -- nothing here parses a token. + +// unknownScopeTokens returns the scope tokens a carrier named that this build +// does not recognize. +// +// The read is lenient because the strict one was a forward-compatibility trap: +// a newer Bomly writing one token an older one does not know made the older +// one drop the whole assertion. CycloneDX could fall back to its native +// scalar; SPDX has no scalar, so a component the document scoped +// "runtime,future" ended up unscoped altogether -- the loss +// bomly-dev/bomly-sdk#64 recorded, closed by the SDK's DecodeScopeSetLenient. +// +// A carrier that is malformed outright -- an empty entry, a token that is not +// shaped like a scope token at all -- yields nothing rather than a list of +// junk to warn about. The SDK draws that line; this only reports what it +// hands back. +func unknownScopeTokens(carrier string) []string { + decoded, err := sdk.DecodeScopeSetLenient(carrier) + if err != nil { + return nil + } + return decoded.Unknown +} + +// mergeUnknownScopeTokens folds tokens into a document's collected set, +// deduplicated and sorted. +// +// Sorted because the list reaches a log line and a test golden, and one +// document's components are read in map order in one codec and slice order in +// the other; an unstable list would make the same document warn differently +// between runs. +func mergeUnknownScopeTokens(collected, tokens []string) []string { + if len(tokens) == 0 { + return collected + } + seen := make(map[string]struct{}, len(collected)+len(tokens)) + for _, token := range collected { + seen[token] = struct{}{} + } + for _, token := range tokens { + if _, duplicate := seen[token]; duplicate { + continue + } + seen[token] = struct{}{} + collected = append(collected, token) + } + sort.Strings(collected) + return collected +} diff --git a/internal/sbom/scope_round_trip_test.go b/internal/sbom/scope_round_trip_test.go index e219749b..aa3e4ee0 100644 --- a/internal/sbom/scope_round_trip_test.go +++ b/internal/sbom/scope_round_trip_test.go @@ -287,3 +287,173 @@ func TestUnscopedSPDXPackagesSurviveARuntimeFilterAndAreReported(t *testing.T) { t.Errorf("report = %v, want the one package named: the filter narrowed nothing and must say so", report.Unasserted) } } + +// cycloneDXComponentScope reads the scalar scope a rendered document wrote. +func cycloneDXComponentScope(t *testing.T, raw []byte) cdx.Scope { + t.Helper() + var bom cdx.BOM + if err := json.Unmarshal(raw, &bom); err != nil { + t.Fatalf("decode: %v", err) + } + if bom.Components == nil || len(*bom.Components) == 0 { + t.Fatalf("no components:\n%s", raw) + } + return (*bom.Components)[0].Scope +} + +// foreignCycloneDX is a document Bomly did not write: a native scalar scope +// and no carrier property beside it. +func foreignCycloneDX(scope string) string { + return `{ + "bomFormat": "CycloneDX", + "specVersion": "1.5", + "version": 1, + "components": [ + {"bom-ref": "pkg:npm/widget@1.0.0", "type": "library", "name": "widget", + "version": "1.0.0", "purl": "pkg:npm/widget@1.0.0", "scope": "` + scope + `"} + ] +}` +} + +// A source document's own scope word comes back out unchanged. +// +// It could not before: the model had nowhere to keep a source-asserted scope +// beside the set it derives, so a component a document marked "optional" was +// re-exported as "required" -- a claim about shipping code that the source had +// deliberately not made (bomly-dev/bomly-sdk#57). Every word in CycloneDX's +// vocabulary is covered, including "excluded", which read as development and +// projected back to "excluded" by luck rather than by preservation. +func TestSourceScopeWordSurvivesACycloneDXRoundTrip(t *testing.T) { + for _, word := range []string{"required", "optional", "excluded"} { + t.Run(word, func(t *testing.T) { + doc, _, err := UnmarshalAutoJSON([]byte(foreignCycloneDX(word))) + if err != nil { + t.Fatalf("ingest: %v", err) + } + if got := componentNamed(t, doc, "widget").SourceScope; got != word { + t.Fatalf("ingested source scope = %q, want %q", got, word) + } + + g, err := ToGraph(doc) + if err != nil { + t.Fatalf("to graph: %v", err) + } + raw, err := MarshalDepGraphJSON(g, TargetCycloneDX16JSON, BuildOptions{}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("export: %v", err) + } + if got := cycloneDXComponentScope(t, raw); string(got) != word { + t.Errorf("re-exported scope = %q, want the source's own word %q\n%s", got, word, raw) + } + }) + } +} + +// When Bomly's own scope set no longer means what the source's word meant, +// the projection is written instead. The word is preserved, not obeyed: it +// describes a set, and a set that changed is no longer the one it describes. +func TestSourceScopeYieldsToTheProjectionWhenTheSetChanges(t *testing.T) { + doc, _, err := UnmarshalAutoJSON([]byte(foreignCycloneDX("optional"))) + if err != nil { + t.Fatalf("ingest: %v", err) + } + g, err := ToGraph(doc) + if err != nil { + t.Fatalf("to graph: %v", err) + } + nodes := g.DependencyNodes() + if len(nodes) != 1 { + t.Fatalf("nodes = %d", len(nodes)) + } + // What propagation does when the package turns out to be reachable from a + // runtime root as well: the set now says something "optional" does not. + // + // It has to be runtime specifically. "optional" already means development + // on its own, so adding development leaves the set exactly what the word + // described and the word is rightly re-emitted -- which is the case the + // test above covers, not this one. + nodes[0].Scopes = append(nodes[0].Scopes, sdk.ScopeRuntime) + + raw, err := MarshalDepGraphJSON(g, TargetCycloneDX16JSON, BuildOptions{}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("export: %v", err) + } + if got := cycloneDXComponentScope(t, raw); got != cdx.ScopeRequired { + t.Errorf("scope = %q, want the projection %q once the set changed\n%s", got, cdx.ScopeRequired, raw) + } +} + +// A carrier naming one token this build does not know keeps the scopes it +// does know, and reports the token. +// +// The strict read was a forward-compatibility trap: one unknown token from a +// newer Bomly dropped the whole assertion. CycloneDX could fall back to its +// scalar; SPDX has no scalar, so the component ended up unscoped altogether -- +// the loss bomly-dev/bomly-sdk#64 recorded. +func TestUnknownScopeTokenKeepsTheKnownScopes(t *testing.T) { + spdxWithCarrier := `{ + "spdxVersion": "SPDX-2.3", + "dataLicense": "CC0-1.0", + "SPDXID": "SPDXRef-DOCUMENT", + "name": "n", + "documentNamespace": "https://acme.example/spdx/n", + "creationInfo": {"created": "2026-01-02T03:04:05Z", "creators": ["Tool: t"]}, + "packages": [ + { + "SPDXID": "SPDXRef-widget", + "name": "widget", + "versionInfo": "1.0.0", + "comment": "bomly:scope=runtime,future-scope", + "externalRefs": [ + {"referenceCategory": "PACKAGE-MANAGER", "referenceType": "purl", "referenceLocator": "pkg:npm/widget@1.0.0"} + ] + } + ] +}` + cycloneDXWithCarrier := `{ + "bomFormat": "CycloneDX", + "specVersion": "1.5", + "version": 1, + "components": [ + {"bom-ref": "pkg:npm/widget@1.0.0", "type": "library", "name": "widget", + "version": "1.0.0", "purl": "pkg:npm/widget@1.0.0", "scope": "excluded", + "properties": [{"name": "bomly:scopes", "value": "runtime,future-scope"}]} + ] +}` + for name, raw := range map[string]string{"spdx": spdxWithCarrier, "cyclonedx": cycloneDXWithCarrier} { + t.Run(name, func(t *testing.T) { + doc, _, err := UnmarshalAutoJSON([]byte(raw)) + if err != nil { + t.Fatalf("ingest: %v", err) + } + got := componentScopes(t, doc, "widget") + if len(got) != 1 || got[0] != sdk.ScopeRuntime { + t.Fatalf("scopes = %v, want the token this build can read", got) + } + if len(doc.UnknownScopeTokens) != 1 || doc.UnknownScopeTokens[0] != "future-scope" { + t.Errorf("unknown tokens = %v, want the one that was dropped", doc.UnknownScopeTokens) + } + }) + } +} + +// A document Bomly wrote names no unknown tokens, so the warning stays quiet +// on the ordinary path. +func TestAKnownCarrierReportsNoUnknownTokens(t *testing.T) { + for _, target := range []Target{TargetSPDX23JSON, TargetCycloneDX16JSON} { + t.Run(string(target), func(t *testing.T) { + g := scopedGraph(t, sdk.ScopeRuntime, sdk.ScopeDevelopment) + raw, err := MarshalDepGraphJSON(g, target, BuildOptions{}, EncodeOptions{Pretty: true}) + if err != nil { + t.Fatalf("export: %v", err) + } + doc, _, err := UnmarshalAutoJSON(raw) + if err != nil { + t.Fatalf("ingest: %v", err) + } + if len(doc.UnknownScopeTokens) != 0 { + t.Errorf("unknown tokens = %v, want none", doc.UnknownScopeTokens) + } + }) + } +} diff --git a/internal/sbom/spdx23.go b/internal/sbom/spdx23.go index 6f754985..54f6fc2c 100644 --- a/internal/sbom/spdx23.go +++ b/internal/sbom/spdx23.go @@ -133,11 +133,15 @@ func (spdx23Codec) encodeJSON(doc *Document, opts EncodeOptions) ([]byte, error) SPDXIdentifier: common.ElementID("DOCUMENT"), DocumentName: doc.NameOrDefault(), DocumentNamespace: doc.NamespaceOrDefault(), - CreationInfo: creation, - DocumentComment: doc.Assertions.Comment, - Packages: packages, - Relationships: relationships, - OtherLicenses: spdxOtherLicenses(extractedLicenses), + // The documents this one was built from, named rather than inherited. + // Empty for a native scan and for a conversion that adopted its single + // source's identity; populated for a merge. + ExternalDocumentReferences: spdxSourceLinks(doc), + CreationInfo: creation, + DocumentComment: doc.Assertions.Comment, + Packages: packages, + Relationships: relationships, + OtherLicenses: spdxOtherLicenses(extractedLicenses), } return marshalJSON(spdxDoc, opts.Pretty) @@ -152,16 +156,19 @@ func (spdx23Codec) decodeJSON(data []byte) (*Document, error) { extractedByRef := spdxExtractedTexts(spdxDoc.OtherLicenses) components := make([]Component, 0, len(spdxDoc.Packages)) + var unknownScopes []string for _, p := range spdxDoc.Packages { if p == nil { continue } id := common.RenderElementID(p.PackageSPDXIdentifier) + scopes, unknown := spdxCommentScopes(p.PackageComment) + unknownScopes = mergeUnknownScopeTokens(unknownScopes, unknown) component := Component{ ID: id, Name: p.PackageName, Version: p.PackageVersion, - Scopes: spdxCommentScopes(p.PackageComment), + Scopes: scopes, Type: parseSPDXComponentType(p), PURL: parseSPDXPURL(p.PackageExternalReferences), Ecosystem: parseSPDXYcosystem(p.PackageExternalReferences), @@ -220,15 +227,16 @@ func (spdx23Codec) decodeJSON(data []byte) (*Document, error) { sort.Strings(roots) return &Document{ - Name: spdxDoc.DocumentName, - Namespace: spdxDoc.DocumentNamespace, - Assertions: spdxDocumentAssertions(&spdxDoc), - Tool: extractSPDXToolName(spdxDoc.CreationInfo), - Tools: extractSPDXToolNames(spdxDoc.CreationInfo), - Created: parseSPDXCreated(spdxDoc.CreationInfo), - Components: components, - Dependencies: dependencies, - Roots: roots, + Name: spdxDoc.DocumentName, + Namespace: spdxDoc.DocumentNamespace, + Assertions: spdxDocumentAssertions(&spdxDoc), + Tool: extractSPDXToolName(spdxDoc.CreationInfo), + Tools: extractSPDXToolNames(spdxDoc.CreationInfo), + Created: parseSPDXCreated(spdxDoc.CreationInfo), + Components: components, + Dependencies: dependencies, + Roots: roots, + UnknownScopeTokens: unknownScopes, }, nil } @@ -372,29 +380,24 @@ func spdxChecksums(digests []Digest) []common.Checksum { return out } +// spdxChecksumAlgorithm renders a digest algorithm in SPDX's spelling. +// +// The registry is the SDK's, not a list here. A hand-written switch stood in +// this spot and knew nine algorithms against the registry's nineteen, so a +// document carrying BLAKE2b, BLAKE3, MD2, MD4, MD6, ADLER32 or Streebog had +// that checksum silently dropped on export -- correct the day it was written +// and quietly lossy once the vocabulary grew. That is the failure this +// project's delegation rule exists to prevent, and Streebog is the example it +// cites. +// +// An algorithm SPDX does not define returns "", which the caller drops. That +// is a real limit of the format rather than a gap in this mapping. func spdxChecksumAlgorithm(algorithm string) common.ChecksumAlgorithm { - switch strings.ToLower(strings.TrimSpace(algorithm)) { - case "md5": - return common.MD5 - case "sha1", "sha-1": - return common.SHA1 - case "sha224", "sha-224": - return common.SHA224 - case "sha256", "sha-256": - return common.SHA256 - case "sha384", "sha-384": - return common.SHA384 - case "sha512", "sha-512": - return common.SHA512 - case "sha3-256": - return common.SHA3_256 - case "sha3-384": - return common.SHA3_384 - case "sha3-512": - return common.SHA3_512 - default: + parsed, err := sdk.ParseDigestAlgorithm(algorithm) + if err != nil { return "" } + return common.ChecksumAlgorithm(parsed.SPDXName()) } func parseSPDXComponentType(p *v23.Package) string { @@ -407,24 +410,27 @@ func parseSPDXComponentType(p *v23.Package) string { return strings.ToLower(strings.TrimSpace(p.PrimaryPackagePurpose)) } -// spdxCommentScopes reads the scope set back out of the package comment. +// spdxCommentScopes reads the scope set back out of the package comment, and +// reports the tokens this build could not read. // // A carrier that will not parse is treated as absent rather than as an error: // this is a comment field on someone else's document, and refusing the whole // component because a Bomly-shaped comment was malformed would lose more than // it protects. The SDK owns what the carrier means in both directions. -// One consequence is not what ADR-0037 asks for. The SDK's decode is -// all-or-nothing, so a carrier naming one known scope beside one unknown token -// yields nothing rather than the known scope plus a warning, and unlike -// CycloneDX there is no native scalar here to fall back to -- the component -// ends up unscoped. Tracked as bomly-dev/bomly-sdk#64; partial decode belongs -// beside the grammar in the SDK rather than being re-derived here. -func spdxCommentScopes(comment string) []sdk.Scope { - scopes, err := sdk.DecodeScopeSet(parseSPDXCommentField(comment, "scope")) +// +// The read is the SDK's lenient one, which is what ADR-0037 asks for: the +// scopes this build knows are kept even when a token beside them is not. SPDX +// has no native scalar to fall back on, so the strict read made a component +// scoped "runtime,future" unscoped outright -- total loss from one token a +// newer Bomly wrote. The tokens that were not read come back so a caller can +// say so; they are the SDK's warning channel, since it does not log. +func spdxCommentScopes(comment string) ([]sdk.Scope, []string) { + carrier := parseSPDXCommentField(comment, "scope") + decoded, err := sdk.DecodeScopeSetLenient(carrier) if err != nil { - return nil + return nil, nil } - return scopes + return decoded.Scopes, decoded.Unknown } func parseSPDXCommentField(comment, field string) string { diff --git a/internal/sbom/spdx23_assertions.go b/internal/sbom/spdx23_assertions.go index f21be379..948d7792 100644 --- a/internal/sbom/spdx23_assertions.go +++ b/internal/sbom/spdx23_assertions.go @@ -1,6 +1,7 @@ package sbom import ( + "fmt" "strconv" "strings" @@ -347,6 +348,7 @@ func spdxDocumentAssertions(doc *v23.Document) sdk.DocumentAssertions { Name: doc.DocumentName, DataLicense: doc.DataLicense, Comment: doc.DocumentComment, + Sources: spdxIngestedSources(doc.ExternalDocumentReferences), } if doc.CreationInfo != nil { assertions.Created = doc.CreationInfo.Created @@ -435,3 +437,102 @@ func spdxDocumentCreators(doc *Document) []common.Creator { } return creators } + +// spdxSourceLinks renders the documents this one was built from as SPDX +// externalDocumentRefs. +// +// This is the half ADR-0042 had to leave out. SPDX names another document +// through externalDocumentRefs, and section 6.6 makes the checksum mandatory +// on every entry -- so until DocumentAssertions could carry one +// (bomly-dev/bomly-sdk#55), a merged SPDX document could name nothing at all +// while the CycloneDX half already linked its sources. The checksum is +// captured at ingest by decodeDocument, over the source document's original +// bytes, because it cannot be recovered from the parsed model afterwards. +// +// A source with no checksum is skipped rather than written with an empty one: +// an entry missing the field is an invalid document, and a validator rejecting +// the whole export is a worse outcome than one unnamed source. That happens +// only for a source that reached the entry without passing through ingest -- +// a detector or plugin writing DocumentAssertions directly. +// +// The reference id is minted by sanitizeSPDXID, the same rule this package +// already uses for package element ids, with the same collision suffix. It is +// declined delegation deliberately: tools-golang owns the "DocumentRef-" +// prefix and the wire shape (common.DocumentID applies both), and the SDK's +// spdxkit mints only LicenseRef ids from license text; neither offers an +// idstring minter for a document identity. Reusing this package's existing +// rule keeps one answer rather than a second one. +func spdxSourceLinks(doc *Document) []v23.ExternalDocumentRef { + // SPDX writes a namespace and no serial, so the namespace is the only + // identity a reader of this document can see, and the only one a source + // link could be redundant with. + links := documentSourceLinks(doc, documentIdentity{Namespace: doc.NamespaceOrDefault()}) + if len(links) == 0 { + return nil + } + refs := make([]v23.ExternalDocumentRef, 0, len(links)) + usedIDs := make(map[string]int, len(links)) + for _, link := range links { + if link.Checksum == nil { + continue + } + // Re-gated on the way out: a link tuple reaching here from a graph + // entry never passed a decoder. + checksum, ok := link.Checksum.Normalized() + if !ok { + continue + } + algorithm := checksum.Algorithm.SPDXName() + if algorithm == "" { + // The SDK's registry says SPDX does not define this algorithm. + // Writing the canonical token instead would publish a value the + // specification does not list. + continue + } + base := sanitizeSPDXID(link.Identity) + seq := usedIDs[base] + usedIDs[base] = seq + 1 + if seq > 0 { + base = fmt.Sprintf("%s-%d", base, seq) + } + refs = append(refs, v23.ExternalDocumentRef{ + // Without the "DocumentRef-" prefix: common.DocumentID adds it on + // the way out and strips it on the way in. + DocumentRefID: common.DocumentID(base), + URI: link.Identity, + Checksum: common.Checksum{ + Algorithm: common.ChecksumAlgorithm(algorithm), + Value: checksum.Value, + }, + }) + } + if len(refs) == 0 { + return nil + } + return refs +} + +// spdxIngestedSources reads the documents an SPDX document says it was built +// from, so a second conversion can name them too (bomly-dev/bomly-sdk#61). +// +// The checksum comes back with the reference, which is what makes an SPDX +// source usable as a link on the next export in either format. The algorithm +// spelling is SPDX's; the SDK's digest registry resolves it, so no mapping is +// written here. +func spdxIngestedSources(refs []v23.ExternalDocumentRef) []sdk.DocumentSource { + if len(refs) == 0 { + return nil + } + sources := make([]sdk.DocumentSource, 0, len(refs)) + for _, ref := range refs { + source := sdk.DocumentSource{Identity: ref.URI} + if checksum, ok := (sdk.Digest{ + Algorithm: sdk.DigestAlgorithm(ref.Checksum.Algorithm), + Value: ref.Checksum.Value, + }).Normalized(); ok { + source.Checksum = &checksum + } + sources = append(sources, source) + } + return sources +} diff --git a/internal/sbom/transform.go b/internal/sbom/transform.go index c265323c..70169866 100644 --- a/internal/sbom/transform.go +++ b/internal/sbom/transform.go @@ -94,6 +94,7 @@ func FromGraphEntries(g *sdk.Graph, entries []sdk.GraphEntry, opts BuildOptions) } if dep, isDep := pkg.(*sdk.DependencyNode); isDep { component.Scopes = append([]sdk.Scope(nil), dep.Scopes...) + component.SourceScope = dep.SourceScope component.Copyright = dep.Copyright component.Licenses = componentLicenses(sdk.DetectionLicenses(dep)) component.Digests = componentDigests(dep.Digests) diff --git a/test/smoke/testdata/golden/scan-swiftpm.golden.json b/test/smoke/testdata/golden/scan-swiftpm.golden.json index 3afeb2d5..85531f80 100644 --- a/test/smoke/testdata/golden/scan-swiftpm.golden.json +++ b/test/smoke/testdata/golden/scan-swiftpm.golden.json @@ -10,7 +10,7 @@ "pkg:swift/github.com/apple/swift-atomics@1.3.1", "pkg:swift/github.com/apple/swift-collections@1.1.4", "pkg:swift/github.com/apple/swift-crypto@3.3.0", - "pkg:swift/github.com/apple/swift-http-types@1.7.0", + "pkg:swift/github.com/apple/swift-http-types@1.8.0", "pkg:swift/github.com/apple/swift-log@1.5.4", "pkg:swift/github.com/apple/swift-metrics@2.11.0", "pkg:swift/github.com/apple/swift-nio-extras@1.22.0", @@ -169,7 +169,7 @@ }, { "depends_on": [], - "id": "pkg:swift/github.com/apple/swift-http-types@1.7.0", + "id": "pkg:swift/github.com/apple/swift-http-types@1.8.0", "licenses": [], "locations": [ { @@ -187,12 +187,12 @@ } ], "name": "github.com/apple:swift-http-types", - "package_ref": "pkg:swift/github.com/apple/swift-http-types@1.7.0", - "purl": "pkg:swift/github.com/apple/swift-http-types@1.7.0", + "package_ref": "pkg:swift/github.com/apple/swift-http-types@1.8.0", + "purl": "pkg:swift/github.com/apple/swift-http-types@1.8.0", "scopes": [ "runtime" ], - "version": "1.7.0" + "version": "1.8.0" }, { "depends_on": [], @@ -696,8 +696,8 @@ "licenses": [], "name": "github.com/apple:swift-http-types", "org": "github.com/apple", - "purl": "pkg:swift/github.com/apple/swift-http-types@1.7.0", - "version": "1.7.0", + "purl": "pkg:swift/github.com/apple/swift-http-types@1.8.0", + "version": "1.8.0", "vulnerabilities": [] }, {