Bump version to 2.2.5

* fix: Resolve symlinked value files correctly

Signed-off-by: jannfis <jann@mistrust.net>

* fix: Resolve symlinked value files correctly

Signed-off-by: jannfis <jann@mistrust.net>

Procfile

@@ -1,6 +1,6 @@
 controller: sh -c "FORCE_LOG_COLORS=1 ARGOCD_FAKE_IN_CLUSTER=true ARGOCD_TLS_DATA_PATH=${ARGOCD_TLS_DATA_PATH:-/tmp/argocd-local/tls} ARGOCD_SSH_DATA_PATH=${ARGOCD_SSH_DATA_PATH:-/tmp/argocd-local/ssh} ARGOCD_BINARY_NAME=argocd-application-controller go run ./cmd/main.go --loglevel debug --redis localhost:${ARGOCD_E2E_REDIS_PORT:-6379} --repo-server localhost:${ARGOCD_E2E_REPOSERVER_PORT:-8081}"
 api-server: sh -c "FORCE_LOG_COLORS=1 ARGOCD_FAKE_IN_CLUSTER=true ARGOCD_TLS_DATA_PATH=${ARGOCD_TLS_DATA_PATH:-/tmp/argocd-local/tls} ARGOCD_SSH_DATA_PATH=${ARGOCD_SSH_DATA_PATH:-/tmp/argocd-local/ssh} ARGOCD_BINARY_NAME=argocd-server go run ./cmd/main.go --loglevel debug --redis localhost:${ARGOCD_E2E_REDIS_PORT:-6379} --disable-auth=${ARGOCD_E2E_DISABLE_AUTH:-'true'} --insecure --dex-server http://localhost:${ARGOCD_E2E_DEX_PORT:-5556} --repo-server localhost:${ARGOCD_E2E_REPOSERVER_PORT:-8081} --port ${ARGOCD_E2E_APISERVER_PORT:-8080} "
-dex: sh -c "ARGOCD_BINARY_NAME=argocd-dex go run github.com/argoproj/argo-cd/v2/cmd gendexcfg -o `pwd`/dist/dex.yaml && docker run --rm -p ${ARGOCD_E2E_DEX_PORT:-5556}:${ARGOCD_E2E_DEX_PORT:-5556} -v `pwd`/dist/dex.yaml:/dex.yaml ghcr.io/dexidp/dex:v2.30.0 serve /dex.yaml"
+dex: sh -c "ARGOCD_BINARY_NAME=argocd-dex go run github.com/argoproj/argo-cd/v2/cmd gendexcfg -o `pwd`/dist/dex.yaml && docker run --rm -p ${ARGOCD_E2E_DEX_PORT:-5556}:${ARGOCD_E2E_DEX_PORT:-5556} -v `pwd`/dist/dex.yaml:/dex.yaml ghcr.io/dexidp/dex:v2.30.2 dex serve /dex.yaml"
 redis: bash -c "if [ \"$ARGOCD_REDIS_LOCAL\" == 'true' ]; then redis-server --save '' --appendonly no --port ${ARGOCD_E2E_REDIS_PORT:-6379}; else docker run --rm --name argocd-redis -i -p ${ARGOCD_E2E_REDIS_PORT:-6379}:${ARGOCD_E2E_REDIS_PORT:-6379} redis:6.2.6-alpine --save '' --appendonly no --port ${ARGOCD_E2E_REDIS_PORT:-6379}; fi"
 repo-server: sh -c "FORCE_LOG_COLORS=1 ARGOCD_FAKE_IN_CLUSTER=true ARGOCD_GNUPGHOME=${ARGOCD_GNUPGHOME:-/tmp/argocd-local/gpg/keys} ARGOCD_PLUGINSOCKFILEPATH=${ARGOCD_PLUGINSOCKFILEPATH:-/tmp/argo-e2e/app/config/plugin}  ARGOCD_GPG_DATA_PATH=${ARGOCD_GPG_DATA_PATH:-/tmp/argocd-local/gpg/source} ARGOCD_TLS_DATA_PATH=${ARGOCD_TLS_DATA_PATH:-/tmp/argocd-local/tls} ARGOCD_SSH_DATA_PATH=${ARGOCD_SSH_DATA_PATH:-/tmp/argocd-local/ssh} ARGOCD_BINARY_NAME=argocd-repo-server ARGOCD_GPG_ENABLED=${ARGOCD_GPG_ENABLED:-false} go run ./cmd/main.go --loglevel debug --port ${ARGOCD_E2E_REPOSERVER_PORT:-8081} --redis localhost:${ARGOCD_E2E_REDIS_PORT:-6379}"
 ui: sh -c 'cd ui && ${ARGOCD_E2E_YARN_CMD:-yarn} start'

