fix(app-router): reject Route Handlers as interception source routes - #2732
Conversation
An RSC request whose path has no App route match is promoted to an interception source route selected by the `x-vinext-interception-context` header. `findIntercept` gates that header against the intercepting route's pattern with descendants allowed, then resolves the claimed pathname through the route trie and returns whichever concrete route it lands on. When that route is a Route Handler, the promoted match reaches the handler dispatch branch, so a crafted header executes a `route.ts` that merely lives under the intercepting route, having only run middleware for the requested target path. Applications that guard route handlers with middleware path checks therefore lose that boundary: a request to an interception-only target returns the protected handler's response. A Route Handler has no page, layouts, or parallel slots, so it can never own or sit inside an interception source tree. Skip it when resolving the concrete descendant source route and fall back to the slot owner, which is the fixed destination Next.js' generated interception rewrite targets. Descendant page routes still resolve concretely so dynamic source params survive. The interception source pathname remains unauthenticated, matching Next.js' `Next-URL` gating. Promoting a descendant *page* route can still render a middleware-guarded page for a target it does not own; closing that requires deciding whether descendant promotion should exist at all, and is left unchanged here.
|
@codex review |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba1cd9d89e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Performance benchmarksCompared 1 improved · 0 regressed · 5 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
…ource Falling back from a rejected Route Handler resolved the slot owner's params with an exact pattern match against the claimed source pathname. That match can never succeed on this path: the source was approved by the descendants-allowed gate precisely because it carries extra segments beyond the owner's pattern, so the match returns null and `sourceMatchedParams` ends up empty. `matchInterceptRoute` derives the promoted owner's params solely from that object, so a dynamic owner such as `/[locale]/feed` rendered with no `locale` for a source of `/en/feed/admin`. Take the owner's params from the prefix the source gate already approved when the exact match fails. The same recovery covers a descendant source that resolves to no concrete route at all, which had the identical gap before this branch existed. The legacy manifest shape, which has no declared `sourceMatchPattern`, still requires an exact match so its secondary gate keeps rejecting unrelated sources.
|
/bigbonk review for issues |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: reject Route Handlers as interception source routes
I traced the full request path and this is a correct, well-scoped fix for the described authorization bypass. Summary of what I verified:
The vulnerability is real and this closes it at the right layer. matchInterceptRoute in entries/app-rsc-entry.ts:1224 returns routes[intercept.sourceRouteIndex], which flows into interceptionPreActionMatch -> preActionMatch in app-rsc-handler.ts:889-896 and then into route-handler dispatch. Putting the guard inside findIntercept means every caller (the RSC promotion path, getSourceRoute in app-page-request.ts:717, server actions, sibling intercepts) inherits it, rather than each dispatch site guarding separately. This matches the ownership decision in the PR description.
Both handler cases are covered:
- Concrete descendant handler (
matchedSourceRoute.routeis a handler) ->concreteSourceRouteisnull, falls back to the slot owner index (app-rsc-route-matching.ts:296-304). - Fallback owner is itself a handler ->
continueat line 312, so no handler index can ever be returned assourceRouteIndex. This correctly covers theroute.ts-with-adjacent-slots shape and sibling intercepts (whosesourceRouteIndexis the declaring route).
The Codex P2 (dynamic owner params) is already addressed. matchSlotOwnerSourceParams handles exactly the /:locale/feed <- /en/feed/admin case Codex raised: exact match returns null for the extra segment, then the descendants-allowed prefix branch calls extractRawParamsForMatchedRoute and yields { locale: "en" }. The keeps the slot owner's dynamic params when falling back from a descendant test asserts this directly, including the no-concrete-route and exact-source variants.
Parity and lazy classification are handled. isAppRouteHandlerRoute keys on both routeHandler != null and typeof __loadRouteHandler === "function", so a lazy handler is classified consistently before/after first load - the lazyMatcher test proves it.
Verification I ran locally:
vp test run tests/app-rsc-route-matching.test.ts tests/app-rsc-handler.test.ts-> 170 passed.vp checkon the changed source + test -> format/lint/types clean.
No blocking issues. Two minor, non-blocking notes below.
One process note: the PR description mentions no changeset is included. Per this repo's workflow, the changeset is generated from the Conventional Commit subject in CI, so the fix(app-router): subject produces a patch changeset automatically - nothing to add manually.
|
Review posted successfully. SummaryI reviewed PR #2732 (security fix rejecting Route Handlers as interception source routes) and posted a Verdict: correct and well-scoped fix, no blocking issues. What I verified by tracing the code:
The two inline comments are documentation notes (the |
2 participants
Warning
This is a high severity security fix. It closes an authorization bypass in the App Router RSC request path that lets an unauthenticated remote client execute a Route Handler which middleware path checks were intended to protect. Please treat this as release-blocking.
Overview
findInterceptrejects Route Handler concrete sources and validates the final fallback owner before promotion; page owners and descendants remain unchangedWhy
Route interception is gated on a client header. vinext reads
x-vinext-interception-context; Next.js readsNext-URL. Neither is authenticated, and neither can be, because it is just the browser reporting its current pathname. That makes the header safe to use as a gate on an intercept the developer already declared, and unsafe to use as a selector for which route the server dispatches.Next.js keeps to that distinction.
generateInterceptionRoutesRewritesemits a rewrite whosedestinationis fixed at build time to the intercepting route's app path, gated by ahascondition matchingNext-URLagainst^<interceptingRoute>(?:/.*)?$. Source params come from matching the header against the intercepting route pattern. Next.js never resolvesNext-URLto a concrete route and never dispatches whatever it lands on.vinext's
findInterceptdoes resolve it. After the source pathname passes the descendants-allowed pattern gate, the matcher re-resolves it through the route trie and returns whichever concrete route it hits assourceRouteIndex(added in #2042 so dynamic descendant source params survive). Since #2256, an RSC request whose path has no direct App route match promotes that resolved route into the request'smatch, which then flows into the normal dispatch branches.Those two behaviours compose into a route confusion. Middleware has already run, for the requested target path. A request naming a target that has no direct route, plus a header naming a descendant of the intercepting route, promotes that descendant. When the descendant is a Route Handler, dispatch executes it. An application that authorizes route handlers with middleware path checks never sees the protected path, so the guard does not fire and the handler runs.
route.tsis not a renderable interception source, although the route graph may retain slots discovered beside itWhat changed
__loadRouteHandler, consistently before and after first loadMaintainer review path
packages/vinext/src/server/app-rsc-route-matching.ts—findIntercept'sconcreteSourceRouteresolution. This is the whole behavioural change and the ownership decision: the guard sits in the shared matcher so every caller inherits it rather than each dispatch site guarding separately.tests/app-rsc-route-matching.test.ts— regression proof, including the lazy-handler classification case and the preserved page-descendant behaviour.Validation
createAppRscRouteMatcherthroughcreateAppRscHandler: a request to an interception-only target with a header naming a middleware-protected Route Handler returned that handler's response, while a direct request to the same path was denied by middleware. After the change the handler is never dispatched. A full matcher-to-handler regression also proves a Route Handler slot owner remains directly reachable while forged interception promotion is rejected.app-rsc-route-matching,app-rsc-handler,app-server-action-execution,app-page-request,app-page-dispatch. 364 tests pass.pnpm run check(format, lint, types, Next.js type sync, shim types) clean.route.tsbeneath an intercepting route, so no existing covered behaviour changes.Commands
Risk / compatibility
Low risk, and the reason is that the removed capability has no legitimate use. Interception renders a page into a parallel slot, so a Route Handler could never have been a working interception source. Any request that previously reached a handler this way was already a route confusion.
Non-goals
children. Closing that means deciding whether the concrete descendant resolution from fix(app-router): preserve dynamic interception source routes #2042 should exist at all, versus always dispatching the slot owner and taking source params from the intercepting-route pattern as Next.js does. That is a behavioural change to a shipped fix with e2e fixtures behind it and belongs in its own PR.Next-URL, and doing so would execute user middleware twice per interception navigation, duplicating side effects such asSet-Cookie, session rotation, and rate limiting.References
generate-interception-routes-rewrites.tsNext-URLused only as ahasgate