Symlink following allows leaking out-of-bound manifests and JSON files from Argo CD repo-server

@@ -16,6 +16,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/argoproj/argo-cd/v2/util/io/files"
+
 	"github.com/Masterminds/semver"
 	"github.com/TomOnTime/utfutil"
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -726,7 +728,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)
 	}
 	if err != nil {
 		return nil, err
@@ -924,12 +927,32 @@ 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) ([]*unstructured.Unstructured, error) {
 	var objs []*unstructured.Unstructured
 	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
+		relPath, err := filepath.Rel(appPath, path)
+		if err != nil {
+			return fmt.Errorf("failed to get relative path of symlink: %w", err)
+		}
+		if files.IsSymlink(f) {
+			realPath, err := filepath.EvalSymlinks(path)
+			if err != nil {
+				logCtx.Debugf("error checking symlink realpath: %s", err)
+				if os.IsNotExist(err) {
+					log.Warnf("ignoring out-of-bounds symlink at %q: %s", relPath, err)
+					return nil
+				} else {
+					return fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
+				}
+			}
+			if !files.Inbound(realPath, appPath) {
+				logCtx.Warnf("illegal filepath in symlink: %s", realPath)
+				return fmt.Errorf("illegal filepath in symlink at %q", relPath)
+			}
+		}
 		if f.IsDir() {
 			if path != appPath && !directory.Recurse {
 				return filepath.SkipDir
@@ -942,10 +965,6 @@ func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory
 			return nil
 		}
 
-		relPath, err := filepath.Rel(appPath, path)
-		if err != nil {
-			return err
-		}
 		if directory.Exclude != "" && glob.Match(directory.Exclude, relPath) {
 			return nil
 		}

@@ -8,12 +8,15 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
 	"testing"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -27,6 +30,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"
@@ -50,7 +54,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)
@@ -79,7 +83,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
 	}
@@ -110,7 +113,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)
@@ -130,7 +133,7 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &src}
 
 	// update this value if we add/remove manifests
-	const countOfManifests = 34
+	const countOfManifests = 41
 
 	res1, err := service.GenerateManifest(context.Background(), &q)
 
@@ -143,6 +146,76 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	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)
+			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)
+	require.NoError(t, err)
+}
+
 func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
 	service := newService("../..")
 
@@ -1610,7 +1683,7 @@ 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,
@@ -1628,7 +1701,7 @@ 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",
 	})
@@ -1641,7 +1714,7 @@ 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",
 	})

@@ -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
+}

@@ -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)
+		})
+	}
+}
\ No newline at end of file

@@ -18,6 +18,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/argoproj/argo-cd/v2/util/io/files"
+
 	"github.com/Masterminds/semver/v3"
 	"github.com/TomOnTime/utfutil"
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -810,7 +812,8 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,
 		if directory = q.ApplicationSource.Directory; directory == nil {
 			directory = &v1alpha1.ApplicationSourceDirectory{}
 		}
