fix(security): repository.GetDetailedProject exposes repo secrets (#24391)

Signed-off-by: Alexander Matyushentsev <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Co-authored-by: Alexander Matyushentsev <[email protected]>

Author: crenshaw-dev

docs/operator-manual/upgrading/2.13-2.14.md

@@ -11,4 +11,12 @@ Eg, `https://github.com/argoproj/argo-cd/manifests/ha/cluster-install?ref=v2.14.
 ## Upgraded Helm Version
 
 Helm was upgraded to 3.16.2 and the skipSchemaValidation Flag was added to
-the [CLI and Application CR](https://argo-cd.readthedocs.io/en/latest/user-guide/helm/#helm-skip-schema-validation). 
+the [CLI and Application CR](https://argo-cd.readthedocs.io/en/latest/user-guide/helm/#helm-skip-schema-validation). 
+
+## Breaking Changes
+
+## Sanitized project API response
+
+Due to security reasons ([GHSA-786q-9hcg-v9ff](https://github.com/argoproj/argo-cd/security/advisories/GHSA-786q-9hcg-v9ff)),
+the project API response was sanitized to remove sensitive information. This includes
+credentials of project-scoped repositories and clusters.

docs/operator-manual/upgrading/2.14-3.0.md

@@ -492,3 +492,9 @@ resource.customizations.ignoreDifferences.apiextensions.k8s.io_CustomResourceDef
 ```
 
 More details for ignored resource updates in the [Diffing customization](../../user-guide/diffing.md) documentation.
+
+### Sanitized project API response
+
+Due to security reasons ([GHSA-786q-9hcg-v9ff](https://github.com/argoproj/argo-cd/security/advisories/GHSA-786q-9hcg-v9ff)),
+the project API response was sanitized to remove sensitive information. This includes
+credentials of project-scoped repositories and clusters.

docs/operator-manual/upgrading/3.0-3.1.md

@@ -55,3 +55,11 @@ Argo CD v3.1 upgrades the bundled Helm version to 3.18.4. There are no breaking
 
 Argo CD v3.1 upgrades the bundled Kustomize version to 5.7.0. There are no breaking changes in Kustomize 5.7 according
 to the [release notes](https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.7.0).
+
+## Breaking Changes
+
+## Sanitized project API response
+
+Due to security reasons ([GHSA-786q-9hcg-v9ff](https://github.com/argoproj/argo-cd/security/advisories/GHSA-786q-9hcg-v9ff)),
+the project API response was sanitized to remove sensitive information. This includes
+credentials of project-scoped repositories and clusters.

pkg/apis/application/v1alpha1/repository_types.go

@@ -347,7 +347,6 @@ func (repo *Repository) Sanitized() *Repository {
 		Repo:                       repo.Repo,
 		Type:                       repo.Type,
 		Name:                       repo.Name,
-		Username:                   repo.Username,
 		Insecure:                   repo.IsInsecure(),
 		EnableLFS:                  repo.EnableLFS,
 		EnableOCI:                  repo.EnableOCI,

pkg/apis/application/v1alpha1/types.go

@@ -2234,6 +2234,32 @@ type Cluster struct {
 	Annotations map[string]string `json:"annotations,omitempty" protobuf:"bytes,13,opt,name=annotations"`
 }
 
+func (c *Cluster) Sanitized() *Cluster {
+	return &Cluster{
+		ID:                 c.ID,
+		Server:             c.Server,
+		Name:               c.Name,
+		Project:            c.Project,
+		Namespaces:         c.Namespaces,
+		Shard:              c.Shard,
+		Labels:             c.Labels,
+		Annotations:        c.Annotations,
+		ClusterResources:   c.ClusterResources,
+		ConnectionState:    c.ConnectionState,
+		ServerVersion:      c.ServerVersion,
+		Info:               c.Info,
+		RefreshRequestedAt: c.RefreshRequestedAt,
+		Config: ClusterConfig{
+			AWSAuthConfig:      c.Config.AWSAuthConfig,
+			ProxyUrl:           c.Config.ProxyUrl,
+			DisableCompression: c.Config.DisableCompression,
+			TLSClientConfig: TLSClientConfig{
+				Insecure: c.Config.Insecure,
+			},
+		},
+	}
+}
+
 // Equals returns true if two cluster objects are considered to be equal
 func (c *Cluster) Equals(other *Cluster) bool {
 	if c.Server != other.Server {

pkg/apis/application/v1alpha1/types_test.go

@@ -4543,3 +4543,58 @@ func TestCluster_ParseProxyUrl(t *testing.T) {
 		}
 	}
 }
+
+func TestSanitized(t *testing.T) {
+	now := metav1.Now()
+	cluster := &Cluster{
+		ID:            "123",
+		Server:        "https://example.com",
+		Name:          "example",
+		ServerVersion: "v1.0.0",
+		Namespaces:    []string{"default", "kube-system"},
+		Project:       "default",
+		Labels: map[string]string{
+			"env": "production",
+		},
+		Annotations: map[string]string{
+			"annotation-key": "annotation-value",
+		},
+		ConnectionState: ConnectionState{
+			Status:     ConnectionStatusSuccessful,
+			Message:    "Connection successful",
+			ModifiedAt: &now,
+		},
+		Config: ClusterConfig{
+			Username:    "admin",
+			Password:    "password123",
+			BearerToken: "abc",
+			TLSClientConfig: TLSClientConfig{
+				Insecure: true,
+			},
+			ExecProviderConfig: &ExecProviderConfig{
+				Command: "test",
+			},
+		},
+	}
+
+	assert.Equal(t, &Cluster{
+		ID:            "123",
+		Server:        "https://example.com",
+		Name:          "example",
+		ServerVersion: "v1.0.0",
+		Namespaces:    []string{"default", "kube-system"},
+		Project:       "default",
+		Labels:        map[string]string{"env": "production"},
+		Annotations:   map[string]string{"annotation-key": "annotation-value"},
+		ConnectionState: ConnectionState{
+			Status:     ConnectionStatusSuccessful,
+			Message:    "Connection successful",
+			ModifiedAt: &now,
+		},
+		Config: ClusterConfig{
+			TLSClientConfig: TLSClientConfig{
+				Insecure: true,
+			},
+		},
+	}, cluster.Sanitized())
+}

server/cluster/cluster.go

@@ -471,19 +471,8 @@ func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*clus
 }
 
 func (s *Server) toAPIResponse(clust *appv1.Cluster) *appv1.Cluster {
+	clust = clust.Sanitized()
 	_ = s.cache.GetClusterInfo(clust.Server, &clust.Info)
-
-	clust.Config.Password = ""
-	clust.Config.BearerToken = ""
-	clust.Config.KeyData = nil
-	if clust.Config.ExecProviderConfig != nil {
-		// We can't know what the user has put into args or
-		// env vars on the exec provider that might be sensitive
-		// (e.g. --private-key=XXX, PASSWORD=XXX)
-		// Implicitly assumes the command executable name is non-sensitive
-		clust.Config.ExecProviderConfig.Env = make(map[string]string)
-		clust.Config.ExecProviderConfig.Args = nil
-	}
 	// populate deprecated fields for backward compatibility
 	//nolint:staticcheck
 	clust.ServerVersion = clust.Info.ServerVersion

server/project/project.go

@@ -310,12 +310,20 @@ func (s *Server) GetDetailedProject(ctx context.Context, q *project.ProjectQuery
 	}
 	proj.NormalizeJWTTokens()
 	globalProjects := argo.GetGlobalProjects(proj, listersv1alpha1.NewAppProjectLister(s.projInformer.GetIndexer()), s.settingsMgr)
+	var apiRepos []*v1alpha1.Repository
+	for _, repo := range repositories {
+		apiRepos = append(apiRepos, repo.Normalize().Sanitized())
+	}
+	var apiClusters []*v1alpha1.Cluster
+	for _, cluster := range clusters {
+		apiClusters = append(apiClusters, cluster.Sanitized())
+	}
 
 	return &project.DetailedProjectsResponse{
 		GlobalProjects: globalProjects,
 		Project:        proj,
-		Repositories:   repositories,
-		Clusters:       clusters,
+		Repositories:   apiRepos,
+		Clusters:       apiClusters,
 	}, err
 }
 

server/repository/repository_test.go

@@ -313,7 +313,7 @@ func TestRepositoryServer(t *testing.T) {
 		testRepo := &appsv1.Repository{
 			Repo:           url,
 			Type:           "git",
-			Username:       "foo",
+			Username:       "",
 			InheritedCreds: true,
 		}
 		db.On("ListRepositories", t.Context()).Return([]*appsv1.Repository{testRepo}, nil)