From fab778b80149dad1a71a2385fcf3fc0607a56582 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Tue, 3 Nov 2026 18:39:26 +1100 Subject: [PATCH 1/2] fix(git): limit size of request bodies buffered in memory Upload-pack negotiation and ensure-refs bodies are read fully into memory, so an oversized or endless (chunked) body could exhaust cachewd's memory. These bodies are now capped by the new git max-request-body-mb option (default 100MB, matching GIT_HTTP_MAX_REQUEST_BUFFER), returning 413 when exceeded. Streamed requests such as cache PUTs (cache warming), pushes, and other proxied requests are unaffected. Amp-Thread-ID: https://ampcode.com/threads/T-01a1a052-93bd-73a1-83b2-d55b78c2b1c0 Co-authored-by: Amp --- internal/strategy/git/git.go | 39 +++++++++++- internal/strategy/git/info_refs_test.go | 7 ++- internal/strategy/git/refs.go | 11 +++- internal/strategy/git/request_limit_test.go | 70 +++++++++++++++++++++ 4 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 internal/strategy/git/request_limit_test.go diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index 829fd395..19e2e9ea 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -58,6 +58,7 @@ type Config struct { RepackInterval time.Duration `hcl:"repack-interval,optional" help:"How often to run full repack. 0 disables." default:"0"` ZstdThreads int `hcl:"zstd-threads,optional" help:"Threads for zstd compression/decompression. 0 = all CPU cores; useful for short-lived CLI invocations but risky on a long-running server where multiple snapshot/restore operations can run concurrently." default:"4"` BundleCacheTTL time.Duration `hcl:"bundle-cache-ttl,optional" help:"TTL of cached server-side git bundles." default:"2h"` + MaxRequestBodyMB int `hcl:"max-request-body-mb,optional" help:"Maximum size in megabytes of git request bodies that are buffered in memory (upload-pack negotiation and ensure-refs). Pushes and other requests streamed to upstream are not limited. 0 disables the limit." default:"100"` SnapshotFilters map[string]string `hcl:"snapshot-filters,optional" help:"Per-repository git partial-clone filter applied to workstation snapshots, keyed by host/org/repo (e.g. {\"github.com/org/repo\": \"blob:none\"}). Filtered snapshots contain full history metadata but only the blobs needed for the HEAD checkout; clients lazily fetch historical blobs through cachew on demand."` } @@ -410,6 +411,10 @@ func (s *Strategy) handleGitRequest(w http.ResponseWriter, r *http.Request, host ctx := r.Context() logger := logging.FromContext(ctx) + if !s.limitRequestBody(w, r) { + return + } + repoPath := ExtractRepoPath(pathValue) upstreamURL := "https://" + host + "/" + repoPath @@ -434,7 +439,9 @@ func (s *Strategy) handleGitRequest(w http.ResponseWriter, r *http.Request, host switch state { case gitclone.StateReady: - if err := s.serveReadyRepo(w, r, repo, host, pathValue, isInfoRefs); err != nil { + if err := s.serveReadyRepo(w, r, repo, host, pathValue, isInfoRefs); requestTooLarge(err) { + writeRequestTooLarge(w, r) + } else if err != nil { logger.ErrorContext(ctx, "Failed to serve from local mirror", "error", err) http.Error(w, "Internal server error", http.StatusInternalServerError) } @@ -446,7 +453,10 @@ func (s *Strategy) handleGitRequest(w http.ResponseWriter, r *http.Request, host return s.startClone(ctx, repo) }) } - if err := s.serveWithSpool(w, r, host, pathValue, upstreamURL); err != nil { + // The body has been partially consumed, so it can't be forwarded upstream. + if err := s.serveWithSpool(w, r, host, pathValue, upstreamURL); requestTooLarge(err) { + writeRequestTooLarge(w, r) + } else if err != nil { logger.WarnContext(ctx, "Spool failed, forwarding to upstream", "error", err) s.forwardToUpstream(w, r, host, pathValue) } @@ -633,6 +643,31 @@ func (s *Strategy) serveWithSpool(w http.ResponseWriter, r *http.Request, host, return nil } +// limitRequestBody caps the body of requests whose body is buffered in memory, so a client can't exhaust memory with +// an oversized or endless body. Declared oversized bodies are rejected immediately; chunked bodies fail on read with an +// error matched by [requestTooLarge]. It returns false if it wrote a response. +func (s *Strategy) limitRequestBody(w http.ResponseWriter, r *http.Request) bool { + if s.config.MaxRequestBodyMB <= 0 || r.Body == nil || r.Body == http.NoBody { + return true + } + limit := int64(s.config.MaxRequestBodyMB) << 20 + if r.ContentLength > limit { + writeRequestTooLarge(w, r) + return false + } + r.Body = http.MaxBytesReader(w, r.Body, limit) + return true +} + +func requestTooLarge(err error) bool { + var maxErr *http.MaxBytesError + return errors.As(err, &maxErr) +} + +func writeRequestTooLarge(w http.ResponseWriter, r *http.Request) { + cachewhttputil.ErrorResponse(w, r, http.StatusRequestEntityTooLarge, "request body too large") +} + // isGitRequest reports whether pathValue matches a git smart HTTP protocol // endpoint (info/refs discovery or git-upload-pack negotiation). func isGitRequest(pathValue string) bool { diff --git a/internal/strategy/git/info_refs_test.go b/internal/strategy/git/info_refs_test.go index fb5f419a..8d3e6a7c 100644 --- a/internal/strategy/git/info_refs_test.go +++ b/internal/strategy/git/info_refs_test.go @@ -55,6 +55,11 @@ type infoRefsFixture struct { func newInfoRefsFixture(t *testing.T) *infoRefsFixture { t.Helper() + return newInfoRefsFixtureWithConfig(t, git.Config{}) +} + +func newInfoRefsFixtureWithConfig(t *testing.T, config git.Config) *infoRefsFixture { + t.Helper() tmpDir := t.TempDir() upstreamPath := filepath.Join(tmpDir, "upstream.git") @@ -78,7 +83,7 @@ func newInfoRefsFixture(t *testing.T) *infoRefsFixture { ctx := logging.ContextWithLogger(t.Context(), logger) mux := newTestMux() manager := gitclone.NewManagerProvider(ctx, gitclone.Config{MirrorRoot: mirrorRoot}, nil) - strategy, err := git.New(ctx, git.Config{}, newTestScheduler(ctx, t), nil, mux, manager, + strategy, err := git.New(ctx, config, newTestScheduler(ctx, t), nil, mux, manager, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil assert.NoError(t, err) waitForReady(t, strategy) diff --git a/internal/strategy/git/refs.go b/internal/strategy/git/refs.go index 401ce2ba..34abf404 100644 --- a/internal/strategy/git/refs.go +++ b/internal/strategy/git/refs.go @@ -80,8 +80,17 @@ func (s *Strategy) handleEnsureRefs(w http.ResponseWriter, r *http.Request, host return } + if !s.limitRequestBody(w, r) { + status = "too_large" + return + } + var req EnsureRefsRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.NewDecoder(r.Body).Decode(&req); requestTooLarge(err) { + writeRequestTooLarge(w, r) + status = "too_large" + return + } else if err != nil { http.Error(w, "invalid request body: "+err.Error(), http.StatusBadRequest) status = "bad_request" span.RecordError(err) diff --git a/internal/strategy/git/request_limit_test.go b/internal/strategy/git/request_limit_test.go new file mode 100644 index 00000000..ced9f7b1 --- /dev/null +++ b/internal/strategy/git/request_limit_test.go @@ -0,0 +1,70 @@ +package git_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/alecthomas/assert/v2" + + "github.com/block/cachew/internal/strategy/git" +) + +func uploadPackBody(want string, haves int) string { + var b strings.Builder + b.WriteString(pkt("want " + want + "\n")) + b.WriteString("0000") + for i := range haves { + b.WriteString(pkt(fmt.Sprintf("have %040x\n", i+1))) + } + b.WriteString(pkt("done\n")) + return b.String() +} + +func (f *infoRefsFixture) post(t *testing.T, path, body string, chunked bool) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequestWithContext(f.ctx, http.MethodPost, "/git/example.test/"+path, strings.NewReader(body)) + if chunked { + req.ContentLength = -1 + } + req.Header.Set("Content-Type", "application/x-git-upload-pack-request") + req.SetPathValue("host", "example.test") + req.SetPathValue("path", path) + w := httptest.NewRecorder() + f.uploadPack.ServeHTTP(w, req) + return w +} + +func TestRequestBodyLimit(t *testing.T) { + const limit = 1 << 20 + fixture := newInfoRefsFixtureWithConfig(t, git.Config{MaxRequestBodyMB: 1}) + setInfoRefsLsRemote(t, fixture.upstreamPath, false) + head := runInfoRefsGit(t, "-C", fixture.mirrorPath, "rev-parse", "HEAD") + oversized := uploadPackBody(head, limit/50) + assert.True(t, len(oversized) > limit) + + tests := []struct { + name string + path string + body string + chunked bool + status int + }{ + {"UploadPackUnderLimit", "org/repo/git-upload-pack", uploadPackBody(head, 15000), true, http.StatusOK}, + {"UploadPackContentLength", "org/repo/git-upload-pack", oversized, false, http.StatusRequestEntityTooLarge}, + {"UploadPackChunked", "org/repo/git-upload-pack", oversized, true, http.StatusRequestEntityTooLarge}, + {"UploadPackChunkedWhileCloning", "org/other/git-upload-pack", oversized, true, http.StatusRequestEntityTooLarge}, + {"EnsureRefsChunked", "org/repo/ensure-refs", `{"commits":["` + strings.Repeat("a", limit) + `"]}`, true, + http.StatusRequestEntityTooLarge}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + before := fixture.transport.hits.Load() + w := fixture.post(t, tt.path, tt.body, tt.chunked) + assert.Equal(t, tt.status, w.Code, w.Body.String()) + assert.Equal(t, before, fixture.transport.hits.Load()) + }) + } +} From 5dd67e83572b401948d9d436eb437241a5c872c0 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Tue, 3 Nov 2026 20:22:26 +1100 Subject: [PATCH 2/2] fix(git): drop comments on short body-limit code Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a1a052-93bd-73a1-83b2-d55b78c2b1c0 --- internal/strategy/git/git.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index 19e2e9ea..e549fcd6 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -453,7 +453,6 @@ func (s *Strategy) handleGitRequest(w http.ResponseWriter, r *http.Request, host return s.startClone(ctx, repo) }) } - // The body has been partially consumed, so it can't be forwarded upstream. if err := s.serveWithSpool(w, r, host, pathValue, upstreamURL); requestTooLarge(err) { writeRequestTooLarge(w, r) } else if err != nil { @@ -643,9 +642,6 @@ func (s *Strategy) serveWithSpool(w http.ResponseWriter, r *http.Request, host, return nil } -// limitRequestBody caps the body of requests whose body is buffered in memory, so a client can't exhaust memory with -// an oversized or endless body. Declared oversized bodies are rejected immediately; chunked bodies fail on read with an -// error matched by [requestTooLarge]. It returns false if it wrote a response. func (s *Strategy) limitRequestBody(w http.ResponseWriter, r *http.Request) bool { if s.config.MaxRequestBodyMB <= 0 || r.Body == nil || r.Body == http.NoBody { return true