Skip to content

Strict SBOM ingest rejects escaped lone surrogates, a third class ADR-0039 does not claim #432

Description

@bomly-guy

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:

  1. 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.
  2. 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 in requireUnambiguousJSON sends 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.Valid and pass jsontext.AllowInvalidUTF8(true) to the decoder for the duplicate-name pass. Confirm that (a) duplicate detection is unaffected by that option, and (b) utf8.Valid over 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.md and dev-docs/SECURITY_ASSURANCE.md describe 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.gorequireUnambiguousJSON and its malformed/ambiguous classification.
  • The two classes are pinned by TestIngestRejectsDuplicateObjectNames, TestIngestRejectsInvalidUTF8, TestStrictIngestDoesNotRejectMerelyUnusualDocuments, and TestMalformedDocumentsAreNotReportedAsAmbiguous.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions