From 5e0e275ec4672c4c3757f3380ae0e6e8ee3eaccf Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Wed, 9 Sep 2026 00:06:18 -0700 Subject: [PATCH 1/3] fix(sbom): the digest vocabulary is the SDK registry's, not local tables Both SBOM formats close their hash enumeration, so an algorithm Bomly cannot name in the target format cannot be published at all -- which makes the mapping, not the encoder, the thing that decides whether a digest survives export. This package had three transcriptions of that mapping and a fourth site that skipped it, and between them they dropped or corrupted every algorithm registered after the switches were written. bomly-sdk v0.9.5 owns the vocabulary in digest.go: ParseDigestAlgorithm resolves any spelling to the canonical token, SPDXName and CycloneDXName render each format's, and the registry references spdx/tools-golang's and cyclonedx-go's own constants rather than copying them -- guarded upstream by digest_registry_test.go, so a member added to either specification fails a build instead of disappearing at runtime. - spdxChecksums delegated; spdxChecksumAlgorithm deleted. It omitted BLAKE2b-256/384/512, BLAKE3, MD2, MD4, MD6, and ADLER32, all of which SPDX 2.3 defines, so a document carrying one lost its checksum. - cycloneDXHashes delegated; cycloneDXHashAlgorithm deleted. Same shape, same omissions, plus Streebog -- the pair this defect class already cost once. It now also drops the SPDX-only members (SHA224, MD2, MD4, MD6, ADLER32) that an ingested SPDX document can carry, rather than writing an algorithm CycloneDX does not define. - cycloneDXEmittedHashes wrote the SDK's canonical token straight into cdx.Hash.Algorithm, so reference hashes exported as "sha256" where the schema says "SHA-256" and an ingested document changed on its second export. This is the ExternalReferenceCategory.SPDXName defect in the digest vocabulary; it now renders through CycloneDXName. - digestHexSizes is keyed by the canonical token instead of listing a row per spelling. The lengths stay local -- the SDK deliberately records no per-algorithm value length, because ecosystems publish digests in hex, in base64, and over subjects that are not files -- but the spellings were a fourth copy of the vocabulary. publishableDigest gives the rule one home: resolve the algorithm, render it in this format, or omit the digest. Both encoders pass a method expression, so they ask the same registry the same question and only the rendering differs. TestExportProjectsEveryRegisteredDigestAlgorithm is the guard. Referencing constants makes a rename a compile error but says nothing about an addition, and an addition is how Streebog was lost: the test enumerates sdk.DigestAlgorithms() and asserts each member either exports in its format's spelling or is absent from that format, so a switch written back in by hand fails here rather than in a user's document. The three defect tests were confirmed to fail against the pre-change code. No golden moves: the goldens carry only sha1/sha256/sha512, whose spellings are unchanged, and the three SBOM smoke tests pass against them. Co-Authored-By: Claude Opus 5 --- internal/sbom/cyclonedx.go | 37 ++---- internal/sbom/cyclonedx_assertions.go | 14 +- internal/sbom/digest.go | 40 ++++++ internal/sbom/digest_test.go | 184 ++++++++++++++++++++++++++ internal/sbom/spdx23.go | 34 +---- internal/sbom/transform.go | 39 +++--- 6 files changed, 273 insertions(+), 75 deletions(-) create mode 100644 internal/sbom/digest.go create mode 100644 internal/sbom/digest_test.go diff --git a/internal/sbom/cyclonedx.go b/internal/sbom/cyclonedx.go index a0d48568..ce3a2c78 100644 --- a/internal/sbom/cyclonedx.go +++ b/internal/sbom/cyclonedx.go @@ -564,47 +564,26 @@ func hasCompoundExpression(values []string) bool { return false } +// cycloneDXHashes maps component digests onto CycloneDX hashes, dropping +// entries whose algorithm CycloneDX has no member for -- SHA224, MD2, MD4, +// MD6, and ADLER32 are SPDX-only, and an SBOM ingested from SPDX can carry +// them. The vocabulary is the SDK registry's, not a local switch -- see +// publishableDigest. func cycloneDXHashes(digests []Digest) []cdx.Hash { if len(digests) == 0 { return nil } out := make([]cdx.Hash, 0, len(digests)) for _, d := range digests { - alg := cycloneDXHashAlgorithm(d.Algorithm) - if alg == "" || strings.TrimSpace(d.Value) == "" { + spelling, value, ok := publishableDigest(d, sdk.DigestAlgorithm.CycloneDXName) + if !ok { continue } - out = append(out, cdx.Hash{Algorithm: alg, Value: d.Value}) + out = append(out, cdx.Hash{Algorithm: cdx.HashAlgorithm(spelling), Value: value}) } return out } -// 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. -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: - return "" - } -} - func cycloneDXEOLProperties(eol *EOL) []cdx.Property { if eol == nil { return nil diff --git a/internal/sbom/cyclonedx_assertions.go b/internal/sbom/cyclonedx_assertions.go index 4bd691ed..a0b3ccb5 100644 --- a/internal/sbom/cyclonedx_assertions.go +++ b/internal/sbom/cyclonedx_assertions.go @@ -296,8 +296,20 @@ func cycloneDXEmittedHashes(digests []sdk.Digest) *[]cdx.Hash { if !ok { continue } + // CycloneDXName, not the algorithm's own string: the SDK holds the + // algorithm in its canonical form ("sha256", "blake2b-256") and + // renders CycloneDX's spelling ("SHA-256", "BLAKE2b-256") + // separately. Emitting the canonical form wrote an algorithm the + // specification does not define, and an ingested "SHA-256" changed + // case on its second export. An empty spelling means CycloneDX has + // no member for the algorithm -- SHA224, MD2, MD4, MD6, and ADLER32 + // are SPDX-only -- so the claim has nowhere to go here. + spelling := normalized.Algorithm.CycloneDXName() + if spelling == "" { + continue + } hashes = append(hashes, cdx.Hash{ - Algorithm: cdx.HashAlgorithm(normalized.Algorithm), + Algorithm: cdx.HashAlgorithm(spelling), Value: normalized.Value, }) } diff --git a/internal/sbom/digest.go b/internal/sbom/digest.go new file mode 100644 index 00000000..9177e09f --- /dev/null +++ b/internal/sbom/digest.go @@ -0,0 +1,40 @@ +package sbom + +import ( + "strings" + + "github.com/bomly-dev/bomly-sdk" +) + +// publishableDigest resolves a component digest's algorithm through the SDK +// registry and renders it in one format's spelling, or reports that the digest +// cannot be published here. +// +// name is a method expression -- sdk.DigestAlgorithm.SPDXName or +// sdk.DigestAlgorithm.CycloneDXName -- so both encoders ask the same registry +// the same question and only the rendering differs. The registry is sourced +// from spdx/tools-golang's and cyclonedx-go's own constants and guarded +// against upstream additions in the SDK; a switch transcribed here is correct +// until a specification grows a member, and then it silently drops the +// digests that use it. This package had two such switches, and between them +// they omitted BLAKE2b, BLAKE3, MD2/MD4/MD6, ADLER32, and Streebog. +// +// Both failure modes collapse to the same answer for a caller: an algorithm +// no format defines, and an algorithm this format has no member for, are both +// "omit this digest". Neither has a spelling that would validate, and SPDX and +// CycloneDX each close their hash enumeration. +func publishableDigest(d Digest, name func(sdk.DigestAlgorithm) string) (spelling, value string, ok bool) { + value = strings.TrimSpace(d.Value) + if value == "" { + return "", "", false + } + algorithm, err := sdk.ParseDigestAlgorithm(d.Algorithm) + if err != nil { + return "", "", false + } + spelling = name(algorithm) + if spelling == "" { + return "", "", false + } + return spelling, value, true +} diff --git a/internal/sbom/digest_test.go b/internal/sbom/digest_test.go new file mode 100644 index 00000000..118efbec --- /dev/null +++ b/internal/sbom/digest_test.go @@ -0,0 +1,184 @@ +package sbom + +import ( + "encoding/json" + "testing" + + cdx "github.com/CycloneDX/cyclonedx-go" + "github.com/bomly-dev/bomly-cli/internal/testnodes" + "github.com/bomly-dev/bomly-sdk" + "github.com/spdx/tools-golang/spdx/v2/common" + v23 "github.com/spdx/tools-golang/spdx/v2/v2_3" +) + +// digestFixtureGraph builds a one-package graph carrying the digests a caller +// hands it, spelled however the caller spelled them. +func digestFixtureGraph(t *testing.T, digests []sdk.Digest) *sdk.Graph { + t.Helper() + + g := sdk.New() + dep := testnodes.DepFrom(sdk.DependencyNode{ + Coordinates: sdk.Coordinates{Name: "left-pad", Version: "1.3.0", Ecosystem: sdk.EcosystemNPM}, + Digests: digests, + }) + if err := g.AddNode(dep); err != nil { + t.Fatalf("add node: %v", err) + } + return g +} + +// TestMarshalDepGraphJSON_SPDX23ChecksumsCoverTheWholeRegistry pins the +// algorithms a hand-written switch used to omit. SPDX 2.3 defines BLAKE3 and +// the BLAKE2b family; a document carrying one had its checksum silently +// dropped, because the export switch was a transcription of the vocabulary +// rather than a call into it. +func TestMarshalDepGraphJSON_SPDX23ChecksumsCoverTheWholeRegistry(t *testing.T) { + g := digestFixtureGraph(t, []sdk.Digest{ + {Algorithm: sdk.DigestAlgorithmBLAKE3, Value: "af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262"}, + // Spelled the way a CycloneDX document would spell it: the export + // resolves the spelling rather than matching it. + {Algorithm: "BLAKE2b-256", Value: "0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8"}, + {Algorithm: "SHA-256", Value: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + {Algorithm: "nuget-content-hash", Value: "abc123"}, + }) + + out, err := MarshalDepGraphJSON(g, TargetSPDX23JSON, BuildOptions{ProjectRoot: &ProjectRoot{Name: "demo"}}, EncodeOptions{}) + if err != nil { + t.Fatalf("marshal spdx: %v", err) + } + var d v23.Document + if err := json.Unmarshal(out, &d); err != nil { + t.Fatalf("unmarshal spdx: %v", err) + } + + got := map[common.ChecksumAlgorithm]string{} + for _, p := range d.Packages { + if p == nil || p.PackageName != "left-pad" { + continue + } + for _, checksum := range p.PackageChecksums { + got[checksum.Algorithm] = checksum.Value + } + } + for _, want := range []common.ChecksumAlgorithm{common.BLAKE3, common.BLAKE2b_256, common.SHA256} { + if got[want] == "" { + t.Fatalf("expected a %s checksum on the exported package, got %+v", want, got) + } + } + // An algorithm no format defines still has nowhere to go: SPDX closes the + // enumeration, so the digest is dropped rather than written unvalidatable. + if len(got) != 3 { + t.Fatalf("expected exactly the three registered checksums, got %+v", got) + } +} + +// TestMarshalDepGraphJSON_CycloneDXHashesCoverTheWholeRegistry is the same +// check on the other encoder, plus the asymmetry between the two vocabularies: +// SHA224 is an SPDX member CycloneDX has no spelling for, so a document +// ingested from SPDX cannot carry it out as CycloneDX. +func TestMarshalDepGraphJSON_CycloneDXHashesCoverTheWholeRegistry(t *testing.T) { + g := digestFixtureGraph(t, []sdk.Digest{ + {Algorithm: sdk.DigestAlgorithmBLAKE3, Value: "af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262"}, + {Algorithm: "blake2b-256", Value: "0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8"}, + {Algorithm: "SHA256", Value: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + {Algorithm: sdk.DigestAlgorithmSHA224, Value: "d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f"}, + }) + + out, err := MarshalDepGraphJSON(g, TargetCycloneDX16JSON, BuildOptions{ProjectRoot: &ProjectRoot{Name: "demo"}}, EncodeOptions{}) + if err != nil { + t.Fatalf("marshal cyclonedx: %v", err) + } + var bom cdx.BOM + if err := json.Unmarshal(out, &bom); err != nil { + t.Fatalf("unmarshal cyclonedx: %v", err) + } + + got := map[cdx.HashAlgorithm]string{} + if bom.Components != nil { + for _, comp := range *bom.Components { + if comp.Name != "left-pad" || comp.Hashes == nil { + continue + } + for _, hash := range *comp.Hashes { + got[hash.Algorithm] = hash.Value + } + } + } + for _, want := range []cdx.HashAlgorithm{cdx.HashAlgoBlake3, cdx.HashAlgoBlake2b_256, cdx.HashAlgoSHA256} { + if got[want] == "" { + t.Fatalf("expected a %s hash on the exported component, got %+v", want, got) + } + } + if len(got) != 3 { + t.Fatalf("expected the SPDX-only SHA224 digest to be dropped, got %+v", got) + } +} + +// TestCycloneDXEmittedHashes_RendersCycloneDXSpelling covers the reference +// hashes an assertion carries. The SDK holds an algorithm in its canonical +// form, which is not what CycloneDX calls it: emitting that form directly +// wrote "sha256" where the schema defines "SHA-256", so an ingested document +// changed on its second export. +func TestCycloneDXEmittedHashes_RendersCycloneDXSpelling(t *testing.T) { + hashes := cycloneDXEmittedHashes([]sdk.Digest{ + {Algorithm: "SHA-256", Value: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + {Algorithm: sdk.DigestAlgorithmBLAKE2b256, Value: "0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8"}, + // SPDX-only, so this reference claim cannot be published here. + {Algorithm: sdk.DigestAlgorithmADLER32, Value: "0424016d"}, + }) + if hashes == nil { + t.Fatal("expected hashes") + } + if len(*hashes) != 2 { + t.Fatalf("expected the ADLER32 claim dropped, got %+v", *hashes) + } + if (*hashes)[0].Algorithm != cdx.HashAlgoSHA256 { + t.Fatalf("expected CycloneDX's spelling, got %q", (*hashes)[0].Algorithm) + } + if (*hashes)[1].Algorithm != cdx.HashAlgoBlake2b_256 { + t.Fatalf("expected CycloneDX's spelling, got %q", (*hashes)[1].Algorithm) + } +} + +// TestExportProjectsEveryRegisteredDigestAlgorithm is the guard on the +// delegation. Referencing the SDK registry makes a rename a compile error but +// says nothing about an addition -- and an addition is how these vocabularies +// lose data: a spelling the table never learned is dropped by the gate that +// exists to reject unpublishable values. Enumerating the registry rather than +// listing algorithms here means a member added upstream, or a switch written +// back in by hand, fails this test instead of disappearing at runtime. +func TestExportProjectsEveryRegisteredDigestAlgorithm(t *testing.T) { + const value = "0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8" + + for _, algorithm := range sdk.DigestAlgorithms() { + digest := Digest{Algorithm: string(algorithm), Value: value} + + checksums := spdxChecksums([]Digest{digest}) + switch spelling := algorithm.SPDXName(); spelling { + case "": + if len(checksums) != 0 { + t.Errorf("%s has no SPDX member but exported %+v", algorithm, checksums) + } + default: + if len(checksums) != 1 { + t.Errorf("%s: expected one SPDX checksum, got %+v", algorithm, checksums) + } else if string(checksums[0].Algorithm) != spelling { + t.Errorf("%s: exported as %q, want %q", algorithm, checksums[0].Algorithm, spelling) + } + } + + hashes := cycloneDXHashes([]Digest{digest}) + switch spelling := algorithm.CycloneDXName(); spelling { + case "": + if len(hashes) != 0 { + t.Errorf("%s has no CycloneDX member but exported %+v", algorithm, hashes) + } + default: + if len(hashes) != 1 { + t.Errorf("%s: expected one CycloneDX hash, got %+v", algorithm, hashes) + } else if string(hashes[0].Algorithm) != spelling { + t.Errorf("%s: exported as %q, want %q", algorithm, hashes[0].Algorithm, spelling) + } + } + } +} diff --git a/internal/sbom/spdx23.go b/internal/sbom/spdx23.go index 6f754985..c403484b 100644 --- a/internal/sbom/spdx23.go +++ b/internal/sbom/spdx23.go @@ -353,18 +353,19 @@ func spdxPackageComment(component Component) string { } // spdxChecksums maps component digests onto SPDX package checksums, dropping -// entries whose algorithm is not part of the SPDX checksum vocabulary. +// entries whose algorithm SPDX has no member for. The vocabulary is the SDK +// registry's, not a local switch -- see publishableDigest. func spdxChecksums(digests []Digest) []common.Checksum { if len(digests) == 0 { return nil } out := make([]common.Checksum, 0, len(digests)) for _, d := range digests { - alg := spdxChecksumAlgorithm(d.Algorithm) - if alg == "" || strings.TrimSpace(d.Value) == "" { + spelling, value, ok := publishableDigest(d, sdk.DigestAlgorithm.SPDXName) + if !ok { continue } - out = append(out, common.Checksum{Algorithm: alg, Value: d.Value}) + out = append(out, common.Checksum{Algorithm: common.ChecksumAlgorithm(spelling), Value: value}) } if len(out) == 0 { return nil @@ -372,31 +373,6 @@ func spdxChecksums(digests []Digest) []common.Checksum { return out } -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: - return "" - } -} - func parseSPDXComponentType(p *v23.Package) string { if p == nil { return "" diff --git a/internal/sbom/transform.go b/internal/sbom/transform.go index 05b00ad4..ea5fc469 100644 --- a/internal/sbom/transform.go +++ b/internal/sbom/transform.go @@ -302,25 +302,32 @@ func componentDigests(digests []sdk.Digest) []Digest { // digestHexSizes maps digest algorithms onto their raw byte lengths, used to // validate base64-encoded values (npm SRI integrity) before hex re-encoding. -var digestHexSizes = map[string]int{ - "md5": 16, - "sha1": 20, - "sha-1": 20, - "sha224": 28, - "sha-224": 28, - "sha256": 32, - "sha-256": 32, - "sha384": 48, - "sha-384": 48, - "sha512": 64, - "sha-512": 64, - "sha3-256": 32, - "sha3-384": 48, - "sha3-512": 64, +// +// Keyed by the SDK's canonical token, so the spelling variants of one +// algorithm resolve through sdk.ParseDigestAlgorithm rather than needing a row +// each. The lengths themselves are not the SDK's to hold: it deliberately +// records no per-algorithm value length, because ecosystems publish digests in +// hex, in base64, and over subjects that are not files. This table exists only +// to recognize a base64 value that is exactly one raw digest, so an algorithm +// missing from it is left verbatim rather than mis-decoded. +var digestHexSizes = map[sdk.DigestAlgorithm]int{ + sdk.DigestAlgorithmMD5: 16, + sdk.DigestAlgorithmSHA1: 20, + sdk.DigestAlgorithmSHA224: 28, + sdk.DigestAlgorithmSHA256: 32, + sdk.DigestAlgorithmSHA384: 48, + sdk.DigestAlgorithmSHA512: 64, + sdk.DigestAlgorithmSHA3256: 32, + sdk.DigestAlgorithmSHA3384: 48, + sdk.DigestAlgorithmSHA3512: 64, } func normalizeDigestValue(algorithm, value string) string { - size, ok := digestHexSizes[strings.ToLower(strings.TrimSpace(algorithm))] + canonical, err := sdk.ParseDigestAlgorithm(algorithm) + if err != nil { + return value + } + size, ok := digestHexSizes[canonical] if !ok { return value } From 4e99cea8f6db6221ec861185e7a908b7ae1eb784 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Wed, 9 Sep 2026 00:11:04 -0700 Subject: [PATCH 2/3] fix(test): a guard file may name the module it forbids main is red. #436 and #437 each added a guard and merged independently: the presentation-layer guard has to spell packageurl-go in order to ban it, and the module-boundary guard reports any file under internal/ that names it. Two rules doing their job, one flagging the other. The exemption is a set of canonical paths now. Not a name -- exempting anything called guards_test.go was the earlier bug in this same line, and it hid a forbidden import in a second guard file. Not one hard-coded path either, which is what made the guards collide the moment a second one existed. Adding a guard costs one line in that set, deliberately: a new exemption should be an edit somebody reviews, not a pattern that widens on its own. The predicate is extracted so the property can be pinned rather than described. TestGuardExemptionIsByPathNotByName fails if a file becomes exempt for being *named* like a guard, and if an entry names a file that no longer exists -- a dead exemption is a rule nobody is applying. The first mutation I ran against the old shape passed, which is how the missing test surfaced. Co-Authored-By: Claude Opus 5 --- internal/detectors/guards_test.go | 55 +++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) 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) + } +} From 16770eccc1d8b3c89f8eca3cb035387419a0d980 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Wed, 9 Sep 2026 00:18:32 -0700 Subject: [PATCH 3/3] fix(sbom): put export digests through the SDK's gate, not a weaker one publishableDigest checked the algorithm and a non-empty value, which is a weaker gate than the one ingest clears in ingestedDigests. Component digests can be built in memory by a detector or a plugin without ever passing through the SDK's JSON hooks, so a value carrying a control character, an interior Unicode space, or invalid UTF-8 reached the encoders intact. That matters most for the algorithms this branch newly admits, which were previously dropped for want of a mapping. encoding/json rewrites invalid UTF-8 as U+FFFD, so such a digest changes as it is serialized -- a digest that changes when written is worse than no digest. Digest.Normalized is now the gate, so ingest and export clear the same one. What it deliberately does not check is length per algorithm: ecosystems publish digests in hex, in base64 (npm SRI), and over subjects that are not files (a Go module "h1:" dirhash), so a per-algorithm hex length would reject values that are correct for their ecosystem. The new test asserts both directions -- malformed values dropped, a base64 SRI value published. Also pins where CycloneDX spec-version scoping lives. Streebog is a 1.7 addition, so a 1.6 document naming it carries a value outside a closed enumeration -- but cyclonedx-go already owns that conversion: EncodeVersion converts through SpecVersion.supportsHashAlgorithm and strips a hash the requested version cannot name. Verified against the encoder's actual output across all four targets rather than argued from the mapping's shape. A version table in publishableDigest would be a second copy of the library's, wrong the day CycloneDX adds an algorithm -- the defect this whole change removes. Co-Authored-By: Claude Opus 5 --- internal/sbom/digest.go | 51 ++++++++++++------- internal/sbom/digest_test.go | 98 ++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 17 deletions(-) diff --git a/internal/sbom/digest.go b/internal/sbom/digest.go index 9177e09f..cd737ae1 100644 --- a/internal/sbom/digest.go +++ b/internal/sbom/digest.go @@ -1,14 +1,12 @@ package sbom import ( - "strings" - "github.com/bomly-dev/bomly-sdk" ) -// publishableDigest resolves a component digest's algorithm through the SDK -// registry and renders it in one format's spelling, or reports that the digest -// cannot be published here. +// publishableDigest puts a component digest through the SDK's gate and renders +// its algorithm in one format's spelling, or reports that the digest cannot be +// published here. // // name is a method expression -- sdk.DigestAlgorithm.SPDXName or // sdk.DigestAlgorithm.CycloneDXName -- so both encoders ask the same registry @@ -19,22 +17,41 @@ import ( // digests that use it. This package had two such switches, and between them // they omitted BLAKE2b, BLAKE3, MD2/MD4/MD6, ADLER32, and Streebog. // -// Both failure modes collapse to the same answer for a caller: an algorithm -// no format defines, and an algorithm this format has no member for, are both -// "omit this digest". Neither has a spelling that would validate, and SPDX and -// CycloneDX each close their hash enumeration. +// Digest.Normalized is the gate, the same one ingest clears in +// ingestedDigests, rather than an algorithm lookup beside a non-empty check. +// A component's digests can be built in memory by a detector or a plugin +// without ever passing through the SDK's JSON hooks, so a value carrying a +// control character, a space, or invalid UTF-8 reaches here intact -- and a +// digest that corrupts the document it lands in is worse than no digest. What +// the gate deliberately does not check is length per algorithm: ecosystems +// publish digests in hex, in base64 (npm's "sha512-..." integrity strings), +// and over subjects that are not files (a Go module "h1:" dirhash), so a +// per-algorithm hex length would reject values that are correct for their +// ecosystem. +// +// Three failure modes collapse to one answer for a caller: a digest the SDK +// will not publish, an algorithm no format defines, and an algorithm this +// format has no member for are all "omit this digest". None has a spelling +// that would validate, and SPDX and CycloneDX each close their hash +// enumeration. +// +// What this does not do is scope the algorithm to a target's spec version. +// CycloneDX added Streebog in 1.7, and cyclonedx-go owns that: EncodeVersion +// converts through SpecVersion.supportsHashAlgorithm, which strips a hash the +// requested version cannot name. Repeating that table here would be the same +// transcription this function exists to remove. +// TestCycloneDXHashesAreScopedToTheTargetSpecVersion pins the behavior. func publishableDigest(d Digest, name func(sdk.DigestAlgorithm) string) (spelling, value string, ok bool) { - value = strings.TrimSpace(d.Value) - if value == "" { - return "", "", false - } - algorithm, err := sdk.ParseDigestAlgorithm(d.Algorithm) - if err != nil { + normalized, publishable := sdk.Digest{ + Algorithm: sdk.DigestAlgorithm(d.Algorithm), + Value: d.Value, + }.Normalized() + if !publishable { return "", "", false } - spelling = name(algorithm) + spelling = name(normalized.Algorithm) if spelling == "" { return "", "", false } - return spelling, value, true + return spelling, normalized.Value, true } diff --git a/internal/sbom/digest_test.go b/internal/sbom/digest_test.go index 118efbec..0a881c1b 100644 --- a/internal/sbom/digest_test.go +++ b/internal/sbom/digest_test.go @@ -182,3 +182,101 @@ func TestExportProjectsEveryRegisteredDigestAlgorithm(t *testing.T) { } } } + +// TestCycloneDXHashesAreScopedToTheTargetSpecVersion settles where version +// scoping lives. CycloneDX added Streebog in 1.7, so a 1.6 document naming it +// carries a value outside a closed enumeration -- but cyclonedx-go already +// owns that conversion: EncodeVersion strips a hash the requested version +// cannot name, through SpecVersion.supportsHashAlgorithm. +// +// The test exists so the question is answered by the encoder's actual output +// rather than re-argued from the mapping's shape. A version table written into +// publishableDigest would be a second copy of the library's, wrong the day +// CycloneDX adds an algorithm -- which is the defect this whole change removes. +func TestCycloneDXHashesAreScopedToTheTargetSpecVersion(t *testing.T) { + g := digestFixtureGraph(t, []sdk.Digest{ + {Algorithm: sdk.DigestAlgorithmStreebog256, Value: "3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb"}, + {Algorithm: sdk.DigestAlgorithmSHA256, Value: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + }) + + for _, tc := range []struct { + target Target + want []cdx.HashAlgorithm + }{ + // Streebog is absent below 1.7, and SHA-256 rides along to show the + // component still carries the hashes the version does define. + {TargetCycloneDX14JSON, []cdx.HashAlgorithm{cdx.HashAlgoSHA256}}, + {TargetCycloneDX15JSON, []cdx.HashAlgorithm{cdx.HashAlgoSHA256}}, + {TargetCycloneDX16JSON, []cdx.HashAlgorithm{cdx.HashAlgoSHA256}}, + {TargetCycloneDX17JSON, []cdx.HashAlgorithm{cdx.HashAlgoStreebog256, cdx.HashAlgoSHA256}}, + } { + out, err := MarshalDepGraphJSON(g, tc.target, BuildOptions{ProjectRoot: &ProjectRoot{Name: "demo"}}, EncodeOptions{}) + if err != nil { + t.Fatalf("%s: marshal: %v", tc.target, err) + } + var bom cdx.BOM + if err := json.Unmarshal(out, &bom); err != nil { + t.Fatalf("%s: unmarshal: %v", tc.target, err) + } + var got []cdx.HashAlgorithm + if bom.Components != nil { + for _, comp := range *bom.Components { + if comp.Name != "left-pad" || comp.Hashes == nil { + continue + } + for _, hash := range *comp.Hashes { + got = append(got, hash.Algorithm) + } + } + } + if len(got) != len(tc.want) { + t.Fatalf("%s: got %v, want %v", tc.target, got, tc.want) + } + for i, want := range tc.want { + if got[i] != want { + t.Fatalf("%s: got %v, want %v", tc.target, got, tc.want) + } + } + } +} + +// TestExportRefusesADigestTheSDKWillNotPublish covers values that reach the +// encoders without having passed the SDK's JSON hooks: a detector or a plugin +// can build DependencyNode.Digests in memory, and a value carrying whitespace, +// a control character, or invalid UTF-8 would otherwise be written straight +// into the document. encoding/json rewrites invalid UTF-8 as U+FFFD, so such a +// digest changes as it is serialized -- worse than no digest at all. +// +// A short value for a long algorithm is deliberately NOT in this list. The SDK +// checks a digest's shape, not its length per algorithm, because ecosystems +// publish digests in hex, in base64, and over subjects that are not files. +func TestExportRefusesADigestTheSDKWillNotPublish(t *testing.T) { + for _, tc := range []struct { + name string + digest Digest + }{ + {"embedded space", Digest{Algorithm: "sha256", Value: "e3b0c442 98fc1c14"}}, + {"control character", Digest{Algorithm: "blake3", Value: "af1349b9\x00f5f9a1a6"}}, + {"invalid utf-8", Digest{Algorithm: "sha512", Value: "e3b0c442\xff\xfe"}}, + // An em space, not an ASCII one: the SDK's check is Unicode-aware, + // and a trailing ASCII space is trimmed rather than refused. + {"interior unicode whitespace", Digest{Algorithm: "sha256", Value: "e3b0c442\u200398fc1c14"}}, + {"empty after trimming", Digest{Algorithm: "sha256", Value: " "}}, + } { + t.Run(tc.name, func(t *testing.T) { + if checksums := spdxChecksums([]Digest{tc.digest}); len(checksums) != 0 { + t.Errorf("SPDX exported %+v", checksums) + } + if hashes := cycloneDXHashes([]Digest{tc.digest}); len(hashes) != 0 { + t.Errorf("CycloneDX exported %+v", hashes) + } + }) + } + + // The counterpart: a base64 SRI value is shorter than the algorithm's hex + // form and must still publish. + sri := Digest{Algorithm: "sha512", Value: "pkJf8Ni4YWlKDgODlNGxi/z1Wd0/hkJH8N4Rq+Cd1lTv7ZZKPXm8mTzcp2xEVSlHoQlUwjzUKh2nGSHTMEUUpg=="} + if checksums := spdxChecksums([]Digest{sri}); len(checksums) != 1 { + t.Fatalf("expected the base64 SRI digest to publish, got %+v", checksums) + } +}