diff --git a/go.mod b/go.mod index bb863a713..13a55599d 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/bacongobbler/browser v1.1.0 github.com/bombsimon/logrusr/v4 v4.1.0 github.com/coreos/go-oidc/v3 v3.11.0 + github.com/cyphar/filepath-securejoin v0.3.1 github.com/evanphx/json-patch/v5 v5.9.0 github.com/fatih/structtag v1.2.0 github.com/gobwas/glob v0.2.3 @@ -28,6 +29,7 @@ require ( github.com/kelseyhightower/envconfig v1.4.0 github.com/klauspost/compress v1.17.9 github.com/oklog/ulid/v2 v2.1.0 + github.com/otiai10/copy v1.14.0 github.com/patrickmn/go-cache v2.1.0+incompatible github.com/rs/cors v1.11.1 github.com/sirupsen/logrus v1.9.3 diff --git a/go.sum b/go.sum index 85bd9c8a3..887af61a8 100644 --- a/go.sum +++ b/go.sum @@ -105,6 +105,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3 github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= +github.com/cyphar/filepath-securejoin v0.3.1 h1:1V7cHiaW+C+39wEfpH6XlLBQo3j/PciWFrgfCLS8XrE= +github.com/cyphar/filepath-securejoin v0.3.1/go.mod h1:F7i41x/9cBF7lzCrVsYs9fuzwRZm4NQsGTBdpp6mETc= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -343,6 +345,10 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug= github.com/opencontainers/image-spec v1.1.0/go.mod h1:W4s4sFTMaBeK1BQLXbG4AdM2szdn85PY75RI83NrTrM= +github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU= +github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w= +github.com/otiai10/mint v1.5.1 h1:XaPLeE+9vGbuyEHem1JNk3bYc7KKqyI/na0/mLd/Kks= +github.com/otiai10/mint v1.5.1/go.mod h1:MJm72SBthJjz8qhefc4z1PYEieWmy8Bku7CjcAqyUSM= github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc= github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= diff --git a/internal/directives/copy_directive.go b/internal/directives/copy_directive.go index 0d1aabf7b..f6fe17c7e 100644 --- a/internal/directives/copy_directive.go +++ b/internal/directives/copy_directive.go @@ -4,6 +4,15 @@ import ( "context" "errors" "fmt" + "io/fs" + "path/filepath" + "strings" + + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/otiai10/copy" + "github.com/xeipuuv/gojsonschema" + + "github.com/akuity/kargo/internal/logging" ) func init() { @@ -12,42 +21,83 @@ func init() { } // copyDirective is a directive that copies a file or directory. -type copyDirective struct{} - -// copyConfig is the configuration for the copy directive. -type copyConfig struct { - // InPath is the path to the file or directory to copy. - InPath string `json:"inPath"` - // OutPath is the path to the destination file or directory. - OutPath string `json:"outPath"` +// +// The copy is recursive, merging directories if the destination directory +// already exists. If the destination is an existing file, it will be +// overwritten. Symlinks are ignored. +type copyDirective struct { + schemaLoader gojsonschema.JSONLoader +} + +func (d *copyDirective) Name() string { + return "copy" } -// Validate validates the copy configuration, returning an error if it is invalid. -func (c *copyConfig) Validate() error { - var err []error - if c.InPath == "" { - err = append(err, errors.New("inPath is required")) +func (d *copyDirective) Run(ctx context.Context, stepCtx *StepContext) (Result, error) { + // Validate the configuration against the JSON Schema. + if err := validate(d.schemaLoader, gojsonschema.NewGoLoader(stepCtx.Config), d.Name()); err != nil { + return ResultFailure, err } - if c.OutPath == "" { - err = append(err, errors.New("outPath is required")) + + // Convert the configuration into a typed object. + cfg, err := configToStruct[CopyConfig](stepCtx.Config) + if err != nil { + return ResultFailure, fmt.Errorf("could not convert config into %s config: %w", d.Name(), err) } - return errors.Join(err...) -} -func (d *copyDirective) Name() string { - return "copy" + return d.run(ctx, stepCtx, cfg) } -func (d *copyDirective) Run(_ context.Context, stepCtx *StepContext) (Result, error) { - cfg, err := configToStruct[copyConfig](stepCtx.Config) +func (d *copyDirective) run(ctx context.Context, stepCtx *StepContext, cfg CopyConfig) (Result, error) { + // Secure join the paths to prevent path traversal attacks. + inPath, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.InPath) if err != nil { - return ResultFailure, fmt.Errorf("could not convert config into copy config: %w", err) + return ResultFailure, fmt.Errorf("could not secure join inPath %q: %w", cfg.InPath, err) } - if err = cfg.Validate(); err != nil { - return ResultFailure, fmt.Errorf("invalid copy config: %w", err) + outPath, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.OutPath) + if err != nil { + return ResultFailure, fmt.Errorf("could not secure join outPath %q: %w", cfg.OutPath, err) } - // TODO: add implementation here - + // Perform the copy operation. + opts := copy.Options{ + OnSymlink: func(src string) copy.SymlinkAction { + logging.LoggerFromContext(ctx).Trace("ignoring symlink", "src", src) + return copy.Skip + }, + OnError: func(_, _ string, err error) error { + return sanitizePathError(err, stepCtx.WorkDir) + }, + } + if err = copy.Copy(inPath, outPath, opts); err != nil { + return ResultFailure, fmt.Errorf("failed to copy %q to %q: %w", cfg.InPath, cfg.OutPath, err) + } return ResultSuccess, nil } + +// sanitizePathError sanitizes the path in a path error to be relative to the +// work directory. If the path cannot be made relative, the filename is used +// instead. +// +// This is useful for making error messages more user-friendly, as the work +// directory is typically a temporary directory that the user does not care +// about. +func sanitizePathError(err error, workDir string) error { + var pathErr *fs.PathError + if errors.As(err, &pathErr) { + sanitizedPath, relErr := filepath.Rel(workDir, pathErr.Path) + if relErr != nil || strings.Contains(sanitizedPath, "..") { + // If we can't make it relative, just use the filename. + sanitizedPath = filepath.Base(pathErr.Path) + } + + // Reconstruct the error with the sanitized path. + return &fs.PathError{ + Op: pathErr.Op, + Path: sanitizedPath, + Err: pathErr.Err, + } + } + // Return the original error if it's not a path error. + return err +} diff --git a/internal/directives/copy_directive_test.go b/internal/directives/copy_directive_test.go new file mode 100644 index 000000000..15c7b8e0a --- /dev/null +++ b/internal/directives/copy_directive_test.go @@ -0,0 +1,214 @@ +package directives + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_copyDirective_run(t *testing.T) { + tests := []struct { + name string + setupFiles func(*testing.T) string + cfg CopyConfig + assertions func(*testing.T, string, Result, error) + }{ + { + name: "succeeds copying file", + setupFiles: func(t *testing.T) string { + tmpDir := t.TempDir() + + inPath := filepath.Join(tmpDir, "input.txt") + require.NoError(t, os.WriteFile(inPath, []byte("test content"), 0o600)) + + return tmpDir + }, + cfg: CopyConfig{ + InPath: "input.txt", + OutPath: "output.txt", + }, + assertions: func(t *testing.T, workDir string, result Result, err error) { + assert.Equal(t, ResultSuccess, result) + assert.NoError(t, err) + + outPath := filepath.Join(workDir, "output.txt") + b, err := os.ReadFile(outPath) + assert.NoError(t, err) + assert.Equal(t, "test content", string(b)) + }, + }, + { + name: "succeeds copying directory", + setupFiles: func(t *testing.T) string { + tmpDir := t.TempDir() + + inDir := filepath.Join(tmpDir, "input") + require.NoError(t, os.Mkdir(inDir, 0o755)) + + filePath := filepath.Join(inDir, "input.txt") + require.NoError(t, os.WriteFile(filePath, []byte("test content"), 0o600)) + + nestedDir := filepath.Join(inDir, "nested") + require.NoError(t, os.Mkdir(nestedDir, 0o755)) + nestedPath := filepath.Join(nestedDir, "nested.txt") + require.NoError(t, os.WriteFile(nestedPath, []byte("nested content"), 0o600)) + + return tmpDir + }, + cfg: CopyConfig{ + InPath: "input/", + OutPath: "output/", + }, + assertions: func(t *testing.T, workDir string, result Result, err error) { + assert.Equal(t, ResultSuccess, result) + assert.NoError(t, err) + + outDir := filepath.Join(workDir, "output") + + outPath := filepath.Join(outDir, "input.txt") + b, err := os.ReadFile(outPath) + assert.NoError(t, err) + assert.Equal(t, "test content", string(b)) + + nestedDir := filepath.Join(outDir, "nested") + nestedPath := filepath.Join(nestedDir, "nested.txt") + b, err = os.ReadFile(nestedPath) + assert.NoError(t, err) + assert.Equal(t, "nested content", string(b)) + }, + }, + { + name: "ignores symlink", + setupFiles: func(t *testing.T) string { + tmpDir := t.TempDir() + + inDir := filepath.Join(tmpDir, "input") + require.NoError(t, os.Mkdir(inDir, 0o755)) + + filePath := filepath.Join(inDir, "input.txt") + require.NoError(t, os.WriteFile(filePath, []byte("test content"), 0o600)) + + symlinkPath := filepath.Join(inDir, "symlink.txt") + require.NoError(t, os.Symlink("input.txt", symlinkPath)) + + return tmpDir + }, + cfg: CopyConfig{ + InPath: "input/", + OutPath: "output/", + }, + assertions: func(t *testing.T, workDir string, result Result, err error) { + assert.Equal(t, ResultSuccess, result) + assert.NoError(t, err) + + outDir := filepath.Join(workDir, "output") + + outPath := filepath.Join(outDir, "input.txt") + b, err := os.ReadFile(outPath) + assert.NoError(t, err) + assert.Equal(t, "test content", string(b)) + + symlinkPath := filepath.Join(outDir, "symlink.txt") + _, err = os.Lstat(symlinkPath) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) + }, + }, + { + name: "fails with invalid input path", + setupFiles: func(t *testing.T) string { + return t.TempDir() + }, + cfg: CopyConfig{ + InPath: "input.txt", + }, + assertions: func(t *testing.T, _ string, result Result, err error) { + require.ErrorContains(t, err, "failed to copy") + assert.Equal(t, ResultFailure, result) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workDir := tt.setupFiles(t) + + d := ©Directive{} + result, err := d.run(context.Background(), &StepContext{WorkDir: workDir}, tt.cfg) + tt.assertions(t, workDir, result, err) + }) + } +} + +func Test_sanitizePathError(t *testing.T) { + tests := []struct { + name string + workDir string + err error + assertions func(*testing.T, error) + }{ + { + name: "PathError with relative path", + workDir: "/tmp/work/dir", + err: &os.PathError{Op: "open", Path: "/tmp/work/dir/file.txt", Err: os.ErrNotExist}, + assertions: func(t *testing.T, result error) { + var pathErr *os.PathError + assert.True(t, errors.As(result, &pathErr)) + assert.Equal(t, "open", pathErr.Op) + assert.Equal(t, "file.txt", pathErr.Path) + assert.Equal(t, os.ErrNotExist, pathErr.Err) + }, + }, + { + name: "PathError with path outside workDir", + workDir: "/tmp/work/dir", + err: &os.PathError{Op: "read", Path: "/etc/config.ini", Err: os.ErrPermission}, + assertions: func(t *testing.T, result error) { + var pathErr *os.PathError + assert.True(t, errors.As(result, &pathErr)) + assert.Equal(t, "read", pathErr.Op) + assert.Equal(t, "config.ini", pathErr.Path) + assert.Equal(t, os.ErrPermission, pathErr.Err) + }, + }, + { + name: "non-PathError", + workDir: "/tmp/work/dir", + err: errors.New("generic error"), + assertions: func(t *testing.T, result error) { + assert.Equal(t, "generic error", result.Error()) + }, + }, + { + name: "PathError with workDir", + workDir: "/tmp/work/dir", + err: &os.PathError{Op: "stat", Path: "/tmp/work/dir", Err: os.ErrNotExist}, + assertions: func(t *testing.T, result error) { + var pathErr *os.PathError + errors.As(result, &pathErr) + assert.Equal(t, "stat", pathErr.Op) + assert.Equal(t, ".", pathErr.Path) + assert.Equal(t, os.ErrNotExist, pathErr.Err) + }, + }, + { + name: "nil error", + err: nil, + assertions: func(t *testing.T, result error) { + assert.Nil(t, result) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := sanitizePathError(tt.err, tt.workDir) + tt.assertions(t, result) + }) + } +} diff --git a/internal/directives/schemas/copy-directive-config.json b/internal/directives/schemas/copy-directive-config.json new file mode 100644 index 000000000..ce7a6f591 --- /dev/null +++ b/internal/directives/schemas/copy-directive-config.json @@ -0,0 +1,19 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "CopyConfig", + "type": "object", + "additionalProperties": false, + "required": ["inPath", "outPath"], + "properties": { + "inPath": { + "type": "string", + "description": "InPath is the path to the file or directory to copy.", + "minLength": 1 + }, + "outPath": { + "type": "string", + "description": "OutPath is the path to the destination file or directory.", + "minLength": 1 + } + } +} diff --git a/internal/directives/zz_config_types.go b/internal/directives/zz_config_types.go index 4dc0c3c31..ea5bdd85e 100644 --- a/internal/directives/zz_config_types.go +++ b/internal/directives/zz_config_types.go @@ -4,6 +4,13 @@ package directives type CommonDefs interface{} +type CopyConfig struct { + // InPath is the path to the file or directory to copy. + InPath string `json:"inPath"` + // OutPath is the path to the destination file or directory. + OutPath string `json:"outPath"` +} + type GitCloneConfig struct { // The commits, branches, or tags to check out from the repository and the paths where they // should be checked out. At least one must be specified.