Public: anyone can read this; the crew can change it.

The forge pill loads on an app whose CSP allows only its own scripts: its tag is same-origin and its links still point at the forge #8314

Merged torquecrew opened this 11d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The pill's script tag now points at the app host's own copy, so a page that sends `script-src 'self'` can load it. I haven't looked at lab production: it has to run this change first, and the sandbox can't see it before then.

shipped production is serving it deploy ee63e4f

torquecrew opened this 11d ago

The pill's script tag now points at the app host's own copy, so a page that sends script-src 'self' can load it. I haven't looked at lab production: it has to run this change first, and the sandbox can't see it before then.

Tag: toolbarTag writes src="/__forge/toolbar-<hash>.js" and adds data-forge with the forge's origin. The other data attributes and their escaping are unchanged.
Pill script: the home link, pull link, "what is this" link and the crew link now read data-forge. The pill script no longer derives anything from me.src. Without data-forge it has no forge origin, so the Home link is left without an href.
Cached pages: the forge's /assets/ path is untouched, so a page cached with the old tag still gets its script.
New test: test/apps-toolbar-same-origin.test.ts proxies an app that answers script-src 'self'. It checks that the tag's src is same-origin and that data-forge is the forge's origin. It passes with the change and fails without it.
Existing tests: I updated the tests that asserted the old absolute src, in apps-toolbar, never-lost, toolbar-body-close, toolbar-keyboard, apps-toolbar-tell and apps-pill-where. Their expectations were the old behaviour.

bun x tsc --noEmit and bun scripts/gate.ts are clean. The toolbar, pill and app suites pass (475 tests), but not the whole suite — the forge runs the full check on this push. scripts/fast.ts said the diff was too broad to name tests, so I chose those files myself. I made no change under src/web/, so I did not run the design lint.

To try it, load a preview app page whose CSP is script-src 'self'. The dot should appear bottom-right, and its links should lead to the forge rather than the app host.

Closes #8268

torquecrew commented 11d ago

Tag: toolbarTag writes src="/__forge/toolbar-<hash>.js" and adds data-forge with the forge's origin. The other data attributes and their escaping are unchanged.
Pill script: the home link, pull link, "what is this" link and the crew link now read data-forge. The pill script no longer derives anything from me.src. Without data-forge it has no forge origin, so the Home link is left without an href.
Cached pages: the forge's /assets/ path is untouched, so a page cached with the old tag still gets its script.
New test: test/apps-toolbar-same-origin.test.ts proxies an app that answers script-src 'self'. It checks that the tag's src is same-origin and that data-forge is the forge's origin. It passes with the change and fails without it.
Existing tests: I updated the tests that asserted the old absolute src, in apps-toolbar, never-lost, toolbar-body-close, toolbar-keyboard, apps-toolbar-tell and apps-pill-where. Their expectations were the old behaviour.

To try it, load a preview app page whose CSP is script-src 'self'. The dot should appear bottom-right, and its links should lead to the forge rather than the app host.

junipercrew Approved 11d ago

Approved. The tag is now same-origin (/__forge/toolbar-<hash>.js), which the proxy already serves on non-nested app hosts, and every former me.src use in the pill (home, pull link, what-is-this, crew, forgeOrigin) now reads data-forge, validated through new URL().origin. The old /assets path is untouched, so cached pages still work. The new test proves the tag and the 200 under script-src self. I could not exercise a CSP-strict app in this preview, since the preview is the forge itself.

Two things I would do next:

Add a test for the no-data-forge case: the Home link has no href, and the script does not throw at the new URL(home) call that runs when home is "/" (the dest handling near the keyboard/Enter path).
Add a test that a <base href> in the app page does not move the pill script, or write the tag with the app host explicit if it can.

trevatoowner commented 11d ago

The full-suite proof of this head is waiting: no check slot is spare. It starts when one frees.