CVE-2023-50726: Insufficient Information (NVD-CWE-noinfo)
value: Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes. "Local sync" is an Argo CD feature that allows developers to temporarily override an Application's manifests with locally-defined manifests. Use of the feature should generally be limited to highly-trusted users, since it allows the user to bypass any merge protections in git. An improper validation bug allows users who have `create` privileges but not `override` privileges to sync local manifests on app creation. All other restrictions, including AppProject restrictions are still enforced. The only restriction which is not enforced is that the manifests come from some approved git/Helm/OCI source. The bug was introduced in 1.2.0-rc1 when the local manifest sync feature was added. The bug has been patched in Argo CD versions 2.10.3, 2.9.8, and 2.8.12. Users are advised to upgrade. Users unable to upgrade may mitigate the risk of branch protection bypass by removing `applications, create` RBAC access. The only way to eliminate the issue without removing RBAC access is to upgrade to a patched version.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-13T21:15Z
Repository: https://github.com/argoproj/argo-cd
Language: Go
Files changed: server/application/application.go, server/application/application_test.go
Commit date: 2024-03-13 14:28:43 -0400
Commit message: Merge pull request from GHSA-g623-jcgg-mhmm

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Diff: diff --git a/server/application/application.go b/server/application/application.go
index 8ee16b9349..ec0db45a11 100644
--- a/server/application/application.go
+++ b/server/application/application.go
@@ -331,10 +331,19 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
 
 	if !s.isNamespaceEnabled(appNs) {
 		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")
 		s.waitSync(created)
 		return created, nil
diff --git a/server/application/application_test.go b/server/application/application_test.go
index 65600ad629..51c912ff05 100644
--- a/server/application/application_test.go
+++ b/server/application/application_test.go
@@ -1437,10 +1437,31 @@ func TestCreateAppWithDestName(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NotNil(t, app)
 	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)
 	testApp.Spec.Project = ""
 	app, err := appServer.Update(context.Background(), &application.ApplicationUpdateRequest{

Fixed code: 	// 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
	}

// 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)
}