fix(#250): refuse to block your own node

Blocking yourself silently hid your own traffic with no visible cause,
and because blocks are offloaded to the radio it was pushed there and
could be pulled back by the connect-time union.

BlockService gains a selfKeyHex, injected by the connector when the self
key is learned and cleared on disconnect. Every add path refuses it
(block, importKeys, maybePromote) — importKeys specifically, so a
self-block already on the radio cannot be re-imported. setSelfKey also
heals an existing self-block and pushes REMOVE, covering the case where
the union pull lands before self-info arrives. Contacts and discovery
sheets hide the Block option for your own node.

Per Gemini review: heal on an unchanged key too (an early return would
strand a self-block introduced after the key was known), and order
setSelfKey so all in-memory state settles synchronously, keeping callers
consistent even though the connector fires it unawaited.

Adds test/services/block_service_test.dart — the first coverage for
BlockService (10 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pull/311/head
Strycher 3 days ago
parent 189751c561
commit 977af98069

@ -2503,8 +2503,19 @@ 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).
void _applySelfKeyToBlockService() {
final service = _blockService;
if (service == null) return;
final key = _selfPublicKey;
unawaited(service.setSelfKey(key == null ? null : pubKeyToHex(key)));
}
void _resetConnectionHandshakeState() {
_selfPublicKey = null;
_applySelfKeyToBlockService();
_selfName = null;
_selfLatitude = null;
_selfLongitude = null;
@ -2671,6 +2682,7 @@ class MeshCoreConnector extends ChangeNotifier {
_conversations.clear();
_loadedConversationKeys.clear();
_selfPublicKey = null;
_applySelfKeyToBlockService();
_selfName = null;
_selfLatitude = null;
_selfLongitude = null;
@ -4489,6 +4501,7 @@ class MeshCoreConnector extends ChangeNotifier {
_currentTxPower = reader.readInt8();
_maxTxPower = reader.readInt8();
_selfPublicKey = reader.readBytes(pubKeySize);
_applySelfKeyToBlockService();
_selfLatitude = reader.readInt32LE() / 1000000.0;
_selfLongitude = reader.readInt32LE() / 1000000.0;
_multiAcks = reader.readByte();

@ -1261,6 +1261,7 @@ class _ContactsScreenState extends State<ContactsScreen>
final isFavorite = contact.isFavorite;
final blockService = context.read<BlockService>();
final isBlocked = blockService.isBlocked(contact.publicKeyHex);
final isSelf = blockService.isSelf(contact.publicKeyHex);
showModalBottomSheet(
context: context,
@ -1268,25 +1269,27 @@ class _ContactsScreenState extends State<ContactsScreen>
child: Column(
mainAxisSize: MainAxisSize.min,
children: [
ListTile(
leading: Icon(
isBlocked ? Icons.check_circle_outline : Icons.block,
color: isBlocked ? null : Colors.red.shade700,
),
title: Text(
isBlocked
? context.l10n.block_unblock
: context.l10n.block_block,
// Blocking your own node is never meaningful (#250).
if (!isSelf)
ListTile(
leading: Icon(
isBlocked ? Icons.check_circle_outline : Icons.block,
color: isBlocked ? null : Colors.red.shade700,
),
title: Text(
isBlocked
? context.l10n.block_unblock
: context.l10n.block_block,
),
onTap: () async {
Navigator.pop(sheetContext);
if (isBlocked) {
await blockService.unblock(contact.publicKeyHex);
} else {
await blockService.block(contact.publicKeyHex);
}
},
),
onTap: () async {
Navigator.pop(sheetContext);
if (isBlocked) {
await blockService.unblock(contact.publicKeyHex);
} else {
await blockService.block(contact.publicKeyHex);
}
},
),
if (isRepeater) ...[
ListTile(
leading: const Icon(Icons.radar, color: Colors.green),

@ -220,6 +220,7 @@ class _DiscoveryScreenState extends State<DiscoveryScreen> {
) async {
final blockService = context.read<BlockService>();
final isBlocked = blockService.isBlocked(contact.publicKeyHex);
final isSelf = blockService.isSelf(contact.publicKeyHex);
final action = await showModalBottomSheet<String>(
context: context,
showDragHandle: true,
@ -239,16 +240,20 @@ class _DiscoveryScreenState extends State<DiscoveryScreen> {
title: Text(l10n.discoveredContacts_copyContact),
onTap: () => Navigator.of(sheetContext).pop('copy_contact'),
),
ListTile(
leading: Icon(
isBlocked ? Icons.check_circle_outline : Icons.block,
color: isBlocked ? null : Colors.red.shade700,
// Blocking your own node is never meaningful (#250).
if (!isSelf)
ListTile(
leading: Icon(
isBlocked ? Icons.check_circle_outline : Icons.block,
color: isBlocked ? null : Colors.red.shade700,
),
title: Text(
isBlocked ? l10n.block_unblock : l10n.block_block,
),
onTap: () => Navigator.of(
sheetContext,
).pop(isBlocked ? 'unblock' : 'block'),
),
title: Text(isBlocked ? l10n.block_unblock : l10n.block_block),
onTap: () => Navigator.of(
sheetContext,
).pop(isBlocked ? 'unblock' : 'block'),
),
ListTile(
leading: const Icon(Icons.delete),
title: Text(l10n.discoveredContacts_deleteContact),

@ -22,6 +22,37 @@ class BlockService extends ChangeNotifier {
/// (the pull side), so a synced key never echoes back to the radio.
void Function(String keyHex, bool blocked)? firmwareSync;
String? _selfKeyHex;
/// The connected node's own public key, injected by the connector once it is
/// learned and cleared on disconnect. Blocking yourself silently hides your
/// own traffic with no way to see why, so every add path refuses it (#250).
String? get selfKeyHex => _selfKeyHex;
bool isSelf(String publicKeyHex) {
final self = _selfKeyHex;
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();
final normalized = (key == null || key.isEmpty) ? null : key;
final changed = normalized != _selfKeyHex;
_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 {
_blockedKeys
@ -45,6 +76,7 @@ class BlockService extends ChangeNotifier {
Future<void> block(String publicKeyHex) async {
final key = publicKeyHex.toLowerCase();
if (isSelf(key)) return;
if (!_blockedKeys.add(key)) return;
await _store.saveKeys(_blockedKeys);
firmwareSync?.call(key, true);
@ -65,7 +97,11 @@ class BlockService extends ChangeNotifier {
Future<void> importKeys(Iterable<String> keysHex) async {
var changed = false;
for (final k in keysHex) {
if (_blockedKeys.add(k.toLowerCase())) changed = true;
final key = k.toLowerCase();
// A self-block already pushed to the radio must not come back via the
// union pull (#250).
if (isSelf(key)) continue;
if (_blockedKeys.add(key)) changed = true;
}
if (!changed) return;
await _store.saveKeys(_blockedKeys);
@ -97,6 +133,13 @@ class BlockService extends ChangeNotifier {
if (n.isEmpty || !_blockedNames.containsKey(n)) return;
final key = publicKeyHex.toLowerCase();
_blockedNames.remove(n);
// Your own name resolving to your own key must not promote into a
// self-block drop the name block and stop (#250).
if (isSelf(key)) {
await _store.saveNames(_blockedNames);
notifyListeners();
return;
}
final added = _blockedKeys.add(key);
await _store.saveNames(_blockedNames);
await _store.saveKeys(_blockedKeys);

@ -0,0 +1,154 @@
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.
class _FakeBlockStore implements BlockStore {
Set<String> keys = {};
Map<String, int> names = {};
@override
Future<Set<String>> loadKeys() async => {...keys};
@override
Future<void> saveKeys(Set<String> value) async => keys = {...value};
@override
Future<Map<String, int>> loadNames() async => {...names};
@override
Future<void> saveNames(Map<String, int> value) async => names = {...value};
}
void main() {
const selfKey = 'aa11bb22cc33dd44';
const otherKey = '99ff88ee77dd66cc';
late _FakeBlockStore store;
late BlockService service;
late List<({String key, bool blocked})> pushes;
setUp(() {
store = _FakeBlockStore();
service = BlockService(store: store);
pushes = [];
service.firmwareSync = (key, blocked) =>
pushes.add((key: key, blocked: blocked));
});
group('self-block guard (#250)', () {
test('block() refuses the self key and pushes nothing', () async {
await service.setSelfKey(selfKey);
pushes.clear();
await service.block(selfKey);
expect(service.isBlocked(selfKey), isFalse);
expect(store.keys, isNot(contains(selfKey)));
expect(pushes, isEmpty);
});
test('block() still blocks a normal key', () async {
await service.setSelfKey(selfKey);
await service.block(otherKey);
expect(service.isBlocked(otherKey), isTrue);
expect(pushes, contains((key: otherKey, blocked: true)));
});
test('importKeys() skips self so the radio cannot re-import it', () async {
await service.setSelfKey(selfKey);
await service.importKeys([selfKey, otherKey]);
expect(service.isBlocked(selfKey), isFalse);
expect(service.isBlocked(otherKey), 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();
await service.setSelfKey(selfKey);
expect(service.isBlocked(selfKey), isFalse);
expect(store.keys, isNot(contains(selfKey)));
expect(pushes, contains((key: selfKey, blocked: false)));
});
test('maybePromote() will not promote a name into a self-block', () async {
await service.setSelfKey(selfKey);
await service.blockName('me');
pushes.clear();
await service.maybePromote('me', selfKey);
expect(service.isBlocked(selfKey), 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);
expect(service.isBlocked(selfKey), isFalse);
expect(
service.isBlocked(otherKey),
isTrue,
reason: 'only self is healed',
);
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);
expect(pushes, isEmpty);
expect(service.isSelf(selfKey), isTrue);
});
test('state is consistent synchronously when not awaited', () async {
await service.setSelfKey(selfKey);
await service.setSelfKey(null);
await service.block(selfKey);
// Fire-and-forget, as the connector does via unawaited().
final future = service.setSelfKey(selfKey);
expect(service.isSelf(selfKey), isTrue);
expect(service.isBlocked(selfKey), isFalse);
await future;
expect(store.keys, isNot(contains(selfKey)));
});
test('isSelf() is false when no self key is known', () {
expect(service.isSelf(selfKey), isFalse);
});
test('clearing the self key stops treating the old key as self', () async {
await service.setSelfKey(selfKey);
await service.setSelfKey(null);
expect(service.isSelf(selfKey), isFalse);
await service.block(selfKey);
expect(service.isBlocked(selfKey), isTrue);
});
});
}
Loading…
Cancel
Save

Powered by TurnKey Linux.