fix(app): stop a duplicate keychain item from crashing the app

Merged
MD
1mo ago
+204-11

Summary

Crashlytics reports a fatal PlatformException(Unexpected security result code, Code: -25299, Message: The specified item already exists in the keychain.) from SharedPreferencesUtil._writeSecureAuthToken (preferences.dart:116). Two defects meet there.

The failure is fatal for no reason. The authToken setter fires its secure-storage write unawaited, so a keychain rejection escapes as an unhandled async error and main.dart records it through recordError(..., fatal: true). The app keeps running; only the crash report says otherwise.

-25299 is reachable in normal use. flutter_secure_storage 9.2.4 writes with a check-then-add (ios/Classes/FlutterSecureStorage.swift:172-222): containsKey first, then SecItemAdd when the probe reports absent. The probe misses an entry that exists in two ways we hit:

  • Entries written between #11917 and #12053 carry the plugin default kSecAttrAccessibleWhenUnlocked; #12053 switched writes to first_unlock. A WhenUnlocked entry cannot be matched while the device is locked — and the app refreshes tokens in the background during BLE streaming and CoreBluetooth restore — so the probe says absent and the add collides. The plugin's own repair path (delete at every accessibility, then add) only runs when the probe found the entry, so it never fires here.
  • The setter is fire-and-forget and the plugin answers each method call on a background dispatch queue, so two overlapping writes both probe "absent" and both add.

What changed

  • Keychain calls are serialized through one queue, so overlapping writes cannot both take the add path.
  • Failures are reported as a value and logged instead of thrown, so a keychain that refuses the write never becomes a fatal crash. The in-memory token keeps the session alive.
  • On -25299 the entry is deleted with no accessibility in the query — the only query that matches whatever accessibility it was stored under — and re-added.
  • Sign-out deletes use that same query. Previously they ran with first_unlock in the query, so a legacy unlocked entry survived logout in the keychain.
  • The migration checks whether the write landed before dropping the prefs copy, so a keychain that never accepts the entry still retries on the next launch instead of losing the token.

Second commit, adjacent defect in the same function: #12053's plaintext mirror (nativeAuthToken) exists for the Android background streaming service, whose Kotlin cannot reach the Flutter keychain wrapper (OmiBackgroundAudioStreamer.kt:306). It runs on every platform, so on iOS and macOS #11917's move of the token into the Keychain was undone by a plaintext NSUserDefaults copy that nothing reads. It is now written only where a native reader exists, and cleared everywhere else so an earlier build's copy is dropped on the next launch.

If the keychain is genuinely locked since boot, the recovery delete cannot match either and the write still fails — now logged rather than fatal, and it self-heals through the plugin's own update path on the next write after unlock.

Tests

app/test/unit/auth_token_secure_storage_migration_test.dart, via a FlutterSecureStorage fake that can reject writes:

  • a duplicate-item write recovers by deleting the stale entry and rewriting;
  • overlapping token writes never run concurrently in the keychain (maxConcurrent == 1), the regression that produced the collision;
  • a keychain that rejects the write leaves the prefs token and the migration flag intact, so the next launch retries;
  • the plaintext mirror is cleared where no native reader consumes it, and a token rotation does not write it back.

Verification

I could not run flutter test for this change. The Flutter SDK path on this machine is unreadable from the agent sandbox (Operation not permitted on every file under it), so the suite and dart format did not run here. The deterministic manifest did: make preflight passes all 13 checks, and the push used the pre-push gate's documented PRE_PUSH_SKIP_DART_FORMAT=1 hatch, so CI's app-dart-format is the first machine to see the formatting. The four tests above are written but unexecuted; please run:

cd app && flutter test test/unit/auth_token_secure_storage_migration_test.dart

Device check worth doing before merge: sign out and back in on iOS, confirm the session survives a cold start, and confirm the crash stops appearing in Crashlytics for builds carrying this change.

Failure class

No existing class in .github/failure-classes/ describes this contract — a fire-and-forget platform write whose rejection escapes as a fatal error, over a store whose existence probe cannot distinguish absent from inaccessible. I did not mint one for a single instance; say the word if you want it registered.

Failure-Class: none