Bump version to 2.2.11

* add tests to demonstrate issue

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

more

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

docs

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

settings tests

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

tests for OIDC handlers, consolidating test helpers

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

consolidate

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

consolidate

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

docs

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

* fix log message

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

.github/workflows/ci-build.yaml

@@ -72,10 +72,10 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v2
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.38.0
-          args: --timeout 10m --exclude SA5011
+          version: v1.46.2
+          args: --timeout 10m --exclude SA5011 --verbose
 
   test-go:
     name: Run unit tests for Go packages

.golangci.yml

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

VERSION

@@ -1 +1 @@
-2.2.8
+2.2.11

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

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

cmd/argocd/commands/admin/project_allowlist.go

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

cmd/argocd/commands/admin/project_allowlist_test.go

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

cmd/argocd/commands/app.go

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

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

controller/sync.go

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

docs/operator-manual/argocd-cm.yaml

@@ -242,3 +242,9 @@ data:
   # published to the repository. Reconciliation by timeout is disabled if timeout is set to 0. Three minutes by default.
   # > Note: argocd-repo-server deployment must be manually restarted after changing the setting.
   timeout.reconciliation: 180s
+
+  # oidc.tls.insecure.skip.verify determines whether certificate verification is skipped when verifying tokens with the
+  # configured OIDC provider (either external or the bundled Dex instance). Setting this to "true" will cause JWT
+  # token verification to pass despite the OIDC provider having an invalid certificate. Only set to "true" if you
+  # understand the risks.
+  oidc.tls.insecure.skip.verify: "false"

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

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

docs/operator-manual/security.md

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

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

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

docs/operator-manual/user-management/index.md

@@ -373,6 +373,21 @@ You are not required to specify a logoutRedirectURL as this is automatically gen
 !!! note
    The post logout redirect URI may need to be whitelisted against your OIDC provider's client settings for ArgoCD.
 
+### Configuring a custom root CA certificate for communicating with the OIDC provider
+
+If your OIDC provider is setup with a certificate which is not signed by one of the well known certificate authorities
+you can provide a custom certificate which will be used in verifying the OIDC provider's TLS certificate when
+communicating with it.  
+Add a `rootCA` to your `oidc.config` which contains the PEM encoded root certificate:
+
+```yaml
+  oidc.config: |
+    ...
+    rootCA: |
+      -----BEGIN CERTIFICATE-----
+      ... encoded certificate data here ...
+      -----END CERTIFICATE-----
+```
 
 
 ## SSO Further Reading
