Bump version to 2.3.5

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

.github/workflows/ci-build.yaml

@@ -12,7 +12,7 @@ on:
 
 env:
   # Golang version to use across CI steps
-  GOLANG_VERSION: '1.17.6'
+  GOLANG_VERSION: '1.17'
 
 jobs:
   check-go:
@@ -61,10 +61,10 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v2
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.38.0
-          args: --timeout 10m --exclude SA5011
+          version: v1.46.2
+          args: --timeout 10m --exclude SA5011 --verbose
 
   test-go:
     name: Run unit tests for Go packages

.github/workflows/image.yaml

@@ -10,7 +10,7 @@ on:
     types: [ labeled, unlabeled, opened, synchronize, reopened ]
 
 env:
-  GOLANG_VERSION: '1.17.6'
+  GOLANG_VERSION: '1.17'
 
 jobs:
   publish:

.github/workflows/release.yaml

@@ -12,7 +12,7 @@ on:
       - '!release-v0*'
 
 env:
-    GOLANG_VERSION: '1.17.6'
+    GOLANG_VERSION: '1.17'
 
 jobs:
   prepare-release:

.golangci.yml

@@ -1,22 +0,0 @@
-run:
-  timeout: 2m
-  skip-files:
-    - ".*\\.pb\\.go"
-  skip-dirs:
-    - pkg/client/
-    - vendor/
-linters:
-  enable:
-    - vet
-    - deadcode
-    - goimports
-    - varcheck
-    - structcheck
-    - ineffassign
-    - unconvert
-    - unparam
-linters-settings:
-  goimports:
-    local-prefixes: github.com/argoproj/argo-cd
-service:
-  golangci-lint-version: 1.21.0

Dockerfile

@@ -4,7 +4,7 @@ ARG BASE_IMAGE=docker.io/library/ubuntu:21.10
 # Initial stage which pulls prepares build dependencies and CLI tooling we need for our final image
 # Also used as the image in CI jobs so needs all dependencies
 ####################################################################################################
-FROM docker.io/library/golang:1.17.6 as builder
+FROM docker.io/library/golang:1.17 as builder
 
 RUN echo 'deb http://deb.debian.org/debian buster-backports main' >> /etc/apt/sources.list
 
@@ -102,7 +102,7 @@ RUN HOST_ARCH='amd64' NODE_ENV='production' NODE_ONLINE_ENV='online' NODE_OPTION
 ####################################################################################################
 # Argo CD Build stage which performs the actual build of Argo CD binaries
 ####################################################################################################
-FROM docker.io/library/golang:1.17.6 as argocd-build
+FROM docker.io/library/golang:1.17 as argocd-build
 
 WORKDIR /go/src/github.com/argoproj/argo-cd
 

VERSION

@@ -1 +1 @@
-2.3.2
+2.3.5

cmd/argocd-repo-server/commands/argocd_repo_server.go

@@ -13,6 +13,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc/health/grpc_health_v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 
 	cmdutil "github.com/argoproj/argo-cd/v2/cmd/util"
 	"github.com/argoproj/argo-cd/v2/common"
