1
0
Fork 0
mirror of https://github.com/imjasonh/terraform-playground synced 2026-07-07 23:35:16 +00:00

address Copilot review: normalize SBOM names, reject wheel '..' segments, Windows-safe cache/wheel rename, fix lock-not-found message

- sbom: normalize component names (PEP 503) for canonical pkg:pypi PURLs and stable ordering
- wheel: reject any member with a '..' path segment (catches escapes that resolve back under the prefix but out of site-packages)
- cache/wheelhouse: replace file atomically across platforms (os.Rename fails on existing dest on Windows)
- project: include requirements.lock in the no-lock-found error
- tests for each

Co-authored-by: Jason Hall <imjasonh@users.noreply.github.com>
This commit is contained in:
Cursor Agent 2026-06-11 02:01:37 +00:00
parent 971e955c35
commit 0cc8bc424f
No known key found for this signature in database
8 changed files with 102 additions and 7 deletions

View file

@ -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
}

View file

@ -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) {

View file

@ -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")

View file

@ -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}},
})
}

View file

@ -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)
}
}

View file

@ -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)

View file

@ -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 {

View file

@ -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