diff --git a/pkg/webhook/client.go b/pkg/webhook/client.go index cec045bd97..d79f72185f 100644 --- a/pkg/webhook/client.go +++ b/pkg/webhook/client.go @@ -12,6 +12,7 @@ import ( "errors" "fmt" "io" + "log/slog" "net" "net/http" "os" @@ -148,16 +149,32 @@ func (c *Client) doHTTPCall(ctx context.Context, body []byte) ([]byte, error) { // 5xx errors indicate webhook operational failures. if resp.StatusCode >= http.StatusInternalServerError { + // Body preview is logged at debug level so operators can troubleshoot, + // but is kept out of the returned error chain to avoid surfacing + // potentially sensitive bytes (e.g. from an internal service reached + // via a misconfigured URL) into higher-level error logs. + slog.Debug("webhook returned server error", + "webhook", c.config.Name, + "url", c.config.URL, + "status_code", resp.StatusCode, + "body_preview", truncateBody(respBody), + ) return nil, NewNetworkError(c.config.Name, - fmt.Errorf("webhook returned HTTP %d: %s", resp.StatusCode, truncateBody(respBody))) + fmt.Errorf("webhook returned HTTP %d", resp.StatusCode)) } // Non-200 responses (excluding 5xx handled above) are treated as invalid. // The StatusCode is surfaced so callers can distinguish HTTP 422 (RFC always-deny) // from other non-2xx codes that may follow the failure policy. if resp.StatusCode != http.StatusOK { + slog.Debug("webhook returned non-2xx response", + "webhook", c.config.Name, + "url", c.config.URL, + "status_code", resp.StatusCode, + "body_preview", truncateBody(respBody), + ) return nil, NewInvalidResponseError(c.config.Name, - fmt.Errorf("webhook returned HTTP %d: %s", resp.StatusCode, truncateBody(respBody)), + fmt.Errorf("webhook returned HTTP %d", resp.StatusCode), resp.StatusCode) } diff --git a/pkg/webhook/client_test.go b/pkg/webhook/client_test.go index 377fc6f0a8..7af2b87815 100644 --- a/pkg/webhook/client_test.go +++ b/pkg/webhook/client_test.go @@ -783,6 +783,69 @@ func TestDoHTTPCallReadError(t *testing.T) { assert.Contains(t, err.Error(), "forced read error") } +// TestClientErrorDoesNotLeakResponseBody verifies that when the webhook returns +// a non-2xx response, the response body bytes are not embedded in the returned +// error chain. This prevents an SSRF-adjacent information-exfiltration path +// where bytes from an internal service reached via a misconfigured webhook URL +// would otherwise be surfaced into higher-level error logs. +func TestClientErrorDoesNotLeakResponseBody(t *testing.T) { + t.Parallel() + + const sentinel = "INTERNAL_LEAK_TOKEN" + + tests := []struct { + name string + statusCode int + }{ + {name: "500 Internal Server Error", statusCode: http.StatusInternalServerError}, + {name: "503 Service Unavailable", statusCode: http.StatusServiceUnavailable}, + {name: "422 Unprocessable Entity", statusCode: http.StatusUnprocessableEntity}, + {name: "418 I'm a teapot", statusCode: http.StatusTeapot}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.statusCode) + _, _ = w.Write([]byte("response containing " + sentinel + " somewhere in the body")) + })) + defer server.Close() + + cfg := Config{ + Name: "leak-test", + URL: server.URL, + Timeout: 5 * time.Second, + FailurePolicy: FailurePolicyFail, + } + + client := newTestClient(cfg, TypeValidating, nil) + + req := &Request{ + Version: APIVersion, + UID: "test-uid", + Timestamp: time.Now(), + Principal: &auth.PrincipalInfo{Subject: "user1"}, + Context: &RequestContext{ + ServerName: "test-server", + SourceIP: "127.0.0.1", + Transport: "sse", + }, + } + + _, err := client.Call(t.Context(), req) + require.Error(t, err) + + errMsg := err.Error() + assert.Contains(t, errMsg, strconv.Itoa(tt.statusCode), + "error should surface the HTTP status code so callers can distinguish failure modes") + assert.NotContains(t, errMsg, sentinel, + "error must not embed response body bytes — they may come from an internal service reached via a misconfigured URL") + }) + } +} + type mockRoundTripper struct { resp *http.Response err error