Trunk and semgrep fixes

.claude/commands/diagnose.md

@@ -26,7 +26,7 @@ Call the meshtastic MCP tool bundle and format a structured health report for on
 
 4. **Render per-device report** as:
 
-   ```
+   ```text
    [nrf52 @ /dev/cu.usbmodem1101]      fw=2.7.23.bce2825, hw=RAK4631
      owner       : Meshtastic 40eb / 40eb
      region/band : US, channel 88, LONG_FAST

.claude/commands/repro.md

@@ -23,7 +23,7 @@ Re-run a single pytest node ID N times in isolation, track pass rate, and surfac
 
 4. **Track a small structured tally**:
 
-   ```
+   ```text
    attempt 1: PASS (42s)
    attempt 2: FAIL (128s)  ← firmware log 200-line tail captured
    attempt 3: PASS (39s)

.github/prompts/mcp-diagnose.prompt.md

@@ -28,7 +28,7 @@ This prompt assumes the meshtastic MCP server is registered with your VS Code Co
 
 4. **Render per-device report** as a compact block:
 
-   ```
+   ```text
    [nrf52 @ /dev/cu.usbmodem1101]      fw=2.7.23.bce2825, hw=RAK4631
      owner       : Meshtastic 40eb / 40eb
      region/band : US, channel 88, LONG_FAST

.github/prompts/mcp-repro.prompt.md

@@ -25,7 +25,7 @@ Equivalent of `.claude/commands/repro.md`. Use when the operator says "that one
 
 4. **Tally** results as you go:
 
-   ```
+   ```text
    attempt 1: PASS (42s)
    attempt 2: FAIL (128s)    ← fw log captured
    attempt 3: PASS (39s)

.trunk/configs/.bandit

@@ -1,2 +1,28 @@
 [bandit]
-skips = B101
+# Rule IDs: https://bandit.readthedocs.io/en/latest/plugins/index.html
+#
+# B101 assert_used
+#   pytest assertions + internal invariants; required for pytest.
+# B110 try_except_pass
+#   best-effort cleanup paths (atexit handlers, pubsub unsubscribe,
+#   session-end file close, socket shutdown). Logging inside the
+#   except block would be worse than the silent pass — teardown is
+#   already at end-of-session and the surrounding caller has context.
+# B112 try_except_continue
+#   defensive loops over flaky sources (pubsub handlers, device
+#   re-enumeration polls). One failed iteration shouldn't abort the loop.
+# B404 import_subprocess
+#   mcp-server wraps PlatformIO, esptool, nrfutil, picotool, and the
+#   pytest test-runner — subprocess is a load-bearing import here, not
+#   a smell. The "consider possible security implications" advisory is
+#   redundant given the file-level review already applied.
+# B603 subprocess_without_shell_equals_true
+#   all subprocess calls use a static argv list; `shell=False` is the
+#   default and we never string-interpolate user input into the command.
+# B606 start_process_with_no_shell
+#   same invariant as B603 — running a binary via argv list (not
+#   `shell=True`) is the safe pattern bandit is asking for.
+#
+# Higher-severity checks (B102 exec_used, B301 pickle, B307 eval,
+# B602 shell=True, etc.) remain enabled.
+skips = B101,B110,B112,B404,B603,B606

mcp-server/README.md

@@ -148,7 +148,7 @@ _The tool tables below document 38 currently registered MCP server tools._
 
 ## Layout
 
-```
+```text
 mcp-server/
 ├── pyproject.toml
 ├── README.md

mcp-server/run-tests.sh

@@ -25,7 +25,7 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
 cd "$SCRIPT_DIR"
 
 VENV_PY="$SCRIPT_DIR/.venv/bin/python"
-if [ ! -x "$VENV_PY" ]; then
+if [[ ! -x $VENV_PY ]]; then
 	echo "error: $VENV_PY not found or not executable." >&2
 	echo "       Bootstrap the venv first:" >&2
 	echo "         cd $SCRIPT_DIR && python3 -m venv .venv && .venv/bin/pip install -e '.[test]'" >&2
