fix(middleware): preserve safe origins for double-slash redirects

[NathanDrake2406]

Summary

• Keep same-origin middleware redirect targets absolute when relativizing them would create a protocol-relative URL.
• Preserve that invariant while adding or removing a configured trailing slash.
• Keep the preserved origin through Pages Router _next/data consumption, including History API mutation and the follow-up HTML fetch.

Why

A safe absolute target such as https://victim.example//evil.example/steal has a pathname beginning with //. Removing the same origin turned it into //evil.example/steal, which browsers interpret as an external authority. The middleware runtime owns server emission, and the Pages Router owns client consumption, so both boundaries now refuse transformations that would change the authority.

Validation

• Demonstrated the server regression tests fail before the runtime fix with protocol-relative Location and x-nextjs-redirect output.
• Demonstrated the Pages Router regression test fails before the client fix because the data redirect loses its origin.
• vp test run tests/shims.test.ts: 1,270 passed.
• vp test run tests/middleware-runtime.test.ts tests/middleware-runtime-trailing-slash.test.ts tests/request-pipeline.test.ts: 138 passed.
• vp check packages/vinext/src/server/middleware-runtime.ts packages/vinext/src/shims/router.ts tests/middleware-runtime.test.ts tests/shims.test.ts: formatting, lint, and types passed.
• vp run vinext#build: passed after both changes.
• Pre-commit full check, staged unit and integration tests, and knip: passed for both commits.

Risk and compatibility

Ordinary same-origin redirect locations remain relative, and cross-origin locations remain absolute. The deliberate compatibility exception is limited to same-origin paths that match the existing open-redirect guard or become authority-shaped after client origin/basePath stripping. Those targets remain in their original safe form so their authority cannot change during server emission or client navigation.

[NathanDrake2406]

@codex review

[pkg-pr-new]

Open in StackBlitz

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

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

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

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

commit: 10edada

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9511cb4f09

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

[github-actions]

Performance benchmarks

Compared 10edada 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.79 s	2.78 s	⚫ -0.2%
Production build time	vinext	2.96 s	2.95 s	⚫ -0.2%
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}

[NathanDrake2406]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 32bac391fc

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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Reviewed the double-slash open-redirect fix. The approach is sound and the fix is correctly split across both trust boundaries:

• Server (relativizeLocation) now keeps same-origin absolute Locations absolute whenever the pathname is isOpenRedirectShaped, so removing the origin can never turn https://victim.example//evil.example/... into a protocol-relative //evil.example/.... I confirmed the trailing-slash refactor (setting loc.pathname then re-running relativizeLocation) is behaviorally identical to the old normalized + search + hash for ordinary paths, and that URL.pathname = "//evil.example/steal/" safely preserves the host (victim.example).
• Client (resolveLocalRedirectUrl) now returns the original same-origin target when toSameOriginAppPath / basePath stripping would expose a leading //, addressing the exact history.replaceState SecurityError path Codex flagged. history.replaceState and the follow-up fetch both receive a same-origin absolute URL, so they stay on-origin. The backslash variant (/\evil.example) is normalized to // by the URL parser on both sides, so it's caught too.

All three x-nextjs-redirect / config-redirect consumers in router.ts (lines 2244, 2564, 2645) funnel through resolveLocalRedirectUrl, so the client guard covers every Pages Router path. The App Router relies on the shared server-side relativizeLocation fix plus browser-enforced same-origin fetch resolution, so it's covered without a separate client change.

Ran vp test run tests/middleware-runtime.test.ts tests/middleware-runtime-trailing-slash.test.ts locally: 34 passed.

One minor consistency note below (non-blocking).

[ask-bonk]

Minor consistency note (non-blocking): the server guard uses isOpenRedirectShaped, which also catches percent-encoded forms (/%2f..., /%5c...), but this client guard only checks appPath.startsWith("//"). I don't think this is exploitable on the client — for a Location like https://victim.example/%2fevil.example/steal, toSameOriginAppPath yields /%2fevil.example/steal (single leading slash), and browsers do not treat %2f as an authority delimiter in history.replaceState or fetch, so the navigation stays same-origin. The backslash form is already normalized to // by the URL parser, so it's covered here.

So behavior is correct, but the two boundaries now use different predicates for the "same" invariant. If you want them to stay obviously in sync, consider reusing isOpenRedirectShaped(appPath) here (it lives in server/open-redirect.ts and has no server-only deps) instead of the inline startsWith("//"), so a future change to the shaped-detection logic updates both sides together. Not required to merge.

[ask-bonk]

Review posted successfully. Here's a summary of my findings:

Review Summary — PR #2766

Verdict: Approve (with one minor, non-blocking consistency note).

The fix correctly closes a protocol-relative open-redirect (https://victim.example//evil.example/steal → //evil.example/steal) across both trust boundaries:

Server (relativizeLocation) — Keeps same-origin absolute Locations absolute when the pathname is isOpenRedirectShaped, so origin removal can never expose a protocol-relative authority. I verified:

• The trailing-slash refactor (loc.pathname = normalized → re-run relativizeLocation) is behaviorally identical to the old normalized + search + hash for ordinary paths.
• URL.pathname = "//evil.example/steal/" safely keeps host = victim.example (confirmed via Node).

Client (resolveLocalRedirectUrl) — Returns the original same-origin target when stripping would expose a leading //. This directly addresses the history.replaceState SecurityError path Codex flagged in commit 9511cb4; both replaceState and the follow-up fetch receive a same-origin absolute URL. The backslash variant is normalized to // by the URL parser on both sides, so it's caught too.

Coverage — All three x-nextjs-redirect/config-redirect consumers in router.ts (2244, 2564, 2645) funnel through resolveLocalRedirectUrl. The App Router inherits the shared server-side fix and relies on browser-enforced same-origin fetch resolution.

Tests — Ran tests/middleware-runtime.test.ts + tests/middleware-runtime-trailing-slash.test.ts locally: 34 passed.

Minor note (non-blocking): The server uses isOpenRedirectShaped (catches %2f/%5c) while the client uses startsWith("//"). I confirmed the %2f case isn't client-exploitable (browsers don't treat %2f as an authority delimiter in replaceState/fetch), so this is correct — but I suggested reusing isOpenRedirectShaped on the client for symmetry so the two boundaries stay in sync if the detection logic changes.

github run