Information Disclosure

Proof-of-concept (actionable steps): Assumptions: - Grafana 12.4.0 (pre-fix) is deployed with a root/general folder that contains dashboards. Some dashboards in root are private (not accessible to a given user), while others are public (accessible to the user). - Test user (let's call it user-A) has read access only to a subset of dashboards, not including the private root dashboards. - The Grafana API exposes a search endpoint (e.g., POST /api/search or similar) that accepts a query in JSON including a folder filter, and the response includes hits with folder identifiers (FolderUID) that may reflect root ("" or "general"). Goal: - Demonstrate that, prior to this fix, a user could discover the existence of root dashboards to which they have no access via search results (information disclosure). Steps (PoC): 1) Prepare two root dashboards: - root_public: in root, accessible to user-A - root_hidden: in root, NOT accessible to user-A 2) As user-A, perform a search that queries dashboards across root (the API supports root via the root sentinel or empty folder UID). Example request (curl): curl -sS -H "Authorization: Bearer <user-A_token>" \ -H "Content-Type: application/json" \ -X POST http://grafana.example/api/search \ -d '{"Query": "", "FolderUIDs": ["", "general"], "Type": "dashboards"}' 3) Inspect the response for any root dashboards: - If root_hidden is present in the hits, the response discloses the existence (and possibly metadata) of a dashboard the user should not know about. - The root dashboards may appear with FolderUID set to "" or "general" depending on backend normalization. 4) Validate fix behavior (after applying this commit): - Re-run the same request as user-A. - The response should exclude root dashboards that user-A does not have access to (the code now skips root folders when listing dashboards user has read access to). - Root dashboards should no longer be exposed solely due to root-folder handling; permissions instrumentation for root dashboards is applied so that only allowed dashboards surface in search results. Prerequisites for PoC: - Access to a Grafana 12.4.0 instance with a root/general folder and at least one dashboard in root that is not accessible to the test user. - A test user with limited permissions (only some dashboards visible). - The API endpoint used for search must be accessible and support filtering by folder/root sentinels as described in the patch (the exact payload shape may vary slightly by Grafana version). Expected outcome: - Before the fix: the test user could observe root dashboards in search results (information disclosure of root dashboards' existence). - After the fix: root dashboards are not exposed via search for users lacking access; only dashboards the user has permission to see are surfaced.

