fix(nextjs): stop flagging response.headers and local Map/Set/Headers in GET handlers (#206) by NisargIO · Pull Request #263 · millionco/react-doctor

NisargIO · 2026-05-16T01:12:50Z

Summary

Re-opens #260, which GitHub auto-closed after a force-push during rebase onto main. Same diff, now rebased and format:check-clean.

Rewrites nextjs-no-side-effect-in-get-handler precision so it no longer floods Next.js codebases with false positives from response-shaping calls, while still catching real CSRF-relevant server-state mutations.

Fixes #206. Supersedes #209, #211, #233, #238. The handler-resolution improvements from #251 (cron route skip + depth-bounded const GET = withAuth(handler) resolution) are also folded in; the broader rule expansion in #251 remains an independent PR.

What changes for users

No longer flagged (false positives — were ~138 errors in one Next.js 14 codebase)

res.headers.set/append/delete(...) and any chain ending in .headers (handles NextResponse.json({...}).headers.set(...), (await fetcher()).headers.append(...)).
Locally-constructed new Map/Set/WeakMap/WeakSet/Headers/URLSearchParams/FormData/Response/NextResponse(...) bindings and any mutation on those aliases.
new URL(...).searchParams.set(...) and any .searchParams.*() chain.
headers() / (await headers()) from next/headers (returns ReadonlyHeaders; any mutation would throw at runtime) and any aliased const h = headers(); h.get(...).
Route handlers under /cron/ or /jobs/cron/ — Vercel Cron always invokes GET and is expected to do real work.

Still flagged (real CSRF-relevant side effects)

ORM mutations: drizzle db.update(table).set({...}).where(...), prisma.user.create(...), db.insert(...), repository.upsert(...).
Module-level mutable state: const cache = new Map() declared outside the handler, then cache.set(...) inside.
fetch(url, { method: 'POST' | 'PUT' | 'DELETE' | 'PATCH' }).
cookies().set/append/delete() in all forms — direct, (await cookies()).set(...), and (new) aliased const cs = await cookies(); cs.set(...) (the previously-missed aliased form).
Mutating route segments (/logout, /signout, /unsubscribe, /delete, …).

Implementation

packages/oxlint-plugin-react-doctor/src/plugin/utils/find-side-effect.ts now takes an options bag and short-circuits any mutation call whose receiver chain is structurally "safe" (a .headers/.searchParams access, a new X() of a safe constructor, a Response/NextResponse factory, or an Identifier in a per-handler safe-bindings set). The rule itself collects two binding sets before scanning:

collectLocallyScopedSafeBindings(handlerBody) — const X = new Map(), const X = new Headers(), const X = NextResponse.json(...), const X = await headers(), etc.
collectLocallyScopedCookieBindings(handlerBody) — const X = cookies() and const X = await cookies(). Any <alias>.set/append/delete() is reported as cookies().<method>() so the diagnostic stays consistent.

The rule also gained depth-bounded handler-binding resolution (GET_HANDLER_BINDING_RESOLUTION_DEPTH = 3) so export const GET = withAuth(handler) and export const GET = app.get('/x', handler) chains get scanned correctly.

Test plan

packages/react-doctor/tests/regressions/nextjs-get-side-effects.test.ts (new) builds 23 isolated synthetic projects covering:

13 false-negative cases that must produce 0 hits, including the verbatim issue False positive: nextjs-no-side-effect-in-get-handler flags Response.headers.set() and similar Headers API calls #206 repro and a 138x stress test that synthesises the original reported codebase.
9 true-positive cases that must produce exactly 1 hit each, with the diagnostic naming the right operation (drizzle, prisma, module-level cache, mutating fetch, direct/awaited/aliased cookies, mutating route segment).

packages/react-doctor/tests/run-oxlint/nextjs.test.ts also gains three integration assertions on new fixtures under tests/fixtures/nextjs-app/src/app/api/ to lock the behavior in the shared Next.js fixture project.

nr typecheck — all packages pass
nr lint — 0 warnings / 0 errors
nr test — 909/909 pass
nr format:check — clean

Made with Cursor

… in GET handlers (#206) Rewrites `nextjs-no-side-effect-in-get-handler` precision so it no longer floods Next.js codebases with false positives from `response.headers.set()` and request-scoped `new Map/Set/Headers/URLSearchParams/FormData(...)` mutations, while still catching real CSRF-relevant writes: drizzle/prisma ORM mutations, module-level mutable state, mutating fetch, and all three forms of `cookies().set/delete()` including aliased `const cs = await cookies(); cs.set(...)`. Also folds in the safe handler-resolution improvements from PR #251 (cron route skip and depth-bounded `const GET = withAuth(handler)` resolution). Supersedes #209, #211, #233, #238. Co-authored-by: Cursor <cursoragent@cursor.com>

The `isHeadersPropertyAccess` predicate was added during the issue #206 work but never imported — its check (`.headers` member access) is fully covered by `isSafeIntrinsicMemberAccess` in `is-safe-mutable-receiver- source.ts` (which also handles `.searchParams`). Removing the dead file to comply with the workspace "remove unused code and don't repeat yourself" rule. Co-authored-by: Cursor <cursoragent@cursor.com>

…ies-call utils `isCookiesCall` in `collect-locally-scoped-cookie-bindings.ts` and `isDirectCookiesCall` in `find-side-effect.ts` were byte-identical, and `isCookiesInit` was structurally the same as `(direct || awaited)` from `find-side-effect.ts`. Extract to single-purpose utils (one per file, per workspace convention) and rewire both consumers so the cookies-detection predicate has one definition. Co-authored-by: Cursor <cursoragent@cursor.com>

…ollectors `collectLocallyScopedSafeBindings` / `collectLocallyScopedCookieBindings` use `walkInsideStatementBlocks`, which returns immediately at any function boundary. `tanstack-start-get-mutation` was passing the `.handler(fn)` callback NODE itself, so both binding sets came back empty — every aliased shape inside a TanStack Start handler was misclassified: - `const customHeaders = new Headers(); customHeaders.set(...)` leaked a false positive. - `const cs = cookies(); cs.set(...)` got reported as `cs.set()` instead of the canonical `cookies().set()`. Unwrap to `handlerFunction.body` (mirroring how the Next.js rule passes `handlerBody` after `resolveGetHandlerBodies`) and add four regression cases that pin this down. `findSideEffect` still gets the body too, which is a no-op for it (walkAst descends through functions) but keeps the inputs consistent across both consumers. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

reactreview · 2026-05-16T01:12:54Z

🔴 React Review — 0/100 (unchanged) · 3 ❌ errors · 1 ⚠️ warning

Copy prompt for agent

Check if these React Review issues are valid. If so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.

Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff

Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issues instead of changing or suppressing the rules.

React Review found 3 errors and 1 warning. This PR leaves the React health score unchanged.

<file name="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx">

<violation number="1" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx:13">
Severity: Error

GET handler has side effects (.set()) — use POST to prevent CSRF and unintended prefetch triggers

Move the side effect to a POST handler and use a <form> or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF

Rule: `nextjs-no-side-effect-in-get-handler`
</violation>

</file>

<file name="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx">

<violation number="1" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx:9">
Severity: Error

GET handler has side effects (db.insert()) — use POST to prevent CSRF and unintended prefetch triggers

Move the side effect to a POST handler and use a <form> or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF

Rule: `nextjs-no-side-effect-in-get-handler`
</violation>

</file>

<file name="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx">

<violation number="1" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx:15">
Severity: Error

GET handler has side effects (cache.set()) — use POST to prevent CSRF and unintended prefetch triggers

Move the side effect to a POST handler and use a <form> or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF

Rule: `nextjs-no-side-effect-in-get-handler`
</violation>

<violation number="2" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx:18">
Severity: Warning

fetch("/api/notify") in a Server Component / route handler defaults to forever-caching — pass { next: { revalidate: <seconds> } } / { next: { tags: [...] } } / { cache: "no-store" } so stale data doesn't quietly persist

Pass `{ next: { revalidate: <seconds> } }` (or `cache: "no-store"` / `next: { tags: [...] }`) so stale cached data doesn't silently persist

Rule: `server-fetch-without-revalidate`
</violation>

</file>

^{Reviewed by react-review for commit bd9778a. Configure here.}

vercel · 2026-05-16T01:12:56Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
react-doctor-website	Ready	Preview, Comment	May 16, 2026 1:12am

reactreview · 2026-05-16T01:13:04Z

+
+// Verbatim repro from issue #206 — used to flood codebases with false
+// positives. Response header shaping must not fire the rule.
+export async function GET(req: NextRequest, ctx: RouteContext) {


Caution
GET handler has side effects (.set()) — use POST to prevent CSRF and unintended prefetch triggers

Move the side effect to a POST handler and use a
or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF

Rule: nextjs-no-side-effect-in-get-handler

Disable nextjs-no-side-effect-in-get-handler for this line

Suggested change

export async function GET(req: NextRequest, ctx: RouteContext) {

// react-doctor-disable-next-line nextjs-no-side-effect-in-get-handler

export async function GET(req: NextRequest, ctx: RouteContext) {

Copy prompt for agent

Check if this React Review issue is valid. If so, understand the root cause and fix it. Run this before and after your changes to verify the result: npx react-doctor@latest --verbose --diff Do not modify the react-doctor configuration unless explicitly asked. Fix the underlying code issue instead of changing or suppressing the rule. <file name="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx"> <violation number="1" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/documents/[id]/route.tsx:13"> Severity: Error GET handler has side effects (.set()) — use POST to prevent CSRF and unintended prefetch triggers Move the side effect to a POST handler and use a <form> or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF Rule: `nextjs-no-side-effect-in-get-handler` </violation> </file>

^{Reviewed by react-review for commit 71ecf0a. Configure here.}

reactreview · 2026-05-16T01:13:04Z

+
+// Vercel Cron always invokes GET — real side effects are expected
+// and the rule must not fire here.
+export async function GET() {


Caution
GET handler has side effects (db.insert()) — use POST to prevent CSRF and unintended prefetch triggers

Move the side effect to a POST handler and use a
or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF

Rule: nextjs-no-side-effect-in-get-handler

Disable nextjs-no-side-effect-in-get-handler for this line

Suggested change

export async function GET() {

// react-doctor-disable-next-line nextjs-no-side-effect-in-get-handler

export async function GET() {

Copy prompt for agent

Check if this React Review issue is valid. If so, understand the root cause and fix it. Run this before and after your changes to verify the result: npx react-doctor@latest --verbose --diff Do not modify the react-doctor configuration unless explicitly asked. Fix the underlying code issue instead of changing or suppressing the rule. <file name="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx"> <violation number="1" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/cron/refresh/route.tsx:9"> Severity: Error GET handler has side effects (db.insert()) — use POST to prevent CSRF and unintended prefetch triggers Move the side effect to a POST handler and use a <form> or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF Rule: `nextjs-no-side-effect-in-get-handler` </violation> </file>

^{Reviewed by react-review for commit 71ecf0a. Configure here.}

reactreview · 2026-05-16T01:13:04Z

+// side effect (server state leaks across requests).
+const cache = new Map<string, unknown>();
+
+export async function GET() {


Caution
GET handler has side effects (cache.set()) — use POST to prevent CSRF and unintended prefetch triggers

Move the side effect to a POST handler and use a
or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF

Rule: nextjs-no-side-effect-in-get-handler

Disable nextjs-no-side-effect-in-get-handler for this line

Suggested change

export async function GET() {

// react-doctor-disable-next-line nextjs-no-side-effect-in-get-handler

export async function GET() {

Copy prompt for agent

Check if this React Review issue is valid. If so, understand the root cause and fix it. Run this before and after your changes to verify the result: npx react-doctor@latest --verbose --diff Do not modify the react-doctor configuration unless explicitly asked. Fix the underlying code issue instead of changing or suppressing the rule. <file name="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx"> <violation number="1" location="packages/react-doctor/tests/fixtures/nextjs-app/src/app/api/admin/route.tsx:15"> Severity: Error GET handler has side effects (cache.set()) — use POST to prevent CSRF and unintended prefetch triggers Move the side effect to a POST handler and use a <form> or fetch with method POST — GET requests can be triggered by prefetching and are vulnerable to CSRF Rule: `nextjs-no-side-effect-in-get-handler` </violation> </file>

^{Reviewed by react-review for commit 71ecf0a. Configure here.}

NisargIO · 2026-05-16T01:18:54Z

Closing in favor of reopened #260 (same diff, same branch).

NisargIO and others added 6 commits May 15, 2026 02:06

fr

5d631c9

chore: drop changeset (not used in this repo)

bd9778a

Co-authored-by: Cursor <cursoragent@cursor.com>

NisargIO mentioned this pull request May 16, 2026

fix(server-auth-actions): accept member-expression auth calls like auth0.getSession() (#239) #264

Closed

3 tasks

NisargIO requested a review from aidenybai May 16, 2026 01:13

reactreview Bot reviewed May 16, 2026

View reviewed changes

NisargIO force-pushed the fix/nextjs-get-handler-side-effects-206 branch from 71ecf0a to bd9778a Compare May 16, 2026 01:18

NisargIO closed this May 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(nextjs): stop flagging response.headers and local Map/Set/Headers in GET handlers (#206)#263

fix(nextjs): stop flagging response.headers and local Map/Set/Headers in GET handlers (#206)#263
NisargIO wants to merge 6 commits into
mainfrom
fix/nextjs-get-handler-side-effects-206

NisargIO commented May 16, 2026

Uh oh!

reactreview Bot commented May 16, 2026 •

edited

Loading

Uh oh!

vercel Bot commented May 16, 2026

Uh oh!

reactreview Bot May 16, 2026

Uh oh!

reactreview Bot May 16, 2026

Uh oh!

reactreview Bot May 16, 2026

Uh oh!

NisargIO commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	export async function GET(req: NextRequest, ctx: RouteContext) {
	// react-doctor-disable-next-line nextjs-no-side-effect-in-get-handler
	export async function GET(req: NextRequest, ctx: RouteContext) {

	export async function GET() {
	// react-doctor-disable-next-line nextjs-no-side-effect-in-get-handler
	export async function GET() {

Conversation

NisargIO commented May 16, 2026

Summary

What changes for users

No longer flagged (false positives — were ~138 errors in one Next.js 14 codebase)

Still flagged (real CSRF-relevant side effects)

Implementation

Test plan

Uh oh!

reactreview Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented May 16, 2026

Uh oh!

reactreview Bot May 16, 2026

Choose a reason for hiding this comment

Uh oh!

reactreview Bot May 16, 2026

Choose a reason for hiding this comment

Uh oh!

reactreview Bot May 16, 2026

Choose a reason for hiding this comment

Uh oh!

NisargIO commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

reactreview Bot commented May 16, 2026 •

edited

Loading