@@ -477,3 +492,20 @@ data:
     clientSecret: $another-secret:oidc.auth0.clientSecret  # Mind the ':'
   ...
 ```
+
+### Skipping certificate verification on OIDC provider connections
+
+By default, all connections made by the API server to OIDC providers (either external providers or the bundled Dex
+instance) must pass certificate validation. These connections occur when getting the OIDC provider's well-known
+configuration, when getting the OIDC provider's keys, and  when exchanging an authorization code or verifying an ID 
+token as part of an OIDC login flow.
+
+Disabling certificate verification might make sense if:
+* You are using the bundled Dex instance **and** your Argo CD instance has TLS configured with a self-signed certificate
+  **and** you understand and accept the risks of skipping OIDC provider cert verification.
+* You are using an external OIDC provider **and** that provider uses an invalid certificate **and** you cannot solve
+  the problem by setting `oidcConfig.rootCA` **and** you understand and accept the risks of skipping OIDC provider cert 
+  verification.
+
+If either of those two applies, then you can disable OIDC provider certificate verification by setting
+`oidc.tls.insecure.skip.verify` to `"true"` in the `argocd-cm` ConfigMap.

go.mod

@@ -61,7 +61,6 @@ require (
 	github.com/spf13/cobra v1.1.3
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.0
-	github.com/undefinedlabs/go-mpatch v1.0.6
 	github.com/yuin/gopher-lua v0.0.0-20200816102855-ee81675732da
 	golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e
 	golang.org/x/net v0.0.0-20210614182718-04defd469f4e

go.sum

@@ -840,8 +840,6 @@ github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
-github.com/undefinedlabs/go-mpatch v1.0.6 h1:h8q5ORH/GaOE1Se1DMhrOyljXZEhRcROO7agMqWXCOY=
-github.com/undefinedlabs/go-mpatch v1.0.6/go.mod h1:TyJZDQ/5AgyN7FSLiBJ8RO9u2c6wbtRvK827b6AVqY4=
 github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
 github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=

hack/installers/install-lint-tools.sh

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

manifests/base/kustomization.yaml

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

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

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

manifests/core-install.yaml

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

manifests/core-install/kustomization.yaml

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

manifests/ha/base/kustomization.yaml

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

manifests/ha/install.yaml

@@ -3709,7 +3709,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -3920,13 +3920,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -3975,7 +3981,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -4202,7 +4208,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -4398,7 +4404,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/ha/namespace-install.yaml

@@ -1068,7 +1068,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -1279,13 +1279,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -1334,7 +1340,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -1561,7 +1567,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -1757,7 +1763,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/install.yaml

@@ -3079,7 +3079,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -3254,13 +3254,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -3309,7 +3315,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -3532,7 +3538,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -3722,7 +3728,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/namespace-install.yaml

@@ -438,7 +438,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -613,13 +613,19 @@ spec:
               key: reposerver.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.max.combined.directory.manifests.size
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -668,7 +674,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -891,7 +897,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -1081,7 +1087,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.8
+        image: quay.io/argoproj/argocd:v2.2.11
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

pkg/apiclient/grpcproxy.go

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

reposerver/repository/repository.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	goio "io"
 	"io/ioutil"
 	"net/url"
 	"os"
@@ -16,6 +17,12 @@ import (
 	"strings"
 	"time"
 
+	kubeyaml "k8s.io/apimachinery/pkg/util/yaml"
+
+	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/argoproj/argo-cd/v2/util/io/files"
+
 	"github.com/argoproj/argo-cd/v2/util/argo"
 
 	"github.com/Masterminds/semver"
@@ -66,6 +73,8 @@ const (
 	ociPrefix                      = "oci://"
 )
 
+var ErrExceededMaxCombinedManifestFileSize = errors.New("exceeded max combined manifest file size")
+
 // Service implements ManifestService interface
 type Service struct {
 	repoLock                  *repositoryLock
@@ -85,6 +94,7 @@ type RepoServerInitConstants struct {
 	PauseGenerationAfterFailedGenerationAttempts int
 	PauseGenerationOnFailureForMinutes           int
 	PauseGenerationOnFailureForRequests          int
+	MaxCombinedDirectoryManifestsSize            resource.Quantity
 }
 
 // NewService returns a new instance of the Manifest service
@@ -329,7 +339,7 @@ func (s *Service) runManifestGen(repoRoot, commitSHA, cacheKey string, ctxSrc op
 	var manifestGenResult *apiclient.ManifestResponse
 	ctx, err := ctxSrc()
 	if err == nil {
-		manifestGenResult, err = GenerateManifests(ctx.appPath, repoRoot, commitSHA, q, false)
+		manifestGenResult, err = GenerateManifests(ctx.appPath, repoRoot, commitSHA, q, false, s.initConstants.MaxCombinedDirectoryManifestsSize)
 	}
 	if err != nil {
 
@@ -704,7 +714,7 @@ func getRepoCredential(repoCredentials []*v1alpha1.RepoCreds, repoURL string) *v
 }
 
 // GenerateManifests generates manifests from a path
-func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool) (*apiclient.ManifestResponse, error) {
+func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.ManifestRequest, isLocal bool, maxCombinedManifestQuantity resource.Quantity) (*apiclient.ManifestResponse, error) {
 	var targetObjs []*unstructured.Unstructured
 	var dest *v1alpha1.ApplicationDestination
 
@@ -752,7 +762,8 @@ func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.Manifest
 		if directory = q.ApplicationSource.Directory; directory == nil {
 			directory = &v1alpha1.ApplicationSourceDirectory{}
 		}
-		targetObjs, err = findManifests(appPath, repoRoot, env, *directory)
+		logCtx := log.WithField("application", q.AppName)
+		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory, maxCombinedManifestQuantity)
 	}
 	if err != nil {
 		return nil, err
@@ -950,44 +961,27 @@ func ksShow(appLabelKey, appPath string, ksonnetOpts *v1alpha1.ApplicationSource
 var manifestFile = regexp.MustCompile(`^.*\.(yaml|yml|json|jsonnet)$`)
 
 // findManifests looks at all yaml files in a directory and unmarshals them into a list of unstructured objects
-func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory) ([]*unstructured.Unstructured, error) {
+func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, maxCombinedManifestQuantity resource.Quantity) ([]*unstructured.Unstructured, error) {
+	// Validate the directory before loading any manifests to save memory.
+	potentiallyValidManifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, directory.Recurse, directory.Include, directory.Exclude, maxCombinedManifestQuantity)
+	if err != nil {
+		logCtx.Errorf("failed to get potentially valid manifests: %s", err)
+		return nil, fmt.Errorf("failed to get potentially valid manifests: %w", err)
+	}
+
 	var objs []*unstructured.Unstructured
-	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if f.IsDir() {
-			if path != appPath && !directory.Recurse {
-				return filepath.SkipDir
-			} else {
-				return nil
-			}
-		}
+	for _, potentiallyValidManifest := range potentiallyValidManifests {
+		manifestPath := potentiallyValidManifest.path
+		manifestFileInfo := potentiallyValidManifest.fileInfo
 
-		if !manifestFile.MatchString(f.Name()) {
-			return nil
-		}
-
-		relPath, err := filepath.Rel(appPath, path)
-		if err != nil {
-			return err
-		}
-		if directory.Exclude != "" && glob.Match(directory.Exclude, relPath) {
-			return nil
-		}
-
-		if directory.Include != "" && !glob.Match(directory.Include, relPath) {
-			return nil
-		}
-
-		if strings.HasSuffix(f.Name(), ".jsonnet") {
+		if strings.HasSuffix(manifestFileInfo.Name(), ".jsonnet") {
 			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
@@ -999,49 +993,207 @@ func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory
 				var jsonObj unstructured.Unstructured
 				err = json.Unmarshal([]byte(jsonStr), &jsonObj)
 				if err != nil {
-					return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal generated json %q: %v", f.Name(), err)
+					return nil, status.Errorf(codes.FailedPrecondition, "Failed to unmarshal generated json %q: %v", manifestFileInfo.Name(), err)
 				}
 				objs = append(objs, &jsonObj)
 			}
 		} else {
-			out, err := utfutil.ReadFile(path, utfutil.UTF8)
+			err := getObjsFromYAMLOrJson(logCtx, manifestPath, manifestFileInfo.Name(), &objs)
 			if err != nil {
-				return err
-			}
-			if strings.HasSuffix(f.Name(), ".json") {
-				var obj unstructured.Unstructured
-				err = json.Unmarshal(out, &obj)
-				if err != nil {
-					return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", f.Name(), err)
-				}
-				objs = append(objs, &obj)
-			} else {
-				yamlObjs, err := kube.SplitYAML(out)
-				if err != nil {
-					if len(yamlObjs) > 0 {
-						// If we get here, we had a multiple objects in a single YAML file which had some
-						// valid k8s objects, but errors parsing others (within the same file). It's very
-						// likely the user messed up a portion of the YAML, so report on that.
-						return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", f.Name(), err)
-					}
-					// Otherwise, let's see if it looks like a resource, if yes, we return error
-					if bytes.Contains(out, []byte("apiVersion:")) &&
-						bytes.Contains(out, []byte("kind:")) &&
-						bytes.Contains(out, []byte("metadata:")) {
-						return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", f.Name(), err)
-					}
-					// Otherwise, it might be a unrelated YAML file which we will ignore
-					return nil
-				}
-				objs = append(objs, yamlObjs...)
+				return nil, err
 			}
 		}
+	}
+	return objs, nil
+}
+
+// getObjsFromYAMLOrJson unmarshals the given yaml or json file and appends it to the given list of objects.
+func getObjsFromYAMLOrJson(logCtx *log.Entry, manifestPath string, filename string, objs *[]*unstructured.Unstructured) error {
+	reader, err := utfutil.OpenFile(manifestPath, utfutil.UTF8)
+	if err != nil {
+		return status.Errorf(codes.FailedPrecondition, "Failed to open %q", manifestPath)
+	}
+	defer func() {
+		err := reader.Close()
+		if err != nil {
+			logCtx.Errorf("failed to close %q - potential memory leak", manifestPath)
+		}
+	}()
+	if strings.HasSuffix(filename, ".json") {
+		var obj unstructured.Unstructured
+		decoder := json.NewDecoder(reader)
+		err = decoder.Decode(&obj)
+		if err != nil {
+			return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", filename, err)
+		}
+		if decoder.More() {
+			return status.Errorf(codes.FailedPrecondition, "Found multiple objects in %q. Only single objects are allowed in JSON files.", filename)
+		}
+		*objs = append(*objs, &obj)
+	} else {
+		yamlObjs, err := splitYAMLOrJSON(reader)
+		if err != nil {
+			if len(yamlObjs) > 0 {
+				// If we get here, we had a multiple objects in a single YAML file which had some
+				// valid k8s objects, but errors parsing others (within the same file). It's very
+				// likely the user messed up a portion of the YAML, so report on that.
+				return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", filename, err)
+			}
+			// Read the whole file to check whether it looks like a manifest.
+			out, err := utfutil.ReadFile(manifestPath, utfutil.UTF8)
+			// Otherwise, let's see if it looks like a resource, if yes, we return error
+			if bytes.Contains(out, []byte("apiVersion:")) &&
+				bytes.Contains(out, []byte("kind:")) &&
+				bytes.Contains(out, []byte("metadata:")) {
+				return status.Errorf(codes.FailedPrecondition, "Failed to unmarshal %q: %v", filename, err)
+			}
+			// Otherwise, it might be an unrelated YAML file which we will ignore
+		}
+		*objs = append(*objs, yamlObjs...)
+	}
+	return nil
+}
+
+// splitYAMLOrJSON reads a YAML or JSON file and gets each document as an unstructured object. If the unmarshaller
+// encounters an error, objects read up until the error are returned.
+func splitYAMLOrJSON(reader goio.Reader) ([]*unstructured.Unstructured, error) {
+	d := kubeyaml.NewYAMLOrJSONDecoder(reader, 4096)
+	var objs []*unstructured.Unstructured
+	for {
+		u := &unstructured.Unstructured{}
+		if err := d.Decode(&u); err != nil {
+			if err == goio.EOF {
+				break
+			}
+			return objs, fmt.Errorf("failed to unmarshal manifest: %v", err)
+		}
+		if u == nil {
+			continue
+		}
+		objs = append(objs, u)
+	}
+	return objs, nil
+}
+
+// getPotentiallyValidManifestFile checks whether the given path/FileInfo may be a valid manifest file. Returns a non-nil error if
+// there was an error that should not be handled by ignoring the file. Returns non-nil realFileInfo if the file is a
+// potential manifest. Returns a non-empty ignoreMessage if there's a message that should be logged about why the file
+// was skipped. If realFileInfo is nil and the ignoreMessage is empty, there's no need to log the ignoreMessage; the
+// file was skipped for a mundane reason.
+//
+// The file is still only a "potentially" valid manifest file because it could be invalid JSON or YAML, or it might not
+// be a valid Kubernetes resource. This function tests everything possible without actually reading the file.
+//
+// repoPath must be absolute.
+func getPotentiallyValidManifestFile(path string, f os.FileInfo, appPath, repoRoot, include, exclude string) (realFileInfo os.FileInfo, warning string, err error) {
+	relPath, err := filepath.Rel(appPath, path)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to get relative path of %q: %w", path, err)
+	}
+
+	if !manifestFile.MatchString(f.Name()) {
+		return nil, "", nil
+	}
+
+	// If the file is a symlink, these will be overridden with the destination file's info.
+	var relRealPath = relPath
+	realFileInfo = f
+
+	if files.IsSymlink(f) {
+		realPath, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			if os.IsNotExist(err) {
+				return nil, fmt.Sprintf("destination of symlink %q is missing", relPath), nil
+			}
+			return nil, "", fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
+		}
+		if !files.Inbound(realPath, repoRoot) {
+			return nil, "", fmt.Errorf("illegal filepath in symlink at %q", relPath)
+		}
+		realFileInfo, err = os.Stat(realPath)
+		if err != nil {
+			if os.IsNotExist(err) {
+				// This should have been caught by filepath.EvalSymlinks, but check again since that function's docs
+				// don't promise to return this error.
+				return nil, fmt.Sprintf("destination of symlink %q is missing at %q", relPath, realPath), nil
+			}
+			return nil, "", fmt.Errorf("failed to get file info for symlink at %q to %q: %w", relPath, realPath, err)
+		}
+		relRealPath, err = filepath.Rel(repoRoot, realPath)
+		if err != nil {
+			return nil, "", fmt.Errorf("failed to get relative path of %q: %w", realPath, err)
+		}
+	}
+
+	// FileInfo.Size() behavior is platform-specific for non-regular files. Allow only regular files, so we guarantee
+	// accurate file sizes.
+	if !realFileInfo.Mode().IsRegular() {
+		return nil, fmt.Sprintf("ignoring symlink at %q to non-regular file %q", relPath, relRealPath), nil
+	}
+
+	if exclude != "" && glob.Match(exclude, relPath) {
+		return nil, "", nil
+	}
+
+	if include != "" && !glob.Match(include, relPath) {
+		return nil, "", nil
+	}
+
+	return realFileInfo, "", nil
+}
+
+type potentiallyValidManifest struct {
+	path     string
+	fileInfo os.FileInfo
+}
+
+// getPotentiallyValidManifests ensures that 1) there are no errors while checking for potential manifest files in the given dir
+// and 2) the combined file size of the potentially-valid manifest files does not exceed the limit.
+func getPotentiallyValidManifests(logCtx *log.Entry, appPath string, repoRoot string, recurse bool, include string, exclude string, maxCombinedManifestQuantity resource.Quantity) ([]potentiallyValidManifest, error) {
+	maxCombinedManifestFileSize := maxCombinedManifestQuantity.Value()
+	var currentCombinedManifestFileSize = int64(0)
+
+	var potentiallyValidManifests []potentiallyValidManifest
+	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if f.IsDir() {
+			if path != appPath && !recurse {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+
+		realFileInfo, warning, err := getPotentiallyValidManifestFile(path, f, appPath, repoRoot, include, exclude)
+		if err != nil {
+			return fmt.Errorf("invalid manifest file %q: %w", path, err)
+		}
+		if realFileInfo == nil {
+			if warning != "" {
+				logCtx.Warnf("skipping manifest file %q: %s", path, warning)
+			}
+			return nil
+		}
+		// Don't count jsonnet file size against max. It's jsonnet's responsibility to manage memory usage.
+		if !strings.HasSuffix(f.Name(), ".jsonnet") {
+			// We use the realFileInfo size (which is guaranteed to be a regular file instead of a symlink or other
+			// non-regular file) because .Size() behavior is platform-specific for non-regular files.
+			currentCombinedManifestFileSize += realFileInfo.Size()
+			if maxCombinedManifestFileSize != 0 && currentCombinedManifestFileSize > maxCombinedManifestFileSize {
+				return ErrExceededMaxCombinedManifestFileSize
+			}
+		}
+		potentiallyValidManifests = append(potentiallyValidManifests, potentiallyValidManifest{path: path, fileInfo: f})
 		return nil
 	})
 	if err != nil {
+		// Not wrapping, because this error should be wrapped by the caller.
 		return nil, err
 	}
-	return objs, nil
+
+	return potentiallyValidManifests, nil
 }
 
 func makeJsonnetVm(appPath string, repoRoot string, sourceJsonnet v1alpha1.ApplicationSourceJsonnet, env *v1alpha1.Env) (*jsonnet.VM, error) {
@@ -1337,8 +1489,12 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
 		return err
 	}
 
-	if err := loadFileIntoIfExists(filepath.Join(appPath, "values.yaml"), &res.Helm.Values); err != nil {
-		return err
+	if resolvedValuesPath, _, err := pathutil.ResolveFilePath(appPath, repoRoot, "values.yaml", []string{}); err == nil {
+		if err := loadFileIntoIfExists(resolvedValuesPath, &res.Helm.Values); err != nil {
+			return err
+		}
+	} else {
+		log.Warnf("Values file %s is not allowed: %v", filepath.Join(appPath, "values.yaml"), err)
 	}
 	var resolvedSelectedValueFiles []pathutil.ResolvedFilePath
 	// drop not allowed values files
@@ -1346,10 +1502,10 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
 		if resolvedFile, _, err := pathutil.ResolveFilePath(appPath, repoRoot, file, q.GetValuesFileSchemes()); err == nil {
 			resolvedSelectedValueFiles = append(resolvedSelectedValueFiles, resolvedFile)
 		} else {
-			log.Debugf("Values file %s is not allowed: %v", file, err)
+			log.Warnf("Values file %s is not allowed: %v", file, err)
 		}
 	}
-	params, err := h.GetParameters(resolvedSelectedValueFiles)
+	params, err := h.GetParameters(resolvedSelectedValueFiles, appPath, repoRoot)
 	if err != nil {
 		return err
 	}
@@ -1368,15 +1524,16 @@ func populateHelmAppDetails(res *apiclient.RepoAppDetailsResponse, appPath strin
 	return nil
 }
 
-func loadFileIntoIfExists(path string, destination *string) error {
-	info, err := os.Stat(path)
+func loadFileIntoIfExists(path pathutil.ResolvedFilePath, destination *string) error {
+	stringPath := string(path)
+	info, err := os.Stat(stringPath)
 
 	if err == nil && !info.IsDir() {
-		if bytes, err := ioutil.ReadFile(path); err != nil {
-			*destination = string(bytes)
-		} else {
+		bytes, err := ioutil.ReadFile(stringPath);
+		if err != nil {
 			return err
 		}
+		*destination = string(bytes)
 	}
 
 	return nil

reposerver/repository/repository_norace_test.go

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

reposerver/repository/repository_test.go

@@ -5,16 +5,19 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/argoproj/argo-cd/v2/util/argo"
+	log "github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/api/resource"
 
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
@@ -29,6 +32,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/reposerver/cache"
 	"github.com/argoproj/argo-cd/v2/reposerver/metrics"
 	fileutil "github.com/argoproj/argo-cd/v2/test/fixture/path"
+	"github.com/argoproj/argo-cd/v2/util/argo"
 	cacheutil "github.com/argoproj/argo-cd/v2/util/cache"
 	"github.com/argoproj/argo-cd/v2/util/git"
 	gitmocks "github.com/argoproj/argo-cd/v2/util/git/mocks"
@@ -52,7 +56,7 @@ func newServiceWithMocks(root string, signed bool) (*Service, *gitmocks.Client)
 	return newServiceWithOpt(func(gitClient *gitmocks.Client) {
 		gitClient.On("Init").Return(nil)
 		gitClient.On("Fetch", mock.Anything).Return(nil)
-		gitClient.On("Checkout", mock.Anything).Return(nil)
+		gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
 		gitClient.On("LsRemote", mock.Anything).Return(mock.Anything, nil)
 		gitClient.On("CommitSHA").Return(mock.Anything, nil)
 		gitClient.On("Root").Return(root)
@@ -81,7 +85,6 @@ func newServiceWithOpt(cf clientFunc) (*Service, *gitmocks.Client) {
 	}}, nil)
 	helmClient.On("ExtractChart", chart, version).Return("./testdata/my-chart", io.NopCloser, nil)
 	helmClient.On("CleanChartCache", chart, version).Return(nil)
-
 	service.newGitClient = func(rawRepoURL string, creds git.Creds, insecure bool, enableLfs bool, prosy string, opts ...git.ClientOpts) (client git.Client, e error) {
 		return gitClient, nil
 	}
@@ -112,7 +115,7 @@ func newServiceWithCommitSHA(root, revision string) *Service {
 	service, gitClient := newServiceWithOpt(func(gitClient *gitmocks.Client) {
 		gitClient.On("Init").Return(nil)
 		gitClient.On("Fetch", mock.Anything).Return(nil)
-		gitClient.On("Checkout", mock.Anything).Return(nil)
+		gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
 		gitClient.On("LsRemote", revision).Return(revision, revisionErr)
 		gitClient.On("CommitSHA").Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
 		gitClient.On("Root").Return(root)
@@ -140,11 +143,81 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	assert.Equal(t, countOfManifests, len(res1.Manifests))
 
 	// this will test concatenated manifests to verify we split YAMLs correctly
-	res2, err := GenerateManifests("./testdata/concatenated", "/", "", &q, false)
+	res2, err := GenerateManifests("./testdata/concatenated", "/", "", &q, false, resource.MustParse("0"))
 	assert.NoError(t, err)
 	assert.Equal(t, 3, len(res2.Manifests))
 }
 
+func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
+	testCases := []struct {
+		name                    string
+		outOfBoundsFilename     string
+		outOfBoundsFileContents string
+		mustNotContain          string // Optional string that must not appear in error or manifest output. If empty, use outOfBoundsFileContents.
+	}{
+		{
+			name:                    "out of bounds JSON file should not appear in error output",
+			outOfBoundsFilename:     "test.json",
+			outOfBoundsFileContents: `{"some": "json"}`,
+		},
+		{
+			name:                    "malformed JSON file contents should not appear in error output",
+			outOfBoundsFilename:     "test.json",
+			outOfBoundsFileContents: "$",
+		},
+		{
+			name:                "out of bounds JSON manifest should not appear in manifest output",
+			outOfBoundsFilename: "test.json",
+			// JSON marshalling is deterministic. So if there's a leak, exactly this should appear in the manifests.
+			outOfBoundsFileContents: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+		},
+		{
+			name:                    "out of bounds YAML manifest should not appear in manifest output",
+			outOfBoundsFilename:     "test.yaml",
+			outOfBoundsFileContents: "apiVersion: v1\nkind: Secret\nmetadata:\n  name: test\n  namespace: default\ntype: Opaque",
+			mustNotContain:          `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCaseCopy := testCase
+		t.Run(testCaseCopy.name, func(t *testing.T) {
+			t.Parallel()
+
+			outOfBoundsDir := t.TempDir()
+			outOfBoundsFile := path.Join(outOfBoundsDir, testCaseCopy.outOfBoundsFilename)
+			err := os.WriteFile(outOfBoundsFile, []byte(testCaseCopy.outOfBoundsFileContents), os.FileMode(0444))
+			require.NoError(t, err)
+
+			repoDir := t.TempDir()
+			err = os.Symlink(outOfBoundsFile, path.Join(repoDir, testCaseCopy.outOfBoundsFilename))
+			require.NoError(t, err)
+
+			var mustNotContain = testCaseCopy.outOfBoundsFileContents
+			if testCaseCopy.mustNotContain != "" {
+				mustNotContain = testCaseCopy.mustNotContain
+			}
+
+			q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
+			res, err := GenerateManifests(repoDir, "", "", &q, false, resource.MustParse("0"))
+			require.Error(t, err)
+			assert.NotContains(t, err.Error(), mustNotContain)
+			assert.Contains(t, err.Error(), "illegal filepath")
+			assert.Nil(t, res)
+		})
+	}
+}
+
+func TestGenerateManifests_MissingSymlinkDestination(t *testing.T) {
+	repoDir := t.TempDir()
+	err := os.Symlink("/obviously/does/not/exist", path.Join(repoDir, "test.yaml"))
+	require.NoError(t, err)
+
+	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
+	_, err = GenerateManifests(repoDir, "", "", &q, false, resource.MustParse("0"))
+	require.NoError(t, err)
+}
+
 func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
 	service := newService("../..")
 
