Skip to content

Commit 9cf9d89

Browse files
committed
Core: Require token for websocket connections
1 parent e6194a8 commit 9cf9d89

File tree

10 files changed

+131
-24
lines changed

10 files changed

+131
-24
lines changed

code/core/src/builder-manager/utils/framework.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,16 @@ export const pluckThirdPartyPackageFromPath = (packagePath: string) =>
3030
export const buildFrameworkGlobalsFromOptions = async (options: Options) => {
3131
const globals: Record<string, any> = {};
3232

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

3535
const frameworkName = await getFrameworkName(options);
3636
const rendererName = await extractProperRendererNameFromFramework(frameworkName);
3737

38+
if (options.configType === 'DEVELOPMENT') {
39+
// Manager only needs the token currently, so we don't pass any other channel options.
40+
globals.CHANNEL_OPTIONS = { wsToken: channelOptions?.wsToken };
41+
}
42+
3843
if (rendererName) {
3944
globals.STORYBOOK_RENDERER =
4045
(await extractProperRendererNameFromFramework(frameworkName)) ?? undefined;

code/core/src/channels/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { PostMessageTransport } from './postmessage';
77
import type { ChannelTransport, Config } from './types';
88
import { WebsocketTransport } from './websocket';
99

10-
const { CONFIG_TYPE } = global;
10+
const { CHANNEL_OPTIONS, CONFIG_TYPE } = global;
1111

1212
export * from './main';
1313

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

4041
transports.push(new WebsocketTransport({ url: channelUrl, onError: () => {}, page }));
4142
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { logger } from '@storybook/core/node-logger';
55
import { MissingBuilderError } from '@storybook/core/server-errors';
66

77
import compression from '@polka/compression';
8+
import assert from 'assert';
89
import polka from 'polka';
910
import invariant from 'tiny-invariant';
1011

@@ -25,9 +26,11 @@ export async function storybookDevServer(options: Options) {
2526
const [server, core] = await Promise.all([getServer(options), options.presets.apply('core')]);
2627
const app = polka({ server });
2728

29+
assert(core?.channelOptions?.wsToken, 'wsToken is required for securing the server channel');
30+
2831
const serverChannel = await options.presets.apply(
2932
'experimental_serverChannel',
30-
getServerChannel(server)
33+
getServerChannel(server, core.channelOptions.wsToken)
3134
);
3235

3336
let indexError: Error | undefined;

code/core/src/core-server/presets/common-preset.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { randomUUID } from 'node:crypto';
12
import { existsSync } from 'node:fs';
23
import { readFile } from 'node:fs/promises';
34
import { dirname, isAbsolute, join } from 'node:path';
@@ -206,7 +207,7 @@ export const experimental_serverAPI = (extension: Record<string, Function>, opti
206207
}
207208
return { ...extension, removeAddon };
208209
};
209-
210+
const wsToken = randomUUID();
210211
/**
211212
* If for some reason this config is not applied, the reason is that likely there is an addon that
212213
* does `export core = () => ({ someConfig })`, instead of `export core = (existing) => ({
@@ -215,6 +216,10 @@ export const experimental_serverAPI = (extension: Record<string, Function>, opti
215216
*/
216217
export const core = async (existing: CoreConfig, options: Options): Promise<CoreConfig> => ({
217218
...existing,
219+
channelOptions: {
220+
...(existing?.channelOptions ?? {}),
221+
...(options.configType === 'DEVELOPMENT' ? { wsToken } : {}),
222+
},
218223
disableTelemetry: options.disableTelemetry === true || options.test === true,
219224
enableCrashReports:
220225
options.enableCrashReports || optionalEnvToBoolean(process.env.STORYBOOK_ENABLE_CRASH_REPORTS),
@@ -273,6 +278,10 @@ export const managerHead = async (_: any, options: Options) => {
273278
return '';
274279
};
275280

281+
export const channelToken = async (value: string | undefined) => {
282+
return value;
283+
};
284+
276285
// eslint-disable-next-line @typescript-eslint/naming-convention
277286
export const experimental_serverChannel = async (
278287
channel: Channel,

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

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,24 @@ import { ServerChannelTransport, getServerChannel } from '../get-server-channel'
1111
describe('getServerChannel', () => {
1212
it('should return a channel', () => {
1313
const server = { on: vi.fn() } as any as Server;
14-
const result = getServerChannel(server);
14+
const result = getServerChannel(server, 'test-token-123');
1515
expect(result).toBeInstanceOf(Channel);
1616
});
1717

1818
it('should attach to the http server', () => {
1919
const server = { on: vi.fn() } as any as Server;
20-
getServerChannel(server);
20+
getServerChannel(server, 'test-token-123');
2121
expect(server.on).toHaveBeenCalledWith('upgrade', expect.any(Function));
2222
});
2323
});
2424

2525
describe('ServerChannelTransport', () => {
26+
const mockToken = 'test-token-123';
27+
2628
it('parses simple JSON', () => {
2729
const server = new EventEmitter() as any as Server;
2830
const socket = new EventEmitter();
29-
const transport = new ServerChannelTransport(server);
31+
const transport = new ServerChannelTransport(server, mockToken);
3032
const handler = vi.fn();
3133
transport.setHandler(handler);
3234

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

3739
expect(handler).toHaveBeenCalledWith('hello');
3840
});
41+
3942
it('parses object JSON', () => {
4043
const server = new EventEmitter() as any as Server;
4144
const socket = new EventEmitter();
42-
const transport = new ServerChannelTransport(server);
45+
const transport = new ServerChannelTransport(server, mockToken);
4346
const handler = vi.fn();
4447
transport.setHandler(handler);
4548

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

5053
expect(handler).toHaveBeenCalledWith({ type: 'hello' });
5154
});
55+
5256
it('supports telejson cyclical data', () => {
5357
const server = new EventEmitter() as any as Server;
5458
const socket = new EventEmitter();
55-
const transport = new ServerChannelTransport(server);
59+
const transport = new ServerChannelTransport(server, mockToken);
5660
const handler = vi.fn();
5761
transport.setHandler(handler);
5862

@@ -73,7 +77,7 @@ describe('ServerChannelTransport', () => {
7377
it('skips telejson classes and functions in data', () => {
7478
const server = new EventEmitter() as any as Server;
7579
const socket = new EventEmitter();
76-
const transport = new ServerChannelTransport(server);
80+
const transport = new ServerChannelTransport(server, mockToken);
7781
const handler = vi.fn();
7882
transport.setHandler(handler);
7983

@@ -86,4 +90,52 @@ describe('ServerChannelTransport', () => {
8690
expect(handler.mock.calls[0][0].a).toEqual(expect.any(String));
8791
expect(handler.mock.calls[0][0].b).toEqual(expect.any(String));
8892
});
93+
94+
it('rejects connections with invalid token', () => {
95+
const server = new EventEmitter() as any as Server;
96+
const socket = new EventEmitter() as any;
97+
socket.write = vi.fn();
98+
socket.destroy = vi.fn();
99+
const destroySpy = vi.spyOn(socket, 'destroy');
100+
new ServerChannelTransport(server, mockToken);
101+
102+
// Simulate upgrade request with wrong token
103+
const request = {
104+
url: '/storybook-server-channel?token=wrong-token',
105+
} as any;
106+
const head = Buffer.from('');
107+
108+
server.listeners('upgrade')[0](request, socket, head);
109+
110+
expect(socket.write).toHaveBeenCalledWith(
111+
'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n'
112+
);
113+
expect(destroySpy).toHaveBeenCalled();
114+
});
115+
116+
it('accepts connections with valid token', () => {
117+
const server = new EventEmitter() as any as Server;
118+
const socket = new EventEmitter() as any;
119+
socket.write = vi.fn();
120+
socket.destroy = vi.fn();
121+
const destroySpy = vi.spyOn(socket, 'destroy');
122+
const handleUpgradeSpy = vi.fn();
123+
const transport = new ServerChannelTransport(server, mockToken);
124+
125+
// Mock handleUpgrade to track if it's called
126+
// @ts-expect-error (accessing private property)
127+
transport.socket.handleUpgrade = handleUpgradeSpy;
128+
129+
// Simulate upgrade request with correct token
130+
const request = {
131+
url: `/storybook-server-channel?token=${mockToken}`,
132+
} as any;
133+
const head = Buffer.from('');
134+
135+
server.listeners('upgrade')[0](request, socket, head);
136+
137+
expect(socket.write).not.toHaveBeenCalled();
138+
expect(destroySpy).not.toHaveBeenCalled();
139+
expect(handleUpgradeSpy).toHaveBeenCalled();
140+
});
89141
});

code/core/src/core-server/utils/get-server-channel.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
import type { IncomingMessage } from 'node:http';
2+
13
import type { ChannelHandler } from '@storybook/core/channels';
24
import { Channel, HEARTBEAT_INTERVAL } from '@storybook/core/channels';
35

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

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

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

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

2023
private handler?: ChannelHandler;
2124

22-
constructor(server: Server) {
25+
private token: string;
26+
27+
constructor(server: Server, token: string) {
28+
this.token = token;
2329
this.socket = new WebSocketServer({ noServer: true });
2430

25-
server.on('upgrade', (request, socket, head) => {
26-
if (request.url === '/storybook-server-channel') {
27-
this.socket.handleUpgrade(request, socket, head, (ws) => {
28-
this.socket.emit('connection', ws, request);
29-
});
31+
server.on('upgrade', (request: IncomingMessage, socket, head) => {
32+
if (request.url) {
33+
const url = new URL(request.url, 'http://localhost');
34+
if (url.pathname === '/storybook-server-channel') {
35+
const requestToken = url.searchParams.get('token');
36+
if (!isValidToken(requestToken, this.token)) {
37+
socket.write('HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n');
38+
socket.destroy();
39+
return;
40+
}
41+
42+
this.socket.handleUpgrade(request, socket, head, (ws) => {
43+
this.socket.emit('connection', ws, request);
44+
});
45+
}
3046
}
3147
});
3248
this.socket.on('connection', (wss) => {
@@ -71,8 +87,8 @@ export class ServerChannelTransport {
7187
}
7288
}
7389

74-
export function getServerChannel(server: Server) {
75-
const transports = [new ServerChannelTransport(server)];
90+
export function getServerChannel(server: Server, token: string) {
91+
const transports = [new ServerChannelTransport(server, token)];
7692

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

code/core/src/core-server/utils/getAccessControlMiddleware.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@ import type { Middleware } from '../../types';
22

33
export function getAccessControlMiddleware(crossOriginIsolated: boolean): Middleware {
44
return (req, res, next) => {
5-
res.setHeader('Access-Control-Allow-Origin', '*');
6-
res.setHeader('Access-Control-Allow-Headers', 'Origin, X-Requested-With, Content-Type, Accept');
75
// These headers are required to enable SharedArrayBuffer
86
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
97
if (crossOriginIsolated) {
10-
// These headers are required to enable SharedArrayBuffer
11-
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer
128
res.setHeader('Cross-Origin-Opener-Policy', 'same-origin');
139
res.setHeader('Cross-Origin-Embedder-Policy', 'require-corp');
1410
}

code/core/src/core-server/utils/stories-json.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ export function useStoriesJson({
6363
const generator = await initializedStoryIndexGenerator;
6464
const index = await generator.getIndex();
6565
res.setHeader('Content-Type', 'application/json');
66+
res.setHeader('Access-Control-Allow-Origin', '*');
67+
res.setHeader(
68+
'Access-Control-Allow-Headers',
69+
'Origin, X-Requested-With, Content-Type, Accept'
70+
);
6671
res.end(JSON.stringify(index));
6772
} catch (err) {
6873
res.statusCode = 500;
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { timingSafeEqual } from 'node:crypto';
2+
3+
/**
4+
* Validates a secret token using constant-time comparison to prevent timing attacks.
5+
*
6+
* @returns `true` if tokens match, `false` otherwise
7+
*/
8+
export function isValidToken(token: string | null, expectedToken: string): boolean {
9+
if (!token || !expectedToken) {
10+
return false;
11+
}
12+
13+
const a = Buffer.from(token, 'utf8');
14+
const b = Buffer.from(expectedToken, 'utf8');
15+
try {
16+
return a.length === b.length && timingSafeEqual(a, b);
17+
} catch {
18+
return false;
19+
}
20+
}

code/core/src/types/modules/core-common.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export interface CoreConfig {
2626
};
2727
renderer?: RendererName;
2828
disableWebpackDefaults?: boolean;
29-
channelOptions?: Partial<TelejsonOptions>;
29+
channelOptions?: Partial<TelejsonOptions> & { wsToken?: string };
3030
/** Disables the generation of project.json, a file containing Storybook metadata */
3131
disableProjectJson?: boolean;
3232
/**

0 commit comments

Comments
 (0)