Builder-Vite: Fix logic related to setting allowedHosts when IP address used #31472

JSMike · 2025-05-14T02:47:02Z

What I did

Previous PR set allowedHosts to true by default, this was reverted but no longer set the allowedHosts value when an IP address was used for options.host.

This change sets allowedHosts to true only if allowedHosts is undefined and an IP address is passed as the host. Otherwise no hostname other than 'localhost' would be allowed to access the devserver.

Note, this is my personal opinion on what the logic should be, to more closely align with existing functionality prior to the breaking change from Vite. If, for security reasons, this is not desired then updating the PR to add a warning to the console would be another good option, along with additional documentation on setting allowedHosts when using the Vite dev-server with Storybook.

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!

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>

Greptile Summary

This PR enhances Vite server security in Storybook by implementing stricter host access controls while maintaining flexibility for custom configurations.

Modified code/builders/builder-vite/src/vite-server.ts to restrict access to localhost by default, following Vite's secure practices
Added support for IP-based host access through conditional allowedHosts configuration
Implemented clear error messaging in both console and iframe for unauthorized host access
Added ability to override host restrictions via --host CLI flag or viteFinal config
Fixed host validation logic when using IP addresses as server hosts

The changes improve security while maintaining backward compatibility and providing clear paths for customization when needed.

_{💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!}

greptile-apps

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

greptile-apps · 2025-05-14T02:47:50Z

code/builders/builder-vite/src/vite-server.ts

