From 4f12e344a8e4fcd22d5b10b3625c94770e48512c Mon Sep 17 00:00:00 2001 From: m17hr1l Date: Sun, 7 Jun 2026 14:23:55 +0200 Subject: [PATCH] xss fixes from audit: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1 case_detail.html: scheme-check source_ref href (block javascript: URLs) - F2 admin.html / F3 admin_federation.html: replace inline onsubmit confirm() with data-attr + global handler in base.html (no more label/domain interpolation into onsubmit attribute string) - federation.register_peer: validate hostname + fingerprint regex at ingest - federation_explore.html: window.PSYC_EXPLORE via | tojson - federation_network.js: DOMAIN_RE guard on peer-supplied domain before building cross-origin fetch URL (also closes open-redirect via 'open their explorer' button) - app.py: nosniff + Referrer-Policy: no-referrer + X-Frame-Options: DENY - sw.js: psyc-v11 cache bump CSP deferred — needs inline scripts moved to external files first. Tests: +2 cases, 245/245 green. --- src/psyc/cockpit/app.py | 13 ++++++++ src/psyc/cockpit/static/federation_network.js | 18 ++++++++++- src/psyc/cockpit/static/sw.js | 2 +- src/psyc/cockpit/templates/admin.html | 2 +- .../cockpit/templates/admin_federation.html | 2 +- src/psyc/cockpit/templates/base.html | 15 ++++++++++ src/psyc/cockpit/templates/case_detail.html | 2 +- .../cockpit/templates/federation_explore.html | 9 ++++-- src/psyc/lines/federation.py | 18 ++++++++++- tests/test_federation.py | 30 +++++++++++++++++++ 10 files changed, 102 insertions(+), 9 deletions(-) diff --git a/src/psyc/cockpit/app.py b/src/psyc/cockpit/app.py index 14196f4..b1c3b08 100644 --- a/src/psyc/cockpit/app.py +++ b/src/psyc/cockpit/app.py @@ -35,6 +35,19 @@ app = FastAPI(title="psyc Operations Cockpit", version="0.1.0") app.add_middleware(SessionMiddleware, secret_key=adminauth.session_secret(), max_age=3600) app.mount("/static", StaticFiles(directory=str(HERE / "static")), name="static") + +@app.middleware("http") +async def _security_headers(request: Request, call_next): + """Defense-in-depth headers. CSP is intentionally NOT set yet — the + cockpit currently uses inline scripts in base.html / journey.html / + federation_explore.html which would need nonces or extraction first.""" + resp = await call_next(request) + resp.headers.setdefault("X-Content-Type-Options", "nosniff") + resp.headers.setdefault("Referrer-Policy", "no-referrer") + resp.headers.setdefault("X-Frame-Options", "DENY") + return resp + + pulse_routes.register(app, TEMPLATES) federation_routes.register(app, TEMPLATES) diff --git a/src/psyc/cockpit/static/federation_network.js b/src/psyc/cockpit/static/federation_network.js index 8aded76..be22b03 100644 --- a/src/psyc/cockpit/static/federation_network.js +++ b/src/psyc/cockpit/static/federation_network.js @@ -673,11 +673,21 @@ // them from any cockpit page. The fetch is best-effort — if the peer is // unreachable, blocked by CSP, or older, we render whatever we got. + // Reject anything that isn't a bare hostname (+ optional port). Stops a + // hostile peer-supplied `domain` value from steering a click to an + // attacker-controlled URL via path/query injection or a different scheme. + const DOMAIN_RE = /^[A-Za-z0-9._\-]+(:\d{1,5})?$/; + async function fetchPeerSelfView(domain, expectedFp) { const sec = detailEl.querySelector(`.fn-remote-sec[data-remote-fp="${expectedFp}"]`); if (!sec) return; const statusEl = sec.querySelector(".fn-remote-status"); const bodyEl = sec.querySelector(".fn-remote-body"); + if (!DOMAIN_RE.test(domain || "")) { + statusEl.textContent = "blocked"; + bodyEl.innerHTML = `peer domain failed hostname validation — not fetching.`; + return; + } const base = `https://${domain}`; let data = null; let kind = ""; @@ -789,7 +799,13 @@ if (!sec) return; const statusEl = sec.querySelector(".fn-topo-status"); const bodyEl = sec.querySelector(".fn-topology-body"); - // Empty domain → same-origin fetch for SELF. Otherwise cross-origin. + // Empty domain → same-origin fetch for SELF. Otherwise cross-origin — + // guard against a poisoned peer-supplied domain steering elsewhere. + if (domain && !DOMAIN_RE.test(domain)) { + statusEl.textContent = "blocked"; + bodyEl.innerHTML = `peer domain failed hostname validation — not fetching topology.`; + return; + } const base = domain ? `https://${domain}` : ""; let data = null; try { diff --git a/src/psyc/cockpit/static/sw.js b/src/psyc/cockpit/static/sw.js index 5e0dd8b..dff3c9b 100644 --- a/src/psyc/cockpit/static/sw.js +++ b/src/psyc/cockpit/static/sw.js @@ -5,7 +5,7 @@ // This makes the cockpit installable as a PWA and survives flaky connections, // without serving stale operational data behind the operator's back. -const CACHE_VERSION = "psyc-v10"; +const CACHE_VERSION = "psyc-v11"; const STATIC_ASSETS = [ "/static/cockpit.css", "/static/psyc-tokens.css", diff --git a/src/psyc/cockpit/templates/admin.html b/src/psyc/cockpit/templates/admin.html index 7660ed1..a25988c 100644 --- a/src/psyc/cockpit/templates/admin.html +++ b/src/psyc/cockpit/templates/admin.html @@ -37,7 +37,7 @@ {{ ((m.last_used or '')[:16] | replace('T', ' ')) or '—' }}
+ data-confirm-revoke="member" data-confirm-name="{{ m.label }}">
diff --git a/src/psyc/cockpit/templates/admin_federation.html b/src/psyc/cockpit/templates/admin_federation.html index 58e8fbe..db285d2 100644 --- a/src/psyc/cockpit/templates/admin_federation.html +++ b/src/psyc/cockpit/templates/admin_federation.html @@ -71,7 +71,7 @@
+ data-confirm-revoke="peer" data-confirm-name="{{ p.domain }}">
diff --git a/src/psyc/cockpit/templates/base.html b/src/psyc/cockpit/templates/base.html index ffb84a3..71ca55d 100644 --- a/src/psyc/cockpit/templates/base.html +++ b/src/psyc/cockpit/templates/base.html @@ -39,6 +39,21 @@ btn.setAttribute("aria-expanded", "false"); })); }); + // data-driven confirms (used by /admin and /admin/federation revoke/remove + // buttons; replaces inline onsubmit which was XSS-vulnerable when the + // confirm prompt interpolated a member label or peer domain). + document.addEventListener("DOMContentLoaded", () => { + document.querySelectorAll("form[data-confirm-revoke]").forEach(form => { + form.addEventListener("submit", ev => { + const kind = form.getAttribute("data-confirm-revoke") || "item"; + const name = form.getAttribute("data-confirm-name") || ""; + const msg = kind === "peer" + ? `Remove ${name}? Their signals will no longer count toward quorum.` + : `Revoke ${name}? Their codes stop working immediately.`; + if (!confirm(msg)) ev.preventDefault(); + }); + }); + }); diff --git a/src/psyc/cockpit/templates/case_detail.html b/src/psyc/cockpit/templates/case_detail.html index f597b9a..afa68b8 100644 --- a/src/psyc/cockpit/templates/case_detail.html +++ b/src/psyc/cockpit/templates/case_detail.html @@ -42,7 +42,7 @@

Source

Type
{{ case.source_type }}
-
Reference
{% if case.source_ref %}{{ case.source_ref }}{% else %}—{% endif %}
+
Reference
{% if case.source_ref %}{% if case.source_ref.startswith(('http://', 'https://')) %}{{ case.source_ref }}{% else %}{{ case.source_ref }}{% endif %}{% else %}—{% endif %}
Observed
{{ case.observed_at.strftime('%Y-%m-%d %H:%M UTC') }}
Ingested
{{ case.ingested_at.strftime('%Y-%m-%d %H:%M UTC') }}
diff --git a/src/psyc/cockpit/templates/federation_explore.html b/src/psyc/cockpit/templates/federation_explore.html index f38fe22..380b83d 100644 --- a/src/psyc/cockpit/templates/federation_explore.html +++ b/src/psyc/cockpit/templates/federation_explore.html @@ -134,10 +134,13 @@ diff --git a/src/psyc/lines/federation.py b/src/psyc/lines/federation.py index 059aa9d..62728ca 100644 --- a/src/psyc/lines/federation.py +++ b/src/psyc/lines/federation.py @@ -16,9 +16,15 @@ import base64 import hashlib import json import os +import re from datetime import datetime, timedelta, timezone from typing import Any, Dict, List, Optional, Tuple +# Hostname-with-optional-port pattern for peer domains. Reject anything else at +# registration so a hostile domain string can't reach a render context where +# it could break out of an HTML attr or JS string. +_DOMAIN_RE = re.compile(r"^[A-Za-z0-9._\-]+(:\d{1,5})?$") + from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import ed25519 from pydantic import BaseModel, Field @@ -420,7 +426,17 @@ def _row_to_peer(row: Dict[str, Any]) -> Peer: def register_peer(domain: str, fingerprint: str, pubkey_pem: str, status: str = "unknown") -> None: - """Insert or update a peer in the registry. Idempotent on `domain`.""" + """Insert or update a peer in the registry. Idempotent on `domain`. + + Rejects malformed domain strings — only hostname chars + optional :port. + Closes a stored-XSS hole where a hostile `domain` would have been rendered + into the admin federation page's confirm() prompt. + """ + domain = (domain or "").strip() + if not _DOMAIN_RE.match(domain): + raise ValueError(f"invalid domain: {domain!r}") + if fingerprint and not re.fullmatch(r"[0-9a-fA-F]{32}", fingerprint): + raise ValueError(f"invalid fingerprint: {fingerprint!r}") now = datetime.now(timezone.utc).isoformat() existing = db.get_peer(domain) discovered_at = existing["discovered_at"] if existing else now diff --git a/tests/test_federation.py b/tests/test_federation.py index b04fddb..b693621 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -230,3 +230,33 @@ def test_peer_registry_crud(fresh_db, fed_dir): federation.remove_peer("peer.example") assert federation.list_peers() == [] + + +def test_register_peer_rejects_malformed_domain(fresh_db, fed_dir): + """XSS guard: domain must look like a hostname (+ optional :port).""" + import pytest + bad = [ + "evil.com'); alert(1); //", + "evil.com