Commita1beefb
fix(security): forking a private repo published it
fix(security): forking a private repo published it POST /:owner/:repo/fork was gated only by requireAuth plus repoExists(). repoExists is a bare filesystem probe — Bun.file(join(repoPath, "HEAD")) .exists() — that never reads the repositories row, so nothing in the handler ever consulted sourceRepo.isPrivate. The insert then hardcoded isPrivate: false. Net effect: any logged-in user could fork a private repository they had no read access to, and the clone landed as a PUBLIC repo under their namespace. A single POST turned someone else's private source public, complete with history. - Gate on resolveRepoAccess() before the clone, not after: cloning first would copy the private objects onto disk even if the DB insert were then refused. - Deny with the same redirect a non-existent repo takes, so the endpoint does not become an oracle for which private repos exist. - Inherit sourceRepo.isPrivate instead of hardcoding false; a fork of a private repo stays private, as it does on GitHub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 files changed+75−1a1beefb867b6eef4821afe72fceedc62174afc9e
2 changed files+75−1
Addedsrc/__tests__/fork-privacy.test.ts+53−0View fileUnifiedSplit
@@ -0,0 +1,53 @@
1/**
2 * Forking must not publish a private repository.
3 *
4 * POST /:owner/:repo/fork was gated only by `requireAuth` plus `repoExists()`.
5 * repoExists is a bare filesystem probe (Bun.file(<repoPath>/HEAD).exists())
6 * that never reads the repositories row, so it never sees isPrivate — and the
7 * insert hardcoded `isPrivate: false`. Any logged-in user could fork a private
8 * repo they had no read access to, and the clone landed as a PUBLIC repo.
9 * One POST turned someone else's private source public.
10 */
11
12import { describe, expect, it } from "bun:test";
13import { readFileSync } from "fs";
14
15const SRC = readFileSync("src/routes/fork.tsx", "utf8");
16const handler = SRC.slice(SRC.indexOf('fork.post("/:owner/:repo/fork"'));
17
18describe("fork access gate", () => {
19 it("checks read access via resolveRepoAccess before cloning", () => {
20 expect(handler).toContain("resolveRepoAccess");
21 expect(handler).toContain('access === "none"');
22 });
23
24 it("gates BEFORE the git clone, not after", () => {
25 const gate = handler.indexOf('access === "none"');
26 const clone = handler.indexOf('"git", "clone"');
27 expect(gate).toBeGreaterThan(-1);
28 expect(clone).toBeGreaterThan(-1);
29 // Cloning first would copy the private objects to disk even if the DB
30 // insert were later refused.
31 expect(gate).toBeLessThan(clone);
32 });
33
34 it("denies indistinguishably from a missing repo", () => {
35 // Must not become an oracle for which private repos exist: the denial
36 // path is the same redirect a non-existent repo takes.
37 const denial = handler.slice(
38 handler.indexOf('access === "none"'),
39 handler.indexOf('"git", "clone"')
40 );
41 expect(denial).toContain("c.redirect(`/${ownerName}/${repoName}`)");
42 expect(denial).not.toMatch(/403|Forbidden|not allowed/i);
43 });
44});
45
46describe("fork visibility", () => {
47 it("inherits the source repo's visibility instead of hardcoding public", () => {
48 expect(handler).toContain("isPrivate: sourceRepo.isPrivate");
49 // Match the code shape, not the prose: the phrase also appears in the
50 // comment explaining what the old behaviour was.
51 expect(handler).not.toMatch(/^\s*isPrivate: false,/m);
52 });
53});
Modifiedsrc/routes/fork.tsx+22−1View fileUnifiedSplit
@@ -21,6 +21,7 @@ import { db } from "../db";
2121import { repositories, users, activityFeed } from "../db/schema";
2222import { softAuth, requireAuth } from "../middleware/auth";
2323import type { AuthEnv } from "../middleware/auth";
24import { resolveRepoAccess } from "../middleware/repo-access";
2425import { getRepoPath, repoExists } from "../git/repository";
2526import { config } from "../lib/config";
2627import { join } from "path";
@@ -604,6 +605,23 @@ fork.post("/:owner/:repo/fork", requireAuth, async (c) => {
604605 .limit(1);
605606 if (!sourceRepo) return c.redirect(`/${ownerName}/${repoName}`);
606607
608 // Nothing above this point consulted sourceRepo.isPrivate: the only gates
609 // were requireAuth and repoExists(), and repoExists is a bare filesystem
610 // probe that never reads the repositories row. Any logged-in user could
611 // therefore fork a private repo they had no read access to — and, because
612 // the insert below hardcoded isPrivate: false, the clone landed as a PUBLIC
613 // repository. One POST turned someone else's private source public.
614 const access = await resolveRepoAccess({
615 repoId: sourceRepo.id,
616 userId: user.id,
617 isPublic: !sourceRepo.isPrivate,
618 });
619 if (access === "none") {
620 // 404-equivalent: redirect exactly as a non-existent repo would, so this
621 // does not become an oracle for which private repos exist.
622 return c.redirect(`/${ownerName}/${repoName}`);
623 }
624
607625 // Clone the bare repo
608626 const sourcePath = getRepoPath(ownerName, repoName);
609627 const destPath = join(config.gitReposPath, user.username, `${repoName}.git`);
@@ -623,7 +641,10 @@ fork.post("/:owner/:repo/fork", requireAuth, async (c) => {
623641 description: sourceRepo.description
624642 ? `Fork of ${ownerName}/${repoName} — ${sourceRepo.description}`
625643 : `Fork of ${ownerName}/${repoName}`,
626 isPrivate: false,
644 // Inherit the source's visibility. Hardcoding false meant a fork of a
645 // private repo was published; GitHub likewise keeps a private repo's
646 // fork private.
647 isPrivate: sourceRepo.isPrivate,
627648 defaultBranch: sourceRepo.defaultBranch,
628649 diskPath: destPath,
629650 forkedFromId: sourceRepo.id,
630651