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

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

Author: 3 people

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

@@ -3,4 +3,12 @@
 ## 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.

pkg/apis/application/v1alpha1/repository_types.go

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

pkg/apis/application/v1alpha1/types.go

@@ -2099,6 +2099,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

@@ -4348,3 +4348,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.TLSClientConfig.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

@@ -311,12 +311,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

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