diff --git a/commands/common/manifest.go b/commands/common/manifest.go index d7db4aa..f511c50 100644 --- a/commands/common/manifest.go +++ b/commands/common/manifest.go @@ -85,11 +85,26 @@ func SaveManifest(mf *model.Manifest, dir ...string) error { return err } -// ReadSourceCode reads the content of the file pointed by SourceCodePath +// ReadSourceCode reads the content of the file pointed by SourceCodePath. +// The path must stay inside the current working directory (where manifest.json is read from), symlinks included. func ReadSourceCode(mf *model.Manifest) (string, error) { log.Debug(fmt.Sprintf("Reading source code from %s", mf.SourceCodePath)) - sourceBytes, err := os.ReadFile(mf.SourceCodePath) + + if !filepath.IsLocal(mf.SourceCodePath) { + return "", invalidManifestErr(fmt.Sprintf("sourceCodePath '%s' must be a relative path inside the worker directory", mf.SourceCodePath)) + } + + root, err := os.OpenRoot(".") + if err != nil { + return "", err + } + defer func() { _ = root.Close() }() + + sourceBytes, err := root.ReadFile(mf.SourceCodePath) if err != nil { + if !os.IsNotExist(err) { + return "", invalidManifestErr(fmt.Sprintf("cannot read sourceCodePath '%s': %v", mf.SourceCodePath, err)) + } return "", err } return string(sourceBytes), nil diff --git a/commands/common/manifest_test.go b/commands/common/manifest_test.go index 22188e8..806b0e6 100644 --- a/commands/common/manifest_test.go +++ b/commands/common/manifest_test.go @@ -140,13 +140,28 @@ func TestReadManifestPreservesOmittedAndEmptyProperties(t *testing.T) { } } +func symlinkOrSkip(t *testing.T, target, link string) { + t.Helper() + if err := os.Symlink(target, link); err != nil { + t.Skipf("cannot create symlink: %v", err) + } +} + +func rejected(t assert.TestingT, err error, _ ...interface{}) bool { + assert.ErrorContains(t, err, "must be a relative path inside the worker directory") + return false +} + func TestManifest_ReadSourceCode(t *testing.T) { tests := []struct { name string sourceCode string manifest *model.Manifest - want string - wantErr assert.ErrorAssertionFunc + // setup optionally prepares the folder and returns the manifest to use instead of manifest. + // outside is an existing file located outside dir. + setup func(t *testing.T, dir, outside string) *model.Manifest + want string + wantErr assert.ErrorAssertionFunc }{ { name: "nominal case", @@ -167,6 +182,76 @@ func TestManifest_ReadSourceCode(t *testing.T) { return false }, }, + { + name: "nested relative path", + setup: func(t *testing.T, dir, _ string) *model.Manifest { + require.NoError(t, os.Mkdir(filepath.Join(dir, "sub"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "my-worker.ts"), []byte("nested"), 0o600)) + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = "sub/../my-worker.ts" }) + }, + want: "nested", + wantErr: assert.NoError, + }, + { + name: "symlink inside the directory", + setup: func(t *testing.T, dir, _ string) *model.Manifest { + require.NoError(t, os.WriteFile(filepath.Join(dir, "my-worker.ts"), []byte("linked"), 0o600)) + symlinkOrSkip(t, "my-worker.ts", filepath.Join(dir, "link.ts")) + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = "link.ts" }) + }, + want: "linked", + wantErr: assert.NoError, + }, + { + name: "absolute path outside the directory rejected", + setup: func(t *testing.T, _, outside string) *model.Manifest { + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = outside }) + }, + wantErr: rejected, + }, + { + name: "absolute path inside the directory rejected", + setup: func(t *testing.T, dir, _ string) *model.Manifest { + require.NoError(t, os.WriteFile(filepath.Join(dir, "my-worker.ts"), []byte("x"), 0o600)) + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = filepath.Join(dir, "my-worker.ts") }) + }, + wantErr: rejected, + }, + { + name: "parent traversal rejected", + setup: func(t *testing.T, dir, outside string) *model.Manifest { + rel, err := filepath.Rel(dir, outside) + require.NoError(t, err) + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = rel }) + }, + wantErr: rejected, + }, + { + name: "symlink escaping the directory rejected", + setup: func(t *testing.T, dir, outside string) *model.Manifest { + symlinkOrSkip(t, outside, filepath.Join(dir, "escape.ts")) + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = "escape.ts" }) + }, + wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { + assert.ErrorContains(t, err, "invalid manifest:") + assert.ErrorContains(t, err, "cannot read sourceCodePath 'escape.ts'") + assert.ErrorContains(t, err, "path escapes") + assert.False(t, os.IsNotExist(err)) + return false + }, + }, + { + name: "directory instead of a file", + setup: func(t *testing.T, dir, _ string) *model.Manifest { + require.NoError(t, os.Mkdir(filepath.Join(dir, "sub"), 0o700)) + return patchedManifestSample(func(mf *model.Manifest) { mf.SourceCodePath = "sub" }) + }, + wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { + assert.ErrorContains(t, err, "invalid manifest: cannot read sourceCodePath 'sub'") + assert.NotContains(t, err.Error(), "path escapes") + return false + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -183,10 +268,17 @@ func TestManifest_ReadSourceCode(t *testing.T) { require.NoError(t, err) } - err = os.Chdir(manifestFolder) - require.NoError(t, err) + manifest := tt.manifest + if tt.setup != nil { + outsideFolder := t.TempDir() + outside := filepath.Join(outsideFolder, "secret.txt") + require.NoError(t, os.WriteFile(outside, []byte("secret"), 0o600)) + manifest = tt.setup(t, manifestFolder, outside) + } + + t.Chdir(manifestFolder) - got, err := ReadSourceCode(manifestSample) + got, err := ReadSourceCode(manifest) if !tt.wantErr(t, err, "ReadSourceCode()") { return } diff --git a/commands/deploy_cmd_test.go b/commands/deploy_cmd_test.go index 676ba3c..f9bc328 100644 --- a/commands/deploy_cmd_test.go +++ b/commands/deploy_cmd_test.go @@ -35,6 +35,18 @@ func TestDeployCommand(t *testing.T) { wantErr error patchManifest func(mf *model.Manifest) }{ + { + name: "fails if source code path escapes the worker directory", + workerAction: "BEFORE_UPLOAD", + workerName: "wk-escape", + serverBehavior: common.NewServerStub(t). + WithGetOneEndpoint(). + WithOptionsEndpoint(), + patchManifest: func(mf *model.Manifest) { + mf.SourceCodePath = "../outside.ts" + }, + wantErr: errors.New("invalid manifest: sourceCodePath '../outside.ts' must be a relative path inside the worker directory"), + }, { name: "create", workerAction: "BEFORE_UPLOAD", diff --git a/commands/dry_run_cmd_test.go b/commands/dry_run_cmd_test.go index e1f260c..22457b9 100644 --- a/commands/dry_run_cmd_test.go +++ b/commands/dry_run_cmd_test.go @@ -35,6 +35,15 @@ func TestDryRun(t *testing.T) { // If provided a temp file will be generated with this content and the file path will be added at the end of the command fileInput string }{ + { + name: "fails if source code path escapes the worker directory", + serverStub: common.NewServerStub(t), + patchManifest: func(mf *model.Manifest) { + mf.SourceCodePath = "../outside.ts" + }, + commandArgs: []string{`{}`}, + assert: common.AssertOutputError("invalid manifest: sourceCodePath '../outside.ts' must be a relative path inside the worker directory"), + }, { name: "nominal case", serverStub: common.NewServerStub(t).