From 8fd477bf50cda3003fbb9e543d9127e44d1667ac Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:38:40 -0600 Subject: [PATCH] fix(releases): apply the request policy to release listing, not only to download (#1506) The request policy is applied when a user downloads (`/api/releases/download`) or requests (`/api/requests`), but not when they list releases. A user whose mode is `request_book` or `blocked` can still search releases and list a torrent's files through the shared Prowlarr and qBittorrent credentials. Only the UI hides the list. This applies the same gate to `/api/releases`, `/api/releases/inspect` and the release-source records route, with the same codes as the download route (`policy_blocked`, `policy_requires_request`, `required_mode`). - `blocked` and `request_book` are refused. `request_release` is allowed, because that mode lists releases so the user can pick one. The UI opens the request dialog directly under `request_book` and never calls `/api/releases` there. - `/api/releases` resolves the mode per source for the content type. A refused source is dropped from the search, so a user blocked on Prowlarr no longer reaches Prowlarr even when other sources are allowed. It answers 403 only when no source is left, or when the one source asked for is refused. - The records route doesn't know the content type before it fetches, so it refuses only when every content type the source supports is refused. - Admins, auth mode `none` and an instance with requests off bypass the gate, as they do on the download route. A user whose default is `blocked` now gets a 403 toast in the release modal, where they used to get a list of "Unavailable" rows. I made no frontend change. Tests (14, new file) cover each route for blocked, request_book, request_release, an admin, and auth mode none, the per-source drop, and the records route. Ruff, basedpyright and the backend suite pass. Co-authored-by: CaliBrain --- shelfmark/core/release_inspect_routes.py | 17 +- shelfmark/main.py | 121 +++++++-- tests/core/test_release_routes_policy.py | 323 +++++++++++++++++++++++ 3 files changed, 441 insertions(+), 20 deletions(-) create mode 100644 tests/core/test_release_routes_policy.py diff --git a/shelfmark/core/release_inspect_routes.py b/shelfmark/core/release_inspect_routes.py index f9381628..81791474 100644 --- a/shelfmark/core/release_inspect_routes.py +++ b/shelfmark/core/release_inspect_routes.py @@ -18,7 +18,8 @@ from shelfmark.release_sources import get_handler if TYPE_CHECKING: from collections.abc import Callable - from flask import Flask, Response + from flask import Flask + from werkzeug.wrappers import Response logger = setup_logger(__name__) @@ -82,8 +83,16 @@ def inspect_release(data: dict[str, Any]) -> dict[str, Any]: def register_release_inspect_routes( app: Flask, login_required: Callable[..., Any], + *, + policy_gate: Callable[[dict[str, Any]], tuple[Response, int] | None] | None = None, ) -> None: - """Register POST /api/releases/inspect.""" + """Register POST /api/releases/inspect. + + `policy_gate` is given the validated payload and returns the refusal response + for a user whose request policy does not let them see the release, or None. + Listing a release's files reaches the source through shared credentials, so + the gate runs before the handler is asked anything. + """ @app.route("/api/releases/inspect", methods=["POST"]) @login_required @@ -99,4 +108,8 @@ def register_release_inspect_routes( get_handler(str(data["source"])) except ValueError as exc: return jsonify({"error": str(exc)}), 400 + if policy_gate is not None: + refusal = policy_gate(data) + if refusal is not None: + return refusal return jsonify(inspect_release(data)) diff --git a/shelfmark/main.py b/shelfmark/main.py index 8f1a0c83..ff334c5b 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -80,6 +80,7 @@ from shelfmark.core.request_helpers import ( normalize_positive_int, ) from shelfmark.core.request_policy import ( + DEFAULT_SUPPORTED_CONTENT_TYPES, PolicyMode, get_source_content_type_capabilities, merge_request_policy_settings, @@ -510,7 +511,9 @@ def _resolve_policy_mode_for_current_user(*, source: Any, content_type: Any) -> return resolved_mode -def _policy_block_response(mode: PolicyMode) -> tuple[Response, int]: +def _policy_block_response( + mode: PolicyMode, *, error: str = "Download not allowed by policy" +) -> tuple[Response, int]: logger.debug( "download policy guard user=%s db_user_id=%s mode=%s", session.get("user_id"), @@ -521,7 +524,7 @@ def _policy_block_response(mode: PolicyMode) -> tuple[Response, int]: return ( jsonify( { - "error": "Download not allowed by policy", + "error": error, "code": "policy_blocked", "required_mode": PolicyMode.BLOCKED.value, } @@ -531,7 +534,7 @@ def _policy_block_response(mode: PolicyMode) -> tuple[Response, int]: return ( jsonify( { - "error": "Download not allowed by policy", + "error": error, "code": "policy_requires_request", "required_mode": mode.value, } @@ -540,6 +543,75 @@ def _policy_block_response(mode: PolicyMode) -> tuple[Response, int]: ) +# Modes under which the user never sees a release list: request_book asks for the +# book as a whole and blocked gets nothing. request_release still lists releases, +# since that user picks the one they ask for. Listing goes through the shared +# source credentials (Prowlarr, torrent clients, scrapers), so the routes that +# list have to refuse these two modes just as the download route does. +_RELEASE_LISTING_REFUSED_MODES = frozenset({PolicyMode.REQUEST_BOOK, PolicyMode.BLOCKED}) + + +def _release_listing_refusal(*, source: Any, content_type: Any) -> PolicyMode | None: + """Return the policy mode when the current user may not list releases from `source`.""" + mode = _resolve_policy_mode_for_current_user(source=source, content_type=content_type) + if mode is not None and mode in _RELEASE_LISTING_REFUSED_MODES: + return mode + return None + + +def _release_listing_block_response(mode: PolicyMode) -> tuple[Response, int]: + return _policy_block_response(mode, error="Release listing not allowed by policy") + + +def _sources_allowed_for_listing( + sources: list[str], content_type: Any +) -> tuple[list[str], PolicyMode | None]: + """Drop the sources the current user may not list releases from. + + Returns the sources left and, when any was dropped, the mode of the first one + dropped, which is what the response reports when nothing is left. + """ + allowed: list[str] = [] + refused_mode: PolicyMode | None = None + for source_name in sources: + refusal = _release_listing_refusal(source=source_name, content_type=content_type) + if refusal is None: + allowed.append(source_name) + elif refused_mode is None: + refused_mode = refusal + return allowed, refused_mode + + +def _release_source_record_refusal(source_name: str) -> PolicyMode | None: + """Policy refusal for resolving one browse record of a release source. + + The record's content type is not known before it is fetched, so the user is + refused only when every content type the source supports is refused. + """ + capabilities = get_source_content_type_capabilities() + content_types = capabilities.get(normalize_source(source_name)) or set( + DEFAULT_SUPPORTED_CONTENT_TYPES + ) + refused_mode: PolicyMode | None = None + for content_type in sorted(content_types): + refusal = _release_listing_refusal(source=source_name, content_type=content_type) + if refusal is None: + return None + if refused_mode is None: + refused_mode = refusal + return refused_mode + + +def _release_payload_listing_gate(data: dict[str, Any]) -> tuple[Response, int] | None: + """Policy gate for routes that take a release payload, shared with the inspect route.""" + source = data["source"] + resolved_content_type, _inferred = _resolve_release_content_type(data, source) + refusal = _release_listing_refusal(source=source, content_type=resolved_content_type) + if refusal is None: + return None + return _release_listing_block_response(refusal) + + def _resolve_download_user_context( db_user_id: Any, username: Any, @@ -1219,7 +1291,7 @@ def _serialize_release(release: Release) -> dict: return result -register_release_inspect_routes(app, login_required) +register_release_inspect_routes(app, login_required, policy_gate=_release_payload_listing_gate) @app.route("/api/releases/download", methods=["POST"]) @@ -3230,8 +3302,6 @@ def api_releases() -> Response | tuple[Response, int]: source_query_filters = None is_source_provider = bool(provider) and source_results_are_releases(provider) - book: BookMetadata - if not provider or not book_id: if not source_filter or not has_browse_filters: return jsonify({"error": "Parameters 'provider' and 'book_id' are required"}), 400 @@ -3239,9 +3309,31 @@ def api_releases() -> Response | tuple[Response, int]: return jsonify( {"error": f"Source does not support browse release search: {source_filter}"} ), 400 - - book = _build_source_query_book(query_text, browse_filters) source_query_filters = browse_filters + + # Determine which release sources to search + if source_filter: + sources_to_search = [source_filter] + elif is_source_provider: + # Source-backed browse flows stay within the source that produced the record. + sources_to_search = [provider] + else: + # Search only enabled sources + sources_to_search = [src["name"] for src in list_available_sources() if src["enabled"]] + + # The policy gate comes before anything is asked of a source: listing goes + # through the shared source credentials, so a user whose policy never shows + # a release list (request_book, blocked) must not reach them. + sources_to_search, refused_mode = _sources_allowed_for_listing( + sources_to_search, content_type + ) + if not sources_to_search and refused_mode is not None: + return _release_listing_block_response(refused_mode) + + book: BookMetadata + + if source_query_filters is not None: + book = _build_source_query_book(query_text, browse_filters) elif is_source_provider: # Source-backed browse flows can reopen the release modal with provider=. # In that flow, treat the source-native record as release-search context instead of @@ -3300,16 +3392,6 @@ def api_releases() -> Response | tuple[Response, int]: book.search_title = title_param book.title = title_param - # Determine which release sources to search - if source_query_filters is not None or source_filter: - sources_to_search = [source_filter] - elif is_source_provider: - # Source-backed browse flows stay within the source that produced the record. - sources_to_search = [provider] - else: - # Search only enabled sources - sources_to_search = [src["name"] for src in list_available_sources() if src["enabled"]] - # Search each source for releases. # # Under a wall-clock budget: this endpoint is synchronous, and the bypass path it @@ -3439,6 +3521,9 @@ def api_release_source_record(source_name: str, record_id: str) -> Response | tu from shelfmark.release_sources import get_source source = get_source(source_name) + refused_mode = _release_source_record_refusal(source_name) + if refused_mode is not None: + return _release_listing_block_response(refused_mode) record = source.get_record(record_id) if record is None: return jsonify({"error": "Record not found"}), 404 diff --git a/tests/core/test_release_routes_policy.py b/tests/core/test_release_routes_policy.py new file mode 100644 index 00000000..3f34e752 --- /dev/null +++ b/tests/core/test_release_routes_policy.py @@ -0,0 +1,323 @@ +"""The request policy gates release listing, not only release download. + +/api/releases, /api/releases/inspect and the release-source record route all reach +upstream sources through shared credentials (Prowlarr, qBittorrent, scrapers). A user +whose policy is request_book or blocked never sees a release list in the UI, so the +routes have to refuse them too; request_release still lists, because that user picks +the release they ask for. +""" + +from __future__ import annotations + +import importlib +import uuid +from unittest.mock import patch + +import pytest + +from shelfmark.download.postprocess.packs import PackFile +from shelfmark.release_sources import BrowseRecord + + +@pytest.fixture(scope="module") +def main_module(): + with patch("shelfmark.download.orchestrator.start"): + import shelfmark.main as main + + importlib.reload(main) + return main + + +@pytest.fixture +def client(main_module): + return main_module.app.test_client() + + +def _login(client, main_module, *, is_admin: bool = False) -> dict: + user = main_module.user_db.create_user( + username=f"reader-{uuid.uuid4().hex[:8]}", role="admin" if is_admin else "user" + ) + with client.session_transaction() as sess: + sess["user_id"] = user["username"] + sess["is_admin"] = is_admin + sess["db_user_id"] = user["id"] + return user + + +def _policy( + *, + default_ebook: str = "download", + default_audiobook: str = "download", + rules: list[dict] | None = None, +) -> dict: + return { + "REQUESTS_ENABLED": True, + "REQUEST_POLICY_DEFAULT_EBOOK": default_ebook, + "REQUEST_POLICY_DEFAULT_AUDIOBOOK": default_audiobook, + "MAX_PENDING_REQUESTS_PER_USER": 20, + "REQUESTS_ALLOW_NOTES": True, + "REQUEST_POLICY_RULES": rules or [], + } + + +def _with_policy(main_module, policy: dict): + return ( + patch.object(main_module, "get_auth_mode", return_value="builtin"), + patch.object(main_module, "load_users_request_policy_settings", return_value=policy), + ) + + +class _Source: + def __init__(self, searched: list[str]) -> None: + self._searched = searched + + def search(self, book, plan, *, expand_search=False, content_type="ebook"): + self._searched.append(book.title) + return [] + + def get_record(self, record_id: str) -> BrowseRecord: + self._searched.append(f"record:{record_id}") + return BrowseRecord(id=record_id, title="Browsed", source="stub") + + def get_column_config(self): + from shelfmark.release_sources import _default_column_config + + return _default_column_config() + + +def _list_releases(client, main_module, policy: dict, *, query: dict | None = None): + """Drive /api/releases with stubbed sources; returns (response, titles searched).""" + searched: list[str] = [] + sources = [ + {"name": "direct_download", "enabled": True}, + {"name": "prowlarr", "enabled": True}, + ] + auth_patch, policy_patch = _with_policy(main_module, policy) + with ( + auth_patch, + policy_patch, + patch("shelfmark.release_sources.list_available_sources", return_value=sources), + patch("shelfmark.release_sources.get_source", return_value=_Source(searched)), + patch("shelfmark.release_sources.source_results_are_releases", return_value=False), + ): + resp = client.get( + "/api/releases", + query_string={"provider": "manual", "book_id": "abc", "title": "Dune", **(query or {})}, + ) + return resp, searched + + +class TestReleaseListPolicy: + def test_request_book_user_cannot_list_releases(self, client, main_module): + _login(client, main_module) + + resp, searched = _list_releases(client, main_module, _policy(default_ebook="request_book")) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_requires_request" + assert resp.json["required_mode"] == "request_book" + assert searched == [] + + def test_blocked_user_cannot_list_releases(self, client, main_module): + _login(client, main_module) + + resp, searched = _list_releases(client, main_module, _policy(default_ebook="blocked")) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_blocked" + assert searched == [] + + def test_request_release_user_still_lists_releases(self, client, main_module): + _login(client, main_module) + + resp, searched = _list_releases( + client, main_module, _policy(default_ebook="request_release") + ) + + assert resp.status_code == 200 + assert searched == ["Dune", "Dune"] + + def test_policy_follows_the_requested_content_type(self, client, main_module): + _login(client, main_module) + policy = _policy(default_ebook="download", default_audiobook="blocked") + + ebook, _ = _list_releases(client, main_module, policy, query={"content_type": "ebook"}) + audiobook, searched = _list_releases( + client, main_module, policy, query={"content_type": "audiobook"} + ) + + assert ebook.status_code == 200 + assert audiobook.status_code == 403 + assert searched == [] + + def test_a_source_the_user_is_blocked_on_is_left_out(self, client, main_module): + _login(client, main_module) + policy = _policy(rules=[{"source": "prowlarr", "content_type": "*", "mode": "blocked"}]) + + resp, searched = _list_releases(client, main_module, policy) + + assert resp.status_code == 200 + assert resp.json["sources_searched"] == ["direct_download"] + assert searched == ["Dune"] + + def test_an_explicitly_requested_blocked_source_is_refused(self, client, main_module): + _login(client, main_module) + policy = _policy(rules=[{"source": "prowlarr", "content_type": "*", "mode": "blocked"}]) + + resp, searched = _list_releases(client, main_module, policy, query={"source": "prowlarr"}) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_blocked" + assert searched == [] + + def test_admin_is_not_gated(self, client, main_module): + _login(client, main_module, is_admin=True) + + resp, searched = _list_releases(client, main_module, _policy(default_ebook="blocked")) + + assert resp.status_code == 200 + assert searched == ["Dune", "Dune"] + + +class _Handler: + def __init__(self) -> None: + self.calls = 0 + + def list_files(self, release_data): + self.calls += 1 + return [PackFile("Dune.epub", 100)] + + +def _inspect(client, main_module, policy: dict, *, content_type: str = "ebook"): + handler = _Handler() + auth_patch, policy_patch = _with_policy(main_module, policy) + with ( + auth_patch, + policy_patch, + patch("shelfmark.core.release_inspect_routes.get_handler", return_value=handler), + ): + resp = client.post( + "/api/releases/inspect", + json={ + "source": "prowlarr", + "source_id": "abc", + "title": "Dune", + "content_type": content_type, + }, + ) + return resp, handler + + +class TestReleaseInspectPolicy: + def test_request_book_user_cannot_inspect(self, client, main_module): + _login(client, main_module) + + resp, handler = _inspect(client, main_module, _policy(default_ebook="request_book")) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_requires_request" + assert handler.calls == 0 + + def test_blocked_user_cannot_inspect(self, client, main_module): + _login(client, main_module) + + resp, handler = _inspect(client, main_module, _policy(default_ebook="blocked")) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_blocked" + assert handler.calls == 0 + + def test_policy_uses_the_payload_content_type(self, client, main_module): + _login(client, main_module) + policy = _policy(default_ebook="blocked", default_audiobook="request_release") + + resp, handler = _inspect(client, main_module, policy, content_type="audiobook") + + assert resp.status_code == 200 + assert resp.json["inspected"] is True + assert handler.calls == 1 + + +def _get_record( + client, + main_module, + policy: dict, + *, + capabilities: dict[str, set[str]], + source: str = "audiobookbay", +): + searched: list[str] = [] + auth_patch, policy_patch = _with_policy(main_module, policy) + with ( + auth_patch, + policy_patch, + patch.object( + main_module, "get_source_content_type_capabilities", return_value=capabilities + ), + patch("shelfmark.release_sources.get_source", return_value=_Source(searched)), + ): + resp = client.get(f"/api/release-sources/{source}/records/rec-1") + return resp, searched + + +class TestReleaseSourceRecordPolicy: + def test_blocked_user_cannot_resolve_a_record(self, client, main_module): + _login(client, main_module) + + resp, searched = _get_record( + client, + main_module, + _policy(default_audiobook="blocked"), + capabilities={"audiobookbay": {"audiobook"}}, + ) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_blocked" + assert searched == [] + + def test_request_book_user_cannot_resolve_a_record(self, client, main_module): + _login(client, main_module) + + resp, searched = _get_record( + client, + main_module, + _policy(default_audiobook="request_book"), + capabilities={"audiobookbay": {"audiobook"}}, + ) + + assert resp.status_code == 403 + assert resp.json["code"] == "policy_requires_request" + assert resp.json["required_mode"] == "request_book" + assert searched == [] + + def test_request_book_becomes_request_release_for_a_release_source(self, client, main_module): + """direct_download's browse rows are concrete releases, so request_book turns into + request_release there (the policy's own rule) and the record stays reachable.""" + _login(client, main_module) + + resp, searched = _get_record( + client, + main_module, + _policy(default_ebook="request_book"), + capabilities={"direct_download": {"ebook"}}, + source="direct_download", + ) + + assert resp.status_code == 200 + assert resp.json["id"] == "rec-1" + assert searched == ["record:rec-1"] + + def test_record_is_refused_only_when_every_content_type_is(self, client, main_module): + _login(client, main_module) + policy = _policy(default_ebook="download", default_audiobook="blocked") + + allowed, _ = _get_record( + client, main_module, policy, capabilities={"audiobookbay": {"ebook", "audiobook"}} + ) + refused, searched = _get_record( + client, main_module, policy, capabilities={"audiobookbay": {"audiobook"}} + ) + + assert allowed.status_code == 200 + assert refused.status_code == 403 + assert searched == []