From b904354c349f7d4dd092327c4bc9767e3eee36e6 Mon Sep 17 00:00:00 2001
From: Brandon Palm <bpalm@redhat.com>
Date: Wed, 30 Sep 2026 15:48:08 -0500
Subject: [PATCH] resolver/cache: validate APIKey before GVK string
 serialization

APIKeyToGVKString previously serialized any APIKey unconditionally,
producing garbage strings like ".." for empty keys. These propagated
into RBAC labels and OperatorGroup provided-API annotations.

Add isValidAPIKey (Kind and Version must be non-empty; Group is
optional for core Kubernetes APIs) and guard APIKeyToGVKString with it.
Update APIKeyToGVKHash to return an error for invalid keys rather than
silently hashing an empty string. Update APISet.String() to filter out
invalid keys. Remove three TODOs that documented the gap.

Tests: add TestAPIKeyToGVKHash covering error and success paths; add
CoreAPIKey/EmptyGroup case; add AllInvalidKeys/EmptyOutput to
TestAPISetString.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../registry/resolver/cache/operators.go      | 22 ++++---
 .../registry/resolver/cache/operators_test.go | 58 +++++++++++++++++--
 2 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/pkg/controller/registry/resolver/cache/operators.go b/pkg/controller/registry/resolver/cache/operators.go
index f636cc8c9e..84ede77343 100644
--- a/pkg/controller/registry/resolver/cache/operators.go
+++ b/pkg/controller/registry/resolver/cache/operators.go
@@ -37,7 +37,6 @@ func (s APISet) PopAPIKey() *opregistry.APIKey {
 
 func GVKStringToProvidedAPISet(gvksStr string) APISet {
 	set := make(APISet)
-	// TODO: Should we make gvk strings lowercase to avoid issues with user set gvks?
 	gvks := strings.Split(strings.Replace(gvksStr, " ", "", -1), ",")
 	for _, gvkStr := range gvks {
 		gvk, _ := schema.ParseKindArg(gvkStr)
@@ -49,12 +48,22 @@ func GVKStringToProvidedAPISet(gvksStr string) APISet {
 	return set
 }
 
+// Group is intentionally excluded — an empty Group is valid for core Kubernetes APIs.
+func isValidAPIKey(key opregistry.APIKey) bool {
+	return key.Kind != "" && key.Version != ""
+}
+
 func APIKeyToGVKString(key opregistry.APIKey) string {
-	// TODO: Add better validation of GVK
+	if !isValidAPIKey(key) {
+		return ""
+	}
 	return strings.Join([]string{key.Kind, key.Version, key.Group}, ".")
 }
 
 func APIKeyToGVKHash(key opregistry.APIKey) (string, error) {
+	if !isValidAPIKey(key) {
+		return "", fmt.Errorf("invalid APIKey: Kind and Version must be non-empty")
+	}
 	hash := fnv.New64a()
 	if _, err := hash.Write([]byte(APIKeyToGVKString(key))); err != nil {
 		return "", err
@@ -63,12 +72,11 @@ func APIKeyToGVKHash(key opregistry.APIKey) (string, error) {
 }
 
 func (s APISet) String() string {
-	gvkStrs := make([]string, len(s))
-	i := 0
+	gvkStrs := make([]string, 0, len(s))
 	for api := range s {
-		// TODO: Only add valid GVK strings
-		gvkStrs[i] = APIKeyToGVKString(api)
-		i++
+		if str := APIKeyToGVKString(api); str != "" {
+			gvkStrs = append(gvkStrs, str)
+		}
 	}
 	sort.Strings(gvkStrs)
 
diff --git a/pkg/controller/registry/resolver/cache/operators_test.go b/pkg/controller/registry/resolver/cache/operators_test.go
index 61f8a8bf3a..91aab9d3e3 100644
--- a/pkg/controller/registry/resolver/cache/operators_test.go
+++ b/pkg/controller/registry/resolver/cache/operators_test.go
@@ -81,18 +81,23 @@ func TestAPIKeyToGVKString(t *testing.T) {
 		{
 			name: "EmptyAPIKey",
 			in:   opregistry.APIKey{},
-			want: "..",
+			want: "",
 		},
 		{
-			name: "BadAPIKey",
+			name: "BadAPIKey/MissingKind",
 			in:   opregistry.APIKey{Group: "birds. ", Version: "-"},
-			want: ".-.birds. ",
+			want: "",
 		},
 		{
 			name: "GoodAPIKey",
 			in:   opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Goose"},
 			want: "Goose.v1alpha1.birds.com",
 		},
+		{
+			name: "CoreAPIKey/EmptyGroup",
+			in:   opregistry.APIKey{Version: "v1", Kind: "Pod"},
+			want: "Pod.v1.",
+		},
 	}
 
 	for _, tt := range tests {
@@ -102,6 +107,43 @@ func TestAPIKeyToGVKString(t *testing.T) {
 	}
 }
 
+func TestAPIKeyToGVKHash(t *testing.T) {
+	tests := []struct {
+		name    string
+		in      opregistry.APIKey
+		wantErr bool
+	}{
+		{
+			name:    "EmptyAPIKey/Error",
+			in:      opregistry.APIKey{},
+			wantErr: true,
+		},
+		{
+			name:    "MissingKind/Error",
+			in:      opregistry.APIKey{Group: "birds.com", Version: "v1alpha1"},
+			wantErr: true,
+		},
+		{
+			name:    "GoodAPIKey/NoError",
+			in:      opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Goose"},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hash, err := APIKeyToGVKHash(tt.in)
+			if tt.wantErr {
+				require.Error(t, err)
+				require.Empty(t, hash)
+			} else {
+				require.NoError(t, err)
+				require.NotEmpty(t, hash)
+			}
+		})
+	}
+}
+
 func TestAPISetString(t *testing.T) {
 	tests := []struct {
 		name string
@@ -135,7 +177,15 @@ func TestAPISetString(t *testing.T) {
 				opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Goose"}: {},
 				opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Egret"}: {},
 			},
-			want: ".v1alpha1.birds.com,Egret.v1alpha1.birds.com,Goose.v1alpha1.birds.com",
+			want: "Egret.v1alpha1.birds.com,Goose.v1alpha1.birds.com",
+		},
+		{
+			name: "AllInvalidKeys/EmptyOutput",
+			in: APISet{
+				opregistry.APIKey{}: {},
+				opregistry.APIKey{Group: "birds.com", Version: "v1alpha1"}: {},
+			},
+			want: "",
 		},
 	}