Commitf0cd0be
fix(security): any signed-in user could overwrite any repo's wiki
fix(security): any signed-in user could overwrite any repo's wiki POST /:owner/:repo/wiki (create) and POST /:owner/:repo/wiki/:slug/edit were guarded by `requireAuth` alone — "is anyone signed in" — with no ownership or collaborator check anywhere in the handler. Any registered account could create pages on, and overwrite pages of, any repository's wiki. The edit path is destructive: it UPDATEs title/body in place and bumps `revision`, so it replaced the owner's content rather than appending to it. The app.tsx private-repo gate does not cover this — it blocks PRIVATE repos, and this bypass worked on public ones. The same file's delete and revert handlers DID check, which is what makes the omission an inconsistency rather than a deliberate open-wiki policy. But they checked `user.id !== resolved.repo.ownerId`, which wrongly refuses collaborators and org members who legitimately have write access. Route all four mutating handlers through a single denyWikiWrite() helper backed by resolveRepoAccess + satisfiesAccess(access, "write"). That closes the hole on create/edit and simultaneously un-breaks collaborator access on delete/revert. Visibility is read from the repo row rather than assumed, so the check is correct for private repos too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 files changed+130−6f0cd0be2c311b75411f2c902733964a760173699
2 changed files+130−6
Addedsrc/__tests__/wiki-write-access.test.ts+66−0View fileUnifiedSplit
@@ -0,0 +1,66 @@
1/**
2 * Wiki writes require write access to the repository.
3 *
4 * POST /:owner/:repo/wiki (create) and POST /:owner/:repo/wiki/:slug/edit were
5 * guarded by `requireAuth` alone — "is anyone signed in" — with no ownership
6 * or collaborator check. Any registered account could create pages on, and
7 * overwrite pages of, any repository's wiki. The edit path is destructive: it
8 * UPDATEs title/body in place and bumps `revision`, replacing the owner's
9 * content rather than appending to it.
10 *
11 * The private-repo gate added in app.tsx does NOT cover this: it only blocks
12 * PRIVATE repos, and the bypass worked on public ones.
13 *
14 * delete and revert did check, but as `user.id !== resolved.repo.ownerId`,
15 * which wrongly refuses collaborators and org members with legitimate write
16 * access. All four now route through resolveRepoAccess.
17 */
18
19import { describe, expect, it } from "bun:test";
20import { readFileSync } from "fs";
21
22const SRC = readFileSync("src/routes/wikis.tsx", "utf8");
23
24describe("every mutating wiki route is gated", () => {
25 it("has a write gate at all four POST handlers", () => {
26 const calls = SRC.match(/await denyWikiWrite\(/g) ?? [];
27 expect(calls.length).toBe(4);
28 });
29
30 it("no owner-only comparison survives", () => {
31 // The over-restrictive shape that locked out collaborators.
32 expect(SRC).not.toMatch(/user\.id !== resolved\.repo\.ownerId/);
33 });
34
35 it("gates create before touching the form or inserting", () => {
36 const create = SRC.slice(
37 SRC.indexOf('wikiRoutes.post("/:owner/:repo/wiki", requireAuth'),
38 SRC.indexOf("wikiRoutes.get(\"/:owner/:repo/wiki/:slug\", softAuth")
39 );
40 const gate = create.indexOf("denyWikiWrite");
41 const insert = create.indexOf("db\n .insert(wikiPages)");
42 expect(gate).toBeGreaterThan(-1);
43 if (insert > -1) expect(gate).toBeLessThan(insert);
44 });
45});
46
47describe("the gate itself", () => {
48 const helper = SRC.slice(
49 SRC.indexOf("async function denyWikiWrite"),
50 SRC.indexOf("async function denyWikiWrite") + 900
51 );
52
53 it("requires write, not merely read", () => {
54 // A read-level check would still let any viewer of a public repo edit.
55 expect(helper).toContain('satisfiesAccess(access, "write")');
56 });
57
58 it("derives visibility from the repo row rather than assuming public", () => {
59 expect(helper).toContain("isPublic: !repo.isPrivate");
60 });
61
62 it("returns a redirect for the caller rather than throwing", () => {
63 expect(helper).toContain("return c.redirect(redirectTo)");
64 expect(helper).toContain("return null");
65 });
66});
Modifiedsrc/routes/wikis.tsx+64−6View fileUnifiedSplit
@@ -483,6 +483,42 @@ async function resolveRepo(ownerName: string, repoName: string) {
483483 }
484484}
485485
486/**
487 * Write gate for every mutating wiki route.
488 *
489 * POST /wiki (create) and POST /wiki/:slug/edit were guarded by `requireAuth`
490 * alone — "is anyone signed in" — with no ownership or collaborator check, so
491 * any registered account could create pages on, and overwrite pages of, any
492 * repository's wiki. The edit path is destructive: it UPDATEs title/body in
493 * place and bumps the revision, so it replaced the owner's content rather
494 * than appending to it.
495 *
496 * delete and revert did check, but as `user.id !== repo.ownerId`, which
497 * wrongly refuses collaborators and org members who legitimately have write
498 * access. Routing all four through resolveRepoAccess fixes both the hole and
499 * the over-restriction.
500 *
501 * Returns a redirect Response to be returned by the caller, or null when the
502 * caller may proceed.
503 */
504async function denyWikiWrite(
505 c: { redirect: (u: string) => Response },
506 repo: { id: string; isPrivate: boolean },
507 userId: string | null,
508 redirectTo: string
509): Promise<Response | null> {
510 const { resolveRepoAccess, satisfiesAccess } = await import(
511 "../middleware/repo-access"
512 );
513 const access = await resolveRepoAccess({
514 repoId: repo.id,
515 userId,
516 isPublic: !repo.isPrivate,
517 });
518 if (!satisfiesAccess(access, "write")) return c.redirect(redirectTo);
519 return null;
520}
521
486522function notFound(user: any, label = "Page not found") {
487523 return (
488524 <Layout title={label} user={user}>
@@ -858,6 +894,13 @@ wikiRoutes.post("/:owner/:repo/wiki", requireAuth, async (c) => {
858894 const user = c.get("user")!;
859895 const resolved = await resolveRepo(ownerName, repoName);
860896 if (!resolved) return c.redirect(`/${ownerName}/${repoName}`);
897 const denied = await denyWikiWrite(
898 c,
899 resolved.repo,
900 user.id,
901 `/${ownerName}/${repoName}/wiki`
902 );
903 if (denied) return denied;
861904
862905 const form = await c.req.formData();
863906 const title = (form.get("title") as string || "").trim();
@@ -1121,6 +1164,13 @@ wikiRoutes.post(
11211164 const user = c.get("user")!;
11221165 const resolved = await resolveRepo(ownerName, repoName);
11231166 if (!resolved) return c.redirect(`/${ownerName}/${repoName}`);
1167 const denied = await denyWikiWrite(
1168 c,
1169 resolved.repo,
1170 user.id,
1171 `/${ownerName}/${repoName}/wiki/${slug}`
1172 );
1173 if (denied) return denied;
11241174
11251175 const form = await c.req.formData();
11261176 const title = (form.get("title") as string || "").trim();
@@ -1178,9 +1228,13 @@ wikiRoutes.post(
11781228 const user = c.get("user")!;
11791229 const resolved = await resolveRepo(ownerName, repoName);
11801230 if (!resolved) return c.redirect(`/${ownerName}/${repoName}`);
1181 if (user.id !== resolved.repo.ownerId) {
1182 return c.redirect(`/${ownerName}/${repoName}/wiki/${slug}`);
1183 }
1231 const denied = await denyWikiWrite(
1232 c,
1233 resolved.repo,
1234 user.id,
1235 `/${ownerName}/${repoName}/wiki/${slug}`
1236 );
1237 if (denied) return denied;
11841238 try {
11851239 await db
11861240 .delete(wikiPages)
@@ -1385,9 +1439,13 @@ wikiRoutes.post(
13851439 const user = c.get("user")!;
13861440 const resolved = await resolveRepo(ownerName, repoName);
13871441 if (!resolved) return c.redirect(`/${ownerName}/${repoName}`);
1388 if (user.id !== resolved.repo.ownerId) {
1389 return c.redirect(`/${ownerName}/${repoName}/wiki/${slug}`);
1390 }
1442 const denied = await denyWikiWrite(
1443 c,
1444 resolved.repo,
1445 user.id,
1446 `/${ownerName}/${repoName}/wiki/${slug}`
1447 );
1448 if (denied) return denied;
13911449 try {
13921450 const [page] = await db
13931451 .select()
13941452