chore(npm): bounded JSON string reader for streampub (S11 Provenance Attestations plan: 4/20)

Why

skipValue in the streampub walker reaches jsontext.Decoder.ReadToken, which buffers a whole JSON string before returning it. A one-key object whose value is a 500 MB string therefore sits inside every platform bound the walker enforces, and is materialized in full just to be discarded. internal/jsonsafe's package doc records the same gap.

Reading the .sigstore bundle needs a reader that bounds its own consumption. base64StringReader cannot be it, because its contract is that any backslash is malformed input, and a serialized Sigstore bundle is nothing but escapes.

The reader lands on its own here and has no caller yet, which the plan records as an accepted smell.

What

  • Consumption counts raw bytes past the opening quote and never passes the cap plus one. A body of exactly the cap succeeds, because the byte that takes the count to cap plus one is the closing quote. A body one byte longer aborts on that same count, because that byte is not the quote. Both outcomes stop after the same bounded read, which is what keeps the abort independent of how far the value actually runs. The cap counts raw bytes, so an escape-heavy body whose decoded length fits under it still aborts.
  • Normal termination is a bare io.EOF. A truncated string returns a wrapped io.ErrUnexpectedEOF that keeps the underlying cause's text. That separation is load-bearing. io.ReadAll ends on an exact io.EOF comparison, and docs/dev/go-style.md tells every author here to test sentinels with errors.Is, so a reader that wrapped io.EOF for truncation hands a caller following house style a short read with a nil error. An earlier revision of this branch did exactly that, measured at 12 bytes of a 24-byte value.
  • This diverges from the plan, deliberately. Step 4's text says the reader "returns the transport error unwrapped on a truncated string, as base64StringReader does". That sentence is wrong three ways: base64reader.go:45 wraps rather than returning it unwrapped, a bare io.EOF on truncation is the short-read bug above, and this reader substitutes io.ErrUnexpectedEOF and keeps only the cause's text. The plan text needs a separate docs(plans) correction, which guardrail 4 bars this MR from making. A non-EOF fault keeps its own error reachable through errors.Is, so a log still tells an i/o timeout from a clean end of input.
  • The reader stops on the byte after the closing quote and leaves the bufio.Reader there, which is the position the walker's resume arithmetic rebases on. A test pins it, because an extra ReadByte there is invisible to every other assertion in the suite.
  • The terminal error is sticky, and a decoded rune runs to four bytes, so a surrogate pair that outruns the caller's buffer carries to the next Read.

Size

841 added lines against the 500 reviewable-LOC ceiling guardrail 18 sets: 270 in jsonstring.go (183 code, 55 blank, 32 comment), 570 in jsonstring_internal_test.go, and one fuzz:npm line in .gitlab-ci.yml.

Splitting is not available. Guardrail 6 forbids landing the reader without its tests, the plan declares Step 4's Go files as exactly these two, and the plan's own "Splits considered and not taken" rejects the one seam a split would use, because the cap counts raw bytes while the decoder emits decoded ones. The test bulk is the plan's acceptance list crossed with the three reader shapes. The source side overran its estimate too, at 183 code lines against about 150, which is what the ten-method decomposition the funlen, cyclop, and gocognit caps force costs.

Test plan

mise exec -- go test ./internal/format/npm/streampub/ -count=1
mise exec -- go test ./internal/format/npm/streampub/ -race -run JSONString -count=1
bash scripts/ci/fuzz.sh -run='^$' -fuzz=FuzzJSONStringReaderBounds -fuzztime=30s ./internal/format/npm/streampub/

198 subtests across 16 functions. Every case the plan's acceptance list names runs crossed with the three reader shapes key_order_test.go uses for its resume assertions (whole body, one byte per read, and half of each read), because escape decoding spans read boundaries.

Covers Test
A plain string, and every escape form TestJSONStringReader_DecodesBody
Exactly the cap, the cap plus one byte, and a body under the cap decoded but over it raw TestJSONStringReader_CapMeasuresRawBytes
An invalid escape, an unpaired surrogate, and a raw control byte TestJSONStringReader_RejectsMalformedEscapes
A truncated body TestJSONStringReader_TruncationReportsUnexpectedEOF
A house-style errors.Is drain cannot short-read a truncation TestJSONStringReader_TruncationSurvivesAnErrorsIsDrain
A non-EOF fault keeps its identity, its arrived bytes, and its truncation verdict when it wraps io.EOF TestJSONStringReader_TransportFaultStaysReachable, ..._TransportFaultAfterBytesKeepsWhatArrived, ..._AWrappedEOFIsStillATruncation
The stream stops on the byte after the closing quote TestJSONStringReader_StopsOnTheByteAfterTheQuote
A decoded rune and a mixed run carry across Read boundaries TestJSONStringReader_CarriesDecodedRuneAcrossReads, ..._CarriesMixedRunAcrossReads
The terminal error repeats, and success is a bare io.EOF TestJSONStringReader_TerminalErrorRepeats, ..._SuccessReturnsBareEOF
A zero-length buffer consumes nothing, and decoding allocates nothing TestJSONStringReader_ZeroLengthBufferConsumesNothing, ..._DecodesWithoutAllocating
The consumption bound over arbitrary input, decoded differentially against encoding/json FuzzJSONStringReaderBounds

FuzzJSONStringReaderBounds is registered in fuzz:npm in .gitlab-ci.yml, beside the package's FuzzProcess target. Nine targets took 331 s of that job's 10m cap, so the tenth's 30 s fits. Locally it ran 718,606 executions with no reproducer.

Guardrails

No e2e scenario is affected (guardrail 12) and no docs/user/ page is affected (guardrail 24). The reader has no caller, so no request path changes and nothing a customer can call, see, or configure moves.

Context for LLM agents

Rationale

  • Add an escape mode to base64StringReader. Rejected because its contract is that a backslash is malformed input, and a mode flag would put two opposite meanings of \ in one branch.
  • Layer an io.LimitedReader over a separate escape decoder. Rejected because the cap counts raw bytes while the decoder emits decoded ones, so the limiter would bound the wrong quantity.
  • Keep a bare io.EOF for truncation and document an identity comparison, which is how internal/jsonsafe/jsonsafe.go:265 solves the same hazard. Rejected because .golangci.yaml sets errorlint.comparison: true, so every such caller needs a //nolint:errorlint. Sentinel separation puts the discipline in the reader instead.
  • Chosen: one reader, one loop over the raw byte stream, tallying and cap-checking every byte it pulls.

Consequences: the raw-consumed count is the reader's own field, so a caller needing the abort offset reads it rather than re-deriving it from the buffered remainder.

Non-goals

  • No UTF-8 validation of the unescaped bytes. Measured on the same inputs, jsontext.Decoder.ReadValue rejects a lone 0xff, a truncated 0xc3 lead, a WTF-8 surrogate (ed a0 80), and an overlong NUL (c0 80). This reader admits all four, so reading the bundle around the tokenizer makes the .sigstore value the one envelope string that can carry such bytes. The plan names exactly three escape failures, so a fourth needs a spec amendment. The bundle's own parse does not close it, because the plan specifies encoding/json v1, which substitutes U+FFFD without erroring. The read path closes it as a 500, which makes such a publish store-now-fail-later.
  • No refusal of a duplicate or misplaced member around the string. The reader starts past the opening quote and ends at the closing one, and the walker owns the surrounding object.

Related to #1273

Edited by Hayley Swimelar

Merge request reports

Loading
Loading