@@ -42,7 +42,7 @@ USERPREFS_SIDECAR="$USERPREFS_PATH.mcp-session-bak"
 # If conftest.py's atexit hook didn't fire (SIGKILL, kernel panic, OS
 # restart), the sidecar is the ground truth. Self-heal before running so we
 # don't bake the previous run's dirty state into this run's firmware.
-if [ -f "$USERPREFS_SIDECAR" ]; then
+if [[ -f $USERPREFS_SIDECAR ]]; then
 	echo "[pre-flight] found $USERPREFS_SIDECAR from a prior abnormal exit;" >&2
 	echo "             restoring userPrefs.jsonc before starting." >&2
 	cp "$USERPREFS_SIDECAR" "$USERPREFS_PATH"
@@ -54,7 +54,12 @@ fi
 # it at the end, which may not be what the operator wants.
 if command -v git >/dev/null 2>&1; then
 	cd "$FIRMWARE_ROOT"
-	if [ -n "$(git status --porcelain userPrefs.jsonc 2>/dev/null)" ]; then
+	# Capture the git status into a local first — SC2312 flags command
+	# substitution inside `[[ -n ... ]]` because the exit code of `git
+	# status` is masked. A two-step assignment makes the failure path
+	# explicit (non-git, missing file) and keeps the bracket test clean.
+	_git_status_porcelain="$(git status --porcelain userPrefs.jsonc 2>/dev/null || true)"
+	if [[ -n $_git_status_porcelain ]]; then
 		echo "[pre-flight] warning: userPrefs.jsonc has uncommitted changes." >&2
 		echo "             Tests will snapshot THIS state and restore to it" >&2
 		echo "             at teardown. If that's not intended, run:" >&2
@@ -68,7 +73,7 @@ fi
 # Per-machine default so repeated runs from the same operator land on the
 # same PSK (makes --assume-baked valid across invocations). Operator can
 # override with an explicit env var if they want isolation (e.g. CI).
-if [ -z "${MESHTASTIC_MCP_SEED-}" ]; then
+if [[ -z ${MESHTASTIC_MCP_SEED-} ]]; then
 	WHO="$(whoami 2>/dev/null || echo anon)"
 	HOST="$(hostname -s 2>/dev/null || echo host)"
 	export MESHTASTIC_MCP_SEED="mcp-${WHO}-${HOST}"
@@ -133,7 +138,7 @@ resolve_env() {
 	upper="$(echo "$role" | tr '[:lower:]' '[:upper:]')"
 	local var="MESHTASTIC_MCP_ENV_${upper}"
 	eval "local override=\${$var:-}"
-	if [ -n "$override" ]; then
+	if [[ -n $override ]]; then
 		echo "$override"
 	else
 		echo "$default"
@@ -144,12 +149,12 @@ NRF52_PORT="$(echo "$ROLES_JSON" | "$VENV_PY" -c 'import json,sys; print(json.lo
 ESP32S3_PORT="$(echo "$ROLES_JSON" | "$VENV_PY" -c 'import json,sys; print(json.loads(sys.stdin.read()).get("esp32s3", ""))')"
 
 DETECTED=""
-if [ -n "$NRF52_PORT" ]; then
+if [[ -n $NRF52_PORT ]]; then
 	NRF52_ENV="$(resolve_env nrf52 rak4631)"
 	export MESHTASTIC_MCP_ENV_NRF52="$NRF52_ENV"
 	DETECTED="${DETECTED}  nrf52   @ ${NRF52_PORT} -> env=${NRF52_ENV}\n"
 fi
-if [ -n "$ESP32S3_PORT" ]; then
+if [[ -n $ESP32S3_PORT ]]; then
 	ESP32S3_ENV="$(resolve_env esp32s3 heltec-v3)"
 	export MESHTASTIC_MCP_ENV_ESP32S3="$ESP32S3_ENV"
 	DETECTED="${DETECTED}  esp32s3 @ ${ESP32S3_PORT} -> env=${ESP32S3_ENV}\n"
@@ -165,6 +170,7 @@ for _arg in "$@"; do
 	case "$_arg" in
 	--assume-baked) _bake_mode="skip (--assume-baked)" ;;
 	--force-bake) _bake_mode="force (--force-bake)" ;;
