Detect JSON-RPC application errors in proxy audit middleware

The audit middleware previously determined outcome solely from the HTTP
status code. Because MCP servers return errors inside HTTP 200 responses
per the JSON-RPC spec, application-level failures (expired tokens, API
errors, invalid parameters) were logged as outcome=success, making them
invisible in audit logs.

This change inspects the response body for a JSON-RPC error field when
the HTTP status is 200. If found, the outcome is recorded as
application_error with the error code and truncated message in metadata.

A new DetectApplicationErrors config flag (default true) controls the
behavior independently of IncludeResponseData, avoiding the need to
enable full response capture just for error detection.

Fixes #4678

Signed-off-by: Guillermo Gomez <guillermogomezmora@gmail.com>

Author: gmogmzGithub

pkg/audit/auditor.go

@@ -106,12 +106,26 @@ func (a *Auditor) isSSETransport() bool {
 	return a.transportType == types.TransportTypeSSE.String()
 }
 
+// errorDetectionBufferSize is the maximum number of bytes buffered from the
+// response body for JSON-RPC error detection. JSON-RPC error responses have
+// the "error" field near the top of the object, so a small prefix is
+// sufficient. This buffer is allocated independently of IncludeResponseData.
+const errorDetectionBufferSize = 512
+
+// maxAuditErrorMessageLength caps the JSON-RPC error message length stored
+// in audit event metadata to keep log entries compact.
+const maxAuditErrorMessageLength = 256
+
 // responseWriter wraps http.ResponseWriter to capture response data and status.
 type responseWriter struct {
 	http.ResponseWriter
 	statusCode int
 	body       *bytes.Buffer
-	auditor    *Auditor
+	// errorDetectionBody is a small prefix buffer used exclusively for
+	// JSON-RPC error detection. It is allocated when DetectApplicationErrors
+	// is true, independent of IncludeResponseData.
+	errorDetectionBody *bytes.Buffer
+	auditor            *Auditor
 }
 
 func (rw *responseWriter) WriteHeader(statusCode int) {
@@ -127,6 +141,15 @@ func (rw *responseWriter) Write(data []byte) (int, error) {
 			rw.body.Write(data)
 		}
 	}
+	// Capture a small prefix for JSON-RPC error detection
+	if rw.errorDetectionBody != nil && rw.errorDetectionBody.Len() < errorDetectionBufferSize {
+		remaining := errorDetectionBufferSize - rw.errorDetectionBody.Len()
+		if len(data) <= remaining {
+			rw.errorDetectionBody.Write(data)
+		} else {
+			rw.errorDetectionBody.Write(data[:remaining])
+		}
+	}
 	return rw.ResponseWriter.Write(data)
 }
 
@@ -201,6 +224,13 @@ func (a *Auditor) Middleware(next http.Handler) http.Handler {
 			rw.body = &bytes.Buffer{}
 		}
 
+		// Allocate a small prefix buffer for JSON-RPC error detection,
+		// independent of IncludeResponseData. When IncludeResponseData
+		// is already true, we reuse rw.body instead of double-buffering.
+		if a.config.ShouldDetectApplicationErrors() && !a.config.IncludeResponseData {
+			rw.errorDetectionBody = &bytes.Buffer{}
+		}
+
 		// Process the request
 		next.ServeHTTP(rw, r)
 
