From 87e36eb41dcf507d6b30b913f46e0e9acf236005 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 10 Jun 2026 14:06:54 +0000 Subject: [PATCH] ptar: reject path traversal and duplicate paths in layers Co-authored-by: Jason Hall --- pymage/internal/ptar/ptar.go | 34 ++++++++++++++++++++++++++++--- pymage/internal/ptar/ptar_test.go | 18 ++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/pymage/internal/ptar/ptar.go b/pymage/internal/ptar/ptar.go index a89198a..4b995f4 100644 --- a/pymage/internal/ptar/ptar.go +++ b/pymage/internal/ptar/ptar.go @@ -17,6 +17,7 @@ package ptar import ( "archive/tar" "bytes" + "fmt" "io" "path" "sort" @@ -63,7 +64,10 @@ func WriteTar(w io.Writer, files []File) error { // stable and parents always precede children. dirs := map[string]struct{}{} for _, f := range files { - clean := path.Clean(strings.TrimPrefix(f.Path, "/")) + clean, err := cleanPath(f.Path) + if err != nil { + return err + } dir := path.Dir(clean) for dir != "." && dir != "/" && dir != "" { dirs[dir] = struct{}{} @@ -80,11 +84,17 @@ func WriteTar(w io.Writer, files []File) error { for d := range dirs { entries = append(entries, entry{name: d + "/", dir: true}) } + // Collisions are surfaced as errors rather than silently keeping an + // arbitrary winner: within a single layer two members must not map to the + // same path (it would make the layer depend on input ordering). seen := map[string]bool{} for _, f := range files { - clean := path.Clean(strings.TrimPrefix(f.Path, "/")) + clean, err := cleanPath(f.Path) + if err != nil { + return err + } if seen[clean] { - continue + return fmt.Errorf("ptar: duplicate path %q in layer", clean) } seen[clean] = true entries = append(entries, entry{name: clean, file: f}) @@ -126,6 +136,24 @@ func WriteTar(w io.Writer, files []File) error { return tw.Close() } +// cleanPath returns the normalized, slash-rooted-relative archive name for p, +// rejecting paths that would escape the archive root via "..". Absolute paths +// are made relative. +func cleanPath(p string) (string, error) { + clean := path.Clean("/" + strings.TrimPrefix(p, "/")) + rel := strings.TrimPrefix(clean, "/") + if rel == "" || rel == "." { + return "", fmt.Errorf("ptar: empty path %q", p) + } + // path.Clean("/"+...) already collapses any ".." that would escape the + // root, but a literal traversal prefix on the input is a strong signal of a + // malicious archive, so reject it explicitly. + if orig := path.Clean(p); orig == ".." || strings.HasPrefix(orig, "../") { + return "", fmt.Errorf("ptar: path %q escapes archive root", p) + } + return rel, nil +} + // TarBytes returns the deterministic tar stream for files. func TarBytes(files []File) ([]byte, error) { var buf bytes.Buffer diff --git a/pymage/internal/ptar/ptar_test.go b/pymage/internal/ptar/ptar_test.go index 2a877d0..3b2fa54 100644 --- a/pymage/internal/ptar/ptar_test.go +++ b/pymage/internal/ptar/ptar_test.go @@ -76,6 +76,24 @@ func TestTarSynthesizesParentDirsAndSorts(t *testing.T) { } } +func TestWriteTarRejectsTraversal(t *testing.T) { + for _, p := range []string{"../etc/passwd", "../../x", "a/../../../etc/shadow"} { + if _, err := TarBytes([]File{{Path: p, Data: []byte("x")}}); err == nil { + t.Errorf("expected error for traversal path %q", p) + } + } +} + +func TestWriteTarRejectsDuplicatePaths(t *testing.T) { + _, err := TarBytes([]File{ + {Path: "a/x.txt", Data: []byte("1")}, + {Path: "a/x.txt", Data: []byte("2")}, + }) + if err == nil { + t.Fatal("expected error for duplicate path") + } +} + func TestLayerStableDigest(t *testing.T) { files := []File{ {Path: "site-packages/foo/__init__.py", Data: []byte("print('hi')\n")},