Fix ownership check

internal/controller/appconfigurationprovider_controller.go

@@ -296,7 +296,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) verifyTargetObjectExi
 		return false, err
 	}
 
-	return true, verifyExistingTargetObject(obj, targetName, provider.Name)
+	return true, verifyExistingTargetObject(obj, targetName, provider)
 }
 
 // collectExistingSecret verifies the existence of a secret and adds it to the existingSecrets map if it exists

internal/controller/appconfigurationprovider_controller_test.go

@@ -1427,4 +1427,134 @@ var _ = Describe("AppConfiguationProvider controller", func() {
 			_ = k8sClient.Delete(ctx, configProvider)
 		})
 	})
+
+	Describe("When target Secret already exists with a foreign owner reference", func() {
+		It("Should refuse to overwrite a plain pre-existing Secret it does not own", func() {
+			By("pre-creating a Secret with no owner references")
+			ctx := context.Background()
+			victimName := "victim-plain-secret"
+			attackerProvider := "attacker-provider-plain"
+
+			victim := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: victimName, Namespace: ProviderNamespace},
+				Type:       corev1.SecretTypeOpaque,
+				Data:       map[string][]byte{"protected": []byte("ORIGINAL-VALUE")},
+			}
+			Expect(k8sClient.Create(ctx, victim)).Should(Succeed())
+
+			attackerSettings := &loader.TargetKeyValueSettings{
+				SecretSettings: map[string]corev1.Secret{
+					victimName: {
+						Data: map[string][]byte{"protected": []byte("ATTACKER-INJECTED")},
+						Type: corev1.SecretTypeOpaque,
+					},
+				},
+				K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
+					victimName: {
+						Type:                    corev1.SecretTypeOpaque,
+						SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
+					},
+				},
+			}
+			mockConfigurationSettings.EXPECT().CreateTargetSettings(gomock.Any(), gomock.Any()).Return(attackerSettings, nil).AnyTimes()
+
+			provider := &acpv1.AzureAppConfigurationProvider{
+				ObjectMeta: metav1.ObjectMeta{Name: attackerProvider, Namespace: ProviderNamespace},
+				Spec: acpv1.AzureAppConfigurationProviderSpec{
+					Endpoint: &EndpointName,
+					Target:   acpv1.ConfigurationGenerationParameters{ConfigMapName: "cm-plain"},
+					Secret: &acpv1.SecretReference{
+						Target: acpv1.SecretGenerationParameters{SecretName: victimName},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, provider)).Should(Succeed())
+
+			By("verifying the victim Secret data remains unchanged")
+			Consistently(func() string {
+				s := &corev1.Secret{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{Name: victimName, Namespace: ProviderNamespace}, s)
+				return string(s.Data["protected"])
+			}, duration, interval).Should(Equal("ORIGINAL-VALUE"))
+
+			_ = k8sClient.Delete(ctx, provider)
+			_ = k8sClient.Delete(ctx, victim)
+		})
+
+		It("Should refuse to overwrite a Secret whose non-provider owner reference name matches the provider name", func() {
+			By("pre-creating a Secret owned by a Deployment sharing the attacker provider's name")
+			ctx := context.Background()
+			victimName := "victim-owned-secret"
+			// The attacker names their provider CR to match an ownerReference already
+			// present on the victim Secret. Here the Secret is owned by a Deployment
+			// named "frontend" (models cert-manager Certificate / workload / operator CR).
+			attackerProvider := "frontend"
+
+			victim := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      victimName,
+					Namespace: ProviderNamespace,
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "apps/v1",
+							Kind:       "Deployment",
+							Name:       "frontend", // non-provider owner, different Kind and UID
+							UID:        types.UID("11111111-1111-1111-1111-111111111111"),
+						},
+					},
+				},
+				Type: corev1.SecretTypeOpaque,
+				Data: map[string][]byte{"protected": []byte("ORIGINAL-VALUE")},
+			}
+			Expect(k8sClient.Create(ctx, victim)).Should(Succeed())
+
+			attackerSettings := &loader.TargetKeyValueSettings{
+				SecretSettings: map[string]corev1.Secret{
+					victimName: {
+						Data: map[string][]byte{
+							"protected": []byte("ATTACKER-INJECTED"),
+							"extra":     []byte("PWNED"),
+						},
+						Type: corev1.SecretTypeOpaque,
+					},
+				},
+				K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
+					victimName: {
+						Type:                    corev1.SecretTypeOpaque,
+						SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
+					},
+				},
+			}
+			mockConfigurationSettings.EXPECT().CreateTargetSettings(gomock.Any(), gomock.Any()).Return(attackerSettings, nil).AnyTimes()
+
+			provider := &acpv1.AzureAppConfigurationProvider{
+				ObjectMeta: metav1.ObjectMeta{Name: attackerProvider, Namespace: ProviderNamespace},
+				Spec: acpv1.AzureAppConfigurationProviderSpec{
+					Endpoint: &EndpointName,
+					Target:   acpv1.ConfigurationGenerationParameters{ConfigMapName: "cm-owned"},
+					Secret: &acpv1.SecretReference{
+						Target: acpv1.SecretGenerationParameters{SecretName: victimName},
+					},
+				},
+			}
+			Expect(k8sClient.Create(ctx, provider)).Should(Succeed())
+
+			By("verifying the victim Secret data and owner reference remain unchanged")
+			Consistently(func() string {
+				s := &corev1.Secret{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{Name: victimName, Namespace: ProviderNamespace}, s)
+				return string(s.Data["protected"])
+			}, duration, interval).Should(Equal("ORIGINAL-VALUE"))
+
+			s := &corev1.Secret{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: victimName, Namespace: ProviderNamespace}, s)).Should(Succeed())
+			Expect(s.Data).ShouldNot(HaveKey("extra"))
+			Expect(len(s.OwnerReferences)).Should(Equal(1))
+			Expect(s.OwnerReferences[0].Kind).Should(Equal("Deployment"))
+			Expect(s.OwnerReferences[0].Name).Should(Equal("frontend"))
+
+			_ = k8sClient.Delete(ctx, provider)
+			_ = k8sClient.Delete(ctx, victim)
+		})
+	})
 })

