🐛 BUG: New SvelteKit project created by C3 rejects all POSTs

[kentonv]

Which Cloudflare product(s) does this pertain to?

C3, Miniflare

What version(s) of the tool(s) are you using?

3.23.0 [Wrangler], 2.9.2 [C3]

What version of Node are you using?

20.11.0

What operating system are you using?

Linux

Describe the Bug

In a newly-created SvelteKit project, all POST requests fail with {"message":"Cross-site POST form submissions are forbidden"}

To reproduce:

1. Run c3.
2. Choose to create a Svelte app.
3. Tell SvelteKit's create tool to create the demo app.
4. Run the app and browse to it.
5. Go to the "sverdle" tab in the app.
6. Try to submit an answer. The submission POST request will be blocked.

This appears to be due to this code in SvelteKit:

https://github.com/sveltejs/kit/blob/043ad0fb7c9247313542253599cd06d6da00a6bf/packages/kit/src/runtime/server/respond.js#L64-L76

I believe in particular request.headers.get('origin') !== url.origin is evaluating true. The reason is, the browser thinks the origin is localhost:8788, but by the time the request reaches workerd, the URL has been rewritten to localhost:5173, which doesn't match.

I'm not familiar with the architecture here but I assume this happens because there's a proxy sitting between the browser and workerd. It seems the proxy is rewriting the Host header when it forwards the HTTP request. If we could convince the proxy not to do that, I think it would solve the problem. That is, the proxy should forward the request with the Host header value it received from the browser, rather than rewrite it to match the workerd socket address.

Please provide a link to a minimal reproduction

No response

Please provide any relevant error logs

No response

[dario-piotrowicz]

The issue appears to be related to the fact that this is the type of pages:dev script we generate:

wrangler pages dev --compatibility-date=2024-01-17 --proxy 5173 -- npm run dev

Since if you build you application and run it through wrangler pages dev then POSTs seem to be working fine:

We've already discussed the use of wrangler pages dev with the proxying functionality on the team and I think we all agree that we should move away from it, since it is not perfect and it can introduce invalid behaviors such as this one.

I think we need to update the Svelte scripts (and probably all of the other frameworks too) to be something like what we have in Next (source), basically remove pages:dev and allow people to use the standard svelte dev command, then provide to them a pages:preview script that they can use to actually preview their application using wrangler pages dev.

