Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,18 @@ function syncToPath(authPath: string, creds: ClaudeCredentials): void {
}
}
}
auth.anthropic = {
type: "oauth",
access: creds.accessToken,
refresh: creds.refreshToken,
expires: creds.expiresAt,
}
auth.anthropic =
creds.authType === "api"
? {
type: "api",
key: creds.accessToken,
}
: {
type: "oauth",
access: creds.accessToken,
refresh: creds.refreshToken,
expires: creds.expiresAt,
}
const dir = dirname(authPath)
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true, mode: 0o700 })
Expand Down Expand Up @@ -181,6 +187,7 @@ export function parseOAuthResponse(
if (!data.access_token) return null

return {
authType: "oauth",
accessToken: data.access_token,
refreshToken: data.refresh_token ?? currentRefreshToken,
expiresAt: now + (data.expires_in ?? 36_000) * 1000,
Expand Down Expand Up @@ -274,6 +281,7 @@ export function refreshIfNeeded(
if (!target) return null

const creds = target.credentials
if (creds.authType === "api") return creds
if (creds.expiresAt > Date.now() + 60_000) return creds

log("refresh_needed", {
Expand Down
88 changes: 67 additions & 21 deletions src/keychain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,21 @@ import { mkdtemp } from "node:fs/promises"

// Mirrors the parseCredentials logic from keychain.ts for unit testing
function parseCredentials(raw: string): {
authType?: "api" | "oauth"
accessToken: string
refreshToken: string
expiresAt: number
subscriptionType?: string
} | null {
if (raw.startsWith("sk-ant-")) {
return {
authType: "api",
accessToken: raw,
refreshToken: "",
expiresAt: Number.MAX_SAFE_INTEGER,
}
}

let parsed: unknown
try {
parsed = JSON.parse(raw)
Expand Down Expand Up @@ -48,6 +58,7 @@ function parseCredentials(raw: string): {
}

return {
authType: "oauth",
accessToken: creds.accessToken,
refreshToken: creds.refreshToken,
expiresAt: creds.expiresAt,
Expand All @@ -60,25 +71,33 @@ function parseCredentials(raw: string): {

// Mirrors listClaudeKeychainServices regex logic for unit testing
function extractServicesFromDump(output: string): string[] {
const PRIMARY = "Claude Code-credentials"
const PRIMARY = ["Claude Code", "Claude Code-credentials"]
const primarySet = new Set(PRIMARY)
const services: string[] = []
const seen = new Set<string>()

const re = /"Claude Code-credentials(?:-[0-9a-f]+)?"/g
let m = re.exec(output)
while (m !== null) {
const svc = m[0].slice(1, -1)
if (!seen.has(svc)) {
seen.add(svc)
services.push(svc)
const patterns = [
/"Claude Code"/g,
/"Claude Code-credentials(?:-[0-9a-f]+)?"/g,
]
for (const pattern of patterns) {
let m = pattern.exec(output)
while (m !== null) {
const svc = m[0].slice(1, -1)
if (!seen.has(svc)) {
seen.add(svc)
services.push(svc)
}
m = pattern.exec(output)
}
m = re.exec(output)
}

const ordered: string[] = []
if (seen.has(PRIMARY)) ordered.push(PRIMARY)
for (const primary of PRIMARY) {
if (seen.has(primary)) ordered.push(primary)
}
for (const svc of services) {
if (svc !== PRIMARY) ordered.push(svc)
if (!primarySet.has(svc)) ordered.push(svc)
}
return ordered
}
Expand Down Expand Up @@ -110,6 +129,7 @@ describe("parseCredentials", () => {
})
const result = parseCredentials(raw)
assert.ok(result)
assert.equal(result.authType, "oauth")
assert.equal(result.accessToken, "at-123")
assert.equal(result.refreshToken, "rt-456")
assert.equal(result.expiresAt, 1700000000000)
Expand All @@ -124,6 +144,7 @@ describe("parseCredentials", () => {
})
const result = parseCredentials(raw)
assert.ok(result)
assert.equal(result.authType, "oauth")
assert.equal(result.accessToken, "at-789")
assert.equal(result.refreshToken, "rt-012")
assert.equal(result.expiresAt, 1700000000000)
Expand All @@ -137,9 +158,20 @@ describe("parseCredentials", () => {
})
const result = parseCredentials(raw)
assert.ok(result)
assert.equal(result.authType, "oauth")
assert.equal(result.subscriptionType, undefined)
})

it("parses managed api keys from the current Claude Code entry", () => {
const result = parseCredentials("sk-ant-api03-example")
assert.deepEqual(result, {
authType: "api",
accessToken: "sk-ant-api03-example",
refreshToken: "",
expiresAt: Number.MAX_SAFE_INTEGER,
})
})

it("returns null for MCP-only entries", () => {
const raw = JSON.stringify({
mcpOAuth: {
Expand Down Expand Up @@ -215,28 +247,35 @@ attributes:
keychain: "/Users/test/Library/Keychains/login.keychain-db"
version: 512
class: "genp"
attributes:
0x00000007 <blob>="Claude Code"
"svce"<blob>="Claude Code"
keychain: "/Users/test/Library/Keychains/login.keychain-db"
version: 512
class: "genp"
attributes:
0x00000007 <blob>="Claude Code-credentials"
"svce"<blob>="Claude Code-credentials"
`

it("discovers all Claude Code-credentials* services", () => {
it("discovers both current and legacy Claude Code services", () => {
const services = extractServicesFromDump(SAMPLE_DUMP)
assert.ok(services.includes("Claude Code"))
assert.ok(services.includes("Claude Code-credentials"))
assert.ok(services.includes("Claude Code-credentials-e8dc196c"))
assert.ok(services.includes("Claude Code-credentials-b28bbb7c"))
assert.equal(services.length, 3)
assert.equal(services.length, 4)
})

it("puts the primary service first", () => {
assert.equal(
extractServicesFromDump(SAMPLE_DUMP)[0],
"Claude Code-credentials",
)
it("puts the current and legacy primary services first", () => {
const services = extractServicesFromDump(SAMPLE_DUMP)
assert.equal(services[0], "Claude Code")
assert.equal(services[1], "Claude Code-credentials")
})

it("deduplicates entries that appear twice (svce and blob line)", () => {
const services = extractServicesFromDump(SAMPLE_DUMP)
assert.equal(services.filter((s) => s === "Claude Code").length, 1)
assert.equal(
services.filter((s) => s === "Claude Code-credentials").length,
1,
Expand All @@ -251,9 +290,13 @@ attributes:
const dump = `
0x00000007 <blob>="Some Other Service"
"svce"<blob>="Some Other Service"
0x00000007 <blob>="Claude Code"
0x00000007 <blob>="Claude Code-credentials"
`
assert.deepEqual(extractServicesFromDump(dump), ["Claude Code-credentials"])
assert.deepEqual(extractServicesFromDump(dump), [
"Claude Code",
"Claude Code-credentials",
])
})

it("returns empty array for a dump with no Claude Code entries", () => {
Expand Down Expand Up @@ -281,11 +324,13 @@ attributes:
it("handles a dump where primary service appears after suffixed ones", () => {
const dump = `
"svce"<blob>="Claude Code-credentials-b28bbb7c"
"svce"<blob>="Claude Code"
"svce"<blob>="Claude Code-credentials"
`
const services = extractServicesFromDump(dump)
assert.equal(services[0], "Claude Code-credentials")
assert.equal(services[1], "Claude Code-credentials-b28bbb7c")
assert.equal(services[0], "Claude Code")
assert.equal(services[1], "Claude Code-credentials")
assert.equal(services[2], "Claude Code-credentials-b28bbb7c")
})

it("handles all five real-world suffixes from a populated keychain", () => {
Expand Down Expand Up @@ -391,6 +436,7 @@ describe("credentials file fallback", () => {
)
const result = readCredentialsFile(credPath)
assert.deepEqual(result, {
authType: "oauth",
accessToken: "file-at",
refreshToken: "file-rt",
expiresAt: 1700000000000,
Expand Down
43 changes: 31 additions & 12 deletions src/keychain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { join } from "node:path"
import { log } from "./logger.ts"

export interface ClaudeCredentials {
authType?: "api" | "oauth"
accessToken: string
refreshToken: string
expiresAt: number
Expand All @@ -17,9 +18,23 @@ export interface ClaudeAccount {
credentials: ClaudeCredentials
}

const PRIMARY_SERVICE = "Claude Code-credentials"
const PRIMARY_SERVICES = ["Claude Code", "Claude Code-credentials"] as const
const PRIMARY_SERVICE_SET: ReadonlySet<string> = new Set(PRIMARY_SERVICES)
const SERVICE_PATTERNS = [
/"Claude Code"/g,
/"Claude Code-credentials(?:-[0-9a-f]+)?"/g,
]
Comment on lines +23 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Module-level stateful regex objects

SERVICE_PATTERNS stores regex literals with the g flag as a module-level constant. g-flag regexes carry mutable lastIndex state — each .exec() call advances it, and it only resets to 0 when .exec() returns null. The current while (m !== null) loop always exhausts matches before exiting, so lastIndex is naturally reset and subsequent calls to extractServicesFromDump start cleanly. But if anyone adds a break, a thrown exception, or an early return inside the loop body, the stranded lastIndex would cause the next call to start mid-string and silently miss earlier entries. Using String.prototype.matchAll() or constructing a new RegExp(...) inside extractServicesFromDump on each call would eliminate the shared-state risk entirely.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/keychain.ts
Line: 23-26

Comment:
**Module-level stateful regex objects**

`SERVICE_PATTERNS` stores regex literals with the `g` flag as a module-level constant. `g`-flag regexes carry mutable `lastIndex` state — each `.exec()` call advances it, and it only resets to 0 when `.exec()` returns `null`. The current `while (m !== null)` loop always exhausts matches before exiting, so `lastIndex` is naturally reset and subsequent calls to `extractServicesFromDump` start cleanly. But if anyone adds a `break`, a thrown exception, or an early return inside the loop body, the stranded `lastIndex` would cause the next call to start mid-string and silently miss earlier entries. Using `String.prototype.matchAll()` or constructing a `new RegExp(...)` inside `extractServicesFromDump` on each call would eliminate the shared-state risk entirely.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Cursor Fix in Claude Code Fix in Codex


function parseCredentials(raw: string): ClaudeCredentials | null {
if (raw.startsWith("sk-ant-")) {
return {
authType: "api",
accessToken: raw,
refreshToken: "",
expiresAt: Number.MAX_SAFE_INTEGER,
}
}

let parsed: unknown
try {
parsed = JSON.parse(raw)
Expand Down Expand Up @@ -63,6 +78,7 @@ function parseCredentials(raw: string): ClaudeCredentials | null {
})

return {
authType: "oauth",
accessToken: creds.accessToken,
refreshToken: creds.refreshToken,
expiresAt: creds.expiresAt,
Expand Down Expand Up @@ -145,26 +161,29 @@ function listClaudeKeychainServices(): string[] {
const services: string[] = []
const seen = new Set<string>()

const re = /"Claude Code-credentials(?:-[0-9a-f]+)?"/g
let m = re.exec(dump)
while (m !== null) {
const svc = m[0].slice(1, -1)
if (!seen.has(svc)) {
seen.add(svc)
services.push(svc)
for (const pattern of SERVICE_PATTERNS) {
let m = pattern.exec(dump)
while (m !== null) {
const svc = m[0].slice(1, -1)
if (!seen.has(svc)) {
seen.add(svc)
services.push(svc)
}
m = pattern.exec(dump)
}
m = re.exec(dump)
}

const ordered: string[] = []
if (seen.has(PRIMARY_SERVICE)) ordered.push(PRIMARY_SERVICE)
for (const primaryService of PRIMARY_SERVICES) {
if (seen.has(primaryService)) ordered.push(primaryService)
}
for (const svc of services) {
if (svc !== PRIMARY_SERVICE) ordered.push(svc)
if (!PRIMARY_SERVICE_SET.has(svc)) ordered.push(svc)
}
log("keychain_list", { servicesFound: ordered })
return ordered
} catch {
return [PRIMARY_SERVICE]
return [...PRIMARY_SERVICES]
}
}

Expand Down
Loading