Stop the legacy validator from overruling a local selection

Local data is always a valid context. The legacy validator predates
that: it treats local as unavailable without a Yattee Server, which is a
feed concern the feed reports itself — and since the correction now
moves the data context with it, that would have quietly put users on
somebody's backend account, overwriting a local choice made on another
device.

Stale feed responses no longer touch anything. Rejecting only the videos
still let a request from a login the cache had moved off overwrite the
new one's loading state, error state and pagination — replacement
requests are allowed to overlap, so both were in flight.

The unreachable retry timers are cancelled when the namespace reloads. A
migrated account reuses its instance's UUID, so the same ID can exist
under the next Apple account, and an old timer would declare that
account reachable on the strength of a verdict about someone else's.

Rewriting a credential's sync flag is skipped when it already matches
and restores the original if the re-add fails. Changing that flag means
delete-then-add, and the delete propagates to every device — turning
sync on should never be able to log the user out.
This commit is contained in:
Arkadiusz Fal
2026-08-27 23:57:28 +02:00
parent 055a8514d5
commit 81aa47196f
5 changed files with 83 additions and 13 deletions

View File

@@ -789,6 +789,12 @@ final class AccountsManager {
func reloadForCurrentIdentity() {
accounts = []
accountStatuses = [:]
// Their accounts belong to the namespace being left. A migrated account
// reuses its instance's UUID, so the same ID can exist under the new
// identity — and an old timer would then declare *that* account
// reachable again on the strength of a verdict about someone else's.
for expiry in unreachableExpiries.values { expiry.cancel() }
unreachableExpiries = [:]
storedContext = StoredDataContext(context: .local)
migratedInstanceIDs = []
pendingUploads = []

View File

@@ -121,22 +121,42 @@ final class AccountKeychainCredentials {
func resyncCredentialsWithCurrentSetting() {
guard let scope = identityScope.current else { return }
let syncEnabled = shouldSyncToiCloud
for accountID in loginsByScope[scope] ?? [] {
let credentialKey = KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID)
let usernameKey = KeychainCredentialStore.usernameKey(scope: scope, accountID: accountID)
let syncEnabled = shouldSyncToiCloud
if let credential = store.value(forKey: credentialKey) {
_ = store.set(credential, forKey: credentialKey, synchronizable: syncEnabled)
}
if let username = store.value(forKey: usernameKey) {
_ = store.set(username, forKey: usernameKey, synchronizable: syncEnabled)
}
rewrite(
key: KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID),
synchronizable: syncEnabled
)
rewrite(
key: KeychainCredentialStore.usernameKey(scope: scope, accountID: accountID),
synchronizable: syncEnabled
)
}
}
LoggingService.shared.info(
"Rewrote stored \(credentialNoun)s with the current iCloud sync setting",
category: .keychain
/// Rewrites one item so it carries the wanted sync flag.
///
/// Skipped when the flag already matches: the only way to change it is to
/// delete and re-add, which risks the login for no reason — and deleting a
/// synchronizable item propagates to every device. If the re-add does fail,
/// the original is put back the way it was rather than left missing.
private func rewrite(key: String, synchronizable: Bool) {
guard let value = store.value(forKey: key) else { return }
let current = store.isSynchronizable(key: key)
guard current != synchronizable else { return }
let status = store.set(value, forKey: key, synchronizable: synchronizable)
guard status != errSecSuccess else { return }
let restored = store.set(value, forKey: key, synchronizable: current ?? false)
LoggingService.shared.error(
restored == errSecSuccess
? "Could not change how a \(credentialNoun) syncs — left it as it was"
: "Could not change how a \(credentialNoun) syncs, and could not restore it",
category: .keychain,
details: "status=\(store.describe(status))"
)
}

View File

@@ -91,6 +91,30 @@ struct KeychainCredentialStore: Sendable {
return SecItemAdd(query as CFDictionary, nil)
}
/// Whether the stored item is marked as syncing through iCloud Keychain.
///
/// `nil` when there is no item under this key. The attribute records what
/// was requested when the item was written — that is exactly the question
/// here, and deliberately not "does this item belong to the current Apple
/// account", which it cannot answer (measured).
func isSynchronizable(key: String) -> Bool? {
for synchronizable in [true, false] {
let query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: service,
kSecAttrAccount as String: key,
kSecAttrSynchronizable as String: synchronizable,
kSecMatchLimit as String: kSecMatchLimitOne
]
if SecItemCopyMatching(query as CFDictionary, nil) == errSecSuccess {
return synchronizable
}
}
return nil
}
/// Writes a value, replacing any existing item in either synchronizable form.
@discardableResult
func set(_ value: String, forKey key: String, synchronizable: Bool) -> OSStatus {

View File

@@ -409,6 +409,7 @@ final class SubscriptionFeedCache {
do {
let pipedAPI = PipedAPI(httpClient: appEnvironment.httpClient)
let feedVideos = try await pipedAPI.feed(instance: instance, authToken: authToken)
guard isCurrentAccountsResult(generation) else { return }
LoggingService.shared.info("Piped feed: Received \(feedVideos.count) videos", category: .general)
@@ -418,6 +419,7 @@ final class SubscriptionFeedCache {
feedLoadState = .ready
} catch {
LoggingService.shared.error("Failed to fetch Piped feed: \(error.localizedDescription)", category: .general)
guard isCurrentAccountsResult(generation) else { return }
feedLoadState = .error(.networkError(error.localizedDescription))
}
@@ -465,6 +467,8 @@ final class SubscriptionFeedCache {
maxResults: 50
)
guard isCurrentAccountsResult(generation) else { return }
let feedVideos = response.videos
LoggingService.shared.info(
"Invidious feed: Received \(feedVideos.count) videos (page \(currentPage))",
@@ -488,6 +492,7 @@ final class SubscriptionFeedCache {
"Failed to fetch Invidious feed: \(error.localizedDescription)",
category: .general
)
guard isCurrentAccountsResult(generation) else { return }
feedLoadState = .error(.networkError(error.localizedDescription))
}
@@ -519,6 +524,8 @@ final class SubscriptionFeedCache {
maxResults: 50
)
guard isCurrentAccountsResult(generation) else { return }
let feedVideos = response.videos
LoggingService.shared.info(
"Invidious feed: Loaded page \(nextPage), received \(feedVideos.count) videos",

View File

@@ -127,6 +127,19 @@ final class SubscriptionAccountValidator {
return
}
// Local is always a valid context. This validator predates that: it
// treats local as unavailable unless a Yattee Server is configured,
// which is a *feed* concern — the feed says so itself — not a reason to
// move the user onto somebody's backend account, or to overwrite a
// local choice synced from another device.
if accountsManager?.currentContext.isLocal == true {
LoggingService.shared.debug(
"Leaving the local selection alone — local data is always available",
category: .general
)
return
}
// A selection whose account still exists is never corrected here. This
// validator predates account-scoped credentials and reads "not signed
// in" as "not available", which is now routinely temporary — an