fix(gsm): secure and harden SMS delivery

CLAUDE.md

@@ -60,7 +60,7 @@ Run for each component actually touched — don't assume one component's green b
 | `server/app/models/` changed | `alembic upgrade head && alembic check` — **run from `server/`** (not `server/app/`) with `DATABASE_URL` set. `alembic check` must report no new operations. Note `pytest` builds its schema with `create_all()` and cannot detect migration drift. |
 | `mobile-app/sapot-mobile-app/` | `pnpm run testAll` (= test + typecheck + lint + expo-doctor), or the individual `pnpm test` / `pnpm run typecheck` / `pnpm run lint` |
 | `admin-frontend/sapot-admin/` | `pnpm run lint && pnpm run build` — **no test script exists in this component**; don't claim test coverage that isn't there |
-| `GSM-module/` | No automated tests exist — verify manually per `docs/getting-started/gsm-module-setup.md` |
+| `GSM-module/` | `pytest` (from `GSM-module/GSM-fastapi/`; serial I/O and database calls are mocked) |
 
 If the change is release-relevant (server), `server/app/version.py` must match the git tag per `VERSIONING.md` before tagging — not typically a per-commit concern, but relevant if asked to prepare a release.
 

GSM-module/CLAUDE.md

@@ -4,21 +4,21 @@ Instructions for Claude Code working in `GSM-module/` — SAPOT's SMS gateway, b
 
 ## Project Overview
 
-Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem over serial; a Python (FastAPI) service on the same machine talking to the Arduino over USB serial; the main `server/` proxying to that Python service over HTTP with a shared secret. There are **two parallel Python implementations** in this directory — they are not both live (see Architecture).
+Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem over serial; a Python (FastAPI) service on the same machine talking to the Arduino over USB serial; the main `server/` proxying outbound calls and receiving inbound callbacks over HTTP authenticated with a shared secret. There are **two parallel Python implementations** in this directory; they are not both live (see Architecture).
 
 ## Architecture — which implementation is live
 
-**`GSM-fastapi/` is the deployed, current implementation.** Evidence: `docs/deployment/gsm-module.md`, `docs/getting-started/gsm-module-setup.md`, and `docs/features/sms-gateway/README.md` all reference only `GSM-fastapi/`; `server/app/api/gsm.py` proxies to `http://localhost:8001`, and `GSM-fastapi/main.py` hardcodes `uvicorn.run("api:app", port=8001, ...)` — an exact match.
+**`GSM-fastapi/` is the deployed, current implementation.** Evidence: `docs/deployment/gsm-module.md`, `docs/getting-started/gsm-module-setup.md`, and `docs/features/sms-gateway/README.md` all reference only `GSM-fastapi/`; `server/app/api/gsm.py` proxies to `http://localhost:8001`, and `GSM-fastapi/main.py` hardcodes `uvicorn.run("api:app", port=8001, ...)`, an exact match.
 
