Bump version to 2.4.2

ubuntu:22.04 bumped the ppc64el baseline to POWER9, rendering this
unusable on existing POWER8 systems.  This allows customization similar
to the top-level Dockerfile.

Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>

Dockerfile

@@ -63,7 +63,7 @@ RUN ln -s /usr/local/bin/entrypoint.sh /usr/local/bin/uid_entrypoint.sh
 # support for mounting configuration from a configmap
 WORKDIR /app/config/ssh
 RUN touch ssh_known_hosts && \
-    ln -s ssh_known_hosts /etc/ssh/ssh_known_hosts 
+    ln -s /app/config/ssh/ssh_known_hosts /etc/ssh/ssh_known_hosts
 
 WORKDIR /app/config
 RUN mkdir -p tls && \

VERSION

@@ -1 +1 @@
-2.4.0
+2.4.2

cmd/argocd-k8s-auth/commands/aws.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"context"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -40,16 +41,7 @@ func newAWSCommand() *cobra.Command {
 	var command = &cobra.Command{
 		Use: "aws",
 		Run: func(c *cobra.Command, args []string) {
-			sess, err := session.NewSession()
-			errors.CheckError(err)
-			stsAPI := sts.New(sess)
-			if roleARN != "" {
-				creds := stscreds.NewCredentials(sess, roleARN)
-				stsAPI = sts.New(sess, &aws.Config{Credentials: creds})
-			}
-			request, _ := stsAPI.GetCallerIdentityRequest(&sts.GetCallerIdentityInput{})
-			request.HTTPRequest.Header.Add(clusterIDHeader, clusterName)
-			presignedURLString, err := request.Presign(requestPresignParam)
+			presignedURLString, err := getSignedRequestWithRetry(time.Minute, 5*time.Second, clusterName, roleARN, getSignedRequest)
 			errors.CheckError(err)
 			token := v1Prefix + base64.RawURLEncoding.EncodeToString([]byte(presignedURLString))
 			// Set token expiration to 1 minute before the presigned URL expires for some cushion
@@ -62,6 +54,43 @@ func newAWSCommand() *cobra.Command {
 	return command
 }
 
+type getSignedRequestFunc func(clusterName, roleARN string) (string, error)
+
+func getSignedRequestWithRetry(timeout, interval time.Duration, clusterName, roleARN string, fn getSignedRequestFunc) (string, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+	for {
+		signed, err := fn(clusterName, roleARN)
+		if err == nil {
+			return signed, nil
+		}
+		select {
+		case <-ctx.Done():
+			return "", fmt.Errorf("timeout while trying to get signed aws request: last error: %s", err)
+		case <-time.After(interval):
+		}
+	}
+}
+
+func getSignedRequest(clusterName, roleARN string) (string, error) {
+	sess, err := session.NewSession()
+	if err != nil {
+		return "", fmt.Errorf("error creating new AWS session: %s", err)
+	}
+	stsAPI := sts.New(sess)
+	if roleARN != "" {
+		creds := stscreds.NewCredentials(sess, roleARN)
+		stsAPI = sts.New(sess, &aws.Config{Credentials: creds})
+	}
+	request, _ := stsAPI.GetCallerIdentityRequest(&sts.GetCallerIdentityInput{})
+	request.HTTPRequest.Header.Add(clusterIDHeader, clusterName)
+	signed, err := request.Presign(requestPresignParam)
+	if err != nil {
+		return "", fmt.Errorf("error presigning AWS request: %s", err)
+	}
+	return signed, nil
+}
+
 func formatJSON(token string, expiration time.Time) string {
 	expirationTimestamp := metav1.NewTime(expiration)
 	execInput := &clientauthv1beta1.ExecCredential{

cmd/argocd-k8s-auth/commands/aws_test.go

@@ -0,0 +1,73 @@
+package commands
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetSignedRequestWithRetry(t *testing.T) {
+	t.Run("will return signed request on first attempt", func(t *testing.T) {
+		// given
+		t.Parallel()
+		mock := &signedRequestMock{
+			returnFunc: func(m *signedRequestMock) (string, error) {
+				return "token", nil
+			},
+		}
+
+		// when
+		signed, err := getSignedRequestWithRetry(time.Second, time.Millisecond, "cluster-name", "", mock.getSignedRequestMock)
+
+		// then
+		assert.NoError(t, err)
+		assert.Equal(t, "token", signed)
+	})
+	t.Run("will return signed request on third attempt", func(t *testing.T) {
+		// given
+		t.Parallel()
+		mock := &signedRequestMock{
+			returnFunc: func(m *signedRequestMock) (string, error) {
+				if m.getSignedRequestCalls < 3 {
+					return "", fmt.Errorf("some error")
+				}
+				return "token", nil
+			},
+		}
+
+		// when
+		signed, err := getSignedRequestWithRetry(time.Second, time.Millisecond, "cluster-name", "", mock.getSignedRequestMock)
+
+		// then
+		assert.NoError(t, err)
+		assert.Equal(t, "token", signed)
+	})
+	t.Run("will return error on timeout", func(t *testing.T) {
+		// given
+		t.Parallel()
+		mock := &signedRequestMock{
+			returnFunc: func(m *signedRequestMock) (string, error) {
+				return "", fmt.Errorf("some error")
+			},
+		}
+
+		// when
+		signed, err := getSignedRequestWithRetry(time.Second, time.Millisecond, "cluster-name", "", mock.getSignedRequestMock)
+
+		// then
+		assert.Error(t, err)
+		assert.Equal(t, "", signed)
+	})
+}
+
+type signedRequestMock struct {
+	getSignedRequestCalls int
+	returnFunc            func(m *signedRequestMock) (string, error)
+}
+
+func (m *signedRequestMock) getSignedRequestMock(clusterName, roleARN string) (string, error) {
+	m.getSignedRequestCalls++
+	return m.returnFunc(m)
+}

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

@@ -14,6 +14,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"
@@ -70,15 +71,16 @@ func getSubmoduleEnabled() bool {
 
 func NewCommand() *cobra.Command {
 	var (
-		parallelismLimit       int64
-		listenPort             int
-		metricsPort            int
-		otlpAddress            string
-		cacheSrc               func() (*reposervercache.Cache, error)
-		tlsConfigCustomizer    tls.ConfigCustomizer
-		tlsConfigCustomizerSrc func() (tls.ConfigCustomizer, error)
-		redisClient            *redis.Client
-		disableTLS             bool
+		parallelismLimit                  int64
+		listenPort                        int
+		metricsPort                       int
+		otlpAddress                       string
+		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,
@@ -98,15 +100,19 @@ func NewCommand() *cobra.Command {
 			cache, err := cacheSrc()
 			errors.CheckError(err)
 
+			maxCombinedDirectoryManifestsQuantity, err := resource.ParseQuantity(maxCombinedDirectoryManifestsSize)
+			errors.CheckError(err)
+
 			askPassServer := askpass.NewServer()
 			metricsServer := metrics.NewMetricsServer()
 			cacheutil.CollectMetrics(redisClient, metricsServer)
 			server, err := reposerver.NewServer(metricsServer, cache, tlsConfigCustomizer, repository.RepoServerInitConstants{
-				ParallelismLimit: parallelismLimit,
+				ParallelismLimit:                             parallelismLimit,
 				PauseGenerationAfterFailedGenerationAttempts: getPauseGenerationAfterFailedGenerationAttempts(),
 				PauseGenerationOnFailureForMinutes:           getPauseGenerationOnFailureForMinutes(),
 				PauseGenerationOnFailureForRequests:          getPauseGenerationOnFailureForRequests(),
 				SubmoduleEnabled:                             getSubmoduleEnabled(),
+				MaxCombinedDirectoryManifestsSize:            maxCombinedDirectoryManifestsQuantity,
 			}, askPassServer)
 			errors.CheckError(err)
 
@@ -182,6 +188,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().IntVar(&metricsPort, "metrics-port", common.DefaultPortRepoServerMetrics, "Start metrics server on given port")
 	command.Flags().StringVar(&otlpAddress, "otlp-address", env.StringFromEnv("ARGOCD_REPO_SERVER_OTLP_ADDRESS", ""), "OpenTelemetry collector address to send traces to")
 	command.Flags().BoolVar(&disableTLS, "disable-tls", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_DISABLE_TLS", false), "Disable TLS on the gRPC endpoint")
+	command.Flags().StringVar(&maxCombinedDirectoryManifestsSize, "max-combined-directory-manifests-size", env.StringFromEnv("ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE", "10M"), "Max combined size of manifest files in a directory-type Application")
 
 	tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(&command)
 	cacheSrc = reposervercache.AddCacheFlagsToCmd(&command, func(client *redis.Client) {

cmd/argocd/commands/app.go

@@ -25,6 +25,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"
@@ -778,7 +779,7 @@ func getLocalObjectsString(app *argoappv1.Application, local, localRepoRoot, app
 		ApiVersions:       apiVersions,
 		Plugins:           configManagementPlugins,
 		TrackingMethod:    trackingMethod,
-	}, true, &git.NoopCredsStore{})
+	}, true, &git.NoopCredsStore{}, resource.MustParse("0"))
 	errors.CheckError(err)
 
 	return res.Manifests

cmd/argocd/commands/context_test.go

@@ -36,13 +36,13 @@ users:
 - auth-token: vErrYS3c3tReFRe$hToken
   name: localhost:8080`
 
-const testConfigFilePath = "./testdata/config"
+const testConfigFilePath = "./testdata/local.config"
 
 func TestContextDelete(t *testing.T) {
-
 	// Write the test config file
 	err := ioutil.WriteFile(testConfigFilePath, []byte(testConfig), os.ModePerm)
 	assert.NoError(t, err)
+	defer os.Remove(testConfigFilePath)
 
 	err = os.Chmod(testConfigFilePath, 0600)
 	require.NoError(t, err, "Could not change the file permission to 0600 %v", err)
@@ -75,9 +75,4 @@ func TestContextDelete(t *testing.T) {
 	assert.NotContains(t, localConfig.Servers, localconfig.Server{PlainText: true, Server: "localhost:8080"})
 	assert.NotContains(t, localConfig.Users, localconfig.User{AuthToken: "vErrYS3c3tReFRe$hToken", Name: "localhost:8080"})
 	assert.Contains(t, localConfig.Contexts, localconfig.ContextRef{Name: "argocd2.example.com:443", Server: "argocd2.example.com:443", User: "argocd2.example.com:443"})
-
-	// Write the file again so that no conflicts are made in git
-	err = ioutil.WriteFile(testConfigFilePath, []byte(testConfig), os.ModePerm)
-	assert.NoError(t, err)
-
 }

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)
 	}

cmd/argocd/commands/logout_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	argocdclient "github.com/argoproj/argo-cd/v2/pkg/apiclient"
 	"github.com/argoproj/argo-cd/v2/util/localconfig"
@@ -16,6 +17,10 @@ func TestLogout(t *testing.T) {
 	// Write the test config file
 	err := ioutil.WriteFile(testConfigFilePath, []byte(testConfig), os.ModePerm)
 	assert.NoError(t, err)
+	defer os.Remove(testConfigFilePath)
+
+	err = os.Chmod(testConfigFilePath, 0600)
+	require.NoError(t, err)
 
 	localConfig, err := localconfig.ReadLocalConfig(testConfigFilePath)
 	assert.NoError(t, err)
@@ -32,9 +37,4 @@ func TestLogout(t *testing.T) {
 	assert.Contains(t, localConfig.Contexts, localconfig.ContextRef{Name: "argocd1.example.com:443", Server: "argocd1.example.com:443", User: "argocd1.example.com:443"})
 	assert.Contains(t, localConfig.Contexts, localconfig.ContextRef{Name: "argocd2.example.com:443", Server: "argocd2.example.com:443", User: "argocd2.example.com:443"})
 	assert.Contains(t, localConfig.Contexts, localconfig.ContextRef{Name: "localhost:8080", Server: "localhost:8080", User: "localhost:8080"})
-
-	// Write the file again so that no conflicts are made in git
-	err = ioutil.WriteFile(testConfigFilePath, []byte(testConfig), os.ModePerm)
-	assert.NoError(t, err)
-
 }

cmpserver/plugin/plugin_test.go

@@ -195,7 +195,8 @@ func TestMatchRepository(t *testing.T) {
 		d := Discover{
 			Find: Find{
 				Command: Command{
-					Command: []string{"sh", "-c", "echo -n $ENV_NO_VAR"},
+					// Use printf instead of echo since OSX prints the "-n" when there's no additional arg.
+					Command: []string{"sh", "-c", `printf "%s" "$ENV_NO_VAR"`},
 				},
 			},
 		}

controller/sync.go

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

docs/operator-manual/applicationset/Generators-Git.md

@@ -150,6 +150,41 @@ Or, a shorter way (using [path.Match](https://golang.org/pkg/path/#Match) syntax
   exclude: true
 ```
 
+### Root Of Git Repo
+
+The Git directory generator can be configured to deploy from the root of the git repository by providing `'*'` as the `path`.
+
+To exclude directories, you only need to put the name/[path.Match](https://golang.org/pkg/path/#Match) of the directory you do not want to deploy.
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: ApplicationSet
+metadata:
+  name: cluster-addons
+  namespace: argocd
+spec:
+  generators:
+  - git:
+      repoURL: https://github.com/example/example-repo.git
+      revision: HEAD
+      directories:
+      - path: '*'
+      - path: donotdeploy
+        exclude: true
+  template:
+    metadata:
+      name: '{{path.basename}}'
+    spec:
+      project: "my-project"
+      source:
+        repoURL: https://github.com/example/example-repo.git
+        targetRevision: HEAD
+        path: '{{path}}'
+      destination:
+        server: https://kubernetes.default.svc
+        namespace: '{{path.basename}}'
+```
+
 ## Git Generator: Files
 
 The Git file generator is the second subtype of the Git generator. The Git file generator generates parameters using the contents of JSON/YAML files found within a specified repository.

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

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

docs/operator-manual/security.md

@@ -223,3 +223,44 @@ to configure IP addresses logging in the proxy server that sits in front of the
 
 Argo CD's ApplicationSets feature has its own [security considerations](./applicationset/Security.md). Be aware of those
 issues before using ApplicationSets.
+
+## Limiting Directory App Memory Usage
+
+> >2.2.10, 2.1.16, >2.3.5
+
+Directory-type Applications (those whose source is raw JSON or YAML files) can consume significant
+[repo-server](architecture.md#repository-server) memory, depending on the size and structure of the YAML files.
+
+To avoid over-using memory in the repo-server (potentially causing a crash and denial of service), set the
+`reposerver.max.combined.directory.manifests.size` config option in [argocd-cmd-params-cm](argocd-cmd-params-cm.yaml).
+
+This option limits the combined size of all JSON or YAML files in an individual app. Note that the in-memory
+representation of a manifest may be as much as 300x the size of the manifest on disk. Also note that the limit is per
+Application. If manifests are generated for multiple applications at once, memory usage will be higher.
+
+**Example:**
+
+Suppose your repo-server has a 10G memory limit, and you have ten Applications which use raw JSON or YAML files. To
+calculate the max safe combined file size per Application, divide 10G by 300 * 10 Apps (300 being the worst-case memory
+growth factor for the manifests).
+
+```
+10G / 300 * 10 = 3M
+```
+
+So a reasonably safe configuration for this setup would be a 3M limit per app.
+
+```yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: argocd-cmd-params-cm
+data:
+  reposerver.max.combined.directory.manifests.size: '3M'
+```
+
+The 300x ratio assumes a maliciously-crafted manifest file. If you only want to protect against accidental excessive
+memory use, it is probably safe to use a smaller ratio.
+
+Keep in mind that if a malicious user can create additional Applications, they can increase the total memory usage.
+Grant [App creation privileges](rbac.md) carefully.

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

@@ -13,28 +13,29 @@ 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)
-      --otlp-address string                  OpenTelemetry collector address to send traces to
-      --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)
+      --otlp-address string                            OpenTelemetry collector address to send traces to
+      --parallelismlimit int                           Limit on number of concurrent manifests generate requests. Any value less the 1 means no limit.
+      --port int                                       Listen on given port for incoming connections (default 8081)
+      --redis string                                   Redis server hostname and port (e.g. argocd-redis:6379). 
+      --redis-ca-certificate string                    Path to Redis server CA certificate (e.g. /etc/certs/redis/ca.crt). If not specified, system trusted CAs will be used for server certificate validation.
+      --redis-client-certificate string                Path to Redis client certificate (e.g. /etc/certs/redis/client.crt).
+      --redis-client-key string                        Path to Redis client key (e.g. /etc/certs/redis/client.crt).
+      --redis-insecure-skip-tls-verify                 Skip Redis server certificate validation.
+      --redis-use-tls                                  Use TLS when connecting to Redis. 
+      --redisdb int                                    Redis database.
+      --repo-cache-expiration duration                 Cache expiration for repo state, incl. app lists, app details, manifest generation, revision meta-data (default 24h0m0s)
+      --revision-cache-expiration duration             Cache expiration for cached revision (default 3m0s)
+      --sentinel stringArray                           Redis sentinel hostname and port (e.g. argocd-redis-ha-announce-0:6379). 
+      --sentinelmaster string                          Redis sentinel master group name. (default "master")
+      --tlsciphers string                              The list of acceptable ciphers to be used when establishing TLS connections. Use 'list' to list available ciphers. (default "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384:TLS_RSA_WITH_AES_256_GCM_SHA384")
+      --tlsmaxversion string                           The maximum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.3")
+      --tlsminversion string                           The minimum SSL/TLS version that is acceptable (one of: 1.0|1.1|1.2|1.3) (default "1.2")
 ```
 

docs/user-guide/environment-variables.md

@@ -0,0 +1,9 @@
+# Environment Variables
+
+The following environment variables can be used with `argocd` CLI:
+
+| Environment Variable | Description |
+| --- | --- |
+| `ARGOCD_SERVER` | the address of the ArgoCD server without `https://` prefix <br> (instead of specifying `--server` for every command) <br> eg. `ARGOCD_SERVER=argocd.mycompany.com` if served through an ingress with DNS |
+| `ARGOCD_AUTH_TOKEN` | the ArgoCD `apiKey` for your ArgoCD user to be able to authenticate |
+| `ARGOCD_OPTS` | command-line options to pass to `argocd` CLI <br> eg. `ARGOCD_OPTS="--grpc-web"` |

docs/user-guide/sync-options.md

@@ -2,7 +2,7 @@
 
 ArgoCD allows users to customize some aspects of how it syncs the desired state in the target cluster. Some Sync Options can defined as annotations in a specific resource. Most of the Sync Options are configured in the Application resource `spec.syncPolicy.syncOptions` attribute.
 
-Bellow you can find details about each available Sync Option:
+Below you can find details about each available Sync Option:
 
 ## No Prune Resources
 

manifests/base/kustomization.yaml

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

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

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

manifests/core-install.yaml

@@ -9385,7 +9385,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -9597,13 +9597,19 @@ spec:
               key: otlp.address
               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.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -9652,7 +9658,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -9839,7 +9845,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/core-install/kustomization.yaml

@@ -12,4 +12,4 @@ resources:
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.4.0
+  newTag: v2.4.2

manifests/ha/base/kustomization.yaml

@@ -11,7 +11,7 @@ patchesStrategicMerge:
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.4.0
+  newTag: v2.4.2
 resources:
 - ../../base/application-controller
 - ../../base/applicationset-controller

manifests/ha/install.yaml

@@ -10320,7 +10320,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -10417,7 +10417,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -10457,7 +10457,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -10696,13 +10696,19 @@ spec:
               key: otlp.address
               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.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -10751,7 +10757,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -10998,7 +11004,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -11206,7 +11212,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/ha/namespace-install.yaml

@@ -1244,7 +1244,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -1341,7 +1341,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -1381,7 +1381,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -1620,13 +1620,19 @@ spec:
               key: otlp.address
               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.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -1675,7 +1681,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -1922,7 +1928,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -2130,7 +2136,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/install.yaml

@@ -9692,7 +9692,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -9789,7 +9789,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -9829,7 +9829,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -10036,13 +10036,19 @@ spec:
               key: otlp.address
               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.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -10091,7 +10097,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -10334,7 +10340,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -10536,7 +10542,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/namespace-install.yaml

@@ -616,7 +616,7 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -713,7 +713,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -753,7 +753,7 @@ spec:
       containers:
       - command:
         - argocd-notifications
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -960,13 +960,19 @@ spec:
               key: otlp.address
               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.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -1015,7 +1021,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -1258,7 +1264,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -1460,7 +1466,7 @@ spec:
               key: otlp.address
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.4.0
+        image: quay.io/argoproj/argocd:v2.4.2
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

mkdocs.yml

@@ -129,6 +129,7 @@ nav:
   - user-guide/compare-options.md
   - user-guide/sync-options.md
   - user-guide/parameters.md
+  - user-guide/environment-variables.md
   - user-guide/build-environment.md
   - user-guide/tracking_strategies.md
   - user-guide/resource_tracking.md

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 {

reposerver/repository/repository.go

@@ -18,6 +18,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/Masterminds/semver/v3"
@@ -71,6 +75,8 @@ const (
 	ociPrefix                      = "oci://"
 )
 
+var ErrExceededMaxCombinedManifestFileSize = errors.New("exceeded max combined manifest file size")
+
 // Service implements ManifestService interface
 type Service struct {
 	gitCredsStore             git.CredsStore
@@ -96,6 +102,7 @@ type RepoServerInitConstants struct {
 	PauseGenerationOnFailureForMinutes           int
 	PauseGenerationOnFailureForRequests          int
 	SubmoduleEnabled                             bool
+	MaxCombinedDirectoryManifestsSize            resource.Quantity
 }
 
 // NewService returns a new instance of the Manifest service
@@ -453,7 +460,7 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
 	var manifestGenResult *apiclient.ManifestResponse
 	opContext, err := opContextSrc()
 	if err == nil {
-		manifestGenResult, err = GenerateManifests(ctx, opContext.appPath, repoRoot, commitSHA, q, false, s.gitCredsStore, WithCMPTarDoneChannel(ch.tarDoneCh))
+		manifestGenResult, err = GenerateManifests(ctx, opContext.appPath, repoRoot, commitSHA, q, false, s.gitCredsStore, s.initConstants.MaxCombinedDirectoryManifestsSize, WithCMPTarDoneChannel(ch.tarDoneCh))
 	}
 	if err != nil {
 		// If manifest generation error caching is enabled
@@ -867,7 +874,7 @@ func WithCMPTarDoneChannel(ch chan<- bool) GenerateManifestOpt {
 }
 
 // GenerateManifests generates manifests from a path
-func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool, gitCredsStore git.CredsStore, opts ...GenerateManifestOpt) (*apiclient.ManifestResponse, error) {
+func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool, gitCredsStore git.CredsStore, maxCombinedManifestQuantity resource.Quantity, opts ...GenerateManifestOpt) (*apiclient.ManifestResponse, error) {
 	opt := newGenerateManifestOpt(opts...)
 	var targetObjs []*unstructured.Unstructured
 	var dest *v1alpha1.ApplicationDestination
@@ -908,7 +915,7 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,
 			directory = &v1alpha1.ApplicationSourceDirectory{}
 		}
 		logCtx := log.WithField("application", q.AppName)
-		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory, q.EnabledSourceTypes)
+		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory, q.EnabledSourceTypes, maxCombinedManifestQuantity)
 	}
 	if err != nil {
 		return nil, err
@@ -1078,63 +1085,30 @@ func isNullList(obj *unstructured.Unstructured) bool {
 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, enabledManifestGeneration map[string]bool) ([]*unstructured.Unstructured, error) {
+func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, enabledManifestGeneration map[string]bool, maxCombinedManifestQuantity resource.Quantity) ([]*unstructured.Unstructured, error) {
+	// Validate the directory before loading any manifests to save memory.
+	potentiallyValidManifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, directory.Recurse, directory.Include, directory.Exclude, maxCombinedManifestQuantity)
+	if err != nil {
+		logCtx.Errorf("failed to get potentially valid manifests: %s", err)
+		return nil, fmt.Errorf("failed to get potentially valid manifests: %w", err)
+	}
+
 	var objs []*unstructured.Unstructured
-	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		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") {
 			if !discovery.IsManifestGenerationEnabled(v1alpha1.ApplicationSourceTypeDirectory, enabledManifestGeneration) {
-				return nil
+				continue
 			}
 			vm, err := makeJsonnetVm(appPath, repoRoot, directory.Jsonnet, env)
 			if err != nil {
-				return err
+				return nil, err
 			}
-			jsonStr, err := vm.EvaluateFile(path)
+			jsonStr, err := vm.EvaluateFile(manifestPath)
 			if err != nil {
-				return status.Errorf(codes.FailedPrecondition, "Failed to evaluate jsonnet %q: %v", f.Name(), err)
+				return nil, status.Errorf(codes.FailedPrecondition, "Failed to evaluate jsonnet %q: %v", manifestFileInfo.Name(), err)
 			}
 
 			// attempt to unmarshal either array or single object
@@ -1146,49 +1120,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) {
@@ -1461,8 +1593,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
@@ -1470,10 +1606,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
 	}
@@ -1492,15 +1628,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

reposerver/repository/repository_test.go

@@ -5,8 +5,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	log "github.com/sirupsen/logrus"
 	goio "io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -17,6 +17,9 @@ import (
 	"testing"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/api/resource"
+
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -145,39 +148,39 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	assert.Equal(t, countOfManifests, len(res1.Manifests))
 
 	// this will test concatenated manifests to verify we split YAMLs correctly
-	res2, err := GenerateManifests(context.Background(), "./testdata/concatenated", "/", "", &q, false, &git.NoopCredsStore{})
+	res2, err := GenerateManifests(context.Background(), "./testdata/concatenated", "/", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
 	assert.NoError(t, err)
 	assert.Equal(t, 3, len(res2.Manifests))
 }
 
 func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
-	testCases := []struct{
-		name string
-		outOfBoundsFilename string
+	testCases := []struct {
+		name                    string
+		outOfBoundsFilename     string
 		outOfBoundsFileContents string
-		mustNotContain string  // Optional string that must not appear in error or manifest output. If empty, use outOfBoundsFileContents.
+		mustNotContain          string // Optional string that must not appear in error or manifest output. If empty, use outOfBoundsFileContents.
 	}{
 		{
-			name: "out of bounds JSON file should not appear in error output",
-			outOfBoundsFilename: "test.json",
+			name:                    "out of bounds JSON file should not appear in error output",
+			outOfBoundsFilename:     "test.json",
 			outOfBoundsFileContents: `{"some": "json"}`,
 		},
 		{
-			name: "malformed JSON file contents should not appear in error output",
-			outOfBoundsFilename: "test.json",
+			name:                    "malformed JSON file contents should not appear in error output",
+			outOfBoundsFilename:     "test.json",
 			outOfBoundsFileContents: "$",
 		},
 		{
-			name: "out of bounds JSON manifest should not appear in manifest output",
+			name:                "out of bounds JSON manifest should not appear in manifest output",
 			outOfBoundsFilename: "test.json",
 			// JSON marshalling is deterministic. So if there's a leak, exactly this should appear in the manifests.
 			outOfBoundsFileContents: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
 		},
 		{
-			name: "out of bounds YAML manifest should not appear in manifest output",
-			outOfBoundsFilename: "test.yaml",
+			name:                    "out of bounds YAML manifest should not appear in manifest output",
+			outOfBoundsFilename:     "test.yaml",
 			outOfBoundsFileContents: "apiVersion: v1\nkind: Secret\nmetadata:\n  name: test\n  namespace: default\ntype: Opaque",
-			mustNotContain: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+			mustNotContain:          `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
 		},
 	}
 
@@ -201,7 +204,7 @@ func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
 			}
 
 			q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
-			res, err := GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{})
+			res, err := GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
 			require.Error(t, err)
 			assert.NotContains(t, err.Error(), mustNotContain)
 			assert.Contains(t, err.Error(), "illegal filepath")
@@ -216,7 +219,7 @@ func TestGenerateManifests_MissingSymlinkDestination(t *testing.T) {
 	require.NoError(t, err)
 
 	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
-	_, err = GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{})
+	_, err = GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
 	require.NoError(t, err)
 }
 
@@ -1107,7 +1110,7 @@ func TestGenerateFromUTF16(t *testing.T) {
 		Repo:              &argoappv1.Repository{},
 		ApplicationSource: &argoappv1.ApplicationSource{},
 	}
-	res1, err := GenerateManifests(context.Background(), "./testdata/utf-16", "/", "", &q, false, &git.NoopCredsStore{})
+	res1, err := GenerateManifests(context.Background(), "./testdata/utf-16", "/", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"))
 	assert.Nil(t, err)
 	assert.Equal(t, 2, len(res1.Manifests))
 }
@@ -1124,11 +1127,13 @@ func TestListApps(t *testing.T) {
 		"app-parameters/single-app-only": "Kustomize",
 		"app-parameters/single-global":   "Kustomize",
 		"invalid-helm":                   "Helm",
+		"in-bounds-values-file-link":     "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)
@@ -1448,6 +1453,7 @@ func mkTempParameters(source string) string {
 func runWithTempTestdata(t *testing.T, path string, runner func(t *testing.T, path string)) {
 	tempDir := mkTempParameters("./testdata/app-parameters")
 	runner(t, filepath.Join(tempDir, "app-parameters", path))
+	os.RemoveAll(tempDir)
 }
 
 func TestGenerateManifestsWithAppParameterFile(t *testing.T) {
@@ -1653,7 +1659,7 @@ func TestFindResources(t *testing.T) {
 				Recurse: true,
 				Include: tc.include,
 				Exclude: tc.exclude,
-			}, map[string]bool{})
+			}, map[string]bool{}, resource.MustParse("0"))
 			if !assert.NoError(t, err) {
 				return
 			}
@@ -1670,7 +1676,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",
-	}, map[string]bool{})
+	}, map[string]bool{}, resource.MustParse("0"))
 
 	if !assert.NoError(t, err) || !assert.Len(t, objs, 1) {
 		return
@@ -1683,7 +1689,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",
-	}, map[string]bool{})
+	}, map[string]bool{}, resource.MustParse("0"))
 
 	if !assert.NoError(t, err) || !assert.Len(t, objs, 2) {
 		return
@@ -1693,6 +1699,467 @@ 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.ErrorContains(t, err, "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.ErrorContains(t, err, "illegal filepath in symlink")
+		})
+	})
+
+	t.Run("symlink to a non-regular file should be skipped with warning", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		dirPath := filepath.Join(appDir, "test.dir")
+		err := os.MkdirAll(dirPath, 0644)
+		require.NoError(t, err)
+		linkPath := filepath.Join(appDir, "test.json")
+		err = os.Symlink(dirPath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Contains(t, ignoreMessage, "non-regular file")
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("non-included file should be skipped with no message", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "not-included.yaml")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "*.json", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("excluded file should be skipped with no message", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "excluded.json")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "excluded.*")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("symlink to a regular file is potentially valid", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		linkPath := filepath.Join(appDir, "link.json")
+		err = os.Symlink(filePath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("a regular file is potentially valid", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file.json")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("realFileInfo is for the destination rather than the symlink", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		linkPath := filepath.Join(appDir, "link.json")
+		err = os.Symlink(filePath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Equal(t, filepath.Base(filePath), realFileInfo.Name())
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+}
+
+func Test_getPotentiallyValidManifests(t *testing.T) {
+	// Tests which return no manifests and an error check to make sure the directory exists before running. A missing
+	// directory would produce those same results.
+
+	logCtx := log.WithField("test", "test")
+
+	t.Run("unreadable file throws error", func(t *testing.T) {
+		appDir := t.TempDir()
+		unreadablePath := filepath.Join(appDir, "unreadable.json")
+		err := os.WriteFile(unreadablePath, []byte{}, 0666)
+		require.NoError(t, err)
+		err = os.Chmod(appDir, 0000)
+		require.NoError(t, err)
+
+		manifests, err := getPotentiallyValidManifests(logCtx, appDir, appDir, false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+
+		// allow cleanup
+		err = os.Chmod(appDir, 0777)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	t.Run("no recursion when recursion is disabled", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/recurse", "./testdata/recurse", false, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("recursion when recursion is enabled", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/recurse", "./testdata/recurse", true, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+	})
+
+	t.Run("non-JSON/YAML is skipped", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/circular-link")
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/out-of-bounds-link")
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("symlink to a regular file works", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		manifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, false, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("symlink to nowhere should be ignored", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("link to over-sized manifest fails", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		// The file is 35 bytes.
+		manifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, false, "", "", resource.MustParse("34"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
+		// There is a total of 10 files, ech file being 10 bytes.
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/several-files", "./testdata/several-files", false, "", "", resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = getPotentiallyValidManifests(logCtx, "./testdata/several-files", "./testdata/several-files", false, "", "", resource.MustParse("100"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+}
+
+func Test_findManifests(t *testing.T) {
+	logCtx := log.WithField("test", "test")
+	noRecurse := argoappv1.ApplicationSourceDirectory{Recurse: false}
+
+	t.Run("unreadable file throws error", func(t *testing.T) {
+		appDir := t.TempDir()
+		unreadablePath := filepath.Join(appDir, "unreadable.json")
+		err := os.WriteFile(unreadablePath, []byte{}, 0666)
+		require.NoError(t, err)
+		err = os.Chmod(appDir, 0000)
+		require.NoError(t, err)
+
+		manifests, err := findManifests(logCtx, appDir, appDir, nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+
+		// allow cleanup
+		err = os.Chmod(appDir, 0777)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	t.Run("no recursion when recursion is disabled", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+	})
+
+	t.Run("recursion when recursion is enabled", func(t *testing.T) {
+		recurse := argoappv1.ApplicationSourceDirectory{Recurse: true}
+		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, recurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 4)
+		assert.NoError(t, err)
+	})
+
+	t.Run("non-JSON/YAML is skipped", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/circular-link")
+		manifests, err := findManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/out-of-bounds-link")
+		manifests, err := findManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("symlink to a regular file works", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("symlink to nowhere should be ignored", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("link to over-sized manifest fails", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		// The file is 35 bytes.
+		manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse, nil, resource.MustParse("34"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
+		// There is a total of 10 files, each file being 10 bytes.
+		manifests, err := findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, nil, resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, nil, resource.MustParse("364"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("jsonnet isn't counted against size limit", func(t *testing.T) {
+		// Each file is 36 bytes. Only the 36-byte json file should be counted against the limit.
+		manifests, err := findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, nil, resource.MustParse("36"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, nil, resource.MustParse("35"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("partially valid YAML file throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/partially-valid-yaml")
+		manifests, err := findManifests(logCtx, "./testdata/partially-valid-yaml", "./testdata/partially-valid-yaml", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalid manifest throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/invalid-manifests")
+		manifests, err := findManifests(logCtx, "./testdata/invalid-manifests", "./testdata/invalid-manifests", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("irrelevant YAML gets skipped, relevant YAML gets parsed", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/irrelevant-yaml", "./testdata/irrelevant-yaml", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("multiple JSON objects in one file throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/json-list")
+		manifests, err := findManifests(logCtx, "./testdata/json-list", "./testdata/json-list", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalid JSON throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/invalid-json")
+		manifests, err := findManifests(logCtx, "./testdata/invalid-json", "./testdata/invalid-json", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("valid JSON returns manifest and no error", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/valid-json", "./testdata/valid-json", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("YAML with an empty document doesn't throw an error", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/yaml-with-empty-document", "./testdata/yaml-with-empty-document", nil, noRecurse, nil, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+}
+
 func TestTestRepoOCI(t *testing.T) {
 	service := newService(".")
 	_, err := service.TestRepository(context.Background(), &apiclient.TestRepositoryRequest{
@@ -1904,3 +2371,23 @@ func Test_populateHelmAppDetails(t *testing.T) {
 	assert.Len(t, res.Helm.Parameters, 3)
 	assert.Len(t, res.Helm.ValueFiles, 4)
 }
+
+func Test_populateHelmAppDetails_values_symlinks(t *testing.T) {
+	t.Run("inbound", func(t *testing.T) {
+		res := apiclient.RepoAppDetailsResponse{}
+		q := apiclient.RepoServerAppDetailsQuery{Repo: &argoappv1.Repository{}, Source: &argoappv1.ApplicationSource{}}
+		err := populateHelmAppDetails(&res, "./testdata/in-bounds-values-file-link/", "./testdata/in-bounds-values-file-link/", &q)
+		require.NoError(t, err)
+		assert.NotEmpty(t, res.Helm.Values)
+		assert.NotEmpty(t, res.Helm.Parameters)
+	})
+
+	t.Run("out of bounds", func(t *testing.T) {
+		res := apiclient.RepoAppDetailsResponse{}
+		q := apiclient.RepoServerAppDetailsQuery{Repo: &argoappv1.Repository{}, Source: &argoappv1.ApplicationSource{}}
+		err := populateHelmAppDetails(&res, "./testdata/out-of-bounds-values-file-link/", "./testdata/out-of-bounds-values-file-link/", &q)
+		require.NoError(t, err)
+		assert.Empty(t, res.Helm.Values)
+		assert.Empty(t, res.Helm.Parameters)
+	})
+}

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

server/server.go

@@ -132,7 +132,7 @@ var backoff = wait.Backoff{
 
 var (
 	clientConstraint = fmt.Sprintf(">= %s", common.MinClientVersion)
-	baseHRefRegex    = regexp.MustCompile(`<base href="(.*)">`)
+	baseHRefRegex    = regexp.MustCompile(`<base href="(.*?)">`)
 	// limits number of concurrent login requests to prevent password brute forcing. If set to 0 then no limit is enforced.
 	maxConcurrentLoginRequestsCount = 50
 	replicasCount                   = 1
@@ -893,7 +893,7 @@ func (s *ArgoCDServer) getIndexData() ([]byte, error) {
 		if s.BaseHRef == "/" || s.BaseHRef == "" {
 			s.indexData = data
 		} else {
-			s.indexData = []byte(baseHRefRegex.ReplaceAllString(string(data), fmt.Sprintf(`<base href="/%s/">`, strings.Trim(s.BaseHRef, "/"))))
+			s.indexData = []byte(replaceBaseHRef(string(data), fmt.Sprintf(`<base href="/%s/">`, strings.Trim(s.BaseHRef, "/"))))
 		}
 	})
 
@@ -978,6 +978,10 @@ func mustRegisterGWHandler(register registerFunc, ctx context.Context, mux *runt
 	}
 }
 
+func replaceBaseHRef(data string, replaceWith string) string {
+	return baseHRefRegex.ReplaceAllString(data, replaceWith)
+}
+
 // Authenticate checks for the presence of a valid token when accessing server-side resources.
 func (a *ArgoCDServer) Authenticate(ctx context.Context) (context.Context, error) {
 	if a.DisableAuth {
@@ -1010,6 +1014,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", "")
 		}
 	}

server/server_test.go

@@ -508,7 +508,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)
@@ -583,7 +583,7 @@ func TestAuthenticate_3rd_party_JWTs(t *testing.T) {
 			anonymousEnabled:      false,
 			claims:                jwt.RegisteredClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewNumericDate(time.Now())},
 			expectedErrorContains: "token is expired",
-			expectedClaims:        jwt.RegisteredClaims{Issuer:"sso"},
+			expectedClaims:        jwt.RegisteredClaims{Issuer: "sso"},
 		},
 		{
 			test:                  "anonymous enabled, expired token, admin claim",
@@ -601,7 +601,7 @@ func TestAuthenticate_3rd_party_JWTs(t *testing.T) {
 			t.Parallel()
 
 			// Must be declared here to avoid race.
-			ctx := context.Background()  //nolint:ineffassign,staticcheck
+			ctx := context.Background() //nolint:ineffassign,staticcheck
 
 			argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, true)
 			testDataCopy.claims.Issuer = fmt.Sprintf("%s/api/dex", dexURL)
@@ -698,7 +698,7 @@ func TestAuthenticate_no_SSO(t *testing.T) {
 			t.Parallel()
 
 			// Must be declared here to avoid race.
-			ctx := context.Background()  //nolint:ineffassign,staticcheck
+			ctx := context.Background() //nolint:ineffassign,staticcheck
 
 			argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, false)
 			token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.RegisteredClaims{Issuer: fmt.Sprintf("%s/api/dex", dexURL)})
@@ -806,7 +806,7 @@ func TestAuthenticate_bad_request_metadata(t *testing.T) {
 			t.Parallel()
 
 			// Must be declared here to avoid race.
-			ctx := context.Background()  //nolint:ineffassign,staticcheck
+			ctx := context.Background() //nolint:ineffassign,staticcheck
 
 			argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true)
 			ctx = metadata.NewIncomingContext(context.Background(), testDataCopy.metadata)
@@ -1065,39 +1065,39 @@ func TestOIDCConfigChangeDetection_NoChange(t *testing.T) {
 }
 
 func TestIsMainJsBundle(t *testing.T) {
-	testCases := []struct{
+	testCases := []struct {
 		name           string
 		url            string
 		isMainJsBundle bool
 	}{
 		{
-			name: "localhost with valid main bundle",
-			url: "https://localhost:8080/main.e4188e5adc97bbfc00c3.js",
+			name:           "localhost with valid main bundle",
+			url:            "https://localhost:8080/main.e4188e5adc97bbfc00c3.js",
 			isMainJsBundle: true,
 		},
 		{
-			name: "localhost and deep path with valid main bundle",
-			url: "https://localhost:8080/some/argo-cd-instance/main.e4188e5adc97bbfc00c3.js",
+			name:           "localhost and deep path with valid main bundle",
+			url:            "https://localhost:8080/some/argo-cd-instance/main.e4188e5adc97bbfc00c3.js",
 			isMainJsBundle: true,
 		},
 		{
-			name: "font file",
-			url: "https://localhost:8080/assets/fonts/google-fonts/Heebo-Bols.woff2",
+			name:           "font file",
+			url:            "https://localhost:8080/assets/fonts/google-fonts/Heebo-Bols.woff2",
 			isMainJsBundle: false,
 		},
 		{
-			name: "no dot after main",
-			url: "https://localhost:8080/main/e4188e5adc97bbfc00c3.js",
+			name:           "no dot after main",
+			url:            "https://localhost:8080/main/e4188e5adc97bbfc00c3.js",
 			isMainJsBundle: false,
 		},
 		{
-			name: "wrong extension character",
-			url: "https://localhost:8080/main.e4188e5adc97bbfc00c3/js",
+			name:           "wrong extension character",
+			url:            "https://localhost:8080/main.e4188e5adc97bbfc00c3/js",
 			isMainJsBundle: false,
 		},
 		{
-			name: "wrong hash length",
-			url: "https://localhost:8080/main.e4188e5adc97bbfc00c3abcdefg.js",
+			name:           "wrong hash length",
+			url:            "https://localhost:8080/main.e4188e5adc97bbfc00c3abcdefg.js",
 			isMainJsBundle: false,
 		},
 	}
@@ -1111,3 +1111,123 @@ func TestIsMainJsBundle(t *testing.T) {
 		})
 	}
 }
+
+func TestReplaceBaseHRef(t *testing.T) {
+	testCases := []struct {
+		name        string
+		data        string
+		expected    string
+		replaceWith string
+	}{
+		{
+			name: "non-root basepath",
+			data: `<!DOCTYPE html>
+<html lang="en">
+
+<head>
+    <meta charset="UTF-8">
+    <title>Argo CD</title>
+    <base href="/">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-32x32.png' sizes='32x32'/>
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-16x16.png' sizes='16x16'/>
+    <link href="assets/fonts.css" rel="stylesheet">
+</head>
+
+<body>
+    <noscript>
+        <p>
+        Your browser does not support JavaScript. Please enable JavaScript to view the site. 
+        Alternatively, Argo CD can be used with the <a href="https://argoproj.github.io/argo-cd/cli_installation/">Argo CD CLI</a>.
+        </p>
+    </noscript>
+    <div id="app"></div>
+</body>
+
+</html>`,
+			expected: `<!DOCTYPE html>
+<html lang="en">
+
+<head>
+    <meta charset="UTF-8">
+    <title>Argo CD</title>
+    <base href="/path1/path2/path3/">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-32x32.png' sizes='32x32'/>
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-16x16.png' sizes='16x16'/>
+    <link href="assets/fonts.css" rel="stylesheet">
+</head>
+
+<body>
+    <noscript>
+        <p>
+        Your browser does not support JavaScript. Please enable JavaScript to view the site. 
+        Alternatively, Argo CD can be used with the <a href="https://argoproj.github.io/argo-cd/cli_installation/">Argo CD CLI</a>.
+        </p>
+    </noscript>
+    <div id="app"></div>
+</body>
+
+</html>`,
+			replaceWith: `<base href="/path1/path2/path3/">`,
+		},
+		{
+			name: "root basepath",
+			data: `<!DOCTYPE html>
+<html lang="en">
+
+<head>
+    <meta charset="UTF-8">
+    <title>Argo CD</title>
+    <base href="/any/path/test/">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-32x32.png' sizes='32x32'/>
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-16x16.png' sizes='16x16'/>
+    <link href="assets/fonts.css" rel="stylesheet">
+</head>
+
+<body>
+    <noscript>
+        <p>
+        Your browser does not support JavaScript. Please enable JavaScript to view the site. 
+        Alternatively, Argo CD can be used with the <a href="https://argoproj.github.io/argo-cd/cli_installation/">Argo CD CLI</a>.
+        </p>
+    </noscript>
+    <div id="app"></div>
+</body>
+
+</html>`,
+			expected: `<!DOCTYPE html>
+<html lang="en">
+
+<head>
+    <meta charset="UTF-8">
+    <title>Argo CD</title>
+    <base href="/">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-32x32.png' sizes='32x32'/>
+    <link rel='icon' type='image/png' href='assets/favicon/favicon-16x16.png' sizes='16x16'/>
+    <link href="assets/fonts.css" rel="stylesheet">
+</head>
+
+<body>
+    <noscript>
+        <p>
+        Your browser does not support JavaScript. Please enable JavaScript to view the site. 
+        Alternatively, Argo CD can be used with the <a href="https://argoproj.github.io/argo-cd/cli_installation/">Argo CD CLI</a>.
+        </p>
+    </noscript>
+    <div id="app"></div>
+</body>
+
+</html>`,
+			replaceWith: `<base href="/">`,
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result := replaceBaseHRef(testCase.data, testCase.replaceWith)
+			assert.Equal(t, testCase.expected, result)
+		})
+	}
+}

test/e2e/fixture/fixture.go

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

test/e2e/selective_sync_test.go

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

test/remote/Dockerfile

@@ -1,9 +1,11 @@
+ARG BASE_IMAGE=docker.io/library/ubuntu:22.04
+
 FROM golang:1.18 AS go
 
 RUN go install github.com/mattn/goreman@latest && \
     go install github.com/kisielk/godepgraph@latest
 
-FROM ubuntu:22.04
+FROM $BASE_IMAGE
 
 ENV DEBIAN_FRONTEND=noninteractive
 RUN  apt-get update && apt-get install --no-install-recommends -y \

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

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

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

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

ui/src/app/shared/services/applications-service.ts

@@ -143,7 +143,7 @@ export class ApplicationsService {
             const searchOptions = optionsToSearch(options);
             search.set('fields', searchOptions.fields);
             search.set('selector', searchOptions.selector);
-            query?.projects?.forEach(project => search.append('project', project));
+            query?.projects?.forEach(project => search.append('projects', project));
         }
         const searchStr = search.toString();
         const url = `/stream/applications${(searchStr && '?' + searchStr) || ''}`;

util/helm/helm.go

@@ -6,9 +6,11 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"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"
@@ -27,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`
@@ -129,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
@@ -156,7 +165,7 @@ func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[strin
 	output := map[string]string{}
 	for _, file := range values {
 		values := map[string]interface{}{}
-		if err = yaml.Unmarshal([]byte(file), &values); err != nil {
+		if err := yaml.Unmarshal([]byte(file), &values); err != nil {
 			return nil, fmt.Errorf("failed to parse values: %s", err)
 		}
 		flatVals(values, output)

util/helm/helm_test.go

@@ -85,7 +85,7 @@ func TestHelmGetParams(t *testing.T) {
 	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"]
@@ -100,7 +100,7 @@ func TestHelmGetParamsValueFiles(t *testing.T) {
 	assert.NoError(t, err)
 	valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil)
 	require.NoError(t, err)
-	params, err := h.GetParameters([]path.ResolvedFilePath{valuesPath})
+	params, err := h.GetParameters([]path.ResolvedFilePath{valuesPath}, repoRootAbs, repoRootAbs)
 	assert.Nil(t, err)
 
 	slaveCountParam := params["cluster.slaveCount"]
@@ -117,7 +117,7 @@ func TestHelmGetParamsValueFilesThatExist(t *testing.T) {
 	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})
+	params, err := h.GetParameters([]path.ResolvedFilePath{valuesMissingPath, valuesProductionPath}, repoRootAbs, repoRootAbs)
 	assert.Nil(t, err)
 
 	slaveCountParam := params["cluster.slaveCount"]

util/helm/testdata/dependency/Chart.lock

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

util/localconfig/file_perm_unix.go

@@ -0,0 +1,16 @@
+//go:build !windows
+
+package localconfig
+
+import (
+	"fmt"
+	"os"
+)
+
+func getFilePermission(fi os.FileInfo) error {
+	if fi.Mode().Perm() == 0600 || fi.Mode().Perm() == 0400 {
+		return nil
+	}
+	return fmt.Errorf("config file has incorrect permission flags:%s."+
+		"change the file permission either to 0400 or 0600.", fi.Mode().Perm().String())
+}

util/localconfig/file_perm_windows.go

@@ -0,0 +1,16 @@
+//go:build windows
+
+package localconfig
+
+import (
+	"fmt"
+	"os"
+)
+
+func getFilePermission(fi os.FileInfo) error {
+	if fi.Mode().Perm() == 0666 || fi.Mode().Perm() == 0444 {
+		return nil
+	}
+	return fmt.Errorf("config file has incorrect permission flags:%s."+
+		"change the file permission either to 0444 or 0666.", fi.Mode().Perm().String())
+}

util/localconfig/localconfig.go

@@ -311,11 +311,3 @@ func GetUsername(subject string) string {
 	}
 	return subject
 }
-
-func getFilePermission(fi os.FileInfo) error {
-	if fi.Mode().Perm() == 0600 || fi.Mode().Perm() == 0400 {
-		return nil
-	}
-	return fmt.Errorf("config file has incorrect permission flags:%s."+
-		"change the file permission either to 0400 or 0600.", fi.Mode().Perm().String())
-}

util/localconfig/localconfig_test.go

@@ -1,3 +1,5 @@
+//go:build !windows
+
 package localconfig
 
 import (
@@ -83,7 +85,7 @@ func TestFilePermission(t *testing.T) {
 			if err := getFilePermission(fi); err != nil {
 				assert.EqualError(t, err, c.expectedError.Error())
 			} else {
-				require.Nil(t, err)
+				require.Nil(t, c.expectedError)
 			}
 		})
 	}

util/oidc/oidc.go

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

util/rand/rand.go

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

util/rand/rand_test.go

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