fix(app-router): validate external RSC rewrites before proxying

[NathanDrake2406]

Overview

Goal	Prevent invalid outside-basePath RSC requests from reaching external config or middleware rewrite destinations.
Core change	Validate cache-busting state at every external-proxy boundary, before proxy I/O begins.
Middleware parity	Keep internal Flight state hidden from user middleware, then restore the original authenticated _rsc and Flight headers for the external destination.
Expected impact	Missing or stale _rsc values receive the canonical 307 consistently; valid external RSC rewrites retain the state their destination needs to render and validate Flight.

Why

RSC cache-busting validation is delayed for requests outside the configured basePath until a basePath: false rule or middleware rewrite claims them. Internal rewrites reached a delayed validation checkpoint, but external config and middleware rewrites returned their proxy response immediately. That allowed invalid GET and HEAD requests to bypass canonicalization.

Middleware also receives a request with internal Flight state removed, as Next.js does. Before this change, vinext reused that stripped request for external proxying, so valid middleware rewrites dropped _rsc and the Flight headers at the destination. The proxy path now retains a separate downstream request and restores the original Flight headers after middleware request-header overrides, making those authenticated headers non-overridable.

This matches Next.js v16.2.6's middleware adapter and external-rewrite fixture:

• Middleware adapter source
• External RSC middleware rewrite test

What changed

Scenario	Before	After
Invalid outside-basePath RSC request claimed by a config external rewrite	Proxied without a validated _rsc token	Returns the canonical 307 before upstream contact
Invalid outside-basePath RSC request claimed by a middleware external rewrite	Proxied before the delayed checkpoint	Returns the canonical 307, preserving safe middleware response headers and the framework-owned Location
Valid middleware external RSC rewrite	_rsc and Flight headers were stripped with the middleware-facing request	Destination receives the original _rsc, RSC, router-state, and prefetch headers
Internal rewrite or unclaimed outside-basePath request	Existing delayed validation or plain 404	Unchanged
POST/action traffic	Existing proxy behavior	Unchanged; validation remains GET/HEAD-only

Maintainer review path

1. packages/vinext/src/server/app-rsc-handler.ts for original-request validation and separate middleware/downstream requests.
2. packages/vinext/src/server/app-middleware.ts for the two pre-proxy gates, safe response-header merge, and non-overridable Flight-header restoration.
3. packages/vinext/src/entries/app-rsc-entry.ts for generated middleware wiring.
4. tests/app-rsc-handler.test.ts for config phases plus normal and forwarded middleware coverage.

Validation

• App RSC handler: 142 passed.
• RSC cache-busting: 37 passed.
• Entry template suite: 52 passed in BigBonk's checkout.
• Targeted format, lint, type, and diff checks passed.
• Exact-head CI: 47/47 jobs passed.
• Independent final review and BigBonk: clean/LGTM.

Risk / compatibility

• No public API or configuration shape changes.
• Validation runs only after an external rewrite claims an outside-basePath request.
• Middleware still cannot observe internal _rsc or Flight transport headers.
• Only the external downstream proxy receives the original authenticated Flight state, matching Next.js behavior.
• Middleware response cookies and custom headers survive a canonicalization response, but middleware cannot replace its framework-owned Location.

[NathanDrake2406]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: d741e8e178

ℹ️ 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".

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2754

npm i https://pkg.pr.new/create-vinext-app@2754

npm i https://pkg.pr.new/@vinext/types@2754

npm i https://pkg.pr.new/vinext@2754

commit: 03fd6d8

[github-actions]

Performance benchmarks

Compared 03fd6d8 against base dc05459 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	134.5 KB	134.5 KB	⚫ -0.0%
Client entry size (gzip)	vinext	122.1 KB	122.1 KB	⚫ -0.0%
Dev server cold start	vinext	2.99 s	2.96 s	⚫ -1.0%
Production build time	vinext	3.10 s	3.11 s	⚫ +0.3%
RSC entry closure size (gzip)	vinext	112.3 KB	112.3 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	189.7 KB	189.7 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review

The change is well-targeted and correct. The core insight is sound: internal rewrites set filesystemRouteEligible = true and then hit a post-rewrite validateClaimedOutsideBasePathRsc() checkpoint (lines 754/827/1065/1115), but external rewrites proxy inline inside applyRewrite() and returned before any checkpoint could run. Adding a pre-proxy validation callback at the external-match boundary closes that gap consistently across all three rewrite phases.

Things I verified:

• validateExternalRewriteRequest runs before clearRequestContext() and proxyExternalRequest(), and only reads request — no mutation or ordering hazard.
• Passing routeClaimed = true from external sites correctly mirrors the "route is now claimed" state that internal rewrites establish by flipping filesystemRouteEligible.
• The default-parameter form (routeClaimed = filesystemRouteEligible) is evaluated at call time, so the existing no-arg checkpoint callers still observe the current mutable value — no closure staleness.
• Validation uses the original request (with _rsc still present), consistent with the other two checkpoints at lines 643 and 716.
• The new test complements the existing valid-proxy test at line 1751: valid _rsc still proxies (200), invalid gets the canonical 307. The expect(receivedUrls).toEqual([]) assertion correctly proves zero upstream contact, and covering GET+HEAD across all three phases matches resolveInvalidRscCacheBustingRequest's method gate.
• Pages Router (pages-request-pipeline.ts) does not participate in _rsc cache-busting, so there is no dev/prod parity gap to mirror there — the mechanism is App Router-specific.

One potential follow-up (non-blocking): a middleware-driven external rewrite (proxyExternalMiddlewareRewrite in app-middleware.ts:164) of an outside-basePath RSC request appears to proxy immediately as well, before the line-754 checkpoint, and middleware rewrites also flip the route to eligible via didMiddlewareRewrite. That path is out of scope for this PR (config rewrites), and confirming whether Next.js validates _rsc ahead of a middleware external rewrite needs its own investigation against .nextjs-ref. Worth a tracking note rather than expanding this PR.

LGTM.

[ask-bonk]

Review posted successfully.

Summary

I reviewed PR #2754 (fix(app-router): validate external RSC rewrites before proxying) and posted a COMMENT review. The change is well-targeted and correct.

Verdict: LGTM

What the PR does: Internal config rewrites flip filesystemRouteEligible = true and then hit a post-rewrite validateClaimedOutsideBasePathRsc() checkpoint, but external rewrites proxy inline inside applyRewrite() and returned before any checkpoint ran. This allowed invalid outside-basePath RSC GET/HEAD requests to bypass canonicalization in all three rewrite phases. The fix injects a pre-proxy validation callback at the external-match boundary.

Verified correct:

• Validation runs before clearRequestContext()/proxyExternalRequest(), only reads the request — no ordering/mutation hazard.
• routeClaimed = true from external sites correctly mirrors the internal "route claimed" state.
• Default-parameter routeClaimed = filesystemRouteEligible evaluates at call time, so existing checkpoint callers still see the current mutable value.
• Test coverage is solid: GET+HEAD across all three phases, plus a zero-upstream-contact assertion.
• No Pages Router parity gap (RSC cache-busting is App Router-specific).

One non-blocking follow-up noted: a middleware-driven external rewrite (app-middleware.ts:164) of an outside-basePath RSC request appears to proxy immediately as well, before the same checkpoint. Out of scope for this PR, but worth a tracking note.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review

The code is correct, well-factored, and thoroughly tested. I verified all targeted suites pass locally (app-rsc-handler 142, app-rsc-cache-busting 37, entry-templates 52) and vp check is clean on the changed source files.

What I verified as correct:

• Every external-proxy path is now gated. applyRewrite() is the sole proxyExternalRequest call site for config rewrites, and all 6 call sites pass validateExternalRewriteRequest. Validation runs before clearRequestContext() and only reads request, so no ordering/mutation hazard.
• The default-parameter routeClaimed = filesystemRouteEligible is evaluated at call time, so the no-arg checkpoint at line 760 still observes the current mutable filesystemRouteEligible. External rewrites force routeClaimed = true, correctly mirroring the "route claimed" state internal rewrites establish.
• validationResponseWithMiddlewareHeaders captures location before the middleware-header merge and restores it after — necessary because mergeMiddlewareResponseHeaders uses set for non-additive headers and would otherwise clobber the canonical 307 Location. set-cookie merging is correctly additive. The middleware test asserts both.
• restoreFlightHeaders only clones when a header actually changes, so the no-override middleware path returns the request untouched.

Main concern — the PR title/body materially understates the scope. The title ("validate external RSC rewrites before proxying") and the "What changed" table describe only config beforeFiles/afterFiles/fallback. But this PR also:

