From fe7cf91488249b34cff60c120599f6247aa6a70e Mon Sep 17 00:00:00 2001 From: serversdown Date: Thu, 11 Jun 2026 23:40:52 +0000 Subject: [PATCH] fix(portal): pre-merge security hardening from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PORTAL_OPEN_LINKS now defaults OFF — /portal/open/* is an unauthenticated, proxy-reachable session-minting path (and a linked project's open link grants the whole client's scope), so it must be explicitly enabled in dev. - Session cookie: enforce server-side expiry (check iat vs COOKIE_MAX_AGE — was browser-only) and guard a non-dict signed body (was an uncaught AttributeError → 500, reachable if SECRET_KEY is the insecure default). - Escape operator-set strings (location/rule/event names) before innerHTML + Leaflet tooltips — they're client-facing, so a name with markup was stored XSS in the client's browser. Global esc() helper applied at every injection point. - WS _scrub_frame drops a non-JSON frame instead of forwarding it raw; /history rows now whitelisted like the other scoped endpoints. - Preview-client slug uses the full project id (an 8-char prefix could collide two projects onto one client). Verified: cookie reader (fresh/expired/non-dict/missing-iat) + open-links default off; templates parse; scoped scrubbing intact. Co-Authored-By: Claude Opus 4.8 --- backend/portal_auth.py | 23 +++++++++++++++-------- backend/routers/portal.py | 15 ++++++++++----- templates/portal/base.html | 2 ++ templates/portal/location.html | 8 ++++---- templates/portal/overview.html | 6 +++--- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/backend/portal_auth.py b/backend/portal_auth.py index 72537cc..d233e2f 100644 --- a/backend/portal_auth.py +++ b/backend/portal_auth.py @@ -40,14 +40,14 @@ if SECRET_KEY == "dev-insecure-change-me": COOKIE_NAME = "portal_session" COOKIE_MAX_AGE = 60 * 60 * 24 * 30 # 30 days -# Dev convenience: plain, no-token portal links (/portal/open/{project_id}) so -# anyone can open a client portal for feedback without minting a magic link. -# Defaults ON for the current prototype (the whole app is open anyway); set -# PORTAL_OPEN_LINKS=false before real clients are on the portal. -PORTAL_OPEN_LINKS = os.getenv("PORTAL_OPEN_LINKS", "true").lower() in ("1", "true", "yes") +# Plain, no-token portal links (/portal/open/{project_id}). These are an +# UNAUTHENTICATED, proxy-reachable session-minting path (and a linked project's +# open link grants the *whole* client's scope), so they default OFF and must be +# explicitly enabled — set PORTAL_OPEN_LINKS=true only in a dev/prototype env. +PORTAL_OPEN_LINKS = os.getenv("PORTAL_OPEN_LINKS", "false").lower() in ("1", "true", "yes") if PORTAL_OPEN_LINKS: logger.warning("[PORTAL] open links ENABLED — no-token /portal/open/* shareable links. " - "Set PORTAL_OPEN_LINKS=false before real clients.") + "Keep this OFF in any internet-facing / production deployment.") class PortalAuthError(Exception): @@ -84,9 +84,16 @@ def _read_session_cookie(value: str): return None try: data = json.loads(base64.urlsafe_b64decode(body.encode())) + if not isinstance(data, dict): + return None + # Server-side expiry: a leaked cookie isn't valid forever (max_age is only a + # browser hint). iat is set by make_session_cookie. + iat = data.get("iat") + if not isinstance(iat, (int, float)) or (time.time() - iat) > COOKIE_MAX_AGE: + return None + return data.get("tid") except Exception: return None - return data.get("tid") # -- the dependency every portal route uses --------------------------------- @@ -137,7 +144,7 @@ def ensure_project_client(project, db) -> Client: if project.client_id: client = db.query(Client).filter_by(id=project.client_id, active=True).first() if client is None: - slug = f"preview-{str(project.id)[:8]}" + slug = f"preview-{project.id}" # full id — an 8-char prefix can collide across projects client = db.query(Client).filter_by(slug=slug).first() if client is None: client = Client(id=str(uuid.uuid4()), diff --git a/backend/routers/portal.py b/backend/routers/portal.py index 412a7f5..8c58614 100644 --- a/backend/routers/portal.py +++ b/backend/routers/portal.py @@ -211,7 +211,9 @@ async def portal_location_history(location_id: str, hours: float = 2.0, return {"status": "ok", "readings": []} if r.status_code != 200: return {"status": "ok", "readings": []} - return {"status": "ok", "readings": (r.json() or {}).get("readings", [])} + raw = (r.json() or {}).get("readings", []) + fields = ("timestamp", "lp", "leq", "lmax", "ln1", "ln2") # whitelist, like the other endpoints + return {"status": "ok", "readings": [{k: x.get(k) for k in fields} for x in raw]} # Whitelist of alert-event fields exposed to a client (no internal ids/ack-by). @@ -272,14 +274,15 @@ async def portal_location_thresholds(location_id: str, # -- live stream (fan-out feed, scoped + scrubbed) --------------------------- -def _scrub_frame(raw: str) -> str: +def _scrub_frame(raw: str): """Project a monitor frame down to the portal whitelist. Drops internal fields (unit_id, raw_payload, lmin) before it reaches a client; passes control fields - (feed_status, heartbeat) + timestamp through.""" + (feed_status, heartbeat) + timestamp through. Returns None for a non-JSON frame + so the caller drops it rather than forwarding anything unscrubbed.""" try: d = json.loads(raw) except Exception: - return raw + return None out = {k: d.get(k) for k in _PORTAL_LIVE_FIELDS if k in d} if "timestamp" in d: out["timestamp"] = d["timestamp"] @@ -327,7 +330,9 @@ async def portal_location_stream(websocket: WebSocket, location_id: str): async def forward_to_client(): async for message in backend_ws: - await websocket.send_text(_scrub_frame(message)) + frame = _scrub_frame(message) + if frame is not None: + await websocket.send_text(frame) async def watch_client(): while True: diff --git a/templates/portal/base.html b/templates/portal/base.html index 375feee..c58f0be 100644 --- a/templates/portal/base.html +++ b/templates/portal/base.html @@ -179,6 +179,8 @@