diff --git a/README.md b/README.md index 87e9f9a..ac537bd 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,8 @@ FactGraph is meant to be spun up during an incident, often in a hurry and someti **Container** - Runs as an unprivileged user (UID 10001) on a slim Python image. -- No access log (request URLs contain board IDs) and no server banner. +- No access log (request URLs contain board IDs) and no server banner. The connection lines Uvicorn still logs show `/ws/boards/` instead of the ID. +- Logs show the client's real IP: the image trusts `X-Forwarded-For` only from loopback and link-local proxies (`169.254.0.0/16`, e.g. Azure App Service front ends), so a client cannot fake it. Behind another reverse proxy, set `FORWARDED_ALLOW_IPS` to its address. - Single port 8080, plain HTTP; TLS is terminated in front (Azure App Service, Caddy, nginx, Traefik, an ingress). The browser automatically uses `wss://` for the relay when the page is served over HTTPS. **What it does not do** diff --git a/app/main.py b/app/main.py index 1b811de..22a14bf 100644 --- a/app/main.py +++ b/app/main.py @@ -10,7 +10,9 @@ import csv import contextvars import json +import logging import os +import re import secrets import zlib from dataclasses import dataclass @@ -80,6 +82,24 @@ class Peer: # A batch ID chosen by the client (file import from the board UI), so the browser recognises its own import exactly. request_batch: contextvars.ContextVar[str | None] = contextvars.ContextVar("factgraph_request_batch", default=None) +BOARD_IN_PATH = re.compile(r"(/boards/)[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}") + + +class HideBoardIds(logging.Filter): + """The board ID is the board's access key. Uvicorn logs every WebSocket connection with its path (/ws/boards/), also + with --no-access-log, and hosting platforms keep container logs (App Service log stream and storage): the ID is + replaced in every Uvicorn record, and in access log lines should someone switch them on.""" + def filter(self, record: logging.LogRecord) -> bool: + if isinstance(record.msg, str): + record.msg = BOARD_IN_PATH.sub(r"\1", record.msg) + if isinstance(record.args, tuple): + record.args = tuple(BOARD_IN_PATH.sub(r"\1", arg) if isinstance(arg, str) else arg for arg in record.args) + return True + + +for _logger in ("uvicorn.error", "uvicorn.access"): + logging.getLogger(_logger).addFilter(HideBoardIds()) + def token_matches(expected: str | None, given: str) -> bool: return bool(expected) and secrets.compare_digest(expected.encode(), given.encode()) @@ -824,21 +844,32 @@ async def receive_message(websocket: WebSocket): return None +async def refuse(websocket: WebSocket, code: int = 1008) -> None: + """Close a connection whose browser may already be gone: a reload or a closed tab right after connecting, many at once + after a server restart. Closing it again is no error worth a traceback.""" + try: + await websocket.close(code=code) + except (RuntimeError, OSError, WebSocketDisconnect): # OSError: uvicorn's ClientDisconnected + pass + + @app.websocket("/ws/boards/{board_id}") async def board_socket(websocket: WebSocket, board_id: str): if not valid_uuid(board_id) or not allowed_origin(websocket): - await websocket.close(code=1008) + await refuse(websocket) return await websocket.accept() # Identity and token arrive in the first message, never in the URL, so they stay out of access logs. try: hello = json.loads(await asyncio.wait_for(websocket.receive_text(), timeout=10)) - except (asyncio.TimeoutError, ValueError, WebSocketDisconnect, RuntimeError): - await websocket.close(code=1008) + except WebSocketDisconnect: + return # the browser left before saying hello + except (asyncio.TimeoutError, ValueError, RuntimeError, KeyError): + await refuse(websocket) return actor = str(hello.get("actor", "")) if isinstance(hello, dict) and hello.get("type") == "hello" else "" if not valid_uuid(actor): - await websocket.close(code=1008) + await refuse(websocket) return name = str(hello.get("name", "Guest")).strip()[:40] or "Guest" session_token = str(hello.get("token", "")).strip()[:200] or None @@ -855,7 +886,7 @@ async def board_socket(websocket: WebSocket, board_id: str): # one actor would otherwise replace each other every few seconds) and offers a new identity instead. try: await previous.websocket.close(code=4001, reason="replaced") - except RuntimeError: + except (RuntimeError, OSError, WebSocketDisconnect): # the replaced tab may be gone already pass for request_id, (writer, future) in list(pending.items()): if writer is previous.websocket and not future.done(): diff --git a/deploy/Dockerfile b/deploy/Dockerfile index 668e05d..9ce4c2e 100644 --- a/deploy/Dockerfile +++ b/deploy/Dockerfile @@ -9,8 +9,12 @@ RUN npm run build # Latest stable Python on an explicitly named Debian release (3.15 is only a release candidate as of October 2026). FROM python:3.14-slim-trixie WORKDIR /app +# Logs show the client's real IP from X-Forwarded-For, trusted only from loopback and link-local proxies (169.254.0.0/16: +# Azure App Service front ends; not reachable as a client, so nobody can fake an IP in the log). Behind another reverse +# proxy, set FORWARDED_ALLOW_IPS to its address. ENV PYTHONDONTWRITEBYTECODE=1 \ - PYTHONUNBUFFERED=1 + PYTHONUNBUFFERED=1 \ + FORWARDED_ALLOW_IPS=127.0.0.1,::1,169.254.0.0/16 # Apply Debian security updates at build time (e.g. libpcre2-8-0), even when the base image lags behind. RUN apt-get update \ && apt-get -y upgrade --no-install-recommends \ diff --git a/tests/test_logs.py b/tests/test_logs.py new file mode 100644 index 0000000..9248e89 --- /dev/null +++ b/tests/test_logs.py @@ -0,0 +1,70 @@ +"""Server logs: no board IDs (they are the boards' access keys), the client's real IP behind a trusted proxy, and no +traceback when a browser leaves before saying hello (a reload, or many tabs reconnecting after a restart).""" +import asyncio +import json +import os +import socket +import subprocess +import sys +import tempfile +import time +import unittest +from pathlib import Path +from urllib.request import urlopen +from uuid import uuid4 + +import websockets + + +class ServerLogTest(unittest.IsolatedAsyncioTestCase): + @classmethod + def setUpClass(cls): + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + cls.port = sock.getsockname()[1] + handle, cls.log = tempfile.mkstemp(suffix=".log") + # As in the Docker image: INFO logs, no access log, proxies trusted only on loopback and link-local addresses. + env = {**os.environ, "FORWARDED_ALLOW_IPS": "127.0.0.1,::1,169.254.0.0/16"} + cls.process = subprocess.Popen([sys.executable, "-m", "uvicorn", "app.main:app", "--host", "127.0.0.1", "--port", str(cls.port), + "--log-level", "info", "--no-access-log"], cwd=Path(__file__).resolve().parents[1], + stdout=handle, stderr=subprocess.STDOUT, env=env) + os.close(handle) + for _ in range(100): + try: + with urlopen(f"http://127.0.0.1:{cls.port}/api/health", timeout=0.1): + return + except Exception: + time.sleep(0.05) + cls.process.terminate() + raise RuntimeError("Test server could not be started") + + @classmethod + def tearDownClass(cls): + cls.process.terminate() + cls.process.wait(5) + os.unlink(cls.log) + + async def test_logs_hide_board_ids_show_real_ip_and_no_traceback(self): + board = str(uuid4()) + url = f"ws://127.0.0.1:{self.port}/ws/boards/{board}" + # The platform's front end forwards the client as "IP:port" (Azure App Service). + proxy = {"X-Forwarded-For": "203.0.113.9:51515"} + # Browsers that leave before saying hello: one closes cleanly, one drops the connection (as after a restart). + leaving = await websockets.connect(url, additional_headers=proxy) + await leaving.close() + dropping = await websockets.connect(url, additional_headers=proxy) + dropping.transport.abort() + async with websockets.connect(url, additional_headers=proxy) as browser: + await browser.send(json.dumps({"type": "hello", "actor": str(uuid4()), "name": "Log test"})) + self.assertEqual(json.loads(await asyncio.wait_for(browser.recv(), timeout=3))["type"], "welcome") + await asyncio.sleep(0.5) + log = Path(self.log).read_text() + self.assertNotIn(board, log, "the board ID is the board's access key") + self.assertIn('"WebSocket /ws/boards/" [accepted]', log) + self.assertIn("203.0.113.9", log, "the client's real IP, not the proxy's") + self.assertNotIn("Traceback", log) + self.assertNotIn("Exception in ASGI application", log) + + +if __name__ == "__main__": + unittest.main()