Skip to content

fix(middleware): authenticate hybrid context bridge#1943

Draft
james-elicx wants to merge 2 commits into
mainfrom
codex/security-middleware-context
Draft

fix(middleware): authenticate hybrid context bridge#1943
james-elicx wants to merge 2 commits into
mainfrom
codex/security-middleware-context

Conversation

@james-elicx

@james-elicx james-elicx commented Jun 12, 2026

Copy link
Copy Markdown
Member

Impact

Externally reachable non-production App Router servers accepted a client-supplied x-vinext-mw-ctx header as trusted hybrid middleware state. An attacker could send {} to suppress real middleware execution, or provide an external rewrite URL to turn the dev server into an SSRF proxy, including forwarding POST bodies and attacker-provided credentials.

Normal NODE_ENV=production handling did not consume this bridge, but exposed development, preview, and shared internal servers were affected.

Reproduction before this fix

Middleware bypass:

curl -i -H 'x-vinext-mw-ctx: {}' http://TARGET:3000/admin

Attacker-controlled POST rewrite / SSRF:

curl -i -X POST \
  -H 'Authorization: Bearer attacker-controlled' \
  -H 'x-vinext-mw-ctx: {"r":"http://127.0.0.1:9000/probe"}' \
  --data 'sensitive-post-body' \
  http://TARGET:3000/admin

Fix

  • Treat x-vinext-mw-ctx as a vinext internal header and strip it at inbound request boundaries.
  • Keep middleware result state exclusively in a bounded process-local registry stored under a Symbol.for key on the Node process object.
  • Forward only a random, one-time, 30-second opaque handle between the hybrid Pages adapter and RSC environment.
  • Atomically consume/delete handles before applying headers, status, rewrites, or proxying.
  • Do not serialize secrets, trust material, middleware results, or registry access into the generated RSC virtual module.
  • Preserve the hybrid behavior where middleware executes exactly once.

This follows Next.js's model of stripping client-supplied middleware control headers rather than trusting public request metadata.

Security regressions

  • Fetches all known Vite virtual-module URL forms and verifies no middleware trust material is disclosed:
    • /@id/virtual:vinext-rsc-entry
    • /@id/__x00__virtual:vinext-rsc-entry
    • /@id/__x00__virtual%3Avinext-rsc-entry
    • /@id/%00virtual:vinext-rsc-entry
  • Client-supplied {} cannot skip middleware.
  • Client-supplied rewrite context cannot trigger proxying.
  • Forged POST rewrites cannot forward authorization headers or request bodies.
  • Registry handles are one-time and cannot be replayed.
  • Trusted hybrid context is accepted without running middleware twice.
  • x-vinext-mw-ctx remains covered by vinext internal-header filtering.

Validation

vp test run tests/app-middleware-context-security.test.ts tests/app-router-middleware-context-security.test.ts
vp test run tests/request-pipeline.test.ts -t "filterInternalHeaders"
PLAYWRIGHT_PROJECT=app-router pnpm run test:e2e -- tests/e2e/app-router/middleware.spec.ts -g "middleware runs exactly once"
vp check packages/vinext/src/server/dev-middleware-context-registry.ts packages/vinext/src/server/headers.ts packages/vinext/src/server/app-middleware.ts packages/vinext/src/server/app-rsc-handler.ts packages/vinext/src/entries/app-rsc-entry.ts packages/vinext/src/index.ts tests/request-pipeline.test.ts tests/app-middleware-context-security.test.ts tests/app-router-middleware-context-security.test.ts

All targeted tests and checks pass.

@pkg-pr-new

pkg-pr-new Bot commented Jun 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 758a13f

@github-actions

Copy link
Copy Markdown
Contributor
Example Preview Production Original
app-router-cloudflare preview production
pages-router-cloudflare preview production
app-router-playground preview production original
realworld-api-rest preview production
nextra-docs-template preview production
benchmarks preview production
hackernews preview production original
workers-cache preview production
web preview production

@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk review for issues

@ask-bonk

ask-bonk Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

/bigbonk review — fix(middleware): authenticate hybrid context bridge

Reviewed the full diff, traced the data flow across dev/prod/worker paths, and ran the cited tests. This is a correct, well-targeted security fix. Summary below, plus a few minor observations (none blocking).

