Argo CD: Kubernetes Secret Extraction via ArgoCD ServerSideDiff via sensitive annotations

@@ -189,10 +189,12 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D
 	Normalize(predictedLive, opts...)
 	unstructured.RemoveNestedField(predictedLive.Object, "metadata", "managedFields")
 	unstructured.RemoveNestedField(predictedLive.Object, "metadata", "resourceVersion")
+	unstructured.RemoveNestedField(predictedLive.Object, "metadata", "annotations", AnnotationLastAppliedConfig)
 
 	Normalize(live, opts...)
 	unstructured.RemoveNestedField(live.Object, "metadata", "managedFields")
 	unstructured.RemoveNestedField(live.Object, "metadata", "resourceVersion")
+  unstructured.RemoveNestedField(live.Object, "metadata", "annotations", AnnotationLastAppliedConfig)
 
 	if isCoreSecret(config) {
 		// Mask Secret data symmetrically before comparison.

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -1254,6 +1255,74 @@ func TestServerSideDiff(t *testing.T) {
 		assert.Contains(t, liveData, "key3", "key3 should still be in live state")
 	})
 
+	t.Run("will strip kubectl.kubernetes.io/last-applied-configuration from both sides", func(t *testing.T) {
+		t.Parallel()
+
+		const lastAppliedRaw = `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"secret","namespace":"default","annotations":{"app":"test"}},"data":{"password":"U0VDUkVUVkFM"},"stringData":{"username":"SECRETVAL"}}`
+
+		liveState := StrToUnstructured(`{
+			"apiVersion": "v1",
+			"kind": "Secret",
+			"metadata": {
+				"name": "secret",
+				"namespace": "default",
+				"annotations": {
+					"app": "test",
+					"kubectl.kubernetes.io/last-applied-configuration": ` + strconv.Quote(lastAppliedRaw) + `
+				}
+			},
+			"type": "Opaque",
+			"data": {
+				"password": "U0VDUkVUVkFM"
+			}
+		}`)
+		desiredState := StrToUnstructured(`{
+			"apiVersion": "v1",
+			"kind": "Secret",
+			"metadata": {
+				"name": "secret",
+				"namespace": "default",
+				"annotations": {
+					"app": "test"
+				}
+			},
+			"type": "Opaque",
+			"data": {
+				"password": "U0VDUkVUVkFM"
+			}
+		}`)
+		predictedLiveJSON := `{
+			"apiVersion": "v1",
+			"kind": "Secret",
+			"metadata": {
+				"name": "secret",
+				"namespace": "default",
+				"annotations": {
+					"app": "test",
+					"kubectl.kubernetes.io/last-applied-configuration": ` + strconv.Quote(lastAppliedRaw) + `
+				}
+			},
+			"type": "Opaque",
+			"data": {
+				"password": "U0VDUkVUVkFM"
+			}
+		}`
+		opts := buildOpts(predictedLiveJSON)
+		opts = append(opts, WithIgnoreMutationWebhook(false))
+
+		// when
+		result, err := serverSideDiff(desiredState, liveState, opts...)
+
+		// then
+		require.NoError(t, err)
+		require.NotNil(t, result)
+		assert.NotContains(t, string(result.PredictedLive), "kubectl.kubernetes.io/last-applied-configuration",
+			"PredictedLive must not contain the last-applied-configuration annotation")
+		assert.NotContains(t, string(result.NormalizedLive), "kubectl.kubernetes.io/last-applied-configuration",
+			"NormalizedLive must not contain the last-applied-configuration annotation")
+		assert.NotContains(t, string(result.PredictedLive), "SECRETVAL",
+			"PredictedLive must not contain raw secret values from last-applied-configuration")
+	})
 	t.Run("will mask Secret data symmetrically so identical values do not produce a spurious diff", func(t *testing.T) {
 		t.Parallel()
 

@@ -190,14 +190,24 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D
 	unstructured.RemoveNestedField(predictedLive.Object, "metadata", "managedFields")
 	unstructured.RemoveNestedField(predictedLive.Object, "metadata", "resourceVersion")
 
+	Normalize(live, opts...)
+	unstructured.RemoveNestedField(live.Object, "metadata", "managedFields")
+	unstructured.RemoveNestedField(live.Object, "metadata", "resourceVersion")
+
+	if isCoreSecret(config) {
+		// Mask Secret data symmetrically before comparison.
+		// Equal values get equal placeholders, different values get different placeholders.
+		predictedLive, live, err = HideSecretData(predictedLive, live, nil)
+		if err != nil {
+			return nil, fmt.Errorf("error hiding secret data for resource %s/%s: %w", config.GetKind(), config.GetName(), err)
+		}
+	}
+
 	predictedLiveBytes, err := json.Marshal(predictedLive)
 	if err != nil {
 		return nil, fmt.Errorf("error marshaling predicted live for resource %s/%s: %w", config.GetKind(), config.GetName(), err)
 	}
 
-	Normalize(live, opts...)
-	unstructured.RemoveNestedField(live.Object, "metadata", "managedFields")
-	unstructured.RemoveNestedField(live.Object, "metadata", "resourceVersion")
 	liveBytes, err := json.Marshal(live)
 	if err != nil {
 		return nil, fmt.Errorf("error marshaling live resource %s/%s: %w", config.GetKind(), config.GetName(), err)
@@ -357,6 +367,15 @@ func jsonStrToUnstructured(jsonString string) (*unstructured.Unstructured, error
 	return &unstructured.Unstructured{Object: res}, nil
 }
 
+// isCoreSecret reports whether obj is a core/v1 Secret (Group="" and Kind="Secret").
+func isCoreSecret(obj *unstructured.Unstructured) bool {
+	if obj == nil {
+		return false
+	}
+	gvk := obj.GroupVersionKind()
+	return gvk.Group == "" && gvk.Kind == "Secret"
+}
+
 // StructuredMergeDiff will calculate the diff using the structured-merge-diff
 // k8s library (https://github.com/kubernetes-sigs/structured-merge-diff).
 func StructuredMergeDiff(config, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*DiffResult, error) {

@@ -1254,6 +1254,125 @@ func TestServerSideDiff(t *testing.T) {
 		assert.Contains(t, liveData, "key3", "key3 should still be in live state")
 	})
 
+	t.Run("will mask Secret data symmetrically so identical values do not produce a spurious diff", func(t *testing.T) {
+		t.Parallel()
+
+		desired := buildSecret("test-secret", "default", map[string]string{"password": "vault:secret/foo"}, nil)
+		live := buildSecret("test-secret", "default", map[string]string{"password": "injected-by-webhook"}, nil)
+		predictedLiveJSON := mustMarshalJSON(t, buildSecret("test-secret", "default", map[string]string{"password": "injected-by-webhook"}, nil))
+
+		opts := append(buildOpts(predictedLiveJSON), WithIgnoreMutationWebhook(false))
+		result, err := serverSideDiff(desired, live, opts...)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+
+		assert.False(t, result.Modified, "identical secret values on both sides must not be flagged as modified after masking")
+
+		predictedData := mustGetSecretData(t, result.PredictedLive)
+		liveData := mustGetSecretData(t, result.NormalizedLive)
+		assert.Equal(t, "++++++++", predictedData["password"], "predicted data must be masked, not raw")
+		assert.Equal(t, "++++++++", liveData["password"], "live data must be masked, not raw")
+	})
+
+	t.Run("will keep Secret data masked but still detect genuine value differences", func(t *testing.T) {
+		t.Parallel()
+
+		desired := buildSecret("test-secret", "default", map[string]string{"password": "vault:secret/foo"}, nil)
+		live := buildSecret("test-secret", "default", map[string]string{"password": "old-value"}, nil)
+		predictedLiveJSON := mustMarshalJSON(t, buildSecret("test-secret", "default", map[string]string{"password": "new-value"}, nil))
+
+		opts := append(buildOpts(predictedLiveJSON), WithIgnoreMutationWebhook(false))
+		result, err := serverSideDiff(desired, live, opts...)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+
+		assert.True(t, result.Modified, "different secret values must still be flagged as modified")
+
+		predictedData := mustGetSecretData(t, result.PredictedLive)
+		liveData := mustGetSecretData(t, result.NormalizedLive)
+		// HideSecretData yields different placeholder lengths for different values, so the
+		// data field is masked on both sides and the two placeholders differ.
+		assert.NotEqual(t, "new-value", predictedData["password"], "raw new value must not leak into PredictedLive")
+		assert.NotEqual(t, "old-value", liveData["password"], "raw old value must not leak into NormalizedLive")
+		assert.NotEqual(t, predictedData["password"], liveData["password"], "differing values must yield differing placeholders")
+	})
+
+	t.Run("will detect Secret key additions and removals", func(t *testing.T) {
+		t.Parallel()
+
+		desired := buildSecret("test-secret", "default", map[string]string{"password": "x", "token": "y"}, nil)
+		live := buildSecret("test-secret", "default", map[string]string{"password": "x"}, nil)
+		predictedLiveJSON := mustMarshalJSON(t, buildSecret("test-secret", "default", map[string]string{"password": "x", "token": "y"}, nil))
+
+		opts := append(buildOpts(predictedLiveJSON), WithIgnoreMutationWebhook(false))
+		result, err := serverSideDiff(desired, live, opts...)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+
+		assert.True(t, result.Modified, "added Secret keys must still be flagged as modified after masking")
+	})
+
+	t.Run("will not mask non-core Secret resources", func(t *testing.T) {
+		// Resources whose Kind is "Secret" but whose Group is non-empty (e.g. CRDs)
+		// must not be touched by the core/v1 Secret masking path.
+		t.Parallel()
+
+		desired := buildSecret("test-secret", "default", map[string]string{"password": "raw-value"}, nil)
+		desired.SetAPIVersion("custom.io/v1")
+		live := buildSecret("test-secret", "default", map[string]string{"password": "raw-value"}, nil)
+		live.SetAPIVersion("custom.io/v1")
+		predictedLiveJSON := mustMarshalJSON(t, desired)
+
+		opts := append(buildOpts(predictedLiveJSON), WithIgnoreMutationWebhook(false))
+		result, err := serverSideDiff(desired, live, opts...)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+
+		predictedData := mustGetSecretData(t, result.PredictedLive)
+		assert.Equal(t, "raw-value", predictedData["password"], "non-core Secret data must be left untouched")
+	})
+}
+
+// buildSecret returns a core/v1 Secret as an *unstructured.Unstructured.
+func buildSecret(name, namespace string, data map[string]string, annotations map[string]string) *unstructured.Unstructured {
+	dataField := make(map[string]any, len(data))
+	for k, v := range data {
+		dataField[k] = v
+	}
+	metadata := map[string]any{
+		"name":      name,
+		"namespace": namespace,
+	}
+	if len(annotations) > 0 {
+		annField := make(map[string]any, len(annotations))
+		for k, v := range annotations {
+			annField[k] = v
+		}
+		metadata["annotations"] = annField
+	}
+	return &unstructured.Unstructured{Object: map[string]any{
+		"apiVersion": "v1",
+		"kind":       "Secret",
+		"metadata":   metadata,
+		"type":       "Opaque",
+		"data":       dataField,
+	}}
+}
+
+func mustMarshalJSON(t *testing.T, obj *unstructured.Unstructured) string {
+	t.Helper()
+	bytes, err := json.Marshal(obj)
+	require.NoError(t, err)
+	return string(bytes)
+}
+
+func mustGetSecretData(t *testing.T, secretBytes []byte) map[string]any {
+	t.Helper()
+	var obj map[string]any
+	require.NoError(t, json.Unmarshal(secretBytes, &obj))
+	data, ok := obj["data"].(map[string]any)
+	require.True(t, ok, "expected data field to be a map")
+	return data
 }
 
 // testIgnoreDifferencesNormalizer implements a simple normalizer that removes specified fields

@@ -20,6 +20,7 @@ import (
 	"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v3/util/argo/normalizers"
 	"github.com/argoproj/argo-cd/v3/util/resource"
+	"github.com/argoproj/argo-cd/v3/util/settings"
 )
 
 func populateNodeInfo(un *unstructured.Unstructured, res *ResourceInfo, customLabels []string) {
@@ -40,6 +41,16 @@ func populateNodeInfo(un *unstructured.Unstructured, res *ResourceInfo, customLa
 
 	for k, v := range un.GetAnnotations() {
 		if strings.HasPrefix(k, common.AnnotationKeyLinkPrefix) {
+			// Annotation values may be either a bare URL or "title|url"; validate
+			// the URL portion to prevent XSS via javascript:/data:/vbscript: URIs
+			// when the value is rendered as an href in the UI.
+			urlPart := v
+			if idx := strings.Index(v, "|"); idx >= 0 {
+				urlPart = v[idx+1:]
+			}
+			if err := settings.ValidateExternalURL(urlPart); err != nil {
+				continue
+			}
 			if res.NetworkingInfo == nil {
 				res.NetworkingInfo = &v1alpha1.ResourceNetworkingInfo{}
 			}

@@ -62,6 +62,30 @@ var (
       ingress:
       - hostname: localhost`)
 
+	testMaliciousLinkAnnotatedService = strToUnstructured(`
+  apiVersion: v1
+  kind: Service
+  metadata:
+    name: helm-guestbook
+    namespace: default
+    resourceVersion: "123"
+    uid: "4"
+    annotations:
+      link.argocd.argoproj.io/javascript: 'javascript:alert(1)'
+      link.argocd.argoproj.io/data: 'data:text/html,<script>alert(1)</script>'
+      link.argocd.argoproj.io/vbscript: 'vbscript:msgbox(1)'
+      link.argocd.argoproj.io/titled-javascript: 'click me|javascript:alert(1)'
+      link.argocd.argoproj.io/no-scheme: 'example.com/foo'
+      link.argocd.argoproj.io/safe: 'http://my-grafana.example.com/pre-generated-link'
+  spec:
+    selector:
+      app: guestbook
+    type: LoadBalancer
+  status:
+    loadBalancer:
+      ingress:
+      - hostname: localhost`)
+
 	testIngress = strToUnstructured(`
   apiVersion: extensions/v1beta1
   kind: Ingress
@@ -1140,6 +1164,15 @@ func TestGetLinkAnnotatedServiceInfo(t *testing.T) {
 	}, info.NetworkingInfo)
 }
 
+func TestMaliciousLinkAnnotatedServiceInfoFiltered(t *testing.T) {
+	info := &ResourceInfo{}
+	populateNodeInfo(testMaliciousLinkAnnotatedService, info, []string{})
+	require.NotNil(t, info.NetworkingInfo)
+	// Only the http URL should make it through; javascript:, data:, vbscript:,
+	// "title|javascript:..." and scheme-less values must be dropped.
+	assert.Equal(t, []string{"http://my-grafana.example.com/pre-generated-link"}, info.NetworkingInfo.ExternalURLs)
+}
+
 func TestGetIstioVirtualServiceInfo(t *testing.T) {
 	info := &ResourceInfo{}
 	populateNodeInfo(testIstioVirtualService, info, []string{})

@@ -20,6 +20,7 @@ import {BadgePanel} from '../../../shared/components';
 import {AuthSettingsCtx, Consumer, ContextApis} from '../../../shared/context';
 import * as models from '../../../shared/models';
 import {services} from '../../../shared/services';
+import {isValidURL} from '../../../shared/utils';
 
 import {ApplicationSyncOptionsField} from '../application-sync-options/application-sync-options';
 import {RevisionFormField} from '../revision-form-field/revision-form-field';
@@ -272,6 +273,10 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => {
                 <div className='application-summary__links-rows'>
                     {urls
                         .map(item => item.split('|'))
+                        // Drop entries whose URL uses an unsafe protocol (e.g. javascript:, data:,
+                        // vbscript:) to prevent XSS via attacker-controlled
+                        // link.argocd.argoproj.io/* annotations on managed resources.
+                        .filter(parts => isValidURL(parts.length > 1 ? parts[1] : parts[0]))
                         .map((parts, i) => (
                             <div className='application-summary__links-row'>
                                 <a key={i} href={parts.length > 1 ? parts[1] : parts[0]} target='_blank'>

@@ -1,4 +1,9 @@
+/* eslint-env jest */
+declare const test: any;
+declare const expect: any;
+declare const describe: any;
 import {concatMaps} from './utils';
+import {isValidURL} from './utils';
 
 test('map concatenation', () => {
     const map1 = {
@@ -12,3 +17,23 @@ test('map concatenation', () => {
     const map3 = concatMaps(map1, map2);
     expect(map3).toEqual(new Map(Object.entries({a: '9', b: '2', c: '8'})));
 });
+
+describe('isValidURL', () => {
+    test('accepts http/https URLs', () => {
+        expect(isValidURL('http://example.com')).toBe(true);
+        expect(isValidURL('https://example.com/path?q=1')).toBe(true);
+    });
+
+    test('accepts relative URLs', () => {
+        // @ts-ignore
+        window.location = new URL('https://localhost:8080/applications');
+        expect(isValidURL('/applications')).toBe(true);
+    });
+
+    test('rejects unsafe protocols', () => {
+        expect(isValidURL('javascript:alert(1)')).toBe(false);
+        expect(isValidURL('JaVaScRiPt:alert(1)')).toBe(false);
+        expect(isValidURL('data:text/html,<script>alert(1)</script>')).toBe(false);
+        expect(isValidURL('vbscript:msgbox(1)')).toBe(false);
+    });
+});