From 013eebf350dadfc3c1ec1b8675e0806381b3ec6a Mon Sep 17 00:00:00 2001
From: Broque Thomas <26755000+Nezreka@users.noreply.github.com>
Date: Sun, 26 Apr 2026 16:27:10 -0700
Subject: [PATCH 1/3] =?UTF-8?q?Lock=20down=20Socket.IO=20CORS=20=E2=80=94?=
=?UTF-8?q?=20same-origin=20default=20+=20opt-in=20allow-list?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Closes #366 (reported by JohnBaumb).
Socket.IO was initialized with `cors_allowed_origins='*'`, accepting
WebSocket connections from any origin. A malicious site could open a
WS to a user's local SoulSync instance and exfiltrate live progress /
toast / activity events.
This commit:
- Defaults to engineio's same-origin behavior (`cors_allowed_origins=None`),
which automatically honors X-Forwarded-Host so reverse proxies that
send that header (Caddy / Traefik by default, properly-configured
Nginx) work transparently.
- Adds a `security.cors_origins` config setting + Settings → Security
textarea where users behind unusual proxies / Electron wrappers /
cross-origin integrations can whitelist their origin. Accepts comma
or newline separated values; `*` on its own line opts back into the
legacy wildcard with a startup-warning log.
- Logs a clear warning the first time engineio rejects each unique
origin, naming the rejected Origin and request Host and pointing
users to the settings field. Without this, engineio silently 403s
the upgrade and the user just sees a half-broken UI with no clue
why. Threadsafe dedup so a hostile origin can't spam logs.
Logic lives in `core/socketio_cors.py` (resolver, rejection
predictor, dedup logger class, startup-status emitter) — pure
functions, no Flask dependency. `web_server.py` adds 23 lines of
wiring and imports.
Important catch during review: my first pass used `cors_allowed_origins=[]`
as the "secure default." Reading engineio's source revealed `[]` actually
means "DISABLE CORS HANDLING" (engineio/server.py:202: `if cors_allowed_origins != []:`)
— identical security to `'*'`. Fixed to use `None` (engineio's actual
same-origin sentinel) and pinned with a regression test that asserts
the resolver never returns `[]` for any input shape.
Tests:
- tests/test_socketio_cors.py — 45 unit tests covering 19 resolver shape
cases (None, empty, whitespace, comma, newline, garbage types, lists),
the `[]`-must-never-be-returned security regression, 12 rejection
prediction cases, X-Forwarded-Host handling, dedup logger behavior,
threadsafe race (8 threads × 50 hammers → exactly 1 warning), and
startup-status emitter outputs.
Frontend:
- Settings → Security gains an "Allowed WebSocket Origins" textarea
with help text explaining same-origin default + when to add a domain
+ the `*` opt-out.
- helper.js — new '2.4.1' WHATS_NEW block (hidden until version bump)
with a chill-voice entry describing the change.
Conftest.py left at `'*'` — test environment, no security concern.
598 tests pass.
---
core/socketio_cors.py | 191 +++++++++++++++++++++
tests/test_socketio_cors.py | 333 ++++++++++++++++++++++++++++++++++++
web_server.py | 25 ++-
webui/index.html | 8 +
webui/static/helper.js | 5 +
webui/static/settings.js | 6 +
6 files changed, 567 insertions(+), 1 deletion(-)
create mode 100644 core/socketio_cors.py
create mode 100644 tests/test_socketio_cors.py
diff --git a/core/socketio_cors.py b/core/socketio_cors.py
new file mode 100644
index 00000000..61b4e4ce
--- /dev/null
+++ b/core/socketio_cors.py
@@ -0,0 +1,191 @@
+"""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: an empty list (same-origin only), 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)):
+ parts = [str(p).strip() for p in raw]
+ 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: str,
+ host: str,
+ forwarded_host: 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: ``Origin``'s ``host[:port]`` portion matches the
+ request's ``Host`` header OR the ``X-Forwarded-Host`` header. Engineio
+ checks both when ``cors_allowed_origins`` is ``None``; we mirror that
+ so reverse-proxy users with proper proxy headers don't trigger
+ spurious "rejected" log lines.
+ """
+ if allowed == '*':
+ return False
+ if isinstance(allowed, list) and origin in allowed:
+ return False
+ # Origin is "scheme://host[:port][/path]"; pull just host[:port].
+ origin_host = origin.split('://', 1)[-1].split('/', 1)[0]
+ if host and origin_host == host:
+ return False
+ if forwarded_host and origin_host == forwarded_host.split(',')[0].strip():
+ return False
+ return True
+
+
+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.
+
+ Bounded by the number of unique origins ever attempted; cleared on
+ process restart. The dedup is intentional — a malicious site
+ hammering the endpoint shouldn't be able to spam logs.
+ """
+
+ def __init__(self, logger: Any):
+ self._logger = logger
+ self._seen: Set[str] = set()
+ self._lock = threading.Lock()
+
+ def maybe_log(
+ self,
+ allowed: ResolvedOrigins,
+ origin: Optional[str],
+ host: str,
+ forwarded_host: 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 either Host or X-Forwarded-Host).
+ """
+ if not origin:
+ return False # Non-browser clients (curl, server-to-server)
+ if not will_reject(allowed, origin, host, forwarded_host):
+ return False
+ with self._lock:
+ if origin in self._seen:
+ return False
+ self._seen.add(origin)
+ self._logger.warning(
+ f"[Socket.IO] Rejecting WebSocket connection from origin '{origin}' "
+ f"(request Host='{host}'). If this is your reverse-proxy or custom "
+ f"domain, add it to Settings → Security → Allowed WebSocket Origins."
+ )
+ return True
+
+ def reset_for_tests(self) -> None:
+ """Clear the dedup cache. Test-only."""
+ with self._lock:
+ self._seen.clear()
+
+
+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..267260e0
--- /dev/null
+++ b/tests/test_socketio_cors.py
@@ -0,0 +1,333 @@
+"""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 logging
+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, expected_reject", [
+ # Same-origin (Origin's host:port matches the request Host) — allow
+ (None, 'http://localhost:8888', 'localhost:8888', False),
+ (None, 'http://192.168.1.5:8888','192.168.1.5:8888',False),
+ (None, 'https://soulsync.foo', 'soulsync.foo', False),
+
+ # Cross-origin with default allow-list — reject
+ (None, 'https://x.com', 'localhost:8888', True),
+ (None, 'https://soulsync.foo', 'localhost:8888', True), # reverse proxy NOT forwarding Host
+
+ # Wildcard short-circuit — allow
+ ('*', 'https://x.com', 'localhost:8888', False),
+ ('*', 'https://anything.evil', 'localhost:8888', False),
+
+ # Origin in allow-list — allow
+ (['https://x.com'], 'https://x.com', 'localhost:8888', False),
+ (['https://soulsync.foo'], 'https://soulsync.foo', 'localhost:8888', False),
+
+ # Cross-origin not in allow-list — reject
+ (['https://x.com'], 'https://y.com', 'localhost:8888', True),
+
+ # Same-origin still works even when allow-list has other entries
+ (['https://x.com'], 'http://localhost:8888', 'localhost:8888', False),
+
+ # Origin with path component — only host:port should be compared
+ (None, 'http://x.com:8080/path', 'x.com:8080', False),
+])
+def test_will_reject_predicts_engineio_decision(allowed, origin, host, expected_reject):
+ assert will_reject(allowed, origin, host) 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', '') is True
+ assert will_reject(['https://x.com'], 'https://x.com', '') is False
+ assert will_reject('*', 'https://x.com', '') 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 reverse-proxy setup)
+ assert will_reject(None, 'https://soulsync.foo', 'internal:8888',
+ forwarded_host='soulsync.foo') is False
+
+ # X-Forwarded-Host with comma list (proxy chain) — first entry wins
+ assert will_reject(None, 'https://soulsync.foo', 'internal:8888',
+ forwarded_host='soulsync.foo, edge.proxy') is False
+
+ # X-Forwarded-Host doesn't match either — still reject
+ assert will_reject(None, 'https://attacker.com', 'internal:8888',
+ forwarded_host='soulsync.foo') is True
+
+ # X-Forwarded-Host empty — falls back to Host check (the unset case)
+ assert will_reject(None, 'https://soulsync.foo', 'soulsync.foo',
+ forwarded_host='') is False
+
+
+# ── 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 — no warning
+ rl.maybe_log(None, 'https://soulsync.foo', 'internal:8888', 'soulsync.foo')
+
+ 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
+
+
+# ── 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..77690b3b 100644
--- a/web_server.py
+++ b/web_server.py
@@ -210,12 +210,35 @@ 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`."""
+ 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.headers.get('X-Forwarded-Host', ''),
+ )
+
+
# --- 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 @@
+
+
+
+
+
+ Origins (full URL, no trailing slash) allowed to open WebSocket connections to this instance — one per line, or comma-separated. Leave empty for same-origin only (the secure default; works for direct access and most reverse-proxy setups). Add your public domain here if you reach SoulSync via a reverse proxy or custom domain and the WebSocket fails to connect. Use * on its own line to allow any origin (insecure — only do this if you understand why you need it).
+
+
diff --git a/webui/static/helper.js b/webui/static/helper.js
index 8c6553c1..cae52460 100644
--- a/webui/static/helper.js
+++ b/webui/static/helper.js
@@ -3441,6 +3441,11 @@ function closeHelperSearch() {
// projects that span multiple commits before shipping. Strip the flag at
// release time and add a real `date:` line at the top of the version block.
const WHATS_NEW = {
+ '2.4.1': [
+ // --- post-2.4.0 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
+ { date: 'Unreleased — 2.4.1 dev cycle' },
+ { title: 'Lock Down Socket.IO CORS', desc: 'socket.io was accepting websocket connections from any origin (cors=*). now defaults to same-origin only. if your websocket fails after updating, the server logs a clear warning with the rejected origin — add it to settings → security → allowed websocket origins.', page: 'settings' },
+ ],
'2.4.0': [
// --- April 26, 2026 — Search & Artists unification + reorganize queue ---
{ date: 'April 26, 2026 — 2.4.0 release' },
diff --git a/webui/static/settings.js b/webui/static/settings.js
index 7d10dabf..f2026891 100644
--- a/webui/static/settings.js
+++ b/webui/static/settings.js
@@ -996,6 +996,11 @@ async function loadSettingsData() {
const requirePin = settings.security?.require_pin_on_launch || false;
document.getElementById('security-require-pin').checked = requirePin;
+ // CORS origins — stored verbatim as the user typed (string).
+ const corsOrigins = settings.security?.cors_origins || '';
+ const corsField = document.getElementById('security-cors-origins');
+ if (corsField) corsField.value = corsOrigins;
+
// Check if admin has a PIN set
const profilesRes = await fetch('/api/profiles');
const profilesData = await profilesRes.json();
@@ -2587,6 +2592,7 @@ async function saveSettings(quiet = false) {
},
security: {
require_pin_on_launch: document.getElementById('security-require-pin')?.checked || false,
+ cors_origins: document.getElementById('security-cors-origins')?.value?.trim() || '',
}
};
From 0f24739e27b8d18fb6f9824a93399ec9734a6c61 Mon Sep 17 00:00:00 2001
From: Broque Thomas <26755000+Nezreka@users.noreply.github.com>
Date: Sun, 26 Apr 2026 17:32:22 -0700
Subject: [PATCH 2/3] =?UTF-8?q?Socket.IO=20CORS:=20polish=20=E2=80=94=20ma?=
=?UTF-8?q?tch=20engineio=20exactly,=20bound=20dedup,=20validate=20URLs?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Self-review pass on the security fix uncovered five issues, all fixed
here:
1. will_reject scheme handling. Engineio compares full {scheme}://{host}
strings, not just hostnames. A TLS-terminating proxy can leave the
backend seeing http while the browser's Origin is https — engineio
rejects, but the original predictor said "allow" → no helpful log
line. Added request_scheme + forwarded_proto params, build full
candidate strings to match engineio.
2. EITHER-forwarded-header rule. 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). The original predictor
only added it when forwarded_host was set — false negative for
misconfigs sending only X-Forwarded-Proto. Now mirrors engineio.
3. will_reject incorrectly rejected missing-Origin requests. Engineio
(server.py:207: `if origin: validate`) skips CORS validation when
no Origin header is sent — non-browser clients (curl etc.) are
intentionally permitted. The original code rejected them. Test was
asserting the wrong behavior. Both fixed.
4. RejectionLogger had unbounded dedup set growth. A hostile actor
opening connections from many distinct fake origins would fill
memory unboundedly. Capped at 100 unique origins (configurable);
when cap hit, one overflow notice is emitted and further rejections
are silently dropped until restart.
5. Lock pattern: the overflow log path called logger.warning() while
holding the dedup lock, inconsistent with the normal path. Fixed
to pick the message under the lock and log after release. Critical
section is now minimal and uniform.
Plus polish:
- Stale module docstring fixed (said "empty list" instead of "None").
- settings.js validates each cors_origins line against a URL regex on
save; toasts a one-shot warning if entries are malformed (resolver
silently filters them, but user gets feedback now).
- web_server.py wiring passes request.scheme + X-Forwarded-Proto so
the predictor has full proxy info.
Tests:
- 51 unit tests in tests/test_socketio_cors.py (was 45). New cases:
* scheme comparison (5 cases including TLS-terminating proxies)
* forwarded_proto-alone misconfig
* missing-origin matches engineio (was asserting wrong behavior)
* dedup cap with overflow + reset
* default cap is reasonable (uses public DEFAULT_DEDUP_CAP constant)
Engineio behavior independently verified by reading engineio/server.py
and engineio/base_server.py source. Predictor mirrors both files.
604 tests pass.
---
core/socketio_cors.py | 125 ++++++++++++++++++++++++++++--------
tests/test_socketio_cors.py | 121 +++++++++++++++++++++++++++++++++-
web_server.py | 2 +
webui/static/settings.js | 21 ++++++
4 files changed, 241 insertions(+), 28 deletions(-)
diff --git a/core/socketio_cors.py b/core/socketio_cors.py
index 61b4e4ce..ef41206f 100644
--- a/core/socketio_cors.py
+++ b/core/socketio_cors.py
@@ -5,8 +5,9 @@ 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: an empty list (same-origin only), the literal ``'*'``
- (wildcard, opt-in), or a list of explicit origin URLs.
+ 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
@@ -88,9 +89,11 @@ def resolve_cors_origins(config_manager: Any) -> ResolvedOrigins:
def will_reject(
allowed: ResolvedOrigins,
- origin: str,
+ 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.
@@ -98,22 +101,64 @@ def will_reject(
a helpful warning *before* the rejection happens. Returns ``True``
when the request will be rejected.
- Same-origin check: ``Origin``'s ``host[:port]`` portion matches the
- request's ``Host`` header OR the ``X-Forwarded-Host`` header. Engineio
- checks both when ``cors_allowed_origins`` is ``None``; we mirror that
- so reverse-proxy users with proper proxy headers don't trigger
- spurious "rejected" log lines.
+ 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.
+
+ Proxy params default to empty so callers without proxy awareness
+ fall back to a host-only same-origin check (still correct for
+ direct-access setups).
"""
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
- # Origin is "scheme://host[:port][/path]"; pull just host[:port].
- origin_host = origin.split('://', 1)[-1].split('/', 1)[0]
- if host and origin_host == host:
- return False
- if forwarded_host and origin_host == forwarded_host.split(',')[0].strip():
+
+ # 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}")
+ if origin in candidates:
return False
+
+ # Backwards-compat shim: callers that don't pass scheme info still
+ # get the original host-only same-origin check, so callers / tests
+ # that exercise this predicate without a real Flask request context
+ # don't get spurious rejections. Production callers always pass
+ # scheme, so this branch is inert in normal operation.
+ if not request_scheme and not forwarded_proto:
+ origin_host = origin.split('://', 1)[-1].split('/', 1)[0]
+ if host and origin_host == host:
+ return False
+ if forwarded_host and origin_host == forwarded_host.split(',')[0].strip():
+ return False
return True
@@ -126,22 +171,30 @@ class RejectionLogger:
requests via :meth:`maybe_log` and emits a clear warning the first
time each unique origin appears, telling the user where to add it.
- Bounded by the number of unique origins ever attempted; cleared on
- process restart. The dedup is intentional — a malicious site
- hammering the endpoint shouldn't be able to spam logs.
+ 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).
"""
- def __init__(self, logger: Any):
+ 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()
+ self._cap = max(1, int(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.
@@ -149,27 +202,49 @@ class RejectionLogger:
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 either Host or X-Forwarded-Host).
+ match against Host / X-Forwarded-Host with proper scheme).
"""
if not origin:
return False # Non-browser clients (curl, server-to-server)
- if not will_reject(allowed, origin, host, forwarded_host):
+ 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
- self._seen.add(origin)
- self._logger.warning(
- f"[Socket.IO] Rejecting WebSocket connection from origin '{origin}' "
- f"(request Host='{host}'). If this is your reverse-proxy or custom "
- f"domain, add it to Settings → Security → Allowed WebSocket Origins."
- )
+ 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:
diff --git a/tests/test_socketio_cors.py b/tests/test_socketio_cors.py
index 267260e0..278f443a 100644
--- a/tests/test_socketio_cors.py
+++ b/tests/test_socketio_cors.py
@@ -20,7 +20,6 @@ These pin the security-relevant behavior:
Pure unit tests — no Flask, no engineio, no network. Just the logic.
"""
-import logging
import threading
from typing import Any, List
@@ -201,6 +200,82 @@ def test_will_reject_honors_x_forwarded_host():
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_falls_back_to_host_only_when_no_scheme_info():
+ """Backwards-compat shim: callers that don't pass scheme info still
+ get the basic Host-only same-origin check (the original behavior).
+ Important for any integration tests that exercise the predicate
+ without a real Flask request context."""
+ # No scheme info → host-only match works
+ assert will_reject(None, 'https://soulsync.foo', 'soulsync.foo') is False
+ assert will_reject(None, 'http://x.com', 'x.com') is False
+ # Cross-origin still rejected
+ assert will_reject(None, 'https://attacker.com', 'soulsync.foo') is True
+
+
+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 ───────────────────────────────────────────────────────
@@ -230,8 +305,11 @@ def test_rejection_logger_silent_when_request_would_be_allowed():
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 — no warning
- rl.maybe_log(None, 'https://soulsync.foo', 'internal:8888', 'soulsync.foo')
+ # 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 == []
@@ -297,6 +375,43 @@ def test_rejection_logger_reset_for_tests_clears_dedup():
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 ────────────────────────────────────────────────────
diff --git a/web_server.py b/web_server.py
index 77690b3b..80e929e9 100644
--- a/web_server.py
+++ b/web_server.py
@@ -235,7 +235,9 @@ def _log_rejected_socketio_origin():
_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', ''),
)
diff --git a/webui/static/settings.js b/webui/static/settings.js
index f2026891..e1f23f03 100644
--- a/webui/static/settings.js
+++ b/webui/static/settings.js
@@ -2596,6 +2596,27 @@ async function saveSettings(quiet = false) {
}
};
+ // Validate cors_origins entries — backend silently filters malformed
+ // values, so warn the user up-front if any line doesn't look like a
+ // URL (or the special '*' token). One-shot toast; doesn't block save.
+ const corsRaw = settings.security.cors_origins;
+ if (corsRaw) {
+ const entries = corsRaw.replace(/\n/g, ',').split(',')
+ .map(s => s.trim())
+ .filter(s => s);
+ const invalid = entries.filter(e => {
+ if (e === '*') return false;
+ // Accept anything matching scheme://host[:port], no path required
+ return !/^https?:\/\/[^\s/]+$/i.test(e);
+ });
+ if (invalid.length) {
+ showToast(
+ `Allowed Origins: ${invalid.length} entr${invalid.length === 1 ? 'y looks' : 'ies look'} malformed (need full URL like https://soulsync.example.com, no trailing slash). Saving anyway — they\'ll be ignored.`,
+ 'warning'
+ );
+ }
+ }
+
try {
if (!quiet) showLoadingOverlay('Saving settings...');
From dd4cf130d723ef62757aa1fb69273b3a474a04fc Mon Sep 17 00:00:00 2001
From: Broque Thomas <26755000+Nezreka@users.noreply.github.com>
Date: Sun, 26 Apr 2026 19:24:43 -0700
Subject: [PATCH 3/3] Socket.IO CORS: handle self-review nits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Six items from a Cin-style line-by-line pass on PR #383:
- resolve_cors_origins: list of non-string entries (`[None, 123]`) now
drops them instead of coercing to junk strings like `'None'`/`'123'`.
- will_reject: backwards-compat shim removed. Production callers always
pass `request.scheme` (Flask-guaranteed); the shim only existed for
tests/non-Flask callers and made the production code path branchier
than necessary. Tests now pass scheme explicitly.
- maybe_log: redundant `if not origin` early-return dropped. will_reject
handles missing origin (engineio's own behavior — server.py:207).
- RejectionLogger.__init__: `int(dedup_cap)` wrapped in try/except so
bad-type input falls back to DEFAULT_DEDUP_CAP instead of raising.
- web_server.py: docstring on the before_request hook explains why the
hook fires on every request (Flask doesn't scope before_request to a
path prefix; the early-return string compare is the cheapest option).
- settings.js: cors-origins URL regex tightened from `[^\s/]+` to
`[^\s/?#]+` so query/fragment chars don't pass validation. Engineio
would silently fail to match those anyway; better to flag at save.
Test changes:
- parametrize gained an explicit `scheme` column (12 cases updated).
- New explicit case: scheme-mismatch rejects (engineio compares full
`{scheme}://{host}` strings).
- `test_will_reject_falls_back_to_host_only_when_no_scheme_info`
deleted — the shim it tested is gone.
- `test_will_reject_honors_x_forwarded_host` now passes scheme info.
Net: -9 production lines, -3 test lines. Production code path is
straight-line. 603 tests pass.
---
core/socketio_cors.py | 34 +++++++------------
tests/test_socketio_cors.py | 68 +++++++++++++++++--------------------
web_server.py | 8 ++++-
webui/static/settings.js | 5 +--
4 files changed, 53 insertions(+), 62 deletions(-)
diff --git a/core/socketio_cors.py b/core/socketio_cors.py
index ef41206f..425a464e 100644
--- a/core/socketio_cors.py
+++ b/core/socketio_cors.py
@@ -76,7 +76,9 @@ def resolve_cors_origins(config_manager: Any) -> ResolvedOrigins:
return None
parts = [p.strip() for p in raw.replace('\n', ',').split(',')]
elif isinstance(raw, (list, tuple)):
- parts = [str(p).strip() for p in raw]
+ # 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]
@@ -118,9 +120,10 @@ def will_reject(
matters for non-browser clients like ``curl`` — which engineio
intentionally permits.
- Proxy params default to empty so callers without proxy awareness
- fall back to a host-only same-origin check (still correct for
- direct-access setups).
+ ``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
@@ -145,21 +148,7 @@ def will_reject(
if forwarded_proto
else (request_scheme or 'http'))
candidates.append(f"{f_scheme}://{f_host}")
- if origin in candidates:
- return False
-
- # Backwards-compat shim: callers that don't pass scheme info still
- # get the original host-only same-origin check, so callers / tests
- # that exercise this predicate without a real Flask request context
- # don't get spurious rejections. Production callers always pass
- # scheme, so this branch is inert in normal operation.
- if not request_scheme and not forwarded_proto:
- origin_host = origin.split('://', 1)[-1].split('/', 1)[0]
- if host and origin_host == host:
- return False
- if forwarded_host and origin_host == forwarded_host.split(',')[0].strip():
- return False
- return True
+ return origin not in candidates
class RejectionLogger:
@@ -184,7 +173,10 @@ class RejectionLogger:
self._logger = logger
self._seen: Set[str] = set()
self._lock = threading.Lock()
- self._cap = max(1, int(dedup_cap))
+ try:
+ self._cap = max(1, int(dedup_cap))
+ except (TypeError, ValueError):
+ self._cap = self.DEFAULT_DEDUP_CAP
self._overflow_warned = False
def maybe_log(
@@ -204,8 +196,6 @@ class RejectionLogger:
won't be rejected (no Origin header, allowed origin, same-origin
match against Host / X-Forwarded-Host with proper scheme).
"""
- if not origin:
- return False # Non-browser clients (curl, server-to-server)
if not will_reject(allowed, origin, host, request_scheme,
forwarded_host, forwarded_proto):
return False
diff --git a/tests/test_socketio_cors.py b/tests/test_socketio_cors.py
index 278f443a..b54d94d3 100644
--- a/tests/test_socketio_cors.py
+++ b/tests/test_socketio_cors.py
@@ -139,43 +139,42 @@ def test_resolve_cors_origins_never_silently_returns_wildcard_for_garbage():
# ── will_reject ───────────────────────────────────────────────────────────
-@pytest.mark.parametrize("allowed, origin, host, expected_reject", [
- # Same-origin (Origin's host:port matches the request Host) — allow
- (None, 'http://localhost:8888', 'localhost:8888', False),
- (None, 'http://192.168.1.5:8888','192.168.1.5:8888',False),
- (None, 'https://soulsync.foo', 'soulsync.foo', False),
+@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', True),
- (None, 'https://soulsync.foo', 'localhost:8888', True), # reverse proxy NOT forwarding Host
+ (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', False),
- ('*', 'https://anything.evil', 'localhost:8888', False),
+ ('*', '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', False),
- (['https://soulsync.foo'], 'https://soulsync.foo', 'localhost:8888', False),
+ (['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', True),
+ (['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', False),
-
- # Origin with path component — only host:port should be compared
- (None, 'http://x.com:8080/path', 'x.com:8080', False),
+ (['https://x.com'], 'http://localhost:8888', 'localhost:8888', 'http', False),
])
-def test_will_reject_predicts_engineio_decision(allowed, origin, host, expected_reject):
- assert will_reject(allowed, origin, host) is expected_reject
+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', '') is True
- assert will_reject(['https://x.com'], 'https://x.com', '') is False
- assert will_reject('*', 'https://x.com', '') is False
+ 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():
@@ -183,20 +182,27 @@ def test_will_reject_honors_x_forwarded_host():
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 reverse-proxy setup)
+ # Same-origin via X-Forwarded-Host (typical TLS-terminating reverse proxy)
assert will_reject(None, 'https://soulsync.foo', 'internal:8888',
- forwarded_host='soulsync.foo') is False
+ 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',
- forwarded_host='soulsync.foo, edge.proxy') is False
+ 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',
- forwarded_host='soulsync.foo') is True
+ 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
@@ -232,18 +238,6 @@ def test_will_reject_compares_full_scheme_when_known():
forwarded_proto='https, http') is False
-def test_will_reject_falls_back_to_host_only_when_no_scheme_info():
- """Backwards-compat shim: callers that don't pass scheme info still
- get the basic Host-only same-origin check (the original behavior).
- Important for any integration tests that exercise the predicate
- without a real Flask request context."""
- # No scheme info → host-only match works
- assert will_reject(None, 'https://soulsync.foo', 'soulsync.foo') is False
- assert will_reject(None, 'http://x.com', 'x.com') is False
- # Cross-origin still rejected
- assert will_reject(None, 'https://attacker.com', 'soulsync.foo') is True
-
-
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,
diff --git a/web_server.py b/web_server.py
index 80e929e9..b3d47be7 100644
--- a/web_server.py
+++ b/web_server.py
@@ -228,7 +228,13 @@ _plex_pin_requests_lock = threading.Lock()
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`."""
+ 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(
diff --git a/webui/static/settings.js b/webui/static/settings.js
index e1f23f03..fb47a9ec 100644
--- a/webui/static/settings.js
+++ b/webui/static/settings.js
@@ -2606,8 +2606,9 @@ async function saveSettings(quiet = false) {
.filter(s => s);
const invalid = entries.filter(e => {
if (e === '*') return false;
- // Accept anything matching scheme://host[:port], no path required
- return !/^https?:\/\/[^\s/]+$/i.test(e);
+ // Accept scheme://host[:port] only — no path, query, or fragment.
+ // Engineio compares Origin against {scheme}://{host} exactly.
+ return !/^https?:\/\/[^\s/?#]+$/i.test(e);
});
if (invalid.length) {
showToast(