From 922de8cc67535c1de0d6eb2a2ac2782816acb7d5 Mon Sep 17 00:00:00 2001 From: Tobias Huttinger Date: Mon, 4 May 2026 22:17:01 +0200 Subject: [PATCH] Security and resilience improvements: We now only record messages that have been sent over radio to db, standard web interface listening port is now localhost and the webserver now checks the X-Sec-Fetch header and blocks if its not same origin. --- docker-compose.yml | 3 +++ src/lorabot/config.py | 4 +++- src/lorabot/handler.py | 4 ++++ src/lorabot/transport.py | 13 ++++++++----- src/lorabot/web.py | 13 +++++++++++++ 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 19537e8..c5f69ad 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -15,6 +15,9 @@ services: LORABOT_LLM__BASE_URL: ${LORABOT_LLM_BASE_URL:?set LORABOT_LLM_BASE_URL} LORABOT_LLM__API_KEY: ${LORABOT_LLM_API_KEY:-not-needed} LORABOT_LLM__MODEL: ${LORABOT_LLM_MODEL:?set LORABOT_LLM_MODEL} + # The app defaults to loopback; inside the container we need 0.0.0.0 so the + # docker port mapping below can reach it. Restrict exposure at the host port. + LORABOT_WEB__HOST: ${LORABOT_WEB_HOST:-0.0.0.0} ports: # Built-in read-only web UI. Override via LORABOT_WEB_PORT. - "${LORABOT_WEB_PORT:-8080}:8080" diff --git a/src/lorabot/config.py b/src/lorabot/config.py index 2d607eb..b3816d9 100644 --- a/src/lorabot/config.py +++ b/src/lorabot/config.py @@ -41,7 +41,9 @@ class MessageCfg(BaseModel): class WebCfg(BaseModel): enabled: bool = True - host: str = "0.0.0.0" + # Default to loopback. Conversation logs are unauthenticated; only set this to + # 0.0.0.0 (e.g. inside Docker) when you understand the exposure. + host: str = "127.0.0.1" port: int = Field(default=8080, gt=0, lt=65536) diff --git a/src/lorabot/handler.py b/src/lorabot/handler.py index 5d06d1a..9fa5493 100644 --- a/src/lorabot/handler.py +++ b/src/lorabot/handler.py @@ -99,6 +99,10 @@ def build_dm_handler( return delivered = await send_chunked(mc, contact, reply, cfg.message.max_bytes) + if not delivered: + # Nothing made it onto the radio; don't persist anything + return + db.add_message(db_conn, public_key, "assistant", delivered, hidden_from_llm=is_command) state.publish("message", { "public_key": public_key, diff --git a/src/lorabot/transport.py b/src/lorabot/transport.py index f4868c6..41669e1 100644 --- a/src/lorabot/transport.py +++ b/src/lorabot/transport.py @@ -32,18 +32,20 @@ async def resolve_contact(mc: MeshCore, prefix: str, lock: asyncio.Lock): async def send_chunked(mc: MeshCore, contact, text: str, max_bytes: int, max_chunks: int = 2) -> str: """Split ``text`` into byte-budgeted chunks and send them in order. - Stops sending on the first transport error. Returns the actually-delivered text - (concatenated chunks that the caller should record as the assistant turn). + Stops sending on the first transport error. Returns the concatenation of the + chunks that were actually accepted by the device, so the caller records the + truth — not what we hoped to send. """ chunks = split_to_bytes(text, max_bytes, max_chunks=max_chunks) - delivered = "".join(chunks) pk_short = contact["public_key"][:12] + planned_bytes = sum(len(c.encode("utf-8")) for c in chunks) - dropped = len(text.encode("utf-8")) - len(delivered.encode("utf-8")) + dropped = len(text.encode("utf-8")) - planned_bytes if dropped > 0: log.info("reply to %s split into %d chunks, dropped %d trailing bytes", pk_short, len(chunks), dropped) + sent: list[str] = [] for i, chunk in enumerate(chunks, 1): log.info("reply to %s (%d/%d, %d bytes): %s", pk_short, i, len(chunks), len(chunk.encode("utf-8")), chunk) @@ -52,4 +54,5 @@ async def send_chunked(mc: MeshCore, contact, text: str, max_bytes: int, max_chu log.error("send_msg failed for %s chunk %d/%d: %s", pk_short, i, len(chunks), result.payload) break - return delivered + sent.append(chunk) + return "".join(sent) diff --git a/src/lorabot/web.py b/src/lorabot/web.py index 9f3c016..40318f6 100644 --- a/src/lorabot/web.py +++ b/src/lorabot/web.py @@ -173,7 +173,20 @@ async def _api_conversations(req: web.Request) -> web.Response: return web.json_response(_list_conversations(_state(req).db)) +def _require_same_origin(req: web.Request) -> None: + """Reject obvious cross-site browser requests on state-changing endpoints. + + ``Sec-Fetch-Site`` is set by all current browsers on every request. ``same-origin`` + is the only value we accept. Non-browser clients (curl, scripts) don't send the + header at all — those pass through, since they're not the CSRF threat model. + """ + site = req.headers.get("Sec-Fetch-Site") + if site is not None and site != "same-origin": + raise web.HTTPForbidden(text=f"cross-origin request rejected (Sec-Fetch-Site: {site})") + + async def _api_advertise(req: web.Request) -> web.Response: + _require_same_origin(req) state = _state(req) if state._advertise is None: raise web.HTTPServiceUnavailable(text="device not connected")