-    !ipRegex.test(options.host)
-  ) {
-    config.server.allowedHosts = [options.host.toLowerCase()];
+  if (options.host && !config.server.allowedHosts) {


style: Check for undefined/null specifically rather than falsy values to avoid overriding empty arrays

Empty array should likely be overridden if options.host is set.

JSMike · 2025-05-14T02:48:38Z

@JReinhold @valentinpalkovic https://github.com/storybookjs/storybook/pull/30523/files#r2079670800

Thanks for the explanation @JSMike. ❤️

Could you open a PR adding the required logic back, with an explanation of which problem it solves? Sounds like you know a lot about this problem already.

greptile-apps

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

greptile-apps · 2025-05-14T02:48:50Z

code/builders/builder-vite/src/vite-server.ts

+
+    if (ipRegex.test(options.host)) {
+      // If options.host is set to an IP address then allow all hosts
+      config.server.allowedHosts = true;


style: Consider adding a warning log when allowing all hosts, as this reduces security

A warning here would be appropriate, let me know what you'd like the warning message to be.

Or, if you'd prefer to not set allowedHosts to true then this would be a good place to add a warning to inform users that only the direct IP address and localhost will be allowed and that other hostnames will be rejected unless an allowedHosts array is added to the viteFinal.

thank you for your patience @JSMike. Can you help me understand why this condition is necessary - why do we need to set true when it's an IP address? What is stopping us from just setting that IP address as the only entry in allowedHosts?

I'm sure there is a reason for this, but I haven't been able to find it through the various related issues/PRs.

My thought is when I set an IP address for the host, I'm usually trying to use the IP/Host configuration assigned to the host machine. When developing with docker containers I typically end up setting to the host to '0.0.0.0' in order to then inherit the binding of the machine, as the exact hostname/url isn't always known. In these cases the default of localhost typically does not apply. If this is an advanced use case that should always require setting the allowedHosts variable that would be understandable. My idea with this logic was also to more closely align with how the configuration worked for Storybook prior to the change from Vite. This is really just my opinion. If you'd prefer to not have this logic then I can update the PR. Or maybe this logic should only apply specifically when using '0.0.0.0', as that's specifying to listen on all network interfaces?

Or maybe this logic should only apply specifically when using '0.0.0.0', as that's specifying to listen on all network interfaces?

I like this approach actually, I think we should make it like this, including a warning stating it, like:

Setting server.allowedHosts to true in the Vite config because you set --host to 0.0.0.0. You can manually override this in your viteFinal configuration, see https://vite.dev/config/server-options.html#server-allowedhosts and https://storybook.js.org/docs/api/main-config/main-config-vite-final

Ok, I'll push something up soon.

@JReinhold I updated the logic and the messaging. Let me know if that looks good or you'd like any additional changes.

nx-cloud · 2025-05-14T02:51:45Z

View your CI Pipeline Execution ↗ for commit 9baeb4d

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

☁️ Nx Cloud last updated this comment at 2025-07-13 03:08:28 UTC

storybook-app-bot · 2025-06-09T19:29:25Z

Package Benchmarks

^{Commit: 9baeb4d, ran on 13 July 2025 at 02:51:01 UTC}

No significant changes detected, all good. 👏

JReinhold

Only a minor request on the logging, otherwise LGTM ❤️

code/builders/builder-vite/src/vite-server.ts

JSMike · 2025-07-13T01:56:06Z

@JReinhold applied the requested changes, let me know if anything else needs fixing.

…ss used

JReinhold · 2025-07-14T08:22:28Z

Thank you ❤️

greptile-apps bot reviewed May 14, 2025

View reviewed changes

JSMike force-pushed the builder-vite-allowedhosts branch 4 times, most recently from a7205f3 to 529154e Compare May 17, 2025 16:41

valentinpalkovic added builder-vite bug labels May 19, 2025

valentinpalkovic assigned JReinhold May 19, 2025

valentinpalkovic added the ci:normal label May 19, 2025

JSMike force-pushed the builder-vite-allowedhosts branch 3 times, most recently from 7185d02 to e80df3b Compare May 28, 2025 01:52

JSMike force-pushed the builder-vite-allowedhosts branch 4 times, most recently from 84aa210 to f30ca5b Compare June 2, 2025 12:15

JSMike force-pushed the builder-vite-allowedhosts branch from f30ca5b to 6585c65 Compare June 9, 2025 19:18

JSMike force-pushed the builder-vite-allowedhosts branch 4 times, most recently from 8280b40 to 554d2d9 Compare June 26, 2025 00:21

JSMike force-pushed the builder-vite-allowedhosts branch 3 times, most recently from ae01da9 to 2a45f63 Compare July 1, 2025 02:08

JReinhold requested changes Jul 1, 2025

View reviewed changes

code/builders/builder-vite/src/vite-server.ts Outdated Show resolved Hide resolved

JSMike force-pushed the builder-vite-allowedhosts branch from b4e50f8 to aea6501 Compare July 13, 2025 01:51

Builder-Vite: Fix logic related to setting allowedHosts when IP addre…

9baeb4d

…ss used

JSMike force-pushed the builder-vite-allowedhosts branch from aea6501 to 9baeb4d Compare July 13, 2025 02:42

JReinhold approved these changes Jul 14, 2025

View reviewed changes

JReinhold merged commit 60ea56a into storybookjs:next Jul 14, 2025
23 of 25 checks passed

github-actions bot mentioned this pull request Jul 14, 2025

Release: Prerelease 9.1.0-alpha.8 #31973

Merged

18 tasks

github-actions bot mentioned this pull request Aug 13, 2025

[master] Michijs Dependabot changes michijs/michijs-storybook-template#68

Closed

Uh oh!

Builder-Vite: Fix logic related to setting allowedHosts when IP address used #31472

Builder-Vite: Fix logic related to setting allowedHosts when IP address used #31472

Uh oh!

Conversation

JSMike commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

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

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot May 14, 2025

Choose a reason for hiding this comment

Uh oh!

JSMike May 14, 2025

Choose a reason for hiding this comment

Uh oh!

JSMike commented May 14, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot May 14, 2025

Choose a reason for hiding this comment

Uh oh!

JSMike May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JReinhold Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

JSMike Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JReinhold Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

JSMike Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

JSMike Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

nx-cloud bot commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

storybook-app-bot bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package Benchmarks

Uh oh!

JReinhold left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JSMike commented Jul 13, 2025

Uh oh!

Uh oh!

JReinhold commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JSMike commented May 14, 2025 •

edited

Loading

JSMike May 14, 2025 •

edited

Loading

JSMike Jun 15, 2025 •

edited

Loading

nx-cloud bot commented May 14, 2025 •

edited

Loading

storybook-app-bot bot commented Jun 9, 2025 •

edited

Loading