Information Disclosure

PoC (Go-like pseudo-code demonstrating prior vulnerability exposure in trash listing): // Prerequisites: Grafana storage backend (unified-storage) in a vulnerable version (<=12.4.0). // 1) Create a normal resource and delete it to place it in trash. // 2) Create a resource marked as provisioned (annotation AnnoKeyManagerKind = "repo"). // 3) Delete the provisioned resource to also place it in trash. // 4) List trash and observe whether the provisioned (non-deleted) resource appears in results. package main import ( "context" "fmt" // Assume access to Grafana's storage backend test helpers and proto types ) func main() { ctx := context.Background() backend := setupTestStorageBackend(/* t or config */) // Step 1: add a normal resource and delete it (should be in trash) normalName := "normal-res" normalObj := createTestObjectWithName(normalName, "default", "data-normal") meta, _ := utils.MetaAccessor(normalObj) rvAdd, _ := backend.WriteEvent(ctx, WriteEvent{ Type: resourcepb.WatchEvent_ADDED, Key: &resourcepb.ResourceKey{Namespace: "default", Group: "apps", Resource: "resources", Name: normalName}, Value: objectToJSONBytes(nil, normalObj), Object: meta, }) backend.WriteEvent(ctx, WriteEvent{ Type: resourcepb.WatchEvent_DELETED, Key: &resourcepb.ResourceKey{Namespace: "default", Group: "apps", Resource: "resources", Name: normalName}, PreviousRV: rvAdd, Object: meta, }) // Step 2: add a provisioned resource (should be filtered out from trash) provName := "provisioned-res" provObj := createTestObjectWithName(provName, "default", "data-prov") provMeta, _ := utils.MetaAccessor(provObj) provMeta.SetAnnotation(utils.AnnoKeyManagerKind, "repo") rvProvAdd, _ := backend.WriteEvent(ctx, WriteEvent{ Type: resourcepb.WatchEvent_ADDED, Key: &resourcepb.ResourceKey{Namespace: "default", Group: "apps", Resource: "resources", Name: provName}, Value: objectToJSONBytes(nil, provObj), Object: provMeta, }) backend.WriteEvent(ctx, WriteEvent{ Type: resourcepb.WatchEvent_DELETED, Key: &resourcepb.ResourceKey{Namespace: "default", Group: "apps", Resource: "resources", Name: provName}, PreviousRV: rvProvAdd, Object: provMeta, }) // Step 3: List trash with RV-based, provisioned-filtered behavior listReq := &resourcepb.ListRequest{ Source: resourcepb.ListRequest_TRASH, Options: &resourcepb.ListOptions{Key: &resourcepb.ResourceKey{Namespace: "default", Group: "apps", Resource: "resources"}}, Limit: 10, } type trashItem struct{ name string; rv int64 } var items []trashItem backend.ListHistory(ctx, listReq, func(iter ListIterator) error { for iter.Next() { items = append(items, trashItem{name: iter.Name(), rv: iter.ResourceVersion()}) } return iter.Error() }) fmt.Printf("Trash items (names/RV): %+v\n", items) // Expected outcome (vulnerable version): provisionsed-res would appear in trash results. // Post-fix expectation (this commit): provisioned-res is filtered out; trash contains non-provisioned items only. }