@@ -298,29 +371,6 @@ func TestGenerateKsonnetManifest(t *testing.T) {
 	assert.Equal(t, "https://kubernetes.default.svc", res.Server)
 }
 
-func TestGenerateHelmChartWithDependencies(t *testing.T) {
-	service := newService("../..")
-
-	cleanup := func() {
-		_ = os.Remove(filepath.Join("../../util/helm/testdata/helm2-dependency", helmDepUpMarkerFile))
-		_ = os.RemoveAll(filepath.Join("../../util/helm/testdata/helm2-dependency", "charts"))
-	}
-	cleanup()
-	defer cleanup()
-
-	helmRepo := argoappv1.Repository{Name: "bitnami", Type: "helm", Repo: "https://charts.bitnami.com/bitnami"}
-	q := apiclient.ManifestRequest{
-		Repo: &argoappv1.Repository{},
-		ApplicationSource: &argoappv1.ApplicationSource{
-			Path: "./util/helm/testdata/helm2-dependency",
-		},
-		Repos: []*argoappv1.Repository{&helmRepo},
-	}
-	res1, err := service.GenerateManifest(context.Background(), &q)
-	assert.Nil(t, err)
-	assert.Len(t, res1.Manifests, 10)
-}
-
 func TestManifestGenErrorCacheByNumRequests(t *testing.T) {
 
 	// Returns the state of the manifest generation cache, by querying the cache for the previously set result
@@ -1051,7 +1101,7 @@ func TestGenerateFromUTF16(t *testing.T) {
 		Repo:              &argoappv1.Repository{},
 		ApplicationSource: &argoappv1.ApplicationSource{},
 	}
-	res1, err := GenerateManifests("./testdata/utf-16", "/", "", &q, false)
+	res1, err := GenerateManifests("./testdata/utf-16", "/", "", &q, false, resource.MustParse("0"))
 	assert.Nil(t, err)
 	assert.Equal(t, 2, len(res1.Manifests))
 }
@@ -1067,12 +1117,15 @@ func TestListApps(t *testing.T) {
 		"app-parameters/multi":           "Kustomize",
 		"app-parameters/single-app-only": "Kustomize",
 		"app-parameters/single-global":   "Kustomize",
+		"in-bounds-values-file-link":     "Helm",
 		"invalid-helm":                   "Helm",
 		"invalid-kustomize":              "Kustomize",
 		"kustomization_yaml":             "Kustomize",
 		"kustomization_yml":              "Kustomize",
 		"my-chart":                       "Helm",
 		"my-chart-2":                     "Helm",
+		"out-of-bounds-values-file-link": "Helm",
+		"values-files":                   "Helm",
 	}
 	assert.Equal(t, expectedApps, res.Apps)
 }
