Public: anyone can read this; the crew can change it.
Signed out, the toolbar page of a public app offers "Report a problem, no account needed" beside Sign in #6294
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6287-signed-out-the-toolbar-page-of-a-public → main 1 commit fork 2 threads · 2 unresolved
3 files
changed +32
−4
src/web/pages/reports.ts +3 −1
| @@ -38,6 +38,7 @@ import { | ||
| 38 | 38 | signInFirst, |
| 39 | 39 | } from "../chrome.ts"; |
| 40 | 40 | import { field, readForm } from "../form.ts"; |
| 41 | import { pageAddress } from "../studio/toolbar.ts"; | |
| 41 | 42 | import { html, when, type Html } from "../html.ts"; |
| 42 | 43 | import { flashTo, page } from "../layout.ts"; |
| 43 | 44 | import { btn, card, empty, field as formField, time } from "../ui.ts"; |
| @@ -83,6 +84,7 @@ function reportForm( | ||
| 83 | 84 | something is wrong. You need no account. A person reads it first; |
| 84 | 85 | nothing acts on it before they do. |
| 85 | 86 | </p> |
| 87 | ${when(values.page !== "", () => html`<p class="small mono">About this page: ${values.page}</p>`)} | |
| 86 | 88 | <form class="mt" method="post" action="${base}/report"> |
| 87 | 89 | ${formField({ |
| 88 | 90 | label: "What is wrong", |
| @@ -152,7 +154,7 @@ function reportBody(ctx: Ctx, repo: Repo): Response { | ||
| 152 | 154 | return reportForm(ctx, repo, { |
| 153 | 155 | title: "", |
| 154 | 156 | body: "", |
| 155 | page: ctx.url.searchParams.get("from") ?? "", | |
| 157 | page: pageAddress(ctx.url.searchParams.get("from") ?? ""), | |
| 156 | 158 | }); |
| 157 | 159 | } |
| 158 | 160 |
src/web/studio/toolbar.ts +4 −2
| @@ -23,7 +23,7 @@ import { NOTE_TOO_LONG } from "../../apps/note_words.ts"; | ||
| 23 | 23 | import { getAppByRepo } from "../../core/apps.ts"; |
| 24 | 24 | import { getIssue, MAX_BODY } from "../../core/issues.ts"; |
| 25 | 25 | import { diffBetween, getPull, pullDir, type Pull } from "../../core/pulls.ts"; |
| 26 | import { canWrite, type Repo } from "../../core/repos.ts"; | |
| 26 | import { canRead, canWrite, type Repo } from "../../core/repos.ts"; | |
| 27 | 27 | import { checkSummary, listRuns } from "../../core/runs.ts"; |
| 28 | 28 | import { mayView, mayViewId } from "../../core/sealed.ts"; |
| 29 | 29 | import { mayDirectCrew } from "../../crew/direct.ts"; |
| @@ -178,10 +178,12 @@ export async function toolbarPage(ctx: Ctx): Promise<Response> { | ||
| 178 | 178 | const rendering = performance.now(); |
| 179 | 179 | if (!user) { |
| 180 | 180 | const next = encodeURIComponent(toolbarPath(base, where.env, address)); |
| 181 | const open = canRead(db, repo, undefined); | |
| 182 | const report = `${base}/report${address === "" ? "" : `?from=${encodeURIComponent(address)}`}`; | |
| 181 | 183 | const response = page({ |
| 182 | 184 | ctx, |
| 183 | 185 | title: `${repo.name} · forge`, |
| 184 | body: html`<div class="narrow-wide">${pageHead(here)}${change}${liveNote}<div class="form-actions">${btn("Sign in to comment", { href: `/signin?next=${next}`, variant: "primary" })}${home}</div></div>`, | |
| 186 | body: html`<div class="narrow-wide">${pageHead(here)}${change}${liveNote}<div class="form-actions">${btn("Sign in to comment", { href: `/signin?next=${next}`, variant: "primary" })}${when(open, () => btn("Report a problem, no account needed", { href: report }))}${home}</div></div>`, | |
| 185 | 187 | }); |
| 186 | 188 | notePhase(request, "render", performance.now() - rendering); |
| 187 | 189 | return response; |
test/toolbar-signed-out.test.ts +25 −1
| @@ -16,6 +16,7 @@ quietLogs(); | ||
| 16 | 16 | let root = ""; |
| 17 | 17 | let app: App; |
| 18 | 18 | let servers: Servers; |
| 19 | let owner: never; | |
|
||
| 19 | 20 | |
| 20 | 21 | beforeAll(async () => { |
| 21 | 22 | root = mkdtempSync(join(tmpdir(), "forge-toolbar-out-")); |
| @@ -30,7 +31,8 @@ beforeAll(async () => { | ||
| 30 | 31 | }); |
| 31 | 32 | if (!u.ok) throw new Error(u.error); |
| 32 | 33 | proveAll(app); |
| 33 | const made = await newApp(app, { id: u.value.id, username: "alice" } as never, { name: "forge", description: "x", visibility: "public" }); | |
| 34 | owner = { id: u.value.id, username: "alice" } as never; | |
| 35 | const made = await newApp(app, owner, { name: "forge", description: "x", visibility: "public" }); | |
| 34 | 36 | if (!made.ok) throw new Error(made.error); |
| 35 | 37 | servers = await serve(app); |
| 36 | 38 | }); |
| @@ -51,6 +53,28 @@ test("signed out, the page offers sign-in that returns to it", async () => { | ||
| 51 | 53 | expect(safeNext(decodeURIComponent(next))).toBe(decodeURIComponent(next)); |
| 52 | 54 | }); |
| 53 | 55 | |
| 56 | test("signed out, a public app's toolbar offers the anonymous report, carrying the page", async () => { | |
| 57 | const addr = "https://x.test/a"; | |
| 58 | const res = await fetch(`${servers.url}/alice/forge/toolbar?env=production&page=${encodeURIComponent(addr + "?t=secret")}`); | |
| 59 | const body = await res.text(); | |
| 60 | const href = `/alice/forge/report?from=${encodeURIComponent(addr)}`; | |
| 61 | expect(body).toContain(`href="${href}"`); | |
| 62 | expect(body).toContain("no account needed"); | |
| 63 | expect(body).not.toContain("secret"); | |
| 64 | const form = await (await fetch(servers.url + href)).text(); | |
| 65 | expect(form).toContain("About this page: https://x.test/a"); | |
| 66 | const evil = await (await fetch(`${servers.url}/alice/forge/report?from=${encodeURIComponent("javascript:alert(1)")}`)).text(); | |
| 67 | expect(evil).not.toContain("About this page"); | |
| 68 | }); | |
| 69 | ||
| 70 | test("a private app's toolbar offers no report link to a guest", async () => { | |
| 71 | const made = await newApp(app, owner, { name: "hush", description: "x", visibility: "private" }); | |
| 72 | if (!made.ok) throw new Error(made.error); | |
| 73 | const res = await fetch(`${servers.url}/alice/hush/toolbar?env=production`); | |
| 74 | expect(res.status).toBe(404); | |
| 75 | expect(await res.text()).not.toContain("/alice/hush/report"); | |
| 76 | }); | |
| 77 | ||
| 54 | 78 | test("a crafted next to another host is refused", () => { |
| 55 | 79 | expect(safeNext("//evil.example/x")).toBeUndefined(); |
| 56 | 80 | expect(safeNext("https://evil.example/")).toBeUndefined(); |
new above
offline
A page module importing from studio/toolbar.ts couples two areas for one helper; extract pageAddress to its own module and keep imports sorted.