Skip to content

Core: Require token for websocket connections#33820

Merged
valentinpalkovic merged 10 commits into
nextfrom
harden-websocket-security
Feb 18, 2026
Merged

Core: Require token for websocket connections#33820
valentinpalkovic merged 10 commits into
nextfrom
harden-websocket-security

Conversation

@ghengeveld
Copy link
Copy Markdown
Member

@ghengeveld ghengeveld commented Feb 10, 2026

See also #33835

What I did

Added token validation for websocket connections.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Steps to test:

  1. Start Storybook dev server:

    yarn task --task sandbox --start-from auto --template react-vite/default-ts
  2. Verify WebSocket connection works:

    • Open Storybook in your browser
    • Open DevTools → Network → WS tab
    • Confirm WebSocket connection to /storybook-server-channel?token=<uuid> is established
    • Verify Storybook functions normally (stories load, interactions work)
  3. Verify token validation (security test):

    • In browser console, try to connect manually without token:

      const ws = new WebSocket('ws://localhost:6006/storybook-server-channel');
    • Should fail with connection error (no token)

    • Try with invalid token:

      const ws = new WebSocket('ws://localhost:6006/storybook-server-channel?token=invalid-token');
    • Should fail with connection error (invalid token)

  4. Verify CORS configuration:

    • Check that /index.json returns Access-Control-Allow-Origin: * header (for Storybook Composition)
    • Verify HTML pages don't expose the token to cross-origin requests (check Network tab headers)
  5. Verify production builds:

    • Build a static Storybook: yarn build-storybook
    • Confirm CHANNEL_OPTIONS.token is undefined in production (no WebSocket connection attempted)

Ensure this is verified to work with a community builder as well (Nuxt, RSPack)

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Note: This is an internal security fix with no user-facing API changes. No documentation updates required.

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This pull request has been released as version 0.0.0-pr-33820-sha-4dcbee69. Try it out in a new sandbox by running npx storybook@0.0.0-pr-33820-sha-4dcbee69 sandbox or in an existing project with npx storybook@0.0.0-pr-33820-sha-4dcbee69 upgrade.

More information
Published version 0.0.0-pr-33820-sha-4dcbee69
Triggered by @yannbf
Repository storybookjs/storybook
Branch harden-websocket-security
Commit 4dcbee69
Datetime Fri Feb 13 11:08:06 UTC 2026 (1770980886)
Workflow run 21984592015

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=33820

Summary by CodeRabbit

  • New Features

    • Added secure token-based authentication for server channel connections
    • Exposed network address and channel options for runtime configuration
    • Endpoint-specific CORS and cross-origin isolation headers for improved security
  • Bug Fixes

    • Strengthened origin and token validation to prevent unauthorized WebSocket access
  • Chores

    • Reduced CI task parallelism for more predictable build resource usage

…WebSocket connections

- Updated `iframe-webpack.config.ts` to remove unused imports.
- Modified `framework.ts` to change the type of `globals` and include `channelOptions` in the applied presets.
- Enhanced `index.ts` to extract `CHANNEL_OPTIONS` from global variables and append the token to the WebSocket channel URL.
- Adjusted `dev-server.ts` to pass the token to the server channel.
- Introduced a new utility `validate-websocket-token.ts` for token validation.
- Updated `get-server-channel.ts` to accept a token parameter and validate incoming WebSocket connections.
- Added tests in `server-channel.test.ts` to ensure proper handling of token validation for WebSocket connections.

This change improves security by ensuring only authorized connections are accepted.
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Feb 10, 2026

View your CI Pipeline Execution ↗ for commit 1ef385e

Command Status Duration Result
nx run-many -t compile,check,knip,test,pretty-d... ❌ Failed 23m 9s View ↗

☁️ Nx Cloud last updated this comment at 2026-02-18 08:22:15 UTC

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A WebSocket token (wsToken) is generated and propagated to clients; server-side upgrade handling now validates the token and origin. CORS headers were removed from the general access-control middleware and added to /index.json. A minor CI build tweak reduces VCPU used for compile tasks from 3 to 2.

Changes