@@ -1411,6 +1464,7 @@ func runWithTempTestdata(t *testing.T, path string, runner func(t *testing.T, pa
 	tempDir := mkTempParameters("./testdata/app-parameters")
 	defer os.RemoveAll(tempDir)
 	runner(t, filepath.Join(tempDir, "app-parameters", path))
+	os.RemoveAll(tempDir)
 }
 
 func TestGenerateManifestsWithAppParameterFile(t *testing.T) {
@@ -1612,11 +1666,11 @@ func TestFindResources(t *testing.T) {
 	for i := range testCases {
 		tc := testCases[i]
 		t.Run(tc.name, func(t *testing.T) {
-			objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+			objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 				Recurse: true,
 				Include: tc.include,
 				Exclude: tc.exclude,
-			})
+			}, resource.MustParse("0"))
 			if !assert.NoError(t, err) {
 				return
 			}
@@ -1630,10 +1684,10 @@ func TestFindResources(t *testing.T) {
 }
 
 func TestFindManifests_Exclude(t *testing.T) {
-	objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "subdir/deploymentSub.yaml",
-	})
+	}, resource.MustParse("0"))
 
 	if !assert.NoError(t, err) || !assert.Len(t, objs, 1) {
 		return
@@ -1643,10 +1697,10 @@ func TestFindManifests_Exclude(t *testing.T) {
 }
 
 func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
-	objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "nothing.yaml",
-	})
+	}, resource.MustParse("0"))
 
 	if !assert.NoError(t, err) || !assert.Len(t, objs, 2) {
 		return
@@ -1656,6 +1710,469 @@ func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
 		[]string{"nginx-deployment", "nginx-deployment-sub"}, []string{objs[0].GetName(), objs[1].GetName()})
 }
 
