Skip to content

Conversation

@AshishKumar4
Copy link
Contributor

@AshishKumar4 AshishKumar4 commented Dec 25, 2025

Summary

Implements a ticket-based authentication system for WebSocket connections, enabling SDK clients to authenticate without relying on browser cookies or origin validation.

Changes

  • New ticket system (worker/middleware/auth/ticketAuth.ts): One-time-use tokens with 15-second TTL for WebSocket auth
  • Ticket manager (worker/utils/wsTicketManager.ts): In-memory storage with automatic expiry cleanup
  • Ticket controller (worker/api/controllers/ticket/controller.ts): POST /api/ws-ticket endpoint for ticket creation
  • Route updates: Added ticket auth support to agent and vault WebSocket routes
  • Auth middleware (worker/middleware/auth/routeAuth.ts): Extended to support dual auth strategies (ticket or JWT)
  • Durable Objects: Added ticket store/consume methods to CodeGeneratorAgent and UserSecretsStore

Motivation

SDK clients (non-browser environments) cannot rely on cookie-based authentication or pass origin headers. This ticket system provides:

  1. Secure, short-lived tokens that can be passed as URL parameters
  2. One-time-use consumption to prevent replay attacks
  3. Origin-agnostic authentication for programmatic access
  4. Ownership verification at ticket creation time

Testing

  1. Create a ticket via POST /api/ws-ticket with JWT auth
  2. Connect to WebSocket with ?ticket=tk_xxx parameter
  3. Verify one-time use (second connection with same ticket fails)
  4. Verify expiry (ticket invalid after 15 seconds)

Security Considerations

  • Tickets are one-time-use and deleted on consumption
  • Short TTL (15 seconds) limits exposure window
  • Ownership verified at ticket creation via existing checkAppOwnership
  • Ticket format includes resource type prefix for validation
  • Rate limiting applied at ticket creation endpoint

Breaking Changes

None - existing JWT/cookie authentication continues to work unchanged.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 25, 2025

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


Code Quality & Security Review (Critical Path)

Recommendation: APPROVE


PR Summary

Implements a ticket-based authentication system for WebSocket connections, enabling SDK clients to authenticate without browser cookies or origin validation. Tickets are one-time-use, short-lived (15s TTL) tokens stored in Durable Object memory.

PR Description

  • Updated: Yes - added comprehensive description with summary, changes, motivation, testing, and security considerations

Addressed Issues


Code Quality

Severity Count Details
Critical 0 -
High 0 -
Medium 1 Missing ticket-per-user limit (see below)
Low 1 Deterministic fallback session ID

Details:

Medium - No Ticket Limit per DO
worker/utils/wsTicketManager.ts: No limit on outstanding tickets. While tickets auto-expire (15s + 1s buffer), a malicious user with valid credentials could spam ticket creation to consume DO memory. Low risk due to short TTL and existing rate limits at creation endpoint.

Low - Deterministic Session ID Fallback
worker/api/controllers/ticket/controller.ts:106: Fallback session ID ticket:${resourceType}:${resourceId} is deterministic. Minor fingerprinting concern but only used when sessionId is absent.


Security

Severity Count Details
Critical 0 -
High 0 -
Medium 1 User ID in vault ticket URL (see inline comment)
Low 0 -

Security Strengths:

  • One-time use: Ticket deleted before validity check (atomic, prevents replay)
  • Short TTL: 15-second expiry limits exposure window
  • Cryptographic randomness: Uses crypto.randomUUID() (122 bits entropy)
  • Ownership verification: Done at ticket creation via existing checkAppOwnership
  • Format validation: Strict regex patterns prevent malformed tokens
  • Rate limiting: Applied at ticket creation endpoint (JWT auth required)
  • Auto-cleanup: Prevents memory leaks via setTimeout

Token Format Analysis:

Agent: tk_<32-hex-chars>  - Opaque, good
Vault: tkv_<userId>_<32-hex-chars> - User ID exposed (see inline comment)

Testing

To verify:

  1. Create ticket: POST /api/ws-ticket with body {"resourceType":"agent","resourceId":"<agent-id>"}
  2. Connect WebSocket: ws://host/api/agent/<id>/ws?ticket=tk_xxx
  3. Verify one-time use: Second connection with same ticket should fail (403)
  4. Verify expiry: Wait 15+ seconds, connection should fail (403)
  5. Verify ownership: Ticket for non-owned agent should fail at creation (403)

Tests to add:

  • Unit tests for WsTicketManager (store, consume, expiry, has, size)
  • Integration tests for ticket authentication flow
  • Edge case: concurrent ticket consumption attempts

Final Recommendation

APPROVE - This is a well-designed authentication system that follows security best practices:

  1. Defense in depth: Ownership checked at creation, format validated at consumption
  2. Minimal attack surface: Short-lived, one-time-use tokens
  3. Non-breaking: Existing JWT auth continues to work unchanged
  4. Clean architecture: Follows existing patterns, proper type safety, reuses code

The medium-severity issues noted are acceptable tradeoffs given the mitigations in place. Consider adding the test suite mentioned above in a follow-up PR.


@AshishKumar4 AshishKumar4 changed the base branch from main to fix/sdk-fixes December 25, 2025 02:42

switch (resourceType) {
case 'vault':
return `${VAULT_TICKET_PREFIX}${resourceId}_${random}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] The vault ticket format tkv_{userId}_{random} embeds the user ID directly in the token. This has some implications:

  1. URL Logging: If URLs are logged (server logs, CDN logs, browser history), user IDs get exposed
  2. Information Disclosure: Anyone intercepting the WebSocket URL can extract the user ID

Mitigation factors:

  • Short 15s TTL limits exposure window
  • Connections should be over TLS
  • User IDs are not highly sensitive in this context

Consider: Using an opaque token format (like agent tickets) and looking up the user via the DO where the ticket is stored. However, given the mitigations and complexity tradeoff, this is acceptable for the current use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User ID is anyways not considered a secret, and it's required to verify the token resource ownership

@AshishKumar4 AshishKumar4 merged commit eb45e2c into fix/sdk-fixes Dec 25, 2025
9 checks passed
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.

1 participant