Builder-Vite: Fix allowedHosts handling for custom hosts

[JSMike]

Update yarn lock to use version of vite that has allowedHosts in ServerOptions.
Add logic to pass forward server.allowedHosts from user's vite config, otherwise check if server is bound to value that should default allowedHosts to true, otherwise use set allowedHosts to contain specified host.

Closes #30338

What I did

Add logic to define allowedHosts either by passing forward config from user's vite config, or setting default. This required updating some of the yarn lock to use a version that contained the type for allowedHosts, but remained in respected semver range. This change should have no impact on prior versions because the allowedHosts value will be ignored by vite.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

1. Run a sandbox for a vite based template, e.g. yarn task --task sandbox --start-from auto --template lit-vite/default-ts
2. Open Storybook in your browser, using your system's hostname instead of localhost, by default this should load as expected.
3. Update the sandbox vite.config.js to contain: { server: { allowedHosts: ['localhost'] } }
4. Restart the storybook yarn run storybook from your sandbox
5. Reload browser with your hostname as the URL and the preview should fail to load, as expected, since it is no longer an allowed hostname.
6. Reload browser with localhost as the URL and the preview should load.

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name	before	after	diff	z	%
createSize	0 B	0 B	0 B	-	-
generateSize	78.3 MB	78.3 MB	0 B	2.85	0%
initSize	131 MB	131 MB	436 B	2.66	0%
diffSize	53 MB	53 MB	436 B	0.59	0%
buildSize	7.17 MB	7.17 MB	0 B	0.35	0%
buildSbAddonsSize	1.85 MB	1.85 MB	0 B	0.23	0%
buildSbCommonSize	195 kB	195 kB	0 B	-	0%
buildSbManagerSize	1.86 MB	1.86 MB	0 B	-0.58	0%
buildSbPreviewSize	0 B	0 B	0 B	-	-
buildStaticSize	0 B	0 B	0 B	-	-
buildPrebuildSize	3.91 MB	3.91 MB	0 B	0.18	0%
buildPreviewSize	3.26 MB	3.26 MB	0 B	0.35	0%
testBuildSize	0 B	0 B	0 B	-	-
testBuildSbAddonsSize	0 B	0 B	0 B	-	-
testBuildSbCommonSize	0 B	0 B	0 B	-	-
testBuildSbManagerSize	0 B	0 B	0 B	-	-
testBuildSbPreviewSize	0 B	0 B	0 B	-	-
testBuildStaticSize	0 B	0 B	0 B	-	-
testBuildPrebuildSize	0 B	0 B	0 B	-	-
testBuildPreviewSize	0 B	0 B	0 B	-	-

name	before	after	diff	z	%
createTime	22.4s	24.2s	1.8s	0.86	7.6%
generateTime	19.4s	17.7s	-1s -683ms	-1.13	-9.5%
initTime	12.6s	11.5s	-1s -173ms	-0.83	-10.2%
buildTime	9.9s	7.9s	-2s -80ms	-0.92	-26.3%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	4.8s	6.5s	1.6s	0.59	25.6%
devManagerResponsive	3.5s	4.9s	1.3s	0.77	28%
devManagerHeaderVisible	816ms	1s	234ms	1.83	🔺22.3%
devManagerIndexVisible	852ms	1.1s	330ms	2.49	🔺27.9%
devStoryVisibleUncached	3.8s	2.9s	-883ms	-1.55	🔰-30.1%
devStoryVisible	853ms	1.1s	330ms	2.45	🔺27.9%
devAutodocsVisible	850ms	957ms	107ms	1.8	🔺11.2%
devMDXVisible	724ms	975ms	251ms	2.22	🔺25.7%
buildManagerHeaderVisible	847ms	3s	2.1s	7.94	🔺72.2%
buildManagerIndexVisible	941ms	3.1s	2.2s	7.56	🔺70.1%
buildStoryVisible	836ms	2.8s	1.9s	7.41	🔺70.5%
buildAutodocsVisible	608ms	2.7s	2.1s	1.85	🔺77.6%
buildMDXVisible	669ms	1.3s	640ms	5.06	🔺48.9%