+func tempDir(t *testing.T) string {
+	dir, err := ioutil.TempDir(".", "")
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		err = os.RemoveAll(dir)
+		if err != nil {
+			panic(err)
+		}
+	})
+	absDir, err := filepath.Abs(dir)
+	require.NoError(t, err)
+	return absDir
+}
+
+func walkFor(t *testing.T, root string, testPath string, run func(info fs.FileInfo)) {
+	var hitExpectedPath = false
+	err := filepath.Walk(root, func(path string, info fs.FileInfo, err error) error {
+		if path == testPath {
+			require.NoError(t, err)
+			hitExpectedPath = true
+			run(info)
+		}
+		return nil
+	})
+	require.NoError(t, err)
+	assert.True(t, hitExpectedPath, "did not hit expected path when walking directory")
+}
+
+func Test_getPotentiallyValidManifestFile(t *testing.T) {
+	// These tests use filepath.Walk instead of os.Stat to get file info, because FileInfo from os.Stat does not return
+	// true for IsSymlink like os.Walk does.
+
+	// These tests do not use t.TempDir() because those directories can contain symlinks which cause test to fail
+	// InBound checks.
+
+	t.Run("non-JSON/YAML is skipped with an empty ignore message", func(t *testing.T) {
+		appDir := tempDir(t)
+		filePath := filepath.Join(appDir, "not-json-or-yaml")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		aPath := filepath.Join(appDir, "a.json")
+		bPath := filepath.Join(appDir, "b.json")
+		err := os.Symlink(bPath, aPath)
+		require.NoError(t, err)
+		err = os.Symlink(aPath, bPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, aPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(aPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), "too many links")
+		})
+	})
+
+	t.Run("symlink with missing destination should throw an error", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		aPath := filepath.Join(appDir, "a.json")
+		bPath := filepath.Join(appDir, "b.json")
+		err := os.Symlink(bPath, aPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, aPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(aPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.NotEmpty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		linkPath := filepath.Join(appDir, "a.json")
+		err := os.Symlink("..", linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), "illegal filepath in symlink")
+		})
+	})
+
+	t.Run("symlink to a non-regular file should be skipped with warning", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		dirPath := filepath.Join(appDir, "test.dir")
+		err := os.MkdirAll(dirPath, 0644)
+		require.NoError(t, err)
+		linkPath := filepath.Join(appDir, "test.json")
+		err = os.Symlink(dirPath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.Nil(t, realFileInfo)
+			assert.Contains(t, ignoreMessage, "non-regular file")
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("non-included file should be skipped with no message", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "not-included.yaml")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "*.json", "")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("excluded file should be skipped with no message", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "excluded.json")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "excluded.*")
+			assert.Nil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("symlink to a regular file is potentially valid", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		linkPath := filepath.Join(appDir, "link.json")
+		err = os.Symlink(filePath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("a regular file is potentially valid", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file.json")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		walkFor(t, appDir, filePath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(filePath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+
+	t.Run("realFileInfo is for the destination rather than the symlink", func(t *testing.T) {
+		appDir := tempDir(t)
+
+		filePath := filepath.Join(appDir, "regular-file")
+		file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, 0644)
+		require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
+
+		linkPath := filepath.Join(appDir, "link.json")
+		err = os.Symlink(filePath, linkPath)
+		require.NoError(t, err)
+
+		walkFor(t, appDir, linkPath, func(info fs.FileInfo) {
+			realFileInfo, ignoreMessage, err := getPotentiallyValidManifestFile(linkPath, info, appDir, appDir, "", "")
+			assert.NotNil(t, realFileInfo)
+			assert.Equal(t, filepath.Base(filePath), realFileInfo.Name())
+			assert.Empty(t, ignoreMessage)
+			assert.NoError(t, err)
+		})
+	})
+}
+
+func Test_getPotentiallyValidManifests(t *testing.T) {
+	// Tests which return no manifests and an error check to make sure the directory exists before running. A missing
+	// directory would produce those same results.
+
+	logCtx := log.WithField("test", "test")
+
+	t.Run("unreadable file throws error", func(t *testing.T) {
+		appDir := t.TempDir()
+		unreadablePath := filepath.Join(appDir, "unreadable.json")
+		err := os.WriteFile(unreadablePath, []byte{}, 0666)
+		require.NoError(t, err)
+		err = os.Chmod(appDir, 0000)
+		require.NoError(t, err)
+
+		manifests, err := getPotentiallyValidManifests(logCtx, appDir, appDir, false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+
+		// allow cleanup
+		err = os.Chmod(appDir, 0777)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	t.Run("no recursion when recursion is disabled", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/recurse", "./testdata/recurse", false, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("recursion when recursion is enabled", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/recurse", "./testdata/recurse", true, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+	})
+
+	t.Run("non-JSON/YAML is skipped", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/circular-link")
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/out-of-bounds-link")
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("symlink to a regular file works", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		manifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, false, "", "", resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("symlink to nowhere should be ignored", func(t *testing.T) {
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", false, "", "", resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("link to over-sized manifest fails", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		// The file is 35 bytes.
+		manifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, false, "", "", resource.MustParse("34"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
+		// There is a total of 10 files, ech file being 10 bytes.
+		manifests, err := getPotentiallyValidManifests(logCtx, "./testdata/several-files", "./testdata/several-files", false, "", "", resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = getPotentiallyValidManifests(logCtx, "./testdata/several-files", "./testdata/several-files", false, "", "", resource.MustParse("100"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+}
+
+func Test_findManifests(t *testing.T) {
+	logCtx := log.WithField("test", "test")
+	noRecurse := argoappv1.ApplicationSourceDirectory{Recurse: false}
+
+	t.Run("unreadable file throws error", func(t *testing.T) {
+		appDir := t.TempDir()
+		unreadablePath := filepath.Join(appDir, "unreadable.json")
+		err := os.WriteFile(unreadablePath, []byte{}, 0666)
+		require.NoError(t, err)
+		err = os.Chmod(appDir, 0000)
+		require.NoError(t, err)
+
+		manifests, err := findManifests(logCtx, appDir, appDir, nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+
+		// allow cleanup
+		err = os.Chmod(appDir, 0777)
+		if err != nil {
+			panic(err)
+		}
+	})
+
+	t.Run("no recursion when recursion is disabled", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, noRecurse, resource.MustParse("0"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+	})
+
+	t.Run("recursion when recursion is enabled", func(t *testing.T) {
+		recurse := argoappv1.ApplicationSourceDirectory{Recurse: true}
+		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, recurse, resource.MustParse("0"))
+		assert.Len(t, manifests, 4)
+		assert.NoError(t, err)
+	})
+
+	t.Run("non-JSON/YAML is skipped", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("circular link should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/circular-link")
+		manifests, err := findManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/out-of-bounds-link")
+		manifests, err := findManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("symlink to a regular file works", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("symlink to nowhere should be ignored", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.NoError(t, err)
+	})
+
+	t.Run("link to over-sized manifest fails", func(t *testing.T) {
+		repoRoot, err := filepath.Abs("./testdata/in-bounds-link")
+		require.NoError(t, err)
+		appPath, err := filepath.Abs("./testdata/in-bounds-link/app")
+		require.NoError(t, err)
+		// The file is 35 bytes.
+		manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse, resource.MustParse("34"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
+		// There is a total of 10 files, each file being 10 bytes.
+		manifests, err := findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, resource.MustParse("365"))
+		assert.Len(t, manifests, 10)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, resource.MustParse("364"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("jsonnet isn't counted against size limit", func(t *testing.T) {
+		// Each file is 36 bytes. Only the 36-byte json file should be counted against the limit.
+		manifests, err := findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, resource.MustParse("36"))
+		assert.Len(t, manifests, 2)
+		assert.NoError(t, err)
+
+		manifests, err = findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, resource.MustParse("35"))
+		assert.Empty(t, manifests)
+		assert.ErrorIs(t, err, ErrExceededMaxCombinedManifestFileSize)
+	})
+
+	t.Run("partially valid YAML file throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/partially-valid-yaml")
+		manifests, err := findManifests(logCtx, "./testdata/partially-valid-yaml", "./testdata/partially-valid-yaml", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalid manifest throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/invalid-manifests")
+		manifests, err := findManifests(logCtx, "./testdata/invalid-manifests", "./testdata/invalid-manifests", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("irrelevant YAML gets skipped, relevant YAML gets parsed", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/irrelevant-yaml", "./testdata/irrelevant-yaml", nil, noRecurse, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("multiple JSON objects in one file throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/json-list")
+		manifests, err := findManifests(logCtx, "./testdata/json-list", "./testdata/json-list", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalid JSON throws an error", func(t *testing.T) {
+		require.DirExists(t, "./testdata/invalid-json")
+		manifests, err := findManifests(logCtx, "./testdata/invalid-json", "./testdata/invalid-json", nil, noRecurse, resource.MustParse("0"))
+		assert.Empty(t, manifests)
+		assert.Error(t, err)
+	})
+
+	t.Run("valid JSON returns manifest and no error", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/valid-json", "./testdata/valid-json", nil, noRecurse, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+
+	t.Run("YAML with an empty document doesn't throw an error", func(t *testing.T) {
+		manifests, err := findManifests(logCtx, "./testdata/yaml-with-empty-document", "./testdata/yaml-with-empty-document", nil, noRecurse, resource.MustParse("0"))
+		assert.Len(t, manifests, 1)
+		assert.NoError(t, err)
+	})
+}
+
 func TestTestRepoOCI(t *testing.T) {
 	service := newService(".")
 	_, err := service.TestRepository(context.Background(), &apiclient.TestRepositoryRequest{
@@ -1679,3 +2196,52 @@ func Test_getHelmDependencyRepos(t *testing.T) {
 	assert.Equal(t, repos[0].Repo, repo1)
 	assert.Equal(t, repos[1].Repo, repo2)
 }
+
+func Test_findHelmValueFilesInPath(t *testing.T) {
+	t.Run("does not exist", func(t *testing.T) {
+		files, err := findHelmValueFilesInPath("/obviously/does/not/exist")
+		assert.Error(t, err)
+		assert.Empty(t, files)
+	})
+	t.Run("values files", func(t *testing.T) {
+		files, err := findHelmValueFilesInPath("./testdata/values-files")
+		assert.NoError(t, err)
+		assert.Len(t, files, 4)
+	})
+}
+
+func Test_populateHelmAppDetails(t *testing.T) {
+	res := apiclient.RepoAppDetailsResponse{}
+	q := apiclient.RepoServerAppDetailsQuery{
+		Repo: &argoappv1.Repository{},
+		Source: &argoappv1.ApplicationSource{
+			Helm: &argoappv1.ApplicationSourceHelm{ValueFiles: []string{"exclude.yaml", "has-the-word-values.yaml"}},
+		},
+	}
+	appPath, err := filepath.Abs("./testdata/values-files/")
+	require.NoError(t, err)
+	err = populateHelmAppDetails(&res, appPath, appPath, &q)
+	require.NoError(t, err)
+	assert.Len(t, res.Helm.Parameters, 3)
+	assert.Len(t, res.Helm.ValueFiles, 4)
+}
+
+func Test_populateHelmAppDetails_values_symlinks(t *testing.T) {
+	t.Run("inbound", func(t *testing.T) {
+		res := apiclient.RepoAppDetailsResponse{}
+		q := apiclient.RepoServerAppDetailsQuery{Repo: &argoappv1.Repository{}, Source: &argoappv1.ApplicationSource{}}
+		err := populateHelmAppDetails(&res, "./testdata/in-bounds-values-file-link/", "./testdata/in-bounds-values-file-link/", &q)
+		require.NoError(t, err)
+		assert.NotEmpty(t, res.Helm.Values)
+		assert.NotEmpty(t, res.Helm.Parameters)
+	})
+
+	t.Run("out of bounds", func(t *testing.T) {
+		res := apiclient.RepoAppDetailsResponse{}
+		q := apiclient.RepoServerAppDetailsQuery{Repo: &argoappv1.Repository{}, Source: &argoappv1.ApplicationSource{}}
+		err := populateHelmAppDetails(&res, "./testdata/out-of-bounds-values-file-link/", "./testdata/out-of-bounds-values-file-link/", &q)
+		require.NoError(t, err)
+		assert.Empty(t, res.Helm.Values)
+		assert.Empty(t, res.Helm.Parameters)
+	})
+}

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

server/server.go

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

server/server_test.go

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

test/e2e/fixture/fixture.go

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

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

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

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

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

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

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

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

util/dex/dex.go

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

util/dex/dex_test.go

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

util/helm/helm.go

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

util/helm/helm_test.go

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

util/helm/testdata/dependency/Chart.lock

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

util/io/bytereadseeker.go

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

util/io/bytereadseeker_test.go

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

util/io/files/util.go

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

util/io/files/util_test.go

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

util/io/path/resolved.go

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

util/lua/custom_actions_test.go

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

util/oidc/oidc.go

@@ -107,10 +107,7 @@ func NewClientApp(settings *settings.ArgoCDSettings, cache OIDCStateStorage, dex
 	if err != nil {
 		return nil, fmt.Errorf("parse redirect-uri: %v", err)
 	}
-	tlsConfig := settings.TLSConfig()
-	if tlsConfig != nil {
-		tlsConfig.InsecureSkipVerify = true
-	}
+	tlsConfig := settings.OIDCTLSConfig()
 	a.client = &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: tlsConfig,
@@ -152,17 +149,23 @@ func (a *ClientApp) oauth2Config(scopes []string) (*oauth2.Config, error) {
 }
 
 // generateAppState creates an app state nonce
-func (a *ClientApp) generateAppState(returnURL string) string {
-	randStr := rand.RandString(10)
+func (a *ClientApp) generateAppState(returnURL string) (string, error) {
+	// According to the spec (https://www.rfc-editor.org/rfc/rfc6749#section-10.10), this must be guessable with
+	// probability <= 2^(-128). The following call generates one of 52^24 random strings, ~= 2^136 possibilities.
+	randStr, err := rand.String(24)
+	if err != nil {
+		return "", fmt.Errorf("failed to generate app state: %w", err)
+	}
 	if returnURL == "" {
 		returnURL = a.baseHRef
 	}
-	err := a.cache.SetOIDCState(randStr, &OIDCState{ReturnURL: returnURL})
+	err = a.cache.SetOIDCState(randStr, &OIDCState{ReturnURL: returnURL})
 	if err != nil {
 		// This should never happen with the in-memory cache
 		log.Errorf("Failed to set app state: %v", err)
+		return "", fmt.Errorf("failed to set app state: %v", err)
 	}
-	return randStr
+	return randStr, nil
 }
 
 func (a *ClientApp) verifyAppState(state string) (*OIDCState, error) {
@@ -251,14 +254,24 @@ func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Invalid redirect URL: the protocol and host (including port) must match and the path must be within allowed URLs if provided", http.StatusBadRequest)
 		return
 	}
-	stateNonce := a.generateAppState(returnURL)
+	stateNonce, err := a.generateAppState(returnURL)
+	if err != nil {
+		log.Errorf("Failed to initiate login flow: %v", err)
+		http.Error(w, "Failed to initiate login flow", http.StatusInternalServerError)
+		return
+	}
 	grantType := InferGrantType(oidcConf)
 	var url string
 	switch grantType {
 	case GrantTypeAuthorizationCode:
 		url = oauth2Config.AuthCodeURL(stateNonce, opts...)
 	case GrantTypeImplicit:
-		url = ImplicitFlowURL(oauth2Config, stateNonce, opts...)
+		url, err = ImplicitFlowURL(oauth2Config, stateNonce, opts...)
+		if err != nil {
+			log.Errorf("Failed to initiate implicit login flow: %v", err)
+			http.Error(w, "Failed to initiate implicit login flow", http.StatusInternalServerError)
+			return
+		}
 	default:
 		http.Error(w, fmt.Sprintf("Unsupported grant type: %v", grantType), http.StatusInternalServerError)
 		return
@@ -390,10 +403,14 @@ func (a *ClientApp) handleImplicitFlow(w http.ResponseWriter, state string) {
 
 // ImplicitFlowURL is an adaptation of oauth2.Config::AuthCodeURL() which returns a URL
 // appropriate for an OAuth2 implicit login flow (as opposed to authorization code flow).
-func ImplicitFlowURL(c *oauth2.Config, state string, opts ...oauth2.AuthCodeOption) string {
+func ImplicitFlowURL(c *oauth2.Config, state string, opts ...oauth2.AuthCodeOption) (string, error) {
 	opts = append(opts, oauth2.SetAuthURLParam("response_type", "id_token"))
-	opts = append(opts, oauth2.SetAuthURLParam("nonce", rand.RandString(10)))
-	return c.AuthCodeURL(state, opts...)
+	randString, err := rand.String(24)
+	if err != nil {
+		return "", fmt.Errorf("failed to generate nonce for implicit flow URL: %w", err)
+	}
+	opts = append(opts, oauth2.SetAuthURLParam("nonce", randString))
+	return c.AuthCodeURL(state, opts...), nil
 }
 
 // OfflineAccess returns whether or not 'offline_access' is a supported scope

util/oidc/oidc_test.go

@@ -1,7 +1,9 @@
 package oidc
 
 import (
+	"crypto/tls"
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"net/http/httptest"
 	"net/url"
@@ -9,9 +11,12 @@ import (
 
 	gooidc "github.com/coreos/go-oidc"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
 
 	"github.com/argoproj/argo-cd/v2/server/settings/oidc"
+	"github.com/argoproj/argo-cd/v2/util/settings"
+	"github.com/argoproj/argo-cd/v2/util/test"
 )
 
 func TestInferGrantType(t *testing.T) {
@@ -97,6 +102,176 @@ func TestHandleCallback(t *testing.T) {
 	assert.Equal(t, "login-failed: <script>alert('hello')</script>\n", w.Body.String())
 }
 
+type mockOIDCStateStorage struct {}
+
+func (m mockOIDCStateStorage) GetOIDCState(key string) (*OIDCState, error) {
+	return nil, nil
+}
+
+func (m mockOIDCStateStorage) SetOIDCState(key string, state *OIDCState) error {
+	return nil
+}
+
+func getTestCache() OIDCStateStorage {
+	return mockOIDCStateStorage{}
+}
+
+func TestClientApp_HandleLogin(t *testing.T) {
+	oidcTestServer := test.GetOIDCTestServer(t)
+	t.Cleanup(oidcTestServer.Close)
+
+	dexTestServer := test.GetDexTestServer(t)
+	t.Cleanup(dexTestServer.Close)
+
+	t.Run("oidc certificate checking during login should toggle on config", func(t *testing.T) {
+		cdSettings := &settings.ArgoCDSettings{
+			URL: "https://argocd.example.com",
+			OIDCConfigRAW: fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`, oidcTestServer.URL),
+		}
+		app, err := NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		req := httptest.NewRequest("GET", "https://argocd.example.com/auth/login", nil)
+
+		w := httptest.NewRecorder()
+
+		app.HandleLogin(w, req)
+
+		assert.Contains(t, w.Body.String(), "certificate is not trusted")
+
+		cdSettings.OIDCTLSInsecureSkipVerify = true
+
+		app, err = NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		w = httptest.NewRecorder()
+
+		app.HandleLogin(w, req)
+
+		assert.NotContains(t, w.Body.String(), "certificate is not trusted")
+	})
+
+	t.Run("dex certificate checking during login should toggle on config", func(t *testing.T) {
+		cdSettings := &settings.ArgoCDSettings{
+			URL: "https://argocd.example.com",
+			DexConfig: `connectors:
+- type: github
+  name: GitHub
+  config:
+    clientID: aabbccddeeff00112233
+    clientSecret: aabbccddeeff00112233`,
+		}
+		cert, err := tls.X509KeyPair(test.Cert, test.PrivateKey)
+		require.NoError(t, err)
+		cdSettings.Certificate = &cert
+
+		app, err := NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		req := httptest.NewRequest("GET", "https://argocd.example.com/auth/login", nil)
+
+		w := httptest.NewRecorder()
+
+		app.HandleLogin(w, req)
+
+		assert.Contains(t, w.Body.String(), "certificate signed by unknown authority")
+
+		cdSettings.OIDCTLSInsecureSkipVerify = true
+
+		app, err = NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		w = httptest.NewRecorder()
+
+		app.HandleLogin(w, req)
+
+		assert.NotContains(t, w.Body.String(), "certificate signed by unknown authority")
+	})
+}
+
+func TestClientApp_HandleCallback(t *testing.T) {
+	oidcTestServer := test.GetOIDCTestServer(t)
+	t.Cleanup(oidcTestServer.Close)
+
+	dexTestServer := test.GetDexTestServer(t)
+	t.Cleanup(dexTestServer.Close)
+
+	t.Run("oidc certificate checking during oidc callback should toggle on config", func(t *testing.T) {
+		cdSettings := &settings.ArgoCDSettings{
+			URL: "https://argocd.example.com",
+			OIDCConfigRAW: fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`, oidcTestServer.URL),
+		}
+		app, err := NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		req := httptest.NewRequest("GET", "https://argocd.example.com/auth/callback", nil)
+
+		w := httptest.NewRecorder()
+
+		app.HandleCallback(w, req)
+
+		assert.Contains(t, w.Body.String(), "certificate is not trusted")
+
+		cdSettings.OIDCTLSInsecureSkipVerify = true
+
+		app, err = NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		w = httptest.NewRecorder()
+
+		app.HandleCallback(w, req)
+
+		assert.NotContains(t, w.Body.String(), "certificate is not trusted")
+	})
+
+	t.Run("dex certificate checking during oidc callback should toggle on config", func(t *testing.T) {
+		cdSettings := &settings.ArgoCDSettings{
+			URL: "https://argocd.example.com",
+			DexConfig: `connectors:
+- type: github
+  name: GitHub
+  config:
+    clientID: aabbccddeeff00112233
+    clientSecret: aabbccddeeff00112233`,
+		}
+		cert, err := tls.X509KeyPair(test.Cert, test.PrivateKey)
+		require.NoError(t, err)
+		cdSettings.Certificate = &cert
+
+		app, err := NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		req := httptest.NewRequest("GET", "https://argocd.example.com/auth/callback", nil)
+
+		w := httptest.NewRecorder()
+
+		app.HandleCallback(w, req)
+
+		assert.Contains(t, w.Body.String(), "certificate signed by unknown authority")
+
+		cdSettings.OIDCTLSInsecureSkipVerify = true
+
+		app, err = NewClientApp(cdSettings, getTestCache(), dexTestServer.URL, "https://argocd.example.com")
+		require.NoError(t, err)
+
+		w = httptest.NewRecorder()
+
+		app.HandleCallback(w, req)
+
+		assert.NotContains(t, w.Body.String(), "certificate signed by unknown authority")
+	})
+}
+
 func TestIsValidRedirect(t *testing.T) {
 	var tests = []struct {
 		name        string

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

util/session/sessionmanager.go

@@ -124,10 +124,7 @@ func NewSessionManager(settingsMgr *settings.SettingsManager, projectsLister v1a
 	if err != nil {
 		panic(err)
 	}
-	tlsConfig := settings.TLSConfig()
-	if tlsConfig != nil {
-		tlsConfig.InsecureSkipVerify = true
-	}
+	tlsConfig := settings.OIDCTLSConfig()
 	s.client = &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: tlsConfig,
@@ -246,7 +243,7 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error)
 	token, err := jwt.ParseWithClaims(tokenString, &claims, func(token *jwt.Token) (interface{}, error) {
 		// Don't forget to validate the alg is what you expect:
 		if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
-			return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"])
+			return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
 		}
 		return argoCDSettings.ServerSignature, nil
 	})
@@ -298,7 +295,7 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error)
 	}
 
 	if account.PasswordMtime != nil && issuedAt.Before(*account.PasswordMtime) {
-		return nil, "", fmt.Errorf("Account password has changed since token issued")
+		return nil, "", fmt.Errorf("account password has changed since token issued")
 	}
 
 	newToken := ""
@@ -515,7 +512,7 @@ func (mgr *SessionManager) VerifyToken(tokenString string) (jwt.Claims, string,
 		// IDP signed token
 		prov, err := mgr.provider()
 		if err != nil {
-			return claims, "", err
+			return nil, "", err
 		}
 
 		// Token must be verified for at least one audience
@@ -527,16 +524,30 @@ func (mgr *SessionManager) VerifyToken(tokenString string) (jwt.Claims, string,
 				break
 			}
 		}
+
+		// The token verification has failed. If the token has expired, we will
+		// return a dummy claims only containing a value for the issuer, so the
+		// UI can handle expired tokens appropriately.
 		if err != nil {
-			return claims, "", err
+			if strings.HasPrefix(err.Error(), "oidc: token is expired") {
+				claims = jwt.StandardClaims{
+					Issuer: "sso",
+				}
+				return claims, "", err
+			}
+			return nil, "", err
 		}
+
 		if idToken == nil {
-			return claims, "", fmt.Errorf("No audience found in the token")
+			return nil, "", fmt.Errorf("no audience found in the token")
 		}
 
 		var claims jwt.MapClaims
 		err = idToken.Claims(&claims)
-		return claims, "", err
+		if err != nil {
+			return nil, "", err
+		}
+		return claims, "", nil
 	}
 }
 

util/session/sessionmanager_test.go

@@ -2,6 +2,7 @@ package session
 
 import (
 	"context"
+	"encoding/pem"
 	"fmt"
 	"math"
 	"os"
@@ -28,6 +29,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/errors"
 	"github.com/argoproj/argo-cd/v2/util/password"
 	"github.com/argoproj/argo-cd/v2/util/settings"
+	utiltest "github.com/argoproj/argo-cd/v2/util/test"
 )
 
 func getProjLister(objects ...runtime.Object) v1alpha1.AppProjectNamespaceLister {
@@ -456,3 +458,239 @@ func TestFailedAttemptsExpiry(t *testing.T) {
 
 	os.Setenv(envLoginFailureWindowSeconds, "")
 }
+
+func getKubeClientWithConfig(config map[string]string, secretConfig map[string][]byte) *fake.Clientset {
+	mergedSecretConfig := map[string][]byte{
+		"server.secretkey": []byte("Hello, world!"),
+	}
+	for key, value := range secretConfig {
+		mergedSecretConfig[key] = value
+	}
+
+	return fake.NewSimpleClientset(&corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "argocd-cm",
+			Namespace: "argocd",
+			Labels: map[string]string{
+				"app.kubernetes.io/part-of": "argocd",
+			},
+		},
+		Data: config,
+	}, &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "argocd-secret",
+			Namespace: "argocd",
+		},
+		Data: mergedSecretConfig,
+	})
+}
+
+func TestSessionManager_VerifyToken(t *testing.T) {
+	oidcTestServer := utiltest.GetOIDCTestServer(t)
+	t.Cleanup(oidcTestServer.Close)
+
+	dexTestServer := utiltest.GetDexTestServer(t)
+	t.Cleanup(dexTestServer.Close)
+
+	t.Run("oidcConfig.rootCA is respected", func(t *testing.T) {
+		cert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: oidcTestServer.TLS.Certificates[0].Certificate[0]})
+
+		dexConfig := map[string]string{
+			"url": "",
+			"oidc.config": fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]
+rootCA: |
+  %s
+`, oidcTestServer.URL, strings.Replace(string(cert), "\n", "\n  ", -1)),
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(dexConfig, nil), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), "", NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.At(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = oidcTestServer.URL
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
+		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		// If the root CA is being respected, we won't get this error.
+		assert.NotContains(t, err.Error(), "certificate is not trusted")
+	})
+
+	t.Run("OIDC provider is Dex, TLS is configured", func(t *testing.T) {
+		dexConfig := map[string]string{
+			"url": dexTestServer.URL,
+			"dex.config": `connectors:
+- type: github
+  name: GitHub
+  config:
+    clientID: aabbccddeeff00112233
+    clientSecret: aabbccddeeff00112233`,
+		}
+
+		// This is not actually used in the test. The test only calls the OIDC test server. But a valid cert/key pair
+		// must be set to test VerifyToken's behavior when Argo CD is configured with TLS enabled.
+		secretConfig := map[string][]byte{
+			"tls.crt": utiltest.Cert,
+			"tls.key": utiltest.PrivateKey,
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(dexConfig, secretConfig), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), dexTestServer.URL, NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.At(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = fmt.Sprintf("%s/api/dex", dexTestServer.URL)
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
+		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "certificate signed by unknown authority")
+	})
+
+	t.Run("OIDC provider is external, TLS is configured", func(t *testing.T) {
+		dexConfig := map[string]string{
+			"url": "",
+			"oidc.config": fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`, oidcTestServer.URL),
+		}
+
+		// This is not actually used in the test. The test only calls the OIDC test server. But a valid cert/key pair
+		// must be set to test VerifyToken's behavior when Argo CD is configured with TLS enabled.
+		secretConfig := map[string][]byte{
+			"tls.crt": utiltest.Cert,
+			"tls.key": utiltest.PrivateKey,
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(dexConfig, secretConfig), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), "", NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.At(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = oidcTestServer.URL
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
+		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "certificate is not trusted")
+	})
+
+	t.Run("OIDC provider is Dex, TLS is configured", func(t *testing.T) {
+		dexConfig := map[string]string{
+			"url": dexTestServer.URL,
+			"dex.config": `connectors:
+- type: github
+  name: GitHub
+  config:
+    clientID: aabbccddeeff00112233
+    clientSecret: aabbccddeeff00112233`,
+		}
+
+		// This is not actually used in the test. The test only calls the OIDC test server. But a valid cert/key pair
+		// must be set to test VerifyToken's behavior when Argo CD is configured with TLS enabled.
+		secretConfig := map[string][]byte{
+			"tls.crt": utiltest.Cert,
+			"tls.key": utiltest.PrivateKey,
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(dexConfig, secretConfig), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), dexTestServer.URL, NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.At(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = fmt.Sprintf("%s/api/dex", dexTestServer.URL)
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
+		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "certificate signed by unknown authority")
+	})
+
+	t.Run("OIDC provider is external, TLS is configured, OIDCTLSInsecureSkipVerify is true", func(t *testing.T) {
+		dexConfig := map[string]string{
+			"url": "",
+			"oidc.config": fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`, oidcTestServer.URL),
+			"oidc.tls.insecure.skip.verify": "true",
+		}
+
+		// This is not actually used in the test. The test only calls the OIDC test server. But a valid cert/key pair
+		// must be set to test VerifyToken's behavior when Argo CD is configured with TLS enabled.
+		secretConfig := map[string][]byte{
+			"tls.crt": utiltest.Cert,
+			"tls.key": utiltest.PrivateKey,
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(dexConfig, secretConfig), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), "", NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.At(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = oidcTestServer.URL
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
+		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		assert.NotContains(t, err.Error(), "certificate signed by unknown authority")
+	})
+
+	t.Run("OIDC provider is external, TLS is not configured, OIDCTLSInsecureSkipVerify is true", func(t *testing.T) {
+		dexConfig := map[string]string{
+			"url": "",
+			"oidc.config": fmt.Sprintf(`
+name: Test
+issuer: %s
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`, oidcTestServer.URL),
+			"oidc.tls.insecure.skip.verify": "true",
+		}
+
+		settingsMgr := settings.NewSettingsManager(context.Background(), getKubeClientWithConfig(dexConfig, nil), "argocd")
+		mgr := NewSessionManager(settingsMgr, getProjLister(), "", NewUserStateStorage(nil))
+		mgr.verificationDelayNoiseEnabled = false
+
+		claims := jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.At(time.Now().Add(time.Hour * 24))}
+		claims.Issuer = oidcTestServer.URL
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		key, err := jwt.ParseRSAPrivateKeyFromPEM(utiltest.PrivateKey)
+		require.NoError(t, err)
+		tokenString, err := token.SignedString(key)
+		require.NoError(t, err)
+
+		_, _, err = mgr.VerifyToken(tokenString)
+		// This is the error thrown when the test server's certificate _is_ being verified.
+		assert.NotContains(t, err.Error(), "certificate is not trusted")
+	})
+}

util/settings/settings.go

@@ -88,6 +88,11 @@ type ArgoCDSettings struct {
 	UiBannerPosition string `json:"uiBannerPosition,omitempty"`
 	// PasswordPattern for password regular expression
 	PasswordPattern string `json:"passwordPattern,omitempty"`
+	// OIDCTLSInsecureSkipVerify determines whether certificate verification is skipped when verifying tokens with the
+	// configured OIDC provider (either external or the bundled Dex instance). Setting this to `true` will cause JWT
+	// token verification to pass despite the OIDC provider having an invalid certificate. Only set to `true` if you
+	// understand the risks.
+	OIDCTLSInsecureSkipVerify bool `json:"oidcTLSInsecureSkipVerify"`
 }
 
 type GoogleAnalytics struct {
@@ -117,6 +122,7 @@ type OIDCConfig struct {
 	RequestedScopes        []string               `json:"requestedScopes,omitempty"`
 	RequestedIDTokenClaims map[string]*oidc.Claim `json:"requestedIDTokenClaims,omitempty"`
 	LogoutURL              string                 `json:"logoutURL,omitempty"`
+	RootCA                 string                 `json:"rootCA,omitempty"`
 }
 
 // DEPRECATED. Helm repository credentials are now managed using RepoCredentials
@@ -382,6 +388,8 @@ const (
 	settingsPasswordPatternKey = "passwordPattern"
 	// helmValuesFileSchemesKey is the key to configure the list of supported helm values file schemas
 	helmValuesFileSchemesKey = "helm.valuesFileSchemes"
+	// oidcTLSInsecureSkipVerifyKey is the key to configure whether TLS cert verification is skipped for OIDC connections
+	oidcTLSInsecureSkipVerifyKey = "oidc.tls.insecure.skip.verify"
 )
 
 // SettingsManager holds config info for a new manager with which to access Kubernetes ConfigMaps.
@@ -1187,6 +1195,7 @@ func updateSettingsFromConfigMap(settings *ArgoCDSettings, argoCDCM *apiv1.Confi
 	if settings.PasswordPattern == "" {
 		settings.PasswordPattern = common.PasswordPatten
 	}
+	settings.OIDCTLSInsecureSkipVerify = argoCDCM.Data[oidcTLSInsecureSkipVerifyKey] == "true"
 }
 
 // validateExternalURL ensures the external URL that is set on the configmap is valid
@@ -1551,6 +1560,33 @@ func (a *ArgoCDSettings) OAuth2ClientSecret() string {
 	return ""
 }
 
+// OIDCTLSConfig returns the TLS config for the OIDC provider. If an external provider is configured, returns a TLS
+// config using the root CAs (if any) specified in the OIDC config. If an external OIDC provider is not configured,
+// returns the API server TLS config, because the API server proxies requests to Dex.
+func (a *ArgoCDSettings) OIDCTLSConfig() *tls.Config {
+	var tlsConfig *tls.Config
+
+	oidcConfig := a.OIDCConfig()
+	if oidcConfig != nil {
+		tlsConfig = &tls.Config{}
+		if oidcConfig.RootCA != "" {
+			certPool := x509.NewCertPool()
+			ok := certPool.AppendCertsFromPEM([]byte(oidcConfig.RootCA))
+			if !ok {
+				log.Warn("failed to append certificates from PEM: proceeding without custom rootCA")
+			} else {
+				tlsConfig.RootCAs = certPool
+			}
+		}
+	} else {
+		tlsConfig = a.TLSConfig()
+	}
+	if tlsConfig != nil && a.OIDCTLSInsecureSkipVerify {
+		tlsConfig.InsecureSkipVerify = true
+	}
+	return tlsConfig
+}
+
 func appendURLPath(inputURL string, inputPath string) (string, error) {
 	u, err := url.Parse(inputURL)
 	if err != nil {

util/settings/settings_test.go

@@ -4,12 +4,17 @@ import (
 	"context"
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"sort"
+	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	testutil "github.com/argoproj/argo-cd/v2/test"
+	"github.com/argoproj/argo-cd/v2/util/test"
 
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
@@ -1016,3 +1021,68 @@ func TestGetHelmSettings(t *testing.T) {
 		})
 	}
 }
+func TestArgoCDSettings_OIDCTLSConfig_OIDCTLSInsecureSkipVerify(t *testing.T) {
+	certParsed, err := tls.X509KeyPair(test.Cert, test.PrivateKey)
+	require.NoError(t, err)
+
+	testCases := []struct{
+		name string
+		settings *ArgoCDSettings
+		expectNilTLSConfig bool
+	}{
+		{
+			name: "OIDC configured, no root CA",
+			settings: &ArgoCDSettings{OIDCConfigRAW: `name: Test
+issuer: aaa
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]`},
+		},
+		{
+			name: "OIDC configured, valid root CA",
+			settings: &ArgoCDSettings{OIDCConfigRAW: fmt.Sprintf(`
+name: Test
+issuer: aaa
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]
+rootCA: |
+  %s
+`, strings.Replace(string(test.Cert), "\n", "\n  ", -1))},
+		},
+		{
+			name: "OIDC configured, invalid root CA",
+			settings: &ArgoCDSettings{OIDCConfigRAW: `name: Test
+issuer: aaa
+clientID: xxx
+clientSecret: yyy
+requestedScopes: ["oidc"]
+rootCA: "invalid"`},
+		},
+		{
+			name: "OIDC not configured, no cert configured",
+			settings: &ArgoCDSettings{},
+			expectNilTLSConfig: true,
+		},
+		{
+			name: "OIDC not configured, cert configured",
+			settings: &ArgoCDSettings{Certificate: &certParsed},
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCase := testCase
+
+		t.Run(testCase.name, func(t *testing.T) {
+			if testCase.expectNilTLSConfig {
+				assert.Nil(t, testCase.settings.OIDCTLSConfig())
+			} else {
+				assert.False(t, testCase.settings.OIDCTLSConfig().InsecureSkipVerify)
+
+				testCase.settings.OIDCTLSInsecureSkipVerify = true
+
+				assert.True(t, testCase.settings.OIDCTLSConfig().InsecureSkipVerify)
+			}
+		})
+	}
+}

util/test/testutil.go

@@ -0,0 +1,140 @@
+package test
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// Cert is a certificate for tests. It was generated like this:
+// 		opts := tls.CertOptions{Hosts: []string{"localhost"}, Organization: "Acme"}
+//		certBytes, privKey, err := tls.generatePEM(opts)
+var Cert = []byte(`-----BEGIN CERTIFICATE-----
+MIIC8zCCAdugAwIBAgIQCSoocl6e/FR4mQy1wX6NbjANBgkqhkiG9w0BAQsFADAP
+MQ0wCwYDVQQKEwRBY21lMB4XDTIyMDYyMjE3Mjk1MloXDTIzMDYyMjE3Mjk1Mlow
+DzENMAsGA1UEChMEQWNtZTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
+ANih5Kdn3tEXh6gLfQYplhHnNq8lmSMoPY7wdwXT95sxX9GzrVpR5tRQBExcR+Ie
+Y2AElGmlhMETTchpU9RoU6fozjAuMYTkm+f0pyNnbdhCE5LnUBSrEhVHSQJ3ajs5
+I6z9qS+H4uG+yVobiwzt+rnwD+Jdpt7ZwLHhkkkyHHFr8yxRVLN8LBzh8TnCRgj9
+We64s8ZepkymC/2fhh6jdezibJQ3/dNbj17FHgwmC9oooBj4QwKOpPDzrH26aixu
+6aAg0yudBS50uahKHI8bfieGYwRFk1PwzhV1mLLc324ZvDmT0KUkhIgQsaYPs47Z
+EHwsmcVweUUPOAmO/H1ziPUCAwEAAaNLMEkwDgYDVR0PAQH/BAQDAgWgMBMGA1Ud
+JQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwFAYDVR0RBA0wC4IJbG9jYWxo
+b3N0MA0GCSqGSIb3DQEBCwUAA4IBAQA+8cGJfYRhXQxan7FATsbtC+1DwW1cPc60
+5eLOuI0jPdvXLDmtOulBEjR4KOfJ5oTKXGjs/+gR3sffP6s8gm2XFQn4+OsmxHbO
+b2RjPHgKUtJmrI4ZCN8iPGlKIar5u6Q8NZwzpeZ2XL0bpPp7RQsfHqMyhsqDinWR
+vvwQB+Bri0oIOtzW2645vWmYc2SaFMd8+8g6Ipa+PRSJezeUxIVZG12zlhsio18F
+9SHY2ONcYISjfrGTIcu4cZRGxCZGTIwMngBlb71mia+K7uH+UE6qfJy/t6KiFsCP
+yOwMb95nGQSQLDNoGr8gwgE2qPuR0kR9Z5OrWF0DoVCyL3xnxr02
+-----END CERTIFICATE-----`)
+
+// PrivateKey is an RSA key used only for tests.
+var PrivateKey = []byte(`-----BEGIN RSA PRIVATE KEY-----
+MIIEogIBAAKCAQEA2KHkp2fe0ReHqAt9BimWEec2ryWZIyg9jvB3BdP3mzFf0bOt
+WlHm1FAETFxH4h5jYASUaaWEwRNNyGlT1GhTp+jOMC4xhOSb5/SnI2dt2EITkudQ
+FKsSFUdJAndqOzkjrP2pL4fi4b7JWhuLDO36ufAP4l2m3tnAseGSSTIccWvzLFFU
+s3wsHOHxOcJGCP1Z7rizxl6mTKYL/Z+GHqN17OJslDf901uPXsUeDCYL2iigGPhD
+Ao6k8POsfbpqLG7poCDTK50FLnS5qEocjxt+J4ZjBEWTU/DOFXWYstzfbhm8OZPQ
+pSSEiBCxpg+zjtkQfCyZxXB5RQ84CY78fXOI9QIDAQABAoIBAG8jL0FLIp62qZvm
+uO9ualUo/37/lP7aaCpq50UQJ9lwjS3yNh8+IWQO4QWj2iUBXg4mi1Vf2ymKk78b
+eixgkXp1D0Lcj/8ToYBwnUami04FKDGXhhf0Y8SS27vuM4vKlqjrQd7modkangYi
+V0X82UKHDD8fuLpfkGIxzXDLypfMzjMuVpSntnWaf2YX3VR/0/66yEp9GejftF2k
+wqhGoWM6r68pN5XuCqWd5PRluSoDy/o4BAFMhYCSfp9PjgZE8aoeWHgYzlZ3gUyn
+r+HaDDNWbibhobXk/9h8lwAJ6KCZ5RZ+HFfh0HuwIxmocT9OCFgy/S0g1p+o3m9K
+VNd5AMkCgYEA5fbS5UK7FBzuLoLgr1hktmbLJhpt8y8IPHNABHcUdE+O4/1xTQNf
+pMUwkKjGG1MtrGjLOIoMGURKKn8lR1GMZueOTSKY0+mAWUGvSzl6vwtJwvJruT8M
+otEO03o0tPnRKGxbFjqxkp2b6iqJ8MxCRZ3lSidc4mdi7PHzv9lwgvsCgYEA8Siq
+7weCri9N6y+tIdORAXgRzcW54BmJyqB147c72RvbMacb6rN28KXpM3qnRXyp3Llb
+yh81TW3FH10GqrjATws7BK8lP9kkAw0Z/7kNiS1NgH3pUbO+5H2kAa/6QW35nzRe
+Jw2lyfYGWqYO4hYXH14ML1kjgS1hgd3XHOQ64M8CgYAKcjDYSzS2UC4dnMJaFLjW
+dErsGy09a7iDDnUs/r/GHMsP3jZkWi/hCzgOiiwdl6SufUAl/FdaWnjH/2iRGco3
+7nLPXC/3CFdVNp+g2iaSQRADtAFis9N+HeL/hkCYq/RtUqa8lsP0NgacF3yWnKCy
+Ct8chDc67ZlXzBHXeCgdOwKBgHHGFPbWXUHeUW1+vbiyvrupsQSanznp8oclMtkv
+Dk48hSokw9fzuU6Jh77gw9/Vk7HtxS9Tj+squZA1bDrJFPl1u+9WzkUUJZhG6xgp
+bwhj1iejv5rrKUlVOTYOlwudXeJNa4oTNz9UEeVcaLMjZt9GmIsSC90a0uDZD26z
+AlAjAoGAEoqm2DcNN7SrH6aVFzj1EVOrNsHYiXj/yefspeiEmf27PSAslP+uF820
+SDpz4h+Bov5qTKkzcxuu1QWtA4M0K8Iy6IYLwb83DZEm1OsAf4i0pODz21PY/I+O
+VHzjB10oYgaInHZgMUdyb6F571UdiYSB6a/IlZ3ngj5touy3VIM=
+-----END RSA PRIVATE KEY-----`)
+
+func dexMockHandler(t *testing.T, url string) func(http.ResponseWriter, *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.RequestURI {
+		case "/api/dex/.well-known/openid-configuration":
+			_, err := io.WriteString(w, fmt.Sprintf(`
+{
+  "issuer": "%[1]s/api/dex",
+  "authorization_endpoint": "%[1]s/api/dex/auth",
+  "token_endpoint": "%[1]s/api/dex/token",
+  "jwks_uri": "%[1]s/api/dex/keys",
+  "userinfo_endpoint": "%[1]s/api/dex/userinfo",
+  "device_authorization_endpoint": "%[1]s/api/dex/device/code",
+  "grant_types_supported": ["authorization_code"],
+  "response_types_supported": ["code"],
+  "subject_types_supported": ["public"],
+  "id_token_signing_alg_values_supported": ["RS512"],
+  "code_challenge_methods_supported": ["S256", "plain"],
+  "scopes_supported": ["openid"],
+  "token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
+  "claims_supported": ["sub", "aud", "exp"]
+}`, url))
+			require.NoError(t, err)
+		default:
+			w.WriteHeader(404)
+		}
+	}
+}
+
+func GetDexTestServer(t *testing.T) *httptest.Server {
+	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Start with a placeholder. We need the server URL before setting up the real handler.
+	}))
+	ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		dexMockHandler(t, ts.URL)(w, r)
+	})
+	return ts
+}
+
+func oidcMockHandler(t *testing.T, url string) func(http.ResponseWriter, *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.RequestURI {
+		case "/.well-known/openid-configuration":
+			_, err := io.WriteString(w, fmt.Sprintf(`
+{
+  "issuer": "%[1]s",
+  "authorization_endpoint": "%[1]s/auth",
+  "token_endpoint": "%[1]s/token",
+  "jwks_uri": "%[1]s/keys",
+  "userinfo_endpoint": "%[1]s/userinfo",
+  "device_authorization_endpoint": "%[1]s/device/code",
+  "grant_types_supported": ["authorization_code"],
+  "response_types_supported": ["code"],
+  "subject_types_supported": ["public"],
+  "id_token_signing_alg_values_supported": ["RS512"],
+  "code_challenge_methods_supported": ["S256", "plain"],
+  "scopes_supported": ["openid"],
+  "token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
+  "claims_supported": ["sub", "aud", "exp"]
+}`, url))
+			require.NoError(t, err)
+		default:
+			w.WriteHeader(404)
+		}
+	}
+}
+
+func GetOIDCTestServer(t *testing.T) *httptest.Server {
+	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Start with a placeholder. We need the server URL before setting up the real handler.
+	}))
+	ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		oidcMockHandler(t, ts.URL)(w, r)
+	})
+	return ts
+}

util/webhook/webhook.go

@@ -270,7 +270,7 @@ func (a *ArgoCDWebhookHandler) storePreviouslyCachedManifests(app *v1alpha1.Appl
 		return err
 	}
 	var cachedManifests cache.CachedManifestResponse
-	if err := a.repoCache.GetManifests(change.shaBefore, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name, &cachedManifests); err == nil {
+	if err := a.repoCache.GetManifests(change.shaBefore, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name, &cachedManifests); err != nil {
 		return err
 	}
 	if err = a.repoCache.SetManifests(change.shaAfter, &app.Spec.Source, &clusterInfo, app.Spec.Destination.Namespace, trackingMethod, appInstanceLabelKey, app.Name, &cachedManifests); err != nil {