diff --git a/pkg/registry/apis/dashboard/search.go b/pkg/registry/apis/dashboard/search.go index 786076e3642cd..c3efe801d7cee 100644 --- a/pkg/registry/apis/dashboard/search.go +++ b/pkg/registry/apis/dashboard/search.go @@ -637,12 +637,11 @@ func convertHttpSearchRequestToResourceSearchRequest(queryParams url.Values, use // hijacks the "name" query param to only search for shared dashboard UIDs names = append(names, dashboardUIDs...) } else if folder != "" { - // root folder is empty in the search index; collapse the canonical - // "general" sentinel to "" before querying. - folder = foldermodel.ToLegacyFolderUID(folder) + // A root folder UID ("general" or the legacy "") is expanded to match + // both root sentinels by the search backend, so pass it through unchanged. searchRequest.Options.Fields = append(searchRequest.Options.Fields, &resourcepb.Requirement{ Key: "folder", - Operator: "=", + Operator: string(selection.Equals), Values: []string{folder}, }) } @@ -744,11 +743,13 @@ func (s *SearchHandler) getDashboardsUIDsSharedWithUser(ctx context.Context, use return sharedDashboards, fmt.Errorf("error retrieving folder information") } - // populate list of unique folder UIDs in the list of dashboards user has read permissions + // populate list of unique folder UIDs in the list of dashboards user has read permissions. + // Root-parented dashboards have no parent folder to check, and the apistore may report root + // as either the legacy "" or the canonical "general" sentinel, so skip both. allFolders := make([]string, 0) for _, dash := range dashboardResult.Results.Rows { folderUid := string(dash.Cells[folderUidIdx]) - if folderUid != "" && !slices.Contains(allFolders, folderUid) { + if !foldermodel.IsRootFolderUID(folderUid) && !slices.Contains(allFolders, folderUid) { allFolders = append(allFolders, folderUid) } } @@ -777,11 +778,12 @@ func (s *SearchHandler) getDashboardsUIDsSharedWithUser(ctx context.Context, use foldersWithAccess = append(foldersWithAccess, fold.Key.Name) } - // add to sharedDashboards dashboards user has access to, but does NOT have access to it's parent folder + // add to sharedDashboards dashboards user has access to, but does NOT have access to it's parent folder. + // Root-parented dashboards (reported as "" or "general") have no parent folder, so skip both sentinels. for _, dash := range dashboardResult.Results.Rows { dashboardUid := dash.Key.Name folderUid := string(dash.Cells[folderUidIdx]) - if folderUid != "" && !slices.Contains(foldersWithAccess, folderUid) { + if !foldermodel.IsRootFolderUID(folderUid) && !slices.Contains(foldersWithAccess, folderUid) { sharedDashboards = append(sharedDashboards, dashboardUid) } } diff --git a/pkg/registry/apis/dashboard/search_test.go b/pkg/registry/apis/dashboard/search_test.go index 4ff445f01e3b3..ced25bef7cc45 100644 --- a/pkg/registry/apis/dashboard/search_test.go +++ b/pkg/registry/apis/dashboard/search_test.go @@ -552,6 +552,90 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { require.NoError(t, err) assert.Equal(t, len(mockResponse3.Results.Rows), len(p.Hits)) }) + + t.Run("should treat the canonical 'general' folder as root", func(t *testing.T) { + // dashboardSearchRequest: one dashboard parented to root via the canonical + // "general" sentinel, and one parented to a private folder. + mockResponse1 := &resourcepb.ResourceSearchResponse{ + Results: &resourcepb.ResourceTable{ + Columns: []*resourcepb.ResourceTableColumnDefinition{{Name: "folder"}}, + Rows: []*resourcepb.ResourceTableRow{ + { + Key: &resourcepb.ResourceKey{Name: "dashboardinroot", Resource: "dashboard"}, + Cells: [][]byte{[]byte(folder.GeneralFolderUID)}, // root reported as "general" + }, + { + Key: &resourcepb.ResourceKey{Name: "dashboardinprivatefolder", Resource: "dashboard"}, + Cells: [][]byte{[]byte("privatefolder")}, + }, + }, + }, + } + + // folderSearchRequest: user has access to no parent folders. + mockResponse2 := &resourcepb.ResourceSearchResponse{ + Results: &resourcepb.ResourceTable{ + Columns: []*resourcepb.ResourceTableColumnDefinition{{Name: "folder"}}, + Rows: []*resourcepb.ResourceTableRow{}, + }, + } + + // final dashboardSearchRequest: only the dashboard whose parent folder the + // user cannot access is shared; the root dashboard is excluded. + mockResponse3 := &resourcepb.ResourceSearchResponse{ + Results: &resourcepb.ResourceTable{ + Columns: []*resourcepb.ResourceTableColumnDefinition{{Name: "folder"}}, + Rows: []*resourcepb.ResourceTableRow{ + { + Key: &resourcepb.ResourceKey{Name: "dashboardinprivatefolder", Resource: "dashboard"}, + Cells: [][]byte{[]byte("privatefolder")}, + }, + }, + }, + } + + mockClient := &MockClient{ + MockResponses: []*resourcepb.ResourceSearchResponse{mockResponse1, mockResponse2, mockResponse3}, + } + + features := featuremgmt.WithFeatures() + searchHandler := SearchHandler{ + log: log.New("test", "test"), + client: mockClient, + tracer: tracing.NewNoopTracerService(), + features: features, + } + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/search?folder=sharedwithme", nil) + req.Header.Add("content-type", "application/json") + allPermissions := make(map[int64]map[string][]string) + permissions := make(map[string][]string) + permissions[dashboards.ActionDashboardsRead] = []string{"dashboards:uid:dashboardinroot", "dashboards:uid:dashboardinprivatefolder"} + allPermissions[1] = permissions + req = req.WithContext(identity.WithRequester(req.Context(), &user.SignedInUser{Namespace: "test", OrgID: 1, Permissions: allPermissions})) + + searchHandler.DoSearch(rr, req) + + assert.Equal(t, 3, mockClient.CallCount) + // the root ("general") dashboard contributes no parent folder to look up + secondCall := mockClient.MockCalls[1] + assert.Equal(t, []string{"privatefolder"}, secondCall.Options.Fields[0].Values) + // and the root dashboard is not treated as shared + thirdCall := mockClient.MockCalls[2] + assert.Equal(t, []string{"dashboardinprivatefolder"}, thirdCall.Options.Fields[1].Values) + + resp := rr.Result() + defer func() { + if err := resp.Body.Close(); err != nil { + t.Fatal(err) + } + }() + + p := &v0alpha1.SearchResults{} + err := json.NewDecoder(resp.Body).Decode(p) + require.NoError(t, err) + assert.Equal(t, len(mockResponse3.Results.Rows), len(p.Hits)) + }) } func TestConvertHttpSearchRequestToResourceSearchRequest(t *testing.T) { @@ -852,12 +936,12 @@ func TestConvertHttpSearchRequestToResourceSearchRequest(t *testing.T) { Federated: []*resourcepb.ResourceKey{folderKey}, }, }, - "root folder should be converted to empty string": { + "root folder is passed through for the search backend to expand": { queryString: "folder=general", expected: &resourcepb.ResourceSearchRequest{ Options: &resourcepb.ListOptions{ Key: dashboardKey, - Fields: []*resourcepb.Requirement{{Key: "folder", Operator: "=", Values: []string{""}}}, + Fields: []*resourcepb.Requirement{{Key: "folder", Operator: "=", Values: []string{"general"}}}, }, Query: "", Limit: 50, diff --git a/pkg/registry/apis/folders/sub_children.go b/pkg/registry/apis/folders/sub_children.go index c82e26ce361c3..847a37b8609c1 100644 --- a/pkg/registry/apis/folders/sub_children.go +++ b/pkg/registry/apis/folders/sub_children.go @@ -8,6 +8,7 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apiserver/pkg/registry/rest" folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1" @@ -83,7 +84,7 @@ func (r *subChildrenREST) Connect(ctx context.Context, name string, _ runtime.Ob }, Fields: []*resourcepb.Requirement{{ Key: resource.SEARCH_FIELD_FOLDER, - Operator: "=", + Operator: string(selection.Equals), Values: []string{name}, }}, }, diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index d0859cdaa2330..13860fa555f05 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -523,7 +523,7 @@ func (dr *DashboardServiceImpl) GetDashboardsByLibraryPanelUID(ctx context.Conte for _, row := range results.Hits { dashes = append(dashes, &dashboards.DashboardRef{ UID: row.Name, - FolderUID: row.Folder, + FolderUID: folder.ToLegacyFolderUID(row.Folder), ID: row.Field.GetNestedInt64(resource.SEARCH_FIELD_LEGACY_ID), // nolint:staticcheck }) } @@ -1225,8 +1225,7 @@ func (dr *DashboardServiceImpl) SetDefaultPermissionsAfterCreate(ctx context.Con return err } permissions := []accesscontrol.SetResourcePermissionCommand{} - isNested := obj.GetFolder() != "" - if isNested { + if !folder.IsRootFolderUID(obj.GetFolder()) { // Don't set any permissions for nested dashboards return nil } @@ -1239,12 +1238,12 @@ func (dr *DashboardServiceImpl) SetDefaultPermissionsAfterCreate(ctx context.Con UserID: uid, Permission: dashboardaccess.PERMISSION_ADMIN.String(), }) } - if !isNested { - permissions = append(permissions, []accesscontrol.SetResourcePermissionCommand{ - {BuiltinRole: string(org.RoleEditor), Permission: dashboardaccess.PERMISSION_EDIT.String()}, - {BuiltinRole: string(org.RoleViewer), Permission: dashboardaccess.PERMISSION_VIEW.String()}, - }...) - } + // Root dashboards (we returned above for nested) get default editor/viewer + // roles in addition to any caller-specific permissions added above. + permissions = append(permissions, []accesscontrol.SetResourcePermissionCommand{ + {BuiltinRole: string(org.RoleEditor), Permission: dashboardaccess.PERMISSION_EDIT.String()}, + {BuiltinRole: string(org.RoleViewer), Permission: dashboardaccess.PERMISSION_VIEW.String()}, + }...) svc := dr.getPermissionsService(key.Resource == "folders") if _, err := svc.SetPermissions(ctx, ns.OrgID, obj.GetName(), permissions...); err != nil { @@ -1518,7 +1517,7 @@ func (dr *DashboardServiceImpl) FindDashboards(ctx context.Context, query *dashb Slug: slugify.Slugify(hit.Title), Description: hit.Description, IsFolder: false, - FolderUID: hit.Folder, + FolderUID: folder.ToLegacyFolderUID(hit.Folder), FolderTitle: folderTitle, FolderID: folderID, FolderSlug: slugify.Slugify(folderTitle), @@ -1948,15 +1947,9 @@ func (dr *DashboardServiceImpl) buildDashboardSearchRequest(query *dashboards.Fi } if len(query.FolderUIDs) > 0 { - // Grafana frontend issues a call to search for dashboards in "general" folder. General folder doesn't exists and - // should return all dashboards without a parent folder. - for i := range query.FolderUIDs { - if query.FolderUIDs[i] == folder.GeneralFolderUID { - query.FolderUIDs[i] = "" - break - } - } - + // A root folder UID ("general" or the legacy "") is expanded to match + // both root sentinels by the search backend, so pass the UIDs through + // unchanged here. req := []*resourcepb.Requirement{{ Key: resource.SEARCH_FIELD_FOLDER, Operator: string(selection.In), @@ -2176,7 +2169,7 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8s(ctx context.Context, UID: hit.Name, Slug: slugify.Slugify(hit.Title), Title: hit.Title, - FolderUID: hit.Folder, + FolderUID: folder.ToLegacyFolderUID(hit.Folder), } } @@ -2260,7 +2253,7 @@ func (dr *DashboardServiceImpl) unstructuredToLegacyDashboardWithUsers(item *uns ID: obj.GetDeprecatedInternalID(), // nolint:staticcheck UID: uid, Slug: slugify.Slugify(title), - FolderUID: obj.GetFolder(), + FolderUID: folder.ToLegacyFolderUID(obj.GetFolder()), Version: int(dashVersion), Data: simplejson.NewFromAny(spec), APIVersion: strings.TrimPrefix(item.GetAPIVersion(), dashboardv0.GROUP+"/"), diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index 4cfff18c53afe..a0f2c73d5b457 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -231,7 +231,7 @@ func (s *Service) SearchFolders(ctx context.Context, query folder.SearchFoldersQ URI: "db/" + slug, URL: dashboards.GetFolderURL(item.Name, slug), Type: model.DashHitFolder, - FolderUID: item.Folder, + FolderUID: folder.ToLegacyFolderUID(item.Folder), Description: item.Description, } } @@ -347,7 +347,8 @@ func (s *Service) getFolderByTitle(ctx context.Context, orgID int64, title strin } // If we're searching for top-level folders (parentUID == nil), and the first result is not in the root folder, remove it from the results. - for parentUID == nil && len(hits.Hits) > 0 && hits.Hits[0].Folder != "" { + // The apistore now writes "general" (canonical) for root parents while older entries still carry the legacy empty string; both denote the root. + for parentUID == nil && len(hits.Hits) > 0 && !folder.IsRootFolderUID(hits.Hits[0].Folder) { hits.Hits = hits.Hits[1:] } diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index 0b561b863b85a..3d37967bcf919 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -231,6 +231,8 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC q.Page = 1 } + // A root folder UID ("" or "general") is expanded to match both root + // sentinels by the search backend, so pass q.UID through unchanged. fields := []*resourcepb.Requirement{{ Key: resource.SEARCH_FIELD_FOLDER, Operator: string(selection.In), @@ -279,10 +281,12 @@ func (ss *FolderUnifiedStoreImpl) doSearchPage(ctx context.Context, orgID int64, hits := make([]*folder.FolderReference, 0, len(res.Hits)) for _, item := range res.Hits { hits = append(hits, &folder.FolderReference{ - ID: item.Field.GetNestedInt64(resource.SEARCH_FIELD_LEGACY_ID), - UID: item.Name, - Title: item.Title, - ParentUID: item.Folder, + ID: item.Field.GetNestedInt64(resource.SEARCH_FIELD_LEGACY_ID), + UID: item.Name, + Title: item.Title, + // Legacy responses convey root with an empty ParentUID; the apistore + // now stores it as "general", so convert back here. + ParentUID: folder.ToLegacyFolderUID(item.Folder), ManagedBy: item.ManagedBy.Kind, }) } diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index 77649baa5d05c..3e4804086b04d 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/stora ... [truncated]