fix(prefs): route every credential read through the tolerant path

The wrong-type recovery only covered reads that went through a
BaseSharedPreferencesService instance. The three stores that hold
credentials read the shared cache directly, so a mistyped value there
still threw a raw TypeError or, for Seerr, was swallowed by a catch-all
and reported as "no session" — the registry documented protection it did
not actually provide.

readPreferenceTolerantly now takes the cache, so CredentialVault,
TrackerAccountStore and SeerrSessionStore get the same classification as
the settings layer. CredentialVault's post-write re-read moves outside
its catch: a wrong-typed value written by another isolate was swallowed
there, and the process then returned a key that never durably landed,
making every ciphertext written under it unreadable on the next launch.

Those stores are consulted long after startup, where a throw is an
unhandled provider error rather than a repair prompt, so SettingsService
initialization now walks the cached key set once and reads every
sensitive key. That puts the failure inside a fatal gate step while the
store is still open and a surgical single-key repair is possible.

The remaining direct reads in settings and storage are routed too; the
only ones left are the library-density dual-type migration, which probes
both types deliberately, and an untyped switch that is type-safe by
construction.
This commit is contained in:
edde746
2026-07-31 21:45:32 +02:00
parent 7f0cad339c
commit 66549e3a67
8 changed files with 259 additions and 49 deletions
@@ -244,27 +244,15 @@ abstract class BaseSharedPreferencesService {
/// [UnreadableSensitivePreferenceException], which the startup gate
/// classifies as repairable so the user gets the same consented repair as an
/// unparseable store.
T? _readTolerant<T>(String key, T? Function() read) {
try {
return read();
} on TypeError catch (error, stackTrace) {
if (isSensitivePrefKey(key)) {
appLogger.e('Credential preference "$key" is unreadable', error: error, stackTrace: stackTrace);
Error.throwWithStackTrace(UnreadableSensitivePreferenceException(key, error), stackTrace);
}
appLogger.w('Dropping preference "$key" with an unreadable stored type', error: error, stackTrace: stackTrace);
unawaited(
_cache.remove(key).catchError((Object e, StackTrace s) {
appLogger.d('Could not drop unreadable preference "$key"', error: e, stackTrace: s);
}),
);
return null;
}
}
T? _readTolerant<T>(String key, T? Function() read) => readPreferenceTolerantly(_cache, key, read);
/// Nullable string read routed through [_readTolerant]. Use instead of
/// `prefs.getString(...)` wherever a mistyped stored value must not throw.
String? readNullableString(String key) => _readTolerant(key, () => _cache.getString(key));
/// Nullable reads routed through [readPreferenceTolerantly]. Use these
/// instead of `prefs.getX(...)` wherever a mistyped stored value must not
/// throw — which is everywhere except a call site that deliberately probes
/// two types to migrate between them.
String? readNullableString(String key) => readTolerantString(_cache, key);
bool? readNullableBool(String key) => _readTolerant(key, () => _cache.getBool(key));
int? readNullableInt(String key) => _readTolerant(key, () => _cache.getInt(key));
/// Typed read helpers — return the stored value or [defaultValue] when missing.
bool readBool(String key, {bool defaultValue = false}) =>
@@ -504,3 +492,43 @@ class JsonPref<T> extends Pref<T> {
@override
Future<void> writeTo(BaseSharedPreferencesService svc, T value) => svc.writeString(key, encode(value));
}
/// Reads a preference, tolerating a stored value whose type no longer matches
/// the declaration.
///
/// `SharedPreferencesWithCache.getX` is an `as T?` cast, so a value written by
/// an older build, hand-edited, or partially recovered throws `TypeError`
/// rather than returning null. A value we cannot read is indistinguishable
/// from one that was never written, so drop the key and fall back to the
/// declared default instead of letting it propagate — before #1732 a single
/// mistyped preference could fail the entire startup gate.
///
/// Credential slots are exempt: silently dropping one would sign the user out
/// with no explanation. Those raise [UnreadableSensitivePreferenceException],
/// which the startup gate classifies as repairable so the user gets the same
/// consented repair as an unparseable store.
///
/// Takes the cache directly so the credential stores — which hold a
/// [SharedPreferencesWithCache] rather than a [BaseSharedPreferencesService] —
/// get the same treatment as the settings layer.
T? readPreferenceTolerantly<T>(SharedPreferencesWithCache cache, String key, T? Function() read) {
try {
return read();
} on TypeError catch (error, stackTrace) {
if (isSensitivePrefKey(key)) {
appLogger.e('Credential preference "$key" is unreadable', error: error, stackTrace: stackTrace);
Error.throwWithStackTrace(UnreadableSensitivePreferenceException(key, error), stackTrace);
}
appLogger.w('Dropping preference "$key" with an unreadable stored type', error: error, stackTrace: stackTrace);
unawaited(
cache.remove(key).catchError((Object e, StackTrace s) {
appLogger.d('Could not drop unreadable preference "$key"', error: e, stackTrace: s);
}),
);
return null;
}
}
/// Tolerant string read for a bare [SharedPreferencesWithCache].
String? readTolerantString(SharedPreferencesWithCache cache, String key) =>
readPreferenceTolerantly(cache, key, () => cache.getString(key));
+14 -6
View File
@@ -6,6 +6,7 @@ import 'package:flutter/foundation.dart' show visibleForTesting;
import '../utils/app_logger.dart';
import 'base_shared_preferences_service.dart';
import 'sensitive_prefs.dart';
/// Encrypts credentials before they are persisted in Drift config/token
/// columns. The database no longer stores raw server tokens; registries
@@ -18,7 +19,7 @@ import 'base_shared_preferences_service.dart';
class CredentialVault {
CredentialVault._();
static const String _keyPref = 'credential_vault_key_v1';
static const String _keyPref = credentialVaultKeyPref;
static const String _prefix = 'enc:v1:';
static final AesGcm _algorithm = AesGcm.with256bits();
static Future<SecretKey>? _secretKey;
@@ -152,7 +153,10 @@ class CredentialVault {
} catch (e) {
appLogger.d('CredentialVault: prefs reload before key check failed', error: e);
}
final stored = prefs.getString(_keyPref);
// Tolerant read: a wrong-typed key must surface as a repairable
// failure, not be mistaken for 'no key yet' and silently replaced —
// that would orphan every ciphertext in the database (#1732).
final stored = readTolerantString(prefs, _keyPref);
if (stored != null && stored.isNotEmpty) {
return SecretKey(base64Decode(stored));
}
@@ -160,13 +164,17 @@ class CredentialVault {
await prefs.setString(_keyPref, base64Encode(bytes));
try {
await prefs.reloadCache();
final settled = prefs.getString(_keyPref);
if (settled != null && settled.isNotEmpty) {
return SecretKey(base64Decode(settled));
}
} catch (e) {
appLogger.d('CredentialVault: prefs re-read after key write failed', error: e);
}
// Outside the catch: if another isolate raced us and left a wrong-typed
// value, swallowing it here would return a key that never durably
// landed, and every ciphertext written under it would be unreadable on
// the next launch. Surface it for repair instead (#1732).
final settled = readTolerantString(prefs, _keyPref);
if (settled != null && settled.isNotEmpty) {
return SecretKey(base64Decode(settled));
}
return SecretKey(bytes);
}();
}
+3 -1
View File
@@ -18,7 +18,9 @@ class SeerrSessionStore {
Future<SeerrSession?> load(String userUuid) async {
final prefs = await BaseSharedPreferencesService.sharedCache();
final raw = prefs.getString(_scopedKey(userUuid));
// Outside the try below on purpose: an unreadable credential must
// reach the repair prompt, not be swallowed as 'no session'.
final raw = readTolerantString(prefs, _scopedKey(userUuid));
if (raw == null) return null;
try {
final session = SeerrSession.decode(raw);
-3
View File
@@ -67,9 +67,6 @@ String? profileScopedCredentialBaseKey(String key) {
return null;
}
/// Whether [key] is a profile-scoped or global tracker session slot.
bool isTrackerSessionPrefKey(String key) => trackerSessionBaseKeys.contains(profileScopedCredentialBaseKey(key));
/// Whether [key] is a profile-scoped or global Seerr session slot.
bool isSeerrSessionPrefKey(String key) => profileScopedCredentialBaseKey(key) == seerrSessionBaseKey;
+28 -8
View File
@@ -14,6 +14,7 @@ import '../i18n/strings.g.dart';
import '../models/mpv_config_models.dart';
import '../models/external_player_models.dart';
import 'base_shared_preferences_service.dart';
import 'sensitive_prefs.dart';
import 'device_performance.dart';
import 'shortcut_action.dart';
export 'base_shared_preferences_service.dart'
@@ -94,7 +95,7 @@ class _BufferSizePref extends IntPref {
int readFrom(BaseSharedPreferencesService svc) {
// SharedPreferences updates in-memory cache synchronously, so the
// unawaited disk-flush futures are safe here (idempotent if re-run).
if (svc.prefs.getBool(_bufferSizeMigratedKey) != true) {
if (svc.readNullableBool(_bufferSizeMigratedKey) != true) {
svc.prefs.remove(key);
svc.prefs.setBool(_bufferSizeMigratedKey, true);
}
@@ -141,7 +142,7 @@ class _EpisodePosterModePref extends EnumPref<EpisodePosterMode> {
@override
EpisodePosterMode readFrom(BaseSharedPreferencesService svc) {
final legacyValue = svc.prefs.getBool(_legacyUseSeasonPosterKey);
final legacyValue = svc.readNullableBool(_legacyUseSeasonPosterKey);
if (legacyValue != null) {
final migrated = legacyValue ? EpisodePosterMode.seasonPoster : EpisodePosterMode.seriesPoster;
svc.prefs.remove(_legacyUseSeasonPosterKey);
@@ -158,7 +159,7 @@ class _AppLocalePref extends Pref<AppLocale> {
@override
AppLocale readFrom(BaseSharedPreferencesService svc) {
final code = svc.prefs.getString(key);
final code = svc.readNullableString(key);
if (code == null || code.isEmpty) {
return resolvePreferredAppLocale(PlatformDispatcher.instance.locales);
}
@@ -176,7 +177,7 @@ class _AutoPipPref extends Pref<bool> {
@override
bool readFrom(BaseSharedPreferencesService svc) {
if (!PlatformDetector.supportsPictureInPicture()) return false;
return svc.prefs.getBool(key) ?? !Platform.isMacOS;
return svc.readNullableBool(key) ?? !Platform.isMacOS;
}
@override
@@ -189,7 +190,7 @@ class _UseExternalPlayerPref extends Pref<bool> {
@override
bool readFrom(BaseSharedPreferencesService svc) {
if (!PlatformDetector.supportsExternalPlayers()) return false;
return svc.prefs.getBool(key) ?? false;
return svc.readNullableBool(key) ?? false;
}
@override
@@ -203,7 +204,7 @@ class _AudioPassthroughPref extends Pref<bool> {
@override
bool readFrom(BaseSharedPreferencesService svc) {
final stored = svc.prefs.getBool(key);
final stored = svc.readNullableBool(key);
if (stored != null) return stored;
// Android TV on ExoPlayer defaults to bitstreaming AC3/EAC3/DTS to the TV/AVR
// (Media3 picks bitstream vs PCM via AudioCapabilities), preserving surround.
@@ -251,10 +252,10 @@ class _MpvConfigTextPref extends StringPref {
@override
String readFrom(BaseSharedPreferencesService svc) {
final text = svc.prefs.getString(key);
final text = svc.readNullableString(key);
if (text != null) return text;
final legacyJson = svc.prefs.getString(_legacyMpvConfigEntriesKey);
final legacyJson = svc.readNullableString(_legacyMpvConfigEntriesKey);
if (legacyJson == null) return '';
try {
@@ -596,6 +597,8 @@ class SettingsService extends BaseSharedPreferencesService {
@override
Future<void> onInit() async {
_assertCredentialsReadable();
const legacyRecentRoomsKey = 'watch_together_recent_rooms';
await prefs.remove(legacyRecentRoomsKey);
@@ -609,6 +612,23 @@ class SettingsService extends BaseSharedPreferencesService {
}
}
/// Raises [UnreadableSensitivePreferenceException] if any stored credential
/// has a type we cannot read.
///
/// The credential stores themselves — `CredentialVault`, `TrackerAccountStore`,
/// `SeerrSessionStore` — are consulted long after startup, where a throw
/// would surface as an unhandled provider error rather than the repair
/// prompt. Checking here puts the failure inside a fatal gate step, while
/// the store is open and a surgical single-key repair is still possible
/// (#1732).
///
/// One pass over the already-cached key set; no I/O.
void _assertCredentialsReadable() {
for (final key in prefs.keys) {
if (isSensitivePrefKey(key)) readTolerantString(prefs, key);
}
}
/// Resolves a video mute toggle without replacing the saved volume with 0.
///
/// `persistedVolume` is the non-zero value callers should keep in [volume],
+10 -10
View File
@@ -152,7 +152,7 @@ class StorageService extends BaseSharedPreferencesService {
}
String? getServerEndpoint(ServerId serverId) {
return prefs.getString('$_prefixServerEndpoint$serverId');
return readNullableString('$_prefixServerEndpoint$serverId');
}
Future<void> clearServerEndpoint(ServerId serverId) async {
@@ -184,7 +184,7 @@ class StorageService extends BaseSharedPreferencesService {
/// sees the same device across launches; not Plex-specific in itself —
/// Jellyfin's `DeviceId` header reuses the same value too.
Future<String> getOrCreateClientIdentifier() async {
final existing = prefs.getString(_keyClientId);
final existing = readNullableString(_keyClientId);
if (existing != null && existing.isNotEmpty) return existing;
final generated = const Uuid().v4();
await prefs.setString(_keyClientId, generated);
@@ -383,7 +383,7 @@ class StorageService extends BaseSharedPreferencesService {
'Only ConnectionBootstrap._promoteActiveProfileFromLegacy may read this.',
)
String? getCurrentUserUUID() {
return prefs.getString(_keyCurrentUserUUID);
return readNullableString(_keyCurrentUserUUID);
}
/// Clears the legacy `currentUserUUID` slot. Used by the upgrade migration.
@@ -409,7 +409,7 @@ class StorageService extends BaseSharedPreferencesService {
'Only ConnectionBootstrap.migrateLegacyPlexAccount may use this.',
)
String? getServersListJson() {
return prefs.getString(_keyServersList);
return readNullableString(_keyServersList);
}
/// Clear the legacy servers list.
@@ -429,7 +429,7 @@ class StorageService extends BaseSharedPreferencesService {
// Active app-level profile (kids mode / multi-user gating)
String? getActiveProfileId() => prefs.getString(_keyActiveProfileId);
String? getActiveProfileId() => readNullableString(_keyActiveProfileId);
Future<void> setActiveProfileId(String id) async {
await prefs.setString(_keyActiveProfileId, id);
@@ -449,7 +449,7 @@ class StorageService extends BaseSharedPreferencesService {
}
String? getPlexHomeUsersCacheJson(String connectionId) {
return prefs.getString('$_prefixPlexHomeUsers$connectionId');
return readNullableString('$_prefixPlexHomeUsers$connectionId');
}
Future<void> clearPlexHomeUsersCache(String connectionId) async {
@@ -469,7 +469,7 @@ class StorageService extends BaseSharedPreferencesService {
}
DateTime? getProfileLastUsed(String profileId) {
final ms = prefs.getInt('$_prefixProfileLastUsed$profileId');
final ms = readNullableInt('$_prefixProfileLastUsed$profileId');
return ms == null ? null : DateTime.fromMillisecondsSinceEpoch(ms);
}
@@ -498,7 +498,7 @@ class StorageService extends BaseSharedPreferencesService {
/// Helper to read and decode JSON `List<String>` from preferences
List<String>? _getStringList(String key) {
final jsonString = prefs.getString(key);
final jsonString = readNullableString(key);
if (jsonString == null) return null;
try {
@@ -515,7 +515,7 @@ class StorageService extends BaseSharedPreferencesService {
/// [legacyStringOk] - If true, returns {'key': value, 'descending': false}
/// when value is a plain string (for legacy library sort)
Map<String, dynamic>? _readJsonMap(String key, {bool legacyStringOk = false}) {
final jsonString = prefs.getString(key);
final jsonString = readNullableString(key);
if (jsonString == null) return null;
return decodeJsonStringToMap(jsonString, legacyStringOk: legacyStringOk);
@@ -554,7 +554,7 @@ class StorageService extends BaseSharedPreferencesService {
_forEachScopedKey(baseKey, (key) => _filterServerEntriesFromStringList(key, serverId));
Future<void> _clearSelectedLibraryForServer(String key, ServerId serverId) async {
final selected = prefs.getString(key);
final selected = readNullableString(key);
if (selected != null && _belongsToServer(selected, serverId)) {
await prefs.remove(key);
}
@@ -30,7 +30,7 @@ class TrackerAccountStore {
Future<TrackerSession?> load(String userUuid) async {
final prefs = await BaseSharedPreferencesService.sharedCache();
final raw = prefs.getString(_scopedKey(userUuid));
final raw = readTolerantString(prefs, _scopedKey(userUuid));
if (raw == null) return null;
try {
return TrackerSession.decode(raw, service: service);