Privilege escalation / Access control bypass

Proof of concept (before fix behavior): Prerequisites: - Grafana instance with a K6 folder whose UID is known (K6FolderUID) and another regular folder (e.g., ParentFolderUID). - Admin or appropriate move-permission token for API calls. - API base URL for the Grafana instance. 1) Attempt to move the K6 folder itself to a different parent (e.g., the root folder): curl -i -X POST \ -H "Authorization: Bearer <TOKEN>" \ -H "Content-Type: application/json" \ -d '{"parentUid": "<ROOT_FOLDER_UID>"}' \ <grafana-host>/api/folders/<K6FolderUID>/move Expected before fix: HTTP 2xx/3xx indicating move success (depending on backend). After the fix: HTTP 400 with message similar to "k6 project may not be moved". 2) Attempt to move another folder into the K6 folder (i.e., move ParentFolderUID under K6FolderUID): curl -i -X POST \ -H "Authorization: Bearer <TOKEN>" \ -H "Content-Type: application/json" \ -d '{"parentUid": "<K6FolderUID>"}' \ <grafana-host>/api/folders/<ParentFolderUID>/move Expected before fix: move succeeds (or is allowed). After the fix: HTTP 400 with message "k6 project may not be moved" (cannot move a folder into K6). Notes: - Replace <TOKEN>, <K6FolderUID>, <ParentFolderUID>, and <ROOT_FOLDER_UID> with real values from your Grafana instance. - If the instance uses a service account path, ensure to provide the appropriate token as described in your environment. - The exact root UID value depends on your Grafana deployment; some setups use a dedicated RootFolderUID constant. The key expectation is that moves involving the K6 folder are rejected by the API after the fix.

