Public: anyone can read this; the crew can change it.
Seen anonymously, a private repository's page and its og card say only the title and 'private' #2450
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/2447-seen-anonymously-a-private-repository-s → main 2 commits fork 2 threads · 2 unresolved
6 files
changed +152
−11
src/web/pages/og.ts +32 −6
| @@ -3,7 +3,8 @@ | ||
| 3 | 3 | * and GET /og/audit.svg. |
| 4 | 4 | * Public by design — it is what a link preview fetches without a session — |
| 5 | 5 | * so it says only what an anonymous reader may already see: a public repo |
| 6 | * (`owner~name`), the public app of one (its slug), or an issue, pull or run | |
| 6 | * (`owner~name`; any other repo, private or not, is a title and the word | |
| 7 | * "private", and a signed-in reader who may read it gets the full card), the public app of one (its slug), or an issue, pull or run | |
| 7 | 8 | * of a public repo (`owner~name~number`, `owner~name~runid`) that an |
| 8 | 9 | * anonymous reader could open (not sealed, runs readable); a public repo's |
| 9 | 10 | * map (`owner~name`) drawn from only the issues and runs it may see; or the |
| @@ -313,16 +314,38 @@ async function auditCardInput(ctx: Ctx): Promise<CardInput> { | ||
| 313 | 314 | return auditInput(rows); |
| 314 | 315 | } |
| 315 | 316 | |
| 317 | /** | |
| 318 | * A repository's card. The public repo, or one the signed-in viewer may read, | |
| 319 | * gets the full card; any other well-formed ref gets a card of its title and | |
| 320 | * the word "private" — the same for a private repo and a name nobody has | |
| 321 | * taken, so the card cannot be used to find out which exist. | |
| 322 | */ | |
| 323 | function repoCard(ctx: Ctx, ref: string): CardInput | undefined { | |
| 324 | const [owner, name, extra] = ref.split("~"); | |
| 325 | if (extra !== undefined || !owner || !name) return undefined; | |
| 326 | if (!isSafeName(owner) || !isSafeName(name)) return undefined; | |
| 327 | const repo = getRepo(ctx.app.db, owner, name); | |
| 328 | if (repo && canRead(ctx.app.db, repo, ctx.auth.user)) | |
| 329 | return repoInput(ctx, repo, "repo"); | |
| 330 | return { kind: "repo", title: `${owner}/${name}`, stat: "private" }; | |
| 331 | } | |
| 332 | ||
| 333 | /** Whether this response is for the signed-in viewer alone, not for a shared cache. */ | |
| 334 | function personal(ctx: Ctx, kind: OgKind, ref: string): boolean { | |
| 335 | if (kind !== "repo" || ctx.auth.user === undefined) return false; | |
| 336 | const [owner, name] = ref.split("~"); | |
| 337 | if (!owner || !name || !isSafeName(owner) || !isSafeName(name)) return false; | |
| 338 | const repo = getRepo(ctx.app.db, owner, name); | |
| 339 | return repo !== undefined && repo.visibility !== "public"; | |
| 340 | } | |
| 341 | ||
| 316 | 342 | async function resolve(ctx: Ctx, kind: OgKind, ref: string): Promise<CardInput | undefined> { |
| 317 | 343 | const db = ctx.app.db; |
| 318 | 344 | if (kind === "issue" || kind === "pull") return threadInput(ctx, kind, ref); |
| 319 | 345 | if (kind === "run") return runInput(ctx, ref); |
| 320 | 346 | if (kind === "map") return mapCardInput(ctx, ref); |
| 321 | 347 | if (kind === "audit") return ref === "" ? auditCardInput(ctx) : undefined; |
| 322 | if (kind === "repo") { | |
| 323 | const repo = twoPart(ctx, ref); | |
| 324 | return repo && repoInput(ctx, repo, "repo"); | |
| 325 | } | |
| 348 | if (kind === "repo") return repoCard(ctx, ref); | |
| 326 | 349 | if (!isSlug(ref)) return undefined; |
| 327 | 350 | const record = getAppBySlug(db, ref); |
| 328 | 351 | if (!record || record.access !== "public") return undefined; |
| @@ -365,7 +388,10 @@ export function ogCard(kind: OgKind, format: "svg" | "png" = "svg") { | ||
| 365 | 388 | const etag = `"${createHash("sha256").update(signature(input)).digest("hex")}${format === "png" ? "-png" : ""}"`; |
| 366 | 389 | const headers = { |
| 367 | 390 | etag, |
| 368 | "cache-control": "public, max-age=300", | |
| 391 | "cache-control": personal(ctx, kind, ctx.params.ref ?? "") | |
| 392 | ? "private, max-age=300" | |
| 393 | : "public, max-age=300", | |
| 394 | ...(kind === "repo" ? { vary: "cookie" } : {}), | |
| 369 | 395 | "content-type": format === "png" ? "image/png" : "image/svg+xml; charset=utf-8", |
| 370 | 396 | }; |
| 371 | 397 | if (ctx.request.headers.get("if-none-match") === etag) |
src/web/pages/repo.ts +2
| @@ -989,6 +989,7 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> { | ||
| 989 | 989 | ctx, |
| 990 | 990 | title: repo.name, |
| 991 | 991 | description: repo.description, |
| 992 | ogImage: `/og/repo/${repo.ownerName}~${repo.name}.png`, | |
| 992 | 993 | feed: { |
| 993 | 994 | href: `${repoBase(repo)}/activity.atom`, |
| 994 | 995 | title: `${repo.ownerName}/${repo.name} activity`, |
| @@ -1036,6 +1037,7 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> { | ||
| 1036 | 1037 | ctx, |
| 1037 | 1038 | title: repo.name, |
| 1038 | 1039 | description: repo.description, |
| 1040 | ogImage: `/og/repo/${repo.ownerName}~${repo.name}.png`, | |
| 1039 | 1041 | feed: { |
| 1040 | 1042 | href: `${repoBase(repo)}/activity.atom`, |
| 1041 | 1043 | title: `${repo.ownerName}/${repo.name} activity`, |
test/og-png.test.ts +2 −2
| @@ -72,8 +72,8 @@ describe("og png", () => { | ||
| 72 | 72 | expect(again.status).toBe(304); |
| 73 | 73 | }); |
| 74 | 74 | |
| 75 | test("private and malformed refs are the same 404", async () => { | |
| 76 | expect((await fetch(`${base}/og/repo/alice~hidden.png`)).status).toBe(404); | |
| 75 | test("malformed refs are 404; a private repo's card is only its title and 'private'", async () => { | |
| 76 | expect((await fetch(`${base}/og/repo/alice~hidden.png`)).status).toBe(200); | |
| 77 | 77 | expect((await fetch(`${base}/og/repo/..%2F..~x.png`)).status).toBe(404); |
The PNG private-card test checks only status 200. Also assert the content-type is image/png and that the body matches the card for an unclaimed name. |
||
| 78 | 78 | }); |
| 79 | 79 |
test/og-private.test.ts added +100
| @@ -0,0 +1,100 @@ | ||
| 1 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 2 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { createApp as createForge, type App } from "../src/app.ts"; | |
| 6 | import { createRepo } from "../src/actions/repos.ts"; | |
| 7 | import { createIssue } from "../src/core/issues.ts"; | |
| 8 | import { getRepo } from "../src/core/repos.ts"; | |
| 9 | import { createUser } from "../src/core/users.ts"; | |
| 10 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 11 | ||
| 12 | const SECRET = "zebra-quartz-plan"; | |
| 13 | const PASSWORD = "a-good-passphrase"; | |
| 14 | ||
| 15 | describe("a private repository's page and card, seen anonymously", () => { | |
| 16 | let dataDir: string; | |
| 17 | let app: App; | |
| 18 | let servers: Servers; | |
| 19 | let cookie = ""; | |
| 20 | ||
| 21 | beforeAll(async () => { | |
| 22 | dataDir = await mkdtemp(join(tmpdir(), "forge-og-private-")); | |
| 23 | app = createForge({ dataDir, port: 0 }); | |
| 24 | servers = await serve(app); | |
| 25 | const u = await createUser(app.db, { | |
| 26 | trusted: true, | |
| 27 | username: "alice", | |
| 28 | email: "[email protected]", | |
| 29 | password: PASSWORD, | |
| 30 | adminIfFirst: true, | |
| 31 | }); | |
| 32 | if (!u.ok) throw new Error(u.error); | |
| 33 | const made = await createRepo(app, u.value, { | |
| 34 | name: "vault", | |
| 35 | visibility: "private", | |
| 36 | description: `Holds the ${SECRET}`, | |
| 37 | }); | |
| 38 | if (!made.ok) throw new Error(String(made.error)); | |
| 39 | const issue = createIssue(app.db, { | |
| 40 | repoId: getRepo(app.db, "alice", "vault")!.id, | |
| 41 | authorId: u.value.id, | |
| 42 | title: "An issue", | |
| 43 | kind: "issue", | |
| 44 | }); | |
| 45 | if (!issue.ok) throw new Error(String(issue.error)); | |
| 46 | const res = await fetch(`${servers.url}/signin`, { | |
| 47 | method: "POST", | |
| 48 | body: new URLSearchParams({ username: "alice", password: PASSWORD }), | |
| 49 | redirect: "manual", | |
| 50 | }); | |
| 51 | cookie = (res.headers.get("set-cookie") ?? "").split(";")[0]!; | |
| 52 | }); | |
| 53 | ||
| 54 | afterAll(async () => { | |
| 55 | await servers.stop(); | |
| 56 | await rm(dataDir, { recursive: true, force: true }); | |
| 57 | }); | |
| 58 | ||
| 59 | const get = (path: string, headers: Record<string, string> = {}) => | |
| 60 | fetch(servers.url + path, { headers, redirect: "manual" }); | |
| 61 | ||
| 62 | test("the page answered anonymously leaks nothing in its head", async () => { | |
| 63 | const res = await get("/alice/vault"); | |
| 64 | const head = (await res.text()).split("</head>")[0]!; | |
| 65 | expect(head).not.toContain(SECRET); | |
| 66 | expect(head).toContain('property="og:title" content="Not found"'); | |
| 67 | expect(head).not.toContain("/og/repo/"); | |
| 68 | }); | |
| 69 | ||
| 70 | test("the card says a title and 'private' and nothing else", async () => { | |
| 71 | const res = await get("/og/repo/alice~vault.svg"); | |
| 72 | expect(res.status).toBe(200); | |
| 73 | expect(res.headers.get("cache-control")).toBe("public, max-age=300"); | |
| 74 | const svg = await res.text(); | |
| 75 | expect(svg).toContain("alice/vault"); | |
| 76 | expect(svg).toContain("private"); | |
| 77 | expect(svg).not.toContain(SECRET); | |
| 78 | expect(svg).not.toMatch(/issues|pulls|ready now|crew/); | |
| 79 | }); | |
| 80 | ||
| 81 | test("a name nobody has taken gets a card of the same shape", async () => { | |
| 82 | const real = await (await get("/og/repo/alice~vault.svg")).text(); | |
| 83 | const none = await (await get("/og/repo/alice~nothing.svg")).text(); | |
| 84 | expect(none).toBe(real.replaceAll("alice/vault", "alice/nothing")); | |
| 85 | }); | |
| 86 | ||
| 87 | test("the owner, signed in, sees the full card, for them alone", async () => { | |
| 88 | const res = await get("/og/repo/alice~vault.svg", { cookie }); | |
| 89 | expect(res.status).toBe(200); | |
| 90 | expect(res.headers.get("cache-control")).toBe("private, max-age=300"); | |
| 91 | const svg = await res.text(); | |
| 92 | expect(svg).toContain(SECRET); | |
| 93 | expect(svg).toContain("1 issues"); | |
| 94 | }); | |
| 95 | ||
| 96 | test("the owner's page points at the card", async () => { | |
| 97 | const html = await (await get("/alice/vault", { cookie })).text(); | |
| 98 | expect(html).toContain("/og/repo/alice~vault.png"); | |
| 99 | }); | |
| 100 | }); |
test/og-route.test.ts +13 −2
| @@ -141,7 +141,11 @@ describe("og cards", () => { | ||
| 141 | 141 | }); |
| 142 | 142 | |
| 143 | 143 | test("a private repo is 404", async () => { |
| 144 | expect((await get(`/og/${kind}/${private_}.svg`)).status).toBe(404); | |
| 144 | // An app's card is refused; a repo's is only its title and "private" | |
| 145 | // (test/og-private.test.ts). | |
| 146 | expect((await get(`/og/${kind}/${private_}.svg`)).status).toBe( | |
| 147 | kind === "app" ? 404 : 200, | |
| 148 | ); | |
| 145 | 149 | }); |
| 146 | 150 | |
| 147 | 151 | test("a private app on a public repo is 404", async () => { |
| @@ -153,7 +157,14 @@ describe("og cards", () => { | ||
| 153 | 157 | }); |
| 154 | 158 | |
| 155 | 159 | test("an unknown or malformed ref is 404", async () => { |
| 156 | for (const ref of ["nobody~nothing", "nothing", "a~b~c", "..%2Fx", "A_B"]) | |
| 160 | // A well-formed repo ref that is not readable is the private card. | |
| 161 | for (const ref of [ | |
| 162 | ...(kind === "app" ? ["nobody~nothing"] : []), | |
| 163 | "nothing", | |
| 164 | "a~b~c", | |
| 165 | "..%2Fx", | |
| 166 | "A_B", | |
| 167 | ]) | |
| 157 | 168 | expect((await get(`/og/${kind}/${ref}.svg`)).status).toBe(404); |
| 158 | 169 | }); |
| 159 | 170 | }); |
test/standards.test.ts +3 −1
| @@ -172,7 +172,9 @@ describe("what the open web expects", () => { | ||
| 172 | 172 | expect(html).toContain( |
| 173 | 173 | 'rel="apple-touch-icon" href="/icons/forge-180.png"', |
| 174 | 174 | ); |
| 175 | expect(html).toContain('name="twitter:card" content="summary"'); | |
| 175 | // A repository page carries its own card (/og/repo), not the icon. | |
| 176 | expect(html).toContain(`content="${base}/og/repo/alice~open.png"`); | |
| 177 | expect(html).toContain('name="twitter:card" content="summary_large_image"'); | |
| 176 | 178 | expect(html).toContain("viewport-fit=cover"); |
| 177 | 179 | }); |
| 178 | 180 |
new above
offline
This comment line is far past the width of the rest of the header. Re-wrap it.