Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐛 Bug]: Server actions don't work with wrangler pages dev #588

Closed
dario-piotrowicz opened this issue Dec 12, 2023 · 3 comments · Fixed by cloudflare/workers-sdk#4630 or cloudflare/workers-sdk#4691
Labels
app router This issue is related to the App router blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) bug Something isn't working

Comments

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Dec 12, 2023

@cloudflare/next-on-pages: 1.8.2

Description

Server actions do work fine when using next dev (with or without next-dev) and also in production, they do however not work locally when previewing the application with wrangler pages dev since there seem to be a discrepancy between the request's origin header which uses localhost and the x-forwarded-host header which uses 127.0.0.1.

Tip

People can still use server actions in local mode if they go to the specific address the worker is running on
eg. instead of going to http://localhost:8788/server-actions/simple-kv-forms they can go to http://127.0.0.1:xxxxx/server-actions/simple-kv-forms and there actions should works as expected

Reproduction

https://github.com/dario-piotrowicz/next-on-pages-server-actions-test/tree/main/next-app

To reproduce the issue:

  • run npm i
  • run npm run pages:build
  • run npm run pages:dev
  • navigate to http://localhost:8788/server-actions/simple-kv-forms
  • try to set a new value using the button

This should be the result:
Screenshot 2023-12-12 at 22 50 40

Additional Information

One solution could be to set, during local development a proper value to allowedOrigins, the issue is that the config doesn't accept wildcards, and the port which is part of x-forwarded-host changes each time, so I think this is not really a feasible solution

Another idea would be to apply some logic in the build process itself so that for server actions we try to convert all potentially localhost urls to a well defined format (as for example localhost without a port), I am not a big fan of dev changes that effect the production build output, but this would likely be such a small one that it might be acceptable

@dario-piotrowicz dario-piotrowicz added bug Something isn't working app router This issue is related to the App router labels Dec 12, 2023
@dario-piotrowicz
Copy link
Member Author

dario-piotrowicz commented Dec 12, 2023

Seems like allowedOrigins is getting wildcards support soon: vercel/next.js#59428

@dario-piotrowicz dario-piotrowicz added the blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) label Dec 14, 2023
@dario-piotrowicz
Copy link
Member Author

I've labeled this issue as "blocked by external" as we need to check with the wrangler team if the x-forwarded-host can be applied on their side

petebacondarwin added a commit to cloudflare/workers-sdk that referenced this issue Dec 19, 2023
…local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Signature` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588
@petebacondarwin
Copy link
Collaborator

PR up in Wrangler to resolve this problem. See cloudflare/workers-sdk#4630

petebacondarwin added a commit to cloudflare/workers-sdk that referenced this issue Dec 20, 2023
…local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Signature` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588
petebacondarwin added a commit to cloudflare/workers-sdk that referenced this issue Dec 20, 2023
…local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588
petebacondarwin added a commit to cloudflare/workers-sdk that referenced this issue Dec 20, 2023
…local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588
petebacondarwin added a commit to cloudflare/workers-sdk that referenced this issue Jan 3, 2024
…local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588
petebacondarwin added a commit to cloudflare/workers-sdk that referenced this issue Jan 3, 2024
…4630)

* fix: miniflare now sets the "Host" header to match the upstream URL

* feat: miniflare exposes unsafeProxySignature config that controls updating host from original URL header

* fix: ensure User Worker gets the correct Host header in wrangler dev local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588

* rename "proxy signature" to "proxy shared secret"

* Move proxy shared secret to where it is needed

This avoids prop drilling.

* Use timingSafeEqual for secret comparison

