Skip to content

Commit 484ec47

Browse files
AlexKantor87claude
andcommitted
fix(sbom): read the tool from a CycloneDX services entry
A hosted generator records itself under metadata.tools.services rather than under components, so the tool came back empty for any document using that slot. The SBOM our own pipeline produces is one: it names Snyk there and nowhere else. Measured on that document, tools went from nil to ["SBOM Export API v1.131.1"], with 1023 packages unchanged. The schema vendored with cyclonedx-go defines metadata.tools as the tools used in the creation, enrichment and validation of the BOM, and its services slot as "a list of services used as tools". CycloneDX's own 1.6 example fills components and services together, so a signing service that signed a document is now listed as a tool. Two existing expectations were updated. Each slot contributes a name and a version and no vendor. Not a new rule: the deprecated slot carries a Vendor field and this reader has always ignored it. An entry that identifies nothing is skipped. Document.Tools has two writers, at sbom.go:273 and sbom.go:441, and each used to record a blank. Measured against the unguarded code: CycloneDX component name " ", version "1.0" -> " 1.0" CycloneDX xml name and version wrapped across lines -> "\n Awesome Tool\n \n 9.1.2\n " SPDX creator "Tool:" -> "" The shared rule is what blank means, not the append. An SPDX creator is one free-text field with no separable version, so routing it through the CycloneDX helper would assert that syft-1.50.0 has no version. Each writer applies the same predicate to whichever field identifies a tool in its format. A block whose entries are all skipped yields nil, not an empty list. Fixtures, and where each came from: cyclonedx-tools-services.json cut from the SBOM the pipeline produced on 2026-09-14: its header, metadata and first component cyclonedx-tools-nameless-service.json that file, with two services entries added, one named "" and one named " " cyclonedx-wrapped-tool-name.xml the repo's CycloneDX 1.6 XML fixture with the tool name and version wrapped across lines, as a pretty-printer emits them spdx-blank-tool-creator.json the repo's SPDX 2.3 fixture, creators list spdx-only-blank-tool-creator.json changed and nothing else; edited as text, because a JSON round trip re-encoded a non-ASCII character in a licence body search: grep -n '\.Tools\s*=' internal/sbom/sbom.go -> two writers, both now reaching blank(). An earlier search for the helper name found only what already routed through it, which is why the second was missed. mutation: guards mutated one at a time. Weakening the CycloneDX check from blank(name) to name == "", dropping the SPDX blank check, or dropping either trim, each turns a named case red. Trimming the SPDX creator turned nothing red, because both spdx readers trim before this code sees the value, so it was removed rather than shipped untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 0840e54 commit 484ec47

7 files changed

Lines changed: 1184 additions & 13 deletions

internal/sbom/sbom.go

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,29 +324,58 @@ func packageCount(components *[]cdx.Component) int {
324324
return count
325325
}
326326

327-
// toolsFromCycloneDX reads both tool layouts. Spec 1.5 moved tools from a
328-
// dedicated list to components, and the library keeps the older list populated
329-
// for documents that use it, so a document may fill either. The services slot
330-
// the same spec added is deliberately skipped: a service a document consumed is
331-
// not a tool that generated it.
327+
// toolsFromCycloneDX reads every tool slot. Spec 1.5 replaced the dedicated list
328+
// with components and services, and the library keeps the older list populated for
329+
// documents that use it, so a document fills either the deprecated list or the
330+
// components and services pair, never both.
331+
//
332+
// A hosted generator such as Snyk's export API records itself only under services.
333+
// Each slot contributes a name and a version; none contributes a vendor, including
334+
// the deprecated one that carries a Vendor field.
332335
func toolsFromCycloneDX(tools *cdx.ToolsChoice) []string {
333336
if tools == nil {
334337
return nil
335338
}
336339
var names []string
337340
if tools.Tools != nil {
338341
for _, tool := range *tools.Tools {
339-
names = append(names, nameAndVersion(tool.Name, tool.Version))
342+
names = appendTool(names, tool.Name, tool.Version)
340343
}
341344
}
342345
if tools.Components != nil {
343346
for _, component := range *tools.Components {
344-
names = append(names, nameAndVersion(component.Name, component.Version))
347+
names = appendTool(names, component.Name, component.Version)
348+
}
349+
}
350+
if tools.Services != nil {
351+
for _, service := range *tools.Services {
352+
names = appendTool(names, service.Name, service.Version)
345353
}
346354
}
347355
return names
348356
}
349357

