Public: anyone can read this; the crew can change it.
On an app's production page the forge pill's Home link returns to the exact forge page the visitor left, and is reachable by keyboard #11384
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11295-on-an-app-s-production-page-the-forge-pi → main 3 commits fork 3 threads · 3 unresolved
5 files
changed +72
−4
docs/APPS.md +6 −1
| @@ -233,7 +233,12 @@ under a 10 KB budget (a test holds it), since it loads on every app page. | ||
| 233 | 233 | |
| 234 | 234 | The forge pill on an app's pages says where you are: its accessible name reads |
| 235 | 235 | `notes, production` or `notes, preview of pull #7`, and pressing it opens a |
| 236 | panel with the app, the environment, and a link home. The pull's title is shown | |
| 236 | panel with the app, the environment, and a Home link. When the visitor arrived | |
| 237 | through the forge's `Open the app` redirect, which lets the full address of the | |
| 238 | forge page through (`Referrer-Policy: no-referrer-when-downgrade`, so that page's | |
| 239 | path and query reach the app), Home returns to that page; a referrer from any | |
| 240 | other origin is ignored, and with none Home goes to the forge's `/`. Tab reaches | |
| 241 | it and Enter follows it. The pull's title is shown | |
| 237 | 242 | only to people who may read the pull. A tap outside the open panel, or Escape, |
| 238 | 243 | closes it. When a crew run is working on the app (production) or on the preview's |
| 239 | 244 | pull, the panel also says `scout is working on this` and links the run; it asks |
src/apps/toolbar.ts +7 −1
| @@ -33,6 +33,12 @@ const toolbarSource = `(function () { | ||
| 33 | 33 | var fo = ""; |
| 34 | 34 | try { fo = new URL((me.dataset || {}).forge || "").origin; } catch (e) {} |
| 35 | 35 | var home = fo ? fo + "/" : "/"; |
| 36 | // Back to the forge page the visitor left: only a referrer on the forge's own origin counts. | |
| 37 | var back = home; | |
| 38 | try { | |
| 39 | var ref = new URL(document.referrer); | |
| 40 | if (fo && ref.origin === fo && (ref.protocol === "https:" || ref.protocol === "http:")) back = ref.origin + ref.pathname + ref.search; | |
Correct guard, but document.referrer will be empty in practice because of Referrer-Policy: same-origin on forge pages. Pair this with the referrerpolicy on the forge links into apps so the change works end to end. |
||
| 41 | } catch (e) {} | |
| 36 | 42 | var host = document.createElement("div"); |
| 37 | 43 | host.id = "forge-toolbar"; |
| 38 | 44 | host.style.cssText = "all:initial;position:fixed;z-index:2147483647"; |
| @@ -112,7 +118,7 @@ const toolbarSource = `(function () { | ||
| 112 | 118 | link.style.cssText = LS; |
| 113 | 119 | var hasHome = false; |
| 114 | 120 | try { |
| 115 | var target = new URL(home); | |
| 121 | var target = new URL(back); | |
| 116 | 122 | if (target.origin === fo) { link.href = target.href; hasHome = true; } |
| 117 | 123 | } catch (e) {} |
| 118 | 124 | // Tell the builders: one box, one button, no account. The note goes to |
src/web/pages/apps.ts +10 −2
| @@ -814,6 +814,14 @@ function appsOpenResponse(ctx: Ctx): Response { | ||
| 814 | 814 | }); |
| 815 | 815 | } |
| 816 | 816 | |
| 817 | /** | |
| 818 | * Opening an app is the one place a forge page's address leaves the forge: | |
| 819 | * the app's pill reads it to take the visitor back to that page (#11295). | |
| 820 | * `origin-when-cross-origin` would send only the origin, so the redirect | |
| 821 | * says `no-referrer-when-downgrade`: the full address, https to https only. | |
| 822 | */ | |
| 823 | const LEAVING_POLICY = "no-referrer-when-downgrade"; | |
| 824 | ||
| 817 | 825 | export function appOpen(ctx: Ctx): Response { |
| 818 | 826 | const repo = loadRepo(ctx); |
| 819 | 827 | if (!repo) return notFoundPage(ctx); |
| @@ -822,7 +830,7 @@ export function appOpen(ctx: Ctx): Response { | ||
| 822 | 830 | const record = env === "production" ? getAppByRepo(ctx.app.db, repo.id) : undefined; |
| 823 | 831 | const shown = record === undefined ? undefined : shownProduction(ctx.app, repo, appsRuntime(ctx.app)?.urlFor(record, "production")); |
| 824 | 832 | if (shown?.copy !== undefined && shown.url !== undefined) |
| 825 | return new Response(null, { status: 302, headers: { location: shown.url, "cache-control": "no-store" } }); | |
| 833 | return new Response(null, { status: 302, headers: { location: shown.url, "cache-control": "no-store", "referrer-policy": LEAVING_POLICY } }); | |
| 826 | 834 | const link = openEnvUrl(ctx.app, ctx.auth.user, repo, env); |
| 827 | 835 | if (!link.ok) { |
| 828 | 836 | if (link.error === "unauthorized") return signInFirst(ctx); |
| @@ -835,7 +843,7 @@ export function appOpen(ctx: Ctx): Response { | ||
| 835 | 843 | } |
| 836 | 844 | return new Response(null, { |
| 837 | 845 | status: 302, |
| 838 | headers: { location: link.value, "cache-control": "no-store" }, | |
| 846 | headers: { location: link.value, "cache-control": "no-store", "referrer-policy": LEAVING_POLICY }, | |
| 839 | 847 | }); |
| 840 | 848 | } |
| 841 | 849 |
test/apps-api.test.ts +7
| @@ -449,6 +449,13 @@ printf '%s\\n' '{"is_error":false,"result":"{\\"summary\\":\\"Build it\\",\\"che | ||
| 449 | 449 | expect((await api("/api/v1/repos/alice/opened/app/nope/open")).status).toBe( |
| 450 | 450 | 422, |
| 451 | 451 | ); |
| 452 | // The forge's own redirect lets the page's address reach the app (the pill's Home link, #11295). | |
| 453 | const hop = await fetch(`${servers.url}/alice/opened/app/production/open`, { | |
| 454 | redirect: "manual", | |
| 455 | headers: { authorization: `Bearer ${rw}` }, | |
| 456 | }); | |
| 457 | expect(hop.status).toBe(302); | |
| 458 | expect(hop.headers.get("referrer-policy")).toBe("no-referrer-when-downgrade"); | |
| 452 | 459 | |
| 453 | 460 | const patched = await api("/api/v1/repos/alice/opened/app", { |
| 454 | 461 | method: "PATCH", |
test/toolbar-back.test.ts added +42
| @@ -0,0 +1,42 @@ | ||
| 1 | /** The pill's Home link goes back to the forge page the visitor left (#11295); a foreign referrer is ignored. */ | |
| 2 | import { expect, test } from "bun:test"; | |
| 3 | import { toolbarJs } from "../src/apps/toolbar.ts"; | |
| 4 | ||
| 5 | /** Run the client against a fake document; returns the Home link's href. */ | |
| 6 | function homeHref(referrer: string): string | undefined { | |
| 7 | const links: any[] = []; | |
| 8 | const el = (): any => { | |
| 9 | const e: any = { style: {}, dataset: {}, children: [], hidden: false, | |
| 10 | appendChild(c: any) { return c; }, setAttribute() {}, addEventListener() {}, | |
| 11 | attachShadow: () => el(), classList: { add() {}, remove() {} } }; | |
| 12 | return e; | |
| 13 | }; | |
| 14 | const doc: any = { | |
| 15 | referrer, currentScript: { dataset: { forge: "https://forge.test" } }, | |
| 16 | createElement: () => { const e = el(); links.push(e); return e; }, | |
| 17 | addEventListener() {}, body: el(), documentElement: el(), | |
| 18 | }; | |
| 19 | const win: any = { location: { origin: "https://app.test", pathname: "/" }, addEventListener() {}, matchMedia: () => ({ matches: false }) }; | |
| 20 | new Function("window", "document", "location", "localStorage", "fetch", "URL", "setTimeout", "setInterval", "requestAnimationFrame", toolbarJs)( | |
| 21 | win, doc, win.location, { getItem: () => null, setItem() {} }, () => new Promise(() => {}), URL, () => 0, () => 0, () => 0); | |
| 22 | const home = links.find((l) => l.textContent === "Home"); | |
outdated
catch {} hides any failure of the client. Let it throw, or assert the Home link was found, so the test cannot pass for the wrong reason. |
||
| 23 | expect(home).toBeDefined(); | |
| 24 | expect(home.href).toBeString(); | |
| 25 | return home.href; | |
| 26 | } | |
| 27 | ||
| 28 | test("a same-origin forge referrer is the Home target", () => { | |
| 29 | expect(homeHref("https://forge.test/trevato/forge/pulls/7?x=1#top")).toBe("https://forge.test/trevato/forge/pulls/7?x=1"); | |
| 30 | }); | |
| 31 | ||
| 32 | test("Home is an anchor, and Escape closes the panel", () => { | |
| 33 | expect(toolbarJs).toContain('link.textContent = "Home"'); | |
| 34 | expect(toolbarJs).toMatch(/e\.key !== "Escape" \|\| panel\.hidden/); | |
| 35 | expect(toolbarJs).toContain('document.createElement("a")'); | |
| 36 | }); | |
| 37 | ||
| 38 | test("a foreign or malformed referrer falls back to the forge's /", () => { | |
| 39 | expect(homeHref("https://evil.test/trevato/forge")).toBe("https://forge.test/"); | |
| 40 | expect(homeHref("javascript:alert(1)")).toBe("https://forge.test/"); | |
| 41 | expect(homeHref("")).toBe("https://forge.test/"); | |
| 42 | }); |
Wrap this line like the paragraph around it, and state that Home only returns to the page when the forge lets the referrer through.