This helps to avoid an attacker guessing the secret via
the timing of the comparison.
renovate bot referenced this issue in Johannes-Andersen/Johannes Jan 4, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [wrangler](https://github.com/cloudflare/workers-sdk)
([source](https://github.com/cloudflare/workers-sdk/tree/HEAD/packages/wrangler))
| [`3.22.2` ->
`3.22.3`](https://renovatebot.com/diffs/npm/wrangler/3.22.2/3.22.3) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/wrangler/3.22.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/wrangler/3.22.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/wrangler/3.22.2/3.22.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/wrangler/3.22.2/3.22.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>cloudflare/workers-sdk (wrangler)</summary>

###
[`v3.22.3`](https://github.com/cloudflare/workers-sdk/blob/HEAD/packages/wrangler/CHANGELOG.md#3223)

[Compare
Source](https://github.com/cloudflare/workers-sdk/compare/wrangler@3.22.2...wrangler@3.22.3)

##### Patch Changes

- [#&#8203;4693](https://github.com/cloudflare/workers-sdk/pull/4693)
[`93e88c43`](https://github.com/cloudflare/workers-sdk/commit/93e88c433fdd82db63b332559efaabef6c482e88)
Thanks [@&#8203;mrbbot](https://github.com/mrbbot)! - fix: ensure
`wrangler dev` exits with code `0` on clean exit

Previously, `wrangler dev` would exit with a non-zero exit code when
pressing <kbd>CTRL</kbd>+<kbd>C</kbd> or <kbd>x</kbd>. This change
ensures `wrangler` exits with code `0` in these cases.

<!---->

- [#&#8203;4630](https://github.com/cloudflare/workers-sdk/pull/4630)
[`037de5ec`](https://github.com/cloudflare/workers-sdk/commit/037de5ec77efc8261860c6d625bc90cd1f2fdd41)
Thanks [@&#8203;petebacondarwin](https://github.com/petebacondarwin)!
- fix: ensure User Worker gets the correct Host header in wrangler dev
local mode

Some full-stack frameworks, such as Next.js, check that the Host header
for a server
side action request matches the host where the application is expected
to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the
actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but
then the actual User
Worker is running on a different host:port combination than that which
the browser thinks
it should be on. This was causing the framework to think the request is
malicious and blocking
    it.

Now we update the request's Host header to that passed from the Proxy
Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret
between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret`
header. This last feature is to
prevent a malicious website from faking the Host header in a request
directly to the User Worker.

Fixes
[https://github.com/cloudflare/next-on-pages/issues/588](https://github.com/cloudflare/next-on-pages/issues/588)

<!---->

- [#&#8203;4695](https://github.com/cloudflare/workers-sdk/pull/4695)
[`0f8a03c0`](https://github.com/cloudflare/workers-sdk/commit/0f8a03c06aa3180799cf03b1e60c348620115600)
Thanks [@&#8203;mrbbot](https://github.com/mrbbot)! - fix: ensure API
failures without additional messages logged correctly

<!---->

- [#&#8203;4693](https://github.com/cloudflare/workers-sdk/pull/4693)
[`93e88c43`](https://github.com/cloudflare/workers-sdk/commit/93e88c433fdd82db63b332559efaabef6c482e88)
Thanks [@&#8203;mrbbot](https://github.com/mrbbot)! - fix: ensure
`wrangler pages dev` exits cleanly

Previously, pressing <kbd>CTRL</kbd>+<kbd>C</kbd> or <kbd>x</kbd> when
running `wrangler pages dev` wouldn't actually exit `wrangler`. You'd
need to press <kbd>CTRL</kbd>+<kbd>C</kbd> a second time to exit the
process. This change ensures `wrangler` exits the first time.

<!---->

- [#&#8203;4696](https://github.com/cloudflare/workers-sdk/pull/4696)
[`624084c4`](https://github.com/cloudflare/workers-sdk/commit/624084c447a4898c4273c26e3ea24ea069a2900b)
Thanks [@&#8203;mrbbot](https://github.com/mrbbot)! - fix: include
additional modules in `largest dependencies` warning

If your Worker fails to deploy because it's too large, Wrangler will
display of list of your Worker's largest dependencies. Previously, this
just included JavaScript dependencies. This change ensures additional
module dependencies (e.g. WebAssembly, text blobs, etc.) are included
when computing this list.

- Updated dependencies
\[[`037de5ec`](https://github.com/cloudflare/workers-sdk/commit/037de5ec77efc8261860c6d625bc90cd1f2fdd41)]:
    -   miniflare@3.20231218.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Johannes-Andersen/Johannes).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app router This issue is related to the App router blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) bug Something isn't working
Projects
None yet
2 participants