diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 0000000..33b0318 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,16 @@ +version: 2 + +workflows: + version: 2 + build: + jobs: + - test + +jobs: + test: + docker: + - image: docker.mirror.hashicorp.services/cimg/go:1.15 + + steps: + - checkout + - run: go test -race ./... diff --git a/go.mod b/go.mod index 49ed430..c27382a 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/hashicorp/go-slug -go 1.12 +go 1.15 diff --git a/slug.go b/slug.go index 9491ca5..b911cf2 100644 --- a/slug.go +++ b/slug.go @@ -19,6 +19,21 @@ type Meta struct { Size int64 } +// IllegalSlugError indicates the provided slug (io.Writer for Pack, io.Reader +// for Unpack) violates a rule about its contents. For example, an absolute or +// external symlink. It implements the error interface. +type IllegalSlugError struct { + Err error +} + +func (e *IllegalSlugError) Error() string { + return fmt.Sprintf("illegal slug error: %v", e.Err) +} + +// Unwrap returns the underlying issue with the provided Slug into the error +// chain. +func (e *IllegalSlugError) Unwrap() error { return e.Err } + // Pack creates a slug from a src directory, and writes the new slug // to w. Returns metadata about the slug and any errors. // @@ -48,12 +63,12 @@ func Pack(src string, w io.Writer, dereference bool) (*Meta, error) { // Flush the tar writer. if err := tarW.Close(); err != nil { - return nil, fmt.Errorf("Failed to close the tar archive: %v", err) + return nil, fmt.Errorf("failed to close the tar archive: %w", err) } // Flush the gzip writer. if err := gzipW.Close(); err != nil { - return nil, fmt.Errorf("Failed to close the gzip writer: %v", err) + return nil, fmt.Errorf("failed to close the gzip writer: %w", err) } return meta, nil @@ -68,7 +83,7 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference // Get the relative path from the current src directory. subpath, err := filepath.Rel(src, path) if err != nil { - return fmt.Errorf("Failed to get relative path for file %q: %v", path, err) + return fmt.Errorf("failed to get relative path for file %q: %w", path, err) } if subpath == "." { return nil @@ -89,7 +104,7 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference // Get the relative path from the initial root directory. subpath, err = filepath.Rel(root, strings.Replace(path, src, dst, 1)) if err != nil { - return fmt.Errorf("Failed to get relative path for file %q: %v", path, err) + return fmt.Errorf("failed to get relative path for file %q: %w", path, err) } if subpath == "." { return nil @@ -120,7 +135,7 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference case fm&os.ModeSymlink != 0: target, err := filepath.EvalSymlinks(path) if err != nil { - return fmt.Errorf("Failed to get symbolic link destination for %q: %v", path, err) + return fmt.Errorf("failed to get symbolic link destination for %q: %w", path, err) } // If the target is within the current source, we @@ -128,7 +143,7 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference if strings.HasPrefix(target, src) { link, err := filepath.Rel(filepath.Dir(path), target) if err != nil { - return fmt.Errorf("Failed to get relative path for symlink destination %q: %v", target, err) + return fmt.Errorf("failed to get relative path for symlink destination %q: %w", target, err) } header.Typeflag = tar.TypeSymlink @@ -148,7 +163,7 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference // Get the file info for the target. info, err = os.Lstat(target) if err != nil { - return fmt.Errorf("Failed to get file info from file %q: %v", target, err) + return fmt.Errorf("failed to get file info from file %q: %w", target, err) } // If the target is a directory we can recurse into the target @@ -166,12 +181,12 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference writeBody = true default: - return fmt.Errorf("Unexpected file mode %v", fm) + return fmt.Errorf("unexpected file mode %v", fm) } // Write the header first to the archive. if err := tarW.WriteHeader(header); err != nil { - return fmt.Errorf("Failed writing archive header for file %q: %v", path, err) + return fmt.Errorf("failed writing archive header for file %q: %w", path, err) } // Account for the file in the list. @@ -184,13 +199,13 @@ func packWalkFn(root, src, dst string, tarW *tar.Writer, meta *Meta, dereference f, err := os.Open(path) if err != nil { - return fmt.Errorf("Failed opening file %q for archiving: %v", path, err) + return fmt.Errorf("failed opening file %q for archiving: %w", path, err) } defer f.Close() size, err := io.Copy(tarW, f) if err != nil { - return fmt.Errorf("Failed copying file %q to archive: %v", path, err) + return fmt.Errorf("failed copying file %q to archive: %w", path, err) } // Add the size we copied to the body. @@ -207,7 +222,7 @@ func Unpack(r io.Reader, dst string) error { // Decompress as we read. uncompressed, err := gzip.NewReader(r) if err != nil { - return fmt.Errorf("Failed to uncompress slug: %v", err) + return fmt.Errorf("failed to uncompress slug: %w", err) } // Untar as we read. @@ -220,7 +235,7 @@ func Unpack(r io.Reader, dst string) error { break } if err != nil { - return fmt.Errorf("Failed to untar slug: %v", err) + return fmt.Errorf("failed to untar slug: %w", err) } // Get rid of absolute paths. @@ -233,7 +248,9 @@ func Unpack(r io.Reader, dst string) error { // Check for paths outside our directory, they are forbidden target := filepath.Clean(path) if !strings.HasPrefix(target, dst) { - return fmt.Errorf("Invalid filename, traversal with \"..\" outside of current directory") + return &IllegalSlugError{ + Err: fmt.Errorf("invalid filename, traversal with \"..\" outside of current directory"), + } } // Ensure the destination is not through any symlinks. This prevents @@ -260,39 +277,48 @@ func Unpack(r io.Reader, dst string) error { break } if err != nil { - return fmt.Errorf("Failed to evaluate path %q: %v", header.Name, err) + return fmt.Errorf("failed to evaluate path %q: %w", header.Name, err) } if fi.Mode()&os.ModeSymlink != 0 { - return fmt.Errorf("Cannot extract %q through symlink", - header.Name) + return &IllegalSlugError{ + Err: fmt.Errorf("cannot extract %q through symlink", header.Name), + } } } // Make the directories to the path. dir := filepath.Dir(path) if err := os.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("Failed to create directory %q: %v", dir, err) + return fmt.Errorf("failed to create directory %q: %w", dir, err) } // Handle symlinks. if header.Typeflag == tar.TypeSymlink { // Disallow absolute targets. if filepath.IsAbs(header.Linkname) { - return fmt.Errorf("Invalid symlink (%q -> %q) has absolute target", - header.Name, header.Linkname) + return &IllegalSlugError{ + Err: fmt.Errorf( + "invalid symlink (%q -> %q) has absolute target", + header.Name, header.Linkname, + ), + } } // Ensure the link target is within the destination directory. This // disallows providing symlinks to external files and directories. target := filepath.Join(dir, header.Linkname) if !strings.HasPrefix(target, dst) { - return fmt.Errorf("Invalid symlink (%q -> %q) has external target", - header.Name, header.Linkname) + return &IllegalSlugError{ + Err: fmt.Errorf( + "invalid symlink (%q -> %q) has external target", + header.Name, header.Linkname, + ), + } } // Create the symlink. if err := os.Symlink(header.Linkname, path); err != nil { - return fmt.Errorf("Failed creating symlink (%q -> %q): %v", + return fmt.Errorf("failed creating symlink (%q -> %q): %w", header.Name, header.Linkname, err) } @@ -303,7 +329,7 @@ func Unpack(r io.Reader, dst string) error { if header.Typeflag == tar.TypeDir { continue } else if header.Typeflag != tar.TypeReg && header.Typeflag != tar.TypeRegA { - return fmt.Errorf("Failed creating %q: unsupported type %c", path, + return fmt.Errorf("failed creating %q: unsupported type %c", path, header.Typeflag) } @@ -319,7 +345,7 @@ func Unpack(r io.Reader, dst string) error { } if err != nil { - return fmt.Errorf("Failed creating file %q: %v", path, err) + return fmt.Errorf("failed creating file %q: %w", path, err) } } @@ -327,14 +353,14 @@ func Unpack(r io.Reader, dst string) error { _, err = io.Copy(fh, untar) fh.Close() if err != nil { - return fmt.Errorf("Failed to copy slug file %q: %v", path, err) + return fmt.Errorf("failed to copy slug file %q: %w", path, err) } // Restore the file mode. We have to do this after writing the file, // since it is possible we have a read-only mode. mode := header.FileInfo().Mode() if err := os.Chmod(path, mode); err != nil { - return fmt.Errorf("Failed setting permissions on %q: %v", path, err) + return fmt.Errorf("failed setting permissions on %q: %w", path, err) } } return nil diff --git a/slug_test.go b/slug_test.go index de6f761..119ce63 100644 --- a/slug_test.go +++ b/slug_test.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "errors" "io" "io/ioutil" "os" @@ -443,7 +444,7 @@ func TestUnpackMaliciousSymlinks(t *testing.T) { Typeflag: tar.TypeSymlink, }, }, - err: `Cannot extract "subdir/parent/escapes" through symlink`, + err: `cannot extract "subdir/parent/escapes" through symlink`, }, { desc: "nested symlinks within symlinked dir", @@ -459,7 +460,7 @@ func TestUnpackMaliciousSymlinks(t *testing.T) { Typeflag: tar.TypeSymlink, }, }, - err: `Cannot extract "subdir/parent/otherdir/escapes" through symlink`, + err: `cannot extract "subdir/parent/otherdir/escapes" through symlink`, }, { desc: "regular file through symlink", @@ -474,7 +475,7 @@ func TestUnpackMaliciousSymlinks(t *testing.T) { Typeflag: tar.TypeReg, }, }, - err: `Cannot extract "subdir/parent/file" through symlink`, + err: `cannot extract "subdir/parent/file" through symlink`, }, { desc: "directory through symlink", @@ -489,7 +490,7 @@ func TestUnpackMaliciousSymlinks(t *testing.T) { Typeflag: tar.TypeDir, }, }, - err: `Cannot extract "subdir/parent/dir" through symlink`, + err: `cannot extract "subdir/parent/dir" through symlink`, }, } @@ -537,9 +538,10 @@ func TestUnpackMaliciousSymlinks(t *testing.T) { defer os.RemoveAll(dst) // Now try unpacking it, which should fail + var e *IllegalSlugError err = Unpack(fh, dst) - if err == nil || !strings.Contains(err.Error(), tc.err) { - t.Fatalf("expected %v, got %v", tc.err, err) + if err == nil || !errors.As(err, &e) || !strings.Contains(err.Error(), tc.err) { + t.Fatalf("expected *IllegalSlugError %v, got %T %v", tc.err, err, err) } }) } @@ -554,12 +556,12 @@ func TestUnpackMaliciousFiles(t *testing.T) { { desc: "filename containing path traversal", name: "../../../../../../../../tmp/test", - err: "Invalid filename, traversal with \"..\" outside of current directory", + err: "invalid filename, traversal with \"..\" outside of current directory", }, { desc: "should fail before attempting to create directories", name: "../../../../../../../../Users/root", - err: "Invalid filename, traversal with \"..\" outside of current directory", + err: "invalid filename, traversal with \"..\" outside of current directory", }, } @@ -614,9 +616,10 @@ func TestUnpackMaliciousFiles(t *testing.T) { defer os.RemoveAll(dst) // Now try unpacking it, which should fail + var e *IllegalSlugError err = Unpack(fh, dst) - if err == nil || !strings.Contains(err.Error(), tc.err) { - t.Fatalf("expected %v, got %v", tc.err, err) + if err == nil || !errors.As(err, &e) || !strings.Contains(err.Error(), tc.err) { + t.Fatalf("expected *IllegalSlugError %v, got %T %v", tc.err, err, err) } }) }