From 48d6714e7b2d71ba21d9501dd74eb6a1ed5d5abb Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 10:48:13 +0800 Subject: [PATCH 01/10] feat(gsm-inbound): warn unregistered SMS senders once. --- GSM-module/GSM-fastapi/api.py | 9 ++- GSM-module/GSM-fastapi/database.py | 20 ++++++ GSM-module/GSM-fastapi/sms_handler.py | 3 +- .../tests/test_database_reconciliation.py | 12 ++++ .../GSM-fastapi/tests/test_incoming_sms.py | 61 ++++++++++++++++++- docs/features/sms-gateway/design.md | 3 +- docs/features/sms-gateway/requirements.md | 1 + docs/features/sms-gateway/testing.md | 4 +- 8 files changed, 106 insertions(+), 7 deletions(-) diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index 6fd2c59d..a285befb 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -152,16 +152,18 @@ def _process_incoming(event): # Send reply back to sender if reply and _worker: - _send_and_log( + reply_sent = _send_and_log( from_number="SERVER", to_number=number, body=reply, ) + if reply_sent and rejection_reason == "NO_ACCOUNT": + database.mark_unregistered_warning(number) def _send_and_log(from_number: str, to_number: str, body: str): - """Send one SMS via the worker and persist result to DB.""" + """Send one SMS, persist its result, and return whether it was sent.""" msg_id = database.log_message( direction="OUT", from_number=from_number, @@ -173,6 +175,7 @@ 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")) + return result["ok"] except OutboundQueueFullError: logger.warning("Internal outbound SMS rejected: QUEUE_FULL") database.update_message_status(msg_id, "failed", "QUEUE_FULL") @@ -183,6 +186,8 @@ def _send_and_log(from_number: str, to_number: str, body: str): logger.error("send_sms error: %s", e) database.update_message_status(msg_id, "failed", str(e)) + return False + # ── App ─────────────────────────────────────────────────────────────────────── diff --git a/GSM-module/GSM-fastapi/database.py b/GSM-module/GSM-fastapi/database.py index 6fce6193..f3c506e5 100644 --- a/GSM-module/GSM-fastapi/database.py +++ b/GSM-module/GSM-fastapi/database.py @@ -14,6 +14,7 @@ RELAY-ONLY (owned by this service, invisible to the main API) sms_session — per-number conversation stage + current target sms_log — every SMS in/out with delivery status + sms_unregistered_warning — numbers that received the registration warning Engine is created once at startup via init(). All public functions are thread-safe (SQLAlchemy handles connection pooling). @@ -81,6 +82,7 @@ def init(db_url: str): Base.metadata.create_all(_engine, tables=[ SmsSession.__table__, SmsLog.__table__, + SmsUnregisteredWarning.__table__, ]) logger.info("Database ready: %s", db_url.split("@")[-1]) # hide credentials @@ -386,6 +388,13 @@ class SmsLog(Base): created_at = Column(BigInteger, default=_now_ms, nullable=False) + +class SmsUnregisteredWarning(Base): + __tablename__ = "sms_unregistered_warning" + + phone = Column(String(20), primary_key=True) + warned_at = Column(BigInteger, default=_now_ms, nullable=False) + # ============================================================================= # USER LOOKUPS (reads from shared `user` table) # ============================================================================= @@ -431,6 +440,17 @@ def get_all_users() -> list[dict]: # SESSION MANAGEMENT (relay-only sms_session table) # ============================================================================= +def has_unregistered_warning(phone: str) -> bool: + with new_get_session() as s: + return s.get(SmsUnregisteredWarning, phone) is not None + + +def mark_unregistered_warning(phone: str): + with new_get_session() as s: + if s.get(SmsUnregisteredWarning, phone) is None: + s.add(SmsUnregisteredWarning(phone=phone)) + s.commit() + def get_session_data(phone: str) -> dict: """Return session row for `phone`, creating it if absent.""" with new_get_session() as s: diff --git a/GSM-module/GSM-fastapi/sms_handler.py b/GSM-module/GSM-fastapi/sms_handler.py index a16473e1..167986dc 100644 --- a/GSM-module/GSM-fastapi/sms_handler.py +++ b/GSM-module/GSM-fastapi/sms_handler.py @@ -41,7 +41,6 @@ MSG_NO_ARG = "Provide a number: [target] +639XXXXXXXXX" # 40 chars MSG_FORWARD_FAIL = "Could not forward your message. Please try again." # 50 - def _msg_target_set(username: str, phone: str) -> str: return f"Target: {username} ({phone}). Messages go to them now." # e.g. "Target: maria_santos (+639281234567). Messages go to them now." = 63 @@ -81,6 +80,8 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: if not sender_user: logger.warning("Account does not exist: %s", number) + if database.has_unregistered_warning(number): + return None, None, None, "NO_ACCOUNT" return MSG_NO_ACCOUNT, None, None, "NO_ACCOUNT" if sender_user.get("banned"): diff --git a/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py b/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py index b884e81f..cf9161ac 100644 --- a/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py +++ b/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py @@ -28,3 +28,15 @@ def test_fail_orphaned_pending_messages_marks_only_pending_rows(tmp_path): assert messages[received_id]["status"] == "received" assert messages[received_id]["failure_reason"] is None assert database.fail_orphaned_pending_messages() == 0 + + +def test_unregistered_warning_survives_session_reset(tmp_path): + database.init(f"sqlite:///{tmp_path / 'gsm.db'}") + phone = "+639171234567" + + assert not database.has_unregistered_warning(phone) + + database.mark_unregistered_warning(phone) + database.reset_session(phone) + + assert database.has_unregistered_warning(phone) diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py index c9348813..e375b187 100644 --- a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -9,7 +9,6 @@ @pytest.mark.parametrize( ("sender", "expected_reason"), [ - (None, "NO_ACCOUNT"), ({"banned": True, "phone_is_verified": True}, "BANNED_SENDER"), ({"banned": False, "phone_is_verified": False}, "UNVERIFIED_SENDER"), ], @@ -31,6 +30,66 @@ def test_handle_incoming_sms_reports_sender_rejection_reason( assert rejection_reason == expected_reason +def test_first_unregistered_message_sends_registration_warning(monkeypatch): + monkeypatch.setattr(sms_handler.database, "get_user_by_phone", lambda _number: None) + monkeypatch.setattr( + sms_handler.database, "has_unregistered_warning", lambda _number: False + ) + + reply, forward_number, forward_body, rejection_reason = ( + sms_handler.handle_incoming_sms("+639171234567", "help") + ) + + assert reply == sms_handler.MSG_NO_ACCOUNT + assert forward_number is None + assert forward_body is None + assert rejection_reason == "NO_ACCOUNT" + + +def test_later_unregistered_messages_are_ignored(monkeypatch): + monkeypatch.setattr(sms_handler.database, "get_user_by_phone", lambda _number: None) + monkeypatch.setattr( + sms_handler.database, + "has_unregistered_warning", + lambda _number: True, + ) + + reply, forward_number, forward_body, rejection_reason = ( + sms_handler.handle_incoming_sms("+639171234567", "help again") + ) + + assert reply is None + assert forward_number is None + assert forward_body is None + assert rejection_reason == "NO_ACCOUNT" + + +@pytest.mark.parametrize("reply_sent", [True, False]) +def test_process_incoming_marks_warning_only_after_successful_reply( + monkeypatch, reply_sent +): + marked_numbers = [] + monkeypatch.setattr(api.database, "log_message", lambda **_kwargs: "message-id") + monkeypatch.setattr(api.database, "update_message_status", lambda *_args: None) + monkeypatch.setattr( + api, + "handle_incoming_sms", + lambda _number, _body: (sms_handler.MSG_NO_ACCOUNT, None, None, "NO_ACCOUNT"), + ) + monkeypatch.setattr(api, "_worker", object()) + monkeypatch.setattr(api, "_send_and_log", lambda **_kwargs: reply_sent) + monkeypatch.setattr( + api.database, + "mark_unregistered_warning", + lambda number: marked_numbers.append(number), + ) + + api._process_incoming(SimpleNamespace(number="+639171234567", body="help")) + + expected = ["+639171234567"] if reply_sent else [] + assert marked_numbers == expected + + def test_process_incoming_persists_rejection_reason(monkeypatch): updates = [] monkeypatch.setattr(api.database, "log_message", lambda **_kwargs: "message-id") diff --git a/docs/features/sms-gateway/design.md b/docs/features/sms-gateway/design.md index 20bb0222..020a885e 100644 --- a/docs/features/sms-gateway/design.md +++ b/docs/features/sms-gateway/design.md @@ -95,7 +95,7 @@ Message bodies may contain pipe characters. `parse_line()` preserves them for `S ## How are inbound messages handled? -The reader places `SMS_RECEIVED` events on `incoming_queue`. The API lifespan task passes each event to `handle_incoming_sms()`, which applies registration, ban, phone-verification, session, and target checks. +The reader places `SMS_RECEIVED` events on `incoming_queue`. The API lifespan task passes each event to `handle_incoming_sms()`, which applies registration, ban, phone-verification, session, and target checks. An unregistered number receives a registration warning on its first message. The gateway records the warning only after the modem confirms that reply was sent. It ignores later messages from that number, preventing repeat reply charges. A separate relay-only record preserves this behavior when an operator resets the number's normal relay session. The handler can return a reply to the sender and a forwarded message for the selected target. Both use the same bounded outbound queue. `database.notify_app()` also calls the main server's `POST /gsm/inbound` route with `X-GSM-Secret`. @@ -107,6 +107,7 @@ The GSM service uses the database configured by required `DB_PATH`. |---|---| | `sms_log` | Inbound and outbound audit rows, delivery status, and failure reason | | `sms_session` | Per-phone conversation stage and selected target | +| `sms_unregistered_warning` | Numbers that received the unregistered-account warning | | Shared user and conversation tables | Lookup and delivery integration with the main server | The committed `sapot.db` file is stale and is not used by the deployed service. diff --git a/docs/features/sms-gateway/requirements.md b/docs/features/sms-gateway/requirements.md index 3aaabb60..7b9575f8 100644 --- a/docs/features/sms-gateway/requirements.md +++ b/docs/features/sms-gateway/requirements.md @@ -102,6 +102,7 @@ Only one request may await a modem confirmation. A confirmation received before - The serial reader must enqueue `SMS_RECEIVED` events for application processing. - `handle_incoming_sms()` must apply the registered-user, banned-user, verified-phone, session, and target rules. +- An unregistered number must receive the registration warning only once after the gateway confirms it was sent. Later inbound messages from that number must be rejected without an outbound reply. - Sender eligibility failures must set the inbound `sms_log` row to `rejected` with `NO_ACCOUNT`, `BANNED_SENDER`, or `UNVERIFIED_SENDER` as the failure reason. - The GSM service must call the main server's `POST /gsm/inbound` route with `X-GSM-Secret` when forwarding into the app. - Failed callbacks are logged. Automatic callback retry is not required. diff --git a/docs/features/sms-gateway/testing.md b/docs/features/sms-gateway/testing.md index 90ee350a..e3cc6c00 100644 --- a/docs/features/sms-gateway/testing.md +++ b/docs/features/sms-gateway/testing.md @@ -43,8 +43,8 @@ pnpm run testAll | `tests/test_config.py` | Default queue capacity, valid range, and startup rejection | | `tests/test_serial_worker.py` | Queue capacity, admission deadlines, lifecycle cutoff, exact-once completion, pre-write races, and serial write timeout | | `tests/test_api_queue.py` | HTTP 503 contracts, message-log updates, diagnostics, worker-pool headroom, and health responsiveness | -| `tests/test_database_reconciliation.py` | Idempotent startup recovery of orphaned pending log rows | -| `tests/test_incoming_sms.py` | Sender rejection reason codes and inbound log status updates | +| `tests/test_database_reconciliation.py` | Idempotent startup recovery of orphaned pending log rows and unregistered-warning persistence across session resets | +| `tests/test_incoming_sms.py` | Sender rejection reason codes, inbound log status updates, and warning marking only after a successful reply | | `tests/test_lifespan.py` | Reconciliation ordering before serial worker startup | | `tests/test_mock_modem.py` | Virtual-phone validation, firmware-compatible normalization, modem state transitions, HTTP responses, PTY framing, reconnects, and subprocess cleanup | | `server/app/tests/test_gsm_proxy.py` | Main-server shared-secret header, status preservation, and timeout headroom for chat, verification, resend, and first-contact requests | From d41f0a196e31856b535d2389d700b00937dfcc8d Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 12:38:12 +0800 Subject: [PATCH 02/10] feat(gsm-sms): add unregistered warning and rate limits. Implemented unregistered user warning and rate limiting for the GSM module. Added new environment variables for configuring queue sizes, daily send limits, sender daily limits, cooldown seconds, and log retention. Updated serial worker and api to support the new features. Updated relevant documentation. --- GSM-module/GSM-fastapi/.env.example | 15 ++ GSM-module/GSM-fastapi/api.py | 76 +++++++++-- GSM-module/GSM-fastapi/config.py | 31 +++++ GSM-module/GSM-fastapi/database.py | 128 +++++++++++++++++- GSM-module/GSM-fastapi/main.py | 8 +- GSM-module/GSM-fastapi/serial_worker.py | 15 +- GSM-module/GSM-fastapi/sms_handler.py | 71 +++++++--- .../GSM-fastapi/tests/test_api_queue.py | 6 +- .../GSM-fastapi/tests/test_serial_worker.py | 10 ++ SECURITY.md | 3 + docs/api/gsm-sms.md | 6 +- docs/deployment/environment-config.md | 16 +++ docs/features/sms-gateway/design.md | 4 +- server/app/api/gsm.py | 28 +++- server/app/tests/test_gsm_health.py | 4 +- server/app/tests/test_gsm_proxy.py | 19 ++- 16 files changed, 384 insertions(+), 56 deletions(-) diff --git a/GSM-module/GSM-fastapi/.env.example b/GSM-module/GSM-fastapi/.env.example index ebc8eebb..88ba79a0 100644 --- a/GSM-module/GSM-fastapi/.env.example +++ b/GSM-module/GSM-fastapi/.env.example @@ -11,6 +11,21 @@ LOG_LEVEL=INFO # Maximum outbound SMS requests waiting behind the one in-flight request. # Must be an integer from 1 through 20. SMS_SEND_QUEUE_MAXSIZE=10 +# Maximum inbound events waiting for serial relay handling. Excess events are +# dropped and counted in /health/detailed rather than consuming memory. +SMS_INCOMING_QUEUE_MAXSIZE=100 +# Hard carrier-spend ceiling. Set to 0 to disable all outgoing SMS immediately. +SMS_DAILY_SEND_LIMIT=100 +# Per-sender daily reply ceiling and sender-target relay ceiling. +SMS_SENDER_DAILY_LIMIT=20 +SMS_SENDER_TARGET_DAILY_LIMIT=10 +# Minimum spacing between replies in the same response category for one sender. +SMS_RESPONSE_COOLDOWN_SECONDS=30 +# Retain redacted operational SMS logs for this many days. Set to 0 to purge +# existing logs at each service start. +SMS_LOG_RETENTION_DAYS=30 +LOG_MAX_BYTES=1000000 +LOG_BACKUP_COUNT=3 SAPOT_API_URL=http://localhost:8000 GSM_SECRET=change-me-to-a-strong-secret SMS_BOT_USER_ID= diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index a285befb..f40a6148 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -68,12 +68,16 @@ async def lifespan(app: FastAPI): "Marked %d orphaned SMS messages as failed after service restart", orphaned_count, ) + purged_count = database.purge_expired_sms_logs(settings.sms_log_retention_days) + if purged_count: + logger.info("Purged %d expired SMS log records", purged_count) # Serial worker _worker = SerialWorker( settings.serial_port, settings.serial_baud, settings.sms_send_queue_maxsize, + settings.sms_incoming_queue_maxsize, ) _worker.start() logger.info("Serial worker started on %s", settings.serial_port) @@ -141,14 +145,28 @@ def _process_incoming(event): if rejection_reason: database.update_message_status(msg_id, "rejected", rejection_reason) - # Forward to target via SMS + if reply and not database.allow_inbound_response( + number, + rejection_reason or "REPLY", + settings.sms_sender_daily_limit, + settings.sms_response_cooldown_seconds, + ): + database.update_message_status(msg_id, "rejected", "RATE_LIMITED") + return + + # Forward to target via SMS. A confirmation is only sent if the target SMS + # was accepted by the modem, so users do not receive a false "Forwarded" state. + target_sent = True if forward_number and forward_body and _worker: - _send_and_log( + target_sent = _send_and_log( from_number="SERVER", to_number=forward_number, body=forward_body, ) + if not target_sent: + reply = "Could not forward your message. Please try again." + # Send reply back to sender if reply and _worker: @@ -162,8 +180,8 @@ def _process_incoming(event): -def _send_and_log(from_number: str, to_number: str, body: str): - """Send one SMS, persist its result, and return whether it was sent.""" +def _send_and_log(from_number: str, to_number: str, body: str) -> bool: + """Send one SMS via the worker and persist result to DB.""" msg_id = database.log_message( direction="OUT", from_number=from_number, @@ -171,6 +189,13 @@ def _send_and_log(from_number: str, to_number: str, body: str): body=body, status="pending", ) + if database.is_sms_opted_out(to_number): + database.update_message_status(msg_id, "failed", "RECIPIENT_OPTED_OUT") + return False + if not database.reserve_outbound_sms(settings.sms_daily_send_limit): + logger.warning("Outbound SMS rejected: daily send limit reached") + database.update_message_status(msg_id, "failed", "DAILY_SEND_LIMIT") + return False try: result = _worker.send_sms(to_number, body, timeout=120) status = "sent" if result["ok"] else "failed" @@ -179,12 +204,15 @@ def _send_and_log(from_number: str, to_number: str, body: str): except OutboundQueueFullError: logger.warning("Internal outbound SMS rejected: QUEUE_FULL") database.update_message_status(msg_id, "failed", "QUEUE_FULL") + return False except WorkerStoppingError: logger.info("Internal outbound SMS rejected: SERVICE_STOPPING") database.update_message_status(msg_id, "failed", "SERVICE_STOPPING") + return False except Exception as e: logger.error("send_sms error: %s", e) database.update_message_status(msg_id, "failed", str(e)) + return False return False @@ -272,7 +300,9 @@ async def health(): ) -@app.get("/health/detailed", tags=["health"]) +@app.get( + "/health/detailed", tags=["health"], dependencies=[Depends(require_gsm_secret)] +) def health_detailed( phone: Optional[str] = None, ): @@ -299,6 +329,8 @@ def pct(value): "connected": _worker.connected, "last_status": _worker.last_status, "queue_depth": _worker.incoming_queue.qsize(), + "inbound_queue_capacity": _worker.incoming_queue.maxsize, + "inbound_queue_dropped": getattr(_worker, "incoming_queue_dropped", 0), "outbound_queue_depth": _worker.outbound_queue_depth, "outbound_queue_capacity": _worker.outbound_queue_capacity, "outbound_in_flight": _worker.outbound_in_flight, @@ -334,7 +366,7 @@ def pct(value): # ── Status ──────────────────────────────────────────────────────────────────── -@app.get("/status", tags=["modem"]) +@app.get("/status", tags=["modem"], dependencies=[Depends(require_gsm_secret)]) def status(): """Modem and serial connection status.""" if _worker is None: @@ -370,6 +402,22 @@ def send_sms(req: SendSMSRequest): status="pending", ) + if database.is_sms_opted_out(req.number): + database.update_message_status(msg_id, "failed", "RECIPIENT_OPTED_OUT") + raise HTTPException(403, { + "message": "Recipient has opted out of SMS relay messages", + "reason": "RECIPIENT_OPTED_OUT", + "msg_id": msg_id, + }) + + if not database.reserve_outbound_sms(settings.sms_daily_send_limit): + database.update_message_status(msg_id, "failed", "DAILY_SEND_LIMIT") + raise HTTPException(503, { + "message": "Daily SMS send limit reached", + "reason": "DAILY_SEND_LIMIT", + "msg_id": msg_id, + }) + try: result = _worker.send_sms(req.number, req.body, timeout=60) except OutboundQueueFullError: @@ -413,7 +461,7 @@ def send_sms(req: SendSMSRequest): } -@app.get("/sms/messages", tags=["sms"]) +@app.get("/sms/messages", tags=["sms"], dependencies=[Depends(require_gsm_secret)]) def list_messages( limit: int = Query(50, ge=1, le=500), direction: Optional[str] = Query(None, pattern="^(IN|OUT)$"), @@ -425,12 +473,12 @@ def list_messages( # ── User endpoints ──────────────────────────────────────────────────────────── -@app.get("/users", tags=["users"]) +@app.get("/users", tags=["users"], dependencies=[Depends(require_gsm_secret)]) def list_users(): return database.get_all_users() -@app.get("/users/{phone}", tags=["users"]) +@app.get("/users/{phone}", tags=["users"], dependencies=[Depends(require_gsm_secret)]) def get_user(phone: str): user = database.lookup_number(phone) if not user: @@ -438,7 +486,9 @@ def get_user(phone: str): return user -@app.post("/users", tags=["users"], status_code=201) +@app.post( + "/users", tags=["users"], status_code=201, dependencies=[Depends(require_gsm_secret)] +) def add_user(req: AddUserRequest): try: return database.add_user(req.phone, req.username, req.app_active) @@ -450,7 +500,7 @@ def add_user(req: AddUserRequest): # ── Session endpoints ───────────────────────────────────────────────────────── -@app.get("/sessions", tags=["sessions"]) +@app.get("/sessions", tags=["sessions"], dependencies=[Depends(require_gsm_secret)]) def list_sessions(): """All sessions currently in the database (for debugging).""" with database._conn() as cx: @@ -458,7 +508,9 @@ def list_sessions(): return [dict(r) for r in rows] -@app.delete("/sessions/{phone}", tags=["sessions"]) +@app.delete( + "/sessions/{phone}", tags=["sessions"], dependencies=[Depends(require_gsm_secret)] +) def reset_session(phone: str): """Reset a user's conversation session back to NEW.""" database.reset_session(phone) diff --git a/GSM-module/GSM-fastapi/config.py b/GSM-module/GSM-fastapi/config.py index 1fd0dcd4..94615604 100644 --- a/GSM-module/GSM-fastapi/config.py +++ b/GSM-module/GSM-fastapi/config.py @@ -31,6 +31,23 @@ def bounded_integer_env(name: str, default: int, maximum: int) -> int: return parsed +def nonnegative_integer_env(name: str, default: 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 a non-negative integer." + ) from error + if parsed < 0: + raise RuntimeError( + f"Environment variable '{name}' must be a non-negative integer." + ) + return parsed + + class Settings: load_dotenv() # Serial port the Arduino is connected to @@ -59,6 +76,20 @@ class Settings: sms_send_queue_maxsize: int = bounded_integer_env( "SMS_SEND_QUEUE_MAXSIZE", 10, MAX_SEND_QUEUE_SIZE ) + sms_incoming_queue_maxsize: int = bounded_integer_env( + "SMS_INCOMING_QUEUE_MAXSIZE", 100, 10_000 + ) + sms_daily_send_limit: int = nonnegative_integer_env("SMS_DAILY_SEND_LIMIT", 100) + sms_sender_daily_limit: int = nonnegative_integer_env("SMS_SENDER_DAILY_LIMIT", 20) + sms_sender_target_daily_limit: int = nonnegative_integer_env( + "SMS_SENDER_TARGET_DAILY_LIMIT", 10 + ) + sms_response_cooldown_seconds: int = nonnegative_integer_env( + "SMS_RESPONSE_COOLDOWN_SECONDS", 30 + ) + sms_log_retention_days: int = nonnegative_integer_env("SMS_LOG_RETENTION_DAYS", 30) + log_max_bytes: int = bounded_integer_env("LOG_MAX_BYTES", 1_000_000, 100_000_000) + log_backup_count: int = bounded_integer_env("LOG_BACKUP_COUNT", 3, 100) settings = Settings() diff --git a/GSM-module/GSM-fastapi/database.py b/GSM-module/GSM-fastapi/database.py index f3c506e5..934be84c 100644 --- a/GSM-module/GSM-fastapi/database.py +++ b/GSM-module/GSM-fastapi/database.py @@ -22,7 +22,6 @@ from sqlalchemy import Column, String, DateTime, ForeignKey, func from sqlalchemy.orm import relationship from datetime import datetime, timezone -from datetime import datetime import logging import time import uuid @@ -33,7 +32,7 @@ BigInteger, Boolean, Column, Enum as SAEnum, ForeignKey, String, Text, UniqueConstraint, text, ) -from sqlalchemy import create_engine, select, update, insert +from sqlalchemy import create_engine, delete, select, update from sqlalchemy.orm import DeclarativeBase, Session, relationship, sessionmaker from sqlalchemy.dialects.mysql import CHAR import uuid @@ -83,6 +82,8 @@ def init(db_url: str): SmsSession.__table__, SmsLog.__table__, SmsUnregisteredWarning.__table__, + SmsRateCounter.__table__, + SmsRecipientPreference.__table__, ]) logger.info("Database ready: %s", db_url.split("@")[-1]) # hide credentials @@ -394,6 +395,20 @@ class SmsUnregisteredWarning(Base): phone = Column(String(20), primary_key=True) warned_at = Column(BigInteger, default=_now_ms, nullable=False) +class SmsRateCounter(Base): + __tablename__ = "sms_rate_counter" + + key = Column(String(160), primary_key=True) + count = Column(BigInteger, default=0, nullable=False) + last_seen = Column(BigInteger, default=_now_ms, nullable=False) + + +class SmsRecipientPreference(Base): + __tablename__ = "sms_recipient_preference" + + phone = Column(String(20), primary_key=True) + opted_out = Column(Boolean, default=False, nullable=False) + updated_at = Column(BigInteger, default=_now_ms, nullable=False) # ============================================================================= # USER LOOKUPS (reads from shared `user` table) @@ -515,12 +530,12 @@ def get_all_sessions() -> list[dict]: def log_message(direction: str, from_number: str, to_number: str, body: str, status: str = "pending") -> str: - """Insert a log row and return its UUID string.""" + """Insert operational metadata without retaining SMS message content.""" row = SmsLog( direction=direction, - from_number=from_number, - to_number=to_number, - body=body, + from_number=_redact_phone(from_number), + to_number=_redact_phone(to_number), + body=f"[redacted: {len(body)} characters]", status=status, ) with new_get_session() as s: @@ -529,6 +544,12 @@ def log_message(direction: str, from_number: str, to_number: str, return str(row.id) +def _redact_phone(phone: str) -> str: + if len(phone) <= 4: + return "[redacted]" + return f"***{phone[-4:]}" + + def update_message_status(msg_id: str, status: str, failure_reason: Optional[str] = None): with new_get_session() as s: @@ -551,6 +572,14 @@ def fail_orphaned_pending_messages() -> int: return result.rowcount +def purge_expired_sms_logs(retention_days: int) -> int: + cutoff = _now_ms() - retention_days * 24 * 60 * 60 * 1000 + with new_get_session() as s: + result = s.execute(delete(SmsLog).where(SmsLog.created_at < cutoff)) + 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: @@ -558,6 +587,7 @@ def get_messages(limit: int = 50, offset: int = 0, direction: Optional[str] = No if direction: q = q.where(SmsLog.direction == direction) if phone: + phone = _redact_phone(phone) q = q.where( (SmsLog.from_number == phone) | (SmsLog.to_number == phone) ) @@ -588,6 +618,92 @@ def get_messages(limit: int = 50, offset: int = 0, direction: Optional[str] = No } +def _counter(session: Session, key: str) -> SmsRateCounter: + row = session.execute( + select(SmsRateCounter).where(SmsRateCounter.key == key).with_for_update() + ).scalar_one_or_none() + if row is None: + row = SmsRateCounter(key=key) + session.add(row) + session.flush() + return row + + +def _day_key() -> str: + return datetime.now(timezone.utc).strftime("%Y-%m-%d") + + +def reserve_outbound_sms(limit: int) -> bool: + """Reserve one daily carrier-send slot before queueing a modem operation.""" + with new_get_session() as s: + row = _counter(s, f"outbound:{_day_key()}") + if limit <= 0 or row.count >= limit: + s.rollback() + return False + row.count += 1 + row.last_seen = _now_ms() + s.commit() + return True + + +def allow_inbound_response( + phone: str, category: str, daily_limit: int, cooldown_seconds: int +) -> bool: + """Apply both a sender-wide ceiling and a response-category cooldown.""" + now = _now_ms() + day = _day_key() + with new_get_session() as s: + sender = _counter(s, f"inbound:{day}:{phone}") + category_counter = _counter(s, f"response:{phone}:{category}") + if ( + daily_limit <= 0 + or sender.count >= daily_limit + or ( + category_counter.count > 0 + and now - category_counter.last_seen < cooldown_seconds * 1000 + ) + ): + s.rollback() + return False + sender.count += 1 + category_counter.count += 1 + category_counter.last_seen = now + s.commit() + return True + + +def allow_sender_target( + sender_phone: str, target_phone: str, daily_limit: int +) -> bool: + """Limit relay attempts for a sender-target pair within the current UTC day.""" + with new_get_session() as s: + row = _counter(s, f"relay:{_day_key()}:{sender_phone}:{target_phone}") + if daily_limit <= 0 or row.count >= daily_limit: + s.rollback() + return False + row.count += 1 + row.last_seen = _now_ms() + s.commit() + return True + + +def set_sms_opt_out(phone: str, opted_out: bool) -> None: + with new_get_session() as s: + row = s.get(SmsRecipientPreference, phone) + if row is None: + row = SmsRecipientPreference(phone=phone) + s.add(row) + row.opted_out = opted_out + row.updated_at = _now_ms() + s.commit() + + +def is_sms_opted_out(phone: str) -> bool: + with new_get_session() as s: + row = s.get(SmsRecipientPreference, phone) + return bool(row and row.opted_out) + + # ============================================================================= # APP FORWARD — write message into shared Conversation + Message tables # so the forwarded SMS appears inside the SAPOT app natively diff --git a/GSM-module/GSM-fastapi/main.py b/GSM-module/GSM-fastapi/main.py index f038c5f6..4386a3fb 100644 --- a/GSM-module/GSM-fastapi/main.py +++ b/GSM-module/GSM-fastapi/main.py @@ -14,6 +14,7 @@ import logging import sys +from logging.handlers import RotatingFileHandler import uvicorn @@ -28,7 +29,12 @@ def setup_logging(): format=fmt, handlers=[ logging.StreamHandler(sys.stdout), - logging.FileHandler("sapot.log", encoding="utf-8"), + RotatingFileHandler( + "sapot.log", + maxBytes=settings.log_max_bytes, + backupCount=settings.log_backup_count, + encoding="utf-8", + ), ], ) # Quiet down uvicorn's access log a little diff --git a/GSM-module/GSM-fastapi/serial_worker.py b/GSM-module/GSM-fastapi/serial_worker.py index dd2fa080..060c111f 100644 --- a/GSM-module/GSM-fastapi/serial_worker.py +++ b/GSM-module/GSM-fastapi/serial_worker.py @@ -75,7 +75,7 @@ class SerialWorker: """ def __init__(self, port: str, baud: int = 9600, - send_queue_maxsize: int = 10): + send_queue_maxsize: int = 10, incoming_queue_maxsize: int = 100): if not 1 <= send_queue_maxsize <= MAX_SEND_QUEUE_SIZE: raise ValueError( f"send_queue_maxsize must be between 1 and {MAX_SEND_QUEUE_SIZE}" @@ -102,7 +102,10 @@ def __init__(self, port: str, baud: int = 9600, self._in_flight_lock = threading.Lock() # Inbound SMS for the application layer - self.incoming_queue: queue.Queue[SerialEvent] = queue.Queue() + self.incoming_queue: queue.Queue[SerialEvent] = queue.Queue( + maxsize=incoming_queue_maxsize + ) + self.incoming_queue_dropped = 0 # Public status flags self.connected = False @@ -347,8 +350,12 @@ def _handle_line(self, line: str): return if etype == EventType.SMS_RECEIVED: - logger.info("SMS_RECEIVED from %s: %r", event.number, event.body) - self.incoming_queue.put(event) + logger.info("SMS_RECEIVED from %s (%d characters)", event.number, len(event.body)) + try: + self.incoming_queue.put_nowait(event) + except queue.Full: + self.incoming_queue_dropped += 1 + logger.warning("Inbound SMS dropped because the queue is full") return logger.debug("Unhandled: %r", event.raw) diff --git a/GSM-module/GSM-fastapi/sms_handler.py b/GSM-module/GSM-fastapi/sms_handler.py index 167986dc..c2f915ff 100644 --- a/GSM-module/GSM-fastapi/sms_handler.py +++ b/GSM-module/GSM-fastapi/sms_handler.py @@ -11,9 +11,12 @@ """ import logging +import re +import unicodedata from typing import Optional, Tuple import database +from config import settings logger = logging.getLogger("sapot.handler") @@ -74,8 +77,12 @@ def _forward_body(sender_phone: str, body: str) -> str: # ── Main entry point ────────────────────────────────────────────────────────── def handle_incoming_sms(number: str, body: str) -> ForwardTuple: - body = body.strip() - logger.info("SMS in %s: %r", number, body) + number = _normalize_phone(number) + if number is None: + logger.warning("Rejected malformed sender number") + return None, None, None, "MALFORMED_SENDER" + body = " ".join(unicodedata.normalize("NFKC", body).split()) + logger.info("SMS received from %s (%d characters)", number, len(body)) sender_user = database.get_user_by_phone(number) if not sender_user: @@ -105,6 +112,13 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: if sender is None: return MSG_NO_ACCOUNT, None, None, "NO_ACCOUNT" + if body.upper() == "STOP": + database.set_sms_opt_out(number, True) + return "SMS relay messages are disabled for this number. Reply START to enable them.", None, None, "OPT_OUT" + if body.upper() == "START": + database.set_sms_opt_out(number, False) + return "SMS relay messages are enabled for this number.", None, None, "OPT_IN" + session = database.get_session(number) stage = session["stage"] @@ -114,16 +128,29 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: if stage == "NEW": database.update_session(number, stage="AWAITING_TARGET") - return MSG_WELCOME, None, None, None + return MSG_WELCOME, None, None, "WELCOME" if stage == "AWAITING_TARGET": - return MSG_NEED_TARGET, None, None, None + return MSG_NEED_TARGET, None, None, "AWAITING_TARGET" if stage == "ACTIVE": return _do_forward(number, body, session) database.reset_session(number) - return MSG_WELCOME, None, None, None + return MSG_WELCOME, None, None, "WELCOME" + + +def _normalize_phone(number: str) -> Optional[str]: + normalized = unicodedata.normalize("NFKC", number).strip().replace(" ", "") + if re.fullmatch(r"09\d{9}", normalized): + normalized = "+63" + normalized[1:] + elif re.fullmatch(r"639\d{9}", normalized): + normalized = "+" + normalized + if normalized.startswith("00"): + normalized = "+" + normalized[2:] + if re.fullmatch(r"\+639\d{9}", normalized): + return normalized + return None # ── Sub-handlers ────────────────────────────────────────────────────────────── @@ -132,20 +159,20 @@ def _cmd_set_target(number: str, body: str) -> ForwardTuple: # body is like "[target] +639281234567" or "[target]" (no arg) parts = body.split(None, 1) if len(parts) < 2 or not parts[1].strip(): - return MSG_NO_ARG, None, None, None + return MSG_NO_ARG, None, None, "TARGET_NO_ARGUMENT" - target_phone = parts[1].strip() + target_phone = _normalize_phone(parts[1]) - if not target_phone.startswith("+") or not target_phone[1:].isdigit(): - return MSG_INVALID_FMT, None, None, None + if target_phone is None: + return MSG_INVALID_FMT, None, None, "TARGET_INVALID_FORMAT" # Sender cannot target themselves if target_phone == number: - return "You cannot set yourself as the target.", None, None, None + return "You cannot set yourself as the target.", None, None, "SELF_TARGET" target = database.lookup_number(target_phone) if target is None: - return MSG_TARGET_NOT_FOUND, None, None, None + return MSG_TARGET_NOT_FOUND, None, None, "TARGET_NOT_FOUND" database.update_session( number, @@ -154,7 +181,7 @@ def _cmd_set_target(number: str, body: str) -> ForwardTuple: target_username=target["username"], ) - return _msg_target_set(target["username"], target_phone), None, None, None + return _msg_target_set(target["username"], target_phone), None, None, "TARGET_SET" def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: @@ -165,7 +192,10 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: if not target_user: logger.warning("Target does not exist: %s", target_phone) - return f"Target {target_phone} does not exist.", sender_phone, None, None + return f"Target {target_phone} does not exist.", None, None, "TARGET_MISSING" + + if database.is_sms_opted_out(target_phone): + return "That target has opted out of SMS relay messages.", None, None, "TARGET_OPTED_OUT" if target_user.get("banned"): logger.warning("Banned: %s", target_phone) @@ -173,20 +203,27 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: f"This number ({target_phone}) has been banned by the system.", None, None, - None, + "TARGET_BANNED", ) if not target_user.get("phone_is_verified"): logger.warning("Unverified number: %s", target_phone) - return f"Target {target_phone} is not verified.", None, None, None + return f"Target {target_phone} is not verified.", None, None, "TARGET_UNVERIFIED" if not target_phone: database.reset_session(sender_phone) - return MSG_WELCOME, None, None, None + return MSG_WELCOME, None, None, "WELCOME" + + if not database.allow_sender_target( + sender_phone, target_phone, settings.sms_sender_target_daily_limit + ): + return "Relay limit reached. Please try again tomorrow.", None, None, "RELAY_LIMIT" ok = database.notify_app(sender_phone, target_phone, body) logger.info("notify_app result: %s (sender=%s target=%s)", ok, sender_phone, target_phone) + if not ok: + return MSG_FORWARD_FAIL, None, None, "APP_DELIVERY_FAILED" # Build clean forward body for the target's SMS fwd = _forward_body(sender_phone, body) - return _msg_forwarded(target_username), target_phone, fwd, None + return _msg_forwarded(target_username), target_phone, fwd, "FORWARDED" diff --git a/GSM-module/GSM-fastapi/tests/test_api_queue.py b/GSM-module/GSM-fastapi/tests/test_api_queue.py index 72804f74..79999995 100644 --- a/GSM-module/GSM-fastapi/tests/test_api_queue.py +++ b/GSM-module/GSM-fastapi/tests/test_api_queue.py @@ -17,8 +17,10 @@ @pytest.fixture(autouse=True) -def reset_worker(): +def reset_worker(monkeypatch): previous = api._worker + monkeypatch.setattr(api.database, "is_sms_opted_out", lambda _phone: False) + monkeypatch.setattr(api.database, "reserve_outbound_sms", lambda _limit: True) yield api._worker = previous @@ -85,7 +87,7 @@ def __init__(self): api._worker = Worker() monkeypatch.setattr(api.database, "get_messages", lambda **_kwargs: {"messages": [], "total": 0}) - response = TestClient(api.app).get("/health/detailed") + response = TestClient(api.app).get("/health/detailed", headers=AUTH_HEADERS) assert response.status_code == 200 assert response.json()["queue_depth"] == 1 diff --git a/GSM-module/GSM-fastapi/tests/test_serial_worker.py b/GSM-module/GSM-fastapi/tests/test_serial_worker.py index db82f464..e501b885 100644 --- a/GSM-module/GSM-fastapi/tests/test_serial_worker.py +++ b/GSM-module/GSM-fastapi/tests/test_serial_worker.py @@ -42,6 +42,16 @@ def test_capacity_excludes_registered_in_flight_request(): assert worker.outbound_queue_capacity == 1 +def test_inbound_queue_drops_excess_messages_without_blocking_reader(): + worker = SerialWorker("fake", incoming_queue_maxsize=1) + + worker._handle_line("SMS_RECEIVED|+639171234567|first") + worker._handle_line("SMS_RECEIVED|+639171234568|second") + + assert worker.incoming_queue.qsize() == 1 + assert worker.incoming_queue_dropped == 1 + + def test_stop_drains_waiting_requests_without_sentinel(): worker = ready_worker() request = _SendRequest("+639171234567", "queued", 1) diff --git a/SECURITY.md b/SECURITY.md index 27b6aeaf..1d5c9fc6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -14,6 +14,9 @@ This is the canonical source of truth for SAPOT's known security-relevant config | CORS wildcard + credentials | `server/app/main.py` | `allow_origins=["*"]` replaced with an explicit allowlist read from `CORS_ALLOWED_ORIGINS` (comma-separated). The app raises `RuntimeError` at import time if unset. | | Testing router in production | `server/app/main.py`, `server/app/api/testing.py` | The router is imported and mounted only in `development` or `staging`. A router-wide dependency also returns 404 outside those environments if the router is mis-mounted. Every state-changing route requires the `X-QA-Token` shared secret. A production-process regression test exercises every testing path. | | Unauthenticated direct GSM sends | `GSM-module/GSM-fastapi/api.py`, `server/app/api/gsm.py` | `GSM_SECRET` is required by both services. The main server sends it as `X-GSM-Secret`, and the gateway validates it before logging or queueing `POST /sms/send`. | +| SMS relay spend and reply abuse | `GSM-module/GSM-fastapi/` | The gateway now enforces a daily send ceiling, per-sender and sender-target limits, response cooldowns, recipient STOP/START preferences, and bounded inbound/outbound queues. The main server rate-limits paid SMS routes and requires verified sender and recipient phones for direct sends. | +| Gateway diagnostic and mutation endpoint exposure | `GSM-module/GSM-fastapi/api.py` | Every application API route except liveness-only `/health` requires `X-GSM-Secret`, preventing internal-network callers from reading message history or resetting sessions. | +| GSM log retention of message content | `GSM-module/GSM-fastapi/` | Operational SMS logs redact message bodies and phone numbers, rotate by size, and purge according to `SMS_LOG_RETENTION_DAYS`. | ## Required environment variables (new) diff --git a/docs/api/gsm-sms.md b/docs/api/gsm-sms.md index abd96933..746bffbe 100644 --- a/docs/api/gsm-sms.md +++ b/docs/api/gsm-sms.md @@ -21,7 +21,7 @@ The GSM endpoints proxy SMS operations from the SAPOT server to the GSM module ( | POST | `/gsm/resend` | None | Resend an SMS OTP. | | GET | `/gsm/phone-is-verified` | JWT Bearer | Check whether the current user's phone number is verified. | | POST | `/gsm/migrate-phone-user` | JWT Bearer | Migrate a guest user's data onto a phone-registered account. | -| POST | `/gsm/contact-unknown-user` | JWT Bearer | Send an onboarding SMS to a phone number not yet registered. | +| POST | `/gsm/contact-unknown-user` | JWT Bearer + verified phone | Send an onboarding SMS to a phone number not yet registered. | | GET | `/gsm/mock/health` | JWT Bearer | Mock variant of `/gsm/health`. | | GET | `/gsm/mock/health/detailed` | JWT Bearer | Mock variant of `/gsm/health/detailed`. | | GET | `/gsm/mock/sms/messages` | JWT Bearer | Mock variant of `/gsm/sms/messages`. | @@ -45,7 +45,7 @@ The GSM module's own standalone hardware-facing API is documented in [`docs/depl ## Outbound sender eligibility -`POST /gsm/sms/send` and its mock variant require a `PhoneVerified` record for the authenticated account. The server checks this before looking up the recipient or contacting the GSM gateway. An unverified account receives HTTP 403: +`POST /gsm/sms/send` and its mock variant require a `PhoneVerified` record for the authenticated account. The production send route also requires the recipient to be phone-verified. The server checks eligibility before contacting the GSM gateway. An unverified account receives HTTP 403: ```json { @@ -72,4 +72,6 @@ The main server preserves synchronous gateway failures for `/gsm/sms/send`, `/gs An unreachable gateway also returns HTTP 503 with `reason: "GATEWAY_UNAVAILABLE"`. Clients should keep rejected messages available for manual retry and should not report verification or onboarding SMS as sent. +The server rate-limits paid SMS routes. The gateway independently enforces a daily send ceiling, sender limits, sender-target limits, and per-response cooldowns, so a client cannot bypass these controls by calling a different paid-send route. + See [gsm-sms.yaml](openapi/gsm-sms.yaml) for exact field-level request/response schemas, or the live server's `/docs` / `/openapi.json`. diff --git a/docs/deployment/environment-config.md b/docs/deployment/environment-config.md index 61521a69..7e0a687d 100644 --- a/docs/deployment/environment-config.md +++ b/docs/deployment/environment-config.md @@ -64,6 +64,14 @@ GSM_GATEWAY_URL=http://127.0.0.1:8001 | `GSM_SECRET` | None; startup raises `RuntimeError` when unset | Shared secret validated for server calls to `/sms/send` and sent by the GSM module on `/gsm/inbound` callbacks. **Must match the server's `GSM_SECRET`** (see above) | | `SMS_BOT_USER_ID` | unset | User ID the GSM module attributes inbound SMS-originated messages to, when the sender can't be resolved to a registered user (`database.py`) | | `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum waiting outbound requests. Integers from `1` through `20` are accepted; other values fail startup. The upper bound leaves capacity in FastAPI's default 40-thread worker pool so overload requests can reach the non-blocking admission check. | +| `SMS_INCOMING_QUEUE_MAXSIZE` | `100` | Maximum inbound SMS events waiting for relay processing. Excess events are dropped and reported by `/health/detailed`. | +| `SMS_DAILY_SEND_LIMIT` | `100` | Gateway-wide daily outbound SMS ceiling. Set to `0` as an immediate carrier-spend kill switch. | +| `SMS_SENDER_DAILY_LIMIT` | `20` | Daily cap on replies generated for one inbound sender. | +| `SMS_SENDER_TARGET_DAILY_LIMIT` | `10` | Daily cap on relay attempts for one sender-target pair. | +| `SMS_RESPONSE_COOLDOWN_SECONDS` | `30` | Minimum spacing between responses in the same category for one sender. | +| `SMS_LOG_RETENTION_DAYS` | `30` | Number of days to retain redacted operational SMS logs. Set to `0` to purge logs on service start. | +| `LOG_MAX_BYTES` | `1000000` | Maximum bytes in one gateway log file before rotation. | +| `LOG_BACKUP_COUNT` | `3` | Number of rotated gateway log files retained. | ### Recommended production `gsm.env` @@ -78,6 +86,14 @@ SAPOT_API_URL=http://127.0.0.1:8000 GSM_SECRET= SMS_BOT_USER_ID= SMS_SEND_QUEUE_MAXSIZE=10 +SMS_INCOMING_QUEUE_MAXSIZE=100 +SMS_DAILY_SEND_LIMIT=100 +SMS_SENDER_DAILY_LIMIT=20 +SMS_SENDER_TARGET_DAILY_LIMIT=10 +SMS_RESPONSE_COOLDOWN_SECONDS=30 +SMS_LOG_RETENTION_DAYS=30 +LOG_MAX_BYTES=1000000 +LOG_BACKUP_COUNT=3 ``` --- diff --git a/docs/features/sms-gateway/design.md b/docs/features/sms-gateway/design.md index 020a885e..715ed7b0 100644 --- a/docs/features/sms-gateway/design.md +++ b/docs/features/sms-gateway/design.md @@ -33,7 +33,7 @@ sequenceDiagram GSM->>Main: POST /gsm/inbound with X-GSM-Secret ``` -The main server authenticates the user-facing `/gsm/sms/send` route with a JSON Web Token (JWT) and requires the sender to have a verified phone number. It rejects an unverified sender before contacting the gateway. The main server authenticates its direct gateway call with the same shared `GSM_SECRET` used for callbacks. The GSM service validates `X-GSM-Secret` before logging or queueing a send, which prevents another container on the internal network from occupying the serial modem. +The main server authenticates the user-facing `/gsm/sms/send` route with a JSON Web Token (JWT) and requires both sender and recipient to have verified phone numbers. It rejects ineligible requests before contacting the gateway. The main server authenticates its direct gateway call with the same shared `GSM_SECRET` used for callbacks. The GSM service validates `X-GSM-Secret` on every endpoint except the liveness-only `/health` route, which prevents another container on the internal network from reading SMS data, resetting sessions, or occupying the serial modem. ## How does outbound admission work? @@ -95,7 +95,7 @@ Message bodies may contain pipe characters. `parse_line()` preserves them for `S ## How are inbound messages handled? -The reader places `SMS_RECEIVED` events on `incoming_queue`. The API lifespan task passes each event to `handle_incoming_sms()`, which applies registration, ban, phone-verification, session, and target checks. An unregistered number receives a registration warning on its first message. The gateway records the warning only after the modem confirms that reply was sent. It ignores later messages from that number, preventing repeat reply charges. A separate relay-only record preserves this behavior when an operator resets the number's normal relay session. +The reader places `SMS_RECEIVED` events on a bounded `incoming_queue`. When full, it drops the new event, increments an overflow counter exposed through `/health/detailed`, and continues reading the modem. The API lifespan task passes each accepted event to `handle_incoming_sms()`, which normalizes Philippine mobile numbers and applies registration, ban, phone-verification, session, target, opt-out, and sender-target quota checks. The API also applies sender-wide and response-category cooldowns before it sends a reply. Gateway logs retain only redacted message metadata, rotate by size, and are purged after the configured retention period. The handler can return a reply to the sender and a forwarded message for the selected target. Both use the same bounded outbound queue. `database.notify_app()` also calls the main server's `POST /gsm/inbound` route with `X-GSM-Secret`. diff --git a/server/app/api/gsm.py b/server/app/api/gsm.py index 892933e7..0842e7e6 100644 --- a/server/app/api/gsm.py +++ b/server/app/api/gsm.py @@ -13,6 +13,7 @@ from app.db_operations.auth import SessionDep from app.db_operations.connection_manager import manager from app.db_operations.token import get_current_user, get_current_user_admin, get_current_user_rescuer, verify_reauth_token +from app.limiter import limiter from app.models.conversation import Conversation, ConversationParticipant, ConversationType from app.models.guest import Guest from app.models.message import Message, MessageType @@ -71,7 +72,9 @@ def _gsm_log_extra(current_user: User | None, path: str) -> dict: async def _get_gsm_health(path: str, current_user: User) -> dict | JSONResponse: try: - response = await _get_gsm_client().get(path) + response = await _get_gsm_client().get( + path, headers={"X-GSM-Secret": GSM_SECRET} + ) except httpx.RequestError as exc: logger.warning( "GSM gateway health check unavailable: %s", @@ -305,7 +308,9 @@ async def gsm_messages( params["phone"] = phone client = _get_gsm_client() - response = await client.get("/sms/messages", params=params) + response = await client.get( + "/sms/messages", params=params, headers={"X-GSM-Secret": GSM_SECRET} + ) return response.json() @@ -321,7 +326,9 @@ def _require_verified_phone(user: User) -> None: @router.post("/sms/send", responses=GSM_SMS_SEND_ERROR_RESPONSES) +@limiter.limit("10/minute") async def send_sms( + request: Request, current_user : Annotated[User, Depends(get_current_user)], user_id: UUID, message: str, @@ -339,6 +346,14 @@ async def send_sms( return { "detail": { "msg": "This user does not exist." }} if not target.phone_number: return { "detail": { "msg": "This user does not have a phone number attached to his/her account." }} + if not target.phone_is_verified: + raise HTTPException( + status_code=403, + detail={ + "reason": "TARGET_PHONE_VERIFICATION_REQUIRED", + "message": "The recipient must verify their phone number before receiving SMS.", + }, + ) return await sendToModule(target.phone_number, message) @@ -365,6 +380,7 @@ async def sendToModule(phone_number: str, message: str): return payload @router.post("/request", responses=GSM_SEND_ERROR_RESPONSES) +@limiter.limit("3/minute") async def request_phone_verification( data: RequestPhoneVerification, request: Request, @@ -501,7 +517,9 @@ def verify_phone_code( # ============================================================================= @router.post("/resend", responses=GSM_SEND_ERROR_RESPONSES) +@limiter.limit("3/minute") async def resend_phone_code( + request: Request, current_user : Annotated[User, Depends(get_current_user)], session: SessionDep ): @@ -673,7 +691,9 @@ def sms_conversation_id(user_id_a: str, user_id_b: str) -> str: @router.post("/contact-unknown-user", responses=GSM_SEND_ERROR_RESPONSES) +@limiter.limit("3/minute") async def contact_unknown_user( + request: Request, current_user : Annotated[User, Depends(get_current_user)], target_phone_number: Annotated[str, Query(pattern=r"^\+639\d{9}$")], session: SessionDep, @@ -684,6 +704,10 @@ async def contact_unknown_user( the conversation between an authenticated user and a user without an account. This does not send any message to the target user. """ + if current_user.banned: + raise HTTPException(403) + _require_verified_phone(current_user) + registered_user = session.exec(select(User).where(User.phone_number == target_phone_number)).first() if registered_user: diff --git a/server/app/tests/test_gsm_health.py b/server/app/tests/test_gsm_health.py index 45d6cf11..afe6e30c 100644 --- a/server/app/tests/test_gsm_health.py +++ b/server/app/tests/test_gsm_health.py @@ -8,12 +8,12 @@ class UnavailableGsmClient: - async def get(self, path: str): + async def get(self, path: str, **_kwargs): raise httpx.ConnectError("All connection attempts failed") class DegradedGsmClient: - async def get(self, path: str): + async def get(self, path: str, **_kwargs): return httpx.Response( 503, json={ diff --git a/server/app/tests/test_gsm_proxy.py b/server/app/tests/test_gsm_proxy.py index caddce40..0e8c60ff 100644 --- a/server/app/tests/test_gsm_proxy.py +++ b/server/app/tests/test_gsm_proxy.py @@ -74,6 +74,13 @@ def _authenticated_user(session, *, phone_verified=False): return user +def _verified_target(session, current_user): + target = session.exec(select(User).where(User.id != current_user.id)).first() + session.add(PhoneVerified(user_id=target.id)) + session.commit() + return target + + def test_proxy_capacity_timeouts_and_gateway_url_cover_gateway_contract(monkeypatch): captured = {} @@ -136,7 +143,7 @@ async def post(self, path: str, **kwargs): def test_send_sms_preserves_queue_full_status(client, session, monkeypatch): current_user = _authenticated_user(session, phone_verified=True) - target = session.exec(select(User).where(User.id != current_user.id)).first() + target = _verified_target(session, current_user) monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SaturatedGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) @@ -151,7 +158,7 @@ def test_send_sms_preserves_queue_full_status(client, session, monkeypatch): def test_send_sms_preserves_service_stopping_status(client, session, monkeypatch): current_user = _authenticated_user(session, phone_verified=True) - target = session.exec(select(User).where(User.id != current_user.id)).first() + target = _verified_target(session, current_user) monkeypatch.setattr(gsm, "_get_gsm_client", lambda: StoppingGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) @@ -201,7 +208,7 @@ def test_phone_verification_resend_preserves_queue_full_status( def test_contact_unknown_user_preserves_queue_full_status(client, session, monkeypatch): - current_user = _authenticated_user(session) + current_user = _authenticated_user(session, phone_verified=True) monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SaturatedGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) @@ -216,7 +223,7 @@ def test_contact_unknown_user_preserves_queue_full_status(client, session, monke def test_send_sms_reports_unavailable_gateway(client, session, monkeypatch): current_user = _authenticated_user(session, phone_verified=True) - target = session.exec(select(User).where(User.id != current_user.id)).first() + target = _verified_target(session, current_user) monkeypatch.setattr(gsm, "_get_gsm_client", lambda: UnavailableGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) @@ -236,7 +243,7 @@ def test_send_sms_reports_unavailable_gateway(client, session, monkeypatch): def test_send_sms_preserves_modem_not_ready_status(client, session, monkeypatch): current_user = _authenticated_user(session, phone_verified=True) - target = session.exec(select(User).where(User.id != current_user.id)).first() + target = _verified_target(session, current_user) monkeypatch.setattr(gsm, "_get_gsm_client", lambda: ModemNotReadyGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) @@ -253,7 +260,7 @@ def test_send_sms_rejects_proxy_pool_exhaustion_without_gateway_send( client, session, monkeypatch ): current_user = _authenticated_user(session, phone_verified=True) - target = session.exec(select(User).where(User.id != current_user.id)).first() + target = _verified_target(session, current_user) monkeypatch.setattr(gsm, "_get_gsm_client", lambda: PoolExhaustedGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) From 937d95dc63986377d59803d73fb4d81c8a06bfce Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 13:50:41 +0800 Subject: [PATCH 03/10] feat(gsm-sms): add sms_outbound_permission table and helpers. --- GSM-module/GSM-fastapi/database.py | 51 +++++++++++++++++++ .../GSM-fastapi/tests/test_incoming_sms.py | 46 +++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/GSM-module/GSM-fastapi/database.py b/GSM-module/GSM-fastapi/database.py index 934be84c..ecced590 100644 --- a/GSM-module/GSM-fastapi/database.py +++ b/GSM-module/GSM-fastapi/database.py @@ -84,6 +84,7 @@ def init(db_url: str): SmsUnregisteredWarning.__table__, SmsRateCounter.__table__, SmsRecipientPreference.__table__, + SmsOutboundPermission.__table__, ]) logger.info("Database ready: %s", db_url.split("@")[-1]) # hide credentials @@ -410,6 +411,16 @@ class SmsRecipientPreference(Base): opted_out = Column(Boolean, default=False, nullable=False) updated_at = Column(BigInteger, default=_now_ms, nullable=False) + +class SmsOutboundPermission(Base): + __tablename__ = "sms_outbound_permission" + + # sapot_phone: the verified SAPOT user who sent the outbound SMS + # external_phone: the non-SAPOT number they contacted + sapot_phone = Column(String(20), primary_key=True, nullable=False) + external_phone = Column(String(20), primary_key=True, nullable=False) + granted_at = Column(BigInteger, default=_now_ms, nullable=False) + # ============================================================================= # USER LOOKUPS (reads from shared `user` table) # ============================================================================= @@ -704,6 +715,46 @@ def is_sms_opted_out(phone: str) -> bool: return bool(row and row.opted_out) +def grant_outbound_permission(sapot_phone: str, external_phone: str) -> None: + """Record that sapot_phone has sent an outbound SMS to external_phone. + + Safe to call multiple times -- upserts on the composite primary key. + """ + with new_get_session() as s: + existing = s.execute( + select(SmsOutboundPermission) + .where(SmsOutboundPermission.sapot_phone == sapot_phone) + .where(SmsOutboundPermission.external_phone == external_phone) + ).scalar_one_or_none() + if existing is None: + s.add(SmsOutboundPermission( + sapot_phone=sapot_phone, + external_phone=external_phone, + )) + s.commit() + + +def has_outbound_permission(sapot_phone: str, external_phone: str) -> bool: + """True iff sapot_phone previously sent an outbound SMS to external_phone.""" + with new_get_session() as s: + row = s.execute( + select(SmsOutboundPermission) + .where(SmsOutboundPermission.sapot_phone == sapot_phone) + .where(SmsOutboundPermission.external_phone == external_phone) + ).scalar_one_or_none() + return row is not None + + +def get_permitted_contacts(external_phone: str) -> list: + """Return the sapot_phone values that have permission to receive from external_phone.""" + with new_get_session() as s: + rows = s.execute( + select(SmsOutboundPermission) + .where(SmsOutboundPermission.external_phone == external_phone) + ).scalars().all() + return [r.sapot_phone for r in rows] + + # ============================================================================= # APP FORWARD — write message into shared Conversation + Message tables # so the forwarded SMS appears inside the SAPOT app natively diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py index e375b187..84634c9c 100644 --- a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -109,3 +109,49 @@ def test_process_incoming_persists_rejection_reason(monkeypatch): ) assert updates == [("message-id", "rejected", "BANNED_SENDER")] + + +import database as _db + + +def _init_test_db(): + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + engine = create_engine("sqlite://", connect_args={"check_same_thread": False}) + _db.Base.metadata.create_all(engine) + _db._Session = sessionmaker(bind=engine, expire_on_commit=False) + _db._engine = engine + + +def test_grant_and_check_outbound_permission(): + _init_test_db() + sapot = "+639171111111" + external = "+639288888888" + assert _db.has_outbound_permission(sapot, external) is False + _db.grant_outbound_permission(sapot, external) + assert _db.has_outbound_permission(sapot, external) is True + + +def test_grant_is_idempotent(): + _init_test_db() + sapot = "+639171111111" + external = "+639288888888" + _db.grant_outbound_permission(sapot, external) + _db.grant_outbound_permission(sapot, external) # must not raise + assert _db.has_outbound_permission(sapot, external) is True + + +def test_get_permitted_contacts_returns_correct_set(): + _init_test_db() + external = "+639288888888" + _db.grant_outbound_permission("+639171111111", external) + _db.grant_outbound_permission("+639172222222", external) + contacts = _db.get_permitted_contacts(external) + assert set(contacts) == {"+639171111111", "+639172222222"} + + +def test_get_permitted_contacts_empty_when_none(): + _init_test_db() + contacts = _db.get_permitted_contacts("+639299999999") + assert contacts == [] + From 1d6db8147c10cb3f0ec7e13e206fd638daf3240a Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 13:57:10 +0800 Subject: [PATCH 04/10] feat(server-gsm): grant relay permission after confirmed outbound send. --- server/app/api/gsm.py | 41 +++++++++++++++++-- server/app/tests/test_gsm_proxy.py | 66 ++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/server/app/api/gsm.py b/server/app/api/gsm.py index 0842e7e6..5d704803 100644 --- a/server/app/api/gsm.py +++ b/server/app/api/gsm.py @@ -325,6 +325,26 @@ def _require_verified_phone(user: User) -> None: ) +async def _grant_outbound_permission(sapot_phone: str, external_phone: str) -> None: + """Notify the GSM gateway to record the outbound permission. + + Fire-and-forget -- a failure is logged but does not fail the SMS response. + """ + try: + client = _get_gsm_client() + r = await client.post( + "/grant-permission", + json={"sapot_phone": sapot_phone, "external_phone": external_phone}, + headers={"X-GSM-Secret": GSM_SECRET}, + ) + if r.status_code != 200: + logger.warning( + "grant_outbound_permission: gateway returned %s", r.status_code + ) + except Exception as exc: + logger.warning("grant_outbound_permission failed: %s", exc) + + @router.post("/sms/send", responses=GSM_SMS_SEND_ERROR_RESPONSES) @limiter.limit("10/minute") async def send_sms( @@ -356,15 +376,22 @@ async def send_sms( ) - return await sendToModule(target.phone_number, message) + result = await sendToModule(target.phone_number, message, current_user.phone_number) + if current_user.phone_number and target.phone_number: + await _grant_outbound_permission( + sapot_phone=current_user.phone_number, + external_phone=target.phone_number, + ) + return result -async def sendToModule(phone_number: str, message: str): +async def sendToModule(phone_number: str, message: str, sender_phone: str | None = None): + from_number = sender_phone if sender_phone else phone_number client = _get_gsm_client() try: response = await client.post( "/sms/send", - json={"number": phone_number, "body": f"FROM {phone_number}: " + message}, + json={"number": phone_number, "body": f"FROM {from_number}: " + message}, headers={"X-GSM-Secret": GSM_SECRET}, ) except httpx.RequestError as exc: @@ -841,7 +868,13 @@ async def MOCK_send_sms( return { "detail": { "msg": "This user does not have a phone number attached to his/her account." }} - return await MOCK_sendToModule(target.phone_number, message) + result = await MOCK_sendToModule(target.phone_number, message) + if current_user.phone_number and target.phone_number: + await _grant_outbound_permission( + sapot_phone=current_user.phone_number, + external_phone=target.phone_number, + ) + return result diff --git a/server/app/tests/test_gsm_proxy.py b/server/app/tests/test_gsm_proxy.py index 0e8c60ff..a16270d2 100644 --- a/server/app/tests/test_gsm_proxy.py +++ b/server/app/tests/test_gsm_proxy.py @@ -316,3 +316,69 @@ def test_mock_send_sms_rejects_unverified_sender(client, session, monkeypatch): assert response.status_code == 403 assert response.json()["detail"]["reason"] == "PHONE_VERIFICATION_REQUIRED" + + +def test_successful_send_sms_grants_outbound_permission(client, session, monkeypatch): + """After a successful send, the server calls /grant-permission on the gateway.""" + current_user = _authenticated_user(session, phone_verified=True) + target = _verified_target(session, current_user) + current_user.phone_number = "+639171111111" + target.phone_number = "+639172222222" + session.commit() + + grant_calls = [] + + class SuccessGsmClient: + async def post(self, path: str, json: dict = None, **kwargs): + if path == "/sms/send": + return FakeGsmResponse(200, {"ok": True, "msg_id": "abc"}) + if path == "/grant-permission": + grant_calls.append(json) + return FakeGsmResponse(200, {"ok": True}) + return FakeGsmResponse(404, {}) + + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SuccessGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Hello"}, + ) + + assert response.status_code == 200 + assert len(grant_calls) == 1 + assert grant_calls[0]["sapot_phone"] == "+639171111111" + assert grant_calls[0]["external_phone"] == "+639172222222" + + +def test_failed_send_sms_does_not_grant_permission(client, session, monkeypatch): + """A failed send must NOT call /grant-permission.""" + current_user = _authenticated_user(session, phone_verified=True) + target = _verified_target(session, current_user) + current_user.phone_number = "+639171111111" + target.phone_number = "+639172222222" + session.commit() + + grant_calls = [] + + class FailGsmClient: + async def post(self, path: str, json: dict = None, **kwargs): + if path == "/sms/send": + return FakeGsmResponse(502, { + "detail": {"message": "Delivery failed", "reason": "MODEM_ERROR", "msg_id": "x"} + }) + if path == "/grant-permission": + grant_calls.append(json) + return FakeGsmResponse(200, {"ok": True}) + + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: FailGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Hello"}, + ) + + assert response.status_code == 502 + assert len(grant_calls) == 0 + From a06107949589cfaf81bedef7f021befefc9e2c5c Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 14:01:15 +0800 Subject: [PATCH 05/10] feat(gsm-sms): add /grant-permission endpoint. --- GSM-module/GSM-fastapi/api.py | 26 +++++++++++++ .../GSM-fastapi/tests/test_incoming_sms.py | 37 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index f40a6148..413b86ce 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -249,6 +249,18 @@ def validate_body(cls, v): return v.strip() +class GrantPermissionRequest(BaseModel): + sapot_phone: str + external_phone: str + + @field_validator("sapot_phone", "external_phone") + @classmethod + def validate_phone(cls, v): + if not re.match(r"^\+\d{7,15}$", v): + raise ValueError("phone must be E.164 format") + return v + + class AddUserRequest(BaseModel): phone: str username: str @@ -471,6 +483,20 @@ def list_messages( return database.get_messages(limit=limit, direction=direction, phone=phone) +@app.post( + "/grant-permission", + tags=["permissions"], + dependencies=[Depends(require_gsm_secret)], +) +def grant_permission(req: GrantPermissionRequest): + """Record that a SAPOT user sent an outbound SMS to an external number. + + Called by the main server after a confirmed /sms/send delivery. + """ + database.grant_outbound_permission(req.sapot_phone, req.external_phone) + return {"ok": True} + + # ── User endpoints ──────────────────────────────────────────────────────────── @app.get("/users", tags=["users"], dependencies=[Depends(require_gsm_secret)]) diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py index 84634c9c..8a0f60c7 100644 --- a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -155,3 +155,40 @@ def test_get_permitted_contacts_empty_when_none(): contacts = _db.get_permitted_contacts("+639299999999") assert contacts == [] + +from fastapi.testclient import TestClient + + +def test_grant_permission_endpoint_stores_permission(monkeypatch): + granted = [] + monkeypatch.setattr( + "database.grant_outbound_permission", + lambda sapot, external: granted.append((sapot, external)) + ) + from api import app as gsm_app + client = TestClient(gsm_app) + + resp = client.post( + "/grant-permission", + json={"sapot_phone": "+639171111111", "external_phone": "+639288888888"}, + headers={"X-GSM-Secret": "test-gsm-secret"}, + ) + + assert resp.status_code == 200 + assert resp.json() == {"ok": True} + assert granted == [("+639171111111", "+639288888888")] + + +def test_grant_permission_endpoint_rejects_wrong_secret(monkeypatch): + from api import app as gsm_app + client = TestClient(gsm_app) + + resp = client.post( + "/grant-permission", + json={"sapot_phone": "+639171111111", "external_phone": "+639288888888"}, + headers={"X-GSM-Secret": "wrong-secret"}, + ) + + assert resp.status_code == 401 + + From cea744a3d5f555454c4f927339de8de170310ea5 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 14:08:31 +0800 Subject: [PATCH 06/10] feat(gsm-sms): enforce outbound permission before allowing target selection. --- GSM-module/GSM-fastapi/sms_handler.py | 33 ++++++++ .../GSM-fastapi/tests/test_incoming_sms.py | 83 +++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/GSM-module/GSM-fastapi/sms_handler.py b/GSM-module/GSM-fastapi/sms_handler.py index c2f915ff..3fa7ab27 100644 --- a/GSM-module/GSM-fastapi/sms_handler.py +++ b/GSM-module/GSM-fastapi/sms_handler.py @@ -44,6 +44,16 @@ MSG_NO_ARG = "Provide a number: [target] +639XXXXXXXXX" # 40 chars MSG_FORWARD_FAIL = "Could not forward your message. Please try again." # 50 +MSG_NOT_PERMITTED = ( + "No SAPOT user has contacted this number. " + "Ask them to message you first." +) # 72 chars + +MSG_TARGET_NOT_PERMITTED = ( + "That number has not contacted you. " + "Only permitted SAPOT contacts can be targeted." +) # 82 chars + def _msg_target_set(username: str, phone: str) -> str: return f"Target: {username} ({phone}). Messages go to them now." # e.g. "Target: maria_santos (+639281234567). Messages go to them now." = 63 @@ -127,6 +137,25 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: return _cmd_set_target(number, body) if stage == "NEW": + permitted = database.get_permitted_contacts(number) + if not permitted: + return MSG_NOT_PERMITTED, None, None, "NOT_PERMITTED" + if len(permitted) == 1: + target = database.lookup_number(permitted[0]) + if target: + database.update_session( + number, + stage="ACTIVE", + target_phone=permitted[0], + target_username=target["username"], + ) + return ( + f"Welcome. Routing to {target['username']}. " + "Send any message to reach them.", + None, + None, + "WELCOME", + ) database.update_session(number, stage="AWAITING_TARGET") return MSG_WELCOME, None, None, "WELCOME" @@ -170,6 +199,10 @@ def _cmd_set_target(number: str, body: str) -> ForwardTuple: if target_phone == number: return "You cannot set yourself as the target.", None, None, "SELF_TARGET" + permitted = database.get_permitted_contacts(number) + if target_phone not in permitted: + return MSG_TARGET_NOT_PERMITTED, None, None, "TARGET_NOT_PERMITTED" + target = database.lookup_number(target_phone) if target is None: return MSG_TARGET_NOT_FOUND, None, None, "TARGET_NOT_FOUND" diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py index 8a0f60c7..47ada2d9 100644 --- a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -83,6 +83,7 @@ def test_process_incoming_marks_warning_only_after_successful_reply( "mark_unregistered_warning", lambda number: marked_numbers.append(number), ) + monkeypatch.setattr(api.database, "allow_inbound_response", lambda *args: True) api._process_incoming(SimpleNamespace(number="+639171234567", body="help")) @@ -192,3 +193,85 @@ def test_grant_permission_endpoint_rejects_wrong_secret(monkeypatch): assert resp.status_code == 401 +REGISTERED_USER = { + "id": "abc123", + "phone": "+639172222222", + "username": "maria", + "first_name": "Maria", + "last_name": "Santos", + "email": "m@s.com", + "app_active": True, +} +SENDER_VERIFIED = {"banned": False, "phone_is_verified": True} + + +def test_new_session_with_no_permitted_contacts_is_rejected(monkeypatch): + """An external number with zero outbound permissions must get NOT_PERMITTED.""" + monkeypatch.setattr(sms_handler.database, "get_user_by_phone", lambda _: SENDER_VERIFIED) + monkeypatch.setattr(sms_handler.database, "lookup_number", lambda p: REGISTERED_USER if p in ("+639172222222", "+639171234567") else None) + monkeypatch.setattr(sms_handler.database, "get_permitted_contacts", lambda _: []) + monkeypatch.setattr(sms_handler.database, "get_session", lambda _: {"stage": "NEW", "target_phone": None, "target_username": None}) + monkeypatch.setattr(sms_handler.database, "update_session", lambda *_a, **_kw: None) + monkeypatch.setattr(sms_handler.database, "has_unregistered_warning", lambda _: False) + + reply, fwd_num, fwd_body, reason = sms_handler.handle_incoming_sms( + "+639171234567", "Hello" + ) + + assert reason == "NOT_PERMITTED" + assert fwd_num is None + + +def test_new_session_with_exactly_one_permitted_contact_auto_routes(monkeypatch): + """When there is exactly one permitted contact, the session goes ACTIVE immediately.""" + permitted = ["+639172222222"] + monkeypatch.setattr(sms_handler.database, "get_user_by_phone", lambda _: SENDER_VERIFIED) + monkeypatch.setattr(sms_handler.database, "lookup_number", lambda p: REGISTERED_USER if p in ("+639172222222", "+639171234567") else None) + monkeypatch.setattr(sms_handler.database, "get_permitted_contacts", lambda _: permitted) + session_updates = [] + monkeypatch.setattr(sms_handler.database, "get_session", lambda _: {"stage": "NEW", "target_phone": None, "target_username": None}) + monkeypatch.setattr(sms_handler.database, "update_session", lambda _phone, **kw: session_updates.append(kw)) + monkeypatch.setattr(sms_handler.database, "has_unregistered_warning", lambda _: False) + + reply, fwd_num, fwd_body, reason = sms_handler.handle_incoming_sms( + "+639171234567", "Hello" + ) + + assert reason == "WELCOME" + assert any(u.get("stage") == "ACTIVE" for u in session_updates) + assert any(u.get("target_phone") == "+639172222222" for u in session_updates) + + +def test_set_target_blocked_when_not_in_permitted_set(monkeypatch): + """[target] with a phone not in permitted contacts returns TARGET_NOT_PERMITTED.""" + permitted = ["+639172222222"] + monkeypatch.setattr(sms_handler.database, "get_user_by_phone", lambda _: SENDER_VERIFIED) + monkeypatch.setattr(sms_handler.database, "lookup_number", lambda p: REGISTERED_USER if p in ("+639171234567", "+639179999999") else None) + monkeypatch.setattr(sms_handler.database, "get_permitted_contacts", lambda _: permitted) + monkeypatch.setattr(sms_handler.database, "get_session", lambda _: {"stage": "AWAITING_TARGET", "target_phone": None, "target_username": None}) + monkeypatch.setattr(sms_handler.database, "update_session", lambda *_a, **_kw: None) + monkeypatch.setattr(sms_handler.database, "has_unregistered_warning", lambda _: False) + + reply, fwd_num, fwd_body, reason = sms_handler.handle_incoming_sms( + "+639171234567", "[target] +639179999999" + ) + + assert reason == "TARGET_NOT_PERMITTED" + assert fwd_num is None + + +def test_set_target_allowed_when_in_permitted_set(monkeypatch): + """[target] with a phone in the permitted set succeeds.""" + permitted = ["+639172222222"] + monkeypatch.setattr(sms_handler.database, "get_user_by_phone", lambda _: SENDER_VERIFIED) + monkeypatch.setattr(sms_handler.database, "lookup_number", lambda p: REGISTERED_USER if p in ("+639172222222", "+639171234567") else None) + monkeypatch.setattr(sms_handler.database, "get_permitted_contacts", lambda _: permitted) + monkeypatch.setattr(sms_handler.database, "get_session", lambda _: {"stage": "AWAITING_TARGET", "target_phone": None, "target_username": None}) + monkeypatch.setattr(sms_handler.database, "update_session", lambda *_a, **_kw: None) + monkeypatch.setattr(sms_handler.database, "has_unregistered_warning", lambda _: False) + + reply, fwd_num, fwd_body, reason = sms_handler.handle_incoming_sms( + "+639171234567", "[target] +639172222222" + ) + + assert reason == "TARGET_SET" From 6330485715b7e10fd16e5eab3f5088be37626adc Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 14:14:34 +0800 Subject: [PATCH 07/10] feat(server-gsm): enforce outbound permission at /gsm/inbound and add /has-permission. --- GSM-module/GSM-fastapi/api.py | 13 ++++ .../GSM-fastapi/tests/test_incoming_sms.py | 35 +++++++++++ server/app/api/gsm.py | 36 ++++++++--- server/app/tests/test_gsm_proxy.py | 59 +++++++++++++++++++ 4 files changed, 136 insertions(+), 7 deletions(-) diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index 413b86ce..f7549502 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -497,6 +497,19 @@ def grant_permission(req: GrantPermissionRequest): return {"ok": True} +@app.get( + "/has-permission", + tags=["permissions"], + dependencies=[Depends(require_gsm_secret)], +) +def check_permission( + sapot_phone: str = Query(..., pattern=r"^\+\d{7,15}$"), + external_phone: str = Query(..., pattern=r"^\+\d{7,15}$"), +): + """Return whether sapot_phone has an active outbound permission for external_phone.""" + return {"permitted": database.has_outbound_permission(sapot_phone, external_phone)} + + # ── User endpoints ──────────────────────────────────────────────────────────── @app.get("/users", tags=["users"], dependencies=[Depends(require_gsm_secret)]) diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py index 47ada2d9..d0bf426e 100644 --- a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -275,3 +275,38 @@ def test_set_target_allowed_when_in_permitted_set(monkeypatch): ) assert reason == "TARGET_SET" + + +def test_has_permission_endpoint_returns_true_when_granted(monkeypatch): + monkeypatch.setattr( + "database.has_outbound_permission", + lambda sapot, external: sapot == "+639171111111" and external == "+639288888888" + ) + from api import app as gsm_app + from fastapi.testclient import TestClient + client = TestClient(gsm_app) + + resp = client.get( + "/has-permission", + params={"sapot_phone": "+639171111111", "external_phone": "+639288888888"}, + headers={"X-GSM-Secret": "test-gsm-secret"}, + ) + + assert resp.status_code == 200 + assert resp.json() == {"permitted": True} + + +def test_has_permission_endpoint_returns_false_when_not_granted(monkeypatch): + monkeypatch.setattr("database.has_outbound_permission", lambda *_: False) + from api import app as gsm_app + from fastapi.testclient import TestClient + client = TestClient(gsm_app) + + resp = client.get( + "/has-permission", + params={"sapot_phone": "+639171111111", "external_phone": "+639299999999"}, + headers={"X-GSM-Secret": "test-gsm-secret"}, + ) + + assert resp.status_code == 200 + assert resp.json() == {"permitted": False} diff --git a/server/app/api/gsm.py b/server/app/api/gsm.py index 5d704803..6179fc01 100644 --- a/server/app/api/gsm.py +++ b/server/app/api/gsm.py @@ -190,7 +190,12 @@ async def inbound_sms( request: Request, session: SessionDep, ): - """Internal endpoint: receives an inbound SMS from the GSM-API and delivers it to the target user via WebSocket.""" + """Internal endpoint: receives an inbound SMS from the GSM-API and delivers it + to the target user via WebSocket. + + Enforces the outbound-permission rule: the sender-target pair is only accepted + if the target previously sent an outbound SMS to the sender via the relay. + """ if not _gsm_secret_ok(request): raise HTTPException(403) @@ -200,7 +205,28 @@ async def inbound_sms( if not sender or not target: raise HTTPException(404, "User not found") - # Look up the existing conversation where both users are participants + # Enforce authorization: target must have previously contacted sender via relay + try: + perm_resp = await _get_gsm_client().get( + "/has-permission", + params={ + "sapot_phone": payload.target_phone, + "external_phone": payload.sender_phone, + }, + headers={"X-GSM-Secret": GSM_SECRET}, + ) + if perm_resp.status_code != 200 or not perm_resp.json().get("permitted"): + logger.warning( + "inbound_sms: unauthorized pair sender=*** target=***" + ) + raise HTTPException(403, "Sender is not authorized to contact this target") + except HTTPException: + raise + except Exception as exc: + logger.error("inbound_sms: permission check failed: %s", exc) + raise HTTPException(502, "Permission check failed") from exc + + # Look up or create the shared conversation conversation = session.exec( select(Conversation) .join(ConversationParticipant, Conversation.id == ConversationParticipant.conversation_id) @@ -212,15 +238,12 @@ async def inbound_sms( ) ) ).first() - print("senderid", sender.id) - print("targetId", target.id) if not conversation: convo_id = UUID(sms_conversation_id(str(sender.id), str(target.id))) conversation = _create_sms_conversation(session, convo_id, sender.id, target.id) convo_id = conversation.id - print("convoid", convo_id) msg = Message( conversation_id=convo_id, @@ -229,7 +252,7 @@ async def inbound_sms( message_type=MessageType.sms, ) session.add(msg) - session.flush() # get msg.id before commit + session.flush() receipt = MessageReceipt( message_id=msg.id, @@ -241,7 +264,6 @@ async def inbound_sms( session.refresh(msg) is_connected = await manager.is_user_connected(target.id) - print(f"[gsm/inbound] sender={sender.username} target={target.username} connected={is_connected} msg_id={msg.id}") ws_payload = { "type": "chat", diff --git a/server/app/tests/test_gsm_proxy.py b/server/app/tests/test_gsm_proxy.py index a16270d2..0d4c1c27 100644 --- a/server/app/tests/test_gsm_proxy.py +++ b/server/app/tests/test_gsm_proxy.py @@ -382,3 +382,62 @@ async def post(self, path: str, json: dict = None, **kwargs): assert response.status_code == 502 assert len(grant_calls) == 0 + + +def test_inbound_sms_rejected_when_no_permission(client, session, monkeypatch): + """POST /gsm/inbound must 403 when the target has not previously contacted the sender.""" + current_user = _authenticated_user(session, phone_verified=True) + target = _verified_target(session, current_user) + current_user.phone_number = "+639171111111" + target.phone_number = "+639172222222" + session.commit() + + class UnauthorizedPermissionClient: + async def get(self, path: str, **kwargs): + if path == "/has-permission": + return FakeGsmResponse(200, {"permitted": False}) + return FakeGsmResponse(404, {}) + + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: UnauthorizedPermissionClient()) + + response = client.post( + "/gsm/inbound", + json={ + "sender_phone": current_user.phone_number, + "target_phone": target.phone_number, + "body": "Hello", + }, + headers={"X-GSM-Secret": gsm.GSM_SECRET}, + ) + + assert response.status_code == 403 + + +def test_inbound_sms_allowed_when_permission_exists(client, session, monkeypatch): + """POST /gsm/inbound must 200 when permission exists.""" + current_user = _authenticated_user(session, phone_verified=True) + target = _verified_target(session, current_user) + current_user.phone_number = "+639171111111" + target.phone_number = "+639172222222" + session.commit() + + class AuthorizedPermissionClient: + async def get(self, path: str, **kwargs): + if path == "/has-permission": + return FakeGsmResponse(200, {"permitted": True}) + return FakeGsmResponse(404, {}) + + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: AuthorizedPermissionClient()) + + response = client.post( + "/gsm/inbound", + json={ + "sender_phone": current_user.phone_number, + "target_phone": target.phone_number, + "body": "Hello", + }, + headers={"X-GSM-Secret": gsm.GSM_SECRET}, + ) + + assert response.status_code == 200 + assert response.json()["ok"] is True From 2ac1256728d9a9f9b6971c714dd571e6b5f8fdab Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 14:20:52 +0800 Subject: [PATCH 08/10] fix(gsm-docs): redact phone numbers in logs, fix comment style, and update persistence docs. --- GSM-module/GSM-fastapi/database.py | 18 +++++++++--------- GSM-module/GSM-fastapi/serial_worker.py | 3 ++- GSM-module/GSM-fastapi/sms_handler.py | 9 ++++++++- docs/features/sms-gateway/design.md | 6 ++++++ 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/GSM-module/GSM-fastapi/database.py b/GSM-module/GSM-fastapi/database.py index ecced590..682de74d 100644 --- a/GSM-module/GSM-fastapi/database.py +++ b/GSM-module/GSM-fastapi/database.py @@ -6,15 +6,15 @@ Two categories of tables ───────────────────────── SHARED (defined by your main SAPOT API, relay reads/writes them) - user — registered accounts; relay looks up phone numbers - conversation — relay creates one per SMS relay session - conversationparticipant — relay adds sender + server bot as participants - message — relay writes each forwarded message here + user -- registered accounts; relay looks up phone numbers + conversation -- relay creates one per SMS relay session + conversationparticipant -- relay adds sender + server bot as participants + message -- relay writes each forwarded message here - RELAY-ONLY (owned by this service, invisible to the main API) - sms_session — per-number conversation stage + current target - sms_log — every SMS in/out with delivery status - sms_unregistered_warning — numbers that received the registration warning + RELAY-ONLY (defined by this service -- invisible to the main API) + sms_session -- per-number conversation stage + current target + sms_log -- every SMS in/out with delivery status + sms_unregistered_warning -- numbers that received the registration warning Engine is created once at startup via init(). All public functions are thread-safe (SQLAlchemy handles connection pooling). @@ -756,7 +756,7 @@ def get_permitted_contacts(external_phone: str) -> list: # ============================================================================= -# APP FORWARD — write message into shared Conversation + Message tables +# APP FORWARD -- write message into shared Conversation + Message tables # so the forwarded SMS appears inside the SAPOT app natively # ============================================================================= diff --git a/GSM-module/GSM-fastapi/serial_worker.py b/GSM-module/GSM-fastapi/serial_worker.py index 060c111f..7aaa2a3e 100644 --- a/GSM-module/GSM-fastapi/serial_worker.py +++ b/GSM-module/GSM-fastapi/serial_worker.py @@ -350,7 +350,8 @@ def _handle_line(self, line: str): return if etype == EventType.SMS_RECEIVED: - logger.info("SMS_RECEIVED from %s (%d characters)", event.number, len(event.body)) + _redacted = f"***{event.number[-4:]}" if len(event.number) > 4 else "[redacted]" + logger.info("SMS_RECEIVED from %s (%d characters)", _redacted, len(event.body)) try: self.incoming_queue.put_nowait(event) except queue.Full: diff --git a/GSM-module/GSM-fastapi/sms_handler.py b/GSM-module/GSM-fastapi/sms_handler.py index 3fa7ab27..a76dad32 100644 --- a/GSM-module/GSM-fastapi/sms_handler.py +++ b/GSM-module/GSM-fastapi/sms_handler.py @@ -54,6 +54,13 @@ "Only permitted SAPOT contacts can be targeted." ) # 82 chars +def _redact_phone(number: str) -> str: + """Redact all but the last 4 digits of a phone number for logging.""" + if len(number) <= 4: + return "[redacted]" + return f"***{number[-4:]}" + + def _msg_target_set(username: str, phone: str) -> str: return f"Target: {username} ({phone}). Messages go to them now." # e.g. "Target: maria_santos (+639281234567). Messages go to them now." = 63 @@ -92,7 +99,7 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: logger.warning("Rejected malformed sender number") return None, None, None, "MALFORMED_SENDER" body = " ".join(unicodedata.normalize("NFKC", body).split()) - logger.info("SMS received from %s (%d characters)", number, len(body)) + logger.info("SMS received from %s (%d characters)", _redact_phone(number), len(body)) sender_user = database.get_user_by_phone(number) if not sender_user: diff --git a/docs/features/sms-gateway/design.md b/docs/features/sms-gateway/design.md index 715ed7b0..d60ff737 100644 --- a/docs/features/sms-gateway/design.md +++ b/docs/features/sms-gateway/design.md @@ -108,6 +108,9 @@ The GSM service uses the database configured by required `DB_PATH`. | `sms_log` | Inbound and outbound audit rows, delivery status, and failure reason | | `sms_session` | Per-phone conversation stage and selected target | | `sms_unregistered_warning` | Numbers that received the unregistered-account warning | +| `sms_rate_counter` | Sliding-window rate limit counters and daily quota tracking | +| `sms_recipient_preference` | Per-phone opt-out status for inbound and outbound SMS relay | +| `sms_outbound_permission` | Permission granted to external numbers contacted by verified SAPOT users | | Shared user and conversation tables | Lookup and delivery integration with the main server | The committed `sapot.db` file is stale and is not used by the deployed service. @@ -129,6 +132,9 @@ The gateway does not re-queue these rows. A crash can happen after the modem tra | Modem reports failure or confirmation times out | HTTP 502 with the modem or timeout reason | | Caller deadline expires while waiting in the queue | Request fails and is never written later | | Main server cannot reach the GSM service | Main server health route returns HTTP 503 | +| Recipient has opted out | HTTP 400 with `RECIPIENT_OPTED_OUT` | +| Daily send limit reached | HTTP 429 with `DAILY_SEND_LIMIT` | +| External number not authorized | `NOT_PERMITTED` / `TARGET_NOT_PERMITTED` / HTTP 403 at `/gsm/inbound` | The main server preserves the gateway status and error detail for synchronous SMS operations. Its 135-second read timeout covers the gateway's 125-second worst case plus HTTP overhead. Nginx allows 155 seconds so the one-second pool, five-second connect, five-second write, and 135-second read phase limits all fit inside the outer proxy limit. The main server permits 22 GSM connections, enough for 21 admitted gateway requests plus one request that observes `QUEUE_FULL`. Further requests fail pool admission within one second and never reach the gateway later. From 8f7810d36050b4d5425089066d3813e21adf2f50 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 14:28:05 +0800 Subject: [PATCH 09/10] fix(gsm): resolve final review findings --- GSM-module/GSM-fastapi/api.py | 22 +++++++-------- GSM-module/GSM-fastapi/config.py | 2 +- GSM-module/GSM-fastapi/serial_worker.py | 11 ++++---- GSM-module/GSM-fastapi/sms_handler.py | 23 ++++++--------- .../GSM-fastapi/tests/test_incoming_sms.py | 4 +-- server/app/tests/test_gsm_proxy.py | 28 +++++++++++++++++++ 6 files changed, 55 insertions(+), 35 deletions(-) diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index f7549502..99254908 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -10,20 +10,20 @@ - Shuts everything down cleanly on exit Endpoints - GET /health — liveness + GSM status - GET /health/detailed — full diagnostic info + GET /health -- liveness + GSM status + GET /health/detailed -- full diagnostic info - POST /sms/send — send an SMS (blocks until delivered) - GET /sms/messages — message log + POST /sms/send -- send an SMS (blocks until delivered) + GET /sms/messages -- message log - GET /users — list registered SAPOT users - POST /users — add a user - GET /users/{phone} — look up a user + GET /users -- list registered SAPOT users + POST /users -- add a user + GET /users/{phone} -- look up a user - GET /sessions — all active sessions (debug) - DELETE /sessions/{phone} — reset a session + GET /sessions -- all active sessions (debug) + DELETE /sessions/{phone} -- reset a session - GET /status — modem + serial status + GET /status -- modem + serial status """ import asyncio @@ -318,7 +318,7 @@ async def health(): def health_detailed( phone: Optional[str] = None, ): - """Full diagnostic — serial state, queue depth, pending SMS.""" + """Full diagnostic -- serial state, queue depth, pending SMS.""" if _worker is None: raise HTTPException(503, "Worker not initialised") diff --git a/GSM-module/GSM-fastapi/config.py b/GSM-module/GSM-fastapi/config.py index 94615604..1b22a300 100644 --- a/GSM-module/GSM-fastapi/config.py +++ b/GSM-module/GSM-fastapi/config.py @@ -53,7 +53,7 @@ class Settings: # Serial port the Arduino is connected to serial_port: str = os.environ.get("SERIAL_PORT", "/dev/ttyACM0") - # Baud rate — must match PC_BAUD in the Arduino sketch (9600) + # Baud rate -- must match PC_BAUD in the Arduino sketch (9600) serial_baud: int = int(os.environ.get("SERIAL_BAUD", "9600")) db_path: str | None = os.environ.get("DB_PATH") diff --git a/GSM-module/GSM-fastapi/serial_worker.py b/GSM-module/GSM-fastapi/serial_worker.py index 7aaa2a3e..6c5e5393 100644 --- a/GSM-module/GSM-fastapi/serial_worker.py +++ b/GSM-module/GSM-fastapi/serial_worker.py @@ -7,7 +7,7 @@ worker.send_sms(number, body, timeout=30) -> dict Enqueues the SMS and blocks until delivery confirmed or timeout. Returns {"ok": True/False, "reason": str|None} - Multiple callers are safe — they queue and execute one at a time. + Multiple callers are safe -- they queue and execute one at a time. worker.gsm_ready -> bool worker.connected -> bool @@ -22,6 +22,7 @@ import logging import queue +import database import threading import time from dataclasses import dataclass, field @@ -66,7 +67,7 @@ class SerialWorker: """ Owns the serial port in one dedicated reader thread. A separate sender thread drains the outbound queue so that - multiple callers to send_sms() always serialise correctly — + multiple callers to send_sms() always serialise correctly -- no "another SMS in flight" errors, no dropped forwards. Flow for each send_sms() call: @@ -144,7 +145,7 @@ def send_sms(self, number: str, body: str, """ Queue an SMS for delivery and block until the modem confirms it. - Multiple concurrent callers are safe — they queue up and each + Multiple concurrent callers are safe -- they queue up and each waits for their own confirmation. Returns {"ok": bool, "reason": str|None} @@ -343,14 +344,14 @@ def _handle_line(self, line: str): return if etype == EventType.SMS_FAILED: - logger.error("SMS_FAILED for %s reason=%s", event.number, + logger.error("SMS_FAILED for %s reason=%s", database._redact_phone(event.number), event.reason) self._resolve_in_flight(event.number, success=False, reason=event.reason) return if etype == EventType.SMS_RECEIVED: - _redacted = f"***{event.number[-4:]}" if len(event.number) > 4 else "[redacted]" + _redacted = database._redact_phone(event.number) logger.info("SMS_RECEIVED from %s (%d characters)", _redacted, len(event.body)) try: self.incoming_queue.put_nowait(event) diff --git a/GSM-module/GSM-fastapi/sms_handler.py b/GSM-module/GSM-fastapi/sms_handler.py index a76dad32..d8257c85 100644 --- a/GSM-module/GSM-fastapi/sms_handler.py +++ b/GSM-module/GSM-fastapi/sms_handler.py @@ -54,13 +54,6 @@ "Only permitted SAPOT contacts can be targeted." ) # 82 chars -def _redact_phone(number: str) -> str: - """Redact all but the last 4 digits of a phone number for logging.""" - if len(number) <= 4: - return "[redacted]" - return f"***{number[-4:]}" - - def _msg_target_set(username: str, phone: str) -> str: return f"Target: {username} ({phone}). Messages go to them now." # e.g. "Target: maria_santos (+639281234567). Messages go to them now." = 63 @@ -99,17 +92,17 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: logger.warning("Rejected malformed sender number") return None, None, None, "MALFORMED_SENDER" body = " ".join(unicodedata.normalize("NFKC", body).split()) - logger.info("SMS received from %s (%d characters)", _redact_phone(number), len(body)) + logger.info("SMS received from %s (%d characters)", database._redact_phone(number), len(body)) sender_user = database.get_user_by_phone(number) if not sender_user: - logger.warning("Account does not exist: %s", number) + logger.warning("Account does not exist: %s", database._redact_phone(number)) if database.has_unregistered_warning(number): return None, None, None, "NO_ACCOUNT" return MSG_NO_ACCOUNT, None, None, "NO_ACCOUNT" if sender_user.get("banned"): - logger.warning("Banned: %s", number) + logger.warning("Banned: %s", database._redact_phone(number)) return ( "This number has been banned by the system", None, @@ -117,7 +110,7 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: "BANNED_SENDER", ) if not sender_user.get("phone_is_verified"): - logger.warning("Unverified number: %s", number) + logger.warning("Unverified number: %s", database._redact_phone(number)) return ( "Please verify your account first.", None, @@ -231,14 +224,14 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: target_user = database.get_user_by_phone(target_phone) if not target_user: - logger.warning("Target does not exist: %s", target_phone) + logger.warning("Target does not exist: %s", database._redact_phone(target_phone)) return f"Target {target_phone} does not exist.", None, None, "TARGET_MISSING" if database.is_sms_opted_out(target_phone): return "That target has opted out of SMS relay messages.", None, None, "TARGET_OPTED_OUT" if target_user.get("banned"): - logger.warning("Banned: %s", target_phone) + logger.warning("Banned: %s", database._redact_phone(target_phone)) return ( f"This number ({target_phone}) has been banned by the system.", None, @@ -246,7 +239,7 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: "TARGET_BANNED", ) if not target_user.get("phone_is_verified"): - logger.warning("Unverified number: %s", target_phone) + logger.warning("Unverified number: %s", database._redact_phone(target_phone)) return f"Target {target_phone} is not verified.", None, None, "TARGET_UNVERIFIED" if not target_phone: @@ -259,7 +252,7 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: return "Relay limit reached. Please try again tomorrow.", None, None, "RELAY_LIMIT" ok = database.notify_app(sender_phone, target_phone, body) - logger.info("notify_app result: %s (sender=%s target=%s)", ok, sender_phone, target_phone) + logger.info("notify_app result: %s (sender=%s target=%s)", ok, database._redact_phone(sender_phone), database._redact_phone(target_phone)) if not ok: return MSG_FORWARD_FAIL, None, None, "APP_DELIVERY_FAILED" diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py index d0bf426e..7278cd0c 100644 --- a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -1,6 +1,7 @@ from types import SimpleNamespace import pytest +from fastapi.testclient import TestClient import api import sms_handler @@ -157,7 +158,6 @@ def test_get_permitted_contacts_empty_when_none(): assert contacts == [] -from fastapi.testclient import TestClient def test_grant_permission_endpoint_stores_permission(monkeypatch): @@ -283,7 +283,6 @@ def test_has_permission_endpoint_returns_true_when_granted(monkeypatch): lambda sapot, external: sapot == "+639171111111" and external == "+639288888888" ) from api import app as gsm_app - from fastapi.testclient import TestClient client = TestClient(gsm_app) resp = client.get( @@ -299,7 +298,6 @@ def test_has_permission_endpoint_returns_true_when_granted(monkeypatch): def test_has_permission_endpoint_returns_false_when_not_granted(monkeypatch): monkeypatch.setattr("database.has_outbound_permission", lambda *_: False) from api import app as gsm_app - from fastapi.testclient import TestClient client = TestClient(gsm_app) resp = client.get( diff --git a/server/app/tests/test_gsm_proxy.py b/server/app/tests/test_gsm_proxy.py index 0d4c1c27..5b606b26 100644 --- a/server/app/tests/test_gsm_proxy.py +++ b/server/app/tests/test_gsm_proxy.py @@ -384,6 +384,34 @@ async def post(self, path: str, json: dict = None, **kwargs): +def test_successful_send_sms_returns_200_when_grant_fails(client, session, monkeypatch): + """A failed /grant-permission should not abort the main send_sms loop.""" + current_user = _authenticated_user(session, phone_verified=True) + target = _verified_target(session, current_user) + current_user.phone_number = "+639171111111" + target.phone_number = "+639172222222" + session.commit() + + class GrantFailGsmClient: + async def post(self, path: str, json: dict = None, **kwargs): + if path == "/sms/send": + return FakeGsmResponse(200, {"ok": True, "msg_id": "abc"}) + if path == "/grant-permission": + return FakeGsmResponse(500, {"detail": "Internal server error"}) + return FakeGsmResponse(404, {}) + + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: GrantFailGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Hello"}, + ) + + assert response.status_code == 200 + + + def test_inbound_sms_rejected_when_no_permission(client, session, monkeypatch): """POST /gsm/inbound must 403 when the target has not previously contacted the sender.""" current_user = _authenticated_user(session, phone_verified=True) From 83f09dcaa46f37055d567024a1821506babe49d6 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Sun, 16 Aug 2026 16:12:08 +0800 Subject: [PATCH 10/10] docs(server-gsm): regenerate openapi specs for gsm-sms --- docs/api/openapi/gsm-sms.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/api/openapi/gsm-sms.yaml b/docs/api/openapi/gsm-sms.yaml index e22a57d1..62cdef13 100644 --- a/docs/api/openapi/gsm-sms.yaml +++ b/docs/api/openapi/gsm-sms.yaml @@ -108,7 +108,12 @@ paths: tags: - gsm summary: Inbound Sms - description: 'Internal endpoint: receives an inbound SMS from the GSM-API and delivers it to the target user via WebSocket.' + description: |- + Internal endpoint: receives an inbound SMS from the GSM-API and delivers it + to the target user via WebSocket. + + Enforces the outbound-permission rule: the sender-target pair is only accepted + if the target previously sent an outbound SMS to the sender via the relay. operationId: inbound_sms_gsm_inbound_post requestBody: content: