Skip to content

Conversation

@AshishKumar4
Copy link
Contributor

@AshishKumar4 AshishKumar4 commented Dec 28, 2025

Summary

This PR introduces ticket-based WebSocket authentication for the SDK and improves SDK documentation, type safety, and cross-runtime compatibility.

Changes

SDK Changes

  • WebSocket Authentication: SDK now uses one-time-use tickets for WebSocket connections instead of passing JWTs in the URL
  • Native WebSocket Only: Removed Node.js-specific WebSocket shim; SDK now requires native WebSocket support (Node.js 22+, browsers, Bun, Cloudflare Workers)
  • Documentation: Comprehensive README rewrite with complete API reference, examples, and compatibility table
  • Type Improvements: Using platform types directly via protocol.ts for better type consistency
  • Build Tooling: Added expand-drizzle-types.ts script for cleaner type exports in SDK bundle

Worker Changes (Security)

  • Ticket Authentication System (worker/middleware/auth/ticketAuth.ts):
    • One-time-use tickets with 15-second TTL
    • Two token formats: tk_{random} (agent) and tkv_{userId}_{random} (vault)
    • Stored in DO memory via WsTicketManager class
  • New Ticket Controller (worker/api/controllers/ticket/controller.ts):
    • POST /api/ws-ticket endpoint for ticket creation
    • Ownership verification before ticket issuance
  • Route Auth Updates (worker/middleware/auth/routeAuth.ts):
    • Added ticketAuth option to setAuthLevel middleware
    • Dual auth strategy: ticket-based (SDK) OR JWT-based (browser)
    • Ticket auth skips origin validation for SDK clients
  • DO Updates: CodeGeneratorAgent and UserSecretsStore now implement ticket storage/consumption

Motivation

  • Improve WebSocket security by avoiding JWT exposure in URLs (prevents URL logging vulnerabilities)
  • Enable SDK usage from any runtime with native WebSocket support
  • Provide better SDK documentation and developer experience
  • Improve type safety with direct platform type usage

Testing

  • Unit tests updated in sdk/test/session-ws.test.ts and sdk/test/ws-routing.test.ts
  • New test server (sdk/test/test-server.ts) simulates ticket-based auth flow
  • Integration tests available via bun run test:integration

To test locally:

cd sdk
bun test                    # Unit tests
bun run test:integration    # Integration tests (requires API key)

Breaking Changes

  • Node.js < 22: SDK no longer supports Node.js versions without native WebSocket. Use Node.js 22+ or switch to Bun.
  • WebSocket URL format: SDK clients must now use ticket-based auth; direct JWT in URL no longer supported for SDK connections.

Related Issues

@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

Claude finished @AshishKumar4's task in 2m 47s —— View job


Code Quality & Security Review (Critical Path)

Recommendation: APPROVE

PR Summary

This PR introduces ticket-based WebSocket authentication for the SDK, replacing direct JWT exposure in WebSocket URLs with secure, one-time-use tickets. It also improves SDK documentation, removes Node.js WebSocket shim requirements, and enhances type safety.

PR Description

  • Updated: No (already comprehensive)
  • Notes: The existing description is well-written with clear sections for changes, motivation, testing, and breaking changes.

Addressed Issues


Security Review

Severity Count
Critical 0
High 0
Medium 0
Low 1

Security Implementation - Well Designed:

Component Assessment
ticketAuth.ts Secure: One-time use, 15s TTL, cryptographically random tokens, proper validation
TicketController Secure: Auth required, ownership verified before ticket issuance
WsTicketManager Secure: In-memory storage, auto-cleanup, atomic consume pattern
routeAuth.ts Secure: Dual auth strategy, immediate 403 on invalid ticket
SDK (http.ts, ws.ts) Secure: Fresh ticket per connect, proper URL encoding

Low Severity:

  • routeAuth.ts:95 contains a pre-existing console.log statement that logs auth requirements and user info. Not introduced in this PR but worth noting for cleanup.

Code Quality

Severity Count
Critical 0
High 0
Medium 0
Low 0

Positive Observations:

  • Clean separation of concerns between ticket generation, storage, and consumption
  • Proper TypeScript types throughout (PendingWsTicket, TicketConsumptionResult, etc.)
  • Good logging with appropriate levels
  • SDK README is comprehensive with clear API documentation
  • Test coverage added for new functionality

Testing

Test Type Status
CI Passing
Unit tests sdk/test/session-ws.test.ts, sdk/test/ws-routing.test.ts, sdk/test/state.test.ts updated
Integration tests New sdk/test/integration/ with worker integration tests
Test server sdk/test/test-server.ts simulates ticket-based auth flow

Architecture Notes

Ticket Flow:

1. SDK calls POST /api/ws-ticket (authenticated)
2. Server verifies ownership, creates ticket, stores in DO memory
3. SDK connects to WebSocket with ?ticket=tk_xxx
4. DO consumes ticket (one-time), establishes authenticated connection
5. On reconnect, SDK fetches fresh ticket

Key Design Decisions:

  • Tickets stored in DO memory (ephemeral) - appropriate for short-lived tokens
  • 15-second TTL prevents replay attacks while allowing for connection latency
  • Agent tickets (tk_*) and vault tickets (tkv_*) have distinct formats
  • Rate limiting happens at ticket creation, not WebSocket connection

Breaking Changes Acknowledged

  • Node.js < 22 no longer supported (requires native WebSocket)
  • SDK clients must use ticket-based auth (JWT in URL no longer supported for SDK)

Final Recommendation

APPROVE - This PR implements a well-designed security improvement that removes JWT exposure in WebSocket URLs. The ticket-based authentication system follows security best practices:

  • Short-lived, one-time-use tokens
  • Secure random generation
  • Proper ownership verification
  • Clean separation of concerns

The code quality is good, types are properly defined, and test coverage has been added. No critical or high severity issues found.


Tasks

  • Gather context and understand changes
  • Review PR description completeness
  • Security review (critical path - auth/middleware)
  • Code quality review
  • Post inline comments for critical issues (none found)
  • Post final summary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants