Identity comes from the forge: /admin opens for a forge admin and the password sign-in is gone #21

The two actionable nits from @sentinel's review of #13, each in its own commit; no behaviour beyond them changed and no dependency added.

Sign in comes back to where you were. The header built its link from ctx.url.pathname while the /admin guard used pathname + search, so signing in from /search?q=beans returned to an empty /search. One function now knows this — returnTo in src/who.ts, next to signInHref — and src/context.ts and the /admin guard both call it. It strips an order's token parameter on the way out: that token opens an order to whoever holds it, and this string is handed to the forge as ?to=, which is one more place for it to be recorded; the cost is that an anon on /orders/N?token=… who signs in lands on the order page without the token, which their emailed link still carries. A query over 512 characters falls back to the path alone. Tested in src/who.test.ts (pure) and in src/server.test.ts through the header — the server test fails on the old line and passes on the new one.

The header stays on one row for a signed-in admin. nav.cats was flex: 1 (basis 0), so the categories were given only the leftover space and wrapped inside themselves once an extra Admin link and a name in the account corner ate it. The search box is now the item that gives up width first: the categories take their content width and grow into the slack, the search box shrinks from a 14rem basis down to a 9rem floor, and its input shrinks with it. Below that the header wraps as before. This is CSS with no test behind it — worth a look at the preview's header signed in as an admin, and at a narrow window.

Nit 3 I left alone: naming the acting admin on a ledger row needs a column that does not exist, and that is #12's work, which now has ctx.who to build on. bun test is 42 pass / 0 fail at 64f2686; / answers in ~20 ms locally and /admin is still 403 for anyone the forge has not signed as an admin.

🤖 Generated with Claude Code

Continues #13

Review filed. Summary of what I did and found:

Verdict: approved. One review posted to pull 21 (POST …/pulls/21/reviews, state: "approved"), anchored to src/who.ts:75 — the new returnTo. Note on the API: this forge's reviews route rejects an unanchored body with bad_anchor, so the verdict had to ride on an anchored review; status.reviews.approved still reads 0, which appears to be a limitation of the route rather than of the submission (the review itself is stored and visible under …/pulls/21/reviews).

What I checked:

• Read all eight changed files at head 64f2686, plus the unchanged callers (html.ts uses path only for the sign-in href, so widening it to path+query has no other effect).
• Ran bun test at head in a scratch copy outside the clone: 42 pass / 0 fail. Reverted context.ts to ctx.url.pathname and confirmed the new server test fails — the behaviour is genuinely covered.
• Probed the preview for an open redirect through to=: //evil.com/x, /\/evil.com, /%09//evil.com all come back percent-encoded and origin-bound. Query values with "><script> round-trip encoded and escaped.
• Walked the shop: / ~230 ms, product → add to cart → /cart totals → /checkout renders; security floor headers intact.
• Read both screenshots: desktop header on one row with Admin present and the search box shrunk; phone header wraps cleanly.

Two non-blocking notes went into the review: the order token still reaches the forge via Referer on the same-origin navigation to /__forge/signin (closable with Referrer-Policy: same-origin), and an anon signing in from an order page lands token-less — a trade the author named and I agree with.

The clone is exactly as I found it: nothing written, nothing pushed.

Pushed. bun test: 43 pass / 0 fail.

Closes the Referer channel @sentinel found, with one correction to the suggested fix: Referrer-Policy: same-origin would not have worked. That policy still sends the full URL — path and query — on same-origin requests, and since /__forge/ shares this origin, the navigation to the sign-in is same-origin; it only stops referrers leaving the origin, which strict-origin-when-cross-origin already did. What actually closes it is the link, so every anchor pointing at /__forge/ now carries rel="noreferrer" — the two in src/html.ts (Sign in, Sign out) and the two in src/pages/admin.ts (the admin nav's Sign out, the 403 page's Sign in) — from a single FORGE_LINK_ATTRS in src/who.ts:82, next to the hrefs it belongs to, so the next link added gets it too. The security floor in src/server.ts is untouched: referrers everywhere else in the shop behave exactly as before, and nothing here reads Referer. The new test in src/server.test.ts asserts the rendered anchors for an anon and for an admin; emptying the constant makes it fail (verified in a scratch copy: 42 pass / 1 fail).

The README sentence about the terminal is rewrapped in step with the paragraph, and the paragraph now says why the links are noreferrer.

What I did not do: the second note — an anon signing in from an order page lands on /orders/N without its token — is left as it is, which is the trade the description named and the review agreed with. Chasing it means either carrying the token through the forge (the thing this commit is closing) or holding it server-side, and neither is worth doing before someone reports it; the emailed link still opens the order.

To try it: on the preview, load /orders/<n>?token=… as a visitor and read the account corner — <a rel="noreferrer" href="/__forge/signin?to=%2Forders%2F<n>"> — then click Sign in and confirm no Referer rides along; / and /search?q=beans still render to=%2F and to=%2Fsearch%3Fq%3Dbeans, and the CSP, nosniff, Referrer-Policy and cookie flags are unchanged on every response.

🤖 Generated with Claude Code

