Commit303c5e2
fix(git): authorize the pusher before buffering and expanding the body
fix(git): authorize the pusher before buffering and expanding the body POST /:owner/:repo.git/git-receive-pack did `await c.req.arrayBuffer()` and then `Bun.gunzipSync(...)` — buffering the whole body and expanding it in full — and only AFTER that checked write access. Any unauthenticated caller who knew a repository name (public ones are listed on /explore) could post a gzip bomb and have the server expand it before deciding they had no right to push. gzip passes 1000:1 on repetitive input, so a 10 MB upload becomes ~10 GB resident. gunzipSync compounded it by being synchronous: decompression blocked the event loop for its whole duration, stalling every other in-flight request. That is a latency DoS on its own, independent of the memory one. Two changes. The access check now runs before the body is touched at all — nothing between the two needed the body, so this is a pure reorder. And decompression goes through gunzipBounded, which streams via DecompressionStream, checks a running total before retaining each chunk, and aborts past the budget. Memory is bounded by the limit rather than by the compression ratio, and awaiting each chunk yields to the event loop. Over-budget bodies get a 413. git-upload-pack was already correct — it gates read access first and streams the body without buffering — so it is unchanged. Dropped git-receive-pack from the no-unauthenticated-writes allowlist: with the guard ahead of the body read the scanner now sees it, and that test correctly failed until the stale entry went. Its sibling git-upload-pack stays listed; that scan still reads gitAccessGate as no guard, which is a scanner limitation, not a hole. Proven by injection: breaking the access check fails the ordering test. The bomb test builds a real one — 64 MiB of zeros compressing under 1 MB. Suite: 3437 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4 files changed+293−18303c5e2d4c3670f1ea8730e2db42499f6e410bcd
4 changed files+293−18
Addedsrc/__tests__/bounded-gunzip.test.ts+165−0View fileUnifiedSplit
@@ -0,0 +1,165 @@
1/**
2 * Regression guard: a git push body must not be able to OOM or stall the
3 * server, and must not be touched at all before the pusher is authorized.
4 *
5 * The bug in POST /:owner/:repo.git/git-receive-pack was an ordering one.
6 * The handler did `await c.req.arrayBuffer()` and then `Bun.gunzipSync(...)`
7 * — buffering the whole body and expanding it in full — and only AFTER that
8 * checked whether the caller had write access. So any unauthenticated caller
9 * who knew a repository name (public ones are listed on /explore) could post
10 * a gzip bomb and have the server expand it before deciding they had no
11 * right to push anything. gzip passes 1000:1 on repetitive input, so a
12 * 10 MB upload becomes ~10 GB of resident memory.
13 *
14 * `gunzipSync` compounded it by being synchronous: decompression blocked the
15 * event loop for its whole duration, stalling every other in-flight request.
16 * That is a latency DoS on its own, independent of the memory one.
17 *
18 * Two things are asserted: the decompressor is bounded, and the handler
19 * still reads the body only after the access gate.
20 */
21
22import { describe, it, expect } from "bun:test";
23import { readFileSync } from "node:fs";
24import { join } from "node:path";
25import {
26 gunzipBounded,
27 isGzip,
28 DecompressedTooLargeError,
29 MAX_GIT_BODY_BYTES,
30} from "../lib/bounded-gunzip";
31
32describe("isGzip", () => {
33 it("detects the gzip magic number", () => {
34 expect(isGzip(Bun.gzipSync(new Uint8Array([1, 2, 3])))).toBe(true);
35 });
36
37 it("rejects plain and truncated input", () => {
38 expect(isGzip(new Uint8Array([]))).toBe(false);
39 expect(isGzip(new Uint8Array([0x1f]))).toBe(false);
40 expect(isGzip(new TextEncoder().encode("0000"))).toBe(false);
41 });
42});
43
44describe("gunzipBounded", () => {
45 it("round-trips ordinary content unchanged", async () => {
46 const original = new TextEncoder().encode(
47 "0032want d4a1f9e0c2b3a4d5e6f70819a2b3c4d5e6f70819\n0000"
48 );
49 const out = await gunzipBounded(Bun.gzipSync(original));
50 expect(new TextDecoder().decode(out)).toBe(
51 new TextDecoder().decode(original)
52 );
53 });
54
55 it("round-trips content spanning many internal chunks", async () => {
56 // Incompressible-ish payload large enough that the stream yields more
57 // than one chunk, exercising the concat path.
58 const big = new Uint8Array(3 * 1024 * 1024);
59 for (let i = 0; i < big.length; i++) big[i] = (i * 2654435761) & 0xff;
60 const out = await gunzipBounded(Bun.gzipSync(big));
61 expect(out.byteLength).toBe(big.byteLength);
62 expect(out[0]).toBe(big[0]);
63 expect(out[big.length - 1]).toBe(big[big.length - 1]);
64 });
65
66 it("refuses a gzip bomb instead of expanding it (the attack)", async () => {
67 // 64 MiB of zeros compresses to a few tens of KB — a ~1000:1 ratio, the
68 // exact shape of the attack. Compressed size is unremarkable; expanded
69 // size is what would have killed the process.
70 const bomb = Bun.gzipSync(new Uint8Array(64 * 1024 * 1024));
71 expect(bomb.byteLength).toBeLessThan(1024 * 1024);
72
73 await expect(gunzipBounded(bomb, 1024 * 1024)).rejects.toBeInstanceOf(
74 DecompressedTooLargeError
75 );
76 });
77
78 it("reports the limit it enforced", async () => {
79 const bomb = Bun.gzipSync(new Uint8Array(8 * 1024 * 1024));
80 try {
81 await gunzipBounded(bomb, 4096);
82 throw new Error("should have thrown");
83 } catch (err) {
84 expect(err).toBeInstanceOf(DecompressedTooLargeError);
85 expect((err as DecompressedTooLargeError).limit).toBe(4096);
86 }
87 });
88
89 it("accepts output that lands exactly on the limit", async () => {
90 const payload = new Uint8Array(1024);
91 const out = await gunzipBounded(Bun.gzipSync(payload), 1024);
92 expect(out.byteLength).toBe(1024);
93 });
94
95 it("rejects one byte over the limit", async () => {
96 const payload = new Uint8Array(1025);
97 await expect(
98 gunzipBounded(Bun.gzipSync(payload), 1024)
99 ).rejects.toBeInstanceOf(DecompressedTooLargeError);
100 });
101
102 it("defaults to a bound that is generous but finite", () => {
103 expect(MAX_GIT_BODY_BYTES).toBeGreaterThan(64 * 1024 * 1024);
104 expect(Number.isFinite(MAX_GIT_BODY_BYTES)).toBe(true);
105 });
106
107 it("surfaces corrupt gzip rather than hanging", async () => {
108 const corrupt = Bun.gzipSync(new TextEncoder().encode("hello"));
109 corrupt[corrupt.length - 4] ^= 0xff; // wreck the trailing CRC
110 await expect(gunzipBounded(corrupt)).rejects.toBeDefined();
111 });
112});
113
114// --- ordering: authorize before touching the body --------------------------
115//
116// Strip ONLY line comments. A block-comment regex eats route paths, which
117// contain a slash followed by a star — and this file is full of them.
118
119describe("git-receive-pack authorizes before reading the body", () => {
120 const src = (() => {
121 const text = readFileSync(
122 join(import.meta.dir, "..", "routes", "git.ts"),
123 "utf8"
124 );
125 return text
126 .split("\n")
127 .filter((l) => !l.trim().startsWith("//"))
128 .join("\n");
129 })();
130
131 const handler = (() => {
132 const start = src.indexOf(`git.post("/:owner/:repo.git/git-receive-pack"`);
133 expect(start).toBeGreaterThan(-1);
134 const body = src.slice(start);
135 expect(body.length).toBeGreaterThan(0);
136 return body;
137 })();
138
139 it("checks write access before arrayBuffer()", () => {
140 const gateAt = handler.indexOf(`satisfiesAccess(access, "write")`);
141 const readAt = handler.indexOf("c.req.arrayBuffer()");
142 expect(gateAt).toBeGreaterThan(-1);
143 expect(readAt).toBeGreaterThan(-1);
144 expect(gateAt).toBeLessThan(readAt);
145 });
146
147 it("checks write access before any decompression", () => {
148 const gateAt = handler.indexOf(`satisfiesAccess(access, "write")`);
149 const gunzipAt = handler.indexOf("gunzipBounded");
150 expect(gunzipAt).toBeGreaterThan(-1);
151 expect(gateAt).toBeLessThan(gunzipAt);
152 });
153
154 it("no longer uses the unbounded synchronous decompressor", () => {
155 expect(handler).not.toContain("gunzipSync");
156 });
157
158 it("still parses refs, so policy and post-receive keep working", () => {
159 // The reorder moved the body read; it must not have dropped ref parsing,
160 // or pushes would silently skip the policy gate and the hook.
161 expect(handler).toContain("parseReceivePackRefs");
162 const gunzipAt = handler.indexOf("gunzipBounded");
163 expect(handler.indexOf("parseReceivePackRefs")).toBeGreaterThan(gunzipAt);
164 });
165});
Modifiedsrc/__tests__/no-unauthenticated-writes.test.ts+3−1View fileUnifiedSplit
@@ -59,7 +59,9 @@ const PUBLIC_WRITES: Record<string, string> = {
5959
6060 // Git Smart HTTP — authenticates itself via lib/git-push-auth.ts.
6161 "POST /:owner/:repo.git/git-upload-pack": "git protocol, own auth",
62 "POST /:owner/:repo.git/git-receive-pack": "git protocol, own auth",
62 // git-receive-pack is deliberately absent: its write-access check now runs
63 // before it touches the request body, so the scanner sees the guard and
64 // the route no longer belongs on this allowlist.
6365
6466 // Third-party webhook receivers — authenticate by signature, not session.
6567 "POST /api/hooks/gatetest": "GateTest callback",
Addedsrc/lib/bounded-gunzip.ts+89−0View fileUnifiedSplit
@@ -0,0 +1,89 @@
1/**
2 * Size-bounded gzip decompression.
3 *
4 * `Bun.gunzipSync` has two properties that make it unsafe on a request path
5 * carrying attacker-controlled bytes:
6 *
7 * 1. No output bound. gzip reaches compression ratios past 1000:1 on
8 * repetitive input, so a 10 MB upload expands to ~10 GB in memory. The
9 * process is killed long before that completes.
10 * 2. It is synchronous. Decompression blocks the event loop for its whole
11 * duration, so a single large body stalls every other request the
12 * server is handling — a latency DoS independent of the memory one.
13 *
14 * This helper streams through `DecompressionStream`, checks a running total
15 * after every chunk, and aborts the moment the budget is exceeded. Memory is
16 * bounded by `limit` rather than by the compression ratio, and awaiting each
17 * chunk yields to the event loop instead of monopolising it.
18 */
19
20/** Thrown when decompressed output exceeds the caller's budget. */
21export class DecompressedTooLargeError extends Error {
22 readonly limit: number;
23 constructor(limit: number) {
24 super(`decompressed body exceeds ${limit} bytes`);
25 this.name = "DecompressedTooLargeError";
26 this.limit = limit;
27 }
28}
29
30/**
31 * Default ceiling for a git push body after decompression.
32 *
33 * Generous on purpose: git objects inside a packfile are already zlib
34 * compressed, so a genuine push gzips at close to 1:1 and a real one this
35 * large would be extraordinary. A bomb, by contrast, blows through this
36 * within the first few chunks and is cut off there.
37 */
38export const MAX_GIT_BODY_BYTES = 1024 * 1024 * 1024; // 1 GiB
39
40/** True if `bytes` starts with the gzip magic number. */
41export function isGzip(bytes: Uint8Array): boolean {
42 return bytes.length >= 2 && bytes[0] === 0x1f && bytes[1] === 0x8b;
43}
44
45/**
46 * Decompress `input`, refusing to buffer more than `limit` bytes of output.
47 *
48 * @throws {DecompressedTooLargeError} as soon as the budget is exceeded —
49 * before the offending data is retained, not after.
50 */
51export async function gunzipBounded(
52 input: Uint8Array,
53 limit: number = MAX_GIT_BODY_BYTES
54): Promise<Uint8Array> {
55 const stream = new Blob([input as BlobPart])
56 .stream()
57 .pipeThrough(new DecompressionStream("gzip"));
58
59 const reader = stream.getReader();
60 const chunks: Uint8Array[] = [];
61 let total = 0;
62
63 try {
64 while (true) {
65 const { done, value } = await reader.read();
66 if (done) break;
67 if (!value) continue;
68
69 total += value.byteLength;
70 // Check BEFORE retaining the chunk, so exceeding the budget never
71 // costs more than one chunk of headroom.
72 if (total > limit) {
73 throw new DecompressedTooLargeError(limit);
74 }
75 chunks.push(value);
76 }
77 } finally {
78 // Releases the underlying source whether we finished or bailed out.
79 reader.cancel().catch(() => {});
80 }
81
82 const out = new Uint8Array(total);
83 let offset = 0;
84 for (const chunk of chunks) {
85 out.set(chunk, offset);
86 offset += chunk.byteLength;
87 }
88 return out;
89}
Modifiedsrc/routes/git.ts+36−17View fileUnifiedSplit
@@ -26,6 +26,11 @@ import {
2626 type RepoAccessLevel,
2727} from "../middleware/repo-access";
2828import { audit } from "../lib/notify";
29import {
30 gunzipBounded,
31 isGzip,
32 DecompressedTooLargeError,
33} from "../lib/bounded-gunzip";
2934
3035const git = new Hono();
3136
@@ -117,23 +122,6 @@ git.post("/:owner/:repo.git/git-receive-pack", async (c) => {
117122 return c.text("Repository not found", 404);
118123 }
119124
120 // Read the body once; we parse refs from it for both pre-receive policy
121 // checks and the existing post-receive hook. Large pushes arrive gzip'd
122 // (same threshold behavior as fetch/clone negotiation — see protocol.ts)
123 // — decompress before ref-parsing or a big push silently skips policy
124 // checks and the post-receive hook (refs parse as empty, not an error).
125 const rawBodyBuffer = await c.req.arrayBuffer();
126 const rawBodyBytes = new Uint8Array(rawBodyBuffer);
127 let bodyBuffer: ArrayBuffer = rawBodyBuffer;
128 if (rawBodyBytes.length >= 2 && rawBodyBytes[0] === 0x1f && rawBodyBytes[1] === 0x8b) {
129 const decompressed = Bun.gunzipSync(rawBodyBytes);
130 bodyBuffer = decompressed.buffer.slice(
131 decompressed.byteOffset,
132 decompressed.byteOffset + decompressed.byteLength
133 ) as ArrayBuffer;
134 }
135 const refs = parseReceivePackRefs(new Uint8Array(bodyBuffer));
136
137125 // Resolve the pusher early so we can pass it to both the policy gate and
138126 // the post-receive hook (e.g. for AI auto-issue attribution).
139127 const authHeader = c.req.header("authorization");
@@ -172,6 +160,37 @@ git.post("/:owner/:repo.git/git-receive-pack", async (c) => {
172160 }
173161 }
174162
163 // Only now touch the body. This used to run FIRST, above the access check,
164 // which meant any unauthenticated caller who knew a repo name could post a
165 // gzip bomb and have the server buffer it and expand it in full before
166 // deciding they had no right to push at all. Decompression was also
167 // synchronous, so it blocked the event loop for every other request.
168 //
169 // We parse refs from the body for both the pre-receive policy checks and
170 // the post-receive hook. Large pushes arrive gzip'd (same threshold
171 // behavior as fetch/clone negotiation — see protocol.ts); decompress before
172 // ref-parsing or a big push silently skips policy checks and the
173 // post-receive hook (refs parse as empty, not an error).
174 const rawBodyBuffer = await c.req.arrayBuffer();
175 const rawBodyBytes = new Uint8Array(rawBodyBuffer);
176 let bodyBuffer: ArrayBuffer = rawBodyBuffer;
177 if (isGzip(rawBodyBytes)) {
178 let decompressed: Uint8Array;
179 try {
180 decompressed = await gunzipBounded(rawBodyBytes);
181 } catch (err) {
182 if (err instanceof DecompressedTooLargeError) {
183 return c.text("Push body too large.", 413);
184 }
185 return c.text("Could not read push body.", 400);
186 }
187 bodyBuffer = decompressed.buffer.slice(
188 decompressed.byteOffset,
189 decompressed.byteOffset + decompressed.byteLength
190 ) as ArrayBuffer;
191 }
192 const refs = parseReceivePackRefs(new Uint8Array(bodyBuffer));
193
175194 // Pre-receive policy: protected tags + ruleset name patterns. Fail-open
176195 // on any DB hiccup (the helper returns {allowed:true} in that case).
177196 // We also retain the repoRow so the pack-inspection hook below can reuse it.
178197