stacklok / toolhive / 30337625765

73%

Build Type

push

github

Committed by web-flow

Commit Message

Emit conformant JSON-RPC on denial and error paths (#6055)

* Emit conformant JSON-RPC on authz denials

handleUnauthorized encoded a *jsonrpc2.Response with encoding/json
directly. That struct carries no json tags and no MarshalJSON, so
reflection emitted Go-capitalized keys and dropped the mandatory
"jsonrpc":"2.0". Worse, jsonrpc2.ID's only field is unexported, so it
rendered as {} unconditionally -- destroying the request id even when
valid, which left clients unable to correlate the denial at all.

Encode through jsonrpc2.EncodeMessage instead, which marshals via the
library's wireCombined struct and gets the lowercase tags, the version
tag, and id,omitempty. Omitting an absent id rather than emitting null
is deliberate: MCP types the error response as id?: string | number,
so null is not representable, and the transport spec says the body
"has no id".

Encoding now happens before any header is written, which removes a
double-write where a 403 header could be followed by an http.Error 500.
Renames a local that was assigning the incoming err parameter rather
than shadowing it.

Refs #5950

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Restore the request id on filter-failure responses

writeErrorResponse marshaled a *jsonrpc2.Response with encoding/json,
so like the denial path it emitted capitalized keys, no "jsonrpc":"2.0",
and an id that always serialized as {}. Its marshal-failure fallback was
not valid JSON-RPC either: a bare {"error": "Internal server error"}
with no envelope at all.

Encode through jsonrpc2.EncodeMessage, before writing the header, and
make the fallback a real JSON-RPC error body.

The protocol-violation path also hardcoded an empty id, so fixing the
encoder alone would still have left that response uncorrelatable. Recover
the real id from the parsed request on the context instead.

The typed jsonrpc2.ID parameter is kept deliberately: an any-typed id is
exactly how a caller reintroduces the {} bug by passing the... (continued)

Coverage Stats

38 of 65 new or added lines in 4 files covered. (58.46%)

20 existing lines in 7 files now uncovered.

81063 of 116288 relevant lines covered (69.71%)

98.47 hits per line