fix: copy operation edge cases and error reporting

- Fix symlink path validation when destination is root (containment check)
- Return proper errors from handleCopyTo/handleCopyFrom for failed operations
- Use cpErrorSent wrapper to avoid duplicate error messages to client
- Detect incomplete transfers when WebSocket closes without end message

Author: rgarcia

cmd/api/api/cp.go

@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -15,6 +16,15 @@ import (
 	mw "github.com/onkernel/hypeman/lib/middleware"
 )
 
+// cpErrorSent wraps an error that has already been sent to the client.
+// The caller should log this error but not send it again to avoid duplicates.
+type cpErrorSent struct {
+	err error
+}
+
+func (e *cpErrorSent) Error() string { return e.err.Error() }
+func (e *cpErrorSent) Unwrap() error { return e.err }
+
 // CpRequest represents the JSON body for copy requests
 type CpRequest struct {
 	// Direction: "to" copies from client to guest, "from" copies from guest to client
@@ -155,8 +165,12 @@ func (s *ApiService) CpHandler(w http.ResponseWriter, r *http.Request) {
 			"subject", subject,
 			"duration_ms", duration.Milliseconds(),
 		)
-		errMsg, _ := json.Marshal(CpError{Type: "error", Message: cpErr.Error()})
-		ws.WriteMessage(websocket.TextMessage, errMsg)
+		// Only send error message if it hasn't already been sent to the client
+		var sentErr *cpErrorSent
+		if !errors.As(cpErr, &sentErr) {
+			errMsg, _ := json.Marshal(CpError{Type: "error", Message: cpErr.Error()})
+			ws.WriteMessage(websocket.TextMessage, errMsg)
+		}
 		return
 	}
 
@@ -205,6 +219,7 @@ func (s *ApiService) handleCopyTo(ctx context.Context, ws *websocket.Conn, inst
 	}
 
 	// Read data chunks from WebSocket and forward to guest
+	var receivedEndMessage bool
 	for {
 		msgType, data, err := ws.ReadMessage()
 		if err != nil {
@@ -219,6 +234,7 @@ func (s *ApiService) handleCopyTo(ctx context.Context, ws *websocket.Conn, inst
 			var msg map[string]interface{}
 			if json.Unmarshal(data, &msg) == nil {
 				if msg["type"] == "end" {
+					receivedEndMessage = true
 					break
 				}
 			}
@@ -232,6 +248,11 @@ func (s *ApiService) handleCopyTo(ctx context.Context, ws *websocket.Conn, inst
 		}
 	}
 
+	// If the WebSocket closed without receiving an end message, the transfer is incomplete
+	if !receivedEndMessage {
+		return fmt.Errorf("client disconnected before completing transfer")
+	}
+
 	// Send end message to guest
 	if err := stream.Send(&guest.CopyToGuestRequest{
 		Request: &guest.CopyToGuestRequest_End{End: &guest.CopyToGuestEnd{}},
@@ -255,8 +276,10 @@ func (s *ApiService) handleCopyTo(ctx context.Context, ws *websocket.Conn, inst
 	resultJSON, _ := json.Marshal(result)
 	ws.WriteMessage(websocket.TextMessage, resultJSON)
 
-	// Don't return an error here - the result message already contains any error info.
-	// Returning an error would cause the caller to send a duplicate error message.
+	if !resp.Success {
+		// Return a wrapped error so the caller logs it correctly but doesn't send a duplicate
+		return &cpErrorSent{err: fmt.Errorf("copy to guest failed: %s", resp.Error)}
+	}
 	return nil
 }
 
@@ -334,9 +357,8 @@ func (s *ApiService) handleCopyFrom(ctx context.Context, ws *websocket.Conn, ins
 			}
 			errJSON, _ := json.Marshal(cpErr)
 			ws.WriteMessage(websocket.TextMessage, errJSON)
-			// Don't return an error here - the error message was already sent to the client.
-			// Returning an error would cause the caller to send a duplicate error message.
-			return nil
+			// Return a wrapped error so the caller logs it correctly but doesn't send a duplicate
+			return &cpErrorSent{err: fmt.Errorf("copy from guest failed: %s", r.Error.Message)}
 		}
 	}
 

lib/guest/client.go

@@ -514,8 +514,16 @@ func CopyFromInstance(ctx context.Context, vsockSocketPath string, opts CopyFrom
 			// Resolve the link target relative to the symlink's parent directory
 			linkDir := filepath.Dir(targetPath)
 			resolvedTarget := filepath.Clean(filepath.Join(linkDir, r.Header.LinkTarget))
-			if !strings.HasPrefix(resolvedTarget, filepath.Clean(opts.DstPath)+string(filepath.Separator)) &&
-				resolvedTarget != filepath.Clean(opts.DstPath) {
+			cleanDst := filepath.Clean(opts.DstPath)
+			// Check path containment - handle root destination specially
+			var contained bool
+			if cleanDst == "/" {
+				// For root destination, any absolute path that doesn't contain ".." after cleaning is valid
+				contained = !strings.Contains(resolvedTarget, "..")
+			} else {
+				contained = strings.HasPrefix(resolvedTarget, cleanDst+string(filepath.Separator)) || resolvedTarget == cleanDst
+			}
+			if !contained {
 				return fmt.Errorf("invalid symlink target (escapes destination): %s", r.Header.LinkTarget)
 			}