Information disclosure

PoC demonstration (conceptual): Environment: Grafana 12.4.x with a Prometheus datasource that requires authentication. 1) Configure a Prometheus datasource (datasourceUID: ds-prom) pointing to a protected Prometheus instance. 2) Trigger a dashboard validator/compatibility check that queries the Prometheus API via the dashvalidator (e.g., POST to the relevant internal endpoint with datasourceUID and datasourceURL). 3) Cause the upstream Prometheus to return an error (e.g., 401 Unauthorized) with a raw body containing sensitive information (HTTP 401 with body like: {"error":"invalid credentials","details":"Secret path /internal/..."}). Before the fix (what an attacker could observe): - The Grafana API response includes the upstream raw body or the upstream URL in the error payload, e.g.: { "message": "Prometheus API returned status 401", "details": {"url": "http://prometheus.internal:9090/api/v1/series", "responseBody": "{...internal stack/secret...}"} } - An attacker could glean internal endpoints, error details, or secrets from the response body. After the fix (what an attacker will observe): - The user-facing error omits sensitive bits and only includes high-level context, e.g.: { "message": "Prometheus API returned status 401", "details": {"datasourceUID": "ds-prom"} } - Internal, sensitive data are logged server-side at DEBUG level (not exposed to clients). Prerequisites/conditions: - A Grafana instance with the dashvalidator and Prometheus fetcher enabled. - A Prometheus datasource requiring authentication that returns error body content on failure. - Access to the Grafana API endpoint that triggers the dashvalidator check.

