From 4a3ef5a5360eff121775b074850707280b09168b Mon Sep 17 00:00:00 2001 From: SyniRon <66834451+SyniRon@users.noreply.github.com> Date: Mon, 29 Jun 2026 22:33:20 -0400 Subject: [PATCH 1/2] test(contract): enforce Vary: Accept-Encoding is declared wherever it ships GzipMiddleware adds Vary: Accept-Encoding to every response that reaches a handler (#229), and the spec documents it on the 2xx responses and the shared Error response. Nothing kept that documentation honest, so a new endpoint could declare Cache-Control (already enforced by TestSpec_Every200DeclaresCacheControl) but forget Vary, and the spec would drift from the wire again, which is the same problem #228 fixed. TestSpec_VaryAccompaniesEveryGzippedResponse closes the gap, modeled on the Cache-Control test. The discriminator is wider, because gzip runs inside auth but outside the mux: Vary ships on all 2xx and on the JSON error tier, and only the 401 responses lack it (auth short-circuits before the gzip layer). The test keys on that seam rather than on 2xx-vs-non-2xx. The shared Error component and every non-401 response must declare Vary; the Unauthorized component and any literal 401 must not. The spec was already correct as of #229, so this is a forward-looking guard. Proved it is non-vacuous by deleting a Vary block and confirming the test fails naming the offending response, then restoring it. > *This was generated by AI* --- contract/spec_test.go | 96 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/contract/spec_test.go b/contract/spec_test.go index 7a4487f..c966749 100644 --- a/contract/spec_test.go +++ b/contract/spec_test.go @@ -937,6 +937,102 @@ func TestSpec_Every200DeclaresCacheControl(t *testing.T) { assert.GreaterOrEqual(t, two00s, 16, "expected a 2xx declaration per public operation, saw %d", two00s) } +// TestSpec_VaryAccompaniesEveryGzippedResponse pins the Vary: Accept-Encoding +// declaration (#229, #230) at the spec layer, both directions, the same way +// TestSpec_Every200DeclaresCacheControl pins Cache-Control. The discriminator +// is different, though, because Vary has a broader footprint than the freshness +// signal: GzipMiddleware runs inside auth but outside the mux, so it stamps Vary +// on EVERY response a handler produces — all 2xx AND the JSON error tier (the +// shared Error response, covering 400/403/404/500 and the default fall-through). +// The only response that carries no Vary is the 401 tier, because AuthMiddleware +// short-circuits before the gzip layer ever runs. So the rule follows that +// "reaches the gzip layer vs the 401 short-circuit" seam, NOT 2xx-vs-non-2xx: +// +// - every operation 2xx response, every non-401 operation error response +// (the shared Error shape, reached through gzip), the operation default +// fall-through, and the shared Error component MUST declare Vary; +// - the shared Unauthorized component and any literal 401 response MUST NOT — +// the pre-routing auth tier never reaches GzipMiddleware. +// +// Without this net a new endpoint can document Cache-Control (which the spec +// test above enforces) but forget Vary, and the spec silently drifts from the +// wire again — the exact #228 class of bug. The VALUES the stack actually sends +// are pinned live in rest/vary_test.go; this is the structural guard keeping the +// docs honest. +func TestSpec_VaryAccompaniesEveryGzippedResponse(t *testing.T) { + _, model := loadSpec(t) + + varyDeclared := 0 + checkResponse := func(where string, wantVary bool, r *v3.Response) { + if r == nil { + return + } + var hdr *v3.Header + if r.Headers != nil { + hdr = r.Headers.GetOrZero("Vary") + } + if !wantVary { + assert.Nil(t, hdr, + "%s: declares Vary but never reaches the gzip layer — the 401 auth tier short-circuits before GzipMiddleware, so it carries no Vary on the wire", where) + return + } + require.NotNil(t, hdr, + "%s: response reaches the gzip layer but declares no Vary header — GzipMiddleware stamps Vary: Accept-Encoding on every handler-produced response (#229), so the spec must say so", where) + // Deliberately NOT required, for the same reason the Cache-Control test + // documents: the validator enforces required response headers, and the + // frozen golden corpus (recorded from the old stack, allowlist-filtered, + // carrying no Vary) replays against this document, so required: true + // breaks TestSpec_GoldenReplay permanently. The always-sent guarantee is + // pinned live instead (rest/vary_test.go). + assert.False(t, hdr.Required, + "%s: Vary must stay optional — required: true fails the frozen-corpus replay (goldens record no Vary)", where) + require.NotNil(t, hdr.Schema, "%s: Vary header declares no schema", where) + s := hdr.Schema.Schema() + require.NotNil(t, s, "%s: Vary header schema does not build", where) + assert.Equal(t, []string{"string"}, s.Type, "%s: Vary schema must be a string", where) + require.NotNil(t, s.Const, + "%s: Vary schema must pin its exact value with const — the declaration IS the recorded cache-keying header", where) + assert.Equal(t, "Accept-Encoding", s.Const.Value, + "%s: Vary const must be Accept-Encoding", where) + varyDeclared++ + } + + if model.Components != nil { + for pair := orderedmap.First(model.Components.Responses); pair != nil; pair = pair.Next() { + // Shared error tier: Error is handler-produced (flows through gzip) + // and carries Vary; Unauthorized is the pre-gzip 401 short-circuit + // and does not. + name := pair.Key() + checkResponse("components.responses."+name, name != "Unauthorized", pair.Value()) + } + } + + for pair := orderedmap.First(model.Paths.PathItems); pair != nil; pair = pair.Next() { + route := pair.Key() + for method, op := range pair.Value().GetOperations().FromOldest() { + opPath := strings.ToUpper(method) + " " + route + if op.Responses == nil { + continue // absence of responses is the replay loop's failure to report + } + for rp := orderedmap.First(op.Responses.Codes); rp != nil; rp = rp.Next() { + code := rp.Key() + // Everything a handler produces is gzipped; only the 401 tier + // short-circuits before the gzip layer runs. + checkResponse(opPath+".responses."+code, code != "401", rp.Value()) + } + // The default fall-through resolves to the shared Error shape, which + // is handler-produced and therefore reached through gzip. + checkResponse(opPath+".responses.default", true, op.Responses.Default) + } + } + + // Non-vacuousness: the walk must actually find the Vary declarations (17 + // success responses, the shared Error component, and every non-401 error + // response that resolves to it). A walk that silently iterated nothing would + // pass every per-response assertion above. + assert.GreaterOrEqual(t, varyDeclared, 17, "expected Vary on every gzipped response, saw %d", varyDeclared) +} + // TestSpec_EveryOperationHasGolden is coverage direction A: every operation // in the spec is exercised by at least one golden — including at least one // 2xx golden, so error-only coverage cannot count as witnessing the success From 9eee9f84b40eb162b9b3fc9d17f028d8711fd128 Mon Sep 17 00:00:00 2001 From: SyniRon <66834451+SyniRon@users.noreply.github.com> Date: Mon, 29 Jun 2026 22:46:35 -0400 Subject: [PATCH 2/2] test(contract): treat the auth-503 as a pre-gzip tier in the Vary guard The Vary spec guard keyed wantVary on `code != "401"`, treating the 401 as the only response that short-circuits before GzipMiddleware. The auth path has a second pre-gzip tier: rest/auth.go returns codeUnavailable (503) on a ValidateApiKey lookup failure, before next.ServeHTTP, so a wire 503 from auth carries no Vary either. The test passes today only because the spec folds 503 into the operation `default`, which resolves to the Vary-carrying Error shape. But the discriminator was brittle: declaring an explicit "503" with the faithful no-Vary body (mirroring how "401" is declared) would flip wantVary to true and fail the guard, blocking a more faithful spec. Key wantVary on the full auth short-circuit set (`code != "401" && code != "503"`) so an explicit 503 is expected to carry no Vary, like the 401. The `default` arm stays Vary-expecting, since the handler errors that resolve to it do. Correct the docstring and comments so they no longer call the 401 the only pre-gzip tier. Test and comments only, no behavior change. > *This was generated by AI* --- contract/spec_test.go | 46 ++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/contract/spec_test.go b/contract/spec_test.go index c966749..95770bb 100644 --- a/contract/spec_test.go +++ b/contract/spec_test.go @@ -944,15 +944,25 @@ func TestSpec_Every200DeclaresCacheControl(t *testing.T) { // signal: GzipMiddleware runs inside auth but outside the mux, so it stamps Vary // on EVERY response a handler produces — all 2xx AND the JSON error tier (the // shared Error response, covering 400/403/404/500 and the default fall-through). -// The only response that carries no Vary is the 401 tier, because AuthMiddleware -// short-circuits before the gzip layer ever runs. So the rule follows that -// "reaches the gzip layer vs the 401 short-circuit" seam, NOT 2xx-vs-non-2xx: +// The responses that carry no Vary are the pre-gzip auth short-circuit tier: +// AuthMiddleware returns before the gzip layer ever runs on two wire statuses — +// the 401 (bad/unknown key) AND the 503 (the DB-outage branch in rest/auth.go, +// codeUnavailable). Both short-circuit upstream of GzipMiddleware, so neither +// carries Vary. So the rule follows that "reaches the gzip layer vs the auth +// short-circuit" seam, NOT 2xx-vs-non-2xx: // -// - every operation 2xx response, every non-401 operation error response -// (the shared Error shape, reached through gzip), the operation default -// fall-through, and the shared Error component MUST declare Vary; -// - the shared Unauthorized component and any literal 401 response MUST NOT — -// the pre-routing auth tier never reaches GzipMiddleware. +// - every operation 2xx response, every operation error response that a +// handler produces (the shared Error shape, reached through gzip), the +// operation default fall-through, and the shared Error component MUST +// declare Vary; +// - the shared Unauthorized component and any literal 401 or 503 response MUST +// NOT — the pre-routing auth tier never reaches GzipMiddleware. +// +// 503 is emitted only by that auth outage path today (codeUnavailable has a +// single producer), so keying an explicit 503 to no-Vary is sound. It is +// currently folded into the operation `default` (no `"503"` is declared), which +// resolves to the Vary-carrying Error shape; this test keeps a faithful future +// `"503"` declaration (no Vary, mirroring the 401) from tripping the guard. // // Without this net a new endpoint can document Cache-Control (which the spec // test above enforces) but forget Vary, and the spec silently drifts from the @@ -973,7 +983,7 @@ func TestSpec_VaryAccompaniesEveryGzippedResponse(t *testing.T) { } if !wantVary { assert.Nil(t, hdr, - "%s: declares Vary but never reaches the gzip layer — the 401 auth tier short-circuits before GzipMiddleware, so it carries no Vary on the wire", where) + "%s: declares Vary but never reaches the gzip layer — the auth tier (401 and the outage-path 503) short-circuits before GzipMiddleware, so it carries no Vary on the wire", where) return } require.NotNil(t, hdr, @@ -1016,12 +1026,20 @@ func TestSpec_VaryAccompaniesEveryGzippedResponse(t *testing.T) { } for rp := orderedmap.First(op.Responses.Codes); rp != nil; rp = rp.Next() { code := rp.Key() - // Everything a handler produces is gzipped; only the 401 tier - // short-circuits before the gzip layer runs. - checkResponse(opPath+".responses."+code, code != "401", rp.Value()) + // Everything a handler produces is gzipped; the auth tier short- + // circuits before the gzip layer runs, and it has two wire + // statuses — the 401 (bad/unknown key) and the 503 (the DB-outage + // branch in rest/auth.go, codeUnavailable). Both return before + // next.ServeHTTP, so neither carries Vary; an explicitly declared + // 401 or 503 is therefore expected to have none. + checkResponse(opPath+".responses."+code, code != "401" && code != "503", rp.Value()) } - // The default fall-through resolves to the shared Error shape, which - // is handler-produced and therefore reached through gzip. + // `default` carries Vary because the handler errors that resolve to it + // do (400/403/404/500, the shared Error shape, all reached through + // gzip). The lone exception folded into `default` is the auth-path 503, + // which OpenAPI's single `default` cannot express separately; that + // outage status is pinned NOT to reach gzip elsewhere (the explicit + // "503"/"401" arm above, and rest/vary_test.go live). checkResponse(opPath+".responses.default", true, op.Responses.Default) } }