fix(managementapi): reject malformed container list cursors at decode
Why
The container image and tag lists are the one management read surface that forwards a forged cursor boundary to the stores: an over-long or control-byte boundary serves a junk page today, and a NUL boundary kills the statement through the simple-protocol pool (SQLSTATE 08P01) and renders a 500, where the spec's Error Cases make malformed-cursor input a 400. The decode-time guard mirrors the sibling maven/npm cursor content checks.
S17's Error Cases rows and the OpenAPI BadRequest description now name a boundary no stored value can hold, amended here after the spec author confirmed the reading in review. Three guards rested on it as inference before: validMavenKeysetText, validMavenFileName, and this MR's validContainerCursorName.
At 774 reviewable LOC this is over the 500 in development-model.md, so the size is justified rather than split. 681 of the 765 added lines are tests. The production change is one file, 80 lines added and 6 removed, of which 17 are executable code under the rest in comment. Splitting the guard from the tests that pin it would put a behavior change and its coverage in separate reviews.
Test plan
| Criterion | Tests |
|---|---|
Spec Error Cases (S17 ~line 370): artifact-list cursor that fails to decode, carries no keyset boundary, or carries a boundary no stored value can hold → 400 bad_request |
New rows cursor past the column length and cursor carrying a control byte in TestContainerImageList_InvalidParams_Return400 and TestContainerTagList_InvalidParams_Return400; pre-existing rows undecodable cursor and cursor with no boundary |
| Spec AC (S17 ~line 347): invalid cursor on any artifact list → 400 with the S01 envelope | Same four rows, each asserting CodeBadRequest via assertErrorEnvelope |
| Over-long name (256 B) → 400, zero store calls, both lists | .../cursor_past_the_column_length in both suites |
| Control byte (NUL) → 400, zero store calls, both lists | .../cursor_carrying_a_control_byte in both suites |
| Exactly 255 B → 200, boundary forwarded unchanged | TestContainerImageList_CursorNameAtColumnBound |
Byte bound preconditions: ValidatePath counts bytes, and both write grammars stay ASCII |
TestValidatePath_BoundsPathInBytes and TestNameGrammars_RejectNonASCII in internal/format/oci, each mutation-checked |
| Existing behavior pinned | Pre-existing empty-name/undecodable rows unchanged; manifests suite untouched; keyset-walk tests green |
| Spec and contract wording | Prose, so no test. lint:openapi validates api/openapi/v1.yaml against the 3.1 meta-schema, and it passes locally on redocly 2.34.0 |
The test commit ran red before green: the four new 400 rows failed 200-vs-400 pre-implementation. Unit and integration-tagged package suites and go-lint-ci are green locally.
No docs/testing/ scenario catalog covers the management API yet, so no e2e scenario file is affected.
Context for LLM agents
Rationale
- Stretching
errCursorNoBoundaryto cover unstorable boundaries. Rejected: a missing boundary and a boundary no stored row can match are different defects, so the decode returns a distincterrCursorUnstorableBoundary, with both mapping to the same 400. - Extracting a predicate shared with the maven/npm cursor guards. Rejected: extraction sits with the store-level guard tracked in Extract a shared keyset-text cursor guard for s... (#451 - closed) • Hayley Swimelar, so the predicate stays a byte-exact sibling copy until then.
- Bounding at the 128-character tag grammar. Rejected: the guard models storability, and the columns' length CHECK admits 255, which the guard measures in bytes.
- Widening the spec rather than hedging the Go comments. The comments asserted a reading the description called inference, so one of the two had to move. The spec author confirmed the reading and asked for the amendment here, so the comments now state the rule and cite no spec.
Non-goals
- Manifest cursor decode. Deliberately unchanged: its boundary fields are typed (uuid, time), and the year-0 guard rationale was empirically refuted.
- Rejecting C1 control bytes. They pass by deliberate parity with the sibling predicate.
- The repository-list twin gap, deferred to Reject malformed repository list cursor names a... (#483) • Unassigned.
- The path-value NUL exposure, deferred to Reject NUL and control bytes in path values bef... (#484) • Hayley Swimelar.
- The raw-length pre-guard on the container cursor decode. An earlier revision of this section said the guard could not compile here. That is no longer true: a rebase brought
maxCursorLengthanderrCursorTooLonginto this branch. It stays out because it bounds the wire string, not the decoded boundary this MR is about. Deferred to Bound the container list cursor length before d... (#485) • Unassigned.
Related to #461 (closed)