fix: address security and reliability issues in cp operations

- Add sanitizeTarPath to prevent path traversal in tar extraction
- Pass followLinks to SDK CpToInstance calls
- Fix symlinks being written twice to tar output
- Update hypeman-go dependency to v0.7.1-0.20251223013311-b97d5b836009

Author: rgarcia

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/gorilla/websocket v1.5.3
 	github.com/itchyny/json2yaml v0.1.4
 	github.com/muesli/reflow v0.3.0
-	github.com/onkernel/hypeman-go v0.7.0
+	github.com/onkernel/hypeman-go v0.7.1-0.20251223013311-b97d5b836009
 	github.com/tidwall/gjson v1.18.0
 	github.com/tidwall/pretty v1.2.1
 	github.com/urfave/cli-docs/v3 v3.0.0-alpha6
@@ -74,5 +74,3 @@ require (
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20250825161204-c5933d9347a5 // indirect
 	google.golang.org/grpc v1.75.1 // indirect
 )
-
-replace github.com/onkernel/hypeman-go => /home/raf/code/hypeman-go

go.sum

@@ -105,6 +105,8 @@ github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s=
 github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8=
 github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc=
 github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
+github.com/onkernel/hypeman-go v0.7.1-0.20251223013311-b97d5b836009 h1:LPFxYVNAEv4reF4ZFzd45rLPLRWr60NPaPIb0Z622X0=
+github.com/onkernel/hypeman-go v0.7.1-0.20251223013311-b97d5b836009/go.mod h1:Wtm4ewVGGPZc2ySeeuQISQyJxujyQuyDjXyksVkIyy8=
 github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
 github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040=

pkg/cmd/cp.go

@@ -186,7 +186,7 @@ func handleCp(ctx context.Context, cmd *cli.Command) error {
 		if srcPath == "-" {
 			return copyFromStdinToInstance(ctx, baseURL, apiKey, instanceID, dstPath, archive)
 		}
-		return copyToInstance(ctx, baseURL, apiKey, instanceID, srcPath, dstPath, quiet, archive)
+		return copyToInstance(ctx, baseURL, apiKey, instanceID, srcPath, dstPath, quiet, archive, followLinks)
 	}
 }
 
@@ -246,6 +246,42 @@ func isWindowsPath(path string) bool {
 	return false
 }
 
+// sanitizeTarPath validates and sanitizes a tar entry path to prevent path traversal attacks.
+// Returns the sanitized target path or an error if the path is malicious.
+func sanitizeTarPath(basePath, entryName string) (string, error) {
+	// Clean the entry name
+	clean := filepath.Clean(entryName)
+
+	// Reject absolute paths
+	if filepath.IsAbs(clean) {
+		return "", fmt.Errorf("invalid tar entry: absolute path not allowed: %s", entryName)
+	}
+
+	// Reject paths that start with ..
+	if strings.HasPrefix(clean, "..") {
+		return "", fmt.Errorf("invalid tar entry: path escapes destination: %s", entryName)
+	}
+
+	// Join with base path
+	targetPath := filepath.Join(basePath, clean)
+
+	// Verify the result is under the base path
+	absBase, err := filepath.Abs(basePath)
+	if err != nil {
+		return "", fmt.Errorf("resolve base path: %w", err)
+	}
+	absTarget, err := filepath.Abs(targetPath)
+	if err != nil {
+		return "", fmt.Errorf("resolve target path: %w", err)
+	}
+
+	if !strings.HasPrefix(absTarget, absBase+string(filepath.Separator)) && absTarget != absBase {
+		return "", fmt.Errorf("invalid tar entry: path escapes destination: %s", entryName)
+	}
+
+	return targetPath, nil
+}
+
 // statGuestPath queries the guest for information about a path
 func statGuestPath(ctx context.Context, baseURL, apiKey, instanceID, guestPath string, followLinks bool) (*cpStatResponse, error) {
 	wsURL, err := buildCpWsURL(baseURL, instanceID)
@@ -389,7 +425,7 @@ func buildCpWsURL(baseURL, instanceID string) (string, error) {
 }
 
 // copyToInstance copies a local file/directory to the instance
-func copyToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, quiet, archive bool) error {
+func copyToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, quiet, archive, followLinks bool) error {
 	// Check for /. suffix (copy contents only)
 	copyContentsOnly := strings.HasSuffix(srcPath, string(filepath.Separator)+".") || strings.HasSuffix(srcPath, "/.")
 	originalSrcPath := srcPath
@@ -413,15 +449,15 @@ func copyToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, d
 	if srcInfo.IsDir() {
 		if copyContentsOnly {
 			// Copy contents of srcPath into resolvedDst
-			return copyDirContentsToInstance(ctx, baseURL, apiKey, instanceID, srcPath, resolvedDst, quiet, archive)
+			return copyDirContentsToInstance(ctx, baseURL, apiKey, instanceID, srcPath, resolvedDst, quiet, archive, followLinks)
 		}
-		return copyDirToInstance(ctx, baseURL, apiKey, instanceID, srcPath, resolvedDst, quiet, archive)
+		return copyDirToInstance(ctx, baseURL, apiKey, instanceID, srcPath, resolvedDst, quiet, archive, followLinks)
 	}
-	return copyFileToInstance(ctx, baseURL, apiKey, instanceID, srcPath, resolvedDst, srcInfo.Mode().Perm(), quiet, archive)
+	return copyFileToInstance(ctx, baseURL, apiKey, instanceID, srcPath, resolvedDst, srcInfo.Mode().Perm(), quiet, archive, followLinks)
 }
 
 // copyFileToInstance copies a single file to the instance using the SDK
