Browse Source

fix(security): allow iframe embedding from trusted origins via env var (#1191)

  Bambuddy ships strict anti-clickjacking headers (X-Frame-Options:
  SAMEORIGIN + CSP frame-ancestors 'none') by default. Internet-exposed
  deployments need this; same-LAN HA Webpage-panel users do not, and
  SAMEORIGIN is port-strict so HA on :8123 + Bambuddy on :8000 always
  fails. azurusnova hit exactly that case.

  Add TRUSTED_FRAME_ORIGINS env var (comma-separated scheme://host[:port]).
  When set, drop X-Frame-Options entirely (modern browsers honor
  frame-ancestors and the legacy ALLOW-FROM syntax is deprecated /
  inconsistent across vendors) and emit "frame-ancestors 'self' <list>"
  on every CSP-bearing route. Origin validation is strict: only http(s),
  no paths, no query/fragment, no wildcards. Bad entries get a warning
  and are dropped — startup never fails.

  Default behaviour (no env var) is unchanged: X-Frame-Options:
  SAMEORIGIN + frame-ancestors 'none', so existing Docker / bare-metal
  deployments are not affected.
maziggy 3 weeks ago
parent
commit
b02350d423
4 changed files with 251 additions and 7 deletions
  1. 8 0
      .env.example
  2. 0 0
      CHANGELOG.md
  3. 81 7
      backend/app/main.py
  4. 162 0
      backend/tests/integration/test_security_headers.py

+ 8 - 0
.env.example

@@ -16,3 +16,11 @@ LOG_TO_FILE=true
 # and these values override any database settings (read-only in UI)
 # and these values override any database settings (read-only in UI)
 # HA_URL=http://supervisor/core
 # HA_URL=http://supervisor/core
 # HA_TOKEN=your-long-lived-access-token
 # HA_TOKEN=your-long-lived-access-token
+
+# Trusted iframe origins (#1191) — comma-separated list of scheme://host[:port]
+# origins permitted to embed Bambuddy via <iframe>. Defaults to empty (strict:
+# only same-origin embedding allowed). Set this to your Home Assistant origin
+# when using the HA Webpage dashboard panel, since HA on port 8123 and Bambuddy
+# on port 8000 are different origins to the browser. Wildcards, paths, and
+# non-http(s) schemes are rejected at startup with a warning.
+# TRUSTED_FRAME_ORIGINS=http://homeassistant.local:8123

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 81 - 7
backend/app/main.py

@@ -1,11 +1,13 @@
 import asyncio
 import asyncio
 import logging
 import logging
 import mimetypes as _mimetypes
 import mimetypes as _mimetypes
+import os
 import posixpath
 import posixpath
 import time
 import time
 from contextlib import asynccontextmanager
 from contextlib import asynccontextmanager
 from datetime import datetime, timedelta, timezone
 from datetime import datetime, timedelta, timezone
 from logging.handlers import RotatingFileHandler
 from logging.handlers import RotatingFileHandler
+from urllib.parse import urlparse
 
 
 from fastapi import FastAPI
 from fastapi import FastAPI
 from fastapi.responses import FileResponse
 from fastapi.responses import FileResponse
@@ -4538,12 +4540,87 @@ PUBLIC_API_PATTERNS = [
 ]
 ]
 
 
 
 
+_security_headers_logger = logging.getLogger("backend.app.main.security_headers")
+
+
+def _parse_trusted_frame_origins() -> tuple[str, ...]:
+    """Parse TRUSTED_FRAME_ORIGINS env var into a validated allowlist (#1191).
+
+    Format: comma-separated list of ``scheme://host[:port]`` origins.
+
+    Used by ``security_headers_middleware`` to relax ``frame-ancestors`` for
+    trusted same-LAN deployments (e.g. Home Assistant Webpage panel embedding
+    Bambuddy from a different port). Defaults to empty — strict ``'none'``.
+
+    Invalid entries are dropped with a warning rather than failing startup, so
+    a typo in one origin doesn't take the whole deployment down.
+    """
+    raw = os.environ.get("TRUSTED_FRAME_ORIGINS", "").strip()
+    if not raw:
+        return ()
+    valid: list[str] = []
+    for item in raw.split(","):
+        candidate = item.strip()
+        if not candidate:
+            continue
+        try:
+            parsed = urlparse(candidate)
+        except ValueError as e:
+            _security_headers_logger.warning("TRUSTED_FRAME_ORIGINS: dropping %r — %s", candidate, e)
+            continue
+        if parsed.scheme not in ("http", "https"):
+            _security_headers_logger.warning("TRUSTED_FRAME_ORIGINS: dropping %r — must be http(s)", candidate)
+            continue
+        if not parsed.netloc:
+            _security_headers_logger.warning("TRUSTED_FRAME_ORIGINS: dropping %r — missing host", candidate)
+            continue
+        if parsed.path and parsed.path != "/":
+            _security_headers_logger.warning("TRUSTED_FRAME_ORIGINS: dropping %r — paths not allowed", candidate)
+            continue
+        if parsed.query or parsed.fragment:
+            _security_headers_logger.warning(
+                "TRUSTED_FRAME_ORIGINS: dropping %r — query/fragment not allowed", candidate
+            )
+            continue
+        if "*" in parsed.netloc:
+            _security_headers_logger.warning("TRUSTED_FRAME_ORIGINS: dropping %r — wildcards not allowed", candidate)
+            continue
+        valid.append(f"{parsed.scheme}://{parsed.netloc}")
+    if valid:
+        _security_headers_logger.info("TRUSTED_FRAME_ORIGINS: %s", ", ".join(valid))
+    return tuple(valid)
+
+
+_TRUSTED_FRAME_ORIGINS: tuple[str, ...] = _parse_trusted_frame_origins()
+
+
+def _frame_ancestors(default_value: str) -> str:
+    """Compose the ``frame-ancestors`` CSP directive (#1191).
+
+    ``default_value`` is the strict directive used when the operator has not
+    configured ``TRUSTED_FRAME_ORIGINS`` — typically ``'none'`` (catch-all and
+    docs) or ``'self'`` (gcode-viewer, served same-origin). When trusted origins
+    are configured, ``'self'`` is always included so same-origin embedding never
+    breaks even if an operator forgets to add their own origin to the list.
+    """
+    if _TRUSTED_FRAME_ORIGINS:
+        return "frame-ancestors 'self' " + " ".join(_TRUSTED_FRAME_ORIGINS) + ";"
+    return f"frame-ancestors {default_value};"
+
+
 @app.middleware("http")
 @app.middleware("http")
 async def security_headers_middleware(request, call_next):
 async def security_headers_middleware(request, call_next):
     """Add standard HTTP security headers to every response."""
     """Add standard HTTP security headers to every response."""
     response = await call_next(request)
     response = await call_next(request)
     response.headers["X-Content-Type-Options"] = "nosniff"
     response.headers["X-Content-Type-Options"] = "nosniff"
-    response.headers["X-Frame-Options"] = "SAMEORIGIN"
+    # X-Frame-Options is the legacy cross-origin embedding control. Modern
+    # browsers honour CSP frame-ancestors instead, and the legacy
+    # `ALLOW-FROM <url>` syntax is deprecated and inconsistent across vendors.
+    # When operators have explicitly allowlisted trusted frame origins (#1191
+    # — typically Home Assistant on a different port), drop X-Frame-Options
+    # and let the CSP-side frame-ancestors directive govern embedding.
+    if not _TRUSTED_FRAME_ORIGINS:
+        response.headers["X-Frame-Options"] = "SAMEORIGIN"
     response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
     response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
     # Content-Security-Policy for the React SPA.
     # Content-Security-Policy for the React SPA.
     # Notes:
     # Notes:
@@ -4566,8 +4643,7 @@ async def security_headers_middleware(request, call_next):
             "font-src 'self' data: https://fonts.gstatic.com; "
             "font-src 'self' data: https://fonts.gstatic.com; "
             "object-src 'none'; "
             "object-src 'none'; "
             "base-uri 'self'; "
             "base-uri 'self'; "
-            "frame-src 'self' http: https:; "
-            "frame-ancestors 'self';"
+            "frame-src 'self' http: https:; " + _frame_ancestors("'self'")
         )
         )
     elif request.url.path in ("/docs", "/redoc", "/docs/oauth2-redirect"):
     elif request.url.path in ("/docs", "/redoc", "/docs/oauth2-redirect"):
         # FastAPI's built-in Swagger UI / ReDoc pages load assets from
         # FastAPI's built-in Swagger UI / ReDoc pages load assets from
