fix(security): harden contact API, patch high-severity deps, unblock CI - #91
Merged
Merged
Conversation
Three issues found auditing the public attack surface (the contact API is
the only route accepting untrusted input).
1. CSRF origin bypass (highest impact). The allowlist accepted any host
matching `endsWith('.vercel.app') && includes('zachlamb')`. Vercel
project names are globally claimable, so anyone could deploy a project
named `zachlamb-evil`, get `zachlamb-evil.vercel.app`, and pass the
check — letting an attacker-controlled page drive the contact form and
send mail to the owner's inbox with an attacker-chosen reply-to.
Now matches exactly against the server-only env vars Vercel injects
(VERCEL_PROJECT_PRODUCTION_URL / VERCEL_BRANCH_URL / VERCEL_URL), so
preview deploys keep working with no wildcard left to abuse.
2. Plaintext localhost origins were trusted in production. Now gated to
non-production only.
3. No type validation on the JSON body. Fields were typed as optional
strings but never checked, so a non-string `name` reached `.trim()`
and threw a TypeError that escaped the handler as an opaque 500.
Body is now parsed as unknown and each field type-checked before use,
per parse-don't-validate. Values are trimmed once and the length caps
apply to the trimmed string.
Also encodes the JSON-LD payload through a new lib/json-ld.ts, which
escapes `<` so a value containing a closing script tag cannot break out
of the dangerouslySetInnerHTML sink. Inputs are static today; this makes
the sink safe by construction rather than by assumption.
The two origin regressions were verified against the old code: both fail
before the fix and pass after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`npm run format:check` was already failing on 17 files at the base commit, which fails CI's Quality job independently of any code change. These are whitespace-only edits produced by `npm run format` — no behavior change. Full suite (288 tests), lint, and typecheck pass. Kept separate from the security commit so that diff stays reviewable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`npm audit --audit-level=high` was failing at the base commit, which fails CI's Quality job. Resolved via `npm audit fix` (in-range, no API changes): next 16.2.6 -> 16.2.12 vite 8.0.10 -> 8.2.0 (GHSA-fx2h-pf6j-xcff, GHSA-v6wh-96g9-6wx3) postcss 8.5.12 -> 8.5.25 (GHSA-r28c-9q8g-f849, path traversal) js-yaml 4.1.1 -> 4.3.1 brace-expansion (transitive) sharp needed an override rather than `audit fix --force`, which wanted to downgrade next to 14.2.35. sharp is an optionalDependency of next (^0.34.5) and the libvips CVEs (CVE-2026-33327/33328/35590/35591) are fixed in 0.35.x, so pinning `sharp: ^0.35.0` in overrides patches it while keeping Next 16. The stale `postcss: ^8.5.10` override was bumped past the advisory range. `npm audit --audit-level=high` now reports 0 vulnerabilities. Lint, format:check, typecheck, 288 tests, and `next build` all pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Security audit of the public attack surface, plus the fixes required to get CI green.
What was audited
The site is a static Next.js 16 portfolio. The only route accepting untrusted input is
POST /api/contact, so that's where the real surface is. Also reviewed: theproxy.tslocale middleware, the JSON-LD injection sink, security headers/CSP, error boundaries, the Upstash rate limiter, and the service-worker kill switch.Much of the app was already well hardened (origin gate, CR/LF header stripping, PII-free Sentry capture, fail-closed
RESEND_FROM_EMAIL, strong CSP + HSTS). Three real issues remained.Findings fixed
1. CSRF origin bypass — the significant one.
app/api/contact/route.tsThe allowlist accepted any host satisfying
endsWith('.vercel.app') && includes('zachlamb'). Vercel project names are globally claimable, so anyone could deploy a project namedzachlamb-evil, getzachlamb-evil.vercel.app, and pass the check. That let an attacker-controlled page drive the contact form cross-origin and send mail into the owner's inbox with an attacker-chosenreply-to— a phishing primitive, not just spam.Now matched exactly against the server-only env vars Vercel injects (
VERCEL_PROJECT_PRODUCTION_URL/VERCEL_BRANCH_URL/VERCEL_URL). Preview deploys keep working; no wildcard remains to abuse.2. Plaintext
localhostorigins trusted in production. Now gated to non-production only.3. No type validation on the JSON body. Fields were typed
string | undefinedbut never checked, so a non-stringnamereached.trim()and threw aTypeErrorthat escaped the handler as an opaque 500. The body is now parsed asunknownwith each field type-checked before use (parse, don't validate), values trimmed once, and length caps applied to the trimmed string so whitespace padding can't smuggle an oversized payload.Defense in depth: the JSON-LD payload now goes through
lib/json-ld.ts, which escapes<so a value containing a closingscripttag can't break out of thedangerouslySetInnerHTMLsink. Inputs are static today — this makes the sink safe by construction rather than by assumption.CI was already red on main
Independent of the security work, three CI gates were failing at the base commit:
npm run format:check— 17 pre-existing unformatted filesnpm audit --audit-level=high— 6 high-severity advisoriesFixed in separate commits so the security diff stays reviewable. Dependency patches:
next 16.2.6→16.2.12,vite 8.0.10→8.2.0,postcss 8.5.12→8.5.25,js-yaml 4.1.1→4.3.1.sharpneeded an override rather thanaudit fix --force, which wanted to downgrade Next to 14.2.35; it's an optional dep of Next and the libvips CVEs are fixed in 0.35.x.Verification
npm audit --audit-level=high→ 0 vulnerabilitiesnext buildall pass locallyNote
GitHub reports 79 Dependabot alerts on the default branch. This PR clears everything
npm audit --audit-level=highsees in the resolved tree; the rest appear to be alerts against the lockfile history / lower severities and are worth a separate pass.Co-Authored-By: Claude Opus 5 noreply@anthropic.com
🤖 Generated with Claude Code