Public: anyone can read this; the crew can change it.
The forge toolbar in an app's preview is pinned by tests: shown on pages, absent from data, never twice #10716
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/10711-the-forge-toolbar-in-an-app-s-preview-is → main 2 commits fork 1 thread · 1 unresolved
5 files
changed +151
−1
docs/README.md +1
| @@ -27,6 +27,7 @@ platform for organizations. | ||
| 27 | 27 | |
| 28 | 28 | - [](ORIGINS.md): connect accounts on other forges, mirror and write through. |
| 29 | 29 | - [](features/forge-origin.md): connect one forge to another. |
| 30 | - [](features/forge-toolbar.md): the pill in every app and preview. | |
| 30 | 31 | |
| 31 | 32 | ## Operate it |
| 32 | 33 |
docs/features/forge-toolbar.md added +22
| @@ -0,0 +1,22 @@ | ||
| 1 | # The forge toolbar | |
| 2 | ||
| 3 | A small pill the app proxy adds to an app's pages (production and every pull's | |
| 4 | preview), so you always know where you are and can get back to the forge. | |
| 5 | ||
| 6 | - **Where it goes:** into an HTML page the app sent whole and uncompressed, | |
| 7 | once, before the last `</body>` (at the end when there is none). JSON, | |
| 8 | images, scripts, styles, attachments, ranges and `HEAD` pass through byte for | |
| 9 | byte. A page that already carries the tag is not given a second. | |
| 10 | - **What it says:** the app, the environment and, on a preview, a link | |
| 11 | "Preview of pull #N" to the pull's page on the forge; Home goes to the forge. | |
| 12 | - **No layout shift:** it is `position: fixed` inside a closed shadow root and | |
| 13 | adds nothing to the page's flow or style sheets. | |
| 14 | - **Escaping:** app and pull names travel as escaped `data-*` attributes and are | |
| 15 | set as text, never as markup. | |
| 16 | - **Content-Security-Policy:** the script is served from the app's own host | |
| 17 | (`/__forge/toolbar-<hash>.js`), so `script-src 'self'` admits it, and an | |
| 18 | app's nonce is copied onto the tag. A policy that admits neither leaves the | |
| 19 | pill out; the app still works. | |
| 20 | - **Off:** `"toolbar": false` in `forge.json`. | |
| 21 | ||
| 22 | Tests: `test/toolbar-injection.test.ts`, `test/apps-toolbar.test.ts`. |
src/apps/toolbar.ts +14 −1
| @@ -658,7 +658,19 @@ function lastBodyClose(buffer: Buffer): number { | ||
| 658 | 658 | return -1; |
| 659 | 659 | } |
| 660 | 660 | |
| 661 | /** `bytes` with the pill's tag before the last `</body>`, or at the end. */ | |
| 661 | /** The opening of the tag the proxy writes, with the hashed name: what a page that has the toolbar contains. */ | |
| 662 | const TAG_OPEN = Buffer.from(`<script src="${TOOLBAR_ASSET_PATH}"`); | |
| 663 | ||
| 664 | /** | |
| 665 | * Whether a page already carries this toolbar's tag (a page served through two | |
| 666 | * forges, or cached with one). A byte search for the exact tag opening: a page | |
| 667 | * that only quotes the path in its text does not match, and nothing is decoded. | |
| 668 | */ | |
| 669 | export function carriesToolbar(bytes: Uint8Array): boolean { | |
| 670 | return Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength).includes(TAG_OPEN); | |
| 671 | } | |
| 672 | ||
| 673 | /** `bytes` with the pill's tag before the last `</body>`, or at the end; a page that has one is returned as it is. */ | |
| 662 | 674 | export function withToolbar( |
| 663 | 675 | bytes: Uint8Array, |
| 664 | 676 | forgeUrl: string, |
| @@ -666,6 +678,7 @@ export function withToolbar( | ||
| 666 | 678 | proof?: string, |
| 667 | 679 | nonce?: string, |
| 668 | 680 | ): Uint8Array<ArrayBuffer> { |
| 681 | if (carriesToolbar(bytes)) return new Uint8Array(bytes); | |
| 669 | 682 | const tag = Buffer.from(toolbarTag(forgeUrl, where, proof, nonce)); |
| 670 | 683 | const buffer = Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength); |
| 671 | 684 | const at = lastBodyClose(buffer); |
test/apps-toolbar.test.ts +24
| @@ -55,6 +55,10 @@ describe("the forge pill", () => { | ||
| 55 | 55 | }) |
| 56 | 56 | : new Response(HTML, { headers }); |
| 57 | 57 | } |
| 58 | if (path === "/has-tag") | |
| 59 | return new Response(`<html><body>hi<script src="${TOOLBAR_ASSET_PATH}" defer></script></body></html>`, { headers: html }); | |
| 60 | if (path === "/quotes") | |
| 61 | return new Response(`<html><body><code><script src="${TOOLBAR_ASSET_PATH}"></code> src="/__forge/toolbar-x"</body></html>`, { headers: html }); | |
| 58 | 62 | if (path === "/json") return Response.json({ a: 1 }); |
| 59 | 63 | if (path === "/img") return new Response(PNG, { headers: { "content-type": "image/png" } }); |
| 60 | 64 | if (path === "/bare") return new Response("<p>no body tag</p>", { headers: html }); |
| @@ -239,6 +243,26 @@ describe("the forge pill", () => { | ||
| 239 | 243 | expect(await (await at("/notmarkup")).text()).toBe('{"a":1}</body>'); |
| 240 | 244 | }); |
| 241 | 245 | |
| 246 | test("through the proxy: data, ranges and HEAD come back byte for byte; a page with the tag keeps exactly one", async () => { | |
| 247 | expect(await (await at("/json")).text()).toBe('{"a":1}'); | |
| 248 | expect(new Uint8Array(await (await at("/img")).arrayBuffer())).toEqual(PNG); | |
| 249 | const range = await at("/range"); | |
| 250 | expect(await range.text()).toBe(HTML.slice(0, 20)); | |
| 251 | const head = await fetch(`${servers.url}/`, { method: "HEAD", headers: { host: `notes.${DOMAIN}` } }); | |
| 252 | expect(head.headers.get("content-length")).toBe(String(HTML.length)); | |
| 253 | expect(await head.text()).toBe(""); | |
| 254 | const had = await (await at("/has-tag")).text(); | |
| 255 | expect(had.split(TOOLBAR_ASSET_PATH).length - 1).toBe(1); | |
| 256 | expect(had).not.toContain("data-forge"); | |
| 257 | }); | |
| 258 | ||
| 259 | test("a page that only quotes the toolbar's path still gets one tag", async () => { | |
| 260 | const quoted = await (await at("/quotes")).text(); | |
| 261 | expect(quoted.split("<script").length - 1).toBe(1); | |
| 262 | expect(quoted).toContain("data-forge"); | |
| 263 | expect(quoted).toContain(`<script src="${TOOLBAR_ASSET_PATH}">`); | |
| 264 | }); | |
| 265 | ||
| 242 | 266 | test("a ranged answer, a HEAD, a huge page and a foreign charset pass through", async () => { |
| 243 | 267 | const range = await at("/range"); |
| 244 | 268 | expect(range.status).toBe(206); |
test/toolbar-injection.test.ts added +90
| @@ -0,0 +1,90 @@ | ||
| 1 | /** | |
| 2 | * The toolbar's injection rules (#10711): into an HTML page once, never into | |
| 3 | * data, never twice, still right without `</body>`; the markup carries names | |
| 4 | * escaped; the pill overlays the page (fixed) so it shifts nothing; and it | |
| 5 | * names the pull's forge page in plain words. | |
| 6 | */ | |
| 7 | ||
| 8 | import { expect, test } from "bun:test"; | |
| 9 | import { | |
| 10 | carriesToolbar, | |
| 11 | htmlAnswer, | |
| 12 | toolbarJs, | |
| 13 | toolbarTag, | |
| 14 | withToolbar, | |
| 15 | } from "../src/apps/toolbar.ts"; | |
| 16 | ||
| 17 | const FORGE = "https://forge.test"; | |
| 18 | const text = (b: Uint8Array) => new TextDecoder().decode(b); | |
| 19 | const enc = (s: string) => new TextEncoder().encode(s); | |
| 20 | const count = (s: string) => s.split("<script").length - 1; | |
| 21 | ||
| 22 | test("only HTML is a page the toolbar goes into", () => { | |
| 23 | const h = (type: string) => new Headers({ "content-type": type }); | |
| 24 | expect(htmlAnswer(h("text/html; charset=utf-8"), 200)).toBe(true); | |
| 25 | for (const type of ["application/json", "image/png", "text/plain", "text/css", "application/javascript", "image/svg+xml"]) | |
| 26 | expect(htmlAnswer(h(type), 200)).toBe(false); | |
| 27 | expect(htmlAnswer(new Headers(), 200)).toBe(false); | |
| 28 | expect(htmlAnswer(h("text/html"), 204)).toBe(false); | |
| 29 | }); | |
| 30 | ||
| 31 | test("a page gets the tag once, before </body>", () => { | |
| 32 | const out = text(withToolbar(enc("<html><body>hi</body></html>"), FORGE, { app: "notes", env: "production" })); | |
| 33 | expect(count(out)).toBe(1); | |
| 34 | expect(out.indexOf("<script")).toBeLessThan(out.indexOf("</body>")); | |
| 35 | }); | |
| 36 | ||
| 37 | test("a page that already carries the toolbar is not given a second", () => { | |
| 38 | const first = withToolbar(enc("<html><body>hi</body></html>"), FORGE, { app: "notes", env: "production" }); | |
| 39 | expect(carriesToolbar(first)).toBe(true); | |
| 40 | const again = withToolbar(first, FORGE, { app: "notes", env: "production" }); | |
| 41 | expect(text(again)).toBe(text(first)); | |
| 42 | expect(count(text(again))).toBe(1); | |
| 43 | }); | |
| 44 | ||
| 45 | test("a page that only quotes the path is not one that carries the tag", () => { | |
| 46 | const quoting = enc(`<body><code>src="/__forge/toolbar-abc.js"</code></body>`); | |
| 47 | expect(carriesToolbar(quoting)).toBe(false); | |
| 48 | expect(count(text(withToolbar(quoting, FORGE)))).toBe(1); | |
| 49 | }); | |
| 50 | ||
| 51 | test("a page without </body> gets the tag at the end", () => { | |
| 52 | const out = text(withToolbar(enc("<p>bare</p>"), FORGE)); | |
| 53 | expect(out.startsWith("<p>bare</p><script")).toBe(true); | |
| 54 | expect(count(out)).toBe(1); | |
| 55 | }); | |
| 56 | ||
| 57 | test("names are escaped in the tag", () => { | |
| 58 | const tag = toolbarTag(FORGE, { | |
| 59 | app: 'a"><script>x</script>', | |
| 60 | env: "pull-1", | |
| 61 | title: `<img src=x onerror="alert(1)"> & 'q'`, | |
| 62 | }); | |
| 63 | expect(tag).not.toContain("<img"); | |
| 64 | expect(tag).not.toContain('"><script>'); | |
| 65 | expect(tag).toContain("<img"); | |
| 66 | expect(tag).toContain("&"); | |
| 67 | expect(tag).toContain("""); | |
| 68 | expect(count(tag)).toBe(1); | |
| 69 | }); | |
| 70 | ||
| 71 | test("a nonce from the app's own policy rides on the tag; a policy without one adds none", () => { | |
| 72 | expect(toolbarTag(FORGE, undefined, undefined, "abc123")).toContain(' nonce="abc123"'); | |
| 73 | expect(toolbarTag(FORGE)).not.toContain("nonce"); | |
| 74 | // The script is served from the app's own host, so `script-src 'self'` admits it. | |
| 75 | expect(toolbarTag(FORGE)).toContain('src="/__forge/'); | |
| 76 | }); | |
| 77 | ||
| 78 | // A source check only; the layout proof is the proxy tests in apps-toolbar.test.ts (data untouched, one tag). | |
| 79 | test("the pill overlays the page: fixed, out of flow, in a shadow root", () => { | |
| 80 | expect(toolbarJs).toContain("position:fixed"); | |
| 81 | expect(toolbarJs).toContain("attachShadow"); | |
| 82 | // Nothing is added to the app's layout flow or style sheets. | |
| 83 | expect(toolbarJs).not.toContain("document.head"); | |
| 84 | expect(toolbarJs).not.toContain("insertRule"); | |
| 85 | }); | |
| 86 | ||
| 87 | test("on a preview the pill links to the pull's forge page in plain words", () => { | |
| 88 | expect(toolbarJs).toContain('"Preview of pull #"'); | |
| 89 | expect(toolbarJs).toContain('"/pulls/"'); | |
| 90 | }); |
Stringifying the whole page and matching a loose prefix is costly and gives false positives (a page that quotes the path). Use
buffer.includeson the exact tag opening, then test it.