Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
188 changes: 188 additions & 0 deletions src/keychain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { join } from "node:path"
import { tmpdir } from "node:os"
import {
buildAccountLabels,
deriveKeychainDescription,
parseKeychainDump,
updateCredentialBlob,
writeBackCredentials,
} from "./keychain.ts"
Expand Down Expand Up @@ -498,6 +500,192 @@ describe("updateCredentialBlob", () => {
})
})

describe("parseKeychainDump", () => {
// Mirrors the real format emitted by `security dump-keychain` on macOS.
// Includes: a primary Claude entry, a renamed/comment-tagged entry whose
// 0x00000007 label and `svce` differ, a non-Claude item that must be
// ignored, and a mix of <NULL> attributes.
const KEYCHAIN_DUMP = `keychain: "/Users/test/Library/Keychains/login.keychain-db"
version: 512
class: "genp"
attributes:
0x00000007 <blob>="Claude Code-credentials"
0x00000008 <blob>=<NULL>
"acct"<blob>="testuser"
"cdat"<timedate>=0x32303230303130313030303030305A00 "20200101000000Z\\000"
"crtr"<uint32>=<NULL>
"desc"<blob>=<NULL>
"icmt"<blob>=<NULL>
"mdat"<timedate>=0x32303230303130313030303030305A00 "20200101000000Z\\000"
"svce"<blob>="Claude Code-credentials"
"type"<uint32>=<NULL>
keychain: "/Users/test/Library/Keychains/login.keychain-db"
version: 512
class: "genp"
attributes:
0x00000007 <blob>="Claude Code-credentials-12345678"
0x00000008 <blob>=<NULL>
"acct"<blob>="testuser"
"cdat"<timedate>=0x32303230303130313030303030305A00 "20200101000000Z\\000"
"desc"<blob>=<NULL>
"icmt"<blob>="Jack Test"
"svce"<blob>="Claude Code-credentials-12345678"
"type"<uint32>=<NULL>
keychain: "/Users/test/Library/Keychains/login.keychain-db"
version: 512
class: "genp"
attributes:
0x00000007 <blob>="Example"
"acct"<blob>="some-db"
"desc"<blob>="application password"
"icmt"<blob>=<NULL>
"svce"<blob>="com.example.Example"
keychain: "/Users/test/Library/Keychains/login.keychain-db"
version: 512
class: "genp"
attributes:
0x00000007 <blob>="Claude Code-credentials-23456789"
"acct"<blob>="testuser"
"desc"<blob>=<NULL>
"icmt"<blob>=<NULL>
"svce"<blob>="Claude Code-credentials-23456789"
"type"<uint32>=<NULL>
`

it("parses every genp item in the dump", () => {
const entries = parseKeychainDump(KEYCHAIN_DUMP)
assert.equal(entries.length, 4)
})

it("extracts the svce attribute as service", () => {
const services = parseKeychainDump(KEYCHAIN_DUMP).map((e) => e.service)
assert.deepEqual(services, [
"Claude Code-credentials",
"Claude Code-credentials-12345678",
"com.example.Example",
"Claude Code-credentials-23456789",
])
})

it("extracts the 0x00000007 attribute as label", () => {
const entries = parseKeychainDump(KEYCHAIN_DUMP)
assert.equal(entries[0].label, "Claude Code-credentials")
assert.equal(entries[1].label, "Claude Code-credentials-12345678")
assert.equal(entries[2].label, "Example")
})

it("extracts the icmt attribute as comment", () => {
const entries = parseKeychainDump(KEYCHAIN_DUMP)
assert.equal(entries[1].comment, "Jack Test")
})

it("extracts the acct attribute as account", () => {
const entries = parseKeychainDump(KEYCHAIN_DUMP)
assert.equal(entries[0].account, "testuser")
assert.equal(entries[2].account, "some-db")
})

it("converts <NULL> attribute values to undefined", () => {
const entries = parseKeychainDump(KEYCHAIN_DUMP)
assert.equal(entries[0].comment, undefined)
assert.equal(entries[0].description, undefined)
assert.equal(entries[3].comment, undefined)
})

it("extracts a non-null desc attribute", () => {
const entries = parseKeychainDump(KEYCHAIN_DUMP)
assert.equal(entries[2].description, "application password")
})

it("skips items that have no svce attribute", () => {
const dump = `class: "genp"
attributes:
"acct"<blob>="orphan"
"svce"<blob>=<NULL>
class: "genp"
attributes:
"acct"<blob>="ok"
"svce"<blob>="real"
`
const entries = parseKeychainDump(dump)
assert.equal(entries.length, 1)
assert.equal(entries[0].service, "real")
})

it("returns an empty array for an empty dump", () => {
assert.deepEqual(parseKeychainDump(""), [])
})

it("returns an empty array when no genp items are present", () => {
assert.deepEqual(
parseKeychainDump(`keychain: "x"\nversion: 512\nclass: "inet"\n`),
[],
)
})

it("captures items whose svce differs from the human label", () => {
// The 12345678 item is invisible to the legacy hex-only regex but visible
// through svce-based parsing — that's the whole reason we rewrote it.
const entries = parseKeychainDump(KEYCHAIN_DUMP)
const acc23456789 = entries.find(
(e) => e.service === "Claude Code-credentials-23456789",
)
assert.ok(acc23456789, "23456789 entry should be parsed")
assert.equal(acc23456789.label, "Claude Code-credentials-23456789")
assert.equal(acc23456789.comment, undefined)
})
})

describe("deriveKeychainDescription", () => {
it("returns the comment when present", () => {
assert.equal(
deriveKeychainDescription({
service: "Claude Code-credentials-12345678",
comment: "Claude Sub 12345678",
}),
"Claude Sub 12345678",
)
})

it("trims surrounding whitespace from the comment", () => {
assert.equal(
deriveKeychainDescription({
service: "svc",
comment: " spaced description ",
}),
"spaced description",
)
})

it("returns undefined for an empty or whitespace-only comment", () => {
assert.equal(
deriveKeychainDescription({ service: "svc", comment: "" }),
undefined,
)
assert.equal(
deriveKeychainDescription({ service: "svc", comment: " " }),
undefined,
)
})

it("returns undefined when comment is absent, ignoring label and desc", () => {
// We deliberately do NOT fall back to label or desc — the icmt field is
// the only user-meaningful annotation in practice.
assert.equal(
deriveKeychainDescription({
service: "Claude Code-credentials-12345678",
label: "Claude Code-credentials-12345678",
description: "application password",
}),
undefined,
)
})

it("returns undefined for a fully empty entry", () => {
assert.equal(deriveKeychainDescription({ service: "svc" }), undefined)
})
})

describe("writeBackCredentials (file source)", () => {
it("reads, updates, and writes back credentials to file", async () => {
const originalHome = process.env.HOME
Expand Down
124 changes: 98 additions & 26 deletions src/keychain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ export interface ClaudeAccount {
label: string
source: string
credentials: ClaudeCredentials
description?: string
}

interface KeychainEntry {
service: string
label?: string
comment?: string
description?: string
account?: string
}

const PRIMARY_SERVICE = "Claude Code-credentials"
Expand Down Expand Up @@ -135,41 +144,80 @@ function readKeychainService(serviceName: string): string | null {
}
}

function listClaudeKeychainServices(): string[] {
export function parseKeychainDump(dump: string): KeychainEntry[] {
const entries: KeychainEntry[] = []
// Each item block begins with `class: "genp"` (or other class).
const blocks = dump.split(/^class:\s*"genp"\s*$/m).slice(1)
for (const block of blocks) {
// Stop at the next `class:` or `keychain:` boundary.
const end = block.search(/^(?:class:|keychain:)/m)
const body = end === -1 ? block : block.slice(0, end)

const read = (re: RegExp): string | undefined => {
const m = re.exec(body)
if (!m) return undefined
const v = m[1]
return v === "<NULL>" ? undefined : v
}

const service = read(/^\s*"svce"<blob>="([^"]*)"\s*$/m)
if (!service) continue

entries.push({
service,
label: read(/^\s*0x00000007 <blob>="([^"]*)"\s*$/m),
comment: read(/^\s*"icmt"<blob>="([^"]*)"\s*$/m),
description: read(/^\s*"desc"<blob>="([^"]*)"\s*$/m),
account: read(/^\s*"acct"<blob>="([^"]*)"\s*$/m),
})
}
return entries
}

export function deriveKeychainDescription(
entry: KeychainEntry,
): string | undefined {
const comment = entry.comment?.trim()
return comment ? comment : undefined
}

function listClaudeKeychainEntries(): KeychainEntry[] {
try {
const dump = execSync("security dump-keychain", {
timeout: 5000,
maxBuffer: 1024 * 1024 * 10, // 10 MB
encoding: "utf-8",
})

const services: string[] = []
const seen = new Set<string>()
const all = parseKeychainDump(dump)
const claude = all.filter((e) => e.service.startsWith(PRIMARY_SERVICE))

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)
}
m = re.exec(dump)
// Dedup by service while preserving order, primary first.
const byService = new Map<string, KeychainEntry>()
for (const e of claude) {
if (!byService.has(e.service)) byService.set(e.service, e)
}

const ordered: string[] = []
if (seen.has(PRIMARY_SERVICE)) ordered.push(PRIMARY_SERVICE)
for (const svc of services) {
if (svc !== PRIMARY_SERVICE) ordered.push(svc)
const ordered: KeychainEntry[] = []
const primary = byService.get(PRIMARY_SERVICE)
if (primary) ordered.push(primary)
for (const [svc, e] of byService) {
if (svc !== PRIMARY_SERVICE) ordered.push(e)
}
log("keychain_list", { servicesFound: ordered })

log("keychain_list", {
servicesFound: ordered.map((e) => e.service),
withDescription: ordered
.filter((e) => deriveKeychainDescription(e))
.map((e) => e.service),
})
return ordered
} catch (err) {
log("keychain_list", {
error: "Failed to list keychain services",
message: err instanceof Error ? err.message : String(err),
})
return [PRIMARY_SERVICE]
return [{ service: PRIMARY_SERVICE }]
}
}

Expand Down Expand Up @@ -216,28 +264,52 @@ export function readAllClaudeAccounts(): ClaudeAccount[] {
return [{ label, source: "file", credentials: creds }]
}

const services = listClaudeKeychainServices()
const rawAccounts: Array<{ source: string; credentials: ClaudeCredentials }> =
[]
const entries = listClaudeKeychainEntries()
const rawAccounts: Array<{
source: string
credentials: ClaudeCredentials
description?: string
}> = []

for (const svc of services) {
const raw = readKeychainService(svc)
for (const entry of entries) {
const raw = readKeychainService(entry.service)
if (!raw) continue
const creds = parseCredentials(raw)
if (!creds) continue
rawAccounts.push({ source: svc, credentials: creds })
rawAccounts.push({
source: entry.service,
credentials: creds,
description: deriveKeychainDescription(entry),
})
}

if (rawAccounts.length === 0) {
const creds = readCredentialsFile()
if (creds) rawAccounts.push({ source: "file", credentials: creds })
}

const labels = buildAccountLabels(rawAccounts.map((a) => a.credentials))
// Build base labels (e.g. "Claude Pro") without disambiguating numbers.
const baseLabels = rawAccounts.map((a) => {
const sub = a.credentials.subscriptionType
if (sub) return `Claude ${sub.charAt(0).toUpperCase() + sub.slice(1)}`
return "Claude"
})

// Only fall back to numeric suffixes for entries without a Keychain comment.
const numberedLabels = buildAccountLabels(
rawAccounts.map((a) => a.credentials),
)

return rawAccounts.map((a, i) => ({
label: labels[i],
// Append the user-set Keychain comment after the plan so the UI shows
// e.g. "Claude Pro - Claude Sub Duncan". Without a comment, fall back to
// the disambiguated label (e.g. "Claude Max 2").
label: a.description
? `${baseLabels[i]} - ${a.description}`
: numberedLabels[i],
source: a.source,
credentials: a.credentials,
description: a.description,
}))
Comment on lines +291 to 313

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Misleading numeric suffix when described and undescribed accounts are mixed

numberedLabels is built from all raw accounts, including those that already have a Keychain description. That means when two "Claude Pro" accounts exist and only one has a comment, buildAccountLabels counts both and produces ["Claude Pro 1", "Claude Pro 2"]. The described account consumes slot 0 silently, so the undescribed one ends up labelled "Claude Pro 2" — implying a "Claude Pro 1" that never appears in the UI.

Concrete case: accounts [{Pro, desc="Work"}, {Pro, no desc}] → user sees "Claude Pro - Work" and "Claude Pro 2". Only passing credentials from undescribed accounts to buildAccountLabels (then stitching the labels back at the right indices) would produce the correct "Claude Pro" (or "Claude Pro 1" / "Claude Pro 2" when multiple undescribed entries share a plan).

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

Comment:
**Misleading numeric suffix when described and undescribed accounts are mixed**

`numberedLabels` is built from **all** raw accounts, including those that already have a Keychain description. That means when two "Claude Pro" accounts exist and only one has a comment, `buildAccountLabels` counts both and produces `["Claude Pro 1", "Claude Pro 2"]`. The described account consumes slot 0 silently, so the undescribed one ends up labelled `"Claude Pro 2"` — implying a "Claude Pro 1" that never appears in the UI.

Concrete case: accounts `[{Pro, desc="Work"}, {Pro, no desc}]` → user sees `"Claude Pro - Work"` and `"Claude Pro 2"`. Only passing credentials from *undescribed* accounts to `buildAccountLabels` (then stitching the labels back at the right indices) would produce the correct `"Claude Pro"` (or `"Claude Pro 1"` / `"Claude Pro 2"` when multiple undescribed entries share a plan).

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

Fix in Cursor Fix in Claude Code Fix in Codex

}

Expand Down
Loading