diff --git a/pkg/registry/apis/folders/validate.go b/pkg/registry/apis/folders/validate.go index eb2f490dd2a5..7da186892aad 100644 --- a/pkg/registry/apis/folders/validate.go +++ b/pkg/registry/apis/folders/validate.go @@ -164,6 +164,11 @@ func validateOnUpdate(ctx context.Context, return nil } + // the k6 folder itself may not be moved (matches legacy folder.Service.Move) + if obj.Name == accesscontrol.K6FolderUID { + return folder.ErrBadRequest.Errorf("k6 project may not be moved") + } + // Validate the move operation newParent := folderObj.GetFolder() @@ -175,7 +180,7 @@ func validateOnUpdate(ctx context.Context, return nil } - // folder cannot be moved to a k6 folder + // folder cannot be moved into the k6 folder if newParent == accesscontrol.K6FolderUID { return folder.ErrFolderCannotBeMovedToK6.Errorf("k6 project may not be moved") } diff --git a/pkg/registry/apis/folders/validate_test.go b/pkg/registry/apis/folders/validate_test.go index b218d37991d2..4a26dae7c2a8 100644 --- a/pkg/registry/apis/folders/validate_test.go +++ b/pkg/registry/apis/folders/validate_test.go @@ -435,6 +435,74 @@ func TestValidateUpdate(t *testing.T) { }, expectedErr: "k6 project may not be moved", }, + { + name: "error to move the k6 folder itself", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "k6-app", + Annotations: map[string]string{ + utils.AnnoKeyFolder: "somewhere", + }, + }, + Spec: folders.FolderSpec{ + Title: "k6", + }, + }, + old: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "k6-app", + }, + Spec: folders.FolderSpec{ + Title: "k6", + }, + }, + expectedErr: "k6 project may not be moved", + }, + { + name: "error to move the k6 folder to root", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "k6-app", + Annotations: map[string]string{ + utils.AnnoKeyFolder: folder.RootFolderUID, + }, + }, + Spec: folders.FolderSpec{ + Title: "k6", + }, + }, + old: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "k6-app", + Annotations: map[string]string{ + utils.AnnoKeyFolder: "somewhere", + }, + }, + Spec: folders.FolderSpec{ + Title: "k6", + }, + }, + expectedErr: "k6 project may not be moved", + }, + { + name: "no-op update on k6 folder is allowed (title change, parent unchanged)", + folder: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "k6-app", + }, + Spec: folders.FolderSpec{ + Title: "renamed", + }, + }, + old: &folders.Folder{ + ObjectMeta: metav1.ObjectMeta{ + Name: "k6-app", + }, + Spec: folders.FolderSpec{ + Title: "k6", + }, + }, + }, { name: "can move a folder to max depth", folder: &folders.Folder{ diff --git a/pkg/tests/apis/folder/parity_test.go b/pkg/tests/apis/folder/parity_test.go index e9d4bb644d16..7a092ab02b7e 100644 --- a/pkg/tests/apis/folder/parity_test.go +++ b/pkg/tests/apis/folder/parity_test.go @@ -82,9 +82,8 @@ func TestIntegrationFolderAPIParity(t *testing.T) { t.Skip("validateOnUpdate misses the escalation check; un-skip when fix lands") assertMoveParity(t, f, f.rbacEditorOnA, "parityA1", "parityB", http.StatusForbidden) }) - t.Run("k6 source folder is rejected (KNOWN GAP)", func(t *testing.T) { - t.Skip("only legacy blocks K6 source; un-skip when fix lands") - assertMoveParity(t, f, f.helper.Org1.Admin, accesscontrol.K6FolderUID, "parityA", http.StatusBadRequest) + t.Run("k6 source folder is rejected", func(t *testing.T) { + assertK6SourceMoveParity(t, f, "parityA", http.StatusBadRequest) }) }) @@ -158,6 +157,7 @@ func newParityFixture(t *testing.T) *parityFixture { for i := 1; i <= 5; i++ { create(fmt.Sprintf("parityB%d", i), "parityB") } + create(accesscontrol.K6FolderUID, "") rbacEditorOnA := helper.CreateUser( "parity-elevated-A", apis.Org1, @@ -329,13 +329,16 @@ func assertNumericIDLabelParity(t *testing.T, f *parityFixture, uid string) { ) } -// assertMoveParity verifies legacy and k8s return the same HTTP status when -// the same user attempts the same move. The fixture is restored after every -// successful attempt so both APIs see the same starting state. +// assertMoveParity verifies legacy and k8s return the same HTTP status for +// the same move. On success it restores the original parent so the next +// assertion starts from the same state. func assertMoveParity(t *testing.T, f *parityFixture, user apis.User, uid, newParent string, expectStatus int) { t.Helper() - original := lookupParent(t, f, user, uid) + var original string + if expectStatus == http.StatusOK { + original = lookupParent(t, f, user, uid) + } legacyStatus := legacyMove(t, f, user, uid, newParent) if legacyStatus != expectStatus { @@ -356,6 +359,46 @@ func assertMoveParity(t *testing.T, f *parityFixture, user apis.User, uid, newPa } } +// assertK6SourceMoveParity drives both APIs through the admin service-account +// token because k6-app is hidden from non-service-account identities. +func assertK6SourceMoveParity(t *testing.T, f *parityFixture, newParent string, expectStatus int) { + t.Helper() + + saToken := f.helper.Org1.AdminServiceAccountToken + require.NotEmpty(t, saToken) + + body, err := json.Marshal(map[string]string{"parentUid": newParent}) + require.NoError(t, err) + rsp := apis.DoRequest(f.helper, apis.RequestParams{ + Method: http.MethodPost, + Path: fmt.Sprintf("/api/folders/%s/move", accesscontrol.K6FolderUID), + Body: body, + Headers: map[string]string{"Authorization": "Bearer " + saToken}, + }, &json.RawMessage{}) + require.NotNil(t, rsp.Response) + require.Equal(t, expectStatus, rsp.Response.StatusCode, + "legacy move k6-app → %s: body=%s", newParent, string(rsp.Body)) + + client := f.helper.GetResourceClient(apis.ResourceClientArgs{ + ServiceAccountToken: saToken, + Namespace: f.namespace(), + GVR: gvr, + }) + got, err := client.Resource.Get(context.Background(), accesscontrol.K6FolderUID, metav1.GetOptions{}) + require.NoError(t, err) + + anns := got.GetAnnotations() + if anns == nil { + anns = map[string]string{} + } + anns[utils.AnnoKeyFolder] = newParent + got.SetAnnotations(anns) + + _, err = client.Resource.Update(context.Background(), got, metav1.UpdateOptions{}) + require.Error(t, err) + require.Equal(t, expectStatus, statusCodeFromK8sError(err)) +} + func assertChildrenParity(t *testing.T, f *parityFixture, user apis.User, parentUID string, want []string) { t.Helper() @@ -398,7 +441,9 @@ func k8sMove(t *testing.T, f *parityFixture, user apis.User, uid, newParent stri t.Helper() client := f.helper.GetResourceClient(apis.ResourceClientArgs{User: user, GVR: gvr}) got, err := client.Resource.Get(context.Background(), uid, metav1.GetOptions{}) - require.NoError(t, err) + if err != nil { + return statusCodeFromK8sError(err) + } anns := got.GetAnnotations() if anns == nil { diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index ec3c613e3899..7c94f8693edf 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -600,9 +600,12 @@ func DoRequest[T any](c *K8sTestHelper, params RequestParams, result *T) K8sResp // Get the URL addr := c.env.Server.HTTPServer.Listener.Addr() baseUrl := fmt.Sprintf("http://%s", addr) - login := params.User.Identity.GetLogin() - if login != "" && params.User.password != "" { - baseUrl = fmt.Sprintf("http://%s:%s@%s", login, params.User.password, addr) + // User may be zero when callers authenticate via params.Headers (bearer token). + if params.User.Identity != nil { + login := params.User.Identity.GetLogin() + if login != "" && params.User.password != "" { + baseUrl = fmt.Sprintf("http://%s:%s@%s", login, params.User.password, addr) + } } contentType := params.ContentType