Core: Harden websocket connection#33859
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds WebSocket token-based validation: server generates a wsToken at startup and exposes it to development clients via rendered globals; clients include the token in the channel URL; server validates upgrade tokens with a timing-safe comparator and rejects invalid upgrades. Also updates CORS for JSON endpoints and release/changelog/package metadata. Changes
Sequence DiagramsequenceDiagram
participant Preset as Preset System
participant Server as Storybook Server
participant Builder as Builder Manager
participant Client as Browser
participant WSTransport as ServerChannelTransport
Preset->>Preset: generate wsToken (randomUUID)
Preset-->>Server: channelOptions { wsToken }
Server->>Builder: renderHTML(presets)
Builder->>Client: HTML + globals (includes wsToken)
Client->>Client: build channel URL /storybook-server-channel?token=<wsToken>
Client->>WSTransport: WebSocket upgrade request (with token)
WSTransport->>WSTransport: parse token from URL
WSTransport->>WSTransport: isValidToken(requestToken, expectedToken)
alt token valid
WSTransport->>Server: accept upgrade -> establish WS
else token invalid
WSTransport->>Client: respond 403, close connection
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code/lib/core-server/src/presets/common-preset.ts (1)
164-177: Avoid overriding a user-specifiedwsToken.
Lines 174‑177 always replace any configured token in development. Consider preserving an existingcore.channelOptions.wsTokento allow deterministic overrides (tests, custom clients).🔧 Suggested adjustment
channelOptions: { ...(existing?.channelOptions ?? {}), - ...(options.configType === 'DEVELOPMENT' ? { wsToken } : {}), + ...(options.configType === 'DEVELOPMENT' + ? { wsToken: existing?.channelOptions?.wsToken ?? wsToken } + : {}), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/core-server/src/presets/common-preset.ts` around lines 164 - 177, The current core preset always injects a generated wsToken into channelOptions during DEVELOPMENT, which overwrites any user-provided token; update the export const core(...) logic so it preserves an existing token (check existing?.channelOptions?.wsToken) and only injects randomUUID() when no token is present (or when explicitly intended), i.e. merge channelOptions but prefer the existing wsToken over wsToken constant; update references in core, channelOptions and wsToken accordingly so tests/custom clients can deterministically set their own token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 3: Update the 7.6.23 changelog entry to fix the typo "websocke" to
"websocket" in the CHANGELOG.md entry text; locate the line containing the
7.6.23 bullet and replace "Harden websocke connection" with "Harden websocket
connection" so the entry reads correctly.
In `@code/builders/builder-manager/src/utils/template.ts`:
- Around line 42-57: The code reads wsToken from the top-level of the value
returned by presets.apply('core'), but presets.apply('core') returns a full
CoreConfig so channelOptions?.wsToken is undefined; update the assignment that
sets globals.CHANNEL_OPTIONS so it pulls the token from the correct nested
property on the CoreConfig returned by presets.apply('core') (for example from
channelOptions.channels.manager.wsToken or the actual nested path used by your
CoreConfig), then assign globals.CHANNEL_OPTIONS = { wsToken: <that nested
value> } so the manager receives the token.
---
Nitpick comments:
In `@code/lib/core-server/src/presets/common-preset.ts`:
- Around line 164-177: The current core preset always injects a generated
wsToken into channelOptions during DEVELOPMENT, which overwrites any
user-provided token; update the export const core(...) logic so it preserves an
existing token (check existing?.channelOptions?.wsToken) and only injects
randomUUID() when no token is present (or when explicitly intended), i.e. merge
channelOptions but prefer the existing wsToken over wsToken constant; update
references in core, channelOptions and wsToken accordingly so tests/custom
clients can deterministically set their own token.
| @@ -1,3 +1,7 @@ | |||
| ## 7.6.23 | |||
|
|
|||
| - Harden websocke connection | |||
There was a problem hiding this comment.
Fix typo in the 7.6.23 entry.
“websocke” should be “websocket”.
✏️ Proposed fix
-- Harden websocke connection
+- Harden websocket connection📝 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.
| - Harden websocke connection | |
| - Harden websocket connection |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 3, Update the 7.6.23 changelog entry to fix the typo
"websocke" to "websocket" in the CHANGELOG.md entry text; locate the line
containing the 7.6.23 bullet and replace "Harden websocke connection" with
"Harden websocket connection" so the entry reads correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/builders/builder-manager/src/utils/template.ts`:
- Around line 53-57: The CHANNEL_OPTIONS global is being assigned a raw object
(globals.CHANNEL_OPTIONS = { wsToken: coreOptions?.channelOptions?.wsToken })
which will render as [object Object]; instead, match the other globals by
JSON-stringifying the value before assignment—replace the raw object assignment
with a JSON.stringify(...) of the object so window['CHANNEL_OPTIONS'] gets valid
JSON; update the code around the configType === 'DEVELOPMENT' branch where
presets.apply('core') is used and globals.CHANNEL_OPTIONS is set, ensuring
coreOptions?.channelOptions?.wsToken is wrapped in JSON.stringify.
|
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/22103594050 |
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake 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-33859-sha-95913f7f. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-33859-sha-95913f7f sandboxor in an existing project withnpx storybook@0.0.0-pr-33859-sha-95913f7f upgrade.More information
0.0.0-pr-33859-sha-95913f7fhotfix/v7.6.2395913f7f1771341936)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=33859Summary by CodeRabbit
Bug Fixes
Chores