Quarantine the unresolved account namespace

The accounts.unresolved.* UserDefaults namespace was found on a real
device holding a stale, contradictory snapshot: a tombstoned account
listed live, context reset to local, every sync outbox still pending.
With iCloud Drive off the identity scope dips to unresolved on every
foreground activation, so that snapshot was loaded — and rewritten —
in an entirely ordinary configuration.

Close it off on four fronts:

- Gate the unprompted writers. reconcileLegacyAccounts() and
  migrateLegacyContextIfNeeded() now no-op while the identity scope is
  unresolved; reloadForCurrentIdentity() re-runs both once a scope
  resolves, so nothing is lost, and nothing synthesizes accounts or
  queues uploads under a namespace that answers to no one.

- Fence the reload gap. Between a scope change and this manager's
  observer reloading, namespace already names the new identity while
  memory still holds the old snapshot. loadedNamespace records what
  memory belongs to; save(), the stored-context flag, and every
  coordinator path that builds, enqueues, applies or confirms
  account-category records refuses when it mismatches. Staged
  downloads stay durable and re-apply after the reload.

- Tie confirmations and conflict retries to the identity that sent
  them. Every outbound account-category record carries an
  identityNamespace field; didSave honours only confirmations whose
  record carries the current identity's stamp, conflict resolutions
  inherit the local record's stamp and are dropped when foreign, and
  a skipped confirmation re-registers the durable outbox so nothing
  strands. The abandoning conflict branches re-register it too.

- Purge the stale blob. Once a scope resolves, the unresolved
  namespace keys and the unresolved credential-fingerprint store are
  removed; the activation-time validators no longer correct the
  context or rewrite the legacy selection from unresolved state.

Hardened through four adversarial review rounds; the last found
nothing.
This commit is contained in:
Arkadiusz Fal
2026-09-01 23:55:47 +02:00
parent b1b7dcd10a
commit 5a77e93558
10 changed files with 839 additions and 24 deletions

View File

