mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-10-15 18:49:52 +00:00
fix(region): never warn the synthetic "Channel User" sender
A channel message with no "Name: " prefix falls back to a stand-in name. That is not a node: every such message shares the identity, so they would accumulate a run together and then get a DM addressed to a contact that does not exist. The hook now passes no sender for those, so they are still counted but can never earn a warning. The literal is a named constant and a test pins the hook's guard to it. Also folds the channel body-budget formula into models.channel_body_limit rather than letting the web viewer keep a fourth copy of "max(130, 160 - len(name) - 2)"; BaseCommand and CommandManager now call it too, and 158 becomes models.DM_BODY_LIMIT.
This commit is contained in:
@@ -27,6 +27,8 @@ Only `global` can earn a warning, and only on positive evidence. If the radio di
|
||||
|
||||
`unknown` is excluded from the unscoped percentage rather than counted as clean, so a mesh the bot cannot classify reads as "no data" instead of "no problem".
|
||||
|
||||
A channel message that arrives with no `Name: ` prefix has no attributable sender. It is still counted, but it can never earn anyone a warning — every such message would otherwise share one synthetic identity.
|
||||
|
||||
Messages the radio cached from before the current connection are skipped entirely — a reconnect replays them as a burst, and counting them would both distort the tallies and let stale traffic earn someone a warning.
|
||||
|
||||
## Reading the page
|
||||
|
||||
@@ -30,7 +30,12 @@ from .config_validation import (
|
||||
_channel_name_is_public,
|
||||
strip_optional_quotes,
|
||||
)
|
||||
from .models import CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD, MeshMessage
|
||||
from .models import (
|
||||
CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD,
|
||||
DM_BODY_LIMIT,
|
||||
MeshMessage,
|
||||
channel_body_limit,
|
||||
)
|
||||
from .plugin_loader import PluginLoader
|
||||
from .security_utils import sanitize_name, validate_safe_path
|
||||
from .utils import check_internet_connectivity_async, decode_escape_sequences, format_keyword_response_with_placeholders
|
||||
@@ -687,7 +692,7 @@ class CommandManager:
|
||||
can be called outside of a specific command instance.
|
||||
"""
|
||||
if message.is_dm:
|
||||
return 158
|
||||
return DM_BODY_LIMIT
|
||||
username: str | None = None
|
||||
try:
|
||||
if hasattr(self.bot, 'meshcore') and self.bot.meshcore:
|
||||
@@ -701,7 +706,7 @@ class CommandManager:
|
||||
pass
|
||||
if not username:
|
||||
username = self.bot.config.get('Bot', 'bot_name', fallback='Bot')
|
||||
max_length = max(130, 160 - len(str(username).encode('utf-8')) - 2)
|
||||
max_length = channel_body_limit(username)
|
||||
if not MeshMessage.is_global_flood_scope(message.effective_outgoing_flood_scope(self.bot)):
|
||||
max_length -= CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD
|
||||
return max_length
|
||||
|
||||
@@ -19,7 +19,12 @@ from ..command_prefix import (
|
||||
normalize_command_content,
|
||||
)
|
||||
from ..config_schema import LEGACY_ENABLED_ALIASES
|
||||
from ..models import CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD, MeshMessage
|
||||
from ..models import (
|
||||
CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD,
|
||||
DM_BODY_LIMIT,
|
||||
MeshMessage,
|
||||
channel_body_limit,
|
||||
)
|
||||
from ..security_utils import validate_pubkey_format
|
||||
from ..utils import (
|
||||
format_elapsed_display,
|
||||
@@ -689,7 +694,7 @@ class BaseCommand(ABC):
|
||||
int: Maximum message body length in UTF-8 bytes.
|
||||
"""
|
||||
if message.is_dm:
|
||||
return 158
|
||||
return DM_BODY_LIMIT
|
||||
|
||||
# For channel messages, calculate based on bot username length
|
||||
# Try to get device username from meshcore first (actual radio username)
|
||||
@@ -712,9 +717,7 @@ class BaseCommand(ABC):
|
||||
if not username:
|
||||
username = self.bot.config.get('Bot', 'bot_name', fallback='Bot')
|
||||
|
||||
# 160 bytes are available for channel messages
|
||||
# Calculate max length: 160 - username_length - 2 (for ": ")
|
||||
max_length = max(130, 160 - len(str(username).encode('utf-8')) - 2)
|
||||
max_length = channel_body_limit(username)
|
||||
if not MeshMessage.is_global_flood_scope(message.effective_outgoing_flood_scope(self.bot)):
|
||||
max_length -= CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD
|
||||
return max_length
|
||||
|
||||
@@ -30,6 +30,10 @@ from .utils import (
|
||||
# MessageHandler.find_recent_rf_data. Anything other than a fallback is known to be
|
||||
# this message's own packet; a fallback is merely the most recent packet heard, so its
|
||||
# route belongs to some other transmission and must not be attributed (issue #80).
|
||||
# Stand-in used when a channel message carries no "Name: " prefix to extract a
|
||||
# sender from. It is not a node: every such message would share this identity.
|
||||
CHANNEL_SENDER_FALLBACK = "Channel User"
|
||||
|
||||
RF_MATCH_KEY = "_rf_match"
|
||||
RF_MATCH_EXACT = "exact"
|
||||
RF_MATCH_PUBKEY = "pubkey"
|
||||
@@ -2608,7 +2612,7 @@ class MessageHandler:
|
||||
|
||||
# Get sender information from text field if it's in "SENDER: message" format
|
||||
text = payload.get("text", "")
|
||||
sender_id = "Channel User" # Default fallback
|
||||
sender_id = CHANNEL_SENDER_FALLBACK # Default fallback
|
||||
|
||||
# Try to extract sender from text field (e.g., "HOWL: Test" -> "HOWL")
|
||||
message_content = text # Default to full text
|
||||
@@ -2837,7 +2841,9 @@ class MessageHandler:
|
||||
# allowlist drops — running it after the gate would blind the
|
||||
# monitor to the traffic it exists to measure.
|
||||
await self._observe_flood_scope(
|
||||
sender_id=sender_id,
|
||||
# A message with no "Name: " prefix has no attributable sender,
|
||||
# so it is counted but can never earn anyone a warning.
|
||||
sender_id=None if sender_id == CHANNEL_SENDER_FALLBACK else sender_id,
|
||||
sender_pubkey=payload.get("pubkey_prefix", ""),
|
||||
channel=channel_name,
|
||||
sender_timestamp=payload.get("sender_timestamp", 0),
|
||||
|
||||
@@ -10,6 +10,24 @@ from typing import Any, Optional
|
||||
# Firmware reserves extra bytes for regional (non-global) TC_FLOOD scope on channel text.
|
||||
CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD = 10
|
||||
|
||||
# A DM carries no username prefix, so the whole cipher block is body.
|
||||
DM_BODY_LIMIT = 158
|
||||
|
||||
|
||||
def channel_body_limit(username: Optional[str]) -> int:
|
||||
"""Global-scope body budget in UTF-8 bytes for a channel message from ``username``.
|
||||
|
||||
Channel messages go out as ``"<username>: <body>"``, so the budget is the
|
||||
160-byte cipher block minus the name and the ``": "``. Regional scope costs
|
||||
a further ``CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD``, which callers
|
||||
subtract themselves once they know the outgoing scope.
|
||||
|
||||
Shared by the command layer and the web viewer, which computes the same
|
||||
number in a process that has no bot object.
|
||||
"""
|
||||
name = str(username or "Bot")
|
||||
return max(130, 160 - len(name.encode("utf-8")) - 2)
|
||||
|
||||
|
||||
@dataclass
|
||||
class MeshMessage:
|
||||
|
||||
@@ -38,6 +38,9 @@ from dataclasses import dataclass, field
|
||||
from datetime import date, datetime, timedelta
|
||||
from typing import Any, Optional
|
||||
|
||||
from modules.models import DM_BODY_LIMIT as _DM_BODY_LIMIT
|
||||
from modules.models import channel_body_limit
|
||||
|
||||
CONFIG_SECTION = "Region_Warnings"
|
||||
|
||||
# Verdicts produced by MessageHandler._classify_channel_flood_scope.
|
||||
@@ -64,8 +67,9 @@ DEFAULT_MESSAGE = (
|
||||
"Setting one in your MeshCore app keeps things quiet. Thanks!"
|
||||
)
|
||||
|
||||
# Body budget for a DM, matching CommandManager.get_max_message_length.
|
||||
DM_BODY_LIMIT = 158
|
||||
# Re-exported so the web viewer (which has no bot object) can size the preview
|
||||
# against the same budget the command layer enforces.
|
||||
DM_BODY_LIMIT = _DM_BODY_LIMIT
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -577,14 +581,17 @@ class RegionWarningMonitor:
|
||||
return ok, body if ok else f"DM send failed (contact unknown or radio busy): {body}"
|
||||
|
||||
def _channel_body_limit(self) -> int:
|
||||
"""Channel body budget for a global-scope send from this node."""
|
||||
"""Channel body budget for a global-scope send from this node.
|
||||
|
||||
Warnings always go out unscoped, so no regional-scope overhead applies.
|
||||
"""
|
||||
try:
|
||||
from modules.models import MeshMessage
|
||||
|
||||
probe = MeshMessage(content="", channel="", is_dm=False, reply_scope="")
|
||||
return int(self.bot.command_manager.get_max_message_length(probe))
|
||||
except Exception:
|
||||
return 130
|
||||
return channel_body_limit(None)
|
||||
|
||||
# -- gating ------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -56,6 +56,7 @@ from modules.db_retention import (
|
||||
)
|
||||
from modules.ini_writer import IniValueError, update_ini_values
|
||||
from modules.maintenance import MaintenanceRunner
|
||||
from modules.models import channel_body_limit
|
||||
from modules.scheduled_message_admin import (
|
||||
SECTION as SCHEDULED_MESSAGES_SECTION,
|
||||
)
|
||||
@@ -4051,9 +4052,15 @@ class BotDataViewer:
|
||||
# ── Region warnings (regional flood scope) ───────────────────────────
|
||||
|
||||
def _region_warning_channel_limit() -> int:
|
||||
"""Channel body budget for a global-scope send, mirroring CommandManager."""
|
||||
name = (self.config.get('Bot', 'bot_name', fallback='Bot') or 'Bot').strip() or 'Bot'
|
||||
return max(130, 160 - len(name.encode('utf-8')) - 2)
|
||||
"""Channel body budget for a global-scope send.
|
||||
|
||||
The device's own name is authoritative for the command layer, but
|
||||
the viewer is a separate process with no radio, so it falls back to
|
||||
the configured one. They match on any install where the bot manages
|
||||
the device name.
|
||||
"""
|
||||
name = (self.config.get('Bot', 'bot_name', fallback='Bot') or 'Bot').strip()
|
||||
return channel_body_limit(name or 'Bot')
|
||||
|
||||
@self.app.route('/api/region-warnings')
|
||||
def api_region_warnings():
|
||||
|
||||
@@ -11,7 +11,11 @@ import pytest
|
||||
from modules import region_warning
|
||||
from modules.db_migrations import MigrationRunner
|
||||
from modules.enums import PayloadType, RouteType
|
||||
from modules.message_handler import RF_MATCH_KEY, MessageHandler
|
||||
from modules.message_handler import (
|
||||
CHANNEL_SENDER_FALLBACK,
|
||||
RF_MATCH_KEY,
|
||||
MessageHandler,
|
||||
)
|
||||
from modules.region_warning import (
|
||||
ACTION_DRY_RUN,
|
||||
ACTION_FAILED,
|
||||
@@ -163,6 +167,71 @@ class TestClassifyFloodScope:
|
||||
) == VERDICT_SCOPED
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestObservationHook:
|
||||
"""The MessageHandler side of the hook, independent of the monitor."""
|
||||
|
||||
def _hooked_handler(self):
|
||||
handler = _handler()
|
||||
handler.bot.region_warning_monitor = MagicMock()
|
||||
handler.bot.region_warning_monitor.observe = AsyncMock()
|
||||
handler.bot.connection_time = None
|
||||
return handler
|
||||
|
||||
async def _observe(self, handler, **overrides):
|
||||
args = {
|
||||
"sender_id": "Ann",
|
||||
"sender_pubkey": "ab",
|
||||
"channel": "#gen",
|
||||
"sender_timestamp": 0,
|
||||
"reply_scope": None,
|
||||
"recent_rf_data": None,
|
||||
"packet_info": None,
|
||||
"scope_rf_data": None,
|
||||
"scope_packet_info": None,
|
||||
}
|
||||
args.update(overrides)
|
||||
await handler._observe_flood_scope(**args)
|
||||
|
||||
async def test_verdict_is_forwarded_to_the_monitor(self):
|
||||
handler = self._hooked_handler()
|
||||
await self._observe(
|
||||
handler, recent_rf_data=_rf(route_type_int=int(RouteType.FLOOD.value)))
|
||||
kwargs = handler.bot.region_warning_monitor.observe.await_args.kwargs
|
||||
assert kwargs["verdict"] == VERDICT_GLOBAL
|
||||
assert kwargs["sender_id"] == "Ann"
|
||||
|
||||
async def test_no_monitor_is_a_noop(self):
|
||||
handler = self._hooked_handler()
|
||||
handler.bot.region_warning_monitor = None
|
||||
await self._observe(handler)
|
||||
|
||||
async def test_old_cached_message_is_skipped(self):
|
||||
"""A reconnect replays cached traffic; counting it would distort everything."""
|
||||
handler = self._hooked_handler()
|
||||
handler.bot.connection_time = 2_000
|
||||
await self._observe(handler, sender_timestamp=1_000)
|
||||
handler.bot.region_warning_monitor.observe.assert_not_awaited()
|
||||
|
||||
async def test_monitor_failure_does_not_escape_to_the_message_path(self):
|
||||
handler = self._hooked_handler()
|
||||
handler.bot.region_warning_monitor.observe = AsyncMock(
|
||||
side_effect=RuntimeError("boom"))
|
||||
await self._observe(handler)
|
||||
handler.logger.exception.assert_called()
|
||||
|
||||
async def test_unattributable_sender_is_passed_through_as_none(self):
|
||||
handler = self._hooked_handler()
|
||||
await self._observe(handler, sender_id=None)
|
||||
kwargs = handler.bot.region_warning_monitor.observe.await_args.kwargs
|
||||
assert kwargs["sender_id"] is None
|
||||
|
||||
|
||||
def test_channel_sender_fallback_is_the_handler_literal():
|
||||
"""The hook drops this name; if the handler renames it, the guard silently dies."""
|
||||
assert CHANNEL_SENDER_FALLBACK == "Channel User"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Settings
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -269,6 +338,17 @@ class TestObservation:
|
||||
verdict=VERDICT_UNKNOWN, sender_id="Ann", sender_pubkey="ab", channel="#gen")
|
||||
monitor.bot.command_manager.send_dm.assert_not_called()
|
||||
|
||||
async def test_unattributable_sender_is_counted_but_never_warned(self):
|
||||
"""A channel message with no "Name: " prefix has no real sender."""
|
||||
monitor = _monitor(enabled="true", dry_run="false", min_unscoped_messages=1)
|
||||
for _ in range(5):
|
||||
await monitor.observe(
|
||||
verdict=VERDICT_GLOBAL, sender_id=None, sender_pubkey="", channel="#gen")
|
||||
monitor.bot.command_manager.send_dm.assert_not_called()
|
||||
rows = monitor.bot.db_manager.execute_query(
|
||||
"SELECT global_count FROM region_scope_daily")
|
||||
assert rows[0]["global_count"] == 5
|
||||
|
||||
async def test_bookkeeping_failure_does_not_raise(self):
|
||||
monitor = _monitor()
|
||||
monitor.bot.db_manager = None
|
||||
|
||||
Reference in New Issue
Block a user