Close authz response filter bypasses via media type and status

[JAORMX]

Summary

The authz response filter (pkg/authz/response_filter.go, FlushAndFilter) is what enforces Cedar policy on tools/list, prompts/list, resources/list, and find_tool results — it strips items the caller may not see. The untrusted MCP server behind the gateway (the primary actor in this codebase's threat model) could bypass it entirely in two ways, both confirmed by an executed test harness during a security review, not merely reasoned about. In both cases the caller receives the complete unfiltered inventory, including items Cedar denied, and nothing is logged.

• Media type not normalized. The filter matched the Content-Type header exactly against application/json / text/event-stream after splitting on ;. RFC 9110 §8.3.1 makes media types case-insensitive and permits whitespace before the parameter separator, so Content-Type: Application/JSON and Content-Type: application/json ; charset=utf-8 are both legal, both parsed as JSON by clients, and both fell to the default: branch — raw, unfiltered, silent passthrough.
• Non-200/202 2xx short-circuited before filtering. Anything other than HTTP 200/202 was passed through unfiltered as "not a successful response". A backend answering tools/list with HTTP 201 sailed past the filter; fetch-based MCP clients (including the reference TypeScript transport) gate on response.ok, which accepts all of 200–299, so the client parses the 201 body as a normal JSON-RPC response and the caller acts on it.

The in-repo precedent is pkg/mcp/tool_filter.go, the sibling tool filter: it already switches on media type before consulting the status code, and its processUnrecognizedMimeType sniffs a mislabeled 2xx body under both supported shapes and fails closed. Its docstring names this exact attack — "a backend deliberately omitting/mislabeling it to smuggle an unfiltered tools list past the filter (the same disguised-result concern handled in pkg/authz for #5257)". The authz filter is simply the one that got left behind; this PR mirrors the sibling's structure and failure posture.

What changed:

• Normalize the media type (lowercase, trim whitespace around the first ;-segment) before the switch, so casing and whitespace games can no longer select the passthrough branch.
• Filter the entire 2xx range instead of only 200/202 (decision rationale below).
• On a 2xx response to a filtered method whose media type is still unrecognized after normalization, sniff the body under both MCP-supported shapes (whole-body JSON-RPC and SSE data: lines, reusing the existing carriesResult fail-closed helper). A body carrying a JSON-RPC result is processed exactly as if it had been labeled correctly — filtered when it is a clean frame, dropped (fail closed, with a WARN log) when it is not. Only a body carrying no result under either shape passes through.

Status-handling decision

• All 2xx are filtered (200 <= status < 300), not just 200/202. The client-side reality is response.ok, which is the whole 2xx range; any 2xx can deliver a list to the caller, so any 2xx must be filtered. The requiresResponseFiltering(method) gate and the Content-Length deletion on the filtering paths are preserved unchanged.
• 204 No Content: in the 2xx range, but has no body — the existing len(rawResponse) == 0 early return passes it through untouched (verified: that check runs before the media-type switch).
• 2xx with an empty body: same early return, unchanged behavior.
• 304 Not Modified: not 2xx, passes through; it carries no body by definition.
• Non-2xx pass through unfiltered, as before: an error body is not a list result and a compliant client will not treat it as one, and rewriting error bodies would only hurt debuggability. A regression case pinning this (500 with a list-shaped body passes through) is in the test table so the decision is explicit, not accidental.
• Unrecognized media type on a filtered-method 2xx now fails closed rather than passing through. I judged this in scope for this PR: it is the third leg of the same disguised-result class, the sibling filter already does it, and it took one small sniffing helper (sseCarriesResult) plus a default: branch that routes to the existing, untouched JSON/SSE processors. Leaving it out would have fixed the two named header spellings while leaving Content-Type: text/plain as a working bypass.

One existing test needed updating as a direct consequence: TestResponseFilteringWriter_ErrorResponse built its "error response" with json.Marshal on the jsonrpc2.Response struct (producing Go field names — {"Result":null,"Error":...}, not a JSON-RPC wire frame) and set no Content-Type. That is precisely the mislabeled-body-with-a-result shape that now fails closed (Go's case-insensitive field matching sees "Result":null as a carried result). The test's intent — a JSON-RPC error response passes through unchanged — is preserved by encoding a real wire frame via jsonrpc2.EncodeMessage and declaring application/json.

Deliberately not fixed here

• The per-line SSE smuggling class. processSSEResponse / writeSSEDataLine are untouched; Parse SSE responses per event, not per line #6088 rewrites that parsing wholesale and owns the territory (Frame SSE filter-failure errors as SSE events #6087 is adjacent). This PR changes zero lines in those functions — the new default: branch only calls the existing SSE processor.
• Pagination-cursor existence oracle. Every list filter copies PaginatedResult (and thus nextCursor) verbatim from the backend response. A caller denied everything on a page still receives an empty page with a cursor, revealing that hidden items exist and roughly how many pages of them there are. Noting it here so it is not lost; it needs its own design discussion (dropping the cursor would break legitimate pagination through mixed pages).

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)

• Linting (task lint)

• Manual testing (describe below)

• New regression test TestResponseFilteringWriter_FilterBypassAttempts (table-driven, t.Parallel(), testify), using a Cedar policy that permits only weather and asserting the denied admin_tool is absent from the client-visible body for: Content-Type: Application/JSON, Content-Type: application/json ; charset=utf-8, HTTP 201 with correct JSON labeling, text/plain labeling over a JSON list body, and a missing Content-Type over an SSE-shaped list body — plus passthrough cases pinning that a resultless unlabeled body and a non-2xx error body remain untouched.

• Fail-before proof: with the production change stashed (tests kept), all five bypass cases fail with "denied tool leaked to the client" and both passthrough cases pass; with the fix restored, go test ./pkg/authz/... is fully green.

• task test ran to completion in my environment (the gotestfmt v2.5.0 panic did not reproduce this run) but reports three failures — TestCompositeProvider_RealProviders, TestNewServer_ReadTimeoutConfigured, TestSecurityHeaders — that are sandbox-environment issues (no keyring/network access) and fail identically on a clean origin/main checkout; go test ./pkg/authz/... was used as the targeted verification.

• task lint with an isolated GOLANGCI_LINT_CACHE: 0 issues; go vet ./... and go build ./... clean.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

No operator API surface is touched.

Changes

File	Change
pkg/authz/response_filter.go	Filter the whole 2xx range; normalize media type per RFC 9110 before matching; sniff unrecognized-media-type 2xx bodies on filtered methods and fail closed on a carried result (sseCarriesResult helper)
pkg/authz/response_filter_test.go	Regression table for both confirmed bypasses and the sniffing paths, with passthrough pins for non-2xx and resultless bodies; fix TestResponseFilteringWriter_ErrorResponse to write a real JSON-RPC wire frame with a declared Content-Type

Does this introduce a user-facing change?

Yes — this is a behavior change, and the point of the PR: backend responses that previously passed through the authz gateway unfiltered are now filtered. In particular, a backend that legitimately returns a non-200 2xx (e.g. 201) for a list method will now have that list filtered by Cedar policy, and a 2xx list response with a missing or unrecognized Content-Type is now filtered or rejected instead of passed through raw. Compliant backends (200/202, correctly labeled) see no change.

Special notes for reviewers

• The status check is now a range check (< 200 || >= 300) rather than an allowlist; 204/empty-body behavior is unchanged because the pre-existing empty-body early return runs first.
• The default: branch intentionally routes sniffed bodies into the existing processJSONResponse/processSSEResponse — no changes to SSE per-line logic, writeSSEDataLine, or errorResponseBody, to stay out of the way of Parse SSE responses per event, not per line #6088's SSE rewrite (which changes zero production lines in FlushAndFilter, the status check, or this switch).
• A sniffed SSE-ish body with no detectable line separator makes processSSEResponse return an error; the middleware then drops the response and logs a warning — fail closed, which is the intended posture for a backend violating the spec on two axes at once.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y5WXPQr5Da9Cox2nZWhg6n

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.32%. Comparing base (e2c4441) to head (70baeaa).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6092      +/-   ##
==========================================
+ Coverage   72.26%   72.32%   +0.06%     
==========================================
  Files         728      728              
  Lines       75523    75541      +18     
==========================================
+ Hits        54575    54637      +62     
+ Misses      17045    16982      -63     
- Partials     3903     3922      +19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.