-		targetObjs, err = findManifests(appPath, repoRoot, env, *directory, q.EnabledSourceTypes)
+		logCtx := log.WithField("application", q.AppName)
+		targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory, q.EnabledSourceTypes)
 	}
 	if err != nil {
 		return nil, err
@@ -1012,12 +1015,32 @@ func ksShow(appLabelKey, appPath string, ksonnetOpts *v1alpha1.ApplicationSource
 var manifestFile = regexp.MustCompile(`^.*\.(yaml|yml|json|jsonnet)$`)
 
 // findManifests looks at all yaml files in a directory and unmarshals them into a list of unstructured objects
-func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, enabledManifestGeneration map[string]bool) ([]*unstructured.Unstructured, error) {
+func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, enabledManifestGeneration map[string]bool) ([]*unstructured.Unstructured, error) {
 	var objs []*unstructured.Unstructured
 	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
+		relPath, err := filepath.Rel(appPath, path)
+		if err != nil {
+			return fmt.Errorf("failed to get relative path of symlink: %w", err)
+		}
+		if files.IsSymlink(f) {
+			realPath, err := filepath.EvalSymlinks(path)
+			if err != nil {
+				logCtx.Debugf("error checking symlink realpath: %s", err)
+				if os.IsNotExist(err) {
+					log.Warnf("ignoring out-of-bounds symlink at %q: %s", relPath, err)
+					return nil
+				} else {
+					return fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
+				}
+			}
+			if !files.Inbound(realPath, appPath) {
+				logCtx.Warnf("illegal filepath in symlink: %s", realPath)
+				return fmt.Errorf("illegal filepath in symlink at %q", relPath)
+			}
+		}
 		if f.IsDir() {
 			if path != appPath && !directory.Recurse {
 				return filepath.SkipDir
@@ -1030,10 +1053,6 @@ func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory
 			return nil
 		}
 
-		relPath, err := filepath.Rel(appPath, path)
-		if err != nil {
-			return err
-		}
 		if directory.Exclude != "" && glob.Match(directory.Exclude, relPath) {
 			return nil
 		}

@@ -16,6 +16,8 @@ import (
 	"testing"
 	"time"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -149,6 +151,76 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	assert.Equal(t, 3, len(res2.Manifests))
 }
 
+func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
+	testCases := []struct {
+		name                    string
+		outOfBoundsFilename     string
+		outOfBoundsFileContents string
+		mustNotContain          string // Optional string that must not appear in error or manifest output. If empty, use outOfBoundsFileContents.
+	}{
+		{
+			name:                    "out of bounds JSON file should not appear in error output",
+			outOfBoundsFilename:     "test.json",
+			outOfBoundsFileContents: `{"some": "json"}`,
+		},
+		{
+			name:                    "malformed JSON file contents should not appear in error output",
+			outOfBoundsFilename:     "test.json",
+			outOfBoundsFileContents: "$",
+		},
+		{
+			name:                "out of bounds JSON manifest should not appear in manifest output",
+			outOfBoundsFilename: "test.json",
+			// JSON marshalling is deterministic. So if there's a leak, exactly this should appear in the manifests.
+			outOfBoundsFileContents: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+		},
+		{
+			name:                    "out of bounds YAML manifest should not appear in manifest output",
+			outOfBoundsFilename:     "test.yaml",
+			outOfBoundsFileContents: "apiVersion: v1\nkind: Secret\nmetadata:\n  name: test\n  namespace: default\ntype: Opaque",
+			mustNotContain:          `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		testCaseCopy := testCase
+		t.Run(testCaseCopy.name, func(t *testing.T) {
+			t.Parallel()
+
+			outOfBoundsDir := t.TempDir()
+			outOfBoundsFile := path.Join(outOfBoundsDir, testCaseCopy.outOfBoundsFilename)
+			err := os.WriteFile(outOfBoundsFile, []byte(testCaseCopy.outOfBoundsFileContents), os.FileMode(0444))
+			require.NoError(t, err)
+
+			repoDir := t.TempDir()
+			err = os.Symlink(outOfBoundsFile, path.Join(repoDir, testCaseCopy.outOfBoundsFilename))
+			require.NoError(t, err)
+
+			var mustNotContain = testCaseCopy.outOfBoundsFileContents
+			if testCaseCopy.mustNotContain != "" {
+				mustNotContain = testCaseCopy.mustNotContain
+			}
+
+			q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
+			res, err := GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{})
+			require.Error(t, err)
+			assert.NotContains(t, err.Error(), mustNotContain)
+			assert.Contains(t, err.Error(), "illegal filepath")
+			assert.Nil(t, res)
+		})
+	}
+}
+
+func TestGenerateManifests_MissingSymlinkDestination(t *testing.T) {
+	repoDir := t.TempDir()
+	err := os.Symlink("/obviously/does/not/exist", path.Join(repoDir, "test.yaml"))
+	require.NoError(t, err)
+
+	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
+	_, err = GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{})
+	require.NoError(t, err)
+}
+
 func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
 	service := newService("../..")
 
@@ -1641,7 +1713,7 @@ 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,
@@ -1659,7 +1731,7 @@ func TestFindResources(t *testing.T) {
 }
 
 func TestFindManifests_Exclude(t *testing.T) {
-	objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "subdir/deploymentSub.yaml",
 	}, map[string]bool{})
@@ -1672,7 +1744,7 @@ func TestFindManifests_Exclude(t *testing.T) {
 }
 
 func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
-	objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
+	objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
 		Recurse: true,
 		Exclude: "nothing.yaml",
 	}, map[string]bool{})

@@ -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
+}

@@ -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)
+		})
+	}
+}
\ No newline at end of file

@@ -16,6 +16,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/argoproj/argo-cd/v2/util/io/files"
+
 	"github.com/argoproj/argo-cd/v2/util/argo"
 
 	"github.com/Masterminds/semver"
@@ -752,7 +754,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)
 	}
 	if err != nil {
 		return nil, err
@@ -950,12 +953,32 @@ 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) ([]*unstructured.Unstructured, error) {
 	var objs []*unstructured.Unstructured
 	err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
+		relPath, err := filepath.Rel(appPath, path)
+		if err != nil {
+			return fmt.Errorf("failed to get relative path of symlink: %w", err)
+		}
+		if files.IsSymlink(f) {
+			realPath, err := filepath.EvalSymlinks(path)
+			if err != nil {
+				logCtx.Debugf("error checking symlink realpath: %s", err)
+				if os.IsNotExist(err) {
+					log.Warnf("ignoring out-of-bounds symlink at %q: %s", relPath, err)
+					return nil
+				} else {
+					return fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
+				}
+			}
+			if !files.Inbound(realPath, appPath) {
+				logCtx.Warnf("illegal filepath in symlink: %s", realPath)
+				return fmt.Errorf("illegal filepath in symlink at %q", relPath)
+			}
+		}
 		if f.IsDir() {
 			if path != appPath && !directory.Recurse {
 				return filepath.SkipDir
@@ -968,10 +991,6 @@ func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory
 			return nil
 		}
 
-		relPath, err := filepath.Rel(appPath, path)
-		if err != nil {
-			return err
-		}
 		if directory.Exclude != "" && glob.Match(directory.Exclude, relPath) {
 			return nil
 		}

@@ -8,13 +8,14 @@ import (
 	"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"
 
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
@@ -29,6 +30,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 +54,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 +83,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 +113,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)
@@ -132,7 +133,7 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &src}
 
 	// update this value if we add/remove manifests
-	const countOfManifests = 34
+	const countOfManifests = 41
 
 	res1, err := service.GenerateManifest(context.Background(), &q)
 
@@ -145,6 +146,76 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
 	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)
+			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)
+	require.NoError(t, err)
+}
+
 func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
 	service := newService("../..")
 
@@ -1612,7 +1683,7 @@ 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,
@@ -1630,7 +1701,7 @@ 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",
 	})
@@ -1643,7 +1714,7 @@ 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",
 	})

@@ -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
+}

@@ -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)
+		})
+	}
+}
\ No newline at end of file