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..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. | 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 | diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm.go b/pkg/app/piped/cloudprovider/kubernetes/helm.go index ec2f58d5ab..419c9a4921 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 := 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 { @@ -99,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) @@ -153,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 { @@ -199,3 +212,62 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa } return executor() } + +// verifyHelmValueFilePath verifies if the path of the values file references +// 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 { + if strings.EqualFold(url.Scheme, s) { + return nil + } + } + + return fmt.Errorf("scheme %s is not allowed to load values file", url.Scheme) + } + + // valueFilePath is a path where non-default Helm values file is located. + if !filepath.IsAbs(valueFilePath) { + valueFilePath = filepath.Join(appDir, valueFilePath) + } + + if isSymlink(valueFilePath) { + if valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, appDir); err != nil { + return err + } + } + + // 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, appDir) { + return fmt.Errorf("values file %s references outside the application configuration directory", valueFilePath) + } + + return nil +} + +// isSymlink returns the path is whether symbolic link or not. +func isSymlink(path string) bool { + lstat, err := os.Lstat(path) + if err != nil { + return false + } + + return lstat.Mode()&os.ModeSymlink == os.ModeSymlink +} + +// resolveSymlinkToAbsPath resolves symbolic link to an absolute path. +func resolveSymlinkToAbsPath(path, absParentDir string) (string, error) { + resolved, err := os.Readlink(path) + if err != nil { + return "", err + } + + if !filepath.IsAbs(resolved) { + resolved = filepath.Join(absParentDir, resolved) + } + + return resolved, nil +} diff --git a/pkg/app/piped/cloudprovider/kubernetes/helm_test.go b/pkg/app/piped/cloudprovider/kubernetes/helm_test.go index 18ef3db08e..f06c25a817 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 allowed remote URL (http)", + appDir: "testdata/testhelm/appconfdir", + valueFilePath: "http://exmaple.com/values.yaml", + wantErr: false, + }, + { + name: "Values file locates allowed remote URL (https)", + appDir: "testdata/testhelm/appconfdir", + valueFilePath: "https://exmaple.com/values.yaml", + wantErr: false, + }, + { + name: "Values file locates disallowed 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..a53324e8c5 --- /dev/null +++ b/pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink @@ -0,0 +1 @@ +dir/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