Skip to content

A wrong /app URL dropped you onto the marketing page - #2

Merged
MaXiMo000 merged 1 commit into
mainfrom
app-404-and-a-truthful-handoff
Aug 26, 2026
Merged

MaXiMo000 merged 1 commit into
mainfrom
app-404-and-a-truthful-handoff

Conversation

@MaXiMo000

Copy link
Copy Markdown
Owner

Second pass over LabLedger — this time the auth core, the access choke point,
the sharing flow, the security headers and the routing. I found no hole in the
security model
; what I found was in the routing, and a to-do list that was
sending readers after finished work.

The bug, and the diagnosis that was wrong

HANDOFF said:

A real 404 route. Unknown /app/* paths currently render the shell empty.

They do not. With no child matching, /app does not match either — so the
top-level * caught them and redirected to /. A signed-in user who
mistyped a URL was dropped out of the app onto the marketing landing page.

That is indistinguishable from having been signed out, and on a screen holding
clinical data it is the one thing a typo must not look like.

Measured both ways, against the real route table (real App, real guard, fake
axios adapter so no Mongo is needed):

before   /app/typo             -> at "/",           no shell, hero page
after    /app/typo             -> at "/app/typo",   in shell, "That page is not here"
         /app/reports/nonsense -> at "…/nonsense",  in shell, same
         /app                  -> "No results yet"      (unchanged)
         /app/review           -> "Nothing to confirm"  (unchanged)

The catch-all is a child of /app and stays inside the guard — redirecting
anywhere would reproduce the bug it fixes. NotFound is imported statically
rather than lazily like the other screens: it is a heading and a link, and
making it a chunk means a network round trip to tell somebody their URL is
wrong, on the one path where the network may be why they are here.

The last empty state without a door

/app/review/learned explained itself well and offered nothing to do. It now
links to the queue the rules come from.

The polish list was mostly stale

Re-read against the code, three of five entries were already done:

Entry Reality
Upload screen doesn't say what happens next Done. Per-file stages, plus "See your results" / "Confirm N uncertain results" once anything resolves
prefers-reduced-motion unhandled Done. CascadeScene.jsx reads matchMedia before animating; tokens.css carries the global rule
Unknown /app/* renders shell empty Wrong diagnosis — see above

HANDOFF is corrected, including keeping the wrong diagnosis on the record. A
to-do list that advertises finished work costs the next reader exactly the time
it cost this one.

What I audited and did not change

  • Session lifecycle. Rotation with a 10s grace, one-step replay detection
    that ends the session, idle timeout enforced in deps._live_session and
    again in /refresh — I went looking specifically for a refresh that could
    walk around the idle timeout, and it is explicitly closed with a comment
    saying why.
  • Login. Rate limited, timing-equalised against user enumeration, MFA
    checked only after the password verifies, per-account cooldown on codes,
    failures audited.
  • Invite claiming. Both halves required, uniform 404 for expired/spent/
    forged, and accepting can never downgrade access already held.
  • Access. One live() filter for revocation and expiry, 404-never-403
    throughout.
  • Headers. no-store on every response, default-src 'none', HSTS in prod.

One genuinely dead thing worth a later tidy: models/session.py exports a
live() helper that nothing imports — auth.py builds the same filter inline
from idle_cutoff(). Two ways to express one rule is how they drift, but it is
not a bug today and did not belong in this change.

Verification

vite build clean, oxlint clean on the touched files, vitest 18/18. The
route behaviour above was driven in a browser against the real App, with a
throwaway fake-adapter harness that was deleted before commit.

HANDOFF said unknown `/app/*` paths render the shell empty. They do
not, and the real behaviour is worse: with no child matching, `/app`
does not match either, so the top-level `*` caught them and redirected
to `/`. A signed-in user who mistyped a URL was dropped out of the app
onto the landing page - which is indistinguishable from having been
signed out, and on a screen holding clinical data that is the one thing
it must not look like.

Measured both ways against the real route table: before, `/app/typo`
ended at `/` showing the hero and no shell; after, it stays at
`/app/typo`, inside the shell, and says the address is wrong and the
session is fine. `/app/reports/nonsense` is caught too, and the real
children still resolve - `/app` and `/app/review` unchanged.

The catch-all is a child of `/app` and stays inside the guard on
purpose. Redirecting anywhere would reproduce the bug it fixes.
NotFound is imported statically rather than lazily like the other
screens: it is a heading and a link, and making it a chunk means a
network round trip to tell somebody their URL is wrong, on the one path
where the network may be why they are here.

Also the last empty state without a door. /app/review/learned explained
itself well and offered nothing to do; it now links to the queue the
rules come from.

And the polish list itself, which was sending the next session after
finished work. Re-read against the code: the upload screen already says
what happens next, prefers-reduced-motion is already handled in
CascadeScene and tokens.css, and the 404 entry had the wrong diagnosis.
Three of five entries were stale. A to-do list that advertises done
work costs the next reader the same time it cost this one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@MaXiMo000
MaXiMo000 merged commit 62375e4 into main Aug 26, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants