You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fix CI cross-platform issues, release log gate, and test dependencies (#384)
## Summary by Sourcery
Implement admin step-up MFA with backend middleware, session tracking,
and a shared frontend modal flow, introduce saver modes and
data-saver-aware endpoints, add durable upload backups, and tighten UI
layout, security, and CI workflows.
New Features:
- Add step-up MFA flow backed by session mfaVerifiedAt, dedicated auth
endpoints, and a reusable frontend modal/provider so privileged admin
actions can require recent 2FA verification.
- Introduce Data Saver and Battery Saver modes with user preferences,
global hooks, and a new settings tab, enabling lighter API responses and
reduced animations for opted-in users.
- Extend the StudyHub video player with watch-progress persistence, A-B
loop controls, and a keyboard shortcut help overlay for study-focused
playback.
- Add a daily backup job that mirrors the uploads volume to Cloudflare
R2 plus a restore script to recover user-uploaded media after volume
loss.
Bug Fixes:
- Fix admin login MFA flag handling to fail closed on missing/errored
flags and support a clearly logged emergency override, preventing silent
MFA bypass.
- Ensure saver-mode preferences are included in authenticated session
payloads so global hooks honor server-side settings across devices.
- Prevent upload path traversal issues on Windows by hardening managed
filename validation against path separators.
- Resolve spacing and layout inconsistencies (sticky offsets, grids,
skeletons, sidebars, inputs) that caused visible jitter and clipping
across key pages.
Enhancements:
- Add requireRecentMfa middleware and apply it to high-risk admin and
payment routes, with comprehensive unit tests and an emergency bypass
log trail.
- Expose WebAuthn credential last-used timestamps and enrich MFA-related
logging to support better security visibility.
- Refine moderation and admin handlers with stricter ID validation,
origin allowlisting, and safer pagination, aligning with documented
security rules.
- Migrate several bespoke modals (reports, legal enforcement, attachment
preview, AI sheet preview, confirm dialog) onto the shared focus-trapped
dialog primitive for consistent accessibility.
- Standardize layout tokens for nav height and spacing, update sticky
offsets and page scaffolds to use them, and improve mobile behavior via
dvh usage and safe-area insets.
- Adjust toast durations under battery saver and integrate data-saver
awareness into feed and messaging flows to reduce bandwidth and
background chatter.
- Tighten R2 helper APIs to support per-bucket operations, streaming
uploads with explicit content length, and configurable backup cadence
and rate limits.
Build:
- Relax CI release-log enforcement for bot-authored and lockfile-only
PRs while narrowing the code-change scope, keeping the human-facing log
gate for functional changes.
- Update production-mode test fixtures to auto-populate all required
secrets and keep security-header tests resilient to new required env
vars.
CI:
- Stop running Android and CodeQL workflows on every pull request,
limiting them to main, tags, schedule, or manual runs to reduce
unnecessary CI usage.
Tests:
- Add focused unit tests for step-up MFA middleware, recovery code
primitives, WebAuthn routes, data-saver negotiation, and admin MFA
fail-closed behavior.
- Update existing tests and skips to reflect the new role-picker, AI
permission, and prefetch mappings while preserving coverage where
behavior is unchanged.
Two real findings from a Codex review pass on `3b09f25a`. Both verified against actual code by our own audit loop (CLAUDE.md A21) before fixing.
34
+
35
+
**P1 (CRITICAL) — Passkey login bypassed `createSession`; admins locked out of step-up-MFA-gated routes.**
36
+
37
+
`POST /api/webauthn/authenticate/verify` was hand-rolling its own session emission (`signAuthToken(fullUser) + setAuthCookie(res, token)`) instead of going through `issueAuthenticatedSession`. The wave-12.11 `requireRecentMfa` middleware checks `req.sessionJti` (populated by `requireAuth` only when the JWT carries a `jti`, which is set only by `createSession`). Net effect:
- Every step-up-gated admin route (`DELETE /api/admin/users/:id`, `PATCH /api/admin/users/:id/{role,trust-level,mfa}`, `POST /api/payments/admin/sync-stripe`) rejected with `MFA_STEP_UP_REQUIRED { reason: 'no_session' }`.
41
+
- The step-up modal's `POST /api/auth/mfa/step-up/verify` then 401'd with `'No active session to refresh.'` because there was no Session row to update.
42
+
- Net: passkey admins could not perform ANY of the 5 protected admin actions. They had to log out and re-login via email-OTP.
43
+
44
+
Fix: webauthn verify now calls `issueAuthenticatedSession(res, user.id, req, null, { mfaVerified: true })` — mirrors `login.challenge.controller` and `login.recovery.controller`. Passkey is an explicit AAL2 factor per NIST 800-63B ("something you have"), so `mfaVerified: true` is correct: the new Session row's `mfaVerifiedAt` is stamped at creation, and `requireRecentMfa` permits step-up-gated routes for the next 15 minutes without re-prompting.
45
+
46
+
Updated `backend/test/unit/webauthn-routes.unit.test.js` to mock the canonical helper (`mocks.authService.issueAuthenticatedSession`) and assert it's called with `{ mfaVerified: true }` on the success path AND NOT called on counter/replay-failure paths. 21/21 tests pass.
47
+
48
+
**P2 (HIGH) — Saver-mode Settings toggle silently did nothing on the current session.**
49
+
50
+
The wave-12.13 fix had `DataAndBatteryTab` seeding localStorage after save, but the consumers (`SaverModeInitializer`, `useFeedData`, `useMessagingData`) all read `useDataSaver({ serverMode: sessionUser?.preferences?.dataSaverMode })`. The hooks' precedence is `serverMode || readStoredMode()` — `serverMode` wins as long as session-context holds the old value. Since `usePreferences().save()` only updated its own local React state, `sessionUser` was stale until the next `/api/auth/me` round-trip. The body `[data-battery-saver]` attribute stayed off, Feed/Messages didn't switch to lite mode, and the user thought the toggle was broken.
51
+
52
+
Fix: `usePreferences().save()` now pulls `setSessionUser` from `useSession()` and pushes the persisted prefs back into session-context after the merge. Every consumer of `user?.preferences` re-derives synchronously on next render — no event plumbing, no `storage` listener (which only fires cross-tab anyway), no hook precedence change. Matches CLAUDE.md A4 ("hydrate UI from the response body's persisted value"). The `DataAndBatteryTab`'s explicit `setStored*` calls stay as belt-and-suspenders (idempotent — the hooks' `serverMode` effect would write the same value to localStorage).
Founder asked for a "fix the spacing everywhere + confirm everything we shipped is production-ready" pass. Two parallel audits:
61
+
62
+
**Feature audit (wave 12.11-12.13) — clean.** All 11 features from `d1a3abe0` / `e9fc07e6` / `e80214b4` verified end-to-end: admin step-up MFA (`requireRecentMfa` on 5 routes + frontend interceptor + step-up modal + 12/12 unit tests), `WebAuthnCredential.lastUsedAt`, `serializeNote` allowlist (incl. wave-12.13 `revision` re-add), upload volume → R2 backup (`runWithHeartbeat`-wrapped, streamed-not-buffered, `R2_BUCKET_UPLOAD_BACKUP` REQUIRED_IN_PRODUCTION), Data Saver + Battery Saver modes (`useDataSaver` / `useBatterySaver` exports `setStored*` helpers, `SaverModeInitializer` mounted in `App.jsx`, server-side `dataSaverNegotiation` 3-signal contract, 11/11 unit tests), StudyHubPlayer (watch-progress persistence, A-B loop, `?` keyboard help overlay), feed lite mode (`?lite=1`), typing-indicator suppression, battery-saver JS gates in `animations.js` + `Toast.jsx`, session-payload preferences. CLAUDE.md A-rules A5/A6/A7/A8/A10/A11/A12/A13/A14/A16 all clean. 60/60 wave-12 unit tests pass. Nothing to finish.
63
+
64
+
**Spacing / chrome pass — fixed.** A spacing audit found ~50 visual drifts; the screenshot-visible one was `.feed-page__aside { top: 86px }` floating ~30px below the actual 56px navbar. Foundation tokens added in `index.css``:root`: `--sh-nav-h: 56px` (single source of truth for sticky offsets) and `--sh-space-xs/sm/md/lg/xl/2xl/3xl` (CANONICAL 4/8/12/16/20/24/32 step aliases — mapped to existing `--space-*` so old call sites keep working). Then migrated the worst drifts:
65
+
66
+
-**Sticky offsets** — `.feed-page__aside`, `.settings-nav`, `PageShell` sidebar, `AiPage` sidebar all now compute `top` from `var(--sh-nav-h)`. No more 74 / 80 / 86px guesses.
67
+
-**Split-panel height** — `.notes-split-panel` / `.messages-split-panel` switched from `calc(100vh - 80)` to `calc(100dvh - var(--sh-nav-h))` with `100vh` fallback so mobile Safari's URL bar doesn't clip the bottom of the panel.
68
+
-**Grid gaps** — `.app-three-col-grid` (22 → 20), `.app-two-col-grid` (20), `.profile-cockpit` (20) all on `--sh-space-xl`. Navigating Feed ↔ Sheets ↔ Profile no longer shifts the column gutter.
69
+
-**Duplicate `.settings-layout`** — removed the index.css copy (which conflicted on `gap`); canonical definition lives in `responsive.css` with `gap: var(--sh-space-2xl)`.
-**Skeleton CLS** — `SkeletonCard` padding `'20px 22px' → var(--card-pad)` (matches real `.sh-card`); `SkeletonFeed` gap 14 → 16; removed the per-card padding override. The shimmer ghost no longer visibly shrinks when content loads.
72
+
-**AiBubble** — bubble + chat window `bottom`/`right` use `calc(N + env(safe-area-inset-*, 0px))` so the iPhone home-indicator strip is clear; chat `maxHeight` switched to `100dvh` for the same URL-bar reason.
73
+
-**iOS auto-zoom fix** — `@media (max-width: 767px) .sh-input { font-size: 16px }` floors the input font so iOS Safari doesn't auto-zoom and jolt the layout on focus. Desktop keeps `--type-sm`.
74
+
-**FeedPage banners** — `padding: '10px 14px'`, `'12px 14px'`, `'7px 16px'` all migrated to the 16/8-step.
75
+
76
+
**Code-reviewer pass — 3 real findings caught + fixed in-wave.** A code-reviewer subagent on the diff caught:
77
+
78
+
1.**CRITICAL** — `AppSidebar.jsx` drawer-mode `<aside>` still had `top: 74` hardcoded (line 431). Tablet/phone traffic — most real users — would have hit a sticky drawer trigger sitting 18px below the navbar. Migrated to the token.
79
+
2.**IMPORTANT** — `AiPage.jsx` chat-panel `height: calc(100vh - ...)` should have been `100dvh` per the inline comment intent. CSS-in-JS can't do the dual-line `100vh / 100dvh` override pattern, so swapped to `100dvh` directly (95%+ browser support since Sep 2022).
80
+
3.**IMPORTANT** — `.settings-layout` responsive overrides at 1024px + 768px were still in index.css contradicting the "moved to responsive.css" comment, AND the 768 vs 767 breakpoint collision produced a 1-pixel window at exactly 768px where the gap dropped to 12px. Moved BOTH the tablet (768–1024px) and phone (≤767px) overrides into responsive.css with the canonical 767 phone breakpoint, plus migrated the `.settings-nav-btn` mobile overrides over so SettingsPage's phone view still renders correctly.
81
+
82
+
Bonus migration during the code-reviewer fix pass: also migrated 4 pre-existing hardcoded sticky offsets that didn't ship in this wave's original scope but use the same `top: 74/80` pattern — `index.css .sh-sidebar-sticky`, `MyCoursesPage.jsx`, `ScholarLists.css .scholar-saved__rail`, `ScholarPage.css .scholar-reader__sidecar`. All now compute from `var(--sh-nav-h)`.
83
+
84
+
Out of scope for this wave (documented in the audit, deferred): a shared `<FormField>` primitive to standardize per-tab form spacing in Settings; a shared `<Modal>` primitive to consolidate the 5 ConfirmDialog reimplementations; bulk migration of remaining ~30 `pageShell(top, bottom)` call sites to one default; bulk 100vh → 100dvh on 40 page roots. Those are structural changes that need founder sign-off on the primitive contract before landing.
85
+
86
+
Validation: backend lint clean (no changes), frontend lint 0 errors (warning count unchanged at 91 — same warnings as baseline, all pre-existing), frontend build green (1.02s). 12 files changed, ~197 insertions / 74 deletions.
0 commit comments