358+
// blank reports a string that would identify nothing once recorded. Both writers of
359+
// Document.Tools apply it, each to the field that identifies a tool in its own format:
360+
// a CycloneDX entry's name, an SPDX creator string. Sharing the rule rather than the
361+
// append keeps the two honest without pretending an SPDX creator has a separate version.
362+
func blank(s string) bool {
363+
return strings.TrimSpace(s) == ""
364+
}
365+
366+
// appendTool records one CycloneDX tool, skipping an entry whose name is blank. No slot
367+
// contributes a vendor, so such an entry would reach the attestation as a bare version
368+
// or an empty string. Name and version are recorded trimmed: encoding/xml keeps the
369+
// surrounding newlines when a document wraps them across lines. A block whose entries
370+
// are all skipped yields nil rather than an empty list, which is how "none recorded" is
371+
// distinguished from "recorded as empty".
372+
func appendTool(names []string, name, version string) []string {
373+
if blank(name) {
374+
return names
375+
}
376+
return append(names, nameAndVersion(strings.TrimSpace(name), strings.TrimSpace(version)))
377+
}
378+
350379
func nameAndVersion(name, version string) string {
351380
if version == "" {
352381
return name
@@ -404,7 +433,11 @@ func documentFromSPDX(doc *spdx.Document) (*Document, error) {
404433
}
405434
out.CreatedAt = created
406435
for _, creator := range doc.CreationInfo.Creators {
407-
if creator.CreatorType == "Tool" {
436+
// A creator of "Tool:" decodes to an empty creator, so the same blank rule
437+
// the CycloneDX reader applies has to hold on this side of the same field.
438+
// No trim here: both spdx readers trim the creator before this sees it, so
439+
// one would be a guard no test could turn red.
440+
if creator.CreatorType == "Tool" && !blank(creator.Creator) {
408441
out.Tools = append(out.Tools, creator.Creator)
409442
}
410443
}

internal/sbom/sbom_test.go

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,32 @@ func TestPackageCountExcludesFiles(t *testing.T) {
8282
assert.Equal(t, 2, got.Document.PackageCount)
8383
}
8484

85-
func TestToolsReadFromBothCycloneDXLayouts(t *testing.T) {
85+
func TestToolsReadFromEveryCycloneDXLayout(t *testing.T) {
8686
for _, tc := range []struct {
8787
name string
8888
file string
89+
want []string
8990
}{
90-
{"post-1.5 components layout", "cyclonedx-tools.json"},
91-
{"deprecated pre-1.5 layout", "cyclonedx-tools-deprecated.json"},
91+
// The spec's own 1.6 example fills components and services together. The schema
92+
// defines metadata.tools as the tools used in the creation, enrichment and
93+
// validation of the BOM, and services as "a list of services used as tools".
94+
{"components and services together", "cyclonedx-tools.json", []string{"Awesome Tool 9.1.2", "Acme Signing Server"}},
95+
{"deprecated pre-1.5 layout", "cyclonedx-tools-deprecated.json", []string{"Awesome Tool 9.1.2"}},
96+
// Cut from the SBOM our own pipeline produced on 2026-09-14.
97+
{"1.5 services layout", "cyclonedx-tools-services.json", []string{"SBOM Export API v1.131.1"}},
98+
// A nameless entry would reach the attestation as " v1.131.1", which names
99+
// nothing.
100+
{"entries named nothing or only whitespace are skipped", "cyclonedx-tools-nameless-service.json", []string{"SBOM Export API v1.131.1"}},
101+
// A pretty-printer wraps character data across lines and encoding/xml keeps the
102+
// newlines and indentation, so a name that is only whitespace, or padded, arrives
103+
// that way. This is the shape a real document produces; " " is contrived.
104+
{"a wrapped xml name and version are trimmed", "cyclonedx-wrapped-tool-name.xml", []string{"Awesome Tool 9.1.2", "Acme Signing Server"}},
92105
} {
93106
t.Run(tc.name, func(t *testing.T) {
94107
got, err := ProcessSBOMFile(fixture(tc.file))
95108

96109
require.NoError(t, err)
97-
assert.Equal(t, []string{"Awesome Tool 9.1.2"}, got.Document.Tools)
110+
assert.Equal(t, tc.want, got.Document.Tools)
98111
})
99112
}
100113
}
@@ -267,6 +280,29 @@ func TestPackageCountExcludesTheSubject(t *testing.T) {
267280
assert.Equal(t, 2, got.Document.PackageCount)
268281
}
269282

283+
func TestSPDXToolCreatorsAreHeldToTheSameBlankRule(t *testing.T) {
284+
// Document.Tools has two writers. A creator string of "Tool:" decodes to an empty
285+
// creator, so without the rule on this side the attestation records a tool that
286+
// names nothing.
287+
t.Run("a blank creator is skipped and a named one is kept", func(t *testing.T) {
288+
got, err := ProcessSBOMFile(fixture("spdx-blank-tool-creator.json"))
289+
290+
require.NoError(t, err)
291+
assert.Equal(t, []string{"real-tool-1.0"}, got.Document.Tools)
292+
})
293+
294+
t.Run("when every tool creator is blank the field is null, not empty", func(t *testing.T) {
295+
got, err := ProcessSBOMFile(fixture("spdx-only-blank-tool-creator.json"))
296+
297+
require.NoError(t, err)
298+
assert.Nil(t, got.Document.Tools)
299+
300+
encoded, err := json.Marshal(got.Document)
301+
require.NoError(t, err)
302+
assert.Contains(t, string(encoded), `"tools":null`)
303+
})
304+
}
305+
270306
func TestToolsAreNullWhenTheSBOMRecordsNone(t *testing.T) {
271307
// The fixture carries a tools block holding nothing, so the tool-reading path
272308
// runs and still yields null: the server's schema types tools as nullable,
@@ -423,7 +459,7 @@ func TestTheXMLReaderPopulatesTheSameFields(t *testing.T) {
423459
got, err := ProcessSBOMFile(fixture("cyclonedx-1.6.xml"))
424460

425461
require.NoError(t, err)
426-
assert.Equal(t, []string{"Awesome Tool 9.1.2"}, got.Document.Tools)
462+
assert.Equal(t, []string{"Awesome Tool 9.1.2", "Acme Signing Server"}, got.Document.Tools)
427463
require.NotNil(t, got.Document.CreatedAt)
428464
assert.Equal(t, "2020-04-07T07:01:00Z", *got.Document.CreatedAt)
429465
assert.Equal(t, 3, got.Document.PackageCount)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
{
2+
"bomFormat": "CycloneDX",
3+
"specVersion": "1.6",
4+
"version": 1,
5+
"metadata": {
6+
"timestamp": "2026-09-14T11:33:15Z",
7+
"tools": {
8+
"services": [
9+
{
10+
"provider": {
11+
"name": "Snyk"
12+
},
13+
"name": "",
14+
"version": "v1.131.1"
15+
},
16+
{
17+
"provider": {
18+
"name": "Snyk"
19+
},
20+
"name": " ",
21+
"version": "v1.131.1"
22+
},
23+
{
24+
"provider": {
25+
"name": "Snyk"
26+
},
27+
"name": "SBOM Export API",
28+
"version": "v1.131.1"
29+
}
30+
]
31+
},
32+
"component": {
33+
"bom-ref": "1-772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely@9239aba",
34+
"type": "container",
35+
"name": "772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely",
36+
"version": "9239aba"
37+
}
38+
},
39+
"components": [
40+
{
41+
"bom-ref": "1-docker-image|772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely@9239aba",
42+
"type": "library",
43+
"group": "alpine",
44+
"name": "docker-image|772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely",
45+
"version": "9239aba",
46+
"purl": "pkg:apk/alpine/merkely@9239aba?distro=alpine-3.24.1&upstream=docker-image%7C772819027869.dkr.ecr.eu-central-1.amazonaws.com"
47+
}
48+
]
49+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"bomFormat": "CycloneDX",
3+
"specVersion": "1.6",
4+
"version": 1,
5+
"metadata": {
6+
"timestamp": "2026-09-14T11:33:15Z",
7+
"tools": {
8+
"services": [
9+
{
10+
"provider": {
11+
"name": "Snyk"
12+
},
13+
"name": "SBOM Export API",
14+
"version": "v1.131.1"
15+
}
16+
]
17+
},
18+
"component": {
19+
"bom-ref": "1-772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely@9239aba",
20+
"type": "container",
21+
"name": "772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely",
22+
"version": "9239aba"
23+
}
24+
},
25+
"components": [
26+
{
27+
"bom-ref": "1-docker-image|772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely@9239aba",
28+
"type": "library",
29+
"group": "alpine",
30+
"name": "docker-image|772819027869.dkr.ecr.eu-central-1.amazonaws.com/merkely",
31+
"version": "9239aba",
32+
"purl": "pkg:apk/alpine/merkely@9239aba?distro=alpine-3.24.1&upstream=docker-image%7C772819027869.dkr.ecr.eu-central-1.amazonaws.com"
33+
}
34+
]
35+
}

internal/sbom/testdata/cyclonedx-wrapped-tool-name.xml

Lines changed: 203 additions & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)