VERSION

@@ -1 +1 @@
-2.2.3
+2.2.5

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

@@ -28,7 +28,7 @@ spec:
           name: dexconfig
       containers:
       - name: dex
-        image: ghcr.io/dexidp/dex:v2.30.0
+        image: ghcr.io/dexidp/dex:v2.30.2
         imagePullPolicy: Always
         command: [/shared/argocd-dex, rundex]
         securityContext:

manifests/base/kustomization.yaml

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

manifests/core-install.yaml

@@ -3018,7 +3018,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -3067,7 +3067,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -3232,7 +3232,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/core-install/kustomization.yaml

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

manifests/ha/base/kustomization.yaml

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

manifests/ha/install.yaml

@@ -3687,7 +3687,7 @@ spec:
       - command:
         - /shared/argocd-dex
         - rundex
-        image: ghcr.io/dexidp/dex:v2.30.0
+        image: ghcr.io/dexidp/dex:v2.30.2
         imagePullPolicy: Always
         name: dex
         ports:
@@ -3709,7 +3709,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -3926,7 +3926,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -3975,7 +3975,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -4202,7 +4202,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -4398,7 +4398,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/ha/namespace-install.yaml

@@ -1046,7 +1046,7 @@ spec:
       - command:
         - /shared/argocd-dex
         - rundex
-        image: ghcr.io/dexidp/dex:v2.30.0
+        image: ghcr.io/dexidp/dex:v2.30.2
         imagePullPolicy: Always
         name: dex
         ports:
@@ -1068,7 +1068,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -1285,7 +1285,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -1334,7 +1334,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -1561,7 +1561,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -1757,7 +1757,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/install.yaml

@@ -3057,7 +3057,7 @@ spec:
       - command:
         - /shared/argocd-dex
         - rundex
-        image: ghcr.io/dexidp/dex:v2.30.0
+        image: ghcr.io/dexidp/dex:v2.30.2
         imagePullPolicy: Always
         name: dex
         ports:
@@ -3079,7 +3079,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -3260,7 +3260,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -3309,7 +3309,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -3532,7 +3532,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -3722,7 +3722,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

manifests/namespace-install.yaml

@@ -416,7 +416,7 @@ spec:
       - command:
         - /shared/argocd-dex
         - rundex
-        image: ghcr.io/dexidp/dex:v2.30.0
+        image: ghcr.io/dexidp/dex:v2.30.2
         imagePullPolicy: Always
         name: dex
         ports:
@@ -438,7 +438,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         name: copyutil
         volumeMounts:
@@ -619,7 +619,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -668,7 +668,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         name: copyutil
         volumeMounts:
         - mountPath: /var/run/argocd
@@ -891,7 +891,7 @@ spec:
               key: server.http.cookie.maxnumber
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -1081,7 +1081,7 @@ spec:
               key: controller.default.cache.expiration
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.2.3
+        image: quay.io/argoproj/argocd:v2.2.5
         imagePullPolicy: Always
         livenessProbe:
           httpGet:

reposerver/repository/repository.go

@@ -51,7 +51,6 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/io"
 	"github.com/argoproj/argo-cd/v2/util/ksonnet"
 	"github.com/argoproj/argo-cd/v2/util/kustomize"
-	"github.com/argoproj/argo-cd/v2/util/security"
 	"github.com/argoproj/argo-cd/v2/util/text"
 )
 
@@ -65,6 +64,9 @@ const (
 	ociPrefix                      = "oci://"
 )
 