+	*) ;; # any other arg: pass-through; bake mode unchanged
 	esac
 done
 
@@ -172,7 +178,7 @@ echo "mcp-server test runner"
 echo "  firmware root : $FIRMWARE_ROOT"
 echo "  seed          : $MESHTASTIC_MCP_SEED"
 echo "  bake          : $_bake_mode"
-if [ -n "$DETECTED" ]; then
+if [[ -n $DETECTED ]]; then
 	echo "  detected hub  :"
 	printf "%b" "$DETECTED"
 else
@@ -185,7 +191,7 @@ echo
 # PASS/FAIL — every hardware test would SKIP with "role not present". We
 # narrow to tests/unit explicitly so the summary reads as "no hardware,
 # unit suite only" instead of "big skip count looks suspicious".
-if [ -z "$DETECTED" ] && [ "$#" -eq 0 ]; then
+if [[ -z $DETECTED && $# -eq 0 ]]; then
 	echo "[pre-flight] no supported devices detected; running unit tier only."
 	echo
 	exec "$VENV_PY" -m pytest tests/unit -v --report-log=tests/reportlog.jsonl
@@ -204,7 +210,7 @@ fi
 # skipping half the hardware tests with "not baked with session profile"
 # errors. Power users who know their hardware is current and want to shave
 # those seconds can pass `--assume-baked` explicitly.
-if [ "$#" -eq 0 ]; then
+if [[ $# -eq 0 ]]; then
 	set -- tests/ \
 		--html=tests/report.html --self-contained-html \
 		--junitxml=tests/junit.xml \
@@ -220,9 +226,10 @@ _has_report_log=0
 for _arg in "$@"; do
 	case "$_arg" in
 	--report-log | --report-log=*) _has_report_log=1 ;;
+	*) ;; # any other arg: no-op; loop continues
 	esac
 done
-if [ "$_has_report_log" -eq 0 ]; then
+if [[ $_has_report_log -eq 0 ]]; then
 	set -- "$@" --report-log=tests/reportlog.jsonl
 fi
 

mcp-server/src/meshtastic_mcp/admin.py

@@ -208,6 +208,7 @@ def set_config(path: str, value: Any, port: str | None = None) -> dict[str, Any]
         set_config("device.role", "ROUTER")
         set_config("mqtt.enabled", True)
         set_config("mqtt.address", "mqtt.example.com")
+
     """
     segments = [s for s in path.split(".") if s]
     if not segments:

mcp-server/src/meshtastic_mcp/cli/_fwlog.py

@@ -46,6 +46,7 @@ class FirmwareLogTailer(threading.Thread):
     wait_s:
         How long to poll for the file's creation before giving up. Default
         30 s; pytest collection on a cold cache can be slow.
+
     """
 
     def __init__(

mcp-server/src/meshtastic_mcp/config.py

@@ -18,7 +18,7 @@ class ConfigError(RuntimeError):
 
 
 def firmware_root() -> Path:
-    """The root of the Meshtastic firmware repo.
+    """Resolve the root of the Meshtastic firmware repo.
 
     Resolution order:
       1. `MESHTASTIC_FIRMWARE_ROOT` env var.

mcp-server/src/meshtastic_mcp/serial_session.py

@@ -19,7 +19,7 @@ import uuid
 from dataclasses import dataclass, field
 from typing import Any
 
-from . import boards, config, pio
+from . import boards, config
 
 _BUFFER_MAX_LINES = 10_000
 _POLL_NEW_PORT_TIMEOUT_S = 3.0

mcp-server/src/meshtastic_mcp/server.py

@@ -205,7 +205,7 @@ def userprefs_get() -> dict[str, Any]:
 
 @app.tool()
 def userprefs_set(prefs: dict[str, Any]) -> dict[str, Any]:
-    """Persistently merge `prefs` into userPrefs.jsonc (uncommenting keys).
+    """Merge `prefs` into userPrefs.jsonc persistently (uncommenting keys).
 
     Existing active values not in `prefs` are kept. To remove a key from the
     active set, call `userprefs_reset` (restores the MCP backup if present)
@@ -275,6 +275,7 @@ def userprefs_testing_profile(
         long_name: optional owner long name stamped into the build.
         disable_mqtt: disable MQTT module + uplink/downlink (default True).
         disable_position: disable GPS + smart-position broadcasts (default False).
+
     """
     return userprefs_mod.build_testing_profile(
         psk_seed=psk_seed,

mcp-server/src/meshtastic_mcp/userprefs.py

@@ -69,6 +69,7 @@ def _parse_jsonc_state(text: str) -> dict[str, Any]:
           "commented": {key: string_value, ...}, # commented examples
           "order": [key, ...]                   # source order for round-trip
         }
+
     """
     active: dict[str, str] = {}
     commented: dict[str, str] = {}
@@ -415,12 +416,14 @@ def build_testing_profile(
             isolation).
         region: short code from `KNOWN_REGIONS`.
         modem_preset: short code from `KNOWN_MODEM_PRESETS`.
-        short_name / long_name: optional owner identity stamps.
+        short_name: optional owner short-name stamp (≤4 chars). None = unset.
+        long_name: optional owner long-name stamp. None = unset.
         disable_mqtt: if True (default), disables the MQTT module and the
             uplink/downlink bridge on the primary channel — so private test
             traffic never leaks to a public broker.
         disable_position: if True, disables GPS + position broadcasts — useful
             when test devices sit on a bench without antennas.
+
     """
     if region not in KNOWN_REGIONS:
         raise ValueError(

mcp-server/tests/admin/test_owner_survives_reboot.py

@@ -21,7 +21,6 @@ def test_owner_survives_reboot(
 ) -> None:
     """Runs once per connected role — proves the reboot-persistence
     round-trip works on each device independently, not just one."""
-    role = baked_single["role"]
     port = baked_single["port"]
 
     pre = info.device_info(port=port, timeout_s=8.0)

mcp-server/tests/conftest.py

@@ -35,7 +35,10 @@ import pytest
 
 # Ensure the MCP server is on `sys.path` without requiring installation in
 # development mode for every checkout (we DO install in .venv but this makes
-# `pytest tests/` work from a fresh clone too).
+# `pytest tests/` work from a fresh clone too). The path mutation must
+# happen before `meshtastic_mcp.*` imports below — hence the `noqa: E402`
+# markers on those imports (ruff's "module-level import not at top of file"
+# rule doesn't understand path-bootstrapping patterns).
 _HERE = pathlib.Path(__file__).resolve().parent
 _MCP_SRC = _HERE.parent / "src"
 if str(_MCP_SRC) not in sys.path:
@@ -44,18 +47,9 @@ if str(_MCP_SRC) not in sys.path:
 # Default firmware root: the repo this mcp-server/ lives inside.
 os.environ.setdefault("MESHTASTIC_FIRMWARE_ROOT", str(_HERE.parent.parent))
 
-from meshtastic_mcp import (
-    admin,
-)
-from meshtastic_mcp import (  # noqa: E402  (import after path setup)
-    devices as devices_module,
-)
-from meshtastic_mcp import (
-    flash,
-    info,
-    serial_session,
-    userprefs,
-)
+from meshtastic_mcp import admin  # noqa: E402
+from meshtastic_mcp import devices as devices_module  # noqa: E402
+from meshtastic_mcp import info, serial_session, userprefs  # noqa: E402
 
 from . import tool_coverage  # noqa: E402
 
@@ -407,7 +401,7 @@ def baked_mesh(
     session_seed: str,
     request: pytest.FixtureRequest,
 ) -> dict[str, Any]:
-    """Verifies that both roles are baked with the session `test_profile`.
+    """Verify that both roles are baked with the session `test_profile`.
 
     Does NOT reflash. `test_00_bake.py` is responsible for applying the bake;
     this fixture just checks the result by connecting to each device and
@@ -696,7 +690,7 @@ class _SerialCapture:
 
 @pytest.fixture
 def serial_capture(hub_devices: dict[str, str], request: pytest.FixtureRequest) -> Any:
-    """Returns a `_SerialCapture` factory.
+    """Return a `_SerialCapture` factory.
 
     Usage:
         cap = serial_capture("esp32s3")
@@ -929,7 +923,7 @@ def pytest_runtest_makereport(item: pytest.Item, call: pytest.CallInfo[Any]) ->
     # Attach serial captures (if the test used `serial_capture`)
     caps = getattr(item, "_serial_captures", None)
     if caps:
-        for i, cap in enumerate(caps):
+        for cap in caps:
             try:
                 lines = _run_with_timeout(lambda c=cap: c.snapshot(max_lines=2000), 5.0)
             except Exception as exc:

mcp-server/tests/mesh/_receive.py

@@ -101,13 +101,16 @@ class ReceiveCollector:
                 pass
 
     def snapshot(self) -> list[dict[str, Any]]:
-        """Current list of collected packets (thread-safe copy)."""
+        """Return a thread-safe copy of the list of collected packets."""
         with self._lock:
             return list(self._packets)
 
     def log_snapshot(self) -> list[str]:
-        """Captured firmware log lines (only populated if `capture_logs=True`
-        AND the device has `security.debug_log_api_enabled=True`)."""
+        """Return captured firmware log lines.
+
+        Only populated if `capture_logs=True` AND the device has
+        `security.debug_log_api_enabled=True`.
+        """
         with self._lock:
             return list(self._log_lines)
 

mcp-server/tests/mesh/test_direct_with_ack.py

@@ -86,7 +86,7 @@ def test_direct_with_ack_roundtrip(
             # sufficient on its own with fresh keys; the retry is purely
             # an airtime-collision safety net.
             got = None
-            for attempt in range(2):
+            for _attempt in range(2):
                 packet = tx_iface.sendText(
                     unique,
                     destinationId=rx_node_num,

mcp-server/tests/mesh/test_mesh_formation.py

@@ -11,7 +11,6 @@ from __future__ import annotations
 from typing import Any
 
 import pytest
-from meshtastic_mcp import info
 from meshtastic_mcp.connection import connect
 
 

mcp-server/tests/monitor/test_boot_log_no_panic.py

@@ -7,8 +7,6 @@ misconfig), this test fails within a minute of reboot.
 
 from __future__ import annotations
 
-import os
-import re
 import time
 from typing import Any
 

mcp-server/tests/tool_coverage.py

@@ -14,7 +14,7 @@ from __future__ import annotations
 
 import json
 import pathlib
-from typing import Any, Callable
+from typing import Any
 
 _counts: dict[str, int] = {}
 _installed = False
@@ -108,14 +108,17 @@ def install() -> None:
     for tool_name, (module_path, attr) in _TOOL_MAP.items():
         # Defense in depth: if someone mutates `_TOOL_MAP` at runtime
         # (shouldn't happen; it's module-level) the whitelist catches it.
+        # `module_path` is a key from the hardcoded `_TOOL_MAP` dict and
+        # is gated above by membership in `_allowed_modules` (itself
+        # derived from the same literal values). There is no path for
+        # untrusted input to reach the `import_module` call below; the
+        # Semgrep suppression must sit on the line immediately preceding
+        # the call (multi-line comment blocks between comment and call
+        # break the rule's scope detection).
         if module_path not in _allowed_modules:
             continue
         try:
             # nosemgrep: python.lang.security.audit.non-literal-import.non-literal-import
-            # `module_path` is a key from the hardcoded `_TOOL_MAP` dict
-            # and is gated above by membership in `_allowed_modules`
-            # (itself derived from the same literal values). There is no
-            # path for untrusted input to reach this call.
             mod = importlib.import_module(module_path)
         except ImportError:
             continue