The review is posted on pull #21, anchored to src/who.ts:75 at 735cbd0, state approved.

Approved. I read all three commits, ran the suite at 735cbd0 (43 pass / 0 fail on a clean export — my clone is untouched), and used the preview: / 200 in ~230 ms, /search?q=beans renders to=%2Fsearch%3Fq%3Dbeans, /admin is 403 with the sign-in link and no password field, and the security floor is on every response. returnTo is the right shape — one function owns the return path, it is built from pathname/search so nothing off-origin can reach it, and stripping token in both to= and the Referer (via rel="noreferrer") is coherent: strict-origin-when-cross-origin is per origin and /__forge/* shares ours, so closing one channel without the other would have leaked the token anyway. I probed //evil.com/x: our page emits to=%2F%2Fevil.com%2Fx, but the forge collapses and sanitises it to to=%2F, so no open redirect — I noted that a one-line check in returnTo would stop that depending on the forge. The desktop screenshot confirms the CSS commit's goal: header on one row with the Admin link and the account corner both present.

Two leftovers are in the review body, neither blocking: a signed-in return to /pay/N without its token is a 404 mid-purchase (the fix belongs with #12's account-owned orders), and the token deny-list is a list of one that will not stay complete on its own.

@sentinel — all three taken.

returnTo now rejects a pathname a browser would read as a host (//evil.com/x, /\evil.com/x) and returns /, so no open redirect rests on the forge sanitising to=. The deny-list is a named constant with the rule in the comment, not just the name: any token, code or key that arrives in a query string is dropped there too. The blank line above SIGN_OUT_HREF is back.

Verified live: our 404 page for //evil.com/x now emits to=%2F before the forge ever sees it, and /search?q=beans is unchanged at to=%2Fsearch%3Fq%3Dbeans. 44 pass / 0 fail.

The /pay/N dead end is filed as #24 rather than fixed here — either fix changes what the header offers on the money path, which is a behaviour change this pull should not carry. The issue notes that #10 closes it for good, so whoever takes #10 can close both.

Identity still comes only from the forge's signed headers; this push is one commit on top of the approved three, addressing the review's leftovers in src/who.ts. returnTo now refuses a pathname a browser would read as a host — //evil.com/x and /\evil.com/x come back as / — so the absence of an open redirect is ours rather than borrowed from the forge's sanitising of to=; verified live, our 404 page emits to=%2F for that request. The token deny-list is a named SECRET_PARAMS constant with the rule spelled out above it, so the next code or key that arrives in a query string is dropped rather than carried, and the blank line above SIGN_OUT_HREF is back. Two tests cover the new behaviour and fail without it: 44 pass / 0 fail.

What I chose not to do: the sign-in dead end on /pay/N?token=…. Both fixes sentinel names change what the header offers on the checkout path or what opens an order — behaviour, on the money path, which this pull should not carry. It is issue #24, which notes that #10 (/pay/:number opens for its buyer) closes it for good, so whoever takes #10 can close both.

To try it: /search?q=beans still renders to=%2Fsearch%3Fq%3Dbeans, /orders/1001?token=… renders to=%2Forders%2F1001, and /admin is still 403 with a sign-in link and no password field.

🤖 Generated with Claude Code

The review is submitted: approved (approved: 1 on #21), with the findings anchored on src/who.ts:85.

What I judged it on:

• returnTo is the only new surface and it holds. The host-looking-path guard rejects //evil.com/x, /\evil.com/x and ///evil.com (I ran it); everything else is a URL pathname, so ?/#/control characters are already encoded, and the value is encodeURIComponentd then escaped into the attribute.
• The token strip is complete. token is the only secret query parameter the app reads (src/shop.ts:221); the others are m, q, added. ?TOKEN=x survives the filter but is not a working token, since the read is case-sensitive.
• rel="noreferrer" closes a real hole entirely — Referrer-Policy does nothing for the same-origin hop to /__forge/, and every anchor to /__forge/ in the tree now goes through FORGE_LINK_ATTRS.
• Verified rather than trusted: bun test is 44/0 at 5b9f4fd in a scratch copy; reverting context.ts to ctx.url.pathname makes both new server assertions fail. On the preview I bought a thing as a guest through the simulated gateway and confirmed the pay page's Sign in carries to=%2Fpay%2FBZ-… with the token gone, /admin/orders is 403 with no password field, and the security floor is on every response.
• One nit left in the body, not blocking: nav.cats { flex: 1 0 auto } can't shrink below its max-content width, so an admin's header (~322px of links) overflows sideways at a 360px viewport instead of wrapping; flex: 1 1 auto; min-width: 0 keeps the fix without that floor.
• The documented cost (sign-in from a token-opened order page → 404) is not a regression and is already tracked as #24, so I didn't open a duplicate.

Two mechanical notes: the documented POST …/pulls/21/reviews with {state, body} returns bad_anchor — that endpoint stores anchored comments only, and the verdict had to go through …/pulls/21/review-state. One stray x comment from probing that API is mine; its thread is resolved and I said so in the review. My clone is untouched and nothing was pushed.