Bound the jsonstream token buffer before pypi_pep_691_json reaches 100%

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Work on this issue
• Close this issue

Summary

workhorse/internal/jsonstream has no bound on the size of a single JSON token, so one oversized string value in an upstream PyPI response is buffered in full inside Workhorse. workhorse/internal/htmlstream already caps this at 1 MiB. The JSON path is the one being rolled out behind pypi_pep_691_json, so the new path currently has weaker protection than the HTML path it sits beside.

This is not a prerequisite for the pypi_pep_691_json rollout. It does not gate the targeted enablement and it does not gate 100%. Worth doing because it is cheap and because the asymmetry with htmlstream is hard to justify to the next reader, not because it is currently dangerous. See "Why this is not urgent today" below.

Found while validating pypi_pep_691_json for its GitLab.com rollout (#592164).

Improvements

What is bounded today

htmlstream.Transform calls z.SetMaxBuf(maxTokenBytes) with maxTokenBytes = 1 << 20, and past that the tokenizer returns html.ErrBufferExceeded instead of growing. This is covered by TestTransformBoundsTokenBuffer. The rationale is already written down in workhorse/internal/htmlstream/transform.go:

maxTokenBytes bounds the tokenizer's internal buffer. A real simple index is millions of small tokens, but the tokenizer grows to fit the largest single one, so without a ceiling an upstream response that is one enormous text node or attribute value would buffer in full -- inside Workhorse, which serves every request.

That reasoning applies unchanged to jsonstream, which has no equivalent.

Measurements

The streaming design itself works. Peak HeapInuse stays flat as a PEP 691 project-detail document grows, measured with the body streamed from a generator so the fixture is not held in memory:

Files streamed	Input	Peak HeapInuse
1,000	0.2 MiB	1.3 MiB
10,000	2.2 MiB	3.2 MiB
100,000	22.3 MiB	2.7 MiB
1,000,000	223.2 MiB	4.9 MiB

A realistically large index is a non-issue: 223 MiB streamed costs 4.9 MiB of peak heap.

The unbounded case is a single giant string value at the rewrite target key. Cost, same method:

Single token	Peak HeapInuse	Returned error
1 MiB	8.5 MiB	none
64 MiB	402 MiB	none
256 MiB	1602 MiB	none

Roughly 6.3x amplification, and it completes silently rather than failing closed. For comparison, htmlstream refuses the equivalent input at 1 MiB.

Proposed change

Give jsonstream the same ceiling htmlstream has, and return an error past it rather than growing. Keep the two limits defined the same way so they do not drift.

Risks

Low. The change only makes a currently-unbounded path fail closed.

• The limit must sit far above any genuine PEP 691 entry. Real file entries run to a few hundred bytes, and the longest legitimate value at the url key is a pythonhosted URL, so 1 MiB leaves several orders of magnitude of headroom. Reusing htmlstream's existing value keeps the two paths consistent.
• Exceeding the limit surfaces the same way a malformed body already does: the 200 status is written before streaming begins (workhorse/internal/sendurl/sendurl.go:201), so the client gets a truncated 200 and the failure is logged server-side. This issue does not change that, and it is not made worse by adding the cap.

Why this is not urgent today

It is not reachable by an ordinary caller. REGISTRY_BASE_URLS is a frozen hash and registry_base_url reads from it, so the upstream is compiled in as https://pypi.org/simple/ rather than being configurable at runtime. On top of that SSRFFilter is on, redirects are disabled, and localhost is only allowed in development. So an attacker cannot point the fetch at a host they control.

That leaves two routes, neither of which is an ordinary request: pypi.org itself serving a pathological body, or a TLS MITM against pypi.org. A publisher can influence the content of a pypi.org response, but only through fields PyPI length-limits itself (package name, filename, URL, requires-python), none of which come close to the sizes that matter here.

Note also that a large number of files is not the problem. That is many small tokens, and peak heap stays flat: 223 MiB streamed costs 4.9 MiB. Only a single oversized token costs anything, and that is a shape a real index does not produce.

Two things make it worth fixing despite all of the above:

• The same jsonstream.Transform is the npm packument transform (ee/lib/ee/api/npm_project_packages.rb), and npm forwarding is already enabled. So this exposure is live in production today and is not introduced by the PyPI rollout.
• The reason it is unreachable is that the upstream host is compiled in. If PyPI forwarding ever gains a configurable upstream, that assumption disappears and this stops being theoretical. Cheaper to bound it now.

Involved components

• workhorse/internal/jsonstream/transform.go — add the bound
• workhorse/internal/jsonstream/transform_test.go — assert it
• workhorse/internal/htmlstream/transform.go — existing maxTokenBytes, if the two limits are shared

Callers of the JSON transform, for context, both unchanged by this issue:

• ee/lib/ee/api/pypi_packages.rb — PyPI simple index, transform_config with format: :json
• ee/lib/ee/api/npm_project_packages.rb — npm packuments, the transform's original caller

Note this also covers npm, which uses the same jsonstream.Transform and is already enabled. The npm path has the same exposure today.

Optional: Missing test coverage

A test asserting the transform returns an error once a single token passes the limit, mirroring TestTransformBoundsTokenBuffer in workhorse/internal/htmlstream/transform_test.go.

Related findings from the same investigation

Recorded here so they are not lost, but each needs its own issue rather than being folded into this one:

1. No idle-read timeout. DialTimeout and ResponseHeaderTimeout are both 10s and verified, but nothing aborts an upstream that answers headers quickly and then dribbles the body. Already tracked in https://gitlab.com/gitlab-org/gitlab/-/work_items/609164.
2. No PEP 691 schema validation. null, a missing files key, or a non-array files are valid JSON, so they pass through as a 200 with no rewriting and pip fails on a non-conforming payload. No 500 and no crash, but no rejection either.
3. No telemetry distinguishing JSON from HTML. config/events/202109151015_api__pypi_packages_list_package.yml carries no label or property for the response format, so JSON adoption and HTML fallback are both unmeasurable. Malformed-upstream failures also never surface as 5xx, because the 200 is already written; they appear only as gitlab_workhorse_send_url_requests{status="request-failed"}, a counter shared with transport failures, plus a SendURL: Copy response log line.