diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 0041ca8d..509f3af1 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -46,6 +46,7 @@ These environment variables are used at startup before the settings system loads | `ENABLE_LOGGING` | Enable file logging under LOG_ROOT/shelfmark/ (including shelfmark.log and startup logs). | boolean | `true` | | `FLASK_HOST` | Host address for the Flask web server. | string | `0.0.0.0` | | `FLASK_PORT` | Port number for the Flask web server. | number | `8084` | +| `SOCKETIO_ALLOWED_ORIGINS` | Extra origins allowed to open the live-update websocket, comma separated (e.g. https://books.example.com). The app's own origin is always allowed, including behind a proxy that sends X-Forwarded-Host and X-Forwarded-Proto. Only needed if your proxy changes Host without sending those headers. | string | `unset` | | `SESSION_COOKIE_SECURE` | Enable secure cookies (requires HTTPS). | boolean | `false` | | `CWA_DB_PATH` | Path to the Calibre-Web database for authentication integration. | string (path) | `/auth/app.db` | | `HIDE_LOCAL_AUTH` | Hide the username/password login form when OIDC is active. | boolean | `false` | @@ -101,6 +102,13 @@ Port number for the Flask web server. - **Type:** number - **Default:** `8084` +#### `SOCKETIO_ALLOWED_ORIGINS` + +Extra origins allowed to open the live-update websocket, comma separated (e.g. https://books.example.com). The app's own origin is always allowed, including behind a proxy that sends X-Forwarded-Host and X-Forwarded-Proto. Only needed if your proxy changes Host without sending those headers. + +- **Type:** string +- **Default:** `unset` + #### `SESSION_COOKIE_SECURE` Enable secure cookies (requires HTTPS). diff --git a/scripts/generate_env_docs.py b/scripts/generate_env_docs.py index ec11e6b6..dbca3226 100755 --- a/scripts/generate_env_docs.py +++ b/scripts/generate_env_docs.py @@ -160,6 +160,12 @@ def _generate_bootstrap_env_docs() -> list[str]: "type": "number", "default": "8084", }, + { + "name": "SOCKETIO_ALLOWED_ORIGINS", + "description": "Extra origins allowed to open the live-update websocket, comma separated (e.g. https://books.example.com). The app's own origin is always allowed, including behind a proxy that sends X-Forwarded-Host and X-Forwarded-Proto. Only needed if your proxy changes Host without sending those headers.", + "type": "string", + "default": "unset", + }, { "name": "SESSION_COOKIE_SECURE", "description": "Enable secure cookies (requires HTTPS).", diff --git a/shelfmark/config/env.py b/shelfmark/config/env.py index 06cd8dc7..b01a31d4 100644 --- a/shelfmark/config/env.py +++ b/shelfmark/config/env.py @@ -155,6 +155,13 @@ ENABLE_LOGGING = string_to_bool(os.getenv("ENABLE_LOGGING", "true")) FLASK_HOST = os.getenv("FLASK_HOST", "0.0.0.0") FLASK_PORT = int(os.getenv("FLASK_PORT", "8084")) +# Extra origins allowed to open the Socket.IO websocket, comma separated. Needed +# only when a reverse proxy changes Host and sends no X-Forwarded-Host/Proto. +SOCKETIO_ALLOWED_ORIGINS = [ + origin.strip().rstrip("/") + for origin in os.getenv("SOCKETIO_ALLOWED_ORIGINS", "").split(",") + if origin.strip().rstrip("/") +] # ============================================================================= diff --git a/shelfmark/core/socketio_origins.py b/shelfmark/core/socketio_origins.py new file mode 100644 index 00000000..d54bb656 --- /dev/null +++ b/shelfmark/core/socketio_origins.py @@ -0,0 +1,48 @@ +"""Origin policy for the Socket.IO handshake.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from collections.abc import Callable + + +def _first(value: str) -> str: + return value.split(",")[0].strip() + + +def _own_origins(environ: dict[str, Any]) -> set[str]: + """Origins python-engineio treats as same-origin when no list is configured.""" + origins: set[str] = set() + scheme = environ.get("wsgi.url_scheme") + host = environ.get("HTTP_HOST") + if scheme and host: + origins.add(f"{scheme}://{host}") + if "HTTP_X_FORWARDED_PROTO" in environ or "HTTP_X_FORWARDED_HOST" in environ: + fwd_scheme = _first(environ.get("HTTP_X_FORWARDED_PROTO", scheme)) + fwd_host = _first(environ.get("HTTP_X_FORWARDED_HOST", host)) + origins.add(f"{fwd_scheme}://{fwd_host}") + return origins + + +def build_cors_allowed_origins( + extra_origins: list[str], +) -> None | str | Callable[[str | None, dict[str, Any]], bool]: + """Value for Flask-SocketIO's ``cors_allowed_origins``. + + With no extras this is ``None``, the library's own same-origin check. With + extras the same check applies and the listed origins are accepted as well. + """ + if not extra_origins: + return None + if "*" in extra_origins: + return "*" + extras = {origin.lower() for origin in extra_origins} + + def is_allowed(origin: str | None, environ: dict[str, Any]) -> bool: + if not origin: + return True + return origin in _own_origins(environ) or origin.lower() in extras + + return is_allowed diff --git a/shelfmark/download/clients/torrent_utils.py b/shelfmark/download/clients/torrent_utils.py index 55a79c14..779af520 100644 --- a/shelfmark/download/clients/torrent_utils.py +++ b/shelfmark/download/clients/torrent_utils.py @@ -4,7 +4,9 @@ from __future__ import annotations import base64 import hashlib +import ipaddress import re +import socket import time from binascii import Error as BinasciiError from dataclasses import dataclass @@ -262,8 +264,10 @@ def _fetch_torrent_info(url: str) -> TorrentInfo: current_url = url redirects_remaining = _TORRENT_FETCH_MAX_REDIRECTS while True: + trusted = _is_trusted_torrent_fetch_url(current_url) + _check_torrent_fetch_hop(current_url, trusted=trusted) request_headers = dict(headers) - if not _is_trusted_torrent_fetch_url(current_url): + if not trusted: request_headers.pop("X-Api-Key", None) resp = requests.get( @@ -328,6 +332,34 @@ def _fetch_torrent_info(url: str) -> TorrentInfo: return TorrentInfo(info_hash=None, torrent_data=None, is_magnet=False, fetch_error=message) +def _resolve_host_addresses(host: str) -> list[str]: + """Resolve a host with the system resolver; raises OSError when it does not resolve.""" + return [str(info[4][0]) for info in socket.getaddrinfo(host, None, proto=socket.IPPROTO_TCP)] + + +def _is_public_address(address: str) -> bool: + ip = ipaddress.ip_address(address.split("%", 1)[0]) + if isinstance(ip, ipaddress.IPv6Address) and ip.ipv4_mapped is not None: + ip = ip.ipv4_mapped + return ip.is_global + + +def _check_torrent_fetch_hop(url: str, *, trusted: bool) -> None: + """Refuse a fetch hop that is not http(s) or that resolves to a non-public address. + + The configured Prowlarr/Newznab origin is exempt because it commonly lives + on the LAN and its own download links point back at it. + """ + parsed = urlparse(url) + if parsed.scheme.lower() not in {"http", "https"} or not parsed.hostname: + raise ValueError("torrent fetch refused: URL is not http or https") + if trusted: + return + addresses = _resolve_host_addresses(parsed.hostname) + if not addresses or not all(_is_public_address(address) for address in addresses): + raise ValueError("torrent fetch refused: address is not public") + + def _is_trusted_torrent_fetch_url(url: str) -> bool: parsed = urlparse(url) origin = _url_origin(parsed) diff --git a/shelfmark/download/postprocess/custom_script.py b/shelfmark/download/postprocess/custom_script.py index 73ce003c..876ddfd9 100644 --- a/shelfmark/download/postprocess/custom_script.py +++ b/shelfmark/download/postprocess/custom_script.py @@ -112,6 +112,18 @@ def prepare_custom_script_execution( ) +def _target_argument(execution: CustomScriptExecution) -> str: + """The target as one argv entry; a relative name starting with "-" gets "./" first. + + Many scripts read a leading "-" as an option. Path() drops "./", so this has + to be applied to the string. Absolute mode starts with "/" and is unchanged. + """ + target = str(execution.target_arg) + if execution.mode == "relative" and target.startswith("-"): + return f"./{target}" + return target + + def run_custom_script( execution: CustomScriptExecution, *, @@ -152,7 +164,7 @@ def run_custom_script( result = run_blocking_io( subprocess.run, - [execution.script_path, str(execution.target_arg)], + [execution.script_path, _target_argument(execution)], **run_kwargs, ) if result.stdout: diff --git a/shelfmark/main.py b/shelfmark/main.py index ff334c5b..edf78e89 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -35,6 +35,7 @@ from shelfmark.config.env import ( RELEASE_VERSION, SESSION_COOKIE_NAME, SESSION_COOKIE_SECURE_ENV, + SOCKETIO_ALLOWED_ORIGINS, _is_config_dir_writable, string_to_bool, ) @@ -92,6 +93,7 @@ from shelfmark.core.requests_service import ( reopen_failed_request, sync_delivery_states_from_queue_status, ) +from shelfmark.core.socketio_origins import build_cors_allowed_origins from shelfmark.core.user_db import UserDB from shelfmark.core.utils import AUDIOBOOK_FORMATS, normalize_base_path from shelfmark.core.viewer_redaction import ( @@ -150,7 +152,8 @@ app.wsgi_app = wsgi_app # We run this app under Gunicorn with a gevent websocket worker (even when DEBUG=true), # so Socket.IO should always use gevent here. async_mode = "gevent" -socketio_cors_allowed_origins = "*" +# Same-origin only (python-engineio default), plus any SOCKETIO_ALLOWED_ORIGINS. +socketio_cors_allowed_origins = build_cors_allowed_origins(SOCKETIO_ALLOWED_ORIGINS) # Initialize Flask-SocketIO with reverse proxy support socketio_path = f"{BASE_PATH}/socket.io" if BASE_PATH else "/socket.io" @@ -165,7 +168,6 @@ socketio_init_kwargs: dict[str, Any] = { "ping_interval": 25, # Allow both websocket and polling for better compatibility "transports": ["websocket", "polling"], - # Enable CORS for all origins (you can restrict this in production) "allow_upgrades": True, # Important for proxies that buffer "http_compression": True, @@ -183,7 +185,10 @@ def _user_room_queue_status(user_id: int | None = None) -> dict[str, dict[str, A ws_manager.set_queue_status_fn(_user_room_queue_status) logger.info("Flask-SocketIO initialized with async_mode='%s'", async_mode) -logger.info("Socket.IO CORS allowed origins: %s", socketio_cors_allowed_origins) +logger.info( + "Socket.IO origin check: same-origin plus %s configured origin(s)", + len(SOCKETIO_ALLOWED_ORIGINS), +) # Ensure all plugins are loaded before starting the download coordinator. # This prevents a race condition where the download loop could try to process diff --git a/tests/core/test_custom_script_argv.py b/tests/core/test_custom_script_argv.py new file mode 100644 index 00000000..e8fb0b63 --- /dev/null +++ b/tests/core/test_custom_script_argv.py @@ -0,0 +1,62 @@ +"""The file name handed to a custom script must not be readable as an option.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from shelfmark.download.postprocess.custom_script import ( + CustomScriptExecution, + run_custom_script, +) + + +def _execution(target_arg: str, mode: str) -> CustomScriptExecution: + destination = Path("/library") + return CustomScriptExecution( + script_path="/scripts/hook.sh", + target_arg=Path(target_arg), + target_abs=destination / target_arg, + destination=destination, + mode=mode, + phase="after_move", + payload_json=None, + ) + + +def _argv_for(execution: CustomScriptExecution) -> list[str]: + with patch( + "shelfmark.download.postprocess.custom_script.run_blocking_io", + return_value=MagicMock(stdout=""), + ) as run: + assert run_custom_script(execution, task_id="t1", status_callback=MagicMock()) + return list(run.call_args.args[1]) + + +@pytest.mark.parametrize( + ("target", "expected"), + [ + ("-rf", "./-rf"), + ("--help.epub", "./--help.epub"), + ("-Author/book.epub", "./-Author/book.epub"), + ("normal.epub", "normal.epub"), + ("Author/-odd.epub", "Author/-odd.epub"), + ], +) +def test_relative_mode_target_never_starts_with_a_dash(target, expected): + assert _argv_for(_execution(target, "relative")) == ["/scripts/hook.sh", expected] + + +def test_absolute_mode_is_unchanged(): + execution = CustomScriptExecution( + script_path="/scripts/hook.sh", + target_arg=Path("/library/-rf"), + target_abs=Path("/library/-rf"), + destination=Path("/library"), + mode="absolute", + phase="after_move", + payload_json=None, + ) + assert _argv_for(execution) == ["/scripts/hook.sh", "/library/-rf"] diff --git a/tests/core/test_socketio_origins.py b/tests/core/test_socketio_origins.py new file mode 100644 index 00000000..21511889 --- /dev/null +++ b/tests/core/test_socketio_origins.py @@ -0,0 +1,75 @@ +"""Socket.IO handshakes are limited to the app's own origin (plus configured extras).""" + +from __future__ import annotations + +from flask import Flask +from flask_socketio import SocketIO + +from shelfmark.core.socketio_origins import build_cors_allowed_origins + +_HANDSHAKE = "/socket.io/?EIO=4&transport=polling" + + +def _client(extra: list[str] | None = None): + app = Flask(__name__) + SocketIO( + app, + async_mode="threading", + cors_allowed_origins=build_cors_allowed_origins(extra or []), + ) + return app.test_client() + + +def _handshake(client, origin: str | None, **headers: str) -> int: + request_headers = dict(headers) + if origin is not None: + request_headers["Origin"] = origin + return client.get( + _HANDSHAKE, headers=request_headers, base_url="http://shelf.example" + ).status_code + + +def test_same_origin_handshake_is_accepted(): + assert _handshake(_client(), "http://shelf.example") == 200 + + +def test_foreign_origin_handshake_is_refused(): + assert _handshake(_client(), "https://evil.example") == 400 + + +def test_sibling_subdomain_is_refused(): + assert _handshake(_client(), "http://other.example") == 400 + + +def test_non_browser_client_without_origin_is_accepted(): + assert _handshake(_client(), None) == 200 + + +def test_forwarded_proto_and_host_origin_is_accepted(): + status = _handshake( + _client(), + "https://books.example.org", + **{"X-Forwarded-Proto": "https", "X-Forwarded-Host": "books.example.org"}, + ) + assert status == 200 + + +def test_https_origin_without_forwarded_headers_is_refused_by_default(): + assert _handshake(_client(), "https://shelf.example") == 400 + + +def test_extra_origin_is_accepted_and_others_still_refused(): + client = _client(["https://shelf.example"]) + assert _handshake(client, "https://shelf.example") == 200 + assert _handshake(client, "http://shelf.example") == 200 + assert _handshake(client, "https://evil.example") == 400 + + +def test_default_is_the_library_same_origin_check(): + assert build_cors_allowed_origins([]) is None + + +def test_app_does_not_allow_any_origin(): + from shelfmark import main as main_module + + assert main_module.socketio.server.eio.cors_allowed_origins is None diff --git a/tests/prowlarr/conftest.py b/tests/prowlarr/conftest.py new file mode 100644 index 00000000..1a2a1ad4 --- /dev/null +++ b/tests/prowlarr/conftest.py @@ -0,0 +1,10 @@ +import pytest + + +@pytest.fixture(autouse=True) +def _public_dns_for_torrent_fetch(monkeypatch): + """Resolve every torrent-fetch hostname to a public address unless a test overrides it.""" + monkeypatch.setattr( + "shelfmark.download.clients.torrent_utils._resolve_host_addresses", + lambda host: ["93.184.216.34"], + ) diff --git a/tests/prowlarr/test_torrent_utils.py b/tests/prowlarr/test_torrent_utils.py index 7a2332b8..8d9a3be7 100644 --- a/tests/prowlarr/test_torrent_utils.py +++ b/tests/prowlarr/test_torrent_utils.py @@ -593,6 +593,126 @@ class TestExtractTorrentInfo: assert mock_get.call_count == 6 +class TestTorrentFetchHopCheck: + """Each hop of the torrent fetch must point at a public address.""" + + @staticmethod + def _setup(monkeypatch, resolutions, responses, config_values=None): + values = config_values or {"PROWLARR_URL": "https://prowlarr.example"} + monkeypatch.setattr( + "shelfmark.download.clients.torrent_utils.config.get", + lambda key, default="": values.get(key, default), + ) + monkeypatch.setattr( + "shelfmark.download.clients.torrent_utils._resolve_host_addresses", + lambda host: resolutions[host], + ) + mock_get = MagicMock(side_effect=responses) + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + return mock_get + + @staticmethod + def _redirect(location): + response = MagicMock(status_code=302) + response.headers = {"Location": location} + return response + + @pytest.mark.parametrize( + "address", + [ + "127.0.0.1", + "169.254.169.254", + "10.0.0.5", + "192.168.1.10", + "::1", + "fe80::1", + "::ffff:127.0.0.1", + ], + ) + def test_redirect_to_non_public_address_is_refused(self, monkeypatch, address): + mock_get = self._setup( + monkeypatch, + {"prowlarr.example": ["10.0.0.2"], "evil.example": [address]}, + [self._redirect("http://evil.example/x")], + ) + + result = extract_torrent_info("https://prowlarr.example/1/download", fetch_torrent=True) + + assert result.torrent_data is None + assert result.fetch_error + mock_get.assert_called_once() + + def test_any_non_public_answer_refuses_the_hop(self, monkeypatch): + mock_get = self._setup( + monkeypatch, + {"mixed.example": ["93.184.216.34", "127.0.0.1"]}, + [MagicMock(status_code=200, content=b"")], + ) + + result = extract_torrent_info("https://mixed.example/a.torrent", fetch_torrent=True) + + assert result.fetch_error + mock_get.assert_not_called() + + def test_initial_url_to_loopback_is_refused(self, monkeypatch): + mock_get = self._setup(monkeypatch, {"127.0.0.1": ["127.0.0.1"]}, []) + + result = extract_torrent_info("http://127.0.0.1:8080/a.torrent", fetch_torrent=True) + + assert result.fetch_error + mock_get.assert_not_called() + + def test_non_http_redirect_scheme_is_refused(self, monkeypatch): + mock_get = self._setup( + monkeypatch, + {"prowlarr.example": ["10.0.0.2"]}, + [self._redirect("ftp://prowlarr.example/x")], + ) + + result = extract_torrent_info("https://prowlarr.example/1/download", fetch_torrent=True) + + assert result.fetch_error + mock_get.assert_called_once() + + def test_configured_prowlarr_host_may_be_on_the_lan(self, monkeypatch): + info_dict = {b"name": b"b", b"length": 1, b"piece length": 16384, b"pieces": b"\x00" * 20} + torrent_data = bencode_encode({b"info": info_dict}) + final = MagicMock(status_code=200, content=torrent_data) + final.raise_for_status = MagicMock() + mock_get = self._setup( + monkeypatch, + {"prowlarr.lan": ["192.168.1.20"]}, + [final], + {"PROWLARR_URL": "http://prowlarr.lan:9696", "PROWLARR_API_KEY": "secret"}, + ) + + result = extract_torrent_info( + "http://prowlarr.lan:9696/1/download?indexer=7", fetch_torrent=True + ) + + assert result.torrent_data == torrent_data + assert mock_get.call_args.kwargs["headers"].get("X-Api-Key") == "secret" + + def test_unresolvable_name_reports_a_fetch_error(self, monkeypatch): + def fail(host): + raise OSError("Name or service not known") + + monkeypatch.setattr( + "shelfmark.download.clients.torrent_utils.config.get", + lambda key, default="": default, + ) + monkeypatch.setattr( + "shelfmark.download.clients.torrent_utils._resolve_host_addresses", fail + ) + mock_get = MagicMock() + monkeypatch.setattr("shelfmark.download.clients.torrent_utils.requests.get", mock_get) + + result = extract_torrent_info("https://nowhere.invalid/a.torrent", fetch_torrent=True) + + assert result.fetch_error + mock_get.assert_not_called() + + class TestExtractHashFromMagnet: """Tests for extracting hash from magnet links."""