Repository navigation
Strict SBOM ingest rejects escaped lone surrogates, a third class ADR-0039 does not claim #432
Description
Filed from review on #431 after that PR's AI-review round cap was reached, so it is validated in a fresh session rather than in another round on a branch that has already rewritten this code twice.
The finding
Reported by Codex on internal/sbom/strict.go:79. Reproduced before filing:
input bytes are valid UTF-8: true
encoding/json v1 accepts: true
strict gate: ambiguous sbom json: jsontext: invalid surrogate pair `\ud800"}]}`
in string within "/components/0/description" after offset 186
Input: a CycloneDX document with "description":"\ud800" — an escaped unpaired surrogate.
Why it matters
ADR-0039 states the guarantee precisely: "The guarantee is exactly those two ambiguity classes — a document with duplicate object names or invalid UTF-8 is rejected with an actionable error. Other reader-divergence classes ... are not covered by json/v2's defaults and are not claimed here."
This document has neither. Its bytes are valid UTF-8 (the escape is ASCII), and it has no repeated member name. So:
- A document that previously imported now fails. v1 accepted it, substituting U+FFFD. Some producers emit lone-surrogate escapes for unpaired surrogates in free-text fields such as descriptions.
- The error is misleading. It is reported as
ErrAmbiguousJSON, whose message tells the user their document "does not have a single unambiguous reading" and to look for a repeated member — which is not there. The classification inrequireUnambiguousJSONsends it there because v1 considers the document valid, so the malformed branch does not fire.
What needs checking
- Codex's suggested fix: check raw bytes with
utf8.Validand passjsontext.AllowInvalidUTF8(true)to the decoder for the duplicate-name pass. Confirm that (a) duplicate detection is unaffected by that option, and (b)utf8.Validover the whole document is an acceptable stand-in for v1's per-string substitution behavior — v1 replaces invalid bytes inside strings, and a whole-document check also covers bytes outside them, where they would be a syntax error anyway. That difference wants a test, not an assumption. - The alternative: keep rejecting lone surrogates, but classify them as their own documented class with their own error and sentinel, and amend ADR-0039 to claim three classes rather than two. Rejecting them is defensible — a lone surrogate is not representable and consumers will disagree about what it becomes — but it must be stated, not arrive as a side effect of a library default.
- Either way,
docs/SBOM.mdanddev-docs/SECURITY_ASSURANCE.mddescribe two classes and would need updating for the second option.
Not urgent
Bomly's own output never contains lone-surrogate escapes, and the failure is a refusal rather than a silent misread — it fails closed. But it is a regression against documents that used to import, and the error points the user at the wrong thing.
Context
internal/sbom/strict.go—requireUnambiguousJSONand its malformed/ambiguous classification.- The two classes are pinned by
TestIngestRejectsDuplicateObjectNames,TestIngestRejectsInvalidUTF8,TestStrictIngestDoesNotRejectMerelyUnusualDocuments, andTestMalformedDocumentsAreNotReportedAsAmbiguous.
Activity