Pre-launch — Gluecron is in final validation. Public signups and git hosting for non-owner users open after launch review.
Commit88f5bd1

fix(packages): stop the npm registry serving private packages to anyone

fix(packages): stop the npm registry serving private packages to anyone

GET /npm/<name> resolved a package by name and returned its packument and
its tarball with no authorization check at all. The platform-wide
private-repo gate in app.tsx keys on the owner/repo path shape, which the
npm protocol routes do not have — they address packages by package name
alone — and which the JSON helper routes under the api prefix also miss.
So `npm install` handed the full source of a private repository's package
to an anonymous caller. The same three read paths leaked the package list
and metadata.

packages.visibility was already being stamped from the owning repo at
publish time; nothing ever read it back.

A package is now publicly readable only when the owning repository is
public AND the package row is not itself marked private. Both are
consulted because they drift in opposite directions: visibility is never
rewritten after publish, so a repo that later flips to private still has
packages marked "public" and the live repo row has to decide; while a
package explicitly marked private inside a public repo must stay private,
where the package row decides. Everything else falls through to
resolveRepoAccess, which grants the owner and accepted collaborators.

Denials are 404, not 403, so probing the registry cannot enumerate which
private package names exist.

The two UI routes do inherit the app.tsx gate, so the repo-private case
was already covered there — but neither filtered an individually-private
package inside a public repo, so both now do.

The rule lives in lib/package-access.ts behind an injectable-deps seam
rather than mock.module: bun loads every test module before running any
test and mock.module swaps the registry entry instead of rebinding
namespaces an importer already holds, so mocking ../db here would leak
into unrelated files. Tests cover the rule and, separately, assert the
read routes still call it — the original bug was a missing call, and a
rule nothing invokes is worth nothing. Both fault classes were proven by
introducing them and watching the suite fail: dropping the call site fails
1 test, weakening the rule fails 3.

Suite: 3414 pass, 0 fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ccantynz-alt committed on July 28, 2026Parent: 57cdb50
4 files changed+417388f5bd1788814318a33e1520e664b6677f58ebf6
4 changed files+417−3
Addedsrc/__tests__/package-access.test.ts+246−0View fileUnifiedSplit
1/**
2 * Regression guard: the package registry must not serve private packages.
3 *
4 * The bug this locks down: `GET /npm/...` resolved a package by name and
5 * returned its packument and tarball with no authorization check whatsoever.
6 * The platform-wide private-repo gate in app.tsx keys on the owner/repo path
7 * shape, which the npm protocol routes do not have (they address packages by
8 * package name alone) and which the JSON helper routes under the api prefix
9 * also miss. So `npm install` returned the FULL SOURCE of a private
10 * repository's package to an anonymous caller.
11 *
12 * Two layers are asserted here:
13 * 1. The decision rule in lib/package-access.ts, driven through its
14 * injectable-deps seam so no database is required.
15 * 2. That the read routes actually still CALL that rule — a rule nothing
16 * invokes is worth nothing, and the original bug was a missing call.
17 *
18 * Deliberately no `mock.module`: bun loads every test module before running
19 * any test, and mock.module swaps the registry entry rather than rebinding
20 * namespaces an importer already holds, so mocking `../db` here would leak
21 * into unrelated files in a full run.
22 */
23
24import { describe, it, expect, afterAll, beforeEach } from "bun:test";
25import { readFileSync } from "node:fs";
26import { join } from "node:path";
27import type { Package } from "../db/schema";
28import type { RepoAccessLevel } from "../middleware/repo-access";
29import {
30 canReadPackage,
31 canReadRepoPackages,
32 isRepoMember,
33 __setPackageAccessDepsForTests,
34} from "../lib/package-access";
35
36const PUBLIC_REPO = { id: "repo-public", isPrivate: false };
37const PRIVATE_REPO = { id: "repo-private", isPrivate: true };
38
39const OWNER_ID = "user-owner";
40const COLLAB_ID = "user-collab";
41const STRANGER_ID = "user-stranger";
42
43/**
44 * Stand-in for the real resolver. Mirrors its contract: the owner and an
45 * accepted collaborator get a level regardless of visibility; everyone else
46 * falls back to "read" for public repos and "none" for private ones.
47 */
48async function fakeResolve(args: {
49 repoId: string;
50 userId: string | null;
51 isPublic: boolean;
52}): Promise<RepoAccessLevel> {
53 if (args.userId === OWNER_ID) return "owner";
54 if (args.userId === COLLAB_ID) return "read";
55 return args.isPublic ? "read" : "none";
56}
57
58const REPOS_BY_ID: Record<string, { id: string; isPrivate: boolean }> = {
59 [PUBLIC_REPO.id]: PUBLIC_REPO,
60 [PRIVATE_REPO.id]: PRIVATE_REPO,
61};
62
63beforeEach(() => {
64 __setPackageAccessDepsForTests({
65 resolveRepoAccess: fakeResolve as any,
66 loadRepoById: async (id: string) => REPOS_BY_ID[id] ?? null,
67 });
68});
69
70afterAll(() => {
71 __setPackageAccessDepsForTests(null);
72});
73
74function pkg(overrides: Partial<Package>): Package {
75 return {
76 id: "pkg-1",
77 repositoryId: PUBLIC_REPO.id,
78 ecosystem: "npm",
79 scope: null,
80 name: "widget",
81 visibility: "public",
82 ...overrides,
83 } as Package;
84}
85
86describe("package read access — repository is private", () => {
87 it("denies an anonymous caller", async () => {
88 expect(await canReadRepoPackages(PRIVATE_REPO, null)).toBe(false);
89 });
90
91 it("denies a signed-in stranger", async () => {
92 expect(await canReadRepoPackages(PRIVATE_REPO, STRANGER_ID)).toBe(false);
93 });
94
95 it("allows the repository owner", async () => {
96 expect(await canReadRepoPackages(PRIVATE_REPO, OWNER_ID)).toBe(true);
97 });
98
99 it("allows an accepted collaborator", async () => {
100 expect(await canReadRepoPackages(PRIVATE_REPO, COLLAB_ID)).toBe(true);
101 });
102});
103
104describe("package read access — repository is public", () => {
105 it("allows an anonymous caller", async () => {
106 expect(await canReadRepoPackages(PUBLIC_REPO, null)).toBe(true);
107 });
108
109 it("still denies a package explicitly marked private", async () => {
110 // visibility is stamped at publish time and never rewritten, so a
111 // package marked private must stay private even in a public repo.
112 expect(await canReadRepoPackages(PUBLIC_REPO, null, "private")).toBe(
113 false
114 );
115 expect(await canReadRepoPackages(PUBLIC_REPO, STRANGER_ID, "private")).toBe(
116 false
117 );
118 });
119
120 it("allows members to see an explicitly private package", async () => {
121 expect(await canReadRepoPackages(PUBLIC_REPO, OWNER_ID, "private")).toBe(
122 true
123 );
124 expect(await canReadRepoPackages(PUBLIC_REPO, COLLAB_ID, "private")).toBe(
125 true
126 );
127 });
128});
129
130describe("canReadPackage resolves the owning repository live", () => {
131 it("denies an anonymous caller a package in a private repo", async () => {
132 // The package row still says "public" because it was published while the
133 // repo was public; the LIVE repo row is what must decide.
134 const stale = pkg({ repositoryId: PRIVATE_REPO.id, visibility: "public" });
135 expect(await canReadPackage(stale, null)).toBe(false);
136 });
137
138 it("allows the owner the same package", async () => {
139 const stale = pkg({ repositoryId: PRIVATE_REPO.id, visibility: "public" });
140 expect(await canReadPackage(stale, OWNER_ID)).toBe(true);
141 });
142
143 it("allows an anonymous caller a public package in a public repo", async () => {
144 expect(await canReadPackage(pkg({}), null)).toBe(true);
145 });
146
147 it("denies when the owning repository no longer exists", async () => {
148 const dangling = pkg({ repositoryId: "repo-deleted" });
149 expect(await canReadPackage(dangling, OWNER_ID)).toBe(false);
150 });
151});
152
153describe("isRepoMember", () => {
154 it("is false for anonymous even on a public repo", async () => {
155 // Membership is not the same as readability — a public reader is not a
156 // member, which is what decides visibility of private packages.
157 expect(await isRepoMember(PUBLIC_REPO.id, null)).toBe(false);
158 });
159
160 it("is false for a signed-in stranger", async () => {
161 expect(await isRepoMember(PUBLIC_REPO.id, STRANGER_ID)).toBe(false);
162 });
163
164 it("is true for the owner and for a collaborator", async () => {
165 expect(await isRepoMember(PUBLIC_REPO.id, OWNER_ID)).toBe(true);
166 expect(await isRepoMember(PUBLIC_REPO.id, COLLAB_ID)).toBe(true);
167 });
168});
169
170// --- call sites stay wired -------------------------------------------------
171//
172// Strip ONLY line comments before asserting. A block-comment regex eats route
173// paths (they contain a slash followed by a star), and these files are full
174// of them.
175
176function sourceWithoutLineComments(relPath: string): string {
177 const text = readFileSync(join(import.meta.dir, "..", relPath), "utf8");
178 const stripped = text
179 .split("\n")
180 .filter((l) => !l.trim().startsWith("//"))
181 .join("\n");
182 expect(stripped.length).toBeGreaterThan(0);
183 return stripped;
184}
185
186/** Slice by anchor, never by character count — handlers move. */
187function handlerBody(src: string, startAnchor: string, endAnchor: string) {
188 const start = src.indexOf(startAnchor);
189 expect(start).toBeGreaterThan(-1);
190 const end = src.indexOf(endAnchor, start + startAnchor.length);
191 expect(end).toBeGreaterThan(start);
192 const body = src.slice(start, end);
193 expect(body.length).toBeGreaterThan(0);
194 return body;
195}
196
197describe("registry read routes invoke the access gate", () => {
198 it("GET /npm/ checks canReadPackage before serving", async () => {
199 const src = sourceWithoutLineComments("routes/packages-api.ts");
200 const body = handlerBody(src, `api.get("/npm/*"`, `api.put("/npm/*"`);
201 expect(body).toContain("canReadPackage");
202 // The tarball branch lives inside this handler, so gating the handler
203 // gates the tarball too — assert the gate precedes the tarball read.
204 const gateAt = body.indexOf("canReadPackage");
205 const tarballAt = body.indexOf("match.tarball");
206 expect(tarballAt).toBeGreaterThan(gateAt);
207 });
208
209 it("the JSON package routes check the gate", async () => {
210 const src = sourceWithoutLineComments("routes/packages-api.ts");
211 const listBody = handlerBody(
212 src,
213 `api.get("/api/packages/:owner/:repo"`,
214 `api.get("/api/packages/:owner/:repo/:pkgName`
215 );
216 expect(listBody).toContain("canReadRepoPackages");
217 expect(listBody).toContain("isRepoMember");
218
219 const detailBody = handlerBody(
220 src,
221 `api.get("/api/packages/:owner/:repo/:pkgName`,
222 `api.get("/npm/*"`
223 );
224 expect(detailBody).toContain("canReadRepoPackages");
225 });
226
227 it("the package UI routes filter private packages for non-members", async () => {
228 const src = sourceWithoutLineComments("routes/packages.tsx");
229 const listBody = handlerBody(
230 src,
231 `ui.get("/:owner/:repo/packages"`,
232 `ui.get("/:owner/:repo/packages/:pkgName`
233 );
234 expect(listBody).toContain("isRepoMember");
235 // The rendered rows must come from the filtered list, not the raw query.
236 expect(listBody).toContain("visiblePkgs.map");
237 expect(listBody).not.toContain("rows = pkgs.map");
238
239 const detailBody = handlerBody(
240 src,
241 `ui.get("/:owner/:repo/packages/:pkgName`,
242 "export default"
243 );
244 expect(detailBody).toContain("isRepoMember");
245 });
246});
Addedsrc/lib/package-access.ts+123−0View fileUnifiedSplit
1/**
2 * Read authorization for the package registry.
3 *
4 * Why this is its own module rather than middleware: the npm protocol routes
5 * address packages by package name alone, so they never carry the
6 * owner-plus-repo path shape that the platform-wide private-repo gate in
7 * app.tsx keys on. The JSON helper routes sit under an api prefix and miss it
8 * for the same reason. Without an explicit check here, a private package's
9 * packument AND tarball are world-readable — `npm install` hands the full
10 * source of a private package to an anonymous caller.
11 *
12 * A package is publicly readable only when the owning repository is public
13 * AND the package row is not itself marked private. Both are consulted
14 * because the two drift apart in opposite directions:
15 *
16 * - `packages.visibility` is stamped from the repository at publish time
17 * and is never rewritten afterwards, so a repo that later flips to
18 * private still has packages marked "public". The live repository row is
19 * the authority there.
20 * - A package explicitly marked private inside a public repository must
21 * stay private, so the package row is the authority in that direction.
22 *
23 * Anything not publicly readable falls through to `resolveRepoAccess`, which
24 * grants the repository owner and accepted collaborators.
25 *
26 * Callers report a denial as 404, never 403, so that probing the registry
27 * cannot be used to enumerate which private package names exist.
28 */
29
30import { eq } from "drizzle-orm";
31import { db } from "../db";
32import { repositories } from "../db/schema";
33import type { Package } from "../db/schema";
34import {
35 resolveRepoAccess as realResolveRepoAccess,
36 satisfiesAccess,
37} from "../middleware/repo-access";
38
39// --- injectable deps -------------------------------------------------------
40//
41// Both external touchpoints go through `_deps` so tests can drive the access
42// RULE without a live database. This is deliberately NOT `mock.module`: bun
43// loads every test module before running any test and `mock.module` swaps the
44// registry entry rather than rebinding namespaces an importer already holds,
45// so mocking a widely-imported module like `../db` here would leak into every
46// unrelated test in the run.
47
48export type RepoRow = { id: string; isPrivate: boolean };
49
50export type PackageAccessDeps = {
51 resolveRepoAccess: typeof realResolveRepoAccess;
52 loadRepoById: (repoId: string) => Promise<RepoRow | null>;
53};
54
55const realLoadRepoById = async (repoId: string): Promise<RepoRow | null> => {
56 const [repo] = await db
57 .select({ id: repositories.id, isPrivate: repositories.isPrivate })
58 .from(repositories)
59 .where(eq(repositories.id, repoId))
60 .limit(1);
61 return repo || null;
62};
63
64const realDeps: PackageAccessDeps = {
65 resolveRepoAccess: realResolveRepoAccess,
66 loadRepoById: realLoadRepoById,
67};
68
69let _deps: PackageAccessDeps = realDeps;
70
71/** Test seam. Pass null to restore the real dependencies. */
72export function __setPackageAccessDepsForTests(
73 overrides: Partial<PackageAccessDeps> | null
74): void {
75 _deps = overrides ? { ...realDeps, ...overrides } : realDeps;
76}
77
78/**
79 * True if `userId` is a member of the repository — its owner or an accepted
80 * collaborator — as opposed to someone who merely gets "read" from the
81 * public fallback.
82 *
83 * Resolving with `isPublic: false` is what draws that line: the public
84 * fallback is switched off, so only a real membership row can satisfy it.
85 * This is the test for whether individually-private packages are visible.
86 */
87export async function isRepoMember(
88 repoId: string,
89 userId: string | null
90): Promise<boolean> {
91 if (!userId) return false;
92 const level = await _deps.resolveRepoAccess({
93 repoId,
94 userId,
95 isPublic: false,
96 });
97 return satisfiesAccess(level, "read");
98}
99
100/** True if `userId` may read packages belonging to `repo`. */
101export async function canReadRepoPackages(
102 repo: { id: string; isPrivate: boolean },
103 userId: string | null,
104 pkgVisibility?: string | null
105): Promise<boolean> {
106 const isPublic = !repo.isPrivate && pkgVisibility !== "private";
107 if (isPublic) return true;
108 return isRepoMember(repo.id, userId);
109}
110
111/**
112 * Read gate for the npm protocol routes, which resolve a package by name and
113 * therefore have to look the owning repository up by id.
114 */
115export async function canReadPackage(
116 pkg: Package,
117 userId: string | null
118): Promise<boolean> {
119 const repo = await _deps.loadRepoById(pkg.repositoryId);
120 // Dangling package with no owning repository: nothing can vouch for it.
121 if (!repo) return false;
122 return canReadRepoPackages(repo, userId, pkg.visibility);
123}
Modifiedsrc/routes/packages-api.ts+28−1View fileUnifiedSplit
2424import type { Package, PackageVersion } from "../db/schema";
2525import { softAuth, requireAuth } from "../middleware/auth";
2626import type { AuthEnv } from "../middleware/auth";
27import {
28 canReadPackage,
29 canReadRepoPackages,
30 isRepoMember,
31} from "../lib/package-access";
2732import { audit } from "../lib/notify";
2833import {
2934 parsePackageName,
154159 try {
155160 const repoRow = await loadRepo(owner, repo);
156161 if (!repoRow) return c.json({ error: "repo not found" }, 404);
162 if (!(await canReadRepoPackages(repoRow, c.get("user")?.id ?? null))) {
163 return c.json({ error: "repo not found" }, 404);
164 }
157165 const rows = await db
158166 .select()
159167 .from(packages)
164172 )
165173 )
166174 .orderBy(desc(packages.updatedAt));
167 return c.json({ packages: rows });
175 // A public repo can still hold individually-private packages. Listing is
176 // repo-scoped, so drop those unless the viewer is a member (owner or
177 // accepted collaborator) rather than a public-fallback reader.
178 const visible = (await isRepoMember(repoRow.id, c.get("user")?.id ?? null))
179 ? rows
180 : rows.filter((r) => r.visibility !== "private");
181 return c.json({ packages: visible });
168182 } catch (err) {
169183 console.error("[packages] list:", err);
170184 return c.json({ error: "service unavailable" }, 503);
180194 if (!repoRow) return c.json({ error: "repo not found" }, 404);
181195 const pkg = await loadPackage(repoRow.id, parsed.scope, parsed.name);
182196 if (!pkg) return c.json({ error: "package not found" }, 404);
197 const canRead = await canReadRepoPackages(
198 repoRow,
199 c.get("user")?.id ?? null,
200 pkg.visibility
201 );
202 if (!canRead) return c.json({ error: "package not found" }, 404);
183203 const versions = await db
184204 .select()
185205 .from(packageVersions)
257277 const pkg = await loadPackageByName(parsed.scope, parsed.name);
258278 if (!pkg) return c.json({ error: "not found" }, 404);
259279
280 // Private packages are indistinguishable from missing ones to a viewer
281 // without read access — same 404, no existence disclosure.
282 const viewer = c.get("user");
283 if (!(await canReadPackage(pkg, viewer?.id ?? null))) {
284 return c.json({ error: "not found" }, 404);
285 }
286
260287 // Tarball request?
261288 if (tail) {
262289 const filename = decodeURIComponent(tail);
Modifiedsrc/routes/packages.tsx+20−2View fileUnifiedSplit
3030import type { AuthEnv } from "../middleware/auth";
3131import { getUnreadCount } from "../lib/unread";
3232import { parsePackageName } from "../lib/packages";
33import { isRepoMember } from "../lib/package-access";
3334
3435const ui = new Hono<AuthEnv>();
3536ui.use("*", softAuth);
546547 )
547548 .orderBy(desc(packages.updatedAt));
548549
550 // The private-repo case is already handled upstream by the platform gate
551 // in app.tsx, which this owner/repo path shape does inherit. What it does
552 // not cover is an individually-private package inside a PUBLIC repo, so
553 // drop those unless the viewer is a member rather than a public reader.
554 const visiblePkgs = (await isRepoMember(repoRow.id, user?.id ?? null))
555 ? pkgs
556 : pkgs.filter((p) => p.visibility !== "private");
557
549558 // Fetch the "latest" tag for each package.
550559 const latest = await Promise.all(
551 pkgs.map(async (p) => {
560 visiblePkgs.map(async (p) => {
552561 try {
553562 const [tag] = await db
554563 .select({
572581 }
573582 })
574583 );
575 rows = pkgs.map((p, i) => ({ ...p, latestVersion: latest[i] }));
584 rows = visiblePkgs.map((p, i) => ({ ...p, latestVersion: latest[i] }));
576585 } catch (err) {
577586 console.error("[packages] ui list:", err);
578587 return c.text("Service unavailable", 503);
723732 pkg =
724733 candidates.find((p) => (p.scope ?? null) === (parsed.scope ?? null)) ||
725734 null;
735 // As in the listing above: the repo-private case is covered upstream, but
736 // an individually-private package in a public repo is not. Treat it as
737 // absent for non-members so the 404 below is what they get.
738 if (
739 pkg?.visibility === "private" &&
740 !(await isRepoMember(repoRow.id, user?.id ?? null))
741 ) {
742 pkg = null;
743 }
726744 if (pkg) {
727745 versions = await db
728746 .select()
729747