fix(site): restore hydration and reject invalid signup bodies - #66
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe pull request replaces static CSP hashing with per-request nonces, validates subscription payload shapes, adds production browser coverage, updates CI, and revises repository and deployment documentation. ChangesWebsite contracts
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Browser
participant NextProxy
participant RootLayout
participant API
Browser->>NextProxy: Request production page
NextProxy->>NextProxy: Generate nonce and CSP
NextProxy->>RootLayout: Forward x-nonce
RootLayout-->>Browser: Return HTML with nonce-bearing scripts
Browser->>API: Submit subscription payload
API-->>Browser: Return validation response
Merge Risk: ⚪ Minimal · up to The updated CSP and subscription validation paths are covered by production-oriented checks, with no actionable merge risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Biome (2.5.10)frontend/app/api/subscribe/route.tsBiome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins. frontend/app/layout.tsxBiome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins. frontend/package.jsonBiome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
@greptileai Review exact head |
✅ Action performedReview finished.
|
biggest-littlest
left a comment
biggest-littlest
left a comment
There was a problem hiding this comment.
Recorded approval after the scoped review and passing Website contracts run 34770128524. Production verification follows promotion.
ALARGECOMPANY
left a comment
ALARGECOMPANY
left a comment
There was a problem hiding this comment.
Recorded approval for the tested candidate 15ddcee. No protection changes or bypasses.
|
Reviewed the completed CodeRabbit output for 15ddcee and checked inline comments; none were raised. The docstring warning reports all four functions as unsupported, so it does not establish missing documentation. The nested-root Biome warning is from the review runner: the repo-owned lint command runs from frontend and passed locally, in the commit hook, and in Website contracts run 34770128524. Greptile was requested once but has not returned review evidence. |
* fix(analytics): forward $raw_user_agent and $host for cookieless ingestion (#52) PostHog's cookieless server-hash step reads $raw_user_agent and $host straight off event.properties and drops the event with a cookieless_missing_user_agent/cookieless_missing_host ingestion warning if either is absent. createCommonProperties rebuilt an allowlisted properties object that dropped both, so every event was silently discarded at ingestion. Forward them through; never add $ip, which PostHog's capture service fills in server-side from the connection. Co-authored-by: scttbnsn <80784472+scttbnsn@users.noreply.github.com> * chore(config): remove the stale Cursor rules (#54) * fix(analytics): promote cookieless ingestion fix to production (#53) PostHog's cookieless server-hash step reads $raw_user_agent and $host straight off event.properties and drops the event with a cookieless_missing_user_agent/cookieless_missing_host ingestion warning if either is absent. createCommonProperties rebuilt an allowlisted properties object that dropped both, so every event was silently discarded at ingestion. Forward them through; never add $ip, which PostHog's capture service fills in server-side from the connection. Co-authored-by: biggest-littlest <zap_inane.2p@icloud.com> * chore(config): drop the stale Cursor rules folder * docs(config): drop dangling .cursorrules references --------- Co-authored-by: biggest-littlest <zap_inane.2p@icloud.com> * docs(readme): describe what this repo is and how it deploys (#55) * chore(gitignore): ignore the root .vercel link and history-backup bundles * feat(analytics): capture $pageleave and send $pathname (#60) Measured over the shared PostHog project, 208 of 432 sessions across the five instrumented sites record zero duration, and PostHog's built-in Web analytics Page/Entry page/Exit page tables return zero rows. capture_pageleave was false, so a session's last recorded timestamp is its last pageview, and a five-minute read of one page scores as zero seconds. Flipping the option alone fixes nothing: sanitizeEvent allowlisted only $pageview, cta activated, and $web_vitals, so every $pageleave posthog-js emitted would have been dropped silently with no error and no ingestion warning. This adds a $pageleave branch that rebuilds the event the same way $pageview does. capture_pageview is false here (pageviews are captured by hand), so posthog-js's _shouldCapturePageleave gate needs an explicit true rather than the default. $pathname is the property PostHog's page tables actually key off, and it was never sent. It's bound to the already-sanitized `path` value, never the raw pathname, so it can't carry a route outside ALLOWED_ROUTES and adds no information the event wasn't already sending. A regression test asserts the two never diverge. No privacy option changes: cookieless_mode, person_profiles, persistence, disable_persistence, respect_dnt, save_referrer, and save_campaign_params are untouched. Part of X16 in the ops execution plan. * fix(seo): repair JSON-LD logo 404 and double-slash base URLs - fix(seo): point Organization.logo at /icon-512x512.png; the referenced /logos/codeswhat-logo-green.png never existed, so crawlers got a 404 - fix(seo): strip trailing slashes from BASE_URL and reuse it in robots.ts and sitemap.ts, so a NEXT_PUBLIC_SITE_URL set with a trailing slash can't emit //sitemap.xml-style URLs - chore(seo): disallow /studio/ in robots.txt; the capture pages already 404 in production but the exclusion shouldn't depend on that guard - chore(seo): 308 the stable *.vercel.app production aliases to codeswhat.com instead of serving duplicate content - fix(api): stop forwarding EmailOctopus error detail to subscribe clients; log it server-side and return a fixed message * build(deps): bump next to ^16.2.11 to clear all 35 Dependabot alerts One-line range bump; npm resolves next 16.3.3, which also pulls the patched transitive versions: postcss 8.5.23, nanoid 3.3.18, sharp 0.35.4. npm audit now reports zero vulnerabilities. No code changes needed: the app has no middleware, rewrites, server actions, CSP nonces, or next/image usage, so none of the fixed CVEs required app-side work. * build(deps): regenerate next-env.d.ts for next 16.3 - build(deps): pick up the root-params.d.ts reference next 16.3 adds - ci(hooks): pass --no-errors-on-unmatched to the biome pre-commit job so committing only biome-ignored files (like next-env.d.ts) doesn't fail * docs(roadmap): track web-analytics table coverage follow-ups (ops X37) * chore: ignore .claude/ with a tracked line (#59) It was covered only by .git/info/exclude, which protects one clone and nobody else's. Without a tracked line, `git add -A` in the parent stages a nested worktree as an embedded gitlink and `git clean -ffd` deletes it. * docs(roadmap): point acquisition-data item at the ops analytics standard * docs(roadmap): pageleave ratio is structural; note the bot-detection canary caveat * chore(config): add .planning/ to .gitignore (#62) * fix(analytics): preserve validated referring hostnames (#64) * fix(site): restore hydration and reject invalid signup bodies (#66) --------- Co-authored-by: biggest-littlest <zap_inane.2p@icloud.com>
3 participants
The live page rendered HTML but its CSP blocked Next’s inline Flight scripts, leaving the theme control and 3D scene uninitialized. Generate a fresh request nonce, forward the same policy to Next, and apply it to the theme script. HTML now renders per request; static asset caching stays in place.
Reject null, arrays, and primitive newsletter bodies with 400 before reading email. Update the setup/deployment docs to reflect the working Git integration and add production-browser checks to the existing CI.
Verified red first, then green: build, lint, typecheck, 28 existing tests, and 4 production-browser/API tests covering actual theme interaction, mounted canvas, fresh caller-independent nonces, blocked unauthorized inline scripts, and malformed bodies. Independent scoped review found no material issues.
Summary by CodeRabbit
Security
Bug Fixes
Tests
Documentation