feat: harden public write endpoints with zero-trust controls#726
feat: harden public write endpoints with zero-trust controls#7262witstudios merged 3 commits intomasterfrom
Conversation
Add rate limiting, payload size caps, and Zod schema validation to /api/track and /api/contact endpoints. These public endpoints now enforce strict abuse controls while remaining unauthenticated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis change hardens two public API endpoints—/api/contact and /api/track—with compensating zero-trust controls: distributed IP-based rate limiting, payload size caps, strict schema validation, and comprehensive request/response error handling. Both endpoints now enforce abuse constraints while maintaining their public access model. Rate-limit presets are updated to reflect stricter policies. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ContactAPI as /api/contact<br/>POST Handler
participant RateLimiter as Rate Limiter<br/>(IP-based)
participant SchemaValidator as Schema<br/>Validator
participant Database as Contact<br/>Submissions DB
participant Logger
Client->>ContactAPI: POST /api/contact<br/>(name, email, subject, message)
ContactAPI->>RateLimiter: Check rate limit for IP
alt Rate Limited
RateLimiter-->>ContactAPI: Rate limit exceeded
ContactAPI-->>Logger: Log rate limit warning
ContactAPI-->>Client: 429 Retry-After
else Rate Limit OK
ContactAPI->>SchemaValidator: Validate payload<br/>(schema + Content-Length)
alt Validation Fails
SchemaValidator-->>ContactAPI: Invalid (400 or 413)
ContactAPI-->>Logger: Log validation error
ContactAPI-->>Client: 400/413 error response
else Validation OK
ContactAPI->>Database: Insert submission<br/>(trimmed fields)
alt DB Success
Database-->>ContactAPI: Inserted
ContactAPI-->>Logger: Log successful submission
ContactAPI-->>Client: 200 OK
else DB Error
Database-->>ContactAPI: Error
ContactAPI-->>Logger: Log DB error
ContactAPI-->>Client: 500 error response
end
end
end
sequenceDiagram
participant Client
participant TrackAPI as /api/track<br/>POST/PUT Handler
participant RateLimiter as Rate Limiter<br/>(IP-based)
participant SchemaValidator as Schema<br/>Validator
participant AuthHelper as Auth Helper<br/>(optional)
participant Analytics as Analytics<br/>Backend
participant Logger
Client->>TrackAPI: POST/PUT<br/>(event type + data)
TrackAPI->>RateLimiter: Check rate limit for IP
alt Rate Limited
RateLimiter-->>TrackAPI: Rate limit exceeded
TrackAPI-->>Logger: Log rate limit warning
TrackAPI-->>Client: 429 Retry-After
else Rate Limit OK
TrackAPI->>SchemaValidator: Validate payload<br/>(schema + Content-Length)
alt Validation Fails
SchemaValidator-->>TrackAPI: Invalid
TrackAPI-->>Logger: Log validation error
TrackAPI-->>Client: 400/413 error response
else Validation OK
TrackAPI->>AuthHelper: Attempt authenticate<br/>(optional, non-blocking)
AuthHelper-->>TrackAPI: userId or null
TrackAPI->>Analytics: Track event<br/>(enriched: IP, user-agent,<br/>timestamp, userId if auth ok)
Analytics-->>TrackAPI: Event recorded
TrackAPI-->>Logger: Log event tracking
TrackAPI-->>Client: 200 { ok: true }
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Provide fallback defaults for optional Zod schema fields passed to tracker functions that require string arguments. Fixes CI build failures caused by string | undefined not being assignable to string. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/src/app/api/track/route.ts (1)
182-206: PUT beacon path bypasses rate limiting for malformed requests.When
JSON.parse(text)fails at line 194, the catch block returns{ ok: true }without ever callingcheckDistributedRateLimit. An attacker could flood the server with malformed PUT requests without being rate-limited. Since no downstream resources (DB, trackers) are consumed in that path, the blast radius is limited to CPU/bandwidth — acceptable for now, but worth noting if you add heavier processing later.Consider moving the rate-limit check before the JSON parse, or adding it to the PUT handler directly:
Sketch: rate-limit in PUT before parsing
export async function PUT(request: Request) { try { + const ip = getClientIP(request); + const rateLimitResult = await checkDistributedRateLimit( + `track:ip:${ip}`, + DISTRIBUTED_RATE_LIMITS.TRACKING + ); + if (!rateLimitResult.allowed) { + return Response.json( + { error: 'Too many requests' }, + { status: 429, headers: { 'Retry-After': String(rateLimitResult.retryAfter || 60) } } + ); + } + const text = await request.text();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/track/route.ts` around lines 182 - 206, The PUT handler currently parses the body before calling any rate-limit, letting malformed requests bypass rate limiting; move or invoke the distributed rate limiter at the top of PUT by calling await checkDistributedRateLimit(request) (or the same helper used by POST) immediately after reading/validating the request size (after the MAX_PAYLOAD_BYTES check) and before JSON.parse, so malformed JSON will still be rate-checked; keep constructing newRequest and forwarding to POST unchanged and ensure any thrown rate-limit error is handled the same way as in POST.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/contact/route.ts`:
- Around line 54-61: The length check uses rawBody.length (UTF-16 code units)
which mis-measures bytes; replace that check with a true byte-length computation
(e.g., const byteLength = typeof Buffer !== 'undefined' ?
Buffer.byteLength(rawBody, 'utf8') : new TextEncoder().encode(rawBody).length)
and compare byteLength to MAX_PAYLOAD_BYTES before returning Response.json;
update the block that reads rawBody (the request.text() / rawBody variable) to
use this byteLength variable in place of rawBody.length so multi-byte characters
are correctly bounded.
- Around line 96-99: The log call is recording raw PII—mask the email before
logging by transforming email.trim() into the established masked form (e.g.,
keep first 3 chars of the local part then '***' + '@' + domain) and pass that
masked value to loggers.api.info instead of the raw email; update the place
where loggers.api.info('Contact submission received', { ip, email: email.trim()
}) is called to compute a maskedEmail (use the same masking logic used in
signup-passkey/route.ts) and log email: maskedEmail.
In `@apps/web/src/app/api/track/route.ts`:
- Around line 76-83: The size check uses rawBody.length (UTF-16 code units)
which can misreport payload bytes; replace that check with a true byte-length
calculation (e.g., use Buffer.byteLength(rawBody, 'utf-8') or
TextEncoder().encode(rawBody).length) when comparing against MAX_PAYLOAD_BYTES
in the route handler that reads request.text(); update the conditional that
returns Response.json({ error: 'Payload too large' }, { status: 413 }) to use
the byte-length value (keep rawBody and MAX_PAYLOAD_BYTES names intact).
---
Nitpick comments:
In `@apps/web/src/app/api/track/route.ts`:
- Around line 182-206: The PUT handler currently parses the body before calling
any rate-limit, letting malformed requests bypass rate limiting; move or invoke
the distributed rate limiter at the top of PUT by calling await
checkDistributedRateLimit(request) (or the same helper used by POST) immediately
after reading/validating the request size (after the MAX_PAYLOAD_BYTES check)
and before JSON.parse, so malformed JSON will still be rate-checked; keep
constructing newRequest and forwarding to POST unchanged and ensure any thrown
rate-limit error is handled the same way as in POST.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/src/app/api/contact/__tests__/route.test.tsapps/web/src/app/api/contact/route.tsapps/web/src/app/api/track/__tests__/route.test.tsapps/web/src/app/api/track/route.tspackages/lib/src/security/__tests__/distributed-rate-limit.test.tspackages/lib/src/security/distributed-rate-limit.ts
- Use Buffer.byteLength() instead of string.length for payload size checks in both track and contact routes (handles multi-byte UTF-8) - Mask email in contact form log output to prevent PII exposure, matching the existing codebase pattern (e.g., jo***@example.com) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Closes #564
Summary
Hardened the two public write endpoints (
/api/trackand/api/contact) with zero-trust abuse controls. Added rate limiting via the existing distributed rate limiter, payload size caps, and strict Zod schema validation to both endpoints. Created 33 tests covering all security properties.Changes
TRACKINGrate limit config (100 req/min), updatedCONTACT_FORMconfig from 5/hour to 10/minute for the public web endpointFollow-up fixes
data.feature || 'unknown',data.message || 'Unknown error') — resolved CI build failuresBuffer.byteLength()instead ofstring.lengthfor byte-accurate payload size checks; masked email PII in contact form logs matching codebase pattern (jo***@example.com)Notes
Response.json()(standard Web API) instead ofNextResponse.json()to avoid unnecessarynext/serverimport — matches the pattern used by the marketing app's contact routedefaultcase that tracked any event namezod/v4matching the project convention seen inapps/web/src/app/api/feedback/route.tsTest plan
/api/track(rate limiting, payload size, schema validation, beacon support)/api/contact(rate limiting, payload size, schema validation, DB errors)🤖 Generated with Claude Code