Cohort / File(s) Summary
Server preset & types
code/core/src/core-server/presets/common-preset.ts, code/core/src/types/modules/core-common.ts
Generate a wsToken (randomUUID), expose channelToken() preset, and add optional wsToken to CoreConfig.channelOptions.
Server channel + transport
code/core/src/core-server/utils/get-server-channel.ts, code/core/src/core-server/utils/__tests__/server-channel.test.ts
ServerChannelTransport and getServerChannel now accept/store a token; upgrade handler validates path, origin, and token, rejecting invalid/missing tokens with 403-like behavior; tests updated for token scenarios.
Core server wiring
code/core/src/core-server/dev-server.ts, code/core/src/builder-manager/utils/framework.ts, code/core/src/channels/index.ts
Channel token read from presets and placed into globals; dev client channel URL appends ?token={wsToken}; server initialization now requires/receives the wsToken.
CORS adjustments
code/core/src/core-server/utils/getAccessControlMiddleware.ts, code/core/src/core-server/utils/index-json.ts
Removed broad CORS headers from access-control middleware; added Access-Control-Allow-Origin and Access-Control-Allow-Headers for /index.json responses.
Miscellaneous & build
code/builders/builder-webpack5/src/preview/iframe-webpack.config.ts, scripts/tasks/compile.ts
Removed unused dirname import in webpack config; lowered CI VCPU count used for compile task from 3 → 2.

Sequence Diagram(s)

sequenceDiagram
    participant Preset as Server (common-preset)
    participant Core as Dev Server (dev-server.ts)
    participant Builder as Builder Manager (framework.ts)
    participant Client as Browser (channels/index.ts)
    participant Transport as Upgrade Handler (get-server-channel.ts)

    Preset->>Preset: Generate wsToken (randomUUID)
    Preset->>Core: Embed wsToken in CoreConfig.channelOptions
    Core->>Builder: expose channelOptions with wsToken
    Builder->>Client: Set globals.CHANNEL_OPTIONS { wsToken }
    Client->>Transport: Open /storybook-server-channel?token={wsToken}
    Transport->>Transport: Parse URL, validate origin and token
    alt token valid and origin allowed
        Transport->>Client: Proceed with WebSocket upgrade (accept)
    else invalid/missing token or origin
        Transport->>Client: Return 403-like response and close socket
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • storybookjs/storybook#33822: Modifies the same scripts/tasks/compile.ts VCPU configuration line (related CI build parallelism change).

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
code/core/src/builder-manager/utils/framework.ts (1)

12-22: ⚠️ Potential issue | 🟡 Minor

Potential null reference if channelOptions is undefined.

If options.presets.apply('core') returns a config where channelOptions is undefined (e.g., due to a misconfigured preset chain), accessing channelOptions.token on line 22 will throw a TypeError.

🛡️ Proposed fix with optional chaining
   const { builder: builderConfig, channelOptions } = await options.presets.apply('core');
   // ...
   // Manager only needs the token currently, so we don't pass any other channel options.
-  globals.CHANNEL_OPTIONS = { token: channelOptions.token };
+  globals.CHANNEL_OPTIONS = { token: channelOptions?.token };
🤖 Fix all issues with AI agents
In `@code/core/src/core-server/dev-server.ts`:
- Around line 31-34: The call to getServerChannel is passing
core?.channelOptions?.token which may be undefined; update the code around
serverChannel creation to ensure a string token is provided (e.g., read token
into a local const and either throw if missing or supply a safe default) before
calling getServerChannel(server, token). Specifically, obtain a validated token
from core?.channelOptions?.token, handle the missing case (throw an error or set
a generated/static token), and then call getServerChannel(server, token) so the
token argument matches the expected string type.

In `@code/core/src/core-server/utils/get-server-channel.ts`:
- Around line 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.
🧹 Nitpick comments (6)
code/core/src/core-server/utils/index-json.ts (1)

57-71: Missing OPTIONS preflight handler for CORS.

The CORS headers are set for all requests, but browsers send an OPTIONS preflight request for cross-origin requests with certain headers. Since app.use() will execute for all methods including OPTIONS, but the handler tries to call getIndex() unconditionally, preflight requests may fail or return unnecessary data.

Consider adding explicit OPTIONS handling:

