fix(hardening): media key, CSP, viewport, kiosk storage, SW updates, redirect-param#37
Closed
bryanfawcett wants to merge 5 commits into
Closed
fix(hardening): media key, CSP, viewport, kiosk storage, SW updates, redirect-param#37bryanfawcett wants to merge 5 commits into
bryanfawcett wants to merge 5 commits into
Conversation
`media.ts` GET/DELETE handlers were stripping only the first `/` from the request path, so a request for `/api/media/events/abc.png` was looking up `api/media/events/abc.png` in R2 — every key written by the upload handler (`events/<id>.<ext>`) missed. Fix to strip the full `/api/media/` prefix with an anchored regex. CLAUDE.md claims the security-headers middleware sets CSP but it didn't. Add `Content-Security-Policy: default-src 'self'; frame-ancestors 'none'; base-uri 'self'` to all non-HTML responses (the worker is JSON-only apart from the `/` status page in health.ts, which uses inline styles + Google Fonts and would break under this CSP — skip it there).
- Remove `maximumScale: 1` from the viewport export. Blocking pinch-zoom violates WCAG 1.4.4 (Resize Text); Next.js's default allows user scaling. - Switch Intercom to `lazyOnload` so the widget stops blocking the main thread during the initial page load. Keeps the widget; just defers it.
Kiosks run on shared hardware (event venue TVs, check-in tablets). Storing the pairing token in localStorage made it persist across browser restarts and exposed it to any XSS payload, with no expiry. - Switch the three `nhimbe_kiosk_token` reads/writes from localStorage to sessionStorage so the token dies with the tab. - Add a `document.visibilitychange` handler in the main KioskPage. If the tab is hidden for >= 30 minutes (operator walked away, screen locked, etc.) the session is cleared on next visibility, dropping back to the pairing screen instead of letting a passer-by hijack it.
Previously the SW was registered with no update handling, so after a Next.js deploy users would keep loading the old SW and hit 404s for evicted `_next/static/*` hashes until they did a full reload by hand. Add the standard `updatefound` / `statechange` pattern: when the new SW reaches `installed` and a controller is already active, call `location.reload()` so the new worker takes over. No new dependencies.
The signin page reads `?return_to=`, but four callers were sending different param names — `redirect=` (rsvp-button, header, mobile-bottom-nav) and `returnUrl=` (admin layout) — so the post-signin landing always fell back to the default `/`, silently dropping the user's intended destination. Align all four senders on `return_to` to match what `signin/page.tsx` reads.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
mukoko-nhimbe-api | edc0027 | Commit Preview URL Branch Preview URL |
May 17 2026, 03:55 PM |
|
Claude finished @bryanfawcett's task —— View job Security & Quality ReviewReviewing this hardening PR for security vulnerabilities, performance issues, code quality, and potential bugs. Todo List
Detailed Analysis✅ Media R2 Key Fix (
✅ CSP Header Implementation (
✅ Viewport Pinch-Zoom Fix (
✅ Kiosk Storage Security (
✅ Intercom Loading Optimization (
✅ PWA Service Worker Updates (
✅ Signin Redirect Standardization (Multiple files)
Final Assessment✅ Excellent hardening work - No security vulnerabilities, performance regressions, or code quality issues found. All fixes are:
Recommended for merge - This represents solid defensive engineering with meaningful security and UX improvements. |
Contributor
Author
|
Closing — consolidating into PR #36 per the "all in one PR" directive. Cherry-picking the 5 commits onto Generated by Claude Code |
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
2 participants
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.
A grab-bag of unrelated hardening fixes surfaced by a recent security review. Each commit stands on its own.
Fixes
worker/src/routes/media.ts:52,99(now:55,103).c.req.path.replace("/", "")only strips the first slash, soGET /api/media/events/abc.pngwas looking upapi/media/events/abc.pngin R2 — every key written by the upload handler (events/<id>.<ext>) missed. Switched to an anchored^/api/media/regex.worker/src/index.ts:97security-headers middleware. CLAUDE.md claimed CSP was set; it wasn't. AddedContent-Security-Policy: default-src 'self'; frame-ancestors 'none'; base-uri 'self'to all non-HTML responses. The one HTML endpoint (/status page inworker/src/routes/health.ts) uses inline styles + Google Fonts and would break under this policy, so it's skipped — called out in the comment.src/app/layout.tsx:24. RemovedmaximumScale: 1(WCAG 1.4.4 violation). Next.js default allows pinch-zoom.src/app/events/[id]/kiosk/page.tsx:82,433,440. Movednhimbe_kiosk_tokenfrom localStorage to sessionStorage (dies with the tab) and added adocument.visibilitychangehandler inKioskPagethat clears the session after the tab has been hidden for >= 30 minutes — kiosks live on shared/public hardware.src/app/layout.tsx:204-209. Switched both<Script>strategies fromafterInteractivetolazyOnload. Widget still loads; just stops blocking the main thread during initial load.src/components/pwa/sw-register.tsx. Added the standardupdatefound/statechangepattern: when the new SW reachesinstalledand a controller already exists, calllocation.reload()so users stop seeing 404s for evicted_next/static/*hashes after a deploy. No new dependencies.src/app/auth/signin/page.tsx:13readsreturn_to, but four callers were sending the wrong param name and silently dropping the redirect:src/app/events/[id]/rsvp-button.tsx:72(wasredirect=),src/app/admin/layout.tsx:60(wasreturnUrl=),src/components/layout/header.tsx:228(wasredirect=),src/components/layout/mobile-bottom-nav.tsx:37(wasredirect=). Standardized all four onreturn_to. The two senders explicitly listed in the brief plus two more I found in the audit that had the same bug.src/components/auth/auth-context.tsx:153was also flagged but it sets theauth_redirectlocalStorage key — a different mechanism, never read anywhere, left alone.Out of scope (intentionally untouched)
links.ts/waitlist.tsTest plan
cd worker && npx tsc --noEmit— cleancd worker && npx vitest run— 231/231 passnpm run lint— 0 errors (36 preexisting warnings)npx vitest run— 160/160 passGET /api/media/events/<id>.<ext>serves it (the bug fix)Content-Security-Policyheader is set on JSON; absent on/status page/https://claude.ai/code/session_01Dp6YFZCHz1HjL9svPWmso2
Generated by Claude Code