diff --git a/mac/Sources/CodeBurnMenubar/AppStore.swift b/mac/Sources/CodeBurnMenubar/AppStore.swift index 33ac8d12..98bd91f8 100644 --- a/mac/Sources/CodeBurnMenubar/AppStore.swift +++ b/mac/Sources/CodeBurnMenubar/AppStore.swift @@ -1069,6 +1069,13 @@ final class AppStore { // resumes after this point detects the disconnect and discards its // result instead of re-populating the cleared state. claudeRefreshGen &+= 1 + if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.keychainDeletedOrAbsent { + // Keychain item still present — keep Connect/Disconnect on the + // connected path so the user can retry delete. + subscriptionError = "Could not fully remove the local Claude credential cache. Disconnect again to retry." + NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) + return + } subscription = nil if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess { subscriptionError = "Could not fully remove the local Claude credential cache." @@ -1137,6 +1144,11 @@ final class AppStore { func disconnectCodex() { CodexSubscriptionService.disconnect() codexRefreshGen &+= 1 + if let result = CodexCredentialStore.lastCacheDeleteResult, !result.keychainDeletedOrAbsent { + codexError = "Could not fully remove the local Codex credential cache. Disconnect again to retry." + NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil) + return + } codexUsage = nil if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess { codexError = "Could not fully remove the local Codex credential cache." diff --git a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift index d9e691e6..438dc397 100644 --- a/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/ClaudeCredentialStore.swift @@ -56,6 +56,7 @@ enum ClaudeCredentialStore { keychainCache = LiveKeychainCredentialCache() lastCacheDeleteResult = nil lastLegacyCleanupFailed = false + unlinkLegacyOverride = nil lock.withLock { memoryCache = nil } } @@ -134,7 +135,11 @@ enum ClaudeCredentialStore { lock.withLock { memoryCache = nil } let result = deleteOurCache() lastCacheDeleteResult = result - isBootstrapCompleted = false + // A failed Keychain delete must not pretend the provider is disconnected. + // Clearing the flag hides Disconnect and orphans the remaining item. + if result.keychainDeletedOrAbsent { + isBootstrapCompleted = false + } return result } @@ -150,6 +155,8 @@ enum ClaudeCredentialStore { /// Last legacy-unlink failure after a verified Keychain write (cleanup retry signal). nonisolated(unsafe) static var lastLegacyCleanupFailed = false + /// Test seam: force legacy unlink to throw so we can assert the 0600 repair. + nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? // MARK: - Public API @@ -490,11 +497,16 @@ enum ClaudeCredentialStore { return } do { - try FileManager.default.removeItem(at: url) + if let unlinkLegacyOverride { + try unlinkLegacyOverride(url) + } else { + try FileManager.default.removeItem(at: url) + } lastLegacyCleanupFailed = false } catch { - // Retain valid Keychain item + 0600 file; surface for later retry. lastLegacyCleanupFailed = true + // Verified Keychain item exists — leftover plaintext must not stay 0644. + try? FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) } } diff --git a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift index 96bf71fd..8ab257e3 100644 --- a/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift +++ b/mac/Sources/CodeBurnMenubar/Data/CodexCredentialStore.swift @@ -43,6 +43,7 @@ enum CodexCredentialStore { keychainCache = LiveKeychainCredentialCache() lastCacheDeleteResult = nil lastLegacyCleanupFailed = false + unlinkLegacyOverride = nil lock.withLock { memoryCache = nil } } @@ -133,7 +134,9 @@ enum CodexCredentialStore { lock.withLock { memoryCache = nil } let result = deleteOurCache() lastCacheDeleteResult = result - isBootstrapCompleted = false + if result.keychainDeletedOrAbsent { + isBootstrapCompleted = false + } return result } @@ -145,6 +148,7 @@ enum CodexCredentialStore { nonisolated(unsafe) static var lastCacheDeleteResult: CacheDeleteResult? nonisolated(unsafe) static var lastLegacyCleanupFailed = false + nonisolated(unsafe) static var unlinkLegacyOverride: ((URL) throws -> Void)? // MARK: - Public API @@ -368,10 +372,15 @@ enum CodexCredentialStore { return } do { - try FileManager.default.removeItem(at: url) + if let unlinkLegacyOverride { + try unlinkLegacyOverride(url) + } else { + try FileManager.default.removeItem(at: url) + } lastLegacyCleanupFailed = false } catch { lastLegacyCleanupFailed = true + try? FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) } } diff --git a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift index 080682d0..6a300a7f 100644 --- a/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift +++ b/mac/Sources/CodeBurnMenubar/Security/SafeFile.swift @@ -136,6 +136,17 @@ enum SafeFile { } defer { Darwin.close(fd) } + var opened = stat() + guard fstat(fd, &opened) == 0 else { + throw Error.readFailed(path, errno) + } + guard (opened.st_mode & S_IFMT) == S_IFREG else { + throw SecureReadError.notRegularFile(path) + } + guard opened.st_uid == expectedOwner else { + throw SecureReadError.wrongOwner(path) + } + if fchmod(fd, 0o600) != 0 { throw SecureReadError.chmodFailed(path, errno) } @@ -153,16 +164,22 @@ enum SafeFile { throw Error.sizeLimitExceeded(path, size) } - var data = Data(count: max(size, 0)) - let readBytes: Int = data.withUnsafeMutableBytes { buffer -> Int in - guard let base = buffer.baseAddress else { return 0 } - return Darwin.read(fd, base, buffer.count) + var data = Data() + data.reserveCapacity(max(size, 0)) + var chunk = [UInt8](repeating: 0, count: 4096) + while data.count < maxBytes { + let n = chunk.withUnsafeMutableBytes { buffer -> Int in + guard let base = buffer.baseAddress else { return 0 } + return Darwin.read(fd, base, buffer.count) + } + guard n >= 0 else { + throw Error.readFailed(path, errno) + } + if n == 0 { break } + data.append(contentsOf: chunk.prefix(n)) } - guard readBytes >= 0 else { - throw Error.readFailed(path, errno) - } - if readBytes < data.count { - data = data.prefix(readBytes) + if data.count > maxBytes { + throw Error.sizeLimitExceeded(path, data.count) } return data } diff --git a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift index 00ffc36f..1b75a9e1 100644 --- a/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift +++ b/mac/Tests/CodeBurnMenubarTests/CredentialKeychainContinuityTests.swift @@ -289,6 +289,7 @@ struct CredentialKeychainContinuityTests { expiresAt: nil, rateLimitTier: nil )) + ClaudeCredentialStore.isBootstrapCompleted = true let controllable = ControllableKeychainCredentialCache(inner: harness.fakeKeychain) controllable.failDelete = true ClaudeCredentialStore.keychainCache = controllable @@ -297,6 +298,30 @@ struct CredentialKeychainContinuityTests { #expect(!failed.isSuccess) #expect(failed.keychainDeletedOrAbsent == false) #expect(ClaudeCredentialStore.lastCacheDeleteResult?.isSuccess == false) + #expect(ClaudeCredentialStore.isBootstrapCompleted == true) + } + } + + @Test("failed unlink after verified Keychain repairs leftover JSON to 0600") + func failedUnlinkRepairsLegacyMode() throws { + try withHarness { _ in + let record = ClaudeCredentialStore.CredentialRecord( + accessToken: accessSentinel, + refreshToken: refreshSentinel, + expiresAt: Date(timeIntervalSince1970: 1_900_000_000), + rateLimitTier: "default" + ) + try ClaudeCredentialStore.writeOurCache(record: record) + try writeLegacyClaude0644(record: record) + ClaudeCredentialStore.isBootstrapCompleted = true + ClaudeCredentialStore.unlinkLegacyOverride = { _ in + throw POSIXError(.EPERM) + } + ClaudeCredentialStore.clearMemoryCacheForTesting() + _ = try ClaudeCredentialStore.currentRecord() + #expect(ClaudeCredentialStore.lastLegacyCleanupFailed == true) + #expect(FileManager.default.fileExists(atPath: ClaudeCredentialStore.cacheFileURL().path)) + #expect(posixMode(at: ClaudeCredentialStore.cacheFileURL()) == 0o600) } }