-**`GSM-API/` is a separate, incomplete rewrite — not deployed, not referenced by any doc or by `server/`.** It has in-memory-only session state (no DB persistence), fire-and-forget SMS sends (no delivery confirmation), and an unsynchronized global (`app/gsm/gsm_runtime.py`'s module-level `ser`) shared across threads with no lock. Default to editing `GSM-fastapi/` for SMS-gateway work; only touch `GSM-API/` if a task explicitly asks for it.
+**`GSM-API/` is a separate, incomplete rewrite that is not deployed or referenced by `server/`.** It has in-memory-only session state (no DB persistence), fire-and-forget SMS sends (no delivery confirmation), and an unsynchronized global (`app/gsm/gsm_runtime.py`'s module-level `ser`) shared across threads with no lock. Default to editing `GSM-fastapi/` for SMS-gateway work; only touch `GSM-API/` if a task explicitly asks for it.
 
-**Known doc/code mismatch:** `docs/features/sms-gateway/design.md` describes a different wire protocol (`SEND:`/`RECV:`/`ACK:`/`ERR:` frames, endpoints `/gsm/send`/`/gsm/status`, tables `sms_outbox`/`sms_inbound`) that matches neither actual Python service nor the Arduino firmware. Trust the code (`GSM-fastapi/protocol.py` + the `.ino` firmware) over that doc — treat `design.md` as aspirational/stale.
+**Documentation source of truth:** `docs/features/sms-gateway/design.md` describes the deployed HTTP routes, queue, lifecycle, and serial flow. Use `GSM-fastapi/protocol.py` and the Arduino firmware as the authoritative definitions for individual serial frames.
 
 ### Data flow (GSM-fastapi, the live path)
 
 **Inbound:** Arduino emits `SMS_RECEIVED|<num>|<body>` over serial → `serial_worker.py`'s `SerialWorker._reader_loop` parses it via `protocol.py` → queued → `api.py`'s async `_inbox_drain()` task offloads to a thread pool → `sms_handler.handle_incoming_sms()` (session/target flow, ban/verified checks against MariaDB) → `database.py`'s `notify_app()` POSTs to the main server's `/gsm/inbound` with an `X-GSM-Secret` header.
 
-**Outbound:** caller (main server or admin frontend's `gsm` page) calls `POST /sms/send` on port 8001 → `SerialWorker.send_sms()` enqueues `SEND_SMS|<num>|<body>` and blocks on an `Event` (timeout 60s) → `_sender_loop` writes to serial → Arduino replies `SMS_SENT|`/`SMS_FAILED|` → the reader thread resolves the waiting request.
+**Outbound:** the main server calls `POST /sms/send` on port 8001 with `X-GSM-Secret`. The gateway validates the secret before `SerialWorker.send_sms()` atomically admits the request to a bounded FIFO queue or rejects saturation with HTTP 503. The sender writes `SEND_SMS|<num>|<body>`, and the reader resolves the request from `SMS_SENT|` or `SMS_FAILED|`. The admin GSM page reads health and message history through the main server. Shutdown rejects queued and active work with `SERVICE_STOPPING`.
 
 `SerialWorker` runs two dedicated threads (`_reader_loop`, `_sender_loop`) with proper request/response correlation over the async serial stream, and auto-reconnects every 10s on disconnect.
 
@@ -33,7 +33,7 @@ Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem ove
 
 - **Wire protocol** — pipe-delimited lines: `SEND_SMS|<num>|<body>` (PC → Arduino), `SMS_RECEIVED|<num>|<body>`, `SMS_SENT|<num>`, `SMS_FAILED|<num>|<reason>`, `GSM_READY`, `NETWORK_OK`/`NETWORK_LOST`, `SIM_MISSING` (Arduino → PC). Implemented identically in `GSM-fastapi/protocol.py`. Any change to this format must be mirrored in the `.ino` firmware's parser/emitter — they are independent implementations of the same contract, not shared code.
 - **Session/target flow** — inbound SMS starts a session (`NEW`), the sender texts `[target] +63...` to select a recipient (`AWAITING_TARGET` → `ACTIVE`), then messages relay through. `GSM-fastapi` persists this to MariaDB (`SmsSession` table) and checks `banned`/`phone_is_verified` on both sender and target; `GSM-API`'s equivalent is in-memory only and skips those checks.
-- **Shared-secret webhook auth** (`X-GSM-Secret` header) — how this service calls back into the main server (`/gsm/inbound`); distinct from the JWT auth used elsewhere in SAPOT (see `../server/CLAUDE.md`).
+- **Shared-secret service auth** (`X-GSM-Secret` header) — authenticates main-server calls to `/sms/send` and GSM callbacks to `/gsm/inbound`; distinct from the JWT auth used elsewhere in SAPOT (see `../server/CLAUDE.md`).
 
 ## Development Conventions
 
@@ -55,7 +55,6 @@ Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem ove
 - `GSM-fastapi/sapot.db` is a stale, unused artifact (confirmed in `../docs/database/migrations.md`) — real storage is MariaDB via `config.py`'s `DB_PATH`. Never read from or write to `sapot.db`.
 - `GSM-API/app/gsm/gsm_runtime.py`'s module-level `ser` is a global shared across threads with no lock — if `GSM-API` is ever revived, this is a live race condition, not a style nit.
 - `GSM-trial-code.ino` is not wire-compatible with either Python service — never point a deployment at it, even for "quick testing."
-- Treating `docs/features/sms-gateway/design.md` as accurate — its protocol/endpoint descriptions don't match the real code (see Architecture).
 - `server/app/api/gsm.py`'s own code comments refer to "GSM-API" as a generic name for **the GSM service it proxies to** (i.e. the live `GSM-fastapi/`, port 8001) — not the literal `GSM-module/GSM-API/` directory documented above as non-deployed. Don't let those comments override the Architecture section above.
 
 ## When Modifying This Project

GSM-module/GSM-fastapi/.env.example

@@ -8,6 +8,9 @@ HOST=127.0.0.1
 # to avoid a port collision.
 PORT=8000
 LOG_LEVEL=INFO
-SAPOT_API_URL=https://localhost:8000
+# Maximum outbound SMS requests waiting behind the one in-flight request.
+# Must be an integer from 1 through 20.
+SMS_SEND_QUEUE_MAXSIZE=10
+SAPOT_API_URL=http://localhost:8000
 GSM_SECRET=change-me-to-a-strong-secret
 SMS_BOT_USER_ID=

GSM-module/GSM-fastapi/api.py

@@ -27,18 +27,23 @@
 """
 
 import asyncio
+import hmac
 import logging
 import re
 from contextlib import asynccontextmanager
-from typing import Counter, Optional
+from typing import Annotated, Counter, Optional
 
-from fastapi import FastAPI, HTTPException, Query, BackgroundTasks
+from fastapi import BackgroundTasks, Depends, FastAPI, Header, HTTPException, Query
 from fastapi.responses import JSONResponse
 from pydantic import BaseModel, field_validator
 
 import database
 from app_version import __version__
-from serial_worker import SerialWorker
+from serial_worker import (
+    OutboundQueueFullError,
+    SerialWorker,
+    WorkerStoppingError,
+)
 from sms_handler import handle_incoming_sms
 from config import settings
 
@@ -57,9 +62,19 @@ async def lifespan(app: FastAPI):
     # Database
     database.init(settings.db_path)
     logger.info("Database ready")
+    orphaned_count = database.fail_orphaned_pending_messages()
+    if orphaned_count:
+        logger.warning(
+            "Marked %d orphaned SMS messages as failed after service restart",
+            orphaned_count,
+        )
 
     # Serial worker
-    _worker = SerialWorker(settings.serial_port, settings.serial_baud)
+    _worker = SerialWorker(
+        settings.serial_port,
+        settings.serial_baud,
+        settings.sms_send_queue_maxsize,
+    )
     _worker.start()
     logger.info("Serial worker started on %s", settings.serial_port)
 
@@ -119,7 +134,12 @@ def _process_incoming(event):
         status="received",
     )
 
-    reply, forward_number, forward_body = handle_incoming_sms(number, body)
+    reply, forward_number, forward_body, rejection_reason = handle_incoming_sms(
+        number, body
+    )
+
+    if rejection_reason:
+        database.update_message_status(msg_id, "rejected", rejection_reason)
 
     # Forward to target via SMS
     if forward_number and forward_body and _worker:
@@ -153,6 +173,12 @@ def _send_and_log(from_number: str, to_number: str, body: str):
         result = _worker.send_sms(to_number, body, timeout=120)
         status = "sent" if result["ok"] else "failed"
         database.update_message_status(msg_id, status, result.get("reason"))
+    except OutboundQueueFullError:
+        logger.warning("Internal outbound SMS rejected: QUEUE_FULL")
+        database.update_message_status(msg_id, "failed", "QUEUE_FULL")
+    except WorkerStoppingError:
+        logger.info("Internal outbound SMS rejected: SERVICE_STOPPING")
+        database.update_message_status(msg_id, "failed", "SERVICE_STOPPING")
     except Exception as e:
         logger.error("send_sms error: %s", e)
         database.update_message_status(msg_id, "failed", str(e))
@@ -203,10 +229,21 @@ def validate_phone(cls, v):
         return v
 
 
+def require_gsm_secret(
+    x_gsm_secret: Annotated[
+        Optional[str], Header(alias="X-GSM-Secret")
+    ] = None,
+):
+    if x_gsm_secret is None or not hmac.compare_digest(
+        x_gsm_secret, settings.gsm_secret
+    ):
+        raise HTTPException(status_code=401, detail="Invalid GSM secret")
+
+
 # ── Health endpoints ──────────────────────────────────────────────────────────
 
 @app.get("/health", tags=["health"])
-def health():
+async def health():
     """
     Liveness check. Returns 200 if the API is running.
     Returns 503 if the GSM modem is not ready (so load balancers can react).
@@ -257,6 +294,9 @@ def pct(value):
         "connected":    _worker.connected,
         "last_status":  _worker.last_status,
         "queue_depth":  _worker.incoming_queue.qsize(),
+        "outbound_queue_depth": _worker.outbound_queue_depth,
+        "outbound_queue_capacity": _worker.outbound_queue_capacity,
+        "outbound_in_flight": _worker.outbound_in_flight,
         "port":         settings.serial_port,
         "baud":         settings.serial_baud,
         "total_messages": total,
@@ -303,7 +343,11 @@ def status():
 
 # ── SMS endpoints ─────────────────────────────────────────────────────────────
 
-@app.post("/sms/send", tags=["sms"])
+@app.post(
+    "/sms/send",
+    tags=["sms"],
+    dependencies=[Depends(require_gsm_secret)],
+)
 def send_sms(req: SendSMSRequest):
     """
     Send an SMS directly. Blocks until the modem confirms delivery.
@@ -323,6 +367,20 @@ def send_sms(req: SendSMSRequest):
 
     try:
         result = _worker.send_sms(req.number, req.body, timeout=60)
+    except OutboundQueueFullError:
+        database.update_message_status(msg_id, "failed", "QUEUE_FULL")
+        raise HTTPException(503, {
+            "message": "Outbound SMS queue is full",
+            "reason": "QUEUE_FULL",
+            "msg_id": msg_id,
+        })
+    except WorkerStoppingError:
+        database.update_message_status(msg_id, "failed", "SERVICE_STOPPING")
+        raise HTTPException(503, {
+            "message": "SMS service is stopping",
+            "reason": "SERVICE_STOPPING",
+            "msg_id": msg_id,
+        })
     except RuntimeError as e:
         database.update_message_status(msg_id, "failed", str(e))
         raise HTTPException(503, str(e))
@@ -331,6 +389,12 @@ def send_sms(req: SendSMSRequest):
     database.update_message_status(msg_id, status, result.get("reason"))
 
     if not result["ok"]:
+        if result["reason"] == "SERVICE_STOPPING":
+            raise HTTPException(503, {
+                "message": "SMS service is stopping",
+                "reason": "SERVICE_STOPPING",
+                "msg_id": msg_id,
+            })
         raise HTTPException(502, {
             "message": "SMS delivery failed",
             "reason":  result["reason"],

GSM-module/GSM-fastapi/config.py

@@ -11,6 +11,24 @@
 
 import os
 from dotenv import load_dotenv
+from serial_worker import MAX_SEND_QUEUE_SIZE
+
+
+def bounded_integer_env(name: str, default: int, maximum: int) -> int:
+    value = os.environ.get(name)
+    if value is None:
+        return default
+    try:
+        parsed = int(value)
+    except ValueError as error:
+        raise RuntimeError(
+            f"Environment variable '{name}' must be an integer between 1 and {maximum}."
+        ) from error
+    if not 1 <= parsed <= maximum:
+        raise RuntimeError(
+            f"Environment variable '{name}' must be an integer between 1 and {maximum}."
+        )
+    return parsed
 
 
 class Settings:
@@ -21,18 +39,26 @@ class Settings:
     # Baud rate — must match PC_BAUD in the Arduino sketch (9600)
     serial_baud: int = int(os.environ.get("SERIAL_BAUD", "9600"))
 
-    # SQLite database file path
     db_path: str | None = os.environ.get("DB_PATH")
 
     if not db_path:
         raise RuntimeError("Environment variable 'DB_PATH' is not set.")
 
+    gsm_secret: str = os.environ.get("GSM_SECRET", "")
+
+    if not gsm_secret:
+        raise RuntimeError("Environment variable 'GSM_SECRET' is not set.")
+
     # FastAPI host and port
     host: str = os.environ.get("HOST", "127.0.0.1")
     port: int = int(os.environ.get("PORT", "8000"))
 
     # Logging level
     log_level: str = os.environ.get("LOG_LEVEL", "INFO")
 
+    sms_send_queue_maxsize: int = bounded_integer_env(
+        "SMS_SEND_QUEUE_MAXSIZE", 10, MAX_SEND_QUEUE_SIZE
+    )
+
 
 settings = Settings()

GSM-module/GSM-fastapi/database.py

@@ -520,6 +520,17 @@ def update_message_status(msg_id: str, status: str,
         s.commit()
 
 
+def fail_orphaned_pending_messages() -> int:
+    with new_get_session() as s:
+        result = s.execute(
+            update(SmsLog)
+            .where(SmsLog.status == "pending")
+            .values(status="failed", failure_reason="SERVICE_CRASHED")
+        )
+        s.commit()
+        return result.rowcount
+
+
 def get_messages(limit: int = 50, offset: int = 0, direction: Optional[str] = None,
                  phone: Optional[str] = None) -> dict:
     with new_get_session() as s: