feat(oci): WWW-Authenticate challenge parser (S16 Step 4)

What

Step 4 of the S16 container remote slice: the WWW-Authenticate challenge parser and auth_url assembly.

Pure functions — RFC 7235 quoted-string parsing including \" escaping, case-insensitive Bearer, realm and service extraction, an absolute-https requirement, and auth_url assembly through url.Values. No HTTP, no state, no database. Every symbol is unexported and has no production caller yet: Step 12 consumes them when it wires the token exchange.

Plan: docs/plans/2026-07-30-container-remote.md, "Step 4: WWW-Authenticate challenge parser".

Notes for the reviewer

The .gitlab-ci.yml hunk is in scope. Step 4's Files entry names only remote_challenge.go, but its Tests entry requires a fuzz target over the header parser, and a target that is not wired into fuzz:oci only ever runs its seed corpus. The six lines add FuzzParseBearerChallenge to the existing job. The plan's Files entries list production files only — they omit remote_challenge_test.go too.

Three deliberate divergences from the Container Registry reference parser. session.go floors a short token lifetime up to 60s, resolves access_token over token, and computes expiry from the response's issued_at. None is reproduced; the first and third belong to Step 5, the second to Step 12. The parser patterns that are copied — expectTokenOrQuoted's escaped-quote rebuild, lowercasing the scheme and every parameter key, walking every header value — are copied deliberately and noted where they land.

One challenge is read per header value, which is the reference parser's shape. RFC 7235 also permits several challenges inside one comma-separated value, where a Bearer challenge after a Basic one is not found. That is the fail-closed direction — no challenge found reads as "no token needed" rather than as a realm the upstream never stated — and a test row pins it.

A realm carrying userinfo is refused. The reason ParseUpstreamBase gives for refusing it does not reach this path: net/http synthesizes an Authorization header from userinfo in http.Client.send, and upstreamhttp.Client.Do issues through Transport.RoundTrip, so it would never reach the wire. It is refused because the assembled URL is persisted on container_remote_repositories.auth_url, which makes userinfo in it a credential at rest that the management API reads back and any log line printing the column carries. This goes beyond S16, which asks only for an absolute https realm; refusing is fail-closed and no known registry states credentials in a realm. Three rows cover the password-bearing, bare-username, and empty-password forms. A realm's fragment is kept and pinned, since it reaches neither the wire nor a credential column.

One upstream WWW-Authenticate value is capped at 8 KiB before the byte scan, mirroring maxChallengeValue in internal/auth/challenge.go:71. The value is upstream-controlled and net/http's default Transport.MaxResponseHeaderBytes allows 10 MiB of response headers. An oversized value is skipped rather than failing the walk, so a legitimate challenge in a later header value is still found.

Acceptance criteria 16 and 24 are only half-owned here. The plan's ownership table assigns both to this step with no gated marker, but each criterion also states the request outcome — serve the cache if a copy exists, else 503 UNAVAILABLE with Retry-After. A pure parser cannot assert that; it lands in Step 13, whose Acceptance is a table over every row of the spec's Error Cases. What this step owns is the distinct sentinel that keeps the two outcomes apart: errChallengeRealmUnusable for a stated-but-unusable realm against errChallengeIncomplete for a missing parameter, which the spec explicitly warns against collapsing.

No e2e scenario is added or affected. This step adds no route, no response, and no client-visible behavior, so neither docs/testing/e2e/oci.md nor docs/testing/e2e/docker.md changes. Conformance is not reachable for the same reason: the distribution suite drives client-facing routes and this MR adds none.

Size

Above the 500-line ceiling in development-model.md, and above the plan's Est. ~400 for this step. The excess is test surface rather than branching: one production file, and a suite carrying the ported reference case set, three property tables, a fuzz target, and two benchmarks.

Review passes

Three review passes ran on this branch.

The first, before the MR opened, found and fixed:

  • A quadratic allocation in unescapeChallengeQuoted, which sized its scratch buffer from the remaining header value rather than from the quoted-string it was unescaping. A 1 MiB header measured 3.80s and 73.7 GB of allocation churn; it now costs 4.9ms.
  • Four comment claims that promised more than the code delivered.
  • An unreached fail-closed branch: a value that opens a backslash escape and never closes its quote had no test row and no fuzz seed.

The second, a coherence check against the plan and spec, found and fixed:

  • A credential leak in the url.Parse rejection. The message wrapped net/url's own cause, and url.EscapeError quotes back the escape it rejected, so a malformed percent-escape inside the realm's password put credential bytes into the error. The rejection now names the component and drops the cause, which is what ParseUpstreamBase does for the same reason.
  • The realm's userinfo and fragment handling, until then recorded only in a commit body, documented in the code and pinned by table rows.
  • Three comment corrections: challengeSeparators also contains HTAB and so is not exactly the printable-ASCII bytes tchar excludes; the measured allocation figures were duplicated between unescapeChallengeQuoted and the benchmark header and disagreed with the benchmark's own sizes (1 MiB against a 256 KiB constant), so both now rest on the 4×-input/16×-bytes ratio instead; and the paragraph arguing why parseBearerChallenge's boolean does not name its verdict is cut to the contract a caller needs. Comments across the file went from 173 lines against 138 of code to 150 against 131.

The third, the automated AppSec and GitLab Duo reviews, found and fixed:

  • Userinfo on a realm is now refused rather than assembled into auth_url, per the section above. Both reviews raised it; both offered stripping or rejecting, and rejecting is what shipped.
  • The 8 KiB cap on one WWW-Authenticate value, also above. It moved the quadratic-allocation guard: at 64 KiB and 256 KiB the adversarial benchmark rows would now be skipped and measure nothing, so they call parseChallengeValue directly and BenchmarkParseBearerChallenge keeps the realistic entry-point row.
  • The credential-leak test strengthened to place credential bytes on both sides of the malformed escape (secret%ZZword), so it proves the escape's neighbours stay out of the message rather than only that the escape does.

Related to #288

Edited by Sylvia Shen

Merge request reports

Loading
Loading