Stop the session secret and the bundle cache from logging people out - #13
Merged
Conversation
A redeploy left browsers reporting Failed to load module script: Expected a JavaScript-or-Wasm module script but the server responded with a MIME type of "text/html" for chunk-H3QWKNOY.js and chunk-UNPCGV2O.js. Angular fingerprints its bundles and index.html names the exact set belonging to one build, so a cached index.html outlives the deploy that produced it and asks for chunks the new build does not contain. The SPA fallback answered those with index.html, turning a missing file into a MIME-type error that says nothing about the cause, and letting the browser cache HTML under a script URL. Two rules: a path whose last segment has an extension is an artefact request and 404s when absent rather than falling through, and index.html is served no-cache so it is revalidated on every load. Fingerprinted bundles are immutable by construction and now say so. The e2e specs were confirmed to fail against the previous behaviour with the reported symptom before the fix was applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3wNzXNdrPw2Z59WjGHYv
SignatureExpired subclasses BadSignature, so catching only the latter reported every routine 12-hour expiry as "Invalid session" — the message that means the server's session secret is not what signed the cookie. That sent us looking for a session-secret bug when the cookie had simply aged out. Expiry now reports "Session expired". "Invalid session" keeps its narrower and much more interesting meaning: a well-formed, in-date cookie this server cannot verify. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3wNzXNdrPw2Z59WjGHYv
The window was 12 hours from login, so a teacher who prepared the evening before was signed out by lecture time. It is now a week, and measured from last use rather than from login: a request made with a cookie older than a day re-issues it, so a session in use cannot lapse mid-lecture, while one genuinely abandoned still ages out. Renewal is a middleware rather than part of the current_user dependency. FastAPI merges a dependency's response headers only when the endpoint returns data to serialise; an endpoint returning a Response directly gets its own headers untouched (routing.py: `response = raw_response`). Setting the cookie in the dependency therefore works for every JSON endpoint and silently does nothing for qr.svg and the SPA fallback — verified against a minimal app, and pinned by a test that renews through qr.svg. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3wNzXNdrPw2Z59WjGHYv
The secret every request verifies the session cookie against was a plain @Property, so it was recomputed on each request, and its create path checked whether the file existed and then wrote it. Both are wrong: - Concurrent callers with no file yet each generated their own and overwrote the last. A test driving twelve at once produced six different secrets from a single cold start. - A plain write truncates, so a concurrent reader saw an empty file and generated yet another. - An unreadable file (one left by an image that ran as a different uid) fell through `except OSError: pass` to a per-process random value, silently. The symptom is a cookie accepted by one request and rejected by the next, reported only as 401 — POST /api/sessions succeeding and every follow-up GET for that session failing. The file is now created with O_EXCL, so exactly one caller wins and the rest adopt its value, and the result is cached per process. The last-resort random secret is logged as an error rather than used in silence. /api/health additionally reports the instance id and a truncated hash of the secret, so two processes that disagree can be told apart from outside the container; the same line is logged at startup. The hash is 32 bits of a 384-bit token — enough to compare, useless for forging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3wNzXNdrPw2Z59WjGHYv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #12, from a console trace showing
POST /api/sessionssucceeding and every GET for the session it just created answering 401.The session secret was being regenerated
That signature — a cookie accepted by one request and rejected moments later — cannot be expiry and cannot be the volume.
/api/healthconfirmsstorage: persistent, so the data directory is writable and the secret file does persist.resolved_session_secretwas a plain@property, so it ran on every request, and its create path checked whether the file existed and then wrote it:Three ways that ends in mismatched cookies:
/state,/join-url,/qr.svgand/participantsalmost simultaneously, which is exactly that burst.write_texttruncates, so a concurrent reader sees an empty file and generates yet another.except OSError: passand fell through to a per-process random secret, silently.Now created with
O_EXCL, so exactly one caller wins and the rest adopt its value, and cached per process. The last-resort random secret is logged as an error instead of used in silence.Diagnosis from outside the container:
/api/healthgainsinstanceandsecret(a truncated SHA-256 — 32 bits of a 384-bit token, enough to compare, useless for forging). If either changes between two calls to the same URL, processes disagree and logins will fail at random. The same line is logged at startup.Missing bundles returned index.html
The other half of the same trace:
Angular fingerprints its bundles and
index.htmlnames the set belonging to one build, so a cachedindex.htmloutlives the deploy that produced it and asks for chunks the new build lacks. The SPA fallback answered those withindex.html, turning a missing file into a MIME error that names nothing useful.A path whose last segment has an extension now 404s when absent, and
index.htmlis servedno-cache. Fingerprinted bundles areimmutable. Both e2e specs were confirmed to fail against the previous behaviour with the reported symptom before the fix.Sessions no longer lapse mid-lecture
The window was 12 hours from login, so a teacher who prepared the evening before was signed out by lecture time. It is now a week, measured from last use: a request carrying a cookie older than a day re-issues it.
Renewal is a middleware, not part of the
current_userdependency. FastAPI merges a dependency's response headers only when the endpoint returns data to serialise — a returnedResponseis used as-is (routing.py:712). Setting the cookie in the dependency works for every JSON endpoint and silently does nothing forqr.svgand the SPA fallback, so the projected Join view would quietly stop renewing. Verified against a minimal app; pinned by a test that renews throughqr.svg.Also:
SignatureExpiredsubclassesBadSignature, so catching only the latter reported every routine expiry asInvalid session— the message that means the secret is wrong. Expiry now saysSession expired, andInvalid sessionkeeps its narrower, more interesting meaning.Tests
78 backend, 5 frontend unit, 8 browser specs. New:
test_session_secret.py(including the concurrent cold start),test_session_cookie.py(valid, expired, wrong-secret, absent, renewal through aResponse-returning endpoint),asset-serving.spec.mjs.Not addressed here
Whether more than one process serves the deployed app — the
instance/secretfields answer that once this is deployed, and it would be a Serve scaling question rather than a code one.Generated by Claude Code