Skip to content

sec(ws): single-use ticket auth on /ws — drop JWT-in-URL leak (#550) - #580

Merged
vybe merged 1 commit into
devfrom
feature/550-websocket-ticket-auth
Apr 29, 2026
Merged

sec(ws): single-use ticket auth on /ws — drop JWT-in-URL leak (#550)#580
vybe merged 1 commit into
devfrom
feature/550-websocket-ticket-auth

Conversation

@dolho

@dolho dolho commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the residual finding from the April 2026 UnderDefense remediation pentest (3.2.1, residual CVSS ~2.1): JWT in the WebSocket URL leaks into nginx access logs, browser history, and upstream proxies, and same-origin auto-cookie behavior opens a CSWSH surface.

New flow

  1. Browser POST /api/ws/ticket (JWT in Authorization header). Backend mints a 32-byte urlsafe opaque ticket, stores it in Redis with a 30s TTL, returns it.
  2. Browser connects to /ws?ticket=<opaque>. Backend atomically GETDELs the Redis key (single-use, Redis 6.2+) and resolves to the authenticated subject before websocket.accept().
  3. Reconnects mint a fresh ticket; the JWT never enters the WebSocket URL.

CSWSH is mitigated because POST /api/ws/ticket requires the JWT in an Authorization header — a malicious page can't mint a ticket on the victim's behalf without an explicit cross-origin request, which CORS rejects.

Components

  • services/ws_ticket_service.py — mint + atomic-GETDEL consume. Fails closed when Redis is down (mint raises 503; consume returns None/ws closes 4001).
  • routers/ws_tickets.pyPOST /api/ws/ticket (JWT-authed).
  • main.py /ws — drops ?token= path entirely; requires ?ticket=.
  • Frontend (utils/websocket.js + stores/network.js) — fetch ticket via existing axios bearer-auth defaults, then connect with ?ticket=. The two real /ws call sites are the only ones touched (useProcessWebSocket.js is dead code, AgentTerminal/useVoiceSession use different endpoints).

Scope decision: /ws/events kept on ?token=

/ws/events (Trinity Connect for external listeners) keeps ?token=trinity_mcp_xxx for compatibility with documented external scripts (websocat, wscat). MCP keys are scoped, named, and revocable so the leak surface is bounded relative to a JWT. If we want to ticket-gate that surface too, it's a separate follow-up — adding tickets there would require updating the Trinity Connect docs and any deployed scripts.

Closes #550.

Test plan

  • Unit: pytest unit/test_ws_ticket_service.py — 7/7 pass (mint+GETDEL single-use, expired/missing/malformed handling, Redis-down fail-closed).
  • Live protocol smoke (run against the running stack):
    • POST /api/ws/ticket unauthed → 401 ✅
    • POST /api/ws/ticket with JWT → 43-char ticket ✅
    • /ws no params → 403 (rejected before accept) ✅
    • /ws?ticket=<valid> → connects, ping/pong works ✅
    • /ws?ticket=<used> (replay) → 403 ✅ (single-use proven)
  • Browser eyeball (DevTools Network → WS): dashboard shows "Connected" + "Live"; both Trinity WS connections (websocket.js, network.js) URL-show ?ticket=… with no JWT; status 101.
  • Frontend build clean (vite build).

Notes for reviewer

  • Redis required (already a hard dependency). Failing closed on Redis-down is intentional — auth must not silently degrade.
  • Old browser tabs with cached pre-SEC: WebSocket JWT passed in URL query param — CSWSH and token leakage #550 JS will hit /ws?token=… and get rejected with 403 → frontend already handles a clean reconnect via the existing setTimeout(connect, 5000) path. Worst case is one failed connect followed by hard refresh; no data loss.

🤖 Generated with Claude Code

Closes the residual finding from the April 2026 UnderDefense
remediation pentest (3.2.1, residual CVSS ~2.1): JWT in the
WebSocket URL leaks into nginx access logs, browser history, and
upstream proxy logs, and the cookie-style same-origin handshake
opens a CSWSH surface.

New flow:
  1. Browser POSTs /api/ws/ticket (JWT in Authorization header).
     Backend mints a 32-byte urlsafe opaque ticket, stores it in
     Redis with a 30s TTL, returns it.
  2. Browser connects to /ws?ticket=<opaque>. Backend atomically
     GETDELs the Redis key (single-use) and resolves to the
     authenticated subject before websocket.accept().
  3. Reconnects re-mint a fresh ticket; the JWT never enters the
     WebSocket URL.

CSWSH is mitigated because POST /api/ws/ticket requires the JWT in
an Authorization header — a malicious page can't mint a ticket on
the victim's behalf without an explicit cross-origin request, which
CORS rejects.

Components:
- services/ws_ticket_service.py: mint + atomic-GETDEL consume.
  Fails closed when Redis is down (mint raises 503; consume
  returns None → /ws closes 4001).
- routers/ws_tickets.py: POST /api/ws/ticket (JWT-authed).
- main.py /ws: drop ?token= path entirely; require ?ticket=.
- frontend (utils/websocket.js + stores/network.js): fetch ticket
  via existing axios bearer-auth defaults, then connect with
  ?ticket=. The two real /ws call sites are the only ones touched
  — useProcessWebSocket.js is dead code (not imported anywhere)
  and AgentTerminal/useVoiceSession use different endpoints.

Scope note: /ws/events keeps ?token=trinity_mcp_xxx for documented
external clients (websocat, wscat scripts). MCP keys are scoped,
named, and revocable so the leak surface is bounded relative to
a JWT.

Tests: 7 unit tests on ws_ticket_service covering mint+GETDEL
single-use, expired/missing/malformed handling, and Redis-down
fail-closed. Live smoke verified protocol end-to-end:
ticket → ws connect → ping/pong; same ticket reused → 403.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@vybe vybe left a comment

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.

LGTM — clean security fix, 7 unit tests, browser-verified, architecture.md updated. Note: /ws/events still uses ?token= by design; recommend tracking a follow-up issue for that surface.

@vybe
vybe merged commit 4142e15 into dev Apr 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants