# FR: pushes over ~1 MiB fail with an undiagnosable `400 bad request`

Status: **fixed** (2026-10-18). Root cause identified; see Resolution.
Filed: 2026-09-19
Observed on: `git.cosmocr.at` (deployed), pushing `rpg137@gmail.com/einkdashboards.git`
Reproduced on: `git@cosmocr.at/demo.git` with a 3 MiB payload (see below)

## Resolution (2026-10-18)

**Root cause: gitweb answered git's probe request with 400.** When a push is
larger than `http.postBuffer`, git (`remote-curl.c`, `probe_rpc`) first sends a
receive-pack POST whose entire body is a flush-pkt, `0000`
(`Content-Length: 4`). It checks the endpoint (and auth) that way before
streaming the real body chunked, and it needs a `200` back. gitweb passed the
probe to `DecodeReceivePack`, which fails on a request with no commands, and
the handler returned a bare `400`. git gave up before sending any of the pack.

That explains every observation below. Pack size alone triggers it, because
the probe is only sent over `postBuffer`. Ancestry makes no difference. Raising
`postBuffer` makes the push work, because then there is no probe. Chunked
decoding tests fine, because the pack never got as far as being decoded.
`Expect: 100-continue` (hypothesis 2) is not involved. A `GIT_CURL_VERBOSE`
trace shows the failing request is the 4-byte POST.

**Fix:**

- `githttp.DecodeReceivePack` reads the first 4 bytes and returns `ErrProbe`
  when they are a flush. Reading them can't block, because git sends the whole
  first pkt-line before it waits. A real push starts with a command.
- The handler answers `ErrProbe` with `200` and an empty result. The probe
  runs after authentication and authorization, as before, so a non-writer is
  still refused at the probe.
- Decode failures are now logged (`gitweb: receive-pack decode …`). A body
  over the cap gets `413` with the limit named; anything else gets `400` with
  the reason.

**Tests:**

- `TestCloneAndPush/large_push_over_http.postBuffer` pushes 3 MiB of random
  data with real git at `http.postBuffer=1048576` (the default) and re-clones
  it. It failed with HTTP 400 before the fix.
- `…/large_push_as_reader_denied` checks that the probe is still authorized.
- `TestDecodeReceivePackProbe`, `TestDecodeReceivePackProbeGzip` and
  `TestDecodeReceivePackChunked` (the regression test proposed below) cover
  the decode path.

The investigation notes below are kept as filed.

## Symptom

Pushing a repository whose pack exceeds git's default `http.postBuffer` (1 MiB)
fails:

```
$ git push --force origin master multi-dashboard
error: RPC failed; HTTP 400 curl 22 The requested URL returned error: 400
fatal: the remote end hung up unexpectedly
Everything up-to-date
```

`git fetch` against the same repo, over the same connection, with the same
credentials, succeeds immediately beforehand — so this is not auth, TLS,
routing, or repo existence.

The workaround is client-side:

```bash
git config http.postBuffer 524288000   # 500 MiB
```

After which the identical push succeeds. Every user has to discover and apply
this per-repo, and nothing in the server's response hints at it.

## What is actually wrong

**The primary defect is that the failure is unattributable.** In
`internal/server/githttp.go:140-143`:

```go
case gitReceivePack:
	req, err := githttp.DecodeReceivePack(r)
	if err != nil {
		http.Error(w, "400 bad request", http.StatusBadRequest)
		return
	}
```

Every possible decode failure — malformed pkt-line, truncated body, a pack
go-git cannot resolve, a body over `maxBody`, a read error mid-stream —
collapses into the same opaque string, and **`err` is discarded without being
logged**. Every other branch in this handler logs (`log.Printf("gitweb:
upload-pack %s: %v", ...)`); this one does not.

Consequence: the server operator cannot tell a client-side protocol error from
a server-side bug, and the client is told nothing actionable. Diagnosing the
incident above took roughly an hour and still did not reach a root cause.

## What has been ruled out

Investigated and eliminated, so nobody repeats the work:

| Hypothesis | Verdict |
|---|---|
| nginx rejecting the body | **No.** nginx does SNI passthrough (`docs/ARCHITECTURE.md:53`); it routes TCP and never parses HTTP, so `client_max_body_size` cannot apply. |
| `maxBody` cap in `requestBody` | **No.** 256 MiB (`internal/githttp/githttp.go:34`); the failing pack was ~5.7 MiB. |
| `MaxRepoBytes` | **No.** 256 MiB default (`main.go:57`), and it is enforced after decode. |
| Server read timeout cutting off a slow upload | **No.** Only `ReadHeaderTimeout` is set (`main.go:112`); there is no `ReadTimeout`. |
| **Chunked transfer-encoding decoding** | **No — tested.** `DecodeReceivePack` handles a chunked body correctly, at both minimal size and 6 MiB. See the test below. |

That last row matters: the obvious theory ("gitweb can't parse chunked
requests") is **false**, and an FR that asserted it would have sent the fix in
the wrong direction. The chunked path decodes; something else about a real
large push does not.

## Controlled reproduction (2026-09-19)

Reproduced on `git@cosmocr.at/demo.git` — a 688 KiB repo unrelated to the
original incident — using two branches identical except for ancestry, each
carrying a 3 MiB incompressible payload (random data; zeros would deflate away
and never exceed `postBuffer` on the wire).

| Branch | Common ancestor with `main` | Pack | `http.postBuffer` | Result |
|---|---|---|---|---|
| `size-only` | **yes** | 3.00 MiB | default (1 MiB) | **HTTP 400** |
| `no-ancestor` (orphan) | no | 3.00 MiB | default (1 MiB) | **HTTP 400** |
| `size-only` | yes | 3.00 MiB | 500 MiB | **succeeded** |

Client error is byte-identical to the original incident:

```
error: RPC failed; HTTP 400 curl 22 The requested URL returned error: 400
fatal: the remote end hung up unexpectedly
```

**Conclusion: pack size alone is sufficient.** The `size-only` branch shares an
ancestor with `main`, so no external delta bases and no thin-pack resolution
are involved, and it fails anyway. Ancestry makes no difference — both shapes
fail identically, and the same objects push cleanly once the client stops using
chunked encoding.

This demotes the thin-pack hypothesis below. The trigger is a pack exceeding
`http.postBuffer`, and the fix belongs in request handling rather than the pack
resolution path.

Also observed in the same session: a **force-push rewinding `main` succeeded**
at the default `postBuffer` (it sends a ref update and no new objects). Ref
update handling is therefore not implicated; only the large-pack path fails.

## Remaining hypotheses

In rough order of likelihood, after the reproduction above:

1. ~~**Thin packs.**~~ **Ruled out** by the controlled reproduction — a push
   sharing an ancestor, with no external delta bases, fails identically.
2. **`Expect: 100-continue` handling.** Now the leading suspect. curl adds the
   header for large streamed bodies; Go sends `100 Continue` only when the
   handler first reads the body, so any path that writes a response before
   reading would strand the client. This fits the evidence: it is specific to
   the large/streamed request path and independent of pack contents.
3. **Deployed binary drift.** `HEAD` here is `5444040` (2026-07-12) on branch
   `sso-browser-login`. Whether the running service matches this source is
   unconfirmed — the bug may already be fixed in tree. Worth eliminating
   cheaply before any code change, since the unit tests in this repo show the
   decode path handling chunked bodies correctly.

## Proposed change

### 1. Log the error (do this first, independently of any fix)

```go
req, err := githttp.DecodeReceivePack(r)
if err != nil {
	log.Printf("gitweb: receive-pack decode %s: %v", repoPath, err)
	http.Error(w, "400 bad request", http.StatusBadRequest)
	return
}
```

One line. It converts this class of incident from guesswork into a log grep,
and it is what tells us which of the hypotheses above is real.

### 2. Return a useful body to the client

git surfaces the response body on `remote:` lines. Distinguish the cases that
have distinct remedies — at minimum, separate "body too large" (413, with the
limit named) from "malformed request" (400). A client seeing
`remote: pack exceeds 256 MiB limit` needs no server log to self-serve.

### 3. Fix the underlying failure

Only once (1) identifies it. If it is thin packs, the fix belongs in the pack
resolution path, not in request handling — and no amount of `postBuffer` tuning
is the right answer for users.

## How to test

### Regression test for the chunked path (passes today — keep it that way)

Add to `internal/githttp/githttp_test.go`. This is the test that disproved the
original theory; it is worth keeping so the chunked path does not silently
regress:

```go
// TestDecodeReceivePackChunked drives a chunked body through a real
// http.Server, which is what git sends once a request exceeds
// http.postBuffer (default 1 MiB).
func TestDecodeReceivePackChunked(t *testing.T) {
	const (
		oldOID = "0000000000000000000000000000000000000000"
		newOID = "1111111111111111111111111111111111111111"
	)
	cmd := fmt.Sprintf("%s %s refs/heads/master\x00report-status\n", oldOID, newOID)
	var body bytes.Buffer
	fmt.Fprintf(&body, "%04x%s", len(cmd)+4, cmd)
	body.WriteString("0000")
	body.WriteString("PACK")
	body.Write([]byte{0, 0, 0, 2, 0, 0, 0, 0})
	body.Write(bytes.Repeat([]byte{0xAB}, 6<<20)) // past postBuffer
	body.Write(make([]byte, 20))

	errCh := make(chan error, 1)
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.ContentLength != -1 {
			t.Errorf("ContentLength = %d, want -1 (chunked)", r.ContentLength)
		}
		req, err := DecodeReceivePack(r)
		if err == nil && req.Packfile != nil {
			_, err = io.Copy(io.Discard, req.Packfile)
		}
		errCh <- err
	}))
	defer srv.Close()

	hreq, _ := http.NewRequest(http.MethodPost, srv.URL+"/git-receive-pack",
		io.NopCloser(bytes.NewReader(body.Bytes())))
	hreq.ContentLength = -1 // force chunked
	resp, err := http.DefaultClient.Do(hreq)
	if err != nil {
		t.Fatalf("request failed: %v", err)
	}
	defer resp.Body.Close()
	if err := <-errCh; err != nil {
		t.Fatalf("chunked body rejected: %v", err)
	}
}
```

Run: `go test ./internal/githttp/ -run Chunked -v`

### End-to-end reproduction (the one that actually fails)

A unit test cannot produce a real git pack. This recipe reproduces it on the
demo repo in under a minute; it is the one used for the results above.

```bash
git clone https://git.cosmocr.at/git@cosmocr.at/demo.git /tmp/repro
cd /tmp/repro

# Push to a scratch ref with an EXPLICIT refspec. This clone ships
# push.default=upstream, so a bare `git push origin <branch>` targets the
# branch's upstream (main) rather than creating a same-named branch —
# that mistake polluted main once already.
git checkout -b fr-repro

# Incompressible payload: zeros would deflate away and never exceed
# postBuffer on the wire.
mkdir -p fr-repro && head -c 3145728 /dev/urandom > fr-repro/payload-3mb.bin
git add fr-repro && git commit -qm "FR-large-push-400 repro payload"

git config --unset http.postBuffer 2>/dev/null   # ensure the 1 MiB default

# Expected: HTTP 400.
GIT_CURL_VERBOSE=1 git push origin fr-repro:refs/heads/fr-repro 2>&1 | tee /tmp/push.log

# With the logging change in place, the server log now names the cause:
sudo docker logs --tail 50 patrick_gitweb_1 | grep 'receive-pack decode'

# Control: identical objects, non-chunked request. Expected: success.
git -c http.postBuffer=524288000 push origin fr-repro:refs/heads/fr-repro

# Clean up the scratch ref.
git push origin :refs/heads/fr-repro
```

To also confirm ancestry is irrelevant, repeat with `git checkout --orphan`
instead of `git checkout -b`; both shapes fail identically.

`GIT_CURL_VERBOSE=1` shows whether the request went out chunked and whether
`Expect: 100-continue` was used — which settles hypothesis 2 directly.

### Acceptance criteria

- A failing push writes a specific, actionable reason to the server log.
- The client receives a response body distinguishing "too large" from
  "malformed".
- A >1 MiB push succeeds at the default `http.postBuffer`, with no client-side
  configuration, for both normal and orphan history.
- `go test ./...` green, chunked regression test included.

## Impact

Low frequency, high friction. It only bites repos over ~1 MiB, but when it does
the failure is opaque, the fix is non-obvious, and the workaround has to be
repeated per-repo per-clone. Of the repos currently hosted, `einkdashboards`
(5.7 MiB) trips it; `mtadash` (328 KiB) does not, which is why this went
unnoticed until now.