@@ -4582,8 +4658,7 @@ async def security_headers_middleware(request, call_next):
             "font-src 'self' data: https://fonts.gstatic.com; "
             "font-src 'self' data: https://fonts.gstatic.com; "
             "worker-src 'self' blob:; "
             "worker-src 'self' blob:; "
             "object-src 'none'; "
             "object-src 'none'; "
-            "base-uri 'self'; "
-            "frame-ancestors 'none';"
+            "base-uri 'self'; " + _frame_ancestors("'none'")
         )
         )
     else:
     else:
         response.headers["Content-Security-Policy"] = (
         response.headers["Content-Security-Policy"] = (
@@ -4596,8 +4671,7 @@ async def security_headers_middleware(request, call_next):
             "font-src 'self' data: https://fonts.gstatic.com; "
             "font-src 'self' data: https://fonts.gstatic.com; "
             "object-src 'none'; "
             "object-src 'none'; "
             "base-uri 'self'; "
             "base-uri 'self'; "
-            "frame-src 'self' http: https:; "
-            "frame-ancestors 'none';"
+            "frame-src 'self' http: https:; " + _frame_ancestors("'none'")
         )
         )
     if request.url.scheme == "https":
     if request.url.scheme == "https":
         response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains"
         response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains"

+ 162 - 0
backend/tests/integration/test_security_headers.py

