fix(#505, #506): per-radio block storage, drop the accidental global list

Children of #471 (parent stays open for AAB hardware validation, #507).

The block list was one global unscoped list on the phone, unioned onto
every radio on connect. A stale block for one of the owner's own radios
therefore rode onto every fresh/erased radio, and clearing a radio never
stuck: any other radio still holding it re-seeded the global list on
connect, which re-pushed it back.

- #505 block_store.dart: scope keys/names by connected device key (like
  the app's other stores); dropLegacyGlobal() deletes the legacy
  block_keys_v1/block_names_v1 (drop-and-start-fresh, owner-approved).
  No global list.
- #505 block_service.dart: load() drops the legacy global and starts
  empty; loadForDevice(deviceKey) swaps the in-memory set per radio and
  runs the #250 self-heal. All mutating ops serialized through a Future
  chain so loadForDevice and importKeys (different connector frame
  handlers, both unawaited) cannot interleave and wipe each other.
- #506 meshcore_connector.dart: the device-key hook loads the connected
  radio's list, so the offload union reconciles within that one radio.

Existing radio-side firmware blocks left untouched (no auto-CLEAR on
migration, owner-approved). Tests: per-radio isolation, clear-sticks
across reconnect, legacy-drop, disconnect-clears, load/import race.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pull/515/head
Strycher 2 months ago
parent 828aa634c7
commit f5441e527c

@ -2768,14 +2768,18 @@ class MeshCoreConnector extends ChangeNotifier {
_maybeStartInitialChannelSync();
}
/// Keep [BlockService]'s notion of "me" in sync with the connected node, so
/// it can refuse a self-block and self-heal one that already exists, the
/// union pull can land before self-info arrives, so healing matters (#250).
/// (Re)load the connected radio's per-radio block list and keep BlockService's
/// notion of "me" in sync. Called when the device key is learned (device-info)
/// and on disconnect/reset (null). loadForDevice sets the store scope + self
/// key synchronously, so an unawaited call is safe against a concurrent block
/// LIST dump; it also runs the #250 self-heal. Per-radio scoping is what stops
/// a block on one radio leaking to another and re-infecting a cleared radio
/// (#471).
void _applySelfKeyToBlockService() {
final service = _blockService;
if (service == null) return;
final key = _selfPublicKey;
unawaited(service.setSelfKey(key == null ? null : pubKeyToHex(key)));
unawaited(service.loadForDevice(key == null ? null : pubKeyToHex(key)));
}
void _resetConnectionHandshakeState() {

@ -2,11 +2,15 @@ import 'package:flutter/foundation.dart';
import '../storage/block_store.dart';
/// App-global block list, backed by [BlockStore] and exposed to the UI.
/// Per-radio block list, backed by [BlockStore] and exposed to the UI.
///
/// Source of truth for the client: DMs + adverts are filtered by public key,
/// channel posts by resolved key or claimed name. Always active regardless of
/// firmware; the firmware offload (Epic B) mirrors this, never replaces it.
///
/// The set is scoped to the connected radio and swapped by [loadForDevice] on
/// connect; a block set on one radio never leaks to another, and clearing a
/// radio stays cleared (#471). There is no global list.
/// See `docs/architecture/block-contract-as-built.md`.
class BlockService extends ChangeNotifier {
BlockService({BlockStore? store}) : _store = store ?? BlockStore();
@ -22,6 +26,19 @@ class BlockService extends ChangeNotifier {
/// (the pull side), so a synced key never echoes back to the radio.
void Function(String keyHex, bool blocked)? firmwareSync;
/// Serializes all state-mutating operations so they can't interleave. The
/// connector fires [loadForDevice] (on connect) and [importKeys] (from the
/// block LIST dump) without awaiting, and they land in different frame
/// handlers; without this chain, loadForDevice's clear+reload could wipe keys
/// importKeys just added, or vice-versa (#471). Runs each op strictly after
/// the previous one settles; failures don't stall the chain.
Future<void> _opChain = Future<void>.value();
Future<T> _serialize<T>(Future<T> Function() op) {
final result = _opChain.then((_) => op());
_opChain = result.then((_) {}, onError: (_) {});
return result;
}
String? _selfKeyHex;
/// The connected node's own public key, injected by the connector once it is
@ -34,27 +51,29 @@ class BlockService extends ChangeNotifier {
return self != null && publicKeyHex.toLowerCase() == self;
}
/// Called by the connector when the self key is learned (or cleared). If the
/// self key is already stored, blocked before this guard existed, or pulled
/// in from the radio's list, drop it here and tell the radio to remove it.
Future<void> setSelfKey(String? publicKeyHex) async {
final key = publicKeyHex?.toLowerCase();
/// Call once during app startup. Drops the legacy unscoped global list
/// (#471, drop-and-start-fresh) and starts empty; the real per-radio list
/// loads via [loadForDevice] when a radio connects.
Future<void> load() => _serialize(() async {
await _store.dropLegacyGlobal();
_blockedKeys.clear();
_blockedNames.clear();
_selfKeyHex = null;
notifyListeners();
});
/// (Re)load the block list for the connected radio, keyed by [deviceKeyHex]
/// (null on disconnect). Called by the connector when the device key is
/// learned or changes. Swaps the in-memory set so a block set on one radio
/// never leaks to another, and runs the #250 self-heal for this radio's own
/// key. Serialized against [importKeys] so a concurrent block LIST dump can
/// neither be wiped by the reload nor wipe it (#471).
Future<void> loadForDevice(String? deviceKeyHex) => _serialize(() async {
final key = deviceKeyHex?.toLowerCase();
final normalized = (key == null || key.isEmpty) ? null : key;
final changed = normalized != _selfKeyHex;
_store.setPublicKeyHex = normalized ?? '';
_selfKeyHex = normalized;
// Heal even when the key is unchanged, a self-block can appear after the
// key is already known (a union pull racing self-info, or a stale store),
// and an unchanged-key early return would strand it.
final healed = normalized != null && _blockedKeys.remove(normalized);
if (healed) firmwareSync?.call(normalized, false);
if (changed || healed) notifyListeners();
// Everything above is synchronous, so a caller that doesn't await this
// still observes consistent state immediately; only the write is deferred.
if (healed) await _store.saveKeys(_blockedKeys);
}
/// Load persisted state. Call once during app startup.
Future<void> load() async {
await _store.dropLegacyGlobal();
_blockedKeys
..clear()
..addAll(await _store.loadKeys());
@ -62,8 +81,14 @@ class BlockService extends ChangeNotifier {
..clear()
..addAll(await _store.loadNames());
await _pruneExpiredNames();
notifyListeners();
// Self-heal: never keep the connected radio's own key in its own list
// (blocking yourself silently hides your own traffic, #250).
if (normalized != null && _blockedKeys.remove(normalized)) {
await _store.saveKeys(_blockedKeys);
firmwareSync?.call(normalized, false);
}
notifyListeners();
});
Set<String> get blockedKeys => Set.unmodifiable(_blockedKeys);
Map<String, int> get blockedNames => Map.unmodifiable(_blockedNames);
@ -74,27 +99,28 @@ class BlockService extends ChangeNotifier {
bool isNameBlocked(String name) =>
_blockedNames.containsKey(name.trim().toLowerCase());
Future<void> block(String publicKeyHex) async {
Future<void> block(String publicKeyHex) => _serialize(() async {
final key = publicKeyHex.toLowerCase();
if (isSelf(key)) return;
if (!_blockedKeys.add(key)) return;
await _store.saveKeys(_blockedKeys);
firmwareSync?.call(key, true);
notifyListeners();
}
});
Future<void> unblock(String publicKeyHex) async {
Future<void> unblock(String publicKeyHex) => _serialize(() async {
final key = publicKeyHex.toLowerCase();
if (!_blockedKeys.remove(key)) return;
await _store.saveKeys(_blockedKeys);
firmwareSync?.call(key, false);
notifyListeners();
}
});
/// Merge keys learned from the firmware block list into the local set WITHOUT
/// echoing them back to the radio (used by the connect-time union pull).
/// Never removes, the union only adds.
Future<void> importKeys(Iterable<String> keysHex) async {
/// Never removes, the union only adds. Serialized against [loadForDevice] so
/// a reload can't wipe these imports and vice-versa (#471).
Future<void> importKeys(Iterable<String> keysHex) => _serialize(() async {
var changed = false;
for (final k in keysHex) {
final key = k.toLowerCase();
@ -106,21 +132,21 @@ class BlockService extends ChangeNotifier {
if (!changed) return;
await _store.saveKeys(_blockedKeys);
notifyListeners();
}
});
Future<void> blockName(String name) async {
Future<void> blockName(String name) => _serialize(() async {
final n = name.trim().toLowerCase();
if (n.isEmpty || _blockedNames.containsKey(n)) return;
_blockedNames[n] = DateTime.now().millisecondsSinceEpoch;
await _store.saveNames(_blockedNames);
notifyListeners();
}
});
Future<void> unblockName(String name) async {
Future<void> unblockName(String name) => _serialize(() async {
if (_blockedNames.remove(name.trim().toLowerCase()) == null) return;
await _store.saveNames(_blockedNames);
notifyListeners();
}
});
/// Name-only blocks older than this are pruned on load (self-cleaning).
static const Duration _nameBlockTtl = Duration(days: 30);
@ -128,7 +154,8 @@ class BlockService extends ChangeNotifier {
/// Promote a name-only block to a durable pubkey block once we learn the
/// identity behind it (observed via an advert or a DM). No-op if the name
/// isn't name-blocked.
Future<void> maybePromote(String name, String publicKeyHex) async {
Future<void> maybePromote(String name, String publicKeyHex) =>
_serialize(() async {
final n = name.trim().toLowerCase();
if (n.isEmpty || !_blockedNames.containsKey(n)) return;
final key = publicKeyHex.toLowerCase();
@ -145,7 +172,7 @@ class BlockService extends ChangeNotifier {
await _store.saveKeys(_blockedKeys);
if (added) firmwareSync?.call(key, true);
notifyListeners();
}
});
/// Drop name-only blocks that never linked to a pubkey within [_nameBlockTtl].
Future<void> _pruneExpiredNames() async {

@ -3,29 +3,62 @@ import 'dart:convert';
import '../utils/app_logger.dart';
import 'prefs_manager.dart';
/// Persistence for the app-global block list.
/// Per-radio persistence for the block list.
///
/// Deliberately **global**, unlike the other stores it is NOT scoped by the
/// connected device key. A block is "I don't want to see this person," keyed by
/// their public key, and must hold regardless of which radio is connected.
/// See `docs/architecture/block-contract-as-built.md`.
/// Scoped by the connected device key (first 10 hex chars), like the app's
/// other stores. A block belongs to the radio it was set on: the app is the
/// enforcer and registry even on firmware that can't offload, and clearing a
/// radio must stay cleared. There is intentionally **no** global list; a
/// global list unioned across radios re-infects a cleared radio (#471).
///
/// The pre-#471 build stored one unscoped global list (`block_keys_v1` /
/// `block_names_v1`). That is dropped, not migrated (owner decision, #471):
/// the feature is new and the global list was the source of the stray
/// self-block. See `docs/architecture/block-contract-as-built.md`.
class BlockStore {
static const String _keysKey = 'block_keys_v1';
static const String _namesKey = 'block_names_v1';
static const String _keysPrefix = 'block_keys_v1';
static const String _namesPrefix = 'block_names_v1';
/// First 10 hex chars of the connected device key. Empty when no radio is
/// connected; loads/saves are no-ops in that state.
String publicKeyHex = '';
set setPublicKeyHex(String value) =>
publicKeyHex = value.length > 10 ? value.substring(0, 10) : '';
String get _keysKey => '$_keysPrefix$publicKeyHex';
String get _namesKey => '$_namesPrefix$publicKeyHex';
/// Delete the legacy unscoped global list. Idempotent, cheap after the first
/// run (the guarded reads avoid a prefs rewrite when the keys are absent).
/// Drop-and-start-fresh: the global list is not migrated onto any radio.
Future<void> dropLegacyGlobal() async {
final prefs = PrefsManager.instance;
if (prefs.get(_keysPrefix) != null) {
appLogger.info('Dropping legacy global block key list (#471)');
await prefs.remove(_keysPrefix);
}
if (prefs.get(_namesPrefix) != null) {
appLogger.info('Dropping legacy global block name list (#471)');
await prefs.remove(_namesPrefix);
}
}
/// Blocked public keys (lowercased hex).
/// Blocked public keys (lowercased hex) for the current radio.
Future<Set<String>> loadKeys() async {
if (publicKeyHex.isEmpty) return {};
final list = PrefsManager.instance.getStringList(_keysKey) ?? const [];
return list.map((k) => k.toLowerCase()).toSet();
}
Future<void> saveKeys(Set<String> keys) async {
if (publicKeyHex.isEmpty) return;
await PrefsManager.instance.setStringList(_keysKey, keys.toList());
}
/// Name-only blocks: lowercased claimed name -> first-blocked epoch millis.
/// The timestamp feeds the promote-and-prune age-out (Epic A / A7).
Future<Map<String, int>> loadNames() async {
if (publicKeyHex.isEmpty) return {};
final raw = PrefsManager.instance.getString(_namesKey);
if (raw == null || raw.isEmpty) return {};
try {
@ -38,6 +71,7 @@ class BlockStore {
}
Future<void> saveNames(Map<String, int> names) async {
if (publicKeyHex.isEmpty) return;
await PrefsManager.instance.setString(_namesKey, jsonEncode(names));
}
}

@ -2,27 +2,53 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:meshcore_open/services/block_service.dart';
import 'package:meshcore_open/storage/block_store.dart';
/// In-memory stand-in so the service can be exercised without SharedPreferences.
/// In-memory, scope-aware stand-in so the service can be exercised without
/// SharedPreferences. Keys/names are stored per device scope (#471).
class _FakeBlockStore implements BlockStore {
Set<String> keys = {};
Map<String, int> names = {};
final Map<String, Set<String>> _keys = {};
final Map<String, Map<String, int>> _names = {};
bool legacyDropped = false;
@override
Future<Set<String>> loadKeys() async => {...keys};
String publicKeyHex = '';
@override
Future<void> saveKeys(Set<String> value) async => keys = {...value};
set setPublicKeyHex(String value) =>
publicKeyHex = value.length > 10 ? value.substring(0, 10) : '';
@override
Future<Map<String, int>> loadNames() async => {...names};
Future<void> dropLegacyGlobal() async => legacyDropped = true;
Set<String> keysFor(String scope) => {...?_keys[scope]};
@override
Future<Set<String>> loadKeys() async =>
publicKeyHex.isEmpty ? {} : {...?_keys[publicKeyHex]};
@override
Future<void> saveKeys(Set<String> value) async {
if (publicKeyHex.isEmpty) return;
_keys[publicKeyHex] = {...value};
}
@override
Future<void> saveNames(Map<String, int> value) async => names = {...value};
Future<Map<String, int>> loadNames() async =>
publicKeyHex.isEmpty ? {} : {...?_names[publicKeyHex]};
@override
Future<void> saveNames(Map<String, int> value) async {
if (publicKeyHex.isEmpty) return;
_names[publicKeyHex] = {...value};
}
}
void main() {
const selfKey = 'aa11bb22cc33dd44';
const otherKey = '99ff88ee77dd66cc';
// Radios (their own keys double as the per-radio scope + self key).
const radioA = 'a1a1a1a1a1a1a1a1';
const radioB = 'b2b2b2b2b2b2b2b2';
// Contacts to block.
const bob = '99ff88ee77dd66cc';
const cara = '4444555566667777';
late _FakeBlockStore store;
late BlockService service;
@ -37,118 +63,150 @@ void main() {
});
group('self-block guard (#250)', () {
test('block() refuses the self key and pushes nothing', () async {
await service.setSelfKey(selfKey);
test('block() refuses the connected radio\'s own key', () async {
await service.loadForDevice(radioA);
pushes.clear();
await service.block(selfKey);
await service.block(radioA);
expect(service.isBlocked(selfKey), isFalse);
expect(store.keys, isNot(contains(selfKey)));
expect(service.isBlocked(radioA), isFalse);
expect(store.keysFor('a1a1a1a1a1'), isNot(contains(radioA)));
expect(pushes, isEmpty);
});
test('block() still blocks a normal key', () async {
await service.setSelfKey(selfKey);
test('block() still blocks a normal contact', () async {
await service.loadForDevice(radioA);
await service.block(otherKey);
await service.block(bob);
expect(service.isBlocked(otherKey), isTrue);
expect(pushes, contains((key: otherKey, blocked: true)));
expect(service.isBlocked(bob), isTrue);
expect(pushes, contains((key: bob, blocked: true)));
});
test('importKeys() skips self so the radio cannot re-import it', () async {
await service.setSelfKey(selfKey);
test('importKeys() skips the self key from the radio dump', () async {
await service.loadForDevice(radioA);
await service.importKeys([selfKey, otherKey]);
await service.importKeys([radioA, bob]);
expect(service.isBlocked(selfKey), isFalse);
expect(service.isBlocked(otherKey), isTrue);
expect(service.isBlocked(radioA), isFalse);
expect(service.isBlocked(bob), isTrue);
});
test('setSelfKey() heals an existing self-block and sends REMOVE', () async {
// Blocked before the guard existed (or pulled in before self-info landed).
await service.block(selfKey);
expect(service.isBlocked(selfKey), isTrue);
pushes.clear();
test(
'loadForDevice heals a stale self-block already in the store',
() async {
// Radio A's stored list contains A's own key (pre-guard / stale).
store.saveKeysForTest('a1a1a1a1a1', {radioA, bob});
await service.setSelfKey(selfKey);
await service.loadForDevice(radioA);
expect(service.isBlocked(selfKey), isFalse);
expect(store.keys, isNot(contains(selfKey)));
expect(pushes, contains((key: selfKey, blocked: false)));
});
expect(service.isBlocked(radioA), isFalse, reason: 'self healed');
expect(service.isBlocked(bob), isTrue);
expect(pushes, contains((key: radioA, blocked: false)));
},
);
test('maybePromote() will not promote a name into a self-block', () async {
await service.setSelfKey(selfKey);
await service.loadForDevice(radioA);
await service.blockName('me');
pushes.clear();
await service.maybePromote('me', selfKey);
await service.maybePromote('me', radioA);
expect(service.isBlocked(selfKey), isFalse);
expect(service.isBlocked(radioA), isFalse);
expect(service.isNameBlocked('me'), isFalse);
expect(pushes, isEmpty);
});
});
test('heals a self-block even when the self key is unchanged', () async {
await service.setSelfKey(selfKey);
// Stale persisted state reloaded while the self key is already known,
// load() does not filter, so this lands a self-block behind the guards.
store.keys = {selfKey, otherKey};
await service.load();
expect(service.isBlocked(selfKey), isTrue);
pushes.clear();
// Same key as before, so an unchanged-key early return would strand it.
await service.setSelfKey(selfKey);
group('per-radio isolation + drop-and-start-fresh (#471)', () {
test('a block on radio A does not appear on radio B', () async {
await service.loadForDevice(radioA);
await service.block(bob);
expect(service.isBlocked(bob), isTrue);
expect(service.isBlocked(selfKey), isFalse);
await service.loadForDevice(radioB);
expect(
service.isBlocked(otherKey),
isTrue,
reason: 'only self is healed',
service.isBlocked(bob),
isFalse,
reason: 'radio B has its own list',
);
expect(pushes, contains((key: selfKey, blocked: false)));
});
test('repeat setSelfKey with nothing to heal pushes nothing', () async {
await service.setSelfKey(selfKey);
pushes.clear();
await service.setSelfKey(selfKey);
await service.block(cara);
expect(service.isBlocked(cara), isTrue);
expect(service.isBlocked(bob), isFalse);
expect(pushes, isEmpty);
expect(service.isSelf(selfKey), isTrue);
// Back to A: A still has bob, not cara.
await service.loadForDevice(radioA);
expect(service.isBlocked(bob), isTrue);
expect(service.isBlocked(cara), isFalse);
});
test('state is consistent synchronously when not awaited', () async {
await service.setSelfKey(selfKey);
await service.setSelfKey(null);
await service.block(selfKey);
test('clearing a radio stays cleared across a reconnect', () async {
await service.loadForDevice(radioA);
await service.block(bob);
await service.unblock(bob);
expect(service.isBlocked(bob), isFalse);
pushes.clear(); // ignore the legitimate ADD/REMOVE above
// Fire-and-forget, as the connector does via unawaited().
final future = service.setSelfKey(selfKey);
// Connect another radio, then come back to A. Nothing re-seeds bob.
await service.loadForDevice(radioB);
await service.loadForDevice(radioA);
expect(
service.isBlocked(bob),
isFalse,
reason: 'no global list to re-push the unblocked key',
);
expect(
pushes.where((p) => p.key == bob && p.blocked),
isEmpty,
reason: 'bob is never re-ADDed to the radio',
);
});
expect(service.isSelf(selfKey), isTrue);
expect(service.isBlocked(selfKey), isFalse);
await future;
expect(store.keys, isNot(contains(selfKey)));
test('load() drops the legacy global list and starts empty', () async {
await service.load();
expect(store.legacyDropped, isTrue);
expect(service.blockedKeys, isEmpty);
});
test('isSelf() is false when no self key is known', () {
expect(service.isSelf(selfKey), isFalse);
test('loadForDevice also drops the legacy global list', () async {
await service.loadForDevice(radioA);
expect(store.legacyDropped, isTrue);
});
test('clearing the self key stops treating the old key as self', () async {
await service.setSelfKey(selfKey);
await service.setSelfKey(null);
test(
'concurrent loadForDevice + importKeys does not wipe imports (race)',
() async {
// Fire the reload without awaiting, then import from the radio's LIST
// dump immediately after; the connector does exactly this across two
// frame handlers. Serialization must order them so neither wipes the
// other; bob (from the dump) must survive.
final load = service.loadForDevice(radioA);
final import = service.importKeys([bob]);
await Future.wait([load, import]);
expect(
service.isBlocked(bob),
isTrue,
reason: 'imported key survived the concurrent reload',
);
expect(store.keysFor('a1a1a1a1a1'), contains(bob));
},
);
expect(service.isSelf(selfKey), isFalse);
test('disconnect (null device) clears the in-memory set', () async {
await service.loadForDevice(radioA);
await service.block(bob);
expect(service.isBlocked(bob), isTrue);
await service.block(selfKey);
expect(service.isBlocked(selfKey), isTrue);
await service.loadForDevice(null);
expect(service.blockedKeys, isEmpty);
expect(service.isSelf(radioA), isFalse);
});
});
}
extension on _FakeBlockStore {
void saveKeysForTest(String scope, Set<String> keys) => _keys[scope] = keys;
}

Loading…
Cancel
Save

Powered by TurnKey Linux.