Hotfix: Backport host/origin validation to requests and websocket connections

.github/workflows/publish.md

@@ -3,12 +3,35 @@
 ## Preparation
 
 1. `git checkout v7`
-2. set node version to `18.20.2`
+2. Set node version to `18.20.2`
 3. `git pull`
-4. `git checkout -b hotfix/v<next-patch-release-version>`
-5. Apply necessary hotfixes
-6. `cd scripts && yarn release:version --deferred --release-type patch --verbose && cd .. && git add . && git commit -m "Bump deferred version"`
-7. Trigger canary release via dispatching the workflow for `publish-canary`
-8. Test the canary release
-9. Merge `hotfix/v<next-patch-release-version>` into `v7`
-10. Observe the `publish-normal` job
+4. Define which next patch version this release will be (last released version + patch e.g. 7.1.2 -> 7.1.3)
+5. `git checkout -b hotfix/v<next-patch-release-version>`
+6. Apply necessary hotfixes, finish the work and make a PR. Save that PR number to use later.
+7. At the root of the repo, run a script to prepare for the new version
+   - `cd scripts && yarn release:version --deferred --release-type patch --verbose && cd .. && git add . && git commit -m "Bump deferred version"`
+8. Manually add a new entry for the new version to the `CHANGELOG.md` file including the description of the change
+9. Trigger canary release to test in real projects
+   1. Add the hotfix branch name as deployment branch here: https://github.com/storybookjs/storybook/settings/environments/1012979736/edit
+   2. Dispatch the `publish-canary` workflow and select the hotfix branch name and PR number: https://github.com/storybookjs/storybook/actions/workflows/publish.yml
+   3. Test the canary release (MealDrop has a [storybook/7.0.0](https://github.com/yannbf/mealdrop/tree/storybook/7.0.0) branch if you like to use for testing)
+   4. Remove the deployment branch name added in step 1
+10. Merge `hotfix/v<next-patch-release-version>` into `v7`
+11. Observe the `publish-normal` job
+12. Observe the generated release in GitHub releases page and make modifications to the release notes if necessary
+
+## Known CI issues
+
+Some CI failures are known and acceptable, so long as they do not impact the patch changes. Here's an overview of currently known and ignorable CI failures:
+
+- ci/circleci: chromatic-sandboxes
+- UI Tests: storybook-ui
+- UI Tests: svelte-kit/skeleton-js
+- UI Tests: svelte-kit/skeleton-ts
+- UI Tests: svelte-vite/default-js
+- UI Tests: svelte-vite/default-ts
+- UI Review: bench/react-vite-default-ts-test-build
+- UI Review: bench/react-webpack-18-ts-test-build
+- UI Review: storybook-ui
+
+The Chromatic CLI is yielding an error status code due to some broken stories.

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 7.6.24
+
+- Add request validation
+
 ## 7.6.23
 
 - Harden websocke connection

code/builders/builder-manager/src/utils/template.ts

@@ -53,7 +53,11 @@ export const renderHTML = async (
   if (configType === 'DEVELOPMENT') {
     const coreOptions = await presets.apply('core');
     // Manager only needs the token currently, so we don't pass any other channel options.
-    globals.CHANNEL_OPTIONS = JSON.stringify({ wsToken: coreOptions?.channelOptions?.wsToken }, null, 2);
+    globals.CHANNEL_OPTIONS = JSON.stringify(
+      { wsToken: coreOptions?.channelOptions?.wsToken },
+      null,
+      2
+    );
   }
 
   return render(templateRef, {

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

@@ -9,12 +9,15 @@ export async function createViteServer(options: Options, devServer: Server) {
 
   const commonCfg = await commonConfig(options, 'development');
 
+  const { allowedHosts } = await presets.apply('core', {});
+
   const config = {
     ...commonCfg,
     // Needed in Vite 5: https://github.com/storybookjs/storybook/issues/25256
     assetsInclude: ['/sb-preview/**'],
     // Set up dev server
     server: {
+      allowedHosts,
       middlewareMode: true,
       hmr: {
         port: options.port,
@@ -28,6 +31,14 @@ export async function createViteServer(options: Options, devServer: Server) {
     optimizeDeps: await getOptimizeDeps(commonCfg, options),
   };
 
+  // '0.0.0.0' binds to all interfaces, which is useful for Docker and other containerized environments
+  if (
+    options.host === '0.0.0.0' &&
+    (!allowedHosts || (Array.isArray(allowedHosts) && allowedHosts.length === 0))
+  ) {
+    config.server.allowedHosts = true;
+  }
+
   const finalConfig = await presets.apply('viteFinal', config, options);
 
   const { createServer } = await import('vite');

code/lib/core-server/src/build-dev.ts

@@ -1,7 +1,6 @@
 import type {
   BuilderOptions,
   CLIOptions,
-  CoreConfig,
   LoadOptions,
   Options,
   StorybookConfig,
@@ -20,7 +19,7 @@ import { global } from '@storybook/global';
 import { telemetry } from '@storybook/telemetry';
 
 import { join, resolve } from 'path';
-import { deprecate } from '@storybook/node-logger';
+import { deprecate, logger } from '@storybook/node-logger';
 import dedent from 'ts-dedent';
 import { readFile } from 'fs-extra';
 import { MissingBuilderError } from '@storybook/core-events/server-errors';
@@ -102,7 +101,20 @@ export async function buildDevStandalone(
     isCritical: true,
   });
 
-  const { renderer, builder, disableTelemetry } = await presets.apply<CoreConfig>('core', {});
+  const { allowedHosts, renderer, builder, disableTelemetry } = await presets.apply('core', {});
+
+  // '0.0.0.0' binds to all interfaces, which is useful for Docker and other containerized environments.
+  // By default we allow requests from all hosts in this case, but the user should be made aware of the risk.
+  if (
+    options.host === '0.0.0.0' &&
+    (!allowedHosts || (allowedHosts !== true && allowedHosts.length === 0))
+  ) {
+    logger.warn(dedent`
+      --host is set to 0.0.0.0 but no allowedHosts are defined. Allowing all hosts.
+      To restrict allowed hosts, set core.allowedHosts in your main Storybook config.
+      See: https://storybook.js.org/docs/api/main-config/main-config-core
+    `);
+  }
 
   if (!builder) {
     throw new MissingBuilderError();
@@ -208,6 +220,7 @@ export async function buildDevStandalone(
         name,
         address,
         networkAddress,
+        allowedHosts,
         managerTotalTime,
         previewTotalTime,
       });

code/lib/core-server/src/dev-server.ts

@@ -24,6 +24,7 @@ import { getStoryIndexGenerator } from './utils/getStoryIndexGenerator';
 import { doTelemetry } from './utils/doTelemetry';
 import { router } from './utils/router';
 import { getAccessControlMiddleware } from './utils/getAccessControlMiddleware';
+import { getHostValidationMiddleware } from './utils/getHostValidationMiddleware';
 import { getCachingMiddleware } from './utils/get-caching-middleware';
 
 export async function storybookDevServer(options: Options) {
@@ -37,9 +38,20 @@ export async function storybookDevServer(options: Options) {
 
   assert(core?.channelOptions?.wsToken, 'wsToken is required for securing the server channel');
 
+  const { port, host, initialPath } = options;
+  invariant(port, 'expected options to have a port');
+  const proto = options.https ? 'https' : 'http';
+  const { address, networkAddress } = getServerAddresses(port, host, proto, initialPath);
+
   const serverChannel = await options.presets.apply(
     'experimental_serverChannel',
-    getServerChannel(server, core.channelOptions.wsToken)
+    getServerChannel(server, {
+      token: core.channelOptions.wsToken,
+      host,
+      allowedHosts: core.allowedHosts,
+      localAddress: address,
+      networkAddress,
+    })
   );
 
   if (features?.storyStoreV7 === false) {
@@ -63,18 +75,21 @@ export async function storybookDevServer(options: Options) {
     options.extendServer(server);
   }
 
+  app.use(
+    getHostValidationMiddleware({
+      host,
+      allowedHosts: core?.allowedHosts,
+      localAddress: address,
+      networkAddress,
+    })
+  );
   app.use(getAccessControlMiddleware(core?.crossOriginIsolated ?? false));
   app.use(getCachingMiddleware());
 
   getMiddleware(options.configDir)(router);
 
   app.use(router);
 
-  const { port, host, initialPath } = options;
-  invariant(port, 'expected options to have a port');
-  const proto = options.https ? 'https' : 'http';
-  const { address, networkAddress } = getServerAddresses(port, host, proto, initialPath);
-
   const listening = new Promise<void>((resolve, reject) => {
     // @ts-expect-error (Following line doesn't match TypeScript signature at all 🤔)
     server.listen({ port, host }, (error: Error) => (error ? reject(error) : resolve()));

code/lib/core-server/src/utils/__tests__/server-channel.test.ts

@@ -1,31 +1,38 @@
-import type { Server } from 'http';
 import { Channel } from '@storybook/channels';
 
 import { EventEmitter } from 'events';
+import type { Server } from 'http';
 import { stringify } from 'telejson';
-import { getServerChannel, ServerChannelTransport } from '../get-server-channel';
+
+import { ServerChannelTransport, getServerChannel } from '../get-server-channel';
+
+const mockToken = 'test-token-123';
+
+const options = {
+  localAddress: 'http://localhost:6006',
+  networkAddress: 'http://192.168.1.100:6006',
+  token: mockToken,
+} as any;
 
 describe('getServerChannel', () => {
   test('should return a channel', () => {
     const server = { on: jest.fn() } as any as Server;
-    const result = getServerChannel(server, 'test-token-123');
+    const result = getServerChannel(server, options);
     expect(result).toBeInstanceOf(Channel);
   });
 
   test('should attach to the http server', () => {
     const server = { on: jest.fn() } as any as Server;
-    getServerChannel(server, 'test-token-123');
+    getServerChannel(server, options);
     expect(server.on).toHaveBeenCalledWith('upgrade', expect.any(Function));
   });
 });
 
 describe('ServerChannelTransport', () => {
-  const mockToken = 'test-token-123';
-
   it('parses simple JSON', () => {
     const server = new EventEmitter() as any as Server;
     const socket = new EventEmitter();
-    const transport = new ServerChannelTransport(server, mockToken);
+    const transport = new ServerChannelTransport(server, options);
     const handler = jest.fn();
     transport.setHandler(handler);
 
@@ -39,7 +46,7 @@ describe('ServerChannelTransport', () => {
   it('parses object JSON', () => {
     const server = new EventEmitter() as any as Server;
     const socket = new EventEmitter();
-    const transport = new ServerChannelTransport(server, mockToken);
+    const transport = new ServerChannelTransport(server, options);
     const handler = jest.fn();
     transport.setHandler(handler);
 
@@ -52,7 +59,7 @@ describe('ServerChannelTransport', () => {
   test('supports telejson cyclical data', () => {
     const server = new EventEmitter() as any as Server;
     const socket = new EventEmitter();
-    const transport = new ServerChannelTransport(server, mockToken);
+    const transport = new ServerChannelTransport(server, options);
     const handler = jest.fn();
     transport.setHandler(handler);
 
@@ -73,7 +80,7 @@ describe('ServerChannelTransport', () => {
   test('skips telejson classes and functions in data', () => {
     const server = new EventEmitter() as any as Server;
     const socket = new EventEmitter();
-    const transport = new ServerChannelTransport(server, mockToken);
+    const transport = new ServerChannelTransport(server, options);
     const handler = jest.fn();
     transport.setHandler(handler);
 
@@ -93,14 +100,45 @@ describe('ServerChannelTransport', () => {
     socket.write = jest.fn();
     socket.destroy = jest.fn();
     const destroySpy = jest.spyOn(socket, 'destroy');
-    const transport = new ServerChannelTransport(server, mockToken);
+    const transport = new ServerChannelTransport(server, options);
 
     const handler = jest.fn();
     transport.setHandler(handler);
 
     // Simulate upgrade request with wrong token
     const request = {
       url: '/storybook-server-channel?token=wrong-token',
+      headers: {
+        origin: 'http://localhost:6006',
+      },
+    } as any;
+    const head = Buffer.from('');
+
+    server.listeners('upgrade')[0](request, socket, head);
+
+    expect(socket.write).toHaveBeenCalledWith(
+      'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n'
+    );
+    expect(destroySpy).toHaveBeenCalled();
+  });
+
+  it('rejects connections with invalid origin', () => {
+    const server = new EventEmitter() as any as Server;
+    const socket = new EventEmitter() as any;
+    socket.write = jest.fn();
+    socket.destroy = jest.fn();
+    const destroySpy = jest.spyOn(socket, 'destroy');
+    const transport = new ServerChannelTransport(server, options);
+
+    const handler = jest.fn();
+    transport.setHandler(handler);
+
+    // Simulate upgrade request with wrong token
+    const request = {
+      url: `/storybook-server-channel?token=${options.token}`,
+      headers: {
+        origin: 'http://illegal-host.com',
+      },
     } as any;
     const head = Buffer.from('');
 
@@ -112,22 +150,25 @@ describe('ServerChannelTransport', () => {
     expect(destroySpy).toHaveBeenCalled();
   });
 
-  it('accepts connections with valid token', () => {
+  it('accepts connections with valid token and origin', () => {
     const server = new EventEmitter() as any as Server;
     const socket = new EventEmitter() as any;
     socket.write = jest.fn();
     socket.destroy = jest.fn();
     const destroySpy = jest.spyOn(socket, 'destroy');
     const handleUpgradeSpy = jest.fn();
-    const transport = new ServerChannelTransport(server, mockToken);
+    const transport = new ServerChannelTransport(server, options);
 
     // Mock handleUpgrade to track if it's called
     // @ts-expect-error (accessing private property)
     transport.socket.handleUpgrade = handleUpgradeSpy;
 
     // Simulate upgrade request with correct token
     const request = {
-      url: `/storybook-server-channel?token=${mockToken}`,
+      url: `/storybook-server-channel?token=${options.token}`,
+      headers: {
+        origin: 'http://localhost:6006',
+      },
     } as any;
     const head = Buffer.from('');