diff --git a/apps/dashvalidator/pkg/app/app.go b/apps/dashvalidator/pkg/app/app.go index 79f4e0ffdbb7..17e0efb1b795 100644 --- a/apps/dashvalidator/pkg/app/app.go +++ b/apps/dashvalidator/pkg/app/app.go @@ -136,6 +136,7 @@ func handleCheckRoute( defer cancel() logger := log.WithContext(ctx) + ctx = logging.Context(ctx, logger) logger.Info("Received compatibility check request") // Step 1: Parse request body @@ -215,6 +216,7 @@ func handleCheckRoute( ) } logger = logger.With("orgID", orgID, "namespace", namespace) + ctx = logging.Context(ctx, logger) // Extract the requester once for per-datasource scoped permission checks user, err := identity.GetRequester(ctx) diff --git a/apps/dashvalidator/pkg/validator/errors.go b/apps/dashvalidator/pkg/validator/errors.go index 77cbea8cecae..e4a84111198c 100644 --- a/apps/dashvalidator/pkg/validator/errors.go +++ b/apps/dashvalidator/pkg/validator/errors.go @@ -98,25 +98,25 @@ func NewDatasourceWrongTypeError(uid string, expectedType string, actualType str WithDetail("actualType", actualType) } -// NewDatasourceUnreachableError creates an error for unreachable datasource -func NewDatasourceUnreachableError(uid string, url string, cause error) *ValidationError { +// NewDatasourceUnreachableError creates an error for unreachable datasource. +// URL is intentionally omitted from the user-facing error; operators recover +// it from DEBUG logs at the caller site. +func NewDatasourceUnreachableError(uid string, cause error) *ValidationError { return NewValidationError( ErrCodeDatasourceUnreachable, - fmt.Sprintf("datasource %s at %s is unreachable", uid, url), + fmt.Sprintf("datasource %s is unreachable", uid), http.StatusServiceUnavailable, ).WithDetail("datasourceUID", uid). - WithDetail("url", url). WithCause(cause) } // NewAPIUnavailableError creates an error for unavailable API -func NewAPIUnavailableError(statusCode int, responseBody string, cause error) *ValidationError { +func NewAPIUnavailableError(statusCode int, cause error) *ValidationError { return NewValidationError( ErrCodeAPIUnavailable, fmt.Sprintf("Prometheus API returned status %d", statusCode), http.StatusBadGateway, ).WithDetail("upstreamStatus", statusCode). - WithDetail("responseBody", responseBody). WithCause(cause) } @@ -129,13 +129,15 @@ func NewAPIInvalidResponseError(message string, cause error) *ValidationError { ).WithCause(cause) } -// NewAPITimeoutError creates an error for API timeout -func NewAPITimeoutError(url string, cause error) *ValidationError { +// NewAPITimeoutError creates an error for API timeout. +// URL is intentionally omitted from the user-facing error; operators recover +// it from DEBUG logs at the caller site. +func NewAPITimeoutError(uid string, cause error) *ValidationError { return NewValidationError( ErrCodeAPITimeout, - fmt.Sprintf("request to %s timed out", url), + fmt.Sprintf("request to datasource %s timed out", uid), http.StatusGatewayTimeout, - ).WithDetail("url", url). + ).WithDetail("datasourceUID", uid). WithCause(cause) } diff --git a/apps/dashvalidator/pkg/validator/prometheus/fetcher.go b/apps/dashvalidator/pkg/validator/prometheus/fetcher.go index e72d664be092..bcfdc32ae79a 100644 --- a/apps/dashvalidator/pkg/validator/prometheus/fetcher.go +++ b/apps/dashvalidator/pkg/validator/prometheus/fetcher.go @@ -11,6 +11,8 @@ import ( "path" "strings" + "github.com/grafana/grafana-app-sdk/logging" + "github.com/grafana/grafana/apps/dashvalidator/pkg/validator" ) @@ -32,15 +34,16 @@ type prometheusResponse struct { // FetchMetrics queries Prometheus to get all available metric names // It uses the /api/v1/label/__name__/values endpoint // The provided HTTP client should have proper authentication configured -func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceURL string, client *http.Client) ([]string, error) { +func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceUID, datasourceURL string, client *http.Client) ([]string, error) { // Build the API URL baseURL, err := url.Parse(datasourceURL) if err != nil { + logging.FromContext(ctx).Debug("invalid datasource URL", "datasourceUID", datasourceUID, "url", datasourceURL, "error", err) return nil, validator.NewValidationError( validator.ErrCodeDatasourceConfig, "invalid datasource URL", http.StatusBadRequest, - ).WithCause(err).WithDetail("url", datasourceURL) + ).WithCause(err).WithDetail("datasourceUID", datasourceUID) } // Append Prometheus API endpoint to base URL path using path.Join @@ -61,12 +64,13 @@ func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceURL string, client // Execute the request using the provided authenticated client resp, err := client.Do(req) if err != nil { + logging.FromContext(ctx).Debug("upstream request failed", "datasourceUID", datasourceUID, "url", baseURL.String(), "error", err) // Check if it's a timeout error if errors.Is(err, context.DeadlineExceeded) || strings.Contains(err.Error(), "timeout") { - return nil, validator.NewAPITimeoutError(baseURL.String(), err) + return nil, validator.NewAPITimeoutError(datasourceUID, err) } // Network or connection error - datasource is unreachable - return nil, validator.NewDatasourceUnreachableError("", datasourceURL, err) + return nil, validator.NewDatasourceUnreachableError(datasourceUID, err) } defer func() { _ = resp.Body.Close() }() @@ -76,46 +80,49 @@ func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceURL string, client body = []byte("<unable to read response body>") } + // Log upstream body once at DEBUG for any non-success status + if resp.StatusCode != http.StatusOK { + logging.FromContext(ctx).Debug("upstream response body", "datasourceUID", datasourceUID, "url", baseURL.String(), "statusCode", resp.StatusCode, "body", string(body)) + } + // Check HTTP status code switch resp.StatusCode { case http.StatusOK: // Success - continue to parse response case http.StatusUnauthorized, http.StatusForbidden: // Authentication or authorization failure - return nil, validator.NewDatasourceAuthError("", resp.StatusCode). - WithDetail("url", baseURL.String()). - WithDetail("responseBody", string(body)) + return nil, validator.NewDatasourceAuthError(datasourceUID, resp.StatusCode) case http.StatusNotFound: // Endpoint not found - might not be a valid Prometheus instance return nil, validator.NewAPIUnavailableError( resp.StatusCode, - string(body), fmt.Errorf("endpoint not found - this may not be a valid Prometheus datasource"), - ).WithDetail("url", baseURL.String()) + ).WithDetail("datasourceUID", datasourceUID) case http.StatusTooManyRequests: // Rate limiting return nil, validator.NewValidationError( validator.ErrCodeAPIRateLimit, "Prometheus API rate limit exceeded", http.StatusTooManyRequests, - ).WithDetail("url", baseURL.String()).WithDetail("responseBody", string(body)) + ).WithDetail("datasourceUID", datasourceUID) case http.StatusServiceUnavailable, http.StatusBadGateway, http.StatusGatewayTimeout: // Upstream service is down or unavailable - return nil, validator.NewAPIUnavailableError(resp.StatusCode, string(body), nil). - WithDetail("url", baseURL.String()) + return nil, validator.NewAPIUnavailableError(resp.StatusCode, nil). + WithDetail("datasourceUID", datasourceUID) default: // Other error status codes - return nil, validator.NewAPIUnavailableError(resp.StatusCode, string(body), nil). - WithDetail("url", baseURL.String()) + return nil, validator.NewAPIUnavailableError(resp.StatusCode, nil). + WithDetail("datasourceUID", datasourceUID) } // Parse the response JSON var promResp prometheusResponse if err := json.Unmarshal(body, &promResp); err != nil { + logging.FromContext(ctx).Debug("upstream response body", "datasourceUID", datasourceUID, "url", baseURL.String(), "statusCode", resp.StatusCode, "body", string(body)) return nil, validator.NewAPIInvalidResponseError( "response is not valid JSON", err, - ).WithDetail("url", baseURL.String()).WithDetail("responseBody", string(body)) + ).WithDetail("datasourceUID", datasourceUID) } // Check Prometheus API status field @@ -124,18 +131,20 @@ func (f *Fetcher) FetchMetrics(ctx context.Context, datasourceURL string, client if errorMsg == "" { errorMsg = "unknown error" } + logging.FromContext(ctx).Debug("upstream returned error status", "datasourceUID", datasourceUID, "url", baseURL.String(), "prometheusError", errorMsg) return nil, validator.NewAPIInvalidResponseError( fmt.Sprintf("Prometheus API returned error status: %s", errorMsg), nil, - ).WithDetail("url", baseURL.String()).WithDetail("prometheusError", errorMsg) + ).WithDetail("datasourceUID", datasourceUID).WithDetail("prometheusError", errorMsg) } // Validate that we got data if promResp.Data == nil { + logging.FromContext(ctx).Debug("upstream response body", "datasourceUID", datasourceUID, "url", baseURL.String(), "statusCode", resp.StatusCode, "body", string(body)) return nil, validator.NewAPIInvalidResponseError( "response missing 'data' field", nil, - ).WithDetail("url", baseURL.String()).WithDetail("responseBody", string(body)) + ).WithDetail("datasourceUID", datasourceUID) } return promResp.Data, nil diff --git a/apps/dashvalidator/pkg/validator/prometheus/fetcher_test.go b/apps/dashvalidator/pkg/validator/prometheus/fetcher_test.go index f18847f423fd..0b5897d8a4bf 100644 --- a/apps/dashvalidator/pkg/validator/prometheus/fetcher_test.go +++ b/apps/dashvalidator/pkg/validator/prometheus/fetcher_test.go @@ -36,7 +36,7 @@ func TestFetchMetrics_Success_ReturnsMetrics(t *testing.T) { // Execute fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(context.Background(), server.URL, server.Client()) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", server.URL, server.Client()) // Verify require.NoError(t, err) @@ -62,7 +62,7 @@ func TestFetchMetrics_Success_URLWithPath(t *testing.T) { // Execute with path suffix fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(context.Background(), server.URL+"/api/prom", server.Client()) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", server.URL+"/api/prom", server.Client()) // Verify require.NoError(t, err) @@ -95,7 +95,7 @@ func TestFetchMetrics_InvalidURL_ReturnsConfigError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(context.Background(), tt.url, &http.Client{}) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", tt.url, &http.Client{}) // Verify error require.Error(t, err) @@ -114,7 +114,7 @@ func TestFetchMetrics_EmptyURL_ReturnsNetworkError(t *testing.T) { // Note: An empty URL is technically parseable by Go's url.Parse // but results in a network error when trying to make the request fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(context.Background(), "", &http.Client{}) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", "", &http.Client{}) // Verify error - empty URL fails at network level, not URL parsing require.Error(t, err) @@ -134,7 +134,7 @@ func TestFetchMetrics_ConnectionRefused_ReturnsUnreachableError(t *testing.T) { fetcher := NewFetcher() client := &http.Client{Timeout: 100 * time.Millisecond} - metrics, err := fetcher.FetchMetrics(context.Background(), "http://127.0.0.1:1", client) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", "http://127.0.0.1:1", client) // Verify error require.Error(t, err) @@ -160,7 +160,7 @@ func TestFetchMetrics_ContextCancelled_ReturnsError(t *testing.T) { cancel() fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(ctx, server.URL, server.Client()) + metrics, err := fetcher.FetchMetrics(ctx, "test-ds-uid", server.URL, server.Client()) // Verify error - context cancellation returns unreachable error require.Error(t, err) @@ -187,7 +187,7 @@ func TestFetchMetrics_HTTPClientTimeout_ReturnsTimeoutError(t *testing.T) { } fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(context.Background(), server.URL, client) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", server.URL, client) // Verify timeout error is returned require.Error(t, err) @@ -214,7 +214,7 @@ func TestFetchMetrics_DeadlineExceeded_ReturnsTimeoutError(t *testing.T) { defer cancel() fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(ctx, server.URL, server.Client()) + metrics, err := fetcher.FetchMetrics(ctx, "test-ds-uid", server.URL, server.Client()) // Verify error require.Error(t, err) @@ -306,7 +306,7 @@ func TestFetchMetrics_HTTPStatusCodes_ReturnsExpectedError(t *testing.T) { defer server.Close() fetcher := NewFetcher() - metrics, err := fetcher.FetchMetrics(context.Background(), server.URL, server.Client()) + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", server.URL, server.Client()) // Verify error require.Error(t, err) @@ -321,6 +321,66 @@ func TestFetchMetrics_HTTPStatusCodes_ReturnsExpectedError(t *testing.T) { } } +func TestFetchMetrics_AuthFailure_ErrorOmitsRawBody(t *testing.T) { + rawBody := "prom-diag-info" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(rawBody)) + })) + defer server.Close() + + fetcher := NewFetcher() + metrics, err := fetcher.FetchMetrics(context.Background(), "test-ds-uid", server.URL, server.Client()) + + require.Error(t, err) + require.Nil(t, metrics) + require.True(t, validator.IsValidationError(err)) + + validationErr := validator.GetValidationError(err) + require.NotContains(t, err.Error(), rawBody) + require.NotContains(t, validationErr.Message, rawBody) + _, hasBodyDetail := validationErr.Details["responseBody"] + require.False(t, hasBodyDetail, "unexpected detail key present") + _, hasURLDetail := validationErr.Details["url"] + require.False(t, hasURLDetail, "unexpected detail key present") + + serialized, jsonErr := json.Marshal(validationErr.Details) + require.NoError(t, jsonErr) + require.NotContains(t, string(serialized), rawBody) + require.NotContains(t, string(serialized), server.URL) + require.NotContains(t, validationErr.Message, server.URL) +} + +func TestFetchMetrics_ServerError_ErrorOmitsRawBody(t *testing.T) { + rawBody := "internal stack trace at /opt/prom-internal/handler.go:42" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(rawBody)) + })) + de ... [truncated]