+// List of protocol schemes allowed for fetching remote value files
+var allowedHelmRemoteProtocols = []string{"http", "https"}
+
 // Service implements ManifestService interface
 type Service struct {
 	repoLock                  *repositoryLock
@@ -553,6 +555,153 @@ func runHelmBuild(appPath string, h helm.Helm) error {
 	return ioutil.WriteFile(markerFile, []byte("marker"), 0644)
 }
 
+// resolveSymbolicLinkRecursive resolves the symlink path recursively to its
+// canonical path on the file system, with a maximum nesting level of maxDepth.
+// If path is not a symlink, returns the verbatim copy of path and err of nil.
+func resolveSymbolicLinkRecursive(path string, maxDepth int) (string, error) {
+	resolved, err := os.Readlink(path)
+	if err != nil {
+		// path is not a symbolic link
+		_, ok := err.(*os.PathError)
+		if ok {
+			return path, nil
+		}
+		// Other error has occured
+		return "", err
+	}
+
+	if maxDepth == 0 {
+		return "", fmt.Errorf("maximum nesting level reached")
+	}
+
+	// If we resolved to a relative symlink, make sure we use the absolute
+	// path for further resolving
+	if !strings.HasPrefix(resolved, "/") {
+		basePath := filepath.Dir(path)
+		resolved = filepath.Join(basePath, resolved)
+	}
+
+	return resolveSymbolicLinkRecursive(resolved, maxDepth-1)
+}
+
+// isURLSchemeAllowed returns true if the protocol scheme is in the list of
+// allowed URL schemes.
+func isURLSchemeAllowed(scheme string, allowed []string) bool {
+	isAllowed := false
+	if len(allowed) > 0 {
+		for _, s := range allowed {
+			if strings.EqualFold(scheme, s) {
+				isAllowed = true
+				break
+			}
+		}
+	}
+
+	// Empty scheme means local file
+	return isAllowed && scheme != ""
+}
+
+// resolveHelmValueFilePath will inspect and resolve a path to a Helm value
+// file, and make sure that its final path is within the boundaries of the
+// path specified in repoRoot.
+//
+// appPath is the path we're operating in, e.g. where a Helm chart was unpacked
+// to. repoRoot is the path to the root of the repository.
+//
+// If either appPath or repoRoot is relative, it will be treated as relative
+// to the current working directory.
+//
+// valueFile is the path to a value file, relative to appPath. If valueFile is
+// specified as an absolute path (i.e. leading slash), it will be treated as
+// relative to the repoRoot. In case valueFile is a symlink in the extracted
+// chart, it will be resolved recursively and the decision of whether it is in
+// the boundary of repoRoot will be made using the final resolved path.
+// valueFile can also be a remote URL with a protocol scheme as prefix,
+// in which case the scheme must be included in the list of allowed schemes
+// specified by allowedURLSchemes.
+//
+// Will return an error if either valueFile is outside the boundaries of the
+// repoRoot, valueFile is an URL with a forbidden protocol scheme or if
+// valueFile is a recursive symlink nested too deep. May return errors for
+// other reasons as well.
+//
+// resolvedPath will hold the absolute, resolved path for valueFile on success
+// or set to the empty string on failure.
+//
+// isRemote will be set to true if valueFile is an URL using an allowed
+// protocol scheme, or to false if it resolved to a local file.
+func resolveHelmValueFilePath(appPath, repoRoot, valueFile string, allowedURLSchemes []string) (resolvedPath string, isRemote bool, err error) {
+
+	// We do not provide the path in the error message, because it will be
+	// returned to the user and could be used for information gathering.
+	// Instead, we log the concrete error details.
+	resolveFailure := func(path string, err error) error {
+		log.Errorf("failed to resolve path '%s': %v", path, err)
+		return fmt.Errorf("internal error: failed to resolve path. Check logs for more details")
+	}
+
+	// A value file can be specified as an URL to a remote resource.
+	// We only allow certain URL schemes for remote value files.
+	url, err := url.Parse(valueFile)
+	if err == nil {
+		// If scheme is empty, it means we parsed a path only
+		if url.Scheme != "" {
+			if isURLSchemeAllowed(url.Scheme, allowedURLSchemes) {
+				return valueFile, true, nil
+			} else {
+				return "", false, fmt.Errorf("the URL scheme '%s' is not allowed", url.Scheme)
+			}
+		}
+	}
+
+	// Ensure that our repository root is absolute
+	absRepoPath, err := filepath.Abs(repoRoot)
+	if err != nil {
+		return "", false, resolveFailure(repoRoot, err)
+	}
+
+	// If the path to the file is relative, join it with the current working directory (appPath)
+	// Otherwise, join it with the repository's root
+	path := valueFile
+	if !filepath.IsAbs(path) {
+		absWorkDir, err := filepath.Abs(appPath)
+		if err != nil {
+			return "", false, resolveFailure(repoRoot, err)
+		}
+		path = filepath.Join(absWorkDir, path)
+	} else {
+		path = filepath.Join(absRepoPath, path)
+	}
+
+	// Ensure any symbolic link is resolved before we
+	delinkedPath, err := resolveSymbolicLinkRecursive(path, 10)
+	if err != nil {
+		return "", false, resolveFailure(path, err)
+	}
+	path = delinkedPath
+
+	// Resolve the joined path to an absolute path
+	path, err = filepath.Abs(path)
+	if err != nil {
+		return "", false, resolveFailure(path, err)
+	}
+
+	// Ensure our root path has a trailing slash, otherwise the following check
+	// would return true if root is /foo and path would be /foo2
+	requiredRootPath := absRepoPath
+	if !strings.HasSuffix(requiredRootPath, "/") {
+		requiredRootPath += "/"
+	}
+
+	// Make sure that the resolved path to values file is within the repository's root path
+	if !strings.HasPrefix(path, requiredRootPath) {
+		return "", false, fmt.Errorf("value file '%s' resolved to outside repository root", valueFile)
+	}
+
+	return path, false, nil
+
+}
+
 func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclient.ManifestRequest, isLocal bool) ([]*unstructured.Unstructured, error) {
 	concurrencyAllowed := isConcurrencyAllowed(appPath)
 	if !concurrencyAllowed {
@@ -582,31 +731,14 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
 		}
 
 		for _, val := range appHelm.ValueFiles {
-			// If val is not a URL, run it against the directory enforcer. If it is a URL, use it without checking
-			if _, err := url.ParseRequestURI(val); err != nil {
 
-				// Ensure that the repo root provided is absolute
-				absRepoPath, err := filepath.Abs(repoRoot)
-				if err != nil {
-					return nil, err
-				}
-
-				// If the path to the file is relative, join it with the current working directory (appPath)
-				path := val
-				if !filepath.IsAbs(path) {
-					absWorkDir, err := filepath.Abs(appPath)
-					if err != nil {
-						return nil, err
-					}
-					path = filepath.Join(absWorkDir, path)
-				}
-
-				_, err = security.EnforceToCurrentRoot(absRepoPath, path)
-				if err != nil {
-					return nil, err
-				}
+			// This will resolve val to an absolute path (or an URL)
+			path, _, err := resolveHelmValueFilePath(appPath, repoRoot, val, allowedHelmRemoteProtocols)
+			if err != nil {
+				return nil, err
 			}
-			templateOpts.Values = append(templateOpts.Values, val)
+
+			templateOpts.Values = append(templateOpts.Values, path)
 		}
 
 		if appHelm.Values != "" {

reposerver/repository/repository_test.go

@@ -728,7 +728,37 @@ func TestHelmManifestFromChartRepoWithValueFileOutsideRepo(t *testing.T) {
 	}
 	request := &apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: source, NoCache: true}
 	_, err := service.GenerateManifest(context.Background(), request)
-	assert.Error(t, err, "should be on or under current directory")
+	assert.Error(t, err)
+}
+
+func TestHelmManifestFromChartRepoWithValueFileLinks(t *testing.T) {
+	t.Run("Valid symlink", func(t *testing.T) {
+		service := newService("../..")
+		source := &argoappv1.ApplicationSource{
+			Chart:          "my-chart",
+			TargetRevision: ">= 1.0.0",
+			Helm: &argoappv1.ApplicationSourceHelm{
+				ValueFiles: []string{"my-chart-link.yaml"},
+			},
+		}
+		request := &apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: source, NoCache: true}
+		_, err := service.GenerateManifest(context.Background(), request)
+		assert.NoError(t, err)
+	})
+	t.Run("Symlink pointing to outside", func(t *testing.T) {
+		service := newService("../..")
+		source := &argoappv1.ApplicationSource{
+			Chart:          "my-chart",
+			TargetRevision: ">= 1.0.0",
+			Helm: &argoappv1.ApplicationSourceHelm{
+				ValueFiles: []string{"my-chart-outside-link.yaml"},
+			},
+		}
+		request := &apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: source, NoCache: true}
+		_, err := service.GenerateManifest(context.Background(), request)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+	})
 }
 
 func TestGenerateHelmWithURL(t *testing.T) {
@@ -751,33 +781,88 @@ func TestGenerateHelmWithURL(t *testing.T) {
 // The requested value file (`../../../../../minio/values.yaml`) is outside the repo directory
 // (`~/go/src/github.com/argoproj/argo-cd`), so it is blocked
 func TestGenerateHelmWithValuesDirectoryTraversalOutsideRepo(t *testing.T) {
-	service := newService("../..")
-	_, err := service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
-		Repo:    &argoappv1.Repository{},
-		AppName: "test",
-		ApplicationSource: &argoappv1.ApplicationSource{
-			Path: "./util/helm/testdata/redis",
-			Helm: &argoappv1.ApplicationSourceHelm{
-				ValueFiles: []string{"../../../../../minio/values.yaml"},
-				Values:     `cluster: {slaveCount: 2}`,
+	t.Run("Values file with relative path pointing outside repo root", func(t *testing.T) {
+		service := newService("../..")
+		_, err := service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
+			Repo:    &argoappv1.Repository{},
+			AppName: "test",
+			ApplicationSource: &argoappv1.ApplicationSource{
+				Path: "./util/helm/testdata/redis",
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValueFiles: []string{"../../../../../minio/values.yaml"},
+					Values:     `cluster: {slaveCount: 2}`,
+				},
 			},
-		},
+		})
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
 	})
-	assert.Error(t, err, "should be on or under current directory")
 
-	service = newService("./testdata/my-chart")
-	_, err = service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
-		Repo:    &argoappv1.Repository{},
-		AppName: "test",
-		ApplicationSource: &argoappv1.ApplicationSource{
-			Path: ".",
-			Helm: &argoappv1.ApplicationSourceHelm{
-				ValueFiles: []string{"../my-chart-2/values.yaml"},
-				Values:     `cluster: {slaveCount: 2}`,
+	t.Run("Values file with relative path pointing inside repo root", func(t *testing.T) {
+		service := newService("./testdata/my-chart")
+		_, err := service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
+			Repo:    &argoappv1.Repository{},
+			AppName: "test",
+			ApplicationSource: &argoappv1.ApplicationSource{
+				Path: ".",
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValueFiles: []string{"../my-chart/my-chart-values.yaml"},
+					Values:     `cluster: {slaveCount: 2}`,
+				},
 			},
-		},
+		})
+		assert.NoError(t, err)
+	})
+
+	t.Run("Values file with absolute path stays within repo root", func(t *testing.T) {
+		service := newService("./testdata/my-chart")
+		_, err := service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
+			Repo:    &argoappv1.Repository{},
+			AppName: "test",
+			ApplicationSource: &argoappv1.ApplicationSource{
+				Path: ".",
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValueFiles: []string{"/my-chart-values.yaml"},
+					Values:     `cluster: {slaveCount: 2}`,
+				},
+			},
+		})
+		assert.NoError(t, err)
+	})
+
+	t.Run("Values file with absolute path using back-references outside repo root", func(t *testing.T) {
+		service := newService("./testdata/my-chart")
+		_, err := service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
+			Repo:    &argoappv1.Repository{},
+			AppName: "test",
+			ApplicationSource: &argoappv1.ApplicationSource{
+				Path: ".",
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValueFiles: []string{"/../../../my-chart-values.yaml"},
+					Values:     `cluster: {slaveCount: 2}`,
+				},
+			},
+		})
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+	})
+
+	t.Run("Remote values file from forbidden protocol", func(t *testing.T) {
+		service := newService("./testdata/my-chart")
+		_, err := service.GenerateManifest(context.Background(), &apiclient.ManifestRequest{
+			Repo:    &argoappv1.Repository{},
+			AppName: "test",
+			ApplicationSource: &argoappv1.ApplicationSource{
+				Path: ".",
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValueFiles: []string{"file://../../../../my-chart-values.yaml"},
+					Values:     `cluster: {slaveCount: 2}`,
+				},
+			},
+		})
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "is not allowed")
 	})
