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

fix: allow CORS from loopback addresses by default #19249

Merged

Conversation

sapphi-red
Copy link
Member

Description

Allow CORS from the following origins by default for convenience.

  • hosts with localhost, domains of .localhost
  • hosts with 127.0.0.1
  • hosts with ::1

Technically, other addresses in 127.0.0/8 are also loopback addresses and safe to be allowed. But I didn't include them as it's not popular and makes the regex complicated.

refs #19239

@timacdonald
Copy link
Contributor

timacdonald commented Jan 21, 2025

@sapphi-red, thoughts on supporting the popular *.test tld here as well?

@sapphi-red
Copy link
Member Author

I guess we can add .test as that TLD is reserved. But let me check if there aren't known any attack vector.

@@ -161,7 +161,7 @@ export default defineConfig({
## server.cors

- **Type:** `boolean | CorsOptions`
- **Default:** `false`
- **Default:** `{ origin: /^https?:\/\/(?:(?:[^:]+\.)?localhost|127\.0\.0\.1|\[::1\])(?::\d+)?$/ }` (allows localhost, `127.0.0.1` and `::1`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can also show that you can use defaultAllowedOrigins here so users don't copy the regex as is

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't expose defaultAllowedOrigins so it's not possible to import it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah didn't notice that. I think it'd be useful to expose it, but we can do that in a later PR.

@sapphi-red sapphi-red merged commit 3d03899 into vitejs:main Jan 21, 2025
15 checks passed
@sapphi-red sapphi-red deleted the fix/allow-loopback-addresses-by-default branch January 21, 2025 10:20
@sapphi-red
Copy link
Member Author

@timacdonald This PR has been released in 6.0.11, 5.4.14, 4.5.9. We didn't include *.test by default (#19250) as we aren't sure if

  • everyone would consider *.test origins to be a trusted source
  • that is not discouraged by the RFC

@timacdonald
Copy link
Contributor

timacdonald commented Jan 21, 2025

Appreciate your work and help here.

We will add *.test in the Laravel plugin as this is the defecto TLD for local development in much of the tooling in the PHP ecosystem and any DNS change would need to be an intentional one.

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.

4 participants