mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-07-31 14:02:26 +02:00
Misc bugs around dm region scope + scope display, and flood-scope leak
This commit is contained in:
+17
-1
@@ -115,6 +115,8 @@ async def create_dm_message_from_decrypted(
|
||||
outgoing: bool = False,
|
||||
realtime: bool = True,
|
||||
packet_hash: str | None = None,
|
||||
transport_code: int | None = None,
|
||||
region: str | None = None,
|
||||
) -> int | None:
|
||||
"""Store a decrypted direct message via the shared message service."""
|
||||
return await _create_dm_message_from_decrypted(
|
||||
@@ -131,6 +133,8 @@ async def create_dm_message_from_decrypted(
|
||||
realtime=realtime,
|
||||
broadcast_fn=broadcast_event,
|
||||
packet_hash=packet_hash,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
|
||||
|
||||
@@ -391,7 +395,15 @@ async def process_raw_packet(
|
||||
elif payload_type == PayloadType.TEXT_MESSAGE:
|
||||
# Try to decrypt direct messages using stored private key and known contacts
|
||||
decrypt_result = await _process_direct_message(
|
||||
raw_bytes, packet_id, ts, packet_info, rssi=rssi, snr=snr, packet_hash=pkt_hash
|
||||
raw_bytes,
|
||||
packet_id,
|
||||
ts,
|
||||
packet_info,
|
||||
rssi=rssi,
|
||||
snr=snr,
|
||||
packet_hash=pkt_hash,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
if decrypt_result:
|
||||
result.update(decrypt_result)
|
||||
@@ -656,6 +668,8 @@ async def _process_direct_message(
|
||||
rssi: int | None = None,
|
||||
snr: float | None = None,
|
||||
packet_hash: str | None = None,
|
||||
transport_code: int | None = None,
|
||||
region: str | None = None,
|
||||
) -> dict | None:
|
||||
"""
|
||||
Process a TEXT_MESSAGE (direct message) packet.
|
||||
@@ -780,6 +794,8 @@ async def _process_direct_message(
|
||||
snr=snr,
|
||||
outgoing=effective_outgoing,
|
||||
packet_hash=packet_hash,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
|
||||
return {
|
||||
|
||||
@@ -155,6 +155,8 @@ async def _store_direct_message(
|
||||
best_effort_content_dedup: bool,
|
||||
linked_packet_dedup: bool,
|
||||
packet_hash: str | None = None,
|
||||
transport_code: int | None = None,
|
||||
region: str | None = None,
|
||||
message_repository=MessageRepository,
|
||||
contact_repository=ContactRepository,
|
||||
raw_packet_repository=RawPacketRepository,
|
||||
@@ -213,6 +215,8 @@ async def _store_direct_message(
|
||||
outgoing=outgoing,
|
||||
sender_key=sender_key,
|
||||
sender_name=sender_name,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
if msg_id is None:
|
||||
await handle_duplicate_message(
|
||||
@@ -248,6 +252,8 @@ async def _store_direct_message(
|
||||
outgoing=outgoing,
|
||||
sender_name=sender_name,
|
||||
packet_id=packet_id,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
broadcast_message(
|
||||
message=message, broadcast_fn=broadcast_fn, realtime=realtime, packet_hash=packet_hash
|
||||
@@ -283,6 +289,8 @@ async def ingest_decrypted_direct_message(
|
||||
realtime: bool = True,
|
||||
broadcast_fn: BroadcastFn,
|
||||
packet_hash: str | None = None,
|
||||
transport_code: int | None = None,
|
||||
region: str | None = None,
|
||||
contact_repository=ContactRepository,
|
||||
) -> Message | None:
|
||||
conversation_key = their_public_key.lower()
|
||||
@@ -343,6 +351,8 @@ async def ingest_decrypted_direct_message(
|
||||
best_effort_content_dedup=outgoing,
|
||||
linked_packet_dedup=True,
|
||||
packet_hash=packet_hash,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
if message is None:
|
||||
return None
|
||||
|
||||
@@ -191,30 +191,6 @@ async def send_channel_message_with_effective_scope(
|
||||
# (channel-default) path leaves the radio untouched when no override is set.
|
||||
apply_scope = desired_scope != baseline_scope and (bool(desired_scope) or scope_explicit)
|
||||
|
||||
if apply_scope:
|
||||
logger.info(
|
||||
"Temporarily applying flood_scope %s for %s",
|
||||
desired_scope or "(unscoped)",
|
||||
channel.name,
|
||||
)
|
||||
override_result = await set_radio_flood_scope(
|
||||
mc, desired_scope, fw_ver=radio_manager.firmware_ver_code
|
||||
)
|
||||
if override_result is not None and override_result.type == EventType.ERROR:
|
||||
logger.warning(
|
||||
"Failed to apply flood_scope %r for %s: %s",
|
||||
desired_scope,
|
||||
channel.name,
|
||||
override_result.payload,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=422,
|
||||
detail=(
|
||||
f"Failed to apply regional override {desired_scope!r} before {action_label}: "
|
||||
f"{override_result.payload}"
|
||||
),
|
||||
)
|
||||
|
||||
# Path hash mode per-channel override
|
||||
override_phm = channel.path_hash_mode_override
|
||||
baseline_phm = radio_manager.path_hash_mode
|
||||
@@ -224,28 +200,59 @@ async def send_channel_message_with_effective_scope(
|
||||
and override_phm != baseline_phm
|
||||
)
|
||||
|
||||
if apply_phm:
|
||||
logger.info(
|
||||
"Temporarily applying channel path_hash_mode override for %s: %d",
|
||||
channel.name,
|
||||
override_phm,
|
||||
)
|
||||
phm_result = await mc.commands.set_path_hash_mode(override_phm)
|
||||
if phm_result is not None and phm_result.type == EventType.ERROR:
|
||||
logger.warning(
|
||||
"Failed to apply channel path_hash_mode override for %s: %s",
|
||||
channel.name,
|
||||
phm_result.payload,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=422,
|
||||
detail=(
|
||||
f"Failed to apply path hash mode override before {action_label}: "
|
||||
f"{phm_result.payload}"
|
||||
),
|
||||
)
|
||||
|
||||
# Apply the temporary overrides and send inside the try so the finally below
|
||||
# always restores the radio's baseline scope/hash-mode -- even when applying an
|
||||
# override raises (e.g. the radio rejects the scope, or the path-hash-mode apply
|
||||
# fails) or the send itself throws. ``apply_scope``/``apply_phm`` are computed
|
||||
# above so the finally can reference them regardless of where we fail, and
|
||||
# restoring to baseline is idempotent, so forcing baseline back after an apply
|
||||
# whose effect on the radio is unknown is safe.
|
||||
try:
|
||||
if apply_scope:
|
||||
logger.info(
|
||||
"Temporarily applying flood_scope %s for %s",
|
||||
desired_scope or "(unscoped)",
|
||||
channel.name,
|
||||
)
|
||||
override_result = await set_radio_flood_scope(
|
||||
mc, desired_scope, fw_ver=radio_manager.firmware_ver_code
|
||||
)
|
||||
if override_result is not None and override_result.type == EventType.ERROR:
|
||||
logger.warning(
|
||||
"Failed to apply flood_scope %r for %s: %s",
|
||||
desired_scope,
|
||||
channel.name,
|
||||
override_result.payload,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=422,
|
||||
detail=(
|
||||
f"Failed to apply regional override {desired_scope!r} before {action_label}: "
|
||||
f"{override_result.payload}"
|
||||
),
|
||||
)
|
||||
|
||||
if apply_phm:
|
||||
logger.info(
|
||||
"Temporarily applying channel path_hash_mode override for %s: %d",
|
||||
channel.name,
|
||||
override_phm,
|
||||
)
|
||||
phm_result = await mc.commands.set_path_hash_mode(override_phm)
|
||||
if phm_result is not None and phm_result.type == EventType.ERROR:
|
||||
logger.warning(
|
||||
"Failed to apply channel path_hash_mode override for %s: %s",
|
||||
channel.name,
|
||||
phm_result.payload,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=422,
|
||||
detail=(
|
||||
f"Failed to apply path hash mode override before {action_label}: "
|
||||
f"{phm_result.payload}"
|
||||
),
|
||||
)
|
||||
|
||||
channel_slot, needs_configure, evicted_channel_key = radio_manager.plan_channel_send_slot(
|
||||
channel_key,
|
||||
preferred_slot=temp_radio_slot,
|
||||
|
||||
@@ -408,6 +408,8 @@ async def create_dm_message_from_decrypted(
|
||||
realtime: bool = True,
|
||||
broadcast_fn: BroadcastFn,
|
||||
packet_hash: str | None = None,
|
||||
transport_code: int | None = None,
|
||||
region: str | None = None,
|
||||
) -> int | None:
|
||||
"""Store and broadcast a decrypted direct message."""
|
||||
from app.services.dm_ingest import ingest_decrypted_direct_message
|
||||
@@ -425,6 +427,8 @@ async def create_dm_message_from_decrypted(
|
||||
realtime=realtime,
|
||||
broadcast_fn=broadcast_fn,
|
||||
packet_hash=packet_hash,
|
||||
transport_code=transport_code,
|
||||
region=region,
|
||||
)
|
||||
return message.id if message is not None else None
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ import { ChannelFloodScopeOverrideModal } from './ChannelFloodScopeOverrideModal
|
||||
import { ChannelPathHashModeOverrideModal } from './ChannelPathHashModeOverrideModal';
|
||||
import { handleKeyboardActivate } from '../utils/a11y';
|
||||
import { isPublicChannelKey } from '../utils/publicChannel';
|
||||
import { stripRegionScopePrefix } from '../utils/regionScope';
|
||||
import { stripRegionScopePrefix, floodScopeOverrideLabel } from '../utils/regionScope';
|
||||
import { isPrefixOnlyContact } from '../utils/pubkey';
|
||||
import { cn } from '../lib/utils';
|
||||
import { ContactAvatar } from './ContactAvatar';
|
||||
@@ -102,7 +102,9 @@ export function ChatHeader({
|
||||
const activeFloodScopeLabel = activeFloodScopeOverride
|
||||
? stripRegionScopePrefix(activeFloodScopeOverride)
|
||||
: null;
|
||||
const activeFloodScopeDisplay = activeFloodScopeOverride ? activeFloodScopeOverride : null;
|
||||
// Badge text: maps the raw override ("*", "#Region", null) to a friendly label
|
||||
// so the unscoped marker renders as "unscoped" instead of a bare "*".
|
||||
const activeFloodScopeBadge = floodScopeOverrideLabel(activeFloodScopeOverride);
|
||||
const activePathHashModeOverride =
|
||||
conversation.type === 'channel' ? (activeChannel?.path_hash_mode_override ?? null) : null;
|
||||
const showPathHashModeOverride =
|
||||
@@ -256,7 +258,7 @@ export function ChatHeader({
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
{conversation.type === 'channel' && activeFloodScopeDisplay && (
|
||||
{conversation.type === 'channel' && activeFloodScopeBadge && (
|
||||
<button
|
||||
className="mt-0.5 flex basis-full items-center gap-1 text-left sm:hidden"
|
||||
onClick={handleEditFloodScopeOverride}
|
||||
@@ -268,7 +270,7 @@ export function ChatHeader({
|
||||
aria-hidden="true"
|
||||
/>
|
||||
<span className="min-w-0 truncate text-[0.6875rem] font-medium text-[hsl(var(--region-override))]">
|
||||
{activeFloodScopeDisplay}
|
||||
{activeFloodScopeBadge}
|
||||
</span>
|
||||
</button>
|
||||
)}
|
||||
@@ -445,9 +447,9 @@ export function ChatHeader({
|
||||
className={`h-4 w-4 ${activeFloodScopeLabel ? 'text-[hsl(var(--region-override))]' : 'text-muted-foreground'}`}
|
||||
aria-hidden="true"
|
||||
/>
|
||||
{activeFloodScopeDisplay && (
|
||||
{activeFloodScopeBadge && (
|
||||
<span className="hidden text-[0.6875rem] font-medium text-[hsl(var(--region-override))] sm:inline">
|
||||
{activeFloodScopeDisplay}
|
||||
{activeFloodScopeBadge}
|
||||
</span>
|
||||
)}
|
||||
</button>
|
||||
@@ -513,7 +515,7 @@ export function ChatHeader({
|
||||
open={channelOverrideOpen}
|
||||
onClose={() => setChannelOverrideOpen(false)}
|
||||
roomName={conversation.name}
|
||||
currentOverride={activeFloodScopeDisplay}
|
||||
currentOverride={activeFloodScopeOverride}
|
||||
onSetOverride={(value) => onSetChannelFloodScopeOverride(conversation.id, value)}
|
||||
/>
|
||||
)}
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
UNSCOPED_OVERRIDE_MARKER,
|
||||
isUnscopedMarker,
|
||||
stripRegionScopePrefix,
|
||||
floodScopeOverrideLabel,
|
||||
} from '../utils/regionScope';
|
||||
|
||||
describe('regionScope helpers', () => {
|
||||
describe('isUnscopedMarker', () => {
|
||||
it('recognizes the unscoped marker', () => {
|
||||
expect(isUnscopedMarker(UNSCOPED_OVERRIDE_MARKER)).toBe(true);
|
||||
expect(isUnscopedMarker('*')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects regions, null, undefined, and empty', () => {
|
||||
expect(isUnscopedMarker('#Esperance')).toBe(false);
|
||||
expect(isUnscopedMarker('Esperance')).toBe(false);
|
||||
expect(isUnscopedMarker(null)).toBe(false);
|
||||
expect(isUnscopedMarker(undefined)).toBe(false);
|
||||
expect(isUnscopedMarker('')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('stripRegionScopePrefix', () => {
|
||||
it('strips a leading hash', () => {
|
||||
expect(stripRegionScopePrefix('#Esperance')).toBe('Esperance');
|
||||
});
|
||||
|
||||
it('leaves unprefixed values (incl. the marker) unchanged', () => {
|
||||
expect(stripRegionScopePrefix('Esperance')).toBe('Esperance');
|
||||
expect(stripRegionScopePrefix('*')).toBe('*');
|
||||
});
|
||||
|
||||
it('returns empty string for nullish input', () => {
|
||||
expect(stripRegionScopePrefix(null)).toBe('');
|
||||
expect(stripRegionScopePrefix(undefined)).toBe('');
|
||||
expect(stripRegionScopePrefix('')).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
describe('floodScopeOverrideLabel', () => {
|
||||
it('returns null when there is no override (inherit)', () => {
|
||||
expect(floodScopeOverrideLabel(null)).toBeNull();
|
||||
expect(floodScopeOverrideLabel(undefined)).toBeNull();
|
||||
expect(floodScopeOverrideLabel('')).toBeNull();
|
||||
});
|
||||
|
||||
it('maps the unscoped marker to a friendly label, not a bare "*"', () => {
|
||||
expect(floodScopeOverrideLabel(UNSCOPED_OVERRIDE_MARKER)).toBe('unscoped');
|
||||
expect(floodScopeOverrideLabel('*')).toBe('unscoped');
|
||||
});
|
||||
|
||||
it('passes a region name through unchanged', () => {
|
||||
expect(floodScopeOverrideLabel('#Esperance')).toBe('#Esperance');
|
||||
expect(floodScopeOverrideLabel('Esperance')).toBe('Esperance');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -11,3 +11,12 @@ export function stripRegionScopePrefix(scope: string | null | undefined): string
|
||||
if (!scope) return '';
|
||||
return scope.startsWith('#') ? scope.slice(1) : scope;
|
||||
}
|
||||
|
||||
// Compact, human-readable label for a channel's persisted flood-scope override,
|
||||
// suitable for a header badge. Returns null when there is no override (inherit),
|
||||
// "unscoped" for the unscoped marker, and the region name as-is otherwise.
|
||||
export function floodScopeOverrideLabel(scope: string | null | undefined): string | null {
|
||||
if (!scope) return null;
|
||||
if (isUnscopedMarker(scope)) return 'unscoped';
|
||||
return scope;
|
||||
}
|
||||
|
||||
@@ -1398,6 +1398,53 @@ class TestCreateDMMessageFromDecrypted:
|
||||
assert broadcast["paths"][0]["path"] == "aabbcc"
|
||||
assert broadcast["paths"][0]["received_at"] == 1700000001
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dm_includes_region_scope_in_broadcast(self, test_db, captured_broadcasts):
|
||||
"""A region-scoped (transport-routed) flood DM threads transport_code/region
|
||||
into the stored row and the broadcast, so bots see `scoped`/`region` for DMs
|
||||
(issue #300 DM half) and the UI can badge the scope."""
|
||||
from app.decoder import DecryptedDirectMessage
|
||||
from app.packet_processor import create_dm_message_from_decrypted
|
||||
|
||||
packet_id, _ = await RawPacketRepository.create(b"region_test_dm", 1700000000)
|
||||
|
||||
decrypted = DecryptedDirectMessage(
|
||||
timestamp=1700000000,
|
||||
flags=0,
|
||||
message="Scoped DM",
|
||||
dest_hash="fa",
|
||||
src_hash="a1",
|
||||
)
|
||||
|
||||
broadcasts, mock_broadcast = captured_broadcasts
|
||||
|
||||
with patch("app.packet_processor.broadcast_event", mock_broadcast):
|
||||
msg_id = await create_dm_message_from_decrypted(
|
||||
packet_id=packet_id,
|
||||
decrypted=decrypted,
|
||||
their_public_key=self.A1B2C3_PUB,
|
||||
our_public_key=self.FACE12_PUB,
|
||||
received_at=1700000001,
|
||||
outgoing=False,
|
||||
transport_code=0x1234,
|
||||
region="#Esperance",
|
||||
)
|
||||
|
||||
assert msg_id is not None
|
||||
|
||||
message_broadcasts = [b for b in broadcasts if b["type"] == "message"]
|
||||
assert len(message_broadcasts) == 1
|
||||
broadcast = message_broadcasts[0]["data"]
|
||||
# Bots derive `scoped = transport_code is not None` and read `region`.
|
||||
assert broadcast["transport_code"] == 0x1234
|
||||
assert broadcast["region"] == "#Esperance"
|
||||
|
||||
# Persisted so the scope survives a raw-packet purge, mirroring channel msgs.
|
||||
stored = await MessageRepository.get_by_id(msg_id)
|
||||
assert stored is not None
|
||||
assert stored.transport_code == 0x1234
|
||||
assert stored.region == "#Esperance"
|
||||
|
||||
|
||||
class TestDMDecryptionFunction:
|
||||
"""Test the DM decryption function with real crypto."""
|
||||
|
||||
@@ -1426,6 +1426,50 @@ class TestPathHashModeOverride:
|
||||
assert "path hash mode" in exc_info.value.detail.lower()
|
||||
mc.commands.send_chan_msg.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_channel_msg_restores_scope_when_phm_apply_fails(self, test_db):
|
||||
"""A flood-scope override that was applied must still be restored when the
|
||||
subsequent path-hash-mode apply fails. Regression: the scope apply and the
|
||||
PHM apply used to sit outside the try/finally, so a PHM error left the radio
|
||||
stuck on the temporary region scope for all later traffic."""
|
||||
mc = _make_mc(name="MyNode")
|
||||
# PHM apply (first call) errors -> raises; PHM restore (second call, in the
|
||||
# finally) succeeds so this test isolates the scope-restore behavior.
|
||||
mc.commands.set_path_hash_mode = AsyncMock(
|
||||
side_effect=[
|
||||
MagicMock(type=EventType.ERROR, payload="unsupported mode"),
|
||||
_make_radio_result(),
|
||||
]
|
||||
)
|
||||
chan_key = "f6" * 16
|
||||
await ChannelRepository.upsert(key=chan_key, name="#both")
|
||||
await ChannelRepository.update_flood_scope_override(chan_key, "Esperance")
|
||||
await ChannelRepository.update_path_hash_mode_override(chan_key, 2)
|
||||
await AppSettingsRepository.update(flood_scope="Baseline")
|
||||
|
||||
radio_manager.path_hash_mode = 0
|
||||
radio_manager.path_hash_mode_supported = True
|
||||
|
||||
with (
|
||||
patch("app.routers.messages.radio_manager.require_connected", return_value=mc),
|
||||
patch.object(radio_manager, "_meshcore", mc),
|
||||
patch("app.routers.messages.broadcast_event"),
|
||||
pytest.raises(HTTPException) as exc_info,
|
||||
):
|
||||
await send_channel_message(
|
||||
SendChannelMessageRequest(channel_key=chan_key, text="hello")
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 422
|
||||
assert "path hash mode" in exc_info.value.detail.lower()
|
||||
# The send never happens...
|
||||
mc.commands.send_chan_msg.assert_not_awaited()
|
||||
# ...but the applied region scope is restored to the global baseline.
|
||||
assert mc.commands.set_flood_scope.await_args_list == [
|
||||
call("#Esperance"),
|
||||
call("#Baseline"),
|
||||
]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_channel_msg_phm_restore_failure_broadcasts_error(self, test_db):
|
||||
"""Message sends OK but restore failure after 3 attempts broadcasts an error."""
|
||||
|
||||
Reference in New Issue
Block a user