diff --git a/pkg/storage/unified/resource/storage_backend.go b/pkg/storage/unified/resource/storage_backend.go index b4140d7e3d887..17e9c147cf467 100644 --- a/pkg/storage/unified/resource/storage_backend.go +++ b/pkg/storage/unified/resource/storage_backend.go @@ -2,6 +2,7 @@ package resource import ( "bytes" + "cmp" "context" "encoding/json" "errors" @@ -10,6 +11,7 @@ import ( "iter" "math/rand/v2" "net/http" + "slices" "sync" "time" @@ -1400,14 +1402,12 @@ func (k *kvStorageBackend) ListHistory(ctx context.Context, req *resourcepb.List key := req.Options.Key // Parse continue token if provided lastSeenRV := int64(0) - lastSeenName := "" if req.NextPageToken != "" { token, err := GetContinueToken(req.NextPageToken) if err != nil { return 0, fmt.Errorf("invalid continue token: %w", err) } lastSeenRV = token.ResourceVersion - lastSeenName = token.Name } // Generate a new resource version for the list @@ -1415,7 +1415,7 @@ func (k *kvStorageBackend) ListHistory(ctx context.Context, req *resourcepb.List // Handle trash differently from regular history if req.Source == resourcepb.ListRequest_TRASH { - return k.processTrashEntries(ctx, req, fn, listRV, lastSeenName) + return k.processTrashEntries(ctx, req, fn, listRV, lastSeenRV) } // Get all history entries by iterating through datastore keys @@ -1471,44 +1471,21 @@ func (k *kvStorageBackend) ListHistory(ctx context.Context, req *resourcepb.List // processTrashEntries handles the special case of listing deleted items (trash). // It streams through keys in ascending order, tracking name groups. For each name, // if the latest event is a delete, it's a trash candidate. +// +// The results are sorted by RV desc: the sorting in this case matters as it's +// the only convenient place to sort results by RV using the datastore before +// doing a BatchGet to fetch the resources. Existing user-facing features (such as +// Restore Dashboards) currently rely on this behaviour. func (k *kvStorageBackend) processTrashEntries( ctx context.Context, req *resourcepb.ListRequest, fn func(ListIterator) error, listRV int64, - lastSeenName string, + lastSeenRV int64, ) (int64, error) { reqKey := req.Options.Key listKey := ListRequestKey{Group: reqKey.Group, Resource: reqKey.Resource, Namespace: reqKey.Namespace, Name: reqKey.Name} - startKey := listKey - if lastSeenName != "" { - // If we are continuing from a previous name (pagination), the start - // key should include the name, but the end key should not. - startKey.Name = lastSeenName - } - - listOptions := ListOptions{ - StartKey: startKey.Prefix(), - EndKey: PrefixRangeEnd(listKey.Prefix()), - Sort: SortOrderAsc, - } - keysIter := func(yield func(DataKey, error) bool) { - for rawKey, err := range k.dataStore.kv.Keys(ctx, dataSection, listOptions) { - if err != nil { - yield(DataKey{}, err) - return - } - dk, err := ParseKey(rawKey) - if err != nil { - yield(DataKey{}, err) - return - } - if !yield(dk, nil) { - return - } - } - } // Stream through keys, tracking name groups to find trash candidates candidates := make([]DataKey, 0, min(defaultListBufferSize, req.Limit+1)) @@ -1519,11 +1496,6 @@ func (k *kvStorageBackend) processTrashEntries( if latestKey == nil { return } - // When resuming from continue token, skip entries for lastSeenName - // (the trash candidate for that name was already yielded) - if lastSeenName != "" && latestKey.Name == lastSeenName { - return - } // Only include if the latest event for this name is a delete (i.e., resource is in trash) if latestKey.Action != DataActionDeleted { return @@ -1535,7 +1507,7 @@ func (k *kvStorageBackend) processTrashEntries( candidates = append(candidates, *latestKey) } - for dk, err := range keysIter { + for dk, err := range k.dataStore.Keys(ctx, listKey, SortOrderAsc) { if err != nil { return 0, err } @@ -1554,6 +1526,15 @@ func (k *kvStorageBackend) processTrashEntries( // Process the final name group processNameGroup() + // Sort candidates by resource version descending so the most recently + // deleted items come first. + slices.SortFunc(candidates, func(a, b DataKey) int { + return cmp.Compare(b.ResourceVersion, a.ResourceVersion) + }) + + // Apply RV-based pagination: skip candidates already seen on previous pages. + candidates = applyPagination(candidates, lastSeenRV) + // Create pull-style iterator from BatchGet next, stop := iter.Pull2(k.dataStore.BatchGet(ctx, candidates)) defer stop() diff --git a/pkg/storage/unified/resource/storage_backend_test.go b/pkg/storage/unified/resource/storage_backend_test.go index bf8d5eeca4dcd..8e7598450a507 100644 --- a/pkg/storage/unified/resource/storage_backend_test.go +++ b/pkg/storage/unified/resource/storage_backend_test.go @@ -1600,101 +1600,96 @@ func TestKvStorageBackend_ListTrash_Success(t *testing.T) { backend := setupTestStorageBackend(t) ctx := context.Background() - // Create a resource - testObj, err := createTestObjectWithName("test-resource", appsNamespace, "test-data") - require.NoError(t, err) + // Helper to create, then delete a resource and return the delete RV. + createAndDelete := func(name string) int64 { + obj, err := createTestObjectWithName(name, appsNamespace, "data-"+name) + require.NoError(t, err) + meta, err := utils.MetaAccessor(obj) + require.NoError(t, err) - metaAccessor, err := utils.MetaAccessor(testObj) - require.NoError(t, err) + addRV, err := backend.WriteEvent(ctx, WriteEvent{ + Type: resourcepb.WatchEvent_ADDED, + Key: &resourcepb.ResourceKey{ + Namespace: "default", Group: "apps", Resource: "resources", Name: name, + }, + Value: objectToJSONBytes(t, obj), + Object: meta, + }) + require.NoError(t, err) - writeEvent := WriteEvent{ - Type: resourcepb.WatchEvent_ADDED, - Key: &resourcepb.ResourceKey{ - Namespace: "default", - Group: "apps", - Resource: "resources", - Name: "test-resource", - }, - Value: objectToJSONBytes(t, testObj), - Object: metaAccessor, - PreviousRV: 0, + delRV, err := backend.WriteEvent(ctx, WriteEvent{ + Type: resourcepb.WatchEvent_DELETED, + Key: &resourcepb.ResourceKey{ + Namespace: "default", Group: "apps", Resource: "resources", Name: name, + }, + Value: objectToJSONBytes(t, obj), + Object: meta, + ObjectOld: meta, + PreviousRV: addRV, + }) + require.NoError(t, err) + return delRV } - rv1, err := backend.WriteEvent(ctx, writeEvent) - require.NoError(t, err) + // Create and delete three resources. Names are alphabetical, but delete + // order (and therefore RV order) is: resource-a, resource-b, resource-c. + rvA := createAndDelete("resource-a") + rvB := createAndDelete("resource-b") + rvC := createAndDelete("resource-c") - // Delete the resource - writeEvent.Type = resourcepb.WatchEvent_DELETED - writeEvent.PreviousRV = rv1 - writeEvent.Object = metaAccessor - writeEvent.ObjectOld = metaAccessor - - rv2, err := backend.WriteEvent(ctx, writeEvent) - require.NoError(t, err) - - // Do the same for a provisioned object - provisionedObj, err := createTestObjectWithName("provisioned-obj", appsNamespace, "test-data") + // Also create a provisioned object — it should be filtered out. + provObj, err := createTestObjectWithName("provisioned-obj", appsNamespace, "test-data") require.NoError(t, err) - metaAccessorProvisioned, err := utils.MetaAccessor(provisionedObj) + provMeta, err := utils.MetaAccessor(provObj) require.NoError(t, err) - metaAccessorProvisioned.SetAnnotation(utils.AnnoKeyManagerKind, "repo") + provMeta.SetAnnotation(utils.AnnoKeyManagerKind, "repo") - writeEventProvisioned := WriteEvent{ + provAddRV, err := backend.WriteEvent(ctx, WriteEvent{ Type: resourcepb.WatchEvent_ADDED, Key: &resourcepb.ResourceKey{ - Namespace: "default", - Group: "apps", - Resource: "resources", - Name: "provisioned-obj", + Namespace: "default", Group: "apps", Resource: "resources", Name: "provisioned-obj", }, - Value: objectToJSONBytes(t, provisionedObj), - Object: metaAccessorProvisioned, - PreviousRV: 0, - } - - rv3, err := backend.WriteEvent(ctx, writeEventProvisioned) + Value: objectToJSONBytes(t, provObj), + Object: provMeta, + }) require.NoError(t, err) - - writeEventProvisioned.Type = resourcepb.WatchEvent_DELETED - writeEventProvisioned.PreviousRV = rv3 - writeEventProvisioned.Object = metaAccessorProvisioned - writeEventProvisioned.ObjectOld = metaAccessorProvisioned - _, err = backend.WriteEvent(ctx, writeEventProvisioned) + _, err = backend.WriteEvent(ctx, WriteEvent{ + Type: resourcepb.WatchEvent_DELETED, + Key: &resourcepb.ResourceKey{ + Namespace: "default", Group: "apps", Resource: "resources", Name: "provisioned-obj", + }, + Value: objectToJSONBytes(t, provObj), + Object: provMeta, + ObjectOld: provMeta, + PreviousRV: provAddRV, + }) require.NoError(t, err) - // List the trash (deleted items) + // List all trash items — should be sorted by RV DESC. listReq := &resourcepb.ListRequest{ Options: &resourcepb.ListOptions{ Key: &resourcepb.ResourceKey{ - Namespace: "default", - Group: "apps", - Resource: "resources", - Name: "test-resource", + Namespace: "default", Group: "apps", Resource: "resources", }, }, Source: resourcepb.ListRequest_TRASH, Limit: 10, } - var trashItems []struct { + type trashItem struct { name string resourceVersion int64 - value []byte } + var items []trashItem rv, err := backend.ListHistory(ctx, listReq, func(iter ListIterator) error { for iter.Next() { if err := iter.Error(); err != nil { return err } - trashItems = append(trashItems, struct { - name string - resourceVersion int64 - value []byte - }{ + items = append(items, trashItem{ name: iter.Name(), resourceVersion: iter.ResourceVersion(), - value: iter.Value(), }) } return iter.Error() @@ -1702,12 +1697,16 @@ func TestKvStorageBackend_ListTrash_Success(t *testing.T) { require.NoError(t, err) require.Greater(t, rv, int64(0)) - require.Len(t, trashItems, 1) // Should have the non-provisioned deleted item - - // Verify the trash item - require.Equal(t, "test-resource", trashItems[0].name) - require.Equal(t, rv2, trashItems[0].resourceVersion) - require.Equal(t, objectToJSONBytes(t, testObj), trashItems[0].value) + // Provisioned item is filtered out, leaving 3 non-provisioned items. + require.Len(t, items, 3) + + // Verify RV descending order: resource-c (highest RV) first. + require.Equal(t, "resource-c", items[0].name) + require.Equal(t, rvC, items[0].resourceVersion) + require.Equal(t, "resource-b", items[1].name) + require.Equal(t, rvB, items[1].resourceVersion) + require.Equal(t, "resource-a", items[2].name) + require.Equal(t, rvA, items[2].resourceVersion) } func TestKvStorageBackend_GetResourceStats_Success(t *testing.T) { @@ -3019,8 +3018,10 @@ func TestKvStorageBackend_ListHistory_Behaviour(t *testing.T) { } require.Len(t, allNames, numResources) - slices.Sort(allNames) - slices.Sort(expectedNames) + + // The names collected should be the reverse of `expectedNames` + // (i.e., in RV desc order) + slices.Reverse(expectedNames) require.Equal(t, expectedNames, allNames) }) }