Skip to content
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dirname, join, resolve } from 'node:path';
import { join, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';

import {
Expand Down
8 changes: 6 additions & 2 deletions code/core/src/builder-manager/utils/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import {
import { type Options, SupportedBuilder } from 'storybook/internal/types';

export const buildFrameworkGlobalsFromOptions = async (options: Options) => {
const globals: Record<string, string | undefined> = {};
const globals: Record<string, any> = {};

const builderConfig = (await options.presets.apply('core')).builder;
const { builder: builderConfig, channelOptions } = await options.presets.apply('core');
const builderName = typeof builderConfig === 'string' ? builderConfig : builderConfig?.name;
const builder = Object.values(SupportedBuilder).find((builder) => builderName?.includes(builder));

Expand All @@ -18,6 +18,10 @@ export const buildFrameworkGlobalsFromOptions = async (options: Options) => {
const framework = frameworkPackages[frameworkPackageName];
const renderer = frameworkToRenderer[framework];

if (options.configType === 'DEVELOPMENT') {
// Manager only needs the token currently, so we don't pass any other channel options.
globals.CHANNEL_OPTIONS = { wsToken: channelOptions?.wsToken };
}
globals.STORYBOOK_BUILDER = builder;
globals.STORYBOOK_FRAMEWORK = framework;
globals.STORYBOOK_RENDERER = renderer;
Expand Down
5 changes: 3 additions & 2 deletions code/core/src/channels/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { PostMessageTransport } from './postmessage';
import type { ChannelTransport, Config } from './types';
import { WebsocketTransport } from './websocket';

const { CONFIG_TYPE } = global;
const { CHANNEL_OPTIONS, CONFIG_TYPE } = global;

export * from './main';

Expand Down Expand Up @@ -35,7 +35,8 @@ export function createBrowserChannel({ page, extraTransports = [] }: Options): C
if (CONFIG_TYPE === 'DEVELOPMENT') {
const protocol = window.location.protocol === 'http:' ? 'ws' : 'wss';
const { hostname, port } = window.location;
const channelUrl = `${protocol}://${hostname}:${port}/storybook-server-channel`;
const { wsToken } = CHANNEL_OPTIONS || {};
const channelUrl = `${protocol}://${hostname}:${port}/storybook-server-channel?token=${wsToken}`;

transports.push(new WebsocketTransport({ url: channelUrl, onError: () => {}, page }));
}
Expand Down
7 changes: 4 additions & 3 deletions code/core/src/core-server/dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { MissingBuilderError } from 'storybook/internal/server-errors';
import type { Options } from 'storybook/internal/types';

import compression from '@polka/compression';
import assert from 'assert';
import polka from 'polka';
import invariant from 'tiny-invariant';

Expand All @@ -28,9 +29,11 @@ export async function storybookDevServer(options: Options) {
const [server, core] = await Promise.all([getServer(options), options.presets.apply('core')]);
const app = polka({ server });

assert(core?.channelOptions?.wsToken, 'wsToken is required for securing the server channel');

const serverChannel = await options.presets.apply(
'experimental_serverChannel',
getServerChannel(server)
getServerChannel(server, core.channelOptions.wsToken)
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const workingDir = process.cwd();
Expand All @@ -52,8 +55,6 @@ export async function storybookDevServer(options: Options) {
options.extendServer(server);
}

// CORS middleware must be registered BEFORE route handlers to ensure all routes
// (including /index.json) receive proper CORS headers for Storybook Composition
app.use(getAccessControlMiddleware(core?.crossOriginIsolated ?? false));
app.use(getCachingMiddleware());

Expand Down
10 changes: 10 additions & 0 deletions code/core/src/core-server/presets/common-preset.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { randomUUID } from 'node:crypto';
import { existsSync } from 'node:fs';
import { readFile } from 'node:fs/promises';

Expand Down Expand Up @@ -190,8 +191,13 @@ export const experimental_serverAPI = (extension: Record<string, Function>, opti
* ...existing, someConfig })`, just overwriting everything and not merging with the existing
* values.
*/
const wsToken = randomUUID();
export const core = async (existing: CoreConfig, options: Options): Promise<CoreConfig> => ({
...existing,
channelOptions: {
...(existing?.channelOptions ?? {}),
...(options.configType === 'DEVELOPMENT' ? { wsToken } : {}),
},
disableTelemetry: options.disableTelemetry === true,
enableCrashReports:
options.enableCrashReports || optionalEnvToBoolean(process.env.STORYBOOK_ENABLE_CRASH_REPORTS),
Expand Down Expand Up @@ -257,6 +263,10 @@ export const managerHead = async (_: any, options: Options) => {
return '';
};

export const channelToken = async (value: string | undefined) => {
return value;
};

export const experimental_serverChannel = async (
channel: Channel,
options: OptionsWithRequiredCache
Expand Down
62 changes: 57 additions & 5 deletions code/core/src/core-server/utils/__tests__/server-channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,24 @@ import { ServerChannelTransport, getServerChannel } from '../get-server-channel'
describe('getServerChannel', () => {
it('should return a channel', () => {
const server = { on: vi.fn() } as any as Server;
const result = getServerChannel(server);
const result = getServerChannel(server, 'test-token-123');
expect(result).toBeInstanceOf(Channel);
});

it('should attach to the http server', () => {
const server = { on: vi.fn() } as any as Server;
getServerChannel(server);
getServerChannel(server, 'test-token-123');
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);
const transport = new ServerChannelTransport(server, mockToken);
const handler = vi.fn();
transport.setHandler(handler);

Expand All @@ -36,10 +38,11 @@ describe('ServerChannelTransport', () => {

expect(handler).toHaveBeenCalledWith('hello');
});

it('parses object JSON', () => {
const server = new EventEmitter() as any as Server;
const socket = new EventEmitter();
const transport = new ServerChannelTransport(server);
const transport = new ServerChannelTransport(server, mockToken);
const handler = vi.fn();
transport.setHandler(handler);

Expand All @@ -49,10 +52,11 @@ describe('ServerChannelTransport', () => {

expect(handler).toHaveBeenCalledWith({ type: 'hello' });
});

it('supports telejson cyclical data', () => {
const server = new EventEmitter() as any as Server;
const socket = new EventEmitter();
const transport = new ServerChannelTransport(server);
const transport = new ServerChannelTransport(server, mockToken);
const handler = vi.fn();
transport.setHandler(handler);

Expand All @@ -70,4 +74,52 @@ describe('ServerChannelTransport', () => {
}
`);
});

it('rejects connections with invalid token', () => {
const server = new EventEmitter() as any as Server;
const socket = new EventEmitter() as any;
socket.write = vi.fn();
socket.destroy = vi.fn();
const destroySpy = vi.spyOn(socket, 'destroy');
new ServerChannelTransport(server, mockToken);

// Simulate upgrade request with wrong token
const request = {
url: '/storybook-server-channel?token=wrong-token',
} 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('accepts connections with valid token', () => {
const server = new EventEmitter() as any as Server;
const socket = new EventEmitter() as any;
socket.write = vi.fn();
socket.destroy = vi.fn();
const destroySpy = vi.spyOn(socket, 'destroy');
const handleUpgradeSpy = vi.fn();
const transport = new ServerChannelTransport(server, mockToken);

// 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}`,
} as any;
const head = Buffer.from('');

server.listeners('upgrade')[0](request, socket, head);

expect(socket.write).not.toHaveBeenCalled();
expect(destroySpy).not.toHaveBeenCalled();
expect(handleUpgradeSpy).toHaveBeenCalled();
});
});
32 changes: 24 additions & 8 deletions code/core/src/core-server/utils/get-server-channel.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type { IncomingMessage } from 'node:http';

import type { ChannelHandler } from 'storybook/internal/channels';
import { Channel, HEARTBEAT_INTERVAL } from 'storybook/internal/channels';

import { isJSON, parse, stringify } from 'telejson';
import WebSocket, { WebSocketServer } from 'ws';

import { UniversalStore } from '../../shared/universal-store';
import { isValidToken } from './validate-websocket-token';

type Server = NonNullable<NonNullable<ConstructorParameters<typeof WebSocketServer>[0]>['server']>;

Expand All @@ -19,14 +22,27 @@ export class ServerChannelTransport {

private handler?: ChannelHandler;

constructor(server: Server) {
private token: string;

constructor(server: Server, token: string) {
this.token = token;
this.socket = new WebSocketServer({ noServer: true });

server.on('upgrade', (request, socket, head) => {
if (request.url === '/storybook-server-channel') {
this.socket.handleUpgrade(request, socket, head, (ws) => {
this.socket.emit('connection', ws, request);
});
server.on('upgrade', (request: IncomingMessage, socket, head) => {
if (request.url) {
const url = new URL(request.url, 'http://localhost');
if (url.pathname === '/storybook-server-channel') {
const requestToken = url.searchParams.get('token');
if (!isValidToken(requestToken, this.token)) {
socket.write('HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n');
socket.destroy();
return;
}

this.socket.handleUpgrade(request, socket, head, (ws) => {
this.socket.emit('connection', ws, request);
});
}
Comment on lines +25 to +45

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard URL parsing to avoid upgrade-handler crashes.

new URL(request.url, ...) can throw on malformed input, which would crash the upgrade handler and potentially the process. Add a try/catch to fail closed with a 400 and close the socket.

💡 Suggested fix
     server.on('upgrade', (request: IncomingMessage, socket, head) => {
-      if (request.url) {
-        const url = new URL(request.url, 'http://localhost');
+      if (request.url) {
+        let url: URL;
+        try {
+          url = new URL(request.url, 'http://localhost');
+        } catch {
+          socket.write('HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n');
+          socket.destroy();
+          return;
+        }
         if (url.pathname === '/storybook-server-channel') {
           const requestToken = url.searchParams.get('token');
           if (!isValidToken(requestToken, this.token)) {
             socket.write('HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n');
             socket.destroy();
             return;
           }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private token: string;
constructor(server: Server, token: string) {
this.token = token;
this.socket = new WebSocketServer({ noServer: true });
server.on('upgrade', (request, socket, head) => {
if (request.url === '/storybook-server-channel') {
this.socket.handleUpgrade(request, socket, head, (ws) => {
this.socket.emit('connection', ws, request);
});
server.on('upgrade', (request: IncomingMessage, socket, head) => {
if (request.url) {
const url = new URL(request.url, 'http://localhost');
if (url.pathname === '/storybook-server-channel') {
const requestToken = url.searchParams.get('token');
if (!isValidToken(requestToken, this.token)) {
socket.write('HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n');
socket.destroy();
return;
}
this.socket.handleUpgrade(request, socket, head, (ws) => {
this.socket.emit('connection', ws, request);
});
}
private token: string;
constructor(server: Server, token: string) {
this.token = token;
this.socket = new WebSocketServer({ noServer: true });
server.on('upgrade', (request: IncomingMessage, socket, head) => {
if (request.url) {
let url: URL;
try {
url = new URL(request.url, 'http://localhost');
} catch {
socket.write('HTTP/1.1 400 Bad Request\r\nConnection: close\r\n\r\n');
socket.destroy();
return;
}
if (url.pathname === '/storybook-server-channel') {
const requestToken = url.searchParams.get('token');
if (!isValidToken(requestToken, this.token)) {
socket.write('HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n');
socket.destroy();
return;
}
this.socket.handleUpgrade(request, socket, head, (ws) => {
this.socket.emit('connection', ws, request);
});
}
🤖 Prompt for AI Agents
In `@code/core/src/core-server/utils/get-server-channel.ts` around lines 25 - 45,
Wrap the URL parsing in the Server upgrade handler in a try/catch so malformed
request.url cannot throw: inside the constructor's server.on('upgrade', ...)
block, surround the new URL(request.url, 'http://localhost') call with a
try/catch, and on catch write an HTTP/1.1 400 Bad Request response to the socket
and destroy it (fail closed) before returning; keep the existing token check via
isValidToken and the this.socket.handleUpgrade / this.socket.emit('connection',
...) flow unchanged for valid requests.

}
});
this.socket.on('connection', (wss) => {
Expand Down Expand Up @@ -68,8 +84,8 @@ export class ServerChannelTransport {
}
}

export function getServerChannel(server: Server) {
const transports = [new ServerChannelTransport(server)];
export function getServerChannel(server: Server, token: string) {
const transports = [new ServerChannelTransport(server, token)];

const channel = new Channel({ transports, async: true });

Expand Down
4 changes: 0 additions & 4 deletions code/core/src/core-server/utils/getAccessControlMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@ import type { Middleware } from '../../types';

export function getAccessControlMiddleware(crossOriginIsolated: boolean): Middleware {
return (req, res, next) => {
res.setHeader('Access-Control-Allow-Origin', '*');
res.setHeader('Access-Control-Allow-Headers', 'Origin, X-Requested-With, Content-Type, Accept');
// These headers are required to enable SharedArrayBuffer
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
if (crossOriginIsolated) {
// These headers are required to enable SharedArrayBuffer
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
res.setHeader('Cross-Origin-Opener-Policy', 'same-origin');
res.setHeader('Cross-Origin-Embedder-Policy', 'require-corp');
}
Expand Down
5 changes: 5 additions & 0 deletions code/core/src/core-server/utils/index-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export function registerIndexJsonRoute({
try {
const index = await (await storyIndexGeneratorPromise).getIndex();
res.setHeader('Content-Type', 'application/json');
res.setHeader('Access-Control-Allow-Origin', '*');
res.setHeader(
'Access-Control-Allow-Headers',
'Origin, X-Requested-With, Content-Type, Accept'
);
res.end(JSON.stringify(index));
} catch (err) {
res.statusCode = 500;
Expand Down
21 changes: 21 additions & 0 deletions code/core/src/core-server/utils/validate-websocket-token.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { timingSafeEqual } from 'node:crypto';

/**
* Validates a secret token using constant-time comparison to prevent timing attacks.
*
* @returns `true` if tokens match, `false` otherwise
*/
export function isValidToken(token: string | null, expectedToken: string): boolean {
if (!token || !expectedToken) {
return false;
}

const a = Buffer.from(token, 'utf8');
const b = Buffer.from(expectedToken, 'utf8');
try {
// TODO: Remove any types as soon as @types/node is updated
return a.length === b.length && timingSafeEqual(a as any, b as any);
} catch {
return false;
}
}
2 changes: 1 addition & 1 deletion code/core/src/types/modules/core-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface CoreConfig {
};
renderer?: RendererName;
disableWebpackDefaults?: boolean;
channelOptions?: Partial<TelejsonOptions>;
channelOptions?: Partial<TelejsonOptions> & { wsToken?: string };
/** Disables the generation of project.json, a file containing Storybook metadata */
disableProjectJson?: boolean;
/**
Expand Down
2 changes: 1 addition & 1 deletion scripts/tasks/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { exec } from '../utils/exec';
import { maxConcurrentTasks } from '../utils/maxConcurrentTasks';

// The amount of VCPUs for the check task on CI is 4 (large resource)
const amountOfVCPUs = 3;
const amountOfVCPUs = 2;

const parallel = `--parallel=${process.env.CI ? amountOfVCPUs - 1 : maxConcurrentTasks}`;

Expand Down
Loading