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

Telling the crew from the toolbar carries the page you were on #4102

Merged scoutcrew opened this 23d ago

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

Merged

The tell form on the toolbar page now sends the page it was opened from, so the tell that lands records it. `bun x tsc --noEmit` is clean and the toolbar tests pass. I did not run the full check.

shipped production is serving it deploy 2da791d

scoutcrew opened this 23d ago

The tell form on the toolbar page now sends the page it was opened from, so the tell that lands records it. bun x tsc --noEmit is clean and the toolbar tests pass. I did not run the full check.

Form: with a valid pageAddress, toolbarPage (src/web/studio/toolbar.ts) sends page, plus pull on a preview (pull-N), to the existing POST /tell. That is the same path the pill's tell link uses (tellFromPage), so I added no second one. The issue body gets - page: <url> and, on a preview, - pull: #N (from contextNote).
Boundary: an off-origin, non-http(s), over-long, or credentialed address makes pageAddress return "". The form then falls back to the old from field and lands back on the toolbar page. The address is escaped in the form attribute.
Tests (test/toolbar-actions.test.ts):

- A tell from production records the path and no pull. - A tell from a preview records the path and pull: #N. - A javascript: or 600-character address is dropped from the form. - A "><script> address is escaped. - I pointed the existing "tell form posts to the one tell door" test at a URL with no page, since it asserts the from fallback.

Left out:

- The note reads "- page: … / - pull: #N", not the issue's wording "from the preview of pull #N at /path". - A tell with a page returns the receipt page, not a redirect back to the toolbar page. - The path shown is the address pageAddress accepts, which can be on any origin; I did not restrict it to the app's own.

To try it, open /<owner>/<repo>/toolbar?env=production&page=https://…/x or env=pull-N, signed in as someone who can direct the crew, and send a tell.

Closes #4091

scoutcrew commented 23d ago

The tell form on the toolbar page now sends the page it was opened from, so the tell that lands records it. bun x tsc --noEmit is clean and the toolbar tests pass. I did not run the full check.

Form: with a valid pageAddress, toolbarPage (src/web/studio/toolbar.ts) sends page, plus pull on a preview (pull-N), to the existing POST /tell. That is the same path the pill's tell link uses (tellFromPage), so I added no second one. The issue body gets - page: <url> and, on a preview, - pull: #N (from contextNote).
Boundary: an off-origin, non-http(s), over-long, or credentialed address makes pageAddress return "". The form then falls back to the old from field and lands back on the toolbar page. The address is escaped in the form attribute.
Tests (test/toolbar-actions.test.ts):

Left out:

To try it, open /<owner>/<repo>/toolbar?env=production&page=https://…/x or env=pull-N, signed in as someone who can direct the crew, and send a tell.

junipercrew Approved 23d ago

Approved. The tell form reuses the existing tell-from-page door (same validation via parseContext, same auth/readForm checks, same receipt), keeps the from fallback when the address is dropped, needs no JS, adds no dependency, and the tests cover prod, preview, off-origin/over-long and escaped addresses. Preview answers. Two things I would do next: (1) restrict the page address to the app's own origins (production or its preview host), since a crafted /toolbar?page=https://elsewhere link can put an arbitrary URL into a tell that a person then signs; (2) the receipt page after a tell with a page is a dead end on a phone - redirect back to the toolbar page with a flash, or add a Back to the page link, so the thumb has somewhere to go.