Security audit fixes: rate limits, IP spoofing, CSS bypass, guestbook block - #5
Merged
Conversation
Nav links, buttons, and step labels now use -webkit-text-stroke to match the heading style: hollow/outlined letters in Vandal Blow font throughout. - .top-bar controls: Vandal Blow + text-stroke 1.5px - .btn primary: white fill, paper-stroke on ink bg - .btn.secondary: transparent fill, ink-stroke - .splash-step-label: Vandal Blow + text-stroke 1.5px Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DWWTyKs6t8zyYbVYavKSBn
The .splash-feature-card h3 override was setting solid color: var(--accent), cancelling the stroke. Now uses color: var(--paper) + text-stroke like all other headings. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DWWTyKs6t8zyYbVYavKSBn
Critical: - globals.css: add [data-theme="dark"] guard to reader-mode dark styles - rateLimit.ts: throw RateLimitError inside try block with explicit ROLLBACK, not after COMMIT outside the exception handler High: - friends.ts: listPublicFriends privacy check now allows page owner to see their own private-paged friends (viewerId === userId || viewerId === f.userId) - globals.css: remove 4 hardcoded border-radius: 2–4px values (→ 0) - guestbook.test.ts: 13 new tests covering sign/moderate/delete/count paths Medium: - globals.css: .btn.secondary text is now color: var(--ink) (was var(--paper) — invisible white text on transparent background) - pageDocument.ts: importPageData validates parsed value is a plain object before casting, not just truthy Low: - globals.css: .studio-pixel-clear uses var(--ink)/var(--accent-ink) instead of hardcoded rgba/hex - guestbook.ts: remove duplicate JSDoc comment on countPendingGuestbookEntries Tests: 194 passed (13 new) Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DWWTyKs6t8zyYbVYavKSBn
Rewrites all hero copy, feature cards, step labels, CTAs, and section headers for the 20-25 urban youth/artist demographic. Swaps corporate startup language for raw manifesto tone: "Claim your spot", "Tag it up", "Find your people", "Join the pool" — no feed, no algorithm, nothing sanitized.
Mobile: - Viewport meta tag (was missing — critical; mobile browsers were rendering at desktop width and scaling down) - Hamburger nav drawer (NavDrawer client component) on ≤640px — collapses the full right-side link list into a tap-to-open drawer with badge count - Responsive typography: h1/h2/h3 scale down on ≤480px screens - 44px min-height enforced on all tappable elements (Apple HIG / WCAG 2.5.5) - SiteNav: explicit width/height on logo img to prevent CLS Performance: - <link rel="preload"> for both self-hosted fonts (VandalBlow, BebasNeue) so they don't block first paint - Replace filter:brightness() hover on .btn with opacity (avoids GPU compositing layer promotion on every hover) - will-change:transform on marquee animation only (legitimate use) - contain:layout style on explore-card and ask-card (paint containment for large card grids) PWA: - /manifest.json — standalone display, theme color, icon - /sw.js — service worker: cache-first for fonts and static assets, network-first for navigation, stale fallback for offline - PwaRegistrar client component registers the SW after mount - Apple web app meta tags (appleWebApp) in Next.js metadata
HIGH: - rateLimit: use rightmost X-Forwarded-For / X-Real-IP only — leftmost value is client-supplied and was trivially forgeable, bypassing all anonymous rate limits including login brute-force protection MEDIUM: - messages: add per-sender 60/min rate limit on all sends within existing threads, not just new-conversation creation - cssScope: block position:fixed/absolute/sticky unconditionally instead of requiring z-index in the same rule — the old check was defeated by splitting the two properties across separate rules for the same element; also closes the sticky+z-index gap LOW: - make/actions: add 20/min rate limit to makeFlowAction — each call does multiple DB writes with no prior guard - studio/actions: add 30/min rate limit to saveDraftAction, saveAndPublishAction, importPageAction — each parses a full JSON page document (up to 50KB) on every call - moderation/page: move ensureModeratorSeed() after auth+moderator check so unauthenticated requests cannot trigger the seed logic - messages/[handle]/page: parse and validate handle before using it in the login redirect URL to prevent open redirect via raw URL segment - guestbook: add blockCheckId param to signGuestbook() — blocked users could previously bypass the check by signing anonymously (authorId=null skipped the block lookup); the action now passes viewer.id separately
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe change adds rate limits to page, studio, and messaging actions; improves anonymous identity resolution; validates message handles; reorders moderation checks; supports guestbook block checks for anonymous entries; and rejects additional CSS positioning rules. ChangesPlatform hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
zowskyy
marked this pull request as ready for review
August 21, 2026 15:42
zowskyy
pushed a commit
that referenced
this pull request
Aug 22, 2026
… design MAJOR Bug #5: Missing Error Handling in signGuestbook/recordEdge - Issue: Guestbook entry was inserted before recordEdge() was called If recordEdge() threw, entry existed but edge was missing (inconsistent state) - Fix: Wrapped recordEdge call in try-catch block Entry is preserved, error is logged for monitoring Preferable to losing guestbook entries if graph system has temporary issue - Impact: Prevents database inconsistency while protecting data integrity MINOR Bug #6: blockCheckId Parameter Confusion - Issue: blockCheckId parameter defaulted to authorId, creating confusion Callers could forget to pass it and block checks would be silently skipped Pattern: signGuestbook(..., blockCheckId: string | null = authorId) - Fix: Made blockCheckId required (not defaulted) Pattern: signGuestbook(..., blockCheckId: string | null) - Impact: Forces callers to explicitly consider block checking Prevents accidental security bypasses from forgotten parameters - Updated: All callsites in guestbook.test.ts and bugs.test.ts to pass explicit blockCheckId - Note: Production callsite in actions.ts already passed blockCheckId correctly Test Results: - All 14 guestbook tests passing - Changes prevent future bugs from missing block checks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018V1rEEt5QTC2ww5ZikioWZ
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.
Findings fixed (9 total, all from second codebase audit)
High
lib/rateLimit.ts) — the leftmost XFF entry is client-supplied and trivially forgeable, bypassing all anonymous rate limits including login brute-force protection. Fixed to useX-Real-IPfirst, then the rightmost XFF entry (appended by our own proxy, not forgeable).Medium
lib/messages.ts) — once a conversation existed, there was zero rate limit on messages within it. Added 60/min per-sender cap covering all sends.lib/cssScope.ts) —position: fixedandz-indexcould be placed in separate rules for the same element, each passing inspection individually while the browser combined them into a full-screen overlay. Fixed by blockingfixed,absolute, andstickypositioning unconditionally (no z-index requirement, no split possible).Low
makeFlowActionno rate limit (make/actions.ts) — each call does multiple DB writes. Added 20/min cap.studio/actions.ts) —saveDraftAction,saveAndPublishAction,importPageActioneach parse a full JSON page document on every call. Added 30/min cap (shared key).ensureModeratorSeed()before auth check (moderation/page.tsx) — unauthenticated requests triggered the seed logic. Moved after auth and moderator check.messages/[handle]/page.tsx) — raw handle used in login redirect beforeparseHandleParamwas called. Parse first, redirect with validated handle.lib/guestbook.ts,[handle]/guestbook/actions.ts) —authorId=nullskipped the block lookup entirely; blocked users could sign anonymously. AddedblockCheckIdparameter; action passesviewer.idregardless of anonymous flag.All 194 tests pass.
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes