fix(routing): make public share, send and fill links reach the recipient

[remko48]

Important

New branch. This supersedes #610, which is closed. Its branch carried a git-history defect that made GitHub declare every review stale the moment it was submitted, so no approval could ever stick. The work is unchanged and re-landed on a clean branch (fix/public-share-link-routing-2), plus the follow-up commits listed below. Review here; #610 needs no further attention.

Every recipient link (share / send / fill) was dead for the person it was sent to. The router moved to createWebHistory, but the link builders still emitted hash URLs (/apps/keepiq/public#/share/link/<token>); the fragment is never read under path routing, so the catch-all redirected to / and the vault guard bounced account-less recipients onto the lock screen — asked for a master password they do not have.

The fix lands as a stack, each commit green on its own:

1. serve the anonymous shell at every /public subpath — a publicShell catch-all route (/public/{path}, mirroring the AppHost dashboard catch-all) serves the #[PublicPage] shell for every recipient path, ahead of the authenticated /{path} fallback.

2. resolve recipient routes as paths and rescue sent hash links — routerBase() extends to /apps/keepiq/public so the same manifest routes resolve on both shells, and a bootstrap handoff rewrites any legacy #/ URL in place (links already in inboxes cannot be regenerated). A legacy in-fragment ?k= send key moves to a new #k= fragment — never into the real query string, which the browser would send to the server on refresh.

3. emit path links from the recipient link builders — link shares, application fill links, fillLink.js and sends; the send access page reads the key from the fragment with a legacy-query fallback.

4. emit path deep links from search, notifications and the PWA manifest — three more hash builders the sweep found on the authenticated shell; their deep links were silently swallowed by the catch-all.

5. test(e2e): cover the anonymous recipient — the first specs to open share/send/fill links in a browser context with no session, including the legacy hash form and the key-stays-out-of-the-query assertion. This gap is why the bug shipped.

6. fix(l10n) — every recipient-facing string translated into Dutch (nl ratchet lowered 382 → 362), plus a new "Invalid password. Please try again." key in all 36 locales — the link-share page compares nothing against a second entry, so "does not match" was wrong there.

7. fix(public-pages) — the recipient card centres on the guest layout (the app mount shrank inside core's flex row); the revealed secret is labelled field rows with per-field copy and masked sensitive values (card/identity JSON parsed into fields, no raw JSON); the not-found/expired state is a proper error page instead of the raw English server message.

8. style(share-dialog) — the one-time link + password in a highlighted panel, real padding and separated sections.

Five more commits landed after #610 was opened, from CI and from a follow-up sweep of the new route:

9. fix(tests): restore the em dashes mangled in the notifier expectations — four KeepiqNotifierTest tests failed in all six CI combos, and the URLs were not why: commit 4 had already moved every expectation to the path form. That same commit round-tripped the file through a CP1252 reader, corrupting all eight em dashes; four sit inside asserted message strings, so exactly four tests failed and the other four occurrences were silent. Restores U+2014 in all eight places, nothing else.

10. fix(spec-coverage): tag the send access page's fragment-key computed — hydra gate-16 failed on EphemeralSendAccess.vue::fragmentKey; the computed was added with an inline comment but no docblock, so the changed-method scan found no @spec line.

11. fix(routing): match the public shell on a segment boundary — isPublicSurface() classified any path merely containing /apps/keepiq/public as a recipient surface, so /apps/keepiq/publications/…, /apps/keepiq/publicfoo and /apps/keepiq/secrets/public all answered true. It is not the security gate (access is authorised server-side by scoped token, and the locked-vault gate is the name-based isPublicRoute()), but a classifier wrong for a whole family of paths invites being mistaken for one. The remainder is now split off first and required to begin at a / boundary; sub-directory installs keep resolving, and the docblock says plainly what the function is not.

12. test(routes): forbid authenticated endpoints under /public — publicShell#pageCatchAll matches every GET below /public/, so a session-gated route mistakenly placed there would be shadowed: the anonymous shell renders, the intended handler never runs, and the author sees a working page rather than a 404. Nothing is exploitable today (the shell exposes no data); the hazard is that a routing slip becomes a silent auth bypass. Adds a warning on the route entry and a contract test that reads the route table as source text and asserts every /public route resolves to a method declaring #[PublicPage] and #[NoCSRFRequired], with a negative control.

13. test(coverage): cover the public shell controller — the coverage ratchet blocked the branch on PublicShellController, visible only once PHPUnit stopped failing first: pageCatchAll() entered the scoped denominator uncovered, in a file with no test class at all. Covers the file rather than the method (page() was untested too) and pins what would actually strand a recipient: RENDER_AS_BASE rather than the session-expecting user layout, the wasm-unsafe-eval directive the Argon2id derivation needs, and the identity of the two methods across the three live link shapes.

Review note: the parent branch this work was originally stacked on (fix/cards-table-type-icon-strength-badge, #607) has since merged into development, so the diff here is the thirteen commits above and nothing else; the merge commit carrying that parent in is the only extra entry in the commit list.

[github-actions]

Quality Report — ConductionNL/keepiq @ 12ce3f2

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 536/536
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					✅
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					✅

---

Quality workflow — 2026-09-04 13:57 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

Verdict: APPROVE (Quick mode)

This is the same tree I reviewed on #610 — head SHA 2b64bc5 is byte-identical, and #617's diff is a strict subset (108 of those 144 files, zero files that weren't already in #610's diff). The diff shrank only because the parent stack (#601/#605/#607) landed in development, so the PR body's "thirteen commits and nothing else" claim checks out.

I re-verified the four fixes independently rather than taking the commit messages at their word:

Fix	Commit	Result
isPublicSurface() segment boundary	c9d7cc3	appRelativePath() requires the remainder to be empty or /-prefixed, so keepiq-old and keepiqfoo reject; /publications/… no longer matches /public/. All three PUBLIC_PATH_PREFIXES end in /, so they're boundary-safe by construction.
/public catch-all guardrail	c9d7cc3 + 6de55b7	Warning sits on the route entry where a future author will actually see it. The contract test is stronger than what I asked for — it has anti-vacuity guards (testTheRouteTableParses, assertGreaterThanOrEqual) so it can't pass on an empty parse, plus a real negative control on dashboard#summary.
Em-dash mojibake	430b6bb	Exactly 8 proper U+2014 (xxd confirms e2 80 94), zero â sequences — matches "all eight places".
PublicShellController coverage	2b64bc5	Pins RENDER_AS_BASE, the wasm-unsafe-eval directive the Argon2id derivation needs, method identity across all three live link shapes, and that the subpath stays out of the template params.

CI is green at head: 0 failures, 0 pending — Hydra Gates, the 6-combo PHPUnit matrix, Newman, CodeQL and the coverage ratchet all pass.

The two non-blocking 🟢 nits from #610 (the PR-body "browser context" wording, and commit 8 labelled style(…) while restructuring the template) are deliberately not re-raised here — you already saw both and chose to leave them, and neither affects the code.

Nice work on the follow-up sweep. Finding isPublicSurface()'s substring match yourself and then splitting the remainder on a boundary — rather than patching the one path I named — is the right shape of fix, and the routing-slip contract test closes a hazard that would otherwise have been invisible until it bit someone.