@@ -68,14 +69,15 @@ func getSubmoduleEnabled() bool {
 
 func NewCommand() *cobra.Command {
 	var (
-		parallelismLimit       int64
-		listenPort             int
-		metricsPort            int
-		cacheSrc               func() (*reposervercache.Cache, error)
-		tlsConfigCustomizer    tls.ConfigCustomizer
-		tlsConfigCustomizerSrc func() (tls.ConfigCustomizer, error)
-		redisClient            *redis.Client
-		disableTLS             bool
+		parallelismLimit                  int64
+		listenPort                        int
+		metricsPort                       int
+		cacheSrc                          func() (*reposervercache.Cache, error)
+		tlsConfigCustomizer               tls.ConfigCustomizer
+		tlsConfigCustomizerSrc            func() (tls.ConfigCustomizer, error)
+		redisClient                       *redis.Client
+		disableTLS                        bool
+		maxCombinedDirectoryManifestsSize string
 	)
 	var command = cobra.Command{
 		Use:               cliName,
@@ -95,15 +97,19 @@ func NewCommand() *cobra.Command {
 			cache, err := cacheSrc()
 			errors.CheckError(err)
 
+			maxCombinedDirectoryManifestsQuantity, err := resource.ParseQuantity(maxCombinedDirectoryManifestsSize)
+			errors.CheckError(err)
+
 			askPassServer := askpass.NewServer()
 			metricsServer := metrics.NewMetricsServer()
 			cacheutil.CollectMetrics(redisClient, metricsServer)
 			server, err := reposerver.NewServer(metricsServer, cache, tlsConfigCustomizer, repository.RepoServerInitConstants{
-				ParallelismLimit: parallelismLimit,
+				ParallelismLimit:                             parallelismLimit,
 				PauseGenerationAfterFailedGenerationAttempts: getPauseGenerationAfterFailedGenerationAttempts(),
 				PauseGenerationOnFailureForMinutes:           getPauseGenerationOnFailureForMinutes(),
 				PauseGenerationOnFailureForRequests:          getPauseGenerationOnFailureForRequests(),
 				SubmoduleEnabled:                             getSubmoduleEnabled(),
+				MaxCombinedDirectoryManifestsSize:            maxCombinedDirectoryManifestsQuantity,
 			}, askPassServer)
 			errors.CheckError(err)
 
@@ -168,6 +174,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().IntVar(&listenPort, "port", common.DefaultPortRepoServer, "Listen on given port for incoming connections")
 	command.Flags().IntVar(&metricsPort, "metrics-port", common.DefaultPortRepoServerMetrics, "Start metrics server on given port")
 	command.Flags().BoolVar(&disableTLS, "disable-tls", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_DISABLE_TLS", false), "Disable TLS on the gRPC endpoint")
+	command.Flags().StringVar(&maxCombinedDirectoryManifestsSize, "max-combined-directory-manifests-size", env.StringFromEnv("ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE", "10M"), "Max combined size of manifest files in a directory-type Application")
 
 	tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(&command)
 	cacheSrc = reposervercache.AddCacheFlagsToCmd(&command, func(client *redis.Client) {

cmd/argocd/commands/admin/project_allowlist.go

@@ -2,6 +2,7 @@ package admin
 
 import (
 	"bufio"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -63,7 +64,10 @@ func NewProjectAllowListGenCommand() *cobra.Command {
 				}()
 			}
 
-			globalProj := generateProjectAllowList(clientConfig, clusterRoleFileName, projName)
+			resourceList, err := getResourceList(clientConfig)
+			errors.CheckError(err)
+			globalProj, err := generateProjectAllowList(resourceList, clusterRoleFileName, projName)
+			errors.CheckError(err)
 
 			yamlBytes, err := yaml.Marshal(globalProj)
 			errors.CheckError(err)
@@ -78,23 +82,38 @@ func NewProjectAllowListGenCommand() *cobra.Command {
 	return command
 }
 
-func generateProjectAllowList(clientConfig clientcmd.ClientConfig, clusterRoleFileName string, projName string) v1alpha1.AppProject {
+func getResourceList(clientConfig clientcmd.ClientConfig) ([]*metav1.APIResourceList, error) {
+	config, err := clientConfig.ClientConfig()
+	if err != nil {
+		return nil, fmt.Errorf("error while creating client config: %s", err)
+	}
+	disco, err := discovery.NewDiscoveryClientForConfig(config)
+	if err != nil {
+		return nil, fmt.Errorf("error while creating discovery client: %s", err)
+	}
+	serverResources, err := disco.ServerPreferredResources()
+	if err != nil {
+		return nil, fmt.Errorf("error while getting server resources: %s", err)
+	}
+	return serverResources, nil
+}
+
+func generateProjectAllowList(serverResources []*metav1.APIResourceList, clusterRoleFileName string, projName string) (*v1alpha1.AppProject, error) {
 	yamlBytes, err := ioutil.ReadFile(clusterRoleFileName)
-	errors.CheckError(err)
+	if err != nil {
+		return nil, fmt.Errorf("error reading cluster role file: %s", err)
+	}
 	var obj unstructured.Unstructured
 	err = yaml.Unmarshal(yamlBytes, &obj)
-	errors.CheckError(err)
+	if err != nil {
+		return nil, fmt.Errorf("error unmarshalling cluster role file yaml: %s", err)
+	}
 
 	clusterRole := &rbacv1.ClusterRole{}
 	err = scheme.Scheme.Convert(&obj, clusterRole, nil)
-	errors.CheckError(err)
-
-	config, err := clientConfig.ClientConfig()
-	errors.CheckError(err)
-	disco, err := discovery.NewDiscoveryClientForConfig(config)
-	errors.CheckError(err)
-	serverResources, err := disco.ServerPreferredResources()
-	errors.CheckError(err)
+	if err != nil {
+		return nil, fmt.Errorf("error converting cluster role yaml into ClusterRole struct: %s", err)
+	}
 
 	resourceList := make([]metav1.GroupKind, 0)
 	for _, rule := range clusterRole.Rules {
@@ -140,5 +159,5 @@ func generateProjectAllowList(clientConfig clientcmd.ClientConfig, clusterRoleFi
 		Spec:       v1alpha1.AppProjectSpec{},
 	}
 	globalProj.Spec.NamespaceResourceWhitelist = resourceList
-	return globalProj
+	return &globalProj, nil
 }

cmd/argocd/commands/admin/project_allowlist_test.go

@@ -1,57 +1,20 @@
 package admin
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/undefinedlabs/go-mpatch"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/discovery"
-	restclient "k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/clientcmd"
 )
 
 func TestProjectAllowListGen(t *testing.T) {
-	useMock := true
-	rules := clientcmd.NewDefaultClientConfigLoadingRules()
-	overrides := &clientcmd.ConfigOverrides{}
-	clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(rules, overrides)
-
-	if useMock {
-		var patchClientConfig *mpatch.Patch
-		patchClientConfig, err := mpatch.PatchInstanceMethodByName(reflect.TypeOf(clientConfig), "ClientConfig", func(*clientcmd.DeferredLoadingClientConfig) (*restclient.Config, error) {
-			return nil, nil
-		})
-		assert.NoError(t, err)
-
-		patch, err := mpatch.PatchMethod(discovery.NewDiscoveryClientForConfig, func(c *restclient.Config) (*discovery.DiscoveryClient, error) {
-			return &discovery.DiscoveryClient{LegacyPrefix: "/api"}, nil
-		})
-		assert.NoError(t, err)
-
-		var patchSeverPreferredResources *mpatch.Patch
-		discoClient := &discovery.DiscoveryClient{}
-		patchSeverPreferredResources, err = mpatch.PatchInstanceMethodByName(reflect.TypeOf(discoClient), "ServerPreferredResources", func(*discovery.DiscoveryClient) ([]*metav1.APIResourceList, error) {
-			res := metav1.APIResource{
-				Name: "services",
-				Kind: "Service",
-			}
-			resourceList := []*metav1.APIResourceList{{APIResources: []metav1.APIResource{res}}}
-			return resourceList, nil
-		})
-		assert.NoError(t, err)
-
-		defer func() {
-			err = patchClientConfig.Unpatch()
-			assert.NoError(t, err)
-			err = patch.Unpatch()
-			assert.NoError(t, err)
-			err = patchSeverPreferredResources.Unpatch()
-			err = patch.Unpatch()
-		}()
+	res := metav1.APIResource{
+		Name: "services",
+		Kind: "Service",
 	}
+	resourceList := []*metav1.APIResourceList{{APIResources: []metav1.APIResource{res}}}
 
-	globalProj := generateProjectAllowList(clientConfig, "testdata/test_clusterrole.yaml", "testproj")
+	globalProj, err := generateProjectAllowList(resourceList, "testdata/test_clusterrole.yaml", "testproj")
+	assert.NoError(t, err)
 	assert.True(t, len(globalProj.Spec.NamespaceResourceWhitelist) > 0)
 }

cmd/argocd/commands/app.go

@@ -26,6 +26,7 @@ import (
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -776,7 +777,7 @@ func getLocalObjectsString(app *argoappv1.Application, local, localRepoRoot, app
 		ApiVersions:       apiVersions,
 		Plugins:           configManagementPlugins,
 		TrackingMethod:    trackingMethod,
-	}, true, &git.NoopCredsStore{})
+	}, true, &git.NoopCredsStore{}, resource.MustParse("0"))
 	errors.CheckError(err)
 
 	return res.Manifests

cmd/argocd/commands/login.go

@@ -200,7 +200,10 @@ func oauth2Login(ctx context.Context, port int, oidcSettings *settingspkg.OIDCCo
 	// completionChan is to signal flow completed. Non-empty string indicates error
 	completionChan := make(chan string)
 	// stateNonce is an OAuth2 state nonce
-	stateNonce := rand.RandString(10)
+	// According to the spec (https://www.rfc-editor.org/rfc/rfc6749#section-10.10), this must be guessable with
+	// probability <= 2^(-128). The following call generates one of 52^24 random strings, ~= 2^136 possibilities.
+	stateNonce, err := rand.String(24)
+	errors.CheckError(err)
 	var tokenString string
 	var refreshToken string
 
@@ -210,7 +213,8 @@ func oauth2Login(ctx context.Context, port int, oidcSettings *settingspkg.OIDCCo
 	}
 
 	// PKCE implementation of https://tools.ietf.org/html/rfc7636
-	codeVerifier := rand.RandStringCharset(43, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~")
+	codeVerifier, err := rand.StringFromCharset(43, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~")
+	errors.CheckError(err)
 	codeChallengeHash := sha256.Sum256([]byte(codeVerifier))
 	codeChallenge := base64.RawURLEncoding.EncodeToString(codeChallengeHash[:])
 
@@ -294,7 +298,8 @@ func oauth2Login(ctx context.Context, port int, oidcSettings *settingspkg.OIDCCo
 		opts = append(opts, oauth2.SetAuthURLParam("code_challenge_method", "S256"))
 		url = oauth2conf.AuthCodeURL(stateNonce, opts...)
 	case oidcutil.GrantTypeImplicit:
-		url = oidcutil.ImplicitFlowURL(oauth2conf, stateNonce, opts...)
+		url, err = oidcutil.ImplicitFlowURL(oauth2conf, stateNonce, opts...)
+		errors.CheckError(err)
 	default:
 		log.Fatalf("Unsupported grant type: %v", grantType)
 	}

controller/sync.go

@@ -140,7 +140,13 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 	}
 
 	atomic.AddUint64(&syncIdPrefix, 1)
-	syncId := fmt.Sprintf("%05d-%s", syncIdPrefix, rand.RandString(5))
+	randSuffix, err := rand.String(5)
+	if err != nil {
+		state.Phase = common.OperationError
+		state.Message = fmt.Sprintf("Failed generate random sync ID: %v", err)
+		return
+	}
+	syncId := fmt.Sprintf("%05d-%s", syncIdPrefix, randSuffix)
 
 	logEntry := log.WithFields(log.Fields{"application": app.Name, "syncId": syncId})
 	initialResourcesRes := make([]common.ResourceSyncResult, 0)

docs/operator-manual/argocd-cmd-params-cm.yaml

@@ -103,4 +103,8 @@ data:
   reposerver.repo.cache.expiration: "24h0m0s"
   # Cache expiration default (default 24h0m0s)
   reposerver.default.cache.expiration: "24h0m0s"
-  
+  # Max combined manifest file size for a single directory-type Application. In-memory manifest representation may be as
+  # much as 300x the manifest file size. Limit this to stay within the memory limits of the repo-server while allowing
+  # for 300x memory expansion and N Applications running at the same time.
+  # (example 10M max * 300 expansion * 10 Apps = 30G max theoretical memory usage).
+  reposerver.max.combined.directory.manifests.size: '10M'

docs/operator-manual/security.md

@@ -210,4 +210,45 @@ Argo CD logs payloads of most API requests except request that are considered se
 can be found in [server/server.go](https://github.com/argoproj/argo-cd/blob/abba8dddce8cd897ba23320e3715690f465b4a95/server/server.go#L516).
 
 Argo CD does not log IP addresses of clients requesting API endpoints, since the API server is typically behind a proxy. Instead, it is recommended
-to configure IP addresses logging in the proxy server that sits in front of the API server.
+to configure IP addresses logging in the proxy server that sits in front of the API server.
+
+## Limiting Directory App Memory Usage
+
+> >2.2.10, 2.1.16, >2.3.5
+
+Directory-type Applications (those whose source is raw JSON or YAML files) can consume significant
+[repo-server](architecture.md#repository-server) memory, depending on the size and structure of the YAML files.
+
+To avoid over-using memory in the repo-server (potentially causing a crash and denial of service), set the
+`reposerver.max.combined.directory.manifests.size` config option in [argocd-cmd-params-cm](argocd-cmd-params-cm.yaml).
+
+This option limits the combined size of all JSON or YAML files in an individual app. Note that the in-memory
+representation of a manifest may be as much as 300x the size of the manifest on disk. Also note that the limit is per
+Application. If manifests are generated for multiple applications at once, memory usage will be higher.
+
+**Example:**
+
+Suppose your repo-server has a 10G memory limit, and you have ten Applications which use raw JSON or YAML files. To
+calculate the max safe combined file size per Application, divide 10G by 300 * 10 Apps (300 being the worst-case memory
+growth factor for the manifests).
+
+```
+10G / 300 * 10 = 3M
+```
+
+So a reasonably safe configuration for this setup would be a 3M limit per app.
+
+```yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: argocd-cmd-params-cm
+data:
+  reposerver.max.combined.directory.manifests.size: '3M'
+```
+
+The 300x ratio assumes a maliciously-crafted manifest file. If you only want to protect against accidental excessive
+memory use, it is probably safe to use a smaller ratio.
+
+Keep in mind that if a malicious user can create additional Applications, they can increase the total memory usage.
+Grant [App creation privileges](rbac.md) carefully.

docs/operator-manual/server-commands/argocd-repo-server.md

@@ -13,27 +13,28 @@ argocd-repo-server [flags]
 ### Options
 
 ```
-      --default-cache-expiration duration    Cache expiration default (default 24h0m0s)
-      --disable-tls                          Disable TLS on the gRPC endpoint
-  -h, --help                                 help for argocd-repo-server
-      --logformat string                     Set the logging format. One of: text|json (default "text")
-      --loglevel string                      Set the logging level. One of: debug|info|warn|error (default "info")
-      --metrics-port int                     Start metrics server on given port (default 8084)
-      --parallelismlimit int                 Limit on number of concurrent manifests generate requests. Any value less the 1 means no limit.
-      --port int                             Listen on given port for incoming connections (default 8081)
-      --redis string                         Redis server hostname and port (e.g. argocd-redis:6379). 
-      --redis-ca-certificate string          Path to Redis server CA certificate (e.g. /etc/certs/redis/ca.crt). If not specified, system trusted CAs will be used for server certificate validation.
-      --redis-client-certificate string      Path to Redis client certificate (e.g. /etc/certs/redis/client.crt).
-      --redis-client-key string              Path to Redis client key (e.g. /etc/certs/redis/client.crt).
-      --redis-insecure-skip-tls-verify       Skip Redis server certificate validation.
-      --redis-use-tls                        Use TLS when connecting to Redis. 
-      --redisdb int                          Redis database.
-      --repo-cache-expiration duration       Cache expiration for repo state, incl. app lists, app details, manifest generation, revision meta-data (default 24h0m0s)
-      --revision-cache-expiration duration   Cache expiration for cached revision (default 3m0s)
-      --sentinel stringArray                 Redis sentinel hostname and port (e.g. argocd-redis-ha-announce-0:6379). 
-      --sentinelmaster string                Redis sentinel master group name. (default "master")
-      --tlsciphers string                    The list of acceptable ciphers to be used when establishing TLS connections. Use 'list' to list available ciphers. (default "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384:TLS_RSA_WITH_AES_256_GCM_SHA384")
-      --tlsmaxversion string                 The maximum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.3")
-      --tlsminversion string                 The minimum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.2")
+      --default-cache-expiration duration              Cache expiration default (default 24h0m0s)
+      --disable-tls                                    Disable TLS on the gRPC endpoint
+  -h, --help                                           help for argocd-repo-server
+      --logformat string                               Set the logging format. One of: text|json (default "text")
+      --loglevel string                                Set the logging level. One of: debug|info|warn|error (default "info")
+      --max-combined-directory-manifests-size string   Max combined size of manifest files in a directory-type Application (default "10M")
+      --metrics-port int                               Start metrics server on given port (default 8084)
+      --parallelismlimit int                           Limit on number of concurrent manifests generate requests. Any value less the 1 means no limit.
+      --port int                                       Listen on given port for incoming connections (default 8081)
+      --redis string                                   Redis server hostname and port (e.g. argocd-redis:6379). 
+      --redis-ca-certificate string                    Path to Redis server CA certificate (e.g. /etc/certs/redis/ca.crt). If not specified, system trusted CAs will be used for server certificate validation.
+      --redis-client-certificate string                Path to Redis client certificate (e.g. /etc/certs/redis/client.crt).
+      --redis-client-key string                        Path to Redis client key (e.g. /etc/certs/redis/client.crt).
+      --redis-insecure-skip-tls-verify                 Skip Redis server certificate validation.
+      --redis-use-tls                                  Use TLS when connecting to Redis. 
+      --redisdb int                                    Redis database.
+      --repo-cache-expiration duration                 Cache expiration for repo state, incl. app lists, app details, manifest generation, revision meta-data (default 24h0m0s)
+      --revision-cache-expiration duration             Cache expiration for cached revision (default 3m0s)
+      --sentinel stringArray                           Redis sentinel hostname and port (e.g. argocd-redis-ha-announce-0:6379). 
+      --sentinelmaster string                          Redis sentinel master group name. (default "master")
+      --tlsciphers string                              The list of acceptable ciphers to be used when establishing TLS connections. Use 'list' to list available ciphers. (default "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384:TLS_RSA_WITH_AES_256_GCM_SHA384")
+      --tlsmaxversion string                           The maximum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.3")
+      --tlsminversion string                           The minimum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.2")
 ```
 

docs/requirements.txt

@@ -1,4 +1,5 @@
 mkdocs==1.2.3
 mkdocs-material==7.1.7
 markdown_include==0.6.0
-pygments==2.7.4
+pygments==2.7.4
+jinja2===3.0.3

docs/roadmap.md

@@ -1,26 +1,29 @@
 # Roadmap
 
 - [Roadmap](#roadmap)
-  - [v2.3](#v23)
-    - [Merge Argo CD Notifications into Argo CD](#merge-argo-cd-notifications-into-argo-cd)
-    - [Merge ApplicationSet controller into Argo CD](#merge-applicationset-controller-into-argo-cd)
-    - [Compact resources tree](#compact-resources-tree)
-    - [Maintain difference in cluster and git values for specific fields](#maintain-difference-in-cluster-and-git-values-for-specific-fields)
-    - [ARM images and CLI binary](#arm-images-and-cli-binary)
+  - [v2.4](#v24)
     - [Server side apply](#server-side-apply)
-  - [v2.4 and beyond](#v24-and-beyond)
-    - [First class support for ApplicationSet resources](#first-class-support-for-applicationset-resources)
     - [Input Forms UI Refresh](#input-forms-ui-refresh)
-    - [Merge Argo CD Image Updater into Argo CD](#merge-argo-cd-image-updater-into-argo-cd)
     - [Web Shell](#web-shell)
     - [Helm values from external repo](#helm-values-from-external-repo)
+    - [Support multiple sources for an Application](#support-multiple-sources-for-an-application)
+    - [Config Management Tools Enhancements: Parametrization & Security Improvements](#config-management-tools-enhancements-parametrization--security-improvements)
+  - [v2.5 and beyond](#v25-and-beyond)
+    - [Config Management Tools Enhancements: UI/CLI](#config-management-tools-enhancements-uicli)
+    - [First class support for ApplicationSet resources](#first-class-support-for-applicationset-resources)
+    - [Merge Argo CD Image Updater into Argo CD](#merge-argo-cd-image-updater-into-argo-cd)
+    - [Sharding application controller](#sharding-application-controller)
     - [Add support for secrets in Application parameters](#add-support-for-secrets-in-application-parameters)
-    - [Config Management Tools Integrations UI/CLI](#config-management-tools-integrations-uicli)
     - [Allow specifying parent/child relationships in config](#allow-specifying-parentchild-relationships-in-config)
     - [Dependencies between applications](#dependencies-between-applications)
     - [Multi-tenancy improvements](#multi-tenancy-improvements)
     - [GitOps Engine Enhancements](#gitops-engine-enhancements)
   - [Completed](#completed)
+    - [✅ Merge Argo CD Notifications into Argo CD](#-merge-argo-cd-notifications-into-argo-cd)
+    - [✅ Merge ApplicationSet controller into Argo CD](#-merge-applicationset-controller-into-argo-cd)
+    - [✅ Compact resources tree](#-compact-resources-tree)
+    - [✅ Maintain difference in cluster and git values for specific fields](#-maintain-difference-in-cluster-and-git-values-for-specific-fields)
+    - [✅ ARM images and CLI binary](#-arm-images-and-cli-binary)
     - [✅ Config Management Tools Integrations (proposal)](#-config-management-tools-integrations-proposal)
     - [✅ Argo CD Extensions (proposal)](#-argo-cd-extensions-proposal)
     - [✅ Project scoped repository and clusters (proposal)](#-project-scoped-repository-and-clusters-proposal)
@@ -34,49 +37,19 @@
     - [✅ Automated Registry Monitoring](#-automated-registry-monitoring)
     - [✅ Projects Enhancements](#-projects-enhancements)
 
-## v2.3
+## v2.4
 
-> ETA: Feb 2021
-
-### Merge Argo CD Notifications into Argo CD
-
-The [Argo CD Notifications](https://github.com/argoproj-labs/argocd-notifications) should be merged into Argo CD and available out-of-the-box: [#7350](https://github.com/argoproj/argo-cd/issues/7350)
-
-### Merge ApplicationSet controller into Argo CD
-
-The ApplicationSet functionality is available in Argo CD out-of-the-box ([#7351](https://github.com/argoproj/argo-cd/issues/7351)).
-
-### Compact resources tree
-
-An ability to collaps leaf resources tree to improve visualization of very large applications: [#7349](https://github.com/argoproj/argo-cd/issues/7349)
-
-### Maintain difference in cluster and git values for specific fields
-
-The feature allows to avoid updating fields excluded from diffing ([#2913](https://github.com/argoproj/argo-cd/issues/2913)).
-
-### ARM images and CLI binary
-
-The release workflow should build and publish ARM images and CLI binaries: ([#4211](https://github.com/argoproj/argo-cd/issues/4211))
+> ETA: May 2022
 
 ### Server side apply
 
 Support using [server side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) during application syncing
 [#2267](https://github.com/argoproj/argo-cd/issues/2267)
 
-## v2.4 and beyond
-
-### First class support for ApplicationSet resources
-
-The Argo CD UI/CLI/API allows to manage ApplicationSet resources same as Argo CD Applications ([#7352](https://github.com/argoproj/argo-cd/issues/7352)).
-
 ### Input Forms UI Refresh
 
 Improved design of the input forms in Argo CD Web UI: https://www.figma.com/file/IIlsFqqmM5UhqMVul9fQNq/Argo-CD?node-id=0%3A1
 
-### Merge Argo CD Image Updater into Argo CD
-
-The [Argo CD Image Updater](https://github.com/argoproj-labs/argocd-image-updater) should be merged into Argo CD and available out-of-the-box: [#7385](https://github.com/argoproj/argo-cd/issues/7385)
-
 ### Web Shell
 
 Exec into the Kubernetes Pod right from Argo CD Web UI! [#4351](https://github.com/argoproj/argo-cd/issues/4351)
@@ -85,15 +58,40 @@ Exec into the Kubernetes Pod right from Argo CD Web UI! [#4351](https://github.c
 
 The feature allows combining of-the-shelf Helm chart and value file in Git repository ([#2789](https://github.com/argoproj/argo-cd/issues/2789))
 
+### Support multiple sources for an Application
+
+Support more than one source for creating an Application [#8322](https://github.com/argoproj/argo-cd/pull/8322).
+
+### Config Management Tools Enhancements: Parametrization & Security Improvements
+
+The continuation of the Config Management Tools of [proposal](https://github.com/argoproj/argo-cd/blob/master/docs/proposals/parameterized-config-management-plugins.md).
+The Argo config management plugin configuration allows users to specify the accepted parameters, default values to eventually power UI and CLI.
+Additionally, plugins implementation should provide better Argo CD tenant isolation and security.
+
+## v2.5 and beyond
+
+### Config Management Tools Enhancements: UI/CLI
+
+The Argo CD should provide a first-class experience for configured third-party config management tools. User should be able to view supported parameters,
+observe default parameter values and override them.
+
+### First class support for ApplicationSet resources
+
+The Argo CD UI/CLI/API allows to manage ApplicationSet resources same as Argo CD Applications ([#7352](https://github.com/argoproj/argo-cd/issues/7352)).
+
+### Merge Argo CD Image Updater into Argo CD
+
+The [Argo CD Image Updater](https://github.com/argoproj-labs/argocd-image-updater) should be merged into Argo CD and available out-of-the-box: [#7385](https://github.com/argoproj/argo-cd/issues/7385)
+
+
+### Sharding application controller 
+
+Application controller to scale automatically to provide high availability[#8340](https://github.com/argoproj/argo-cd/issues/8340).
+
 ### Add support for secrets in Application parameters
 
 The feature allows referencing secrets in Application parameters. [#1786](https://github.com/argoproj/argo-cd/issues/1786).
 
-### Config Management Tools Integrations UI/CLI
-
-The continuation of the Config Management Tools of [proposal](https://github.com/argoproj/argo-cd/pull/5927). The Argo CD UI/CLI
-should provide first class experience for configured third-party config management tools: [#5734](https://github.com/argoproj/argo-cd/issues/5734).
-
 ### Allow specifying parent/child relationships in config
 
 The feature [#5082](https://github.com/argoproj/argo-cd/issues/5082) allows configuring parent/child relationships between resources. This allows to correctly
@@ -123,13 +121,32 @@ A lot of Argo CD features are still not available in GitOps engine. The followin
 
 ## Completed
 
+### ✅ Merge Argo CD Notifications into Argo CD
+
+The [Argo CD Notifications](https://github.com/argoproj-labs/argocd-notifications) should be merged into Argo CD and available out-of-the-box: [#7350](https://github.com/argoproj/argo-cd/issues/7350)
+
+### ✅ Merge ApplicationSet controller into Argo CD
+
+The ApplicationSet functionality is available in Argo CD out-of-the-box ([#7351](https://github.com/argoproj/argo-cd/issues/7351)).
+
+### ✅ Compact resources tree
+
+An ability to collaps leaf resources tree to improve visualization of very large applications: [#7349](https://github.com/argoproj/argo-cd/issues/7349)
+
+### ✅ Maintain difference in cluster and git values for specific fields
+
+The feature allows to avoid updating fields excluded from diffing ([#2913](https://github.com/argoproj/argo-cd/issues/2913)).
+
+### ✅ ARM images and CLI binary
+
+The release workflow should build and publish ARM images and CLI binaries: ([#4211](https://github.com/argoproj/argo-cd/issues/4211))
+
 ### ✅ Config Management Tools Integrations ([proposal](https://github.com/argoproj/argo-cd/pull/5927))
 
 The community likes the first class support of Helm, Kustomize and keeps requesting support for more tools.
 Argo CD provides a mechanism to integrate with any config management tool. We need to investigate why
 it is not enough and implement missing features.
 
-
 ### ✅ Argo CD Extensions ([proposal](https://github.com/argoproj/argo-cd/pull/6240))
 
 Argo CD supports customizing handling of Kubernetes resources via diffing customizations,

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d
 	github.com/alicebob/miniredis v2.5.0+incompatible
 	github.com/alicebob/miniredis/v2 v2.14.2
-	github.com/argoproj/gitops-engine v0.6.1
+	github.com/argoproj/gitops-engine v0.6.2
 	github.com/argoproj/notifications-engine v0.3.1-0.20220127183449-91deed20b998
 	github.com/argoproj/pkg v0.11.1-0.20211203175135-36c59d8fafe0
 	github.com/bombsimon/logrusr/v2 v2.0.1
@@ -64,7 +64,6 @@ require (
 	github.com/spf13/cobra v1.2.1
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.0
-	github.com/undefinedlabs/go-mpatch v1.0.6
 	github.com/whilp/git-urls v0.0.0-20191001220047-6db9661140c0
 	github.com/yuin/gopher-lua v0.0.0-20200816102855-ee81675732da
 	golang.org/x/crypto v0.0.0-20210817164053-32db794688a5

go.sum

@@ -125,8 +125,8 @@ github.com/antonmedv/expr v1.8.9/go.mod h1:5qsM3oLGDND7sDmQGDXHkYfkjYMUX14qsgqmH
 github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
 github.com/appscode/go v0.0.0-20190808133642-1d4ef1f1c1e0/go.mod h1:iy07dV61Z7QQdCKJCIvUoDL21u6AIceRhZzyleh2ymc=
-github.com/argoproj/gitops-engine v0.6.1 h1:fiaMQ+0OfBHQWInekgkO645i4dolvl3KA//0F7n4PK8=
-github.com/argoproj/gitops-engine v0.6.1/go.mod h1:pRgVpLW7pZqf7n3COJ7UcDepk4cI61LAcJd64Q3Jq/c=
+github.com/argoproj/gitops-engine v0.6.2 h1:hM+pQeplCeIPAvfAmr1f91+ykxqaU0GAzuxVujqlKHM=
+github.com/argoproj/gitops-engine v0.6.2/go.mod h1:pRgVpLW7pZqf7n3COJ7UcDepk4cI61LAcJd64Q3Jq/c=
 github.com/argoproj/notifications-engine v0.3.1-0.20220127183449-91deed20b998 h1:V9RDg+IZeebnm3XjkfkbN07VM21Fu1Cy/RJNoHO++VM=
 github.com/argoproj/notifications-engine v0.3.1-0.20220127183449-91deed20b998/go.mod h1:5mKv7zEgI3NO0L+fsuRSwBSY9EIXSuyIsDND8O8TTIw=
 github.com/argoproj/pkg v0.11.1-0.20211203175135-36c59d8fafe0 h1:Cfp7rO/HpVxnwlRqJe0jHiBbZ77ZgXhB6HWlYD02Xdc=
@@ -963,8 +963,6 @@ github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802/go.mod h1
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
 github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw=
 github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY=
-github.com/undefinedlabs/go-mpatch v1.0.6 h1:h8q5ORH/GaOE1Se1DMhrOyljXZEhRcROO7agMqWXCOY=
-github.com/undefinedlabs/go-mpatch v1.0.6/go.mod h1:TyJZDQ/5AgyN7FSLiBJ8RO9u2c6wbtRvK827b6AVqY4=
 github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
 github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=

hack/installers/install-lint-tools.sh

@@ -1,4 +1,4 @@
 #!/bin/bash
 set -eux -o pipefail
 
-GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.38.0
+GO111MODULE=on go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.2

manifests/base/kustomization.yaml

@@ -5,7 +5,7 @@ kind: Kustomization
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.3.2
+  newTag: v2.3.5
 resources:
 - ./application-controller
 - ./dex

manifests/base/repo-server/argocd-repo-server-deployment.yaml

@@ -98,6 +98,12 @@ spec:
                   name: argocd-cmd-params-cm
                   key: reposerver.default.cache.expiration
                   optional: true
+          - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+            valueFrom:
+              configMapKeyRef:
+                name: argocd-cmd-params-cm
+                key: reposerver.max.combined.directory.manifests.size
+                optional: true
           - name: HELM_CACHE_HOME
             value: /helm-working-dir
           - name: HELM_CONFIG_HOME

manifests/core-install.yaml

@@ -9686,13 +9686,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -9741,7 +9747,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -9906,7 +9912,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/core-install/kustomization.yaml

@@ -11,4 +11,4 @@ resources:
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.3.2
+  newTag: v2.3.5

manifests/ha/base/kustomization.yaml

@@ -11,7 +11,7 @@ patchesStrategicMerge:
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.3.2
+  newTag: v2.3.5
 resources:
 - ../../base/application-controller
 - ../../base/dex

manifests/ha/install.yaml

@@ -10516,7 +10516,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -10549,7 +10549,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -10776,13 +10776,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -10831,7 +10837,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -11058,7 +11064,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -11254,7 +11260,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/ha/namespace-install.yaml

@@ -7812,7 +7812,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -7845,7 +7845,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -8072,13 +8072,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -8127,7 +8133,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -8354,7 +8360,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -8550,7 +8556,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/install.yaml

@@ -9886,7 +9886,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -9919,7 +9919,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -10110,13 +10110,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -10165,7 +10171,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -10388,7 +10394,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -10578,7 +10584,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/namespace-install.yaml

@@ -7182,7 +7182,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -7215,7 +7215,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -7406,13 +7406,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -7461,7 +7467,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -7684,7 +7690,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -7874,7 +7880,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.3.2
+        image: quay.io/argoproj/argocd:v2.3.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

pkg/apiclient/grpcproxy.go

@@ -100,7 +100,11 @@ func (c *client) executeRequest(fullMethodName string, msg []byte, md metadata.M
 }
 
 func (c *client) startGRPCProxy() (*grpc.Server, net.Listener, error) {
-	serverAddr := fmt.Sprintf("%s/argocd-%s.sock", os.TempDir(), rand.RandString(16))
+	randSuffix, err := rand.String(16)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to generate random socket filename: %w", err)
+	}
+	serverAddr := fmt.Sprintf("%s/argocd-%s.sock", os.TempDir(), randSuffix)
 	ln, err := net.Listen("unix", serverAddr)
 
 	if err != nil {

pkg/apis/application/v1alpha1/cluster_constants.go

@@ -48,17 +48,17 @@ var (
 	K8sMaxIdleConnections = env.ParseNumFromEnv(EnvK8sClientMaxIdleConnections, 500, 0, math.MaxInt32)
 
 	// K8sTLSHandshakeTimeout defines the maximum duration to wait for a TLS handshake to complete
-	K8sTLSHandshakeTimeout = env.ParseDurationFromEnv(EnvK8sTLSHandshakeTimeout, 10*time.Second, 0, math.MaxInt32)
+	K8sTLSHandshakeTimeout = env.ParseDurationFromEnv(EnvK8sTLSHandshakeTimeout, 10*time.Second, 0, math.MaxInt32*time.Second)
 
 	// K8sTCPTimeout defines the TCP timeout to use when performing K8s API requests
-	K8sTCPTimeout = env.ParseDurationFromEnv(EnvK8sTCPTimeout, 30*time.Second, 0, math.MaxInt32)
+	K8sTCPTimeout = env.ParseDurationFromEnv(EnvK8sTCPTimeout, 30*time.Second, 0, math.MaxInt32*time.Second)
 
 	// K8sTCPKeepAlive defines the interval for sending TCP keep alive to K8s API server
-	K8sTCPKeepAlive = env.ParseDurationFromEnv(EnvK8sTCPKeepAlive, 30*time.Second, 0, math.MaxInt32)
+	K8sTCPKeepAlive = env.ParseDurationFromEnv(EnvK8sTCPKeepAlive, 30*time.Second, 0, math.MaxInt32*time.Second)
 
 	// K8sTCPIdleConnTimeout defines the duration for keeping idle TCP connections to the K8s API server
-	K8sTCPIdleConnTimeout = env.ParseDurationFromEnv(EnvK8sTCPIdleConnTimeout, 5*time.Minute, 0, math.MaxInt32)
+	K8sTCPIdleConnTimeout = env.ParseDurationFromEnv(EnvK8sTCPIdleConnTimeout, 5*time.Minute, 0, math.MaxInt32*time.Second)
 
 	// K8sServerSideTimeout defines which server side timeout to send with each API request
-	K8sServerSideTimeout = env.ParseDurationFromEnv(EnvK8sTCPTimeout, 32*time.Second, 0, math.MaxInt32)
+	K8sServerSideTimeout = env.ParseDurationFromEnv(EnvK8sTCPTimeout, 0, 0, math.MaxInt32*time.Second)
 )

pkg/apis/application/v1alpha1/types.go

@@ -2477,6 +2477,8 @@ func (c *Cluster) RawRestConfig() *rest.Config {
 		panic(fmt.Sprintf("Unable to create K8s REST config: %v", err))
 	}
 	config.Timeout = K8sServerSideTimeout
+	config.QPS = K8sClientConfigQPS
+	config.Burst = K8sClientConfigBurst
 	return config
 }
 

reposerver/repository/repository.go

@@ -18,6 +18,12 @@ import (
 	"strings"
 	"time"
 
+	kubeyaml "k8s.io/apimachinery/pkg/util/yaml"
+
+	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/argoproj/argo-cd/v2/util/io/files"
+
 	"github.com/Masterminds/semver/v3"
 	"github.com/TomOnTime/utfutil"
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -69,6 +75,8 @@ const (
 	ociPrefix                      = "oci://"
 )
 
+var ErrExceededMaxCombinedManifestFileSize = errors.New("exceeded max combined manifest file size")
+
 // Service implements ManifestService interface
 type Service struct {
 	gitCredsStore             git.CredsStore
@@ -94,6 +102,7 @@ type RepoServerInitConstants struct {
 	PauseGenerationOnFailureForMinutes           int
 	PauseGenerationOnFailureForRequests          int
 	SubmoduleEnabled                             bool
+	MaxCombinedDirectoryManifestsSize            resource.Quantity
 }
 
 // NewService returns a new instance of the Manifest service
@@ -383,7 +392,7 @@ func (s *Service) runManifestGen(ctx context.Context, repoRoot, commitSHA, cache
 	var manifestGenResult *apiclient.ManifestResponse
 	opContext, err := opContextSrc()
 	if err == nil {
-		manifestGenResult, err = GenerateManifests(ctx, opContext.appPath, repoRoot, commitSHA, q, false, s.gitCredsStore)
+		manifestGenResult, err = GenerateManifests(ctx, opContext.appPath, repoRoot, commitSHA, q, false, s.gitCredsStore, s.initConstants.MaxCombinedDirectoryManifestsSize)
 	}
 	if err != nil {
 
@@ -769,7 +778,7 @@ func getRepoCredential(repoCredentials []*v1alpha1.RepoCreds, repoURL string) *v
 }
 
 // GenerateManifests generates manifests from a path
-func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool, gitCredsStore git.CredsStore) (*apiclient.ManifestResponse, error) {
+func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool, gitCredsStore git.CredsStore, maxCombinedManifestQuantity resource.Quantity) (*apiclient.ManifestResponse, error) {
 	var targetObjs []*unstructured.Unstructured
 	var dest *v1alpha1.ApplicationDestination
 
@@ -810,7 +819,8 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,
 		if directory = q.ApplicationSource.Directory; directory == nil {
 			directory = &v1alpha1.ApplicationSourceDirectory{}
 		}
-		targetObjs, err = findManifests(appPath, repoRoot, env, *directory, q.EnabledSourceTypes)
+		logCtx := log.WithField("application", q.AppName)
+		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory, q.EnabledSourceTypes, maxCombinedManifestQuantity)
 	}
 	if err != nil {
 		return nil, err
@@ -1012,47 +1022,30 @@ func ksShow(appLabelKey, appPath string, ksonnetOpts *v1alpha1.ApplicationSource
 var manifestFile = regexp.MustCompile(`^.*\.(yaml|yml|json|jsonnet)$`)
 
 // findManifests looks at all yaml files in a directory and unmarshals them into a list of unstructured objects
-func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, enabledManifestGeneration map[string]bool) ([]*unstructured.Unstructured, error) {
+func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, enabledManifestGeneration map[string]bool, maxCombinedManifestQuantity resource.Quantity) ([]*unstructured.Unstructured, error) {
+	// Validate the directory before loading any manifests to save memory.
+	potentiallyValidManifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, directory.Recurse, directory.Include, directory.Exclude, maxCombinedManifestQuantity)
+	if err != nil {
+		logCtx.Errorf("failed to get potentially valid manifests: %s", err)
+		return nil, fmt.Errorf("failed to get potentially valid manifests: %w", err)
+	}
+
 	var objs []*unstructured.Unstructured
-	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if f.IsDir() {
-			if path != appPath && !directory.Recurse {
-				return filepath.SkipDir
-			} else {
-				return nil
-			}
-		}
+	for _, potentiallyValidManifest := range potentiallyValidManifests {
+		manifestPath := potentiallyValidManifest.path
+		manifestFileInfo := potentiallyValidManifest.fileInfo
 
-		if !manifestFile.MatchString(f.Name()) {
-			return nil
-		}
-
-		relPath, err := filepath.Rel(appPath, path)
-		if err != nil {
-			return err
-		}
-		if directory.Exclude != "" && glob.Match(directory.Exclude, relPath) {
-			return nil
-		}
-
-		if directory.Include != "" && !glob.Match(directory.Include, relPath) {
-			return nil
-		}
-
-		if strings.HasSuffix(f.Name(), ".jsonnet") {
+		if strings.HasSuffix(manifestFileInfo.Name(), ".jsonnet") {
 			if !discovery.IsManifestGenerationEnabled(v1alpha1.ApplicationSourceTypeDirectory, enabledManifestGeneration) {
-				return nil
+				continue
 			}
 			vm, err := makeJsonnetVm(appPath, repoRoot, directory.Jsonnet, env)
 			if err != nil {
-				return err
+				return nil, err
 			}
-			jsonStr, err := vm.EvaluateFile(path)
+			jsonStr, err := vm.EvaluateFile(manifestPath)
 			if err != nil {
-				return status.Errorf(codes.FailedPrecondition, "Failed to evaluate jsonnet %q: %v", f.Name(), err)
+				return nil, status.Errorf(codes.FailedPrecondition, "Failed to evaluate jsonnet %q: %v", manifestFileInfo.Name(), err)
 			}
 
 			// attempt to unmarshal either array or single object
@@ -1064,49 +1057,207 @@ func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory
 				var jsonObj unstructured.Unstructured
 				err = json.Unmarshal([]byte(jsonStr), &jsonObj)
 				if err != nil {
-					return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal generated json %q: %v", f.Name(), err)
+					return nil, status.Errorf(codes.FailedPrecondition, "Failed to unmarshal generated json %q: %v", manifestFileInfo.Name(), err)
 				}
 				objs = append(objs, &jsonObj)
 			}
 		} else {
-			out, err := utfutil.ReadFile(path, utfutil.UTF8)
+			err := getObjsFromYAMLOrJson(logCtx, manifestPath, manifestFileInfo.Name(), &objs)
 			if err != nil {
-				return err
-			}
-			if strings.HasSuffix(f.Name(), ".json") {
-				var obj unstructured.Unstructured
-				err = json.Unmarshal(out, &obj)
-				if err != nil {
-					return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", f.Name(), err)
-				}
-				objs = append(objs, &obj)
-			} else {
-				yamlObjs, err := kube.SplitYAML(out)
-				if err != nil {
-					if len(yamlObjs) > 0 {
-						// If we get here, we had a multiple objects in a single YAML file which had some
-						// valid k8s objects, but errors parsing others (within the same file). It's very
-						// likely the user messed up a portion of the YAML, so report on that.
-						return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", f.Name(), err)
-					}
-					// Otherwise, let's see if it looks like a resource, if yes, we return error
-					if bytes.Contains(out, []byte("apiVersion:")) &&
-						bytes.Contains(out, []byte("kind:")) &&
-						bytes.Contains(out, []byte("metadata:")) {
-						return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", f.Name(), err)
-					}
-					// Otherwise, it might be a unrelated YAML file which we will ignore
-					return nil
-				}
-				objs = append(objs, yamlObjs...)
+				return nil, err
 			}
 		}
+	}
+	return objs, nil
+}
+
+// getObjsFromYAMLOrJson unmarshals the given yaml or json file and appends it to the given list of objects.
+func getObjsFromYAMLOrJson(logCtx *log.Entry, manifestPath string, filename string, objs *[]*unstructured.Unstructured) error {
+	reader, err := utfutil.OpenFile(manifestPath, utfutil.UTF8)
+	if err != nil {
+		return status.Errorf(codes.FailedPrecondition, "Failed to open %q", manifestPath)
+	}
+	defer func() {
+		err := reader.Close()
+		if err != nil {
+			logCtx.Errorf("failed to close %q - potential memory leak", manifestPath)
+		}
+	}()
+	if strings.HasSuffix(filename, ".json") {
+		var obj unstructured.Unstructured
+		decoder := json.NewDecoder(reader)
+		err = decoder.Decode(&obj)
+		if err != nil {
+			return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", filename, err)
+		}
+		if decoder.More() {
+			return status.Errorf(codes.FailedPrecondition, "Found multiple objects in %q. Only single objects are allowed in JSON files.", filename)
+		}
+		*objs = append(*objs, &obj)
+	} else {
+		yamlObjs, err := splitYAMLOrJSON(reader)
+		if err != nil {
+			if len(yamlObjs) > 0 {
+				// If we get here, we had a multiple objects in a single YAML file which had some
+				// valid k8s objects, but errors parsing others (within the same file). It's very
+				// likely the user messed up a portion of the YAML, so report on that.
+				return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", filename, err)
+			}
+			// Read the whole file to check whether it looks like a manifest.
+			out, err := utfutil.ReadFile(manifestPath, utfutil.UTF8)
+			// Otherwise, let's see if it looks like a resource, if yes, we return error
+			if bytes.Contains(out, []byte("apiVersion:")) &&
+				bytes.Contains(out, []byte("kind:")) &&
+				bytes.Contains(out, []byte("metadata:")) {
+				return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", filename, err)
+			}
+			// Otherwise, it might be an unrelated YAML file which we will ignore
+		}
+		*objs = append(*objs, yamlObjs...)
+	}
+	return nil
+}
+
+// splitYAMLOrJSON reads a YAML or JSON file and gets each document as an unstructured object. If the unmarshaller
+// encounters an error, objects read up until the error are returned.
+func splitYAMLOrJSON(reader goio.Reader) ([]*unstructured.Unstructured, error) {
+	d := kubeyaml.NewYAMLOrJSONDecoder(reader, 4096)
+	var objs []*unstructured.Unstructured
+	for {
+		u := &unstructured.Unstructured{}
+		if err := d.Decode(&u); err != nil {
+			if err == goio.EOF {
+				break
+			}
+			return objs, fmt.Errorf("failed to unmarshal manifest: %v", err)
+		}
+		if u == nil {
+			continue
+		}
+		objs = append(objs, u)
+	}
+	return objs, nil
+}
+
+// getPotentiallyValidManifestFile checks whether the given path/FileInfo may be a valid manifest file. Returns a non-nil error if
+// there was an error that should not be handled by ignoring the file. Returns non-nil realFileInfo if the file is a
+// potential manifest. Returns a non-empty ignoreMessage if there's a message that should be logged about why the file
+// was skipped. If realFileInfo is nil and the ignoreMessage is empty, there's no need to log the ignoreMessage; the
+// file was skipped for a mundane reason.
+//
+// The file is still only a "potentially" valid manifest file because it could be invalid JSON or YAML, or it might not
+// be a valid Kubernetes resource. This function tests everything possible without actually reading the file.
+//
+// repoPath must be absolute.
+func getPotentiallyValidManifestFile(path string, f os.FileInfo, appPath, repoRoot, include, exclude string) (realFileInfo os.FileInfo, warning string, err error) {
+	relPath, err := filepath.Rel(appPath, path)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to get relative path of %q: %w", path, err)
+	}
+
+	if !manifestFile.MatchString(f.Name()) {
+		return nil, "", nil
+	}
+
+	// If the file is a symlink, these will be overridden with the destination file's info.
+	var relRealPath = relPath
+	realFileInfo = f
+
+	if files.IsSymlink(f) {
+		realPath, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			if os.IsNotExist(err) {
+				return nil, fmt.Sprintf("destination of symlink %q is missing", relPath), nil
+			}
+			return nil, "", fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
+		}
+		if !files.Inbound(realPath, repoRoot) {
+			return nil, "", fmt.Errorf("illegal filepath in symlink at %q", relPath)
+		}
+		realFileInfo, err = os.Stat(realPath)
+		if err != nil {
+			if os.IsNotExist(err) {
+				// This should have been caught by filepath.EvalSymlinks, but check again since that function's docs
+				// don't promise to return this error.
+				return nil, fmt.Sprintf("destination of symlink %q is missing at %q", relPath, realPath), nil
+			}
+			return nil, "", fmt.Errorf("failed to get file info for symlink at %q to %q: %w", relPath, realPath, err)
+		}
+		relRealPath, err = filepath.Rel(repoRoot, realPath)
+		if err != nil {
+			return nil, "", fmt.Errorf("failed to get relative path of %q: %w", realPath, err)
+		}
+	}
+
+	// FileInfo.Size() behavior is platform-specific for non-regular files. Allow only regular files, so we guarantee
+	// accurate file sizes.
+	if !realFileInfo.Mode().IsRegular() {
+		return nil, fmt.Sprintf("ignoring symlink at %q to non-regular file %q", relPath, relRealPath), nil
+	}
+
+	if exclude != "" && glob.Match(exclude, relPath) {
+		return nil, "", nil
+	}
+
+	if include != "" && !glob.Match(include, relPath) {
+		return nil, "", nil
+	}
+
+	return realFileInfo, "", nil
+}
+
+type potentiallyValidManifest struct {
+	path     string
+	fileInfo os.FileInfo
+}
+
+// getPotentiallyValidManifests ensures that 1) there are no errors while checking for potential manifest files in the given dir
+// and 2) the combined file size of the potentially-valid manifest files does not exceed the limit.
+func getPotentiallyValidManifests(logCtx *log.Entry, appPath string, repoRoot string, recurse bool, include string, exclude string, maxCombinedManifestQuantity resource.Quantity) ([]potentiallyValidManifest, error) {
+	maxCombinedManifestFileSize := maxCombinedManifestQuantity.Value()
+	var currentCombinedManifestFileSize = int64(0)
+
+	var potentiallyValidManifests []potentiallyValidManifest
+	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if f.IsDir() {
+			if path != appPath && !recurse {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+
+		realFileInfo, warning, err := getPotentiallyValidManifestFile(path, f, appPath, repoRoot, include, exclude)
+		if err != nil {
+			return fmt.Errorf("invalid manifest file %q: %w", path, err)
+		}
+		if realFileInfo == nil {
+			if warning != "" {
+				logCtx.Warnf("skipping manifest file %q: %s", path, warning)
+			}
+			return nil
+		}
+		// Don't count jsonnet file size against max. It's jsonnet's responsibility to manage memory usage.
+		if !strings.HasSuffix(f.Name(), ".jsonnet") {
+			// We use the realFileInfo size (which is guaranteed to be a regular file instead of a symlink or other
+			// non-regular file) because .Size() behavior is platform-specific for non-regular files.
+			currentCombinedManifestFileSize += realFileInfo.Size()
+			if maxCombinedManifestFileSize != 0 && currentCombinedManifestFileSize > maxCombinedManifestFileSize {
+				return ErrExceededMaxCombinedManifestFileSize
+			}
+		}
+		potentiallyValidManifests = append(potentiallyValidManifests, potentiallyValidManifest{path: path, fileInfo: f})
 		return nil
 	})
 	if err != nil {
+		// Not wrapping, because this error should be wrapped by the caller.
 		return nil, err
 	}
-	return objs, nil
+
+	return potentiallyValidManifests, nil
 }
 
 func makeJsonnetVm(appPath string, repoRoot string, sourceJsonnet v1alpha1.ApplicationSourceJsonnet, env *v1alpha1.Env) (*jsonnet.VM, error) {
@@ -1412,8 +1563,12 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
 		return err
 	}
 
-	if err := loadFileIntoIfExists(filepath.Join(appPath, "values.yaml"), &res.Helm.Values); err != nil {
-		return err
+	if resolvedValuesPath, _, err := pathutil.ResolveFilePath(appPath, repoRoot, "values.yaml", []string{}); err == nil {
+		if err := loadFileIntoIfExists(resolvedValuesPath, &res.Helm.Values); err != nil {
+			return err
+		}
+	} else {
+		log.Warnf("Values file %s is not allowed: %v", filepath.Join(appPath, "values.yaml"), err)
 	}
 	var resolvedSelectedValueFiles []pathutil.ResolvedFilePath
 	// drop not allowed values files
@@ -1421,10 +1576,10 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
 		if resolvedFile, _, err := pathutil.ResolveFilePath(appPath, repoRoot, file, q.GetValuesFileSchemes()); err == nil {
 			resolvedSelectedValueFiles = append(resolvedSelectedValueFiles, resolvedFile)
 		} else {
-			log.Debugf("Values file %s is not allowed: %v", file, err)
+			log.Warnf("Values file %s is not allowed: %v", file, err)
 		}
 	}
-	params, err := h.GetParameters(resolvedSelectedValueFiles)
+	params, err := h.GetParameters(resolvedSelectedValueFiles, appPath, repoRoot)
 	if err != nil {
 		return err
 	}
@@ -1443,15 +1598,16 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
 	return nil
 }
 
-func loadFileIntoIfExists(path string, destination *string) error {
-	info, err := os.Stat(path)
+func loadFileIntoIfExists(path pathutil.ResolvedFilePath, destination *string) error {
+	stringPath := string(path)
+	info, err := os.Stat(stringPath)
 
 	if err == nil && !info.IsDir() {
-		if bytes, err := ioutil.ReadFile(path); err != nil {
-			*destination = string(bytes)
-		} else {
+		bytes, err := ioutil.ReadFile(stringPath);
+		if err != nil {
 			return err
 		}
+		*destination = string(bytes)
 	}
 
 	return nil
@@ -1660,33 +1816,40 @@ func directoryPermissionInitializer(rootPath string) goio.Closer {
 // nolint:unparam
 func (s *Service) checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool) (goio.Closer, error) {
 	closer := s.gitRepoInitializer(gitClient.Root())
+	return closer, checkoutRevision(gitClient, revision, submoduleEnabled)
+}
+
+func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bool) error {
 	err := gitClient.Init()
 	if err != nil {
-		return closer, status.Errorf(codes.Internal, "Failed to initialize git repo: %v", err)
+		return status.Errorf(codes.Internal, "Failed to initialize git repo: %v", err)
 	}
 
-	err = gitClient.Fetch(revision)
-
+	// Fetching with no revision first. Fetching with an explicit version can cause repo bloat. https://github.com/argoproj/argo-cd/issues/8845
+	err = gitClient.Fetch("")
 	if err != nil {
-		log.Infof("Failed to fetch revision %s: %v", revision, err)
-		log.Infof("Fallback to fetch default")
-		err = gitClient.Fetch("")
-		if err != nil {
-			return closer, status.Errorf(codes.Internal, "Failed to fetch default: %v", err)
-		}
-		err = gitClient.Checkout(revision, submoduleEnabled)
-		if err != nil {
-			return closer, status.Errorf(codes.Internal, "Failed to checkout revision %s: %v", revision, err)
-		}
-		return closer, err
+		return status.Errorf(codes.Internal, "Failed to fetch default: %v", err)
 	}
 
-	err = gitClient.Checkout("FETCH_HEAD", submoduleEnabled)
+	err = gitClient.Checkout(revision, submoduleEnabled)
 	if err != nil {
-		return closer, status.Errorf(codes.Internal, "Failed to checkout FETCH_HEAD: %v", err)
+		// When fetching with no revision, only refs/heads/* and refs/remotes/origin/* are fetched. If checkout fails
+		// for the given revision, try explicitly fetching it.
+		log.Infof("Failed to checkout revision %s: %v", revision, err)
+		log.Infof("Fallback to fetching specific revision %s. ref might not have been in the default refspec fetched.", revision)
+
+		err = gitClient.Fetch(revision)
+		if err != nil {
+			return status.Errorf(codes.Internal, "Failed to checkout revision %s: %v", revision, err)
+		}
+
+		err = gitClient.Checkout("FETCH_HEAD", submoduleEnabled)
+		if err != nil {
+			return status.Errorf(codes.Internal, "Failed to checkout FETCH_HEAD: %v", err)
+		}
 	}
 
-	return closer, err
+	return err
 }
 
 func (s *Service) GetHelmCharts(ctx context.Context, q *apiclient.HelmChartsRequest) (*apiclient.HelmChartsResponse, error) {

reposerver/repository/repository_norace_test.go

@@ -1,47 +0,0 @@
-//go:build !race
-// +build !race
-
-package repository
-
-import (
-	"os"
-	"path/filepath"
-	"sync"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-
-	argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
-	"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
-)
-
-func TestHelmDependencyWithConcurrency(t *testing.T) {
-
-	// !race:
-	// Un-synchronized use of a random source, will be fixed when this is merged:
-	// https://github.com/argoproj/argo-cd/issues/4728
-
-	cleanup := func() {
-		_ = os.Remove(filepath.Join("../../util/helm/testdata/helm2-dependency", helmDepUpMarkerFile))
-		_ = os.RemoveAll(filepath.Join("../../util/helm/testdata/helm2-dependency", "charts"))
-	}
-	cleanup()
-	defer cleanup()
-
-	helmRepo := argoappv1.Repository{Name: "bitnami", Type: "helm", Repo: "https://charts.bitnami.com/bitnami"}
-	var wg sync.WaitGroup
-	wg.Add(3)
-	for i := 0; i < 3; i++ {
-		go func() {
-			res, err := helmTemplate("../../util/helm/testdata/helm2-dependency", "../..", nil, &apiclient.ManifestRequest{
-				ApplicationSource: &argoappv1.ApplicationSource{},
-				Repos:             []*argoappv1.Repository{&helmRepo},
-			}, false)
-
-			assert.NoError(t, err)
-			assert.NotNil(t, res)
-			wg.Done()
-		}()
-	}
-	wg.Wait()
-}

reposerver/repository/repository_test.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	goio "io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -16,6 +17,9 @@ import (
 	"testing"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/api/resource"
+
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -144,11 +148,81 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	assert.Equal(t, countOfManifests, len(res1.Manifests))
 
 	// this will test concatenated manifests to verify we split YAMLs correctly
-	res2, err := GenerateManifests(context.Background(), "./testdata/concatenated", "/", "", &q, false, &git.NoopCredsStore{})
+	res2, err := GenerateManifests(context.Background(), "./testdata/concatenated", "/", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
 	assert.NoError(t, err)
 	assert.Equal(t, 3, len(res2.Manifests))
 }
 
+func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
+	testCases := []struct {
+		name                    string
+		outOfBoundsFilename     string
+		outOfBoundsFileContents string
+		mustNotContain          string // Optional string that must not appear in error or manifest output. If empty, use outOfBoundsFileContents.
+	}{
+		{
+			name:                    "out of bounds JSON file should not appear in error output",
+			outOfBoundsFilename:     "test.json",
+			outOfBoundsFileContents: `{"some": "json"}`,
+		},
+		{
+			name:                    "malformed JSON file contents should not appear in error output",
+			outOfBoundsFilename:     "test.json",
+			outOfBoundsFileContents: "$",
+		},
+		{
+			name:                "out of bounds JSON manifest should not appear in manifest output",
+			outOfBoundsFilename: "test.json",
+			// JSON marshalling is deterministic. So if there's a leak, exactly this should appear in the manifests.
+			outOfBoundsFileContents: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+		},
+		{
+			name:                    "out of bounds YAML manifest should not appear in manifest output",
+			outOfBoundsFilename:     "test.yaml",
+			outOfBoundsFileContents: "apiVersion: v1\nkind: Secret\nmetadata:\n  name: test\n  namespace: default\ntype: Opaque",
+			mustNotContain:          `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCaseCopy := testCase
+		t.Run(testCaseCopy.name, func(t *testing.T) {
+			t.Parallel()
+
+			outOfBoundsDir := t.TempDir()
+			outOfBoundsFile := path.Join(outOfBoundsDir, testCaseCopy.outOfBoundsFilename)
+			err := os.WriteFile(outOfBoundsFile, []byte(testCaseCopy.outOfBoundsFileContents), os.FileMode(0444))
+			require.NoError(t, err)
+
+			repoDir := t.TempDir()
+			err = os.Symlink(outOfBoundsFile, path.Join(repoDir, testCaseCopy.outOfBoundsFilename))
+			require.NoError(t, err)
+
+			var mustNotContain = testCaseCopy.outOfBoundsFileContents
+			if testCaseCopy.mustNotContain != "" {
+				mustNotContain = testCaseCopy.mustNotContain
+			}
+
+			q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
+			res, err := GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
+			require.Error(t, err)
+			assert.NotContains(t, err.Error(), mustNotContain)
+			assert.Contains(t, err.Error(), "illegal filepath")
+			assert.Nil(t, res)
+		})
+	}
+}
+
+func TestGenerateManifests_MissingSymlinkDestination(t *testing.T) {
+	repoDir := t.TempDir()
+	err := os.Symlink("/obviously/does/not/exist", path.Join(repoDir, "test.yaml"))
+	require.NoError(t, err)
+
+	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
+	_, err = GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
+	require.NoError(t, err)
+}
+
 func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
 	service := newService("../..")
 
@@ -302,29 +376,6 @@ func TestGenerateKsonnetManifest(t *testing.T) {
 	assert.Equal(t, "https://kubernetes.default.svc", res.Server)
 }
 
-func TestGenerateHelmChartWithDependencies(t *testing.T) {
-	service := newService("../..")
-
-	cleanup := func() {
-		_ = os.Remove(filepath.Join("../../util/helm/testdata/helm2-dependency", helmDepUpMarkerFile))
-		_ = os.RemoveAll(filepath.Join("../../util/helm/testdata/helm2-dependency", "charts"))
-	}
-	cleanup()
-	defer cleanup()
-
-	helmRepo := argoappv1.Repository{Name: "bitnami", Type: "helm", Repo: "https://charts.bitnami.com/bitnami"}
-	q := apiclient.ManifestRequest{
-		Repo: &argoappv1.Repository{},
-		ApplicationSource: &argoappv1.ApplicationSource{
-			Path: "./util/helm/testdata/helm2-dependency",
-		},
-		Repos: []*argoappv1.Repository{&helmRepo},
-	}
-	res1, err := service.GenerateManifest(context.Background(), &q)
-	assert.Nil(t, err)
-	assert.Len(t, res1.Manifests, 10)
-}
-
 func TestManifestGenErrorCacheByNumRequests(t *testing.T) {
 
 	// Returns the state of the manifest generation cache, by querying the cache for the previously set result
@@ -1080,7 +1131,7 @@ func TestGenerateFromUTF16(t *testing.T) {
 		Repo:              &argoappv1.Repository{},
 		ApplicationSource: &argoappv1.ApplicationSource{},
 	}
-	res1, err := GenerateManifests(context.Background(), "./testdata/utf-16", "/", "", &q, false, &git.NoopCredsStore{})
+	res1, err := GenerateManifests(context.Background(), "./testdata/utf-16", "/", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
 	assert.Nil(t, err)
 	assert.Equal(t, 2, len(res1.Manifests))
 }
@@ -1096,12 +1147,15 @@ func TestListApps(t *testing.T) {
 		"app-parameters/multi":           "Kustomize",
 		"app-parameters/single-app-only": "Kustomize",
 		"app-parameters/single-global":   "Kustomize",
+		"in-bounds-values-file-link":     "Helm",
 		"invalid-helm":                   "Helm",
 		"invalid-kustomize":              "Kustomize",
 		"kustomization_yaml":             "Kustomize",
 		"kustomization_yml":              "Kustomize",
 		"my-chart":                       "Helm",
 		"my-chart-2":                     "Helm",
+		"out-of-bounds-values-file-link": "Helm",
+		"values-files":                   "Helm",
 	}
 	assert.Equal(t, expectedApps, res.Apps)
 }
@@ -1440,6 +1494,7 @@ func runWithTempTestdata(t *testing.T, path string, runner func(t *testing.T, pa
 	tempDir := mkTempParameters("./testdata/app-parameters")
 	defer os.RemoveAll(tempDir)
 	runner(t, filepath.Join(tempDir, "app-parameters", path))
+	os.RemoveAll(tempDir)
 }
 
 func TestGenerateManifestsWithAppParameterFile(t *testing.T) {
@@ -1641,11 +1696,11 @@ func TestFindResources(t *testing.T) {
 	for i := range testCases {
 		tc := testCases[i]
 		t.Run(tc.name, func(t *testing.T) {
-			objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+			objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 				Recurse: true,
 				Include: tc.include,
 				Exclude: tc.exclude,
-			}, map[string]bool{})
+			}, map[string]bool{}, resource.MustParse("0"))
 			if !assert.NoError(t, err) {
 				return
 			}
@@ -1659,10 +1714,10 @@ func TestFindResources(t *testing.T) {
 }
 
 func TestFindManifests_Exclude(t *testing.T) {
-	objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "subdir/deploymentSub.yaml",
-	}, map[string]bool{})
+	}, map[string]bool{}, resource.MustParse("0"))
 
 	if !assert.NoError(t, err) || !assert.Len(t, objs, 1) {
 		return
@@ -1672,10 +1727,10 @@ func TestFindManifests_Exclude(t *testing.T) {
 }
 
 func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
-	objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "nothing.yaml",
-	}, map[string]bool{})
+	}, map[string]bool{}, resource.MustParse("0"))
 
 	if !assert.NoError(t, err) || !assert.Len(t, objs, 2) {
 		return
@@ -1685,6 +1740,469 @@ func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
 		[]string{"nginx-deployment", "nginx-deployment-sub"}, []string{objs[0].GetName(), objs[1].GetName()})
 }
 
+func tempDir(t *testing.T) string {
+	dir, err := ioutil.TempDir(".", "")
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		err = os.RemoveAll(dir)
+		if err != nil {
+			panic(err)
+		}
+	})
+	absDir, err := filepath.Abs(dir)
+	require.NoError(t, err)
+	return absDir
+}
+
+func walkFor(t *testing.T, root string, testPath string, run func(info fs.FileInfo)) {
+	var hitExpectedPath = false
+	err := filepath.Walk(root, func(path string, info fs.FileInfo, err error) error {
+		if path == testPath {
+			require.NoError(t, err)
+			hitExpectedPath = true
+			run(info)
+		}
+		return nil
+	})
+	require.NoError(t, err)
+	assert.True(t, hitExpectedPath, "did not hit expected path when walking directory")
+}
+
+func Test_getPotentiallyValidManifestFile(t *testing.T) {
+	// These tests use filepath.Walk instead of os.Stat to get file info, because FileInfo from os.Stat does not return
+	// true for IsSymlink like os.Walk does.
+
+	// These tests do not use t.TempDir() because those directories can contain symlinks which cause test to fail
+	// InBound checks.
+
+	t.Run("non-JSON/YAML is skipped with an empty ignore message", func(t *testing.T) {
+		appDir := tempDir(t)
+		filePath := filepath.Join(appDir, "not-json-or-yaml")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		aPath := filepath.Join(appDir, "a.json")
+		bPath := filepath.Join(appDir, "b.json")
+		err := os.Symlink(bPath, aPath)
+		require.NoError(t, err)
+		err = os.Symlink(aPath, bPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, aPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(aPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), "too many links")
+		})
+	})
+
+	t.Run("symlink with missing destination should throw an error", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		aPath := filepath.Join(appDir, "a.json")
+		bPath := filepath.Join(appDir, "b.json")
+		err := os.Symlink(bPath, aPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, aPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(aPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.NotEmpty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		linkPath := filepath.Join(appDir, "a.json")
+		err := os.Symlink("..", linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), "illegal filepath in symlink")
+		})
+	})
+
+	t.Run("symlink to a non-regular file should be skipped with warning", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		dirPath := filepath.Join(appDir, "test.dir")
+		err := os.MkdirAll(dirPath, 0644)
+		require.NoError(t, err)
+		linkPath := filepath.Join(appDir, "test.json")
+		err = os.Symlink(dirPath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Contains(t, ignoreMessage, "non-regular file")
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("non-included file should be skipped with no message", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "not-included.yaml")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "*.json", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("excluded file should be skipped with no message", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "excluded.json")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "excluded.*")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("symlink to a regular file is potentially valid", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		linkPath := filepath.Join(appDir, "link.json")
+		err = os.Symlink(filePath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("a regular file is potentially valid", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file.json")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("realFileInfo is for the destination rather than the symlink", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		linkPath := filepath.Join(appDir, "link.json")
+		err = os.Symlink(filePath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Equal(t, filepath.Base(filePath), realFileInfo.Name())
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+}
+
+func Test_getPotentiallyValidManifests(t *testing.T) {
+	// Tests which return no manifests and an error check to make sure the directory exists before running. A missing
+	// directory would produce those same results.
+
+	logCtx := log.WithField("test", "test")
+
+	t.Run("unreadable file throws error", func(t *testing.T) {
+		appDir := t.TempDir()
+		unreadablePath := filepath.Join(appDir, "unreadable.json")
+		err := os.WriteFile(unreadablePath, []byte{}, 0666)
+		require.NoError(t, err)
+		err = os.Chmod(appDir, 0000)
+		require.NoError(t, err)
+
+		manifests, err := getPotentiallyValidManifests(logCtx, appDir, appDir, false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+
+		// allow cleanup
+		err = os.Chmod(appDir, 0777)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	t.Run("no recursion when recursion is disabled", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/recurse", "./testdata/recurse", false, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("recursion when recursion is enabled", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/recurse", "./testdata/recurse", true, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+	})
+
+	t.Run("non-JSON/YAML is skipped", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/circular-link")
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/out-of-bounds-link")
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("symlink to a regular file works", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		manifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, false, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("symlink to nowhere should be ignored", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("link to over-sized manifest fails", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		// The file is 35 bytes.
+		manifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, false, "", "", resource.MustParse("34"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
+		// There is a total of 10 files, ech file being 10 bytes.
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/several-files", "./testdata/several-files", false, "", "", resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = getPotentiallyValidManifests(logCtx, "./testdata/several-files", "./testdata/several-files", false, "", "", resource.MustParse("100"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+}
+
+func Test_findManifests(t *testing.T) {
+	logCtx := log.WithField("test", "test")
+	noRecurse := argoappv1.ApplicationSourceDirectory{Recurse: false}
+
+	t.Run("unreadable file throws error", func(t *testing.T) {
+		appDir := t.TempDir()
+		unreadablePath := filepath.Join(appDir, "unreadable.json")
+		err := os.WriteFile(unreadablePath, []byte{}, 0666)
+		require.NoError(t, err)
+		err = os.Chmod(appDir, 0000)
+		require.NoError(t, err)
+
+		manifests, err := findManifests(logCtx, appDir, appDir, nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+
+		// allow cleanup
+		err = os.Chmod(appDir, 0777)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	t.Run("no recursion when recursion is disabled", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+	})
+
+	t.Run("recursion when recursion is enabled", func(t *testing.T) {
+		recurse := argoappv1.ApplicationSourceDirectory{Recurse: true}
+		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, recurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 4)
+		assert.NoError(t, err)
+	})
+
+	t.Run("non-JSON/YAML is skipped", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/circular-link")
+		manifests, err := findManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/out-of-bounds-link")
+		manifests, err := findManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("symlink to a regular file works", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("symlink to nowhere should be ignored", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("link to over-sized manifest fails", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		// The file is 35 bytes.
+		manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse, nil, resource.MustParse("34"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
+		// There is a total of 10 files, each file being 10 bytes.
+		manifests, err := findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, nil, resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, nil, resource.MustParse("364"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("jsonnet isn't counted against size limit", func(t *testing.T) {
+		// Each file is 36 bytes. Only the 36-byte json file should be counted against the limit.
+		manifests, err := findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, nil, resource.MustParse("36"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, nil, resource.MustParse("35"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("partially valid YAML file throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/partially-valid-yaml")
+		manifests, err := findManifests(logCtx, "./testdata/partially-valid-yaml", "./testdata/partially-valid-yaml", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalid manifest throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/invalid-manifests")
+		manifests, err := findManifests(logCtx, "./testdata/invalid-manifests", "./testdata/invalid-manifests", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("irrelevant YAML gets skipped, relevant YAML gets parsed", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/irrelevant-yaml", "./testdata/irrelevant-yaml", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("multiple JSON objects in one file throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/json-list")
+		manifests, err := findManifests(logCtx, "./testdata/json-list", "./testdata/json-list", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalid JSON throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/invalid-json")
+		manifests, err := findManifests(logCtx, "./testdata/invalid-json", "./testdata/invalid-json", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("valid JSON returns manifest and no error", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/valid-json", "./testdata/valid-json", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("YAML with an empty document doesn't throw an error", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/yaml-with-empty-document", "./testdata/yaml-with-empty-document", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+}
+
 func TestTestRepoOCI(t *testing.T) {
 	service := newService(".")
 	_, err := service.TestRepository(context.Background(), &apiclient.TestRepositoryRequest{
@@ -1816,3 +2334,100 @@ func TestInit(t *testing.T) {
 	require.Error(t, err)
 	require.NoError(t, initGitRepo(path.Join(dir, "repo2"), "https://github.com/argo-cd/test-repo2"))
 }
+
+// TestCheckoutRevisionCanGetNonstandardRefs shows that we can fetch a revision that points to a non-standard ref. In
+// other words, we haven't regressed and caused this issue again: https://github.com/argoproj/argo-cd/issues/4935
+func TestCheckoutRevisionCanGetNonstandardRefs(t *testing.T) {
+	rootPath, err := ioutil.TempDir("", "")
+	require.NoError(t, err)
+
+	sourceRepoPath, err := ioutil.TempDir(rootPath, "")
+	require.NoError(t, err)
+
+	// Create a repo such that one commit is on a non-standard ref _and nowhere else_. This is meant to simulate, for
+	// example, a GitHub ref for a pull into one repo from a fork of that repo.
+	runGit(t, sourceRepoPath, "init")
+	runGit(t, sourceRepoPath, "checkout", "-b", "main") // make sure there's a main branch to switch back to
+	runGit(t, sourceRepoPath, "commit", "-m", "empty", "--allow-empty")
+	runGit(t, sourceRepoPath, "checkout", "-b", "branch")
+	runGit(t, sourceRepoPath, "commit", "-m", "empty", "--allow-empty")
+	sha := runGit(t, sourceRepoPath, "rev-parse", "HEAD")
+	runGit(t, sourceRepoPath, "update-ref", "refs/pull/123/head", strings.TrimSuffix(sha, "\n"))
+	runGit(t, sourceRepoPath, "checkout", "main")
+	runGit(t, sourceRepoPath, "branch", "-D", "branch")
+
+	destRepoPath, err := ioutil.TempDir(rootPath, "")
+	require.NoError(t, err)
+
+	gitClient, err := git.NewClientExt("file://"+sourceRepoPath, destRepoPath, &git.NopCreds{}, true, false, "")
+	require.NoError(t, err)
+
+	pullSha, err := gitClient.LsRemote("refs/pull/123/head")
+	require.NoError(t, err)
+
+	err = checkoutRevision(gitClient, "does-not-exist", false)
+	assert.Error(t, err)
+
+	err = checkoutRevision(gitClient, pullSha, false)
+	assert.NoError(t, err)
+}
+
+// runGit runs a git command in the given working directory. If the command succeeds, it returns the combined standard
+// and error output. If it fails, it stops the test with a failure message.
+func runGit(t *testing.T, workDir string, args ...string) string {
+	cmd := exec.Command("git", args...)
+	cmd.Dir = workDir
+	out, err := cmd.CombinedOutput()
+	stringOut := string(out)
+	require.NoError(t, err, stringOut)
+	return stringOut
+}
+
+func Test_findHelmValueFilesInPath(t *testing.T) {
+	t.Run("does not exist", func(t *testing.T) {
+		files, err := findHelmValueFilesInPath("/obviously/does/not/exist")
+		assert.Error(t, err)
+		assert.Empty(t, files)
+	})
+	t.Run("values files", func(t *testing.T) {
+		files, err := findHelmValueFilesInPath("./testdata/values-files")
+		assert.NoError(t, err)
+		assert.Len(t, files, 4)
+	})
+}
+
+func Test_populateHelmAppDetails(t *testing.T) {
+	res := apiclient.RepoAppDetailsResponse{}
+	q := apiclient.RepoServerAppDetailsQuery{
+		Repo: &argoappv1.Repository{},
+		Source: &argoappv1.ApplicationSource{
+			Helm: &argoappv1.ApplicationSourceHelm{ValueFiles: []string{"exclude.yaml", "has-the-word-values.yaml"}},
+		},
+	}
+	appPath, err := filepath.Abs("./testdata/values-files/")
+	require.NoError(t, err)
+	err = populateHelmAppDetails(&res, appPath, appPath, &q)
+	require.NoError(t, err)
+	assert.Len(t, res.Helm.Parameters, 3)
+	assert.Len(t, res.Helm.ValueFiles, 4)
+}
+
+func Test_populateHelmAppDetails_values_symlinks(t *testing.T) {
+	t.Run("inbound", func(t *testing.T) {
+		res := apiclient.RepoAppDetailsResponse{}
+		q := apiclient.RepoServerAppDetailsQuery{Repo: &argoappv1.Repository{}, Source: &argoappv1.ApplicationSource{}}
+		err := populateHelmAppDetails(&res, "./testdata/in-bounds-values-file-link/", "./testdata/in-bounds-values-file-link/", &q)
+		require.NoError(t, err)
+		assert.NotEmpty(t, res.Helm.Values)
+		assert.NotEmpty(t, res.Helm.Parameters)
+	})
+
+	t.Run("out of bounds", func(t *testing.T) {
+		res := apiclient.RepoAppDetailsResponse{}
+		q := apiclient.RepoServerAppDetailsQuery{Repo: &argoappv1.Repository{}, Source: &argoappv1.ApplicationSource{}}
+		err := populateHelmAppDetails(&res, "./testdata/out-of-bounds-values-file-link/", "./testdata/out-of-bounds-values-file-link/", &q)
+		require.NoError(t, err)
+		assert.Empty(t, res.Helm.Values)
+		assert.Empty(t, res.Helm.Parameters)
+	})
+}

reposerver/repository/testdata/circular-link/a.json

@@ -0,0 +1 @@
+b.json

reposerver/repository/testdata/circular-link/b.json

@@ -0,0 +1 @@
+a.json

reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml

@@ -0,0 +1 @@
+../cm.yaml

reposerver/repository/testdata/in-bounds-link/cm.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ServiceAccount

reposerver/repository/testdata/in-bounds-values-file-link/Chart.yaml

@@ -0,0 +1,2 @@
+name: my-chart
+version: 1.1.0

reposerver/repository/testdata/in-bounds-values-file-link/values-2.yaml

@@ -0,0 +1 @@
+some: yaml

reposerver/repository/testdata/in-bounds-values-file-link/values.yaml

@@ -0,0 +1 @@
+values-2.yaml

reposerver/repository/testdata/invalid-json/invalid.json

@@ -0,0 +1 @@
+[

reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml

@@ -0,0 +1 @@
+some: [irrelevant, yaml]

reposerver/repository/testdata/irrelevant-yaml/relevant.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

reposerver/repository/testdata/json-list/list.json

@@ -0,0 +1,2 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/jsonnet-and-json/test.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "Pod"}

reposerver/repository/testdata/jsonnet-and-json/test.jsonnet

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "Pod"}

reposerver/repository/testdata/link-to-nowhere/nowhere.json

@@ -0,0 +1 @@
+nowhere

reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json

@@ -0,0 +1 @@
+../out-of-bounds.json

reposerver/repository/testdata/out-of-bounds-values-file-link/Chart.yaml

@@ -0,0 +1,2 @@
+name: my-chart
+version: 1.1.0

reposerver/repository/testdata/out-of-bounds-values-file-link/values.yaml

@@ -0,0 +1 @@
+../out-of-bounds.yaml

reposerver/repository/testdata/out-of-bounds.yaml

@@ -0,0 +1 @@
+some: yaml

reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ConfigMap
+---
+invalid:

reposerver/repository/testdata/several-files/0.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/several-files/0.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

reposerver/repository/testdata/several-files/1.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/several-files/1.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

reposerver/repository/testdata/several-files/2.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/several-files/2.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

reposerver/repository/testdata/several-files/3.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/several-files/3.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

reposerver/repository/testdata/several-files/4.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/several-files/4.yaml

@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap

reposerver/repository/testdata/several-files/README.md

@@ -0,0 +1 @@
+This file shouldn't be counted in the manifest file size limit, because it isn't JSON or YAML.

reposerver/repository/testdata/valid-json/valid.json

@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}

reposerver/repository/testdata/values-files/Chart.yaml

@@ -0,0 +1,2 @@
+name: my-chart
+version: 1.1.0

reposerver/repository/testdata/values-files/exclude.yaml

@@ -0,0 +1 @@
+exclude: yaml

reposerver/repository/testdata/values-files/has-the-word-values.yaml

@@ -0,0 +1,4 @@
+has:
+  the:
+    word:
+      values: yaml

reposerver/repository/testdata/values-files/values.yaml

@@ -0,0 +1 @@
+values: yaml

reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ConfigMap
+---
+---

server/server.go

@@ -951,6 +951,9 @@ func (a *ArgoCDServer) Authenticate(ctx context.Context) (context.Context, error
 		}
 		if !argoCDSettings.AnonymousUserEnabled {
 			return ctx, claimsErr
+		} else {
+			// nolint:staticcheck
+			ctx = context.WithValue(ctx, "claims", "")
 		}
 	}
 

server/server_test.go

@@ -3,6 +3,8 @@ package server
 import (
 	"context"
 	"fmt"
+	"io"
+	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
@@ -12,6 +14,7 @@ import (
 	"github.com/golang-jwt/jwt/v4"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/metadata"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
@@ -432,6 +435,396 @@ func TestAuthenticate(t *testing.T) {
 	}
 }
 
+func dexMockHandler(t *testing.T, url string) func(http.ResponseWriter, *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.RequestURI {
+		case "/api/dex/.well-known/openid-configuration":
+			_, err := io.WriteString(w, fmt.Sprintf(`
+{
+  "issuer": "%[1]s/api/dex",
+  "authorization_endpoint": "%[1]s/api/dex/auth",
+  "token_endpoint": "%[1]s/api/dex/token",
+  "jwks_uri": "%[1]s/api/dex/keys",
+  "userinfo_endpoint": "%[1]s/api/dex/userinfo",
+  "device_authorization_endpoint": "%[1]s/api/dex/device/code",
+  "grant_types_supported": [
+    "authorization_code",
+    "refresh_token",
+    "urn:ietf:params:oauth:grant-type:device_code"
+  ],
+  "response_types_supported": [
+    "code"
+  ],
+  "subject_types_supported": [
+    "public"
+  ],
+  "id_token_signing_alg_values_supported": [
+    "RS256", "HS256"
+  ],
+  "code_challenge_methods_supported": [
+    "S256",
+    "plain"
+  ],
+  "scopes_supported": [
+    "openid",
+    "email",
+    "groups",
+    "profile",
+    "offline_access"
+  ],
+  "token_endpoint_auth_methods_supported": [
+    "client_secret_basic",
+    "client_secret_post"
+  ],
+  "claims_supported": [
+    "iss",
+    "sub",
+    "aud",
+    "iat",
+    "exp",
+    "email",
+    "email_verified",
+    "locale",
+    "name",
+    "preferred_username",
+    "at_hash"
+  ]
+}`, url))
+			if err != nil {
+				t.Fail()
+			}
+		default:
+			w.WriteHeader(404)
+		}
+	}
+}
+
+func getTestServer(t *testing.T, anonymousEnabled bool, withFakeSSO bool) (argocd *ArgoCDServer, dexURL string) {
+	cm := test.NewFakeConfigMap()
+	if anonymousEnabled {
+		cm.Data["users.anonymous.enabled"] = "true"
+	}
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Start with a placeholder. We need the server URL before setting up the real handler.
+	}))
+	ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		dexMockHandler(t, ts.URL)(w, r)
+	})
+	if withFakeSSO {
+		cm.Data["url"] = ts.URL
+		cm.Data["dex.config"] = `
+connectors:
+  # OIDC
+  - type: OIDC
+    id: oidc
+    name: OIDC
+    config:
+    issuer: https://auth.example.gom
+    clientID: test-client
+    clientSecret: $dex.oidc.clientSecret`
+	}
+	secret := test.NewFakeSecret()
+	kubeclientset := fake.NewSimpleClientset(cm, secret)
+	appClientSet := apps.NewSimpleClientset()
+	argoCDOpts := ArgoCDServerOpts{
+		Namespace:     test.FakeArgoCDNamespace,
+		KubeClientset: kubeclientset,
+		AppClientset:  appClientSet,
+	}
+	if withFakeSSO {
+		argoCDOpts.DexServerAddr = ts.URL
+	}
+	argocd = NewServer(context.Background(), argoCDOpts)
+	return argocd, ts.URL
+}
+
+func TestAuthenticate_3rd_party_JWTs(t *testing.T) {
+	type testData struct {
+		test                  string
+		anonymousEnabled      bool
+		claims                jwt.RegisteredClaims
+		expectedErrorContains string
+		expectedClaims        interface{}
+	}
+	var tests = []testData{
+		{
+			test:                  "anonymous disabled, no audience",
+			anonymousEnabled:      false,
+			claims:                jwt.RegisteredClaims{},
+			expectedErrorContains: "no audience found in the token",
+			expectedClaims:        nil,
+		},
+		{
+			test:                  "anonymous enabled, no audience",
+			anonymousEnabled:      true,
+			claims:                jwt.RegisteredClaims{},
+			expectedErrorContains: "",
+			expectedClaims:        "",
+		},
+		{
+			test:                  "anonymous disabled, unexpired token, admin claim",
+			anonymousEnabled:      false,
+			claims:                jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour * 24))},
+			expectedErrorContains: "id token signed with unsupported algorithm",
+			expectedClaims:        nil,
+		},
+		{
+			test:                  "anonymous enabled, unexpired token, admin claim",
+			anonymousEnabled:      true,
+			claims:                jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour * 24))},
+			expectedErrorContains: "",
+			expectedClaims:        "",
+		},
+		{
+			test:                  "anonymous disabled, expired token, admin claim",
+			anonymousEnabled:      false,
+			claims:                jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now())},
+			expectedErrorContains: "token is expired",
+			expectedClaims:        jwt.RegisteredClaims{Issuer:"sso"},
+		},
+		{
+			test:                  "anonymous enabled, expired token, admin claim",
+			anonymousEnabled:      true,
+			claims:                jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now())},
+			expectedErrorContains: "",
+			expectedClaims:        "",
+		},
+	}
+
+	for _, testData := range tests {
+		testDataCopy := testData
+
+		t.Run(testDataCopy.test, func(t *testing.T) {
+			t.Parallel()
+
+			// Must be declared here to avoid race.
+			ctx := context.Background()  //nolint:ineffassign,staticcheck
+
+			argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, true)
+			testDataCopy.claims.Issuer = fmt.Sprintf("%s/api/dex", dexURL)
+			token := jwt.NewWithClaims(jwt.SigningMethodHS256, testDataCopy.claims)
+			tokenString, err := token.SignedString([]byte("key"))
+			require.NoError(t, err)
+			ctx = metadata.NewIncomingContext(context.Background(), metadata.Pairs(apiclient.MetaDataTokenKey, tokenString))
+
+			ctx, err = argocd.Authenticate(ctx)
+			claims := ctx.Value("claims")
+			if testDataCopy.expectedClaims == nil {
+				assert.Nil(t, claims)
+			} else {
+				assert.Equal(t, testDataCopy.expectedClaims, claims)
+			}
+			if testDataCopy.expectedErrorContains != "" {
+				assert.Error(t, err, "Authenticate should have thrown an error and blocked the request")
+				assert.Contains(t, err.Error(), testDataCopy.expectedErrorContains)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestAuthenticate_no_request_metadata(t *testing.T) {
+	type testData struct {
+		test                  string
+		anonymousEnabled      bool
+		expectedErrorContains string
+		expectedClaims        interface{}
+	}
+	var tests = []testData{
+		{
+			test:                  "anonymous disabled",
+			anonymousEnabled:      false,
+			expectedErrorContains: "no session information",
+			expectedClaims:        nil,
+		},
+		{
+			test:                  "anonymous enabled",
+			anonymousEnabled:      true,
+			expectedErrorContains: "",
+			expectedClaims:        "",
+		},
+	}
+
+	for _, testData := range tests {
+		testDataCopy := testData
+
+		t.Run(testDataCopy.test, func(t *testing.T) {
+			t.Parallel()
+
+			argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true)
+			ctx := context.Background()
+
+			ctx, err := argocd.Authenticate(ctx)
+			claims := ctx.Value("claims")
+			assert.Equal(t, testDataCopy.expectedClaims, claims)
+			if testDataCopy.expectedErrorContains != "" {
+				assert.Error(t, err, "Authenticate should have thrown an error and blocked the request")
+				assert.Contains(t, err.Error(), testDataCopy.expectedErrorContains)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestAuthenticate_no_SSO(t *testing.T) {
+	type testData struct {
+		test                 string
+		anonymousEnabled     bool
+		expectedErrorMessage string
+		expectedClaims       interface{}
+	}
+	var tests = []testData{
+		{
+			test:                 "anonymous disabled",
+			anonymousEnabled:     false,
+			expectedErrorMessage: "SSO is not configured",
+			expectedClaims:       nil,
+		},
+		{
+			test:                 "anonymous enabled",
+			anonymousEnabled:     true,
+			expectedErrorMessage: "",
+			expectedClaims:       "",
+		},
+	}
+
+	for _, testData := range tests {
+		testDataCopy := testData
+
+		t.Run(testDataCopy.test, func(t *testing.T) {
+			t.Parallel()
+
+			// Must be declared here to avoid race.
+			ctx := context.Background()  //nolint:ineffassign,staticcheck
+
+			argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, false)
+			token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.RegisteredClaims{Issuer: fmt.Sprintf("%s/api/dex", dexURL)})
+			tokenString, err := token.SignedString([]byte("key"))
+			require.NoError(t, err)
+			ctx = metadata.NewIncomingContext(context.Background(), metadata.Pairs(apiclient.MetaDataTokenKey, tokenString))
+
+			ctx, err = argocd.Authenticate(ctx)
+			claims := ctx.Value("claims")
+			assert.Equal(t, testDataCopy.expectedClaims, claims)
+			if testDataCopy.expectedErrorMessage != "" {
+				assert.Error(t, err, "Authenticate should have thrown an error and blocked the request")
+				assert.Contains(t, err.Error(), testDataCopy.expectedErrorMessage)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestAuthenticate_bad_request_metadata(t *testing.T) {
+	type testData struct {
+		test                 string
+		anonymousEnabled     bool
+		metadata             metadata.MD
+		expectedErrorMessage string
+		expectedClaims       interface{}
+	}
+	var tests = []testData{
+		{
+			test:                 "anonymous disabled, empty metadata",
+			anonymousEnabled:     false,
+			metadata:             metadata.MD{},
+			expectedErrorMessage: "no session information",
+			expectedClaims:       nil,
+		},
+		{
+			test:                 "anonymous enabled, empty metadata",
+			anonymousEnabled:     true,
+			metadata:             metadata.MD{},
+			expectedErrorMessage: "",
+			expectedClaims:       "",
+		},
+		{
+			test:                 "anonymous disabled, empty tokens",
+			anonymousEnabled:     false,
+			metadata:             metadata.MD{apiclient.MetaDataTokenKey: []string{}},
+			expectedErrorMessage: "no session information",
+			expectedClaims:       nil,
+		},
+		{
+			test:                 "anonymous enabled, empty tokens",
+			anonymousEnabled:     true,
+			metadata:             metadata.MD{apiclient.MetaDataTokenKey: []string{}},
+			expectedErrorMessage: "",
+			expectedClaims:       "",
+		},
+		{
+			test:                 "anonymous disabled, bad tokens",
+			anonymousEnabled:     false,
+			metadata:             metadata.Pairs(apiclient.MetaDataTokenKey, "bad"),
+			expectedErrorMessage: "token contains an invalid number of segments",
+			expectedClaims:       nil,
+		},
+		{
+			test:                 "anonymous enabled, bad tokens",
+			anonymousEnabled:     true,
+			metadata:             metadata.Pairs(apiclient.MetaDataTokenKey, "bad"),
+			expectedErrorMessage: "",
+			expectedClaims:       "",
+		},
+		{
+			test:                 "anonymous disabled, bad auth header",
+			anonymousEnabled:     false,
+			metadata:             metadata.MD{"authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
+			expectedErrorMessage: "no audience found in the token",
+			expectedClaims:       nil,
+		},
+		{
+			test:                 "anonymous enabled, bad auth header",
+			anonymousEnabled:     true,
+			metadata:             metadata.MD{"authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
+			expectedErrorMessage: "",
+			expectedClaims:       "",
+		},
+		{
+			test:                 "anonymous disabled, bad auth cookie",
+			anonymousEnabled:     false,
+			metadata:             metadata.MD{"grpcgateway-cookie": []string{"argocd.token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
+			expectedErrorMessage: "no audience found in the token",
+			expectedClaims:       nil,
+		},
+		{
+			test:                 "anonymous enabled, bad auth cookie",
+			anonymousEnabled:     true,
+			metadata:             metadata.MD{"grpcgateway-cookie": []string{"argocd.token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
+			expectedErrorMessage: "",
+			expectedClaims:       "",
+		},
+	}
+
+	for _, testData := range tests {
+		testDataCopy := testData
+
+		t.Run(testDataCopy.test, func(t *testing.T) {
+			t.Parallel()
+
+			// Must be declared here to avoid race.
+			ctx := context.Background()  //nolint:ineffassign,staticcheck
+
+			argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true)
+			ctx = metadata.NewIncomingContext(context.Background(), testDataCopy.metadata)
+
+			ctx, err := argocd.Authenticate(ctx)
+			claims := ctx.Value("claims")
+			assert.Equal(t, testDataCopy.expectedClaims, claims)
+			if testDataCopy.expectedErrorMessage != "" {
+				assert.Error(t, err, "Authenticate should have thrown an error and blocked the request")
+				assert.Contains(t, err.Error(), testDataCopy.expectedErrorMessage)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func Test_getToken(t *testing.T) {
 	token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"
 	t.Run("Empty", func(t *testing.T) {

test/container/Dockerfile

@@ -2,7 +2,7 @@ FROM redis:6.2.6 as redis
 
 FROM node:12.18.4 as node
 
-FROM golang:1.17.6 as golang
+FROM golang:1.17 as golang
 
 FROM registry:2.7.1 as registry
 

test/e2e/fixture/fixture.go

@@ -554,7 +554,9 @@ func EnsureCleanState(t *testing.T) {
 	FailOnErr(Run("", "mkdir", "-p", TmpDir))
 
 	// random id - unique across test runs
-	postFix := "-" + strings.ToLower(rand.RandString(5))
+	randString, err := rand.String(5)
+	CheckError(err)
+	postFix := "-" + strings.ToLower(randString)
 	id = t.Name() + postFix
 	name = DnsFriendly(t.Name(), "")
 	deploymentNamespace = DnsFriendly(fmt.Sprintf("argocd-e2e-%s", t.Name()), postFix)

test/e2e/selective_sync_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/argoproj/gitops-engine/pkg/health"
 	. "github.com/argoproj/gitops-engine/pkg/sync/common"
+	"github.com/stretchr/testify/require"
 
 	. "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/test/e2e/fixture"
@@ -110,7 +111,9 @@ func TestSelectiveSyncWithNamespace(t *testing.T) {
 }
 
 func getNewNamespace(t *testing.T) string {
-	postFix := "-" + strings.ToLower(rand.RandString(5))
+	randStr, err := rand.String(5)
+	require.NoError(t, err)
+	postFix := "-" + strings.ToLower(randStr)
 	name := fixture.DnsFriendly(t.Name(), "")
 	return fixture.DnsFriendly(fmt.Sprintf("argocd-e2e-%s", name), postFix)
 }

test/remote/Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.17.6 AS go
+FROM golang:1.17 AS go
 
 RUN go install github.com/mattn/goreman@latest && \
     go install github.com/kisielk/godepgraph@latest

ui/src/app/applications/components/application-urls.test.ts

@@ -0,0 +1,20 @@
+import {ExternalLink, InvalidExternalLinkError} from './application-urls';
+
+test('rejects malicious URLs', () => {
+    expect(() => {
+        const _ = new ExternalLink('javascript:alert("hi")');
+    }).toThrowError(InvalidExternalLinkError);
+    expect(() => {
+        const _ = new ExternalLink('data:text/html;<h1>hi</h1>');
+    }).toThrowError(InvalidExternalLinkError);
+});
+
+test('allows absolute URLs', () => {
+    expect(new ExternalLink('https://localhost:8080/applications').ref).toEqual('https://localhost:8080/applications');
+});
+
+test('allows relative URLs', () => {
+    // @ts-ignore
+    window.location = new URL('https://localhost:8080/applications');
+    expect(new ExternalLink('/applications').ref).toEqual('/applications');
+});

ui/src/app/applications/components/application-urls.tsx

@@ -1,7 +1,15 @@
 import {DropDownMenu} from 'argo-ui';
 import * as React from 'react';
 
-class ExternalLink {
+export class InvalidExternalLinkError extends Error {
+    constructor(message: string) {
+        super(message);
+        Object.setPrototypeOf(this, InvalidExternalLinkError.prototype);
+        this.name = 'InvalidExternalLinkError';
+    }
+}
+
+export class ExternalLink {
     public title: string;
     public ref: string;
 
@@ -14,13 +22,36 @@ class ExternalLink {
             this.title = url;
             this.ref = url;
         }
+        if (!ExternalLink.isValidURL(this.ref)) {
+            throw new InvalidExternalLinkError('Invalid URL');
+        }
+    }
+
+    private static isValidURL(url: string): boolean {
+        try {
+            const parsedUrl = new URL(url);
+            return parsedUrl.protocol !== 'javascript:' && parsedUrl.protocol !== 'data:';
+        } catch (TypeError) {
+            try {
+                // Try parsing as a relative URL.
+                const parsedUrl = new URL(url, window.location.origin);
+                return parsedUrl.protocol !== 'javascript:' && parsedUrl.protocol !== 'data:';
+            } catch (TypeError) {
+                return false;
+            }
+        }
     }
 }
 
 export const ApplicationURLs = ({urls}: {urls: string[]}) => {
     const externalLinks: ExternalLink[] = [];
     for (const url of urls || []) {
-        externalLinks.push(new ExternalLink(url));
+        try {
+            const externalLink = new ExternalLink(url);
+            externalLinks.push(externalLink);
+        } catch (InvalidExternalLinkError) {
+            continue;
+        }
     }
 
     // sorted alphabetically & links with titles first

ui/src/app/login/components/login.tsx

@@ -20,7 +20,7 @@ interface State {
     loginError: string;
     loginInProgress: boolean;
     returnUrl: string;
-    ssoLoginError: string;
+    hasSsoLoginError: boolean;
 }
 
 export class Login extends React.Component<RouteComponentProps<{}>, State> {
@@ -31,13 +31,13 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
     public static getDerivedStateFromProps(props: RouteComponentProps<{}>): Partial<State> {
         const search = new URLSearchParams(props.history.location.search);
         const returnUrl = search.get('return_url') || '';
-        const ssoLoginError = search.get('sso_error') || '';
-        return {ssoLoginError, returnUrl};
+        const hasSsoLoginError = search.get('has_sso_error') === 'true';
+        return {hasSsoLoginError, returnUrl};
     }
 
     constructor(props: RouteComponentProps<{}>) {
         super(props);
-        this.state = {authSettings: null, loginError: null, returnUrl: null, ssoLoginError: null, loginInProgress: false};
+        this.state = {authSettings: null, loginError: null, returnUrl: null, hasSsoLoginError: false, loginInProgress: false};
     }
 
     public async componentDidMount() {
@@ -69,7 +69,7 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
                                         )}
                                 </button>
                             </a>
-                            {this.state.ssoLoginError && <div className='argo-form-row__error-msg'>{this.state.ssoLoginError}</div>}
+                            {this.state.hasSsoLoginError && <div className='argo-form-row__error-msg'>Login failed.</div>}
                             {authSettings && !authSettings.userLoginsDisabled && (
                                 <div className='login__saml-separator'>
                                     <span>or</span>

ui/src/app/webpack.config.js

@@ -88,6 +88,10 @@ const config = {
                 {
                     from: 'node_modules/redoc/bundles/redoc.standalone.js',
                     to: 'assets/scripts/redoc.standalone.js'
+                },
+                {
+                    from: 'node_modules/monaco-editor/min/vs/base/browser/ui/codicons/codicon',
+                    to: 'assets/fonts'
                 }
             ]
         }),

ui/src/assets/fonts.css

@@ -1,3 +1,8 @@
+@font-face {
+	font-family: "codicon";
+	src: url("./fonts/codicon.ttf") format("truetype");
+}
+
 /* === Heebo - 300 */
 @font-face {
 	font-family: 'Heebo';

util/dex/dex.go

@@ -3,20 +3,18 @@ package dex
 import (
 	"bytes"
 	"fmt"
-	"html"
 	"io/ioutil"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
 	"path"
-	"regexp"
 	"strconv"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/argoproj/argo-cd/v2/util/errors"
 )
 
-var messageRe = regexp.MustCompile(`<p>(.*)([\s\S]*?)<\/p>`)
-
 func decorateDirector(director func(req *http.Request), target *url.URL) func(req *http.Request) {
 	return func(req *http.Request) {
 		director(req)
@@ -44,16 +42,10 @@ func NewDexHTTPReverseProxy(serverAddr string, baseHRef string) func(writer http
 			if err != nil {
 				return err
 			}
-			var message string
-			matches := messageRe.FindSubmatch(b)
-			if len(matches) > 1 {
-				message = html.UnescapeString(string(matches[1]))
-			} else {
-				message = "Unknown error"
-			}
+			log.Errorf("received error from dex: %s", string(b))
 			resp.ContentLength = 0
 			resp.Header.Set("Content-Length", strconv.Itoa(0))
-			resp.Header.Set("Location", fmt.Sprintf("%s?sso_error=%s", path.Join(baseHRef, "login"), url.QueryEscape(message)))
+			resp.Header.Set("Location", fmt.Sprintf("%s?has_sso_error=true", path.Join(baseHRef, "login")))
 			resp.StatusCode = http.StatusSeeOther
 			resp.Body = ioutil.NopCloser(bytes.NewReader(make([]byte, 0)))
 			return nil

util/dex/dex_test.go

@@ -408,7 +408,7 @@ func Test_DexReverseProxy(t *testing.T) {
 		assert.Equal(t, http.StatusSeeOther, resp.StatusCode)
 		location, _ := resp.Location()
 		fmt.Printf("%s %s\n", resp.Status, location.RequestURI())
-		assert.True(t, strings.HasPrefix(location.RequestURI(), "/login?sso_error"))
+		assert.True(t, strings.HasPrefix(location.RequestURI(), "/login?has_sso_error=true"))
 	})
 
 	t.Run("Invalid URL for Dex reverse proxy", func(t *testing.T) {

util/helm/helm.go

@@ -6,10 +6,11 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
-	"path"
+	"path/filepath"
 	"strings"
 
 	"github.com/ghodss/yaml"
+	log "github.com/sirupsen/logrus"
 
 	"github.com/argoproj/argo-cd/v2/util/config"
 	executil "github.com/argoproj/argo-cd/v2/util/exec"
@@ -28,7 +29,7 @@ type Helm interface {
 	// Template returns a list of unstructured objects from a `helm template` command
 	Template(opts *TemplateOpts) (string, error)
 	// GetParameters returns a list of chart parameters taking into account values in provided YAML files.
-	GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[string]string, error)
+	GetParameters(valuesFiles []pathutil.ResolvedFilePath, appPath, repoRoot string) (map[string]string, error)
 	// DependencyBuild runs `helm dependency build` to download a chart's dependencies
 	DependencyBuild() error
 	// Init runs `helm init --client-only`
@@ -130,12 +131,19 @@ func Version(shortForm bool) (string, error) {
 	return strings.TrimSpace(version), nil
 }
 
-func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[string]string, error) {
-	out, err := h.cmd.inspectValues(".")
-	if err != nil {
-		return nil, err
+func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath, appPath, repoRoot string) (map[string]string, error) {
+	var values []string
+	// Don't load values.yaml if it's an out-of-bounds link.
+	if resolved, _, err := pathutil.ResolveFilePath(appPath, repoRoot, "values.yaml", []string{}); err == nil {
+		fmt.Println(resolved)
+		out, err := h.cmd.inspectValues(".")
+		if err != nil {
+			return nil, err
+		}
+		values = append(values, out)
+	} else {
+		log.Warnf("Values file %s is not allowed: %v", filepath.Join(appPath, "values.yaml"), err)
 	}
-	values := []string{out}
 	for i := range valuesFiles {
 		file := string(valuesFiles[i])
 		var fileValues []byte
@@ -143,11 +151,10 @@ func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[strin
 		if err == nil && (parsedURL.Scheme == "http" || parsedURL.Scheme == "https") {
 			fileValues, err = config.ReadRemoteFile(file)
 		} else {
-			filePath := path.Join(h.cmd.WorkDir, file)
-			if _, err := os.Stat(filePath); os.IsNotExist(err) {
+			if _, err := os.Stat(file); os.IsNotExist(err) {
 				continue
 			}
-			fileValues, err = ioutil.ReadFile(filePath)
+			fileValues, err = ioutil.ReadFile(file)
 		}
 		if err != nil {
 			return nil, fmt.Errorf("failed to read value file %s: %s", file, err)
@@ -158,7 +165,7 @@ func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[strin
 	output := map[string]string{}
 	for _, file := range values {
 		values := map[string]interface{}{}
-		if err = yaml.Unmarshal([]byte(file), &values); err != nil {
+		if err := yaml.Unmarshal([]byte(file), &values); err != nil {
 			return nil, fmt.Errorf("failed to parse values: %s", err)
 		}
 		flatVals(values, output)

util/helm/helm_test.go

@@ -1,10 +1,11 @@
 package helm
 
 import (
-	"fmt"
-	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/argoproj/argo-cd/v2/util/io/path"
 
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -53,11 +54,16 @@ func TestHelmTemplateParams(t *testing.T) {
 }
 
 func TestHelmTemplateValues(t *testing.T) {
-	h, err := NewHelmApp("./testdata/redis", []HelmRepository{}, false, "", "", false)
+	repoRoot := "./testdata/redis"
+	repoRootAbs, err := filepath.Abs(repoRoot)
+	require.NoError(t, err)
+	h, err := NewHelmApp(repoRootAbs, []HelmRepository{}, false, "", "", false)
 	assert.NoError(t, err)
+	valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil)
+	require.NoError(t, err)
 	opts := TemplateOpts{
 		Name:   "test",
-		Values: []path.ResolvedFilePath{"values-production.yaml"},
+		Values: []path.ResolvedFilePath{valuesPath},
 	}
 	objs, err := template(h, &opts)
 	assert.Nil(t, err)
@@ -74,59 +80,48 @@ func TestHelmTemplateValues(t *testing.T) {
 }
 
 func TestHelmGetParams(t *testing.T) {
-	h, err := NewHelmApp("./testdata/redis", nil, false, "", "", false)
+	repoRoot := "./testdata/redis"
+	repoRootAbs, err := filepath.Abs(repoRoot)
+	require.NoError(t, err)
+	h, err := NewHelmApp(repoRootAbs, nil, false, "", "", false)
 	assert.NoError(t, err)
-	params, err := h.GetParameters(nil)
+	params, err := h.GetParameters(nil, repoRootAbs, repoRootAbs)
 	assert.Nil(t, err)
 
 	slaveCountParam := params["cluster.slaveCount"]
-	assert.Equal(t, slaveCountParam, "1")
+	assert.Equal(t, "1", slaveCountParam)
 }
 
 func TestHelmGetParamsValueFiles(t *testing.T) {
-	h, err := NewHelmApp("./testdata/redis", nil, false, "", "", false)
+	repoRoot := "./testdata/redis"
+	repoRootAbs, err := filepath.Abs(repoRoot)
+	require.NoError(t, err)
+	h, err := NewHelmApp(repoRootAbs, nil, false, "", "", false)
 	assert.NoError(t, err)
-	params, err := h.GetParameters([]path.ResolvedFilePath{"values-production.yaml"})
+	valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil)
+	require.NoError(t, err)
+	params, err := h.GetParameters([]path.ResolvedFilePath{valuesPath}, repoRootAbs, repoRootAbs)
 	assert.Nil(t, err)
 
 	slaveCountParam := params["cluster.slaveCount"]
-	assert.Equal(t, slaveCountParam, "3")
+	assert.Equal(t, "3", slaveCountParam)
 }
 
 func TestHelmGetParamsValueFilesThatExist(t *testing.T) {
-	h, err := NewHelmApp("./testdata/redis", nil, false, "", "", false)
+	repoRoot := "./testdata/redis"
+	repoRootAbs, err := filepath.Abs(repoRoot)
+	require.NoError(t, err)
+	h, err := NewHelmApp(repoRootAbs, nil, false, "", "", false)
 	assert.NoError(t, err)
-	params, err := h.GetParameters([]path.ResolvedFilePath{"values-missing.yaml", "values-production.yaml"})
+	valuesMissingPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-missing.yaml", nil)
+	require.NoError(t, err)
+	valuesProductionPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil)
+	require.NoError(t, err)
+	params, err := h.GetParameters([]path.ResolvedFilePath{valuesMissingPath, valuesProductionPath}, repoRootAbs, repoRootAbs)
 	assert.Nil(t, err)
 
 	slaveCountParam := params["cluster.slaveCount"]
-	assert.Equal(t, slaveCountParam, "3")
-}
-
-func TestHelmDependencyBuild(t *testing.T) {
-	testCases := map[string]string{"Helm": "dependency", "Helm2": "helm2-dependency"}
-	helmRepos := []HelmRepository{{Name: "bitnami", Repo: "https://charts.bitnami.com/bitnami"}}
-	for name := range testCases {
-		t.Run(name, func(t *testing.T) {
-			chart := testCases[name]
-			clean := func() {
-				_ = os.RemoveAll(fmt.Sprintf("./testdata/%s/charts", chart))
-				_ = os.RemoveAll(fmt.Sprintf("./testdata/%s/Chart.lock", chart))
-			}
-			clean()
-			defer clean()
-			h, err := NewHelmApp(fmt.Sprintf("./testdata/%s", chart), helmRepos, false, "", "", false)
-			assert.NoError(t, err)
-			err = h.Init()
-			assert.NoError(t, err)
-			_, err = h.Template(&TemplateOpts{Name: "wordpress"})
-			assert.Error(t, err)
-			err = h.DependencyBuild()
-			assert.NoError(t, err)
-			_, err = h.Template(&TemplateOpts{Name: "wordpress"})
-			assert.NoError(t, err)
-		})
-	}
+	assert.Equal(t, "3", slaveCountParam)
 }
 
 func TestHelmTemplateReleaseNameOverwrite(t *testing.T) {

util/helm/testdata/dependency/Chart.lock

@@ -0,0 +1,9 @@
+dependencies:
+- name: mongodb
+  repository: https://charts.bitnami.com/bitnami
+  version: 7.8.10
+- name: eventstore
+  repository: https://eventstore.github.io/EventStore.Charts
+  version: 0.2.5
+digest: sha256:94b7537c078de2547173e28289cbae155893cd797aa9296f6c78ba922d6a1e8a
+generated: "2022-05-19T11:55:59.559189-04:00"

util/io/bytereadseeker.go

@@ -14,7 +14,7 @@ type byteReadSeeker struct {
 	offset int64
 }
 
-func (f byteReadSeeker) Read(b []byte) (int, error) {
+func (f *byteReadSeeker) Read(b []byte) (int, error) {
 	if f.offset >= int64(len(f.data)) {
 		return 0, io.EOF
 	}
@@ -23,7 +23,7 @@ func (f byteReadSeeker) Read(b []byte) (int, error) {
 	return n, nil
 }
 
-func (f byteReadSeeker) Seek(offset int64, whence int) (int64, error) {
+func (f *byteReadSeeker) Seek(offset int64, whence int) (int64, error) {
 	switch whence {
 	case 1:
 		offset += f.offset

util/io/bytereadseeker_test.go

@@ -0,0 +1,71 @@
+package io
+
+import (
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"io"
+	"testing"
+)
+
+func TestByteReadSeeker_Read(t *testing.T) {
+	inString := "hello world"
+	reader := NewByteReadSeeker([]byte(inString))
+	var bytes = make([]byte, 11)
+	n, err := reader.Read(bytes)
+	require.NoError(t, err)
+	assert.Equal(t, len(inString), n)
+	assert.Equal(t, inString, string(bytes))
+	_, err = reader.Read(bytes)
+	assert.ErrorIs(t, err, io.EOF)
+}
+
+func TestByteReadSeeker_Seek_Start(t *testing.T) {
+	inString := "hello world"
+	reader := NewByteReadSeeker([]byte(inString))
+	offset, err := reader.Seek(6, io.SeekStart)
+	require.NoError(t, err)
+	assert.Equal(t, int64(6), offset)
+	var bytes = make([]byte, 5)
+	n, err := reader.Read(bytes)
+	require.NoError(t, err)
+	assert.Equal(t, 5, n)
+	assert.Equal(t, "world", string(bytes))
+}
+
+func TestByteReadSeeker_Seek_Current(t *testing.T) {
+	inString := "hello world"
+	reader := NewByteReadSeeker([]byte(inString))
+	offset, err := reader.Seek(3, io.SeekCurrent)
+	require.NoError(t, err)
+	assert.Equal(t, int64(3), offset)
+	offset, err = reader.Seek(3, io.SeekCurrent)
+	require.NoError(t, err)
+	assert.Equal(t, int64(6), offset)
+	var bytes = make([]byte, 5)
+	n, err := reader.Read(bytes)
+	require.NoError(t, err)
+	assert.Equal(t, 5, n)
+	assert.Equal(t, "world", string(bytes))
+}
+
+func TestByteReadSeeker_Seek_End(t *testing.T) {
+	inString := "hello world"
+	reader := NewByteReadSeeker([]byte(inString))
+	offset, err := reader.Seek(-5, io.SeekEnd)
+	require.NoError(t, err)
+	assert.Equal(t, int64(6), offset)
+	var bytes = make([]byte, 5)
+	n, err := reader.Read(bytes)
+	require.NoError(t, err)
+	assert.Equal(t, 5, n)
+	assert.Equal(t, "world", string(bytes))
+}
+
+func TestByteReadSeeker_Seek_OutOfBounds(t *testing.T) {
+	inString := "hello world"
+	reader := NewByteReadSeeker([]byte(inString))
+	_, err := reader.Seek(12, io.SeekStart)
+	assert.Error(t, err)
+	_, err = reader.Seek(-1, io.SeekStart)
+	assert.Error(t, err)
+}

util/io/files/util.go

@@ -0,0 +1,35 @@
+package files
+
+import (
+	"io/fs"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// Inbound will validate if the given candidate path is inside the
+// baseDir. This is useful to make sure that malicious candidates
+// are not targeting a file outside of baseDir boundaries.
+// Considerations:
+// - baseDir must be absolute path. Will return false otherwise
+// - candidate can be absolute or relative path
+// - candidate should not be symlink as only syntatic validation is
+// applied by this function
+func Inbound(candidate, baseDir string) bool {
+	if !filepath.IsAbs(baseDir) {
+		return false
+	}
+	var target string
+	if filepath.IsAbs(candidate) {
+		target = filepath.Clean(candidate)
+	} else {
+		target = filepath.Join(baseDir, candidate)
+	}
+	return strings.HasPrefix(target, filepath.Clean(baseDir)+string(os.PathSeparator))
+}
+
+// IsSymlink return true if the given FileInfo relates to a
+// symlink file. Returns false otherwise.
+func IsSymlink(fi os.FileInfo) bool {
+	return fi.Mode()&fs.ModeSymlink == fs.ModeSymlink
+}

util/io/files/util_test.go

@@ -0,0 +1,63 @@
+package files_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/argoproj/argo-cd/v2/util/io/files"
+)
+
+func TestInbound(t *testing.T) {
+	type testcase struct {
+		name      string
+		candidate string
+		basedir   string
+		expected  bool
+	}
+	cases := []testcase{
+		{
+			name:      "will return true if candidate is inbound",
+			candidate: "/home/test/app/readme.md",
+			basedir:   "/home/test",
+			expected:  true,
+		},
+		{
+			name:      "will return false if candidate is not inbound",
+			candidate: "/home/test/../readme.md",
+			basedir:   "/home/test",
+			expected:  false,
+		},
+		{
+			name:      "will return true if candidate is relative inbound",
+			candidate: "./readme.md",
+			basedir:   "/home/test",
+			expected:  true,
+		},
+		{
+			name:      "will return false if candidate is relative outbound",
+			candidate: "../readme.md",
+			basedir:   "/home/test",
+			expected:  false,
+		},
+		{
+			name:      "will return false if basedir is relative",
+			candidate: "/home/test/app/readme.md",
+			basedir:   "./test",
+			expected:  false,
+		},
+	}
+	for _, c := range cases {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			// given
+			t.Parallel()
+
+			// when
+			inbound := files.Inbound(c.candidate, c.basedir)
+
+			// then
+			assert.Equal(t, c.expected, inbound)
+		})
+	}
+}

util/io/path/resolved.go

@@ -11,6 +11,7 @@ import (
 )
 
 // ResolvedFilePath represents a resolved file path and intended to prevent unintentional use of not verified file path.
+// It is always either a URL or an absolute path.
 type ResolvedFilePath string
 
 // resolveSymbolicLinkRecursive resolves the symlink path recursively to its

util/lua/custom_actions_test.go

@@ -7,9 +7,6 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-	"time"
-
-	"github.com/undefinedlabs/go-mpatch"
 
 	"github.com/argoproj/gitops-engine/pkg/diff"
 	"github.com/ghodss/yaml"
@@ -87,13 +84,9 @@ func TestLuaResourceActionsScript(t *testing.T) {
 				action, err := vm.GetResourceAction(obj, test.Action)
 				assert.NoError(t, err)
 
-				// freeze time so that lua test has predictable time output (will return 0001-01-01T00:00:00Z)
-				patch, err := mpatch.PatchMethod(time.Now, func() time.Time { return time.Time{} })
 				assert.NoError(t, err)
 				result, err := vm.ExecuteResourceAction(obj, action.ActionLua)
 				assert.NoError(t, err)
-				err = patch.Unpatch()
-				assert.NoError(t, err)
 
 				expectedObj := getObj(filepath.Join(dir, test.ExpectedOutputPath))
 				// Ideally, we would use a assert.Equal to detect the difference, but the Lua VM returns a object with float64 instead of the original int32.  As a result, the assert.Equal is never true despite that the change has been applied.

util/oidc/oidc.go

@@ -144,7 +144,12 @@ func (a *ClientApp) oauth2Config(scopes []string) (*oauth2.Config, error) {
 
 // generateAppState creates an app state nonce
 func (a *ClientApp) generateAppState(returnURL string, w http.ResponseWriter) (string, error) {
-	randStr := rand.RandString(10)
+	// According to the spec (https://www.rfc-editor.org/rfc/rfc6749#section-10.10), this must be guessable with
+	// probability <= 2^(-128). The following call generates one of 52^24 random strings, ~= 2^136 possibilities.
+	randStr, err := rand.String(24)
+	if err != nil {
+		return "", fmt.Errorf("failed to generate app state: %w", err)
+	}
 	if returnURL == "" {
 		returnURL = a.baseHRef
 	}
@@ -283,7 +288,12 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) {
 	case GrantTypeAuthorizationCode:
 		url = oauth2Config.AuthCodeURL(stateNonce, opts...)
 	case GrantTypeImplicit:
-		url = ImplicitFlowURL(oauth2Config, stateNonce, opts...)
+		url, err = ImplicitFlowURL(oauth2Config, stateNonce, opts...)
+		if err != nil {
+			log.Errorf("Failed to initiate implicit login flow: %v", err)
+			http.Error(w, "Failed to initiate implicit login flow", http.StatusInternalServerError)
+			return
+		}
 	default:
 		http.Error(w, fmt.Sprintf("Unsupported grant type: %v", grantType), http.StatusInternalServerError)
 		return
@@ -415,10 +425,14 @@ func (a *ClientApp) handleImplicitFlow(r *http.Request, w http.ResponseWriter, s
 
 // ImplicitFlowURL is an adaptation of oauth2.Config::AuthCodeURL() which returns a URL
 // appropriate for an OAuth2 implicit login flow (as opposed to authorization code flow).
-func ImplicitFlowURL(c *oauth2.Config, state string, opts ...oauth2.AuthCodeOption) string {
+func ImplicitFlowURL(c *oauth2.Config, state string, opts ...oauth2.AuthCodeOption) (string, error) {
 	opts = append(opts, oauth2.SetAuthURLParam("response_type", "id_token"))
-	opts = append(opts, oauth2.SetAuthURLParam("nonce", rand.RandString(10)))
-	return c.AuthCodeURL(state, opts...)
+	randString, err := rand.String(24)
+	if err != nil {
+		return "", fmt.Errorf("failed to generate nonce for implicit flow URL: %w", err)
+	}
+	opts = append(opts, oauth2.SetAuthURLParam("nonce", randString))
+	return c.AuthCodeURL(state, opts...), nil
 }
 
 // OfflineAccess returns whether or not 'offline_access' is a supported scope

util/rand/rand.go

@@ -1,37 +1,30 @@
 package rand
 
 import (
-	"math/rand"
-	"time"
+	"crypto/rand"
+	"fmt"
+	"math/big"
 )
 
 const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
-const (
-	letterIdxBits = 6                    // 6 bits to represent a letter index
-	letterIdxMask = 1<<letterIdxBits - 1 // All 1-bits, as many as letterIdxBits
-	letterIdxMax  = 63 / letterIdxBits   // # of letter indices fitting in 63 bits
-)
 
-var src = rand.NewSource(time.Now().UnixNano())
-
-// RandString generates, from a given charset, a cryptographically-secure pseudo-random string of a given length.
-func RandString(n int) string {
-	return RandStringCharset(n, letterBytes)
+// String generates, from the set of capital and lowercase letters, a cryptographically-secure pseudo-random string of a given length.
+func String(n int) (string, error) {
+	return StringFromCharset(n, letterBytes)
 }
 
-func RandStringCharset(n int, charset string) string {
+// StringFromCharset generates, from a given charset, a cryptographically-secure pseudo-random string of a given length.
+func StringFromCharset(n int, charset string) (string, error) {
 	b := make([]byte, n)
-	// A src.Int63() generates 63 random bits, enough for letterIdxMax characters!
-	for i, cache, remain := n-1, src.Int63(), letterIdxMax; i >= 0; {
-		if remain == 0 {
-			cache, remain = src.Int63(), letterIdxMax
+	maxIdx := big.NewInt(int64(len(charset)))
+	for i := 0; i < n; i++ {
+		randIdx, err := rand.Int(rand.Reader, maxIdx)
+		if err != nil {
+			return "", fmt.Errorf("failed to generate random string: %w", err)
 		}
-		if idx := int(cache & letterIdxMask); idx < len(charset) {
-			b[i] = charset[idx]
-			i--
-		}
-		cache >>= letterIdxBits
-		remain--
+		// randIdx is necessarily safe to convert to int, because the max came from an int.
+		randIdxInt := int(randIdx.Int64())
+		b[i] = charset[randIdxInt]
 	}
-	return string(b)
+	return string(b), nil
 }

util/rand/rand_test.go

@@ -2,15 +2,17 @@ package rand
 
 import (
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestRandString(t *testing.T) {
-	ss := RandStringCharset(10, "A")
-	if ss != "AAAAAAAAAA" {
-		t.Errorf("Expected 10 As, but got %q", ss)
-	}
-	ss = RandStringCharset(5, "ABC123")
-	if len(ss) != 5 {
-		t.Errorf("Expected random string of length 10, but got %q", ss)
-	}
+	ss, err := StringFromCharset(10, "A")
+	require.NoError(t, err)
+	assert.Equal(t, "AAAAAAAAAA", ss)
+
+	ss, err = StringFromCharset(5, "ABC123")
+	require.NoError(t, err)
+	assert.Len(t, ss, 5)
 }