That is in my opinion the best option here (I am not familiar how the proxy works either, but since we already know that it is not the optimal solution, instead of trying to fix it there I'd advocate for just not using it (which is something that we were already planning to do in any case))

One caveat here is that with the standard Svelte dev script (vite dev) currently people can't access their Bindings (as SvelteKit itself documents), I hope that that can change soon: sveltejs/kit#11323
(But note that also with the current pages:dev script developers don't get access to bindings anyways)

[mrbbot]

Thanks for the report. I'm guessing this is a duplicate of #4761.

[petebacondarwin]

I believe that this is actually a problem with the "other" proxy that is being used by the wrangler pages dev --proxy command to send requests to the vite dev command rather than run then using workerd.

I tried following the reproduction steps above (thanks @kentonv) and I can see the problem both with the current Wrangler+MIniflare releases but also with wrangler@3.18.0, which is before the current wrangler dev proxy was implemented, and even wrangler@2.21.0, which was before we moved over to workerd in Miniflare.

[petebacondarwin]

Indeed debugging into this we have this code when wrangler pages dev is proxying requests to a third party dev server (as is the case here):

workers-sdk/packages/wrangler/src/miniflare-cli/assets.ts

Lines 41 to 46 in 0de1e16

	const proxyRequest = new Request(url, miniflareRequest as RequestInit);
	if (proxyRequest.headers.get("Upgrade") === "websocket") {
	proxyRequest.headers.delete("Sec-WebSocket-Accept");
	proxyRequest.headers.delete("Sec-WebSocket-Key");
	}
	return await fetch(proxyRequest);

The call to fetch will follow the HTTP fetch spec with regards to CORS and Origin header - i.e. it will overwrite the header on the request with the origin from the request.url.

Perhaps there is an option we can pass to Undici's fetch to override this? For example I see in the Undici code it checks for request.responseTainting === "cors" before doing the overwrite and/or the request.referrerPolicy.

[petebacondarwin]

The proxy provided by Pages used in this issue is not easy to fix for the scenario set out here. Really this is not its correct use anyway. The original idea of this proxy was for SPA frameworks, like React (not full-stack, like Svelte), which had a dev server that hosted client side assets and was able to engage in things like incremental builds and hot-module reloading. In that case it makes sense for Pages to delegate the hosting of the static assets to such a dev server to avoid having to manage that itself. The idea was that code in Pages Functions would be handled by Wrangler (i.e. workerd) and static assets would be handled by the 3rd party dev server.

This doesn't really make sense for full stack frameworks, where the dev server actually wants to (and can) handle the server side behaviour as well as the static asset hosting. So going forward we are recommending the following approach.

Simple 3rd party dev server

Use the built in dev server to do day-to-day development with fast feedback cycles.
This will use the 3rd party's dev tooling to build and serve both the static (client-side) assets, and the server-side actions.
Note that in this scenario you are not running you code in an actual Cloudflare Worker nor do you have access to any Cloudflare bindings. But this is the simplest and fastest way to get started.
In the case of the Sveltekit project built by C3 this is just running npm dev, which can be seen to work as expected in the reproduction.

3rd party dev server + getBindingsProxy() helper

This is the same as the first option but additionally configuring the full-stack framework to get a proxy (agh yet another one!) but this time a JS Proxy Object that gives the node.js code running the server-side code access to real Cloudflare Worker bindings hosted by Miniflare locally.
We are working with each full-stack framework to make this as easy as possible to set up. As an example here are the docs on how to do this with Next-on-pages: https://github.com/cloudflare/next-on-pages/tree/main/internal-packages/next-dev.

3rd party build + wrangler pages dev

This last option has the highest fidelity to what will be deployed. It involves building the Pages application (incorporating any full-stack framework adaptors for Cloudflare) into a Worker and then running that locally via wrangler pages dev. Wrangler hosts both the client-side static assets and also the server-side code inside a local Worker. This Worker has full access to local bindings just like in normal Worker local development.
The downside to this approach is that on each change to the code you must rebuild the Worker and restart Wrangler, which slows down the development feedback cycle.

With these three options available, we should change the C3 starter to avoid encouraging the Pages proxy setup and document clearly how to use these options. Once that is done I think we can close this issue as won't fix.

[mrbbot]

Unrelated to whether we want to continue supporting this, I was observing strange behaviour where I could reproduce the issue when following the steps above (which installed wrangler@3.23.0 at the time), but not if I used a local checkout of wrangler@3.23.0 and ran with that. It turns out the repro was installing and using undici@5.28.2, but the local checkout was using undici@5.23.0. Normally, wrangler bundles undici, but the code for fetching from an upstream port uses miniflare's fetch() for WebSocket support, so was picking up this difference. As Kenton points out, the issue is that we're rewriting the Host header when we shouldn't be. Bisecting between undici@5.23.0 and 5.28.2 on that requirement gives this PR: nodejs/undici#2322. It seems like Vercel had a similar issue with their dev server proxy setup (vercel/vercel#10767), and switched to undici.request(), which doesn't have this behaviour. We should consider doing the same.

[petebacondarwin]

Nice work @mrbbot - I can reproduce this locally by patching miniflare to use Unidic's request rather than fetch although I haven't worked out quite the right incantation (it has a bit of a different API) to get it to work for all requests.

[mrbbot]

Nice! 🙂 Another option would be to use a custom undici Dispatcher that added back the original Host header if it were set. We did something similar to this in Miniflare 2 to remove default headers that were added by undici's fetch(), but not the Workers runtime.

See https://github.com/cloudflare/miniflare/blob/04134bb73b2d8e1dd5a7a946a703d446ea022e69/packages/core/src/standards/http.ts#L782-L837 for the dispatcher implementation and https://github.com/cloudflare/miniflare/blob/04134bb73b2d8e1dd5a7a946a703d446ea022e69/packages/core/src/standards/http.ts#L882-L887 for the usage.

miniflare's fetch() is correctly setup to receive custom undici Dispatchers via the RequestInit dispatcher property too.

[petebacondarwin]

Although #4850 does solve the original problem described above Let's also see if we can fix wrangler pages dev's proxy server as well using one of the suggested approaches discussed here.

[dario-piotrowicz]

@petebacondarwin shall we rename the issue then so not to potentially confuse people?
(maybe I'm overthinking it 😅)

[petebacondarwin]

I'm going to work on a fix this week. So hopefully no need to overthink this.