@@ -220,6 +250,29 @@ func (a *Auditor) logAuditEvent(r *http.Request, rw *responseWriter, requestData
 	// Determine outcome based on status code
 	outcome := a.determineOutcome(rw.statusCode)
 
+	// When HTTP status indicates success, check the response body for
+	// JSON-RPC errors (e.g., expired tokens wrapped inside HTTP 200).
+	// Reuse rw.body when IncludeResponseData is on to avoid double-buffering.
+	var mcpResponse *mcp.ParsedMCPResponse
+	if outcome == OutcomeSuccess && a.config.ShouldDetectApplicationErrors() {
+		var prefix []byte
+		if rw.body != nil && rw.body.Len() > 0 {
+			prefix = rw.body.Bytes()
+			if len(prefix) > errorDetectionBufferSize {
+				prefix = prefix[:errorDetectionBufferSize]
+			}
+		} else if rw.errorDetectionBody != nil && rw.errorDetectionBody.Len() > 0 {
+			prefix = rw.errorDetectionBody.Bytes()
+		}
+		// Only attempt JSON parse if the prefix looks like a JSON object
+		if len(prefix) > 0 && prefix[0] == '{' {
+			mcpResponse = mcp.ParseMCPResponse(prefix)
+			if mcpResponse.HasError {
+				outcome = OutcomeApplicationError
+			}
+		}
+	}
+
 	// Check if we should audit this event
 	if !a.config.ShouldAuditEvent(eventType) {
 		return
@@ -246,6 +299,20 @@ func (a *Auditor) logAuditEvent(r *http.Request, rw *responseWriter, requestData
 	// Add metadata
 	a.addMetadata(event, r, duration, rw)
 
+	// Attach JSON-RPC error details so operators can see the error code
+	// and message without enabling full response data capture.
+	if outcome == OutcomeApplicationError {
+		if event.Metadata.Extra == nil {
+			event.Metadata.Extra = make(map[string]any)
+		}
+		event.Metadata.Extra["jsonrpc_error_code"] = mcpResponse.ErrorCode
+		msg := mcpResponse.ErrorMessage
+		if len(msg) > maxAuditErrorMessageLength {
+			msg = msg[:maxAuditErrorMessageLength]
+		}
+		event.Metadata.Extra["jsonrpc_error_message"] = msg
+	}
+
 	// Add request/response data if configured
 	a.addEventData(event, r, rw, requestData)
 

pkg/audit/auditor_test.go

@@ -803,3 +803,202 @@ func TestExtractSourceWithHeaders(t *testing.T) {
 	assert.Equal(t, "TestAgent/1.0", source.Extra[SourceExtraKeyUserAgent])
 	assert.Equal(t, "req-12345", source.Extra[SourceExtraKeyRequestID])
 }
+
+func TestErrorDetectionBodyCapture(t *testing.T) {
+	t.Parallel()
+
+	t.Run("captures prefix when DetectApplicationErrors is enabled", func(t *testing.T) {
+		t.Parallel()
+		detectErrors := true
+		config := &Config{
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+
+		rw := &responseWriter{
+			ResponseWriter:     httptest.NewRecorder(),
+			statusCode:         http.StatusOK,
+			auditor:            auditor,
+			errorDetectionBody: &bytes.Buffer{},
+		}
+
+		responseData := `{"jsonrpc":"2.0","id":"1","error":{"code":-32603,"message":"test error"}}`
+		_, err = rw.Write([]byte(responseData))
+		require.NoError(t, err)
+
+		assert.Equal(t, responseData, rw.errorDetectionBody.String())
+	})
+
+	t.Run("does not capture when DetectApplicationErrors is disabled", func(t *testing.T) {
+		t.Parallel()
+		detectErrors := false
+		config := &Config{
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+
+		rw := &responseWriter{
+			ResponseWriter: httptest.NewRecorder(),
+			statusCode:     http.StatusOK,
+			auditor:        auditor,
+			// errorDetectionBody is nil when detection is disabled
+		}
+
+		_, err = rw.Write([]byte(`{"error":{"code":-32603}}`))
+		require.NoError(t, err)
+
+		assert.Nil(t, rw.errorDetectionBody)
+	})
+
+	t.Run("truncates capture at buffer size limit", func(t *testing.T) {
+		t.Parallel()
+		detectErrors := true
+		config := &Config{
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+
+		rw := &responseWriter{
+			ResponseWriter:     httptest.NewRecorder(),
+			statusCode:         http.StatusOK,
+			auditor:            auditor,
+			errorDetectionBody: &bytes.Buffer{},
+		}
+
+		// Write more than errorDetectionBufferSize bytes
+		largeData := bytes.Repeat([]byte("x"), errorDetectionBufferSize+100)
+		_, err = rw.Write(largeData)
+		require.NoError(t, err)
+
+		assert.Equal(t, errorDetectionBufferSize, rw.errorDetectionBody.Len())
+	})
+
+	t.Run("captures independently of IncludeResponseData", func(t *testing.T) {
+		t.Parallel()
+		detectErrors := true
+		config := &Config{
+			IncludeResponseData:     false,
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+
+		rw := &responseWriter{
+			ResponseWriter:     httptest.NewRecorder(),
+			statusCode:         http.StatusOK,
+			auditor:            auditor,
+			errorDetectionBody: &bytes.Buffer{},
+			// body is nil because IncludeResponseData is false
+		}
+
+		responseData := `{"jsonrpc":"2.0","id":"1","error":{"code":-32603,"message":"unauthorized"}}`
+		_, err = rw.Write([]byte(responseData))
+		require.NoError(t, err)
+
+		// errorDetectionBody should capture even though body is nil
+		assert.Equal(t, responseData, rw.errorDetectionBody.String())
+		assert.Nil(t, rw.body)
+	})
+}
+
+func TestMiddlewareDetectsJSONRPCErrors(t *testing.T) {
+	t.Parallel()
+
+	t.Run("overrides outcome to application_error for JSON-RPC error response", func(t *testing.T) {
+		t.Parallel()
+		var logBuf bytes.Buffer
+		detectErrors := true
+		config := &Config{
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+		auditor.auditLogger = NewAuditLogger(&logBuf)
+
+		errorResponse := `{"jsonrpc":"2.0","id":"1","error":{"code":-32603,"message":"GitLab API error: 401 Unauthorized"}}`
+		handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			_, err := w.Write([]byte(errorResponse))
+			require.NoError(t, err)
+		})
+
+		middleware := auditor.Middleware(handler)
+		req := httptest.NewRequest("POST", "/mcp", strings.NewReader(`{"jsonrpc":"2.0","id":"1","method":"tools/call","params":{"name":"test"}}`))
+		req.Header.Set("Content-Type", "application/json")
+		rr := httptest.NewRecorder()
+
+		middleware.ServeHTTP(rr, req)
+
+		// The response should still be passed through unchanged
+		assert.Equal(t, http.StatusOK, rr.Code)
+		assert.Equal(t, errorResponse, rr.Body.String())
+
+		// The audit log should contain application_error
+		logOutput := logBuf.String()
+		assert.Contains(t, logOutput, OutcomeApplicationError)
+		assert.Contains(t, logOutput, "jsonrpc_error_code")
+	})
+
+	t.Run("keeps outcome=success for valid JSON-RPC result", func(t *testing.T) {
+		t.Parallel()
+		var logBuf bytes.Buffer
+		detectErrors := true
+		config := &Config{
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+		auditor.auditLogger = NewAuditLogger(&logBuf)
+
+		successResponse := `{"jsonrpc":"2.0","id":"1","result":{"content":[{"type":"text","text":"hello"}]}}`
+		handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			_, err := w.Write([]byte(successResponse))
+			require.NoError(t, err)
+		})
+
+		middleware := auditor.Middleware(handler)
+		req := httptest.NewRequest("POST", "/mcp", strings.NewReader(`{"jsonrpc":"2.0","id":"1","method":"tools/call","params":{"name":"test"}}`))
+		req.Header.Set("Content-Type", "application/json")
+		rr := httptest.NewRecorder()
+
+		middleware.ServeHTTP(rr, req)
+
+		assert.Equal(t, http.StatusOK, rr.Code)
+
+		logOutput := logBuf.String()
+		assert.NotContains(t, logOutput, OutcomeApplicationError)
+	})
+
+	t.Run("does not inspect body when DetectApplicationErrors is disabled", func(t *testing.T) {
+		t.Parallel()
+		var logBuf bytes.Buffer
+		detectErrors := false
+		config := &Config{
+			DetectApplicationErrors: &detectErrors,
+		}
+		auditor, err := NewAuditorWithTransport(config, "streamable-http")
+		require.NoError(t, err)
+		auditor.auditLogger = NewAuditLogger(&logBuf)
+
+		errorResponse := `{"jsonrpc":"2.0","id":"1","error":{"code":-32603,"message":"should not be detected"}}`
+		handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			_, err := w.Write([]byte(errorResponse))
+			require.NoError(t, err)
+		})
+
+		middleware := auditor.Middleware(handler)
+		req := httptest.NewRequest("POST", "/mcp", strings.NewReader(`{"jsonrpc":"2.0","id":"1","method":"tools/call","params":{"name":"test"}}`))
+		req.Header.Set("Content-Type", "application/json")
+		rr := httptest.NewRecorder()
+
+		middleware.ServeHTTP(rr, req)
+
+		logOutput := logBuf.String()
+		assert.NotContains(t, logOutput, OutcomeApplicationError)
+	})
+}

pkg/audit/config.go

@@ -40,6 +40,14 @@ type Config struct {
 	// +kubebuilder:default=false
 	// +optional
 	IncludeResponseData bool `json:"includeResponseData,omitempty" yaml:"includeResponseData,omitempty"`
+	// DetectApplicationErrors controls whether the audit middleware inspects
+	// JSON-RPC response bodies for application-level errors when the HTTP
+	// status code indicates success (2xx). When enabled, a small prefix of
+	// the response body is buffered to detect JSON-RPC error fields,
+	// independent of the IncludeResponseData setting.
+	// +kubebuilder:default=true
+	// +optional
+	DetectApplicationErrors *bool `json:"detectApplicationErrors,omitempty" yaml:"detectApplicationErrors,omitempty"`
 	// MaxDataSize limits the size of request/response data included in audit logs (in bytes).
 	// +kubebuilder:default=1024
 	// +optional
@@ -66,13 +74,24 @@ func (c *Config) GetLogWriter() (io.Writer, error) {
 
 // DefaultConfig returns a default audit configuration.
 func DefaultConfig() *Config {
+	detectErrors := true
 	return &Config{
 		// Note, these defaults are also present on the kubebuilder annotations above.
 		// If you change these defaults, you must also change the kubebuilder annotations.
-		IncludeRequestData:  false, // Disabled by default for privacy
-		IncludeResponseData: false, // Disabled by default for privacy
-		MaxDataSize:         1024,  // 1KB default limit
+		IncludeRequestData:      false,        // Disabled by default for privacy
+		IncludeResponseData:     false,        // Disabled by default for privacy
+		MaxDataSize:             1024,         // 1KB default limit
+		DetectApplicationErrors: &detectErrors, // Enabled by default to surface JSON-RPC errors
+	}
+}
+
+// ShouldDetectApplicationErrors returns whether JSON-RPC error detection is enabled.
+// Defaults to true when DetectApplicationErrors is nil.
+func (c *Config) ShouldDetectApplicationErrors() bool {
+	if c.DetectApplicationErrors == nil {
+		return true
 	}
+	return *c.DetectApplicationErrors
 }
 
 // LoadFromFile loads audit configuration from a file.

pkg/audit/event.go

@@ -210,6 +210,10 @@ const (
 	OutcomeError = "error"
 	// OutcomeDenied indicates the event was denied (e.g., by authorization)
 	OutcomeDenied = "denied"
+	// OutcomeApplicationError indicates the HTTP transport succeeded but the
+	// JSON-RPC response body contained an application-level error (e.g.,
+	// expired tokens, backend failures, invalid parameters).
+	OutcomeApplicationError = "application_error"
 )
 
 // Common source types