Merge pull request #40 from kluctl/fix-modtimes

fix: Properly take file modification time into account when extracting

embed_util/embedded_files.go

@@ -9,6 +9,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"time"
 )
 
 type EmbeddedFiles struct {
@@ -128,7 +129,7 @@ func (e *EmbeddedFiles) copyEmbeddedFilesToTmp(embedFs fs.FS, fl *fileList) erro
 			if resolvedFle.Mode.Type() == existingSt.Mode().Type() {
 				if resolvedFle.Mode.IsDir() {
 					continue
-				} else if existingSt.Size() == resolvedFle.Size {
+				} else if existingSt.Size() == resolvedFle.Size && existingSt.ModTime().Unix() == resolvedFle.ModTime {
 					// unchanged
 					continue
 				}
@@ -176,6 +177,12 @@ func (e *EmbeddedFiles) copyEmbeddedFilesToTmp(embedFs fs.FS, fl *fileList) erro
 		if err != nil {
 			return err
 		}
+		if !resolvedFle.Mode.IsDir() {
+			err = os.Chtimes(path, time.Time{}, time.Unix(resolvedFle.ModTime, 0))
+			if err != nil {
+				return err
+			}
+		}
 	}
 
 	return nil

embed_util/file_list.go

@@ -20,6 +20,7 @@ type fileList struct {
 type fileListEntry struct {
 	Name       string      `json:"name"`
 	Size       int64       `json:"size"`
+	ModTime    int64       `json:"modTime"`
 	Mode       fs.FileMode `json:"perm"`
 	Symlink    string      `json:"symlink,omitempty"`
 	Compressed bool        `json:"compressed,omitempty"`
@@ -48,9 +49,10 @@ func buildFileListFromDir(dir string) (*fileList, error) {
 		}
 
 		fle := fileListEntry{
-			Name: relPath,
-			Size: info.Size(),
-			Mode: info.Mode(),
+			Name:    relPath,
+			Size:    info.Size(),
+			ModTime: info.ModTime().Unix(),
+			Mode:    info.Mode(),
 		}
 
 		if info.Mode().Type() == fs.ModeSymlink {
@@ -92,15 +94,17 @@ func buildFileListFromFs(embedFs fs.FS) (*fileList, error) {
 		}
 
 		fle := fileListEntry{
-			Name: path,
-			Size: info.Size(),
-			Mode: info.Mode() | 0o600,
+			Name:    path,
+			Size:    info.Size(),
+			ModTime: info.ModTime().Unix(),
+			Mode:    info.Mode() | 0o600,
 		}
 
 		if info.Mode().Type() == fs.ModeSymlink {
 			return fmt.Errorf("symlink not supported in buildFileListFromFs")
 		} else if info.Mode().IsDir() {
 			fle.Size = 0
+			fle.ModTime = 0
 		}
 
 		fl.Files = append(fl.Files, fle)

go.mod

@@ -3,13 +3,12 @@ module github.com/kluctl/go-embed-python
 go 1.19
 
 require (
-	github.com/cyphar/filepath-securejoin v0.2.4
 	github.com/gobwas/glob v0.2.3
-	github.com/klauspost/compress v1.17.7
+	github.com/klauspost/compress v1.17.8
 	github.com/rogpeppe/go-internal v1.12.0
 	github.com/sirupsen/logrus v1.9.3
 	github.com/stretchr/testify v1.9.0
-	golang.org/x/sync v0.6.0
+	golang.org/x/sync v0.7.0
 )
 
 require (

go.sum

@@ -1,12 +1,12 @@
-github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
-github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
 github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
 github.com/klauspost/compress v1.17.7 h1:ehO88t2UGzQK66LMdE8tibEd1ErmzZjNEqWkjLAKQQg=
 github.com/klauspost/compress v1.17.7/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=
+github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU=
+github.com/klauspost/compress v1.17.8/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
@@ -19,6 +19,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
 golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
+golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc=
 golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=

internal/tar.go

@@ -3,11 +3,10 @@ package internal
 import (
 	"archive/tar"
 	"fmt"
-	securejoin "github.com/cyphar/filepath-securejoin"
 	"io"
-	"io/fs"
 	"os"
 	"path/filepath"
+	"strings"
 )
 
 func ExtractTarStream(r io.Reader, targetPath string) error {
@@ -22,12 +21,13 @@ func ExtractTarStream(r io.Reader, targetPath string) error {
 			return fmt.Errorf("ExtractTarStream: Next() failed: %w", err)
 		}
 
-		header.Name = filepath.FromSlash(header.Name)
-
-		p, err := securejoin.SecureJoin(targetPath, header.Name)
-		if err != nil {
-			return err
+		if !validRelPath(header.Name) {
+			return fmt.Errorf("tar contained invalid name error %q", header.Name)
 		}
+
+		p := filepath.FromSlash(header.Name)
+		p = filepath.Join(targetPath, p)
+
 		err = os.MkdirAll(filepath.Dir(p), 0755)
 		if err != nil {
 			return err
@@ -39,6 +39,7 @@ func ExtractTarStream(r io.Reader, targetPath string) error {
 				return fmt.Errorf("ExtractTarStream: Mkdir() failed: %w", err)
 			}
 		case tar.TypeReg:
+			_ = os.Remove(p) // we allow overwriting, which easily happens on case insensitive filesystems
 			outFile, err := os.Create(p)
 			if err != nil {
 				return fmt.Errorf("ExtractTarStream: Create() failed: %w", err)
@@ -57,6 +58,7 @@ func ExtractTarStream(r io.Reader, targetPath string) error {
 				return err
 			}
 		case tar.TypeSymlink:
+			_ = os.Remove(p) // we allow overwriting, which easily happens on case insensitive filesystems
 			if err := os.Symlink(header.Linkname, p); err != nil {
 				return fmt.Errorf("ExtractTarStream: Symlink() failed: %w", err)
 			}
@@ -67,74 +69,9 @@ func ExtractTarStream(r io.Reader, targetPath string) error {
 	return nil
 }
 
-func AddToTar(tw *tar.Writer, pth string, name string, filter func(h *tar.Header, size int64) (*tar.Header, error)) error {
-	fi, err := os.Lstat(pth)
-	if err != nil {
-		return err
-	}
-
-	var linkName string
-	if fi.Mode().Type() == fs.ModeSymlink {
-		x, err := os.Readlink(pth)
-		if err != nil {
-			return err
-		}
-		linkName = x
-	}
-
-	h, err := tar.FileInfoHeader(fi, linkName)
-	if err != nil {
-		return err
-	}
-	h.Name = filepath.ToSlash(name)
-
-	if filter != nil {
-		s := fi.Size()
-		if fi.IsDir() {
-			s = 0
-		}
-		h, err = filter(h, s)
-		if err != nil {
-			return err
-		}
-		if h == nil {
-			return nil
-		}
-	}
-
-	err = tw.WriteHeader(h)
-	if err != nil {
-		return err
-	}
-
-	if fi.Mode().Type() == fs.ModeSymlink {
-		return nil
-	}
-
-	if fi.Mode().IsDir() {
-		des, err := os.ReadDir(pth)
-		if err != nil {
-			return err
-		}
-		for _, d := range des {
-			err = AddToTar(tw, filepath.Join(pth, d.Name()), filepath.Join(name, d.Name()), filter)
-			if err != nil {
-				return err
-			}
-		}
-		return nil
-	} else if fi.Mode().IsRegular() {
-		f, err := os.Open(pth)
-		if err != nil {
-			return err
-		}
-		defer f.Close()
-		_, err = io.Copy(tw, f)
-		if err != nil {
-			return err
-		}
-		return nil
-	} else {
-		return fmt.Errorf("unsupported file type/mode %s", fi.Mode().String())
+func validRelPath(p string) bool {
+	if p == "" || strings.Contains(p, `\`) || strings.HasPrefix(p, "/") || strings.Contains(p, "../") {
+		return false
 	}
+	return true
 }