From 6bf370b1b8d1621ccebc28678d2f00b555a37d07 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Fri, 3 Jun 2022 19:09:52 +0900
Subject: [PATCH 01/20] Add checkHelmValueFilePath function

---
 .../piped/cloudprovider/kubernetes/helm.go    | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index ec2f58d5ab..666542c7ec 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -30,6 +31,10 @@ import (
 	"github.com/pipe-cd/pipecd/pkg/config"
 )
 
+var (
+	allowedURLSchemes = []string{"http", "https"}
+)
+
 type Helm struct {
 	version  string
 	execPath string
@@ -63,6 +68,10 @@ func (c *Helm) TemplateLocalChart(ctx context.Context, appName, appDir, namespac
 
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
+			if err := checkHelmValueFilePath(appDir, chartPath, v); err != nil {
+				c.logger.Error("failed to verify value file path", zap.Error(err))
+				return "", err
+			}
 			args = append(args, "-f", v)
 		}
 		for k, v := range opts.SetFiles {
@@ -199,3 +208,43 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa
 	}
 	return executor()
 }
+
+// checkHelmValueFilePath checks if the path of the values file points
+// outside the path where the Chart file is located.
+func checkHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
+	url, err := url.Parse(valueFilePath)
+	if err == nil && url.Scheme != "" {
+		for _, s := range allowedURLSchemes {
+			if strings.EqualFold(url.Scheme, s) {
+				return nil
+			}
+		}
+
+		return fmt.Errorf("scheme %s is not allowed", url.Scheme)
+	}
+
+	// absAppDir is a path where ".pipecd.yaml" is located.
+	absAppDir, err := filepath.Abs(appDir)
+	if err != nil {
+		return err
+	}
+
+	// absChartPath is a path where "Chart.yaml" and some manifest templates is located.
+	absChartPath := filepath.Join(absAppDir, chartPath)
+
+	// absValueFilePath is a path where Helm values file (e.g. "values.yaml") is located.
+	// TODO: resolve symbolic link
+	absValueFilePath := valueFilePath
+	if !filepath.IsAbs(absValueFilePath) {
+		absValueFilePath = filepath.Join(absChartPath, absValueFilePath)
+	}
+
+	// If a path outside of absChartPath is specified as the path for the values file,
+	// it may indicate that someone trying to illegally read a file that
+	// exists in the environment where Piped is running.
+	if !strings.HasPrefix(absValueFilePath, absChartPath) {
+		return fmt.Errorf("value file %s references outside the chart directory", valueFilePath)
+	}
+
+	return nil
+}

From a6d49a3cf2d91defcb3352332ce09e5838c2c3f2 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Mon, 6 Jun 2022 14:29:28 +0900
Subject: [PATCH 02/20] Add function to resolve symlink

