Store the GitHub sync token in the OS keystore, migrating off plaintext

The token moved from plaintext SharedPreferences into flutter_secure_storage
(Android Keystore / libsecret). Only the non-secret owner/repo/clientId stay in
prefs. Migration is confirm-before-delete: load() reads the keystore first and
falls back to the legacy plaintext token, migrating it only once a secure write
succeeds; save() likewise keeps writing plaintext if no secret service is
available, so we degrade to — never below — the old behaviour.

157 tests, 100% line coverage, analyze clean. Verified on-device: Settings
stays connected and sync succeeds after the one-time migration.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

lib/sync/sync_settings.dart

@@ -1,11 +1,14 @@
+import 'package:flutter_secure_storage/flutter_secure_storage.dart';
 import 'package:shared_preferences/shared_preferences.dart';
 
 /// Locally-stored GitHub sync configuration.
 ///
-/// NOTE: the token is currently stored in plain `SharedPreferences`. That is
+/// The GitHub token is kept in the OS keystore (Android Keystore / libsecret)
-/// acceptable for a personal dogfood build, but should move to
+/// via [flutter_secure_storage]; only the non-secret owner/repo/clientId live
-/// `flutter_secure_storage` (Android Keystore / libsecret) before this is
+/// in `SharedPreferences`. Older builds stored the token in plaintext prefs;
-/// considered done. Tracked as a follow-up.
+/// [load]/[save] migrate it transparently and never drop the plaintext copy
+/// until a secure write is confirmed (so we degrade to — never below — the old
+/// behaviour when no secret service is available).
 class SyncSettings {
   const SyncSettings({
     required this.owner,
@@ -37,9 +40,18 @@ class SyncSettings {
 
   static const _kOwner = 'sync.owner';
   static const _kRepo = 'sync.repo';
+  // Legacy plaintext location for the token; read-only now and removed once the
+  // token has been migrated into secure storage.
   static const _kToken = 'sync.token';
   static const _kClientId = 'sync.clientId';
 
+  /// Key for the token inside the OS keystore.
+  static const _secureToken = 'sync.token';
+
+  /// Default options keep us off the deprecated `encryptedSharedPreferences`
+  /// path on Android and use libsecret on Linux.
+  static const _secure = FlutterSecureStorage();
+
   /// Loads settings, defaulting the repo to `kuhyx/todo-sync` and the client id
   /// to the baked-in [defaultClientId] so first run (and any reinstall) needs
   /// nothing but a single "Connect GitHub" tap.
@@ -48,17 +60,58 @@ class SyncSettings {
     return SyncSettings(
       owner: prefs.getString(_kOwner) ?? 'kuhyx',
       repo: prefs.getString(_kRepo) ?? 'todo-sync',
-      token: prefs.getString(_kToken) ?? '',
+      token: await _loadToken(prefs),
       clientId: prefs.getString(_kClientId) ?? defaultClientId,
     );
   }
 
+  /// Reads the token, preferring the keystore and falling back to the legacy
+  /// plaintext value. A legacy value is migrated into the keystore on read, but
+  /// only dropped from prefs once that secure write succeeds.
+  static Future<String> _loadToken(SharedPreferences prefs) async {
+    String? secure;
+    try {
+      secure = await _secure.read(key: _secureToken);
+    } catch (_) {
+      // No secret service available — fall back to the legacy plaintext copy.
+      secure = null;
+    }
+    if (secure != null && secure.isNotEmpty) return secure;
+
+    final legacy = prefs.getString(_kToken) ?? '';
+    if (legacy.isNotEmpty && await _writeSecureToken(legacy)) {
+      await prefs.remove(_kToken);
+    }
+    return legacy;
+  }
+
   Future<void> save() async {
     final prefs = await SharedPreferences.getInstance();
     await prefs.setString(_kOwner, owner);
     await prefs.setString(_kRepo, repo);
-    await prefs.setString(_kToken, token);
     await prefs.setString(_kClientId, clientId);
+    // Confirm-before-delete: only remove the plaintext copy once the keystore
+    // write succeeds; otherwise keep persisting it to prefs as before.
+    if (await _writeSecureToken(token)) {
+      await prefs.remove(_kToken);
+    } else {
+      await prefs.setString(_kToken, token);
+    }
+  }
+
+  /// Writes [token] to the keystore (deleting the entry when empty). Returns
+  /// false if the platform secret service is unavailable.
+  static Future<bool> _writeSecureToken(String token) async {
+    try {
+      if (token.isEmpty) {
+        await _secure.delete(key: _secureToken);
+      } else {
+        await _secure.write(key: _secureToken, value: token);
+      }
+      return true;
+    } catch (_) {
+      return false;
+    }
   }
 
   SyncSettings copyWith({

linux/flutter/generated_plugin_registrant.cc

@@ -7,12 +7,16 @@
 #include "generated_plugin_registrant.h"
 
 #include <file_selector_linux/file_selector_plugin.h>
+#include <flutter_secure_storage_linux/flutter_secure_storage_linux_plugin.h>
 #include <url_launcher_linux/url_launcher_plugin.h>
 
 void fl_register_plugins(FlPluginRegistry* registry) {
   g_autoptr(FlPluginRegistrar) file_selector_linux_registrar =
       fl_plugin_registry_get_registrar_for_plugin(registry, "FileSelectorPlugin");
   file_selector_plugin_register_with_registrar(file_selector_linux_registrar);
+  g_autoptr(FlPluginRegistrar) flutter_secure_storage_linux_registrar =
+      fl_plugin_registry_get_registrar_for_plugin(registry, "FlutterSecureStorageLinuxPlugin");
+  flutter_secure_storage_linux_plugin_register_with_registrar(flutter_secure_storage_linux_registrar);
   g_autoptr(FlPluginRegistrar) url_launcher_linux_registrar =
       fl_plugin_registry_get_registrar_for_plugin(registry, "UrlLauncherPlugin");
   url_launcher_plugin_register_with_registrar(url_launcher_linux_registrar);

linux/flutter/generated_plugins.cmake

@@ -4,6 +4,7 @@
 
 list(APPEND FLUTTER_PLUGIN_LIST
   file_selector_linux
+  flutter_secure_storage_linux
   url_launcher_linux
 )
 

pubspec.lock

@@ -222,6 +222,54 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "6.0.0"
+  flutter_secure_storage:
+    dependency: "direct main"
+    description:
+      name: flutter_secure_storage
+      sha256: "7686b1d6a29985dcbb808c59518226e603e3bfa7c0ddfd1a0d00e4cda77c868e"
+      url: "https://pub.dev"
+    source: hosted
+    version: "10.3.1"
+  flutter_secure_storage_darwin:
+    dependency: transitive
+    description:
+      name: flutter_secure_storage_darwin
+      sha256: "82329fa5cdf343773b1b6897dea959105a29f092454259edff92f9f6637e8149"
+      url: "https://pub.dev"
+    source: hosted
+    version: "0.3.2"
+  flutter_secure_storage_linux:
+    dependency: transitive
+    description:
+      name: flutter_secure_storage_linux
+      sha256: a5f35ddab43cf5c8215d2feb4ce1957851f28c5c37e6f04335066a0602087bf5
+      url: "https://pub.dev"
+    source: hosted
+    version: "3.0.1"
+  flutter_secure_storage_platform_interface:
+    dependency: transitive
+    description:
+      name: flutter_secure_storage_platform_interface
+      sha256: "8ceea1223bee3c6ac1a22dabd8feefc550e4729b3675de4b5900f55afcb435d6"
+      url: "https://pub.dev"
+    source: hosted
+    version: "2.0.1"
+  flutter_secure_storage_web:
+    dependency: transitive
+    description:
+      name: flutter_secure_storage_web
+      sha256: "073a62b3aeb866ab4ce795f960413948e51e5a42a9b0c8333b6daf5bb3208a1c"
+      url: "https://pub.dev"
+    source: hosted
+    version: "2.1.1"
+  flutter_secure_storage_windows:
+    dependency: transitive
+    description:
+      name: flutter_secure_storage_windows
+      sha256: "471951813a97006d899db4948acc654a4f28c440083ea08178935ce20b173ec1"
+      url: "https://pub.dev"
+    source: hosted
+    version: "4.2.2"
   flutter_test:
     dependency: "direct dev"
     description: flutter

pubspec.yaml

@@ -44,6 +44,9 @@ dependencies:
   url_launcher: ^6.3.2
   share_plus: ^13.1.0
   file_selector: ^1.1.0
+  # Stores the GitHub sync token in the OS keystore (Android Keystore /
+  # libsecret) instead of plaintext SharedPreferences.
+  flutter_secure_storage: ^10.3.1
 dev_dependencies:
   flutter_test:
     sdk: flutter

test/capture_screen_test.dart

@@ -10,6 +10,7 @@ import 'package:todo/ui/capture_screen.dart';
 import 'package:todo/ui/settings_screen.dart';
 
 import 'fake_note_repository.dart';
+import 'fake_secure_storage.dart';
 
 void main() {
   // A real CRDT DB schedules sqflite timers that never drain under the
@@ -24,6 +25,7 @@ void main() {
     LocalBackup? localBackup,
   }) async {
     SharedPreferences.setMockInitialValues(prefs);
+    installFakeSecureStorage();
     // Tall surface so a pushed settings screen builds its whole ListView.
     tester.view.physicalSize = const Size(1200, 2800);
     tester.view.devicePixelRatio = 1.0;

test/fake_secure_storage.dart

@@ -0,0 +1,50 @@
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+/// In-memory fake for the `flutter_secure_storage` platform channel so tests
+/// never touch the real OS keystore. Install it from a test (or `setUp`) and it
+/// auto-removes on tear down.
+///
+/// Pass [throwing] to simulate a host with no secret service: every call raises
+/// a [PlatformException], which exercises the plaintext-fallback paths in
+/// [SyncSettings].
+void installFakeSecureStorage({
+  Map<String, String>? initial,
+  bool throwing = false,
+}) {
+  const channel = MethodChannel('plugins.it_nomads.com/flutter_secure_storage');
+  final store = <String, String>{...?initial};
+  final messenger =
+      TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger;
+
+  messenger.setMockMethodCallHandler(channel, (call) async {
+    if (throwing) {
+      throw PlatformException(code: 'unavailable');
+    }
+    final args = (call.arguments as Map?) ?? const <Object?, Object?>{};
+    final key = args['key'] as String?;
+    switch (call.method) {
+      case 'read':
+        return store[key];
+      case 'write':
+        store[key!] = args['value'] as String;
+        return null;
+      case 'delete':
+        store.remove(key);
+        return null;
+      case 'containsKey':
+        return store.containsKey(key);
+      case 'readAll':
+        return Map<String, String>.from(store);
+      case 'deleteAll':
+        store.clear();
+        return null;
+      default:
+        return null;
+    }
+  });
+
+  addTearDown(() {
+    messenger.setMockMethodCallHandler(channel, null);
+  });
+}

test/settings_screen_test.dart

@@ -17,6 +17,7 @@ import 'package:url_launcher_platform_interface/link.dart';
 import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart';
 
 import 'fake_note_repository.dart';
+import 'fake_secure_storage.dart';
 
 /// Stub file picker that returns a fixed in-memory file (no disk I/O, so the
 /// `_import` flow stays timer-free and deterministic under the widget tester).
@@ -85,6 +86,7 @@ void main() {
     FakeNoteRepository? repository,
   }) async {
     SharedPreferences.setMockInitialValues({});
+    installFakeSecureStorage();
     // Tall surface so the whole settings ListView builds (its Backup section
     // is below the default 800×600 fold and would otherwise be lazy-skipped).
     tester.view.physicalSize = const Size(1200, 2800);
@@ -316,6 +318,7 @@ void main() {
     tester,
   ) async {
     SharedPreferences.setMockInitialValues({});
+    installFakeSecureStorage();
     tester.view.physicalSize = const Size(1200, 2800);
     tester.view.devicePixelRatio = 1.0;
     addTearDown(tester.view.resetPhysicalSize);

test/sync_settings_test.dart

@@ -2,11 +2,18 @@ import 'package:flutter_test/flutter_test.dart';
 import 'package:shared_preferences/shared_preferences.dart';
 import 'package:todo/sync/sync_settings.dart';
 
+import 'fake_secure_storage.dart';
+
 void main() {
+  // installFakeSecureStorage touches the test binary messenger, which needs the
+  // binding up first (widget tests get this for free via testWidgets).
+  TestWidgetsFlutterBinding.ensureInitialized();
+
   test(
     'load returns the kuhyx/todo-sync defaults on a fresh install',
     () async {
       SharedPreferences.setMockInitialValues({});
+      installFakeSecureStorage();
       final s = await SyncSettings.load();
       expect(s.owner, 'kuhyx');
       expect(s.repo, 'todo-sync');
@@ -16,8 +23,9 @@ void main() {
     },
   );
 
-  test('save then load round-trips all fields', () async {
+  test('save stores the token in the keystore, not in prefs', () async {
     SharedPreferences.setMockInitialValues({});
+    installFakeSecureStorage();
     await const SyncSettings(
       owner: 'me',
       repo: 'notes',
@@ -25,6 +33,10 @@ void main() {
       clientId: 'cid',
     ).save();
 
+    // Token must not linger in plaintext prefs once secured.
+    final prefs = await SharedPreferences.getInstance();
+    expect(prefs.getString('sync.token'), isNull);
+
     final s = await SyncSettings.load();
     expect(s.owner, 'me');
     expect(s.repo, 'notes');
@@ -32,6 +44,64 @@ void main() {
     expect(s.clientId, 'cid');
   });
 
+  test('load reads the token straight from the keystore', () async {
+    SharedPreferences.setMockInitialValues({});
+    installFakeSecureStorage(initial: {'sync.token': 'fromKeystore'});
+    final s = await SyncSettings.load();
+    expect(s.token, 'fromKeystore');
+  });
+
+  test('load migrates a legacy plaintext token into the keystore', () async {
+    SharedPreferences.setMockInitialValues({'sync.token': 'legacy'});
+    installFakeSecureStorage();
+
+    final s = await SyncSettings.load();
+    expect(s.token, 'legacy');
+
+    // The plaintext copy is dropped once the secure write succeeds, and the
+    // value now resolves from the keystore on the next load.
+    final prefs = await SharedPreferences.getInstance();
+    expect(prefs.getString('sync.token'), isNull);
+    final again = await SyncSettings.load();
+    expect(again.token, 'legacy');
+  });
+
+  test(
+    'load keeps the plaintext token when no secret service is available',
+    () async {
+      SharedPreferences.setMockInitialValues({'sync.token': 'plain'});
+      installFakeSecureStorage(throwing: true);
+
+      final s = await SyncSettings.load();
+      expect(s.token, 'plain');
+      // Never drop the only copy when the keystore write can't be confirmed.
+      final prefs = await SharedPreferences.getInstance();
+      expect(prefs.getString('sync.token'), 'plain');
+    },
+  );
+
+  test('save falls back to plaintext prefs when the keystore fails', () async {
+    SharedPreferences.setMockInitialValues({});
+    installFakeSecureStorage(throwing: true);
+    await const SyncSettings(owner: 'o', repo: 'r', token: 'tok').save();
+
+    final prefs = await SharedPreferences.getInstance();
+    expect(prefs.getString('sync.token'), 'tok');
+  });
+
+  test('save with an empty token clears the keystore entry', () async {
+    // Seed a keystore token, then save an empty token: it must be deleted and
+    // no plaintext copy written.
+    SharedPreferences.setMockInitialValues({});
+    installFakeSecureStorage(initial: {'sync.token': 'old'});
+    await const SyncSettings(owner: 'o', repo: 'r', token: '').save();
+
+    final prefs = await SharedPreferences.getInstance();
+    expect(prefs.getString('sync.token'), isNull);
+    final s = await SyncSettings.load();
+    expect(s.token, '');
+  });
+
   test('isConfigured requires owner, repo and token', () {
     expect(
       const SyncSettings(owner: 'o', repo: 'r', token: 't').isConfigured,