Security: Authentication token sent in WebSocket query string#648
Conversation
…ry strin The shell WebSocket URL appends the auth token as a `?token=` query parameter. Query-string tokens are prone to leakage via logs, monitoring tools, reverse proxies, browser history, and diagnostics. This increases risk of token exposure and session hijacking. Affected files: socket.ts Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com>
📝 WalkthroughWalkthroughThe Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/shell/utils/socket.ts (1)
7-11:⚠️ Potential issue | 🟠 MajorSimplify redundant branch and fix type signature mismatch.
Both branches return the same URL string, making the
IS_PLATFORMconditional unnecessary. Additionally,getShellWebSocketUrl()always returns a string in both paths but is declared to returnstring | null. This type mismatch creates dead code in the caller: the null check at useShellConnection.ts:119 (if (!wsUrl)) can never be triggered.Either change the return type to
stringor ensure the function has a code path that can actually return null.Suggested cleanup
-export function getShellWebSocketUrl(): string | null { +export function getShellWebSocketUrl(): string { const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; - - if (IS_PLATFORM) { - return `${protocol}//${window.location.host}/shell`; - } - return `${protocol}//${window.location.host}/shell`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/shell/utils/socket.ts` around lines 7 - 11, The function getShellWebSocketUrl currently has a redundant IS_PLATFORM branch and is typed to return string | null while both branches always return a string; change the signature to return string, remove the IS_PLATFORM conditional and return the computed `${protocol}//${window.location.host}/shell` directly, and then update callers (e.g. the wsUrl null check in useShellConnection where it does `if (!wsUrl)`) to assume a non-null string or remove the unreachable null-guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/shell/utils/socket.ts`:
- Line 11: The client WebSocket URL builder in socket.ts currently returns
`${protocol}//${window.location.host}/shell` with no token, so
server.verifyClient (which calls authenticateWebSocket) receives undefined and
rejects the connection; fix this by restoring token transport from the client
when opening the shell socket—modify the URL builder to append the client token
(e.g., ?token=...) or set an Authorization header on the WebSocket upgrade by
reading the existing client auth helper (e.g., getAuthToken/getClientToken or
localStorage/session) so authenticateWebSocket receives the token;
alternatively, if you prefer a server change, update server/index.js
verifyClient to accept a session-based credential (cookie/session) instead of
requiring a query/header token.
---
Outside diff comments:
In `@src/components/shell/utils/socket.ts`:
- Around line 7-11: The function getShellWebSocketUrl currently has a redundant
IS_PLATFORM branch and is typed to return string | null while both branches
always return a string; change the signature to return string, remove the
IS_PLATFORM conditional and return the computed
`${protocol}//${window.location.host}/shell` directly, and then update callers
(e.g. the wsUrl null check in useShellConnection where it does `if (!wsUrl)`) to
assume a non-null string or remove the unreachable null-guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7f1aec4f-62fd-4291-85ac-15fb1dce9396
📒 Files selected for processing (1)
src/components/shell/utils/socket.ts
| } | ||
|
|
||
| return `${protocol}//${window.location.host}/shell?token=${encodeURIComponent(token)}`; | ||
| return `${protocol}//${window.location.host}/shell`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm /shell connection path and handler signature
rg -n -C3 "pathname === '/shell'|handleShellConnection\\s*\\("
# 2) Confirm OSS WebSocket auth expectations
rg -n -C3 "authenticateWebSocket\\s*\\(|if\\s*\\(!token\\)"
# 3) Inspect shell handler implementation for auth enforcement and request/cookie access
rg -n -C4 "function\\s+handleShellConnection|const\\s+handleShellConnection|handleShellConnection\\s*=|cookie|authorization|token|jwt"Repository: siteboon/claudecodeui
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full handleShellConnection function implementation (grep for function start and capture lines until next function or EOF)
rg -n -A50 "^function handleShellConnection" server/index.js | head -80Repository: siteboon/claudecodeui
Length of output: 2867
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full handleShellConnection including all event handlers
rg -n -A150 "^function handleShellConnection" server/index.js | head -200Repository: siteboon/claudecodeui
Length of output: 8784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the verifyClient or upgrade handler for /shell
rg -n -B10 "handleShellConnection" server/index.js | grep -E "(verifyClient|upgrade|pathname|/shell)" | head -30Repository: siteboon/claudecodeui
Length of output: 208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find verifyClient function and examine auth for both /shell and /ws
rg -n -B5 -A15 "verifyClient\s*[:=]" server/index.js | head -100Repository: siteboon/claudecodeui
Length of output: 859
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full verifyClient function
rg -n -A30 "verifyClient\s*:\s*\(info\)" server/index.jsRepository: siteboon/claudecodeui
Length of output: 1399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the getShellWebSocketUrl or similar function to see if headers include token
rg -n -A10 "socket\.ts|getShellWebSocketUrl|/shell" src/components/shell/utils/socket.ts 2>/dev/null || rg -n -B5 -A10 "return.*shell" src/components/shell/utils/socket.tsRepository: siteboon/claudecodeui
Length of output: 567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's a WebSocket connection initiation with headers
rg -n -A10 "new WebSocket|getShellWebSocketUrl" src/components/shell --type ts --type tsxRepository: siteboon/claudecodeui
Length of output: 92
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check shell component for WebSocket connection
rg -n "getShellWebSocketUrl\|new WebSocket" src/components/shell/Repository: siteboon/claudecodeui
Length of output: 47
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find files importing or using getShellWebSocketUrl
rg -l "getShellWebSocketUrl" src/Repository: siteboon/claudecodeui
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how shell WebSocket connection is initiated
cat src/components/shell/hooks/useShellConnection.tsRepository: siteboon/claudecodeui
Length of output: 7442
OSS mode shell connections will fail authentication after removing client token transport.
The client now connects to /shell without any token in the URL or headers (Line 11). However, server/index.js Lines 295–325 (verifyClient) extracts the token from the query string or Authorization header and rejects the connection if authentication fails. Since the shell WebSocket sends neither, authenticateWebSocket(undefined) returns null, causing verifyClient to reject the connection.
This breaks shell functionality in OSS mode. Either restore token transport in the client URL, or implement an alternative auth mechanism (e.g., extract credentials from the WebSocket upgrade request metadata or use a secure session-based approach) on the server.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/shell/utils/socket.ts` at line 11, The client WebSocket URL
builder in socket.ts currently returns
`${protocol}//${window.location.host}/shell` with no token, so
server.verifyClient (which calls authenticateWebSocket) receives undefined and
rejects the connection; fix this by restoring token transport from the client
when opening the shell socket—modify the URL builder to append the client token
(e.g., ?token=...) or set an Authorization header on the WebSocket upgrade by
reading the existing client auth helper (e.g., getAuthToken/getClientToken or
localStorage/session) so authenticateWebSocket receives the token;
alternatively, if you prefer a server change, update server/index.js
verifyClient to accept a session-based credential (cookie/session) instead of
requiring a query/header token.
1 participant
Problem
The shell WebSocket URL appends the auth token as a
?token=query parameter. Query-string tokens are prone to leakage via logs, monitoring tools, reverse proxies, browser history, and diagnostics. This increases risk of token exposure and session hijacking.Severity:
mediumFile:
src/components/shell/utils/socket.tsSolution
Avoid putting bearer tokens in URLs. Prefer secure, HTTP-only session cookies for WS auth, or use a short-lived one-time WS token exchanged over authenticated HTTPS before establishing the socket. Ensure server-side logs redact sensitive parameters.
Changes
src/components/shell/utils/socket.ts(modified)Testing
Summary by CodeRabbit
Release Notes