Security hardening: redirect sanitization, header stripping, ALS isolation tests, and safer command execution#105
Merged
threepointone merged 3 commits intomainfrom Feb 26, 2026
Merged
Conversation
Replace execSync calls with execFileSync (pass argv arrays) to avoid shell interpolation and adjust package-install logic to split package manager and args. Sanitize redirect destinations in SSR paths to prevent protocol-relative open-redirects and use sanitizeDestinationLocal for Next.js redirects. Ensure middleware receives a cleaned pathname by stripping internal .rsc suffix before creating NextRequest and add a test for this behavior. Strip sensitive headers (cookie, authorization and x-middleware-*) from proxied external requests and add a test. Document cache key hazards for auth headers in fetch-cache, add a security note for dangerouslySetInnerHTML in head SSR, and set the Draft Mode cookie to include Secure in production (with tests for production/dev behavior).
commit: |
|
Change middleware header name from x-middleware-test to x-custom-middleware across fixtures and tests, and clarify server comment about stripping x-middleware-* headers from responses (reserved for internal routing signals). Files updated: packages/vinext/src/server/prod-server.ts (comment tweak), tests/fixtures/pages-basic/middleware.ts (header set), and tests/pages-router.test.ts (expectations and test titles/comments). This ensures tests reflect the new public header name while preserving internal x-middleware-* semantics.
Normalize redirect/rewrite destinations by collapsing any leading slashes or backslashes to a single "/" to avoid protocol-relative redirects and handle browser-backslash behavior. Strip additional sensitive headers (x-api-key, proxy-authorization) from proxied requests and ensure all x-middleware-* internal headers are removed from responses (broadened from x-middleware-request-*). Update dev/server sanitization logic accordingly. Update tests and fixture middleware to use shortened x-mw-* header names and add tests covering backslash normalization.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.combecomes//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 toredirect.destinationfromgetServerSideProps/getStaticProps.index.ts— ApplysanitizeDestinationLocal()(same logic) to redirect paths in the Cloudflare Worker entry, ensuring dev/prod parity.External Rewrite Proxy Hardening (
config-matchers.ts)cookie,authorization,x-api-key, andproxy-authorizationfrom requests before forwarding to external rewrite destinations. Previously, all request headers (including session cookies and auth tokens) were forwarded verbatim to third-party hosts.x-middleware-*internal routing headers from proxied requests.AbortSignaltimeout on upstream fetch calls. Returns504 Gateway Timeouton abort.Internal
x-middleware-*Header StrippingThe
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 allx-middleware-*headers from the response after unpackingx-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 onlyx-middleware-request-*was previously removed.x-middleware-testtox-custom-middleware(Pages Router) andx-middleware-ran/x-middleware-pathnametox-mw-ran/x-mw-pathname(App Router) since the prefix is now correctly reserved..rscSuffix Stripped from Middleware NextRequest (app-dev-server.ts)The
.rscsuffix is an internal transport detail for RSC stream requests. Previously,NextRequest.nextUrl.pathnameinside middleware would be/about.rscinstead of/about, causing exact-match guards to silently fail. Now the URL is cleaned before constructing theNextRequest.Draft Mode Cookie Security (
headers.ts)Add
Secureflag to the__prerender_bypassdraft mode cookie whenNODE_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 toexecFileSync()with argument arrays. Defense-in-depth against future changes that might introduce user-controlled input.Security Documentation
head.ts— Document SSR/CSR divergence fordangerouslySetInnerHTMLand stored XSS risk.fetch-cache.ts— DocumentAUTH_HEADERSallowlist limitation (only 3 headers keyed; custom auth headers needcache: "no-store").Tests Added
ALS Per-Request Isolation (
tests/nextjs-compat/als-isolation.test.ts)Four concurrency regression tests verifying
AsyncLocalStorageproperly isolates per-request state:x-request-idOther Regression Tests
.rscsuffix stripping (tests/app-router.test.ts) — Asserts middleware sees/about, not/about.rsctests/nextjs-compat/draft-mode.test.ts) — Asserts cookie contains; Securein productiontests/shims.test.ts) — Assertscookie,authorization,x-api-key,proxy-authorization, andx-middleware-*are strippedtests/shims.test.ts) — Asserts\/evil.com,\\evil.com, etc. are all collapsed to/evil.comFiles Changed
config-matchers.tsserver/prod-server.tsx-middleware-*headers from client responsesserver/app-dev-server.ts.rscfrom middleware URL + stripx-middleware-*from responsesserver/dev-server.tsindex.tsshims/headers.tsSecureflag on draft mode cookie in productionshims/head.tsshims/fetch-cache.tsdeploy.tsexecSync→execFileSynccli.tsexecSync→execFileSynctests/nextjs-compat/als-isolation.test.tstests/app-router.test.ts.rscsuffix + header renametests/nextjs-compat/draft-mode.test.tstests/shims.test.tstests/e2e/pages-router/middleware.spec.tstests/e2e/app-router/headers-cookies.spec.tstests/fixtures/app-basic/middleware.tstests/fixtures/pages-basic/middleware.tstests/fixtures/pages-basic/dist/server/entry.js