@@ -684,6 +684,13 @@ final class AccountsManager {
/// migrated account would simply recreate it, because `usesAccountLogin`
/// remains true on the instance forever.
func reconcileLegacyAccounts() {
// Nothing may be synthesized into the unresolved namespace: the app
// cannot yet tell whose accounts these are, and the marker it would
// write there is permanent while the namespace itself is discarded.
// Nothing is lost by waiting — resolution runs
// `reloadForCurrentIdentity()`, which reconciles under the namespace
// that will actually be read.
guard identityScope.isResolved else { return }
guard let instancesManager else { return }
var didChange = false
@@ -742,6 +749,11 @@ final class AccountsManager {
/// previous release wrote the key to iCloud but never imported it back — on
/// a fresh install the only copy lives in KVS.
func migrateLegacyContextIfNeeded() {
// A one-way migration is spent the moment it runs, so running it
// against the unresolved namespace would consume it against storage
// that is thrown away — and mark a context chosen that the identity
// owning the choice never sees.
guard identityScope.isResolved else { return }
guard !hasStoredContext else { return }
guard let legacy = legacySubscriptionAccount() else { return }
@@ -792,16 +804,41 @@ final class AccountsManager {
identityScope.current ?? "unresolved"
}
/// Whether the identity everything here is keyed by is known.
///
/// Exposed because the corrections other services make — rewriting the
/// legacy selection, resetting a context whose account is "missing" — are
/// only meaningful once this is true. While it is false the app is looking
/// at a namespace it will discard, and acting on what it finds there
/// corrects real state from state that was never authoritative.
var isIdentityResolved: Bool { identityScope.isResolved }
/// The namespace the in-memory state was loaded from.
///
/// `namespace` moves the instant the identity scope does, but the reload
/// that refills memory runs on a main-queue observer afterwards. In that
/// window everything held here still belongs to the previous identity, so
/// writes are fenced on this rather than on `namespace` — otherwise one
/// Apple account's accounts would be persisted, and uploaded, under
/// another's. Kept out of observation: it is a storage fence, not state
/// anything renders.
@ObservationIgnored private(set) var loadedNamespace = ""
private func key(_ name: String) -> String {
"accounts.\(namespace).\(name)"
}
private var hasStoredContext: Bool {
get { defaults.bool(forKey: key("hasContext")) }
set { defaults.set(newValue, forKey: key("hasContext")) }
set {
guard loadedNamespace == namespace else { return }
defaults.set(newValue, forKey: key("hasContext"))
}
}
private func load() {
loadedNamespace = namespace
if let data = defaults.data(forKey: key("accounts")),
let decoded = try? JSONDecoder().decode([Account].self, from: data) {
accounts = decoded
@@ -837,9 +874,53 @@ final class AccountsManager {
}
pendingContextUpload = defaults.bool(forKey: key("pendingContextUpload"))
purgeUnresolvedNamespace()
}
/// Every key `save()` writes, for purging a namespace wholesale.
private static let storedKeyNames = [
"accounts", "context", "hasContext", "migrated", "pendingUploads",
"pendingDeletions", "pendingMarkerUploads", "deletedAccounts",
"deletedInstances", "pendingContextUpload"
]
/// Deletes whatever was written while the identity was still unresolved.
///
/// That namespace is a holding pen no resolved identity ever reads back,
/// and what accumulates there is actively wrong: a snapshot from before a
/// tombstone was applied, outboxes owing uploads to a zone nothing will
/// ever send them to. With iCloud Drive switched off the scope dips to
/// unresolved on every foreground activation, so it is written in ordinary
/// use rather than only in exotic states.
///
/// Run from `load()`, which covers both a launch that resolves immediately
/// (where no reload ever runs) and one that resolves later. Repeatable, so
/// there is nothing to remember about having done it.
private func purgeUnresolvedNamespace() {
guard identityScope.isResolved else { return }
for name in Self.storedKeyNames {
defaults.removeObject(forKey: "accounts.unresolved.\(name)")
}
}
private func save() {
// The namespace moved and the reload has not run yet, so this is the
// previous identity's state being written under the new identity's
// keys. Dropped rather than deferred: the reload that follows loads
// whatever storage already holds for the new identity, and everything
// dropped here is either that identity's — and still on disk — or the
// previous one's, which its own namespace still holds.
guard loadedNamespace == namespace else {
LoggingService.shared.warning(
"Dropping an account save made while the identity namespace was reloading",
category: .general,
details: "loaded=\(loadedNamespace), current=\(namespace)"
)
return
}
if let data = try? JSONEncoder().encode(accounts) {
defaults.set(data, forKey: key("accounts"))
}
@@ -879,6 +960,12 @@ final class AccountsManager {
load()
reconcileLegacyAccounts()
// Both migrations are held back while the scope is unresolved, and this
// is where a launch that resolved late gets them. Reconciliation only
// runs the context migration when it changed something, so a namespace
// whose accounts were already migrated would otherwise keep its legacy
// selection until the next relaunch. Idempotent via `hasStoredContext`.
migrateLegacyContextIfNeeded()
NotificationCenter.default.post(name: .accountsDidChange, object: nil)
// The context belongs to the namespace too, so this reload replaced it.

View File

@@ -94,6 +94,18 @@ final class AppEnvironment {
}
}
/// Drops the fingerprints recorded while the identity was unresolved.
///
/// Nothing legitimate is written under that key — `refreshAccountStatuses`
/// refuses to read the Keychain until the scope resolves — so anything left
/// there is a digest of a credential read under a scope the app could not
/// vouch for, and comparing a real credential against it would report a
/// replacement that never happened.
private func purgeUnresolvedCredentialFingerprints() {
guard credentialIdentityScope.isResolved else { return }
UserDefaults.standard.removeObject(forKey: "accountCredentialFingerprints.unresolved")
}
private func saveCredentialFingerprints() {
let stored = credentialFingerprints.reduce(into: [String: String]()) { result, entry in
result[entry.key.uuidString] = entry.value
@@ -476,6 +488,7 @@ final class AppEnvironment {
// Fingerprints are recorded per identity, like everything else
// keyed to one.
self.loadCredentialFingerprints()
self.purgeUnresolvedCredentialFingerprints()
self.refreshAccountStatuses()
// Compared against the recorded owner rather than cleared
@@ -488,6 +501,7 @@ final class AppEnvironment {
}
loadCredentialFingerprints()
purgeUnresolvedCredentialFingerprints()
discardCachesFromAnotherIdentityIfNeeded(atLaunch: true)
// Turn any pre-revamp single-instance logins into accounts. Repeatable:
@@ -651,6 +665,14 @@ final class AppEnvironment {
clearAccountScopedCaches()
}
// The context in memory belongs to whichever namespace was loaded, and
// an unresolved scope is not one whose accounts anybody has looked at
// yet. Mirroring from it would rewrite the legacy selection to `.local`
// on every activation dip — several times a day with iCloud Drive off —
// and the user would find their picker reset. The observer refires when
// the scope comes back, so the mirror catches up rather than being lost.
guard credentialIdentityScope.isResolved else { return }
let legacy: SubscriptionAccount
switch accountsManager.currentContext {

View File

@@ -52,6 +52,13 @@ final class AccountSyncCoordinator {
/// lost by not having one.
private var enqueueChanges: EnqueueChanges?
/// Drops the conflict-resolved account records the engine holds for upload.
/// That cache is keyed by record name only, and the names are deterministic
/// across identities, so it is emptied when the identity moves rather than
/// left holding the previous one's records. Their stamps would refuse them
/// anyway; this is what keeps them from lingering in memory until they do.
private var discardCachedRecords: (@MainActor () -> Void)?
/// The private database, for the bootstrap fetch that runs outside the
/// engine's change token.
private weak var database: CKDatabase?
@@ -82,6 +89,11 @@ final class AccountSyncCoordinator {
private var isActivating = false
private var activationRequested = false
/// Set while staged changes are being applied. Applying posts the very
/// notifications this coordinator retries deferred activations from, so
/// without it a run would ask itself to run again.
private var isApplyingStagedChanges = false
private nonisolated(unsafe) var identityObserver: NSObjectProtocol?
private nonisolated(unsafe) var accountsObserver: NSObjectProtocol?
private nonisolated(unsafe) var contextObserver: NSObjectProtocol?
@@ -151,12 +163,14 @@ final class AccountSyncCoordinator {
// Local edits mark themselves dirty in AccountsManager; these turn that
// into engine registrations without every call site knowing about sync.
// They are also how `reloadForCurrentIdentity()` announces itself, which
// is what releases records held back by the reload fence.
accountsObserver = NotificationCenter.default.addObserver(
forName: .accountsDidChange,
object: nil,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated { self?.enqueueDirtyRecords() }
MainActor.assumeIsolated { self?.accountStateDidChange() }
}
contextObserver = NotificationCenter.default.addObserver(
@@ -164,7 +178,7 @@ final class AccountSyncCoordinator {
object: nil,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated { self?.enqueueDirtyRecords() }
MainActor.assumeIsolated { self?.accountStateDidChange() }
}
}
@@ -187,22 +201,52 @@ final class AccountSyncCoordinator {
func identityDidChange() {
identityGeneration += 1
// Records already built for the previous identity are still sitting in
// the engine's conflict cache, which is not namespaced and would serve
// them at send time behind this coordinator's back.
discardCachedRecords?()
guard namespace != nil else { return }
Task { await activate() }
}
/// Re-runs an activation that was held back because `AccountsManager` had
/// not reloaded yet.
///
/// Only meaningful while something is staged and unapplied — activation
/// itself posts the notifications this is called from, so it would
/// otherwise ask for a run on the way out of every run it performs.
private func accountStateDidChange() {
enqueueDirtyRecords()
guard !isApplyingStagedChanges else { return }
guard let namespace, isManagerLoaded(for: namespace) else { return }
guard !store.stagedChanges(in: namespace).isEmpty else { return }
Task { await activate() }
}
// MARK: - Engine attachment
/// Connects the coordinator to a live sync engine.
func attach(database: CKDatabase?, enqueueChanges: @escaping EnqueueChanges) {
///
/// - Parameter discardingCachedRecords: empties whatever account records the
/// engine is holding from conflict resolution, for when the identity moves.
func attach(
database: CKDatabase?,
discardingCachedRecords discardCachedRecords: (@MainActor () -> Void)? = nil,
enqueueChanges: @escaping EnqueueChanges
) {
self.database = database
self.enqueueChanges = enqueueChanges
self.discardCachedRecords = discardCachedRecords
}
/// Disconnects when the engine is torn down. The durable state — staging,
/// outbox, bootstrap flag — is deliberately left untouched.
func detach() {
enqueueChanges = nil
discardCachedRecords = nil
database = nil
}
@@ -313,6 +357,22 @@ final class AccountSyncCoordinator {
guard store.isBootstrapCompleted(in: namespace) else { return }
guard namespace == self.namespace else { return }
// Applying is a write into `AccountsManager`'s memory, and during the
// reload gap that memory still belongs to the identity being left. The
// damage is not hypothetical: an incoming account whose deterministic ID
// matches a tombstone the *previous* identity holds would be read as a
// resurrection and deleted from the new identity's zone, and a tombstone
// applied against the previous identity's list removes that account —
// taking its Keychain credential with it. Staging is durable, so this
// defers rather than drops; `accountStateDidChange()` retries it when the
// reload announces itself.
guard isManagerLoaded(for: namespace) else {
LoggingService.shared.logCloudKit(
"Account records stay staged: the accounts in memory still belong to the previous identity"
)
return
}
applyStagedChanges(in: namespace)
enqueueDirtyRecords()
}
@@ -347,6 +407,9 @@ final class AccountSyncCoordinator {
let staged = store.stagedChanges(in: namespace)
guard !staged.isEmpty else { return }
isApplyingStagedChanges = true
defer { isApplyingStagedChanges = false }
// Tombstone-first. A deletion that arrived in a later event than the
// account record it kills must still be applied before it, or the
// account becomes visible — and selectable — in between.
@@ -553,6 +616,7 @@ final class AccountSyncCoordinator {
func enqueueDirtyRecords() {
guard let enqueueChanges, let namespace else { return }
guard isCategoryEnabled(), store.isBootstrapCompleted(in: namespace) else { return }
guard isManagerLoaded(for: namespace) else { return }
var changes: [CKSyncEngine.PendingRecordZoneChange] = []
@@ -584,10 +648,50 @@ final class AccountSyncCoordinator {
/// Registers every account record for upload, for a zone with nothing in it
/// — first enable, or a zone recreated after being deleted elsewhere.
func uploadEverything() {
// Marking everything dirty commits the *in-memory* list to this
// identity's zone, so it must not run against a snapshot loaded under
// another one.
guard let namespace, isManagerLoaded(for: namespace) else { return }
accountsManager.markAllRecordsForUpload()
enqueueDirtyRecords()
}
/// Whether the account manager's in-memory state belongs to the namespace
/// this coordinator uploads into.
///
/// The identity scope moves before `AccountsManager` reloads — the reload
/// runs from a main-queue observer — so in that window `namespace` already
/// names the new identity while the accounts, the outboxes and the selected
/// context in memory are still the previous one's. Engine callbacks can
/// land there, and building records from that snapshot would publish one
/// Apple account's accounts into another's zone. Observer ordering is not
/// contractual, so this fences rather than assuming an order; the outbox is
/// durable, and the reload re-enqueues whatever is still owed.
private func isManagerLoaded(for namespace: String) -> Bool {
accountsManager.loadedNamespace == namespace
}
/// Whether an account-category record belongs to the identity that is
/// current now — and may therefore be uploaded, cached for retry, or have
/// its confirmation acted on.
///
/// Judged on the record's own stamp rather than on anything kept beside it.
/// A side table keyed by record name cannot answer this: the names are
/// deterministic, so two identities' copies of the "same" record collide in
/// it, and the second send silently relabels the first one's. The engine
/// also delivers delegate events from a previous identity to the same
/// instance, and an async conflict handler can resume long after the
/// identity moved — both arrive holding a record, which is the one thing
/// that still knows where it came from.
///
/// An unstamped record fails closed: nothing this build sends is unstamped,
/// so it is not ours to act on.
func belongsToCurrentIdentity(_ record: CKRecord) -> Bool {
guard let namespace, isManagerLoaded(for: namespace) else { return false }
return record[AccountRecordName.identityNamespaceField] as? String == namespace
}
/// Builds the record for a pending save at send time.
///
/// Returning `nil` withdraws the change from the engine — used both for
@@ -596,7 +700,19 @@ final class AccountSyncCoordinator {
/// change is registered again as soon as what it waits on is confirmed.
func recordToSave(named recordName: String) -> CKRecord? {
guard let namespace, isCategoryEnabled(), store.isBootstrapCompleted(in: namespace) else { return nil }
guard isManagerLoaded(for: namespace) else { return nil }
// Stamped at the moment of building, which is the only moment that says
// for certain whose zone this copy of the record is going into. It
// travels with the record from here on: to the server, back in the
// confirmation, and into the engine's retry cache.
let record = buildRecord(named: recordName)
record?[AccountRecordName.identityNamespaceField] = namespace as CKRecordValue
return record
}
private func buildRecord(named recordName: String) -> CKRecord? {
switch AccountRecordName.parse(recordName) {
case let .account(id):
guard !accountsManager.isDeleted(id), let account = accountsManager.account(id: id) else { return nil }
@@ -626,11 +742,33 @@ final class AccountSyncCoordinator {
/// Clears outbox entries the server has confirmed, and releases whatever was
/// waiting on them.
///
/// A confirmation only ever clears state for the identity that sent the
/// record. The names are deterministic, so the identity that took over
/// during the send has an outbox entry under the same name; honouring the
/// previous identity's confirmation would clear that entry without the
/// server having confirmed anything — and, for a tombstone, register the
/// deletion of the new identity's live account record.
func didSave(records: [CKRecord]) {
var didSaveAccountRecord = false
var didIgnoreConfirmation = false
for record in records {
switch AccountRecordName.parse(record.recordID.recordName) {
let recordName = record.recordID.recordName
guard !AccountRecordName.isAccountCategory(recordName) || belongsToCurrentIdentity(record) else {
// Left pending rather than confirmed: the outbox is durable and
// the record is registered again for whichever identity still
// owes it. Nothing is enqueued from here — least of all the
// deletion a tombstone's confirmation releases.
didIgnoreConfirmation = true
LoggingService.shared.logCloudKit(
"Ignoring a confirmation for \(recordName) sent under a different Apple account"
)
continue
}
switch AccountRecordName.parse(recordName) {
case let .account(id):
accountsManager.confirmUpload(id, updatedAt: record["updatedAt"] as? Date)
didSaveAccountRecord = true
@@ -658,7 +796,13 @@ final class AccountSyncCoordinator {
// Whatever was held behind these can go now — a marker waiting on its
// account, or a context the user changed while the previous one was
// still in flight, which confirmation deliberately left dirty.
if didSaveAccountRecord {
//
// An ignored confirmation needs the same call, for the opposite reason:
// the send it belonged to consumed the engine's pending change for that
// record name, and the current identity's reload — which is what would
// otherwise re-register it — has already been and gone. Without this the
// entry stays owed in a durable outbox that nothing is reading.
if didSaveAccountRecord || didIgnoreConfirmation {
enqueueDirtyRecords()
}
}
@@ -675,26 +819,15 @@ final class AccountSyncCoordinator {
/// Resolves a conflict on an account-category record, and applies the
/// outcome locally.
///
/// The local apply matters here in a way it does not for the other
/// categories: CloudKit never delivers back a record this device saved, so
/// a conflict the server won would otherwise leave stale state on screen
/// until some unrelated future change to the same record.
func resolveConflict(local: CKRecord, server: CKRecord) async -> CKRecord {
let resolved: CKRecord
switch server.recordType {
case RecordType.account:
resolved = await conflictResolver.resolveAccountConflict(local: local, server: server)
if let account = try? mapper.toAccount(from: resolved), !accountsManager.isDeleted(account.id) {
accountsManager.applySyncedAccount(account)
}
case RecordType.dataContext:
resolved = await conflictResolver.resolveDataContextConflict(local: local, server: server)
if let context = try? mapper.toStoredDataContext(from: resolved) {
accountsManager.applySyncedContext(context)
}
case RecordType.legacyAccountMigration, RecordType.accountDeletion:
resolved = await conflictResolver.resolveWriteOnceConflict(local: local, server: server)
@@ -703,9 +836,52 @@ final class AccountSyncCoordinator {
resolved = server
}
// The merge can take fields from the server's copy, including its stamp,
// so provenance is restored from the local side: this record is a retry
// of the record *this device built*, and that is whose zone it is going
// into. Stamping it with whatever is current instead would launder a
// resolution begun under the previous Apple account into the one that
// took over while it was being computed.
inheritProvenance(from: local, to: resolved)
applyResolvedRecord(resolved)
return resolved
}
/// Copies the identity stamp of the record a resolution started from onto
/// its outcome.
private func inheritProvenance(from local: CKRecord, to resolved: CKRecord) {
guard resolved !== local else { return }
resolved[AccountRecordName.identityNamespaceField] = local[AccountRecordName.identityNamespaceField]
}
/// Writes a resolution's outcome into memory.
///
/// Matters here in a way it does not for the other categories: CloudKit
/// never delivers back a record this device saved, so a conflict the server
/// won would otherwise leave stale state on screen until some unrelated
/// future change to the same record. Fenced on the record's own provenance —
/// the resolution suspends, and the identity can move across that suspension.
private func applyResolvedRecord(_ resolved: CKRecord) {
guard belongsToCurrentIdentity(resolved) else { return }
switch resolved.recordType {
case RecordType.account:
guard let account = try? mapper.toAccount(from: resolved),
!accountsManager.isDeleted(account.id) else { return }
accountsManager.applySyncedAccount(account)
case RecordType.dataContext:
guard let context = try? mapper.toStoredDataContext(from: resolved) else { return }
accountsManager.applySyncedContext(context)
default:
// Write-once records carry no local state to correct.
break
}
}
// MARK: - Helpers
private func recordID(for type: SyncableRecordType) -> CKRecord.ID {

View File

@@ -399,7 +399,12 @@ final class CloudKitSyncEngine: @unchecked Sendable {
// to complete before anything is uploaded, or a device upgrading
// from the previous release would publish its unmerged local list
// over records it has not seen yet.
accountSync?.attach(database: database) { [weak self] changes in
accountSync?.attach(
database: database,
discardingCachedRecords: { [weak self] in
self?.discardConflictResolvedAccountRecords()
}
) { [weak self] changes in
self?.addPendingChanges(changes)
}
await accountSync?.activate()
@@ -1759,7 +1764,17 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
// A conflict-resolved record (carrying the server change tag) takes precedence
if let resolved = conflictResolvedRecords[recordName] {
return resolved
if mayUseResolvedRecord(resolved) {
return resolved
}
// An account record built under another identity can never become
// valid here — including one resolved after the identity moved but
// before the accounts in memory caught up, which the invalidation on
// the change itself cannot have seen. Dropping it costs at most one
// more conflict round-trip; serving it would upload another Apple
// account's data.
conflictResolvedRecords.removeValue(forKey: recordName)
}
if let record = await materializeRecord(named: recordName) {
@@ -1771,6 +1786,52 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
return nil
}
/// Whether a conflict-resolved record may be cached for retry, and served
/// when that retry is sent.
///
/// This cache is keyed by record name alone, and account record names are
/// deterministic across Apple accounts — so the name says nothing about
/// whose record this is, and the record's own stamp has to be asked. Both
/// ends need the question: the resolution runs asynchronously and can finish
/// after the identity moved (too late for the invalidation that change
/// triggers), and the send can come later still. Falling through to
/// materialization is safe — the coordinator holds the change there, and the
/// durable outbox re-registers it. Every other category is unaffected:
/// nothing else here is namespaced by Apple account.
private func mayUseResolvedRecord(_ record: CKRecord) -> Bool {
guard AccountRecordName.isAccountCategory(record.recordID.recordName) else { return true }
return accountSync?.belongsToCurrentIdentity(record) ?? false
}
/// Puts an account record's engine change back when a conflict is abandoned.
///
/// The send consumed the pending change, and these paths return without
/// registering another — but the account outbox is durable and still owes
/// the record, and unlike the SwiftData categories nothing re-derives it
/// from a model change. It would sit there until an unrelated edit, an
/// identity reload or a relaunch happened to re-register it. The coordinator
/// decides what is actually still owed; a record that is not simply does
/// nothing here. Other categories keep the existing behaviour.
private func restoreAccountOutboxIfNeeded(for recordName: String) {
guard AccountRecordName.isAccountCategory(recordName) else { return }
accountSync?.enqueueDirtyRecords()
}
/// Drops the account records held from conflict resolution, for when the
/// identity moves. Called by the coordinator, which is what observes that.
private func discardConflictResolvedAccountRecords() {
let stale = conflictResolvedRecords.keys.filter { AccountRecordName.isAccountCategory($0) }
guard !stale.isEmpty else { return }
for recordName in stale {
conflictResolvedRecords.removeValue(forKey: recordName)
}
LoggingService.shared.logCloudKit(
"Discarded \(stale.count) conflict-resolved account records after the Apple account changed"
)
}
// MARK: - Scope-Aware Local Lookups
/// Bare-ID lookups can return an entity from a different source scope when
@@ -1935,6 +1996,7 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
guard let serverRecord = error.serverRecord else {
LoggingService.shared.logCloudKitError("No server record in conflict error for \(recordName)", error: error)
conflictResolvedRecords.removeValue(forKey: recordName)
restoreAccountOutboxIfNeeded(for: recordName)
return
}
@@ -1946,6 +2008,7 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
)
conflictResolvedRecords.removeValue(forKey: recordName)
retryCount.removeValue(forKey: recordName)
restoreAccountOutboxIfNeeded(for: recordName)
return
}
retryCount[recordName] = attempts + 1
@@ -1954,6 +2017,20 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
// The resolved record keeps the server change tag, so the retry
// is accepted as an update.
let resolved = await resolveConflict(local: record, server: serverRecord)
// The resolution suspends, and the Apple account can move across
// that suspension: this handler may have begun for an identity that
// is no longer the one this cache would serve the retry to. Neither
// the record nor its retry is this identity's to send.
guard mayUseResolvedRecord(resolved) else {
conflictResolvedRecords.removeValue(forKey: recordName)
retryCount.removeValue(forKey: recordName)
LoggingService.shared.logCloudKit(
"Dropping the conflict resolution for \(recordName) — it was built under a different Apple account"
)
return
}
conflictResolvedRecords[recordName] = resolved
syncEngine?.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)])
LoggingService.shared.logCloudKit("Resolved \(serverRecord.recordType) conflict for \(recordName), retrying (attempt \(attempts + 1))")

View File

@@ -197,6 +197,16 @@ enum AccountRecordName {
static let accountDeletionPrefix = "accountDeletion-"
static let dataContext = "datacontext-current"
/// The field every outbound account-category record carries, naming the
/// identity that built it.
///
/// Record names here are deterministic, so they are identical under every
/// Apple account and prove nothing about whose record this is — provenance
/// has to travel inside the record itself. Written and read only by the
/// account sync coordinator for that comparison; the mappers never look at
/// it, so it cannot reach the `Account` model.
static let identityNamespaceField = "identityNamespace"
static func account(_ id: UUID) -> String { accountPrefix + id.uuidString }
static func legacyMigration(_ instanceID: UUID) -> String { legacyMigrationPrefix + instanceID.uuidString }
static func accountDeletion(_ accountID: UUID) -> String { accountDeletionPrefix + accountID.uuidString }

View File

@@ -224,6 +224,23 @@ final class CredentialIdentityScope {
state = fixedScope.map { State.resolved($0) } ?? .unresolved
}
/// Test seam: moves a pinned scope after construction.
///
/// - Parameter announcing: whether to take the real `setState` path, which
/// posts the change notification everything namespaced by identity
/// reloads on. `false` mutates the state silently, which is the only way
/// to reproduce the window between the scope moving and those observers
/// running — the window writes have to be fenced against.
func setFixedScope(_ scope: String?, announcing: Bool = true) {
let newState = scope.map { State.resolved($0) } ?? .unresolved
if announcing {
setState(newState)
} else {
state = newState
}
}
// MARK: - Resolution
/// Re-reads the Apple account and restores the cached scope if there is

View File

@@ -165,6 +165,10 @@ final class DataContextValidator {
/// must never reach here — those are all "unknown", and treating them as
/// removal is how a user loses their selection during a slow sync.
func correctIfContextRemoved() {
// An unresolved identity has not loaded anybody's accounts yet, so an
// account "missing" from that state proves nothing — least of all a
// deletion, which is the only thing allowed to move the context here.
guard accountsManager.isIdentityResolved else { return }
guard case let .account(id) = accountsManager.currentContext else { return }
guard accountsManager.account(id: id) == nil else { return }

View File

@@ -117,6 +117,20 @@ final class SubscriptionAccountValidator {
/// Validates the current account and auto-corrects if invalid.
/// Call this on app launch and when configuration changes.
func validateAndCorrectIfNeeded() {
// Availability is decided from account statuses, and every status is
// "cannot tell yet" while the credential identity scope is unresolved.
// This runs on every foreground activation, so correcting from that
// would overwrite the user's selection with `.local` on a state that
// was never an answer. The observation loop re-runs when the scope
// resolves and the statuses become real.
if accountsManager?.isIdentityResolved == false {
LoggingService.shared.debug(
"Leaving the subscription account alone — the credential identity is still unresolved",
category: .general
)
return
}
guard !isCurrentAccountValid else {
LoggingService.shared.debug(
"Current subscription account is valid: \(settingsManager.subscriptionAccount.type)",

View File

@@ -63,6 +63,16 @@ struct AccountSyncTests {
let category: CategorySwitch
let mapper: CloudKitRecordMapper
let scope: String
/// Confirms records the way the server does: what comes back is the
/// copy the sending identity built, stamped with that identity.
@MainActor
func confirm(_ records: [CKRecord]) {
for record in records {
record[AccountRecordName.identityNamespaceField] = scope as CKRecordValue
}
coordinator.didSave(records: records)
}
}
/// A coordinator with its bootstrap already satisfied.
@@ -535,7 +545,7 @@ struct AccountSyncTests {
#expect(harness.coordinator.recordToSave(named: "legacymigrated-\(instanceID.uuidString)") == nil)
harness.recorder.clear()
harness.coordinator.didSave(records: [harness.mapper.toCKRecord(account: account)])
harness.confirm([harness.mapper.toCKRecord(account: account)])
#expect(!harness.accounts.pendingUploads.contains(account.id))
#expect(harness.recorder.savedNames.contains("legacymigrated-\(instanceID.uuidString)"))
@@ -553,7 +563,7 @@ struct AccountSyncTests {
#expect(!harness.accounts.canUploadContext)
#expect(harness.coordinator.recordToSave(named: "datacontext-current") == nil)
harness.coordinator.didSave(records: [harness.mapper.toCKRecord(account: account)])
harness.confirm([harness.mapper.toCKRecord(account: account)])
#expect(harness.accounts.canUploadContext)
#expect(harness.coordinator.recordToSave(named: "datacontext-current") != nil)
}
@@ -573,7 +583,7 @@ struct AccountSyncTests {
// The record deletion is registered only once the durable half landed.
harness.recorder.clear()
harness.coordinator.didSave(records: [harness.mapper.toCKRecord(accountDeletionID: account.id)])
harness.confirm([harness.mapper.toCKRecord(accountDeletionID: account.id)])
#expect(!harness.accounts.pendingDeletions.contains(account.id))
#expect(harness.recorder.deletedNames.contains("account-\(account.id.uuidString)"))
@@ -601,7 +611,7 @@ struct AccountSyncTests {
let sent = harness.mapper.toCKRecord(account: account)
harness.accounts.updateDisplayName("Renamed", for: account.id)
harness.coordinator.didSave(records: [sent])
harness.confirm([sent])
#expect(harness.accounts.pendingUploads.contains(account.id))
}
@@ -629,7 +639,7 @@ struct AccountSyncTests {
#expect(!harness.recorder.savedNames.contains("legacymigrated-\(kept.instanceID.uuidString)"))
harness.recorder.clear()
harness.coordinator.didSave(records: [harness.mapper.toCKRecord(account: kept)])
harness.confirm([harness.mapper.toCKRecord(account: kept)])
#expect(harness.recorder.savedNames.contains("legacymigrated-\(kept.instanceID.uuidString)"))
}
@@ -763,6 +773,254 @@ struct AccountSyncTests {
#expect(!store.isBootstrapCompleted(in: "identity-B"))
}
@Test("Nothing is uploaded from a snapshot loaded under another identity")
func reloadGapBlocksUploads() {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-A")
// The manager still holds what it loaded while the scope was
// unresolved; the coordinator is already namespaced by the identity
// that resolved. That is the window between the scope moving and the
// manager's reload observer running, and engine callbacks land in it.
let accounts = AccountsManager(
identityScope: CredentialIdentityScope(fixedScope: nil),
defaults: defaults
)
let account = accounts.addAccount(instanceID: UUID())
accounts.setContext(.account(account.id))
let recorder = Recorder()
let coordinator = AccountSyncCoordinator(
accountsManager: accounts,
namespaceProvider: { "identity-A" },
store: store,
isCategoryEnabled: { true }
)
coordinator.attach(database: nil) { recorder.record($0) }
coordinator.enqueueDirtyRecords()
#expect(recorder.changes.isEmpty)
// Nothing is built at send time either — a change registered before the
// scope moved is withdrawn rather than filled in from the wrong state.
#expect(coordinator.recordToSave(named: "account-\(account.id.uuidString)") == nil)
coordinator.uploadEverything()
#expect(recorder.changes.isEmpty)
// And the outbox of the identity that owns the namespace is untouched:
// marking everything dirty would have committed another identity's list
// to this one's zone.
for name in ["accounts", "context", "hasContext", "pendingUploads", "pendingContextUpload"] {
#expect(defaults.object(forKey: "accounts.identity-A.\(name)") == nil)
}
}
@Test("Staged records are never applied to another identity's accounts")
func reloadGapDefersStagedChanges() async throws {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-B")
// identity-A's list, still in memory: one live account, and a tombstone
// for another whose ID identity-B happens to reuse — migrated accounts
// take their instance's UUID, so the collision is ordinary rather than
// exotic.
let scope = CredentialIdentityScope(fixedScope: "identity-A")
let accounts = AccountsManager(identityScope: scope, defaults: defaults)
let livingAccount = accounts.addAccount(instanceID: UUID())
let sharedID = accounts.addAccount(instanceID: UUID()).id
accounts.removeAccount(sharedID)
// The scope moved; the reload that refills memory has not run yet.
scope.setFixedScope("identity-B", announcing: false)
let recorder = Recorder()
let coordinator = AccountSyncCoordinator(
accountsManager: accounts,
namespaceProvider: { scope.current },
store: store,
isCategoryEnabled: { true }
)
coordinator.attach(database: nil) { recorder.record($0) }
var deletedAccounts: [(UUID, Account?)] = []
coordinator.onAccountDeleted = { deletedAccounts.append(($0, $1)) }
let mapper = Self.makeMapper()
coordinator.stageFetched(
records: [
mapper.toCKRecord(accountDeletionID: livingAccount.id),
mapper.toCKRecord(account: Account(id: sharedID, instanceID: UUID()))
],
deletions: []
)
await coordinator.activate()
// Nothing applied: identity-B's tombstone would have removed
// identity-A's account and taken its credential with it, and
// identity-B's account would have been read as a resurrection of
// identity-A's tombstone and deleted from identity-B's zone.
#expect(accounts.account(id: livingAccount.id) != nil)
#expect(deletedAccounts.isEmpty)
#expect(recorder.changes.isEmpty)
#expect(!store.stagedChanges(in: "identity-B").isEmpty)
// Deferred, not dropped. The reload announces itself, and that is what
// releases them — against the right list this time.
accounts.reloadForCurrentIdentity()
try await Task.sleep(for: .milliseconds(200))
#expect(store.stagedChanges(in: "identity-B").isEmpty)
#expect(accounts.account(id: sharedID) != nil)
#expect(!recorder.deletedNames.contains("account-\(sharedID.uuidString)"))
// The tombstone applies to identity-B, which never had that account —
// so the credential removal it triggers carries no account of
// identity-A's.
#expect(deletedAccounts.map(\.0) == [livingAccount.id])
#expect(deletedAccounts.first?.1 == nil)
}
@Test("A confirmation from the previous identity clears nothing")
func confirmationsAreTiedToTheIdentityThatSentThem() async throws {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-A")
store.markBootstrapCompleted(in: "identity-B")
let scope = CredentialIdentityScope(fixedScope: "identity-A")
let accounts = AccountsManager(identityScope: scope, defaults: defaults)
let identity = Identity("identity-A")
let recorder = Recorder()
let coordinator = AccountSyncCoordinator(
accountsManager: accounts,
namespaceProvider: { identity.value },
store: store,
isCategoryEnabled: { true }
)
coordinator.attach(database: nil) { recorder.record($0) }
// A deletion built and sent under identity-A, in flight when the Apple
// account changes.
let sharedID = accounts.addAccount(instanceID: UUID()).id
accounts.removeAccount(sharedID)
coordinator.enqueueDirtyRecords()
#expect(recorder.savedNames.contains("accountDeletion-\(sharedID.uuidString)"))
let sentByA = try #require(coordinator.recordToSave(named: "accountDeletion-\(sharedID.uuidString)"))
scope.setFixedScope("identity-B")
try await Task.sleep(for: .milliseconds(200))
identity.value = "identity-B"
// identity-B owes the same record name — the IDs are deterministic — and
// has had no answer from the server about it. It even builds and sends
// its own copy, which is what makes anything kept *beside* the record,
// keyed by that shared name, unable to tell the two apart.
accounts.applySyncedAccount(Account(id: sharedID, instanceID: UUID()))
accounts.removeAccount(sharedID)
let sentByB = try #require(coordinator.recordToSave(named: "accountDeletion-\(sharedID.uuidString)"))
recorder.clear()
coordinator.didSave(records: [sentByA])
// Still owed, and no deletion was registered off the back of it:
// releasing the account record's deletion here would delete identity-B's
// record on the strength of identity-A's confirmation.
#expect(accounts.pendingDeletions.contains(sharedID))
#expect(recorder.deletedNames.isEmpty)
// identity-B's own copy is its own confirmation.
recorder.clear()
coordinator.didSave(records: [sentByB])
#expect(!accounts.pendingDeletions.contains(sharedID))
#expect(recorder.deletedNames.contains("account-\(sharedID.uuidString)"))
}
@Test("A skipped confirmation puts back what the current identity still owes")
func skippedConfirmationRestoresTheOutbox() async throws {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-A")
store.markBootstrapCompleted(in: "identity-B")
let scope = CredentialIdentityScope(fixedScope: "identity-A")
let accounts = AccountsManager(identityScope: scope, defaults: defaults)
let identity = Identity("identity-A")
let recorder = Recorder()
let coordinator = AccountSyncCoordinator(
accountsManager: accounts,
namespaceProvider: { identity.value },
store: store,
isCategoryEnabled: { true }
)
coordinator.attach(database: nil) { recorder.record($0) }
let accountID = accounts.addAccount(instanceID: UUID()).id
let sentByA = try #require(coordinator.recordToSave(named: "account-\(accountID.uuidString)"))
scope.setFixedScope("identity-B")
try await Task.sleep(for: .milliseconds(200))
identity.value = "identity-B"
// identity-B's own upload, registered by the reload that has already
// happened. identity-A's send consumed the engine's pending change for
// that record name, so after its confirmation is skipped nothing else
// is left to re-register the debt.
let owed = accounts.addAccount(instanceID: UUID()).id
recorder.clear()
coordinator.didSave(records: [sentByA])
#expect(accounts.pendingUploads.contains(owed))
#expect(recorder.savedNames.contains("account-\(owed.uuidString)"))
#expect(recorder.deletedNames.isEmpty)
}
@Test("A change of identity invalidates the records the engine cached")
func identityChangeDropsCachedRecords() throws {
let defaults = Self.makeDefaults()
let store = AccountSyncStore(defaults: defaults)
store.markBootstrapCompleted(in: "identity-A")
let scope = CredentialIdentityScope(fixedScope: "identity-A")
let accounts = AccountsManager(identityScope: scope, defaults: defaults)
let identity = Identity("identity-A")
let coordinator = AccountSyncCoordinator(
accountsManager: accounts,
namespaceProvider: { identity.value },
store: store,
isCategoryEnabled: { true }
)
var didDiscardCachedRecords = false
coordinator.attach(
database: nil,
discardingCachedRecords: { didDiscardCachedRecords = true }
) { _ in }
// The engine serves conflict-resolved records ahead of anything the
// coordinator builds, and that cache is keyed by record name alone —
// which every identity shares. The record's own stamp is what tells them
// apart.
let accountID = accounts.addAccount(instanceID: UUID()).id
let builtByA = try #require(coordinator.recordToSave(named: "account-\(accountID.uuidString)"))
#expect(coordinator.belongsToCurrentIdentity(builtByA))
identity.value = "identity-B"
#expect(!coordinator.belongsToCurrentIdentity(builtByA))
// And an unstamped record — one this build never sent — is nobody's.
#expect(!coordinator.belongsToCurrentIdentity(Self.makeMapper().toCKRecord(account: Account(instanceID: UUID()))))
coordinator.identityDidChange()
#expect(didDiscardCachedRecords)
}
/// A mutable identity, for driving scope changes deterministically.
private final class Identity {
var value: String?

View File

@@ -339,6 +339,156 @@ struct AccountsManagerTests {
#expect(relaunched.currentContext == .local)
}
/// Every key the manager persists, spelled out here rather than read from
/// it: a purge that stopped covering one of them would otherwise be
/// invisible to these tests.
private static let storedKeyNames = [
"accounts", "context", "hasContext", "migrated", "pendingUploads",
"pendingDeletions", "pendingMarkerUploads", "deletedAccounts",
"deletedInstances", "pendingContextUpload"
]
private func unresolvedKeysAreAbsent(in defaults: UserDefaults) -> Bool {
Self.storedKeyNames.allSatisfy { defaults.object(forKey: "accounts.unresolved.\($0)") == nil }
}
private func makeLegacyInstance() -> Instance {
Instance(
type: .invidious,
url: URL(string: "https://legacy-\(UUID().uuidString).example.com")!,
usesAccountLogin: true
)
}
@Test("An unresolved identity never migrates a legacy login")
func unresolvedIdentityDoesNotMigrate() {
let defaults = makeDefaults()
let instances = InstancesManager()
let instance = makeLegacyInstance()
instances.add(instance)
defer { instances.remove(instance) }
let unresolved = AccountsManager(
identityScope: CredentialIdentityScope(fixedScope: nil),
instancesManager: instances,
defaults: defaults
)
unresolved.reconcileLegacyAccounts()
// Synthesizing here would file the account — and the permanent marker
// that suppresses synthesis everywhere else — under a namespace no
// identity ever reads back.
#expect(unresolved.accounts.isEmpty)
#expect(unresolved.migratedInstanceIDs.isEmpty)
#expect(unresolved.pendingUploads.isEmpty)
#expect(unresolved.pendingMarkerUploads.isEmpty)
#expect(unresolvedKeysAreAbsent(in: defaults))
// Held back, not lost: the same instance migrates as soon as there is
// an identity to migrate it for.
let resolved = makeManager(defaults: defaults, instances: instances)
resolved.reconcileLegacyAccounts()
#expect(resolved.accounts(forInstance: instance.id).count == 1)
#expect(resolved.migratedInstanceIDs.contains(instance.id))
}
@Test("A scope that dips to unresolved and returns writes nothing in between")
func scopeRoundTripLeavesTheUnresolvedNamespaceEmpty() async throws {
let defaults = makeDefaults()
let instances = InstancesManager()
let instance = makeLegacyInstance()
instances.add(instance)
defer { instances.remove(instance) }
let scope = CredentialIdentityScope(fixedScope: "identity-A")
let manager = AccountsManager(
identityScope: scope,
instancesManager: instances,
defaults: defaults
)
manager.reconcileLegacyAccounts()
let account = try #require(manager.accounts(forInstance: instance.id).first)
manager.setContext(.account(account.id))
// With iCloud Drive switched off this happens on every foreground
// activation — the scope drops and comes back as the same account.
scope.setFixedScope(nil)
try await Task.sleep(for: .milliseconds(50))
#expect(manager.accounts.isEmpty)
// Instances keep arriving over iCloud while this lasts, and each
// arrival asks for reconciliation.
NotificationCenter.default.post(name: .instancesDidChange, object: nil)
try await Task.sleep(for: .milliseconds(50))
#expect(manager.accounts.isEmpty)
#expect(manager.migratedInstanceIDs.isEmpty)
// Checked here as well as after the flip back: once an identity is
// resolved the purge would hide a write made while it was not.
#expect(unresolvedKeysAreAbsent(in: defaults))
scope.setFixedScope("identity-A")
try await Task.sleep(for: .milliseconds(50))
#expect(manager.accounts(forInstance: instance.id).map(\.id) == [account.id])
#expect(manager.currentContext == .account(account.id))
#expect(unresolvedKeysAreAbsent(in: defaults))
}
@Test("Resolving an identity purges what was stored while it was unknown")
func resolutionPurgesTheUnresolvedNamespace() {
let defaults = makeDefaults()
let identityA = makeManager(defaults: defaults)
let account = identityA.addAccount(instanceID: UUID(), displayName: "Main")
// The blob measured on the device: a snapshot written before the
// identity was known, contradicting the one that owns the storage.
let unresolved = AccountsManager(
identityScope: CredentialIdentityScope(fixedScope: nil),
defaults: defaults
)
let stale = unresolved.addAccount(instanceID: UUID())
unresolved.setContext(.account(stale.id))
#expect(defaults.data(forKey: "accounts.unresolved.accounts") != nil)
let relaunched = makeManager(defaults: defaults)
#expect(unresolvedKeysAreAbsent(in: defaults))
#expect(relaunched.account(id: account.id)?.displayName == "Main")
#expect(relaunched.account(id: stale.id) == nil)
}
@Test("A save made while the namespace is reloading is dropped")
func saveDuringTheReloadGapIsDropped() async throws {
let defaults = makeDefaults()
let scope = CredentialIdentityScope(fixedScope: "identity-A")
let manager = AccountsManager(identityScope: scope, defaults: defaults)
let account = manager.addAccount(instanceID: UUID())
// The scope moves before the observer that reloads this manager runs,
// so `namespace` already names the new identity while everything in
// memory is still the previous one's.
scope.setFixedScope("identity-B", announcing: false)
manager.setContext(.account(account.id))
// identity-B must not be handed identity-A's selection...
#expect(defaults.data(forKey: "accounts.identity-B.context") == nil)
#expect(!defaults.bool(forKey: "accounts.identity-B.hasContext"))
// ...and identity-A's stored copy must not have moved either: the write
// was aimed at keys that no longer describe what is in memory.
let storedA = try #require(defaults.data(forKey: "accounts.identity-A.context"))
#expect(try JSONDecoder().decode(StoredDataContext.self, from: storedA).context == .local)
// The announced flip is the real path, and it reloads cleanly.
scope.setFixedScope("identity-A")
try await Task.sleep(for: .milliseconds(50))
#expect(manager.accounts.map(\.id) == [account.id])
#expect(manager.currentContext == .local)
}
@Test("A migrated context loses to any explicit choice")
func migratedContextLosesToExplicitChoice() {
let manager = makeManager()