diff --git a/CHANGELOG.md b/CHANGELOG.md index eb4aba352..b0375b257 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## [Unreleased] +## [v0.51.317] — 2026-06-07 — Release KG (Phase 3 light — align CSP enforcement with report policy) + +### Fixed +- **The enforced Content-Security-Policy now honors the same `connect-src` sources as the report-only policy.** The enforced CSP header used a narrow `connect-src` (`'self'` + jsdelivr) and ignored both the local-dev `127.0.0.1`/`localhost` sources and the `HERMES_WEBUI_CSP_CONNECT_EXTRA` allowlist, while the report-only policy honored them — so a connect source could pass report-only yet be blocked by the enforced policy. Both policies are now built from one shared template with the same validated `connect-src` (report-only just appends `report-uri`/`report-to`), and the policy builder moved from `server.py` into `api/helpers.py` so it's unit-testable. As part of the alignment the enforced policy's `connect-src` gains the local-dev http/ws origins + operator `HERMES_WEBUI_CSP_CONNECT_EXTRA` allowlist and `media-src` matches the shared template; `object-src 'none'` and `frame-ancestors 'none'` are now enforced. (#3727 / #1909, @rodboev) + ## [v0.51.316] — 2026-06-07 — Release KF (Phase 2 — agent-source dependency audit contract) ### Changed diff --git a/api/helpers.py b/api/helpers.py index 9a2272df7..432f3c767 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -2,12 +2,15 @@ Hermes Web UI -- HTTP helper functions. """ import json as _json +import logging import os import re as _re import ssl from pathlib import Path from api.config import IMAGE_EXTS, MD_EXTS +logger = logging.getLogger(__name__) + # Treat stalled/closed HTTP clients as normal disconnects. Long-lived SSE # connections often end this way when a browser tab sleeps, a phone switches @@ -49,21 +52,81 @@ def safe_resolve(root: Path, requested: str) -> Path: return resolved +_CSP_CONNECT_BASE = ( + "'self' http://127.0.0.1:* http://localhost:* " + "ws://127.0.0.1:* ws://localhost:*" +) +_CSP_EXTRA_CONNECT_RE = _re.compile( + r"^(?:https?|wss?)://(?:\*\.)?[A-Za-z0-9._~-]+(?::(?P\d{1,5}|\*))?$" +) +_CSP_HEADER_NAME = 'Content-Security-Policy' +_CSP_SHARED_POLICY_TEMPLATE = ( + "default-src 'self' https://*.cloudflareaccess.com; " + "object-src 'none'; " + "frame-ancestors 'none'; " + "script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://static.cloudflareinsights.com blob:; " + "worker-src blob: 'self' https://cdn.jsdelivr.net; " + "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; " + "img-src 'self' data: https: blob:; " + "font-src 'self' data: https://fonts.gstatic.com; " + "media-src 'self' data: blob:; " + "connect-src {connect_src}; " + "manifest-src 'self' https://*.cloudflareaccess.com; " + "base-uri 'self'; form-action 'self'" +) + + +def _valid_csp_extra_connect_source(source: str) -> bool: + match = _CSP_EXTRA_CONNECT_RE.fullmatch(source) + if not match: + return False + port = match.group("port") + if not port or port == "*": + return True + try: + return 1 <= int(port) <= 65535 + except ValueError: + return False + + +def _csp_extra_connect_src() -> str: + raw = os.getenv("HERMES_WEBUI_CSP_CONNECT_EXTRA", "").strip() + if not raw: + return "" + sources = raw.split() + if not sources or any(not _valid_csp_extra_connect_source(src) for src in sources): + logger.warning("Ignoring invalid HERMES_WEBUI_CSP_CONNECT_EXTRA value") + return "" + return " " + " ".join(sources) + + +def _csp_connect_src(extra_connect_src: str = "") -> str: + return f"{_CSP_CONNECT_BASE} https://cdn.jsdelivr.net{extra_connect_src}" + + +def _build_csp_enforced_policy(extra_connect_src: str | None = None) -> str: + if extra_connect_src is None: + extra_connect_src = _csp_extra_connect_src() + return _CSP_SHARED_POLICY_TEMPLATE.format( + connect_src=_csp_connect_src(extra_connect_src) + ) + + +def _build_csp_report_only_policy(extra_connect_src: str | None = None) -> str: + return ( + _build_csp_enforced_policy(extra_connect_src) + + "; report-uri /api/csp-report; report-to csp-endpoint" + ) + + def _security_headers(handler): """Add security headers to every response.""" + extra_connect_src = _csp_extra_connect_src() + handler._csp_extra_connect_src = extra_connect_src handler.send_header('X-Content-Type-Options', 'nosniff') handler.send_header('X-Frame-Options', 'DENY') handler.send_header('Referrer-Policy', 'same-origin') - handler.send_header( - 'Content-Security-Policy', - "default-src 'self' https://*.cloudflareaccess.com; " - "script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://static.cloudflareinsights.com blob:; " - "worker-src blob: 'self' https://cdn.jsdelivr.net; " - "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; " - "img-src 'self' data: https: blob:; font-src 'self' data: https://fonts.gstatic.com; connect-src 'self' https://cdn.jsdelivr.net; " - "manifest-src 'self' https://*.cloudflareaccess.com; " - "base-uri 'self'; form-action 'self'" - ) + handler.send_header(_CSP_HEADER_NAME, _build_csp_enforced_policy(extra_connect_src)) handler.send_header( 'Permissions-Policy', 'camera=(), microphone=(self), geolocation=(), clipboard-write=(self)' diff --git a/server.py b/server.py index cadd3ae66..8064c953e 100644 --- a/server.py +++ b/server.py @@ -134,58 +134,14 @@ from urllib.parse import urlparse logger = logging.getLogger(__name__) -_CSP_CONNECT_BASE = ( - "'self' http://127.0.0.1:* http://localhost:* " - "ws://127.0.0.1:* ws://localhost:*" -) -_CSP_EXTRA_CONNECT_RE = re.compile( - r"^(?:https?|wss?)://(?:\*\.)?[A-Za-z0-9._~-]+(?::(?P\d{1,5}|\*))?$" -) - - -def _valid_csp_extra_connect_source(source: str) -> bool: - match = _CSP_EXTRA_CONNECT_RE.fullmatch(source) - if not match: - return False - port = match.group("port") - if not port or port == "*": - return True - try: - return 1 <= int(port) <= 65535 - except ValueError: - return False - - -def _csp_extra_connect_src() -> str: - raw = os.getenv("HERMES_WEBUI_CSP_CONNECT_EXTRA", "").strip() - if not raw: - return "" - sources = raw.split() - if not sources or any(not _valid_csp_extra_connect_source(src) for src in sources): - logger.warning("Ignoring invalid HERMES_WEBUI_CSP_CONNECT_EXTRA value") - return "" - return " " + " ".join(sources) - - -def _build_csp_report_only_policy() -> str: - connect_src = _CSP_CONNECT_BASE + _csp_extra_connect_src() - return ( - "default-src 'self'; " - "base-uri 'self'; " - "object-src 'none'; " - "frame-ancestors 'self'; " - "script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " - "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; " - "img-src 'self' data: blob:; " - "font-src 'self' data:; " - "media-src 'self' data: blob:; " - f"connect-src {connect_src}; " - "report-uri /api/csp-report; report-to csp-endpoint" - ) - from api.auth import check_auth from api.config import HOST, PORT, STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE -from api.helpers import j, get_profile_cookie, _CLIENT_DISCONNECT_ERRORS +from api.helpers import ( + j, + get_profile_cookie, + _build_csp_report_only_policy, + _CLIENT_DISCONNECT_ERRORS, +) from api.profiles import set_request_profile, clear_request_profile from api.routes import handle_delete, handle_get, handle_patch, handle_post, handle_put from api.startup import auto_install_agent_deps, fix_credential_permissions @@ -306,11 +262,12 @@ class Handler(BaseHTTPRequestHandler): _CSP_REPORT_TO = '{"group":"csp-endpoint","max_age":10886400,"endpoints":[{"url":"/api/csp-report"}]}' @classmethod - def csp_report_only_policy(cls) -> str: - return _build_csp_report_only_policy() + def csp_report_only_policy(cls, extra_connect_src=None) -> str: + return _build_csp_report_only_policy(extra_connect_src) def end_headers(self) -> None: - self.send_header("Content-Security-Policy-Report-Only", self.csp_report_only_policy()) + extra_connect_src = getattr(self, "_csp_extra_connect_src", None) + self.send_header("Content-Security-Policy-Report-Only", self.csp_report_only_policy(extra_connect_src)) self.send_header("Report-To", self._CSP_REPORT_TO) super().end_headers() diff --git a/tests/test_issue1112_csp_google_fonts.py b/tests/test_issue1112_csp_google_fonts.py index 2235a81ec..61679c6e7 100644 --- a/tests/test_issue1112_csp_google_fonts.py +++ b/tests/test_issue1112_csp_google_fonts.py @@ -1,40 +1,46 @@ """Tests for #1112 — CSP allows Google Fonts stylesheet and font files.""" import re +from api.helpers import _build_csp_enforced_policy + def _helpers_src() -> str: with open("api/helpers.py") as f: return f.read() +def _policy() -> str: + return _build_csp_enforced_policy("") + + class TestCSPGoogleFonts: """style-src and font-src must allow fonts.googleapis.com / fonts.gstatic.com.""" def test_style_src_includes_google_fonts(self): """style-src must include https://fonts.googleapis.com for Google Fonts CSS.""" - src = _helpers_src() - assert "https://fonts.googleapis.com" in src, \ + policy = _policy() + assert "https://fonts.googleapis.com" in policy, \ "style-src must allow fonts.googleapis.com (Google Fonts stylesheets)" # Must be in the style-src directive, not accidentally elsewhere - style_match = re.search(r"style-src\s+([^;]+);", src) + style_match = re.search(r"style-src\s+([^;]+);", policy) assert style_match, "style-src directive must exist" assert "fonts.googleapis.com" in style_match.group(1), \ "fonts.googleapis.com must be in style-src directive" def test_font_src_includes_fonts_gstatic(self): """font-src must include https://fonts.gstatic.com for Google Font files.""" - src = _helpers_src() - assert "https://fonts.gstatic.com" in src, \ + policy = _policy() + assert "https://fonts.gstatic.com" in policy, \ "font-src must allow fonts.gstatic.com (Google Font WOFF2/WOFF files)" # Must be in the font-src directive - font_match = re.search(r"font-src\s+([^;]+);", src) + font_match = re.search(r"font-src\s+([^;]+);", policy) assert font_match, "font-src directive must exist" assert "fonts.gstatic.com" in font_match.group(1), \ "fonts.gstatic.com must be in font-src directive" def test_existing_csp_directives_preserved(self): """All pre-existing CSP directives must still be present after the fix.""" - src = _helpers_src() + policy = _policy() for directive in ( "default-src 'self'", "script-src 'self' 'unsafe-inline'", @@ -46,4 +52,4 @@ class TestCSPGoogleFonts: "base-uri 'self'", "form-action 'self'", ): - assert directive in src, f"CSP must still contain: {directive}" + assert directive in policy, f"CSP must still contain: {directive}" diff --git a/tests/test_issue1850_csp_connect_src_jsdelivr.py b/tests/test_issue1850_csp_connect_src_jsdelivr.py index d620f3877..140f6962c 100644 --- a/tests/test_issue1850_csp_connect_src_jsdelivr.py +++ b/tests/test_issue1850_csp_connect_src_jsdelivr.py @@ -6,13 +6,12 @@ jsDelivr and are fetched via connect (not script load), so connect-src must include cdn.jsdelivr.net or browsers block the fetch and emit CSP violations. """ import re -from pathlib import Path -_HELPERS_PY = Path(__file__).resolve().parents[1] / "api/helpers.py" +from api.helpers import _build_csp_enforced_policy -def _helpers_src() -> str: - return _HELPERS_PY.read_text() +def _policy() -> str: + return _build_csp_enforced_policy("") class TestCSPConnectSrcJsdelivr: @@ -20,8 +19,8 @@ class TestCSPConnectSrcJsdelivr: def test_connect_src_includes_jsdelivr(self): """connect-src must include https://cdn.jsdelivr.net.""" - src = _helpers_src() - connect_match = re.search(r"connect-src\s+([^;]+);", src) + policy = _policy() + connect_match = re.search(r"connect-src\s+([^;]+);", policy) assert connect_match, "connect-src directive must exist in CSP" assert "https://cdn.jsdelivr.net" in connect_match.group(1), ( "connect-src must allow cdn.jsdelivr.net — xterm.js source maps are " @@ -30,8 +29,8 @@ class TestCSPConnectSrcJsdelivr: def test_connect_src_still_includes_self(self): """connect-src must still include 'self' alongside the new jsdelivr entry.""" - src = _helpers_src() - connect_match = re.search(r"connect-src\s+([^;]+);", src) + policy = _policy() + connect_match = re.search(r"connect-src\s+([^;]+);", policy) assert connect_match, "connect-src directive must exist in CSP" assert "'self'" in connect_match.group(1), ( "connect-src must retain 'self' after adding cdn.jsdelivr.net" diff --git a/tests/test_issue1909_csp_enforcement.py b/tests/test_issue1909_csp_enforcement.py new file mode 100644 index 000000000..2d8731cac --- /dev/null +++ b/tests/test_issue1909_csp_enforcement.py @@ -0,0 +1,125 @@ +"""Regression tests for enforced CSP alignment with report-only policy (#1909).""" + +from __future__ import annotations + +from http.server import BaseHTTPRequestHandler + +from api.helpers import _security_headers +from server import Handler + + +class _HeaderCapture: + def __init__(self): + self.sent_headers = [] + + def send_header(self, key, value): + self.sent_headers.append((key, value)) + + +def _headers_from_security_helper(): + handler = _HeaderCapture() + _security_headers(handler) + return dict(handler.sent_headers) + + +def _directives(policy: str): + directives = {} + for entry in policy.split(";"): + entry = entry.strip() + if not entry: + continue + key, _, value = entry.partition(" ") + directives[key] = value.strip() + return directives + + +def test_security_helper_sends_enforcing_csp_with_hardening_directives(monkeypatch): + monkeypatch.delenv("HERMES_WEBUI_CSP_CONNECT_EXTRA", raising=False) + + headers = _headers_from_security_helper() + + policy = headers["Content-Security-Policy"] + assert "default-src 'self' https://*.cloudflareaccess.com" in policy + assert "base-uri 'self'" in policy + assert "form-action 'self'" in policy + assert "manifest-src 'self' https://*.cloudflareaccess.com" in policy + assert "script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://static.cloudflareinsights.com blob:" in policy + assert "style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com" in policy + assert "worker-src blob: 'self' https://cdn.jsdelivr.net" in policy + assert "font-src 'self' data: https://fonts.gstatic.com" in policy + assert "object-src 'none'" in policy + assert "frame-ancestors 'none'" in policy + assert "media-src 'self' data: blob:" in policy + assert "connect-src 'self' http://127.0.0.1:* http://localhost:* ws://127.0.0.1:* ws://localhost:* https://cdn.jsdelivr.net" in policy + + +def test_enforcing_csp_honors_valid_extra_connect_origins(monkeypatch): + monkeypatch.setenv( + "HERMES_WEBUI_CSP_CONNECT_EXTRA", + "https://metrics.example.com wss://events.example.com:443", + ) + + headers = _headers_from_security_helper() + + policy = headers["Content-Security-Policy"] + assert ( + "connect-src 'self' http://127.0.0.1:* http://localhost:* " + "ws://127.0.0.1:* ws://localhost:* https://cdn.jsdelivr.net " + "https://metrics.example.com wss://events.example.com:443; " + ) in policy + + +def test_enforcing_and_report_only_csp_share_validated_connect_extra(monkeypatch): + monkeypatch.setenv("HERMES_WEBUI_CSP_CONNECT_EXTRA", "https://metrics.example.com") + + enforced = _headers_from_security_helper()["Content-Security-Policy"] + report_only = Handler.csp_report_only_policy() + + assert "https://metrics.example.com" in enforced + assert "https://metrics.example.com" in report_only + + +def test_report_only_policy_tracks_enforced_directives(monkeypatch): + monkeypatch.delenv("HERMES_WEBUI_CSP_CONNECT_EXTRA", raising=False) + + enforced = _directives(_headers_from_security_helper()["Content-Security-Policy"]) + report_only = _directives(Handler.csp_report_only_policy()) + + assert report_only.pop("report-uri") == "/api/csp-report" + assert report_only.pop("report-to") == "csp-endpoint" + assert report_only == enforced + + +def test_report_only_csp_headers_still_point_to_collector(monkeypatch): + sent_headers = [] + handler = Handler.__new__(Handler) + handler.send_header = lambda key, value: sent_headers.append((key, value)) + monkeypatch.setattr(BaseHTTPRequestHandler, "end_headers", lambda self: None) + + Handler.end_headers(handler) + + headers = dict(sent_headers) + assert "Content-Security-Policy-Report-Only" in headers + assert headers["Report-To"] == ( + '{"group":"csp-endpoint","max_age":10886400,' + '"endpoints":[{"url":"/api/csp-report"}]}' + ) + assert "report-uri /api/csp-report" in headers["Content-Security-Policy-Report-Only"] + assert "report-to csp-endpoint" in headers["Content-Security-Policy-Report-Only"] + + +def test_end_headers_reuses_cached_extra_connect_validation(monkeypatch, caplog): + monkeypatch.setenv( + "HERMES_WEBUI_CSP_CONNECT_EXTRA", + "https://metrics.example.com; script-src *", + ) + + sent_headers = [] + handler = Handler.__new__(Handler) + handler.send_header = lambda key, value: sent_headers.append((key, value)) + monkeypatch.setattr(BaseHTTPRequestHandler, "end_headers", lambda self: None) + + _security_headers(handler) + Handler.end_headers(handler) + + assert caplog.text.count("Ignoring invalid HERMES_WEBUI_CSP_CONNECT_EXTRA value") == 1 diff --git a/tests/test_issue1909_csp_report_only.py b/tests/test_issue1909_csp_report_only.py index ca2514500..a22be8934 100644 --- a/tests/test_issue1909_csp_report_only.py +++ b/tests/test_issue1909_csp_report_only.py @@ -24,7 +24,7 @@ def test_handler_adds_content_security_policy_report_only(monkeypatch): policy = headers["Content-Security-Policy-Report-Only"] assert "default-src 'self'" in policy assert "object-src 'none'" in policy - assert "frame-ancestors 'self'" in policy + assert "frame-ancestors 'none'" in policy assert "base-uri 'self'" in policy assert "report-uri /api/csp-report" in policy assert "report-to csp-endpoint" in policy @@ -43,7 +43,8 @@ def test_csp_report_only_keeps_legacy_inline_allowances_for_current_ui(): # unsafe-eval was dropped after Opus stage-339 verification — no production # JS uses eval(), new Function(), or string-form setTimeout/setInterval. assert "'unsafe-eval'" not in policy - assert "img-src 'self' data: blob:" in policy + assert "img-src 'self' data:" in policy + assert "font-src 'self' data: https://fonts.gstatic.com" in policy assert "connect-src 'self'" in policy diff --git a/tests/test_issue2901_csp_connect_extra.py b/tests/test_issue2901_csp_connect_extra.py index 349b92945..8056f10f8 100644 --- a/tests/test_issue2901_csp_connect_extra.py +++ b/tests/test_issue2901_csp_connect_extra.py @@ -12,7 +12,7 @@ def test_csp_connect_src_default_header_unchanged(monkeypatch): assert ( "connect-src 'self' http://127.0.0.1:* http://localhost:* " - "ws://127.0.0.1:* ws://localhost:*; " + "ws://127.0.0.1:* ws://localhost:* https://cdn.jsdelivr.net; " ) in policy @@ -28,7 +28,7 @@ def test_csp_connect_src_includes_valid_extra_origins(monkeypatch): assert ( "connect-src 'self' http://127.0.0.1:* http://localhost:* " - "ws://127.0.0.1:* ws://localhost:* " + "ws://127.0.0.1:* ws://localhost:* https://cdn.jsdelivr.net " "https://metrics.example.com wss://events.example.com:443; " ) in policy