feat(sbom): read CycloneDX and SPDX bills of materials by AlexKantor87 · Pull Request #1165 · kosli-dev/cli

AlexKantor87 · 2026-09-11T14:26:22Z

Slice 1 of #1162. Adds internal/sbom/, the reader the forthcoming kosli attest sbom command builds on. Nothing calls it yet — the command is slice 2.

Part of the SBOM work tracked by kosli-dev/server#6836. Not releasable until kosli-dev/server#6839 is in production.

What it does

Parses CycloneDX (JSON, XML) and SPDX (JSON, tag-value), confirms the file identifies itself as the format it parses as, and extracts a normalised summary. No schema validation — that is deliberate and stated in the ticket.

Both libraries are permissive enough that an identity check is the load-bearing part: the CycloneDX decoder returns an empty document rather than an error for any well-formed JSON, and the SPDX readers accept anything carrying a version field.

Reviewed before this was pushed

Because every commit here starts a review relay, the change went through the full gate locally first — including a local run of this repo's own claude-pr-review.yml prompt against the staged diff, and two conformance passes against the ticket.

That surfaced 13 findings, which are already fixed in this commit rather than arriving as review comments. The two that mattered:

A panic on user input. An SPDX file with "packages":[null] reached an unguarded dereference in tools-golang and took the process down. Now an error.
Silently wrong attestation data. Format detection scanned the whole file for spdxVersion, so a CycloneDX file merely containing that key was reported as spdx-2.3 with an empty document. The fix is an SPDX identity check after parsing, mirroring the CycloneDX one — note the ticket's trap 3 warned against relying on the library's rejection behaviour, which is exactly what the first version did.

Also fixed: nested components undercounted, SPDX purl never populated, DESCRIBED_BY relationships ignored, a UTF-8 BOM defeating detection, a quoted header in a <text> block being read as the document's own version, prefixed XML namespaces rejected, and no passing-case test for SPDX subject extraction.

The cross-repo one worth your attention

The ticket's prose says every field inside document is "present with an explicit null when the SBOM does not carry it", but the Go struct it specifies has CreatedAt string, which can only ever produce "".

The server schema in kosli-dev/server#6839 was written to the prose: created_at is {type: ["string","null"], format: date-time} and subject.sha256 is {type: ["string","null"], pattern: "^[a-f0-9]{64}$"}. An empty string satisfies neither.

