From 3b8f673f06c2d228e01cbc830e5cb57cef008978 Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Wed, 13 Mar 2024 14:28:43 -0400
Subject: [PATCH] Merge pull request from GHSA-g623-jcgg-mhmm

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 server/application/application.go      |  9 +++++++++
 server/application/application_test.go | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/server/application/application.go b/server/application/application.go
index 8ee16b93494c..ec0db45a11f2 100644
--- a/server/application/application.go
+++ b/server/application/application.go
@@ -333,6 +333,15 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
 		return nil, security.NamespaceNotPermittedError(appNs)
 	}
 
+	// Don't let the app creator set the operation explicitly. Those requests should always go through the Sync API.
+	if a.Operation != nil {
+		log.WithFields(log.Fields{
+			"application":            a.Name,
+			argocommon.SecurityField: argocommon.SecurityLow,
+		}).Warn("User attempted to set operation on application creation. This could have allowed them to bypass branch protection rules by setting manifests directly. Ignoring the set operation.")
+		a.Operation = nil
+	}
+
 	created, err := s.appclientset.ArgoprojV1alpha1().Applications(appNs).Create(ctx, a, metav1.CreateOptions{})
 	if err == nil {
 		s.logAppEvent(created, ctx, argo.EventReasonResourceCreated, "created application")
diff --git a/server/application/application_test.go b/server/application/application_test.go
index 65600ad629d3..51c912ff0510 100644
--- a/server/application/application_test.go
+++ b/server/application/application_test.go
@@ -1439,6 +1439,27 @@ func TestCreateAppWithDestName(t *testing.T) {
 	assert.Equal(t, app.Spec.Destination.Server, "https://cluster-api.example.com")
 }
 
+// TestCreateAppWithOperation tests that an application created with an operation is created with the operation removed.
+// Avoids regressions of https://github.com/argoproj/argo-cd/security/advisories/GHSA-g623-jcgg-mhmm
+func TestCreateAppWithOperation(t *testing.T) {
+	appServer := newTestAppServer(t)
+	testApp := newTestAppWithDestName()
+	testApp.Operation = &appsv1.Operation{
+		Sync: &appsv1.SyncOperation{
+			Manifests: []string{
+				"test",
+			},
+		},
+	}
+	createReq := application.ApplicationCreateRequest{
+		Application: testApp,
+	}
+	app, err := appServer.Create(context.Background(), &createReq)
+	require.NoError(t, err)
+	require.NotNil(t, app)
+	assert.Nil(t, app.Operation)
+}
+
 func TestUpdateApp(t *testing.T) {
 	testApp := newTestApp()
 	appServer := newTestAppServer(t, testApp)