Skip to content

Commit ca45e2a

Browse files
committed
fix: multiple cp improvements
- Close resp.Body in copyTarFileToInstance on dial failure - Add receivedFinal flag to detect incomplete transfers in copyFromInstanceToStdout - Use path package for guest paths in sanitizeTarPath (cross-platform compatibility) - Update hypeman-go dependency
1 parent b9933ce commit ca45e2a

File tree

3 files changed

+21
-20
lines changed

3 files changed

+21
-20
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ require (
1212
github.com/gorilla/websocket v1.5.3
1313
github.com/itchyny/json2yaml v0.1.4
1414
github.com/muesli/reflow v0.3.0
15-
github.com/onkernel/hypeman-go v0.7.1-0.20251223024018-6970d69a9e1e
15+
github.com/onkernel/hypeman-go v0.7.1-0.20251223025630-30d438f9919f
1616
github.com/tidwall/gjson v1.18.0
1717
github.com/tidwall/pretty v1.2.1
1818
github.com/urfave/cli-docs/v3 v3.0.0-alpha6

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s=
105105
github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8=
106106
github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc=
107107
github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
108-
github.com/onkernel/hypeman-go v0.7.1-0.20251223024018-6970d69a9e1e h1:XY7ZZOqyYfSVhGOtx+Z7rK4RyvqYWgfseTmqGTXIGMI=
109-
github.com/onkernel/hypeman-go v0.7.1-0.20251223024018-6970d69a9e1e/go.mod h1:Wtm4ewVGGPZc2ySeeuQISQyJxujyQuyDjXyksVkIyy8=
108+
github.com/onkernel/hypeman-go v0.7.1-0.20251223025630-30d438f9919f h1:5Tnk4IxErhHxg1D2z+OSfpC2RtISoqRlZEsPsvyZVLk=
109+
github.com/onkernel/hypeman-go v0.7.1-0.20251223025630-30d438f9919f/go.mod h1:Wtm4ewVGGPZc2ySeeuQISQyJxujyQuyDjXyksVkIyy8=
110110
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
111111
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
112112
github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040=

pkg/cmd/cp.go

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -249,34 +249,28 @@ func isWindowsPath(path string) bool {
249249

250250
// sanitizeTarPath validates and sanitizes a tar entry path to prevent path traversal attacks.
251251
// Returns the sanitized target path or an error if the path is malicious.
252+
// Uses path package (not filepath) because tar paths and guest paths use forward slashes.
252253
func sanitizeTarPath(basePath, entryName string) (string, error) {
253-
// Clean the entry name
254-
clean := filepath.Clean(entryName)
254+
// Clean the entry name using path.Clean (forward slashes for guest/tar paths)
255+
clean := path.Clean(entryName)
255256

256-
// Reject absolute paths
257-
if filepath.IsAbs(clean) {
257+
// Reject absolute paths (Linux paths start with /)
258+
if strings.HasPrefix(clean, "/") {
258259
return "", fmt.Errorf("invalid tar entry: absolute path not allowed: %s", entryName)
259260
}
260261

261-
// Reject paths that start with ..
262+
// Reject paths that start with .. (escaping destination)
262263
if strings.HasPrefix(clean, "..") {
263264
return "", fmt.Errorf("invalid tar entry: path escapes destination: %s", entryName)
264265
}
265266

266-
// Join with base path
267-
targetPath := filepath.Join(basePath, clean)
267+
// Join with base path using path.Join (forward slashes for guest paths)
268+
targetPath := path.Join(basePath, clean)
268269

269270
// Verify the result is under the base path
270-
absBase, err := filepath.Abs(basePath)
271-
if err != nil {
272-
return "", fmt.Errorf("resolve base path: %w", err)
273-
}
274-
absTarget, err := filepath.Abs(targetPath)
275-
if err != nil {
276-
return "", fmt.Errorf("resolve target path: %w", err)
277-
}
278-
279-
if !strings.HasPrefix(absTarget, absBase+string(filepath.Separator)) && absTarget != absBase {
271+
// path.Clean removes trailing slashes, so compare cleaned versions
272+
cleanBase := path.Clean(basePath)
273+
if !strings.HasPrefix(targetPath, cleanBase+"/") && targetPath != cleanBase {
280274
return "", fmt.Errorf("invalid tar entry: path escapes destination: %s", entryName)
281275
}
282276

@@ -793,6 +787,7 @@ func copyTarFileToInstance(ctx context.Context, baseURL, apiKey, instanceID stri
793787
ws, resp, err := dialer.DialContext(ctx, wsURL, headers)
794788
if err != nil {
795789
if resp != nil {
790+
defer resp.Body.Close()
796791
body, _ := io.ReadAll(resp.Body)
797792
return fmt.Errorf("websocket connect failed (HTTP %d): %s", resp.StatusCode, string(body))
798793
}
@@ -894,6 +889,7 @@ func copyFromInstanceToStdout(ctx context.Context, baseURL, apiKey, instanceID,
894889

895890
var currentHeader *cpFileHeader
896891
var fileData []byte
892+
var receivedFinal bool
897893

898894
for {
899895
msgType, message, err := ws.ReadMessage()
@@ -970,6 +966,7 @@ func copyFromInstanceToStdout(ctx context.Context, baseURL, apiKey, instanceID,
970966
var endMarker cpEndMarker
971967
json.Unmarshal(message, &endMarker)
972968
if endMarker.Final {
969+
receivedFinal = true
973970
return nil
974971
}
975972

@@ -984,6 +981,10 @@ func copyFromInstanceToStdout(ctx context.Context, baseURL, apiKey, instanceID,
984981
}
985982
}
986983

984+
// If connection closed without receiving final marker, the transfer was incomplete
985+
if !receivedFinal {
986+
return fmt.Errorf("copy stream ended without completion marker")
987+
}
987988
return nil
988989
}
989990

0 commit comments

Comments
 (0)