fix(plugins): PLUGINS is read from adn-server.yaml and follows SIGHUP

Review of #104: master_kill and revoking PLUGINS.send did not take
effect on SIGHUP, because merge_top_level_config never re-read PLUGINS.
Going through the real path showed it goes further: YamlConfigLoader
builds the config from a fixed list of sections and dropped PLUGINS
altogether, so the documented block (directory, master_kill, overrides)
was never read, at startup either. Both predate this PR.

- YamlConfigLoader keeps PLUGINS when present (like OBP_PROXY).
- merge_top_level_config replaces PLUGINS on every reload, absent
  included: removing the block removes everything in it.
- Tests through prepare_reload_config + merge_top_level_config +
  swap_runtime_config and a ConfigProxy, as the server wires them:
  entry removed, master_kill, whole section removed, a TG granted; and
  one with the real loader on a real adn-server.yaml, boot and reload.

Also from the review:
- parse_dmrd_header uses call_attributes(); PluginIngress uses
  server_id_bytes().
- A max_frames_per_s that is not a finite positive number grants nothing.
- The allowlist is documented as a guard against buggy plugins, not a
  sandbox: a plugin runs in-process with the live config.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/en/server/user-guide/plugins.md

@@ -47,7 +47,7 @@ PLUGINS:
 | `overrides` | Per-plugin config patches without editing `plugins/<name>/config.yaml` |
 | `send` | Per-plugin permission to send unit data and group voice — see [Sending](#sending-unit-data-and-group-voice-opt-in) |
 
-On **SIGHUP**, `PluginManager.rescan()` loads new plugins, unloads removed ones, and calls `on_reload()` when `config.yaml` changed.
+On **SIGHUP**, the `PLUGINS` block is re-read from `adn-server.yaml` (removing it counts as removing everything in it) and `PluginManager.rescan()` loads new plugins, unloads removed ones, and calls `on_reload()` when `config.yaml` changed.
 
 ### `config.yaml` reserved keys
 
@@ -116,6 +116,7 @@ PLUGINS:
 ```
 
 - `send_dmrd` is `None` unless the plugin has an entry with at least one source ID.
+- The allowlist and the rate limit guard against a **buggy** plugin (sending as a radio, flooding the mesh). They are no sandbox: a plugin runs in-process with the live config and could rewrite its own entry, so only install plugins you trust.
 - Each frame is checked against the **current** config: removing the entry (SIGHUP) or `master_kill` stops sending at once. Granting it to a plugin already loaded needs that plugin reloaded.
 - Rejected frames (not unit data, source not allowed, over the rate) return `False` and are logged and counted; the first frame of each stream is logged at INFO.
 - Called on the reactor thread (`on_event`, `call_later`), the frame is routed at once and the result is whether the server **accepted** it; a plugin sending voice must stop when it gets `False`. From another thread the frame is queued to the reactor and `True` only means the guards passed.

docs/es/server/user-guide/plugins.md

@@ -47,7 +47,7 @@ PLUGINS:
 | `overrides` | Parches por plugin sin editar `plugins/<name>/config.yaml` |
 | `send` | Permiso por plugin para enviar datos y voz de grupo — ver [Envío](#envío-de-datos-y-voz-de-grupo-opcional) |
 
-Con **SIGHUP**, `PluginManager.rescan()` carga plugins nuevos, descarga los eliminados y llama `on_reload()` si cambió `config.yaml`.
+Con **SIGHUP**, el bloque `PLUGINS` se vuelve a leer de `adn-server.yaml` (quitarlo equivale a quitar todo lo que contenía) y `PluginManager.rescan()` carga plugins nuevos, descarga los eliminados y llama `on_reload()` si cambió `config.yaml`.
 
 ### Claves reservadas en `config.yaml`
 
@@ -116,6 +116,7 @@ PLUGINS:
 ```
 
 - `send_dmrd` es `None` salvo que el plugin tenga una entrada con al menos un ID de origen.
+- La lista de IDs permitidos y el límite de ritmo protegen frente a un plugin **con fallos** (que emita como una radio o inunde la red). No son un aislamiento: un plugin corre en el mismo proceso con la configuración viva y podría reescribir su propia entrada, así que instala solo plugins de confianza.
 - Cada trama se comprueba contra la configuración **actual**: quitar la entrada (SIGHUP) o `master_kill` corta el envío al instante. Autorizar a un plugin ya cargado exige recargar ese plugin.
 - Las tramas rechazadas (no son datos, origen no permitido, exceso de ritmo) devuelven `False` y se registran y cuentan; la primera trama de cada stream se registra en INFO.
 - Llamado desde el hilo del reactor (`on_event`, `call_later`), la trama se enruta en el acto y el resultado indica si el servidor la **aceptó**; un plugin que emite voz debe parar cuando recibe `False`. Desde otro hilo la trama se encola al reactor y `True` solo significa que pasó las salvaguardas.

src/adn_server/application/plugins/application/ingress.py

@@ -26,7 +26,8 @@ import logging
 import time
 from typing import Any, Callable
 
-from ....domain import HBPF_DATA_SYNC, HBPF_SLT_VHEAD, HBPF_SLT_VTERM, bytes_4, int_id
+from ....domain import HBPF_DATA_SYNC, HBPF_SLT_VHEAD, HBPF_SLT_VTERM
+from ....domain.mesh_engine import server_id_bytes
 from ...routing.announcement_ptt_inject import announcement_ptt_system, inject_plugin_dmrd
 from ...routing.helpers import slot_voice_held_by_other_stream
 from ..domain.send import GROUP_VOICE, UNIT_DATA, parse_dmrd_header, plugin_frame_kind
@@ -106,7 +107,4 @@ class PluginIngress:
             slot["TX_TYPE"] = HBPF_SLT_VTERM
 
     def _server_id(self) -> bytes:
-        server_id = self._config.get("GLOBAL", {}).get("SERVER_ID", b"\x00\x00\x00\x00")
-        if isinstance(server_id, bytes) and len(server_id) >= 4:
-            return server_id[:4]
-        return bytes_4(int(int_id(server_id) if isinstance(server_id, bytes) else server_id or 0) & 0xFFFFFFFF)
+        return server_id_bytes(self._config.get("GLOBAL", {}).get("SERVER_ID"))[:4]

src/adn_server/application/plugins/domain/send.py

@@ -22,10 +22,12 @@
 
 from __future__ import annotations
 
+import math
 from dataclasses import dataclass
 from typing import Any, NamedTuple
 
 from ....domain import HBPF_DATA_SYNC, HBPF_SLT_VHEAD, HBPF_SLT_VTERM, HBPF_VOICE, HBPF_VOICE_SYNC
+from ....domain.mesh_admission import call_attributes
 
 # CSBK, data header, rate 1/2 and rate 3/4 data blocks: ARS, LRRP, SMS and the like.
 PLUGIN_SENDABLE_DTYPES = frozenset({3, 6, 7, 8})
@@ -50,22 +52,16 @@ def parse_dmrd_header(pkt: bytes) -> DmrdHeader | None:
     """The HBP DMRD header fields of any call type (group, vcsbk or unit)."""
     if len(pkt) < 53 or pkt[:4] != b"DMRD":
         return None
-    bits = pkt[15]
-    if bits & 0x40:
-        call_type = "unit"
-    elif (bits & 0x23) == 0x23:
-        call_type = "vcsbk"
-    else:
-        call_type = "group"
+    attrs = call_attributes(pkt[15])
     return DmrdHeader(
         seq=pkt[4],
         rf_src=pkt[5:8],
         dst_id=pkt[8:11],
         peer_id=pkt[11:15],
-        slot=2 if bits & 0x80 else 1,
-        call_type=call_type,
-        frame_type=(bits & 0x30) >> 4,
-        dtype_vseq=bits & 0xF,
+        slot=attrs.slot,
+        call_type=attrs.call_type,
+        frame_type=attrs.frame_type,
+        dtype_vseq=attrs.dtype_vseq,
         stream_id=pkt[16:20],
     )
 
@@ -97,8 +93,10 @@ class SendPermission:
 def send_permission(server_config: dict[str, Any], plugin: str) -> SendPermission | None:
     """The plugin's entry in ``PLUGINS.send``, or None when it may not send.
 
-    An entry without source IDs grants nothing: the allowlist is what stops a
-    plugin from sending as a radio.
+    An entry without source IDs grants nothing. The allowlist and the rate limit
+    guard against a *buggy* plugin (sending as a radio, flooding the mesh); they are
+    no sandbox: a plugin runs in-process with the live config and could rewrite its
+    own entry, so only install plugins you trust.
     """
     plugins_cfg = server_config.get("PLUGINS") or {}
     if plugins_cfg.get("master_kill"):
@@ -112,6 +110,6 @@ def send_permission(server_config: dict[str, Any], plugin: str) -> SendPermissio
         tgs = frozenset(int(t) for t in entry.get("group_voice_tgs") or ())
     except (TypeError, ValueError):
         return None
-    if not ids or rate <= 0:
+    if not ids or not math.isfinite(rate) or rate <= 0:
         return None
     return SendPermission(ids, rate, tgs)

src/adn_server/infrastructure/config_loader.py

@@ -114,6 +114,9 @@ class YamlConfigLoader:
         # "absent" (defaults apply) from "present but disabled".
         if isinstance(data.get("OBP_PROXY"), dict):
             config["OBP_PROXY"] = data["OBP_PROXY"]
+        # PLUGINS (directory, master_kill, overrides, send) is read by the plugin manager.
+        if isinstance(data.get("PLUGINS"), dict):
+            config["PLUGINS"] = data["PLUGINS"]
         apply_proxy_env_overrides(config)
         # Ensure REPORT_CLIENTS is list
         if "REPORT_CLIENTS" in config["REPORTS"] and isinstance(config["REPORTS"]["REPORT_CLIENTS"], str):

src/adn_server/infrastructure/config_reload.py

@@ -119,12 +119,15 @@ def merge_system_config(old_cfg: dict[str, Any], new_cfg: dict[str, Any]) -> dic
 
 
 def merge_top_level_config(config: dict[str, Any], incoming: dict[str, Any]) -> None:
-    """Update GLOBAL / REPORTS / ALIASES / LOGGER in the live config dict."""
+    """Update GLOBAL / REPORTS / ALIASES / LOGGER / … / PLUGINS in the live config dict."""
     kill_flag = config.get("GLOBAL", {}).get("_KILL_SERVER")
     for key in ("GLOBAL", "REPORTS", "ALIASES", "LOGGER", "PROXY", "DATABASE", "SELF_SERVICE"):
         if key not in incoming:
             continue
         config[key] = copy.deepcopy(incoming[key])
+    # PLUGINS always follows the file, absent included: it carries master_kill and the
+    # per-plugin send permissions, whose removal must take effect on this reload.
+    config["PLUGINS"] = copy.deepcopy(incoming.get("PLUGINS") or {})
     if kill_flag is not None:
         config.setdefault("GLOBAL", {})["_KILL_SERVER"] = kill_flag
     for rk in _RUNTIME_TOP_KEYS:

tests/application/test_plugin_sender.py

@@ -24,6 +24,8 @@ from __future__ import annotations
 
 from pathlib import Path
 
+import pytest
+
 from tests.harness.deterministic import PacketSpec
 
 from adn_server.application.plugins.application.bus import PluginBus
@@ -168,3 +170,83 @@ def test_only_the_granted_plugin_gets_send_dmrd(tmp_path) -> None:
     assert type(loaded["d-aprs"]).ctx.send_dmrd is not None
     assert type(loaded["logger"]).ctx.send_dmrd is None
     assert made == ["d-aprs"]
+
+
+# --- through the real SIGHUP reload path (review of #104) ---
+
+from adn_server.application.runtime_context import (  # noqa: E402
+    ConfigProxy,
+    RuntimeContext,
+    RuntimeContextHolder,
+    prepare_reload_config,
+    swap_runtime_config,
+)
+from adn_server.infrastructure.config_reload import merge_top_level_config  # noqa: E402
+
+
+def _reload(holder: RuntimeContextHolder, incoming: dict) -> None:
+    """What a SIGHUP does with a freshly parsed adn-server.yaml (``incoming``)."""
+    new_config = prepare_reload_config(holder)
+    merge_top_level_config(new_config, incoming)
+    swap_runtime_config(holder, new_config)
+
+
+@pytest.mark.parametrize(
+    "incoming",
+    [
+        {"GLOBAL": {}, "PLUGINS": {"send": {}}},  # entry removed
+        {"GLOBAL": {}, "PLUGINS": {"master_kill": True, **_config()["PLUGINS"]}},  # master_kill
+        {"GLOBAL": {}},  # the whole PLUGINS section removed
+    ],
+    ids=["entry-removed", "master-kill", "section-removed"],
+)
+def test_a_sighup_reload_revokes_sending(incoming) -> None:
+    holder = RuntimeContextHolder(RuntimeContext(config={"GLOBAL": {}, **_config()}))
+    sender, delivered = _sender(ConfigProxy(holder))
+    assert sender(_frame()) is True
+    _reload(holder, incoming)
+    assert sender(_frame()) is False
+    assert len(delivered) == 1
+
+
+def test_a_sighup_reload_can_grant_a_new_talkgroup() -> None:
+    holder = RuntimeContextHolder(RuntimeContext(config={"GLOBAL": {}, **_config()}))
+    sender, _ = _sender(ConfigProxy(holder))
+    voice = PacketSpec(rf_src=GATEWAY_ID, dst_id=213, call_type="group", frame_type=HBPF_VOICE, dtype_vseq=1).data()
+    assert sender(voice) is False
+    _reload(holder, {"GLOBAL": {}, **_config(group_voice_tgs=[213])})
+    assert sender(voice) is True
+
+
+def test_plugins_section_is_read_from_adn_server_yaml_and_followed_on_reload(tmp_path) -> None:
+    """The whole chain with the real loader: PLUGINS used to be dropped by YamlConfigLoader,
+    so neither master_kill, overrides nor send permissions ever reached the server."""
+    import logging
+    from pathlib import Path
+
+    from adn_server.infrastructure.config_loader import YamlConfigLoader
+    from adn_server.infrastructure.config_reload import prepare_incoming_config
+
+    example = Path(__file__).resolve().parents[2] / "adn-server.example.yaml"
+    base = example.read_text(encoding="utf-8")
+    path = tmp_path / "adn-server.yaml"
+    grant = "\nPLUGINS:\n  send:\n    d-aprs:\n      allowed_src_ids: [900999]\n"
+    path.write_text(base + grant, encoding="utf-8")
+    log = logging.getLogger("test")
+
+    boot = prepare_incoming_config(YamlConfigLoader(), str(path), log)
+    assert boot["PLUGINS"]["send"]["d-aprs"]["allowed_src_ids"] == [900999]
+    holder = RuntimeContextHolder(RuntimeContext(config=boot))
+    sender, _ = _sender(ConfigProxy(holder))
+    assert sender(_frame()) is True
+
+    path.write_text(base + "\nPLUGINS:\n  master_kill: true\n" + grant[len("\nPLUGINS:\n"):], encoding="utf-8")
+    _reload(holder, prepare_incoming_config(YamlConfigLoader(), str(path), log))
+    assert sender(_frame()) is False
+
+
+@pytest.mark.parametrize("rate", [float("inf"), float("nan"), 0, -5])
+def test_a_rate_that_is_not_a_positive_number_grants_nothing(rate) -> None:
+    from adn_server.application.plugins.domain.send import send_permission
+
+    assert send_permission(_config(max_frames_per_s=rate), "d-aprs") is None