claude-code/docs/jira/CROSS-AUDIT-MULTI-AUTH-2026-05-06.md

Cross-Audit: Multi-Auth PR-1/PR-2/PR-3/PR-4

• Date: 2026-05-06
• Range: HEAD~9..HEAD (commits a82de394, 656e6bc5, 70756362, 26634121, 633a425b, ffa33963, ca004a17, 69df7be2)
• Scope: ~5524 insertions / ~131 deletions across 59 files
• Method: Read-only static review; no source files modified
• Files audited: 28 source files (18 prod + 10 test, plus 4 P2 client diffs)

---

Summary table (dimension x severity)

Dim	CRITICAL	HIGH	MEDIUM	LOW	Total
A. Silent failures	0	1	3	1	5
B. Resource leaks	0	0	1	1	2
C. Concurrency / race	0	3	2	0	5
D. Input validation / overflow	0	2	4	1	7
E. Path traversal / security	1	1	2	1	5
F. Crypto correctness	0	2	1	0	3
G. Error matrix / UX text	0	0	2	2	4
H. Duplication	0	0	3	0	3
I. Test coverage gap	0	1	2	0	3
J. Performance / edge	0	0	2	1	3
TOTAL	1	10	22	7	40

---

A. Silent failures

A1. HIGH — loadProviders() corrupt file silently falls back to defaults

File: src/services/providerRegistry/loader.ts:96-112 The Zod-failure / JSON-parse-failure paths only call logError() and return [...DEFAULT_PROVIDERS]. A user who edited providers.json and broke it will see their custom providers silently disappear with only a stderr log line. They will assume their config works. Fix: Surface a one-line warning to the user-facing channel (or the /providers list view should render a "config error" banner using existsSync(filePath) && parseFailed).

// In ProviderView when invoked, also surface load errors:
const loadResult = loadProvidersWithDiagnostic()  // {providers, error?: string}

A2. MEDIUM — readVaultFile() swallows JSON parse error

File: src/services/localVault/store.ts:178-180

} catch {
  return {}
}

A corrupt local-vault.enc.json returns {}, masking data loss. getSecret(...) returns null instead of erroring. User thinks key was never set. Fix: Differentiate ENOENT (return {}) from JSON-parse-error (throw LocalVaultDecryptionError("vault file corrupt — restore from backup")).

A3. MEDIUM — tryKeychain.list() swallows corrupt index

File: src/services/localVault/keychain.ts:93-96 A corrupt __index__ JSON returns []. New entries via _addToIndex will rebuild the index losing all references to existing keys (in keychain but unindexed, undeletable via delete). Fix: On parse failure, throw KeychainUnavailableError("index corrupt; reset via …") so caller can fall back rather than data-stranding.

A4. MEDIUM — chmodSync failure is logged but flow continues with insecure file

File: src/services/localVault/store.ts:83-93

try { chmodSync(passphraseFile, 0o600) } catch { logError(...) }

On Windows the file is written with default ACL (often readable by all users in same group). logError is informational — the user has no way to act on it before encryption proceeds. Fix: On Windows, recommend explicit ACL via icacls in the warning, OR strongly recommend CLAUDE_LOCAL_VAULT_PASSPHRASE env var as primary path.

A5. LOW — sendEventToRemoteSession returns false on network/auth error

File: src/utils/teleport/api.ts:442-445 (pre-existing pattern, not new but adjacent to PR scope) — not in PR diff, excluded from finding count.

---

B. Resource leaks

B1. MEDIUM — cipher/decipher not explicitly disposed; AES key Buffer not zeroed

File: src/services/localVault/store.ts:121-161 createCipheriv / createDecipheriv return objects that hold internal state. Node will GC them, but the key256: Buffer derived from passphrase remains in heap until GC. For a long-running process, multiple calls to setSecret keep these in memory. Fix: After encrypt/decrypt, key256.fill(0) to zero out the derived key. While JS GC makes this best-effort, it limits the window.

try {
  const enc = encrypt(value, key256)
  // ...
} finally {
  key256.fill(0)
}

B2. LOW — _resetKeychainModuleCache is exported but only useful for tests

File: src/services/localVault/keychain.ts:54-56 Test-only export pollutes public API surface. Use a __tests__/ re-export or export internal.

---

C. Concurrency / race

C1. HIGH — localVault/store.ts setSecret is non-atomic (TOCTOU on read-modify-write of vault file)

File: src/services/localVault/store.ts:212-216

const vaultData = await readVaultFile()      // ← read
vaultData[key] = encrypt(value, key256)
await writeVaultFile(vaultData)              // ← write  (lost-update on concurrent setSecret)

Two parallel setSecret('a', 'A') and setSecret('b', 'B') calls each read the same baseline; whichever writes last wins, dropping the other. Not theoretical — /local-vault set from two terminals or Promise.all([setSecret(...), setSecret(...)]) triggers it. Fix: Write to <file>.tmp then renameSync (atomic on POSIX), AND wrap with an in-process mutex (e.g. proper-lockfile or a queue). Cross-process safety requires file locking.

C2. HIGH — multiStore.ts setEntry is non-atomic (no .tmp + rename)

File: src/services/SessionMemory/multiStore.ts:106

writeFileSync(entryPath, value, 'utf8')

A crash mid-write leaves a half-written .md file. A reader (getEntry) sees truncated content. Fix: writeFileSync(tmp, value); renameSync(tmp, entryPath).

C3. HIGH — loader.ts saveProviders() overwrites without locking; lost-update race

File: src/services/providerRegistry/loader.ts:148-178 Same pattern as C1. Two /providers add invocations interleave: each loads current → adds its entry → writes. One loses. Fix: Atomic write (.tmp + rename) plus advisory file lock. /providers add from REPL is rarely concurrent, but spec allows scripted use.

C4. MEDIUM — _addToIndex / _removeFromIndex race

File: src/services/localVault/keychain.ts:99-114 existing = await this.list() then setPassword(JSON.stringify([...existing, account])). Concurrent set/delete on different keys race the index. Fix: Wrap index ops in a process-level Mutex (Bun has Bun.lock or use a small async-lock).

C5. MEDIUM — getOrCreatePassphrase may double-write on first run

File: src/services/localVault/store.ts:62-103 Two parallel first-run setSecret calls each see !existsSync(passphraseFile), both randomBytes(32) then both writeFileSync — different passphrases. The second wins; the first call's encrypted record is now undecryptable forever. Fix: Use writeFileSync(file, generated, { flag: 'wx' }) (exclusive create); on EEXIST re-read from file.

---

D. Input validation / overflow

D1. HIGH — setSecret(key, value) has no upper bound on value size

File: src/services/localVault/store.ts:194-217 A 100 MB value is loaded into memory, encrypted (~100 MB cipher buffer), JSON-stringified (~200 MB hex), then written. OS keychain typically rejects > 4 KB but the file fallback path accepts unlimited input → OOM on cheap machines. Fix: Reject value.length > 64 * 1024 with a clear error before encryption.

D2. HIGH — multiStore.setEntry has no upper bound on value size

File: src/services/SessionMemory/multiStore.ts:98-107 Same problem; entries are user-facing notes but nothing prevents writing a 1 GB string. Fix: Cap at 1 MB; document in parseArgs.ts USAGE.

D3. MEDIUM — parseLocalVaultArgs set <key> <value> keys can be --reveal or any flag

File: src/commands/local-vault/parseArgs.ts:39-54 set --reveal foo is parsed as key='--reveal', value='foo' — accepted. Probably intended to error. Fix: Validate key does not start with - (reserved for flags).

D4. MEDIUM — parseLocalVaultArgs value-extraction breaks on key containing regex special chars or repeating substring

File: src/commands/local-vault/parseArgs.ts:46

const rest = trimmed.slice(trimmed.indexOf(key) + key.length).trim()

If key = 'set' (someone tries set set value) or key has the same substring as the subcmd, indexOf returns the subcmd position, slicing wrongly. Same fragility in parseLocalMemoryArgs:68 (uses two-arg indexOf to mitigate but still string-search). Fix: Use tokens.slice(2).join(' ') for value, not substring math.

D5. MEDIUM — prepareWorkspaceApiRequest reveals first 13 chars of malformed key

File: src/utils/teleport/api.ts:199

`got prefix "${apiKey.slice(0, 13)}..."`

If a user pastes the wrong secret (e.g., a real OpenAI sk-proj-… or AWS key), the first 13 chars include high-entropy bits of the actual secret. Logged in error → potentially copied into bug report. Fix: Reveal at most first 4 chars: apiKey.slice(0, 4).

D6. MEDIUM — parseLocalMemoryArgs store <store> <key> <value> value-extraction same fragility

File: src/commands/local-memory/parseArgs.ts:68-69 indexOf(key, ...) is fragile if key matches store name or appears earlier. Fix: tokens.slice(3).join(' ').

D7. LOW — parseProviderArgs: use cerebras extra args silently ignores trailing tokens

File: src/commands/provider/parseArgs.ts:45-46 "Take only the first token as the id" — but does not warn user about extra tokens that may have been a typo. Fix: If rest.split(/\s+/).length > 1, return invalid with hint.

---

E. Path traversal / security

E1. CRITICAL — multiStore.setEntry allows store=..\..\X via Windows path separator regex gap

File: src/services/SessionMemory/multiStore.ts:34-46

function getEntryPath(store: string, key: string): string {
  const safeKey = key.replace(/[/\\]/g, '_')   // ← key sanitized
  return join(getStoreDir(store), `${safeKey}.md`)  // ← store NOT sanitized here
}
function validateStoreName(store: string): void {
  if (!store || /[/\\]/.test(store) || store.startsWith('.')) { ... }  // ← rejects '../' but...
}

The validator rejects / \\ and leading ., BUT does not reject null bytes (store='x\0../etc'), nor does it reject Windows drive prefixes (store='C:foo' → join(base, 'C:foo') resolves to C:foo on Windows, escaping base!), nor URL-encoded sequences. Also: store='foo\u0000' truncates the path on certain Node versions exposing ~/.claude/local-memory/foo. Importantly key regex only strips / and \\ — does not reject .. segments after sanitisation: key='..' → safeKey='..' → entry path …/store/...md (no escape due to .md suffix), but key='\0' → safeKey='_' (ok). The store-name check is the bigger risk. Repro: /local-memory store C:hack k v on Windows → writes to C:hack/k.md (workspace-relative, escapes ~/.claude/local-memory/). Fix: Add to validator: reject \0, reject :, reject .., normalize via path.basename(store) and assert basename(store) === store.

function validateStoreName(store: string): void {
  if (!store) throw new Error('empty')
  if (store !== path.basename(store)) throw new Error('path-like')
  if (/[/\\\0:]/.test(store)) throw new Error('illegal char')
  if (store.startsWith('.') || store === '..') throw new Error('reserved')
  if (store.length > 255) throw new Error('too long')
}

E2. HIGH — assertWorkspaceHost URL parse permits https://api.anthropic.com@evil.com/ (legacy URL credentials)

File: src/services/auth/hostGuard.ts:25-42 new URL('https://api.anthropic.com@evil.com/x').hostname → 'evil.com' so this is caught. BUT: callers construct URLs by string concat: ${BASE_API_URL}/v1/agents. If BASE_API_URL is influenced by env (e.g., ANTHROPIC_BASE_URL override or test override), a misconfiguration like https://api.anthropic.com.evil.com would be caught. So hostname !== 'api.anthropic.com' is sufficient but relies on BASE_API_URL always being trustworthy. There is no audit of where getOauthConfig().BASE_API_URL comes from in this layer. Fix: Document that BASE_API_URL MUST NOT be user-controllable for workspace clients. Add a unit test that asserts assertWorkspaceHost('https://api.anthropic.com.evil.com/') throws (currently untested per hostGuard.test.ts).

E3. MEDIUM — getAuthStatus.maskApiKey leaks last 2 chars of short keys

File: src/commands/login/getAuthStatus.ts:82-87 For a 14-char malformed key (e.g. user pasted only the prefix), preview shows sk-a...3- (14 chars) — 6 of 14 chars exposed (43%). Fix: If len < 20, show [redacted] (N chars) only.

E4. MEDIUM — loader.saveProviders round-trips full provider config through JSON.stringify for diff check

File: src/services/providerRegistry/loader.ts:170

if (defaultEntry && JSON.stringify(defaultEntry) !== JSON.stringify(p)) { ... }

Key-order in spread {...p} vs DEFAULT_PROVIDERS matters — JSON.stringify is order-sensitive. A semantically equivalent override that has different key order writes spuriously. Not a security issue but causes file churn / spurious diffs. Fix: Compare by sorted keys or use a deep-equal helper.

E5. LOW — console.warn for new passphrase file leaks file path to terminal log capture

File: src/services/localVault/store.ts:95-100 The path itself isn't sensitive but console.warn may end up in shell history or session capture — generally logError is preferred for consistency. Fix: Use logError like elsewhere in the file, or document that this is a one-time first-run warning by design.

---

F. Crypto correctness

F1. HIGH — Key derivation uses single SHA-256 of passphrase (not PBKDF2/scrypt/argon2)

File: src/services/localVault/store.ts:56-60

return createHash('sha256').update(passphrase).digest()

Comment claims this is "intentionally simple" because file is on local FS. However:

• The auto-generated passphrase is 64 hex = 256 bits of entropy, which IS secure under SHA-256.
• The user-provided CLAUDE_LOCAL_VAULT_PASSPHRASE env var passphrase may be a low-entropy human-memorable string (mypass123). With SHA-256 (no salt, no work factor), brute force is trivial if attacker steals the file. Fix: Use scryptSync(passphrase, salt, 32) with per-vault random salt stored alongside the encrypted blob. This is industry-standard for password-derived keys.

F2. HIGH — No salt: same passphrase → same key for every file ever

File: src/services/localVault/store.ts:56-60 Combined with F1, an attacker who compromises one vault file can pre-compute a rainbow table for common passphrases that works for ALL users with the same passphrase. Fix: Generate salt = randomBytes(16) on first encryption, store at top of vault file, use scrypt(pass, salt, 32).

F3. MEDIUM — IV is per-record, but no associated-data (AAD) binding

File: src/services/localVault/store.ts:119-133 GCM with no AAD means an attacker who can swap encrypted records (e.g., cross-user swap on shared filesystem) gets a successful decrypt with valid auth tag for the wrong key. Less of a real-world concern but plain best practice. Fix: cipher.setAAD(Buffer.from(key)) — bind the entry-key into the auth tag so swapping records fails decryption.

---

G. Error matrix / UX text

G1. MEDIUM — prepareWorkspaceApiRequest error mentions "Subscription OAuth … cannot reach these endpoints" — confusing for first-time users

File: src/utils/teleport/api.ts:191-202 The error implies user did something wrong; really they just don't have a workspace key yet. PR-4 adds a nice setup guide in WorkspaceKeyInstructions UI but the API-layer error is shown for non-/login paths. Fix: Refer the user to /login to see setup instructions: … run /login to see how to enable workspace endpoints.

G2. MEDIUM — 4 P2 clients duplicate identical 401/403/404/429 messages with copy-paste; one off-by-one

Files: agentsApi.ts:80-98, vaultsApi.ts:114-138, memoryStoresApi.ts, skillsApi.ts agents: no 429 handler; vaults/memory/skills: have 429 handler. Inconsistent UX. Fix: Extract classifyWorkspaceApiError(err, resourceName, id?) to one helper.

G3. LOW — switchProvider warning is plain text; user sees it once via logError then forgets

File: src/services/providerRegistry/switcher.ts:45 assertNoAnthropicEnvForOpenAI() only logs to stderr. The CLI render of /providers use cerebras does not surface this warning to the Ink view. Fix: switchProvider() should include the warning in result.warnings rather than relying on side-channel logging.

G4. LOW — LocalVaultDecryptionError message says "wrong passphrase or tampered data" but does not direct user to recovery

File: src/services/localVault/store.ts:158-160 Fix: Append: Restore from your backup of ~/.claude/.local-vault-passphrase, or delete ~/.claude/local-vault.enc.json to reset (DESTROYS ALL SECRETS).

---

H. Duplication

H1. MEDIUM — 4× buildHeaders(), classifyError(), withRetry(), parseRetryAfterMs(), sanitizeId() duplicated across vaultsApi/agentsApi/memoryStoresApi/skillsApi

Files: src/commands/{vault,agents-platform,memory-stores,skill-store}/*Api.ts Each file has its own class XxxApiError, identical withRetry body (60+ lines), identical parseRetryAfterMs. Total duplication ~400 lines. Fix: Extract src/services/auth/workspaceApiClient.ts exporting createWorkspaceClient(resourcePath, betaHeader) returning { list, get, post, archive, withRetry, classifyError }.

H2. MEDIUM — 6 commands (vault, memory-stores, agents-platform, skill-store, local-vault, local-memory, provider) all share parseArgs / launch / View shape

Each implements ~60 lines of parseArgs.ts, ~120 lines of launch*.tsx, ~120 lines of View.tsx. Fix: Add src/commands/_shared/launchCommand.ts taking a { parse, dispatch, render } triple — cuts boilerplate in half.

H3. MEDIUM — sanitizeId defined identically in 4 P2 client files

Fix: Move to src/services/auth/sanitize.ts.

---

I. Test coverage gap

I1. HIGH — No test asserts secret value never appears in any log stream

Files: src/services/localVault/__tests__/*.test.ts, src/commands/local-vault/__tests__/*.test.ts The test suite has happy-path round-trip (encrypt → decrypt = original) but no assertion like:

expect(logErrorMock.mock.calls.flat().join(' ')).not.toContain(SECRET_VALUE)
expect(consoleWarnMock.mock.calls.flat().join(' ')).not.toContain(SECRET_VALUE)

This is the security invariant the design claims; without explicit grep-style tests it can regress silently. Fix: Add tests/security-invariants/local-vault-no-leak.test.ts.

I2. MEDIUM — No test for AES-GCM tamper detection

File: src/services/localVault/__tests__/store.test.ts Should include: (1) flip a byte in data → expect LocalVaultDecryptionError; (2) flip a byte in tag → same; (3) swap IVs between records → same.

I3. MEDIUM — No test for multiStore path traversal attempts

File: src/services/SessionMemory/__tests__/multiStore.test.ts Should test: setEntry('..', 'k', 'v'), setEntry('a/b', ...), setEntry('C:hack', ...), setEntry('foo\\u0000', ...).

---

J. Performance / edge

J1. MEDIUM — loadProviders() does fresh disk read on every findProvider() call

File: src/services/providerRegistry/loader.ts:133-138 Hot path: getAuthStatus() → loadProviders() → 4 file reads in /login flow alone. Not crippling but unnecessary. Fix: Memoize per-process with file mtime invalidation.

J2. MEDIUM — setSecret reads entire vault file, parses JSON, writes entire file every call

File: src/services/localVault/store.ts:194-217 For users with 100+ secrets each call is O(N). At 1000 entries x 1KB = 1MB read+write per setSecret. Fix: OS keychain primary path is O(1), so only file-fallback users hit this. Acceptable for v1; document scale limit (~100 entries) in README.

J3. LOW — applyCompatRule() deep-copies messages array (.map returning new objects)

File: src/services/providerRegistry/providerCompatMatrix.ts:132-176 Per chat completion, ~messages.length object allocations. For 100-turn conversations this is 100 small alloc per request — probably negligible vs network latency. Fix: None for now; revisit if profiler shows hot.

---

OVERALL VERDICT

• Total findings: 40 (1 CRITICAL · 10 HIGH · 22 MEDIUM · 7 LOW)
• Net assessment: Code is functional, well-tested at the unit level, and safer than the cross-audit baseline (2026-04-29 found 0/5/13). However, the single CRITICAL (E1: Windows path traversal in multiStore) is a real escalation surface — a user on Windows can write to arbitrary locations via /local-memory store C:foo k v. The 3 concurrency HIGHs (C1/C2/C3) are correctness issues that will bite in scripted use. The crypto HIGHs (F1/F2) reduce the security promise of the file-fallback path under low-entropy passphrases.

TOP 5 must-fix (recommended for PR-5)

1. E1 (CRITICAL) — Strengthen multiStore.validateStoreName to reject :, .., null bytes, drive prefixes, and assert store === basename(store). Add path-traversal regression tests (I3). ~40 LOC + 10 tests.
2. C1 + C2 + C3 (HIGH x3) — Atomic .tmp + rename for localVault/store.ts, multiStore.ts, providerRegistry/loader.ts writes; add in-process mutex for setSecret and saveProviders. ~80 LOC + 6 tests.
3. F1 + F2 (HIGH x2) — Replace SHA-256 KDF with scryptSync + per-vault random salt. ~30 LOC + 3 tests. Backward compat: detect old-format files (no salt field) and migrate on first decrypt.
4. D1 + D2 (HIGH x2) — Add MAX_VALUE_BYTES (64KB local-vault, 1MB local-memory) checks at write entry points. ~20 LOC + 4 tests.
5. I1 (HIGH) — Add explicit no-leak grep tests for local-vault and local-memory paths (assert SECRET never in any mock log/warn/onDone capture). ~50 LOC of test code.

Estimated PR-5 fix workload

• TOP-5 critical/high fixes: ~220 LOC source + ~150 LOC tests across ~6 files → 1 PR
• Remaining 9 HIGH (G1, H1-H3 dedup, I2-I3, J1-J2, A1, A4): ~400 LOC refactor / dedup → 1 PR
• 22 MEDIUM: mostly small UX/validation tightening → 2 PRs

Total estimated work: ~770 LOC source + ~250 LOC tests → 4 PRs over ~2 days.

The code overall demonstrates sound engineering discipline (immutable patterns in applyCompatRule, hostGuard early-detection, per-IV randomization, secret-never-in-onDone in launch files). The findings here are mostly tightening the perimeter rather than rewrites.