diff --git a/Yattee/Services/Credentials/AccountKeychainCredentials.swift b/Yattee/Services/Credentials/AccountKeychainCredentials.swift index 30d01dfc..28ad0ebe 100644 --- a/Yattee/Services/Credentials/AccountKeychainCredentials.swift +++ b/Yattee/Services/Credentials/AccountKeychainCredentials.swift @@ -236,15 +236,20 @@ final class AccountKeychainCredentials { return false } - // Nothing readable is there: the add reported success but produced - // no usable item, so rolling it back destroys nothing and lets the - // adoption offer be retried. + // Nothing readable is there. Tempting to roll the write back, but a + // delete cannot be conditioned on the value that was just read: + // `SecItemDelete` ignores `kSecValueData` (measured — a delete + // issued with a deliberately wrong value still removes the item), + // so the rollback would be a blind delete of whatever occupies the + // key by then, including a credential that arrived from another + // device in the gap — and it would propagate. Nothing is deleted; + // no marker is written, so the account simply stays signed out and + // an ordinary sign-in (a replacing write) recovers it. LoggingService.shared.error( - "Legacy \(credentialNoun) adoption failed verification", + "Legacy \(credentialNoun) adoption failed verification — sign-in required", category: .keychain, details: "accountID=\(account.id)" ) - store.delete(key: key) return false } @@ -287,6 +292,28 @@ final class AccountKeychainCredentials { return } + // Re-read immediately before deleting. The Keychain offers no + // compare-and-delete — `SecItemDelete` ignores `kSecValueData` + // (measured) — so the fingerprint check above can only ever validate a + // snapshot. Re-reading here shrinks the window to a single call gap and + // catches a replacement that landed while the fingerprint was being + // computed. + // + // A residual race remains by construction: if another device running + // the previous release writes a new legacy value inside that gap, this + // deletes their fresh login and the deletion propagates. Accepted — + // closing it is not possible with these APIs, and the alternative + // (never deleting) leaves a credential the user explicitly removed + // sitting in their iCloud Keychain indefinitely. + guard store.value(forKey: legacyKey) == currentValue else { + LoggingService.shared.info( + "Legacy \(credentialNoun) changed while being retired — leaving it in place", + category: .keychain, + details: "instanceID=\(account.instanceID)" + ) + return + } + store.delete(key: legacyKey) markers.removeMarker(service: store.service, instanceID: account.instanceID, scope: scope) diff --git a/Yattee/Services/Credentials/KeychainProvenanceReport.swift b/Yattee/Services/Credentials/KeychainProvenanceReport.swift index 2e777251..0de77023 100644 --- a/Yattee/Services/Credentials/KeychainProvenanceReport.swift +++ b/Yattee/Services/Credentials/KeychainProvenanceReport.swift @@ -70,7 +70,11 @@ enum KeychainProvenanceReportBuilder { /// - label: free-form run label, e.g. "iPhone 15 — signed in as identity B". /// - userRecordID: resolved asynchronously by the caller (network), `nil` /// when unavailable. - static func build(label: String, userRecordID: String?) -> String { + static func build( + label: String, + userRecordID: String?, + accountAvailability: CloudKitIdentity.AccountAvailability + ) -> String { let results = KeychainProvenanceProbe.queryAll() var lines: [String] = [] @@ -82,7 +86,11 @@ enum KeychainProvenanceReportBuilder { lines.append("--- Identity ---") lines.append("CloudKit entitlement: \(CloudKitAvailability.isAvailable ? "available" : "missing (sideload)")") - lines.append("ubiquityIdentityToken: \(CloudKitIdentity.ubiquityFingerprint.map { "present, fp=\($0)" } ?? "none — no Apple account signed in")") + // A nil token does NOT mean signed out — Foundation also returns nil + // when iCloud Drive is off, so the report must not assert either way. + // CloudKit's account status is what disambiguates. + lines.append("ubiquityIdentityToken: \(CloudKitIdentity.ubiquityFingerprint.map { "present, fp=\($0)" } ?? "unavailable (signed out, or iCloud Drive off)")") + lines.append("CloudKit account: \(describe(accountAvailability))") lines.append("CloudKit user record: \(userRecordID ?? "unresolved")") lines.append("") @@ -128,6 +136,14 @@ enum KeychainProvenanceReportBuilder { accounts.isEmpty ? "none" : accounts.sorted().joined(separator: ", ") } + private static func describe(_ availability: CloudKitIdentity.AccountAvailability) -> String { + switch availability { + case .noAccount: "no account signed in" + case .available: "signed in" + case .indeterminate: "could not be determined" + } + } + // MARK: - Environment private static var platformDescription: String { diff --git a/Yattee/Views/Settings/KeychainProvenanceExperimentView.swift b/Yattee/Views/Settings/KeychainProvenanceExperimentView.swift index 4154e456..ba223318 100644 --- a/Yattee/Views/Settings/KeychainProvenanceExperimentView.swift +++ b/Yattee/Views/Settings/KeychainProvenanceExperimentView.swift @@ -21,6 +21,7 @@ import UIKit struct KeychainProvenanceExperimentView: View { @State private var label = "" @State private var userRecordID: String? + @State private var accountAvailability: CloudKitIdentity.AccountAvailability = .indeterminate @State private var resolvingUserRecordID = true @State private var reports: [KeychainProvenanceReport] = KeychainProvenanceReportStore.load() @State private var latestStatus: String? @@ -38,6 +39,7 @@ struct KeychainProvenanceExperimentView: View { .navigationBarTitleDisplayMode(.inline) #endif .task { + accountAvailability = await CloudKitIdentity.accountAvailability() userRecordID = await CloudKitIdentity.userRecordID() resolvingUserRecordID = false } @@ -78,10 +80,13 @@ struct KeychainProvenanceExperimentView: View { private var identitySection: some View { SettingsFormSection( "Identity", - footer: "The fingerprint is an opaque hash of ubiquityIdentityToken. It must differ between accounts A and B — if it does not, the sign-out did not take effect." + footer: "The fingerprint is an opaque hash of ubiquityIdentityToken and must differ between accounts A and B — if it does not, the sign-out did not take effect. An unavailable token does not by itself mean signed out: it also goes missing when iCloud Drive is off, which is why the account status is shown separately." ) { row("CloudKit entitlement", CloudKitAvailability.isAvailable ? "available" : "missing") - row("Apple account", CloudKitIdentity.ubiquityFingerprint.map { "fp=\($0)" } ?? "signed out") + // A nil token is ambiguous — it also means iCloud Drive is off — + // so the account status is shown beside it rather than inferred. + row("Ubiquity token", CloudKitIdentity.ubiquityFingerprint.map { "fp=\($0)" } ?? "unavailable") + row("Apple account", accountStatusText) row("User record", resolvingUserRecordID ? "resolving…" : (userRecordID ?? "unresolved")) } } @@ -179,7 +184,11 @@ struct KeychainProvenanceExperimentView: View { } private func capture() { - let text = KeychainProvenanceReportBuilder.build(label: label, userRecordID: userRecordID) + let text = KeychainProvenanceReportBuilder.build( + label: label, + userRecordID: userRecordID, + accountAvailability: accountAvailability + ) reports = KeychainProvenanceReportStore.append( KeychainProvenanceReport(label: label, text: text) ) @@ -195,6 +204,15 @@ struct KeychainProvenanceExperimentView: View { latestStatus = "Captured report \(reports.count)." } + private var accountStatusText: String { + if resolvingUserRecordID { return "checking…" } + switch accountAvailability { + case .noAccount: return "no account signed in" + case .available: return "signed in" + case .indeterminate: return "could not be determined" + } + } + private func payloadText() -> String { let fingerprint = CloudKitIdentity.ubiquityFingerprint ?? "none" let written = Date().formatted(.iso8601)