♻️ Proposed fix to handle OPTIONS preflight
   app.use('/index.json', async (req, res) => {
+    res.setHeader('Access-Control-Allow-Origin', '*');
+    res.setHeader(
+      'Access-Control-Allow-Headers',
+      'Origin, X-Requested-With, Content-Type, Accept'
+    );
+
+    if (req.method === 'OPTIONS') {
+      res.statusCode = 204;
+      res.end();
+      return;
+    }
+
     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));
code/core/src/channels/index.ts (1)

38-41: Consider handling missing token more explicitly.

If CHANNEL_OPTIONS is undefined or missing the token, the URL will contain the literal string "undefined" (e.g., ?token=undefined), which will fail server-side validation. While this is fail-safe behavior, it could make debugging harder.

♻️ Optional: Add early warning for missing token
   if (CONFIG_TYPE === 'DEVELOPMENT') {
     const protocol = window.location.protocol === 'http:' ? 'ws' : 'wss';
     const { hostname, port } = window.location;
     const { token } = CHANNEL_OPTIONS || {};
+    if (!token) {
+      console.warn('[Storybook] WebSocket channel token is missing. Connection will fail.');
+    }
     const channelUrl = `${protocol}://${hostname}:${port}/storybook-server-channel?token=${token}`;
code/core/src/core-server/presets/common-preset.ts (1)

263-265: Clarify purpose of channelToken function.

This function simply returns its input value unchanged. If it's intended as a preset hook for addons/frameworks to override the token, consider adding a JSDoc comment explaining its purpose. Otherwise, if it's unused, it could be removed.

📝 Suggested documentation
+/**
+ * Preset hook allowing addons/frameworks to override the channel token.
+ * By default, returns the token unchanged.
+ */
 export const channelToken = async (value: string | undefined) => {
   return value;
 };
code/core/src/builder-manager/utils/framework.ts (1)

10-10: Type widening reduces type safety.

Changing from Record<string, string | undefined> to Record<string, any> removes type checking for the globals object values. Consider using a more specific union type if CHANNEL_OPTIONS needs an object value.

♻️ Suggested type improvement
-  const globals: Record<string, any> = {};
+  const globals: Record<string, string | { token?: string } | undefined> = {};
code/core/src/core-server/utils/__tests__/server-channel.test.ts (2)

78-99: Redundant vi.spyOn after vi.fn().

socket.destroy is already assigned vi.fn() on line 82, so vi.spyOn(socket, 'destroy') on line 83 is redundant. The spy replaces the already-mocked function.

♻️ Simplified mock pattern
     const socket = new EventEmitter() as any;
     socket.write = vi.fn();
-    socket.destroy = vi.fn();
-    const destroySpy = vi.spyOn(socket, 'destroy');
+    const destroySpy = socket.destroy = vi.fn();
     const transport = new ServerChannelTransport(server, mockToken);
     // ...
     expect(destroySpy).toHaveBeenCalled();

The same pattern applies to the tests at lines 101-122 and 124-149.


25-149: Good test coverage for token validation scenarios.

The tests properly cover the key security scenarios: missing token, invalid token, and valid token. The assertions verify both the 403 response and socket destruction for rejection cases, and the upgrade progression for valid tokens.

Consider adding an edge case test for when request.url is undefined to verify the upgrade handler safely skips non-URL requests.

Would you like me to generate the additional edge case test?

Comment thread code/core/src/core-server/dev-server.ts
Comment on lines +25 to +45
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);
});
}
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.

@ghengeveld ghengeveld changed the title Core: Add token-based authentication for WebSocket connections Core: Add token to websocket connection string Feb 10, 2026
@ghengeveld ghengeveld changed the title Core: Add token to websocket connection string Core: Validate websocket connections Feb 11, 2026
@ghengeveld ghengeveld force-pushed the harden-websocket-security branch from fba3173 to 58ed3df Compare February 12, 2026 15:47
@ghengeveld ghengeveld changed the title Core: Validate websocket connections Core: Require token for websocket connections Feb 12, 2026
@valentinpalkovic valentinpalkovic moved this to In Progress in Core Team Projects Feb 13, 2026
@valentinpalkovic valentinpalkovic added patch:yes Bugfix & documentation PR that need to be picked to main branch ci:daily Run the CI jobs that normally run in the daily job. and removed ci:normal labels Feb 13, 2026
@storybook-bot
Copy link
Copy Markdown
Contributor

