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 @@
## 10.2.10

- Core: Require token for websocket connections - [#33820](https://github.com/storybookjs/storybook/pull/33820), thanks @ghengeveld!

## 10.2.9

- Addon-Vitest: Improve config file detection in monorepos - [#33814](https://github.com/storybookjs/storybook/pull/33814), thanks @valentinpalkovic!
Expand Down
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
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 @@ -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)
);

const workingDir = process.cwd();
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 @@ -250,6 +256,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);
});
}
}
});
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
1 change: 0 additions & 1 deletion code/frameworks/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
"./images/next-legacy-image": "./dist/images/next-legacy-image.js",
"./link.mock": {
"types": "./dist/export-mocks/link/index.d.ts",
"code": "./src/export-mocks/link/index.tsx",
"default": "./dist/export-mocks/link/index.js"
},
"./navigation.mock": {
Expand Down
3 changes: 2 additions & 1 deletion code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -220,5 +220,6 @@
"Dependency Upgrades"
]
]
}
},
"deferredNextVersion": "10.2.10"
}
2 changes: 1 addition & 1 deletion docs/versions/latest.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"10.2.9","info":{"plain":"- Addon-Vitest: Improve config file detection in monorepos - [#33814](https://github.com/storybookjs/storybook/pull/33814), thanks @valentinpalkovic!\n- Builder-Vite: Update dependencies react-vite framework - [#33810](https://github.com/storybookjs/storybook/pull/33810), thanks @valentinpalkovic!\n- Builder-Vite: Use relative path for mocker entry in production builds - [#33792](https://github.com/storybookjs/storybook/pull/33792), thanks @DukeDeSouth!\n- Next.js: Fix Link component override in appDirectory configuration - [#31251](https://github.com/storybookjs/storybook/pull/31251), thanks @yatishgoel!"}}
{"version":"10.2.10","info":{"plain":"- Core: Require token for websocket connections - [#33820](https://github.com/storybookjs/storybook/pull/33820), thanks @ghengeveld!"}}
2 changes: 1 addition & 1 deletion docs/versions/next.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"10.3.0-alpha.5","info":{"plain":"- Builder-Vite: Use relative path for mocker entry in production builds - [#33792](https://github.com/storybookjs/storybook/pull/33792), thanks @DukeDeSouth!\n- CLI: Support addon-vitest setup when --skip-install is passed - [#33718](https://github.com/storybookjs/storybook/pull/33718), thanks @valentinpalkovic!\n- CSF: Fix cross-file story imports in csf-factories codemod - [#33723](https://github.com/storybookjs/storybook/pull/33723), thanks @yatishgoel!\n- Compile: reduce VCPUs for CI check task from 4 to 3 - [#33822](https://github.com/storybookjs/storybook/pull/33822), thanks @valentinpalkovic!\n- Core: Ignore empty files when indexing - [#33782](https://github.com/storybookjs/storybook/pull/33782), thanks @JReinhold!\n- Globals: Repair dynamicTitle: false for user-defined tools - [#33284](https://github.com/storybookjs/storybook/pull/33284), thanks @ia319!\n- Logger: Honor --loglevel for npmlog output - [#33776](https://github.com/storybookjs/storybook/pull/33776), thanks @LouisLau-art!\n- Telemetry: Add Expo metaframework - [#33783](https://github.com/storybookjs/storybook/pull/33783), thanks @copilot-swe-agent!\n- Telemetry: Add init exit event - [#33773](https://github.com/storybookjs/storybook/pull/33773), thanks @valentinpalkovic!\n- Telemetry: Add share events - [#33766](https://github.com/storybookjs/storybook/pull/33766), thanks @ndelangen!\n- Test: Update event creation logic in user-event package - [#33787](https://github.com/storybookjs/storybook/pull/33787), thanks @valentinpalkovic!\n- Viewport: Skip viewport validation before parameters load - [#33794](https://github.com/storybookjs/storybook/pull/33794), thanks @ia319!"}}
{"version":"10.3.0-alpha.6","info":{"plain":"- Addon-Vitest: Improve config file detection in monorepos - [#33814](https://github.com/storybookjs/storybook/pull/33814), thanks @valentinpalkovic!\n- Addon-Vitest: Support Vitest canaries - [#33833](https://github.com/storybookjs/storybook/pull/33833), thanks @valentinpalkovic!\n- Builder-Vite: Update dependencies react-vite framework - [#33810](https://github.com/storybookjs/storybook/pull/33810), thanks @valentinpalkovic!\n- Next.js: Fix Link component override in appDirectory configuration - [#31251](https://github.com/storybookjs/storybook/pull/31251), thanks @yatishgoel!"}}
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 = 4;
const amountOfVCPUs = 2;

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

Expand Down
Loading