From eef7f10da7851585048db694003434d9ba5cdbcd Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Sat, 21 Oct 2023 05:14:29 +1100 Subject: [PATCH] fix: a bunch of bugs with scaffolding (#514) - Fixes "tainted" zip paths - Fixes `go mod tidy` failing against the scaffolding go.mod.tmpl --- cmd/ftl/cmd_init.go | 8 +---- .../scaffolding/{go.mod => go.mod.tmpl} | 0 ...ower }}.go => {{ .Name | lower }}.go.tmpl} | 0 internal/scaffolder.go | 13 ++++++++- internal/zip.go | 29 +++---------------- 5 files changed, 17 insertions(+), 33 deletions(-) rename go-runtime/scaffolding/{go.mod => go.mod.tmpl} (100%) rename go-runtime/scaffolding/{{{ .Name | lower }}.go => {{ .Name | lower }}.go.tmpl} (100%) diff --git a/cmd/ftl/cmd_init.go b/cmd/ftl/cmd_init.go index fa75b7ea9b..583199af11 100644 --- a/cmd/ftl/cmd_init.go +++ b/cmd/ftl/cmd_init.go @@ -1,14 +1,11 @@ package main import ( - "context" "os" "path/filepath" "github.com/alecthomas/errors" - "github.com/TBD54566975/ftl/backend/common/exec" - "github.com/TBD54566975/ftl/backend/common/log" goruntime "github.com/TBD54566975/ftl/go-runtime" "github.com/TBD54566975/ftl/internal" kotlinruntime "github.com/TBD54566975/ftl/kotlin-runtime" @@ -26,7 +23,7 @@ type initGoCmd struct { GoModule string `short:"G" required:"" help:"Go module import path."` } -func (i initGoCmd) Run(ctx context.Context, parent *initCmd) error { +func (i initGoCmd) Run(parent *initCmd) error { if i.Name == "" { i.Name = filepath.Base(i.Dir) } @@ -42,9 +39,6 @@ func (i initGoCmd) Run(ctx context.Context, parent *initCmd) error { return errors.WithStack(err) } } - if err := exec.Command(ctx, log.Info, i.Dir, "go", "mod", "tidy").Run(); err != nil { - return errors.WithStack(err) - } return nil } diff --git a/go-runtime/scaffolding/go.mod b/go-runtime/scaffolding/go.mod.tmpl similarity index 100% rename from go-runtime/scaffolding/go.mod rename to go-runtime/scaffolding/go.mod.tmpl diff --git a/go-runtime/scaffolding/{{ .Name | lower }}.go b/go-runtime/scaffolding/{{ .Name | lower }}.go.tmpl similarity index 100% rename from go-runtime/scaffolding/{{ .Name | lower }}.go rename to go-runtime/scaffolding/{{ .Name | lower }}.go.tmpl diff --git a/internal/scaffolder.go b/internal/scaffolder.go index 25e8ab8024..48b90bbd43 100644 --- a/internal/scaffolder.go +++ b/internal/scaffolder.go @@ -17,6 +17,8 @@ import ( // // Both paths and file contents are evaluated. // +// If a file name ends with ".tmpl", the ".tmpl" suffix is removed. +// // The functions "snake", "camel", "lowerCamel", "kebab", "upper", and "lower" // are available. func Scaffold(destination string, ctx any) error { @@ -26,7 +28,15 @@ func Scaffold(destination string, ctx any) error { return errors.WithStack(err) } - // Evaluate path name templates. + if strings.HasSuffix(path, ".tmpl") { + newPath := strings.TrimSuffix(path, ".tmpl") + if err = os.Rename(path, newPath); err != nil { + return errors.Wrap(err, "failed to rename file") + } + path = newPath + } + + // Evaluate the last component of path name templates. dir := filepath.Dir(path) base := filepath.Base(path) newName, err := evaluate(base, ctx) @@ -64,6 +74,7 @@ func Scaffold(destination string, ctx any) error { })) } +// Walk dir executing fn after each entry. func walkDir(dir string, fn func(path string, d fs.DirEntry) error) error { entries, err := os.ReadDir(dir) if err != nil { diff --git a/internal/zip.go b/internal/zip.go index 1f8b2f3aa6..b9d6f1b5b1 100644 --- a/internal/zip.go +++ b/internal/zip.go @@ -3,7 +3,6 @@ package internal import ( "archive/zip" "bytes" - "fmt" "io" "os" "path/filepath" @@ -19,11 +18,10 @@ func UnzipDir(zipReader *zip.Reader, destDir string) error { return errors.WithStack(err) } for _, file := range zipReader.File { - destPath, err := sanitizeArchivePath(destDir, file.Name) - if err != nil { - return errors.WithStack(err) + destPath := filepath.Clean(filepath.Join(destDir, file.Name)) //nolint:gosec + if !strings.HasPrefix(destPath, destDir) { + return errors.Errorf("invalid file path: %q", destPath) } - // Create directory if it doesn't exist if file.FileInfo().IsDir() { err := os.MkdirAll(destPath, file.Mode()) @@ -44,16 +42,7 @@ func UnzipDir(zipReader *zip.Reader, destDir string) error { if err != nil { return errors.WithStack(err) } - // This is probably a little bit aggressive, in that the symlink can - // only be beneath its parent directory, rather than the root of the - // zip file. But it's good enough for now. - symlinkDir := filepath.Dir(destPath) - symlinkPath, err := sanitizeArchivePath(symlinkDir, buf.String()) - if err != nil { - return errors.WithStack(err) - } - symlinkPath = strings.TrimPrefix(symlinkPath, symlinkDir+"/") - err = os.Symlink(symlinkPath, destPath) + err = os.Symlink(buf.String(), destPath) if err != nil { return errors.WithStack(err) } @@ -149,13 +138,3 @@ func ZipDir(srcDir, destZipFile string) error { return nil })) } - -// Sanitize archive file pathing from "G305: Zip Slip vulnerability" -func sanitizeArchivePath(d, t string) (v string, err error) { - v = filepath.Join(d, t) - if strings.HasPrefix(v, filepath.Clean(d)) { - return v, nil - } - - return "", fmt.Errorf("%s: %s", "content filepath is tainted", t) -}