Failed to publish canary version of this pull request, triggered by @valentinpalkovic. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/21981011491

@storybook-bot
Copy link
Copy Markdown
Contributor

Failed to publish canary version of this pull request, triggered by @valentinpalkovic. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/21981245600

@storybook-bot
Copy link
Copy Markdown
Contributor

Failed to publish canary version of this pull request, triggered by @valentinpalkovic. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/21982548158

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@code/core/src/builder-manager/utils/framework.ts`:
- Around line 21-22: The manager currently assigns globals.CHANNEL_OPTIONS = {
wsToken: channelOptions?.wsToken } inside buildFrameworkGlobalsFromOptions (so
only wsToken is exposed to manager consumers), but the preview/iframe builders
(builder-webpack5 and builder-vite) set CHANNEL_OPTIONS = channelOptions which
exposes the full channelOptions (including wsToken) into iframe HTML; make this
consistent by either keeping the manager behavior and changing the preview
builders to assign only wsToken (i.e., set CHANNEL_OPTIONS = { wsToken:
channelOptions?.wsToken }) or, if full exposure is intentional, add a clear
comment in buildFrameworkGlobalsFromOptions and the preview builders explaining
why channelOptions visibility differs and add a unit/test ensuring wsToken is
not injected into iframe HTML unless explicitly intended.

In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts`:
- Around line 78-97: The test fails because it calls the server's 'upgrade'
listener before creating the ServerChannelTransport, so instantiate
ServerChannelTransport with the test's server (e.g., new
ServerChannelTransport(server, ...defaultArgs)) before invoking
server.listeners('upgrade')[0](request, socket, head); this ensures the
transport registers the 'upgrade' handler (as done in the other tests using
ServerChannelTransport) and prevents server.listeners('upgrade')[0] from being
undefined.
🧹 Nitpick comments (1)
code/core/src/builder-manager/utils/framework.ts (1)

9-12: Changing globals type from Record<string, any> to Record<string, unknown> would require coordination across multiple files.

The suggestion to tighten the type is reasonable for type safety. However, Record<string, any> is used consistently across the call chain: buildFrameworkGlobalsFromOptions (framework.ts:10), its callers in builder-manager/index.ts, and the consumer renderHTML in template.ts all expect Record<string, any>. Additionally, the globals object is immediately JSON-stringified before use, limiting the practical benefit of stricter typing for catching mistakes like the hypothetical typo mentioned.

If this refactoring is pursued, update consumers in code/core/src/builder-manager/index.ts and code/core/src/builder-manager/utils/template.ts to maintain type consistency.

Comment on lines +21 to +22
// Manager only needs the token currently, so we don't pass any other channel options.
globals.CHANNEL_OPTIONS = { wsToken: channelOptions?.wsToken };
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check call sites for this globals builder.
rg -n -C3 'buildFrameworkGlobalsFromOptions' --type=ts --type=js --type=tsx --type=jsx

# Check CHANNEL_OPTIONS usage and expected property name.
echo "=== CHANNEL_OPTIONS usage ==="
rg -n -C3 'CHANNEL_OPTIONS' --type=ts --type=js --type=tsx --type=jsx

# Check wsToken and token usage to understand property access patterns
echo "=== wsToken/token usage pattern ==="
rg -n -C2 'CHANNEL_OPTIONS\s*\.' --type=ts --type=js --type=tsx --type=jsx

Repository: storybookjs/storybook

Length of output: 231


🏁 Script executed:

#!/bin/bash
# Search for buildFrameworkGlobalsFromOptions call sites
echo "=== buildFrameworkGlobalsFromOptions call sites ==="
rg -n -C3 'buildFrameworkGlobalsFromOptions' .

echo ""
echo "=== CHANNEL_OPTIONS usage ==="
rg -n -C3 'CHANNEL_OPTIONS' .

echo ""
echo "=== CHANNEL_OPTIONS property access ==="
rg -n 'CHANNEL_OPTIONS\s*\.' .

