Commit44b89b0
fix(orgs): stop the team page leaking rosters and private repo names
fix(orgs): stop the team page leaking rosters and private repo names
GET /orgs/:org/teams/:team was mounted with softAuth and no membership
check of any kind, while every sibling org route required auth. It served
any anonymous caller the team's member roster, its permission level, and
its repository list — and that repo query joins `repositories` with no
isPrivate filter, so the NAMES of an organization's private repositories
leaked to anyone who could guess an org and team name. Private repo names
routinely disclose unreleased products, customers and acquisitions.
The policy is not invented here: the people page states it outright in
its own comment ("org membership is non-public") and bounces non-members.
This route just did not follow it. Anonymous callers now bounce to login
before any database work; non-members get 404 rather than 403, so the
page cannot be used to enumerate which orgs and teams exist.
Membership is resolved inline against orgMembers rather than through
loadOrgForUser, because that helper keys on organizations.slug while this
route keys on organizations.name — routing it through the helper would
have quietly changed which URLs resolve.
Deliberately NOT changed: GET /orgs/:org shows the roster and team names
to any authenticated user without requiring membership. It exposes no
repository list, `organizations` has no visibility column, and making org
profile pages member-only is a product decision rather than a security
patch. Flagging it rather than silently widening scope.
Also dropped from the punch list this iteration: "anonymous posting to
discussions" does not hold up. All eight POST routes in discussions.tsx
carry requireAuth, both comment paths refuse locked or closed threads
before inserting, lock and pin require the repo owner, and close plus
both answer routes require owner-or-author.
Both fault classes proven by injection: disabling the anonymous bounce
fails 4 tests, removing the membership lookup fails 2.
Suite: 3469 pass, 0 fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>2 files changed+153−044b89b0bcf5a27f12577ca6ed61ccb90e4ca0c32
2 changed files+153−0
Addedsrc/__tests__/org-team-roster.test.ts+128−0View fileUnifiedSplit
@@ -0,0 +1,128 @@
1/**
2 * Regression guard: an organization's team page must not be world-readable.
3 *
4 * GET /orgs/:org/teams/:team was mounted with `softAuth` and no membership
5 * check of any kind, while every sibling org route required auth. It served
6 * any anonymous caller:
7 *
8 * - the team's full member roster (usernames)
9 * - the team's permission level (read / write / admin)
10 * - the team's repository list
11 *
12 * The last one is the serious part: that query joins `repositories` with no
13 * `isPrivate` filter, so the NAMES of an organization's private repositories
14 * leaked to anyone who could guess an org and team name. Private repo names
15 * routinely disclose unreleased products, customers, and acquisitions.
16 *
17 * The policy being restored is not invented here — the people page states it
18 * outright in its own comment ("org membership is non-public") and bounces
19 * non-members. This route simply did not follow it.
20 *
21 * Mount only the router under test rather than importing `../app`: that is
22 * one singleton shared by the whole test run, and a static import binds
23 * whichever `../db` was mocked when some other file first triggered its load.
24 */
25
26import { describe, it, expect } from "bun:test";
27import { Hono } from "hono";
28import { readFileSync } from "node:fs";
29import { join } from "node:path";
30import orgRoutes from "../routes/orgs";
31
32const app = new Hono();
33app.route("/", orgRoutes);
34
35describe("anonymous access to a team page", () => {
36 // The anonymous branch returns before any database call, so this exercises
37 // the real handler without needing a DATABASE_URL.
38
39 it("redirects to login instead of rendering the roster", async () => {
40 const res = await app.request("/orgs/acme/teams/platform");
41 expect(res.status).toBe(302);
42 const location = res.headers.get("location") || "";
43 expect(location).toStartWith("/login?redirect=");
44 });
45
46 it("round-trips the requested team as the post-login target", async () => {
47 const res = await app.request("/orgs/acme/teams/platform");
48 const location = res.headers.get("location") || "";
49 const target = decodeURIComponent(location.split("redirect=")[1] || "");
50 expect(target).toBe("/orgs/acme/teams/platform");
51 });
52
53 it("encodes the redirect target so it stays a single relative path", async () => {
54 // An org or team name carrying a slash or a scheme must not be able to
55 // turn the login bounce into an off-origin redirect. lib/safe-redirect
56 // is the backstop, but the target should be well-formed to begin with.
57 const res = await app.request(
58 "/orgs/acme/teams/" + encodeURIComponent("https://evil.example")
59 );
60 const location = res.headers.get("location") || "";
61 expect(location).toStartWith("/login?redirect=");
62 expect(location).not.toContain("//evil.example");
63 });
64
65 it("does not leak roster or repository markup in the response body", async () => {
66 const body = await (await app.request("/orgs/acme/teams/platform")).text();
67 expect(body).not.toContain("Members (");
68 expect(body).not.toContain("access</span>");
69 });
70});
71
72// --- the membership check must stay in place -------------------------------
73//
74// Strip ONLY line comments: a block-comment regex eats route paths, which
75// contain a slash followed by a star.
76
77describe("the team route enforces org membership", () => {
78 const handler = (() => {
79 const text = readFileSync(
80 join(import.meta.dir, "..", "routes", "orgs.tsx"),
81 "utf8"
82 );
83 const src = text
84 .split("\n")
85 .filter((l) => !l.trim().startsWith("//"))
86 .join("\n");
87
88 const start = src.indexOf(`orgRoutes.get("/orgs/:org/teams/:team"`);
89 expect(start).toBeGreaterThan(-1);
90 // Slice by anchor to the end of the handler, not by character count.
91 const end = src.indexOf("orgRoutes.", start + 40);
92 const body = end > start ? src.slice(start, end) : src.slice(start);
93 expect(body.length).toBeGreaterThan(0);
94 return body;
95 })();
96
97 it("looks the viewer up in orgMembers", () => {
98 expect(handler).toContain("orgMembers");
99 expect(handler).toContain("eq(orgMembers.userId, user.id)");
100 });
101
102 it("bounces anonymous callers before touching the database", () => {
103 const anonAt = handler.indexOf("if (!user)");
104 const dbAt = handler.indexOf("db.select()");
105 expect(anonAt).toBeGreaterThan(-1);
106 expect(dbAt).toBeGreaterThan(-1);
107 expect(anonAt).toBeLessThan(dbAt);
108 });
109
110 it("resolves membership before reading the team or its repos", () => {
111 const memberAt = handler.indexOf("eq(orgMembers.userId, user.id)");
112 const teamAt = handler.indexOf("from(teams)");
113 const reposAt = handler.indexOf("from(teamRepos)");
114 expect(teamAt).toBeGreaterThan(memberAt);
115 expect(reposAt).toBeGreaterThan(memberAt);
116 });
117
118 it("answers a non-member with 404, not 403", () => {
119 // 403 would confirm the org and team exist, turning the page into an
120 // enumeration oracle for private org structure.
121 const memberAt = handler.indexOf("eq(orgMembers.userId, user.id)");
122 const after = handler.slice(memberAt);
123 expect(after).toContain("c.notFound()");
124 expect(after.indexOf("c.notFound()")).toBeLessThan(
125 after.indexOf("from(teams)")
126 );
127 });
128});
Modifiedsrc/routes/orgs.tsx+25−0View fileUnifiedSplit
@@ -1476,10 +1476,35 @@ orgRoutes.get("/orgs/:org/teams/:team", softAuth, async (c) => {
14761476 const teamName = c.req.param("team");
14771477 const user = c.get("user");
14781478
1479 // Team rosters are org-private — the same policy the people page states
1480 // outright ("org membership is non-public"). This route was softAuth with
1481 // no membership check at all, so it served any anonymous caller the team's
1482 // member list, its permission level, AND its repository list. That last
1483 // one joins `repositories` unfiltered, so the names of an organization's
1484 // PRIVATE repos leaked to anyone who could guess an org and team name.
1485 // Every sibling org route already requires auth; this one did not.
1486 if (!user) {
1487 return c.redirect(
1488 `/login?redirect=${encodeURIComponent(
1489 `/orgs/${orgName}/teams/${teamName}`
1490 )}`
1491 );
1492 }
1493
14791494 let org: any, team: any;
14801495 try {
14811496 [org] = await db.select().from(organizations).where(eq(organizations.name, orgName)).limit(1);
14821497 if (!org) return c.notFound();
1498
1499 // Non-members get the same 404 as a team that does not exist, so this
1500 // cannot be used to probe which orgs and teams are real.
1501 const [membership] = await db
1502 .select({ role: orgMembers.role })
1503 .from(orgMembers)
1504 .where(and(eq(orgMembers.orgId, org.id), eq(orgMembers.userId, user.id)))
1505 .limit(1);
1506 if (!membership) return c.notFound();
1507
14831508 [team] = await db.select().from(teams).where(and(eq(teams.orgId, org.id), eq(teams.name, teamName))).limit(1);
14841509 if (!team) return c.notFound();
14851510 } catch {
14861511