diff --git a/pymage/internal/cache/cache.go b/pymage/internal/cache/cache.go index 27feecb..58d08f8 100644 --- a/pymage/internal/cache/cache.go +++ b/pymage/internal/cache/cache.go @@ -85,7 +85,7 @@ func (c *Cache) PutText(key, val string) error { if err := tmp.Close(); err != nil { return err } - return os.Rename(tmpName, p) + return replaceFile(tmpName, p) } // Put stores the layer's compressed blob under key. Writes are atomic (temp @@ -111,5 +111,20 @@ func (c *Cache) Put(key string, l v1.Layer) error { if err := tmp.Close(); err != nil { return err } - return os.Rename(tmpName, c.path(key)) + return replaceFile(tmpName, c.path(key)) +} + +// replaceFile atomically moves src onto dst. os.Rename replaces an existing +// dst on POSIX but fails on Windows, so retry after removing dst there. The +// store is content-addressed (or overwrites an equivalent value), so removing +// an existing dst is safe. +func replaceFile(src, dst string) error { + if err := os.Rename(src, dst); err != nil { + if _, statErr := os.Stat(dst); statErr == nil { + _ = os.Remove(dst) + return os.Rename(src, dst) + } + return err + } + return nil } diff --git a/pymage/internal/cache/cache_test.go b/pymage/internal/cache/cache_test.go index 59061b0..e7f6b99 100644 --- a/pymage/internal/cache/cache_test.go +++ b/pymage/internal/cache/cache_test.go @@ -20,6 +20,14 @@ func TestTextRoundTrip(t *testing.T) { if v, ok := c.GetText("k"); !ok || v != "3.14" { t.Fatalf("GetText = %q,%v; want 3.14,true", v, ok) } + // Overwriting an existing key must succeed on all platforms (os.Rename onto + // an existing dest fails on Windows; replaceFile handles it). + if err := c.PutText("k", "3.15"); err != nil { + t.Fatalf("overwrite PutText: %v", err) + } + if v, ok := c.GetText("k"); !ok || v != "3.15" { + t.Fatalf("GetText after overwrite = %q,%v; want 3.15,true", v, ok) + } } func TestPutGetRoundTrip(t *testing.T) { diff --git a/pymage/internal/project/project.go b/pymage/internal/project/project.go index 107269d..5979f3b 100644 --- a/pymage/internal/project/project.go +++ b/pymage/internal/project/project.go @@ -75,7 +75,7 @@ func Discover(dir string) (Info, error) { } } if info.LockFile == "" { - return info, fmt.Errorf("no uv.lock or requirements.txt in %s", abs) + return info, fmt.Errorf("no uv.lock, requirements.lock, or requirements.txt in %s", abs) } pyproject := filepath.Join(abs, "pyproject.toml") diff --git a/pymage/internal/sbom/sbom.go b/pymage/internal/sbom/sbom.go index 0f531e7..922e410 100644 --- a/pymage/internal/sbom/sbom.go +++ b/pymage/internal/sbom/sbom.go @@ -8,6 +8,7 @@ import ( "encoding/json" "sort" + "github.com/imjasonh/terraform-playground/pymage/internal/lock" "github.com/imjasonh/terraform-playground/pymage/internal/wheelhouse" ) @@ -33,21 +34,25 @@ type hash struct { // Generate returns a deterministic CycloneDX JSON document for the wheels. func Generate(wheels []wheelhouse.ResolvedWheel) ([]byte, error) { + // Project names are case-insensitive (PEP 503), so sort and build PURLs from + // the normalized name for stable ordering and canonical pkg:pypi PURLs. sorted := append([]wheelhouse.ResolvedWheel(nil), wheels...) sort.Slice(sorted, func(i, j int) bool { - if sorted[i].Name != sorted[j].Name { - return sorted[i].Name < sorted[j].Name + ni, nj := lock.NormalizeName(sorted[i].Name), lock.NormalizeName(sorted[j].Name) + if ni != nj { + return ni < nj } return sorted[i].Version < sorted[j].Version }) d := doc{BOMFormat: "CycloneDX", SpecVersion: "1.5", Version: 1} for _, w := range sorted { + norm := lock.NormalizeName(w.Name) d.Components = append(d.Components, component{ Type: "library", - Name: w.Name, + Name: norm, Version: w.Version, - PURL: "pkg:pypi/" + w.Name + "@" + w.Version, + PURL: "pkg:pypi/" + norm + "@" + w.Version, Hashes: []hash{{Alg: "SHA-256", Content: w.SHA256}}, }) } diff --git a/pymage/internal/sbom/sbom_test.go b/pymage/internal/sbom/sbom_test.go index 219b56a..cc602fc 100644 --- a/pymage/internal/sbom/sbom_test.go +++ b/pymage/internal/sbom/sbom_test.go @@ -32,3 +32,25 @@ func TestGenerateDeterministicAndSorted(t *testing.T) { t.Error("components not sorted (alpha should precede beta)") } } + +func TestGenerateNormalizesNames(t *testing.T) { + // Names are normalized (PEP 503) for canonical PURLs and stable ordering. + wheels := []wheelhouse.ResolvedWheel{ + {Name: "Werkzeug", Version: "2.3.7", SHA256: "cccc"}, + {Name: "typing_extensions", Version: "4.0", SHA256: "dddd"}, + } + out, err := Generate(wheels) + if err != nil { + t.Fatal(err) + } + s := string(out) + if !strings.Contains(s, "pkg:pypi/werkzeug@2.3.7") { + t.Errorf("Werkzeug not normalized in PURL:\n%s", s) + } + if !strings.Contains(s, "pkg:pypi/typing-extensions@4.0") { + t.Errorf("typing_extensions not normalized in PURL:\n%s", s) + } + if strings.Contains(s, "Werkzeug") || strings.Contains(s, "typing_extensions") { + t.Errorf("non-normalized names present in SBOM:\n%s", s) + } +} diff --git a/pymage/internal/wheel/wheel.go b/pymage/internal/wheel/wheel.go index 5940223..73a36ce 100644 --- a/pymage/internal/wheel/wheel.go +++ b/pymage/internal/wheel/wheel.go @@ -263,6 +263,12 @@ func (w *Wheel) Files(layout Layout) ([]ptar.File, error) { if f.FileInfo().IsDir() { continue } + // Reject any ".." segment outright: even a member that resolves back + // under the prefix (e.g. "foo/../../bin/x") could escape site-packages + // and clobber other installed files. within() below is defense in depth. + if hasDotDotSegment(f.Name) { + return nil, fmt.Errorf("wheel: member %q contains a %q path segment", f.Name, "..") + } dst, exec, skip := w.dest(f.Name, site, bin, prefix) if skip { continue @@ -289,6 +295,17 @@ func (w *Wheel) Files(layout Layout) ([]ptar.File, error) { return out, nil } +// hasDotDotSegment reports whether any "/"- or "\"-separated segment of name is +// "..", which would let a wheel member traverse out of its install directory. +func hasDotDotSegment(name string) bool { + for _, seg := range strings.FieldsFunc(name, func(r rune) bool { return r == '/' || r == '\\' }) { + if seg == ".." { + return true + } + } + return false +} + // within reports whether child is parent itself or nested under it. func within(parent, child string) bool { parent = path.Clean(parent) diff --git a/pymage/internal/wheel/wheel_test.go b/pymage/internal/wheel/wheel_test.go index b629caf..af9677e 100644 --- a/pymage/internal/wheel/wheel_test.go +++ b/pymage/internal/wheel/wheel_test.go @@ -104,6 +104,25 @@ func TestFilesRejectsEscapingMember(t *testing.T) { } } +func TestFilesRejectsInternalDotDot(t *testing.T) { + // A member with a ".." segment that resolves back under the prefix (here, + // out of site-packages into the prefix's bin) must still be rejected. + dir := t.TempDir() + path, _ := testwheel.Write(t, dir, testwheel.Spec{ + Name: "evil", + Version: "1.0", + Modules: map[string]string{"pkg/../../../../bin/evil": "x\n"}, + }) + w, err := Open(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = w.Close() }() + if _, err := w.Files(layout); err == nil { + t.Fatal("expected error for member with an internal '..' segment") + } +} + func TestFilesLayout(t *testing.T) { w, err := Open(writeFixture(t)) if err != nil { diff --git a/pymage/internal/wheelhouse/fetch.go b/pymage/internal/wheelhouse/fetch.go index 3093734..d8452d9 100644 --- a/pymage/internal/wheelhouse/fetch.go +++ b/pymage/internal/wheelhouse/fetch.go @@ -73,6 +73,15 @@ func (c *wheelCache) put(sum string, r io.Reader) (string, error) { } dest := c.path(sum) if err := os.Rename(tmpName, dest); err != nil { + // os.Rename fails on Windows if dest exists; the cache is keyed by the + // content hash, so an existing dest is the same wheel — replace it. + if _, statErr := os.Stat(dest); statErr == nil { + _ = os.Remove(dest) + if err := os.Rename(tmpName, dest); err != nil { + return "", err + } + return dest, nil + } return "", err } return dest, nil