Merge pull request #383 from Nezreka/fix/socketio-cors-wildcard
Lock down Socket.IO CORS — same-origin default + opt-in allow-list
This commit is contained in:
BoulderBadgeDad
GitHub
commit
9e8b5aee3b
6 changed files with 767 additions and 1 deletions
256
core/socketio_cors.py
Normal file
256
core/socketio_cors.py
Normal file
|
|
@ -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}")
|
||||
442
tests/test_socketio_cors.py
Normal file
442
tests/test_socketio_cors.py
Normal file
|
|
@ -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 == []
|
||||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -5409,6 +5409,14 @@
|
|||
<div class="form-group" id="security-change-pin-section" style="display: none;">
|
||||
<button class="auth-button" onclick="showChangeSecurityPin()">Change PIN</button>
|
||||
</div>
|
||||
|
||||
<div class="form-group">
|
||||
<label for="security-cors-origins">Allowed WebSocket Origins:</label>
|
||||
<textarea id="security-cors-origins" rows="3" placeholder="https://soulsync.example.com http://192.168.1.5:8888" style="width: 100%; font-family: monospace; font-size: 12px;"></textarea>
|
||||
<div class="setting-help-text">
|
||||
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 <code>*</code> on its own line to allow any origin (insecure — only do this if you understand why you need it).
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- Discovery Settings -->
|
||||
|
|
|
|||
|
|
@ -3444,6 +3444,7 @@ 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' },
|
||||
{ title: 'Faster Docker Startup — yt-dlp Pinned', desc: 'docker startup used to run `pip install -U yt-dlp` on every container start. removed that — yt-dlp is now pinned in requirements.txt so startup is fast and reproducible. tradeoff: youtube fixes ship via soulsync releases now instead of next container restart.' },
|
||||
],
|
||||
'2.4.0': [
|
||||
|
|
|
|||
|
|
@ -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,9 +2592,32 @@ 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() || '',
|
||||
}
|
||||
};
|
||||
|
||||
// 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 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(
|
||||
`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...');
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue