Preserve the original host on forwarded Server Action requests - #96552
Preserve the original host on forwarded Server Action requests#96552EnricoOrt wants to merge 2 commits into
Conversation
Failing test suitesCommit: f363259 | About building and testing Next.js
Expand output● back navigation before hydration after reload › no Suspense boundary above the page › reconciles the URL with the rendered content once hydration completes ● back navigation before hydration after reload › no Suspense boundary above the page › reconciles when the traversed entry differs only in search params ● back navigation before hydration after reload › no Suspense boundary above the page › other history changes before hydration › leaves the traversal unhandled when a third-party write lands before the replay
Expand output● Error overlay - RSC build errors › Skipped in webpack › should handle successive HMR changes with errors correctly |
Stats from current PR🔴 2 regressions
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (9 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsDiff too large to display app-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsDiff too large to display app-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsDiff too large to display app-page-tur..time.prod.jsDiff too large to display app-page.runtime.dev.jsDiff too large to display app-page.runtime.prod.jsDiff too large to display server.runtime.prod.jsDiff too large to display 📎 Tarball URLCommit: f363259 |
|
good start |
When an action POST lands on a route that does not bundle the action, it is
forwarded to a worker that does, via an internal self-fetch to
`__NEXT_PRIVATE_ORIGIN`. `host` is a forbidden `fetch` header, so it cannot be
carried onto the subrequest: the action saw `headers().get('host')` as
`localhost:PORT` instead of the host the user requested. That breaks
host-based multi-tenancy for exactly those actions that happen to get
forwarded, which depends on the build's static import graph rather than
anything visible in the application.
Restore `host` from `x-forwarded-host`, which does survive the forward,
guarded on the request being marked `x-action-forwarded` and having arrived at
the internal origin. `x-forwarded-host` is already preferred over `host` by
`parseHostHeader` for the CSRF origin check, so this aligns userland
`headers()` with the value the framework already trusts.
Fixes vercel#96344
a9ba24e to
f363259
Compare
What
Restores the original
Hostheader on a Server Action request that Next.js forwarded to another worker, soheaders().get('host')inside the action reports the host the user requested instead of the server's internal origin.Fixes #96344
Why
When an action POST lands on a route whose page doesn't bundle that action,
handleActionforwards it to a worker that does (selectWorkerForForwarding→createForwardedActionResponse). The forward is an HTTP self-fetch to the server's own origin, normallyhttp://localhost:PORT.hostis a forbiddenfetchheader, so it can't be carried onto the subrequest;fetchderivesHostfrom the URL it is given. The action ends up seeing:Reading the tenant from
headers().get('host')is a common self-hosting pattern, and it breaks only for the subset of actions that happen to get forwarded, which depends on the build's static import graph versus where the action is actually invoked. Shared client components and intercepted or parallel routes routinely land a POST on a route that doesn't bundle the action. The reporter measures about 1,000 failed actions a day across 9 domains, all surfacing as "project not found: localhost:3000".x-forwarded-hostdoes survive the forward and still carries the original host, so the information is recoverable.How
restoreForwardedActionHostrewriteshostfromx-forwarded-host. It runs inbase-server.tsimmediately afterattachRequestMeta, and the placement is load-bearing in both directions. It has to be afterattachRequestMeta, because the origin it compares against can come from theinitURLrequest meta, and that call is what sets it. It has to be beforei18nProvider.detectDomainLocale, a few lines further down, which is the first thing in the request lifecycle to readhost. Doing the rewrite insidehandleActionwould leave the locale lookup reading the internal origin.The marker header is not authenticated, so it is treated as a hint rather than proof. The rewrite requires all four of:
x-action-forwardedis exactly1, the only valuecreateForwardedActionResponseever sends. A repeated header arrives comma-joined and fails this.handleActiononly forwards a POST carrying an action id, so no other request shape can have come through the forwarding path.x-forwarded-hostis present.hostmatches the host of the origin this server forwards to, port included.The send and receive sides have to agree on that origin, so both now go through
getActionForwardingOrigin(req):__NEXT_PRIVATE_ORIGINwhen it is set, otherwise the origin ofinitURL. That also replaces two identical copies of the resolution block, increateForwardedActionResponseandcreateRedirectRenderResult.The
initURLfallback needed a correction from the first version of this PR, which claimedinitURLalready carries the right host. It only does underexperimental.trustHostHeader. When the server is started with a hostname and port, which is the self-hosted case in the issue,attachRequestMetabuildsinitURLas${protocol}://${fetchHostname}:${port}${req.url}and the bug is still live, so that path now gets the rewrite too. ThetrustHostHeadercase is deliberately excluded: thereinitURLcomes from the request's ownhostheader, which would make guard 4 vacuously true.base-serverpasseshasConfiguredOrigin: Boolean(this.fetchHostname && this.port), mirroring the condition inattachRequestMeta, and the helper bails when there is neither a private origin nor a configured one.On trust.
parseHostHeaderalready prefersx-forwarded-hostoverhostfor the action CSRF origin check, so the value itself isn't newly trusted. What is new is that it becomes visible asheaders().get('host'), and that is the value host-based tenancy reads. So, plainly: the marker is a hint, not authentication. In the normal topology the forwarding origin is loopback and unreachable from outside, so guard 4 can't be satisfied over the network. A proxy that presents an internalHostto Next.js while also passing through a client-controlledx-forwarded-hostwould satisfy all four guards. Authenticating the marker with a per-server secret would close that. I left it out because it's a larger change than the issue calls for, and I'd rather have a maintainer opinion on it first.The obvious alternative doesn't work:
x-action-forwardedcan't just be added toINTERNAL_HEADERS.filterInternalHeadersruns on every ingress inrouter-server.ts, including the genuine loopback request, so listing it there would strip the real marker.ACTION_FORWARDED_HEADER,ACTION_FORWARDED_VALUE,getForwardedHostValue,getActionForwardingOriginandrestoreForwardedActionHostnow live inapp-render/action-forwarding.ts, shared betweenbase-server.tsandaction-handler.ts.getForwardedHostValueis the existingx-forwarded-hostlist parsing lifted out ofparseHostHeaderrather than duplicated.Tests
Unit,
action-forwarding.test.ts, 25 cases. Both list forms ofx-forwarded-host, both origin sources, and the rejection paths: a forged marker arriving at a host that isn't the internal one, marker values"","true","0","1, 1"and"yes", non fetch-action requests, missingx-forwarded-host, a malformed private origin, and thetrustHostHeadershape whereinitURLis derived from the request.e2e,
test/e2e/app-dir/action-forward-host./with-actionbundles the action and/without-actiondoesn't, so a POST to the latter goes through the forwarding path. Both arms are asserted, so the test pins the invariant rather than only the symptom, and it checksx-action-forwardedto prove forwarding actually ran. A third case sends a forged marker together with a forgedx-forwarded-hostand assertshostcomes through unchanged.It uses raw
node:httpinstead ofnext.fetch, because undici refuses to sethost. Same forbidden-header rule that causes the bug.skipDeployment: true. The test needs a local port to connect to, has to send aHostdifferent from the one it connected to, and reads the action's output from the running server's stdout. A deploy instance has a remote URL, no local app port, and build logs rather than live server output. The behaviour under test is a loopback self-fetch to the server's own origin, which is inherently self-hosted.Before and after:
Green after rebasing onto current canary: the new suite in dev and start for both webpack and Turbopack,
action-forward-loop,actions-allowed-originsincluding "should error if x-forwarded-host does not match the origin", the new unit suite, andtsc.Out of scope
createRedirectRenderResultdoes a second self-fetch for app-relative redirect streaming and losesHostthe same way. It shares the origin resolver now, but not the restoration: it setsx-action-forwardedonly when the original action was itself forwarded, so it needs its own marker and the same provenance decision as above. Happy to follow up separately.For an Edge action target,
runEdgeFunctionbuilds the request URL frominitURL, sorequest.urlstill shows the internal origin even thoughheaders().get('host')is restored.