From 84a256d0ac8ac2c3be6c4ffdbd422eda47f3abcc Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Thu, 12 Feb 2026 01:23:29 +0700 Subject: [PATCH] fix(cursor): address second-round review feedback for auth module - Add comprehensive unit tests for cursor-auth.test.ts - validateToken: valid/invalid tokens, short tokens, UUID formats, empty strings - extractUserInfo: JWT parsing, email handling, non-JWT tokens, malformed base64 - saveCredentials/loadCredentials: round-trip, invalid JSON/types, missing fields - checkAuthStatus: authenticated/not authenticated, expired tokens, JWT exp, invalid dates - deleteCredentials: delete existing/non-existent files, multiple deletes - All tests use CCS_HOME env var for isolation, real file I/O, no mocks - Fix dead try-catch around new Date() in checkAuthStatus() - Replace try-catch with isNaN check (new Date('garbage') returns Invalid Date, not throw) - Properly handle Invalid Date by checking isNaN(getTime()) - Fix email populated with sub claim in extractUserInfo() - Change email: decoded.email || decoded.sub to email: decoded.email || undefined - Prevent non-email values (UUIDs) from populating email field - Add type guards for JSON.parse result in extractUserInfo() - Cast to Record and validate types - Use typeof checks for email, userId, exp fields --- src/cursor/cursor-auth.ts | 19 +- tests/unit/cursor/cursor-auth.test.ts | 388 ++++++++++++++++++++++++++ 2 files changed, 399 insertions(+), 8 deletions(-) create mode 100644 tests/unit/cursor/cursor-auth.test.ts diff --git a/src/cursor/cursor-auth.ts b/src/cursor/cursor-auth.ts index 460d044f..58d6d8da 100644 --- a/src/cursor/cursor-auth.ts +++ b/src/cursor/cursor-auth.ts @@ -164,11 +164,16 @@ export function extractUserInfo( } const decoded = JSON.parse( Buffer.from(payload.replace(/-/g, '+').replace(/_/g, '/'), 'base64').toString() - ); + ) as Record; return { - email: decoded.email || decoded.sub, - userId: decoded.sub || decoded.user_id, - exp: decoded.exp, + email: typeof decoded.email === 'string' ? decoded.email : undefined, + userId: + typeof decoded.sub === 'string' + ? decoded.sub + : typeof decoded.user_id === 'string' + ? decoded.user_id + : undefined, + exp: typeof decoded.exp === 'number' ? decoded.exp : undefined, }; } } catch { @@ -274,13 +279,11 @@ export function checkAuthStatus(): CursorAuthStatus { } else { // Fallback to importedAt heuristic const TOKEN_EXPIRY_HOURS = 24; - try { - const importedDate = new Date(credentials.importedAt); + const importedDate = new Date(credentials.importedAt); + if (!isNaN(importedDate.getTime())) { const now = new Date(); tokenAge = Math.floor((now.getTime() - importedDate.getTime()) / (1000 * 60 * 60)); expired = tokenAge >= TOKEN_EXPIRY_HOURS; - } catch { - // Invalid date format } } diff --git a/tests/unit/cursor/cursor-auth.test.ts b/tests/unit/cursor/cursor-auth.test.ts new file mode 100644 index 00000000..e66e2901 --- /dev/null +++ b/tests/unit/cursor/cursor-auth.test.ts @@ -0,0 +1,388 @@ +/** + * Unit tests for Cursor authentication module + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import type { CursorCredentials } from '../../../src/cursor/types'; +import { + validateToken, + extractUserInfo, + saveCredentials, + loadCredentials, + checkAuthStatus, + deleteCredentials, +} from '../../../src/cursor/cursor-auth'; + +// Test isolation +let originalCcsHome: string | undefined; +let tempDir: string; + +beforeEach(() => { + // Save original CCS_HOME + originalCcsHome = process.env.CCS_HOME; + + // Create temp directory for test isolation + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-cursor-test-')); + process.env.CCS_HOME = tempDir; +}); + +afterEach(() => { + // Restore original CCS_HOME + if (originalCcsHome !== undefined) { + process.env.CCS_HOME = originalCcsHome; + } else { + delete process.env.CCS_HOME; + } + + // Clean up temp directory + if (fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +describe('validateToken', () => { + it('should accept valid token and machineId', () => { + const token = 'a'.repeat(50); // 50 chars minimum + const machineId = 'a'.repeat(32); // 32 hex chars + expect(validateToken(token, machineId)).toBe(true); + }); + + it('should reject 49-char token (too short)', () => { + const token = 'a'.repeat(49); // Just under minimum + const machineId = 'a'.repeat(32); + expect(validateToken(token, machineId)).toBe(false); + }); + + it('should reject 31-char hex UUID (too short)', () => { + const token = 'a'.repeat(50); + const machineId = 'a'.repeat(31); // Just under 32 + expect(validateToken(token, machineId)).toBe(false); + }); + + it('should accept UUID with hyphens (strips them)', () => { + const token = 'a'.repeat(50); + const machineId = '12345678-1234-1234-1234-123456789abc'; // 36 chars with hyphens + expect(validateToken(token, machineId)).toBe(true); + }); + + it('should reject empty token', () => { + const machineId = 'a'.repeat(32); + expect(validateToken('', machineId)).toBe(false); + }); + + it('should reject empty machineId', () => { + const token = 'a'.repeat(50); + expect(validateToken(token, '')).toBe(false); + }); + + it('should reject non-hex characters in machineId', () => { + const token = 'a'.repeat(50); + const machineId = 'g'.repeat(32); // 'g' is not valid hex + expect(validateToken(token, machineId)).toBe(false); + }); +}); + +describe('extractUserInfo', () => { + it('should extract email and sub from valid JWT', () => { + // JWT: {"email":"user@example.com","sub":"12345","exp":1234567890} + const payload = Buffer.from( + JSON.stringify({ email: 'user@example.com', sub: '12345', exp: 1234567890 }) + ).toString('base64'); + const token = `header.${payload}.signature`; + + const result = extractUserInfo(token); + expect(result).toEqual({ + email: 'user@example.com', + userId: '12345', + exp: 1234567890, + }); + }); + + it('should return undefined email when only sub claim exists', () => { + // JWT: {"sub":"uuid-12345","exp":1234567890} + const payload = Buffer.from( + JSON.stringify({ sub: 'uuid-12345', exp: 1234567890 }) + ).toString('base64'); + const token = `header.${payload}.signature`; + + const result = extractUserInfo(token); + expect(result).toEqual({ + email: undefined, + userId: 'uuid-12345', + exp: 1234567890, + }); + }); + + it('should return null for non-JWT token', () => { + const token = 'a'.repeat(50); // Plain token + const result = extractUserInfo(token); + expect(result).toBe(null); + }); + + it('should return null for malformed base64', () => { + const token = 'header.!!!invalid-base64!!!.signature'; + const result = extractUserInfo(token); + expect(result).toBe(null); + }); + + it('should handle JWT with user_id instead of sub', () => { + // JWT: {"email":"user@example.com","user_id":"67890"} + const payload = Buffer.from( + JSON.stringify({ email: 'user@example.com', user_id: '67890' }) + ).toString('base64'); + const token = `header.${payload}.signature`; + + const result = extractUserInfo(token); + expect(result).toEqual({ + email: 'user@example.com', + userId: '67890', + exp: undefined, + }); + }); +}); + +describe('saveCredentials and loadCredentials', () => { + it('should save and load credentials successfully', () => { + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'auto-detect', + importedAt: new Date().toISOString(), + }; + + saveCredentials(credentials); + const loaded = loadCredentials(); + + expect(loaded).toEqual(credentials); + }); + + it('should return null when no credentials file exists', () => { + const loaded = loadCredentials(); + expect(loaded).toBe(null); + }); + + it('should create directory with restrictive permissions', () => { + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'manual', + importedAt: new Date().toISOString(), + }; + + saveCredentials(credentials); + + // CCS_HOME is set to tempDir, but getCcsDir() appends '.ccs' to it + const credDir = path.join(tempDir, '.ccs', 'cursor'); + expect(fs.existsSync(credDir)).toBe(true); + + // Check directory permissions (skip on Windows) + if (process.platform !== 'win32') { + const stats = fs.statSync(credDir); + const mode = stats.mode & 0o777; + expect(mode).toBe(0o700); + } + }); + + it('should return null for invalid JSON in credentials file', () => { + const credPath = path.join(tempDir, 'cursor', 'credentials.json'); + fs.mkdirSync(path.dirname(credPath), { recursive: true }); + fs.writeFileSync(credPath, 'invalid json{{{'); + + const loaded = loadCredentials(); + expect(loaded).toBe(null); + }); + + it('should return null for credentials missing required fields', () => { + const credPath = path.join(tempDir, 'cursor', 'credentials.json'); + fs.mkdirSync(path.dirname(credPath), { recursive: true }); + fs.writeFileSync( + credPath, + JSON.stringify({ + accessToken: 'token', + // Missing machineId, authMethod, importedAt + }) + ); + + const loaded = loadCredentials(); + expect(loaded).toBe(null); + }); + + it('should return null for credentials with wrong types', () => { + const credPath = path.join(tempDir, 'cursor', 'credentials.json'); + fs.mkdirSync(path.dirname(credPath), { recursive: true }); + fs.writeFileSync( + credPath, + JSON.stringify({ + accessToken: 123, // Wrong type (number instead of string) + machineId: 'abc', + authMethod: 'auto-detect', + importedAt: new Date().toISOString(), + }) + ); + + const loaded = loadCredentials(); + expect(loaded).toBe(null); + }); + + it('should return null for invalid authMethod', () => { + const credPath = path.join(tempDir, 'cursor', 'credentials.json'); + fs.mkdirSync(path.dirname(credPath), { recursive: true }); + fs.writeFileSync( + credPath, + JSON.stringify({ + accessToken: 'token', + machineId: 'abc', + authMethod: 'invalid-method', // Invalid authMethod + importedAt: new Date().toISOString(), + }) + ); + + const loaded = loadCredentials(); + expect(loaded).toBe(null); + }); +}); + +describe('checkAuthStatus', () => { + it('should return not authenticated when no credentials exist', () => { + const status = checkAuthStatus(); + expect(status.authenticated).toBe(false); + expect(status.credentials).toBeUndefined(); + }); + + it('should return authenticated for valid credentials', () => { + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'auto-detect', + importedAt: new Date().toISOString(), + }; + + saveCredentials(credentials); + const status = checkAuthStatus(); + + expect(status.authenticated).toBe(true); + expect(status.credentials).toEqual(credentials); + expect(status.expired).toBe(false); + expect(status.tokenAge).toBeDefined(); + expect(status.tokenAge).toBeLessThan(1); // Just imported + }); + + it('should detect expired credentials (importedAt > 24h ago)', () => { + // Create credentials from 25 hours ago + const past = new Date(); + past.setHours(past.getHours() - 25); + + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'manual', + importedAt: past.toISOString(), + }; + + saveCredentials(credentials); + const status = checkAuthStatus(); + + expect(status.authenticated).toBe(true); + expect(status.expired).toBe(true); + expect(status.tokenAge).toBeGreaterThanOrEqual(24); + }); + + it('should return not authenticated for invalid token format', () => { + const credentials: CursorCredentials = { + accessToken: 'short', // Invalid (too short) + machineId: 'b'.repeat(32), + authMethod: 'manual', + importedAt: new Date().toISOString(), + }; + + saveCredentials(credentials); + const status = checkAuthStatus(); + + expect(status.authenticated).toBe(false); + }); + + it('should use JWT exp claim when available', () => { + // Create JWT token that expired 1 hour ago + const expiredTime = Math.floor(Date.now() / 1000) - 3600; + const payload = Buffer.from( + JSON.stringify({ email: 'test@example.com', sub: '123', exp: expiredTime }) + ).toString('base64'); + const jwtToken = `header.${payload}.signature`; + + const credentials: CursorCredentials = { + accessToken: jwtToken, + machineId: 'b'.repeat(32), + authMethod: 'auto-detect', + importedAt: new Date().toISOString(), // Recent import + }; + + saveCredentials(credentials); + const status = checkAuthStatus(); + + expect(status.authenticated).toBe(true); + expect(status.expired).toBe(true); // Should detect expiry from JWT exp + }); + + it('should handle invalid importedAt date gracefully', () => { + // Create credentials with valid format but garbage date value + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'manual', + importedAt: 'invalid-date-garbage-2026-99-99T99:99:99Z', + }; + + saveCredentials(credentials); + const status = checkAuthStatus(); + + // Should still authenticate if token format is valid + expect(status.authenticated).toBe(true); + // tokenAge should be undefined due to invalid date (NaN from getTime()) + expect(status.tokenAge).toBeUndefined(); + // expired should be false (defaults to false when date parsing fails) + expect(status.expired).toBe(false); + }); +}); + +describe('deleteCredentials', () => { + it('should delete existing credentials file and return true', () => { + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'auto-detect', + importedAt: new Date().toISOString(), + }; + + saveCredentials(credentials); + expect(loadCredentials()).not.toBe(null); + + const result = deleteCredentials(); + expect(result).toBe(true); + expect(loadCredentials()).toBe(null); + }); + + it('should return false when credentials file does not exist', () => { + const result = deleteCredentials(); + expect(result).toBe(false); + }); + + it('should handle multiple delete calls gracefully', () => { + const credentials: CursorCredentials = { + accessToken: 'a'.repeat(50), + machineId: 'b'.repeat(32), + authMethod: 'manual', + importedAt: new Date().toISOString(), + }; + + saveCredentials(credentials); + + // First delete should succeed + expect(deleteCredentials()).toBe(true); + + // Second delete should return false (already deleted) + expect(deleteCredentials()).toBe(false); + }); +});