fix(auto-download): only fulfil requests the owner could download directly (#1676)

Automatic downloads fulfil pending Hardcover requests as an admin, and
they picked up every pending request with provider `hardcover`. A
request a user makes from the search UI carries that provider too, so
with automatic downloads on, a request from a user whose policy needs
approval was approved by the pass instead of waiting for an admin. I
added that behaviour in #1516 and #1535 and missed this case.

The pass now runs the same check the download route makes. A request is
searched only on the release sources where its owner's resolved policy
(global settings plus their own overrides) is Download. Requests owned
by an admin, installs without login, and users with requests turned off
are not limited, since all of them can already download directly. A
request left with no source stays pending and untouched: no search, no
backoff state, no note. The 7-day and quick retries only reopen a failed
download on the same terms, so a request the pass can't fulfil isn't
moved back to pending for nothing.

The pass summary gains an `awaiting_approval` count. The
`AUTO_DOWNLOAD_ENABLED` description, the env var reference and the
Hardcover sync docs say which requests it approves.

Tests cover approval needed, direct download allowed, admin owned,
blocked, a per-source rule, a per-user override, requests turned off, no
login, and the retry reopen.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Bug Fixes**
* Automatic downloads now respect each request owner’s policy and
permitted sources. Requests that require approval or are blocked by
policy remain pending rather than being searched.
* Failed downloads are reopened for retry only when the owner’s policy
allows direct fulfillment.
* Admin-owned requests and requests in unauthenticated mode retain their
existing automatic fulfillment behavior.
* **Documentation**
* Clarified how request ownership, download policies, approval
requirements, and retries affect automatic downloads.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
splitsec2 authored and GitHub committed 2026-11-08 22:39:07 -07:00
1 parent 8c06862ec7
commit a01e700c81
5 files changed
+305 -10

No files matched your search

+2 -2
View File
@@ -1127,7 +1127,7 @@ Custom label for the OIDC sign-in button on the login page.
| `HARDCOVER_SYNC_CONTENT_TYPE` | Content type for synced requests. "Ebooks and audiobooks" creates one request of each per shelf book; every request is checked against its own library and downloaded with its own format guard and source priority. | string (choice) | `audiobook` |
| `HARDCOVER_SYNC_INTERVAL` | How often the scheduled sync runs (minimum 1 minute). | number | `6` |
| `HARDCOVER_SYNC_INTERVAL_UNIT` | Whether the sync interval is measured in minutes or hours. | string (choice) | `hours` |
| `AUTO_DOWNLOAD_ENABLED` | Auto-approve and auto-download matching releases for synced requests. | boolean | `false` |
| `AUTO_DOWNLOAD_ENABLED` | Auto-approve and auto-download matching releases for Hardcover requests. Another user's request is only approved this way where their request policy already lets them download directly; the rest wait for an admin. | boolean | `false` |
| `AUTO_DOWNLOAD_SOURCE_PRIORITY` | Drag to set which release sources are tried first for audiobook requests. The first source with a strict match wins. Disabled sources are skipped. | JSON array | _empty list_ |
| `AUTO_DOWNLOAD_EBOOK_SOURCE_PRIORITY` | Drag to set which release sources are tried first for ebook requests. The first source with a strict match wins. Disabled sources are skipped. | JSON array | _empty list_ |
| `AUTO_DOWNLOAD_MIN_SEEDERS` | Skip torrent releases with fewer seeders than this. | number | `1` |
@@ -1204,7 +1204,7 @@ Whether the sync interval is measured in minutes or hours.
**Enable automatic downloads**
Auto-approve and auto-download matching releases for synced requests.
Auto-approve and auto-download matching releases for Hardcover requests. Another user's request is only approved this way where their request policy already lets them download directly; the rest wait for an admin.
- **Type:** boolean
- **Default:** `false`
+4 -2
View File
@@ -21,6 +21,8 @@ A book is skipped when:
When enabled, each pending request that came from Hardcover is searched across your release sources in the priority order configured for its content type (audiobook requests use the audiobook list, ebook requests the ebook list). The first source that yields a **strict match** wins, and the best candidate from that source is queued through the normal request fulfilment path, so it appears in the activity view and download history exactly like a request an admin approved.
Automatic downloads act for an admin, so they only approve what the request's owner could have downloaded without asking. Synced requests belong to an admin and are always searched. A request another user made is searched only on the release sources where their request policy (including any per-user override) resolves to **Download**, and only while requests are turned on for them; with requests off they download directly anyway. A request whose policy needs approval, or blocks that content type, is left pending for an admin, untouched, like any other request awaiting review. A failed download is only reopened for a retry on the same terms.
A release is a strict match only when all of the following hold:
- at least 85% of the significant words in the book's title appear in the release title. Numbers from 1 to 20 count as the same word however they are written, so *Part II*, *Part Two* and *Part 2* agree, while *Part One* never stands in for *Part Two*;
@@ -74,7 +76,7 @@ The check **fails closed** here: if an enabled library cannot be read and has no
| `HARDCOVER_SYNC_CONTENT_TYPE` | Request as | Audiobooks | `Audiobooks`, `Ebooks`, or `Ebooks and audiobooks` — the last creates one request of each per shelf book, each checked against its own library |
| `HARDCOVER_SYNC_INTERVAL` | Sync interval | 6 | Combined with the unit below |
| `HARDCOVER_SYNC_INTERVAL_UNIT` | Interval unit | Hours | Minutes or hours. The effective interval never drops below 60 seconds |
| `AUTO_DOWNLOAD_ENABLED` | Enable automatic downloads | Off | Auto-approve and queue strict matches for every pending Hardcover request, synced or made by a user with Request Book. The download belongs to the user who asked, and admins see every user's requests |
| `AUTO_DOWNLOAD_ENABLED` | Enable automatic downloads | Off | Auto-approve and queue strict matches for pending Hardcover requests: synced ones, and a user's own only on the sources where their request policy already lets them download directly; the rest wait for an admin. The download belongs to the user who asked, and admins see every user's requests |
| `AUTO_DOWNLOAD_SOURCE_PRIORITY` | Audiobook source priority | All usable sources | Drag to order; used for audiobook requests. Sources you disable here, or that are not configured, are skipped |
| `AUTO_DOWNLOAD_EBOOK_SOURCE_PRIORITY` | Ebook source priority | All usable sources | Same, for ebook requests |
| `AUTO_DOWNLOAD_MIN_SEEDERS` | Minimum seeders (torrents) | 1 | Torrent releases below this are ignored |
@@ -108,7 +110,7 @@ Only one run happens at a time. If a scheduled cycle is in progress, **Sync now*
**Sync adds nothing and the log says "no Hardcover token configured"** — set **Hardcover API Token** on this tab, or configure the token on the Hardcover metadata provider. A sync without a usable token counts as an error and creates no requests.
**Requests appear but nothing downloads** — check **Enable automatic downloads** is on, that the source-priority list for the request's content type contains at least one configured source, and look for `no strict match` lines in the log. Lowering **Minimum seeders** helps on quiet trackers; loosening the title or author match is deliberately not an option.
**Requests appear but nothing downloads** — check **Enable automatic downloads** is on, that the source-priority list for the request's content type contains at least one configured source, and look for `no strict match` lines in the log. A request from another user is only searched on the sources where their policy allows a direct download (or, with requests turned off for them, on every source); with no such source it stays pending for an admin and is not searched. Lowering **Minimum seeders** helps on quiet trackers; loosening the title or author match is deliberately not an option.
**Books you own keep being added** — enable the library check for the format you request (see [Library Check](library-check.md)) and use its test button. A library that cannot be read shows up in the log as a skipped cycle.
+7 -2
View File
@@ -141,7 +141,8 @@ def hardcover_sync_settings() -> list[SettingsField]:
title="Automatic Downloads",
description=(
"When enabled, pending Hardcover requests (synced from a shelf, or made by a "
"user with Request Book) are auto-approved and downloaded from the "
"user whose request policy lets them download directly) are auto-approved and "
"downloaded from the "
"first source in the matching list below that yields a strict title/author/"
"format match for the request's content type (ebook formats for ebook "
"requests, audiobook formats for audiobook requests). If no confident match "
@@ -151,7 +152,11 @@ def hardcover_sync_settings() -> list[SettingsField]:
CheckboxField(
key="AUTO_DOWNLOAD_ENABLED",
label="Enable automatic downloads",
description="Auto-approve and auto-download matching releases for Hardcover requests.",
description=(
"Auto-approve and auto-download matching releases for Hardcover requests. "
"Another user's request is only approved this way where their request "
"policy already lets them download directly; the rest wait for an admin."
),
default=False,
),
OrderableListField(
+129 -4
View File
@@ -31,8 +31,17 @@ from shelfmark.core.config import config as app_config
from shelfmark.core.logger import setup_logger
from shelfmark.core.models import QueueStatus
from shelfmark.core.release_search import search_source_releases
from shelfmark.core.request_helpers import coerce_int
from shelfmark.core.request_policy import normalize_content_type
from shelfmark.core.request_helpers import (
coerce_bool,
coerce_int,
load_users_request_policy_settings,
)
from shelfmark.core.request_policy import (
PolicyMode,
merge_request_policy_settings,
normalize_content_type,
resolve_policy_mode,
)
from shelfmark.core.request_validation import RequestStatus
from shelfmark.core.text_match import (
DEFAULT_TITLE_MATCH_THRESHOLD,
@@ -56,7 +65,7 @@ from shelfmark.download.postprocess.policy import (
)
if TYPE_CHECKING:
from collections.abc import Callable
from collections.abc import Callable, Mapping
from shelfmark.core.user_db import UserDB
from shelfmark.metadata_providers import BookMetadata
@@ -1169,6 +1178,7 @@ def reopen_stale_failures(
failure_cooldown: timedelta | None = None,
only_kinds: frozenset[str] | None = None,
reopened_ids: list[int] | None = None,
owner_policy: OwnerPolicy | None = None,
) -> int:
"""Return synced requests whose download never arrived to pending once the cooldown passes.
@@ -1188,6 +1198,9 @@ def reopen_stale_failures(
skipped next time, and is given up on after ``MAX_FAILURE_RETRIES``. Other cancelled requests
and rejected ones are decisions and are never touched. Returns how many were reopened; ``cooldown_days`` of 0 or less turns retries off,
and without readable history nothing is retried, since guessing would be worse.
A request auto-download could not fulfil on its owner's behalf (see ``OwnerPolicy``) is
not reopened either: nothing would pick it up again without an admin.
"""
if cooldown_days <= 0 and failure_cooldown is None:
return 0
@@ -1222,11 +1235,14 @@ def reopen_stale_failures(
return 0
stale.sort(key=lambda item: item[0])
policy = owner_policy or OwnerPolicy(user_db)
reopened = 0
for _changed, row, state in stale:
if reopened >= max_reopen:
break
request_id = int(row["id"])
if not policy.may_fulfil(row):
continue
kind = _failure_kind(history.get(request_id))
if kind is None or (state == _CANCELLED and kind != _STALLED):
continue
@@ -1315,6 +1331,95 @@ def _configured_source_priority(content_type: str) -> list[str]:
return [src["name"] for src in available if src.get("enabled")]
class OwnerPolicy:
"""What a request's owner could download without asking, read once per pass.
Auto-download fulfils a request as an admin, so it may only do what the owner could have
done alone: the same check the download route makes before it queues a release. An admin,
an install without login, or a user for whom requests are turned off downloads directly
from any source. Anyone else may only use the sources whose resolved policy mode is
``download``. A request with no such source is left exactly as it is, pending for an
admin, like any other request awaiting review.
"""
def __init__(self, user_db: UserDB, *, auth_mode: str | None = None) -> None:
"""Read nothing yet.
The policy settings and each owner are loaded on first use and kept for the pass.
"""
from shelfmark.core.auth_modes import AUTH_MODE_NONE, load_active_auth_mode
self._user_db = user_db
self._bypass = (auth_mode or load_active_auth_mode()) == AUTH_MODE_NONE
self._global: Mapping[str, Any] | None = None
self._owners: dict[int, tuple[bool, dict[str, Any]] | None] = {}
def _owner(self, user_id: int) -> tuple[bool, dict[str, Any]] | None:
"""(owner may skip the policy, their settings), or None when they cannot be read."""
if user_id not in self._owners:
try:
user = self._user_db.get_user(user_id=user_id)
settings = self._user_db.get_user_settings(user_id) if user else {}
except sqlite3.Error as exc:
logger.warning("auto-download: could not read user %s: %s", user_id, exc)
user, settings = None, {}
if user is None:
self._owners[user_id] = None
else:
if self._global is None:
self._global = load_users_request_policy_settings()
effective = merge_request_policy_settings(self._global, settings)
exempt = user.get("role") == "admin" or not coerce_bool(
effective.get("REQUESTS_ENABLED"), default=False
)
self._owners[user_id] = (exempt, settings)
return self._owners[user_id]
def download_sources(
self, row: dict[str, Any], content_type: str, sources: list[str]
) -> list[str]:
"""The sources in ``sources`` the owner of ``row`` may download from directly."""
if self._bypass:
return list(sources)
user_id = coerce_int(row.get("user_id"), 0)
owner = self._owner(user_id) if user_id > 0 else None
if owner is None:
return []
exempt, settings = owner
if exempt:
return list(sources)
return [
source
for source in sources
if resolve_policy_mode(
source=source,
content_type=content_type,
global_settings=self._global,
user_settings=settings,
)
== PolicyMode.DOWNLOAD
]
def may_fulfil(self, row: dict[str, Any]) -> bool:
"""True when auto-download could fulfil ``row`` from at least one configured source."""
if self._bypass:
return True
user_id = coerce_int(row.get("user_id"), 0)
owner = self._owner(user_id) if user_id > 0 else None
if owner is None:
return False
if owner[0]:
return True
book_data = row.get("book_data")
content_type = normalize_content_type(
row.get("content_type")
or (book_data.get("content_type") if isinstance(book_data, dict) else None)
)
return bool(
self.download_sources(row, content_type, _configured_source_priority(content_type))
)
def _acted_on_since_listed(user_db: UserDB, request_id: int) -> bool:
"""True only when the request is read back and is positively no longer waiting.
@@ -1636,6 +1741,7 @@ def retry_failed_downloads(
if minutes <= 0 or not bool(app_config.get("AUTO_DOWNLOAD_ENABLED", False)):
return None
reopened_ids: list[int] = []
owner_policy = OwnerPolicy(user_db)
reopen_stale_failures(
user_db,
cooldown_days=0,
@@ -1644,6 +1750,7 @@ def retry_failed_downloads(
failure_cooldown=timedelta(minutes=minutes),
only_kinds=frozenset({_FAILED, _STALLED}),
reopened_ids=reopened_ids,
owner_policy=owner_policy,
)
# A failed download the status poll already reopened is pending with no release; those
# are searched here as well, so they are not left for the next full pass.
@@ -1692,6 +1799,10 @@ def auto_download_pending(
priority list, format guard and library check all follow the request. Returns a summary
count dict. No-ops (returns zeros) unless AUTO_DOWNLOAD_ENABLED. ``admin_user_id``
defaults to the lowest-id admin, who fulfils the requests.
A request is only searched from the sources its owner could download from directly
(``OwnerPolicy``); one with none is left untouched for an admin to review and counted
under ``awaiting_approval``.
"""
zeros = {"queued": 0, "no_match": 0, "in_library": 0, "skipped": 0, "error": 0}
if not bool(app_config.get("AUTO_DOWNLOAD_ENABLED", False)):
@@ -1705,6 +1816,7 @@ def auto_download_pending(
logger.warning("auto-download: no admin user exists to fulfil requests")
return zeros
owner_policy = OwnerPolicy(user_db)
retry_days = coerce_int(
app_config.get(RETRY_DAYS_SETTING, DEFAULT_RETRY_DAYS), DEFAULT_RETRY_DAYS
)
@@ -1712,7 +1824,11 @@ def auto_download_pending(
0
if only_request_ids is not None
else reopen_stale_failures(
user_db, cooldown_days=retry_days, db_path=db_path, provider_filter=provider_filter
user_db,
cooldown_days=retry_days,
db_path=db_path,
provider_filter=provider_filter,
owner_policy=owner_policy,
)
)
if reopened:
@@ -1764,6 +1880,15 @@ def auto_download_pending(
if not sources:
summary["skipped"] += 1
continue
# Only what the owner could download without asking; the rest waits for an admin.
sources = owner_policy.download_sources(row, content_type, sources)
if not sources:
logger.debug(
"auto-download: request %s needs an admin's approval under its owner's policy",
row.get("id"),
)
summary["awaiting_approval"] = summary.get("awaiting_approval", 0) + 1
continue
if not _search_is_due(book_data, datetime.now(UTC), backoff_days):
logger.debug(
+163
View File
@@ -3620,3 +3620,166 @@ class TestHistoryOrder:
== 1
)
assert suite._state(user_db, rid) == "cancelled"
class TestOwnerPolicyGate:
"""Auto-download fulfils a request only where its owner could have downloaded it directly.
It runs as an admin, so without this a request made under an approval policy would be
approved by the pass instead of waiting for an admin.
"""
@staticmethod
def _setup(monkeypatch, *, policy, auth_mode="builtin", searched=None):
"""Turn auto-download on with two sources, the given policy and auth mode.
Every source has a match.
"""
monkeypatch.setattr(auto_download, "app_config", _Config(AUTO_DOWNLOAD_ENABLED=True))
monkeypatch.setattr(
auto_download, "_configured_source_priority", lambda _ct: ["first", "second"]
)
monkeypatch.setattr("shelfmark.core.auth_modes.load_active_auth_mode", lambda: auth_mode)
monkeypatch.setattr(
auto_download, "load_users_request_policy_settings", lambda: dict(policy)
)
monkeypatch.setattr(
"shelfmark.core.library_index.is_in_library", lambda *_args, **_kwargs: False
)
_stub_provider(monkeypatch, _book())
def _search(source_name, *_args, **_kwargs):
"""Return one matching release from any source asked, recording its name."""
if searched is not None:
searched.append(source_name)
return None, [_release(source=source_name)], None
monkeypatch.setattr(auto_download, "search_source_releases", _search)
@staticmethod
def _queue(release_data, priority, *, user_id, username):
return True, None
def _run(self, user_db, admin):
return auto_download.auto_download_pending(
user_db, queue_release=self._queue, admin_user_id=admin["id"]
)
APPROVAL = {"REQUESTS_ENABLED": True, "REQUEST_POLICY_DEFAULT_AUDIOBOOK": "request_book"}
DIRECT = {"REQUESTS_ENABLED": True, "REQUEST_POLICY_DEFAULT_AUDIOBOOK": "download"}
def test_a_request_awaiting_approval_is_left_exactly_as_it_was(self, user_db, monkeypatch):
searched: list[str] = []
self._setup(monkeypatch, policy=self.APPROVAL, searched=searched)
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
row = _pending_request(user_db, reader["id"])
summary = self._run(user_db, admin)
assert summary["queued"] == 0
assert summary["awaiting_approval"] == 1
assert searched == []
# No backoff state, note or review: the same as a manual request awaiting review.
assert user_db.get_request(row["id"]) == row
def test_a_request_the_owner_could_download_directly_is_fulfilled(self, user_db, monkeypatch):
self._setup(monkeypatch, policy=self.DIRECT)
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
row = _pending_request(user_db, reader["id"])
summary = self._run(user_db, admin)
assert summary["queued"] == 1
assert user_db.get_request(row["id"])["status"] == "fulfilled"
def test_an_admins_synced_request_is_fulfilled_whatever_the_policy(self, user_db, monkeypatch):
self._setup(monkeypatch, policy=self.APPROVAL)
admin = user_db.create_user(username="admin", role="admin")
row = _pending_request(user_db, admin["id"])
summary = self._run(user_db, admin)
assert summary["queued"] == 1
assert user_db.get_request(row["id"])["status"] == "fulfilled"
def test_a_blocked_content_type_is_left_untouched(self, user_db, monkeypatch):
self._setup(
monkeypatch,
policy={"REQUESTS_ENABLED": True, "REQUEST_POLICY_DEFAULT_AUDIOBOOK": "blocked"},
)
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
row = _pending_request(user_db, reader["id"])
summary = self._run(user_db, admin)
assert summary["awaiting_approval"] == 1
assert user_db.get_request(row["id"]) == row
def test_only_the_sources_the_owner_may_download_from_are_searched(self, user_db, monkeypatch):
searched: list[str] = []
policy = {
**self.DIRECT,
"REQUEST_POLICY_RULES": [
{"source": "first", "content_type": "audiobook", "mode": "request_release"}
],
}
self._setup(monkeypatch, policy=policy, searched=searched)
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
_pending_request(user_db, reader["id"])
summary = self._run(user_db, admin)
assert summary["queued"] == 1
assert searched == ["second"]
def test_a_per_user_override_counts(self, user_db, monkeypatch):
self._setup(monkeypatch, policy=self.APPROVAL)
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
user_db.set_user_settings(reader["id"], {"REQUEST_POLICY_DEFAULT_AUDIOBOOK": "download"})
_pending_request(user_db, reader["id"])
assert self._run(user_db, admin)["queued"] == 1
def test_with_requests_off_the_owner_downloads_directly_so_the_pass_fulfils(
self, user_db, monkeypatch
):
self._setup(monkeypatch, policy={**self.APPROVAL, "REQUESTS_ENABLED": False})
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
_pending_request(user_db, reader["id"])
assert self._run(user_db, admin)["queued"] == 1
def test_without_login_there_is_no_policy_to_apply(self, user_db, monkeypatch):
self._setup(monkeypatch, policy=self.APPROVAL, auth_mode="none")
admin = user_db.create_user(username="admin", role="admin")
reader = user_db.create_user(username="reader", role="user")
_pending_request(user_db, reader["id"])
assert self._run(user_db, admin)["queued"] == 1
@pytest.mark.parametrize(("policy", "reopened"), [(APPROVAL, 0), (DIRECT, 1)])
def test_a_failed_download_is_only_reopened_when_the_pass_could_fulfil_it(
self, user_db, monkeypatch, policy, reopened
):
"""A failed download is reopened only when its owner's policy lets the pass fulfil it."""
self._setup(monkeypatch, policy=policy)
reader = user_db.create_user(username="reader", role="user")
row = TestRetryAfterCooldown._request(user_db, reader["id"], provider_id="1")
assert (
auto_download.reopen_stale_failures(
user_db,
cooldown_days=7,
db_path=user_db._db_path,
now=TestRetryAfterCooldown.NOW,
)
== reopened
)
expected = "pending" if reopened else "fulfilled"
assert user_db.get_request(row["id"])["status"] == expected