fix: bound SCIM request bodies and machine-check the invariant - #28181

Summary

Bounds the last unbounded path from a request body to a decoded value, SCIM, and then machine-checks the invariant the whole series establishes with a ruleguard rule.

Third and last of three PRs split out of #28048. Stacked on #28175, which is stacked on #28168. Review those first; the diff shown here is against #28175's branch.

Closes PLAT-463 together with the two PRs below it. Completes the remediation for SEC-416 (CWE-770, CVSS 7.5) and SEC-392.

SCIM

SCIM does not decode through httpapi.Read and so does not inherit its limit. Both implementations need bounding: the legacy handler decodes r.Body directly, and github.com/elimity-com/scim calls io.ReadAll(r.Body) on every method that carries a body, including the .search POST. The library's read happens before any error handling of its own could report it, which is why the bound sits outside both handlers rather than at the decode sites.

The ticket scoped only the three direct decoders in legacyscim.go. CODER_SCIM_USE_LEGACY still defaults to true, so that is today's default path, but there is a TODO to flip it and the SCIM 2.0 handler is unbounded too, so both are bounded here.

The ordering is the part worth reviewing. scimLimitRequestBody buffers, so it must run after the SCIM API key check. An unauthenticated caller is rejected at a header comparison having caused no read; mounting the bound ahead of that would let one spend the full limit per request on a route with no rate limiter, which is the exposure the bound exists to remove. Legacy orders the two middlewares; SCIM 2.0 authenticates inside its own handler, so Handler takes the middleware and runs it after the key check.

The limit is enforced on bytes read rather than Content-Length, which is absent under chunked transfer encoding and caller-controlled otherwise.

The rule

Coverage through httpapi.Read is conventional rather than structural, and an unenforced convention regresses. The rule rejects unbounded r.Body reads in coderd's non-test code outside an annotated allowlist.

A correct http.MaxBytesReader wrap is textually indistinguishable from a missing one, so those sites are enumerated rather than detected. An allowlist entry asserts that the site bounds its own body and answers 413; it is not a way to opt out of having a bound. Binding the body to a local first, as in body := r.Body, defeats the match, since ruleguard compares selector expressions and does not follow aliases. The rule documents that, so it is a nudge toward the idiom rather than a proof, and should not be read as one.

One thing the rule caught immediately

enterprise/coderd/aimodelprices.go landed in main after this work began, and the rule flags it. Verified against a cold golangci-lint cache: without an allowlist entry, make lint fails at aimodelprices.go:69.

It qualifies for the allowlist on the merits, since it bounds at 1 MiB and answers 413, and it reads the body whole because it unmarshals the same bytes twice to tell an absent price from an explicit null. It is allowlisted, and it now records the limit so its 413 is attributed to the body rather than counted under reason="other". Its Detail carried the stdlib http: request body too large string, replaced with the phrasing every other bounded site answers.

This is the drift a rule like this is for, and it arrived within two days of the rule being written.

The allowlist entry for csp.go, files.go and exp_chats.go was anchored on /coderd$, which also matches enterprise/coderd and so exempted files there that the entry says nothing about. It is anchored on /v2/coderd$ instead.

Behavior change

An oversized SCIM request body is answered 413 in RFC 7644 shape on both implementations, where it previously read to completion.