Strip leading UTF-8 BOM before filtering list responses

[Yanhaoxi]

Summary

A client strips a leading UTF-8 BOM per the WHATWG UTF-8 decode algorithm before parsing, but the response filters decoded and sniffed the raw bytes. Go's encoding/json treats EF BB BF as a syntax error (not whitespace), so a BOM-prefixed list response failed every decode and smuggled-result sniff and passed through unfiltered — leaking tools/prompts/resources that the Cedar policy (authz) or tool filter (mcp) was supposed to remove. This is the same #5257-class leak the SSE processing path was already hardened against; the JSON decode path, both FlushAndFilter default-branch sniffs, and the tool filter's unrecognized-media-type sniff were not.

WHAT changed:

• Strip one leading BOM in FlushAndFilter before the media-type dispatch (pkg/authz/response_filter.go), covering the application/json decode path plus the JSON and SSE smuggled-result sniffs in the default branch.
• Strip one leading BOM at the top of processUnrecognizedMimeType (pkg/mcp/tool_filter.go), covering both its JSON sniff and sniffSSEToolsList.
• Add regression tests mirroring the existing SSE BOM test for each fixed route.

Fixes #6301

Type of change

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

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Run: go test ./pkg/authz/... ./pkg/mcp/.... New tests:
TestResponseFilteringWriter_JSON_LeadingBOMBypass (JSON path + JSON sniff), TestResponseFilteringWriter_Sniff_SSE_LeadingBOMBypass (SSE sniff), TestNewListToolsMappingMiddleware_BOMCannotSmuggleToolsList (tool filter, JSON + SSE). Verified the new tests FAIL without the source fix and PASS with it; full pkg/authz/... and pkg/mcp/... suites pass.

Lint: task lint reports only 3 pre-existing gci issues in files this PR does not touch (pkg/authz/annotation_cache.go, pkg/authz/authorizers.go, pkg/authz/config.go), confirmed present on clean main; none of this PR's 4 files are flagged. go vet clean, gofmt clean.

API Compatibility

• This PR does not break the v1beta1 API.

Changes

File	Change
pkg/authz/response_filter.go	Strip leading BOM in FlushAndFilter before media-type dispatch
pkg/authz/response_filter_test.go	Add JSON-path and SSE-sniff BOM regression tests
pkg/mcp/tool_filter.go	Strip leading BOM in processUnrecognizedMimeType
pkg/mcp/tool_filter_test.go	Add tool-filter BOM regression tests (JSON + SSE)

Does this introduce a user-facing change?

Yes: a misbehaving/untrusted upstream that prepends a UTF-8 BOM to a list response can no longer bypass list filtering; the denied items are filtered as before.

Implementation plan

Approved implementation plan

Four code points share one root cause: no leading-BOM strip before decode/sniff.

• authz (3 points in one call chain): processJSONResponse decode + fail-closed check, JSON sniff (carriesResult), SSE sniff (sseCarriesResult). Fixed with one bytes.TrimPrefix in FlushAndFilter before the media-type switch.
• mcp (1 point, separate component): processUnrecognizedMimeType JSON sniff + sniffSSEToolsList. Fixed with one bytes.TrimPrefix at the function entry (covers both sniffs).
• Regression tests mirror TestResponseFilteringWriter_SSE_LeadingBOMBypass per route.

Special notes for reviewers

• The existing SSE-processing BOM strip (processSSEResponse) is retained; bytes.TrimPrefix is idempotent so the redundant strip is harmless.
• The unrecognized-media-type sniff branch is only reached when carriesResult/sseCarriesResult would previously have returned false on the BOM — the fix restores the fail-closed behavior those branches were built for (authz: SSE response filter leaks unfiltered tools/list on undecodable / non-Response data lines (bypasses cedar) #5257).
• Known limitation unchanged: the tool filter's sniff has no "incomplete, wait for more" notion (documented in processUnrecognizedMimeType) — the BOM fix does not alter that.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.01%. Comparing base (7854115) to head (404241d).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6304      +/-   ##
==========================================
+ Coverage   72.82%   73.01%   +0.18%     
==========================================
  Files         742      742              
  Lines       77762    78401     +639     
==========================================
+ Hits        56632    57241     +609     
+ Misses      17155    17153       -2     
- Partials     3975     4007      +32     

☔ 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.

[reyortiz3]

Thanks for the thorough writeup and tests here — this is a solid fix for the #6301 bypass class.

One correctness issue found on review:

pkg/authz/response_filter.go: the BOM strip in FlushAndFilter breaks Content-Length on the fallback passthrough path

The new bytes.TrimPrefix at line ~101 unconditionally strips the BOM from rawResponse before the media-type switch. Every branch that goes on to write a body already deletes Content-Length first (application/json, text/event-stream, and both "carries result" branches in default) — but the final fallback in default, reached when neither carriesResult nor sseCarriesResult matches, writes rawResponse directly with no Content-Length deletion:

rfw.ResponseWriter.WriteHeader(rfw.statusCode)
_, err := rfw.ResponseWriter.Write(rawResponse)
return err

Before this PR, that passthrough never altered bytes, so Content-Length stayed accurate. Now, any body that starts with a BOM but doesn't carry a recognizable JSON-RPC result under either sniff (a non-list response, or a legitimately BOM-prefixed body that isn't smuggling anything) gets 3 bytes silently trimmed while Content-Length still reflects the original size — which, per the comments elsewhere in this file, causes Go's HTTP server to detect the mismatch and tear down the connection.

Suggest deleting Content-Length before that final Write too, same as the other branches.

Minor: the BOM literal is now duplicated three times

"\xEF\xBB\xBF" appears at the pre-existing processSSEResponse (line 258), and the two new strips here and in pkg/mcp/tool_filter.go. Since this is a security-relevant magic value, worth hoisting into a single exported constant — pkg/mcp/utils.go looks like a natural home (already the place for small cross-cutting JSON-RPC helpers), something like:

// UTF8BOM is the 3-byte UTF-8 byte order mark. Clients that follow the
// WHATWG UTF-8 decode algorithm strip a leading BOM before parsing; any
// code decoding or sniffing a JSON-RPC body from the wire must strip it
// first to match that behavior.
var UTF8BOM = []byte("\xEF\xBB\xBF")

pkg/authz already imports pkg/mcp as mcpparser, so this doesn't need a new dependency. Not blocking, just reduces the risk of a future edit fixing only two of the three copies.

[Yanhaoxi]

Thanks for the review — addressed both points.

• Cleared Content-Length in the final fallback passthrough before writing the BOM-stripped body, preventing the three-byte length mismatch.
• Hoisted the shared BOM bytes into exported mcp.UTF8BOM and updated all three stripping sites to use it.
• Added a regression test covering a BOM-prefixed fallback passthrough response with an upstream Content-Length.

The PR has been updated.

[reyortiz3]

Thanks @Yanhaoxi !! Approved.