Greptile Summary

Let me provide a clear and concise summary of this pull request:

This PR fixes an issue where Storybook was not accessible via local hostnames after upgrading to Vite 6. The key changes include:

• Added proper handling of allowedHosts configuration in Vite server setup
• Preserves user-defined allowedHosts settings from vite config
• Automatically sets allowedHosts: true for common local addresses (::/0.0.0.0/127.0.0.1)
• Falls back to including specific host address in allowedHosts array when not using common local addresses
• Updates dependencies and documentation to support these changes

Key files changed:

• code/builders/builder-vite/src/vite-server.ts: Main implementation of allowedHosts handling
• CHANGELOG.md: Documents the fix for hostname accessibility issues
• Various framework documentation files: Updated sidebar ordering

The changes are focused and minimal, addressing issue #30338 where Storybook became inaccessible via local hostnames after the Vite 6 upgrade. The implementation properly handles backward compatibility and provides secure defaults for development environments.

_{💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!}

[greptile-apps]

_{1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

LGTM

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 2e7da03.

Command	Status	Duration	Result
nx run-many -t build --parallel=3	✅ Succeeded	1m 48s	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-02-05 01:05:48 UTC

[shilman]

@JSMike Thanks so much for putting this together!!

A few questions:

1. Have you tested with an older version of Vite? Will it just ignore the option in that case?
2. In the fallback case of allowedHosts = true, is that a security issue? Or it doesn't really apply because we're only running in dev?

[JSMike]

@shilman

1. Have you tested with an older version of Vite? Will it just ignore the option in that case?

I confirmed with vite 4.0.4 (oldest version that was listed within the project), and setting server.allowedHosts has no impact.

2. In the fallback case of allowedHosts = true, is that a security issue? Or it doesn't really apply because we're only running in dev?

This setting only applies when running Storybook/Vite in server mode, and the vite config is being passed a 'devServer' instance from Storybook core-server/dev-server. This would have no impact on the built storybook application assets. Developers should be discouraged from using Storybook devServer in production. This PR defaults functionality back to how the devServer behaved prior to the addition of allowHosts from Vite. There are some security reasons for that setting, ie, spoofing host names and malicious calls to the devServer that then make return data as if it were from the host, while these should be considered it is best practice to not hit production data while doing development, and against best practices to use a development server to host in production. If you're concerned I would suggest reviewing with a security professional to get their opinion, but defaulting to something other than true would result in a breaking change and require configuration changes from applications that are setting ip addresses as the host. That type of change would be better suited for Storybook v9 if you want to change the default behavior.

One additional note, in all of my testing of different combinations of --hosts and allowedHosts, I noticed that that the configuration for the storybook devServer getServerAddresses is hard coded to localhost and shows in the CLI even if you bind to an ip address other than localhost. If you bind to 127.0.1.1 for example, if that's your local hostname's IP address and you set the allowedHosts in your viteConfig to be your system's hostname instead of localhost, then the CLI will still direct you to use http://localhost:6006 even though you didn't bind the server to a port associated with localhost and the page will not load on localhost. This issue is not related/due to this PR, this is a separate pre-existing issue.

[greptile-apps]

_{24 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

style: Consider extracting this regex to a constant at module level since it's a static pattern that could be reused

[greptile-apps]

style: Consider adding a comment explaining the logic: true allows all hosts, array restricts to specific hosts

[sapphi-red]

Or it doesn't really apply because we're only running in dev?

@shilman Answering from me as well, as I found this PR and wanted to confirm that you know the impact of doing so.

If you are ok with having your source code (and other contents served on the dev server) viewed by other people, it is fine. For example, if you have to make your source code secret, then setting allowHosts: true would be a problem. If your source code (and the content served on the dev server) is open source and doesn't matter if the source code is leaked before you push to the repository, it is fine to set allowHosts: true.

If you are going to set allowedHosts: true by default, I'll suggest printing a warning in the CLI or at least have a section in the docs informing the user that the source code should be treated as being exposed to the Internet.

[shilman]

@sapphi-red thanks so much for jumping in. it's a grey area since users should never run storybook dev in production. but i agree with you that when in doubt it probably makes sense to be more secure.

@JSMike i also agree with you that this is an easier change to make in a major release, and we are coming up on Storybook 9 in the next few months. so one option would be to attach allowedHosts: true to a feature flag and then remove it in 9.0.

I'll discuss with the team see if anybody has strong opinions one way or the other.

[shilman]

@sapphi-red does this exploit expose .env files or anything that's not explicitly served by the dev server?

[sapphi-red]

since users should never run storybook dev in production.

I think this is the opposite. This vulnerability mainly affects when a web browser is used alongside the dev server. In production, you won't have a web browser running, so I think in most cases it doesn't affect in prod.
https://github.com/vitejs/vite/security/advisories/GHSA-vg6x-rcgg-rjx6#:~:text=the%20HMR%20messages.-,%5B3%5D%3A%20Lack%20of%20validation%20on%20the%20Host%20header%20for%20HTTP%20requests,of%20http%3A//127.0.0.1%3A5173/main.js%20bypassing%20the%20same%20origin%20policy.,-Impact

[sapphi-red]

@sapphi-red does this exploit expose .env files or anything that's not explicitly served by the dev server?

No. This only exposes the files allowed by server.fs.allow and not disallowed by server.fs.deny. .env files are disallowed by default so unless storybook overrides server.fs.deny, it won't be exposed.

[JSMike]

@shilman
From my understanding this would only impact development where the host is binding to a public / open port where an attacker is able to find your ip address and detect that you're using an hmr dev server that's bound to an open port without hostname restriction on the devServer. Hopefully most developers with sensitive data aren't binding to public ports on open networks, but I'm sure that's not always the case, the real risk is when someone uses vite server with hmr in prod / open networks. I understand the concern and wanting a safe default but this has been an existing issue since starting to use vite+hmr. I'm unsure if you've gotten any complaints about this issue from any storybook consumers.

My personal suggestion would be to keep status-quo, add some documentation about this setting and print a warning message when server.allowedHosts === true when running storybook dev server. That is basically what webpack does when using webpackDevServer with an equivalent setting. Obviously talk with your team and some security experts to make the best decision for Storybook.

[sapphi-red]

From my understanding this would only impact development where the host is binding to a public / open port where an attacker is able to find your ip address and detect that you're using an hmr dev server that's bound to an open port without hostname restriction on the devServer.

@JSMike I agree that most developers with sensitive data aren’t / shouldn't binding to public ports on open networks. However, this attack described in the report is possible even if the dev server is only binding to the local port. It also does not require to know the IP address. The attacker needs to know the port number. But the default port number is normally used and the attacker can simply brute force the ports if needed, so the port number itself isn’t necessarily a barrier.

[sapphi-red]

@shilman Was this released in v8.5.4 intentionally? (no criticism, just confirming. I understand that prioritizing not to break things is a valid option.) If so, was the reason for doing so is because you assumed that most people would not care about this? I'm planning to post messages on social media to tell people that if they need to stop exposing the source code, they have to set server.allowedHosts explicitly even if they upgraded Vite. I think users won't notice that they have to do it without those kind of messages.

[shilman]

@sapphi-red thanks so much for checking, and for your hard work on this issue. We didn't have a chance to discuss as a team, but our next release will be a semver major so we can do it properly if we choose.

[dominikg]

In my opinion you should release a patch that reverts this change and let users opt-in with explicit allowedHosts if they need it.

Waiting for the next major is going to take time and not every user will be able to update immediately, leaving users of storybook@8 vulnerable by default, which should prompt someone to refile the vite security advisory against storybook.

[dominikg]

please also consider that we had multiple advisories against fs.deny bypasses in the past, so if there was another way to bypass that, this would lead to full fs access through storybook dev for any website visited in a browser on that host.

[shilman]

Thanks @dominikg. I'll discuss with the team and follow up here.

[JReinhold]

We've decided to partially revert this. #30523