Skip to content

fix: CSRF protection is inert on all /api/* routes - #656

Open
vedant7007 wants to merge 1 commit into
vishnukothakapu:mainfrom
vedant7007:fix/csrf-api-matcher
Open

fix: CSRF protection is inert on all /api/* routes#656
vedant7007 wants to merge 1 commit into
vishnukothakapu:mainfrom
vedant7007:fix/csrf-api-matcher

Conversation

@vedant7007

@vedant7007 vedant7007 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Closes #646

Problem

applyCsrfProtection is imported and invoked in middleware.ts, but the middleware config.matcher excluded api ("/((?!api|_next/...).*)"). So the middleware — and therefore the CSRF check — never ran for any /api/* route, leaving every mutating API endpoint unprotected.

Fix

  • Include /api in the matcher.
  • Short-circuit API requests at the top of the middleware to run only applyCsrfProtection, so the page-navigation logic (auth redirects, custom-domain rewrite, CSP nonce) is not applied to API calls — preserving current behaviour for API routes apart from adding CSRF.

The existing CSRF decision logic already scopes protection to POST/PUT/PATCH/DELETE and excludes safe paths (/api/auth, /api/csrf, /api/contact-us, /api/links/click, /api/analytics/aggregate), so no legitimate request is affected.

Testing

  • npx tsc --noEmit clean on middleware.ts.
  • Existing CSRF unit tests (lib/csrf.test.ts, lib/middleware/csrf.test.ts) pass (8/8).

Contributing as part of Elite Coders Summer of Code (ECSoC 2026).

Summary by CodeRabbit

  • Bug Fixes
    • API requests now receive CSRF protection before other request processing.
    • API routes avoid page-specific authentication, domain rewriting, and content security handling, improving compatibility and reliability.
    • Updated request matching ensures API endpoints are handled consistently.

The middleware matcher excluded /api, so applyCsrfProtection (imported and
called in the middleware) never ran for any API route — CSRF was inert
exactly where the mutating endpoints live. Include /api in the matcher and
short-circuit API requests to CSRF-only, so the page-only logic (auth
redirects, custom-domain rewrite, CSP nonce) still doesn't run on API calls.

Fixes vishnukothakapu#646
@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

@vedant7007 is attempting to deploy a commit to the vishnukothakapu's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The middleware now processes /api requests before authentication and page-specific logic. It applies CSRF protection and continues only when no CSRF response exists. The matcher includes API paths.

Changes

API CSRF Protection

Layer / File(s) Summary
Route matching and CSRF short-circuit
middleware.ts
API requests now reach the middleware, receive CSRF protection, and bypass authentication redirects, domain rewriting, CSP nonce generation, and page-specific handling.

Estimated code review effort: 3 (Moderate) | ~15 minutes

Possibly related issues

  • #515 — The change includes /api routes in the middleware matcher so CSRF protection runs on API requests.
  • #534 — The change applies CSRF protection before other middleware processing for API requests.

Suggested reviewers: vishnukothakapu, jivan-patel

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: enabling CSRF protection for all /api/* routes.
Linked Issues check ✅ Passed The middleware matcher includes /api/* routes and short-circuits them through applyCsrfProtection, satisfying issue #646.
Out of Scope Changes check ✅ Passed The changes remain within scope by limiting API requests to CSRF handling and skipping unrelated page middleware logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@middleware.ts`:
- Around line 14-16: Update the API branch in the middleware to match only the
exact `/api` path or paths beginning with `/api/`, rather than any pathname
starting with `/api`. Preserve the existing applyCsrfProtection flow for those
API paths and allow paths such as `/api-docs` and `/apiary` to continue through
the normal middleware handling.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 43f63342-e38f-45a3-8fe7-48213b13a384

📥 Commits

Reviewing files that changed from the base of the PR and between 20d95d9 and 0e01c53.

📒 Files selected for processing (1)
  • middleware.ts

Comment thread middleware.ts
Comment on lines +14 to +16
if (pathname.startsWith("/api")) {
const csrfResponse = await applyCsrfProtection(req);
return csrfResponse ?? NextResponse.next();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restrict the API branch to an API path boundary.

Line 14 also matches non-API paths such as /api-docs and /apiary. Those requests skip page authentication redirects, custom-domain rewrites, and CSP handling. Match /api exactly or require the /api/ prefix.

Proposed fix
-    if (pathname.startsWith("/api")) {
+    if (pathname === "/api" || pathname.startsWith("/api/")) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (pathname.startsWith("/api")) {
const csrfResponse = await applyCsrfProtection(req);
return csrfResponse ?? NextResponse.next();
if (pathname === "/api" || pathname.startsWith("/api/")) {
const csrfResponse = await applyCsrfProtection(req);
return csrfResponse ?? NextResponse.next();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@middleware.ts` around lines 14 - 16, Update the API branch in the middleware
to match only the exact `/api` path or paths beginning with `/api/`, rather than
any pathname starting with `/api`. Preserve the existing applyCsrfProtection
flow for those API paths and allow paths such as `/api-docs` and `/apiary` to
continue through the normal middleware handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] CSRF protection is inert on all /api/* routes (middleware matcher excludes 'api')

1 participant