feat(cursor): add credential detection, SQLite token extraction, and auth status

[kaitranntt]

Summary

• Auto-detects Cursor IDE credentials from SQLite state database
• Extracts accessToken, machineId with secure file handling
• Auth status checking with token expiry detection (JWT exp claim or 24h heuristic)
• Part of feat: add Cursor IDE proxy integration with protobuf support #517, Closes feat: Cursor IDE — auth module with SQLite auto-detect #519

Files Added (2)

• src/cursor/cursor-auth.ts — Credential detection, SQLite extraction, auth status
• src/cursor/types.ts — Shared type definitions (CursorCredentials, CursorAuthStatus, CursorDaemonStatus)

Edge Cases Fixed (code review)

• HIGH: Credentials stored with mode 0o600/0o700 file permissions
• MEDIUM: execSync → execFileSync to prevent shell injection
• MEDIUM: Token expiry uses JWT exp claim when available, falls back to importedAt heuristic
• MEDIUM: typeof validation for loadCredentials fields
• LOW: Windows platform check with helpful error message
• LOW: ENOENT detection for sqlite3 availability
• LOW: deleteCredentials function added for cleanup

Test plan

• TypeScript compiles clean
• ESLint passes
• Prettier format check passes
• SQLite token extraction on macOS/Linux

[ccs-reviewer]

Code Review: PR #526 — feat(cursor): add credential detection, SQLite token extraction, and auth status

---

📋 Summary

This PR introduces a new src/cursor/ module with two files: cursor-auth.ts (credential auto-detection from Cursor IDE's SQLite database, validation, persistence) and types.ts (shared type definitions). The auth module is well-structured with good defensive coding (file permissions, execFileSync over execSync, graceful error handling). However, there are a SQL injection concern, missing tests, a UUID validation bug, and YAGNI violations in the types file that should be addressed.

---

✅ Strengths

1. ⭐ Restrictive file permissions — Credentials written with 0o600, directories with 0o700 (cursor-auth.ts:187-194). Follows the established codebase pattern from gemini-token-refresh.ts, startup-lock.ts, etc.
2. ⭐ execFileSync over execSync — Prevents shell injection by avoiding shell interpretation (cursor-auth.ts:55). Good security practice.
3. getCcsDir() used correctly for credential storage path (cursor-auth.ts:175), respecting CCS_HOME test isolation per CLAUDE.md.
4. Safe JSON parsing — loadCredentials() parses as unknown then validates with in operator before casting (cursor-auth.ts:209-221). Defensive pattern.
5. Platform-aware paths — getTokenStoragePath() handles all three platforms correctly (cursor-auth.ts:27-48).
6. Graceful Windows fallback — Auto-detection returns clear error message for unsupported platform (cursor-auth.ts:71-77).
7. Token expiry tracking — checkAuthStatus() includes expired field with 24-hour threshold (cursor-auth.ts:244-256).

---

🔍 Observations & Suggestions

🔴 HIGH — SQL Injection in queryStateDb()

File: cursor-auth.ts:57

The key parameter is interpolated directly into the SQL query string:

[dbPath, `SELECT value FROM itemTable WHERE key='${key}'`]

While currently called only with hardcoded strings, this is a latent injection vector. If any future caller passes user-controlled input, a crafted key like '; DROP TABLE itemTable; -- would execute arbitrary SQL.

Suggested fix — escape single quotes:

function queryStateDb(dbPath: string, key: string): string | null {
  // Escape single quotes to prevent SQL injection
  const sanitizedKey = key.replace(/'/g, "''");
  try {
    const result = execFileSync(
      'sqlite3',
      [dbPath, `SELECT value FROM itemTable WHERE key='${sanitizedKey}'`],
      { encoding: 'utf8', timeout: 5000, stdio: ['pipe', 'pipe', 'ignore'] }
    ).trim();
    return result || null;
  } catch {
    return null;
  }
}

---

🔴 HIGH — No Tests

The PR adds 407 lines of new code with zero test coverage. Per CLAUDE.md: "Tests added/updated — if behavior changed". The cursor module is highly testable:

• validateToken() — pure function, easy to unit test
• extractUserInfo() — pure function with JWT edge cases
• getTokenStoragePath() — platform-dependent, testable with mocks
• saveCredentials() / loadCredentials() — uses getCcsDir() which supports CCS_HOME isolation
• checkAuthStatus() — composition of above

Missing test file: tests/unit/cursor/cursor-auth.test.ts

---

🟡 MEDIUM — UUID Validation Regex Bug

File: cursor-auth.ts:134-136

const uuidRegex = /^[a-f0-9-]{32,}$/i;
if (!uuidRegex.test(machineId.replace(/-/g, ''))) {

Two issues:

1. The - in the character class is redundant — hyphens were already stripped by .replace(/-/g, '')
2. {32,} (32 or more) is overly permissive — a UUID without hyphens is exactly 32 hex characters. This would accept a 1000-character hex string.

Fix:

const hexRegex = /^[a-f0-9]{32}$/i;
if (!hexRegex.test(machineId.replace(/-/g, ''))) {

---

🟡 MEDIUM — Token Expiry Based on Import Time, Not Actual Expiry

File: cursor-auth.ts:244-253

Token age is calculated from importedAt (when CCS saved it), not from the actual JWT exp claim. This means:

• A token imported 1 second before its real expiry appears valid for 24 hours
• A fresh token with a 1-hour server-side expiry appears valid for 24 hours

Suggestion: If the token is a JWT, extract the exp claim in extractUserInfo() and use it for expiry checking. Fall back to importedAt-based heuristic for non-JWT tokens.

---

🟡 MEDIUM — YAGNI Violation in types.ts

File: types.ts:39-129

The file defines 7 interfaces that are not used by cursor-auth.ts:

• CursorDaemonStatus (line 42)
• CursorModel (line 54)
• MessageRole (line 68)
• CursorMessage (line 73)
• CursorToolCall (line 87)
• CursorToolResult (line 106)
• ProtobufExtractResult (line 120)

Per CLAUDE.md: "YAGNI: No features 'just in case'". These types belong in the PRs that implement the modules using them. Only CursorCredentials, CursorAuthStatus, and AutoDetectResult are needed for this PR.

With noUnusedLocals: true in tsconfig, these will pass compilation since they're exported, but they violate the project's design principles.

---

🟡 MEDIUM — PR Description / Code Mismatch on refreshToken

The PR description states "Extracts accessToken, refreshToken, machineId" but the code does not extract refreshToken:

• Not queried in autoDetectTokens()
• Not defined in CursorCredentials interface
• Not stored in credentials file

Either update the code to extract refreshToken or correct the PR description.

---

🟢 LOW — Weak Type Validation in loadCredentials()

File: cursor-auth.ts:212-220

The in operator checks only verify key existence, not value types:

if (
  typeof parsed === 'object' &&
  parsed !== null &&
  'accessToken' in parsed &&   // could be number, null, etc.
  'machineId' in parsed &&
  'authMethod' in parsed &&
  'importedAt' in parsed
) {
  return parsed as CursorCredentials;
}

A corrupted file with { accessToken: 123, machineId: null, authMethod: true, importedAt: [] } would pass validation and cause runtime errors downstream. Consider adding type checks:

const obj = parsed as Record<string, unknown>;
if (
  typeof obj.accessToken === 'string' &&
  typeof obj.machineId === 'string' &&
  (obj.authMethod === 'auto-detect' || obj.authMethod === 'manual') &&
  typeof obj.importedAt === 'string'
) {
  return obj as CursorCredentials;
}

---

🟢 LOW — Missing sqlite3 Availability Check

File: cursor-auth.ts:53-64

If sqlite3 is not installed on Linux/macOS, execFileSync throws ENOENT. The catch block silently returns null, and autoDetectTokens() reports "Access token not found in database" — misleading when the real issue is a missing binary.

Suggestion: Add a specific error case:

} catch (err: unknown) {
  if (err instanceof Error && 'code' in err && (err as NodeJS.ErrnoException).code === 'ENOENT') {
    return null; // sqlite3 not found — caller should handle
  }
  return null;
}

Or check for sqlite3 availability before querying in autoDetectTokens().

---

🟢 LOW — No deleteCredentials() Function

The module has saveCredentials() and loadCredentials() but no way to remove stored credentials. Consider adding for completeness:

export function deleteCredentials(): boolean {
  const credPath = getCredentialsPath();
  if (fs.existsSync(credPath)) {
    fs.unlinkSync(credPath);
    return true;
  }
  return false;
}

---

🔒 Security Considerations

Check	Status	Notes
Command injection prevention	✅	execFileSync used (no shell interpretation)
SQL injection prevention	❌	Key parameter interpolated directly into SQL query
File permission hardening	✅	0o600 for files, 0o700 for directories
Secrets in source code	✅	No hardcoded secrets
Input validation	⚠️	Token validated; SQL key not sanitized; loaded JSON weakly validated
Path traversal prevention	✅	Paths constructed from platform APIs, not user input
JWT handling	✅	Decode-only, no verification (appropriate for local token extraction)
Credential storage	⚠️	Plaintext JSON on disk (acceptable with 0o600 perms, consistent with codebase)

---

📊 Code Quality Checklist

Criteria	Status	Notes
Follows CLAUDE.md guidelines	⚠️	YAGNI violation in types.ts
Uses getCcsDir() for CCS paths	✅	Correctly used for credential storage
Uses os.homedir() appropriately	✅	Only for Cursor's own install paths (not CCS paths)
No any types	✅	Uses unknown for parsed JSON
No non-null assertions	✅	None found
Conventional commit format	✅	feat(cursor): and fix(cursor):
Tests included	❌	No tests for 407 lines of new code
ASCII-only CLI output	✅	No CLI output in this module (library code)
Error handling	✅	Graceful fallbacks throughout
Cross-platform support	✅	Linux, macOS, Windows paths handled

---

💡 Recommendations

Priority	Issue	Action
🔴 High	SQL injection in queryStateDb()	Sanitize key param by escaping single quotes
🔴 High	No test coverage	Add tests/unit/cursor/cursor-auth.test.ts
🟡 Medium	UUID regex bug	Change {32,} to {32}, remove - from char class
🟡 Medium	YAGNI in types.ts	Remove unused interfaces; add them in their respective PRs
🟡 Medium	PR description mentions refreshToken	Either implement or update description
🟡 Medium	Token expiry based on import time	Consider using JWT exp claim when available
🟢 Low	Weak loadCredentials() validation	Add typeof checks on values, not just key existence
🟢 Low	Silent sqlite3 failure	Detect ENOENT and return descriptive error
🟢 Low	No deleteCredentials()	Add credential cleanup function

---

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES

The module demonstrates solid security awareness (file permissions, execFileSync, graceful error handling) and follows most codebase conventions. However, two items should be addressed before merge:

1. SQL injection in queryStateDb() — While currently called only with hardcoded keys, this is a latent vulnerability that violates defense-in-depth. A one-line fix (key.replace(/'/g, "''")) eliminates the risk.
2. Missing tests — 407 lines of security-sensitive credential handling code with zero tests is a significant gap. The module's pure functions (validateToken, extractUserInfo) and getCcsDir()-based storage make it straightforward to test.

The YAGNI issues in types.ts and UUID regex bug are worth fixing but not blocking.

---

🤖 Reviewed by gemini-claude-opus-4-6-thinking

[ccs-reviewer]

Code Review: PR #526 — feat(cursor): add credential detection, SQLite token extraction, and auth status

Follow-up review after author addressed feedback from the initial review (SQL injection fix, UUID regex fix, YAGNI cleanup, file permissions, execFileSync migration, Windows check, token expiry field).

---

📋 Summary

This PR adds a src/cursor/ module (2 files, ~317 lines) for auto-detecting Cursor IDE credentials from its SQLite state database, validating tokens, persisting credentials with secure file permissions, and checking auth status. The author has addressed the critical SQL injection and UUID regex issues from the prior review. Several medium and low priority items remain, most notably the absence of tests and a subtle Date constructor behavior bug.

---

✅ Strengths

1. ⭐ All critical security fixes applied — execFileSync over execSync (cursor-auth.ts:57), SQL key sanitization (cursor-auth.ts:56), restrictive file permissions 0o600/0o700 (cursor-auth.ts:189-196).
2. ⭐ YAGNI cleanup — types.ts reduced from ~130 lines to 51, retaining only the 3 interfaces needed by this module.
3. Correct getCcsDir() usage for credential storage (cursor-auth.ts:177), fully respecting CCS_HOME test isolation per CLAUDE.md.
4. Defensive JSON parsing — loadCredentials() parses as unknown then validates with in operator before casting (cursor-auth.ts:211-221).
5. Clear error messages — autoDetectTokens() returns context-specific error strings that guide the user toward resolution (cursor-auth.ts:74-108).
6. Good UUID regex fix — {32} is exact, and hyphen was removed from stripped character class (cursor-auth.ts:136).

---

🔍 Observations & Suggestions

🔴 HIGH — No Test Coverage

Status: Unresolved from prior review.

The module adds 317 lines of security-sensitive credential handling code with zero tests. Per CLAUDE.md pre-commit checklist: "Tests added/updated — if behavior changed".

Highly testable pure functions:

• validateToken() — boundary cases (49-char token, 31-char hex, hyphens in UUID)
• extractUserInfo() — valid JWT, invalid JWT, non-JWT token
• saveCredentials() / loadCredentials() — round-trip with CCS_HOME isolation
• checkAuthStatus() — no creds, valid creds, expired creds, corrupted date

Expected location: tests/unit/cursor/cursor-auth.test.ts

---

🟡 MEDIUM — new Date() Never Throws — Dead try-catch

File: cursor-auth.ts:251-258

try {
  const importedDate = new Date(credentials.importedAt);
  const now = new Date();
  tokenAge = Math.floor((now.getTime() - importedDate.getTime()) / (1000 * 60 * 60));
  expired = tokenAge >= TOKEN_EXPIRY_HOURS;
} catch {
  // Invalid date format
}

new Date('garbage') does not throw — it returns an Invalid Date object. getTime() then returns NaN, making tokenAge = NaN and expired = false (since NaN >= 24 evaluates to false). The catch block is dead code.

Impact: A corrupted importedAt value silently results in expired: false and tokenAge: NaN, which could mislead callers into treating expired tokens as valid.

Suggested fix:

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;
}

This removes the dead try-catch and correctly handles invalid dates by leaving tokenAge as undefined and expired as false (or better: treat invalid dates as expired).

---

🟡 MEDIUM — extractUserInfo() Populates email with Non-Email Value

File: cursor-auth.ts:161-164

return {
  email: decoded.email || decoded.sub,
  userId: decoded.sub || decoded.user_id,
};

If the JWT has no email claim but has sub (a common JWT pattern), the email field receives the subject identifier (typically a UUID or opaque ID), not an email address. Downstream code that displays this as "Logged in as: {email}" would show garbage.

Suggested fix:

return {
  email: decoded.email || undefined,
  userId: decoded.sub || decoded.user_id,
};

---

🟡 MEDIUM — PR Description Mentions refreshToken Not In Code

The PR body states "Extracts accessToken, refreshToken, machineId" but the code does not extract or store refreshToken:

• Not queried in autoDetectTokens()
• Not in the CursorCredentials interface
• Not persisted in credentials.json

Update the PR description to match the implementation.

---

🟢 LOW — Weak Type Validation in loadCredentials()

File: cursor-auth.ts:214-221

The in operator only checks key existence, not value types. A corrupted file like { accessToken: 123, machineId: null, authMethod: true, importedAt: [] } passes validation and causes runtime errors downstream.

Suggested fix:

const obj = parsed as Record<string, unknown>;
if (
  typeof obj.accessToken === 'string' &&
  typeof obj.machineId === 'string' &&
  (obj.authMethod === 'auto-detect' || obj.authMethod === 'manual') &&
  typeof obj.importedAt === 'string'
) {
  return obj as CursorCredentials;
}

Note: validateToken() is called in checkAuthStatus(), which provides some downstream protection — but loadCredentials() is a public function that could be called independently.

---

🟢 LOW — Silent sqlite3 ENOENT Returns Misleading Error

File: cursor-auth.ts:53-66

If sqlite3 is not installed on Linux/macOS, execFileSync throws ENOENT. The catch block silently returns null, and autoDetectTokens() reports "Access token not found in database" — misleading when the real issue is a missing sqlite3 binary.

Suggestion: Check for sqlite3 availability in autoDetectTokens() before querying, or inspect the error code in queryStateDb():

} catch (err: unknown) {
  if (err && typeof err === 'object' && 'code' in err && err.code === 'ENOENT') {
    // sqlite3 binary not found — could surface this to caller
  }
  return null;
}

---

🟢 LOW — decoded Is Implicitly any from JSON.parse

File: cursor-auth.ts:158

const decoded = JSON.parse(
  Buffer.from(payload.replace(/-/g, '+').replace(/_/g, '/'), 'base64').toString()
);

JSON.parse returns any. While the ESLint config prohibits explicit any, this implicit any from JSON.parse allows unchecked property access on decoded.email, decoded.sub, etc. Consider casting to Record<string, unknown> and using type guards for safer access.

---

🔒 Security Considerations

Check	Status	Notes
Command injection prevention	✅	execFileSync — no shell interpretation
SQL injection prevention	✅	Key parameter sanitized with single-quote escaping
File permission hardening	✅	0o600 for credentials, 0o700 for directory
Secrets in source code	✅	No hardcoded secrets
Input validation	⚠️	Token validated; loaded JSON weakly validated (key existence only)
Path traversal prevention	✅	Paths from platform APIs, not user input
JWT handling	✅	Decode-only, no verification (appropriate for local extraction)
Credential storage	✅	Plaintext with 0o600 permissions (consistent with codebase pattern)

---

📊 Code Quality Checklist

Criteria	Status	Notes
Follows CLAUDE.md guidelines	✅	YAGNI cleaned up, getCcsDir() used correctly
Uses getCcsDir() for CCS paths	✅	cursor-auth.ts:177
Uses os.homedir() appropriately	✅	Only for Cursor's own install paths (not CCS paths)
No any types	⚠️	Implicit any from JSON.parse at line 158
No non-null assertions	✅	None found
Conventional commit format	✅	feat(cursor):, fix(cursor):
Tests included	❌	No tests for 317 lines of new code
ASCII-only CLI output	✅	Module is library code with no CLI output
Error handling	⚠️	Dead try-catch around new Date()
Cross-platform support	✅	Linux, macOS paths handled; Windows returns clear error

---

💡 Recommendations

Priority	Issue	Action
🔴 High	No test coverage	Add tests/unit/cursor/cursor-auth.test.ts covering validateToken, extractUserInfo, credential persistence, and checkAuthStatus
🟡 Medium	Dead try-catch around new Date()	Replace with isNaN() check — new Date() never throws
🟡 Medium	email populated with sub claim	Use decoded.email || undefined instead of decoded.email || decoded.sub
🟡 Medium	PR description mentions refreshToken	Update description to match implementation
🟢 Low	Weak loadCredentials() validation	Add typeof checks on values, not just key existence
🟢 Low	Silent sqlite3 ENOENT	Surface "sqlite3 not installed" error to user
🟢 Low	Implicit any from JSON.parse	Cast to Record<string, unknown> with type guards

---

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES

The author has done good work addressing the critical security findings from the initial review — SQL injection is mitigated, execFileSync prevents shell interpretation, file permissions are restrictive, and the YAGNI cleanup leaves a focused type surface. The module is well-structured and follows codebase conventions.

Remaining items to address:

1. Tests (HIGH) — This is security-sensitive credential handling code. Unit tests for the pure functions are straightforward and should be included before merge.
2. Dead try-catch (MEDIUM) — new Date() never throws; a corrupted importedAt silently reports expired: false. Quick fix with isNaN().
3. Email/sub confusion (MEDIUM) — Minor but could surface incorrect data to users.

The low-priority items are non-blocking suggestions for improvement.

---

🤖 Reviewed by gemini-claude-opus-4-6-thinking

[kaitranntt]

/review

[ccs-reviewer]

Code Review: PR #526 — feat(cursor): add credential detection, SQLite token extraction, and auth status

---

📋 Summary

This PR introduces a Cursor IDE authentication module with SQLite-based credential auto-detection, JWT token parsing, and credential persistence. The code is well-structured with clean separation between types and logic, proper use of getCcsDir() for test isolation, and defensive security practices (restricted file permissions, execFileSync to prevent shell injection). However, four unit tests contain a path bug that causes them to pass for the wrong reason, and the tokenAge calculation when using JWT exp makes a fragile 24-hour assumption.

---

✅ Strengths

1. ⭐ Correct use of execFileSync over execSync — Prevents shell injection by passing arguments as an array rather than concatenating into a shell command (src/cursor/cursor-auth.ts:57-60).

2. ⭐ Proper credential file permissions — Directory created with 0o700, file written with 0o600. This is defense-in-depth for credential storage (src/cursor/cursor-auth.ts:202,208).

3. Robust loadCredentials validation — Parses as unknown, validates structure with in checks, then validates types individually before casting. Prevents malformed JSON from being treated as valid credentials (src/cursor/cursor-auth.ts:224-245).

4. Good SQL sanitization — Single quotes escaped via replace(/'/g, "''") in queryStateDb, and the function is private with only hardcoded key callers, making injection effectively impossible (src/cursor/cursor-auth.ts:56).

5. Clean type separation — Types in types.ts, logic in cursor-auth.ts. Interfaces are well-documented with JSDoc comments. Types follow YAGNI — only what's needed for this module.

6. Correct getCcsDir() usage — Credentials path uses getCcsDir() (not os.homedir()) per CLAUDE.md requirements (src/cursor/cursor-auth.ts:190).

7. Test isolation — beforeEach/afterEach properly saves/restores CCS_HOME and cleans up temp directories (tests/unit/cursor/cursor-auth.test.ts:23-44).

8. JWT base64url handling — Correctly replaces - with + and _ with / before decoding, and adds padding. Handles the full base64url spec (src/cursor/cursor-auth.ts:162-166).

9. isNaN fix for date validation — Correctly uses isNaN(importedDate.getTime()) instead of try-catch, since new Date('garbage') returns Invalid Date rather than throwing (src/cursor/cursor-auth.ts:283).

10. Defensive email extraction — email: typeof decoded.email === 'string' ? decoded.email : undefined prevents non-email values (like UUIDs from sub) from populating the email field (src/cursor/cursor-auth.ts:169).

---

🔍 Observations & Suggestions

🔴 HIGH — Test path mismatch: 4 negative tests pass for the wrong reason

tests/unit/cursor/cursor-auth.test.ts:189-245

Four loadCredentials tests write credentials to the wrong path, causing them to pass because the file isn't found (not because the validation logic rejects them).

The tests write to:

path.join(tempDir, 'cursor', 'credentials.json')

But loadCredentials() reads from:

getCcsDir() + '/cursor/credentials.json'
→ path.join(tempDir, '.ccs', 'cursor', 'credentials.json')

Note the missing .ccs subdirectory. getCcsDir() when CCS_HOME is set returns path.join(CCS_HOME, '.ccs'), not CCS_HOME directly.

Affected tests:

Line	Test Description	Why it falsely passes
189	should return null for invalid JSON	File not found, not invalid JSON
198	should return null for credentials missing required fields	File not found, not missing fields
213	should return null for credentials with wrong types	File not found, not wrong types
230	should return null for invalid authMethod	File not found, not invalid method

Fix: Use getCredentialsPath() or construct the correct path:

// Option A: Use the exported helper
import { getCredentialsPath } from '../../../src/cursor/cursor-auth';
const credPath = getCredentialsPath();

// Option B: Include .ccs in the path
const credPath = path.join(tempDir, '.ccs', 'cursor', 'credentials.json');

---

🟡 MEDIUM — Fragile tokenAge assumption with JWT exp

src/cursor/cursor-auth.ts:278

tokenAge = Math.floor((now - (userInfo.exp - 24 * 60 * 60)) / (60 * 60)); // Assume 24h token

This reverse-engineers the token's issued-at time by subtracting 24 hours from exp. If Cursor changes their token lifetime (e.g., to 1 hour or 7 days), this calculation silently produces incorrect tokenAge values.

Suggestion: If JWT has an iat (issued-at) claim, use that instead. Otherwise, fall back to importedAt for tokenAge even when exp is available (use exp only for expired status):

if (userInfo?.exp) {
  const now = Math.floor(Date.now() / 1000);
  expired = now >= userInfo.exp;
  // Use importedAt for tokenAge even when JWT exp is available
  const importedDate = new Date(credentials.importedAt);
  if (!isNaN(importedDate.getTime())) {
    tokenAge = Math.floor((Date.now() - importedDate.getTime()) / (1000 * 60 * 60));
  }
}

---

🟡 MEDIUM — autoDetectTokens not tested

src/cursor/cursor-auth.ts:76-120

The autoDetectTokens function has no unit tests. While testing the SQLite query path is difficult without a real database, the Windows platform check (line 78) can easily be tested, and the fs.existsSync branch (line 89) can be tested by pointing to a known-missing path.

Suggestion: Add at minimum:

• Test for the Windows branch return value (if platform mocking is feasible)
• Test for missing database file path (using a non-existent temp path)

---

🟢 LOW — TOCTOU race in deleteCredentials

src/cursor/cursor-auth.ts:301-313

if (!fs.existsSync(credPath)) {
  return false;
}
try {
  fs.unlinkSync(credPath);

A file could be deleted between existsSync and unlinkSync. For a single-user CLI tool this is negligible risk, but could be simplified:

export function deleteCredentials(): boolean {
  try {
    fs.unlinkSync(getCredentialsPath());
    return true;
  } catch {
    return false;
  }
}

This eliminates both the race condition and the extra filesystem call.

---

🟢 LOW — extractUserInfo returns empty object for JWTs without expected claims

src/cursor/cursor-auth.ts:153-184

A JWT with no email, sub, user_id, or exp claims returns { email: undefined, userId: undefined, exp: undefined } instead of null. This is a minor inconsistency: callers cannot distinguish "not a JWT" from "JWT with no useful claims" without checking if the return value is null vs an all-undefined object.

Not a bug currently (callers only use userInfo?.exp which handles both), but worth noting for future consumers.

---

🔒 Security Considerations

Check	Status	Notes
Command injection prevention	✅	execFileSync with array args
SQL injection prevention	✅	Single-quote escaping + private function with hardcoded keys only
File permission hardening	✅	0o600 file, 0o700 directory
No secrets in logs/output	✅	No console.log of tokens
Input validation at boundaries	✅	loadCredentials validates types before cast
JWT signature verification	❌ N/A	Intentionally not verified — this extracts metadata, not authenticates. Documented behavior.
Credential cleanup available	✅	deleteCredentials() provided
Test isolation from real ~/.ccs	✅	CCS_HOME env var respected
Windows APPDATA fallback	✅	Falls back to ~/AppData/Roaming if env var missing

---

📊 Code Quality Checklist

Criterion	Status	Notes
TypeScript strict mode compatible	✅	Proper type narrowing, no any
Uses getCcsDir() (not os.homedir())	✅	cursor-auth.ts:190
No emojis in CLI output	✅	No terminal output in this module
Error handling	✅	Fail-fast with clear error messages
Conventional commits	✅	All 5 commits follow format
YAGNI compliance	✅	Types trimmed in commit 7e4f080
Cross-platform paths	✅	Platform-specific getTokenStoragePath()
Test isolation	✅	CCS_HOME + temp directory cleanup
No unused imports	✅	All imports consumed

---

💡 Recommendations

Priority	Item	Location
🔴 High	Fix test path mismatch — 4 tests write to wrong directory and pass vacuously	tests/unit/cursor/cursor-auth.test.ts:189-245
🟡 Medium	Use importedAt for tokenAge when JWT exp is available, use exp only for expired flag	src/cursor/cursor-auth.ts:278
🟡 Medium	Add autoDetectTokens tests for at minimum the Windows and missing-db branches	tests/unit/cursor/
🟢 Low	Simplify deleteCredentials by removing redundant existsSync	src/cursor/cursor-auth.ts:301-313

---

🎯 Overall Assessment

⚠️ APPROVED WITH NOTES

The implementation is solid with good security practices, proper test isolation, and clean architecture. The four broken tests (they pass for the wrong reason due to a path mismatch) should be fixed before merge — they currently provide false confidence that validation logic works when in fact they're only testing "file not found". The tokenAge calculation with JWT exp is fragile but functional. Overall, this is a well-thought-out auth module that follows the project's conventions.

🤖 Reviewed by claude-opus-4-6-thinking

[ccs-reviewer]

Code Review: PR #526 — feat(cursor): add credential detection, SQLite token extraction, and auth status

---

📋 Summary

This PR adds a Cursor IDE authentication module with credential auto-detection from Cursor's SQLite state database, JWT-based token info extraction, and secure credential persistence. The implementation is well-structured, follows existing codebase patterns (file permissions, getCcsDir(), test isolation), and includes solid test coverage with 448 lines of tests across validation, persistence, auth status, and auto-detection scenarios.

---

✅ Strengths

1. ⭐ Secure by default — Credentials stored with 0o600 file permissions, directories with 0o700, consistent with src/auth/profile-registry.ts and other auth modules (cursor-auth.ts:206,212)
2. ⭐ execFileSync over execSync — Prevents shell injection by passing arguments as an array rather than a single shell string (cursor-auth.ts:57-61)
3. Correct getCcsDir() usage — Properly uses the config-manager utility instead of os.homedir() directly, respecting CCS_HOME for test isolation (cursor-auth.ts:22,194)
4. Robust credential loading — loadCredentials() validates structure AND types of all required fields before casting, parsing unknown safely (cursor-auth.ts:228-250)
5. JWT exp claim priority — checkAuthStatus() correctly prioritizes JWT exp claim over the importedAt heuristic for expiry detection (cursor-auth.ts:278-293)
6. Thorough test isolation — Tests properly save/restore CCS_HOME, create temp directories, and clean up in afterEach (cursor-auth.test.ts:20-45)
7. Edge case tests — Invalid JSON, missing fields, wrong types, invalid dates, JWT with no meaningful claims, multiple deletes (cursor-auth.test.ts:199-260,344-362,387-402)

---

🔍 Observations & Suggestions

1. [Low] JWT exp: 0 treated as falsy — cursor-auth.ts:278

if (userInfo?.exp) {

Uses a truthiness check on exp. If a JWT has exp: 0 (epoch 0 = 1970-01-01), this branch is skipped and the fallback importedAt heuristic may incorrectly report the token as not expired. Practically impossible for a real Cursor token, but for correctness:

if (userInfo?.exp !== undefined) {

2. [Low] PR description mentions CursorDaemonStatus but type is absent

The PR body states types.ts includes CursorDaemonStatus, but this type does not exist in the file or anywhere in the codebase. If it was intentionally deferred, the PR description should be updated for accuracy.

3. [Info] No test for getTokenStoragePath() platform logic

The function has three branches (win32, darwin, linux) but no unit tests. The branching is straightforward, but if platform path logic changes, tests would catch regressions. Consider adding basic assertions for each platform using Object.defineProperty(process, 'platform', ...) (already used in the autoDetect test).

4. [Info] queryStateDb key sanitization scope

The single-quote escaping (key.replace(/'/g, "''")) at cursor-auth.ts:56 is sufficient since keys are hardcoded internal constants (cursorAuth/accessToken, storage.serviceMachineId). If keys ever become user-supplied, a parameterized query approach or stricter validation would be needed. Current implementation is fine for the use case.

5. [Info] autoDetectTokens test coverage is limited

The SQLite query path (queryStateDb) is not tested since it requires a real sqlite3 binary and a populated database. This is understandable for unit tests. Consider adding an integration test or at minimum documenting that this path requires manual verification.

---

🔒 Security Considerations

Check	Status	Notes
Shell injection prevention	✅	execFileSync with args array, not execSync
SQL injection in SQLite queries	✅	Single-quote escaping; keys are hardcoded constants
Credential file permissions	✅	0o600 files, 0o700 directories
No secrets in logs/errors	✅	Error messages contain no token data
Test isolation from real ~/.ccs/	✅	Uses CCS_HOME env var via getCcsDir()
No hardcoded secrets	✅	Clean
JWT parsed without verification	✅	Appropriate — extracting claims, not making auth decisions
Timeout on external process	✅	5000ms timeout on execFileSync

---

📊 Code Quality Checklist

Criterion	Status	Notes
TypeScript strict mode compliant	✅	unknown parsing, type guards, no any
Follows existing codebase patterns	✅	Matches copilot-auth, cliproxy/auth patterns
Test isolation (CLAUDE.md)	✅	CCS_HOME properly used
No emojis in CLI output	✅	No CLI output in this module
Error handling	✅	Graceful null/false returns
Cross-platform consideration	✅	win32/darwin/linux paths; Windows auto-detect blocked with message
YAGNI / KISS compliance	✅	Focused scope, no unnecessary abstractions
Conventional commit format	✅	feat(cursor): prefix

---

💡 Recommendations

Priority	Recommendation
🟢 Low	Fix exp truthiness check at cursor-auth.ts:278 — use !== undefined instead of truthiness to handle theoretical exp: 0
🟢 Low	Update PR description to remove CursorDaemonStatus mention or note it as deferred
🟢 Low	Consider adding unit tests for getTokenStoragePath() platform branching

---

🎯 Overall Assessment

✅ APPROVED

This is a well-implemented authentication module that follows established codebase patterns for credential handling, file security, and test isolation. The code is focused, type-safe, and defensively programmed with appropriate error handling. The test suite is thorough with good edge case coverage. The minor observations above are all low-priority and non-blocking.

---

🤖 Reviewed by claude-opus-4-6-thinking