From d7178ee752631e62517a021c819ec41eb644b9a7 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:40:41 +1100 Subject: [PATCH] Cap puppet tarball scan size and clamp release pagination --- internal/provider/puppet/local.go | 42 ++++++++---- internal/provider/puppet/local_test.go | 91 +++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 16 deletions(-) diff --git a/internal/provider/puppet/local.go b/internal/provider/puppet/local.go index 145e6f1..b9a3a4c 100644 --- a/internal/provider/puppet/local.go +++ b/internal/provider/puppet/local.go @@ -2,7 +2,6 @@ package puppet import ( "archive/tar" - "bytes" "compress/gzip" "context" "crypto/md5" @@ -67,28 +66,32 @@ func (p *Provider) AfterUpload(ctx context.Context, repoName, storagePath, conte return } defer func() { _ = reader.Close() }() - - raw, err := io.ReadAll(reader) - if err != nil { - slog.Error("puppet metadata: read failed", "repo", repoName, "path", storagePath, "error", err) + if size > maxModuleTarball { + slog.Error("puppet metadata: tarball too large", "repo", repoName, "path", storagePath, "size", size) return } - meta, err := parseModule(raw) + + hash := md5.New() + src := io.TeeReader(io.LimitReader(reader, maxModuleTarball), hash) + meta, err := parseModule(src) if err != nil { slog.Error("puppet metadata: parse failed", "repo", repoName, "path", storagePath, "error", err) return } + if _, err := io.Copy(io.Discard, src); err != nil { + slog.Error("puppet metadata: read failed", "repo", repoName, "path", storagePath, "error", err) + return + } slug, version, _ := parseRelease(strings.TrimSuffix(path.Base(storagePath), ".tar.gz")) if meta.Slug != slug || meta.Version != version { slog.Error("puppet metadata: metadata.json does not match filename", "repo", repoName, "path", storagePath, "name", meta.Slug, "version", meta.Version) return } - sum := md5.Sum(raw) meta.RepoName = repoName meta.FilePath = storagePath meta.ContentHash = contentHash meta.Size = size - meta.MD5 = hex.EncodeToString(sum[:]) + meta.MD5 = hex.EncodeToString(hash.Sum(nil)) store, ok := db.(provider.PuppetMetadataStore) if !ok { @@ -112,15 +115,25 @@ func (p *Provider) AfterDelete(ctx context.Context, repoName, storagePath string const maxMetadataJSON = 1 << 20 -// parseModule reads the top-level /metadata.json from a module tarball. -func parseModule(raw []byte) (*provider.PuppetMetadata, error) { - zr, err := gzip.NewReader(bytes.NewReader(raw)) +var ( + maxModuleTarball int64 = 100 << 20 + maxModuleScan int64 = 200 << 20 +) + +// parseModule reads the top-level /metadata.json from a module tarball, +// scanning at most maxModuleScan decompressed bytes. +func parseModule(r io.Reader) (*provider.PuppetMetadata, error) { + zr, err := gzip.NewReader(r) if err != nil { return nil, err } - tr := tar.NewReader(zr) + scan := &io.LimitedReader{R: zr, N: maxModuleScan} + tr := tar.NewReader(scan) for { hdr, err := tr.Next() + if scan.N <= 0 { + return nil, errors.New("module tarball exceeds decompressed scan limit before metadata.json") + } if err == io.EOF { return nil, errors.New("no metadata.json in module tarball") } @@ -128,7 +141,7 @@ func parseModule(raw []byte) (*provider.PuppetMetadata, error) { return nil, err } dir, file := path.Split(strings.TrimPrefix(path.Clean(hdr.Name), "./")) - if file != "metadata.json" || strings.Count(dir, "/") != 1 { + if hdr.Typeflag != tar.TypeReg || file != "metadata.json" || strings.Count(dir, "/") != 1 { continue } body, err := io.ReadAll(io.LimitReader(tr, maxMetadataJSON)) @@ -225,8 +238,9 @@ func serveReleaseList(w http.ResponseWriter, r *http.Request, list func(string) http.Error(w, err.Error(), http.StatusInternalServerError) return } + offset = min(offset, len(metas)) results := []map[string]any{} - for _, m := range metas[min(offset, len(metas)):min(offset+limit, len(metas))] { + for _, m := range metas[offset : offset+min(limit, len(metas)-offset)] { results = append(results, releaseObject(m)) } diff --git a/internal/provider/puppet/local_test.go b/internal/provider/puppet/local_test.go index 637a755..57927d3 100644 --- a/internal/provider/puppet/local_test.go +++ b/internal/provider/puppet/local_test.go @@ -104,14 +104,101 @@ func TestAfterUploadRejectsMismatchedFilename(t *testing.T) { } func TestParseModuleRequiresTopLevelMetadata(t *testing.T) { - if _, err := parseModule(moduleTarball(t, "a/b", helloMetadata)); err == nil { + if _, err := parseModule(bytes.NewReader(moduleTarball(t, "a/b", helloMetadata))); err == nil { t.Error("nested metadata.json accepted") } - if _, err := parseModule(moduleTarball(t, "x", `{"name":"unkin-hello"}`)); err == nil { + if _, err := parseModule(bytes.NewReader(moduleTarball(t, "x", `{"name":"unkin-hello"}`))); err == nil { t.Error("missing version accepted") } } +func tarballOf(t *testing.T, entries ...*tar.Header) []byte { + t.Helper() + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + tw := tar.NewWriter(zw) + for _, h := range entries { + if err := tw.WriteHeader(h); err != nil { + t.Fatal(err) + } + if h.Typeflag == tar.TypeReg { + body := helloMetadata + if !strings.HasSuffix(h.Name, "metadata.json") { + body = strings.Repeat("x", int(h.Size)) + } + _, _ = tw.Write([]byte(body)) + } + } + _ = tw.Close() + _ = zw.Close() + return buf.Bytes() +} + +func TestParseModuleSkipsNonRegularMetadata(t *testing.T) { + raw := tarballOf(t, + &tar.Header{Name: "unkin-hello-0.1.0/metadata.json", Typeflag: tar.TypeSymlink, Linkname: "/etc/passwd"}, + &tar.Header{Name: "unkin-hello-0.1.0/metadata.json/", Typeflag: tar.TypeDir, Mode: 0o755}, + &tar.Header{Name: "unkin-hello-0.1.0/metadata.json", Typeflag: tar.TypeReg, Mode: 0o644, Size: int64(len(helloMetadata))}, + ) + meta, err := parseModule(bytes.NewReader(raw)) + if err != nil || meta.Slug != "unkin-hello" || meta.Version != "0.1.0" { + t.Fatalf("got %v, %v", meta, err) + } +} + +func TestParseModuleCapsDecompressedScan(t *testing.T) { + defer func(n int64) { maxModuleScan = n }(maxModuleScan) + maxModuleScan = 64 << 10 + raw := tarballOf(t, + &tar.Header{Name: "unkin-hello-0.1.0/big", Typeflag: tar.TypeReg, Mode: 0o644, Size: 1 << 20}, + &tar.Header{Name: "unkin-hello-0.1.0/metadata.json", Typeflag: tar.TypeReg, Mode: 0o644, Size: int64(len(helloMetadata))}, + ) + if _, err := parseModule(bytes.NewReader(raw)); err == nil || !strings.Contains(err.Error(), "scan limit") { + t.Fatalf("expected scan limit error, got %v", err) + } + maxModuleScan = 2 << 20 + if _, err := parseModule(bytes.NewReader(raw)); err != nil { + t.Fatalf("under cap: %v", err) + } +} + +func TestAfterUploadCapsTarballSize(t *testing.T) { + defer func(n int64) { maxModuleTarball = n }(maxModuleTarball) + raw := moduleTarball(t, "unkin-hello-0.1.0", helloMetadata) + maxModuleTarball = int64(len(raw)) - 1 + store := &fakeStore{} + (&Provider{}).AfterUpload(context.Background(), "forge", "v3/files/unkin-hello-0.1.0.tar.gz", "sha256:abc", fakeBlobs{raw}, store) + if store.inserted != nil { + t.Error("oversized tarball indexed") + } + maxModuleTarball = int64(len(raw)) + (&Provider{}).AfterUpload(context.Background(), "forge", "v3/files/unkin-hello-0.1.0.tar.gz", "sha256:abc", fakeBlobs{raw}, store) + if store.inserted == nil { + t.Error("tarball at cap not indexed") + } +} + +func TestReleaseListClampsPagination(t *testing.T) { + files := releases("0.1.0", "0.2.0", "0.3.0") + for _, q := range []string{"offset=9223372036854775807", "offset=9223372036854775807&limit=9223372036854775807", "offset=-5&limit=-5", "limit=9223372036854775807"} { + code, body := get(t, files, "v3/releases?module=unkin-hello&"+q) + if code != http.StatusOK { + t.Fatalf("%s: code %d", q, code) + } + pg := body["pagination"].(map[string]any) + if l := pg["limit"].(float64); l < 1 || l > 100 { + t.Errorf("%s: limit %v", q, l) + } + if o := pg["offset"].(float64); o < 0 || o > 3 { + t.Errorf("%s: offset %v", q, o) + } + } + _, body := get(t, files, "v3/releases?module=unkin-hello&offset=9223372036854775807") + if len(body["results"].([]any)) != 0 || body["pagination"].(map[string]any)["next"] != nil { + t.Errorf("past-end page: %v", body) + } +} + func get(t *testing.T, files provider.FileStore, target string) (int, map[string]any) { t.Helper() rec := httptest.NewRecorder()