fix(pwa): escape the CSRF token in the hidden form field - #75
Merged
ralyodio merged 1 commit intoJul 28, 2026
Merged
Conversation
csrfInput() interpolated req.csrfToken straight into the value attribute. That token is read from the mc_csrf cookie, which the app never validates, so a quote in it closes the attribute early. The browser then submits a truncated token, csrfGuard rejects the POST, and every cookie-session form in the app 403s until the cookie is cleared by hand. A token carrying markup escapes the attribute entirely and injects into the page. appBar() already escapes this exact token, so this only brings csrfInput() in line with it. Adds apps/pwa/test/csrf-input-escaping.test.mjs covering the round trip through a rendered form, plus a guard test so the escaping cannot loosen csrfGuard.
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.
Every cookie-session form in the PWA renders its hidden
_csrffield throughcsrfInput()inapps/pwa/src/lib/session.mjs, which interpolated the token straight into the value attribute:That token is
req.cookies.mc_csrf.sessionMiddlewareonly mints a new one when the cookie is absent, and never checks what is in it, so whatever the client sends back is what gets rendered.appBar()inlib/html.mjsalready escapes this exact same token.csrfInput()did not.Reproduction
Booted the real
authRouter+pagesRouteragainst a throwaway libsql database on unmodifiedeceb4f0, requested/settingswith anmc_csrfcookie, then submitted the "Save channels" form exactly as a browser would from the returned markup (decoding entities in the attribute first).mc_csrfvaluecsrfInput()rendersabc123def456value="abc123def456"abc123def456a"bvalue="a"b"a"><script>alert(1)</script><b x="value=""Two things go wrong once the value contains a quote:
csrfGuardsees a mismatch and returns 403. That hits sign-in, sign-out, settings, channels, API keys, approve, kill, device authorize and buy-credits alike, and it persists for the 30-day cookie lifetime with no way to recover from the UI.<script>element into the page. The cookie is sethttpOnly:falseand is not origin-isolated, so any subdomain able to write a cookie for the site can plant it.Fix
Escape it, the same way
appBar()already does. One import plus one call, no behaviour change for a well-formed token.With the fix all three rows above return 302 and the markup stays intact — the escaped token decodes back to the original on submit, so it still matches the cookie.
Tests
New
apps/pwa/test/csrf-input-escaping.test.mjs(5 tests, same skip-guard and throwaway-database pattern aslogout-csrf.test.mjs): the ordinary-token control, the two cases above driven through a real rendered form, a check thatcsrfInput()andappBar()agree, and a guard test asserting a genuinely mismatched token is still rejected with 403.Verified with
git stash push -- apps/pwa/src/: 2 pass / 3 fail unpatched, 5/5 patched. The control and the guard test pass either way, so they are not measuring the fix.Full suites, 0 failures both before and after:
apps/pwa27 → 30, rootnpm test201 → 204.One note on the test harness, since it is easy to get wrong: it decodes HTML entities in the value attribute before submitting. Comparing the raw attribute text instead would measure the markup rather than what actually reaches the server, and would report the fixed code as still broken.