internal/controller/utils.go

@@ -199,15 +199,23 @@ func verifyAuthObject(auth *acpv1.AzureAppConfigurationProviderAuth) error {
 	return nil
 }
 
-func verifyExistingTargetObject[T client.Object](targetObj T, targetName string, providerName string) error {
+func verifyExistingTargetObject[T client.Object](targetObj T, targetName string, provider *acpv1.AzureAppConfigurationProvider) error {
 	objectKind := targetObj.GetObjectKind().GroupVersionKind().Kind
 	if targetObj.GetName() != targetName {
 		return nil
 	}
 
-	// If existing object is created by current provider, just skip it.
+	// If existing object is owned by the current provider, just skip it.
+	// A valid ownership must identify the same AzureAppConfigurationProvider by
+	// API version, kind, name and UID. Comparing the name alone is not sufficient:
+	// a foreign owner reference that happens to share the provider's name (for
+	// example, a different resource kind) must not authorize adoption or update.
+	expectedAPIVersion := acpv1.GroupVersion.String()
 	for _, ownerRef := range targetObj.GetOwnerReferences() {
-		if ownerRef.Name == providerName {
+		if ownerRef.APIVersion == expectedAPIVersion &&
+			ownerRef.Kind == ProviderName &&
+			ownerRef.Name == provider.Name &&
+			ownerRef.UID == provider.UID {
 			return nil
 		}
 	}

internal/controller/utils_test.go

@@ -1063,18 +1063,26 @@ func TestVerifyAuthObject(t *testing.T) {
 
 func TestVerifyExistingTargetObject(t *testing.T) {
 	providerNamespace := "default"
+	expectedAPIVersion := acpv1.GroupVersion.String()
 
 	t.Run("Should return no error if existing configMap is valid", func(t *testing.T) {
 		providerName := "providerName"
 		configMapName := "configMapName"
+		provider := &acpv1.AzureAppConfigurationProvider{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      providerName,
+				Namespace: providerNamespace,
+				UID:       "provider-uid",
+			},
+		}
 
 		// Case 1: ConfigMap owned by the provider
 		configMap := &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      configMapName,
 				Namespace: providerNamespace,
 				OwnerReferences: []metav1.OwnerReference{
-					{Kind: "AzureAppConfigurationProvider", Name: providerName},
+					{APIVersion: expectedAPIVersion, Kind: "AzureAppConfigurationProvider", Name: providerName, UID: "provider-uid"},
 				},
 			},
 		}
@@ -1085,22 +1093,29 @@ func TestVerifyExistingTargetObject(t *testing.T) {
 				Name:      "anotherConfigMap",
 				Namespace: providerNamespace,
 				OwnerReferences: []metav1.OwnerReference{
-					{Kind: "AzureAppConfigurationProvider", Name: providerName},
+					{APIVersion: expectedAPIVersion, Kind: "AzureAppConfigurationProvider", Name: providerName, UID: "provider-uid"},
 				},
 			},
 		}
 
 		// Case 3: Empty ConfigMap
 		emptyConfigMap := &corev1.ConfigMap{}
 
-		assert.Nil(t, verifyExistingTargetObject(emptyConfigMap, configMapName, providerName))
-		assert.Nil(t, verifyExistingTargetObject(configMap, configMapName, providerName))
-		assert.Nil(t, verifyExistingTargetObject(configMap2, configMapName, providerName))
+		assert.Nil(t, verifyExistingTargetObject(emptyConfigMap, configMapName, provider))
+		assert.Nil(t, verifyExistingTargetObject(configMap, configMapName, provider))
+		assert.Nil(t, verifyExistingTargetObject(configMap2, configMapName, provider))
 	})
 
 	t.Run("Should return error if configMap is not valid", func(t *testing.T) {
 		providerName := "providerName"
 		configMapName := "configMapName"
+		provider := &acpv1.AzureAppConfigurationProvider{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      providerName,
+				Namespace: providerNamespace,
+				UID:       "provider-uid",
+			},
+		}
 
 		// Case 1: ConfigMap exists but is not owned by the provider
 		configMap1 := &corev1.ConfigMap{
@@ -1122,16 +1137,52 @@ func TestVerifyExistingTargetObject(t *testing.T) {
 				Name:      configMapName,
 				Namespace: providerNamespace,
 				OwnerReferences: []metav1.OwnerReference{
-					{Kind: "AzureAppConfigurationProvider", Name: "anotherProvider"},
+					{APIVersion: expectedAPIVersion, Kind: "AzureAppConfigurationProvider", Name: "anotherProvider", UID: "another-uid"},
+				},
+			},
+		}
+
+		// Case 3: ConfigMap owned by a foreign resource that shares the provider's name.
+		// The name matches but the API version, kind and UID do not, so it must be rejected.
+		configMap3 := &corev1.ConfigMap{
+			TypeMeta: metav1.TypeMeta{
+				Kind: "ConfigMap",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      configMapName,
+				Namespace: providerNamespace,
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: "apps/v1", Kind: "Deployment", Name: providerName, UID: "foreign-uid"},
+				},
+			},
+		}
+
+		// Case 4: ConfigMap owned by an AzureAppConfigurationProvider with the same
+		// name but a different UID (for example, a deleted and recreated provider).
+		configMap4 := &corev1.ConfigMap{
+			TypeMeta: metav1.TypeMeta{
+				Kind: "ConfigMap",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      configMapName,
+				Namespace: providerNamespace,
+				OwnerReferences: []metav1.OwnerReference{
+					{APIVersion: expectedAPIVersion, Kind: "AzureAppConfigurationProvider", Name: providerName, UID: "stale-uid"},
 				},
 			},
 		}
 
-		err1 := verifyExistingTargetObject(configMap1, configMapName, providerName)
+		err1 := verifyExistingTargetObject(configMap1, configMapName, provider)
 		assert.Equal(t, "a ConfigMap with name 'configMapName' already exists in namespace 'default'", err1.Error())
 
-		err2 := verifyExistingTargetObject(configMap2, configMapName, providerName)
+		err2 := verifyExistingTargetObject(configMap2, configMapName, provider)
 		assert.Equal(t, "a ConfigMap with name 'configMapName' already exists in namespace 'default'", err2.Error())
+
+		err3 := verifyExistingTargetObject(configMap3, configMapName, provider)
+		assert.Equal(t, "a ConfigMap with name 'configMapName' already exists in namespace 'default'", err3.Error())
+
+		err4 := verifyExistingTargetObject(configMap4, configMapName, provider)
+		assert.Equal(t, "a ConfigMap with name 'configMapName' already exists in namespace 'default'", err4.Error())
 	})
 }