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 wrappedio.ErrUnexpectedEOFthat keeps the underlying cause's text. That separation is load-bearing.io.ReadAllends on an exactio.EOFcomparison, anddocs/dev/go-style.mdtells every author here to test sentinels witherrors.Is, so a reader that wrappedio.EOFfor 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
base64StringReaderdoes". That sentence is wrong three ways:base64reader.go:45wraps rather than returning it unwrapped, a bareio.EOFon truncation is the short-read bug above, and this reader substitutesio.ErrUnexpectedEOFand keeps only the cause's text. The plan text needs a separatedocs(plans)correction, which guardrail 4 bars this MR from making. A non-EOF fault keeps its own error reachable througherrors.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.Readerthere, which is the position the walker's resume arithmetic rebases on. A test pins it, because an extraReadBytethere 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.LimitedReaderover 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.EOFfor truncation and document an identity comparison, which is howinternal/jsonsafe/jsonsafe.go:265solves the same hazard. Rejected because.golangci.yamlsetserrorlint.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.ReadValuerejects a lone0xff, a truncated0xc3lead, 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.sigstorevalue 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 specifiesencoding/jsonv1, 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