fix(portal): pre-merge security hardening from code review

- 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 <noreply@anthropic.com>
This commit is contained in:
2026-06-11 23:40:52 +00:00
parent c1bc391ba2
commit fe7cf91488
5 changed files with 34 additions and 20 deletions
+15 -8
View File
@@ -40,14 +40,14 @@ if SECRET_KEY == "dev-insecure-change-me":
COOKIE_NAME = "portal_session" COOKIE_NAME = "portal_session"
COOKIE_MAX_AGE = 60 * 60 * 24 * 30 # 30 days COOKIE_MAX_AGE = 60 * 60 * 24 * 30 # 30 days
# Dev convenience: plain, no-token portal links (/portal/open/{project_id}) so # Plain, no-token portal links (/portal/open/{project_id}). These are an
# anyone can open a client portal for feedback without minting a magic link. # UNAUTHENTICATED, proxy-reachable session-minting path (and a linked project's
# Defaults ON for the current prototype (the whole app is open anyway); set # open link grants the *whole* client's scope), so they default OFF and must be
# PORTAL_OPEN_LINKS=false before real clients are on the portal. # explicitly enabled — set PORTAL_OPEN_LINKS=true only in a dev/prototype env.
PORTAL_OPEN_LINKS = os.getenv("PORTAL_OPEN_LINKS", "true").lower() in ("1", "true", "yes") PORTAL_OPEN_LINKS = os.getenv("PORTAL_OPEN_LINKS", "false").lower() in ("1", "true", "yes")
if PORTAL_OPEN_LINKS: if PORTAL_OPEN_LINKS:
logger.warning("[PORTAL] open links ENABLED — no-token /portal/open/* shareable 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): class PortalAuthError(Exception):
@@ -84,9 +84,16 @@ def _read_session_cookie(value: str):
return None return None
try: try:
data = json.loads(base64.urlsafe_b64decode(body.encode())) 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: except Exception:
return None return None
return data.get("tid")
# -- the dependency every portal route uses --------------------------------- # -- the dependency every portal route uses ---------------------------------
@@ -137,7 +144,7 @@ def ensure_project_client(project, db) -> Client:
if project.client_id: if project.client_id:
client = db.query(Client).filter_by(id=project.client_id, active=True).first() client = db.query(Client).filter_by(id=project.client_id, active=True).first()
if client is None: 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() client = db.query(Client).filter_by(slug=slug).first()
if client is None: if client is None:
client = Client(id=str(uuid.uuid4()), client = Client(id=str(uuid.uuid4()),
+10 -5
View File
@@ -211,7 +211,9 @@ async def portal_location_history(location_id: str, hours: float = 2.0,
return {"status": "ok", "readings": []} return {"status": "ok", "readings": []}
if r.status_code != 200: if r.status_code != 200:
return {"status": "ok", "readings": []} 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). # 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) --------------------------- # -- 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 """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 (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: try:
d = json.loads(raw) d = json.loads(raw)
except Exception: except Exception:
return raw return None
out = {k: d.get(k) for k in _PORTAL_LIVE_FIELDS if k in d} out = {k: d.get(k) for k in _PORTAL_LIVE_FIELDS if k in d}
if "timestamp" in d: if "timestamp" in d:
out["timestamp"] = d["timestamp"] out["timestamp"] = d["timestamp"]
@@ -327,7 +330,9 @@ async def portal_location_stream(websocket: WebSocket, location_id: str):
async def forward_to_client(): async def forward_to_client():
async for message in backend_ws: 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(): async def watch_client():
while True: while True:
+2
View File
@@ -179,6 +179,8 @@
<script> <script>
// Theme toggle. Pages can listen for 'portal-theme' to re-skin canvases/maps. // Theme toggle. Pages can listen for 'portal-theme' to re-skin canvases/maps.
function cssVar(n) { return getComputedStyle(document.documentElement).getPropertyValue(n).trim(); } function cssVar(n) { return getComputedStyle(document.documentElement).getPropertyValue(n).trim(); }
// HTML-escape operator-set strings (location/rule names) before innerHTML/tooltip injection.
function esc(s) { return String(s == null ? '' : s).replace(/[&<>"']/g, c => ({ '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;' }[c])); }
function togglePortalTheme() { function togglePortalTheme() {
const cur = document.documentElement.getAttribute('data-theme') === 'light' ? 'light' : 'dark'; const cur = document.documentElement.getAttribute('data-theme') === 'light' ? 'light' : 'dark';
const next = cur === 'light' ? 'dark' : 'light'; const next = cur === 'light' ? 'dark' : 'light';
+4 -4
View File
@@ -268,7 +268,7 @@ function fmtAlertTime(iso) { return iso ? new Date(iso.endsWith('Z') ? iso : iso
// ---- alert limits (the active thresholds, read-only) --------------------- // ---- alert limits (the active thresholds, read-only) ---------------------
function fmtThreshold(r) { function fmtThreshold(r) {
const m = EV_METRIC[r.metric] || r.metric; const m = EV_METRIC[r.metric] || esc(r.metric);
const cmp = r.comparison === 'below' ? 'below' : 'above'; const cmp = r.comparison === 'below' ? 'below' : 'above';
let s = `${m} ${cmp} ${r.threshold_db} dB`; let s = `${m} ${cmp} ${r.threshold_db} dB`;
if (r.duration_s) s += ` for ${r.duration_s}s`; if (r.duration_s) s += ` for ${r.duration_s}s`;
@@ -287,7 +287,7 @@ async function loadThresholds() {
const row = document.createElement('div'); const row = document.createElement('div');
row.className = 'panel px-3.5 py-2.5 text-sm flex items-center gap-2.5'; row.className = 'panel px-3.5 py-2.5 text-sm flex items-center gap-2.5';
row.innerHTML = `<span class="w-1.5 h-1.5 rounded-full bg-seismo-orange shrink-0"></span> row.innerHTML = `<span class="w-1.5 h-1.5 rounded-full bg-seismo-orange shrink-0"></span>
<span class="text-[var(--text)]">${r.name || 'Alert'}</span> <span class="text-[var(--text)]">${esc(r.name || 'Alert')}</span>
<span class="text-[var(--text-dim)] font-mono text-xs">${fmtThreshold(r)}</span>`; <span class="text-[var(--text-dim)] font-mono text-xs">${fmtThreshold(r)}</span>`;
list.appendChild(row); list.appendChild(row);
} }
@@ -309,14 +309,14 @@ async function loadEvents() {
if (!events.length) { list.innerHTML = '<div class="text-sm text-[var(--text-dim)]">No alerts have fired.</div>'; return; } if (!events.length) { list.innerHTML = '<div class="text-sm text-[var(--text-dim)]">No alerts have fired.</div>'; return; }
list.innerHTML = ''; list.innerHTML = '';
for (const e of events) { for (const e of events) {
const m = EV_METRIC[e.metric] || e.metric; const m = EV_METRIC[e.metric] || esc(e.metric);
const active = e.status === 'active'; const active = e.status === 'active';
const when = active ? `since ${fmtAlertTime(e.onset_at)}` const when = active ? `since ${fmtAlertTime(e.onset_at)}`
: `${fmtAlertTime(e.onset_at)}${fmtAlertTime(e.clear_at)}`; : `${fmtAlertTime(e.onset_at)}${fmtAlertTime(e.clear_at)}`;
const peak = (e.peak_value != null) ? ` · peak ${e.peak_value} dB` : ''; const peak = (e.peak_value != null) ? ` · peak ${e.peak_value} dB` : '';
const row = document.createElement('div'); const row = document.createElement('div');
row.className = 'panel px-3.5 py-2.5 text-sm ' + (active ? 'border-[rgba(220,38,38,0.4)]' : ''); row.className = 'panel px-3.5 py-2.5 text-sm ' + (active ? 'border-[rgba(220,38,38,0.4)]' : '');
row.innerHTML = `<div class="${active ? 'text-[var(--lvl-bad)] font-medium' : 'text-[var(--text)]'}">${e.rule_name || 'Alert'} <span class="text-xs text-[var(--text-dim)] font-mono">· ${m} ${e.threshold_db} dB</span></div> row.innerHTML = `<div class="${active ? 'text-[var(--lvl-bad)] font-medium' : 'text-[var(--text)]'}">${esc(e.rule_name || 'Alert')} <span class="text-xs text-[var(--text-dim)] font-mono">· ${m} ${e.threshold_db} dB</span></div>
<div class="text-xs text-[var(--text-dim)] font-mono mt-0.5">${when}${peak}</div>`; <div class="text-xs text-[var(--text-dim)] font-mono mt-0.5">${when}${peak}</div>`;
list.appendChild(row); list.appendChild(row);
} }
+3 -3
View File
@@ -96,9 +96,9 @@ function updateMarker(loc) {
const m = markersById[loc.id]; if (!m) return; const m = markersById[loc.id]; if (!m) return;
const st = liveState[loc.id]; const st = liveState[loc.id];
m.setStyle({ fillColor: levelColor(st) }); m.setStyle({ fillColor: levelColor(st) });
let label = `<b>${loc.name}</b>`; let label = `<b>${esc(loc.name)}</b>`;
if (st) { if (st) {
if (st.status === 'measuring') label += ` &middot; ${st.leqStr} dB Leq`; if (st.status === 'measuring') label += ` &middot; ${esc(st.leqStr)} dB Leq`;
else if (st.status === 'stopped') label += ' &middot; stopped'; else if (st.status === 'stopped') label += ' &middot; stopped';
else if (st.status === 'nodevice') label += ' &middot; no device'; else if (st.status === 'nodevice') label += ' &middot; no device';
else label += ' &middot; offline'; else label += ' &middot; offline';
@@ -180,7 +180,7 @@ if (withCoords.length) {
markersById[l.id] = L.circleMarker([la, lo], { markersById[l.id] = L.circleMarker([la, lo], {
radius: 7, fillColor: levelColor(liveState[l.id]), color: '#fff', radius: 7, fillColor: levelColor(liveState[l.id]), color: '#fff',
weight: 2, opacity: 0.9, fillOpacity: 0.95, weight: 2, opacity: 0.9, fillOpacity: 0.95,
}).addTo(map).bindTooltip(l.name, { direction: 'top', offset: [0, -6] }); }).addTo(map).bindTooltip(esc(l.name), { direction: 'top', offset: [0, -6] });
pts.push([la, lo]); pts.push([la, lo]);
} }
}); });