From 880556e638500b7e6d1e0be4e3b8e6cba63af6b6 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Thu, 4 Sep 2025 11:32:52 -0400 Subject: [PATCH] fix(security): repository.GetDetailedProject exposes repo secrets (#24390) Signed-off-by: Alexander Matyushentsev Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Alexander Matyushentsev --- docs/operator-manual/upgrading/2.13-2.14.md | 10 +++- docs/operator-manual/upgrading/2.14-3.0.md | 7 ++- .../application/v1alpha1/repository_types.go | 1 - pkg/apis/application/v1alpha1/types.go | 26 +++++++++ pkg/apis/application/v1alpha1/types_test.go | 55 +++++++++++++++++++ server/cluster/cluster.go | 13 +---- server/project/project.go | 12 +++- server/repository/repository_test.go | 2 +- 8 files changed, 108 insertions(+), 18 deletions(-) diff --git a/docs/operator-manual/upgrading/2.13-2.14.md b/docs/operator-manual/upgrading/2.13-2.14.md index 3b8a473fb43ec..385dcbc3244e6 100644 --- a/docs/operator-manual/upgrading/2.13-2.14.md +++ b/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). \ No newline at end of file +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. diff --git a/docs/operator-manual/upgrading/2.14-3.0.md b/docs/operator-manual/upgrading/2.14-3.0.md index d8aac5c72afbd..07c92310c92a1 100644 --- a/docs/operator-manual/upgrading/2.14-3.0.md +++ b/docs/operator-manual/upgrading/2.14-3.0.md @@ -437,4 +437,9 @@ data: ignoreResourceStatusField: crd ``` -More details for ignored resource updates in the [Diffing customization](../../user-guide/diffing.md) documentation. \ No newline at end of file +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. \ No newline at end of file diff --git a/pkg/apis/application/v1alpha1/repository_types.go b/pkg/apis/application/v1alpha1/repository_types.go index 373fd2c137142..79a78a8b90021 100644 --- a/pkg/apis/application/v1alpha1/repository_types.go +++ b/pkg/apis/application/v1alpha1/repository_types.go @@ -321,7 +321,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, diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index 5230b23f9e37f..d46b92913280f 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -2154,6 +2154,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 { diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index cdcae94e49f01..3bfd34a989694 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -4507,3 +4507,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()) +} diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go index b78f229a4ddfe..ac060d053edcd 100644 --- a/server/cluster/cluster.go +++ b/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 diff --git a/server/project/project.go b/server/project/project.go index dd3399aaea9d8..7bee5ecb86cb6 100644 --- a/server/project/project.go +++ b/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 } diff --git a/server/repository/repository_test.go b/server/repository/repository_test.go index d6ed9d5cd3fed..f791ca360eaf2 100644 --- a/server/repository/repository_test.go +++ b/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)