---
 .../piped/cloudprovider/kubernetes/helm.go    | 39 +++++++++++++++----
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index 666542c7ec..c81787f8aa 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -68,7 +68,7 @@ func (c *Helm) TemplateLocalChart(ctx context.Context, appName, appDir, namespac
 
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
-			if err := checkHelmValueFilePath(appDir, chartPath, v); err != nil {
+			if err := verifyHelmValueFilePath(appDir, chartPath, v); err != nil {
 				c.logger.Error("failed to verify value file path", zap.Error(err))
 				return "", err
 			}
@@ -209,9 +209,9 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa
 	return executor()
 }
 
-// checkHelmValueFilePath checks if the path of the values file points
-// outside the path where the Chart file is located.
-func checkHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
+// verifyHelmValueFilePath verifies if the path of the values file references
+// inside the path where the Chart file is located.
+func verifyHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
 	url, err := url.Parse(valueFilePath)
 	if err == nil && url.Scheme != "" {
 		for _, s := range allowedURLSchemes {
@@ -234,17 +234,40 @@ func checkHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
 
 	// absValueFilePath is a path where Helm values file (e.g. "values.yaml") is located.
 	// TODO: resolve symbolic link
-	absValueFilePath := valueFilePath
-	if !filepath.IsAbs(absValueFilePath) {
-		absValueFilePath = filepath.Join(absChartPath, absValueFilePath)
+	if !filepath.IsAbs(valueFilePath) {
+		valueFilePath = filepath.Join(absChartPath, valueFilePath)
+	}
+
+	valueFilePath, err = resolveSymlink(valueFilePath)
+	if err != nil {
+		return err
 	}
 
 	// If a path outside of absChartPath is specified as the path for the values file,
 	// it may indicate that someone trying to illegally read a file that
 	// exists in the environment where Piped is running.
-	if !strings.HasPrefix(absValueFilePath, absChartPath) {
+	if !strings.HasPrefix(valueFilePath, absChartPath) {
 		return fmt.Errorf("value file %s references outside the chart directory", valueFilePath)
 	}
 
 	return nil
 }
+
+// resolveSymlink resolves symbolic link.
+func resolveSymlink(path string) (string, error) {
+	lstat, err := os.Lstat(path)
+	if err != nil {
+		return "", err
+	}
+
+	if lstat.Mode()&os.ModeSymlink == os.ModeSymlink {
+		resolved, err := os.Readlink(path)
+		if err != nil {
+			return "", err
+		}
+
+		return resolveSymlink(resolved)
+	}
+
+	return path, nil
+}

From 56cec2ed0c2581cf62e3b9eb83503b1c2e406f50 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Mon, 6 Jun 2022 17:59:09 +0900
Subject: [PATCH 03/20] Update logic to prevent the values file from being
 placed outside of the app config directory

---
 .../piped/cloudprovider/kubernetes/helm.go    | 22 ++++++++-----------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index c81787f8aa..d0a515df7e 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -68,7 +68,7 @@ func (c *Helm) TemplateLocalChart(ctx context.Context, appName, appDir, namespac
 
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
-			if err := verifyHelmValueFilePath(appDir, chartPath, v); err != nil {
+			if err := verifyHelmValueFilePath(appDir, v); err != nil {
 				c.logger.Error("failed to verify value file path", zap.Error(err))
 				return "", err
 			}
@@ -210,8 +210,8 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa
 }
 
 // verifyHelmValueFilePath verifies if the path of the values file references
-// inside the path where the Chart file is located.
-func verifyHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
+// a remote URL or inside the path where the application configuration file (i.e. *.pipecd.yaml) is located.
+func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 	url, err := url.Parse(valueFilePath)
 	if err == nil && url.Scheme != "" {
 		for _, s := range allowedURLSchemes {
@@ -220,7 +220,7 @@ func verifyHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
 			}
 		}
 
-		return fmt.Errorf("scheme %s is not allowed", url.Scheme)
+		return fmt.Errorf("scheme %s is not allowed to load values file", url.Scheme)
 	}
 
 	// absAppDir is a path where ".pipecd.yaml" is located.
@@ -229,13 +229,9 @@ func verifyHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
 		return err
 	}
 
-	// absChartPath is a path where "Chart.yaml" and some manifest templates is located.
-	absChartPath := filepath.Join(absAppDir, chartPath)
-
-	// absValueFilePath is a path where Helm values file (e.g. "values.yaml") is located.
-	// TODO: resolve symbolic link
+	// valueFilePath is a path where non-default Helm values file is located.
 	if !filepath.IsAbs(valueFilePath) {
-		valueFilePath = filepath.Join(absChartPath, valueFilePath)
+		valueFilePath = filepath.Join(absAppDir, valueFilePath)
 	}
 
 	valueFilePath, err = resolveSymlink(valueFilePath)
@@ -243,11 +239,11 @@ func verifyHelmValueFilePath(appDir, chartPath, valueFilePath string) error {
 		return err
 	}
 
-	// If a path outside of absChartPath is specified as the path for the values file,
+	// If a path outside of absAppDir is specified as the path for the values file,
 	// it may indicate that someone trying to illegally read a file that
 	// exists in the environment where Piped is running.
-	if !strings.HasPrefix(valueFilePath, absChartPath) {
-		return fmt.Errorf("value file %s references outside the chart directory", valueFilePath)
+	if !strings.HasPrefix(valueFilePath, absAppDir) {
+		return fmt.Errorf("value file %s references outside the application configuration directory", valueFilePath)
 	}
 
 	return nil

From 8d53e6e19f012dc74d6408c94835fea2cb020ad3 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Mon, 6 Jun 2022 18:36:17 +0900
Subject: [PATCH 04/20] Add unit tests

---
 .../piped/cloudprovider/kubernetes/helm.go    |  3 +
 .../cloudprovider/kubernetes/helm_test.go     | 89 +++++++++++++++++++
 .../testhelm/appconfdir/app.pipecd.yaml       |  0
 .../testhelm/appconfdir/dir/values.yaml       |  0
 .../testhelm/appconfdir/invalid-symlink       |  1 +
 .../testhelm/appconfdir/valid-symlink         |  1 +
 .../testdata/testhelm/appconfdir/values.yaml  |  0
 .../kubernetes/testdata/testhelm/values.yaml  |  0
 8 files changed, 94 insertions(+)
 create mode 100644 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/app.pipecd.yaml
 create mode 100644 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/dir/values.yaml
 create mode 120000 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/invalid-symlink
 create mode 120000 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink
 create mode 100644 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/values.yaml
 create mode 100644 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/values.yaml

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index d0a515df7e..8e1f48f1c4 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -253,6 +253,9 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 func resolveSymlink(path string) (string, error) {
 	lstat, err := os.Lstat(path)
 	if err != nil {
+		if _, ok := err.(*os.PathError); ok {
+			return path, nil
+		}
 		return "", err
 	}
 
diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
index 18ef3db08e..a41592e679 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
@@ -77,3 +77,92 @@ func TestTemplateLocalChart_WithNamespace(t *testing.T) {
 		require.Equal(t, namespace, metadata["namespace"])
 	}
 }
+
+func TestVerifyHelmValueFilePath(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		name          string
+		appDir        string
+		valueFilePath string
+		wantErr       bool
+	}{
+		{
+			name:          "Values file locates inside the app dir",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates inside the app dir (with ..)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "../../../testdata/testhelm/appconfdir/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates under the app dir",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "dir/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates under the app dir (with ..)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "../../../testdata/testhelm/appconfdir/dir/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "arbitrary file locates outside the app dir",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "/etc/hosts",
+			wantErr:       true,
+		},
+		{
+			name:          "arbitrary file locates outside the app dir (with ..)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "../../../../../../../../../../../../etc/hosts",
+			wantErr:       true,
+		},
+		{
+			name:          "Values file locates remote URL (http)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "http://exmaple.com/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates remote URL (http)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "https://exmaple.com/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates remote URL (ftp)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "ftp://exmaple.com/values.yaml",
+			wantErr:       true,
+		},
+		{
+			name:          "Values file is symlink targeting valid values file",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "valid-symlink",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file is symlink targeting invalid values file",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "invalid-symlink",
+			wantErr:       true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := verifyHelmValueFilePath(tc.appDir, tc.valueFilePath)
+			if tc.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/app.pipecd.yaml b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/app.pipecd.yaml
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/dir/values.yaml b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/dir/values.yaml
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/invalid-symlink b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/invalid-symlink
new file mode 120000
index 0000000000..555dec973e
--- /dev/null
+++ b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/invalid-symlink
@@ -0,0 +1 @@
+/etc/hosts
\ No newline at end of file
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink
new file mode 120000
index 0000000000..7d10100961
--- /dev/null
+++ b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink
@@ -0,0 +1 @@
+values.yaml
\ No newline at end of file
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/values.yaml b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/values.yaml
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/values.yaml b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/values.yaml
new file mode 100644
index 0000000000..e69de29bb2

From 41fcec9c92bd8eb175939566b9ddb7e27d38c7e1 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 14:48:09 +0900
Subject: [PATCH 05/20] Fix bug in resolve symlink

---
 .../piped/cloudprovider/kubernetes/helm.go    |  5 +++
 .../cloudprovider/kubernetes/helm_test.go     |  2 +-
 .../testdata/testhelm/appconfdir/test.go      | 33 +++++++++++++++++++
 .../testhelm/appconfdir/valid-symlink         |  2 +-
 4 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index 8e1f48f1c4..b0e0c9c257 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -239,6 +239,10 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 		return err
 	}
 
+	if !filepath.IsAbs(valueFilePath) {
+		valueFilePath = filepath.Join(absAppDir, valueFilePath)
+	}
+
 	// If a path outside of absAppDir is specified as the path for the values file,
 	// it may indicate that someone trying to illegally read a file that
 	// exists in the environment where Piped is running.
@@ -251,6 +255,7 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 
 // resolveSymlink resolves symbolic link.
 func resolveSymlink(path string) (string, error) {
+	fmt.Println("path:", path)
 	lstat, err := os.Lstat(path)
 	if err != nil {
 		if _, ok := err.(*os.PathError); ok {
diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
index a41592e679..6844513bb1 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
@@ -130,7 +130,7 @@ func TestVerifyHelmValueFilePath(t *testing.T) {
 			wantErr:       false,
 		},
 		{
-			name:          "Values file locates remote URL (http)",
+			name:          "Values file locates remote URL (https)",
 			appDir:        "testdata/testhelm/appconfdir",
 			valueFilePath: "https://exmaple.com/values.yaml",
 			wantErr:       false,
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go
new file mode 100644
index 0000000000..23a56ceff4
--- /dev/null
+++ b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go
@@ -0,0 +1,33 @@
+package main
+
+import (
+	"fmt"
+	"os"
+)
+
+// resolveSymlink resolves symbolic link.
+func resolveSymlink(path string) (string, error) {
+	fmt.Println("path:", path)
+	lstat, err := os.Lstat(path)
+	if err != nil {
+		if _, ok := err.(*os.PathError); ok {
+			return path, nil
+		}
+		return "", err
+	}
+
+	if lstat.Mode()&os.ModeSymlink == os.ModeSymlink {
+		resolved, err := os.Readlink(path)
+		if err != nil {
+			return "", err
+		}
+
+		return resolveSymlink(resolved)
+	}
+
+	return path, nil
+}
+
+func main() {
+	resolveSymlink("./invalid-symlink")
+}
diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink
index 7d10100961..a53324e8c5 120000
--- a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink
+++ b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink
@@ -1 +1 @@
-values.yaml
\ No newline at end of file
+dir/values.yaml
\ No newline at end of file

From 955829bd65051c00e8853ec4fb97976f5ff612ca Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 14:55:21 +0900
Subject: [PATCH 06/20] Add verification for remote chart

---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index b0e0c9c257..1972804487 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -69,7 +69,7 @@ func (c *Helm) TemplateLocalChart(ctx context.Context, appName, appDir, namespac
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
 			if err := verifyHelmValueFilePath(appDir, v); err != nil {
-				c.logger.Error("failed to verify value file path", zap.Error(err))
+				c.logger.Error("failed to verify values file path", zap.Error(err))
 				return "", err
 			}
 			args = append(args, "-f", v)
@@ -108,7 +108,7 @@ type helmRemoteGitChart struct {
 }
 
 func (c *Helm) TemplateRemoteGitChart(ctx context.Context, appName, appDir, namespace string, chart helmRemoteGitChart, gitClient gitClient, opts *config.InputHelmOptions) (string, error) {
-	// Firstly, we need to download the remote repositoy.
+	// Firstly, we need to download the remote repository.
 	repoDir, err := os.MkdirTemp("", "helm-remote-chart")
 	if err != nil {
 		return "", fmt.Errorf("unable to created temporary directory for storing remote helm chart: %w", err)
@@ -162,6 +162,10 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa
 
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
+			if err := verifyHelmValueFilePath(appDir, v); err != nil {
+				c.logger.Error("failed to verify values file path", zap.Error(err))
+				return "", err
+			}
 			args = append(args, "-f", v)
 		}
 		for k, v := range opts.SetFiles {

From 447572ed8b9369ecb25895e0cd1201278d47ebc5 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:01:54 +0900
Subject: [PATCH 07/20] Add comment

---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index 1972804487..d9da6b7dbc 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -243,12 +243,13 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 		return err
 	}
 
+	// convert to absolute path again in case symlink is resolved.
 	if !filepath.IsAbs(valueFilePath) {
 		valueFilePath = filepath.Join(absAppDir, valueFilePath)
 	}
 
 	// If a path outside of absAppDir is specified as the path for the values file,
-	// it may indicate that someone trying to illegally read a file that
+	// it may indicate that someone trying to illegally read a file as values file that
 	// exists in the environment where Piped is running.
 	if !strings.HasPrefix(valueFilePath, absAppDir) {
 		return fmt.Errorf("value file %s references outside the application configuration directory", valueFilePath)

From 9a013fa74fdbf3c900d2afddba23c0bf85c9f351 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:02:09 +0900
Subject: [PATCH 08/20] Remove test code

---
 .../testdata/testhelm/appconfdir/test.go      | 33 -------------------
 1 file changed, 33 deletions(-)
 delete mode 100644 pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go

diff --git a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go
deleted file mode 100644
index 23a56ceff4..0000000000
--- a/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/test.go
+++ /dev/null
@@ -1,33 +0,0 @@
-package main
-
-import (
-	"fmt"
-	"os"
-)
-
-// resolveSymlink resolves symbolic link.
-func resolveSymlink(path string) (string, error) {
-	fmt.Println("path:", path)
-	lstat, err := os.Lstat(path)
-	if err != nil {
-		if _, ok := err.(*os.PathError); ok {
-			return path, nil
-		}
-		return "", err
-	}
-
-	if lstat.Mode()&os.ModeSymlink == os.ModeSymlink {
-		resolved, err := os.Readlink(path)
-		if err != nil {
-			return "", err
-		}
-
-		return resolveSymlink(resolved)
-	}
-
-	return path, nil
-}
-
-func main() {
-	resolveSymlink("./invalid-symlink")
-}

From 4b9cf96bda7b5b73887878d2b4acfb4b624d9b15 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:05:03 +0900
Subject: [PATCH 09/20] Add document

---
 docs/content/en/docs-dev/user-guide/configuration-reference.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/content/en/docs-dev/user-guide/configuration-reference.md b/docs/content/en/docs-dev/user-guide/configuration-reference.md
index 0007320dd4..b336b3f0fd 100644
--- a/docs/content/en/docs-dev/user-guide/configuration-reference.md
+++ b/docs/content/en/docs-dev/user-guide/configuration-reference.md
@@ -314,7 +314,7 @@ One of `yamlField` or `regex` is required.
 | Field | Type | Description | Required |
 |-|-|-|-|
 | releaseName | string | The release name of helm deployment. By default, the release name is equal to the application name. | No |
-| valueFiles | []string | List of value files should be loaded. | No |
+| valueFiles | []string | List of value files should be loaded. The paths that can be specified are limited to the directory where the application configuration file exists or under it. | No |
 | setFiles | map[string]string | List of file path for values. | No |
 | apiVersions | []string | Kubernetes api versions used for Capabilities.APIVersions. | No |
 | kubeVersion | string | Kubernetes version used for Capabilities.KubeVersion. | No |

From fa9737f0cd6a349cdc378179979c908c75217afc Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:07:22 +0900
Subject: [PATCH 10/20] Update docs

---
 docs/content/en/docs-dev/user-guide/configuration-reference.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/content/en/docs-dev/user-guide/configuration-reference.md b/docs/content/en/docs-dev/user-guide/configuration-reference.md
index b336b3f0fd..abb3b56ca0 100644
--- a/docs/content/en/docs-dev/user-guide/configuration-reference.md
+++ b/docs/content/en/docs-dev/user-guide/configuration-reference.md
@@ -314,7 +314,7 @@ One of `yamlField` or `regex` is required.
 | Field | Type | Description | Required |
 |-|-|-|-|
 | releaseName | string | The release name of helm deployment. By default, the release name is equal to the application name. | No |
-| valueFiles | []string | List of value files should be loaded. The paths that can be specified are limited to the directory where the application configuration file exists or under it. | No |
+| valueFiles | []string | List of value files should be loaded. The paths that can be specified are limited to the inside of the directory where the application configuration file exists. | No |
 | setFiles | map[string]string | List of file path for values. | No |
 | apiVersions | []string | Kubernetes api versions used for Capabilities.APIVersions. | No |
 | kubeVersion | string | Kubernetes version used for Capabilities.KubeVersion. | No |

From e296c2b13096afac6f300b41c2e8e0a2a6e64093 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:17:12 +0900
Subject: [PATCH 11/20] Remove debug code

---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index d9da6b7dbc..5df1332b7d 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -258,9 +258,8 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 	return nil
 }
 
-// resolveSymlink resolves symbolic link.
+// resolveSymlink resolves symbolic link recursively.
 func resolveSymlink(path string) (string, error) {
-	fmt.Println("path:", path)
 	lstat, err := os.Lstat(path)
 	if err != nil {
 		if _, ok := err.(*os.PathError); ok {

From a771d3e02c04a2d99293c96c638398c76307f0db Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:18:31 +0900
Subject: [PATCH 12/20] Update test name

---
 pkg/app/piped/cloudprovider/kubernetes/helm_test.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
index 6844513bb1..f06c25a817 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go
@@ -124,19 +124,19 @@ func TestVerifyHelmValueFilePath(t *testing.T) {
 			wantErr:       true,
 		},
 		{
-			name:          "Values file locates remote URL (http)",
+			name:          "Values file locates allowed remote URL (http)",
 			appDir:        "testdata/testhelm/appconfdir",
 			valueFilePath: "http://exmaple.com/values.yaml",
 			wantErr:       false,
 		},
 		{
-			name:          "Values file locates remote URL (https)",
+			name:          "Values file locates allowed remote URL (https)",
 			appDir:        "testdata/testhelm/appconfdir",
 			valueFilePath: "https://exmaple.com/values.yaml",
 			wantErr:       false,
 		},
 		{
-			name:          "Values file locates remote URL (ftp)",
+			name:          "Values file locates disallowed remote URL (ftp)",
 			appDir:        "testdata/testhelm/appconfdir",
 			valueFilePath: "ftp://exmaple.com/values.yaml",
 			wantErr:       true,

From bdc1a2538ef1a0ee4cfe18295a47033c35aa85f0 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 15:50:32 +0900
Subject: [PATCH 13/20] Fix typo

---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index 5df1332b7d..d1631e0173 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -252,7 +252,7 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 	// it may indicate that someone trying to illegally read a file as values file that
 	// exists in the environment where Piped is running.
 	if !strings.HasPrefix(valueFilePath, absAppDir) {
-		return fmt.Errorf("value file %s references outside the application configuration directory", valueFilePath)
+		return fmt.Errorf("values file %s references outside the application configuration directory", valueFilePath)
 	}
 
 	return nil

From f1f978db3e391d169645314fae9dee8a3137cc00 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <tsubasa.umeuchi@gmail.com>
Date: Tue, 7 Jun 2022 16:38:35 +0900
Subject: [PATCH 14/20] Update
 docs/content/en/docs-dev/user-guide/configuration-reference.md

Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com>
---
 docs/content/en/docs-dev/user-guide/configuration-reference.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/content/en/docs-dev/user-guide/configuration-reference.md b/docs/content/en/docs-dev/user-guide/configuration-reference.md
index abb3b56ca0..6f7c7528da 100644
--- a/docs/content/en/docs-dev/user-guide/configuration-reference.md
+++ b/docs/content/en/docs-dev/user-guide/configuration-reference.md
@@ -314,7 +314,7 @@ One of `yamlField` or `regex` is required.
 | Field | Type | Description | Required |
 |-|-|-|-|
 | releaseName | string | The release name of helm deployment. By default, the release name is equal to the application name. | No |
-| valueFiles | []string | List of value files should be loaded. The paths that can be specified are limited to the inside of the directory where the application configuration file exists. | No |
+| valueFiles | []string | List of value files should be loaded. Only files stored under the application directory are allowed | No |
 | setFiles | map[string]string | List of file path for values. | No |
 | apiVersions | []string | Kubernetes api versions used for Capabilities.APIVersions. | No |
 | kubeVersion | string | Kubernetes version used for Capabilities.KubeVersion. | No |

From b17e2c0179d0f21bbe7bf98089d91c2658ac6ad4 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 16:39:13 +0900
Subject: [PATCH 15/20] Fix typo

---
 docs/content/en/docs-dev/user-guide/configuration-reference.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/content/en/docs-dev/user-guide/configuration-reference.md b/docs/content/en/docs-dev/user-guide/configuration-reference.md
index 6f7c7528da..becc31e80e 100644
--- a/docs/content/en/docs-dev/user-guide/configuration-reference.md
+++ b/docs/content/en/docs-dev/user-guide/configuration-reference.md
@@ -314,7 +314,7 @@ One of `yamlField` or `regex` is required.
 | Field | Type | Description | Required |
 |-|-|-|-|
 | releaseName | string | The release name of helm deployment. By default, the release name is equal to the application name. | No |
-| valueFiles | []string | List of value files should be loaded. Only files stored under the application directory are allowed | No |
+| valueFiles | []string | List of value files should be loaded. Only files stored under the application directory are allowed. | No |
 | setFiles | map[string]string | List of file path for values. | No |
 | apiVersions | []string | Kubernetes api versions used for Capabilities.APIVersions. | No |
 | kubeVersion | string | Kubernetes version used for Capabilities.KubeVersion. | No |

From 4b2f23ab40bd7eab2098c5602d75b592ee990b06 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 16:59:21 +0900
Subject: [PATCH 16/20] Refactoring logic about resolve symlink

---
 .../piped/cloudprovider/kubernetes/helm.go    | 41 +++++++++----------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index d1631e0173..721a540902 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -238,14 +238,11 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 		valueFilePath = filepath.Join(absAppDir, valueFilePath)
 	}
 
-	valueFilePath, err = resolveSymlink(valueFilePath)
-	if err != nil {
-		return err
-	}
-
-	// convert to absolute path again in case symlink is resolved.
-	if !filepath.IsAbs(valueFilePath) {
-		valueFilePath = filepath.Join(absAppDir, valueFilePath)
+	for isSymlink(valueFilePath) {
+		valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, absAppDir)
+		if err != nil {
+			return err
+		}
 	}
 
 	// If a path outside of absAppDir is specified as the path for the values file,
@@ -258,24 +255,26 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 	return nil
 }
 
-// resolveSymlink resolves symbolic link recursively.
-func resolveSymlink(path string) (string, error) {
+// isSymlink returns the path is whether symbolic link or not.
+func isSymlink(path string) bool {
 	lstat, err := os.Lstat(path)
 	if err != nil {
-		if _, ok := err.(*os.PathError); ok {
-			return path, nil
-		}
-		return "", err
+		return false
 	}
 
-	if lstat.Mode()&os.ModeSymlink == os.ModeSymlink {
-		resolved, err := os.Readlink(path)
-		if err != nil {
-			return "", err
-		}
+	return lstat.Mode()&os.ModeSymlink == os.ModeSymlink
+}
+
+// resolveSymlink resolves symbolic link to an absolute path.
+func resolveSymlinkToAbsPath(path, absParentDir string) (string, error) {
+	resolved, err := os.Readlink(path)
+	if err != nil {
+		return "", err
+	}
 
-		return resolveSymlink(resolved)
+	if !filepath.IsAbs(resolved) {
+		resolved = filepath.Join(absParentDir, resolved)
 	}
 
-	return path, nil
+	return resolved, nil
 }

From 492019eb0f3c2b85eba055c86a92ced2e1202466 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 17:00:58 +0900
Subject: [PATCH 17/20] Fix comment typo

---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index 721a540902..f381db20ac 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -265,7 +265,7 @@ func isSymlink(path string) bool {
 	return lstat.Mode()&os.ModeSymlink == os.ModeSymlink
 }
 
-// resolveSymlink resolves symbolic link to an absolute path.
+// resolveSymlinkToAbsPath resolves symbolic link to an absolute path.
 func resolveSymlinkToAbsPath(path, absParentDir string) (string, error) {
 	resolved, err := os.Readlink(path)
 	if err != nil {

From d7d47dd7a2548863d82efacd5355fbf0e05f1242 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <tsubasa.umeuchi@gmail.com>
Date: Tue, 7 Jun 2022 17:11:14 +0900
Subject: [PATCH 18/20] Update pkg/app/piped/cloudprovider/kubernetes/helm.go

Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com>
---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index f381db20ac..c2c89c8c46 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -238,9 +238,8 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 		valueFilePath = filepath.Join(absAppDir, valueFilePath)
 	}
 
-	for isSymlink(valueFilePath) {
-		valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, absAppDir)
-		if err != nil {
+	if isSymlink(valueFilePath) {
+		if valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, absAppDir); err != nil {
 			return err
 		}
 	}

From 582f83ed7c6b93fea633a33b39acfccdc536d16a Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 18:09:25 +0900
Subject: [PATCH 19/20] Update docs

---
 docs/content/en/docs-dev/user-guide/configuration-reference.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/content/en/docs-dev/user-guide/configuration-reference.md b/docs/content/en/docs-dev/user-guide/configuration-reference.md
index becc31e80e..c712bb5d41 100644
--- a/docs/content/en/docs-dev/user-guide/configuration-reference.md
+++ b/docs/content/en/docs-dev/user-guide/configuration-reference.md
@@ -314,7 +314,7 @@ One of `yamlField` or `regex` is required.
 | Field | Type | Description | Required |
 |-|-|-|-|
 | releaseName | string | The release name of helm deployment. By default, the release name is equal to the application name. | No |
-| valueFiles | []string | List of value files should be loaded. Only files stored under the application directory are allowed. | No |
+| valueFiles | []string | List of value files should be loaded. Only local files stored under the application directory or remote files served at the http(s) endpoint are allowed. | No |
 | setFiles | map[string]string | List of file path for values. | No |
 | apiVersions | []string | Kubernetes api versions used for Capabilities.APIVersions. | No |
 | kubeVersion | string | Kubernetes version used for Capabilities.KubeVersion. | No |

From 3ac6d692af5234cf3e62588e6b6563df73158a98 Mon Sep 17 00:00:00 2001
From: Tsubasa Umeuchi <umeuchi_tsubasa@cyberagent.co.jp>
Date: Tue, 7 Jun 2022 18:37:45 +0900
Subject: [PATCH 20/20] Remove `absAppDir` because `appDir` is always absolute
 path

---
 pkg/app/piped/cloudprovider/kubernetes/helm.go | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go
index c2c89c8c46..419c9a4921 100644
--- a/pkg/app/piped/cloudprovider/kubernetes/helm.go
+++ b/pkg/app/piped/cloudprovider/kubernetes/helm.go
@@ -227,27 +227,21 @@ func verifyHelmValueFilePath(appDir, valueFilePath string) error {
 		return fmt.Errorf("scheme %s is not allowed to load values file", url.Scheme)
 	}
 
-	// absAppDir is a path where ".pipecd.yaml" is located.
-	absAppDir, err := filepath.Abs(appDir)
-	if err != nil {
-		return err
-	}
-
 	// valueFilePath is a path where non-default Helm values file is located.
 	if !filepath.IsAbs(valueFilePath) {
-		valueFilePath = filepath.Join(absAppDir, valueFilePath)
+		valueFilePath = filepath.Join(appDir, valueFilePath)
 	}
 
 	if isSymlink(valueFilePath) {
-		if valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, absAppDir); err != nil {
+		if valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, appDir); err != nil {
 			return err
 		}
 	}
 
-	// If a path outside of absAppDir is specified as the path for the values file,
+	// If a path outside of appDir is specified as the path for the values file,
 	// it may indicate that someone trying to illegally read a file as values file that
 	// exists in the environment where Piped is running.
-	if !strings.HasPrefix(valueFilePath, absAppDir) {
+	if !strings.HasPrefix(valueFilePath, appDir) {
 		return fmt.Errorf("values file %s references outside the application configuration directory", valueFilePath)
 	}