feat(web): require token auth when bound to a non-loopback host - #105
Conversation
The web server had no authentication — the only gate was an Origin check, which does not stop direct (non-browser) requests. Binding to a non-loopback host (`--host 0.0.0.0`) therefore exposed full agent control (shell/file tools, MCP) to anyone who could reach the port. When bound to a non-loopback host (or when a token is explicitly provided), the server now requires a bearer token: - internal/web/auth.go: authMiddleware enforces the token on all non-exempt requests (positive allowlist: SPA assets, /api/health, /api/auth/verify). Token is read from Authorization: Bearer, the `jcode-auth` WebSocket subprotocol, or a ?token= fallback; compared in constant time. - command/web.go: new --auth-token flag (also JCODE_WEB_TOKEN); otherwise a token is auto-generated, persisted to ~/.jcode/web_token (0600), and printed in the startup banner. Loopback binds keep the existing no-auth behaviour. - Frontend: a TokenGate login page (shown when /api/health reports auth_required), Authorization-header injection, and WS token via subprotocol. - pty.go: fix the PTY WebSocket CheckOrigin (was unconditionally true) to isAllowedWebOrigin as defence in depth. Loopback default deployments keep no-auth (unchanged); the separate simple-request CSRF hardening for that case is tracked independently. Tests: internal/web/auth_test.go covers loopback detection, token extraction, constant-time compare, the exemption allowlist, and middleware allow/deny. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds end-to-end bearer token authentication to the web server. The CLI resolves or generates a cryptographic token, passes it into the HTTP server, which enforces it via middleware (exempting CORS preflights, ChangesWeb Token Authentication
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (App.vue)
participant API as /api/health & /api/auth/verify
participant AuthMW as authMiddleware
participant TokenGate as TokenGate.vue
Browser->>API: GET /api/health (no auth needed)
API-->>Browser: { auth_required: true }
Browser->>TokenGate: render overlay (needsAuth=true)
TokenGate->>API: POST /api/auth/verify (Authorization: Bearer <token>)
AuthMW->>AuthMW: isAuthExempt → pass through
API-->>TokenGate: 200 { ok: true }
TokenGate->>Browser: emit authed, setAuthToken
Browser->>Browser: onAuthed() → boot() with token
Browser->>API: subsequent /api/* requests (Authorization header injected)
AuthMW->>AuthMW: validToken → pass through
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
cnjack
left a comment
cnjack
left a comment
There was a problem hiding this comment.
Code Review — Senior Staff Engineer
The structure here is sound. Constant-time comparison, 256-bit auto-generated tokens, positive exemption allowlist, WebSocket subprotocol token carriage — all correct. The PR closes a real pre-existing RCE. What follows are concrete defects that need to be addressed before this ships to users who run --host 0.0.0.0.
Finding 1 (High): Token travels in cleartext — no TLS for non-loopback bind
Impact
The entire PR closes "unauthenticated shell access over the network." But the server serves only HTTP. When a user runs jcode web --host 0.0.0.0 over a LAN or VPN, the bearer token is sent in cleartext on every HTTP request and every WebSocket handshake. A passive observer on the same L2 segment captures the token from the first authenticated request and gains identical shell access. The startup banner even prints the URL as http://.
Evidence
internal/command/web.go — no TLS anywhere; the startup message reads Open http://%s:%d/.
Suggested Fix
At minimum, add a prominent warning at startup that the token provides no protection against passive network observers without TLS. Ideally, add --tls-cert/--tls-key flags (or auto-generate a self-signed cert and print its fingerprint) and serve HTTPS when binding non-loopback. Without TLS the auth layer is cosmetic against a LAN attacker, which is the exact deployment this PR targets.
Finding 2 (High): /api/health discloses sensitive data to unauthenticated callers
Impact
Any unauthenticated peer that can reach the port — the threat actor this PR targets — can call GET /api/health without a token and learn: the working directory (pwd), AI provider and model, a live session_id, whether a task is running, and the server version. This bypasses the token gate entirely.
Evidence
internal/web/auth.go:
if r.Method == http.MethodGet && p == "/api/health" {
return true // the frontend probes this before it has a token
}handleHealth returns the full payload — pwd, provider, model, session_id, running, image_support — to unauthenticated callers regardless.
The exemption exists because the frontend must discover auth_required before it can show the login gate — a valid UX constraint. But only that flag needs to be public.
Suggested Fix
In handleHealth, when s.requireAuth is true and the request carries no valid token, return only:
{"auth_required": true, "version": "..."}The full payload should only be returned to authenticated callers.
Finding 3 (Medium): ?token= query-parameter is active in production and leaks into logs
Impact
extractToken falls back to r.URL.Query().Get("token"). This value lands in Go's default HTTP logger, any reverse proxy access log, browser history, and Referer headers. The WebSocket URL pattern ws://host:8080/api/pty/{id}/ws?token=… is particularly damaging — browser devtools, network traces, and proxies capture it verbatim.
Evidence
internal/web/auth.go:55:
return r.URL.Query().Get("token")Suggested Fix
Remove this fallback. Browser WebSocket clients have Sec-WebSocket-Protocol; fetch callers have Authorization: Bearer. If it must exist for non-browser CLI tools, gate it behind a server-side log warning so operators can detect its use.
Finding 4 (Medium): Token stored in localStorage — one XSS away from permanent shell credential theft
Impact
authToken.ts stores the token in localStorage with no TTL and no rotation. jcode renders AI-generated content, tool outputs, and file diffs. Any XSS (in a third-party dependency, in rendered content) can call localStorage.getItem('jcode_web_token') and exfiltrate a token giving interactive shell access indefinitely.
Evidence
web/src/composables/authToken.ts:14:
const token = ref<string>(localStorage.getItem(STORAGE_KEY) || '')No expiration, no rotation, no logout endpoint.
Suggested Fix
Use sessionStorage instead of localStorage to limit persistence to the tab lifetime. Or — better — set an HttpOnly, SameSite=Strict cookie server-side after a successful /api/auth/verify response; JavaScript never holds the token, eliminating the XSS surface entirely.
Finding 5 (Low): Token printed to stdout on every restart — captured by service supervisors
Impact
fmt.Printf emits the full bearer token whenever requireAuth == true, including on restarts where the token was loaded from the existing ~/.jcode/web_token file. Under systemd, docker logs, or CI/CD pipelines this ends up in logs with broader read access than the operator's terminal.
Evidence
internal/command/web.go:
if requireAuth {
fmt.Printf("\n🔐 Web access token (required when reaching %s):\n %s\n", host, webToken)Suggested Fix
On first generation, print the full token. On subsequent restarts (token loaded from file), print only the file path and a short suffix hint: Token loaded from ~/.jcode/web_token (…xxxx).
Minor: Middleware comment describes execution order backwards
internal/web/server.go comment reads // Auth (token) then CORS. but corsMiddleware(s.authMiddleware(mux)) means CORS runs first (outer closure). Change to // CORS first (outer), auth second (inner).
Overall Risk: Medium
The auth mechanism itself is structurally correct and closes a critical pre-existing hole. The remaining risk after this PR merges: a passive LAN attacker can still steal the token over plaintext HTTP (Finding 1 — breaks the core guarantee of the PR); an unauthenticated attacker can enumerate pwd, session state, and model from the health endpoint (Finding 2); a future XSS makes the localStorage token a permanent shell credential (Finding 4).
Findings 1 and 2 should be addressed before merge. Findings 3 and 4 can ship as a close follow-on if timeline is tight.
Generated by Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
web/src/composables/authToken.ts (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn a readonly token ref.
Callers only need to observe auth state here. Returning the mutable ref lets components bypass
setAuthToken()and leavelocalStorageout of sync with the in-memory token.Proposed fix
-import { ref } from 'vue' +import { readonly, ref } from 'vue' @@ export function useAuthToken() { - return token + return readonly(token) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/composables/authToken.ts` around lines 34 - 35, The useAuthToken() composable is returning the mutable token ref, which lets callers mutate auth state directly and bypass setAuthToken(). Change useAuthToken() to expose a readonly token reference instead, while keeping setAuthToken() as the only mutation path so the in-memory token and localStorage stay synchronized.web/src/composables/api.ts (1)
16-24: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize
HeadersInitbefore merging.
RequestOptionsadvertises fullRequestInit, but this merge only works for plain-object headers.Headersinstances get dropped and tuple arrays spread into numeric keys, so the helper breaks its own contract.Proposed fix
async function request<T>(path: string, options?: RequestOptions): Promise<T> { const token = getAuthToken() + const headers = new Headers(options?.headers) + if (!headers.has('Content-Type')) headers.set('Content-Type', 'application/json') + if (token && !options?.skipAuth && !headers.has('Authorization')) { + headers.set('Authorization', `Bearer ${token}`) + } const resp = await fetch(`${apiBase}${path}`, { ...options, - headers: { - 'Content-Type': 'application/json', - ...(token && !options?.skipAuth ? { Authorization: `Bearer ${token}` } : {}), - ...options?.headers, - }, + headers, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/composables/api.ts` around lines 16 - 24, The request helper in request() is merging headers as if options.headers were always a plain object, which breaks the stated RequestInit contract for Headers and tuple-array HeaderInit values. Normalize options.headers to a Headers object before merging in fetch, then set the Content-Type and Authorization values on that normalized instance so all HeadersInit forms are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/command/web.go`:
- Around line 603-605: The auth-token diagnostics in the web startup path are
writing directly to stdout via fmt.Printf, which bypasses the app’s output
routing. Update the requireAuth branch in web startup logic to send any
user-visible messaging through the existing command/UI output channel or
config.Logger() instead of printing directly, and ensure the webToken secret is
not emitted to debug logs or other logger output.
- Around line 127-137: Create the config directory before persisting the web
token so first-run remote starts do not fall back to a session-only token; in
the web token flow inside the token persistence logic, ensure ConfigDir() exists
before the os.WriteFile call. Update the token generation path around
generateWebToken and the subsequent write to create the parent directory with
appropriate permissions, then proceed with writing web_token so the saved token
survives restarts.
- Around line 118-120: The explicit token path in getToken currently accepts any
value, but that same token is later reused as a WebSocket subprotocol in the web
client, so invalid characters can break startup. Update the explicit-token
handling in internal/command/web.go (the getToken logic) to reject or normalize
tokens that are not valid WebSocket subprotocol values, and make sure the token
returned here is safe for the later new WebSocket(..., ['jcode-auth', token])
call.
In `@internal/web/auth.go`:
- Around line 43-55: The token extraction in extractToken currently falls back
to r.URL.Query().Get("token") for every request, which should be limited to
WebSocket-only endpoints. Update the auth flow so Authorization remains the only
token source for normal HTTP API routes, and allow the query-token fallback only
when the request path matches the WebSocket endpoints used by /api/ws and
/api/pty/.../ws. Use extractToken and its caller(s) in the auth middleware to
gate the fallback by path before returning the query token.
In `@web/src/App.vue`:
- Around line 337-345: The auth gate in App.vue is treating every verifyToken()
failure as an invalid token, which wrongly sends users to TokenGate on transient
/api/auth/verify outages. Update the auth-check logic around verifyToken() so
only a 401/unauthorized response sets needsAuth to true, while transport,
timeout, or 5xx errors are surfaced as connection/retry failures; apply the same
fix to the duplicate auth-gate flow referenced in the other App.vue block. Use
the existing verifyToken(), needsAuth, and TokenGate paths to locate both spots.
In `@web/src/components/TokenGate.vue`:
- Around line 148-150: The `.auth-error` style in `TokenGate.vue` still uses a
hardcoded hex fallback in `color`; replace `var(--color-danger-fg, `#dc2626`)`
with the existing token-only custom property from `src/styles/tokens.css` so the
stylesheet relies on a defined design token only. Update the `auth-error` rule
in the Vue component and keep the color reference consistent with the token
naming used elsewhere in the app.
- Around line 26-31: The TokenGate.vue auth flow is treating every failure from
api.authVerify(candidate) as an invalid token, which hides network and
server-side errors. Update the try/catch around api.authVerify in the TokenGate
component so only 401 responses set error.value to t('auth.invalid') and still
emit('authed')/setAuthToken(candidate) on success, while all other failures
surface a retryable or server-error message instead of bad-credentials.
---
Nitpick comments:
In `@web/src/composables/api.ts`:
- Around line 16-24: The request helper in request() is merging headers as if
options.headers were always a plain object, which breaks the stated RequestInit
contract for Headers and tuple-array HeaderInit values. Normalize
options.headers to a Headers object before merging in fetch, then set the
Content-Type and Authorization values on that normalized instance so all
HeadersInit forms are preserved.
In `@web/src/composables/authToken.ts`:
- Around line 34-35: The useAuthToken() composable is returning the mutable
token ref, which lets callers mutate auth state directly and bypass
setAuthToken(). Change useAuthToken() to expose a readonly token reference
instead, while keeping setAuthToken() as the only mutation path so the in-memory
token and localStorage stay synchronized.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 904d53c8-4703-476e-810a-2dba07b3a585
📒 Files selected for processing (16)
internal/command/web.gointernal/web/auth.gointernal/web/auth_test.gointernal/web/pty.gointernal/web/server.goweb/src/App.vueweb/src/components/TerminalInstance.vueweb/src/components/TokenGate.vueweb/src/composables/api.tsweb/src/composables/authToken.tsweb/src/composables/ws.tsweb/src/i18n/locales/en.tsweb/src/i18n/locales/ja.tsweb/src/i18n/locales/ko.tsweb/src/i18n/locales/zh-Hans.tsweb/src/i18n/locales/zh-Hant.ts
- auth.go: limit ?token= fallback to the WebSocket endpoints only (keep bearer tokens out of HTTP access/proxy logs and history); add IsValidWSSubprotocolToken. - command/web.go: reject explicit tokens that aren't valid WS subprotocol values; MkdirAll ~/.jcode before persisting the auto-generated token so a first remote start doesn't silently fall back to a session-only token. - api.ts: normalize HeadersInit via Headers; attach response status to thrown errors so callers can distinguish 401 from transport/5xx. - App.vue / TokenGate.vue: only treat 401 as an invalid token; transport/5xx now surface as connection/server errors instead of forcing the login gate. - authToken.ts: return a readonly token ref from useAuthToken. - TokenGate.vue: use the --color-error-fg design token (drop hardcoded hex). - i18n: add auth.serverError (en/zh-Hans/zh-Hant/ja/ko). - auth_test.go: cover ws-only query fallback and IsValidWSSubprotocolToken. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(web): require token auth when bound to a non-loopback host The web server had no authentication — the only gate was an Origin check, which does not stop direct (non-browser) requests. Binding to a non-loopback host (`--host 0.0.0.0`) therefore exposed full agent control (shell/file tools, MCP) to anyone who could reach the port. When bound to a non-loopback host (or when a token is explicitly provided), the server now requires a bearer token: - internal/web/auth.go: authMiddleware enforces the token on all non-exempt requests (positive allowlist: SPA assets, /api/health, /api/auth/verify). Token is read from Authorization: Bearer, the `jcode-auth` WebSocket subprotocol, or a ?token= fallback; compared in constant time. - command/web.go: new --auth-token flag (also JCODE_WEB_TOKEN); otherwise a token is auto-generated, persisted to ~/.jcode/web_token (0600), and printed in the startup banner. Loopback binds keep the existing no-auth behaviour. - Frontend: a TokenGate login page (shown when /api/health reports auth_required), Authorization-header injection, and WS token via subprotocol. - pty.go: fix the PTY WebSocket CheckOrigin (was unconditionally true) to isAllowedWebOrigin as defence in depth. Loopback default deployments keep no-auth (unchanged); the separate simple-request CSRF hardening for that case is tracked independently. Tests: internal/web/auth_test.go covers loopback detection, token extraction, constant-time compare, the exemption allowlist, and middleware allow/deny. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(web): address CodeRabbit review on token auth - auth.go: limit ?token= fallback to the WebSocket endpoints only (keep bearer tokens out of HTTP access/proxy logs and history); add IsValidWSSubprotocolToken. - command/web.go: reject explicit tokens that aren't valid WS subprotocol values; MkdirAll ~/.jcode before persisting the auto-generated token so a first remote start doesn't silently fall back to a session-only token. - api.ts: normalize HeadersInit via Headers; attach response status to thrown errors so callers can distinguish 401 from transport/5xx. - App.vue / TokenGate.vue: only treat 401 as an invalid token; transport/5xx now surface as connection/server errors instead of forcing the login gate. - authToken.ts: return a readonly token ref from useAuthToken. - TokenGate.vue: use the --color-error-fg design token (drop hardcoded hex). - i18n: add auth.serverError (en/zh-Hans/zh-Hant/ja/ko). - auth_test.go: cover ws-only query fallback and IsValidWSSubprotocolToken. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
背景
jcode 的 web 服务此前无任何鉴权,唯一准入是 Origin 网关——而 Origin 网关只能挡浏览器跨源,挡不住直接(非浏览器)HTTP 请求。一旦用
--host 0.0.0.0把服务暴露到网络(远程访问 / SSH 隧道 / 开发盒子等真实场景),任何能访问端口的人即可完全控制 agent(执行 shell、读写文件、跑 MCP)。本 PR:当 web 绑定到非 loopback 地址时,强制 token 鉴权,并提供一个登录页让远程用户输入 token。 loopback(默认
127.0.0.1/ 桌面 sidecar)行为完全不变。改动
服务端
internal/web/auth.go(新):authMiddleware对所有非豁免请求强制 token。GET /api/health、POST /api/auth/verify),避免“漏注册的 /api 路径 fallback 到 SPA 被放行”。Authorization: Bearer→jcode-authWebSocket 子协议 →?token=兜底;crypto/subtle.ConstantTimeCompare常量时间比较。server.go:中间件链corsMiddleware(s.authMiddleware(mux))(cors 在外层吞掉 OPTIONS 预检);新增POST /api/auth/verify;/api/health返回auth_required;wsUpgrader声明jcode-auth子协议。command/web.go:新增--auth-tokenflag(也支持JCODE_WEB_TOKEN环境变量);否则非 loopback 时自动生成 token、持久化到~/.jcode/web_token(0600)、在启动横幅打印。pty.go:顺手修复 PTY WebSocket 的CheckOrigin(此前无条件return true,在 0.0.0.0 下是直接拿 shell 的 RCE)→ 改为isAllowedWebOrigin(纵深防御)。前端
composables/authToken.ts(新):模块级 token 存取(避开 api↔store 循环依赖)。composables/api.ts:统一注入Authorization: Bearer、401 触发登录态失效、authVerify。composables/ws.ts/components/TerminalInstance.vue:WebSocket 用['jcode-auth', token]子协议带 token(不进 URL/日志),重连实时读取。components/TokenGate.vue(新):全屏登录页;App.vue的boot()插入 auth gate(严格早于 setup gate,因为/api/setup/*也受保护);5 个语言文件加auth.*文案。安全说明
Content-Type: application/json触发预检)与本次 token 鉴权正交,已单独跟进,不在此 PR。测试
internal/web/auth_test.go:loopback 判定、token 三来源提取、常量时间比较、豁免白名单、中间件 allow/deny。go build ./...、go test ./internal/web/ ./internal/runner/、gofmt、go vet均通过。vue-tsc类型检查 +vite build通过。jcode web --host 0.0.0.0→ 横幅打印 token → 浏览器输入。Reviewer 注意
requireAuth=false,行为零改动(向后兼容)。~/.jcode/web_token持久化复用;--auth-token/JCODE_WEB_TOKEN不落盘(适合 CI / 容器)。🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation