Bump version to 2.2.10

chore: fix docs gen
2026-03-02 06:28:47 +01:00 · 2022-06-21 16:27:37 +00:00 · 2022-06-21 16:27:23 +00:00 · 2022-06-21 10:46:49 -04:00 · 2022-06-21 09:40:36 -04:00 · 2022-06-21 09:39:56 -04:00
84 changed files with 1280 additions and 430 deletions
--- a/.github/workflows/ci-build.yaml
+++ b/.github/workflows/ci-build.yaml
@@ -72,10 +72,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
--- a/.golangci.yml
+++ b/.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
--- a/2
+++ b/2
@@ -1 +1 @@
-2.2.9
+2.2.10
--- a/cmd/argocd-repo-server/commands/argocd_repo_server.go
+++ b/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"
@@ -63,14 +64,15 @@ func getPauseGenerationOnFailureForRequests() int {

 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,
@@ -90,13 +92,17 @@ func NewCommand() *cobra.Command {
 			cache, err := cacheSrc()
 			errors.CheckError(err)

+			maxCombinedDirectoryManifestsQuantity, err := resource.ParseQuantity(maxCombinedDirectoryManifestsSize)
+			errors.CheckError(err)
+
 			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(),
+				MaxCombinedDirectoryManifestsSize:            maxCombinedDirectoryManifestsQuantity,
 			})
 			errors.CheckError(err)

@@ -160,6 +166,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) {
--- a/cmd/argocd/commands/admin/project_allowlist.go
+++ b/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
 }
--- a/cmd/argocd/commands/admin/project_allowlist_test.go
+++ b/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)
 }
--- a/cmd/argocd/commands/app.go
+++ b/cmd/argocd/commands/app.go
@@ -27,6 +27,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"
@@ -766,7 +767,7 @@ func getLocalObjectsString(app *argoappv1.Application, local, localRepoRoot, app
 		KubeVersion:       kubeVersion,
 		Plugins:           configManagementPlugins,
 		TrackingMethod:    trackingMethod,
-	}, true)
+	}, true, resource.MustParse("0"))
 	errors.CheckError(err)

 	return res.Manifests
--- a/cmd/argocd/commands/login.go
+++ b/cmd/argocd/commands/login.go
@@ -202,7 +202,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

@@ -212,7 +215,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[:])

@@ -296,7 +300,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)
 	}
--- a/controller/sync.go
+++ b/controller/sync.go
@@ -137,7 +137,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)
--- a/docs/operator-manual/argocd-cmd-params-cm.yaml
+++ b/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'
--- a/docs/operator-manual/security.md
+++ b/docs/operator-manual/security.md
@@ -205,4 +205,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.
--- a/docs/operator-manual/server-commands/argocd-repo-server.md
+++ b/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")
 ```

--- a/go.mod
+++ b/go.mod
@@ -61,7 +61,6 @@ require (
 	github.com/spf13/cobra v1.1.3
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.0
-	github.com/undefinedlabs/go-mpatch v1.0.6
 	github.com/yuin/gopher-lua v0.0.0-20200816102855-ee81675732da
 	golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e
 	golang.org/x/net v0.0.0-20210614182718-04defd469f4e
--- a/go.sum
+++ b/go.sum
@@ -840,8 +840,6 @@ github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
-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=
--- a/hack/installers/install-lint-tools.sh
+++ b/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
--- a/manifests/base/kustomization.yaml
+++ b/manifests/base/kustomization.yaml
@@ -5,7 +5,7 @@ kind: Kustomization
 images:
 - name: quay.io/argoproj/argocd
  newName: quay.io/argoproj/argocd
-  newTag: v2.2.9
+  newTag: v2.2.10
 resources:
 - ./application-controller
 - ./dex
--- a/manifests/base/repo-server/argocd-repo-server-deployment.yaml
+++ b/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
--- a/manifests/core-install.yaml
+++ b/manifests/core-install.yaml
@@ -3012,13 +3012,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.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -3067,7 +3073,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        name: copyutil
        volumeMounts:
        - mountPath: /var/run/argocd
@@ -3232,7 +3238,7 @@ spec:
              key: controller.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
--- a/manifests/core-install/kustomization.yaml
+++ b/manifests/core-install/kustomization.yaml
@@ -11,4 +11,4 @@ resources:
 images:
 - name: quay.io/argoproj/argocd
  newName: quay.io/argoproj/argocd
-  newTag: v2.2.9
+  newTag: v2.2.10
--- a/manifests/ha/base/kustomization.yaml
+++ b/manifests/ha/base/kustomization.yaml
@@ -11,7 +11,7 @@ patchesStrategicMerge:
 images:
 - name: quay.io/argoproj/argocd
  newName: quay.io/argoproj/argocd
-  newTag: v2.2.9
+  newTag: v2.2.10
 resources:
 - ../../base/application-controller
 - ../../base/dex
--- a/manifests/ha/install.yaml
+++ b/manifests/ha/install.yaml
@@ -3709,7 +3709,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        name: copyutil
        volumeMounts:
@@ -3920,13 +3920,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.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -3975,7 +3981,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        name: copyutil
        volumeMounts:
        - mountPath: /var/run/argocd
@@ -4202,7 +4208,7 @@ spec:
              key: server.http.cookie.maxnumber
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -4398,7 +4404,7 @@ spec:
              key: controller.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
--- a/manifests/ha/namespace-install.yaml
+++ b/manifests/ha/namespace-install.yaml
@@ -1068,7 +1068,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        name: copyutil
        volumeMounts:
@@ -1279,13 +1279,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.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -1334,7 +1340,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        name: copyutil
        volumeMounts:
        - mountPath: /var/run/argocd
@@ -1561,7 +1567,7 @@ spec:
              key: server.http.cookie.maxnumber
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -1757,7 +1763,7 @@ spec:
              key: controller.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
--- a/manifests/install.yaml
+++ b/manifests/install.yaml
@@ -3079,7 +3079,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        name: copyutil
        volumeMounts:
@@ -3254,13 +3254,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.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -3309,7 +3315,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        name: copyutil
        volumeMounts:
        - mountPath: /var/run/argocd
@@ -3532,7 +3538,7 @@ spec:
              key: server.http.cookie.maxnumber
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -3722,7 +3728,7 @@ spec:
              key: controller.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
--- a/manifests/namespace-install.yaml
+++ b/manifests/namespace-install.yaml
@@ -438,7 +438,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        name: copyutil
        volumeMounts:
@@ -613,13 +613,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.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -668,7 +674,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        name: copyutil
        volumeMounts:
        - mountPath: /var/run/argocd
@@ -891,7 +897,7 @@ spec:
              key: server.http.cookie.maxnumber
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -1081,7 +1087,7 @@ spec:
              key: controller.default.cache.expiration
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.2.9
+        image: quay.io/argoproj/argocd:v2.2.10
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
--- a/pkg/apiclient/grpcproxy.go
+++ b/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 {
--- a/reposerver/repository/repository.go
+++ b/reposerver/repository/repository.go
@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	goio "io"
 	"io/ioutil"
 	"net/url"
 	"os"
@@ -16,6 +17,10 @@ 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/argoproj/argo-cd/v2/util/argo"
@@ -68,6 +73,8 @@ const (
 	ociPrefix                      = "oci://"
 )

+var ErrExceededMaxCombinedManifestFileSize = errors.New("exceeded max combined manifest file size")
+
 // Service implements ManifestService interface
 type Service struct {
 	repoLock                  *repositoryLock
@@ -87,6 +94,7 @@ type RepoServerInitConstants struct {
 	PauseGenerationAfterFailedGenerationAttempts int
 	PauseGenerationOnFailureForMinutes           int
 	PauseGenerationOnFailureForRequests          int
+	MaxCombinedDirectoryManifestsSize            resource.Quantity
 }

 // NewService returns a new instance of the Manifest service
@@ -331,7 +339,7 @@ func (s *Service) runManifestGen(repoRoot, commitSHA, cacheKey string, ctxSrc op
 	var manifestGenResult *apiclient.ManifestResponse
 	ctx, err := ctxSrc()
 	if err == nil {
-		manifestGenResult, err = GenerateManifests(ctx.appPath, repoRoot, commitSHA, q, false)
+		manifestGenResult, err = GenerateManifests(ctx.appPath, repoRoot, commitSHA, q, false, s.initConstants.MaxCombinedDirectoryManifestsSize)
 	}
 	if err != nil {

@@ -706,7 +714,7 @@ func getRepoCredential(repoCredentials []*v1alpha1.RepoCreds, repoURL string) *v
 }

 // GenerateManifests generates manifests from a path
-func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool) (*apiclient.ManifestResponse, error) {
+func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool, maxCombinedManifestQuantity resource.Quantity) (*apiclient.ManifestResponse, error) {
 	var targetObjs []*unstructured.Unstructured
 	var dest *v1alpha1.ApplicationDestination

@@ -755,7 +763,7 @@ func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.Manifest
 			directory = &v1alpha1.ApplicationSourceDirectory{}
 		}
 		logCtx := log.WithField("application", q.AppName)
-		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory)
+		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory, maxCombinedManifestQuantity)
 	}
 	if err != nil {
 		return nil, err
@@ -953,60 +961,27 @@ 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(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory) ([]*unstructured.Unstructured, error) {
+func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, 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
-		}
-		relPath, err := filepath.Rel(appPath, path)
-		if err != nil {
-			return fmt.Errorf("failed to get relative path of symlink: %w", err)
-		}
-		if files.IsSymlink(f) {
-			realPath, err := filepath.EvalSymlinks(path)
-			if err != nil {
-				logCtx.Debugf("error checking symlink realpath: %s", err)
-				if os.IsNotExist(err) {
-					log.Warnf("ignoring out-of-bounds symlink at %q: %s", relPath, err)
-					return nil
-				} else {
-					return fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
-				}
-			}
-			if !files.Inbound(realPath, appPath) {
-				logCtx.Warnf("illegal filepath in symlink: %s", realPath)
-				return fmt.Errorf("illegal filepath in symlink at %q", relPath)
-			}
-		}
-		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
-		}
-
-		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") {
 			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
@@ -1018,49 +993,207 @@ func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1al
 				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) {
@@ -1356,8 +1489,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
@@ -1365,10 +1502,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
 	}
@@ -1387,15 +1524,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
--- a/reposerver/repository/repository_norace_test.go
+++ b/reposerver/repository/repository_norace_test.go
@@ -1,46 +0,0 @@
-// +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()
-}
--- a/reposerver/repository/repository_test.go
+++ b/reposerver/repository/repository_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -16,6 +17,7 @@ import (
 	"time"

 	log "github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/api/resource"

 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
@@ -133,7 +135,7 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &src}

 	// update this value if we add/remove manifests
-	const countOfManifests = 41
+	const countOfManifests = 34

 	res1, err := service.GenerateManifest(context.Background(), &q)

@@ -141,7 +143,7 @@ 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("./testdata/concatenated", "/", "", &q, false)
+	res2, err := GenerateManifests("./testdata/concatenated", "/", "", &q, false, resource.MustParse("0"))
 	assert.NoError(t, err)
 	assert.Equal(t, 3, len(res2.Manifests))
 }
@@ -197,7 +199,7 @@ func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
 			}

 			q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
-			res, err := GenerateManifests(repoDir, "", "", &q, false)
+			res, err := GenerateManifests(repoDir, "", "", &q, false, resource.MustParse("0"))
 			require.Error(t, err)
 			assert.NotContains(t, err.Error(), mustNotContain)
 			assert.Contains(t, err.Error(), "illegal filepath")
@@ -212,7 +214,7 @@ func TestGenerateManifests_MissingSymlinkDestination(t *testing.T) {
 	require.NoError(t, err)

 	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
-	_, err = GenerateManifests(repoDir, "", "", &q, false)
+	_, err = GenerateManifests(repoDir, "", "", &q, false, resource.MustParse("0"))
 	require.NoError(t, err)
 }

@@ -369,29 +371,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
@@ -1122,7 +1101,7 @@ func TestGenerateFromUTF16(t *testing.T) {
 		Repo:              &argoappv1.Repository{},
 		ApplicationSource: &argoappv1.ApplicationSource{},
 	}
-	res1, err := GenerateManifests("./testdata/utf-16", "/", "", &q, false)
+	res1, err := GenerateManifests("./testdata/utf-16", "/", "", &q, false, resource.MustParse("0"))
 	assert.Nil(t, err)
 	assert.Equal(t, 2, len(res1.Manifests))
 }
@@ -1138,12 +1117,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)
 }
@@ -1482,6 +1464,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) {
@@ -1687,7 +1670,7 @@ func TestFindResources(t *testing.T) {
 				Recurse: true,
 				Include: tc.include,
 				Exclude: tc.exclude,
-			})
+			}, resource.MustParse("0"))
 			if !assert.NoError(t, err) {
 				return
 			}
@@ -1704,7 +1687,7 @@ func TestFindManifests_Exclude(t *testing.T) {
 	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "subdir/deploymentSub.yaml",
-	})
+	}, resource.MustParse("0"))

 	if !assert.NoError(t, err) || !assert.Len(t, objs, 1) {
 		return
@@ -1717,7 +1700,7 @@ func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
 	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "nothing.yaml",
-	})
+	}, resource.MustParse("0"))

 	if !assert.NoError(t, err) || !assert.Len(t, objs, 2) {
 		return
@@ -1727,6 +1710,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, 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, 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, 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, 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, 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, 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, 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, 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, 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, resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, 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, 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, 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, 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, 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, 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, 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, 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, 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, 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{
@@ -1750,3 +2196,52 @@ func Test_getHelmDependencyRepos(t *testing.T) {
 	assert.Equal(t, repos[0].Repo, repo1)
 	assert.Equal(t, repos[1].Repo, repo2)
 }
+
+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)
+	})
+}
--- a/reposerver/repository/testdata/circular-link/a.json
+++ b/reposerver/repository/testdata/circular-link/a.json
@@ -0,0 +1 @@
+b.json
--- a/reposerver/repository/testdata/circular-link/b.json
+++ b/reposerver/repository/testdata/circular-link/b.json
@@ -0,0 +1 @@
+a.json
--- a/reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml
+++ b/reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml
@@ -0,0 +1 @@
+../cm.yaml
--- a/reposerver/repository/testdata/in-bounds-link/cm.yaml
+++ b/reposerver/repository/testdata/in-bounds-link/cm.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ServiceAccount
--- a/reposerver/repository/testdata/in-bounds-values-file-link/Chart.yaml
+++ b/reposerver/repository/testdata/in-bounds-values-file-link/Chart.yaml
@@ -0,0 +1,2 @@
+name: my-chart
+version: 1.1.0
--- a/reposerver/repository/testdata/in-bounds-values-file-link/values-2.yaml
+++ b/reposerver/repository/testdata/in-bounds-values-file-link/values-2.yaml
@@ -0,0 +1 @@
+some: yaml
--- a/reposerver/repository/testdata/in-bounds-values-file-link/values.yaml
+++ b/reposerver/repository/testdata/in-bounds-values-file-link/values.yaml
@@ -0,0 +1 @@
+values-2.yaml
--- a/reposerver/repository/testdata/invalid-json/invalid.json
+++ b/reposerver/repository/testdata/invalid-json/invalid.json
@@ -0,0 +1 @@
+[
--- a/reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml
+++ b/reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml
@@ -0,0 +1 @@
+some: [irrelevant, yaml]
--- a/reposerver/repository/testdata/irrelevant-yaml/relevant.yaml
+++ b/reposerver/repository/testdata/irrelevant-yaml/relevant.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap
--- a/reposerver/repository/testdata/json-list/list.json
+++ b/reposerver/repository/testdata/json-list/list.json
@@ -0,0 +1,2 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/jsonnet-and-json/test.json
+++ b/reposerver/repository/testdata/jsonnet-and-json/test.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "Pod"}
--- a/reposerver/repository/testdata/jsonnet-and-json/test.jsonnet
+++ b/reposerver/repository/testdata/jsonnet-and-json/test.jsonnet
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "Pod"}
--- a/reposerver/repository/testdata/link-to-nowhere/nowhere.json
+++ b/reposerver/repository/testdata/link-to-nowhere/nowhere.json
@@ -0,0 +1 @@
+nowhere
--- a/reposerver/repository/testdata/non-manifest-file/not-json-or-yaml
+++ b/reposerver/repository/testdata/non-manifest-file/not-json-or-yaml
--- a/reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json
+++ b/reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json
@@ -0,0 +1 @@
+../out-of-bounds.json
--- a/reposerver/repository/testdata/out-of-bounds-values-file-link/Chart.yaml
+++ b/reposerver/repository/testdata/out-of-bounds-values-file-link/Chart.yaml
@@ -0,0 +1,2 @@
+name: my-chart
+version: 1.1.0
--- a/reposerver/repository/testdata/out-of-bounds-values-file-link/values.yaml
+++ b/reposerver/repository/testdata/out-of-bounds-values-file-link/values.yaml
@@ -0,0 +1 @@
+../out-of-bounds.yaml
--- a/reposerver/repository/testdata/out-of-bounds.json
+++ b/reposerver/repository/testdata/out-of-bounds.json
--- a/reposerver/repository/testdata/out-of-bounds.yaml
+++ b/reposerver/repository/testdata/out-of-bounds.yaml
@@ -0,0 +1 @@
+some: yaml
--- a/reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml
+++ b/reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml
@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ConfigMap
+---
+invalid:
--- a/reposerver/repository/testdata/several-files/0.json
+++ b/reposerver/repository/testdata/several-files/0.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/several-files/0.yaml
+++ b/reposerver/repository/testdata/several-files/0.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap
--- a/reposerver/repository/testdata/several-files/1.json
+++ b/reposerver/repository/testdata/several-files/1.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/several-files/1.yaml
+++ b/reposerver/repository/testdata/several-files/1.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap
--- a/reposerver/repository/testdata/several-files/2.json
+++ b/reposerver/repository/testdata/several-files/2.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/several-files/2.yaml
+++ b/reposerver/repository/testdata/several-files/2.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap
--- a/reposerver/repository/testdata/several-files/3.json
+++ b/reposerver/repository/testdata/several-files/3.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/several-files/3.yaml
+++ b/reposerver/repository/testdata/several-files/3.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap
--- a/reposerver/repository/testdata/several-files/4.json
+++ b/reposerver/repository/testdata/several-files/4.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/several-files/4.yaml
+++ b/reposerver/repository/testdata/several-files/4.yaml
@@ -0,0 +1,2 @@
+apiVersion: v1
+kind: ConfigMap
--- a/reposerver/repository/testdata/several-files/README.md
+++ b/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.
--- a/reposerver/repository/testdata/valid-json/valid.json
+++ b/reposerver/repository/testdata/valid-json/valid.json
@@ -0,0 +1 @@
+{"apiVersion": "v1", "kind": "ConfigMap"}
--- a/reposerver/repository/testdata/values-files/Chart.yaml
+++ b/reposerver/repository/testdata/values-files/Chart.yaml
@@ -0,0 +1,2 @@
+name: my-chart
+version: 1.1.0
--- a/reposerver/repository/testdata/values-files/caps-extn-values.YAML
+++ b/reposerver/repository/testdata/values-files/caps-extn-values.YAML
--- a/reposerver/repository/testdata/values-files/exclude.yaml
+++ b/reposerver/repository/testdata/values-files/exclude.yaml
@@ -0,0 +1 @@
+exclude: yaml
--- a/reposerver/repository/testdata/values-files/has-the-word-values.yaml
+++ b/reposerver/repository/testdata/values-files/has-the-word-values.yaml
@@ -0,0 +1,4 @@
+has:
+  the:
+    word:
+      values: yaml
--- a/reposerver/repository/testdata/values-files/values.yaml
+++ b/reposerver/repository/testdata/values-files/values.yaml
@@ -0,0 +1 @@
+values: yaml
--- a/reposerver/repository/testdata/values-files/values.yml
+++ b/reposerver/repository/testdata/values-files/values.yml
--- a/reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml
+++ b/reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml
@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ConfigMap
+---
+---
--- a/server/server.go
+++ b/server/server.go
@@ -950,6 +950,7 @@ func (a *ArgoCDServer) Authenticate(ctx context.Context) (context.Context, error
 		if !argoCDSettings.AnonymousUserEnabled {
 			return ctx, claimsErr
 		} else {
+			// nolint:staticcheck
 			ctx = context.WithValue(ctx, "claims", "")
 		}
 	}
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -504,7 +504,7 @@ func getTestServer(t *testing.T, anonymousEnabled bool, withFakeSSO bool) (argoc
 		cm.Data["users.anonymous.enabled"] = "true"
 	}
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		return // Start with a placeholder. We need the server URL before setting up the real handler.
+		// 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)
@@ -597,12 +597,11 @@ func TestAuthenticate_3rd_party_JWTs(t *testing.T) {
 			t.Parallel()

 			argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, true)
-			ctx := context.Background()
 			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 := metadata.NewIncomingContext(context.Background(), metadata.Pairs(apiclient.MetaDataTokenKey, tokenString))

 			ctx, err = argocd.Authenticate(ctx)
 			claims := ctx.Value("claims")
@@ -692,11 +691,10 @@ func TestAuthenticate_no_SSO(t *testing.T) {
 			t.Parallel()

 			argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, false)
-			ctx := context.Background()
 			token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.StandardClaims{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 := metadata.NewIncomingContext(context.Background(), metadata.Pairs(apiclient.MetaDataTokenKey, tokenString))

 			ctx, err = argocd.Authenticate(ctx)
 			claims := ctx.Value("claims")
@@ -798,8 +796,7 @@ func TestAuthenticate_bad_request_metadata(t *testing.T) {
 			t.Parallel()

 			argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true)
-			ctx := context.Background()
-			ctx = metadata.NewIncomingContext(context.Background(), testDataCopy.metadata)
+			ctx := metadata.NewIncomingContext(context.Background(), testDataCopy.metadata)

 			ctx, err := argocd.Authenticate(ctx)
 			claims := ctx.Value("claims")
--- a/test/e2e/fixture/fixture.go
+++ b/test/e2e/fixture/fixture.go
@@ -543,7 +543,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)
--- a/test/e2e/selective_sync_test.go
+++ b/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)
 }
--- a/ui/src/app/applications/components/application-urls.test.ts
+++ b/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');
+});
--- a/ui/src/app/applications/components/application-urls.tsx
+++ b/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
--- a/util/helm/helm.go
+++ b/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)
--- a/util/helm/helm_test.go
+++ b/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) {
--- a/util/helm/testdata/dependency/Chart.lock
+++ b/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"
--- a/util/io/bytereadseeker.go
+++ b/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
--- a/util/io/bytereadseeker_test.go
+++ b/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)
+}
--- a/util/io/path/resolved.go
+++ b/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
--- a/util/lua/custom_actions_test.go
+++ b/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.
--- a/util/oidc/oidc.go
+++ b/util/oidc/oidc.go
@@ -152,17 +152,23 @@ func (a *ClientApp) oauth2Config(scopes []string) (*oauth2.Config, error) {
 }

 // generateAppState creates an app state nonce
-func (a *ClientApp) generateAppState(returnURL string) string {
-	randStr := rand.RandString(10)
+func (a *ClientApp) generateAppState(returnURL string) (string, error) {
+	// 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
 	}
-	err := a.cache.SetOIDCState(randStr, &OIDCState{ReturnURL: returnURL})
+	err = a.cache.SetOIDCState(randStr, &OIDCState{ReturnURL: returnURL})
 	if err != nil {
 		// This should never happen with the in-memory cache
 		log.Errorf("Failed to set app state: %v", err)
+		return "", fmt.Errorf("failed to set app state: %v", err)
 	}
-	return randStr
+	return randStr, nil
 }

 func (a *ClientApp) verifyAppState(state string) (*OIDCState, error) {
@@ -251,14 +257,24 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Invalid redirect URL: the protocol and host (including port) must match and the path must be within allowed URLs if provided", http.StatusBadRequest)
 		return
 	}
-	stateNonce := a.generateAppState(returnURL)
+	stateNonce, err := a.generateAppState(returnURL)
+	if err != nil {
+		log.Errorf("Failed to initiate login flow: %v", err)
+		http.Error(w, "Failed to initiate login flow", http.StatusInternalServerError)
+		return
+	}
 	grantType := InferGrantType(oidcConf)
 	var url string
 	switch grantType {
 	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
@@ -390,10 +406,14 @@ func (a *ClientApp) handleImplicitFlow(w http.ResponseWriter, state string) {

 // 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
--- a/util/rand/rand.go
+++ b/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
 }
--- a/util/rand/rand_test.go
+++ b/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)
 }
Author	SHA1	Message	Date
argo-bot	8db0e57b73	Bump version to 2.2.10	2022-06-21 16:27:37 +00:00
argo-bot	80a10d4185	Bump version to 2.2.10	2022-06-21 16:27:23 +00:00
Michael Crenshaw	29521a9aa4	chore: fix docs gen Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-21 10:46:49 -04:00
Michael Crenshaw	58cccd526e	Merge pull request from GHSA-jhqp-vf4w-rpwq Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> defer instead of multiple close calls Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> oops Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> don't count jsonnet against max Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix codegen Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> add caveat about 300x ratio Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix versions Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix tests/lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-21 09:40:36 -04:00
Michael Crenshaw	265a64409e	Merge pull request from GHSA-q4w5-4gq2-98vm Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-21 09:39:56 -04:00
Michael Crenshaw	1fe95747c4	Merge pull request from GHSA-2m7h-86qq-fp4v Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix references Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> use long enough state param for oauth2 Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> typo Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> more entropy Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix test Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-21 09:39:01 -04:00
Michael Crenshaw	05e9079233	Merge pull request from GHSA-h4w9-6x78-8vrj Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-21 09:36:38 -04:00
Michael Crenshaw	fd42ba766d	fix: missing Helm params (#9565 ) (#9566 ) * fix: missing Helm params Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * use absolute paths, fix tests Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix race in test Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-14 22:25:05 -04:00
Michael Crenshaw	4040dee0ee	test: directory app manifest generation (#9503 ) * test: directory app manifest generation Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * git doesn't support empty dirs Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix bad cherry-pick Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-14 16:27:01 -04:00
Michael Crenshaw	845cfdee6f	test: fix erroneous test change Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-13 18:00:09 -04:00
Michael Crenshaw	f4d17fff60	chore: eliminate go-mpatch dependency (#9045 ) * chore: eliminate go-mpatch dependency Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: abstract out resource list function Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: don't exit the program in anything but the main function Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: better error messages Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: better error messages Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-13 17:58:56 -04:00
jannfis	a9238104c0	chore: Make unit tests run on platforms other than amd64 (#8995 ) Signed-off-by: jannfis <jann@mistrust.net> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-13 17:54:52 -04:00
Alexander Matyushentsev	dc8785ee1b	chore: remove obsolete repo-server unit test (#9559 ) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>	2022-06-13 17:54:31 -04:00
Tommaso Sardelli	ad778e87bb	chore: upgrade golangci-lint to v1.46.2 (#9448 ) * chore: upgrade golangci-lint to v1.46.2 Because: * Installation of golangci-lint v1.45.2 is currently broken and fails silently due to a redacted dependency (https://github.com/blizzy78/varnamelen/issues/13) This commit: * Upgrades golangci-lint to v1.46.2 Signed-off-by: Tommaso Sardelli <lacapannadelloziotom@gmail.com> * fix: lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix: lint Signed-off-by: Tommaso Sardelli <lacapannadelloziotom@gmail.com> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-13 17:49:49 -04:00
Michael Crenshaw	273bc30a2a	chore: update golangci-lint (#8988 ) * chore: update golangci-lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>	2022-06-13 17:37:59 -04:00
@@ -1 +1 @@
 .2.9
 .2.10
				`@@ -0,0 +1 @@`
				`This file shouldn't be counted in the manifest file size limit, because it isn't JSON or YAML.`