-func copyFileToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, mode fs.FileMode, quiet, archive bool) error {
+func copyFileToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, mode fs.FileMode, quiet, archive, followLinks bool) error {
 	srcInfo, err := os.Stat(srcPath)
 	if err != nil {
 		return fmt.Errorf("stat source: %w", err)
@@ -442,12 +478,13 @@ func copyFileToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPat
 	}
 
 	err = lib.CpToInstance(ctx, cfg, lib.CpToInstanceOptions{
-		InstanceID: instanceID,
-		SrcPath:    srcPath,
-		DstPath:    dstPath,
-		Mode:       mode,
-		Archive:    archive,
-		Callbacks:  callbacks,
+		InstanceID:  instanceID,
+		SrcPath:     srcPath,
+		DstPath:     dstPath,
+		Mode:        mode,
+		Archive:     archive,
+		FollowLinks: followLinks,
+		Callbacks:   callbacks,
 	})
 	if err != nil {
 		return err
@@ -457,7 +494,7 @@ func copyFileToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPat
 }
 
 // copyDirToInstance copies a directory recursively to the instance using the SDK
-func copyDirToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, quiet, archive bool) error {
+func copyDirToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, quiet, archive, followLinks bool) error {
 	cfg := lib.CpConfig{
 		BaseURL: baseURL,
 		APIKey:  apiKey,
@@ -474,11 +511,12 @@ func copyDirToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath
 
 	// First create the destination directory
 	err := lib.CpToInstance(ctx, cfg, lib.CpToInstanceOptions{
-		InstanceID: instanceID,
-		SrcPath:    srcPath,
-		DstPath:    dstPath,
-		Archive:    archive,
-		Callbacks:  callbacks,
+		InstanceID:  instanceID,
+		SrcPath:     srcPath,
+		DstPath:     dstPath,
+		Archive:     archive,
+		FollowLinks: followLinks,
+		Callbacks:   callbacks,
 	})
 	if err != nil {
 		return err
@@ -489,7 +527,7 @@ func copyDirToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath
 
 // copyDirContentsToInstance copies only the contents of a directory (not the directory itself)
 // This implements the /. suffix behavior from docker cp
-func copyDirContentsToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, quiet, archive bool) error {
+func copyDirContentsToInstance(ctx context.Context, baseURL, apiKey, instanceID, srcPath, dstPath string, quiet, archive, followLinks bool) error {
 	entries, err := os.ReadDir(srcPath)
 	if err != nil {
 		return fmt.Errorf("read directory: %w", err)
@@ -519,12 +557,13 @@ func copyDirContentsToInstance(ctx context.Context, baseURL, apiKey, instanceID,
 		}
 
 		if err := lib.CpToInstance(ctx, cfg, lib.CpToInstanceOptions{
-			InstanceID: instanceID,
-			SrcPath:    srcEntryPath,
-			DstPath:    dstEntryPath,
-			Mode:       info.Mode().Perm(),
-			Archive:    archive,
-			Callbacks:  callbacks,
+			InstanceID:  instanceID,
+			SrcPath:     srcEntryPath,
+			DstPath:     dstEntryPath,
+			Mode:        info.Mode().Perm(),
+			Archive:     archive,
+			FollowLinks: followLinks,
+			Callbacks:   callbacks,
 		}); err != nil {
 			return err
 		}
@@ -695,7 +734,11 @@ func copyFromStdinToInstance(ctx context.Context, baseURL, apiKey, instanceID, d
 			return fmt.Errorf("read tar header: %w", err)
 		}
 
-		targetPath := filepath.Join(dstPath, header.Name)
+		// Sanitize tar entry path to prevent path traversal attacks
+		targetPath, err := sanitizeTarPath(dstPath, header.Name)
+		if err != nil {
+			return err
+		}
 
 		// Extract uid/gid from tar header if archive mode
 		var uid, gid uint32
@@ -860,8 +903,8 @@ func copyFromInstanceToStdout(ctx context.Context, baseURL, apiKey, instanceID,
 
 			switch msgTypeStr {
 			case "header":
-				// Flush previous file if any
-				if currentHeader != nil && !currentHeader.IsDir {
+				// Flush previous file if any (but not symlinks - they have no data)
+				if currentHeader != nil && !currentHeader.IsDir && !currentHeader.IsSymlink {
 					if err := writeTarEntry(tw, currentHeader, fileData); err != nil {
 						return err
 					}