Fix tar-extract

Tar-extract had a bug with the zipSlip guard. This PR fixes so we can
extract files properly the issue was taking abs of the target multiple
times and not the destDir, stringified the paths were equal, but bytes
representation were different.

This commit does not include a regression test (yet).

pkg/extract/tar.go

@@ -2,6 +2,7 @@ package extract
 
 import (
 	"archive/tar"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -12,6 +13,9 @@ import (
 	"github.com/replicate/pget/pkg/logging"
 )
 
+var ErrZipSlip = errors.New("archive (tar) file contains file outside of target directory")
+var ErrEmptyHeaderName = errors.New("tar file contains entry with empty name")
+
 type link struct {
 	linkType byte
 	oldName  string
@@ -135,18 +139,18 @@ func createSymlink(oldName, newName string, overwrite bool) error {
 
 func guardAgainstZipSlip(header *tar.Header, destDir string) error {
 	if header.Name == "" {
-		return fmt.Errorf("tar file contains entry with empty name")
+		return ErrEmptyHeaderName
 	}
 	target, err := filepath.Abs(filepath.Join(destDir, header.Name))
 	if err != nil {
 		return fmt.Errorf("error getting absolute path of destDir %s: %w", header.Name, err)
 	}
-	filePath, err := filepath.Abs(target)
+	destAbs, err := filepath.Abs(destDir)
 	if err != nil {
-		return fmt.Errorf("error getting absolute path of %s: %w", target, err)
+		return fmt.Errorf("error getting absolute path of %s: %w", destDir, err)
 	}
-	if !strings.HasPrefix(filePath, destDir) {
-		return fmt.Errorf("archive (tar) file contains file (%s) outside of target directory: %s", filePath, target)
+	if !strings.HasPrefix(target, destAbs) {
+		return fmt.Errorf("%w: `%s` outside of `%s`", ErrZipSlip, target, destAbs)
 	}
 	return nil
 }

pkg/extract/tar_test.go

@@ -169,52 +169,62 @@ func TestGuardAgainstZipSlip(t *testing.T) {
 		description   string
 		header        *tar.Header
 		destDir       string
-		expectedError string
+		expectedError error
 	}{
 		{
 			description: "valid file path within directory",
 			header: &tar.Header{
 				Name: "valid_file",
 			},
 			destDir:       "/tmp/valid_dir",
-			expectedError: "",
+			expectedError: nil,
 		},
 		{
 			description: "file path outside directory",
 			header: &tar.Header{
 				Name: "../invalid_file",
 			},
 			destDir:       "/tmp/valid_dir",
-			expectedError: "archive (tar) file contains file (/tmp/invalid_file) outside of target directory: ",
+			expectedError: ErrZipSlip,
 		},
 		{
 			description: "directory traversal with invalid file",
 			header: &tar.Header{
 				Name: "./../../tmp/invalid_dir/invalid_file",
 			},
 			destDir:       "/tmp/valid_dir",
-			expectedError: "archive (tar) file contains file (/tmp/invalid_dir/invalid_file) outside of target directory: ",
+			expectedError: ErrZipSlip,
 		},
 		{
 			description: "Empty header name",
 			header: &tar.Header{
 				Name: "",
 			},
 			destDir:       "/tmp",
-			expectedError: "tar file contains entry with empty name",
+			expectedError: ErrEmptyHeaderName,
+		},
+		{
+			description: "relative destDir path, valid file",
+			header: &tar.Header{
+				Name: "bar.txt",
+			},
+			destDir:       "foo",
+			expectedError: nil,
+		},
+		{
+			description: "relative path, invalid file",
+			header: &tar.Header{
+				Name: "../../bar.txt",
+			},
+			destDir:       "foo",
+			expectedError: ErrZipSlip,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			err := guardAgainstZipSlip(test.header, test.destDir)
-			if test.expectedError != "" {
-				if assert.Error(t, err) {
-					assert.Contains(t, err.Error(), test.expectedError)
-				}
-			} else {
-				assert.NoError(t, err)
-			}
+			assert.ErrorIs(t, err, test.expectedError)
 		})
 	}
 }