-	assert.Error(t, err, "should be on or under current directory")
 }
 
 // The requested file parameter (`/tmp/external-secret.txt`) is outside the app path
@@ -1561,3 +1646,172 @@ func Test_getHelmDependencyRepos(t *testing.T) {
 	assert.Equal(t, repos[0].Repo, repo1)
 	assert.Equal(t, repos[1].Repo, repo2)
 }
+
+func Test_resolveSymlinkRecursive(t *testing.T) {
+	testsDir, err := filepath.Abs("./testdata/symlinks")
+	if err != nil {
+		panic(err)
+	}
+	t.Run("Resolve non-symlink", func(t *testing.T) {
+		r, err := resolveSymbolicLinkRecursive(testsDir+"/foo", 2)
+		assert.NoError(t, err)
+		assert.Equal(t, testsDir+"/foo", r)
+	})
+	t.Run("Successfully resolve symlink", func(t *testing.T) {
+		r, err := resolveSymbolicLinkRecursive(testsDir+"/bar", 2)
+		assert.NoError(t, err)
+		assert.Equal(t, testsDir+"/foo", r)
+	})
+	t.Run("Do not allow symlink at all", func(t *testing.T) {
+		r, err := resolveSymbolicLinkRecursive(testsDir+"/bar", 0)
+		assert.Error(t, err)
+		assert.Equal(t, "", r)
+	})
+	t.Run("Error because too nested symlink", func(t *testing.T) {
+		r, err := resolveSymbolicLinkRecursive(testsDir+"/bam", 2)
+		assert.Error(t, err)
+		assert.Equal(t, "", r)
+	})
+	t.Run("No such file or directory", func(t *testing.T) {
+		r, err := resolveSymbolicLinkRecursive(testsDir+"/foobar", 2)
+		assert.NoError(t, err)
+		assert.Equal(t, testsDir+"/foobar", r)
+	})
+}
+
+func Test_isURLSchemeAllowed(t *testing.T) {
+	type testdata struct {
+		name     string
+		scheme   string
+		allowed  []string
+		expected bool
+	}
+	var tts []testdata = []testdata{
+		{
+			name:     "Allowed scheme matches",
+			scheme:   "http",
+			allowed:  []string{"http", "https"},
+			expected: true,
+		},
+		{
+			name:     "Allowed scheme matches only partially",
+			scheme:   "http",
+			allowed:  []string{"https"},
+			expected: false,
+		},
+		{
+			name:     "Scheme is not allowed",
+			scheme:   "file",
+			allowed:  []string{"http", "https"},
+			expected: false,
+		},
+		{
+			name:     "Empty scheme with valid allowances is forbidden",
+			scheme:   "",
+			allowed:  []string{"http", "https"},
+			expected: false,
+		},
+		{
+			name:     "Empty scheme with empty allowances is forbidden",
+			scheme:   "",
+			allowed:  []string{},
+			expected: false,
+		},
+		{
+			name:     "Some scheme with empty allowances is forbidden",
+			scheme:   "file",
+			allowed:  []string{},
+			expected: false,
+		},
+	}
+	for _, tt := range tts {
+		t.Run(tt.name, func(t *testing.T) {
+			r := isURLSchemeAllowed(tt.scheme, tt.allowed)
+			assert.Equal(t, tt.expected, r)
+		})
+	}
+}
+
+func Test_resolveHelmValueFilePath(t *testing.T) {
+	t.Run("Resolve normal relative path into absolute path", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath("/foo/bar", "/foo", "baz/bim.yaml", allowedHelmRemoteProtocols)
+		assert.NoError(t, err)
+		assert.False(t, remote)
+		assert.Equal(t, "/foo/bar/baz/bim.yaml", p)
+	})
+	t.Run("Resolve normal relative path into absolute path", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath("/foo/bar", "/foo", "baz/../../bim.yaml", allowedHelmRemoteProtocols)
+		assert.NoError(t, err)
+		assert.False(t, remote)
+		assert.Equal(t, "/foo/bim.yaml", p)
+	})
+	t.Run("Error on path resolving outside repository root", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath("/foo/bar", "/foo", "baz/../../../bim.yaml", allowedHelmRemoteProtocols)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+		assert.False(t, remote)
+		assert.Equal(t, "", p)
+	})
+	t.Run("Return verbatim URL", func(t *testing.T) {
+		url := "https://some.where/foo,yaml"
+		p, remote, err := resolveHelmValueFilePath("/foo/bar", "/foo", url, allowedHelmRemoteProtocols)
+		assert.NoError(t, err)
+		assert.True(t, remote)
+		assert.Equal(t, url, p)
+	})
+	t.Run("URL scheme not allowed", func(t *testing.T) {
+		url := "file:///some.where/foo,yaml"
+		p, remote, err := resolveHelmValueFilePath("/foo/bar", "/foo", url, allowedHelmRemoteProtocols)
+		assert.Error(t, err)
+		assert.False(t, remote)
+		assert.Equal(t, "", p)
+	})
+	t.Run("Implicit URL by absolute path", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath("/foo/bar", "/foo", "/baz.yaml", allowedHelmRemoteProtocols)
+		assert.NoError(t, err)
+		assert.False(t, remote)
+		assert.Equal(t, "/foo/baz.yaml", p)
+	})
+	t.Run("Relative app path", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath(".", "/foo", "/baz.yaml", allowedHelmRemoteProtocols)
+		assert.NoError(t, err)
+		assert.False(t, remote)
+		assert.Equal(t, "/foo/baz.yaml", p)
+	})
+	t.Run("Relative repo path", func(t *testing.T) {
+		c, err := os.Getwd()
+		require.NoError(t, err)
+		p, remote, err := resolveHelmValueFilePath(".", ".", "baz.yaml", allowedHelmRemoteProtocols)
+		assert.NoError(t, err)
+		assert.False(t, remote)
+		assert.Equal(t, c+"/baz.yaml", p)
+	})
+	t.Run("Overlapping root prefix without trailing slash", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath(".", "/foo", "../foo2/baz.yaml", allowedHelmRemoteProtocols)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+		assert.False(t, remote)
+		assert.Equal(t, "", p)
+	})
+	t.Run("Overlapping root prefix with trailing slash", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath(".", "/foo/", "../foo2/baz.yaml", allowedHelmRemoteProtocols)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+		assert.False(t, remote)
+		assert.Equal(t, "", p)
+	})
+	t.Run("Garbage input as values file", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath(".", "/foo/", "kfdj\\ks&&&321209.,---e32908923%$§!\"", allowedHelmRemoteProtocols)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+		assert.False(t, remote)
+		assert.Equal(t, "", p)
+	})
+	t.Run("NUL-byte path input as values file", func(t *testing.T) {
+		p, remote, err := resolveHelmValueFilePath(".", "/foo/", "\000", allowedHelmRemoteProtocols)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside repository root")
+		assert.False(t, remote)
+		assert.Equal(t, "", p)
+	})
+}

reposerver/repository/testdata/my-chart/my-chart-link.yaml

@@ -0,0 +1 @@
+my-chart-values.yaml

reposerver/repository/testdata/my-chart/my-chart-outside-link.yaml

@@ -0,0 +1 @@
+../my-chart-2/my-chart-2-values.yaml

reposerver/repository/testdata/symlinks/bam

@@ -0,0 +1 @@
+baz

reposerver/repository/testdata/symlinks/bar

@@ -0,0 +1 @@
+foo

reposerver/repository/testdata/symlinks/baz

@@ -0,0 +1 @@
+bar

reposerver/repository/testdata/symlinks/foo

@@ -0,0 +1 @@
+hello