Commitf815168
fix(security): private repo source was readable anonymously via api-v2
fix(security): private repo source was readable anonymously via api-v2 Five git-content endpoints under /api/v2/repos/:owner/:repo gated only on `repoExists()`. That helper is a bare filesystem probe — `Bun.file(join(repoPath, "HEAD")).exists()` — so it never reads the repositories row and never sees isPrivate. branches, commits, commits/:sha, tree/:ref and contents/:path therefore served the full content of every private repository to unauthenticated callers, while the HTML surface correctly 404'd. Walking tree/:ref?recursive=1 then contents/:path exfiltrated the entire private source tree, including commit history with author email addresses and any committed credential files. Verified live against a private repo before the fix: the web UI returned 404 while /api/v2/.../branches, /commits, /tree/Main and /contents/package.json all returned 200 with real data. - Add repoPrivacyGate as router-level middleware over the whole /repos/:owner/:repo subtree. Per-handler gating is exactly what let five endpoints be missed; anything added later now inherits the check. - Gate on resolveRepoAccess(), not an ownerId comparison. The two handlers that already inlined a check compared `user.id !== owner.id`, which wrongly denies collaborators and org members. Those inline checks are now redundant but left in place. - Deny with 404, never 403 — a private repo must not confirm it exists. - Unknown repos fall through so each handler keeps its own 404 shape. Found by a multi-agent audit and independently reproduced before fixing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 files changed+124−0f81516877b8d3d649be6e20624a8ef81e21d8d33
2 changed files+124−0
Addedsrc/__tests__/api-v2-repo-privacy.test.ts+80−0View fileUnifiedSplit
@@ -0,0 +1,80 @@
1/**
2 * Private repositories must not be readable through the v2 REST API.
3 *
4 * Regression test for a live data leak found 2026-07-27: five git-content
5 * endpoints under /api/v2/repos/:owner/:repo gated only on `repoExists()`,
6 * which is a bare filesystem probe (Bun.file(<repoPath>/HEAD).exists()) and
7 * never reads the repositories row — so it never sees isPrivate. Anonymous
8 * callers could read branches, commit history (with author emails), the full
9 * recursive tree, and raw file contents of any private repo, while the HTML
10 * surface correctly returned 404.
11 *
12 * These are structural assertions rather than live HTTP calls: the leak was a
13 * missing gate, and what must not regress is that the gate exists and is
14 * mounted at the router rather than sprinkled per-handler.
15 */
16
17import { describe, expect, it } from "bun:test";
18import { readFileSync } from "fs";
19
20const SRC = readFileSync("src/routes/api-v2.ts", "utf8");
21
22describe("api-v2 repo privacy gate", () => {
23 it("is mounted on the whole /repos/:owner/:repo subtree", () => {
24 expect(SRC).toContain('apiv2.use("/repos/:owner/:repo", repoPrivacyGate)');
25 expect(SRC).toContain('apiv2.use("/repos/:owner/:repo/*", repoPrivacyGate)');
26 });
27
28 it("denies with 404, never 403 — a private repo must not confirm it exists", () => {
29 const gate = SRC.slice(
30 SRC.indexOf("async function repoPrivacyGate"),
31 SRC.indexOf("// ─── Helper")
32 );
33 expect(gate).toContain('access === "none"');
34 expect(gate).toContain('c.json({ error: "Not found" }, 404)');
35 // No 403 as an actual returned status (the word appears in a comment
36 // explaining why 404 is the right choice, hence matching the call shape).
37 expect(gate).not.toMatch(/,\s*403\s*\)/);
38 });
39
40 it("uses resolveRepoAccess, not an ownerId comparison", () => {
41 const gate = SRC.slice(
42 SRC.indexOf("async function repoPrivacyGate"),
43 SRC.indexOf("// ─── Helper")
44 );
45 // An ownerId equality check would wrongly deny collaborators and org
46 // members — that was the flaw in the pre-existing inline checks.
47 expect(gate).toContain("resolveRepoAccess");
48 expect(gate).not.toMatch(/user\.id\s*!==\s*\w*[Oo]wner/);
49 });
50
51 it("runs after apiAuth so the viewer identity is populated", () => {
52 // If the gate were registered before apiAuth, c.get("user") would always
53 // be undefined and every private repo would 404 even for its owner.
54 expect(SRC.indexOf('apiv2.use("*", apiAuth)')).toBeLessThan(
55 SRC.indexOf('apiv2.use("/repos/:owner/:repo", repoPrivacyGate)')
56 );
57 });
58
59 it("lets unknown repos fall through to each handler's own 404", () => {
60 const gate = SRC.slice(
61 SRC.indexOf("async function repoPrivacyGate"),
62 SRC.indexOf("// ─── Helper")
63 );
64 expect(gate).toContain("if (!resolved) return next()");
65 });
66});
67
68describe("repoExists is not a privacy check", () => {
69 it("never consults the repositories table", () => {
70 const repo = readFileSync("src/git/repository.ts", "utf8");
71 const fn = repo.slice(
72 repo.indexOf("export async function repoExists"),
73 repo.indexOf("export async function repoExists") + 400
74 );
75 // Documents WHY the router gate is required: this helper cannot answer
76 // "may this caller see it?", only "is there a directory on disk?".
77 expect(fn).not.toContain("isPrivate");
78 expect(fn).not.toContain("repositories");
79 });
80});
Modifiedsrc/routes/api-v2.ts+44−0View fileUnifiedSplit
@@ -101,6 +101,50 @@ apiv2.use(
101101 enforceAgentBranchNamespace
102102);
103103
104/**
105 * Repo privacy gate for the whole `/repos/:owner/:repo/*` subtree.
106 *
107 * Most read handlers below gated only on `repoExists()`, which is a bare
108 * filesystem probe (`Bun.file(<repoPath>/HEAD).exists()`) — it never reads the
109 * `repositories` row, so it never sees `isPrivate`. That left the full content
110 * of every private repository readable by anonymous callers via `branches`,
111 * `commits`, `commits/:sha`, `tree/:ref` and `contents/:path`, even though the
112 * HTML surface correctly 404s. Walking `tree/:ref?recursive=1` then
113 * `contents/:path` exfiltrated the entire private source tree.
114 *
115 * Gating once at the router is deliberate: per-handler checks are what allowed
116 * five endpoints to be missed, and any handler added later inherits this
117 * automatically. The two handlers that already inline a check keep theirs —
118 * they are now redundant but harmless.
119 *
120 * Uses `resolveRepoAccess`, not an `ownerId` comparison: the pre-existing
121 * inline checks tested `user.id !== owner.id`, which wrongly denies
122 * collaborators and org members. Unknown repos fall through untouched so each
123 * handler keeps emitting its own 404 shape.
124 */
125apiv2.use("/repos/:owner/:repo", repoPrivacyGate);
126apiv2.use("/repos/:owner/:repo/*", repoPrivacyGate);
127
128async function repoPrivacyGate(c: any, next: any) {
129 const owner = c.req.param("owner");
130 const repo = c.req.param("repo");
131 if (!owner || !repo) return next();
132
133 const resolved = await resolveRepo(owner, repo);
134 if (!resolved) return next();
135
136 const viewer = c.get("user");
137 const access = await resolveRepoAccess({
138 repoId: (resolved.repo as any).id,
139 userId: viewer?.id ?? null,
140 isPublic: !(resolved.repo as any).isPrivate,
141 });
142 // 404 rather than 403 — a private repo must not confirm its own existence.
143 if (access === "none") return c.json({ error: "Not found" }, 404);
144
145 return next();
146}
147
104148// ─── Helper ─────────────────────────────────────────────────────────────────
105149
106150async function resolveRepo(ownerName: string, repoName: string) {
107151