echo ""
echo "=== wsToken usage ==="
rg -n 'wsToken' .

Repository: storybookjs/storybook

Length of output: 10024


Confirm CHANNEL_OPTIONS.wsToken wiring is correct; verify preview/iframe token exposure separately.

The manager implementation correctly sets only { wsToken } and consumers properly access it (e.g., code/core/src/channels/index.ts:38). Both call sites of buildFrameworkGlobalsFromOptions are in the manager builder (code/core/src/builder-manager/index.ts:201,299), so the token is safe here.

However, note that preview/iframe builders (builder-webpack5 and builder-vite) set CHANNEL_OPTIONS with the entire channelOptions object, which exposes wsToken in iframe HTML. This inconsistency may warrant a separate review if the intent is to limit token exposure.

🤖 Prompt for AI Agents
In `@code/core/src/builder-manager/utils/framework.ts` around lines 21 - 22, The
manager currently assigns globals.CHANNEL_OPTIONS = { wsToken:
channelOptions?.wsToken } inside buildFrameworkGlobalsFromOptions (so only
wsToken is exposed to manager consumers), but the preview/iframe builders
(builder-webpack5 and builder-vite) set CHANNEL_OPTIONS = channelOptions which
exposes the full channelOptions (including wsToken) into iframe HTML; make this
consistent by either keeping the manager behavior and changing the preview
builders to assign only wsToken (i.e., set CHANNEL_OPTIONS = { wsToken:
channelOptions?.wsToken }) or, if full exposure is intentional, add a clear
comment in buildFrameworkGlobalsFromOptions and the preview builders explaining
why channelOptions visibility differs and add a unit/test ensuring wsToken is
not injected into iframe HTML unless explicitly intended.

Comment on lines +78 to +97
it('rejects connections without 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');

// Simulate upgrade request without token
const request = {
url: '/storybook-server-channel',
} 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();
});
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 | 🔴 Critical

Missing ServerChannelTransport instantiation causes test to fail.

This test never creates a ServerChannelTransport instance, so no 'upgrade' listener is registered on the server. Line 91 will throw a TypeError because server.listeners('upgrade')[0] is undefined.

Compare with lines 105 and 128 which correctly instantiate the transport before calling the upgrade listener.

🐛 Proposed fix
     const destroySpy = vi.spyOn(socket, 'destroy');
+    new ServerChannelTransport(server, mockToken);

     // Simulate upgrade request without token
📝 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
it('rejects connections without 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');
// Simulate upgrade request without token
const request = {
url: '/storybook-server-channel',
} 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 without 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 without token
const request = {
url: '/storybook-server-channel',
} 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();
});
🤖 Prompt for AI Agents
In `@code/core/src/core-server/utils/__tests__/server-channel.test.ts` around
lines 78 - 97, The test fails because it calls the server's 'upgrade' listener
before creating the ServerChannelTransport, so instantiate
ServerChannelTransport with the test's server (e.g., new
ServerChannelTransport(server, ...defaultArgs)) before invoking
server.listeners('upgrade')[0](request, socket, head); this ensures the
transport registers the 'upgrade' handler (as done in the other tests using
ServerChannelTransport) and prevents server.listeners('upgrade')[0] from being
undefined.

@storybook-app-bot
Copy link
Copy Markdown

storybook-app-bot Bot commented Feb 13, 2026

Package Benchmarks

Commit: 1ef385e, ran on 18 February 2026 at 08:11:14 UTC

No significant changes detected, all good. 👏

@valentinpalkovic valentinpalkovic merged commit 5f8dace into next Feb 18, 2026
236 of 293 checks passed
@valentinpalkovic valentinpalkovic deleted the harden-websocket-security branch February 18, 2026 08:12
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Core Team Projects Feb 18, 2026
valentinpalkovic added a commit that referenced this pull request Feb 18, 2026
Core: Require token for websocket connections
(cherry picked from commit 5f8dace)
@github-actions github-actions Bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Feb 18, 2026
@github-actions github-actions Bot mentioned this pull request Feb 18, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:daily Run the CI jobs that normally run in the daily job. patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants