From b0fe91a713ed26da96d035d5869d33760bcc2252 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 11:54:50 -0700 Subject: [PATCH 1/7] fix(claude): stop a fail-once ownership fault from deleting the credentials it just wrote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During managed-account add, `writeOauthAccount` re-runs the ownership gate after `writeCredentials` has already written `.credentials.json`. A transient fault there (a cold WSL distro, a probe timeout, an antivirus lock) threw out of `persist` into `cleanupFailedAdd`, whose re-gate then succeeded — the fault was transient — and `rmSync`'d the whole account directory, credentials included. Three collapses made that unavoidable, and Claude had no typed errors at all, so no caller could branch on any of them: - `assertOwnedWsl` reported `timedOut` and every non-zero exit as definitive absence, and its catch-all relabelled everything (including that absence) as a trust verdict. - `resolveOwnedClaudeManagedAuthPath` returned the same `null` for a stranger's directory and for a `realpathSync` that failed. - `cleanupFailedAdd` deleted on any add failure. Adds the `owned | untrusted | indeterminate` verdict and the two error classes the Codex lane already has (mirrored, not extracted — merging the lanes is STA-5616), classifies both probes against it, and stops cleanup from deleting on an unproven observation: it leaks the directory and rethrows the original failure instead. The WSL guest now states its observation on a tagged line rather than encoding it in an exit status no caller could tell apart from `wsl.exe` failing to start. User-requested removal is unchanged and still deletes everything, keychain credentials included. Fixes STA-5674. --- .../claude-account-registration.ts | 35 +- ...ged-auth-fail-once-credential-loss.test.ts | 406 ++++++++++++++++++ .../claude-managed-auth-ownership.ts | 68 +++ .../claude-managed-auth-storage.ts | 112 ++--- .../managed-auth-ownership-verdict.test.ts | 175 ++++++++ src/main/claude-accounts/managed-auth-path.ts | 174 +++++--- .../wsl-managed-auth-probe.test.ts | 119 +++++ .../claude-accounts/wsl-managed-auth-probe.ts | 109 +++++ 8 files changed, 1094 insertions(+), 104 deletions(-) create mode 100644 src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts create mode 100644 src/main/claude-accounts/claude-managed-auth-ownership.ts create mode 100644 src/main/claude-accounts/managed-auth-ownership-verdict.test.ts create mode 100644 src/main/claude-accounts/wsl-managed-auth-probe.test.ts create mode 100644 src/main/claude-accounts/wsl-managed-auth-probe.ts diff --git a/src/main/claude-accounts/claude-account-registration.ts b/src/main/claude-accounts/claude-account-registration.ts index bebc26be6804..a968e9cfcc35 100644 --- a/src/main/claude-accounts/claude-account-registration.ts +++ b/src/main/claude-accounts/claude-account-registration.ts @@ -6,6 +6,7 @@ import type { import type { Store } from '../persistence' import type { RateLimitService } from '../rate-limits/service' import { findDuplicateClaudeAccount } from './claude-duplicate-account' +import { isUnprovenManagedClaudeAuthError } from './claude-managed-auth-ownership' import type { CapturedClaudeAuth } from './claude-auth-capture' import type { ClaudeManagedAuthLocation, @@ -64,7 +65,7 @@ export class ClaudeAccountRegistration { const captured = await this.dependencies.login(location) return await this.persist(accountId, location, previousSettings, captured) } catch (error) { - await this.cleanupFailedAdd(accountId, location.managedAuthPath, previousSettings, error) + await this.runCleanupFailedAdd(accountId, location.managedAuthPath, previousSettings, error) throw error } } @@ -83,7 +84,7 @@ export class ClaudeAccountRegistration { ) return await this.persist(accountId, location, previousSettings, captured) } catch (error) { - await this.cleanupFailedAdd(accountId, location.managedAuthPath, previousSettings, error) + await this.runCleanupFailedAdd(accountId, location.managedAuthPath, previousSettings, error) throw error } } @@ -191,6 +192,24 @@ export class ClaudeAccountRegistration { return this.dependencies.selection.snapshot() } + /** + * The add's own failure is what the caller must see. Cleanup runs for its + * side effects only, so a throw from any step of it -- not just the + * rematerialize -- must not replace the login or write error that caused it. + */ + private async runCleanupFailedAdd( + accountId: string, + managedAuthPath: string, + previousSettings: ReturnType, + error: unknown + ): Promise { + try { + await this.cleanupFailedAdd(accountId, managedAuthPath, previousSettings, error) + } catch (cleanupError) { + console.warn('[claude-accounts] Cleanup after a failed add did not complete:', cleanupError) + } + } + private async cleanupFailedAdd( accountId: string, managedAuthPath: string, @@ -207,6 +226,18 @@ export class ClaudeAccountRegistration { } catch (rollbackError) { console.warn('[claude-accounts] Rollback rematerialization failed:', rollbackError) } + // Why leak instead of delete: the add already wrote `.credentials.json` + // before the gate that failed, and a fail-once fault re-gates clean a moment + // later -- so `remove()` would happily delete the credentials the user just + // logged in for. An observation Orca could not complete is not authority to + // delete anything (STA-5674). + if (isUnprovenManagedClaudeAuthError(error)) { + console.warn( + '[claude-accounts] Leaving managed auth in place: ownership could not be established.', + error + ) + return + } await this.dependencies.removeManagedAuth(accountId, managedAuthPath) } diff --git a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts new file mode 100644 index 000000000000..b1095eb39c2d --- /dev/null +++ b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts @@ -0,0 +1,406 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { restorePlatform, setPlatform } from './claude-account-service-test-harness' +import type * as WslPathsModule from '../../shared/wsl-paths' +import type * as WslModule from '../wsl' +import type * as ManagedAuthPathModule from './managed-auth-path' +import type * as NodeFsModule from 'node:fs' + +// STA-5674 regression. A fail-once ownership probe during managed-account add +// makes `writeOauthAccount` throw AFTER `writeCredentials` has already written +// `.credentials.json`; cleanup used to re-gate successfully and rmSync the whole +// account directory. Injecting at `writeCredentials` proves nothing — that gate +// runs before any bytes exist. + +// Hoisted holder, not a hoisted mkdtemp: vi.hoisted() runs before the static +// imports it would need. +const paths = vi.hoisted(() => ({ userDataRoot: '' })) + +vi.mock('electron', () => ({ + app: { getPath: () => paths.userDataRoot } +})) + +vi.mock('../codex-cli/command', () => ({ + resolveClaudeCommand: vi.fn(() => 'claude') +})) + +vi.mock('./keychain', () => ({ + deleteActiveClaudeKeychainCredentialsStrict: vi.fn(async () => {}), + deleteManagedClaudeKeychainCredentials: vi.fn(async () => {}), + readActiveClaudeKeychainCredentials: vi.fn(), + readActiveClaudeKeychainCredentialsStrict: vi.fn(), + readManagedClaudeKeychainCredentials: vi.fn(), + writeActiveClaudeKeychainCredentials: vi.fn(async () => {}), + writeManagedClaudeKeychainCredentials: vi.fn(async () => {}) +})) + +// The two path-spelling shims below are the ONLY concession to running the +// win32 lane on a POSIX host: they make the distro's guest paths addressable by +// real `node:fs` so the credential bytes under test are real bytes. +vi.mock('../../shared/wsl-paths', async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + parseWslUncPath: (path: string) => + path.includes('/.local/share/orca/claude-accounts/') + ? { distro: 'Ubuntu', linuxPath: path } + : original.parseWslUncPath(path) + } +}) + +vi.mock('../wsl', async (importOriginal) => { + const original = await importOriginal() + return { ...original, toWindowsWslPath: (linuxPath: string) => linuxPath } +}) + +const wslMocks = vi.hoisted(() => ({ runWslProcess: vi.fn() })) + +vi.mock('../wsl/wsl-runner', () => ({ + runWslProcess: wslMocks.runWslProcess, + DEFAULT_WSL_TIMEOUT_MS: 30_000 +})) + +// The host-lane fault is injected at the filesystem, not at the ownership +// function's return value: a transient lock is an errno, and it is the +// classifier under test that has to decide what an EBUSY means. A mock that +// hands back a pre-decided `null` would be asserting the answer. +const fsFaults = vi.hoisted(() => ({ realpathLocked: false })) + +vi.mock('node:fs', async (importOriginal) => { + const original = await importOriginal() + const realpathSync = ((path: Parameters[0], options?: never) => { + if (fsFaults.realpathLocked) { + const error = new Error( + `EBUSY: resource busy or locked, realpath '${String(path)}'` + ) as NodeJS.ErrnoException + error.code = 'EBUSY' + error.errno = -16 + error.syscall = 'realpath' + throw error + } + return original.realpathSync(path, options) + }) as typeof original.realpathSync + realpathSync.native = original.realpathSync.native + const mocked = { ...original, realpathSync } + return { ...mocked, default: mocked } +}) + +const authPathMocks = vi.hoisted(() => ({ + failOwnershipOnCall: 0, + ownershipCalls: 0, + credentialsAtFault: null as string | null +})) + +vi.mock('./managed-auth-path', async (importOriginal) => { + const original = await importOriginal() + const withFaultOnNthCall = (candidatePath: string, run: () => T): T => { + authPathMocks.ownershipCalls += 1 + if (authPathMocks.ownershipCalls !== authPathMocks.failOwnershipOnCall) { + return run() + } + authPathMocks.credentialsAtFault = readIfPresent(`${candidatePath}/.credentials.json`) + fsFaults.realpathLocked = true + try { + return run() + } finally { + fsFaults.realpathLocked = false + } + } + return { + ...original, + resolveClaudeManagedAuthVerdict: ( + accountId: string, + candidatePath: string, + options?: { adoptLegacyMarker?: boolean } + ) => + withFaultOnNthCall(candidatePath, () => + original.resolveClaudeManagedAuthVerdict(accountId, candidatePath, options) + ), + resolveOwnedClaudeManagedAuthPath: ( + accountId: string, + candidatePath: string, + options?: { adoptLegacyMarker?: boolean } + ) => + withFaultOnNthCall(candidatePath, () => + original.resolveOwnedClaudeManagedAuthPath(accountId, candidatePath, options) + ) + } +}) + +// Read through a plain require-free helper so the mock factory stays hoistable. +function readIfPresent(path: string): string | null { + try { + return readFileSync(path, 'utf-8') + } catch { + return null + } +} + +const CREDENTIALS = '{"claudeAiOauth":{"accessToken":"sta5674-real-token"}}\n' +const VERDICT_TAG = 'ORCA_CLAUDE_AUTH_VERDICT:' + +function wslOk(stdout: string) { + return { environmentResolved: true, code: 0, stdout, stderr: '', timedOut: false } +} + +function wslTimeout() { + return { environmentResolved: true, code: null, stdout: '', stderr: '', timedOut: true } +} + +/** What the guest script prints when it finds a valid marker under the root. */ +function wslOwnedVerdict(candidate: string) { + return wslOk(`${VERDICT_TAG}owned:${Buffer.from(candidate, 'utf-8').toString('base64')}\n`) +} + +type ServiceHarness = { + service: { + addAccount: (target?: { runtime?: 'host' | 'wsl'; wslDistro?: string }) => Promise + removeAccount: (accountId: string) => Promise + } + settings: () => { claudeManagedAccounts: unknown[] } + // Resolved from the same registry the service was built from: `vi.resetModules()` + // means a top-level import is a different class object than the one thrown. + temporarilyUnavailable: new (...args: never[]) => Error + keychainDelete: ReturnType + failLoginWith: (error: Error) => void +} + +async function buildService( + options: { throwOnUpdateSettingsCall?: number } = {} +): Promise { + let updateSettingsCalls = 0 + let settings = { + claudeManagedAccounts: [] as unknown[], + activeClaudeManagedAccountId: null, + activeClaudeManagedAccountIdsByRuntime: { host: null, wsl: {} } + } + const store = { + getSettings: vi.fn(() => settings), + updateSettings: vi.fn((updates: Partial) => { + updateSettingsCalls += 1 + if (updateSettingsCalls === options.throwOnUpdateSettingsCall) { + throw new Error('settings write failed during cleanup') + } + settings = { ...settings, ...updates } + return settings + }) + } + const runtimeAuth = { + clearLastWrittenCredentialsJson: vi.fn(), + syncForCurrentSelection: vi.fn(async () => {}), + forceMaterializeCurrentSelectionForRollback: vi.fn(async () => {}), + getRuntimeConfigDir: vi.fn(() => paths.userDataRoot) + } + const rateLimits = { + evictInactiveClaudeCache: vi.fn(), + refreshForClaudeAccountChange: vi.fn(async () => ({ accounts: [], activeAccountId: null })) + } + const { ClaudeAccountService } = await import('./service') + const service = new ClaudeAccountService( + store as never, + rateLimits as never, + runtimeAuth as never + ) + ;( + service as unknown as { runClaudeLoginAndCapture: () => Promise } + ).runClaudeLoginAndCapture = vi.fn(async () => ({ + credentialsJson: CREDENTIALS, + oauthAccount: { accountUuid: 'sta5674' }, + identity: { email: 'sta5674@example.com', organizationUuid: null, organizationName: null } + })) + const login = service as unknown as { runClaudeLoginAndCapture: () => Promise } + const ownership = await import('./claude-managed-auth-ownership') + const keychain = await import('./keychain') + return { + service: service as unknown as ServiceHarness['service'], + settings: () => settings, + temporarilyUnavailable: ownership.ManagedClaudeAuthTemporarilyUnavailableError, + keychainDelete: vi.mocked(keychain.deleteManagedClaudeKeychainCredentials), + failLoginWith: (error: Error) => { + login.runClaudeLoginAndCapture = vi.fn(async () => { + throw error + }) + } + } +} + +describe('STA-5674: fail-once ownership probe during Claude managed-account add', () => { + let guestHome: string | null = null + + beforeEach(() => { + vi.resetModules() + wslMocks.runWslProcess.mockReset() + authPathMocks.failOwnershipOnCall = 0 + authPathMocks.ownershipCalls = 0 + authPathMocks.credentialsAtFault = null + fsFaults.realpathLocked = false + guestHome = mkdtempSync(join(tmpdir(), 'sta5674-guest-')) + paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-userdata-')) + }) + + afterEach(() => { + restorePlatform() + fsFaults.realpathLocked = false + if (guestHome) { + rmSync(guestHome, { recursive: true, force: true }) + } + if (paths.userDataRoot) { + rmSync(paths.userDataRoot, { recursive: true, force: true }) + } + }) + + it('win32/WSL lane: a one-shot probe timeout at writeOauthAccount keeps the account directory and the credentials just written', async () => { + setPlatform('win32') + const home = guestHome! + let ownershipProbes = 0 + const observed: { credentialsAtFault: string | null; accountDir: string | null } = { + credentialsAtFault: null, + accountDir: null + } + + // A fake WSL guest that executes each script's intent against the real + // temp filesystem, so ownership is decided by real files. + wslMocks.runWslProcess.mockImplementation(async (spec: { script: string; args?: string[] }) => { + if (spec.script.includes('WSL_DISTRO_NAME')) { + return wslOk(`Ubuntu\n${home}\n`) + } + if (spec.script.includes('mkdir -p')) { + const [dir, accountId] = spec.args ?? [] + mkdirSync(dir, { recursive: true, mode: 0o700 }) + writeFileSync(join(dir, '.orca-managed-claude-auth'), `${accountId}\n`, { mode: 0o600 }) + return wslOk('') + } + // Ownership probe. Probe order: 1 = create(), 2 = writeCredentials, + // 3 = writeOauthAccount (the fault). + ownershipProbes += 1 + const candidate = spec.script.match(/candidate='([^']+)'/)?.[1] ?? '' + if (ownershipProbes === 3) { + const credentialsPath = join(candidate, '.credentials.json') + observed.credentialsAtFault = existsSync(credentialsPath) + ? readFileSync(credentialsPath, 'utf-8') + : null + observed.accountDir = join(candidate, '..') + return wslTimeout() + } + const marker = join(candidate, '.orca-managed-claude-auth') + return existsSync(marker) + ? wslOwnedVerdict(candidate) + : wslOk(`${VERDICT_TAG}missing-marker\n`) + }) + + const { service, settings, temporarilyUnavailable, keychainDelete } = await buildService() + const failure = await service + .addAccount({ runtime: 'wsl', wslDistro: 'Ubuntu' }) + .then(() => null) + .catch((error: unknown) => error) + + // A probe that never completed is reported as such, not as a trust verdict. + expect(failure).toBeInstanceOf(temporarilyUnavailable) + expect((failure as Error).message).toMatch(/temporarily locked/) + + // The fault landed after real credential bytes existed. + expect(observed.credentialsAtFault).toBe(CREDENTIALS) + + const accountDir = observed.accountDir! + expect(existsSync(accountDir)).toBe(true) + const credentialsPath = join(accountDir, 'auth', '.credentials.json') + expect(existsSync(credentialsPath)).toBe(true) + expect(readFileSync(credentialsPath, 'utf-8')).toBe(CREDENTIALS) + // 3 probes, not 4: cleanup must not re-gate the path at all. The re-gate is + // what used to succeed and authorise the delete. + expect(ownershipProbes).toBe(3) + // Leaked, not registered: the add still fails and nothing durable moves. + expect(settings().claudeManagedAccounts).toHaveLength(0) + expect(keychainDelete).not.toHaveBeenCalled() + }) + + it('host lane (no path shims): a one-shot ownership fault at writeOauthAccount keeps the account directory and the credentials just written', async () => { + setPlatform('linux') + // create() probe = 1, writeCredentials = 2, writeOauthAccount = 3. + authPathMocks.failOwnershipOnCall = 3 + + const { service, settings, temporarilyUnavailable, keychainDelete } = await buildService() + const failure = await service + .addAccount({ runtime: 'host' }) + .then(() => null) + .catch((error: unknown) => error) + + expect(failure).toBeInstanceOf(temporarilyUnavailable) + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + // The fault landed after real credential bytes existed. + expect(authPathMocks.credentialsAtFault).toBe(CREDENTIALS) + const accountId = (settings().claudeManagedAccounts[0] as { id?: string } | undefined)?.id + expect(accountId).toBeUndefined() + // Three probes, not four: cleanup must not re-gate the path at all. + expect(authPathMocks.ownershipCalls).toBe(3) + const { readdirSync } = await import('node:fs') + const leaked = readdirSync(accountsRoot) + expect(leaked).toHaveLength(1) + const credentialsPath = join(accountsRoot, leaked[0], 'auth', '.credentials.json') + expect(readFileSync(credentialsPath, 'utf-8')).toBe(CREDENTIALS) + expect(keychainDelete).not.toHaveBeenCalled() + }) + + it('CONTROL — no fault: the account directory and its credentials survive the same add', async () => { + setPlatform('linux') + authPathMocks.failOwnershipOnCall = 0 + + const { service, settings } = await buildService() + await service.addAccount({ runtime: 'host' }) + + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id + const credentialsPath = join(accountsRoot, accountId, 'auth', '.credentials.json') + expect(existsSync(credentialsPath)).toBe(true) + expect(readFileSync(credentialsPath, 'utf-8')).toBe(CREDENTIALS) + }) + + it('CONTROL — injecting at writeCredentials proves nothing: no credential bytes exist when that gate fails', async () => { + setPlatform('linux') + // create() probe = 1, writeCredentials = 2. Its gate runs BEFORE any write. + authPathMocks.failOwnershipOnCall = 2 + + const { service } = await buildService() + await expect(service.addAccount({ runtime: 'host' })).rejects.toThrow(/temporarily locked/) + + // The directory is left in place either way, but there were never any + // credentials to lose — an "assert credentials survived" test here is vacuous. + expect(authPathMocks.credentialsAtFault).toBeNull() + const { readdirSync } = await import('node:fs') + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + const leaked = readdirSync(accountsRoot) + expect(leaked).toHaveLength(1) + expect(existsSync(join(accountsRoot, leaked[0], 'auth', '.credentials.json'))).toBe(false) + }) + + it('reports the add failure, not the cleanup failure, when cleanup itself throws', async () => { + setPlatform('linux') + authPathMocks.failOwnershipOnCall = 0 + // restoreSettings() is cleanup's first settings write, and it is the only + // one that runs before removeManagedAuth. + const { service, failLoginWith } = await buildService({ throwOnUpdateSettingsCall: 1 }) + const loginFailure = new Error('Claude login was cancelled.') + failLoginWith(loginFailure) + + await expect(service.addAccount({ runtime: 'host' })).rejects.toBe(loginFailure) + }) + + it('CONTROL — a user-requested removal still deletes the account directory and its keychain credentials', async () => { + setPlatform('linux') + authPathMocks.failOwnershipOnCall = 0 + + const { service, settings, keychainDelete } = await buildService() + await service.addAccount({ runtime: 'host' }) + const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + expect(existsSync(join(accountsRoot, accountId, 'auth', '.credentials.json'))).toBe(true) + + await service.removeAccount(accountId) + + expect(existsSync(join(accountsRoot, accountId))).toBe(false) + expect(keychainDelete).toHaveBeenCalledWith(accountId) + expect(settings().claudeManagedAccounts).toHaveLength(0) + }) +}) diff --git a/src/main/claude-accounts/claude-managed-auth-ownership.ts b/src/main/claude-accounts/claude-managed-auth-ownership.ts new file mode 100644 index 000000000000..3dca8c8f13c1 --- /dev/null +++ b/src/main/claude-accounts/claude-managed-auth-ownership.ts @@ -0,0 +1,68 @@ +/** + * Why: the Claude managed-auth gate answers two different questions and callers + * must act on them differently. `untrusted` is a *successful observation* that + * failed a trust check (or a definitive absence, where absence is the verdict); + * `indeterminate` means the storage could not be read at all, so the directory + * may be perfectly valid and callers must refuse to act without erasing durable + * state. + * + * Claude had no typed errors at all before STA-5674 — every failure arrived as a + * bare `Error` with a trust-flavoured string — so `cleanupFailedAdd` could not + * tell "this is not your directory" from "we could not look", and deleted the + * account on both. Mirrors the Codex lane's `HostCodexManagedHomeVerdict` + * vocabulary deliberately; merging the two lanes into one module is STA-5616. + */ +export type ClaudeManagedAuthVerdict = + | { kind: 'owned'; authPath: string } + | { kind: 'untrusted'; reason: string } + | { kind: 'indeterminate'; error: unknown } + +export const MISSING_MANAGED_AUTH_MESSAGE = 'Managed Claude auth directory does not exist on disk.' +export const UNTRUSTED_MANAGED_AUTH_MESSAGE = 'Managed Claude auth storage is not owned by Orca.' +export const OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE = + 'Managed WSL Claude auth storage is outside Orca account storage.' + +/** Thrown for a proven trust failure; safe to refuse and to clean up on. */ +export class UntrustedManagedClaudeAuthError extends Error {} + +/** + * Thrown when the storage could not be read. Callers must refuse the operation + * and leave persisted settings, credentials, and managed directories untouched. + */ +export class ManagedClaudeAuthTemporarilyUnavailableError extends Error { + constructor( + message = 'Claude account files are temporarily locked. Retry in a moment.', + options?: { cause?: unknown } + ) { + super(message, options) + } +} + +/** Throwing wrapper for the write paths that must never proceed on unproven storage. */ +export function assertOwnedClaudeManagedAuthPath(verdict: ClaudeManagedAuthVerdict): string { + if (verdict.kind === 'owned') { + return verdict.authPath + } + if (verdict.kind === 'untrusted') { + throw new UntrustedManagedClaudeAuthError(verdict.reason) + } + throw new ManagedClaudeAuthTemporarilyUnavailableError(undefined, { cause: verdict.error }) +} + +const MAX_CAUSE_DEPTH = 8 + +/** + * True when `error`, or anything it wraps, reports storage Orca could not read. + * Walks `cause` because a failure crosses several layers before the caller that + * has to decide whether deleting is authorised. + */ +export function isUnprovenManagedClaudeAuthError(error: unknown): boolean { + let current = error + for (let depth = 0; depth < MAX_CAUSE_DEPTH && current instanceof Error; depth += 1) { + if (current instanceof ManagedClaudeAuthTemporarilyUnavailableError) { + return true + } + current = (current as { cause?: unknown }).cause + } + return false +} diff --git a/src/main/claude-accounts/claude-managed-auth-storage.ts b/src/main/claude-accounts/claude-managed-auth-storage.ts index 970202c82106..5cb0e34ecf83 100644 --- a/src/main/claude-accounts/claude-managed-auth-storage.ts +++ b/src/main/claude-accounts/claude-managed-auth-storage.ts @@ -1,14 +1,27 @@ -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { lstatSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' import { join, relative, resolve, sep } from 'node:path' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { parseWslUncPath } from '../../shared/wsl-paths' import { toWindowsWslPath } from '../wsl' import { runWslProcess } from '../wsl/wsl-runner' +import { + assertOwnedClaudeManagedAuthPath, + MISSING_MANAGED_AUTH_MESSAGE, + OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE, + UNTRUSTED_MANAGED_AUTH_MESSAGE, + type ClaudeManagedAuthVerdict +} from './claude-managed-auth-ownership' import { getClaudeManagedAccountsRoot, + MANAGED_AUTH_MARKER, readClaudeManagedAuthFile, - resolveOwnedClaudeManagedAuthPath, + resolveClaudeManagedAuthVerdict, writeClaudeManagedAuthFile } from './managed-auth-path' +import { + buildWslManagedAuthProbeScript, + classifyWslManagedAuthProbe +} from './wsl-managed-auth-probe' import { deleteManagedClaudeKeychainCredentials, readManagedClaudeKeychainCredentials, @@ -32,10 +45,6 @@ export type ClaudeManagedAuthTarget = { wslDistro?: string | null } -function shellQuote(value: string): string { - return `'${value.replace(/'/g, "'\\''")}'` -} - export class ClaudeManagedAuthStorage { async create( accountId: string, @@ -47,7 +56,7 @@ export class ClaudeManagedAuthStorage { } const managedAuthPath = join(this.getRoot(), accountId, 'auth') mkdirSync(managedAuthPath, { recursive: true, mode: 0o700 }) - writeFileSync(join(managedAuthPath, '.orca-managed-claude-auth'), `${accountId}\n`, { + writeFileSync(join(managedAuthPath, MANAGED_AUTH_MARKER), `${accountId}\n`, { encoding: 'utf-8', mode: 0o600 }) @@ -149,22 +158,30 @@ export class ClaudeManagedAuthStorage { } async assertOwned(candidatePath: string, expectedAccountId?: string): Promise { + return assertOwnedClaudeManagedAuthPath( + await this.resolveVerdict(candidatePath, expectedAccountId) + ) + } + + /** Non-throwing view for callers that must branch on *why* the gate refused. */ + async resolveVerdict( + candidatePath: string, + expectedAccountId?: string + ): Promise { const wslInfo = parseWslUncPath(candidatePath) if (wslInfo) { - return this.assertOwnedWsl(candidatePath, wslInfo, expectedAccountId) + return this.resolveWslVerdict(candidatePath, wslInfo, expectedAccountId) + } + try { + this.getRoot() + } catch (error) { + return { kind: 'indeterminate', error } } - this.getRoot() const accountId = expectedAccountId ?? this.readAccountId(candidatePath) if (!accountId || (expectedAccountId && accountId !== expectedAccountId)) { - throw new Error('Managed Claude auth directory does not exist on disk.') - } - const trustedPath = resolveOwnedClaudeManagedAuthPath(accountId, candidatePath, { - adoptLegacyMarker: true - }) - if (!trustedPath) { - throw new Error('Managed Claude auth storage is not owned by Orca.') + return { kind: 'untrusted', reason: MISSING_MANAGED_AUTH_MESSAGE } } - return trustedPath + return resolveClaudeManagedAuthVerdict(accountId, candidatePath, { adoptLegacyMarker: true }) } private async tryCreateWsl( @@ -215,56 +232,35 @@ export class ClaudeManagedAuthStorage { } } - private async assertOwnedWsl( + private async resolveWslVerdict( candidatePath: string, wslInfo: NonNullable>, expectedAccountId?: string - ): Promise { + ): Promise { if ( !wslInfo.linuxPath.includes('/.local/share/orca/claude-accounts/') || !wslInfo.linuxPath.endsWith('/auth') ) { - throw new Error('Managed WSL Claude auth storage is outside Orca account storage.') + return { kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE } } if (process.platform !== 'win32') { - if ( - !existsSync(candidatePath) || - !existsSync(join(candidatePath, '.orca-managed-claude-auth')) - ) { - throw new Error('Managed Claude auth storage is not owned by Orca.') - } - return candidatePath + return resolveHostVisibleGuestVerdict(candidatePath) } + let probe: Awaited> try { - const expected = expectedAccountId - ? `test "$(cat "$candidate_real/.orca-managed-claude-auth")" = ${shellQuote(expectedAccountId)}` - : 'test -n "$(cat "$candidate_real/.orca-managed-claude-auth")"' - const owned = await runWslProcess({ + probe = await runWslProcess({ distro: wslInfo.distro, loginPath: 'none', shell: 'bash', - script: [ - 'set -euo pipefail', - `candidate=${shellQuote(wslInfo.linuxPath)}`, - 'managed_root="${HOME%/}/.local/share/orca/claude-accounts"', - 'candidate_real=$(readlink -f -- "$candidate")', - 'managed_root_real=$(readlink -f -- "$managed_root")', - 'test -f "$candidate_real/.orca-managed-claude-auth"', - expected, - 'case "$candidate_real" in "$managed_root_real"/*/auth) printf "%s\\n" "$candidate_real" ;; *) exit 35 ;; esac' - ].join('\n'), + script: buildWslManagedAuthProbeScript(wslInfo.linuxPath, expectedAccountId), timeoutMs: 5000 }) - const canonicalPath = owned.stdout.trim() - if (owned.code !== 0 || owned.timedOut || !canonicalPath) { - throw new Error('Managed Claude auth directory does not exist on disk.') - } - return toWindowsWslPath(canonicalPath, wslInfo.distro) } catch (error) { - throw new Error('Managed WSL Claude auth storage is outside Orca account storage.', { - cause: error - }) + // A spawn failure says nothing about the directory; it says wsl.exe did + // not run. + return { kind: 'indeterminate', error } } + return classifyWslManagedAuthProbe(probe, wslInfo.distro) } private getRoot(): string { @@ -279,3 +275,21 @@ export class ClaudeManagedAuthStorage { return parts.length === 2 && parts[1] === 'auth' ? parts[0] : null } } + +/** + * A guest path that the host can address directly (a non-win32 host reading a + * WSL-spelled record). Only a definitive absence is a verdict; an unreadable + * directory is not evidence that it is a stranger's. + */ +function resolveHostVisibleGuestVerdict(candidatePath: string): ClaudeManagedAuthVerdict { + for (const path of [candidatePath, join(candidatePath, MANAGED_AUTH_MARKER)]) { + try { + lstatSync(path) + } catch (error) { + return isDefinitiveAbsence(error) + ? { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } + : { kind: 'indeterminate', error } + } + } + return { kind: 'owned', authPath: candidatePath } +} diff --git a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts new file mode 100644 index 000000000000..632430208423 --- /dev/null +++ b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts @@ -0,0 +1,175 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { restorePlatform, setPlatform } from './claude-account-service-test-harness' +import type * as NodeFsModule from 'node:fs' + +// Fault injection at the filesystem, so the classifier under test is the thing +// deciding what an errno means. Keyed by path suffix: the probe reads several +// paths and only one of them is meant to be locked. +const fsFaults = vi.hoisted(() => ({ + lockedReadSuffix: null as string | null, + lockedLstatSuffix: null as string | null +})) + +vi.mock('node:fs', async (importOriginal) => { + const original = await importOriginal() + const locked = (path: unknown, suffix: string | null) => + suffix !== null && String(path).endsWith(suffix) + const busy = (path: unknown, syscall: string) => { + const error = new Error( + `EBUSY: resource busy or locked, ${syscall} '${String(path)}'` + ) as NodeJS.ErrnoException + error.code = 'EBUSY' + error.syscall = syscall + return error + } + const readFileSync = ((path: never, options: never) => { + if (locked(path, fsFaults.lockedReadSuffix)) { + throw busy(path, 'read') + } + return original.readFileSync(path, options) + }) as typeof original.readFileSync + const lstatSync = ((path: never, options: never) => { + if (locked(path, fsFaults.lockedLstatSuffix)) { + throw busy(path, 'lstat') + } + return original.lstatSync(path, options) + }) as typeof original.lstatSync + const mocked = { ...original, readFileSync, lstatSync } + return { ...mocked, default: mocked } +}) + +const paths = vi.hoisted(() => ({ userDataRoot: '' })) + +vi.mock('electron', () => ({ app: { getPath: () => paths.userDataRoot } })) + +vi.mock('./keychain', () => ({ + deleteManagedClaudeKeychainCredentials: vi.fn(async () => {}), + readManagedClaudeKeychainCredentials: vi.fn(), + writeManagedClaudeKeychainCredentials: vi.fn(async () => {}) +})) + +const { MANAGED_AUTH_MARKER, resolveClaudeManagedAuthVerdict } = await import('./managed-auth-path') +const { ClaudeManagedAuthStorage } = await import('./claude-managed-auth-storage') +const { MISSING_MANAGED_AUTH_MESSAGE, OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE } = + await import('./claude-managed-auth-ownership') + +const ACCOUNT_ID = 'acct-5674' + +function seedAccount(markerContents: string | null): string { + const authPath = join(paths.userDataRoot, 'claude-accounts', ACCOUNT_ID, 'auth') + mkdirSync(authPath, { recursive: true }) + if (markerContents !== null) { + writeFileSync(join(authPath, MANAGED_AUTH_MARKER), markerContents) + } + return authPath +} + +describe('host Claude managed-auth verdict', () => { + beforeEach(() => { + fsFaults.lockedReadSuffix = null + fsFaults.lockedLstatSuffix = null + paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-verdict-')) + }) + + afterEach(() => { + restorePlatform() + fsFaults.lockedReadSuffix = null + fsFaults.lockedLstatSuffix = null + rmSync(paths.userDataRoot, { recursive: true, force: true }) + }) + + it('accepts a directory whose marker names the account', () => { + const authPath = seedAccount(`${ACCOUNT_ID}\n`) + expect(resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath)).toMatchObject({ kind: 'owned' }) + }) + + it('refuses a symlinked candidate rather than following it', () => { + const elsewhere = mkdtempSync(join(tmpdir(), 'sta5674-elsewhere-')) + writeFileSync(join(elsewhere, MANAGED_AUTH_MARKER), `${ACCOUNT_ID}\n`) + const accountRoot = join(paths.userDataRoot, 'claude-accounts', ACCOUNT_ID) + mkdirSync(accountRoot, { recursive: true }) + const authPath = join(accountRoot, 'auth') + symlinkSync(elsewhere, authPath, 'dir') + try { + expect(resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath).kind).toBe('untrusted') + } finally { + rmSync(elsewhere, { recursive: true, force: true }) + } + }) + + it("refuses a symlink even when it resolves to this account's own auth directory", () => { + // The canonical-path checks alone would accept this: the link resolves to a + // valid `//auth`. Only the lstat guard rejects a persisted + // path that is a link rather than the directory itself. + seedAccount(`${ACCOUNT_ID}\n`) + const linkPath = join(paths.userDataRoot, 'claude-accounts', ACCOUNT_ID, 'auth-link') + symlinkSync('auth', linkPath, 'dir') + expect(resolveClaudeManagedAuthVerdict(ACCOUNT_ID, linkPath).kind).toBe('untrusted') + }) + + it('reports an unreadable marker as indeterminate, not as a stranger directory', () => { + const authPath = seedAccount(`${ACCOUNT_ID}\n`) + fsFaults.lockedReadSuffix = MANAGED_AUTH_MARKER + expect(resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath).kind).toBe('indeterminate') + }) + + it('reports an unstattable marker as indeterminate', () => { + const authPath = seedAccount(`${ACCOUNT_ID}\n`) + fsFaults.lockedLstatSuffix = MANAGED_AUTH_MARKER + expect(resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath).kind).toBe('indeterminate') + }) + + it('treats a marker naming another account as a trust failure even when adoption is allowed', () => { + // Adoption writes with `wx`, so the existing marker makes it EEXIST: proof + // that a marker is there and is not ours, not a write we failed to make. + const authPath = seedAccount('someone-elses-account\n') + expect( + resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath, { adoptLegacyMarker: true }).kind + ).toBe('untrusted') + }) + + it('adopts a legacy directory that has no marker at all', () => { + const authPath = seedAccount(null) + expect( + resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath, { adoptLegacyMarker: true }) + ).toMatchObject({ kind: 'owned' }) + }) + + it('reports a missing directory as a definitive absence', () => { + const authPath = join(paths.userDataRoot, 'claude-accounts', ACCOUNT_ID, 'auth') + mkdirSync(join(paths.userDataRoot, 'claude-accounts'), { recursive: true }) + expect(resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath)).toEqual({ + kind: 'untrusted', + reason: MISSING_MANAGED_AUTH_MESSAGE + }) + }) +}) + +describe('WSL Claude managed-auth verdict', () => { + beforeEach(() => { + paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-verdict-wsl-')) + }) + + afterEach(() => { + restorePlatform() + rmSync(paths.userDataRoot, { recursive: true, force: true }) + }) + + it('refuses a guest path outside the managed accounts root without running a probe', async () => { + setPlatform('win32') + const storage = new ClaudeManagedAuthStorage() + // Would throw if it reached `runWslProcess`, which is not mocked here. + expect(await storage.resolveVerdict('\\\\wsl$\\Ubuntu\\home\\dev\\.ssh')).toEqual({ + kind: 'untrusted', + reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE + }) + expect( + await storage.resolveVerdict( + '\\\\wsl$\\Ubuntu\\home\\dev\\.local\\share\\orca\\claude-accounts\\acct\\elsewhere' + ) + ).toEqual({ kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE }) + }) +}) diff --git a/src/main/claude-accounts/managed-auth-path.ts b/src/main/claude-accounts/managed-auth-path.ts index 2914b72e0334..98f2b1d50c99 100644 --- a/src/main/claude-accounts/managed-auth-path.ts +++ b/src/main/claude-accounts/managed-auth-path.ts @@ -1,58 +1,141 @@ import { existsSync, lstatSync, readFileSync, realpathSync, writeFileSync } from 'node:fs' import { join, relative, resolve, sep } from 'node:path' import { app } from 'electron' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { writeFileAtomically } from '../codex-accounts/fs-utils' +import { + MISSING_MANAGED_AUTH_MESSAGE, + UNTRUSTED_MANAGED_AUTH_MESSAGE, + type ClaudeManagedAuthVerdict +} from './claude-managed-auth-ownership' -const MANAGED_AUTH_MARKER = '.orca-managed-claude-auth' +export const MANAGED_AUTH_MARKER = '.orca-managed-claude-auth' export function getClaudeManagedAccountsRoot(): string { return join(app.getPath('userData'), 'claude-accounts') } -export function resolveOwnedClaudeManagedAuthPath( +/** + * Why a verdict rather than `string | null`: the previous catch-all turned a + * transient `realpathSync`/`lstatSync` failure into the same null a stranger's + * directory produces, and `cleanupFailedAdd` deleted the account on both + * (STA-5674). Only a definitive absence or a completed observation may say + * `untrusted`; an unreadable path is `indeterminate`. + */ +export function resolveClaudeManagedAuthVerdict( accountId: string, candidatePath: string, options: { adoptLegacyMarker?: boolean } = {} -): string | null { - const rootPath = getClaudeManagedAccountsRoot() +): ClaudeManagedAuthVerdict { const resolvedCandidate = resolve(candidatePath) - if (!existsSync(resolvedCandidate) || !existsSync(rootPath)) { - return null - } + let canonicalCandidate: string try { + // lstat before realpath: a symlinked candidate is a trust failure, not a + // path to follow. if (lstatSync(resolvedCandidate).isSymbolicLink()) { - return null - } - const canonicalCandidate = realpathSync(resolvedCandidate) - const canonicalRoot = realpathSync(rootPath) - if ( - canonicalCandidate === canonicalRoot || - !canonicalCandidate.startsWith(canonicalRoot + sep) - ) { - return null + return { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } } - const relativePath = relative(canonicalRoot, canonicalCandidate) - const relativeParts = relativePath.split(sep) - const escaped = relativePath.startsWith('..') || relativePath.includes(`..${sep}`) - if ( - escaped || - relativeParts.length !== 2 || - relativeParts[0] !== accountId || - relativeParts[1] !== 'auth' - ) { - return null - } - const markerPath = join(canonicalCandidate, MANAGED_AUTH_MARKER) - const markerValid = isManagedAuthMarkerValid(markerPath, accountId) - if (!markerValid && options.adoptLegacyMarker) { - writeFileSync(markerPath, `${accountId}\n`, { encoding: 'utf-8', mode: 0o600, flag: 'wx' }) - } - if (!markerValid && !isManagedAuthMarkerValid(markerPath, accountId)) { - return null - } - return canonicalCandidate - } catch { - return null + canonicalCandidate = realpathSync(resolvedCandidate) + } catch (error) { + return classifyReadFailure(error, MISSING_MANAGED_AUTH_MESSAGE) + } + let canonicalRoot: string + try { + canonicalRoot = realpathSync(getClaudeManagedAccountsRoot()) + } catch (error) { + return classifyReadFailure(error, MISSING_MANAGED_AUTH_MESSAGE) + } + if (canonicalCandidate === canonicalRoot || !canonicalCandidate.startsWith(canonicalRoot + sep)) { + return { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } + } + const relativePath = relative(canonicalRoot, canonicalCandidate) + const relativeParts = relativePath.split(sep) + if ( + relativePath.startsWith('..') || + relativeParts.length !== 2 || + relativeParts[0] !== accountId || + relativeParts[1] !== 'auth' + ) { + return { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } + } + return resolveMarkerVerdict(canonicalCandidate, accountId, options) +} + +/** + * The lossy view, for the read-only callers (rate-limit refresh, runtime + * materialization) whose null means "skip this account" and which touch nothing + * durable. Any caller that deletes or clears durable state must branch on + * `resolveClaudeManagedAuthVerdict` instead — collapsing there is STA-5674. + */ +export function resolveOwnedClaudeManagedAuthPath( + accountId: string, + candidatePath: string, + options: { adoptLegacyMarker?: boolean } = {} +): string | null { + const verdict = resolveClaudeManagedAuthVerdict(accountId, candidatePath, options) + return verdict.kind === 'owned' ? verdict.authPath : null +} + +function classifyReadFailure(error: unknown, absenceReason: string): ClaudeManagedAuthVerdict { + return isDefinitiveAbsence(error) + ? { kind: 'untrusted', reason: absenceReason } + : { kind: 'indeterminate', error } +} + +type MarkerState = { kind: 'valid' | 'invalid' } | { kind: 'indeterminate'; error: unknown } + +function resolveMarkerVerdict( + canonicalCandidate: string, + accountId: string, + options: { adoptLegacyMarker?: boolean } +): ClaudeManagedAuthVerdict { + const markerPath = join(canonicalCandidate, MANAGED_AUTH_MARKER) + const marker = readMarkerState(markerPath, accountId) + if (marker.kind !== 'invalid' || !options.adoptLegacyMarker) { + return toMarkerVerdict(marker, canonicalCandidate) + } + try { + writeFileSync(markerPath, `${accountId}\n`, { encoding: 'utf-8', mode: 0o600, flag: 'wx' }) + } catch (error) { + // EEXIST means a marker is present but did not validate: a real trust + // failure. Anything else is a write we could not complete. + return (error as NodeJS.ErrnoException | null)?.code === 'EEXIST' + ? { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } + : { kind: 'indeterminate', error } + } + return toMarkerVerdict(readMarkerState(markerPath, accountId), canonicalCandidate) +} + +function toMarkerVerdict( + marker: MarkerState, + canonicalCandidate: string +): ClaudeManagedAuthVerdict { + if (marker.kind === 'indeterminate') { + return marker + } + return marker.kind === 'valid' + ? { kind: 'owned', authPath: canonicalCandidate } + : { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } +} + +function readMarkerState(markerPath: string, accountId: string): MarkerState { + let markerStats: ReturnType + try { + markerStats = lstatSync(markerPath) + } catch (error) { + // The marker is required, so its definitive absence is structural — but an + // unreadable marker is not evidence of anything. + return isDefinitiveAbsence(error) ? { kind: 'invalid' } : { kind: 'indeterminate', error } + } + if (markerStats.isSymbolicLink() || !markerStats.isFile()) { + return { kind: 'invalid' } + } + try { + return readFileSync(markerPath, 'utf-8').trim() === accountId + ? { kind: 'valid' } + : { kind: 'invalid' } + } catch (error) { + return isDefinitiveAbsence(error) ? { kind: 'invalid' } : { kind: 'indeterminate', error } } } @@ -83,21 +166,6 @@ export function writeClaudeManagedAuthFile( writeFileAtomically(filePath, contents, { mode: 0o600 }) } -function isManagedAuthMarkerValid(markerPath: string, accountId: string): boolean { - try { - if ( - !existsSync(markerPath) || - lstatSync(markerPath).isSymbolicLink() || - !lstatSync(markerPath).isFile() - ) { - return false - } - return readFileSync(markerPath, 'utf-8').trim() === accountId - } catch { - return false - } -} - function isOwnedChildFile(managedAuthPath: string, filePath: string): boolean { if ( !existsSync(filePath) || diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts new file mode 100644 index 000000000000..4a87c52278f2 --- /dev/null +++ b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it, vi } from 'vitest' +import { + isUnprovenManagedClaudeAuthError, + ManagedClaudeAuthTemporarilyUnavailableError, + MISSING_MANAGED_AUTH_MESSAGE, + OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE, + UntrustedManagedClaudeAuthError, + UNTRUSTED_MANAGED_AUTH_MESSAGE +} from './claude-managed-auth-ownership' + +vi.mock('../wsl', () => ({ + toWindowsWslPath: (linuxPath: string) => `\\\\wsl$\\Ubuntu${linuxPath}` +})) + +import { + buildWslManagedAuthProbeScript, + classifyWslManagedAuthProbe +} from './wsl-managed-auth-probe' + +const TAG = 'ORCA_CLAUDE_AUTH_VERDICT:' +const GUEST_PATH = '/home/dev/.local/share/orca/claude-accounts/acct-1/auth' + +function probe(overrides: Partial[0]> = {}) { + return classifyWslManagedAuthProbe( + { environmentResolved: true, code: 0, stdout: '', stderr: '', timedOut: false, ...overrides }, + 'Ubuntu' + ) +} + +function ownedTag(path: string) { + return `${TAG}owned:${Buffer.from(path, 'utf-8').toString('base64')}\n` +} + +describe('WSL Claude managed-auth probe classification', () => { + it('reports a completed guest observation as the verdict it names', () => { + expect(probe({ stdout: ownedTag(GUEST_PATH) })).toEqual({ + kind: 'owned', + authPath: `\\\\wsl$\\Ubuntu${GUEST_PATH}` + }) + expect(probe({ stdout: `${TAG}missing-marker\n` })).toEqual({ + kind: 'untrusted', + reason: MISSING_MANAGED_AUTH_MESSAGE + }) + expect(probe({ stdout: `${TAG}marker-mismatch\n` })).toEqual({ + kind: 'untrusted', + reason: UNTRUSTED_MANAGED_AUTH_MESSAGE + }) + expect(probe({ stdout: `${TAG}outside-managed-root\n` })).toEqual({ + kind: 'untrusted', + reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE + }) + }) + + it('survives a guest path containing a colon and CRLF line endings', () => { + const oddPath = '/home/de:v/.local/share/orca/claude-accounts/acct-1/auth' + expect(probe({ stdout: `${ownedTag(oddPath).trimEnd()}\r\n` })).toEqual({ + kind: 'owned', + authPath: `\\\\wsl$\\Ubuntu${oddPath}` + }) + }) + + // Each of these used to be reported as a trust or absence verdict, which is + // what let a cold distro read as "this is not your auth directory". + it.each([ + ['a timeout', { timedOut: true, code: null, stdout: '' }], + ['a timeout that still reported exit 0', { timedOut: true, code: 0, stdout: '' }], + ['an unresolved distro environment', { environmentResolved: false }], + ['a non-zero exit', { code: 1, stdout: '' }], + ['a non-zero exit that printed a verdict', { code: 1, stdout: `${TAG}missing-marker\n` }], + ['no verdict at all', { stdout: 'bash: base64: command not found\n' }], + ['empty output', { stdout: '' }], + ['more than one verdict', { stdout: `${TAG}missing-marker\n${TAG}outside-managed-root\n` }], + ['a verdict that is not the last line', { stdout: `${TAG}missing-marker\ntrailing\n` }], + ['an unknown verdict', { stdout: `${TAG}sideways\n` }], + ['an undecodable owned path', { stdout: `${TAG}owned:!!!not-base64!!!\n` }], + ['an owned verdict with no path', { stdout: `${TAG}owned:\n` }] + ])('reports %s as indeterminate', (_label, overrides) => { + expect(probe(overrides).kind).toBe('indeterminate') + }) + + it('builds a guest script that quotes its inputs and never relies on set -e for meaning', () => { + const script = buildWslManagedAuthProbeScript(GUEST_PATH, "acct'1") + expect(script).toContain(`candidate='${GUEST_PATH}'`) + expect(script).toContain(`test "$contents" = 'acct'\\''1'`) + expect(script.split('\n')[0]).toBe('set -uo pipefail') + // Without an expected account ID the marker only has to be non-empty. + expect(buildWslManagedAuthProbeScript(GUEST_PATH)).toContain('test -n "$contents"') + }) +}) + +describe('Claude managed-auth error vocabulary', () => { + it('recognises an unproven failure through a chain of causes', () => { + const unavailable = new ManagedClaudeAuthTemporarilyUnavailableError() + expect(isUnprovenManagedClaudeAuthError(unavailable)).toBe(true) + expect(isUnprovenManagedClaudeAuthError(new Error('wrapped', { cause: unavailable }))).toBe( + true + ) + expect( + isUnprovenManagedClaudeAuthError( + new Error('outer', { cause: new Error('inner', { cause: unavailable }) }) + ) + ).toBe(true) + }) + + it('does not mistake a proven trust failure, or anything else, for an unproven one', () => { + expect(isUnprovenManagedClaudeAuthError(new UntrustedManagedClaudeAuthError('nope'))).toBe( + false + ) + expect(isUnprovenManagedClaudeAuthError(new Error('login failed'))).toBe(false) + expect(isUnprovenManagedClaudeAuthError(null)).toBe(false) + expect(isUnprovenManagedClaudeAuthError('temporarily locked')).toBe(false) + }) + + it('terminates on a self-referential cause chain', () => { + const looping = new Error('loop') as Error & { cause?: unknown } + looping.cause = looping + expect(isUnprovenManagedClaudeAuthError(looping)).toBe(false) + }) +}) diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.ts b/src/main/claude-accounts/wsl-managed-auth-probe.ts new file mode 100644 index 000000000000..bbb212989a3b --- /dev/null +++ b/src/main/claude-accounts/wsl-managed-auth-probe.ts @@ -0,0 +1,109 @@ +import { toWindowsWslPath } from '../wsl' +import type { WslResult } from '../wsl/wsl-runner' +import { + MISSING_MANAGED_AUTH_MESSAGE, + OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE, + UNTRUSTED_MANAGED_AUTH_MESSAGE, + type ClaudeManagedAuthVerdict +} from './claude-managed-auth-ownership' + +/** + * Why a tagged line instead of exit codes: under `set -e` a missing marker, a + * marker for another account, a `readlink` failure, and `wsl.exe` failing to + * start the distro all abort with the same status and empty stdout, so no exit + * code is observable evidence of *which* happened. The guest instead states its + * observation and exits 0; only a parsed tag is dispositive, and everything + * else — no tag, extra output, non-zero exit, timeout, spawn failure — is + * indeterminate. That inverts the old default under which a cold distro read as + * "this is not your auth directory" (STA-5674). + */ +const VERDICT_TAG = 'ORCA_CLAUDE_AUTH_VERDICT:' + +function shellQuote(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'` +} + +/** + * The canonical path is base64'd because it is interpolated into a line-oriented + * protocol: a newline anywhere in `$HOME` would otherwise split the verdict in + * two and read as a malformed probe. + */ +export function buildWslManagedAuthProbeScript( + linuxPath: string, + expectedAccountId?: string +): string { + const markerTest = expectedAccountId + ? `test "$contents" = ${shellQuote(expectedAccountId)}` + : 'test -n "$contents"' + return [ + 'set -uo pipefail', + `tag() { printf '${VERDICT_TAG}%s\\n' "$1"; exit 0; }`, + `candidate=${shellQuote(linuxPath)}`, + 'managed_root="${HOME%/}/.local/share/orca/claude-accounts"', + 'candidate_real=$(readlink -f -- "$candidate") || exit 1', + 'managed_root_real=$(readlink -f -- "$managed_root") || exit 1', + 'marker="$candidate_real/.orca-managed-claude-auth"', + 'test -f "$marker" || tag missing-marker', + 'contents=$(cat -- "$marker") || exit 1', + `${markerTest} || tag marker-mismatch`, + 'case "$candidate_real" in "$managed_root_real"/*/auth) ;; *) tag outside-managed-root ;; esac', + "encoded=$(printf '%s' \"$candidate_real\" | base64 | tr -d '\\n') || exit 1", + 'tag "owned:$encoded"' + ].join('\n') +} + +function indeterminate(message: string, cause?: unknown): ClaudeManagedAuthVerdict { + return { kind: 'indeterminate', error: new Error(message, cause ? { cause } : undefined) } +} + +/** Base64 round-trips so a truncated or garbled payload cannot become a path. */ +function decodeCanonicalPath(encoded: string): string | null { + if (!encoded) { + return null + } + const decoded = Buffer.from(encoded, 'base64').toString('utf-8') + return decoded && Buffer.from(decoded, 'utf-8').toString('base64') === encoded ? decoded : null +} + +export function classifyWslManagedAuthProbe( + probe: WslResult, + distro: string +): ClaudeManagedAuthVerdict { + if (probe.timedOut) { + return indeterminate('WSL Claude ownership probe timed out.') + } + if (!probe.environmentResolved) { + return indeterminate('WSL Claude ownership probe could not resolve the distro environment.') + } + if (probe.code !== 0) { + return indeterminate(`WSL Claude ownership probe exited with code ${String(probe.code)}.`) + } + const lines = probe.stdout + .replaceAll(String.fromCharCode(0), '') + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line.length > 0) + const tagged = lines.filter((line) => line.startsWith(VERDICT_TAG)) + // The tag is the guest's last act, so anything after it means the run did not + // end where the protocol says it ends. + if (tagged.length !== 1 || lines.at(-1) !== tagged[0]) { + return indeterminate('WSL Claude ownership probe did not report exactly one verdict.') + } + const value = tagged[0].slice(VERDICT_TAG.length) + if (value === 'missing-marker') { + return { kind: 'untrusted', reason: MISSING_MANAGED_AUTH_MESSAGE } + } + if (value === 'marker-mismatch') { + return { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } + } + if (value === 'outside-managed-root') { + return { kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE } + } + if (!value.startsWith('owned:')) { + return indeterminate('WSL Claude ownership probe reported an unknown verdict.') + } + const canonicalPath = decodeCanonicalPath(value.slice('owned:'.length)) + return canonicalPath + ? { kind: 'owned', authPath: toWindowsWslPath(canonicalPath, distro) } + : indeterminate('WSL Claude ownership probe reported an undecodable path.') +} From c518a6e9c3f6701765931317bff860803bcedada Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 13:29:28 -0700 Subject: [PATCH 2/7] fix(claude): stop over-refusing on removal and under-refusing in the second probe Follow-up to the #17993 review. The tri-state landed in one lane and one caller; five places still turned a failed observation into a verdict, and one turned a user's explicit request into a refusal. - Explicit removal no longer probes at all. The user's request is the authority, so a gate that cannot complete must not leave credentials on disk with nothing in the UI still pointing at them. The directory is derived lexically from the account ID instead, which is stricter than the probe it replaces: the old path deleted `resolve(canonicalAuthPath, '..')` and therefore followed a symlink out of the managed root. Automatic failed-add cleanup keeps requiring a dispositive verdict, and the two authorities are now wired to two different methods rather than sharing one. - runtime-auth had a second, separate WSL probe whose null cleared the user's active account. It now shares the verdict resolver, and sync leaves the selection alone on an indeterminate one. Once ownership is proven, the credential read reuses that path: a second probe could fail where the first succeeded and reach sync as "missing credentials", which clears the selection just the same. - The guest script proved a directory was readable and searchable before believing anything it did not find. `test -f` reports an unsearchable directory exactly as it reports an empty one, so an EACCES was arriving as a dispositive `missing-marker`. It also rejects symlinked candidates and markers now, matching the host gate. - `isDefinitiveAbsence` and the unproven-error predicate are total. Reading `.code` or `.cause` off a caught value can itself throw, which escaped the very path meant to fail closed. A shape that cannot be inspected counts as unproven, because deletion requires proof. - The host-visible WSL branch validates marker contents against the account ID and rejects symlinks, instead of accepting any directory with a marker file. The guest script's tagged-line behaviour is now covered by executing it under bash against a real temp tree, including the mode-000 cases. --- ...ged-auth-fail-once-credential-loss.test.ts | 55 ++++ .../claude-managed-auth-ownership.ts | 28 +- .../claude-managed-auth-storage.ts | 148 ++++++---- .../managed-auth-ownership-verdict.test.ts | 83 ++++++ src/main/claude-accounts/managed-auth-path.ts | 31 +- ...auth-service-wsl-ownership-verdict.test.ts | 267 ++++++++++++++++++ .../runtime-auth-managed-credentials.ts | 98 +++---- .../runtime-auth/runtime-auth-sync.ts | 30 +- src/main/claude-accounts/service.ts | 11 +- .../wsl-managed-auth-probe.test.ts | 198 ++++++++++++- .../claude-accounts/wsl-managed-auth-probe.ts | 129 ++++++++- .../definitive-filesystem-absence.test.ts | 38 +++ src/shared/definitive-filesystem-absence.ts | 23 +- 13 files changed, 981 insertions(+), 158 deletions(-) create mode 100644 src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts create mode 100644 src/shared/definitive-filesystem-absence.test.ts diff --git a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts index b1095eb39c2d..f9e97d73da99 100644 --- a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts +++ b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts @@ -375,6 +375,41 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' expect(existsSync(join(accountsRoot, leaked[0], 'auth', '.credentials.json'))).toBe(false) }) + it("a user-requested removal does not delete a path that is not this account's own storage", async () => { + setPlatform('linux') + const { service, settings, keychainDelete } = await buildService() + await service.addAccount({ runtime: 'host' }) + const account = settings().claudeManagedAccounts[0] as { id: string; managedAuthPath: string } + // Settings tampering, or a stale record: the persisted path is not the one + // Orca would have chosen for this account. + const foreign = mkdtempSync(join(tmpdir(), 'sta5674-foreign-')) + writeFileSync(join(foreign, 'precious.txt'), 'not ours to delete') + account.managedAuthPath = join(foreign, 'auth') + + await service.removeAccount(account.id) + + expect(existsSync(join(foreign, 'precious.txt'))).toBe(true) + expect(settings().claudeManagedAccounts).toHaveLength(0) + expect(keychainDelete).toHaveBeenCalledWith(account.id) + rmSync(foreign, { recursive: true, force: true }) + }) + + it('cleanup after a non-ownership add failure still refuses to delete on an unprovable gate', async () => { + setPlatform('linux') + // The add fails at login, so the failure carries no ownership verdict and + // cleanup's error check cannot short-circuit. The removal gate is then the + // only thing standing between a transient fault and a deleted directory. + const { service, failLoginWith } = await buildService() + failLoginWith(new Error('Claude login was cancelled.')) + // create() is probe 1; cleanup's own gate is probe 2. + authPathMocks.failOwnershipOnCall = 2 + + await expect(service.addAccount({ runtime: 'host' })).rejects.toThrow(/login was cancelled/) + + const { readdirSync } = await import('node:fs') + expect(readdirSync(join(paths.userDataRoot, 'claude-accounts'))).toHaveLength(1) + }) + it('reports the add failure, not the cleanup failure, when cleanup itself throws', async () => { setPlatform('linux') authPathMocks.failOwnershipOnCall = 0 @@ -387,6 +422,26 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' await expect(service.addAccount({ runtime: 'host' })).rejects.toBe(loginFailure) }) + it('a user-requested removal whose probe cannot complete still deletes the account directory', async () => { + setPlatform('linux') + const { service, settings, keychainDelete } = await buildService() + await service.addAccount({ runtime: 'host' }) + const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + // The add used probes 1-3; the removal's own gate is the next one. + authPathMocks.failOwnershipOnCall = 4 + + await service.removeAccount(accountId) + + // The user asked for the account to be gone. Refusing to delete on an + // unprovable probe leaves credentials on disk with no UI reference to them + // — quietly kept is worse than visibly lost. + expect(settings().claudeManagedAccounts).toHaveLength(0) + expect(existsSync(join(accountsRoot, accountId, 'auth', '.credentials.json'))).toBe(false) + expect(existsSync(join(accountsRoot, accountId))).toBe(false) + expect(keychainDelete).toHaveBeenCalledWith(accountId) + }) + it('CONTROL — a user-requested removal still deletes the account directory and its keychain credentials', async () => { setPlatform('linux') authPathMocks.failOwnershipOnCall = 0 diff --git a/src/main/claude-accounts/claude-managed-auth-ownership.ts b/src/main/claude-accounts/claude-managed-auth-ownership.ts index 3dca8c8f13c1..87809bbd5759 100644 --- a/src/main/claude-accounts/claude-managed-auth-ownership.ts +++ b/src/main/claude-accounts/claude-managed-auth-ownership.ts @@ -53,16 +53,36 @@ const MAX_CAUSE_DEPTH = 8 /** * True when `error`, or anything it wraps, reports storage Orca could not read. - * Walks `cause` because a failure crosses several layers before the caller that - * has to decide whether deleting is authorised. + * + * Walks `cause` because the failure crosses several layers before reaching the + * caller that decides whether deleting is authorised, and traverses any object + * rather than only `Error` -- an IPC or dependency boundary can reject with a + * plain wrapper around the typed error. + * + * A value it cannot inspect answers `true`, not `false`: the question is whether + * the failure is *proven* dispositive, and a shape that throws when read has + * proven nothing. Deletion requires proof, so the unreadable case must land on + * the side that keeps the directory. */ export function isUnprovenManagedClaudeAuthError(error: unknown): boolean { + const seen = new Set() let current = error - for (let depth = 0; depth < MAX_CAUSE_DEPTH && current instanceof Error; depth += 1) { + for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth += 1) { + if (current === null || (typeof current !== 'object' && typeof current !== 'function')) { + return false + } + if (seen.has(current)) { + return false + } + seen.add(current) if (current instanceof ManagedClaudeAuthTemporarilyUnavailableError) { return true } - current = (current as { cause?: unknown }).cause + try { + current = (current as { cause?: unknown }).cause + } catch { + return true + } } return false } diff --git a/src/main/claude-accounts/claude-managed-auth-storage.ts b/src/main/claude-accounts/claude-managed-auth-storage.ts index 5cb0e34ecf83..3039d5a2f904 100644 --- a/src/main/claude-accounts/claude-managed-auth-storage.ts +++ b/src/main/claude-accounts/claude-managed-auth-storage.ts @@ -1,14 +1,11 @@ -import { lstatSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, realpathSync, rmSync, writeFileSync } from 'node:fs' import { join, relative, resolve, sep } from 'node:path' -import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { parseWslUncPath } from '../../shared/wsl-paths' import { toWindowsWslPath } from '../wsl' import { runWslProcess } from '../wsl/wsl-runner' import { assertOwnedClaudeManagedAuthPath, MISSING_MANAGED_AUTH_MESSAGE, - OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE, - UNTRUSTED_MANAGED_AUTH_MESSAGE, type ClaudeManagedAuthVerdict } from './claude-managed-auth-ownership' import { @@ -18,10 +15,7 @@ import { resolveClaudeManagedAuthVerdict, writeClaudeManagedAuthFile } from './managed-auth-path' -import { - buildWslManagedAuthProbeScript, - classifyWslManagedAuthProbe -} from './wsl-managed-auth-probe' +import { resolveWslManagedAuthVerdict } from './wsl-managed-auth-probe' import { deleteManagedClaudeKeychainCredentials, readManagedClaudeKeychainCredentials, @@ -147,14 +141,93 @@ export class ClaudeManagedAuthStorage { } } + /** + * Removal the user asked for. Their request is the authority, so this runs no + * ownership probe: a gate that cannot complete must not turn "remove it" into + * "quietly keep it", which leaves credentials on disk with nothing in the UI + * still pointing at them (STA-5674 follow-up). + * + * Safety comes from the spelling instead, and it is stricter than the probe it + * replaces: the old path deleted `resolve(canonicalAuthPath, '..')`, which + * follows a symlink out of the managed root, while this deletes only the + * directory Orca itself would have created for this account ID. + */ async remove(accountId: string, candidatePath: string): Promise { + const accountDir = this.resolveOwnSpellingAccountDir(accountId, candidatePath) + if (accountDir === null) { + console.warn( + '[claude-accounts] Not removing a managed auth path Orca did not choose:', + candidatePath + ) + } else { + try { + // Recursive removal never traverses a symlink -- it unlinks the link -- + // so a planted link cannot redirect this outside the root. + rmSync(accountDir, { recursive: true, force: true }) + } catch (error) { + // Non-throwing by contract: the caller has already committed the + // settings change and must not roll it back over a failed unlink. + console.warn('[claude-accounts] Could not remove managed auth directory:', error) + } + } + await deleteManagedClaudeKeychainCredentials(accountId) + } + + /** + * Cleanup Orca decided to do on its own after a failed add. Unlike an explicit + * removal this has no user intent behind it, so only a dispositive verdict + * authorises deleting anything -- including the keychain entry. + */ + async removeAfterFailedAdd(accountId: string, candidatePath: string): Promise { + const verdict = await this.resolveVerdict(candidatePath, accountId) + if (verdict.kind === 'indeterminate') { + console.warn( + '[claude-accounts] Leaving managed auth in place after a failed add:', + verdict.error + ) + return + } + await this.remove(accountId, candidatePath) + } + + private resolveOwnSpellingAccountDir(accountId: string, candidatePath: string): string | null { + const wslInfo = parseWslUncPath(candidatePath) + if (wslInfo) { + const suffix = `/.local/share/orca/claude-accounts/${accountId}/auth` + return wslInfo.linuxPath.endsWith(suffix) + ? toWindowsWslPath(wslInfo.linuxPath.slice(0, -'/auth'.length), wslInfo.distro) + : null + } + const resolvedCandidate = resolve(candidatePath) + for (const root of this.getAccountsRootSpellings()) { + if (pathsEqual(resolvedCandidate, resolve(root, accountId, 'auth'))) { + return resolve(root, accountId) + } + } + return null + } + + /** + * Both spellings of the accounts root. The persisted path is canonical (the + * gate that produced it resolved symlinks) while `getRoot()` is not, so a + * userData directory behind a symlink makes the two disagree — and this is + * spelling normalisation, not an ownership check, so a failed realpath just + * leaves the lexical spelling to match against. + */ + private getAccountsRootSpellings(): string[] { + let root: string try { - const managedAuthPath = await this.assertOwned(candidatePath, accountId) - rmSync(resolve(managedAuthPath, '..'), { recursive: true, force: true }) + root = resolve(this.getRoot()) } catch (error) { - console.warn('[claude-accounts] Refusing to remove untrusted managed auth:', error) + console.warn('[claude-accounts] Could not resolve the managed accounts root:', error) + return [] + } + try { + const canonicalRoot = realpathSync(root) + return pathsEqual(canonicalRoot, root) ? [root] : [root, canonicalRoot] + } catch { + return [root] } - await deleteManagedClaudeKeychainCredentials(accountId) } async assertOwned(candidatePath: string, expectedAccountId?: string): Promise { @@ -170,7 +243,7 @@ export class ClaudeManagedAuthStorage { ): Promise { const wslInfo = parseWslUncPath(candidatePath) if (wslInfo) { - return this.resolveWslVerdict(candidatePath, wslInfo, expectedAccountId) + return resolveWslManagedAuthVerdict(candidatePath, wslInfo, expectedAccountId) } try { this.getRoot() @@ -232,37 +305,6 @@ export class ClaudeManagedAuthStorage { } } - private async resolveWslVerdict( - candidatePath: string, - wslInfo: NonNullable>, - expectedAccountId?: string - ): Promise { - if ( - !wslInfo.linuxPath.includes('/.local/share/orca/claude-accounts/') || - !wslInfo.linuxPath.endsWith('/auth') - ) { - return { kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE } - } - if (process.platform !== 'win32') { - return resolveHostVisibleGuestVerdict(candidatePath) - } - let probe: Awaited> - try { - probe = await runWslProcess({ - distro: wslInfo.distro, - loginPath: 'none', - shell: 'bash', - script: buildWslManagedAuthProbeScript(wslInfo.linuxPath, expectedAccountId), - timeoutMs: 5000 - }) - } catch (error) { - // A spawn failure says nothing about the directory; it says wsl.exe did - // not run. - return { kind: 'indeterminate', error } - } - return classifyWslManagedAuthProbe(probe, wslInfo.distro) - } - private getRoot(): string { const root = getClaudeManagedAccountsRoot() mkdirSync(root, { recursive: true, mode: 0o700 }) @@ -276,20 +318,6 @@ export class ClaudeManagedAuthStorage { } } -/** - * A guest path that the host can address directly (a non-win32 host reading a - * WSL-spelled record). Only a definitive absence is a verdict; an unreadable - * directory is not evidence that it is a stranger's. - */ -function resolveHostVisibleGuestVerdict(candidatePath: string): ClaudeManagedAuthVerdict { - for (const path of [candidatePath, join(candidatePath, MANAGED_AUTH_MARKER)]) { - try { - lstatSync(path) - } catch (error) { - return isDefinitiveAbsence(error) - ? { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } - : { kind: 'indeterminate', error } - } - } - return { kind: 'owned', authPath: candidatePath } +function pathsEqual(left: string, right: string): boolean { + return process.platform === 'win32' ? left.toLowerCase() === right.toLowerCase() : left === right } diff --git a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts index 632430208423..337d3d4a327e 100644 --- a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts +++ b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { restorePlatform, setPlatform } from './claude-account-service-test-harness' import type * as NodeFsModule from 'node:fs' +import type * as WslPathsModule from '../../shared/wsl-paths' // Fault injection at the filesystem, so the classifier under test is the thing // deciding what an errno means. Keyed by path suffix: the probe reads several @@ -45,6 +46,19 @@ const paths = vi.hoisted(() => ({ userDataRoot: '' })) vi.mock('electron', () => ({ app: { getPath: () => paths.userDataRoot } })) +// Makes a POSIX temp path addressable as a guest path, so the host-visible WSL +// branch can be exercised without a distro. +vi.mock('../../shared/wsl-paths', async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + parseWslUncPath: (path: string) => + path.includes('/.local/share/orca/claude-accounts/') + ? { distro: 'Ubuntu', linuxPath: path } + : original.parseWslUncPath(path) + } +}) + vi.mock('./keychain', () => ({ deleteManagedClaudeKeychainCredentials: vi.fn(async () => {}), readManagedClaudeKeychainCredentials: vi.fn(), @@ -150,14 +164,83 @@ describe('host Claude managed-auth verdict', () => { describe('WSL Claude managed-auth verdict', () => { beforeEach(() => { + fsFaults.lockedReadSuffix = null + fsFaults.lockedLstatSuffix = null paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-verdict-wsl-')) }) afterEach(() => { restorePlatform() + fsFaults.lockedReadSuffix = null + fsFaults.lockedLstatSuffix = null rmSync(paths.userDataRoot, { recursive: true, force: true }) }) + function seedGuestAuth(accountId: string, marker: { contents?: string; symlink?: boolean }) { + const guestAuth = join( + paths.userDataRoot, + 'home/dev/.local/share/orca/claude-accounts', + accountId, + 'auth' + ) + mkdirSync(guestAuth, { recursive: true }) + const markerPath = join(guestAuth, MANAGED_AUTH_MARKER) + if (marker.symlink) { + writeFileSync(join(guestAuth, 'real-marker'), marker.contents ?? '') + symlinkSync('real-marker', markerPath) + } else if (marker.contents !== undefined) { + writeFileSync(markerPath, marker.contents) + } + return guestAuth + } + + it('host-visible guest path: a marker naming another account is not proof of ownership', async () => { + setPlatform('linux') + const guestAuth = seedGuestAuth(ACCOUNT_ID, { contents: 'someone-elses-account\n' }) + const storage = new ClaudeManagedAuthStorage() + expect((await storage.resolveVerdict(guestAuth, ACCOUNT_ID)).kind).toBe('untrusted') + }) + + it('host-visible guest path: a symlinked marker is not proof of ownership', async () => { + setPlatform('linux') + const guestAuth = seedGuestAuth(ACCOUNT_ID, { contents: `${ACCOUNT_ID}\n`, symlink: true }) + const storage = new ClaudeManagedAuthStorage() + expect((await storage.resolveVerdict(guestAuth, ACCOUNT_ID)).kind).toBe('untrusted') + }) + + it('host-visible guest path: a path whose account segment is another account is refused', async () => { + setPlatform('linux') + const guestAuth = seedGuestAuth('another-account', { contents: `${ACCOUNT_ID}\n` }) + const storage = new ClaudeManagedAuthStorage() + expect((await storage.resolveVerdict(guestAuth, ACCOUNT_ID)).kind).toBe('untrusted') + }) + + it('host-visible guest path: a matching marker is owned', async () => { + setPlatform('linux') + const guestAuth = seedGuestAuth(ACCOUNT_ID, { contents: `${ACCOUNT_ID}\n` }) + const storage = new ClaudeManagedAuthStorage() + expect(await storage.resolveVerdict(guestAuth, ACCOUNT_ID)).toMatchObject({ kind: 'owned' }) + }) + + it('host-visible guest path: an unreadable marker is indeterminate', async () => { + setPlatform('linux') + const guestAuth = seedGuestAuth(ACCOUNT_ID, { contents: `${ACCOUNT_ID}\n` }) + fsFaults.lockedReadSuffix = MANAGED_AUTH_MARKER + const storage = new ClaudeManagedAuthStorage() + expect((await storage.resolveVerdict(guestAuth, ACCOUNT_ID)).kind).toBe('indeterminate') + }) + + it('host-visible guest path: an empty marker is not proof, even with no expected account', async () => { + setPlatform('linux') + // `assertOwned(path)` with no account ID still requires the marker to name + // some account; an empty file is a marker Orca never wrote. + const guestAuth = seedGuestAuth(ACCOUNT_ID, { contents: ' \n' }) + const storage = new ClaudeManagedAuthStorage() + expect((await storage.resolveVerdict(guestAuth)).kind).toBe('untrusted') + const named = seedGuestAuth('other-account', { contents: 'other-account\n' }) + expect(await storage.resolveVerdict(named)).toMatchObject({ kind: 'owned' }) + }) + it('refuses a guest path outside the managed accounts root without running a probe', async () => { setPlatform('win32') const storage = new ClaudeManagedAuthStorage() diff --git a/src/main/claude-accounts/managed-auth-path.ts b/src/main/claude-accounts/managed-auth-path.ts index 98f2b1d50c99..3a8d56742774 100644 --- a/src/main/claude-accounts/managed-auth-path.ts +++ b/src/main/claude-accounts/managed-auth-path.ts @@ -1,7 +1,7 @@ import { existsSync, lstatSync, readFileSync, realpathSync, writeFileSync } from 'node:fs' import { join, relative, resolve, sep } from 'node:path' import { app } from 'electron' -import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' +import { isDefinitiveAbsence, readErrnoCode } from '../../shared/definitive-filesystem-absence' import { writeFileAtomically } from '../codex-accounts/fs-utils' import { MISSING_MANAGED_AUTH_MESSAGE, @@ -82,7 +82,9 @@ function classifyReadFailure(error: unknown, absenceReason: string): ClaudeManag : { kind: 'indeterminate', error } } -type MarkerState = { kind: 'valid' | 'invalid' } | { kind: 'indeterminate'; error: unknown } +export type ManagedAuthMarkerState = + | { kind: 'valid' | 'invalid' } + | { kind: 'indeterminate'; error: unknown } function resolveMarkerVerdict( canonicalCandidate: string, @@ -90,7 +92,7 @@ function resolveMarkerVerdict( options: { adoptLegacyMarker?: boolean } ): ClaudeManagedAuthVerdict { const markerPath = join(canonicalCandidate, MANAGED_AUTH_MARKER) - const marker = readMarkerState(markerPath, accountId) + const marker = readManagedAuthMarkerState(markerPath, accountId) if (marker.kind !== 'invalid' || !options.adoptLegacyMarker) { return toMarkerVerdict(marker, canonicalCandidate) } @@ -99,15 +101,15 @@ function resolveMarkerVerdict( } catch (error) { // EEXIST means a marker is present but did not validate: a real trust // failure. Anything else is a write we could not complete. - return (error as NodeJS.ErrnoException | null)?.code === 'EEXIST' + return readErrnoCode(error) === 'EEXIST' ? { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } : { kind: 'indeterminate', error } } - return toMarkerVerdict(readMarkerState(markerPath, accountId), canonicalCandidate) + return toMarkerVerdict(readManagedAuthMarkerState(markerPath, accountId), canonicalCandidate) } function toMarkerVerdict( - marker: MarkerState, + marker: ManagedAuthMarkerState, canonicalCandidate: string ): ClaudeManagedAuthVerdict { if (marker.kind === 'indeterminate') { @@ -118,7 +120,15 @@ function toMarkerVerdict( : { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } } -function readMarkerState(markerPath: string, accountId: string): MarkerState { +/** + * Whether the ownership marker in `markerPath` proves this directory is ours. + * With no `accountId` the marker only has to name some account, which is the + * weaker question `assertOwned` asks when the caller did not supply an ID. + */ +export function readManagedAuthMarkerState( + markerPath: string, + accountId?: string +): ManagedAuthMarkerState { let markerStats: ReturnType try { markerStats = lstatSync(markerPath) @@ -130,13 +140,14 @@ function readMarkerState(markerPath: string, accountId: string): MarkerState { if (markerStats.isSymbolicLink() || !markerStats.isFile()) { return { kind: 'invalid' } } + let contents: string try { - return readFileSync(markerPath, 'utf-8').trim() === accountId - ? { kind: 'valid' } - : { kind: 'invalid' } + contents = readFileSync(markerPath, 'utf-8').trim() } catch (error) { return isDefinitiveAbsence(error) ? { kind: 'invalid' } : { kind: 'indeterminate', error } } + const matches = accountId === undefined ? contents.length > 0 : contents === accountId + return matches ? { kind: 'valid' } : { kind: 'invalid' } } export function readClaudeManagedAuthFile( diff --git a/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts b/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts new file mode 100644 index 000000000000..79be6a6d33c5 --- /dev/null +++ b/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts @@ -0,0 +1,267 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' +import type * as NodeFsModule from 'node:fs' +import type * as WslPathsModule from '../../shared/wsl-paths' +import { + cleanupRuntimeAuthTestState, + createClaudeAccount, + createClaudeCredentialsJson, + createElectronMock, + createManagedClaudeAuth, + createKeychainMock, + createOauthRefreshMock, + createSettings, + createStore, + resetRuntimeAuthTestState, + setPlatform, + testState +} from './runtime-auth-service-test-harness' + +const fsFaults = vi.hoisted(() => ({ lockedRealpathSuffix: null as string | null })) + +vi.mock('node:fs', async (importOriginal) => { + const original = await importOriginal() + const realpathSync = ((path: never, options: never) => { + if ( + fsFaults.lockedRealpathSuffix !== null && + String(path).endsWith(fsFaults.lockedRealpathSuffix) + ) { + const error = new Error(`EBUSY: resource busy or locked, realpath`) as NodeJS.ErrnoException + error.code = 'EBUSY' + throw error + } + return original.realpathSync(path, options) + }) as typeof original.realpathSync + realpathSync.native = original.realpathSync.native + const mocked = { ...original, realpathSync } + return { ...mocked, default: mocked } +}) + +vi.mock('electron', () => createElectronMock()) +vi.mock('./oauth-refresh', () => createOauthRefreshMock()) +vi.mock('./keychain', () => createKeychainMock()) + +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal() // eslint-disable-line @typescript-eslint/consistent-type-imports -- harness parity + return { ...actual, homedir: () => testState.fakeHomeDir } +}) + +// The guest tree lives in the test's temp dir so the credential bytes the sync +// reads after a proven verdict are real bytes; these two shims are what make a +// guest path addressable by `node:fs` on a POSIX host. +vi.mock('../wsl', () => ({ + getDefaultWslDistro: () => 'Ubuntu', + getWslHome: () => null, + toWindowsWslPath: (linuxPath: string) => linuxPath +})) + +vi.mock('../../shared/wsl-paths', async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + parseWslUncPath: (path: string) => + path.includes('/.local/share/orca/claude-accounts/') + ? { distro: 'Ubuntu', linuxPath: path } + : original.parseWslUncPath(path) + } +}) + +const wslMocks = vi.hoisted(() => ({ runWslProcess: vi.fn() })) + +vi.mock('../wsl/wsl-runner', () => ({ + runWslProcess: wslMocks.runWslProcess, + DEFAULT_WSL_TIMEOUT_MS: 30_000 +})) + +const TAG = 'ORCA_CLAUDE_AUTH_VERDICT:' + +function guestAuthPath(): string { + return join(testState.userDataDir, 'guest/.local/share/orca/claude-accounts/ubuntu-account/auth') +} + +function seedGuestAuth(): string { + const authPath = guestAuthPath() + mkdirSync(authPath, { recursive: true }) + writeFileSync(join(authPath, '.orca-managed-claude-auth'), 'ubuntu-account\n') + writeFileSync( + join(authPath, '.credentials.json'), + createClaudeCredentialsJson('alice@example.com', 'alice-token') + ) + writeFileSync(join(authPath, 'oauth-account.json'), '{"accountUuid":"ubuntu-account"}\n') + return authPath +} + +function wslResult(overrides: Record = {}) { + return { + environmentResolved: true, + code: 0, + stdout: '', + stderr: '', + timedOut: false, + ...overrides + } +} + +function buildStore() { + const settings = createSettings({ + localAccountRuntime: 'wsl', + localAccountWslDistro: 'Ubuntu', + claudeManagedAccounts: [ + createClaudeAccount('ubuntu-account', guestAuthPath(), { + managedAuthRuntime: 'wsl', + wslDistro: 'Ubuntu', + wslLinuxAuthPath: guestAuthPath() + }) + ], + activeClaudeManagedAccountId: null, + activeClaudeManagedAccountIdsByRuntime: { host: null, wsl: { Ubuntu: 'ubuntu-account' } } + }) + return createStore(settings) +} + +async function syncWithProbe(probe: ReturnType | Error) { + const store = buildStore() + wslMocks.runWslProcess.mockImplementation(async () => { + if (probe instanceof Error) { + throw probe + } + return probe + }) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + const service = new ClaudeRuntimeAuthService(store as never) + await service.syncForCurrentSelection({ runtime: 'wsl', wslDistro: 'Ubuntu' }) + return store +} + +function selectedUbuntuAccount(store: ReturnType): string | null { + return store.getSettings().activeClaudeManagedAccountIdsByRuntime?.wsl?.Ubuntu ?? null +} + +describe('runtime-auth WSL ownership probe', () => { + beforeEach(() => { + resetRuntimeAuthTestState() + wslMocks.runWslProcess.mockReset() + fsFaults.lockedRealpathSuffix = null + setPlatform('win32') + }) + + afterEach(() => { + fsFaults.lockedRealpathSuffix = null + cleanupRuntimeAuthTestState() + }) + + // Each of these is a probe that did not complete. Clearing the user's active + // WSL account on one is the STA-5674 defect in the second lane: "we could not + // check" reported as "this is not your account". + it.each([ + ['a non-zero exit', wslResult({ code: 1 })], + ['a timeout', wslResult({ timedOut: true, code: null })], + ['an unresolved distro environment', wslResult({ environmentResolved: false })], + ['output with no verdict', wslResult({ stdout: 'bash: base64: not found\n' })], + ['a spawn failure', new Error('wsl.exe is not on PATH')] + ])('keeps the active WSL account when the probe reports %s', async (_label, probe) => { + const store = await syncWithProbe(probe) + expect(selectedUbuntuAccount(store)).toBe('ubuntu-account') + }) + + it('still clears the active WSL account on a completed guest observation of untrust', async () => { + const store = await syncWithProbe(wslResult({ stdout: `${TAG}marker-mismatch\n` })) + expect(selectedUbuntuAccount(store)).toBeNull() + }) + + it('keeps the active WSL account when the guest proves ownership', async () => { + const authPath = seedGuestAuth() + const owned = `${TAG}owned:${Buffer.from(authPath, 'utf-8').toString('base64')}\n` + const store = await syncWithProbe(wslResult({ stdout: owned })) + expect(selectedUbuntuAccount(store)).toBe('ubuntu-account') + }) + + it('clears the active WSL account when a proven-owned directory has no credentials', async () => { + const authPath = guestAuthPath() + mkdirSync(authPath, { recursive: true }) + writeFileSync(join(authPath, '.orca-managed-claude-auth'), 'ubuntu-account\n') + const owned = `${TAG}owned:${Buffer.from(authPath, 'utf-8').toString('base64')}\n` + const store = await syncWithProbe(wslResult({ stdout: owned })) + expect(selectedUbuntuAccount(store)).toBeNull() + }) + + it('does not re-probe after ownership is proven', async () => { + // A second probe can fail where the first succeeded, and its null reaches + // sync as "missing credentials" — which clears the selection just the same. + const authPath = seedGuestAuth() + const owned = `${TAG}owned:${Buffer.from(authPath, 'utf-8').toString('base64')}\n` + const store = buildStore() + let probes = 0 + wslMocks.runWslProcess.mockImplementation(async () => { + probes += 1 + return probes === 1 ? wslResult({ stdout: owned }) : wslResult({ code: 1 }) + }) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + const service = new ClaudeRuntimeAuthService(store as never) + await service.syncForCurrentSelection({ runtime: 'wsl', wslDistro: 'Ubuntu' }) + + // Not a probe-count assertion: `syncForCurrentSelection` legitimately runs + // more than one pass. What must hold is that no pass turns a proven verdict + // into a cleared selection by asking the guest a second time. + expect(probes).toBeGreaterThan(0) + expect(selectedUbuntuAccount(store)).toBe('ubuntu-account') + }) +}) + +describe('runtime-auth host ownership probe', () => { + beforeEach(() => { + resetRuntimeAuthTestState() + fsFaults.lockedRealpathSuffix = null + setPlatform('linux') + }) + + afterEach(() => { + fsFaults.lockedRealpathSuffix = null + cleanupRuntimeAuthTestState() + }) + + async function syncHost() { + const managedAuthPath = createManagedClaudeAuth( + testState.userDataDir, + 'host-account', + createClaudeCredentialsJson('alice@example.com', 'alice-token') + ) + const settings = createSettings({ + claudeManagedAccounts: [createClaudeAccount('host-account', managedAuthPath)], + activeClaudeManagedAccountId: 'host-account', + activeClaudeManagedAccountIdsByRuntime: { host: 'host-account', wsl: {} } + }) + const store = createStore(settings) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + await new ClaudeRuntimeAuthService(store as never).syncForCurrentSelection() + return store + } + + it('keeps the active host account when the ownership check cannot complete', async () => { + fsFaults.lockedRealpathSuffix = join('host-account', 'auth') + const store = await syncHost() + expect(store.getSettings().activeClaudeManagedAccountId).toBe('host-account') + }) + + it('still clears the active host account when its directory is proven gone', async () => { + const store = await (async () => { + const managedAuthPath = createManagedClaudeAuth( + testState.userDataDir, + 'host-account', + createClaudeCredentialsJson('alice@example.com', 'alice-token') + ) + rmSync(managedAuthPath, { recursive: true, force: true }) + const settings = createSettings({ + claudeManagedAccounts: [createClaudeAccount('host-account', managedAuthPath)], + activeClaudeManagedAccountId: 'host-account', + activeClaudeManagedAccountIdsByRuntime: { host: 'host-account', wsl: {} } + }) + const store = createStore(settings) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + await new ClaudeRuntimeAuthService(store as never).syncForCurrentSelection() + return store + })() + expect(store.getSettings().activeClaudeManagedAccountId).toBeNull() + }) +}) diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts index ad68d59eadb7..d819b8a91e36 100644 --- a/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts @@ -1,13 +1,15 @@ -import { existsSync } from 'node:fs' import type { ClaudeManagedAccount } from '../../../shared/managed-account-types' import { parseWslUncPath } from '../../../shared/wsl-paths' -import { toWindowsWslPath } from '../../wsl' -import { runWslProcess } from '../../wsl/wsl-runner' +import { + assertOwnedClaudeManagedAuthPath, + type ClaudeManagedAuthVerdict +} from '../claude-managed-auth-ownership' import { readClaudeManagedAuthFile, - resolveOwnedClaudeManagedAuthPath, + resolveClaudeManagedAuthVerdict, writeClaudeManagedAuthFile } from '../managed-auth-path' +import { resolveWslManagedAuthVerdict } from '../wsl-managed-auth-probe' import { isOauthTokenExpiring, refreshClaudeOauthCredentials } from '../oauth-refresh' import { readManagedClaudeKeychainCredentials, @@ -15,18 +17,22 @@ import { } from '../keychain' import { ClaudeRuntimeAuthCredentialIdentity } from './runtime-auth-credential-identity' -const OWNERSHIP_PROBE_TIMEOUT = 'orca-wsl-ownership-probe-timeout' - -function shellQuote(value: string): string { - return `'${value.replace(/'/g, "'\\''")}'` -} - export class ClaudeRuntimeAuthManagedCredentials extends ClaudeRuntimeAuthCredentialIdentity { protected async readManagedCredentials(account: ClaudeManagedAccount): Promise { const managedAuthPath = await this.getOwnedManagedAuthPath(account) - if (!managedAuthPath) { - return null - } + return managedAuthPath === null ? null : this.readManagedCredentialsAt(account, managedAuthPath) + } + + /** + * Why a separate entry point: the caller that already holds a proven verdict + * must not re-probe. A second probe can fail where the first succeeded, and + * `null` from it reaches `doSyncForCurrentSelection` as "missing credentials", + * which clears the user's selection (STA-5674). + */ + protected async readManagedCredentialsAt( + account: ClaudeManagedAccount, + managedAuthPath: string + ): Promise { if (process.platform === 'darwin') { return readManagedClaudeKeychainCredentials(account.id) } @@ -37,10 +43,9 @@ export class ClaudeRuntimeAuthManagedCredentials extends ClaudeRuntimeAuthCreden account: ClaudeManagedAccount, credentialsJson: string ): Promise { - const managedAuthPath = await this.getOwnedManagedAuthPath(account) - if (!managedAuthPath) { - throw new Error('Managed Claude auth storage is not owned by Orca.') - } + const managedAuthPath = assertOwnedClaudeManagedAuthPath( + await this.resolveManagedAuthVerdict(account) + ) if (process.platform === 'darwin') { await writeManagedClaudeKeychainCredentials(account.id, credentialsJson) return @@ -90,54 +95,23 @@ export class ClaudeRuntimeAuthManagedCredentials extends ClaudeRuntimeAuthCreden } protected async getOwnedManagedAuthPath(account: ClaudeManagedAccount): Promise { + const verdict = await this.resolveManagedAuthVerdict(account) + return verdict.kind === 'owned' ? verdict.authPath : null + } + + /** + * The ownership question, unflattened. Callers that clear the user's active + * account must branch on this rather than on `getOwnedManagedAuthPath`'s null, + * which cannot tell a stranger's directory from a distro that would not start. + */ + protected async resolveManagedAuthVerdict( + account: ClaudeManagedAccount + ): Promise { const wslInfo = parseWslUncPath(account.managedAuthPath) if (wslInfo) { - if ( - !wslInfo.linuxPath.includes('/.local/share/orca/claude-accounts/') || - !wslInfo.linuxPath.endsWith('/auth') - ) { - return null - } - if (process.platform === 'win32') { - try { - const owned = await runWslProcess({ - distro: wslInfo.distro, - loginPath: 'none', - shell: 'bash', - script: [ - 'set -euo pipefail', - `candidate=${shellQuote(wslInfo.linuxPath)}`, - 'managed_root="${HOME%/}/.local/share/orca/claude-accounts"', - 'candidate_real=$(readlink -f -- "$candidate")', - 'managed_root_real=$(readlink -f -- "$managed_root")', - 'test -f "$candidate_real/.orca-managed-claude-auth"', - `test "$(cat "$candidate_real/.orca-managed-claude-auth")" = ${shellQuote(account.id)}`, - 'case "$candidate_real" in "$managed_root_real"/*/auth) printf "%s\\n" "$candidate_real" ;; *) exit 35 ;; esac' - ].join('\n'), - timeoutMs: 5000 - }) - if (owned.timedOut) { - throw new Error(OWNERSHIP_PROBE_TIMEOUT) - } - if (owned.code !== 0) { - return null - } - const canonicalLinuxPath = owned.stdout.trim() - return canonicalLinuxPath ? toWindowsWslPath(canonicalLinuxPath, wslInfo.distro) : null - } catch (error) { - // Why rethrow a timeout: null means "not owned by Orca", and the - // caller persists that -- clearing the user's account selection. A - // slow distro must not decide ownership. Swallowing it here is what - // made the previous guard dead code. - if (error instanceof Error && error.message === OWNERSHIP_PROBE_TIMEOUT) { - throw error - } - return null - } - } - return existsSync(account.managedAuthPath) ? account.managedAuthPath : null + return resolveWslManagedAuthVerdict(account.managedAuthPath, wslInfo, account.id) } - return resolveOwnedClaudeManagedAuthPath(account.id, account.managedAuthPath, { + return resolveClaudeManagedAuthVerdict(account.id, account.managedAuthPath, { adoptLegacyMarker: true }) } diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts index f191689a4466..dc8ce57eca55 100644 --- a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts @@ -94,7 +94,18 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { } if (activeAccount.managedAuthRuntime === 'wsl') { - if (!(await this.getOwnedManagedAuthPath(activeAccount))) { + const wslOwnership = await this.resolveManagedAuthVerdict(activeAccount) + if (wslOwnership.kind === 'indeterminate') { + // Why return rather than clear: a distro that would not start, or a + // probe that timed out, is not evidence about the account. Clearing here + // is how a cold distro used to log the user out (STA-5674). + console.warn( + '[claude-runtime-auth] Could not verify the active WSL managed account; leaving the selection in place', + wslOwnership.error + ) + return + } + if (wslOwnership.kind === 'untrusted') { console.warn( '[claude-runtime-auth] Active WSL managed account is not owned by Orca, restoring system default' ) @@ -110,7 +121,10 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { }) return } - const credentialsJson = await this.readManagedCredentials(activeAccount) + const credentialsJson = await this.readManagedCredentialsAt( + activeAccount, + wslOwnership.authPath + ) if (!credentialsJson || !this.isValidCredentialsJsonObject(credentialsJson)) { console.warn( '[claude-runtime-auth] Active WSL managed account is missing or has invalid credentials, restoring system default' @@ -132,7 +146,15 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { return } - if (!(await this.getOwnedManagedAuthPath(activeAccount))) { + const hostOwnership = await this.resolveManagedAuthVerdict(activeAccount) + if (hostOwnership.kind === 'indeterminate') { + console.warn( + '[claude-runtime-auth] Could not verify the active managed account; leaving the selection in place', + hostOwnership.error + ) + return + } + if (hostOwnership.kind === 'untrusted') { console.warn( '[claude-runtime-auth] Active managed account is not owned by Orca, restoring system default' ) @@ -156,7 +178,7 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { return } - let credentialsJson = await this.readManagedCredentials(activeAccount) + let credentialsJson = await this.readManagedCredentialsAt(activeAccount, hostOwnership.authPath) if (!credentialsJson || !this.isValidCredentialsJsonObject(credentialsJson)) { console.warn( '[claude-runtime-auth] Active managed account is missing or has invalid credentials, restoring system default' diff --git a/src/main/claude-accounts/service.ts b/src/main/claude-accounts/service.ts index 8f1077ba1a53..3a7e98bcbe30 100644 --- a/src/main/claude-accounts/service.ts +++ b/src/main/claude-accounts/service.ts @@ -43,8 +43,11 @@ export class ClaudeAccountService { rateLimits: RateLimitService, private readonly runtimeAuth: ClaudeRuntimeAuthService ) { + // Two different authorities, deliberately wired to two different methods: + // selection removals are the user's explicit request, registration cleanup + // is Orca's own decision after a failure. this.selection = new ClaudeAccountSelection(store, rateLimits, runtimeAuth, (accountId, path) => - this.safeRemoveManagedAuth(accountId, path) + this.storage.remove(accountId, path) ) this.registration = new ClaudeAccountRegistration({ store, @@ -53,7 +56,7 @@ export class ClaudeAccountService { selection: this.selection, createManagedAuth: (accountId, target) => this.storage.create(accountId, target), assertManagedAuth: (path, accountId) => this.storage.assertOwned(path, accountId), - removeManagedAuth: (accountId, path) => this.safeRemoveManagedAuth(accountId, path), + removeManagedAuth: (accountId, path) => this.storage.removeAfterFailedAdd(accountId, path), writeManagedAuth: (accountId, path, captured) => this.writeManagedAuth(accountId, path, captured), writeCredentials: (accountId, path, value) => @@ -192,8 +195,4 @@ export class ClaudeAccountService { ): Promise { return this.storage.writeAuth(accountId, managedAuthPath, captured) } - - private safeRemoveManagedAuth(accountId: string, path: string): Promise { - return this.storage.remove(accountId, path) - } } diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts index 4a87c52278f2..67eacded9ddc 100644 --- a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts +++ b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts @@ -1,4 +1,9 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { execFileSync } from 'node:child_process' +import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { realpathSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { isUnprovenManagedClaudeAuthError, ManagedClaudeAuthTemporarilyUnavailableError, @@ -49,6 +54,21 @@ describe('WSL Claude managed-auth probe classification', () => { kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE }) + expect(probe({ stdout: `${TAG}missing-directory\n` })).toEqual({ + kind: 'untrusted', + reason: MISSING_MANAGED_AUTH_MESSAGE + }) + for (const tag of [ + 'marker-not-a-file', + 'marker-is-symlink', + 'candidate-is-symlink', + 'not-a-directory' + ]) { + expect(probe({ stdout: `${TAG}${tag}\n` })).toEqual({ + kind: 'untrusted', + reason: UNTRUSTED_MANAGED_AUTH_MESSAGE + }) + } }) it('survives a guest path containing a colon and CRLF line endings', () => { @@ -111,9 +131,185 @@ describe('Claude managed-auth error vocabulary', () => { expect(isUnprovenManagedClaudeAuthError('temporarily locked')).toBe(false) }) + it('recognises the typed error through a non-Error wrapper', () => { + // An IPC or dependency boundary can reject with a plain object; the payload + // is still the only evidence of whether the observation completed. + expect( + isUnprovenManagedClaudeAuthError({ + cause: new ManagedClaudeAuthTemporarilyUnavailableError() + }) + ).toBe(true) + }) + + it('treats a shape it cannot inspect as unproven rather than as a clean failure', () => { + const hostile = { + get cause(): unknown { + throw new Error('nope') + } + } + expect(isUnprovenManagedClaudeAuthError(hostile)).toBe(true) + }) + it('terminates on a self-referential cause chain', () => { const looping = new Error('loop') as Error & { cause?: unknown } looping.cause = looping expect(isUnprovenManagedClaudeAuthError(looping)).toBe(false) }) }) + +/** + * The script runs in the guest, so the only honest way to pin what it reports is + * to run it. `test -f` returns false for an unsearchable directory exactly as it + * does for an empty one, and that indistinguishability is the bug this covers. + */ +function bashCanRunTheProbe(): boolean { + try { + execFileSync('bash', ['-c', 'readlink -f -- / >/dev/null && command -v base64 >/dev/null'], { + stdio: 'ignore' + }) + // A root-owned run ignores mode bits, so the permission cases would not deny. + return process.getuid?.() !== 0 + } catch { + return false + } +} + +describe.skipIf(!bashCanRunTheProbe())('WSL guest probe script, executed by bash', () => { + let guestHome = '' + const ACCOUNT = 'acct-1' + + function authPathFor(accountId = ACCOUNT): string { + return join(guestHome, '.local/share/orca/claude-accounts', accountId, 'auth') + } + + function runProbe(candidate: string, expectedAccountId: string | undefined = ACCOUNT) { + const script = buildWslManagedAuthProbeScript(candidate, expectedAccountId) + try { + const stdout = execFileSync('bash', ['-c', script], { + env: { ...process.env, HOME: guestHome }, + encoding: 'utf-8' + }) + return classifyWslManagedAuthProbe( + { environmentResolved: true, code: 0, stdout, stderr: '', timedOut: false }, + 'Ubuntu' + ) + } catch (error) { + const failure = error as { status?: number; stdout?: string } + return classifyWslManagedAuthProbe( + { + environmentResolved: true, + code: failure.status ?? 1, + stdout: failure.stdout ?? '', + stderr: '', + timedOut: false + }, + 'Ubuntu' + ) + } + } + + beforeEach(() => { + guestHome = mkdtempSync(join(tmpdir(), 'sta5674-guest-script-')) + const authPath = authPathFor() + mkdirSync(authPath, { recursive: true }) + writeFileSync(join(authPath, '.orca-managed-claude-auth'), `${ACCOUNT}\n`) + }) + + afterEach(() => { + // Restore search permission first, or the cleanup cannot descend either. + try { + chmodSync(authPathFor(), 0o700) + } catch { + // The case under test may have removed the directory. + } + rmSync(guestHome, { recursive: true, force: true }) + }) + + it('proves ownership of a real marker under the managed root', () => { + // The guest reports the canonical spelling, which is what the caller stores. + expect(runProbe(authPathFor())).toEqual({ + kind: 'owned', + authPath: `\\\\wsl$\\Ubuntu${realpathSync(authPathFor())}` + }) + }) + + it('reports a directory it cannot search as indeterminate, not as a missing marker', () => { + chmodSync(authPathFor(), 0o000) + expect(runProbe(authPathFor()).kind).toBe('indeterminate') + }) + + it('reports a marker it cannot read as indeterminate', () => { + chmodSync(join(authPathFor(), '.orca-managed-claude-auth'), 0o000) + expect(runProbe(authPathFor()).kind).toBe('indeterminate') + }) + + it('reports a genuinely absent marker as a dispositive absence', () => { + rmSync(join(authPathFor(), '.orca-managed-claude-auth')) + expect(runProbe(authPathFor())).toEqual({ + kind: 'untrusted', + reason: MISSING_MANAGED_AUTH_MESSAGE + }) + }) + + it('refuses a candidate that is a file rather than a directory', () => { + const authPath = authPathFor() + rmSync(authPath, { recursive: true }) + writeFileSync(authPath, 'not a directory') + expect(runProbe(authPath)).toEqual({ + kind: 'untrusted', + reason: UNTRUSTED_MANAGED_AUTH_MESSAGE + }) + }) + + it('never reports a dispositive verdict for an absent directory it could not resolve', () => { + // GNU `readlink -f` tolerates one missing trailing component and reaches the + // `missing-directory` tag; BSD and BusyBox fail outright, which exits 1. + // Both land on a refusal — the point is that neither invents a trust verdict. + rmSync(authPathFor(), { recursive: true }) + const verdict = runProbe(authPathFor()) + expect(verdict.kind === 'indeterminate' || verdict.kind === 'untrusted').toBe(true) + if (verdict.kind === 'untrusted') { + expect(verdict.reason).toBe(MISSING_MANAGED_AUTH_MESSAGE) + } + }) + + it('refuses a marker naming another account', () => { + writeFileSync(join(authPathFor(), '.orca-managed-claude-auth'), 'someone-else\n') + expect(runProbe(authPathFor())).toEqual({ + kind: 'untrusted', + reason: UNTRUSTED_MANAGED_AUTH_MESSAGE + }) + }) + + it('refuses a symlinked marker', () => { + const authPath = authPathFor() + rmSync(join(authPath, '.orca-managed-claude-auth')) + writeFileSync(join(authPath, 'real-marker'), `${ACCOUNT}\n`) + symlinkSync('real-marker', join(authPath, '.orca-managed-claude-auth')) + expect(runProbe(authPath)).toEqual({ + kind: 'untrusted', + reason: UNTRUSTED_MANAGED_AUTH_MESSAGE + }) + }) + + it('refuses a symlinked candidate that resolves to a valid auth directory', () => { + // Separate fixture on purpose: with the marker also symlinked, this passes + // on the marker check alone and says nothing about the candidate check. + const linkedCandidate = join(guestHome, '.local/share/orca/claude-accounts', ACCOUNT, 'link') + symlinkSync(authPathFor(), linkedCandidate) + expect(runProbe(linkedCandidate)).toEqual({ + kind: 'untrusted', + reason: UNTRUSTED_MANAGED_AUTH_MESSAGE + }) + }) + + it('refuses a directory outside the managed root', () => { + const outside = join(guestHome, 'elsewhere/auth') + mkdirSync(outside, { recursive: true }) + writeFileSync(join(outside, '.orca-managed-claude-auth'), `${ACCOUNT}\n`) + expect(runProbe(outside)).toEqual({ + kind: 'untrusted', + reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE + }) + }) +}) diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.ts b/src/main/claude-accounts/wsl-managed-auth-probe.ts index bbb212989a3b..8bfbec624e96 100644 --- a/src/main/claude-accounts/wsl-managed-auth-probe.ts +++ b/src/main/claude-accounts/wsl-managed-auth-probe.ts @@ -1,11 +1,17 @@ +import { lstatSync } from 'node:fs' +import { join } from 'node:path' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { toWindowsWslPath } from '../wsl' -import type { WslResult } from '../wsl/wsl-runner' +import { runWslProcess, type WslResult } from '../wsl/wsl-runner' import { MISSING_MANAGED_AUTH_MESSAGE, OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE, UNTRUSTED_MANAGED_AUTH_MESSAGE, type ClaudeManagedAuthVerdict } from './claude-managed-auth-ownership' +import { MANAGED_AUTH_MARKER, readManagedAuthMarkerState } from './managed-auth-path' + +const MANAGED_GUEST_ROOT_SEGMENT = '/.local/share/orca/claude-accounts/' /** * Why a tagged line instead of exit codes: under `set -e` a missing marker, a @@ -40,10 +46,26 @@ export function buildWslManagedAuthProbeScript( `tag() { printf '${VERDICT_TAG}%s\\n' "$1"; exit 0; }`, `candidate=${shellQuote(linuxPath)}`, 'managed_root="${HOME%/}/.local/share/orca/claude-accounts"', + 'test -h "$candidate" && tag candidate-is-symlink', 'candidate_real=$(readlink -f -- "$candidate") || exit 1', 'managed_root_real=$(readlink -f -- "$managed_root") || exit 1', + // A directory we cannot search reports "no such marker" exactly as an empty + // one does, so prove we can look before believing anything we did not find. + // (`readlink -f` above tolerates a missing trailing component on GNU but not + // on BSD/BusyBox; there an absent directory exits 1, which is a refusal too.) + 'if test ! -d "$candidate_real"; then', + ' test -e "$candidate_real" && tag not-a-directory', + ' parent=$(dirname -- "$candidate_real") || exit 1', + ' test -d "$parent" && test -r "$parent" && test -x "$parent" || exit 1', + ' tag missing-directory', + 'fi', + 'test -r "$candidate_real" && test -x "$candidate_real" || exit 1', 'marker="$candidate_real/.orca-managed-claude-auth"', - 'test -f "$marker" || tag missing-marker', + 'test -h "$marker" && tag marker-is-symlink', + 'if test ! -f "$marker"; then', + ' test -e "$marker" && tag marker-not-a-file', + ' tag missing-marker', + 'fi', 'contents=$(cat -- "$marker") || exit 1', `${markerTest} || tag marker-mismatch`, 'case "$candidate_real" in "$managed_root_real"/*/auth) ;; *) tag outside-managed-root ;; esac', @@ -52,6 +74,22 @@ export function buildWslManagedAuthProbeScript( ].join('\n') } +/** + * Every tag the guest emits after an observation it completed. A tag absent from + * this table is not a default — an unknown verdict is indeterminate, because it + * means the guest and this parser disagree about the protocol. + */ +const DISPOSITIVE_TAGS: Record = { + 'missing-directory': MISSING_MANAGED_AUTH_MESSAGE, + 'missing-marker': MISSING_MANAGED_AUTH_MESSAGE, + 'marker-mismatch': UNTRUSTED_MANAGED_AUTH_MESSAGE, + 'marker-not-a-file': UNTRUSTED_MANAGED_AUTH_MESSAGE, + 'marker-is-symlink': UNTRUSTED_MANAGED_AUTH_MESSAGE, + 'candidate-is-symlink': UNTRUSTED_MANAGED_AUTH_MESSAGE, + 'not-a-directory': UNTRUSTED_MANAGED_AUTH_MESSAGE, + 'outside-managed-root': OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE +} + function indeterminate(message: string, cause?: unknown): ClaudeManagedAuthVerdict { return { kind: 'indeterminate', error: new Error(message, cause ? { cause } : undefined) } } @@ -90,14 +128,9 @@ export function classifyWslManagedAuthProbe( return indeterminate('WSL Claude ownership probe did not report exactly one verdict.') } const value = tagged[0].slice(VERDICT_TAG.length) - if (value === 'missing-marker') { - return { kind: 'untrusted', reason: MISSING_MANAGED_AUTH_MESSAGE } - } - if (value === 'marker-mismatch') { - return { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } - } - if (value === 'outside-managed-root') { - return { kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE } + const untrustedReason = DISPOSITIVE_TAGS[value] + if (untrustedReason !== undefined) { + return { kind: 'untrusted', reason: untrustedReason } } if (!value.startsWith('owned:')) { return indeterminate('WSL Claude ownership probe reported an unknown verdict.') @@ -107,3 +140,79 @@ export function classifyWslManagedAuthProbe( ? { kind: 'owned', authPath: toWindowsWslPath(canonicalPath, distro) } : indeterminate('WSL Claude ownership probe reported an undecodable path.') } + +/** + * The one place a WSL-spelled managed auth path is judged, shared by the storage + * gate and the runtime-auth sync so the two lanes cannot drift apart on what a + * failed probe means (STA-5674). + */ +export async function resolveWslManagedAuthVerdict( + candidatePath: string, + wslInfo: { distro: string; linuxPath: string }, + expectedAccountId?: string +): Promise { + if (!hasManagedGuestPathShape(wslInfo.linuxPath, expectedAccountId)) { + return { kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE } + } + if (process.platform !== 'win32') { + return resolveHostVisibleGuestVerdict(candidatePath, expectedAccountId) + } + let probe: WslResult + try { + probe = await runWslProcess({ + distro: wslInfo.distro, + loginPath: 'none', + shell: 'bash', + script: buildWslManagedAuthProbeScript(wslInfo.linuxPath, expectedAccountId), + timeoutMs: 5000 + }) + } catch (error) { + // A spawn failure says nothing about the directory; it says wsl.exe did not run. + return { kind: 'indeterminate', error } + } + return classifyWslManagedAuthProbe(probe, wslInfo.distro) +} + +function hasManagedGuestPathShape(linuxPath: string, expectedAccountId?: string): boolean { + if (!linuxPath.includes(MANAGED_GUEST_ROOT_SEGMENT) || !linuxPath.endsWith('/auth')) { + return false + } + // The account segment is the caller's claim about whose storage this is; + // another account's directory is not evidence about this one. + return ( + expectedAccountId === undefined || + linuxPath.endsWith(`/claude-accounts/${expectedAccountId}/auth`) + ) +} + +/** + * A guest path the host can address directly (a non-win32 host holding a + * WSL-spelled record). Held to the same bar as the guest probe: a symlinked + * candidate or marker, and a marker naming another account, are all refusals. + */ +function resolveHostVisibleGuestVerdict( + candidatePath: string, + expectedAccountId?: string +): ClaudeManagedAuthVerdict { + let candidateStats: ReturnType + try { + candidateStats = lstatSync(candidatePath) + } catch (error) { + return isDefinitiveAbsence(error) + ? { kind: 'untrusted', reason: MISSING_MANAGED_AUTH_MESSAGE } + : { kind: 'indeterminate', error } + } + if (candidateStats.isSymbolicLink() || !candidateStats.isDirectory()) { + return { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } + } + const marker = readManagedAuthMarkerState( + join(candidatePath, MANAGED_AUTH_MARKER), + expectedAccountId + ) + if (marker.kind === 'indeterminate') { + return marker + } + return marker.kind === 'valid' + ? { kind: 'owned', authPath: candidatePath } + : { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } +} diff --git a/src/shared/definitive-filesystem-absence.test.ts b/src/shared/definitive-filesystem-absence.test.ts new file mode 100644 index 000000000000..6b8c122c6a40 --- /dev/null +++ b/src/shared/definitive-filesystem-absence.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { isDefinitiveAbsence } from './definitive-filesystem-absence' + +function errno(code: string): NodeJS.ErrnoException { + const error = new Error(code) as NodeJS.ErrnoException + error.code = code + return error +} + +describe('isDefinitiveAbsence', () => { + it('accepts only the two codes that mean the path is not there', () => { + expect(isDefinitiveAbsence(errno('ENOENT'))).toBe(true) + expect(isDefinitiveAbsence(errno('ENOTDIR'))).toBe(true) + for (const code of ['EPERM', 'EACCES', 'EBUSY', 'EIO', 'UNKNOWN', 'EEXIST']) { + expect(isDefinitiveAbsence(errno(code))).toBe(false) + } + }) + + it('is total over every rejection shape a catch block can receive', () => { + expect(isDefinitiveAbsence(null)).toBe(false) + expect(isDefinitiveAbsence(undefined)).toBe(false) + expect(isDefinitiveAbsence('ENOENT')).toBe(false) + expect(isDefinitiveAbsence(Object.create(null))).toBe(false) + expect(isDefinitiveAbsence({ code: Symbol('ENOENT') })).toBe(false) + }) + + it('does not throw out of the classifier when reading the code throws', () => { + // The predicate exists to keep a failed observation from becoming a verdict. + // Throwing here would escape the fail-closed path it is guarding. + const hostile = { + get code(): string { + throw new Error('nope') + } + } + expect(() => isDefinitiveAbsence(hostile)).not.toThrow() + expect(isDefinitiveAbsence(hostile)).toBe(false) + }) +}) diff --git a/src/shared/definitive-filesystem-absence.ts b/src/shared/definitive-filesystem-absence.ts index 8140d0588e3b..f4a4738996f3 100644 --- a/src/shared/definitive-filesystem-absence.ts +++ b/src/shared/definitive-filesystem-absence.ts @@ -12,6 +12,27 @@ * category error this predicate exists to prevent. */ export function isDefinitiveAbsence(error: unknown): boolean { - const code = (error as NodeJS.ErrnoException | null)?.code + const code = readErrnoCode(error) return code === 'ENOENT' || code === 'ENOTDIR' } + +/** + * The errno of a caught value, or null when there is not one to read. + * + * Why guarded: a caught value is whatever was thrown, and reading `.code` off it + * can itself throw -- a partial error object, a proxy, a getter that rejects. + * That throw would escape the classifier this module exists to keep total, and + * a caller would see a raw failure on the exact path meant to fail closed. + * An unreadable code is never a code. + */ +export function readErrnoCode(error: unknown): string | null { + if (error === null || (typeof error !== 'object' && typeof error !== 'function')) { + return null + } + try { + const code = (error as NodeJS.ErrnoException).code + return typeof code === 'string' ? code : null + } catch { + return null + } +} From d08089071b315a4e6283748e361e8c8030c66870 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 13:50:53 -0700 Subject: [PATCH 3/7] fix(claude): guard the adoption errno read in-lane and leave the shared predicate to STA-5616 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts this branch's edit to `src/shared/definitive-filesystem-absence.ts`. `349510a18e` on `brennanb2025/sta-5616-codex-wsl-ownership` already guards `isDefinitiveAbsence`'s errno read, with the reasoning written out there; two branches rewriting the same shared file only produces a conflict. This branch inherits that fix when the two land, and the reasoning is not restated here. Note the sibling keeps its `readErrnoCode` private, so the import this branch had added would not have resolved even after that change landed. What remains in-lane is the Claude-specific read: the legacy-marker adoption path classifies its own write failure, and `EEXIST` was being read straight off a caught value. A `catch` receives whatever was thrown, so a throwing `code` accessor escaped the branch meant to classify it. Only `EEXIST` is proof of a marker that is already there; anything unreadable is indeterminate. Also spelling out what the previous commit understated: replacing the ownership probe in explicit removal with a lexically derived directory is a security hardening, not a refactor. The path it replaces deleted `resolve(canonicalAuthPath, '..')` — the parent of the *canonicalised* path — so a symlink at the auth directory redirected a recursive delete to the link target's parent, outside the managed root entirely. Deriving `/` from the account ID and requiring the persisted path to spell `//auth` bounds the delete to the directory Orca created, and recursive removal unlinks a symlink rather than following it. Refs STA-5674. --- .../managed-auth-ownership-verdict.test.ts | 30 ++++++++++++++- src/main/claude-accounts/managed-auth-path.ts | 22 ++++++++++- .../definitive-filesystem-absence.test.ts | 38 ------------------- src/shared/definitive-filesystem-absence.ts | 23 +---------- 4 files changed, 49 insertions(+), 64 deletions(-) delete mode 100644 src/shared/definitive-filesystem-absence.test.ts diff --git a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts index 337d3d4a327e..7f032d63a8ab 100644 --- a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts +++ b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts @@ -11,7 +11,8 @@ import type * as WslPathsModule from '../../shared/wsl-paths' // paths and only one of them is meant to be locked. const fsFaults = vi.hoisted(() => ({ lockedReadSuffix: null as string | null, - lockedLstatSuffix: null as string | null + lockedLstatSuffix: null as string | null, + hostileWriteSuffix: null as string | null })) vi.mock('node:fs', async (importOriginal) => { @@ -38,7 +39,18 @@ vi.mock('node:fs', async (importOriginal) => { } return original.lstatSync(path, options) }) as typeof original.lstatSync - const mocked = { ...original, readFileSync, lstatSync } + const writeFileSync = ((path: never, data: never, options: never) => { + if (locked(path, fsFaults.hostileWriteSuffix)) { + // What a `catch` can actually receive: a value whose `code` accessor throws. + throw { + get code(): string { + throw new Error('unreadable errno') + } + } + } + return original.writeFileSync(path, data, options) + }) as typeof original.writeFileSync + const mocked = { ...original, readFileSync, lstatSync, writeFileSync } return { ...mocked, default: mocked } }) @@ -85,6 +97,7 @@ describe('host Claude managed-auth verdict', () => { beforeEach(() => { fsFaults.lockedReadSuffix = null fsFaults.lockedLstatSuffix = null + fsFaults.hostileWriteSuffix = null paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-verdict-')) }) @@ -92,6 +105,7 @@ describe('host Claude managed-auth verdict', () => { restorePlatform() fsFaults.lockedReadSuffix = null fsFaults.lockedLstatSuffix = null + fsFaults.hostileWriteSuffix = null rmSync(paths.userDataRoot, { recursive: true, force: true }) }) @@ -145,6 +159,18 @@ describe('host Claude managed-auth verdict', () => { ).toBe('untrusted') }) + it('does not let an unreadable adoption failure escape as a trust verdict', () => { + // The write can fail for reasons that are not EEXIST, and the thrown value is + // whatever the failing layer produced — reading `.code` off it must not throw + // out of the classifier that exists to fail closed. + const authPath = seedAccount('someone-elses-account\n') + fsFaults.hostileWriteSuffix = MANAGED_AUTH_MARKER + const verdict = resolveClaudeManagedAuthVerdict(ACCOUNT_ID, authPath, { + adoptLegacyMarker: true + }) + expect(verdict.kind).toBe('indeterminate') + }) + it('adopts a legacy directory that has no marker at all', () => { const authPath = seedAccount(null) expect( diff --git a/src/main/claude-accounts/managed-auth-path.ts b/src/main/claude-accounts/managed-auth-path.ts index 3a8d56742774..a5feef1da4eb 100644 --- a/src/main/claude-accounts/managed-auth-path.ts +++ b/src/main/claude-accounts/managed-auth-path.ts @@ -1,7 +1,7 @@ import { existsSync, lstatSync, readFileSync, realpathSync, writeFileSync } from 'node:fs' import { join, relative, resolve, sep } from 'node:path' import { app } from 'electron' -import { isDefinitiveAbsence, readErrnoCode } from '../../shared/definitive-filesystem-absence' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { writeFileAtomically } from '../codex-accounts/fs-utils' import { MISSING_MANAGED_AUTH_MESSAGE, @@ -101,7 +101,7 @@ function resolveMarkerVerdict( } catch (error) { // EEXIST means a marker is present but did not validate: a real trust // failure. Anything else is a write we could not complete. - return readErrnoCode(error) === 'EEXIST' + return isExistingMarkerCollision(error) ? { kind: 'untrusted', reason: UNTRUSTED_MANAGED_AUTH_MESSAGE } : { kind: 'indeterminate', error } } @@ -189,3 +189,21 @@ function isOwnedChildFile(managedAuthPath: string, filePath: string): boolean { const canonicalFilePath = realpathSync(filePath) return canonicalFilePath.startsWith(canonicalAuthPath + sep) } + +/** + * Whether the adoption write lost a race to a marker that is already there. + * + * Guarded for the reason `isDefinitiveAbsence` documents: a `catch` receives + * whatever was thrown, and a `code` accessor that throws would escape the branch + * meant to classify it. Only EEXIST is proof; anything unreadable is not. + */ +function isExistingMarkerCollision(error: unknown): boolean { + if (error === null || typeof error !== 'object') { + return false + } + try { + return (error as { code?: unknown }).code === 'EEXIST' + } catch { + return false + } +} diff --git a/src/shared/definitive-filesystem-absence.test.ts b/src/shared/definitive-filesystem-absence.test.ts deleted file mode 100644 index 6b8c122c6a40..000000000000 --- a/src/shared/definitive-filesystem-absence.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { isDefinitiveAbsence } from './definitive-filesystem-absence' - -function errno(code: string): NodeJS.ErrnoException { - const error = new Error(code) as NodeJS.ErrnoException - error.code = code - return error -} - -describe('isDefinitiveAbsence', () => { - it('accepts only the two codes that mean the path is not there', () => { - expect(isDefinitiveAbsence(errno('ENOENT'))).toBe(true) - expect(isDefinitiveAbsence(errno('ENOTDIR'))).toBe(true) - for (const code of ['EPERM', 'EACCES', 'EBUSY', 'EIO', 'UNKNOWN', 'EEXIST']) { - expect(isDefinitiveAbsence(errno(code))).toBe(false) - } - }) - - it('is total over every rejection shape a catch block can receive', () => { - expect(isDefinitiveAbsence(null)).toBe(false) - expect(isDefinitiveAbsence(undefined)).toBe(false) - expect(isDefinitiveAbsence('ENOENT')).toBe(false) - expect(isDefinitiveAbsence(Object.create(null))).toBe(false) - expect(isDefinitiveAbsence({ code: Symbol('ENOENT') })).toBe(false) - }) - - it('does not throw out of the classifier when reading the code throws', () => { - // The predicate exists to keep a failed observation from becoming a verdict. - // Throwing here would escape the fail-closed path it is guarding. - const hostile = { - get code(): string { - throw new Error('nope') - } - } - expect(() => isDefinitiveAbsence(hostile)).not.toThrow() - expect(isDefinitiveAbsence(hostile)).toBe(false) - }) -}) diff --git a/src/shared/definitive-filesystem-absence.ts b/src/shared/definitive-filesystem-absence.ts index f4a4738996f3..8140d0588e3b 100644 --- a/src/shared/definitive-filesystem-absence.ts +++ b/src/shared/definitive-filesystem-absence.ts @@ -12,27 +12,6 @@ * category error this predicate exists to prevent. */ export function isDefinitiveAbsence(error: unknown): boolean { - const code = readErrnoCode(error) + const code = (error as NodeJS.ErrnoException | null)?.code return code === 'ENOENT' || code === 'ENOTDIR' } - -/** - * The errno of a caught value, or null when there is not one to read. - * - * Why guarded: a caught value is whatever was thrown, and reading `.code` off it - * can itself throw -- a partial error object, a proxy, a getter that rejects. - * That throw would escape the classifier this module exists to keep total, and - * a caller would see a raw failure on the exact path meant to fail closed. - * An unreadable code is never a code. - */ -export function readErrnoCode(error: unknown): string | null { - if (error === null || (typeof error !== 'object' && typeof error !== 'function')) { - return null - } - try { - const code = (error as NodeJS.ErrnoException).code - return typeof code === 'string' ? code : null - } catch { - return null - } -} From e5858f1026848622a3231048ae3a28ef0a6ef55b Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 14:24:48 -0700 Subject: [PATCH 4/7] test(claude): merge the duplicate node:fs import The code-quality lint pass runs a rule the pre-commit hook does not, so a duplicated `node:fs` import passed locally and failed static analysis in CI. --- .../claude-accounts/wsl-managed-auth-probe.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts index 67eacded9ddc..216caacad6c8 100644 --- a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts +++ b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts @@ -1,7 +1,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { execFileSync } from 'node:child_process' -import { chmodSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' -import { realpathSync } from 'node:fs' +import { + chmodSync, + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { From 466a745cb929612bb74bf3718ab76facdb8c61c9 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 16:25:59 -0700 Subject: [PATCH 5/7] fix(claude): make every dispositive verdict prove itself, not just the probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 of review on #17993. The tri-state had reached the ownership probe and one caller; five other places still turned a failed observation into an answer, and one turned a user's request into a refusal. Enumerated every path in the lane that can produce a dispositive verdict — path checks, marker reads, credential reads, error classification, and the four decisions that clear or delete durable state — and fixed them together rather than one at a time. - The credentials read is now a result, not `string | null`. Its null said "there are no credentials", which `doSyncForCurrentSelection` acts on by clearing the user's account; a locked file, an unsearchable directory, or a failed realpath produced the same null. Both sync branches now leave the selection alone on an indeterminate read and clear only on a completed absence or invalid content. The darwin keychain read is classified the same way: its helper resolves null only for a genuine not-found, and every other failure throws. - The account path check requires exactly one segment between the managed root and `auth`. A shell `*` matches `/`, so the guest accepted `/other/acct/auth` as account `acct`, and the host-side `endsWith` agreed. Orca only ever creates `//auth`. - `isDefinitiveAbsence` guards its errno read again. Reverting it last commit was wrong: the sibling STA-5616 fix is on another branch, so this branch shipped a fail-closed predicate that throws. Written byte-identical to `349510a18e` so the merge is a no-op rather than a conflict. - The unproven-error predicate is total. `instanceof` runs a prototype lookup a Proxy can trap and throw from, so it moves inside the guard; a chain longer than the inspection depth counts as unproven, because those links were never looked at. A cycle still answers `false` — every reachable link was seen. - Explicit removal surfaces a failed unlink instead of reporting success. The caller already rolls its settings change back and rethrows, so the account returns and the user can retry; telling them it is gone while the credentials are still on disk is the same silent retention this path was fixed to stop. A root spelling that could not be canonicalised is reported too: with one spelling to compare against, "not ours" was never established. Three tests asserted a failure scenario without injecting the failure — the removal test armed a probe that no longer runs, a darwin test used a one-shot rejection an earlier call consumed, and keychain call counts accumulated across tests. Each now proves the fault reached the code under test. Refs STA-5674. --- ...ged-auth-fail-once-credential-loss.test.ts | 63 ++++++++++++- .../claude-managed-auth-ownership.ts | 14 ++- .../claude-managed-auth-storage.ts | 94 +++++++++++++------ .../managed-auth-ownership-verdict.test.ts | 28 ++++++ src/main/claude-accounts/managed-auth-path.ts | 62 ++++++++++-- ...auth-service-wsl-ownership-verdict.test.ts | 54 ++++++++++- .../runtime-auth-managed-credentials.ts | 27 +++++- .../runtime-auth/runtime-auth-sync.ts | 75 ++++++++------- .../wsl-managed-auth-probe.test.ts | 36 ++++++- .../claude-accounts/wsl-managed-auth-probe.ts | 29 ++++-- .../definitive-filesystem-absence.test.ts | 54 +++++++++++ src/shared/definitive-filesystem-absence.ts | 22 ++++- 12 files changed, 464 insertions(+), 94 deletions(-) create mode 100644 src/shared/definitive-filesystem-absence.test.ts diff --git a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts index f9e97d73da99..d5ad986126d4 100644 --- a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts +++ b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts @@ -66,7 +66,7 @@ vi.mock('../wsl/wsl-runner', () => ({ // function's return value: a transient lock is an errno, and it is the // classifier under test that has to decide what an EBUSY means. A mock that // hands back a pre-decided `null` would be asserting the answer. -const fsFaults = vi.hoisted(() => ({ realpathLocked: false })) +const fsFaults = vi.hoisted(() => ({ realpathLocked: false, rmLocked: false })) vi.mock('node:fs', async (importOriginal) => { const original = await importOriginal() @@ -83,7 +83,17 @@ vi.mock('node:fs', async (importOriginal) => { return original.realpathSync(path, options) }) as typeof original.realpathSync realpathSync.native = original.realpathSync.native - const mocked = { ...original, realpathSync } + const rmSync = ((path: never, options: never) => { + if (fsFaults.rmLocked) { + const error = new Error( + `EBUSY: resource busy or locked, rm '${String(path)}'` + ) as NodeJS.ErrnoException + error.code = 'EBUSY' + throw error + } + return original.rmSync(path, options) + }) as typeof original.rmSync + const mocked = { ...original, realpathSync, rmSync } return { ...mocked, default: mocked } }) @@ -213,6 +223,10 @@ async function buildService( const login = service as unknown as { runClaudeLoginAndCapture: () => Promise } const ownership = await import('./claude-managed-auth-ownership') const keychain = await import('./keychain') + // The factory is cached across `vi.resetModules()`, so call counts would + // otherwise accumulate across tests and make a `not.toHaveBeenCalled` pass or + // fail on test order rather than on behaviour. + vi.mocked(keychain.deleteManagedClaudeKeychainCredentials).mockClear() return { service: service as unknown as ServiceHarness['service'], settings: () => settings, @@ -236,6 +250,7 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' authPathMocks.ownershipCalls = 0 authPathMocks.credentialsAtFault = null fsFaults.realpathLocked = false + fsFaults.rmLocked = false guestHome = mkdtempSync(join(tmpdir(), 'sta5674-guest-')) paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-userdata-')) }) @@ -243,6 +258,7 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' afterEach(() => { restorePlatform() fsFaults.realpathLocked = false + fsFaults.rmLocked = false if (guestHome) { rmSync(guestHome, { recursive: true, force: true }) } @@ -422,17 +438,21 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' await expect(service.addAccount({ runtime: 'host' })).rejects.toBe(loginFailure) }) - it('a user-requested removal whose probe cannot complete still deletes the account directory', async () => { + it('a user-requested removal deletes the account directory without consulting a probe that would fail', async () => { setPlatform('linux') const { service, settings, keychainDelete } = await buildService() await service.addAccount({ runtime: 'host' }) const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id const accountsRoot = join(paths.userDataRoot, 'claude-accounts') - // The add used probes 1-3; the removal's own gate is the next one. + // The add used probes 1-3. Arm the next one to fail: if removal consults an + // ownership gate at all it gets an unprovable answer, and the assertion on + // the call count below is what proves the fault was live rather than unused. authPathMocks.failOwnershipOnCall = 4 await service.removeAccount(accountId) + expect(authPathMocks.ownershipCalls).toBe(3) + // The user asked for the account to be gone. Refusing to delete on an // unprovable probe leaves credentials on disk with no UI reference to them // — quietly kept is worse than visibly lost. @@ -442,6 +462,41 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' expect(keychainDelete).toHaveBeenCalledWith(accountId) }) + it('a user-requested removal that cannot delete the files reports the failure and keeps the account', async () => { + setPlatform('linux') + const { service, settings, keychainDelete } = await buildService() + await service.addAccount({ runtime: 'host' }) + const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + fsFaults.rmLocked = true + + await expect(service.removeAccount(accountId)).rejects.toThrow(/EBUSY/) + + // Telling the user it is gone while the credentials are still on disk is the + // same silent retention as refusing to delete; the account must come back so + // they can retry. + expect(existsSync(join(accountsRoot, accountId, 'auth', '.credentials.json'))).toBe(true) + expect(settings().claudeManagedAccounts).toHaveLength(1) + expect(keychainDelete).not.toHaveBeenCalled() + }) + + it('a user-requested removal that cannot resolve the accounts root refuses rather than reporting success', async () => { + setPlatform('linux') + const { service, settings, keychainDelete } = await buildService() + await service.addAccount({ runtime: 'host' }) + const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id + const accountsRoot = join(paths.userDataRoot, 'claude-accounts') + // Without the canonical spelling a non-match proves nothing: the persisted + // path is canonical while the root is not. + fsFaults.realpathLocked = true + + await expect(service.removeAccount(accountId)).rejects.toThrow(/could not locate/i) + + expect(existsSync(join(accountsRoot, accountId, 'auth', '.credentials.json'))).toBe(true) + expect(settings().claudeManagedAccounts).toHaveLength(1) + expect(keychainDelete).not.toHaveBeenCalled() + }) + it('CONTROL — a user-requested removal still deletes the account directory and its keychain credentials', async () => { setPlatform('linux') authPathMocks.failOwnershipOnCall = 0 diff --git a/src/main/claude-accounts/claude-managed-auth-ownership.ts b/src/main/claude-accounts/claude-managed-auth-ownership.ts index 87809bbd5759..6516ebfb5b7c 100644 --- a/src/main/claude-accounts/claude-managed-auth-ownership.ts +++ b/src/main/claude-accounts/claude-managed-auth-ownership.ts @@ -72,17 +72,23 @@ export function isUnprovenManagedClaudeAuthError(error: unknown): boolean { return false } if (seen.has(current)) { + // A cycle means every reachable link has already been inspected, so this + // is a completed answer rather than an abandoned one. return false } seen.add(current) - if (current instanceof ManagedClaudeAuthTemporarilyUnavailableError) { - return true - } try { + if (current instanceof ManagedClaudeAuthTemporarilyUnavailableError) { + return true + } current = (current as { cause?: unknown }).cause } catch { + // `instanceof` runs a prototype lookup, which a Proxy can trap and throw + // from, and so can a `cause` accessor. Either way the shape defeated + // inspection. return true } } - return false + // Depth exhausted: unlike a cycle, links remain that were never looked at. + return true } diff --git a/src/main/claude-accounts/claude-managed-auth-storage.ts b/src/main/claude-accounts/claude-managed-auth-storage.ts index 3039d5a2f904..ca7a71b6ee40 100644 --- a/src/main/claude-accounts/claude-managed-auth-storage.ts +++ b/src/main/claude-accounts/claude-managed-auth-storage.ts @@ -5,6 +5,7 @@ import { toWindowsWslPath } from '../wsl' import { runWslProcess } from '../wsl/wsl-runner' import { assertOwnedClaudeManagedAuthPath, + ManagedClaudeAuthTemporarilyUnavailableError, MISSING_MANAGED_AUTH_MESSAGE, type ClaudeManagedAuthVerdict } from './claude-managed-auth-ownership' @@ -153,22 +154,28 @@ export class ClaudeManagedAuthStorage { * directory Orca itself would have created for this account ID. */ async remove(accountId: string, candidatePath: string): Promise { - const accountDir = this.resolveOwnSpellingAccountDir(accountId, candidatePath) - if (accountDir === null) { + const target = this.resolveOwnSpellingAccountDir(accountId, candidatePath) + if (target.kind === 'unresolvable') { + // We could not work out which directory this is, so we cannot report it + // gone. Throwing rolls the caller's settings change back, which is what + // keeps "removed" from being said about files that are still there. + throw new ManagedClaudeAuthTemporarilyUnavailableError( + "Orca could not locate this account's files to remove them. Retry in a moment.", + { cause: target.error } + ) + } + if (target.kind === 'foreign') { + // Nothing of ours to delete at a path we never chose; the record can go. console.warn( '[claude-accounts] Not removing a managed auth path Orca did not choose:', candidatePath ) } else { - try { - // Recursive removal never traverses a symlink -- it unlinks the link -- - // so a planted link cannot redirect this outside the root. - rmSync(accountDir, { recursive: true, force: true }) - } catch (error) { - // Non-throwing by contract: the caller has already committed the - // settings change and must not roll it back over a failed unlink. - console.warn('[claude-accounts] Could not remove managed auth directory:', error) - } + // Recursive removal never traverses a symlink -- it unlinks the link -- + // so a planted link cannot redirect this outside the root. A failure here + // must reach the user: silently keeping the files while the account + // disappears from settings is the orphaning this method exists to stop. + rmSync(target.accountDir, { recursive: true, force: true }) } await deleteManagedClaudeKeychainCredentials(accountId) } @@ -190,43 +197,68 @@ export class ClaudeManagedAuthStorage { await this.remove(accountId, candidatePath) } - private resolveOwnSpellingAccountDir(accountId: string, candidatePath: string): string | null { + /** + * Which directory an explicit removal may delete, decided from the persisted + * spelling alone. `foreign` is a completed answer -- this path is not ours -- + * while `unresolvable` means we could not work the question out, and the two + * must not both read as "nothing to do". + */ + private resolveOwnSpellingAccountDir( + accountId: string, + candidatePath: string + ): + | { kind: 'own'; accountDir: string } + | { kind: 'foreign' } + | { kind: 'unresolvable'; error: unknown } { const wslInfo = parseWslUncPath(candidatePath) if (wslInfo) { const suffix = `/.local/share/orca/claude-accounts/${accountId}/auth` return wslInfo.linuxPath.endsWith(suffix) - ? toWindowsWslPath(wslInfo.linuxPath.slice(0, -'/auth'.length), wslInfo.distro) - : null + ? { + kind: 'own', + accountDir: toWindowsWslPath( + wslInfo.linuxPath.slice(0, -'/auth'.length), + wslInfo.distro + ) + } + : { kind: 'foreign' } + } + let spellings: { roots: string[]; canonicalizationError: unknown } + try { + spellings = this.getAccountsRootSpellings() + } catch (error) { + return { kind: 'unresolvable', error } } const resolvedCandidate = resolve(candidatePath) - for (const root of this.getAccountsRootSpellings()) { + for (const root of spellings.roots) { if (pathsEqual(resolvedCandidate, resolve(root, accountId, 'auth'))) { - return resolve(root, accountId) + return { kind: 'own', accountDir: resolve(root, accountId) } } } - return null + // No spelling matched, but one spelling was never computed -- so "not ours" + // is not something we actually established. + return spellings.canonicalizationError === undefined + ? { kind: 'foreign' } + : { kind: 'unresolvable', error: spellings.canonicalizationError } } /** * Both spellings of the accounts root. The persisted path is canonical (the * gate that produced it resolved symlinks) while `getRoot()` is not, so a - * userData directory behind a symlink makes the two disagree — and this is - * spelling normalisation, not an ownership check, so a failed realpath just - * leaves the lexical spelling to match against. + * userData directory behind a symlink makes the two disagree. A failed + * realpath is reported rather than swallowed: with only the lexical spelling + * to compare against, a non-match proves nothing. */ - private getAccountsRootSpellings(): string[] { - let root: string - try { - root = resolve(this.getRoot()) - } catch (error) { - console.warn('[claude-accounts] Could not resolve the managed accounts root:', error) - return [] - } + private getAccountsRootSpellings(): { roots: string[]; canonicalizationError: unknown } { + const root = resolve(this.getRoot()) try { const canonicalRoot = realpathSync(root) - return pathsEqual(canonicalRoot, root) ? [root] : [root, canonicalRoot] - } catch { - return [root] + return { + roots: pathsEqual(canonicalRoot, root) ? [root] : [root, canonicalRoot], + canonicalizationError: undefined + } + } catch (error) { + return { roots: [root], canonicalizationError: error } } } diff --git a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts index 7f032d63a8ab..09b8efc521f5 100644 --- a/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts +++ b/src/main/claude-accounts/managed-auth-ownership-verdict.test.ts @@ -267,6 +267,34 @@ describe('WSL Claude managed-auth verdict', () => { expect(await storage.resolveVerdict(named)).toMatchObject({ kind: 'owned' }) }) + it('refuses a nested account path that merely shares the suffix, without a probe', async () => { + setPlatform('win32') + const storage = new ClaudeManagedAuthStorage() + // Would throw if it reached `runWslProcess`, which is not mocked here. + expect( + await storage.resolveVerdict( + '\\\\wsl$\\Ubuntu\\home\\dev\\.local\\share\\orca\\claude-accounts\\other\\acct\\auth', + 'acct' + ) + ).toEqual({ kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE }) + // The discriminating case: this one really does end in + // `/claude-accounts/acct/auth`, so only a segment-aware check rejects it. + expect( + await storage.resolveVerdict( + '\\\\wsl$\\Ubuntu\\home\\dev\\.local\\share\\orca\\claude-accounts\\nested\\claude-accounts\\acct\\auth', + 'acct' + ) + ).toEqual({ kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE }) + // The one-segment form for the same account is accepted as far as the shape + // check goes, so the rejection above is the nesting and not the account ID. + expect( + await storage.resolveVerdict( + '\\\\wsl$\\Ubuntu\\home\\dev\\.local\\share\\orca\\claude-accounts\\acct\\auth\\deeper', + 'acct' + ) + ).toEqual({ kind: 'untrusted', reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE }) + }) + it('refuses a guest path outside the managed accounts root without running a probe', async () => { setPlatform('win32') const storage = new ClaudeManagedAuthStorage() diff --git a/src/main/claude-accounts/managed-auth-path.ts b/src/main/claude-accounts/managed-auth-path.ts index a5feef1da4eb..d9bc850341d5 100644 --- a/src/main/claude-accounts/managed-auth-path.ts +++ b/src/main/claude-accounts/managed-auth-path.ts @@ -150,19 +150,64 @@ export function readManagedAuthMarkerState( return matches ? { kind: 'valid' } : { kind: 'invalid' } } -export function readClaudeManagedAuthFile( +/** + * Why a result rather than `string | null`: a null told callers "there are no + * credentials", and `doSyncForCurrentSelection` acts on that by clearing the + * user's selected account. A locked file, an unsearchable directory, or a failed + * `realpath` produced the same null, so a transient fault logged the user out + * (STA-5674). A shape that cannot say "failed" is what keeps that from + * recurring. + */ +export type ClaudeManagedAuthFileRead = + | { kind: 'present'; contents: string } + | { kind: 'absent' } + | { kind: 'indeterminate'; error: unknown } + +export function readClaudeManagedAuthFileResult( managedAuthPath: string, filename: '.credentials.json' | 'oauth-account.json' -): string | null { +): ClaudeManagedAuthFileRead { const filePath = resolve(managedAuthPath, filename) + let fileStats: ReturnType try { - if (!isOwnedChildFile(managedAuthPath, filePath)) { - return null - } - return readFileSync(filePath, 'utf-8') - } catch { - return null + fileStats = lstatSync(filePath) + } catch (error) { + return isDefinitiveAbsence(error) ? { kind: 'absent' } : { kind: 'indeterminate', error } + } + // A symlink or a non-file in the credentials position is a completed + // observation that Orca's file is not there, not a failure to look. + if (fileStats.isSymbolicLink() || !fileStats.isFile()) { + return { kind: 'absent' } + } + let canonicalAuthPath: string + let canonicalFilePath: string + try { + canonicalAuthPath = realpathSync(managedAuthPath) + canonicalFilePath = realpathSync(filePath) + } catch (error) { + return isDefinitiveAbsence(error) ? { kind: 'absent' } : { kind: 'indeterminate', error } } + if (!canonicalFilePath.startsWith(canonicalAuthPath + sep)) { + return { kind: 'absent' } + } + try { + return { kind: 'present', contents: readFileSync(filePath, 'utf-8') } + } catch (error) { + return isDefinitiveAbsence(error) ? { kind: 'absent' } : { kind: 'indeterminate', error } + } +} + +/** + * The lossy view, for read-only callers that treat null as "skip this account" + * and touch nothing durable. Anything that clears state must branch on + * `readClaudeManagedAuthFileResult`. + */ +export function readClaudeManagedAuthFile( + managedAuthPath: string, + filename: '.credentials.json' | 'oauth-account.json' +): string | null { + const read = readClaudeManagedAuthFileResult(managedAuthPath, filename) + return read.kind === 'present' ? read.contents : null } export function writeClaudeManagedAuthFile( @@ -177,6 +222,7 @@ export function writeClaudeManagedAuthFile( writeFileAtomically(filePath, contents, { mode: 0o600 }) } +/** Write-path guard only; the read path classifies its own failures above. */ function isOwnedChildFile(managedAuthPath: string, filePath: string): boolean { if ( !existsSync(filePath) || diff --git a/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts b/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts index 79be6a6d33c5..b186ea323d5a 100644 --- a/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts +++ b/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts @@ -18,7 +18,10 @@ import { testState } from './runtime-auth-service-test-harness' -const fsFaults = vi.hoisted(() => ({ lockedRealpathSuffix: null as string | null })) +const fsFaults = vi.hoisted(() => ({ + lockedRealpathSuffix: null as string | null, + lockedReadSuffix: null as string | null +})) vi.mock('node:fs', async (importOriginal) => { const original = await importOriginal() @@ -34,7 +37,15 @@ vi.mock('node:fs', async (importOriginal) => { return original.realpathSync(path, options) }) as typeof original.realpathSync realpathSync.native = original.realpathSync.native - const mocked = { ...original, realpathSync } + const readFileSync = ((path: never, options: never) => { + if (fsFaults.lockedReadSuffix !== null && String(path).endsWith(fsFaults.lockedReadSuffix)) { + const error = new Error('EBUSY: resource busy or locked, read') as NodeJS.ErrnoException + error.code = 'EBUSY' + throw error + } + return original.readFileSync(path, options) + }) as typeof original.readFileSync + const mocked = { ...original, realpathSync, readFileSync } return { ...mocked, default: mocked } }) @@ -143,11 +154,13 @@ describe('runtime-auth WSL ownership probe', () => { resetRuntimeAuthTestState() wslMocks.runWslProcess.mockReset() fsFaults.lockedRealpathSuffix = null + fsFaults.lockedReadSuffix = null setPlatform('win32') }) afterEach(() => { fsFaults.lockedRealpathSuffix = null + fsFaults.lockedReadSuffix = null cleanupRuntimeAuthTestState() }) @@ -177,6 +190,16 @@ describe('runtime-auth WSL ownership probe', () => { expect(selectedUbuntuAccount(store)).toBe('ubuntu-account') }) + it('keeps the active WSL account when the credentials file cannot be read', async () => { + // The ownership probe proved the directory; the second observation is the + // one that failed. A locked file is not a missing one. + const authPath = seedGuestAuth() + fsFaults.lockedReadSuffix = '.credentials.json' + const owned = `${TAG}owned:${Buffer.from(authPath, 'utf-8').toString('base64')}\n` + const store = await syncWithProbe(wslResult({ stdout: owned })) + expect(selectedUbuntuAccount(store)).toBe('ubuntu-account') + }) + it('clears the active WSL account when a proven-owned directory has no credentials', async () => { const authPath = guestAuthPath() mkdirSync(authPath, { recursive: true }) @@ -213,6 +236,7 @@ describe('runtime-auth host ownership probe', () => { beforeEach(() => { resetRuntimeAuthTestState() fsFaults.lockedRealpathSuffix = null + fsFaults.lockedReadSuffix = null setPlatform('linux') }) @@ -244,6 +268,32 @@ describe('runtime-auth host ownership probe', () => { expect(store.getSettings().activeClaudeManagedAccountId).toBe('host-account') }) + it('keeps the active host account when the credentials file cannot be read', async () => { + fsFaults.lockedReadSuffix = '.credentials.json' + const store = await syncHost() + expect(store.getSettings().activeClaudeManagedAccountId).toBe('host-account') + }) + + it('keeps the active host account when the keychain read fails on darwin', async () => { + setPlatform('darwin') + const keychain = await import('./keychain') + const read = vi.mocked(keychain.readManagedClaudeKeychainCredentials) + const original = read.getMockImplementation() + // Every call, not `Once`: sync reads the keychain more than once, and a + // single-shot rejection gets consumed before the read under test. + read.mockRejectedValue(new Error('User interaction is not allowed.')) + try { + const store = await syncHost() + expect(read).toHaveBeenCalled() + expect(store.getSettings().activeClaudeManagedAccountId).toBe('host-account') + } finally { + read.mockReset() + if (original) { + read.mockImplementation(original) + } + } + }) + it('still clears the active host account when its directory is proven gone', async () => { const store = await (async () => { const managedAuthPath = createManagedClaudeAuth( diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts index d819b8a91e36..50ffd6518b7d 100644 --- a/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts @@ -6,8 +6,10 @@ import { } from '../claude-managed-auth-ownership' import { readClaudeManagedAuthFile, + readClaudeManagedAuthFileResult, resolveClaudeManagedAuthVerdict, - writeClaudeManagedAuthFile + writeClaudeManagedAuthFile, + type ClaudeManagedAuthFileRead } from '../managed-auth-path' import { resolveWslManagedAuthVerdict } from '../wsl-managed-auth-probe' import { isOauthTokenExpiring, refreshClaudeOauthCredentials } from '../oauth-refresh' @@ -33,10 +35,29 @@ export class ClaudeRuntimeAuthManagedCredentials extends ClaudeRuntimeAuthCreden account: ClaudeManagedAccount, managedAuthPath: string ): Promise { + const read = await this.readManagedCredentialsResultAt(account, managedAuthPath) + return read.kind === 'present' ? read.contents : null + } + + /** + * The unflattened read, for the caller that clears the user's selection when + * credentials look missing. A locked file is not a missing one. + */ + protected async readManagedCredentialsResultAt( + account: ClaudeManagedAccount, + managedAuthPath: string + ): Promise { if (process.platform === 'darwin') { - return readManagedClaudeKeychainCredentials(account.id) + try { + const contents = await readManagedClaudeKeychainCredentials(account.id) + return contents === null ? { kind: 'absent' } : { kind: 'present', contents } + } catch (error) { + // The keychain helper resolves null only for a genuine not-found; every + // other failure throws, and none of them are an absent credential. + return { kind: 'indeterminate', error } + } } - return readClaudeManagedAuthFile(managedAuthPath, '.credentials.json') + return readClaudeManagedAuthFileResult(managedAuthPath, '.credentials.json') } protected async writeManagedCredentials( diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts index dc8ce57eca55..0d6c8200b5e4 100644 --- a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts @@ -67,16 +67,7 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { } if (!activeAccount) { if (activeAccountId) { - const nextSelection = setSelectedClaudeAccountIdForTarget( - normalizeClaudeRuntimeSelection(settings), - null, - normalizedTarget - ) - this.store.updateSettings({ - activeClaudeManagedAccountId: - normalizedTarget.runtime === 'host' ? null : settings.activeClaudeManagedAccountId, - activeClaudeManagedAccountIdsByRuntime: nextSelection - }) + this.clearSelectionForTarget(settings, normalizedTarget) } if (normalizedTarget.runtime === 'wsl') { return @@ -109,36 +100,26 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { console.warn( '[claude-runtime-auth] Active WSL managed account is not owned by Orca, restoring system default' ) - const nextSelection = setSelectedClaudeAccountIdForTarget( - normalizeClaudeRuntimeSelection(settings), - null, - normalizedTarget - ) - this.store.updateSettings({ - activeClaudeManagedAccountId: - normalizedTarget.runtime === 'host' ? null : settings.activeClaudeManagedAccountId, - activeClaudeManagedAccountIdsByRuntime: nextSelection - }) + this.clearSelectionForTarget(settings, normalizedTarget) return } - const credentialsJson = await this.readManagedCredentialsAt( + const wslCredentials = await this.readManagedCredentialsResultAt( activeAccount, wslOwnership.authPath ) + if (wslCredentials.kind === 'indeterminate') { + console.warn( + '[claude-runtime-auth] Could not read the active WSL managed credentials; leaving the selection in place', + wslCredentials.error + ) + return + } + const credentialsJson = wslCredentials.kind === 'present' ? wslCredentials.contents : null if (!credentialsJson || !this.isValidCredentialsJsonObject(credentialsJson)) { console.warn( '[claude-runtime-auth] Active WSL managed account is missing or has invalid credentials, restoring system default' ) - const nextSelection = setSelectedClaudeAccountIdForTarget( - normalizeClaudeRuntimeSelection(settings), - null, - normalizedTarget - ) - this.store.updateSettings({ - activeClaudeManagedAccountId: - normalizedTarget.runtime === 'host' ? null : settings.activeClaudeManagedAccountId, - activeClaudeManagedAccountIdsByRuntime: nextSelection - }) + this.clearSelectionForTarget(settings, normalizedTarget) return } // Why: WSL managed accounts are isolated by their Linux CLAUDE_CONFIG_DIR; materializing into Windows ~/.claude would mix two auth stores. @@ -178,7 +159,18 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { return } - let credentialsJson = await this.readManagedCredentialsAt(activeAccount, hostOwnership.authPath) + const hostCredentials = await this.readManagedCredentialsResultAt( + activeAccount, + hostOwnership.authPath + ) + if (hostCredentials.kind === 'indeterminate') { + console.warn( + '[claude-runtime-auth] Could not read the active managed credentials; leaving the selection in place', + hostCredentials.error + ) + return + } + let credentialsJson = hostCredentials.kind === 'present' ? hostCredentials.contents : null if (!credentialsJson || !this.isValidCredentialsJsonObject(credentialsJson)) { console.warn( '[claude-runtime-auth] Active managed account is missing or has invalid credentials, restoring system default' @@ -303,4 +295,23 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { this.lastSyncedAccountId = activeAccount.id this.hasMaterializedRuntimeAuth = true } + + /** + * Drop the account selected for `target`, keeping the legacy host field in + * step. Three call sites did this identically. + */ + private clearSelectionForTarget( + settings: ReturnType, + target: ReturnType + ): void { + this.store.updateSettings({ + activeClaudeManagedAccountId: + target.runtime === 'host' ? null : settings.activeClaudeManagedAccountId, + activeClaudeManagedAccountIdsByRuntime: setSelectedClaudeAccountIdForTarget( + normalizeClaudeRuntimeSelection(settings), + null, + target + ) + }) + } } diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts index 216caacad6c8..986b0e167d23 100644 --- a/src/main/claude-accounts/wsl-managed-auth-probe.test.ts +++ b/src/main/claude-accounts/wsl-managed-auth-probe.test.ts @@ -157,7 +157,30 @@ describe('Claude managed-auth error vocabulary', () => { expect(isUnprovenManagedClaudeAuthError(hostile)).toBe(true) }) - it('terminates on a self-referential cause chain', () => { + it('treats a Proxy that rejects prototype inspection as unproven', () => { + const hostile = new Proxy(new Error('wrapped'), { + getPrototypeOf() { + throw new Error('nope') + }, + get() { + throw new Error('nope') + } + }) + expect(isUnprovenManagedClaudeAuthError(hostile)).toBe(true) + }) + + it('treats a chain longer than the inspection depth as unproven', () => { + // The links past the cap were never looked at, so nothing about them is + // proven -- unlike a cycle, where every reachable link has been seen. + let chain: unknown = new ManagedClaudeAuthTemporarilyUnavailableError() + for (let i = 0; i < 12; i += 1) { + chain = new Error(`layer ${i}`, { cause: chain }) + } + expect(isUnprovenManagedClaudeAuthError(chain)).toBe(true) + }) + + it('terminates on a self-referential cause chain, and answers it', () => { + // A cycle is a completed inspection: every reachable link was visited. const looping = new Error('loop') as Error & { cause?: unknown } looping.cause = looping expect(isUnprovenManagedClaudeAuthError(looping)).toBe(false) @@ -310,6 +333,17 @@ describe.skipIf(!bashCanRunTheProbe())('WSL guest probe script, executed by bash }) }) + it('refuses a nested account path that merely shares the suffix', () => { + // `/other/acct/auth` is not `/acct/auth`; a shell `*` matched it. + const nested = join(guestHome, '.local/share/orca/claude-accounts/other', ACCOUNT, 'auth') + mkdirSync(nested, { recursive: true }) + writeFileSync(join(nested, '.orca-managed-claude-auth'), `${ACCOUNT}\n`) + expect(runProbe(nested)).toEqual({ + kind: 'untrusted', + reason: OUTSIDE_MANAGED_AUTH_ROOT_MESSAGE + }) + }) + it('refuses a directory outside the managed root', () => { const outside = join(guestHome, 'elsewhere/auth') mkdirSync(outside, { recursive: true }) diff --git a/src/main/claude-accounts/wsl-managed-auth-probe.ts b/src/main/claude-accounts/wsl-managed-auth-probe.ts index 8bfbec624e96..02f743fb4c5e 100644 --- a/src/main/claude-accounts/wsl-managed-auth-probe.ts +++ b/src/main/claude-accounts/wsl-managed-auth-probe.ts @@ -68,7 +68,14 @@ export function buildWslManagedAuthProbeScript( 'fi', 'contents=$(cat -- "$marker") || exit 1', `${markerTest} || tag marker-mismatch`, - 'case "$candidate_real" in "$managed_root_real"/*/auth) ;; *) tag outside-managed-root ;; esac', + // A shell `*` matches `/` too, so `/*/auth` also matched + // `/other/acct/auth`. Strip the root and require exactly one segment + // before `auth`. + 'rest=${candidate_real#"$managed_root_real"/}', + 'test "$rest" != "$candidate_real" || tag outside-managed-root', + 'account_segment=${rest%/auth}', + 'test "$account_segment" != "$rest" || tag outside-managed-root', + 'case "$account_segment" in \'\'|*/*) tag outside-managed-root ;; esac', "encoded=$(printf '%s' \"$candidate_real\" | base64 | tr -d '\\n') || exit 1", 'tag "owned:$encoded"' ].join('\n') @@ -173,16 +180,22 @@ export async function resolveWslManagedAuthVerdict( return classifyWslManagedAuthProbe(probe, wslInfo.distro) } +/** + * Orca only ever creates `//auth`, so anything with extra + * components between the root and `auth` is not that directory however much of + * the suffix it shares. A suffix test accepted `/other/acct/auth` as + * account `acct`. + */ function hasManagedGuestPathShape(linuxPath: string, expectedAccountId?: string): boolean { - if (!linuxPath.includes(MANAGED_GUEST_ROOT_SEGMENT) || !linuxPath.endsWith('/auth')) { + const rootIndex = linuxPath.indexOf(MANAGED_GUEST_ROOT_SEGMENT) + if (rootIndex === -1) { return false } - // The account segment is the caller's claim about whose storage this is; - // another account's directory is not evidence about this one. - return ( - expectedAccountId === undefined || - linuxPath.endsWith(`/claude-accounts/${expectedAccountId}/auth`) - ) + const segments = linuxPath.slice(rootIndex + MANAGED_GUEST_ROOT_SEGMENT.length).split('/') + if (segments.length !== 2 || segments[0].length === 0 || segments[1] !== 'auth') { + return false + } + return expectedAccountId === undefined || segments[0] === expectedAccountId } /** diff --git a/src/shared/definitive-filesystem-absence.test.ts b/src/shared/definitive-filesystem-absence.test.ts new file mode 100644 index 000000000000..6786c0a3930a --- /dev/null +++ b/src/shared/definitive-filesystem-absence.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import { isDefinitiveAbsence } from './definitive-filesystem-absence' + +function errno(code: string): NodeJS.ErrnoException { + const error = new Error(code) as NodeJS.ErrnoException + error.code = code + return error +} + +describe('isDefinitiveAbsence', () => { + it('accepts only the two codes that mean the path is not there', () => { + expect(isDefinitiveAbsence(errno('ENOENT'))).toBe(true) + expect(isDefinitiveAbsence(errno('ENOTDIR'))).toBe(true) + for (const code of ['EPERM', 'EACCES', 'EBUSY', 'EIO', 'UNKNOWN', 'EEXIST']) { + expect(isDefinitiveAbsence(errno(code))).toBe(false) + } + }) + + it('is total over every rejection shape a catch block can receive', () => { + expect(isDefinitiveAbsence(null)).toBe(false) + expect(isDefinitiveAbsence(undefined)).toBe(false) + expect(isDefinitiveAbsence('ENOENT')).toBe(false) + expect(isDefinitiveAbsence(Object.create(null))).toBe(false) + expect(isDefinitiveAbsence({ code: Symbol('ENOENT') })).toBe(false) + }) + + it('does not throw out of the classifier when reading the code throws', () => { + // The predicate exists to keep a failed observation from becoming a verdict. + // Throwing here would escape the fail-closed path it is guarding. + const hostile = { + get code(): string { + throw new Error('nope') + } + } + expect(() => isDefinitiveAbsence(hostile)).not.toThrow() + expect(isDefinitiveAbsence(hostile)).toBe(false) + }) + + it('does not throw for a Proxy whose traps reject inspection', () => { + const hostile = new Proxy( + {}, + { + get() { + throw new Error('nope') + }, + getPrototypeOf() { + throw new Error('nope') + } + } + ) + expect(() => isDefinitiveAbsence(hostile)).not.toThrow() + expect(isDefinitiveAbsence(hostile)).toBe(false) + }) +}) diff --git a/src/shared/definitive-filesystem-absence.ts b/src/shared/definitive-filesystem-absence.ts index 8140d0588e3b..235d58440d80 100644 --- a/src/shared/definitive-filesystem-absence.ts +++ b/src/shared/definitive-filesystem-absence.ts @@ -12,6 +12,26 @@ * category error this predicate exists to prevent. */ export function isDefinitiveAbsence(error: unknown): boolean { - const code = (error as NodeJS.ErrnoException | null)?.code + const code = readErrnoCode(error) return code === 'ENOENT' || code === 'ENOTDIR' } + +/** + * Why the guard rather than a plain `?.code`: this runs inside `catch` blocks + * that must fail closed, and a `catch` receives whatever was thrown — a string, + * a Proxy, an object whose `code` is an accessor that throws. Reading the + * property directly lets that accessor's exception escape the very catch meant + * to normalise it, so the caller never gets the indeterminate verdict. An + * unreadable or non-string code is not absence; it is not evidence at all. + */ +function readErrnoCode(error: unknown): string | undefined { + if (typeof error !== 'object' || error === null) { + return undefined + } + try { + const code = (error as { code?: unknown }).code + return typeof code === 'string' ? code : undefined + } catch { + return undefined + } +} From e5b751e4afa40ba386173d9b24be2cda1247788c Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 17:07:03 -0700 Subject: [PATCH 6/7] fix(claude): don't clear a managed selection when the outgoing oauth identity is unreadable Item 18 of the enumeration, which I had deferred as "clears nothing durable". That was wrong, and the review's reading of runtime-auth-sync is right. `readManagedOauthAccount`'s null is the third disjunct of the test that decides whether Orca restores the user's system default before dropping a managed selection. `runtimeOauthAccountMatches(null)` is false, so a failed read did not cause a wrong restore -- it caused NO restore, while the `updateSettings({ activeClaudeManagedAccountId: null })` a few lines later ran regardless. The runtime kept holding the managed account's credentials with nothing in Orca pointing at them: not a wrong undo, a missing one. It is also not a rare corner. The disjunct only decides when the account is unchanged AND `hasMaterializedRuntimeAuth` is false, and the service constructor seeds `lastSyncedAccountId` from persisted settings without materializing -- so that is the state of the FIRST sync after every app start with a selected host managed account, and the oauth identity is the only evidence available. The read is now tri-state, and an indeterminate one defers the whole transition: neither the restore nor the clear. Deferring rather than restoring-anyway is deliberate -- `restoreSystemDefaultSnapshotForMissingManagedCredentials` consumes the same oauth value for its own ownership checks, so running it on a failed read would degrade exactly the checks that decide what is safe to overwrite. Malformed JSON stays dispositive: that is a completed observation of the file. The two teardown blocks were byte-identical once both used the hoisted read, so they are now one method, and the selection-clearing helpers moved to their own module to stay under the line cap. Refs STA-5674. --- ...auth-service-wsl-ownership-verdict.test.ts | 99 +++++++++++++++- .../runtime-auth-managed-credentials.ts | 40 +++++-- .../runtime-auth-selection-teardown.ts | 50 ++++++++ .../runtime-auth/runtime-auth-sync.ts | 111 +++++++++--------- 4 files changed, 236 insertions(+), 64 deletions(-) create mode 100644 src/main/claude-accounts/runtime-auth/runtime-auth-selection-teardown.ts diff --git a/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts b/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts index b186ea323d5a..c561098de63a 100644 --- a/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts +++ b/src/main/claude-accounts/runtime-auth-service-wsl-ownership-verdict.test.ts @@ -20,7 +20,9 @@ import { const fsFaults = vi.hoisted(() => ({ lockedRealpathSuffix: null as string | null, - lockedReadSuffix: null as string | null + lockedReadSuffix: null as string | null, + lockedReadHits: 0, + lockedRealpathHits: 0 })) vi.mock('node:fs', async (importOriginal) => { @@ -30,6 +32,7 @@ vi.mock('node:fs', async (importOriginal) => { fsFaults.lockedRealpathSuffix !== null && String(path).endsWith(fsFaults.lockedRealpathSuffix) ) { + fsFaults.lockedRealpathHits += 1 const error = new Error(`EBUSY: resource busy or locked, realpath`) as NodeJS.ErrnoException error.code = 'EBUSY' throw error @@ -39,6 +42,7 @@ vi.mock('node:fs', async (importOriginal) => { realpathSync.native = original.realpathSync.native const readFileSync = ((path: never, options: never) => { if (fsFaults.lockedReadSuffix !== null && String(path).endsWith(fsFaults.lockedReadSuffix)) { + fsFaults.lockedReadHits += 1 const error = new Error('EBUSY: resource busy or locked, read') as NodeJS.ErrnoException error.code = 'EBUSY' throw error @@ -237,6 +241,8 @@ describe('runtime-auth host ownership probe', () => { resetRuntimeAuthTestState() fsFaults.lockedRealpathSuffix = null fsFaults.lockedReadSuffix = null + fsFaults.lockedReadHits = 0 + fsFaults.lockedRealpathHits = 0 setPlatform('linux') }) @@ -294,6 +300,97 @@ describe('runtime-auth host ownership probe', () => { } }) + it('keeps the active host account when the outgoing oauth identity cannot be read', async () => { + // Reaching the teardown needs the credentials gone (a dispositive absence); + // the oauth read is then the only thing deciding whether the user's default + // gets restored before the selection is dropped. + const managedAuthPath = createManagedClaudeAuth( + testState.userDataDir, + 'host-account', + createClaudeCredentialsJson('alice@example.com', 'alice-token') + ) + rmSync(join(managedAuthPath, '.credentials.json')) + fsFaults.lockedReadSuffix = 'oauth-account.json' + const settings = createSettings({ + claudeManagedAccounts: [createClaudeAccount('host-account', managedAuthPath)], + activeClaudeManagedAccountId: 'host-account', + activeClaudeManagedAccountIdsByRuntime: { host: 'host-account', wsl: {} } + }) + const store = createStore(settings) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + await new ClaudeRuntimeAuthService(store as never).syncForCurrentSelection() + + // Prove the injected fault was actually consumed, not merely armed. + expect(fsFaults.lockedReadHits).toBeGreaterThan(0) + // Clearing here without restoring would leave the runtime holding this + // account's credentials with nothing in Orca pointing at them. + expect(store.getSettings().activeClaudeManagedAccountId).toBe('host-account') + }) + + it('clears the active host account when the outgoing oauth file is malformed', async () => { + // Control for the case above: malformed JSON is a completed observation, so + // the teardown must proceed rather than defer forever on a corrupt file. + const managedAuthPath = createManagedClaudeAuth( + testState.userDataDir, + 'host-account', + createClaudeCredentialsJson('alice@example.com', 'alice-token') + ) + rmSync(join(managedAuthPath, '.credentials.json')) + writeFileSync(join(managedAuthPath, 'oauth-account.json'), '{ not json') + const settings = createSettings({ + claudeManagedAccounts: [createClaudeAccount('host-account', managedAuthPath)], + activeClaudeManagedAccountId: 'host-account', + activeClaudeManagedAccountIdsByRuntime: { host: 'host-account', wsl: {} } + }) + const store = createStore(settings) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + await new ClaudeRuntimeAuthService(store as never).syncForCurrentSelection() + + expect(store.getSettings().activeClaudeManagedAccountId).toBeNull() + }) + + it('keeps the selection when the OUTGOING account directory cannot be read during a switch', async () => { + // Switching away: the outgoing account is not the active one, so the restore + // is warranted -- but it consumes the outgoing oauth identity, so an + // unreadable directory means the restore cannot be run safely either. + const outgoing = createManagedClaudeAuth( + testState.userDataDir, + 'outgoing-account', + createClaudeCredentialsJson('alice@example.com', 'alice-token') + ) + const incoming = createManagedClaudeAuth( + testState.userDataDir, + 'incoming-account', + createClaudeCredentialsJson('bob@example.com', 'bob-token') + ) + let settings = createSettings({ + claudeManagedAccounts: [ + createClaudeAccount('outgoing-account', outgoing), + createClaudeAccount('incoming-account', incoming) + ], + activeClaudeManagedAccountId: 'outgoing-account', + activeClaudeManagedAccountIdsByRuntime: { host: 'outgoing-account', wsl: {} } + }) + const store = createStore(settings) + const { ClaudeRuntimeAuthService } = await import('./runtime-auth-service') + const service = new ClaudeRuntimeAuthService(store as never) + await service.syncForCurrentSelection() + + // Now select the incoming account and make it dispositively unusable, so the + // teardown runs with the outgoing account as `previousAccount`. + settings = store.updateSettings({ + activeClaudeManagedAccountId: 'incoming-account', + activeClaudeManagedAccountIdsByRuntime: { host: 'incoming-account', wsl: {} } + }) as typeof settings + rmSync(incoming, { recursive: true, force: true }) + fsFaults.lockedRealpathSuffix = join('outgoing-account', 'auth') + + await service.syncForCurrentSelection() + + expect(fsFaults.lockedRealpathHits).toBeGreaterThan(0) + expect(store.getSettings().activeClaudeManagedAccountId).toBe('incoming-account') + }) + it('still clears the active host account when its directory is proven gone', async () => { const store = await (async () => { const managedAuthPath = createManagedClaudeAuth( diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts index 50ffd6518b7d..4fb4d049b91a 100644 --- a/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-managed-credentials.ts @@ -5,7 +5,6 @@ import { type ClaudeManagedAuthVerdict } from '../claude-managed-auth-ownership' import { - readClaudeManagedAuthFile, readClaudeManagedAuthFileResult, resolveClaudeManagedAuthVerdict, writeClaudeManagedAuthFile, @@ -19,6 +18,18 @@ import { } from '../keychain' import { ClaudeRuntimeAuthCredentialIdentity } from './runtime-auth-credential-identity' +/** + * Why this needs a result too: the null it replaces is the third disjunct of the + * decision that restores the user's system default before Orca clears a managed + * selection. A failed read made that disjunct false, so no restore ran and the + * selection was cleared anyway -- the runtime kept holding managed credentials + * for an account Orca had forgotten. + */ +export type ClaudeManagedOauthRead = + | { kind: 'present'; value: unknown } + | { kind: 'absent' } + | { kind: 'indeterminate'; error: unknown } + export class ClaudeRuntimeAuthManagedCredentials extends ClaudeRuntimeAuthCredentialIdentity { protected async readManagedCredentials(account: ClaudeManagedAccount): Promise { const managedAuthPath = await this.getOwnedManagedAuthPath(account) @@ -103,15 +114,30 @@ export class ClaudeRuntimeAuthManagedCredentials extends ClaudeRuntimeAuthCreden } protected async readManagedOauthAccount(account: ClaudeManagedAccount): Promise { - const managedAuthPath = await this.getOwnedManagedAuthPath(account) - if (!managedAuthPath) { - return null + const read = await this.readManagedOauthAccountResult(account) + return read.kind === 'present' ? read.value : null + } + + protected async readManagedOauthAccountResult( + account: ClaudeManagedAccount + ): Promise { + const verdict = await this.resolveManagedAuthVerdict(account) + if (verdict.kind === 'indeterminate') { + return { kind: 'indeterminate', error: verdict.error } + } + if (verdict.kind === 'untrusted') { + return { kind: 'absent' } + } + const read = readClaudeManagedAuthFileResult(verdict.authPath, 'oauth-account.json') + if (read.kind !== 'present') { + return read } try { - const contents = readClaudeManagedAuthFile(managedAuthPath, 'oauth-account.json') - return contents ? (JSON.parse(contents) as unknown) : null + return { kind: 'present', value: JSON.parse(read.contents) as unknown } } catch { - return null + // Malformed JSON is a completed observation of the file, not a failure to + // read it. + return { kind: 'absent' } } } diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-selection-teardown.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-selection-teardown.ts new file mode 100644 index 000000000000..70c2f60cd7ac --- /dev/null +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-selection-teardown.ts @@ -0,0 +1,50 @@ +import type { Store } from '../../persistence' +import { + normalizeClaudeRuntimeSelection, + setSelectedClaudeAccountIdForTarget, + type ClaudeAccountSelectionTarget +} from '../runtime-selection' +import type { ClaudeManagedOauthRead } from './runtime-auth-managed-credentials' + +/** + * Drop the account selected for `target`, keeping the legacy host field in step. + * Three call sites in the sync did this identically. + */ +export function clearClaudeSelectionForTarget( + store: Store, + settings: ReturnType, + target: ClaudeAccountSelectionTarget +): void { + store.updateSettings({ + activeClaudeManagedAccountId: + target.runtime === 'host' ? null : settings.activeClaudeManagedAccountId, + activeClaudeManagedAccountIdsByRuntime: setSelectedClaudeAccountIdForTarget( + normalizeClaudeRuntimeSelection(settings), + null, + target + ) + }) +} + +/** + * The outgoing account's oauth identity is the only evidence that the runtime is + * still holding THIS account's credentials, and it is the sole decider on the + * first sync after a restart: the service seeds `lastSyncedAccountId` from + * settings without materializing, so the account is unchanged and + * `hasMaterializedRuntimeAuth` is false. + * + * If it could not be read we can neither prove a restore is needed nor safely + * run one -- the restore consumes the same value for its own ownership checks -- + * so the caller must leave the whole transition for the next sync rather than + * clear the selection without putting the user's default back. + */ +export function shouldDeferOnUnreadableOauth(read: ClaudeManagedOauthRead | null): boolean { + if (read?.kind !== 'indeterminate') { + return false + } + console.warn( + '[claude-runtime-auth] Could not read the outgoing account oauth identity; leaving the selection in place', + read.error + ) + return true +} diff --git a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts index 0d6c8200b5e4..2b1c338fc0af 100644 --- a/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts +++ b/src/main/claude-accounts/runtime-auth/runtime-auth-sync.ts @@ -2,14 +2,18 @@ import { existsSync, readFileSync } from 'node:fs' import { getSelectedClaudeAccountIdForTarget, normalizeClaudeAccountSelectionTarget, - normalizeClaudeRuntimeSelection, - setSelectedClaudeAccountIdForTarget, type ClaudeAccountSelectionTarget } from '../runtime-selection' import { hasLiveClaudePtys } from '../live-pty-gate' import { isOauthTokenExpiring } from '../oauth-refresh' import { writeActiveClaudeKeychainCredentialsForRuntime } from '../keychain' import { ClaudeRuntimeAuthPreparationService } from './runtime-auth-preparation' +import type { ClaudeManagedAccount } from '../../../shared/managed-account-types' +import type { ClaudeManagedOauthRead } from './runtime-auth-managed-credentials' +import { + clearClaudeSelectionForTarget, + shouldDeferOnUnreadableOauth +} from './runtime-auth-selection-teardown' export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { protected async doSyncForCurrentSelection(target?: ClaudeAccountSelectionTarget): Promise { @@ -26,9 +30,11 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { const previousManagedCredentialsJson = previousAccount ? await this.readManagedCredentials(previousAccount) : null - const previousManagedOauthAccount = previousAccount - ? await this.readManagedOauthAccount(previousAccount) + const previousManagedOauthRead = previousAccount + ? await this.readManagedOauthAccountResult(previousAccount) : null + const previousManagedOauthAccount = + previousManagedOauthRead?.kind === 'present' ? previousManagedOauthRead.value : null if (previousAccount && previousAccount.id !== activeAccount?.id) { if (previousManagedCredentialsJson) { const outgoingReadBackResult = await this.readBackRefreshedTokens( @@ -67,7 +73,7 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { } if (!activeAccount) { if (activeAccountId) { - this.clearSelectionForTarget(settings, normalizedTarget) + clearClaudeSelectionForTarget(this.store, settings, normalizedTarget) } if (normalizedTarget.runtime === 'wsl') { return @@ -100,7 +106,7 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { console.warn( '[claude-runtime-auth] Active WSL managed account is not owned by Orca, restoring system default' ) - this.clearSelectionForTarget(settings, normalizedTarget) + clearClaudeSelectionForTarget(this.store, settings, normalizedTarget) return } const wslCredentials = await this.readManagedCredentialsResultAt( @@ -119,7 +125,7 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { console.warn( '[claude-runtime-auth] Active WSL managed account is missing or has invalid credentials, restoring system default' ) - this.clearSelectionForTarget(settings, normalizedTarget) + clearClaudeSelectionForTarget(this.store, settings, normalizedTarget) return } // Why: WSL managed accounts are isolated by their Linux CLAUDE_CONFIG_DIR; materializing into Windows ~/.claude would mix two auth stores. @@ -139,23 +145,12 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { console.warn( '[claude-runtime-auth] Active managed account is not owned by Orca, restoring system default' ) - if (this.lastSyncedAccountId !== null) { - if ( - previousAccount && - (previousAccount.id !== activeAccount.id || - this.hasMaterializedRuntimeAuth || - this.runtimeOauthAccountMatches(await this.readManagedOauthAccount(previousAccount))) - ) { - await this.restoreSystemDefaultSnapshotForMissingManagedCredentials( - previousAccount, - previousManagedOauthAccount - ) - } else if (!previousAccount && this.hasMaterializedRuntimeAuth) { - await this.restoreSystemDefaultSnapshot(this.lastWrittenCredentialsJson, undefined) - } - } - this.store.updateSettings({ activeClaudeManagedAccountId: null }) - this.lastSyncedAccountId = null + await this.restoreDefaultAndDropHostSelection( + activeAccount, + previousAccount, + previousManagedOauthRead, + previousManagedOauthAccount + ) return } @@ -175,23 +170,12 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { console.warn( '[claude-runtime-auth] Active managed account is missing or has invalid credentials, restoring system default' ) - if (this.lastSyncedAccountId !== null) { - if ( - previousAccount && - (previousAccount.id !== activeAccount.id || - this.hasMaterializedRuntimeAuth || - this.runtimeOauthAccountMatches(previousManagedOauthAccount)) - ) { - await this.restoreSystemDefaultSnapshotForMissingManagedCredentials( - previousAccount, - previousManagedOauthAccount - ) - } else if (!previousAccount && this.hasMaterializedRuntimeAuth) { - await this.restoreSystemDefaultSnapshot(this.lastWrittenCredentialsJson, undefined) - } - } - this.store.updateSettings({ activeClaudeManagedAccountId: null }) - this.lastSyncedAccountId = null + await this.restoreDefaultAndDropHostSelection( + activeAccount, + previousAccount, + previousManagedOauthRead, + previousManagedOauthAccount + ) return } @@ -297,21 +281,36 @@ export class ClaudeRuntimeAuthSync extends ClaudeRuntimeAuthPreparationService { } /** - * Drop the account selected for `target`, keeping the legacy host field in - * step. Three call sites did this identically. + * Undo the managed materialization, if we can establish that it happened, then + * drop the host selection. When the oauth identity cannot be read this does + * nothing at all -- neither the restore nor the clear -- so the next sync + * retries the whole transition rather than completing half of it. */ - private clearSelectionForTarget( - settings: ReturnType, - target: ReturnType - ): void { - this.store.updateSettings({ - activeClaudeManagedAccountId: - target.runtime === 'host' ? null : settings.activeClaudeManagedAccountId, - activeClaudeManagedAccountIdsByRuntime: setSelectedClaudeAccountIdForTarget( - normalizeClaudeRuntimeSelection(settings), - null, - target - ) - }) + private async restoreDefaultAndDropHostSelection( + activeAccount: ClaudeManagedAccount, + previousAccount: ClaudeManagedAccount | null, + previousManagedOauthRead: ClaudeManagedOauthRead | null, + previousManagedOauthAccount: unknown + ): Promise { + if (this.lastSyncedAccountId !== null) { + if (shouldDeferOnUnreadableOauth(previousManagedOauthRead)) { + return + } + if ( + previousAccount && + (previousAccount.id !== activeAccount.id || + this.hasMaterializedRuntimeAuth || + this.runtimeOauthAccountMatches(previousManagedOauthAccount)) + ) { + await this.restoreSystemDefaultSnapshotForMissingManagedCredentials( + previousAccount, + previousManagedOauthAccount + ) + } else if (!previousAccount && this.hasMaterializedRuntimeAuth) { + await this.restoreSystemDefaultSnapshot(this.lastWrittenCredentialsJson, undefined) + } + } + this.store.updateSettings({ activeClaudeManagedAccountId: null }) + this.lastSyncedAccountId = null } } From 29f7d4e4a9a94bb1c1e768d76d5c137be4073cd4 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Tue, 1 Sep 2026 17:51:19 -0700 Subject: [PATCH 7/7] test(claude): build the two-spelling removal case instead of inheriting it from the OS CI caught this and local green could not: the removal test that asserts a refusal when the accounts root cannot be canonicalised only reaches that branch when the persisted path differs from the literal root. On macOS it does, because /var is a symlink and the ownership gate stores the canonical spelling. On Linux CI it does not, so the literal spelling matched on the first pass, ownership was genuinely established from a successful observation, and removal correctly proceeded -- the test asserted a branch that had nothing to reach it. The scenario is now constructed: userData is a symlink to a real directory, so the canonical persisted path cannot equal the literal root on any platform. Two preconditions assert that mismatch before the fault is armed, so the test cannot silently decay back into a one-spelling test. Every fault-injecting test in this file now asserts its fault actually fired. `credentialsAtFault === null` in particular was passing both when the gate saw no credentials and when the fault never ran at all. No product code changed; the behaviour CI exercised was correct. Refs STA-5674. --- ...ged-auth-fail-once-credential-loss.test.ts | 71 ++++++++++++++++--- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts index d5ad986126d4..25cb11ce5082 100644 --- a/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts +++ b/src/main/claude-accounts/claude-managed-auth-fail-once-credential-loss.test.ts @@ -1,5 +1,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { restorePlatform, setPlatform } from './claude-account-service-test-harness' @@ -66,12 +75,18 @@ vi.mock('../wsl/wsl-runner', () => ({ // function's return value: a transient lock is an errno, and it is the // classifier under test that has to decide what an EBUSY means. A mock that // hands back a pre-decided `null` would be asserting the answer. -const fsFaults = vi.hoisted(() => ({ realpathLocked: false, rmLocked: false })) +const fsFaults = vi.hoisted(() => ({ + realpathLocked: false, + realpathHits: 0, + rmLocked: false, + rmHits: 0 +})) vi.mock('node:fs', async (importOriginal) => { const original = await importOriginal() const realpathSync = ((path: Parameters[0], options?: never) => { if (fsFaults.realpathLocked) { + fsFaults.realpathHits += 1 const error = new Error( `EBUSY: resource busy or locked, realpath '${String(path)}'` ) as NodeJS.ErrnoException @@ -85,6 +100,7 @@ vi.mock('node:fs', async (importOriginal) => { realpathSync.native = original.realpathSync.native const rmSync = ((path: never, options: never) => { if (fsFaults.rmLocked) { + fsFaults.rmHits += 1 const error = new Error( `EBUSY: resource busy or locked, rm '${String(path)}'` ) as NodeJS.ErrnoException @@ -100,6 +116,7 @@ vi.mock('node:fs', async (importOriginal) => { const authPathMocks = vi.hoisted(() => ({ failOwnershipOnCall: 0, ownershipCalls: 0, + faultsFired: 0, credentialsAtFault: null as string | null })) @@ -110,6 +127,7 @@ vi.mock('./managed-auth-path', async (importOriginal) => { if (authPathMocks.ownershipCalls !== authPathMocks.failOwnershipOnCall) { return run() } + authPathMocks.faultsFired += 1 authPathMocks.credentialsAtFault = readIfPresent(`${candidatePath}/.credentials.json`) fsFaults.realpathLocked = true try { @@ -242,15 +260,20 @@ async function buildService( describe('STA-5674: fail-once ownership probe during Claude managed-account add', () => { let guestHome: string | null = null + const extraTempDirs: string[] = [] beforeEach(() => { vi.resetModules() wslMocks.runWslProcess.mockReset() authPathMocks.failOwnershipOnCall = 0 authPathMocks.ownershipCalls = 0 + authPathMocks.faultsFired = 0 authPathMocks.credentialsAtFault = null fsFaults.realpathLocked = false + fsFaults.realpathHits = 0 fsFaults.rmLocked = false + fsFaults.rmHits = 0 + extraTempDirs.length = 0 guestHome = mkdtempSync(join(tmpdir(), 'sta5674-guest-')) paths.userDataRoot = mkdtempSync(join(tmpdir(), 'sta5674-userdata-')) }) @@ -259,6 +282,9 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' restorePlatform() fsFaults.realpathLocked = false fsFaults.rmLocked = false + for (const dir of extraTempDirs) { + rmSync(dir, { recursive: true, force: true }) + } if (guestHome) { rmSync(guestHome, { recursive: true, force: true }) } @@ -346,6 +372,7 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' expect(failure).toBeInstanceOf(temporarilyUnavailable) const accountsRoot = join(paths.userDataRoot, 'claude-accounts') // The fault landed after real credential bytes existed. + expect(authPathMocks.faultsFired).toBe(1) expect(authPathMocks.credentialsAtFault).toBe(CREDENTIALS) const accountId = (settings().claudeManagedAccounts[0] as { id?: string } | undefined)?.id expect(accountId).toBeUndefined() @@ -381,8 +408,9 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' const { service } = await buildService() await expect(service.addAccount({ runtime: 'host' })).rejects.toThrow(/temporarily locked/) - // The directory is left in place either way, but there were never any - // credentials to lose — an "assert credentials survived" test here is vacuous. + // `credentialsAtFault` is null both when the gate saw no credentials and + // when the fault never fired, so assert the fault first. + expect(authPathMocks.faultsFired).toBe(1) expect(authPathMocks.credentialsAtFault).toBeNull() const { readdirSync } = await import('node:fs') const accountsRoot = join(paths.userDataRoot, 'claude-accounts') @@ -422,6 +450,7 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' await expect(service.addAccount({ runtime: 'host' })).rejects.toThrow(/login was cancelled/) + expect(authPathMocks.faultsFired).toBe(1) const { readdirSync } = await import('node:fs') expect(readdirSync(join(paths.userDataRoot, 'claude-accounts'))).toHaveLength(1) }) @@ -472,6 +501,8 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' await expect(service.removeAccount(accountId)).rejects.toThrow(/EBUSY/) + expect(fsFaults.rmHits).toBeGreaterThan(0) + // Telling the user it is gone while the credentials are still on disk is the // same silent retention as refusing to delete; the account must come back so // they can retry. @@ -482,17 +513,35 @@ describe('STA-5674: fail-once ownership probe during Claude managed-account add' it('a user-requested removal that cannot resolve the accounts root refuses rather than reporting success', async () => { setPlatform('linux') + // Build the two-spelling situation rather than inheriting it: userData is a + // symlink, so the persisted path (canonical, from the ownership gate) cannot + // equal the literal root and canonicalising the root is genuinely required. + // Relying on the OS for this passed on macOS, where /var is a symlink, and + // silently tested nothing on Linux CI, where it is not. + const realRoot = mkdtempSync(join(tmpdir(), 'sta5674-realroot-')) + const linkHome = mkdtempSync(join(tmpdir(), 'sta5674-linkhome-')) + extraTempDirs.push(realRoot, linkHome) + const linkedRoot = join(linkHome, 'userdata') + symlinkSync(realRoot, linkedRoot, 'dir') + paths.userDataRoot = linkedRoot + const { service, settings, keychainDelete } = await buildService() await service.addAccount({ runtime: 'host' }) - const accountId = (settings().claudeManagedAccounts[0] as { id: string }).id - const accountsRoot = join(paths.userDataRoot, 'claude-accounts') - // Without the canonical spelling a non-match proves nothing: the persisted - // path is canonical while the root is not. - fsFaults.realpathLocked = true + const account = settings().claudeManagedAccounts[0] as { id: string; managedAuthPath: string } + // Precondition, so this cannot silently decay back into a one-spelling test: + // the persisted path is the canonical spelling and the literal root is not a + // prefix of it. + expect(account.managedAuthPath.startsWith(realpathSync(realRoot))).toBe(true) + expect(account.managedAuthPath.startsWith(linkedRoot)).toBe(false) - await expect(service.removeAccount(accountId)).rejects.toThrow(/could not locate/i) + fsFaults.realpathLocked = true + await expect(service.removeAccount(account.id)).rejects.toThrow(/could not locate/i) - expect(existsSync(join(accountsRoot, accountId, 'auth', '.credentials.json'))).toBe(true) + // The refusal came from the locked canonicalisation, not from never looking. + expect(fsFaults.realpathHits).toBeGreaterThan(0) + expect( + existsSync(join(realRoot, 'claude-accounts', account.id, 'auth', '.credentials.json')) + ).toBe(true) expect(settings().claudeManagedAccounts).toHaveLength(1) expect(keychainDelete).not.toHaveBeenCalled() })