@@ -0,0 +1,162 @@
+"""Integration tests for security_headers_middleware (#1191).
+
+Default behaviour is strict: ``X-Frame-Options: SAMEORIGIN`` plus
+``frame-ancestors 'none'`` on the catch-all route, ``frame-ancestors 'self'``
+on /gcode-viewer/. Operators can opt into iframe embedding from trusted
+origins (e.g. Home Assistant on a different port) via the
+``TRUSTED_FRAME_ORIGINS`` env var; when set, X-Frame-Options is dropped and
+``frame-ancestors`` includes the allowlist.
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import AsyncClient
+
+# ─── helpers ──────────────────────────────────────────────────────────────
+
+
+def _parse_origins(value: str) -> tuple[str, ...]:
+    """Re-import the parser with a specific env var set, return its result.
+
+    Uses a fresh import so the module-level _TRUSTED_FRAME_ORIGINS is
+    re-evaluated against the patched os.environ.
+    """
+    import os
+
+    from backend.app import main as main_module
+
+    old = os.environ.get("TRUSTED_FRAME_ORIGINS")
+    try:
+        if value is None:
+            os.environ.pop("TRUSTED_FRAME_ORIGINS", None)
+        else:
+            os.environ["TRUSTED_FRAME_ORIGINS"] = value
+        # Function reads from os.environ each call.
+        return main_module._parse_trusted_frame_origins()
+    finally:
+        if old is None:
+            os.environ.pop("TRUSTED_FRAME_ORIGINS", None)
+        else:
+            os.environ["TRUSTED_FRAME_ORIGINS"] = old
+
+
+# ─── env-var parsing ──────────────────────────────────────────────────────
+
+
+class TestParseTrustedFrameOrigins:
+    """Unit tests for _parse_trusted_frame_origins."""
+
+    def test_empty_env_returns_empty_tuple(self):
+        assert _parse_origins("") == ()
+
+    def test_unset_env_returns_empty_tuple(self):
+        assert _parse_origins(None) == ()  # type: ignore[arg-type]
+
+    def test_single_origin(self):
+        assert _parse_origins("http://homeassistant.local:8123") == ("http://homeassistant.local:8123",)
+
+    def test_multiple_origins(self):
+        result = _parse_origins("http://homeassistant.local:8123,https://ha.example.com")
+        assert result == ("http://homeassistant.local:8123", "https://ha.example.com")
+
+    def test_whitespace_around_entries_stripped(self):
+        result = _parse_origins("  http://a.local:1 ,   https://b.local:2  ")
+        assert result == ("http://a.local:1", "https://b.local:2")
+
+    def test_empty_segment_skipped(self):
+        result = _parse_origins("http://a.local,,https://b.local")
+        assert result == ("http://a.local", "https://b.local")
+
+    def test_non_http_scheme_dropped(self):
+        # ftp://, javascript:, file:// etc. — never a valid frame ancestor.
+        assert _parse_origins("ftp://attacker.example,http://ok.local") == ("http://ok.local",)
+        assert _parse_origins("javascript:alert(1)") == ()
+
+    def test_missing_host_dropped(self):
+        # "http://" with no host
+        assert _parse_origins("http://") == ()
+
+    def test_path_dropped(self):
+        # frame-ancestors only takes scheme://host[:port], no path
+        assert _parse_origins("http://ha.local/dashboard") == ()
+
+    def test_query_or_fragment_dropped(self):
+        assert _parse_origins("http://ha.local?foo=1") == ()
+        assert _parse_origins("http://ha.local#frag") == ()
+
+    def test_wildcard_in_host_dropped(self):
+        # Wildcards would defeat the allowlist purpose; reject explicitly.
+        assert _parse_origins("http://*.example.com") == ()
+
+    def test_root_path_kept(self):
+        # Trailing slash is a degenerate but harmless path; treat as bare host.
+        assert _parse_origins("http://ha.local:8123/") == ("http://ha.local:8123",)
+
+
+# ─── HTTP integration: middleware emits expected headers ──────────────────
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_default_headers_strict(async_client: AsyncClient, monkeypatch):
+    """Without env var: X-Frame-Options=SAMEORIGIN and frame-ancestors 'none'."""
+    monkeypatch.delenv("TRUSTED_FRAME_ORIGINS", raising=False)
+    # Re-import the module-level constant so the middleware closes over the new value.
+    from backend.app import main as main_module
+
+    monkeypatch.setattr(main_module, "_TRUSTED_FRAME_ORIGINS", ())
+
+    resp = await async_client.get("/api/v1/auth/status")
+    assert resp.headers.get("X-Frame-Options") == "SAMEORIGIN"
+    assert "frame-ancestors 'none'" in resp.headers.get("Content-Security-Policy", "")
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_trusted_origins_relaxes_csp_and_drops_xfo(async_client: AsyncClient, monkeypatch):
+    """With env var set: X-Frame-Options is absent, frame-ancestors lists the origins."""
+    from backend.app import main as main_module
+
+    monkeypatch.setattr(
+        main_module,
+        "_TRUSTED_FRAME_ORIGINS",
+        ("http://homeassistant.local:8123",),
+    )
+
+    resp = await async_client.get("/api/v1/auth/status")
+    assert "X-Frame-Options" not in resp.headers
+    csp = resp.headers.get("Content-Security-Policy", "")
+    assert "frame-ancestors 'self' http://homeassistant.local:8123;" in csp
+    assert "'none'" not in csp.split("frame-ancestors")[1].split(";")[0]
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_trusted_origins_applies_to_docs_branch(async_client: AsyncClient, monkeypatch):
+    """The /docs CSP also honors the allowlist (consistent with main app)."""
+    from backend.app import main as main_module
+
+    monkeypatch.setattr(
+        main_module,
+        "_TRUSTED_FRAME_ORIGINS",
+        ("https://ha.example.com",),
+    )
+
+    resp = await async_client.get("/docs")
+    csp = resp.headers.get("Content-Security-Policy", "")
+    assert "frame-ancestors 'self' https://ha.example.com;" in csp
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_other_security_headers_unchanged(async_client: AsyncClient, monkeypatch):
+    """Other headers (X-Content-Type-Options, Referrer-Policy) are not affected."""
+    from backend.app import main as main_module
+
+    # Test in both modes — headers should be the same regardless.
+    for origins in [(), ("http://homeassistant.local:8123",)]:
+        monkeypatch.setattr(main_module, "_TRUSTED_FRAME_ORIGINS", origins)
+        resp = await async_client.get("/api/v1/auth/status")
+        assert resp.headers.get("X-Content-Type-Options") == "nosniff"
+        assert resp.headers.get("Referrer-Policy") == "strict-origin-when-cross-origin"

Some files were not shown because too many files changed in this diff