diff --git a/core/socketio_cors.py b/core/socketio_cors.py new file mode 100644 index 00000000..425a464e --- /dev/null +++ b/core/socketio_cors.py @@ -0,0 +1,256 @@ +"""Socket.IO CORS allow-list resolution + rejection logging. + +Three concerns lifted out of `web_server.py`: + +- :func:`resolve_cors_origins` — read the user's + ``security.cors_origins`` config setting (string, list, or unset) and + return what to hand to Flask-SocketIO's ``cors_allowed_origins`` + parameter: ``None`` (engineio same-origin default — the secure + default), the literal ``'*'`` (wildcard, opt-in), or a list of + explicit origin URLs. + +- :func:`will_reject` — predict whether engineio's CORS check will + reject a request, given the resolved allow-list, the request's + ``Origin`` header, and the request's ``Host`` header. Used to log a + helpful warning *before* engineio silently 403s a WebSocket upgrade. + (Without this, the user just sees a half-broken UI with no live + updates and nothing in the logs explaining why.) + +- :class:`RejectionLogger` — threadsafe dedup wrapper around the warning + emitter. Each unique origin is logged once per process so a malicious + site repeatedly hammering the WS endpoint can't spam logs. + +Pure logic, no Flask app dependency. Web_server.py imports these and +wires them into the SocketIO init + a Flask ``before_request`` hook. +""" + +from __future__ import annotations + +import threading +from typing import Any, List, Optional, Set, Union + + +# What ``cors_allowed_origins`` accepts and what we hand to Flask-SocketIO: +# +# - ``None`` → engineio's same-origin default. engineio computes the +# allowed origin list from the request itself: ``scheme://HTTP_HOST`` +# plus ``X-Forwarded-Proto://X-Forwarded-Host`` when those headers are +# present. Reverse proxies that set X-Forwarded-Host (Nginx with +# ``proxy_set_header X-Forwarded-Host`` — and Caddy/Traefik by default) +# work transparently. THE SECURE DEFAULT. +# +# - ``'*'`` → allow any origin. Insecure; opt-in only. +# +# - ``[origin, ...]`` → explicit allow-list. For setups whose Origin +# matches neither the backend's Host nor any forwarded header. +# +# IMPORTANT: do NOT use ``[]``. In engineio that means "disable CORS +# handling entirely" (server.py:202: ``if cors_allowed_origins != []:``) +# which is identical to the ``'*'`` wildcard from a security standpoint. +ResolvedOrigins = Union[List[str], str, None] + + +def resolve_cors_origins(config_manager: Any) -> ResolvedOrigins: + """Resolve the configured Socket.IO allow-list. + + Reads ``security.cors_origins`` from ``config_manager`` and normalizes + whatever shape the user typed (or didn't) into one of three values: + + - ``None`` (the secure default). Hand to Flask-SocketIO and engineio + enforces same-origin, with automatic support for X-Forwarded-Host + so reverse-proxy users don't need to configure anything. + - ``'*'`` — literal wildcard. Allows any origin. Insecure; opt-in. + - ``[origin, ...]`` — list of explicit origin URLs. For users behind + a proxy that doesn't send the forwarded headers OR for custom + contexts (Electron wrappers, browser extensions). + + Accepts the config value as either a string (comma OR newline + separated, since the settings UI is a textarea) or a list. Anything + else falls back to ``None`` — the secure default. + """ + raw = config_manager.get('security.cors_origins', None) if config_manager else None + if raw is None: + return None + if isinstance(raw, str): + if not raw.strip(): + return None + parts = [p.strip() for p in raw.replace('\n', ',').split(',')] + elif isinstance(raw, (list, tuple)): + # Drop non-string entries instead of stringifying — `[None]` would + # otherwise coerce to ``['None']`` and become a junk allow-list entry. + parts = [p.strip() for p in raw if isinstance(p, str)] + else: + return None + parts = [p for p in parts if p] + if not parts: + return None + if any(p == '*' for p in parts): + return '*' + return parts + + +def will_reject( + allowed: ResolvedOrigins, + origin: Optional[str], + host: str, + request_scheme: str = '', + forwarded_host: str = '', + forwarded_proto: str = '', +) -> bool: + """Predict whether engineio's CORS check will reject this request. + + Mirrors engineio's allow-list / same-origin logic so callers can log + a helpful warning *before* the rejection happens. Returns ``True`` + when the request will be rejected. + + Same-origin check: engineio builds full ``{scheme}://{host}`` strings + from the request URL — and adds a second candidate from the + forwarded headers when EITHER ``X-Forwarded-Proto`` OR + ``X-Forwarded-Host`` is present (engineio falls back to the request + Host / scheme for whichever forwarded header is missing). We mirror + that exactly. Comparing scheme matters: a TLS-terminating proxy can + leave the backend seeing ``http://soulsync.foo`` while the browser's + Origin is ``https://soulsync.foo`` — engineio treats those as + different strings and rejects, so we should too. + + Defensive against ``None`` / empty origin: returns ``False`` (allow), + matching engineio's actual behavior (server.py:207: ``if origin:`` + skips the validation block entirely when no Origin header is sent). + Browsers always send Origin for WebSocket upgrades, so this only + matters for non-browser clients like ``curl`` — which engineio + intentionally permits. + + ``request_scheme`` is required for an accurate same-origin match — + engineio compares full ``{scheme}://{host}`` strings, so callers + that omit it default to ``'http'``. Production wires Flask's + ``request.scheme`` here, which WSGI guarantees to be non-empty. + """ + if allowed == '*': + return False + if not origin: + return False # Engineio skips CORS validation when no Origin header + if isinstance(allowed, list) and origin in allowed: + return False + + # Engineio's same-origin check builds full {scheme}://{host} strings. + # Build the candidate set from the request + any forwarded headers. + candidates = [] + if host: + scheme = request_scheme or 'http' + candidates.append(f"{scheme}://{host}") + if forwarded_host or forwarded_proto: + # Mirror engineio: when EITHER forwarded header is present, build + # a candidate from both, falling back to the request value for + # whichever is missing. (engineio/base_server.py:_cors_allowed_origins.) + f_host = forwarded_host.split(',')[0].strip() if forwarded_host else host + if f_host: + f_scheme = (forwarded_proto.split(',')[0].strip() + if forwarded_proto + else (request_scheme or 'http')) + candidates.append(f"{f_scheme}://{f_host}") + return origin not in candidates + + +class RejectionLogger: + """Threadsafe dedup wrapper that logs each rejected origin only once. + + Engineio silently 403s WebSocket upgrades from disallowed origins. + Without a log line the user sees a half-broken UI (no live progress, + no toasts) and has no idea what's wrong. This class watches incoming + requests via :meth:`maybe_log` and emits a clear warning the first + time each unique origin appears, telling the user where to add it. + + The dedup set is capped (default 100 unique origins) so a hostile + actor opening connections from many distinct fake origins can't grow + memory unbounded. When the cap is hit, a single overflow warning is + emitted and further rejections are silently dropped until the next + process restart (or :meth:`reset_for_tests` for tests). + """ + + DEFAULT_DEDUP_CAP = 100 + + def __init__(self, logger: Any, dedup_cap: int = DEFAULT_DEDUP_CAP): + self._logger = logger + self._seen: Set[str] = set() + self._lock = threading.Lock() + try: + self._cap = max(1, int(dedup_cap)) + except (TypeError, ValueError): + self._cap = self.DEFAULT_DEDUP_CAP + self._overflow_warned = False + + def maybe_log( + self, + allowed: ResolvedOrigins, + origin: Optional[str], + host: str, + request_scheme: str = '', + forwarded_host: str = '', + forwarded_proto: str = '', + ) -> bool: + """Log a rejection warning if applicable, deduped. + + Returns ``True`` if a warning was emitted this call. Designed to + be safe to call from a Flask ``before_request`` hook on every + Socket.IO request — it short-circuits early on requests that + won't be rejected (no Origin header, allowed origin, same-origin + match against Host / X-Forwarded-Host with proper scheme). + """ + if not will_reject(allowed, origin, host, request_scheme, + forwarded_host, forwarded_proto): + return False + + # Pick the message to emit (or bail) under the lock. Actual + # logger.warning() call happens AFTER the lock releases — keeps + # the critical section minimal and avoids holding our lock while + # the logging framework acquires its own internal locks. + msg: Optional[str] = None + with self._lock: + if origin in self._seen: + return False + if len(self._seen) >= self._cap: + if self._overflow_warned: + return False # Already emitted overflow notice; suppress. + self._overflow_warned = True + msg = ( + f"[Socket.IO] Rejection-log dedup cache hit cap " + f"({self._cap} unique origins). Suppressing further " + f"rejection warnings this session — likely indicates " + f"hostile traffic or a misconfigured client. Restart " + f"to reset the cache." + ) + else: + self._seen.add(origin) + msg = ( + f"[Socket.IO] Rejecting WebSocket connection from origin " + f"'{origin}' (request Host='{host}'). If this is your " + f"reverse-proxy or custom domain, add it to " + f"Settings → Security → Allowed WebSocket Origins." + ) + self._logger.warning(msg) + return True + + def reset_for_tests(self) -> None: + """Clear the dedup cache. Test-only.""" + with self._lock: + self._seen.clear() + self._overflow_warned = False + + +def log_startup_status(allowed: ResolvedOrigins, logger: Any) -> None: + """Emit a one-shot startup log line describing the resolved policy. + + - For ``'*'`` (wildcard) → warning, since it's a security risk. + - For a non-empty list → info, so the user can confirm their config + took effect. + - For ``None`` (same-origin default) → silent. That's the default; + nothing noteworthy. + """ + if allowed == '*': + logger.warning( + "[Socket.IO] cors_allowed_origins is set to '*' — any website can open " + "a WebSocket to this instance. Set Settings → Security → Allowed Origins " + "to a specific list (or leave empty for same-origin only) to lock this down." + ) + elif allowed: + logger.info(f"[Socket.IO] Allowed cross-origin connections from: {allowed}") diff --git a/tests/test_socketio_cors.py b/tests/test_socketio_cors.py new file mode 100644 index 00000000..b54d94d3 --- /dev/null +++ b/tests/test_socketio_cors.py @@ -0,0 +1,442 @@ +"""Tests for `core.socketio_cors` — the resolver, rejection predictor, +and dedup logger that gate Socket.IO WebSocket origins. + +These pin the security-relevant behavior: + +- The resolver returns ``None`` (engineio's same-origin default — also + the secure default) for anything other than an explicit allow-list or + the wildcard. CRITICAL: the resolver must NEVER return ``[]`` — in + engineio that means "disable CORS handling" which is identical to the + ``'*'`` wildcard from a security standpoint (engineio/server.py:202: + ``if cors_allowed_origins != []``). And it must never silently turn + into ``'*'`` from a misshapen config value. +- The rejection predictor must mirror engineio's same-origin check + exactly so the warning we log is accurate. This includes accepting + matches against ``X-Forwarded-Host`` since engineio honors that + automatically when ``cors_allowed_origins`` is ``None``. +- The dedup logger must emit each unique origin only once so a malicious + site repeatedly hammering the WS endpoint can't spam logs. + +Pure unit tests — no Flask, no engineio, no network. Just the logic. +""" + +import threading +from typing import Any, List + +import pytest + +from core.socketio_cors import ( + RejectionLogger, + log_startup_status, + resolve_cors_origins, + will_reject, +) + + +# ── helpers ─────────────────────────────────────────────────────────────── + + +class _FakeConfig: + """Minimal config_manager stub that returns one canned value for the + `security.cors_origins` key. Anything else returns the default.""" + + def __init__(self, value: Any): + self._value = value + + def get(self, key: str, default: Any = None) -> Any: + if key == 'security.cors_origins': + return self._value + return default + + +class _CapturingLogger: + """Stand-in logger that records every warning/info call so tests can + assert what was emitted (and how many times).""" + + def __init__(self): + self.warnings: List[str] = [] + self.infos: List[str] = [] + + def warning(self, msg: str) -> None: + self.warnings.append(msg) + + def info(self, msg: str) -> None: + self.infos.append(msg) + + +# ── resolve_cors_origins ────────────────────────────────────────────────── + + +@pytest.mark.parametrize("value, expected", [ + # Unset / empty / whitespace / bogus types → None (engineio same-origin default) + (None, None), + ('', None), + (' ', None), + ('\n\n', None), + (',,,', None), + (12345, None), # numeric — invalid type + ({'a': 1}, None), # dict — invalid type + ([], None), # explicit empty list + ([' ', ''], None), # list of all-empty strings + + # Wildcard + ('*', '*'), + (' * ', '*'), + (['*'], '*'), + (['https://x.com', '*'], '*'), # wildcard in a list still wins + + # Single origin + ('https://x.com', ['https://x.com']), + (['https://x.com'], ['https://x.com']), + + # Multiple origins, comma-separated + ('https://x.com, http://y.com', ['https://x.com', 'http://y.com']), + + # Multiple origins, newline-separated (textarea input) + ('https://x.com\nhttp://y.com', ['https://x.com', 'http://y.com']), + + # Mixed separators + extra commas / whitespace get cleaned + ('https://x.com,, http://y.com,\n http://z.com', ['https://x.com', 'http://y.com', 'http://z.com']), + + # List with mixed types (bytes-like → str coerce) + (['https://x.com', ' ', 'http://y.com'], ['https://x.com', 'http://y.com']), +]) +def test_resolve_cors_origins_normalizes_input(value, expected): + assert resolve_cors_origins(_FakeConfig(value)) == expected + + +def test_resolve_cors_origins_handles_missing_config_manager(): + """Defensive: if config_manager is None (e.g., very early init), the + resolver must fall back to the secure default rather than crashing.""" + assert resolve_cors_origins(None) is None + + +def test_resolve_cors_origins_never_returns_empty_list(): + """SECURITY CRITICAL: ``cors_allowed_origins=[]`` in engineio means + "disable CORS handling entirely" — identical security to ``'*'`` + (engineio/server.py:202). The resolver must return ``None`` for the + secure default, never ``[]``, regardless of what the user typed.""" + edge_cases = [None, '', ' ', '\n\n', ',,,', 12345, 3.14, {'a': 1}, + object(), True, False, [], [' '], ['', ' '], (' ',)] + for value in edge_cases: + result = resolve_cors_origins(_FakeConfig(value)) + assert result != [], ( + f"resolve_cors_origins({value!r}) returned [] — that disables " + f"engineio's CORS check entirely, allowing all origins. Must be None." + ) + + +def test_resolve_cors_origins_never_silently_returns_wildcard_for_garbage(): + """Security-critical: a misshapen config value must NEVER turn into + `'*'` by accident. Anything we can't parse falls back to same-origin.""" + for bogus in [12345, 3.14, {'a': 1}, object(), True, False]: + assert resolve_cors_origins(_FakeConfig(bogus)) is None, ( + f"resolve_cors_origins({bogus!r}) returned a non-None value — " + f"bogus inputs must default to same-origin only" + ) + + +# ── will_reject ─────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize("allowed, origin, host, scheme, expected_reject", [ + # Same-origin (Origin's full {scheme}://{host} matches request) — allow + (None, 'http://localhost:8888', 'localhost:8888', 'http', False), + (None, 'http://192.168.1.5:8888', '192.168.1.5:8888', 'http', False), + (None, 'https://soulsync.foo', 'soulsync.foo', 'https', False), + + # Cross-origin with default allow-list — reject + (None, 'https://x.com', 'localhost:8888', 'http', True), + (None, 'https://soulsync.foo', 'localhost:8888', 'http', True), # reverse proxy NOT forwarding Host + # Scheme mismatch — engineio rejects, so do we + (None, 'https://soulsync.foo', 'soulsync.foo', 'http', True), + + # Wildcard short-circuit — allow + ('*', 'https://x.com', 'localhost:8888', 'http', False), + ('*', 'https://anything.evil', 'localhost:8888', 'http', False), + + # Origin in allow-list — allow + (['https://x.com'], 'https://x.com', 'localhost:8888', 'http', False), + (['https://soulsync.foo'], 'https://soulsync.foo', 'localhost:8888', 'http', False), + + # Cross-origin not in allow-list — reject + (['https://x.com'], 'https://y.com', 'localhost:8888', 'http', True), + + # Same-origin still works even when allow-list has other entries + (['https://x.com'], 'http://localhost:8888', 'localhost:8888', 'http', False), +]) +def test_will_reject_predicts_engineio_decision(allowed, origin, host, scheme, expected_reject): + assert will_reject(allowed, origin, host, request_scheme=scheme) is expected_reject + + +def test_will_reject_with_empty_host_only_uses_allowlist(): + """If the request somehow has no Host header (shouldn't happen but be + safe), same-origin can't be checked — fall through to allow-list only.""" + assert will_reject(None, 'https://x.com', '', request_scheme='https') is True + assert will_reject(['https://x.com'], 'https://x.com', '', request_scheme='https') is False + assert will_reject('*', 'https://x.com', '', request_scheme='https') is False + + +def test_will_reject_honors_x_forwarded_host(): + """Engineio honors X-Forwarded-Host automatically when + cors_allowed_origins is None (engineio/base_server.py:_cors_allowed_origins). + Our predictor must mirror that — otherwise reverse-proxy users with + proper proxy headers would trigger spurious "rejected" log lines.""" + # Same-origin via X-Forwarded-Host (typical TLS-terminating reverse proxy) + assert will_reject(None, 'https://soulsync.foo', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo', + forwarded_proto='https') is False + + # X-Forwarded-Host with comma list (proxy chain) — first entry wins + assert will_reject(None, 'https://soulsync.foo', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo, edge.proxy', + forwarded_proto='https') is False + + # X-Forwarded-Host doesn't match either — still reject + assert will_reject(None, 'https://attacker.com', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo', + forwarded_proto='https') is True + + # X-Forwarded-Host empty — falls back to Host check (the unset case) + assert will_reject(None, 'https://soulsync.foo', 'soulsync.foo', + request_scheme='https', + forwarded_host='') is False + + +def test_will_reject_compares_full_scheme_when_known(): + """When the caller provides scheme info, engineio compares full + {scheme}://{host} strings. A TLS-terminating proxy can leave the + backend seeing http while the browser's Origin is https — engineio + rejects, our predictor must too (otherwise we miss logging it).""" + # Backend sees http, browser sent https → engineio rejects → we predict reject + assert will_reject(None, 'https://soulsync.foo', 'soulsync.foo', + request_scheme='http') is True + + # Backend sees http, browser sent http → match → allow + assert will_reject(None, 'http://soulsync.foo', 'soulsync.foo', + request_scheme='http') is False + + # X-Forwarded-Proto says the public request was https → match origin's https + assert will_reject(None, 'https://soulsync.foo', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo', + forwarded_proto='https') is False + + # X-Forwarded-Proto says https but Origin is http → mismatch → reject + assert will_reject(None, 'http://soulsync.foo', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo', + forwarded_proto='https') is True + + # Comma-separated X-Forwarded-Proto (proxy chain) — first wins, like engineio + assert will_reject(None, 'https://soulsync.foo', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo', + forwarded_proto='https, http') is False + + +def test_will_reject_allows_missing_origin_matching_engineio(): + """Engineio (server.py:207: ``if origin:``) skips CORS validation + entirely when no Origin header is sent — non-browser clients (curl, + server-to-server) are intentionally permitted. Our predictor must + match that or we'd log spurious "rejected" warnings for legitimate + non-browser traffic. Must also not raise on None input.""" + # Wildcard permits missing origin — and so does the default policy + # (matches engineio's actual behavior). + assert will_reject('*', None, 'localhost:8888') is False + assert will_reject('*', '', 'localhost:8888') is False + assert will_reject(None, None, 'localhost:8888') is False + assert will_reject(None, '', 'localhost:8888') is False + assert will_reject(['https://x.com'], None, 'localhost:8888') is False + + +def test_will_reject_honors_forwarded_proto_alone(): + """Engineio adds the forwarded candidate when EITHER X-Forwarded-Proto + OR X-Forwarded-Host is present (it falls back to HTTP_HOST for the + missing one). Our predictor must mirror that — otherwise a misconfig + sending only X-Forwarded-Proto would look like a rejection in our + log even though engineio actually allows it.""" + # forwarded_proto alone: backend host stands in for forwarded_host + assert will_reject(None, 'https://localhost:8888', 'localhost:8888', + request_scheme='http', + forwarded_proto='https') is False + + # forwarded_proto alone but origin's host doesn't match the backend host + assert will_reject(None, 'https://attacker.com', 'localhost:8888', + request_scheme='http', + forwarded_proto='https') is True + + +# ── RejectionLogger ─────────────────────────────────────────────────────── + + +def test_rejection_logger_emits_once_per_unique_origin(): + log = _CapturingLogger() + rl = RejectionLogger(log) + + # Same origin three times — only one warning + for _ in range(3): + rl.maybe_log(None, 'https://attacker.com', 'localhost:8888') + assert len(log.warnings) == 1 + assert 'attacker.com' in log.warnings[0] + + # Different origin — separate warning + rl.maybe_log(None, 'https://other.evil', 'localhost:8888') + assert len(log.warnings) == 2 + assert 'other.evil' in log.warnings[1] + + +def test_rejection_logger_silent_when_request_would_be_allowed(): + log = _CapturingLogger() + rl = RejectionLogger(log) + + # Same-origin — no warning + rl.maybe_log(None, 'http://localhost:8888', 'localhost:8888') + # Wildcard — no warning + rl.maybe_log('*', 'https://x.com', 'localhost:8888') + # In allow-list — no warning + rl.maybe_log(['https://x.com'], 'https://x.com', 'localhost:8888') + # Same-origin via X-Forwarded-Host (with proxy scheme info) — no warning + rl.maybe_log(None, 'https://soulsync.foo', 'internal:8888', + request_scheme='http', + forwarded_host='soulsync.foo', + forwarded_proto='https') + + assert log.warnings == [] + + +def test_rejection_logger_silent_when_no_origin_header(): + """Non-browser clients (curl, server-to-server) don't send Origin — + they should not trigger the warning.""" + log = _CapturingLogger() + rl = RejectionLogger(log) + + rl.maybe_log(None, None, 'localhost:8888') + rl.maybe_log(None, '', 'localhost:8888') + + assert log.warnings == [] + + +def test_rejection_logger_warning_message_points_user_to_settings(): + """The warning is the ONLY signal users get when their reverse proxy + setup is broken. It must name the origin AND tell them where to fix it.""" + log = _CapturingLogger() + rl = RejectionLogger(log) + + rl.maybe_log(None, 'https://soulsync.example.com', 'internal-host:8888') + + assert len(log.warnings) == 1 + msg = log.warnings[0] + assert 'soulsync.example.com' in msg, "warning must include the rejected origin" + assert 'internal-host:8888' in msg, "warning must include the request Host so users can debug proxy config" + assert 'Settings' in msg, "warning must point users to Settings" + assert 'Allowed' in msg, "warning must name the field they need to edit" + + +def test_rejection_logger_dedup_is_threadsafe(): + """Two threads racing on the same novel origin must result in exactly + one warning, not two. Locks the dedup set internally.""" + log = _CapturingLogger() + rl = RejectionLogger(log) + barrier = threading.Barrier(8) + + def hammer(): + barrier.wait() + for _ in range(50): + rl.maybe_log(None, 'https://race.test', 'localhost:8888') + + threads = [threading.Thread(target=hammer) for _ in range(8)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert len(log.warnings) == 1 + + +def test_rejection_logger_reset_for_tests_clears_dedup(): + log = _CapturingLogger() + rl = RejectionLogger(log) + + rl.maybe_log(None, 'https://x.com', 'localhost:8888') + assert len(log.warnings) == 1 + + rl.reset_for_tests() + rl.maybe_log(None, 'https://x.com', 'localhost:8888') + assert len(log.warnings) == 2 # logged again after reset + + +def test_rejection_logger_caps_dedup_set_at_configured_limit(): + """A hostile actor opening connections from many distinct fake origins + would otherwise grow the dedup set unbounded. After the cap is hit, + further rejections are silently dropped (after one overflow notice).""" + log = _CapturingLogger() + rl = RejectionLogger(log, dedup_cap=5) + + # Fill the cap + for i in range(5): + rl.maybe_log(None, f'https://fake{i}.com', 'localhost:8888') + assert len(log.warnings) == 5 + + # Next unique origin → overflow notice, NOT a per-origin warning + rl.maybe_log(None, 'https://fake5.com', 'localhost:8888') + assert len(log.warnings) == 6 + assert 'cap' in log.warnings[5].lower() or 'suppress' in log.warnings[5].lower() + + # Further unique origins → silently dropped (overflow notice already emitted) + for i in range(6, 20): + rl.maybe_log(None, f'https://fake{i}.com', 'localhost:8888') + assert len(log.warnings) == 6 # unchanged + + # After reset, cap restarts + rl.reset_for_tests() + rl.maybe_log(None, 'https://fake0.com', 'localhost:8888') + assert len(log.warnings) == 7 + + +def test_rejection_logger_default_cap_is_reasonable(): + """The default cap should be high enough that legitimate-but-unusual + setups (e.g., a power user with a dozen reverse-proxy domains rotating) + don't hit the overflow notice during normal use.""" + assert RejectionLogger.DEFAULT_DEDUP_CAP >= 50, ( + "default dedup cap should fit normal usage" + ) + + +# ── log_startup_status ──────────────────────────────────────────────────── + + +def test_startup_status_warns_on_wildcard(): + """The wildcard is a security risk — startup must log a warning that + points users to the settings page, not just an info line.""" + log = _CapturingLogger() + log_startup_status('*', log) + + assert len(log.warnings) == 1 + assert "'*'" in log.warnings[0] + assert 'Settings' in log.warnings[0] + assert log.infos == [] + + +def test_startup_status_info_logs_nonempty_allowlist(): + """Non-empty allow-list → info, so users can confirm their config + actually took effect.""" + log = _CapturingLogger() + log_startup_status(['https://x.com', 'https://y.com'], log) + + assert log.warnings == [] + assert len(log.infos) == 1 + assert 'https://x.com' in log.infos[0] + + +def test_startup_status_silent_on_default_same_origin(): + """None (default) → no log. Same-origin-only is the default; + nothing noteworthy to announce on every startup.""" + log = _CapturingLogger() + log_startup_status(None, log) + + assert log.warnings == [] + assert log.infos == [] diff --git a/web_server.py b/web_server.py index c473e037..b3d47be7 100644 --- a/web_server.py +++ b/web_server.py @@ -210,12 +210,43 @@ def _init_flask_secret_key(): app.secret_key = _init_flask_secret_key() # --- WebSocket (Socket.IO) Setup --- -socketio = SocketIO(app, async_mode='threading', cors_allowed_origins='*') +from core.socketio_cors import ( + resolve_cors_origins as _resolve_socketio_cors_origins, + RejectionLogger as _SocketIORejectionLogger, + log_startup_status as _log_socketio_startup_status, +) +_socketio_cors_origins = _resolve_socketio_cors_origins(config_manager) +socketio = SocketIO(app, async_mode='threading', cors_allowed_origins=_socketio_cors_origins) +_log_socketio_startup_status(_socketio_cors_origins, logger) +_socketio_rejection_logger = _SocketIORejectionLogger(logger) # Plex PIN auth requests stored in memory for polling _plex_pin_requests = {} _plex_pin_requests_lock = threading.Lock() +@app.before_request +def _log_rejected_socketio_origin(): + """Hook the WS upgrade path so users see a clear log line when their + Origin is about to be rejected (engineio otherwise just silently 403s + the upgrade). Dedup + threading lives in `core/socketio_cors`. + + Note: Flask's ``before_request`` runs on every HTTP request to every + endpoint — there's no path-scoped equivalent for arbitrary URL + prefixes. We early-return on non-/socket.io/ paths to keep the + overhead to one string compare per request. + """ + if not request.path.startswith('/socket.io/'): + return + _socketio_rejection_logger.maybe_log( + _socketio_cors_origins, + request.headers.get('Origin'), + request.headers.get('Host', ''), + request.scheme, + request.headers.get('X-Forwarded-Host', ''), + request.headers.get('X-Forwarded-Proto', ''), + ) + + # --- Profile Context (before_request hook) --- @app.before_request def _set_profile_context(): diff --git a/webui/index.html b/webui/index.html index 9732573f..1bd71056 100644 --- a/webui/index.html +++ b/webui/index.html @@ -5409,6 +5409,14 @@
+ +* on its own line to allow any origin (insecure — only do this if you understand why you need it).
+