Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 9.1.19

- Harden websocket connection

## 9.1.18

- No-op release. No changes.
Expand Down
7 changes: 6 additions & 1 deletion code/core/src/builder-manager/utils/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,16 @@ export const pluckThirdPartyPackageFromPath = (packagePath: string) =>
export const buildFrameworkGlobalsFromOptions = async (options: Options) => {
const globals: Record<string, any> = {};

const { builder } = await options.presets.apply('core');
const { builder, channelOptions } = await options.presets.apply('core');

const frameworkName = await getFrameworkName(options);
const rendererName = await extractProperRendererNameFromFramework(frameworkName);

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 };
}

if (rendererName) {
globals.STORYBOOK_RENDERER =
(await extractProperRendererNameFromFramework(frameworkName)) ?? undefined;
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
5 changes: 4 additions & 1 deletion 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 @@ -26,9 +27,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)
);

let indexError: Error | undefined;
Expand Down
11 changes: 10 additions & 1 deletion 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';
import { dirname, isAbsolute, join } from 'node:path';
Expand Down Expand Up @@ -181,7 +182,7 @@ export const experimental_serverAPI = (extension: Record<string, Function>, opti
}
return { ...extension, removeAddon };
};

const wsToken = randomUUID();
/**
* If for some reason this config is not applied, the reason is that likely there is an addon that
* does `export core = () => ({ someConfig })`, instead of `export core = (existing) => ({
Expand All @@ -191,6 +192,10 @@ export const experimental_serverAPI = (extension: Record<string, Function>, opti
export const core = async (existing: CoreConfig, options: Options): Promise<CoreConfig> => ({
...existing,
disableTelemetry: options.disableTelemetry === true,
channelOptions: {
...(existing?.channelOptions ?? {}),
...(options.configType === 'DEVELOPMENT' ? { wsToken } : {}),
},
enableCrashReports:
options.enableCrashReports || optionalEnvToBoolean(process.env.STORYBOOK_ENABLE_CRASH_REPORTS),
});
Expand Down Expand Up @@ -247,6 +252,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
81 changes: 76 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,71 @@ describe('ServerChannelTransport', () => {
}
`);
});

it('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 handler = vi.fn();
transport.setHandler(handler);

// @ts-expect-error (an internal API)
transport.socket.emit('connection', socket);

const input = { a() {}, b: class {}, c: true, d: 3 };
socket.emit('message', stringify(input));

console.log(handler.mock.calls);

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 | 🟡 Minor

Remove debug console.log statement.

This appears to be a leftover debug artifact. As per coding guidelines, console.log should not be used directly in code.

Proposed fix
-    console.log(handler.mock.calls);
-
     expect(handler.mock.calls[0][0].a).toBeUndefined();
📝 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
console.log(handler.mock.calls);
expect(handler.mock.calls[0][0].a).toBeUndefined();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts` at line 91,
Remove the leftover debug console.log in the test: delete the statement that
prints handler.mock.calls in server-channel.test.ts (the
console.log(handler.mock.calls); line) so the test contains only assertions and
no direct console output; ensure any needed inspection of handler mock is done
via Jest assertions on handler (e.g., handler.mock.calls or
handler.mock.calls.length) rather than logging.


expect(handler.mock.calls[0][0].a).toBeUndefined();
expect(handler.mock.calls[0][0].b).toBeUndefined();
});

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);
});
}
}
});
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/stories-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ export function useStoriesJson({
const generator = await initializedStoryIndexGenerator;
const index = await generator.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
20 changes: 20 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,20 @@
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 {
return a.length === b.length && timingSafeEqual(a, b);
} 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 @@ -26,7 +26,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
3 changes: 2 additions & 1 deletion code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -283,5 +283,6 @@
"Dependency Upgrades"
]
]
}
},
"deferredNextVersion": "9.1.19"
}