Information disclosure / Access-control hardening

PoC (proof of concept): Prerequisites: - A Grafana instance with a K6 technical folder present in the search index (folder UID: k6-app). - A user account that is not a service account and has permission to perform unified searches. - Access token for this non-service-account user. Steps to reproduce (pre-fix behavior): 1) Authenticate as a regular user and obtain an API token. 2) Run a search that would include the K6 folder (e.g., query=*): curl -s -H "Authorization: Bearer <USER_TOKEN>" 'https://grafana.example.com/api/search?query=*' 3) Inspect the JSON response and look for an entry that corresponds to the K6 technical folder (e.g., a row with something like title/name mapping to k6-app or a resource with UID k6-app). 4) Observe that the K6 folder (internal RBAC artifact) is exposed in the search results, constituting an information-disclosure vulnerability. Steps to verify the fix (post-fix behavior): 1) Authenticate as the same non-service-account user and run the same search again. 2) The response should no longer include the K6 folder entry in the results. Notes: - The code path shows the filter is applied only for non-service accounts, using NotIn on the search field with the K6FolderUID (k6-app). This PoC demonstrates the presence (pre-fix) and absence (post-fix) of that entry in the search results.

diff --git a/pkg/registry/apis/dashboard/search.go b/pkg/registry/apis/dashboard/search.go index 4e3bad201547a..965f1146c3a43 100644 --- a/pkg/registry/apis/dashboard/search.go +++ b/pkg/registry/apis/dashboard/search.go @@ -12,9 +12,11 @@ import ( "strconv" "strings" + claims "github.com/grafana/authlib/types" "go.opentelemetry.io/otel/trace" apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/spec3" "k8s.io/kube-openapi/pkg/validation/spec" @@ -23,6 +25,7 @@ import ( folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/builder" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" @@ -580,6 +583,16 @@ func convertHttpSearchRequestToResourceSearchRequest(queryParams url.Values, use }) } + // K6 creates a technical folder for some rbac handling that should not be visible to normal user. + // The legacy search backend ignores NotIn on name, but we should be mostly in mode 4+ now. + if !user.IsIdentityType(claims.TypeServiceAccount) { + searchRequest.Options.Fields = append(searchRequest.Options.Fields, &resourcepb.Requirement{ + Key: resource.SEARCH_FIELD_NAME, + Operator: string(selection.NotIn), + Values: []string{accesscontrol.K6FolderUID}, + }) + } + if searchRequest.Query == "*" { searchRequest.Query = "" // will match everything } else if searchRequest.Query != "" { diff --git a/pkg/registry/apis/dashboard/search_test.go b/pkg/registry/apis/dashboard/search_test.go index 9a1c153fab7ce..7f38ff3cc7d89 100644 --- a/pkg/registry/apis/dashboard/search_test.go +++ b/pkg/registry/apis/dashboard/search_test.go @@ -10,11 +10,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "k8s.io/apimachinery/pkg/selection" "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -104,6 +106,43 @@ func TestSearchHandler(t *testing.T) { assert.Equal(t, mockResults[2].Value, p.Hits[0].Title) assert.Equal(t, mockResults[1].Value, p.Hits[3].Title) }) + + t.Run("filters k6 technical folder from results for non-service accounts", func(t *testing.T) { + mockResponse := &resourcepb.ResourceSearchResponse{ + Results: &resourcepb.ResourceTable{ + Columns: []*resourcepb.ResourceTableColumnDefinition{ + {Name: resource.SEARCH_FIELD_TITLE}, + }, + Rows: []*resourcepb.ResourceTableRow{}, + }, + } + + mockClient := &MockClient{ + MockResponses: []*resourcepb.ResourceSearchResponse{mockResponse}, + } + + searchHandler := SearchHandler{ + log: log.New("test", "test"), + client: mockClient, + tracer: tracing.NewNoopTracerService(), + features: featuremgmt.WithFeatures(), + } + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/search", nil) + req = req.WithContext(identity.WithRequester(req.Context(), &user.SignedInUser{Namespace: "test"})) + + searchHandler.DoSearch(rr, req) + resp := rr.Result() + defer func() { + if err := resp.Body.Close(); err != nil { + t.Fatal(err) + } + }() + + assert.Equal(t, string(selection.NotIn), mockClient.MockCalls[0].Options.Fields[0].Operator) + assert.Equal(t, []string{"k6-app"}, mockClient.MockCalls[0].Options.Fields[0].Values) + }) } func TestSearchHandlerSharedDashboards(t *testing.T) { @@ -359,7 +398,7 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { // first call gets all dashboards user has permission for firstCall := mockClient.MockCalls[0] - assert.Equal(t, firstCall.Options.Fields[0].Values, []string{"dashboardinroot", "dashboardinprivatefolder", "dashboardinpublicfolder", "sharedfolder"}) + assert.Equal(t, []string{"dashboardinroot", "dashboardinprivatefolder", "dashboardinpublicfolder", "sharedfolder"}, firstCall.Options.Fields[0].Values) // verify federated field is set to include folders assert.NotNil(t, firstCall.Federated) assert.Equal(t, 1, len(firstCall.Federated)) @@ -367,11 +406,11 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { assert.Equal(t, "folders", firstCall.Federated[0].Resource) // second call gets folders associated with the previous dashboards secondCall := mockClient.MockCalls[1] - assert.Equal(t, secondCall.Options.Fields[0].Values, []string{"privatefolder", "publicfolder"}) + assert.Equal(t, []string{"privatefolder", "publicfolder"}, secondCall.Options.Fields[0].Values) // lastly, search ONLY for dashboards and folders user has permission to read that are within folders the user does NOT have // permission to read thirdCall := mockClient.MockCalls[2] - assert.Equal(t, thirdCall.Options.Fields[0].Values, []string{"dashboardinprivatefolder", "sharedfolder"}) + assert.Equal(t, []string{"dashboardinprivatefolder", "sharedfolder"}, thirdCall.Options.Fields[1].Values) resp := rr.Result() defer func() { @@ -499,7 +538,7 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { thirdCall := mockClient.MockCalls[2] assert.Equal(t, int64(dashboardaccess.PERMISSION_EDIT), thirdCall.Permission) - assert.Equal(t, []string{"dashboardinprivatefolder"}, thirdCall.Options.Fields[0].Values) + assert.Equal(t, []string{"dashboardinprivatefolder"}, thirdCall.Options.Fields[1].Values) resp := rr.Result() defer func() { @@ -517,8 +556,9 @@ func TestSearchHandlerSharedDashboards(t *testing.T) { func TestConvertHttpSearchRequestToResourceSearchRequest(t *testing.T) { testUser := &user.SignedInUser{ - Namespace: "test-namespace", - OrgID: 1, + Namespace: "test-namespace", + OrgID: 1, + IsServiceAccount: true, } dashboardKey := &resourcepb.ResourceKey{ @@ -969,6 +1009,42 @@ func TestConvertHttpSearchRequestToResourceSearchRequest(t *testing.T) { assert.Equal(t, tt.expected, result) }) } + + t.Run("adds k6 exclusion for non-service accounts", func(t *testing.T) { + queryParams, err := url.ParseQuery("") + require.NoError(t, err) + + result, err := convertHttpSearchRequestToResourceSearchRequest(queryParams, &user.SignedInUser{ + Namespace: "test-namespace", + OrgID: 1, + }, func(requestedPermission dashboardaccess.PermissionType) ([]string, error) { + return nil, nil + }) + + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, result.Options.Fields, 1) + assert.Equal(t, resource.SEARCH_FIELD_NAME, result.Options.Fields[0].Key) + assert.Equal(t, string(selection.NotIn), result.Options.Fields[0].Operator) + assert.Equal(t, []string{accesscontrol.K6FolderUID}, result.Options.Fields[0].Values) + }) + + t.Run("keeps k6 folder visible for service accounts", func(t *testing.T) { + queryParams, err := url.ParseQuery("") + require.NoError(t, err) + + result, err := convertHttpSearchRequestToResourceSearchRequest(queryParams, &user.SignedInUser{ + Namespace: "test-namespace", + OrgID: 1, + IsServiceAccount: true, + }, func(requestedPermission dashboardaccess.PermissionType) ([]string, error) { + return nil, nil + }) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Empty(t, result.Options.Fields) + }) } // MockClient implements the ResourceIndexClient interface for testing diff --git a/pkg/tests/apis/dashboard/testdata/searchV0/t01-query-single-word.json b/pkg/tests/apis/dashboard/testdata/searchV0/t01-query-single-word.json index ebab0953d2937..7b9243e3ec55b 100644 --- a/pkg/tests/apis/dashboard/testdata/searchV0/t01-query-single-word.json +++ b/pkg/tests/apis/dashboard/testdata/searchV0/t01-query-single-word.json @@ -10,7 +10,7 @@ "panel-tests", "graph-ng" ], - "score": 0.122 + "score": 0.15 }, { "resource": "dashboards", @@ -21,8 +21,8 @@ "panel-tests", "graph-ng" ], - "score": 0.116 + "score": 0.144 } ], - "maxScore": 0.122 + "maxScore": 0.15 } \ No newline at end of file diff --git a/pkg/tests/apis/dashboard/testdata/searchV0/t02-query-multiple-words.json b/pkg/tests/apis/dashboard/testdata/searchV0/t02-query-multiple-words.json index f446ad000cb6d..969b2f5c74ea6 100644 --- a/pkg/tests/apis/dashboard/testdata/searchV0/t02-query-multiple-words.json +++ b/pkg/tests/apis/dashboard/testdata/searchV0/t02-query-multiple-words.json @@ -10,8 +10,8 @@ "panel-tests", "graph-ng" ], - "score": 0.184 + "score": 0.211 } ], - "maxScore": 0.184 + "maxScore": 0.211 } \ No newline at end of file diff --git a/pkg/tests/apis/dashboard/testdata/searchV0/t03-with-text-panel.json b/pkg/tests/apis/dashboard/testdata/searchV0/t03-with-text-panel.json index a758e4dfc06bd..441cc63e8160b 100644 --- a/pkg/tests/apis/dashboard/testdata/searchV0/t03-with-text-panel.json +++ b/pkg/tests/apis/dashboard/testdata/searchV0/t03-with-text-panel.json @@ -14,5 +14,5 @@ } } ], - "maxScore": 1.125 + "maxScore": 1.417 } \ No newline at end of file diff --git a/pkg/tests/apis/dashboard/testdata/searchV0/t04-title-ngram-prefix.json b/pkg/tests/apis/dashboard/testdata/searchV0/t04-title-ngram-prefix.json index d073d6dc4b9e4..f7aa9f4418a66 100644 --- a/pkg/tests/apis/dashboard/testdata/searchV0/t04-title-ngram-prefix.json +++ b/pkg/tests/apis/dashboard/testdata/searchV0/t04-title-ngram-prefix.json @@ -10,8 +10,8 @@ "panel-tests", "graph-ng" ], - "score": 0.225 + "score": 0.253 } ], - "maxScore": 0.225 + "maxScore": 0.253 } \ No newline at end of file diff --git a/pkg/tests/apis/dashboard/testdata/searchV0/t05-title-ngram-middle-word.json b/pkg/tests/apis/dashboard/testdata/searchV0/t05-title-ngram-middle-word.json index d073d6dc4b9e4..f7aa9f4418a66 100644 --- a/pkg/tests/apis/dashboard/testdata/searchV0/t05-title-ngram-middle-word.json +++ b/pkg/tests/apis/dashboard/testdata/searchV0/t05-title-ngram-middle-word.json @@ -10,8 +10,8 @@ "panel-tests", "graph-ng" ], - "score": 0.225 + "score": 0.253 } ], - "maxScore": 0.225 + "maxScore": 0.253 } \ No newline at end of file diff --git a/pkg/tests/apis/dashboard/testdata/searchV0/t06-panel-title-orange.json b/pkg/tests/apis/dashboard/testdata/searchV0/t06-panel-title-orange.json index acb9f65ee3da3..b834af9ab9a1b 100644 --- a/pkg/tests/apis/dashboard/testdata/searchV0/t06-panel-title-orange.json +++ b/pkg/tests/apis/dashboard/testdata/searchV0/t06-panel-title-orange.json @@ -10,7 +10,7 @@ "panel-tests", "graph-ng" ], - "score": 0.102, + "score": 0.128, "explain": { "children": [ { @@ -21,68 +21,110 @@ "children": [ { "children": [ - { - "message": "boost", - "value": 5 - }, - { - "message": "idf(docFreq=1, maxDocs=17)", - "value": 2.485 - }, - { - "message": "queryNorm", - "value": 0.026 - } - ], - "message": "queryWeight(fields.panel_title:orange^5.000000), product of:", - "value": 0.321 - }, - { - "children": [ - { - "message": "tf(termFreq(fields.panel_title:orange)=3", - "value": 1.732 - }, { "children": [ { - "message": "fieldNorm(field=fields.panel_title), b=0.750000, fieldLength=38.000002, avgFieldLength=17.000000)", - "value": 1.926 + "message": "boost", + "value": 5 + }, + { + "message": "idf(docFreq=1, maxDocs=17)", + "value": 2.485 + }, + { + "message": "queryNorm", + "value": 0.026 } ], - "message": "saturation(term:), k1=1.200000/(tf=1.732051 + k1*fieldNorm=1.926471))", - "value": 0.297 + "message": "queryWeight(fields.panel_title:orange^5.000000), product of:", + "value": 0.32 }, { - "message": "idf(docFreq=1, maxDocs=17)", - "value": 2.485 + "children": [ + { + "message": "tf(termFreq(fields.panel_title:orange)=3", + "value": 1.732 + }, + { + "children": [ + { + "message": "fieldNorm(field=fields.panel_title), b=0.750000, fieldLength=38.000002, avgFieldLength=17.000000)", + "value": 1.926 + } + ], + "message": "saturation(term:), k1=1.200000/(tf=1.732051 + k1*fieldNorm=1.926471))", + "value": 0.297 + }, + { + "message": "idf(docFreq=1, maxDocs=17)", + "value": 2.485 + } + ], + "message": "fieldWeight(fields.panel_title:orange in \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0001), as per bm25 model, product of:", + "value": 1.277 } ... [truncated]