Proxy audit middleware records outcome=success for JSON-RPC application-level errors in HTTP 200 responses

[gmogmzGithub]

Bug description

The proxy audit middleware determines the outcome field exclusively from the HTTP status code (pkg/audit/auditor.go:308-322, determineOutcome()). Because MCP servers return application-level errors inside HTTP 200 responses per the JSON-RPC spec, the audit log records outcome=success even when the response body contains a JSON-RPC error object.

This means failures like expired tokens, API errors, and invalid parameters are invisible in the audit trail.

Question for maintainers: Is this transport-only outcome determination a deliberate design choice (keeping the audit middleware protocol-agnostic), or is it a gap worth fixing? If you are open for discussion before submitting a PR, I'm in.

Steps to reproduce

1. Run an MCP server through ToolHive with audit enabled (--audit)
2. Trigger a tool call that results in an application-level error (e.g., use an expired PAT, request a non-existent resource)
3. The MCP server returns HTTP 200 with a JSON-RPC error body:

{
  "jsonrpc": "2.0",
  "id": "...",
  "error": {
    "code": -32603,
    "message": "GitLab API error: 401 Unauthorized"
  }
}

4. Check the proxy audit log — the event is recorded as outcome=success

Expected behavior

The audit log should distinguish between a true success (HTTP 200 with a valid JSON-RPC result) and an application-level failure (HTTP 200 with a JSON-RPC error field). For example, recording outcome=application_error with the JSON-RPC error code and a truncated message.

Actual behavior

All HTTP 200 responses are logged as outcome=success, regardless of the JSON-RPC response body content.

Relevant code — pkg/audit/auditor.go:

// Line 308-322
func (*Auditor) determineOutcome(statusCode int) string {
    switch {
    case statusCode >= 200 && statusCode < 300:
        return OutcomeSuccess
    // ...
    }
}

The response body is only captured when IncludeResponseData is enabled (line 200-202), and even then it's only used for data logging in addEventData() (line 250) — never for outcome determination.

The impact we Observed

During an internal MCP stability pilot with multiple users and workloads, we observed:

• Expired authentication tokens persisting for 5+ days with zero visibility in audit logs. All tool calls showed outcome=success in the proxy log while the MCP log contained repeated code=-32603, "401 Unauthorized" errors.
• API errors (400 Bad Request, 502 Bad Gateway) wrapped in JSON-RPC error responses, all recorded as successful.
• File-not-found and routing errors across multiple MCP servers (GitLab, Slack, Google Workspace), invisible in proxy audit.

In total, we identified 3,700+ silent failures across 5 workloads in the pilot data.

Environment

• ToolHive: latest main branch
• OS: macOS

Additional context

If the team agrees this is worth addressing, I'd like to submit a PR. The approach I have in mind:

1. Always buffer a small portion of the response body (even when IncludeResponseData is off) — enough to detect a JSON-RPC error field
2. In logAuditEvent(), after determining the HTTP-based outcome, check the buffered body for a top-level "error" field when the outcome is success
3. Add OutcomeApplicationError as a new outcome constant and record the JSON-RPC error code/message as metadata

This would be additive — existing outcome values remain unchanged, and consumers that don't know about application_error won't break.

[ChrisJBurns]

Great bug report — the analysis and impact data are compelling. I've got some agents to dig into the codebase to validate the proposed approach and have some findings that should help shape the PR.

The bug is confirmed

determineOutcome() at pkg/audit/auditor.go:309 is purely HTTP-status-code-based. JSON-RPC errors inside HTTP 200 bodies are indeed invisible to audit. This is a gap, not a deliberate design choice.

Key architectural finding: the MCP parser can't help here

The proposed approach of checking the response body is correct, but we want to flag that the MCP parser (pkg/mcp/parser.go) only parses requests, not responses (line 152-157 explicitly discards non-*jsonrpc2.Request messages). Additionally, context.Context flows inward with the request — there's no mechanism to propagate response data back outward through context. So the fix must live entirely within the audit middleware itself.

Transport-specific limitations to be aware of

Not all transports allow response body inspection:

Transport	Response visible to audit middleware?	Fix feasible?
Streamable HTTP (JSON response)	Yes — full body flows through responseWriter.Write()	Yes
Streamable HTTP (SSE response)	Partial (SSE event frames)	Possible but complex
Legacy SSE (POST → 202 Accepted)	No — JSON-RPC response arrives on a separate GET connection	Not possible via this approach

For the legacy SSE transport, the POST returns 202 immediately and the actual JSON-RPC response flows asynchronously over a different HTTP connection. Any response-body-based approach can't cover this path.

Implementation guidance

1. Scope the initial PR to Streamable HTTP JSON responses — this is the primary modern transport and covers the most common case.
2. Add a dedicated config flag (e.g., DetectApplicationErrors, default true) rather than piggybacking on IncludeResponseData. That flag is privacy-sensitive and off by default — gating error detection behind it would leave the bug unfixed in the common configuration.
3. Buffer a small response prefix unconditionally (~256-512 bytes) in responseWriter.Write(), independent of IncludeResponseData. JSON-RPC error responses have "error" near the top of the object, so a small prefix is sufficient. The per-request overhead of a small buffer is negligible compared to the JSON-RPC parsing and container I/O already happening.
4. Add a lightweight ParseMCPResponse() helper in pkg/mcp/ (separate from the request parser) that the audit middleware can call on the captured prefix bytes. The jsonrpc2.DecodeMessage function from golang.org/x/exp/jsonrpc2 already handles response decoding.
5. In logAuditEvent(), after determineOutcome(rw.statusCode), check the buffered prefix for a JSON-RPC error when the HTTP outcome is success, and override to OutcomeApplicationError with the error code/message as metadata.
6. Document the SSE transport limitation and consider a follow-up issue for it.

The middleware chain order is already favorable — the MCP parser runs before audit, so mcp.GetMCPMethod(r.Context()) is available for contextualizing which methods had application errors.

Happy to review a PR when it's ready.