What I verified ✅

  1. Root cause is real. Before this PR, VINEXT_INTERNAL_HEADERS contained only VINEXT_PRERENDER_ROUTE_PARAMS_HEADER, so the inbound strip loops in index.ts (for (const header of VINEXT_INTERNAL_HEADERS) delete req.headers[header]) and filterInternalHeaders() never removed x-vinext-mw-ctx. A client-supplied header survived to app-rsc-handler.ts:864 and was trusted by applyForwardedMiddlewareContext. The {} bypass and {"r":"..."} SSRF in the description are both reproducible against the unpatched dev/preview server.

  2. The fix closes both vectors:

    • Adding VINEXT_MW_CTX_HEADER to VINEXT_INTERNAL_HEADERS (headers.ts:204) means the client value is now stripped at every inbound boundary — the dev connect handler (index.ts:3569-3571), filterInternalHeaders in app-rsc-handler.ts:878, and prod-server (prod-server.ts:757,1706).
    • The data.t !== secret gate (app-middleware.ts:190) is checked before any mutation of context.headers/context.status/rewrite handling, so an unauthenticated payload can never apply state.
  3. Secret provenance is consistent. middlewareContextSecret is a single randomBytes(32) value created once at plugin-instance scope (index.ts:807), captured in the closure, then (a) embedded into the generated RSC entry via the single generateRscEntry call and (b) attached at runtime by the trusted hybrid adapter (index.ts:3648). Same closure → values match in dev. 256-bit, per-instance, never sent to clients → not guessable. This mirrors the existing prerenderSecret/draftModeSecret pattern.

  4. Defense in depth holds for the undefined secret case. The option type allows middlewareContextSecret?: string; app-middleware.ts:180 short-circuits on !secretapplied: false → middleware runs normally. Safe by default.

  5. Prod/worker parity confirmed. The bridge is only consumed in app-middleware.ts, gated on NODE_ENV !== "production". Prod/worker entries never trust it, and now also strip it via filterInternalHeaders. No analogous unauthenticated consumption elsewhere.

  6. Tests pass (app-middleware-context-security.test.ts 3/3, request-pipeline.test.ts -t filterInternalHeaders 7/7) and vp check is clean on all touched files.

Minor observations (non-blocking)

  • app-rsc-handler.ts:864/880 re-attaches the raw mwCtx even in production. Because applyForwardedMiddlewareContext returns early in prod and nothing else reads x-vinext-mw-ctx, this is harmless today — but it does mean a forged value briefly survives into the filtered request in the prod RSC path. Consider only re-attaching when NODE_ENV !== "production", or letting it be stripped, to avoid a future reader accidentally trusting it. Low priority.

  • Non-constant-time comparison (data.t !== secret). A !== string compare is technically timing-observable. For a 256-bit per-instance secret over JSON parsing noise this is not a practical concern, so I'd leave it — just flagging it for awareness.

  • Test coverage gaps. The new test always passes middlewareContextSecret: "trusted-secret". Two cheap additions would lock in the remaining guarantees: (a) a case with middlewareContextSecret: undefined + a forged authenticated payload (expect middleware to still run), and (b) a wrong-secret case (t: "wrong"). The production short-circuit can't easily be unit-tested without env mutation, so it's fine to leave that to the behavioral argument above.

  • isForwardedMiddlewareContext is just an object check (!!value && typeof value === "object"); the real authentication is data.t !== secret. That's fine, but worth a one-line comment noting the type guard is shape-only and the secret check is the trust boundary.

Verdict

LGTM. The approach matches Next.js's model of stripping client-supplied middleware control headers, the secret-based authentication is the right mechanism for the legitimate hybrid bridge, and the ordering of the secret check relative to context mutation is correct. The minor items above are polish, not blockers.

@ask-bonk

ask-bonk Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review posted to PR #1943.

Summary

Verdict: LGTM — correct, well-targeted security fix.

The PR fixes a real vulnerability: before it, x-vinext-mw-ctx was missing from VINEXT_INTERNAL_HEADERS, so a client-supplied header was never stripped and was trusted by the hybrid dev/preview App Router server, enabling middleware bypass ({}) and SSRF ({"r":"..."}).

What I verified:

  • Adding the header to VINEXT_INTERNAL_HEADERS strips the client value at every inbound boundary (dev connect handler, filterInternalHeaders, prod-server).
  • The data.t !== secret check runs before any context mutation — unauthenticated payloads can't apply state.
  • The secret is a single per-plugin-instance randomBytes(32), shared via closure between the embedded RSC entry and the trusted dev adapter, so they match; never exposed to clients.
  • undefined secret short-circuits safely (middleware runs).
  • Prod/worker entries never consume the bridge; parity holds.
  • Cited tests pass (3/3 + 7/7) and vp check is clean.

Minor non-blocking notes I flagged: the prod RSC path re-attaches the raw mwCtx (harmless today but could be guarded), non-constant-time secret compare (not a practical concern for a 256-bit secret), and two cheap extra test cases (undefined-secret + wrong-secret).

github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant