Information disclosure via verbose error handling / reduced error detail exposure

PoC (exploit demonstration of the behavior change): - Context: Grafana instance backed by Kubernetes-style folder resources. An attacker could trigger a NotFound on a folder resource when referencing a non-existent folder UID from a dashboard operation, potentially causing the API to leak internal details through the error payload. - PoC approaches: 1) Go unit-test style demonstration (server-side): - Create a NotFound error for the folder resource and feed it into ToDashboardErrorResponse. - Expect a 400 Bad Request with the generic folder-not-found message. Example (pseudo-Go, uncompiled sketch): package apierrors_test import ( "context" "net/http" k8sErrors "k8s.io/apimachinery/pkg/api/errors" folderv1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1" ) func TestFolderNotFoundMappedTo400(t *testing.T) { err := k8sErrors.NewNotFound(folderv1.FolderResourceInfo.GroupResource(), "unknown") resp := ToDashboardErrorResponse(context.Background(), pluginStoreWithoutPlugin, err) if resp.GetStatusCode() != http.StatusBadRequest { t.Fatalf("expected 400, got %d", resp.GetStatusCode()) } } 2) Client-side curl-based trigger (runtime behavior): - POST a dashboard payload that references a non-existent folder UID, which would cause the underlying folder-not-found condition. curl -X POST -H "Content-Type: application/json" \ -d '{"dashboard": {"title": "X","uid":"x","folderUid":"unknown-folder"}}' \ http://grafana.example/api/dashboards/db Expected after the fix: HTTP 400 with a body indicating folder not found (not exposing internal Kubernetes error details). Notes: - The exact request payload shape depends on Grafana’s API usage (k8s-style vs non-k8s payloads). The essential exploit path is triggering a not-found on the folder resource and observing that the response is sanitized to a 400 with a generic message, rather than leaking details from the Kubernetes error. - If testing locally, ensure the Grafana instance is configured to route folder operations via the Kubernetes-backed folder resource so that a NotFound on the folder resource is generated by the API layer during dashboard processing.

diff --git a/pkg/api/apierrors/dashboard.go b/pkg/api/apierrors/dashboard.go index 3f7c33e96cad5..ba6bbb9139413 100644 --- a/pkg/api/apierrors/dashboard.go +++ b/pkg/api/apierrors/dashboard.go @@ -6,6 +6,9 @@ import ( "fmt" "net/http" + apierrors "k8s.io/apimachinery/pkg/api/errors" + + folderv1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/dashboards" @@ -13,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/util" - apierrors "k8s.io/apimachinery/pkg/api/errors" ) // ToDashboardErrorResponse returns a different response status according to the dashboard error type @@ -53,8 +55,16 @@ func ToDashboardErrorResponse(ctx context.Context, pluginStore pluginstore.Store } // --- Kubernetes status errors --- - var statusErr *apierrors.StatusError - if errors.As(err, &statusErr) { + if statusErr, ok := errors.AsType[*apierrors.StatusError](err); ok { + // The k8s dashboard apiserver returns NotFound on the folders resource when the + // referenced folder UID does not exist. Map that back to the legacy + // /api/dashboards/db contract (400 + "folder not found") so existing clients keep + // working after the direct-to-client routing change. + if apierrors.IsNotFound(err) { + if d := statusErr.ErrStatus.Details; d != nil && d.Group == folderv1.APIGroup && d.Kind == folderv1.RESOURCE { + return response.Error(http.StatusBadRequest, dashboards.ErrFolderNotFound.Error(), nil) + } + } return response.Error(int(statusErr.ErrStatus.Code), statusErr.ErrStatus.Message, err) } diff --git a/pkg/api/apierrors/dashboard_test.go b/pkg/api/apierrors/dashboard_test.go index 1e1512e07d39c..b968f26235faf 100644 --- a/pkg/api/apierrors/dashboard_test.go +++ b/pkg/api/apierrors/dashboard_test.go @@ -10,12 +10,15 @@ import ( "github.com/stretchr/testify/require" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + folderv1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/util" ) @@ -53,6 +56,62 @@ func TestToDashboardErrorResponse(t *testing.T) { input: dashboardaccess.DashboardErr{Reason: "Bad Request", StatusCode: http.StatusBadRequest}, want: response.Error(http.StatusBadRequest, "Bad Request", nil), }, + // Per-error pins for the typed dashboard errors that the legacy + // /api/dashboards/db endpoint used to map explicitly. They all flow + // through the generic DashboardErr branch today, but the explicit cases + // catch regressions if an error's StatusCode/Status fields change. + { + name: "ErrDashboardWithSameUIDExists maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardWithSameUIDExists, + want: response.Error(http.StatusBadRequest, dashboards.ErrDashboardWithSameUIDExists.Error(), nil), + }, + { + name: "ErrDashboardFolderCannotHaveParent maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardFolderCannotHaveParent, + want: response.Error(http.StatusBadRequest, dashboards.ErrDashboardFolderCannotHaveParent.Error(), nil), + }, + { + name: "ErrDashboardTypeMismatch maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardTypeMismatch, + want: response.Error(http.StatusBadRequest, dashboards.ErrDashboardTypeMismatch.Error(), nil), + }, + { + name: "ErrDashboardInvalidUid maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardInvalidUid, + want: response.Error(http.StatusBadRequest, dashboards.ErrDashboardInvalidUid.Error(), nil), + }, + { + name: "ErrDashboardUidTooLong maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardUidTooLong, + want: response.Error(http.StatusBadRequest, dashboards.ErrDashboardUidTooLong.Error(), nil), + }, + { + name: "ErrDashboardCannotSaveProvisionedDashboard maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardCannotSaveProvisionedDashboard, + want: response.Error(http.StatusBadRequest, dashboards.ErrDashboardCannotSaveProvisionedDashboard.Error(), nil), + }, + { + // ErrDashboardTitleEmpty carries Status="empty-name", so the response + // must be a JSON body, not a bare error string. + name: "ErrDashboardTitleEmpty maps to 400 with status body", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardTitleEmpty, + want: response.JSON(http.StatusBadRequest, util.DynMap{"status": "empty-name", "message": dashboards.ErrDashboardTitleEmpty.Error()}), + }, + { + // folder.ErrNameExists has its own explicit branch in + // ToDashboardErrorResponse alongside dashboards.ErrFolderNotFound. + name: "folder.ErrNameExists maps to 400", + pluginStore: pluginStoreWithoutPlugin, + input: folder.ErrNameExists.Errorf("%s", "A folder with that name already exists"), + want: response.Error(http.StatusBadRequest, folder.ErrNameExists.Errorf("%s", "A folder with that name already exists").Error(), nil), + }, // --- 403 Forbidden --- { name: "dashboard error with a forbidden status", @@ -60,6 +119,12 @@ func TestToDashboardErrorResponse(t *testing.T) { input: &k8sErrors.StatusError{ErrStatus: metav1.Status{Code: http.StatusForbidden, Message: "access denied"}}, want: response.Error(http.StatusForbidden, "access denied", &k8sErrors.StatusError{ErrStatus: metav1.Status{Code: http.StatusForbidden, Message: "access denied"}}), }, + { + name: "ErrDashboardUpdateAccessDenied maps to 403", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardUpdateAccessDenied, + want: response.Error(http.StatusForbidden, dashboards.ErrDashboardUpdateAccessDenied.Error(), dashboards.ErrDashboardUpdateAccessDenied), + }, // --- 404 Not Found --- { name: "folder not found error", @@ -67,6 +132,28 @@ func TestToDashboardErrorResponse(t *testing.T) { input: dashboards.ErrFolderNotFound, want: response.Error(http.StatusBadRequest, dashboards.ErrFolderNotFound.Error(), nil), }, + { + // ErrDashboardNotFound carries Status="not-found", so the response + // is a JSON body produced by DashboardErr.Body(). + name: "ErrDashboardNotFound maps to 404 with status body", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardNotFound, + want: response.JSON(http.StatusNotFound, util.DynMap{"status": "not-found", "message": dashboards.ErrDashboardNotFound.Error()}), + }, + { + // k8s dashboard apiserver surfaces a NotFound on the folders resource when the + // referenced folder UID does not exist; legacy /api/dashboards/db must stay 400. + name: "k8s folder not found is mapped to legacy 400", + pluginStore: pluginStoreWithoutPlugin, + input: k8sErrors.NewNotFound(folderv1.FolderResourceInfo.GroupResource(), "unknown"), + want: response.Error(http.StatusBadRequest, dashboards.ErrFolderNotFound.Error(), nil), + }, + { + name: "k8s not found for non-folder resource passes through", + pluginStore: pluginStoreWithoutPlugin, + input: k8sErrors.NewNotFound(schema.GroupResource{Group: "dashboard.grafana.app", Resource: "dashboards"}, "abc"), + want: response.Error(http.StatusNotFound, `dashboards.dashboard.grafana.app "abc" not found`, k8sErrors.NewNotFound(schema.GroupResource{Group: "dashboard.grafana.app", Resource: "dashboards"}, "abc")), + }, { name: "dashboard error with a non-bad-request status", pluginStore: pluginStoreWithoutPlugin, @@ -86,6 +173,14 @@ func TestToDashboardErrorResponse(t *testing.T) { input: dashboards.UpdatePluginDashboardError{PluginId: "unknown-plugin"}, want: response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": "The dashboard belongs to plugin unknown-plugin."}), }, + { + // ErrDashboardVersionMismatch carries Status="version-mismatch", so + // the response is the JSON body produced by DashboardErr.Body(). + name: "ErrDashboardVersionMismatch maps to 412 with status body", + pluginStore: pluginStoreWithoutPlugin, + input: dashboards.ErrDashboardVersionMismatch, + want: response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "version-mismatch", "message": dashboards.ErrDashboardVersionMismatch.Error()}), + }, // --- 413 Payload Too Large --- { name: "request entity too large error", diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 99a3cd561e646..d3276819b2f0c 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -18,6 +18,7 @@ import ( "k8s.io/client-go/dynamic" claims "github.com/grafana/authlib/types" + dashboardsV0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" dashboardsV1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1" "github.com/grafana/grafana/pkg/api/apierrors" "github.com/grafana/grafana/pkg/api/dtos" @@ -397,9 +398,13 @@ func (hs *HTTPServer) postDashboard(c *contextmodel.ReqContext, cmd dashboards.S " OR it should include a object wrapper with an explicit 'apiVersion' and move the body into a 'spec' element", nil) } + obj := &unstructured.Unstructured{Object: map[string]interface{}{ + "spec": spec, + }} + // Items with metadata, spec, etc if dashboards.LooksLikeK8sResource(spec) { - obj := &unstructured.Unstructured{Object: spec} + obj.Object = spec apiVersion := obj.GetAPIVersion() switch { case strings.HasPrefix(apiVersion, dashboardsV1.GROUP): @@ -415,83 +420,19 @@ func (hs *HTTPServer) postDashboard(c *contextmodel.ReqContext, cmd dashboards.S obj.SetName(uid) // overwrite the incoming name -- this might happen from TF providers } hs.log.Warn("DEPRECATION WARNING: Accepting k8s style dashboard in legacy /api/dashboards/db. Please use the /apis/dashboard.grafana.app/ API to manage this resource", "dashboard", obj.GetName()) - return hs.saveDashboardViaK8s(c, cmd, obj) case apiVersion == "": return response.Error(http.StatusBadRequest, "Dashboard appears to be a k8s style resource, but is missing an explicit apiVersion.", nil) + default: + return response.Error(http.StatusBadRequest, "The dashboard payload references a non dashboard apiVersion. This should be sent to the requested api directly", nil) } - return response.Error(http.StatusBadRequest, "The dashboard payload references a non dashboard apiVersion. This should be sent to the requested api directly", nil) - } - - _, found := spec["title"] - if !found { - return response.Error(http.StatusBadRequest, "Dashboard is missing required title property", nil) - } - - ctx = c.Req.Context() - - var userID int64 - if id, err := identity.UserIdentifier(c.GetID()); err == nil { - userID = id - } - - cmd.OrgID = c.GetOrgID() - cmd.UserID = userID - - dash := cmd.GetDashboardModel() - newDashboard := dash.ID == 0 - if newDashboard { - limitReached, err := hs.QuotaService.QuotaReached(c, dashboards.QuotaTargetSrv) - if err != nil { - return response.Error(http.StatusInternalServerError, "Failed to get quota", err) - } - if limitReached { - return response.Error(http.StatusForbidden, "Quota reached", nil) - } - } - - var provisioningData *dashboards.DashboardProvisioning - if dash.ID != 0 { - data, err := hs.dashboardProvisioningService.GetProvisionedDashboardDataByDashboardID(c.Req.Context(), dash.ID) - if err != nil { - return response.Error(http.StatusInternalServerError, "Error while checking if dashboard is provisioned using ID", err) - } - provisioningData = data - } else if dash.UID != "" { - data, err := hs.dashboardProvisioningService.GetProvisionedDashboardDataByDashboardUID(c.Req.Context(), dash.OrgID, dash.UID) - if err != nil && !errors.Is(err, dashboards.ErrProvisionedDashboardNotFound) && !errors.Is(err, dashboards.ErrDashboardNotFound) { - return response.Error(http.StatusInternalServerError, "Error while checking if dashboard is provisioned", err) - } - provisioningData = data - } - - allowUiUpdate := true - if provisioningData != nil { - allowUiUpdate = hs.ProvisioningService.GetAllowUIUpdatesFromConfig(provisioningData.Name) - } - - dashItem := &dashboards.SaveDashboardDTO{ - Dashboard: dash, - Message: cmd.Message, - OrgID: c.GetOrgID(), - User: c.SignedInUser, - Overwrite: cmd.Overwrite, - } - - dashboard, saveErr := hs.DashboardService.SaveDashboard(ctx, dashItem, allowUiUpdate) - if saveErr != nil { - return apierrors.ToDashboardErrorResponse(ctx, hs.pluginStore, saveErr) + } else { + // Default legacy POSTs to v0alpha1: matches the prior DashboardService.SaveDashboard + // behavior. v0 lets the dashboard apiserver mutate hook strip uid/version/id without + // running the v1 schema migrations, so legacy callers' panel content is preserved. + obj.SetAPIVersion(dashboardsV0.APIVERSION) } - - return response.JSON(http.StatusOK, util.DynMap{ - "status": "success", - "slug": dashboard.Slug, - "version": dashboard.Version, - "id": dashboard.ID, - "uid": dashboard.UID, - "url": dashboard.GetURL(), - "folderUid": dashboard.FolderUID, - }) + return hs.saveDashboardViaK8s(c, cmd, obj) } func (hs *HTTPServer) saveDashboardViaK8s(c *contextmodel.ReqContext, cmd dashboards.SaveDashboardCommand, obj *unstructured.Unstructured) response.Response { @@ -513,121 +454,174 @@ func (hs *HTTPServer) saveDashboardViaK8s(c *contextmodel.ReqContext, cmd dashbo } client := tmp.Resource(gv.WithResource(dashboardsV1.DASHBOARD_RESOURCE)).Namespace(namespace) - obj.SetKind("Dashboard") // Writing to the dashboard API + // /api/dashboards/db lets clients place identity (uid, id) and version + // inside the spec; lift them onto k8s metadata, then strip the originals + // so the apistore mutate hooks see a clean spec. + specUID, _, _ := unstructured.NestedString(obj.Object, "spec", "uid") + internalID, err := nestedInternalID(obj.Object) + if err != nil { + ret ... [truncated]