(Correction since this was written: only the pattern half is actually enforced in the server's production image — jsonschema registers a date-time checker only when rfc3339_validator imports, and that package is in the server's dev lock but not its runtime lock. Raised on kosli-dev/server#6839. The CLI validates the timestamp itself regardless, so a bad value fails before the upload either way.)

This repo's own positive fixture — CycloneDX's official valid-bom.json — has a metadata.component with no hashes, so the first real attestation of a standard file would have returned 400. Nothing in either repo's tests would have caught it; it only appears when a real CLI talks to a real server.

Resolved here by making the nullable fields pointers. SHA-256 values are also lowercased, since CycloneDX permits uppercase hex and the server pattern is lowercase-only.

Tests

34 cases. Every guard was mutation-checked — 17 mutations, each turning the expected test red:

the five traps the ticket names, each proven to fail without its guard
the nine fixes above
the file-component count filter, the deprecated tools layout, and the prefixed-namespace fallback

Fixtures are the two libraries' official examples, byte-identical. SPDX's own 2.2 tag-value example cannot be read at all — a snippet section after a package is rejected by the tag-value parser, at 2.2 and 2.3 alike — so it is kept as the case for the error that points the user at the JSON form, with a snippet-free copy carrying the version test. Worth raising upstream.

Deliberately not done

No size bound. The ticket assigns that to slice 2. One constraint it must honour: ProcessSBOMFile uses os.ReadFile, so slice 2 has to stat the file before parsing, or a large file is read into memory before the friendly error can fire.
No schema_version field, unlike the sibling SnykData. The ticket pins the attestation_data shape and the server schema is written to it; adding an unexpected key risks rejection. Raised as a question on the ticket rather than changed silently.

go build ./..., go vet ./..., golangci-lint (0 issues), gofmt, and go mod tidy all clean; tidy is idempotent.

🤖 Generated with Claude Code

Adds internal/sbom, which parses CycloneDX (JSON and XML) and SPDX (JSON and tag-value), confirms the file identifies itself as the format it parses as, and extracts the fields every format has in common. It is the reader the forthcoming kosli attest sbom command builds on; nothing calls it yet. Neither library validates against the format's schema, and this does not either. The bar is that a file parses and says what it is, which is what stops a non-SBOM being attested as one. Both parsers are permissive enough to need that stated explicitly: the CycloneDX decoder returns an empty document rather than an error for any well-formed JSON, and the SPDX readers accept anything carrying a version field. Each is therefore paired with an identity check. Several behaviours here look arbitrary and are not: The spec version is compared numerically rather than as text. The enum starts at one, so absent is zero, but rendering zero gives "SpecVersion(0)" rather than an empty string, and a comparison against "" would pass for a document with no version at all. An XML document is identified by its namespace rather than by bomFormat, because bomFormat and specVersion are excluded from the XML mapping and are only ever populated for JSON. The SPDX version is read from the file's bytes rather than from the parsed document. Both readers convert every document up to their newest internal model, so a 2.2 file reports itself as 2.3 once parsed, and the format recorded on an attestation would not match the file. Fields a document does not carry are recorded as null rather than as an empty string, so a consumer can tell an absent value from an empty one. This is also what the server's schema requires: it types these fields as nullable and enforces a date-time format and a hex pattern, either of which an empty string fails. A file the user supplies must not end the process, so a panic inside the SPDX readers is converted to an error. A null entry in a document's package list reaches an unguarded dereference upstream. Fixtures are the two libraries' own official examples, unmodified. SPDX's 2.2 tag-value example cannot be read at all: a snippet section following a package is rejected by the tag-value parser, at 2.2 and 2.3 alike, so that file is kept as the case for the error message pointing the user at the JSON form, and a snippet-free copy carries the version test.

claude · 2026-09-11T14:26:52Z

Claude finished @AlexKantor87's task in 3m 51s —— View job

Review round 7

Static analysis — go build ./..., go vet ./..., go mod tidy + git diff --exit-code go.mod go.sum all clean
Reviewed 9678b660 (the XML extraction test) and re-read the whole file
Posted findings

go test is denied in this runner, so everything below is traced through the fixtures in this PR and the pinned library sources rather than observed.

The XML test is right, and it found something on the way in

ToolsChoice genuinely has separate hand-written UnmarshalXML and UnmarshalJSON, so the new test covers code the JSON assertions never touched. Noting in the commit message that the two official fixtures are not the same document — third component, different type on the first, different timestamp — is the right call: without it the 3-vs-2 count reads as a bug.

One finding

subjectFromSPDX drops the subject for a document describing one package alongside a file — internal/sbom/sbom.go:422.

spdxlib.GetDescribedPackageIDs returns the target of every DESCRIBES, files included, despite its name and despite its own comment claiming it verifies the IDs are packages. spdx-2.3.json describes SPDXRef-File (a file, :233, inside the files array at :158) and SPDXRef-Package (:31, inside packages at :28), so len(described) == 2 and the subject is nil — for a document that describes exactly one package, which is the rule the function's own doc comment states.

TestSPDXSubjectIsNullWhenSeveralPackagesAreDescribed pins that under a name asserting several packages are described. Its PackageCount > 1 assertion reads as corroboration but measures how many packages the document holds, not how many it describes. Sixth instance of the green-for-the-wrong-reason class in this PR, and the only one that predates the review.

On the rest

Everything from rounds 1–6 landed. Worth naming three, because they were the ones where the finding was only half right and the reply improved on it rather than just complying:

the normaliseNullElements early return, which kept a nil slice nil — a difference my suggested version introduced and yours removed;
declining the tools: [] change on the actual server schema rather than the one I guessed at, and declining the package_count symmetry on measured syft behaviour while taking the doc fix;
the Token() benchmark. My claim that json.Unmarshal into a narrow struct materialises the document was wrong, and the numbers are the right way to settle it.

The correction discipline in this thread is unusual — three cross-repo claims about rfc3339_validator were wrong in successive rounds and each was retracted with the source quoted rather than argued down. The read-the-dependency-rather-than-recall rule that came out of it is the change worth keeping.

Nothing else blocking from me.
· branch claude/1162-sbom-reader

…eject Addresses the automated review on this PR. The deferred recover wrapped the whole of readSPDX, so a nil dereference in the mapping code here would have been reported to the user as their file failing to parse. It now guards the library call alone, which is what it was for; a defect in this package surfaces as one. A creation timestamp is checked against RFC 3339 before it is recorded. The server's schema types the field as a date-time, so a generator emitting a bare date produced a rejection after the upload rather than a message here. Recording null instead was the alternative and is worse: null means the SBOM did not carry the value, and it did. SPDX RDF, SPDX YAML and SPDX 3.x JSON-LD are recognised and named. All three are valid SPDX documents this reader cannot read, and the RDF form was previously reported as a broken CycloneDX file, which points the reader at the wrong thing. Detecting the form is this slice's job even though parsing it is not. The JSON branch now routes on the top-level spdxVersion key rather than a regex over the whole file, which cannot match a value nested anywhere in the document. The version still comes from the file rather than the parsed document, because the readers convert every document up to their newest model. Two findings are not taken, both because the premise does not hold. tools encoding as null is what the server's schema asks for, and an empty list would lose the distinction between no tools recorded and an empty list recorded. The package count differing by one between the formats is real, and left alone: the CycloneDX figure is measured against generator output, the subject sits outside the component list in that format and inside the package list in the other, and the godoc now says so.

Narrowing the panic guard to the library call left a crash reachable for SPDX 2.1. Only the 2.2 and 2.3 models define UnmarshalJSON, and it is theirs that drops null relationships and trips over a null package while still inside the reader. A 2.1 document is unmarshalled with plain encoding/json and converted by reflection, which preserves nil elements, so they survive into the mapping code and into the library's own described-package lookup, neither of which is covered by the guard around the reader. Every version now gets the handling 2.2 and 2.3 get for free: a null package is rejected, and null relationships are dropped the way their readers drop them. The verification behind the earlier change covered 2.2 and 2.3 only, which is exactly the pair where the model hides the problem. This one was checked against every position a 2.1 document can carry a null: packages, relationships, files, external references, checksums and creation info. None of the others reach a dereference, and a test covers the two that do.

…r one The timestamp guard added in the previous commit used time.Parse with time.RFC3339, which is stricter than the format in three ways that matter. RFC 3339 section 5.6 permits a lowercase t and z, and permits a leap second; Go accepts neither, and its layout matching is byte-for-byte so there is no second chance. The server's validator is a case-insensitive regex that takes all three. So the guard refused timestamps the server would have accepted: a rejection the CLI invented, on a conformant file, which is the failure direction the rest of this work exists to avoid. It now mirrors the server's rule. A malformed JSON file is also reported as malformed JSON. The routing probe discarded its unmarshal error, so a truncated SPDX file fell through to the CycloneDX reader and came back blamed on a format it was never in.

The previous commit replaced time.Parse with a regex described as mirroring the server's validator. It was not a mirror; it was a strict superset, because every component was a bare two-digit match. It accepted month 13, day 45, hour 99 and an offset of +99:99, and it accepted a leap second, which the validator's own docstring says it does not support. A fixture pinned that leap second as valid, so the tests asserted the CLI should send a value the server rejects. Read against rfc3339_validator 0.1.4, the version the server locks. Uppercasing the value and parsing it with time.RFC3339 agrees with it on every case tried except year zero, which that validator rejects explicitly and Go accepts, so the year is checked here. The uppercase is what admits the lowercase t and z the format permits, and is what the server does too: jsonschema validates instance.upper(). Also moves safeSPDXRead's doc comment back onto safeSPDXRead. Inserting normaliseNullElements landed it between that comment and its function, which left the reason the recover is narrow attached to a helper that does no recovering, and the function it warns about undocumented.

The fixture had one package, and the described-package lookup returns that package without reading relationships when there is only one. So the nil entry it carried was never dereferenced and the subtest passed with the relationship-stripping half of normaliseNullElements deleted. The claim that removing the normalisation turned both subtests red held for packages only. The previous round's mutation removed the whole function, so the package half masked the relationship half. Each half is now mutated on its own. Also stops a key of the wrong type being reported as a malformed file. The routing probe surfaced every unmarshal error as "not valid JSON", but a numeric spdxVersion is well-formed JSON the probe simply cannot read, and the file is usually CycloneDX and reads as it. Only a syntax error is reported as one now. The creation timestamp is recorded as the SBOM wrote it rather than rewritten to a canonical form, which would drop the sub-second precision time.RFC3339 does not carry. The doc comment covered acceptance and was silent on this; it now says so.

…rces The subject SHA-256 was lowercased and sent unchecked. Neither format constrains the field in practice — SPDX leaves checksumValue a free string, and cyclonedx-go does not apply the spec pattern — so a generator writing an OCI-style "sha256:..." digest produced a value the server's schema rejects, and that pattern, unlike its date-time rule, is enforced in the production image. So the effort had gone to the rule that does not fire and skipped the one that does. The digest is now refused locally, at both call sites, with the value in the message. The SPDX half of the timestamp guard had no test: deleting it left the suite green, because every SPDX fixture carries a valid created and only the CycloneDX call site was covered. One call site is not evidence for its twin. Both halves of both guards are now pinned and mutated separately. The relationship slice is no longer copied when it holds no nil, which is every real document: a syft SBOM carries a relationship per file. Returning early also leaves a nil slice nil, which the described-package lookup treats differently from an empty one.

The YAML probe read the raw file while the tag-value probe read the copy with text blocks removed. That copy exists because a tag-value document may quote another document's header inside a text block, which is already covered by a fixture. Adding the YAML check reopened that trap one branch earlier and made it worse: a valid tag-value SBOM quoting a YAML header was refused as YAML and never reached a reader that would have read it. Both probes now see the stripped copy, and the duplicate strip is gone. The tag-value reader had no extraction test. Every fixture using it asserted the format string only, so documentFromSPDX was exercised through the JSON reader alone, and the two reach spdx.Document by different routes. The fields read here are where they differ: a creator arrives as "Creator: Tool: x" lines the tag-value parser splits itself, and a checksum as "PackageChecksum: SHA256: <value>", which is where the digest guard runs. The digest pattern now comes from internal/digest rather than a second copy.

…YAML The tag-value extraction test's comment claimed it covered the checksum path. It did not. Both tag-value fixtures describe two packages, so subjectFromSPDX returns before the package lookup, the purl scan and the checksum loop, and the digest guard had never run on a "PackageChecksum: SHA256:" value in any test. A fixture describing exactly one package now covers all three, and the comment says what the test actually does. The YAML probe allowed a double-quoted value only, so a single-quoted spdxVersion — which YAML permits and emitters produce — fell through to "unrecognised file", the misdirection that probe exists to remove. The existing fixture is unquoted, so the gap had no test either. A local variable named digest shadowed the internal/digest import in the two functions that hold it. Nothing needs the package there today, so it compiled; a second digest call in either function would not have.

The XML fixture appeared once, asserting the format string. Everything about what this package extracts was proven through the JSON reader, and the two reach cdx.BOM through separate hand-written unmarshallers — ToolsChoice has an UnmarshalXML and an UnmarshalJSON, each dispatching differently — so one proves nothing about the other. The two official fixtures are not the same document, which is worth stating because it looks like a bug: the XML one carries a third component, types the first differently and records a different timestamp. The test asserts the XML fixture's own values rather than parity with its JSON sibling.

@AlexKantor87

kosli-cli 2.41.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre># New features - Added `kosli attest sbom` command (beta) to report a software bill of materials (CycloneDX JSON/XML or SPDX JSON/tag-value) to an artifact or trail in a Kosli flow. The SBOM file checksum, format, and a parsed summary are recorded; `sbom_format` and `sbom_sha256` are automatically added as annotations. - Added hidden `--server-side` flag to trail evaluation commands, enabling server-side policy evaluation (experimental, not yet a stable contract).  ## What's Changed * feat(sbom): read CycloneDX and SPDX bills of materials by @AlexKantor87 in kosli-dev/cli#1165 * feat(attest-sbom): add kosli attest sbom by @AlexKantor87 in kosli-dev/cli#1168 * fix(sonar): never send the API token to a redirect target by @mbevc1 in kosli-dev/cli#1170 * fix(snapshot azure): reject zip entries that would extract outside the temp dir by @mbevc1 in kosli-dev/cli#1175 * fix(sbom): read the tool from a CycloneDX services entry by @AlexKantor87 in kosli-dev/cli#1179 * fix(snapshot azure): stop a container spoofing its digest in logs mode by @mbevc1 in kosli-dev/cli#1176 * chore: improve PR follwo-up reviews by @mbevc1 in kosli-dev/cli#1181 * feat(evaluate): evaluate a trail server-side behind a hidden flag by @jumboduck in kosli-dev/cli#1171 * fix: align review turns by @mbevc1 in kosli-dev/cli#1185 * refactor(aws): compare the lambda package type with the SDK constant by @mbevc1 in kosli-dev/cli#1184 **Full Changelog**: kosli-dev/cli@v2.40.1...v2.41.0 </pre> <p>View the full release notes at <a href="https://github.com/kosli-dev/cli/releases/tag/v2.41.0">https://github.com/kosli-dev/cli/releases/tag/v2.41.0</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!20275

github-actions Bot added the go Pull requests that update go code label Sep 11, 2026

github-actions Bot added the feat label Sep 11, 2026