[PR #105] [MERGED] Security hardening: redirect sanitization, header stripping, ALS isolation tests, and safer command execution #315

New issue

Closed

opened 2026-05-06 12:39:08 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 12:39:08 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/105
Author: @threepointone
Created: 2/26/2026
Status: ✅ Merged
Merged: 2/26/2026
Merged by: @threepointone

Base: main ← Head: s-fixes

📝 Commits (3)

d6d2c8d Sanitize redirects/headers and use execFileSync
996c3d9 Rename middleware header to x-custom-middleware
7002bc6 Normalize leading slashes and strip headers

📊 Changes

19 files changed (+385 additions, -60 deletions)

View changed files

📝 packages/vinext/src/cli.ts (+4 -4)
📝 packages/vinext/src/config/config-matchers.ts (+32 -8)
📝 packages/vinext/src/deploy.ts (+6 -4)
📝 packages/vinext/src/index.ts (+6 -5)
📝 packages/vinext/src/server/app-dev-server.ts (+13 -4)
📝 packages/vinext/src/server/dev-server.ts (+13 -2)
📝 packages/vinext/src/server/prod-server.ts (+5 -1)
📝 packages/vinext/src/shims/fetch-cache.ts (+6 -0)
📝 packages/vinext/src/shims/head.ts (+4 -1)
📝 packages/vinext/src/shims/headers.ts (+4 -2)
📝 tests/app-router.test.ts (+13 -5)
📝 tests/e2e/app-router/headers-cookies.spec.ts (+5 -5)
📝 tests/e2e/pages-router/middleware.spec.ts (+5 -5)
📝 tests/fixtures/app-basic/middleware.ts (+6 -6)
📝 tests/fixtures/pages-basic/middleware.ts (+1 -1)
➕ tests/nextjs-compat/als-isolation.test.ts (+164 -0)
📝 tests/nextjs-compat/draft-mode.test.ts (+37 -0)
📝 tests/pages-router.test.ts (+7 -7)
📝 tests/shims.test.ts (+54 -0)

📄 Description

Summary

Comprehensive security hardening pass across the server, shims, and CLI layers. Fixes several classes of vulnerabilities and adds regression tests to prevent regressions.

Changes

Redirect & Rewrite Sanitization

config-matchers.ts — sanitizeDestination() now normalizes both leading slashes AND backslashes. Browsers interpret \ as / in URL contexts, so \/evil.com becomes //evil.com (protocol-relative redirect). The regex ^[\\/]+ collapses any mix of leading slashes and backslashes to a single /.
dev-server.ts — Apply the same backslash-aware sanitization to redirect.destination from getServerSideProps/getStaticProps.
index.ts — Apply sanitizeDestinationLocal() (same logic) to redirect paths in the Cloudflare Worker entry, ensuring dev/prod parity.

External Rewrite Proxy Hardening (`config-matchers.ts`)

Strip all credential headers — Remove cookie, authorization, x-api-key, and proxy-authorization from requests before forwarding to external rewrite destinations. Previously, all request headers (including session cookies and auth tokens) were forwarded verbatim to third-party hosts.
Strip internal headers — Remove all x-middleware-* internal routing headers from proxied requests.
Add request timeout — Enforce a 30-second AbortSignal timeout on upstream fetch calls. Returns 504 Gateway Timeout on abort.

Internal `x-middleware-*` Header Stripping

The x-middleware-* prefix is reserved for internal routing signals (continue, rewrite, override-headers, invoke, request-* unpacking) and must never reach clients. This matches Next.js behavior.

prod-server.ts — Strip all x-middleware-* headers from the response after unpacking x-middleware-request-* into the actual request.
app-dev-server.ts — Same stripping applied to the App Router path, fixing a dev/prod parity gap where only x-middleware-request-* was previously removed.
Test fixtures — Renamed custom middleware headers from x-middleware-test to x-custom-middleware (Pages Router) and x-middleware-ran/x-middleware-pathname to x-mw-ran/x-mw-pathname (App Router) since the prefix is now correctly reserved.

`.rsc` Suffix Stripped from Middleware NextRequest (`app-dev-server.ts`)

The .rsc suffix is an internal transport detail for RSC stream requests. Previously, NextRequest.nextUrl.pathname inside middleware would be /about.rsc instead of /about, causing exact-match guards to silently fail. Now the URL is cleaned before constructing the NextRequest.

Draft Mode Cookie Security (`headers.ts`)

Add Secure flag to the __prerender_bypass draft mode cookie when NODE_ENV === "production". Prevents the cookie from being transmitted over unencrypted HTTP in production.

Safer Command Execution (`deploy.ts`, `cli.ts`)

Convert all execSync() calls with string interpolation to execFileSync() with argument arrays. Defense-in-depth against future changes that might introduce user-controlled input.

Security Documentation

head.ts — Document SSR/CSR divergence for dangerouslySetInnerHTML and stored XSS risk.
fetch-cache.ts — Document AUTH_HEADERS allowlist limitation (only 3 headers keyed; custom auth headers need cache: "no-store").

Tests Added

ALS Per-Request Isolation (`tests/nextjs-compat/als-isolation.test.ts`)

Four concurrency regression tests verifying AsyncLocalStorage properly isolates per-request state:

Headers — 20 concurrent requests each see only their own x-request-id
Navigation context — 20 concurrent requests each see only their own pathname
Fetch cache scopes — 20 concurrent scopes maintain independent tag state
Cookies — 20 concurrent requests each see only their own session cookie

Other Regression Tests

.rsc suffix stripping (tests/app-router.test.ts) — Asserts middleware sees /about, not /about.rsc
Draft mode Secure flag (tests/nextjs-compat/draft-mode.test.ts) — Asserts cookie contains ; Secure in production
External proxy credential stripping (tests/shims.test.ts) — Asserts cookie, authorization, x-api-key, proxy-authorization, and x-middleware-* are stripped
Backslash normalization (tests/shims.test.ts) — Asserts \/evil.com, \\evil.com, etc. are all collapsed to /evil.com

Files Changed

File	What
`config-matchers.ts`	Backslash-aware sanitizeDestination + proxy credential/timeout hardening
`server/prod-server.ts`	Strip all `x-middleware-*` headers from client responses
`server/app-dev-server.ts`	Strip `.rsc` from middleware URL + strip `x-middleware-*` from responses
`server/dev-server.ts`	Backslash-aware redirect sanitization from GSP/GSSP
`index.ts`	Backslash-aware sanitizeDestinationLocal for Worker entry
`shims/headers.ts`	`Secure` flag on draft mode cookie in production
`shims/head.ts`	Security documentation
`shims/fetch-cache.ts`	Auth header cache key documentation
`deploy.ts`	`execSync` → `execFileSync`
`cli.ts`	`execSync` → `execFileSync`
`tests/nextjs-compat/als-isolation.test.ts`	New — ALS concurrency tests (4 tests)
`tests/app-router.test.ts`	`.rsc` suffix + header rename
`tests/nextjs-compat/draft-mode.test.ts`	Secure flag test
`tests/shims.test.ts`	Proxy stripping + backslash tests
`tests/e2e/pages-router/middleware.spec.ts`	Header rename
`tests/e2e/app-router/headers-cookies.spec.ts`	Header rename
`tests/fixtures/app-basic/middleware.ts`	Header rename
`tests/fixtures/pages-basic/middleware.ts`	Header rename
`tests/fixtures/pages-basic/dist/server/entry.js`	Header rename (pre-built)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/105 **Author:** [@threepointone](https://github.com/threepointone) **Created:** 2/26/2026 **Status:** ✅ Merged **Merged:** 2/26/2026 **Merged by:** [@threepointone](https://github.com/threepointone) **Base:** `main` ← **Head:** `s-fixes` --- ### 📝 Commits (3) - [`d6d2c8d`](https://github.com/cloudflare/vinext/commit/d6d2c8d016f8bd61486672f3350e5a96f785a87b) Sanitize redirects/headers and use execFileSync - [`996c3d9`](https://github.com/cloudflare/vinext/commit/996c3d9d0c47b42029f91834acd6186c1337421d) Rename middleware header to x-custom-middleware - [`7002bc6`](https://github.com/cloudflare/vinext/commit/7002bc602594b3554ab12bf34a27948b63b53272) Normalize leading slashes and strip headers ### 📊 Changes **19 files changed** (+385 additions, -60 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/cli.ts` (+4 -4) 📝 `packages/vinext/src/config/config-matchers.ts` (+32 -8) 📝 `packages/vinext/src/deploy.ts` (+6 -4) 📝 `packages/vinext/src/index.ts` (+6 -5) 📝 `packages/vinext/src/server/app-dev-server.ts` (+13 -4) 📝 `packages/vinext/src/server/dev-server.ts` (+13 -2) 📝 `packages/vinext/src/server/prod-server.ts` (+5 -1) 📝 `packages/vinext/src/shims/fetch-cache.ts` (+6 -0) 📝 `packages/vinext/src/shims/head.ts` (+4 -1) 📝 `packages/vinext/src/shims/headers.ts` (+4 -2) 📝 `tests/app-router.test.ts` (+13 -5) 📝 `tests/e2e/app-router/headers-cookies.spec.ts` (+5 -5) 📝 `tests/e2e/pages-router/middleware.spec.ts` (+5 -5) 📝 `tests/fixtures/app-basic/middleware.ts` (+6 -6) 📝 `tests/fixtures/pages-basic/middleware.ts` (+1 -1) ➕ `tests/nextjs-compat/als-isolation.test.ts` (+164 -0) 📝 `tests/nextjs-compat/draft-mode.test.ts` (+37 -0) 📝 `tests/pages-router.test.ts` (+7 -7) 📝 `tests/shims.test.ts` (+54 -0) </details> ### 📄 Description ## Summary Comprehensive security hardening pass across the server, shims, and CLI layers. Fixes several classes of vulnerabilities and adds regression tests to prevent regressions. --- ## Changes ### Redirect & Rewrite Sanitization - **`config-matchers.ts`** — `sanitizeDestination()` now normalizes both leading slashes AND backslashes. Browsers interpret `\` as `/` in URL contexts, so `\/evil.com` becomes `//evil.com` (protocol-relative redirect). The regex `^[\\/]+` collapses any mix of leading slashes and backslashes to a single `/`. - **`dev-server.ts`** — Apply the same backslash-aware sanitization to `redirect.destination` from `getServerSideProps`/`getStaticProps`. - **`index.ts`** — Apply `sanitizeDestinationLocal()` (same logic) to redirect paths in the Cloudflare Worker entry, ensuring dev/prod parity. ### External Rewrite Proxy Hardening (`config-matchers.ts`) - **Strip all credential headers** — Remove `cookie`, `authorization`, `x-api-key`, and `proxy-authorization` from requests before forwarding to external rewrite destinations. Previously, all request headers (including session cookies and auth tokens) were forwarded verbatim to third-party hosts. - **Strip internal headers** — Remove all `x-middleware-*` internal routing headers from proxied requests. - **Add request timeout** — Enforce a 30-second `AbortSignal` timeout on upstream fetch calls. Returns `504 Gateway Timeout` on abort. ### Internal `x-middleware-*` Header Stripping The `x-middleware-*` prefix is reserved for internal routing signals (continue, rewrite, override-headers, invoke, request-* unpacking) and must never reach clients. This matches Next.js behavior. - **`prod-server.ts`** — Strip all `x-middleware-*` headers from the response after unpacking `x-middleware-request-*` into the actual request. - **`app-dev-server.ts`** — Same stripping applied to the App Router path, fixing a dev/prod parity gap where only `x-middleware-request-*` was previously removed. - **Test fixtures** — Renamed custom middleware headers from `x-middleware-test` to `x-custom-middleware` (Pages Router) and `x-middleware-ran`/`x-middleware-pathname` to `x-mw-ran`/`x-mw-pathname` (App Router) since the prefix is now correctly reserved. ### `.rsc` Suffix Stripped from Middleware NextRequest (`app-dev-server.ts`) The `.rsc` suffix is an internal transport detail for RSC stream requests. Previously, `NextRequest.nextUrl.pathname` inside middleware would be `/about.rsc` instead of `/about`, causing exact-match guards to silently fail. Now the URL is cleaned before constructing the `NextRequest`. ### Draft Mode Cookie Security (`headers.ts`) Add `Secure` flag to the `__prerender_bypass` draft mode cookie when `NODE_ENV === "production"`. Prevents the cookie from being transmitted over unencrypted HTTP in production. ### Safer Command Execution (`deploy.ts`, `cli.ts`) Convert all `execSync()` calls with string interpolation to `execFileSync()` with argument arrays. Defense-in-depth against future changes that might introduce user-controlled input. ### Security Documentation - **`head.ts`** — Document SSR/CSR divergence for `dangerouslySetInnerHTML` and stored XSS risk. - **`fetch-cache.ts`** — Document `AUTH_HEADERS` allowlist limitation (only 3 headers keyed; custom auth headers need `cache: "no-store"`). --- ## Tests Added ### ALS Per-Request Isolation (`tests/nextjs-compat/als-isolation.test.ts`) Four concurrency regression tests verifying `AsyncLocalStorage` properly isolates per-request state: 1. **Headers** — 20 concurrent requests each see only their own `x-request-id` 2. **Navigation context** — 20 concurrent requests each see only their own pathname 3. **Fetch cache scopes** — 20 concurrent scopes maintain independent tag state 4. **Cookies** — 20 concurrent requests each see only their own session cookie ### Other Regression Tests - **`.rsc` suffix stripping** (`tests/app-router.test.ts`) — Asserts middleware sees `/about`, not `/about.rsc` - **Draft mode Secure flag** (`tests/nextjs-compat/draft-mode.test.ts`) — Asserts cookie contains `; Secure` in production - **External proxy credential stripping** (`tests/shims.test.ts`) — Asserts `cookie`, `authorization`, `x-api-key`, `proxy-authorization`, and `x-middleware-*` are stripped - **Backslash normalization** (`tests/shims.test.ts`) — Asserts `\/evil.com`, `\\evil.com`, etc. are all collapsed to `/evil.com` --- ## Files Changed | File | What | |------|------| | `config-matchers.ts` | Backslash-aware sanitizeDestination + proxy credential/timeout hardening | | `server/prod-server.ts` | Strip all `x-middleware-*` headers from client responses | | `server/app-dev-server.ts` | Strip `.rsc` from middleware URL + strip `x-middleware-*` from responses | | `server/dev-server.ts` | Backslash-aware redirect sanitization from GSP/GSSP | | `index.ts` | Backslash-aware sanitizeDestinationLocal for Worker entry | | `shims/headers.ts` | `Secure` flag on draft mode cookie in production | | `shims/head.ts` | Security documentation | | `shims/fetch-cache.ts` | Auth header cache key documentation | | `deploy.ts` | `execSync` → `execFileSync` | | `cli.ts` | `execSync` → `execFileSync` | | `tests/nextjs-compat/als-isolation.test.ts` | **New** — ALS concurrency tests (4 tests) | | `tests/app-router.test.ts` | `.rsc` suffix + header rename | | `tests/nextjs-compat/draft-mode.test.ts` | Secure flag test | | `tests/shims.test.ts` | Proxy stripping + backslash tests | | `tests/e2e/pages-router/middleware.spec.ts` | Header rename | | `tests/e2e/app-router/headers-cookies.spec.ts` | Header rename | | `tests/fixtures/app-basic/middleware.ts` | Header rename | | `tests/fixtures/pages-basic/middleware.ts` | Header rename | | `tests/fixtures/pages-basic/dist/server/entry.js` | Header rename (pre-built) | --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>