1. Adds validation to both middleware external-rewrite paths (forwarded-context and executed-middleware) in app-middleware.ts — the ask-bonk review explicitly called the middleware path "out of scope for this PR," yet the diff clearly modifies it (that review appears to predate the middleware commits).
2. Changes which headers are forwarded to external origins from middleware rewrites. On main, the middleware external path stripped Flight headers via requestWithoutFlightHeaders(options.request). This PR switches to externalRewriteRequest ?? options.request + restoreFlightHeaders, so rsc, next-router-state-tree, next-router-prefetch, etc. (and _rsc) are now forwarded to the external destination.

The "Risk / compatibility" section says "No public API or configuration shape changes" and "Validation runs only after an external rewrite matches" — neither mentions this header-forwarding change, which is the most behaviorally significant part of the diff. It does bring the middleware path into parity with the config path (which already used normalizedUserlandRequest on main), and forwarding _rsc is what lets the destination validate the request, so I believe the change is intentional and correct. But because it changes what headers reach arbitrary external origins, it warrants: (a) an explicit note in the PR body, and (b) confirmation against .nextjs-ref that Next.js forwards Flight headers to middleware external rewrite destinations (AGENTS.md requires verifying Next.js behavior for header/security-adjacent changes). If Next.js strips them, this is a divergence that should be a deliberate, documented decision.

None of this blocks merge on correctness grounds — the implementation is sound and CI-green. The asks are documentation/verification, not code changes.

Verdict: LGTM on the code; please update the PR description to cover the middleware path + Flight-header forwarding, and cite the Next.js reference for the header behavior.

[ask-bonk]

Behavioral change worth calling out in the PR body: on main this line was requestWithoutFlightHeaders(options.request), which stripped rsc / next-router-state-tree / next-router-prefetch / etc. before proxying to the external origin. With externalRewriteRequest ?? options.request (and restoreFlightHeaders in proxyExternalMiddlewareRewrite), those Flight headers plus _rsc are now forwarded to the destination.

This brings the middleware path into parity with the config-rewrite path (which already used normalizedUserlandRequest on main), and forwarding _rsc is what lets the destination validate the request — so it looks intentional. Please (1) mention it in the PR description, and (2) confirm against .nextjs-ref that Next.js forwards Flight headers to middleware external rewrite destinations, since this changes what reaches arbitrary external origins. Same applies to the forwarded-context branch at line 282's sibling around line 363.

[ask-bonk]

restoreFlightHeaders unconditionally restores the original Flight header values, overriding any middleware x-middleware-request-rsc/-next-router-* overrides (the "forwards valid RSC cache-busting params" test asserts the upstream sees the original rsc: 1 even though middleware set rsc: 0). That's a reasonable anti-tampering posture, but it means middleware cannot legitimately rewrite Flight headers for an external destination. Worth a one-line comment here documenting that this deliberately wins over middleware overrides, so a future reader doesn't "fix" it.

[ask-bonk]

Review posted successfully.

Summary

I reviewed PR #2754 and posted a COMMENT review with two inline comments.

Verdict: LGTM on the code — but the PR description understates the scope.

What the PR does (correctly): Closes a gap where invalid outside-basePath RSC GET/HEAD requests could be proxied to external rewrite destinations without _rsc canonicalization. It injects a validateExternalRewriteRequest callback at every external-match boundary — all 6 config applyRewrite call sites plus both middleware external-rewrite paths.

What I verified as correct:

• applyRewrite() is the only proxyExternalRequest call site for config rewrites; all sites are gated. Validation runs before clearRequestContext() and only reads the request.
• Default-param routeClaimed = filesystemRouteEligible evaluates at call time (no closure staleness); external rewrites force true.
• validationResponseWithMiddlewareHeaders correctly captures/restores location around the header merge (which uses set for non-additive headers), and set-cookie merges additively — both asserted in tests.
• All targeted suites pass (142 + 37 + 52) and vp check is clean.

Main concern I raised (docs/verification, not code):

1. The title/body describe only config beforeFiles/afterFiles/fallback, but the PR also modifies both middleware external-rewrite paths — contradicting the earlier ask-bonk review's "out of scope" claim (that review predates the middleware commits).
2. A real, undocumented behavioral change: on main, middleware external rewrites stripped Flight headers (requestWithoutFlightHeaders); this PR now forwards them (rsc, next-router-state-tree, _rsc, etc.) to external origins via restoreFlightHeaders. This brings middleware into parity with the config path and enables destination _rsc validation, so it looks intentional — but since it changes what reaches arbitrary external origins, the PR body should call it out and cite .nextjs-ref per AGENTS.md.

I also flagged that restoreFlightHeaders deliberately overrides middleware Flight-header rewrites (anti-tampering) and suggested a clarifying comment so a future reader doesn't "fix" it.

github run