mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-15 12:28:34 +00:00
fix(download): trust a pinned AA mirror and repair the full e2e profile (#1518)
The destination guard from the oceanofpdf/SSRF fix (f837506) trusts only
hosts in
the mirror lists and refuses any other host that resolves to a private
address.
The full e2e profile has failed on every PR since then: its AA mirror
was mock-aa,
but the slow-download links pointed at cf.mock.test (172.30.0.11). The
guard refused
the slow-download page before the internal Chrome bypasser ever started,
so the
download ended with "All download sources failed". Adding cf.mock.test
as a second
AA mirror would not help: AAMirrorSelector.rewrite would then move the
gated link
onto mock-aa, and the test would pass without Chrome solving anything.
The profile
now uses the gate as its only AA mirror. The cloudflare mock takes
CF_GATED_PATHS,
and the full profile challenges only /slow_download/, so search, detail
and the
final file pass through while the download still needs Chrome. The
unused
SLOW_DOWNLOAD_BASE indirection is gone.
The same allowlist missed a supported setup: Primary Mirror
(AA_BASE_URL) may pin a
URL that is not in the mirror list, and network uses it as the active AA
base. With
it on a LAN address, every AA request was refused. The guard now trusts
the pinned
URL too (mirrors.get_pinned_aa_base_url), and
has_aa_mirror_configuration reuses
the same helper.
download_url also ran the guard a second time outside its try block,
which repeated
the DNS lookup and could raise instead of returning None. The duplicate
call is gone.
This commit is contained in:
1 parent
2fa0ad27f3
commit
1aa9846f22
12 files changed
+92
-68
No files matched your search
@@ -107,17 +107,24 @@ def get_aa_mirrors() -> list[str]:
|
||||
return _normalize_configured_urls(config.get("AA_ADDITIONAL_URLS", ""))
|
||||
|
||||
|
||||
def has_aa_mirror_configuration() -> bool:
|
||||
"""Return True when direct-download search has at least one AA base URL to use."""
|
||||
if get_aa_mirrors():
|
||||
return True
|
||||
def get_pinned_aa_base_url() -> str | None:
|
||||
"""Return the AA URL the Primary Mirror setting pins, or None when it is Auto.
|
||||
|
||||
The pinned URL need not be in the mirror list.
|
||||
"""
|
||||
configured_base_url = normalize_http_url(
|
||||
_string_config_value(_get_config().get("AA_BASE_URL", "auto")),
|
||||
default_scheme="https",
|
||||
allow_special=("auto",),
|
||||
)
|
||||
return bool(configured_base_url and configured_base_url != "auto")
|
||||
if configured_base_url and configured_base_url != "auto":
|
||||
return configured_base_url
|
||||
return None
|
||||
|
||||
|
||||
def has_aa_mirror_configuration() -> bool:
|
||||
"""Return True when direct-download search has at least one AA base URL to use."""
|
||||
return bool(get_aa_mirrors()) or get_pinned_aa_base_url() is not None
|
||||
|
||||
|
||||
def get_libgen_mirrors() -> list[str]:
|
||||
|
||||
@@ -890,7 +890,6 @@ def download_url(
|
||||
except network.UnsafeDestinationError as e:
|
||||
logger.warning("Blocked download: %s", e)
|
||||
return None
|
||||
network.ensure_public_destination(current_url)
|
||||
|
||||
# Build headers with optional referer
|
||||
headers = DOWNLOAD_HEADERS.copy()
|
||||
|
||||
@@ -592,8 +592,10 @@ def _configured_mirror_hosts() -> set[str]:
|
||||
"""Hostnames of every mirror the operator configured, which are trusted as given."""
|
||||
from shelfmark.core import mirrors
|
||||
|
||||
pinned_aa_url = mirrors.get_pinned_aa_base_url()
|
||||
urls = [
|
||||
*mirrors.get_aa_mirrors(),
|
||||
*([pinned_aa_url] if pinned_aa_url else []),
|
||||
*mirrors.get_libgen_mirrors(),
|
||||
*mirrors.get_zlib_mirrors(),
|
||||
*mirrors.get_welib_mirrors(),
|
||||
|
||||
@@ -131,10 +131,7 @@ export const AdvancedFilters = ({
|
||||
const wrapperClassName = formClassName ? 'px-2' : 'px-2 lg:ml-16 lg:w-[calc(50vw+4rem)]';
|
||||
|
||||
const settingsForm = (
|
||||
<div
|
||||
className={wrapperClassName}
|
||||
onMouseDown={(e) => e.stopPropagation()}
|
||||
>
|
||||
<div className={wrapperClassName} role="presentation" onMouseDown={(e) => e.stopPropagation()}>
|
||||
{onClose && (
|
||||
<div className="mb-1 flex justify-end">
|
||||
<button
|
||||
|
||||
@@ -86,6 +86,27 @@ def test_configured_mirror_on_a_private_address_is_allowed(monkeypatch):
|
||||
network.ensure_public_destination("http://lan-mirror.home/book")
|
||||
|
||||
|
||||
def test_pinned_aa_base_url_on_a_private_address_is_allowed(monkeypatch):
|
||||
# The Primary Mirror setting can pin a URL that is not in the mirror list, and
|
||||
# network uses it as the active AA base, so the guard must trust it too.
|
||||
monkeypatch.setattr(
|
||||
mirrors, "_get_config", lambda: {"AA_BASE_URL": "http://aa.lan:8080", "AA_MIRROR_URLS": []}
|
||||
)
|
||||
_resolve_to(monkeypatch, {"aa.lan": "192.168.1.30", "other.lan": "192.168.1.31"})
|
||||
|
||||
network.ensure_public_destination("http://aa.lan:8080/search?q=moby")
|
||||
with pytest.raises(network.UnsafeDestinationError):
|
||||
network.ensure_public_destination("http://other.lan/")
|
||||
|
||||
|
||||
def test_auto_aa_base_url_is_not_a_trusted_host(monkeypatch):
|
||||
monkeypatch.setattr(mirrors, "_get_config", lambda: {"AA_BASE_URL": "auto"})
|
||||
_resolve_to(monkeypatch, {"auto": "192.168.1.30"})
|
||||
|
||||
with pytest.raises(network.UnsafeDestinationError):
|
||||
network.ensure_public_destination("http://auto/")
|
||||
|
||||
|
||||
def test_redirect_hook_checks_each_hop(monkeypatch):
|
||||
_resolve_to(monkeypatch, {"ok.example": "93.184.216.34", "internal.example": "10.0.0.9"})
|
||||
hook = network.REDIRECT_GUARD_HOOKS["response"][0]
|
||||
|
||||
@@ -95,8 +95,8 @@ end-to-end (`docker compose up` + suite + teardown) and passes.
|
||||
> gate. The bypasser profiles therefore assert that a gated search *recovers* when the
|
||||
> external bypasser is available and still fails cleanly when it is off, while the
|
||||
> **`full` profile exercises the real end-to-end
|
||||
> CF solve**: AA search/detail are reachable, but the AA slow-download link points
|
||||
> at the gate, so downloading Moby-Dick forces the in-image headless Chromium to
|
||||
> CF solve**: the gate is the AA mirror but challenges only the slow-download page,
|
||||
> so downloading Moby-Dick forces the in-image headless Chromium to
|
||||
> detect the challenge, solve it (`_bypass_method_cdp_gui_click`), and fetch the file —
|
||||
> verified live (`Challenge detected: cloudflare` → `Bypass successful` → Moby-Dick
|
||||
> in `/books`).
|
||||
@@ -146,10 +146,13 @@ The "everything real" heavy profile (test book: **Moby-Dick**), run nightly / on
|
||||
demand (excluded from the PR matrix). It spins up, with **no** mock bypasser, and
|
||||
**passes live** (6 passed):
|
||||
|
||||
- **Real Chrome solves Cloudflare, end to end (VERIFIED).** AA search/detail are
|
||||
reachable (`mock-aa`), but the AA *slow-download* link points at the Cloudflare
|
||||
gate (`mock-cf`), whose challenge page runs JS that issues `cf_clearance` and
|
||||
reloads. Downloading Moby-Dick forces the in-image headless Chromium (seleniumbase
|
||||
- **Real Chrome solves Cloudflare, end to end (VERIFIED).** The AA mirror is the
|
||||
Cloudflare gate (`mock-cf`, `CF_GATED_PATHS=/slow_download/`): it passes
|
||||
search/detail and the final file through to `mock-aa` but challenges the AA
|
||||
*slow-download* page, whose challenge runs JS that issues `cf_clearance` and
|
||||
reloads. The gate has to be the only AA mirror: shelfmark refuses unconfigured
|
||||
hosts that resolve to private addresses, and with a second mirror the mirror
|
||||
selector would rewrite the gated link onto the ungated host. Downloading Moby-Dick forces the in-image headless Chromium (seleniumbase
|
||||
CDP, in the `shelfmark` image via `xvfb`+`chromium`) to load the gate, detect the
|
||||
challenge (`Challenge detected: cloudflare`), solve it (`_bypass_method_cdp_gui_click`),
|
||||
and fetch the cleared "Download now" page → the file lands in `/books`. That
|
||||
|
||||
@@ -112,11 +112,10 @@ services:
|
||||
container_name: e2e-mock-aa
|
||||
environment:
|
||||
MOCK_ROLE: origin-aa
|
||||
# When set (the `full` profile sets it to the CF gate), AA slow-download
|
||||
# links point through Cloudflare so a real download forces the internal
|
||||
# Chrome bypasser to solve the challenge. Empty -> same-origin (no CF).
|
||||
SLOW_DOWNLOAD_BASE: "${SM_SLOW_DOWNLOAD_BASE:-}"
|
||||
AA_FILE_BASE: "http://mock-aa"
|
||||
# Host of the final file link on the slow-download page. The `full` profile
|
||||
# points it at the CF gate, its only AA mirror: shelfmark refuses
|
||||
# unconfigured hosts on private addresses.
|
||||
AA_FILE_BASE: "${SM_AA_FILE_BASE:-http://mock-aa}"
|
||||
networks:
|
||||
e2e:
|
||||
ipv4_address: 172.30.0.10
|
||||
@@ -129,9 +128,9 @@ services:
|
||||
retries: 10
|
||||
|
||||
# ----- Cloudflare gate (profiles: bypasser-external, full) -------------- #
|
||||
# `full` puts AA's slow-download behind this gate so the real Chrome bypasser
|
||||
# must solve it; `bypasser-external` puts AA search behind it for the
|
||||
# FlareSolverr negative/positive controls.
|
||||
# `full` uses it as the AA mirror but gates only the slow-download so the real
|
||||
# Chrome bypasser must solve it; `bypasser-external` puts AA search behind it
|
||||
# for the FlareSolverr negative/positive controls.
|
||||
mock-cf:
|
||||
<<: *mock-build
|
||||
container_name: e2e-mock-cf
|
||||
@@ -139,6 +138,8 @@ services:
|
||||
environment:
|
||||
MOCK_ROLE: cloudflare
|
||||
ORIGIN_INTERNAL_URL: http://mock-aa
|
||||
# Path prefixes to challenge; empty gates every path.
|
||||
CF_GATED_PATHS: "${SM_CF_GATED_PATHS:-}"
|
||||
networks:
|
||||
e2e:
|
||||
ipv4_address: 172.30.0.11
|
||||
|
||||
Vendored
+14
-10
@@ -3,12 +3,12 @@
|
||||
# Test book: Moby-Dick (Herman Melville, public domain).
|
||||
#
|
||||
# What it exercises end to end, with NO mock bypasser:
|
||||
# 1. REAL Chrome internal bypasser: AA search/detail are reachable (mock-aa), but
|
||||
# the AA *slow-download* links point through the Cloudflare gate. So a real
|
||||
# DOWNLOAD forces the in-image headless Chrome (seleniumbase CDP) to solve the
|
||||
# JS challenge and harvest cf_clearance — the literal "spin a chrome browser"
|
||||
# path. Success = Moby-Dick lands in /books, which is only possible if Chrome
|
||||
# solved the gate.
|
||||
# 1. REAL Chrome internal bypasser: the AA mirror is the Cloudflare gate, which
|
||||
# passes search/detail through but challenges the *slow-download* page. So a
|
||||
# real DOWNLOAD forces the in-image headless Chrome (seleniumbase CDP) to solve
|
||||
# the JS challenge and harvest cf_clearance — the literal "spin a chrome
|
||||
# browser" path. Success = Moby-Dick lands in /books, which is only possible
|
||||
# if Chrome solved the gate.
|
||||
# 2. DoH enabled (USE_DOH) so the DNS-over-HTTPS code path runs at boot.
|
||||
# 3. REAL download client: a mock Prowlarr returns a webseed .torrent; a real
|
||||
# qBittorrent downloads the payload over HTTP from mock-aa and shelfmark
|
||||
@@ -17,10 +17,14 @@
|
||||
# Heavy (Chrome + qBittorrent), so it is excluded from the PR matrix.
|
||||
COMPOSE_PROFILES=full
|
||||
|
||||
# AA search/detail reachable directly; the DOWNLOAD goes through the CF gate so the
|
||||
# internal Chrome bypasser must solve it (search uses no bypasser, by design).
|
||||
SM_AA_URL=http://mock-aa
|
||||
SM_SLOW_DOWNLOAD_BASE=http://cf.mock.test
|
||||
# The CF gate is the only AA mirror: shelfmark refuses unconfigured hosts that
|
||||
# resolve to private addresses, and a second AA mirror would let the mirror selector
|
||||
# rewrite the gated link onto the ungated one. Search/detail and the final file pass
|
||||
# straight through; the slow-download page is challenged, so the internal Chrome
|
||||
# bypasser must solve it (search uses no bypasser, by design).
|
||||
SM_AA_URL=http://cf.mock.test
|
||||
SM_CF_GATED_PATHS=/slow_download/
|
||||
SM_AA_FILE_BASE=http://cf.mock.test
|
||||
SM_USE_CF_BYPASS=true
|
||||
SM_USING_EXTERNAL_BYPASSER=false
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
<div>
|
||||
<div><img src="/img/cover-__MD5__.jpg"></div>
|
||||
<div class="main-inner">
|
||||
<div><div class="md-meta"><div><div><span>Language</span><span>English [en]</span></div><div><span>Year</span><span>1851</span></div></div></div><div>🔍 Moby Dick</div><div>epub · 1.2 MB · fiction</div><div><a href="__SLOW_BASE__/slow_download/__MD5__/0/0">Slow Partner Server #1</a> — no waitlist, but the download is slow</div><div><a href="__SLOW_BASE__/slow_download/__MD5__/0/1">Slow Partner Server #2</a> — waitlist, but faster</div><div><a href="/get.php?md5=__MD5__&key=e2ekey">GET</a></div></div>
|
||||
<div><div class="md-meta"><div><div><span>Language</span><span>English [en]</span></div><div><span>Year</span><span>1851</span></div></div></div><div>🔍 Moby Dick</div><div>epub · 1.2 MB · fiction</div><div><a href="/slow_download/__MD5__/0/0">Slow Partner Server #1</a> — no waitlist, but the download is slow</div><div><a href="/slow_download/__MD5__/0/1">Slow Partner Server #2</a> — waitlist, but faster</div><div><a href="/get.php?md5=__MD5__&key=e2ekey">GET</a></div></div>
|
||||
</div>
|
||||
<div class="js-md5-top-box-description">Moby-Dick; or, The Whale by Herman Melville.</div>
|
||||
</div>
|
||||
|
||||
@@ -15,7 +15,8 @@ Roles
|
||||
``cloudflare`` Cloudflare-protected origin: returns a 403 "Just a moment..."
|
||||
challenge page (with ``cf-mitigated: challenge``) until the
|
||||
request carries a ``cf_clearance`` cookie, then serves the real
|
||||
content. Exercises the app's CF detection + bypasser routing
|
||||
content. ``CF_GATED_PATHS`` limits the challenge to some paths.
|
||||
Exercises the app's CF detection + bypasser routing
|
||||
(#284, #226, #202, #1030) without running real CF JS.
|
||||
``flaresolverr`` Mock FlareSolverr implementing the ``/v1`` contract. It fetches
|
||||
the requested URL *with* a clearance cookie and returns the
|
||||
@@ -64,11 +65,12 @@ CLEARANCE_VALUE = "e2e-cleared-token"
|
||||
PAYLOAD_NAME = "moby-dick.epub"
|
||||
PAYLOAD_URL = f"{os.environ.get('PAYLOAD_PUBLIC_URL', 'http://mock-aa')}/payload/{PAYLOAD_NAME}"
|
||||
|
||||
# Base for AA slow-download links on the detail page. When set to the Cloudflare
|
||||
# gate (the `full` profile sets it to http://cf.mock.test), the *download* — not the
|
||||
# search — is forced through the gate, so a real download triggers the internal
|
||||
# Chrome bypasser to solve the challenge. Empty -> same-origin (no CF).
|
||||
SLOW_DOWNLOAD_BASE = os.environ.get("SLOW_DOWNLOAD_BASE", "").rstrip("/")
|
||||
# Comma-separated path prefixes the cloudflare role challenges. Empty -> every path.
|
||||
# The `full` profile gates only /slow_download/, so search/detail pass straight through
|
||||
# and the *download* is what makes the internal Chrome bypasser solve the challenge.
|
||||
CF_GATED_PATHS = tuple(
|
||||
prefix.strip() for prefix in os.environ.get("CF_GATED_PATHS", "").split(",") if prefix.strip()
|
||||
)
|
||||
|
||||
# Real public-domain opening of Moby-Dick (Chapter 1, "Loomings").
|
||||
_MOBY_DICK_TEXT = (
|
||||
@@ -183,14 +185,7 @@ def register_origin_aa(flask_app: Flask) -> None:
|
||||
|
||||
@flask_app.route("/md5/<book_id>")
|
||||
def aa_detail(book_id: str) -> Response:
|
||||
# __SLOW_BASE__ controls where the AA "slow partner server" links point.
|
||||
# In the `full` profile it's the Cloudflare gate, so the *download* (not the
|
||||
# search/detail) is what forces the internal Chrome bypasser to solve CF.
|
||||
body = (
|
||||
_read_fixture("aa_detail.html")
|
||||
.replace("__MD5__", book_id)
|
||||
.replace("__SLOW_BASE__", SLOW_DOWNLOAD_BASE)
|
||||
)
|
||||
body = _read_fixture("aa_detail.html").replace("__MD5__", book_id)
|
||||
return make_response(body, 200)
|
||||
|
||||
@flask_app.route("/get.php")
|
||||
@@ -202,8 +197,8 @@ def register_origin_aa(flask_app: Flask) -> None:
|
||||
def aa_slow(rest: str) -> Response:
|
||||
# The AA "slow partner server" page: an HTML page whose "Download now" link
|
||||
# is the final file URL. shelfmark's _extract_slow_download_url parses this.
|
||||
# Reached (in the full profile) only after the internal Chrome bypasser
|
||||
# solves the Cloudflare gate in front of it.
|
||||
# In the full profile this sits behind the Cloudflare gate, so it is reached
|
||||
# only after the internal Chrome bypasser solves the challenge.
|
||||
del rest
|
||||
file_url = f"{os.environ.get('AA_FILE_BASE', 'http://mock-aa')}/file/{PAYLOAD_NAME}"
|
||||
# The visible text must exceed the internal bypasser's "still loading"
|
||||
@@ -307,8 +302,9 @@ def register_cloudflare(flask_app: Flask) -> None:
|
||||
@flask_app.route("/", defaults={"path": ""})
|
||||
@flask_app.route("/<path:path>")
|
||||
def cf_gate(path: str) -> Response:
|
||||
if request.cookies.get(CLEARANCE_COOKIE) == CLEARANCE_VALUE:
|
||||
# Cleared: proxy the request through to the real AA origin behaviour.
|
||||
gated = not CF_GATED_PATHS or request.path.startswith(CF_GATED_PATHS)
|
||||
if not gated or request.cookies.get(CLEARANCE_COOKIE) == CLEARANCE_VALUE:
|
||||
# Ungated or cleared: proxy the request through to the real AA origin.
|
||||
return _cleared_passthrough(path)
|
||||
challenge = _read_fixture("cf_challenge.html")
|
||||
resp = make_response(challenge, 403)
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
|
||||
Test book: *Moby-Dick* by Herman Melville (public domain). Validated live.
|
||||
|
||||
* **Real Chrome solves Cloudflare, end to end (VERIFIED).** AA search/detail are
|
||||
reachable (mock-aa), but the AA *slow-download* links point through the
|
||||
Cloudflare gate (mock-cf). Downloading therefore forces the in-image headless
|
||||
* **Real Chrome solves Cloudflare, end to end (VERIFIED).** The AA mirror is the
|
||||
Cloudflare gate (mock-cf): it passes search/detail through but challenges the
|
||||
AA *slow-download* page. Downloading therefore forces the in-image headless
|
||||
Chromium (seleniumbase CDP internal bypasser) to execute the challenge JS,
|
||||
harvest ``cf_clearance``, and fetch the gated slow-download page. Moby-Dick
|
||||
landing in ``/books`` is only possible if Chrome actually solved the gate —
|
||||
|
||||
@@ -6,11 +6,8 @@ particularly that clicking on the AdvancedFilters panel does not close
|
||||
the main search menu (issue #1301).
|
||||
"""
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# Path to the frontend source code
|
||||
FRONTEND_SRC_DIR = Path(__file__).parent.parent.parent / "src" / "frontend" / "src"
|
||||
|
||||
@@ -39,9 +36,7 @@ class TestSearchMenuDismiss:
|
||||
# Verify that the onMouseDown stopPropagation handler is present
|
||||
# This prevents the mousedown event from bubbling up to the document
|
||||
# and triggering the useDismiss hook in SearchBar
|
||||
assert (
|
||||
"onMouseDown={(e) => e.stopPropagation()}" in source_code
|
||||
), (
|
||||
assert "onMouseDown={(e) => e.stopPropagation()}" in source_code, (
|
||||
"AdvancedFilters must prevent mousedown events from bubbling up "
|
||||
"to the document. Without this, clicking on dropdown options inside "
|
||||
"AdvancedFilters will close the main search menu (issue #1301)."
|
||||
@@ -85,10 +80,9 @@ class TestSearchMenuDismiss:
|
||||
"SearchBar should use useDismiss hook for menu dismissal"
|
||||
)
|
||||
|
||||
# Verify that useDismiss is called with the appropriate refs
|
||||
assert "selectorRef" in source_code, (
|
||||
"SearchBar should track selectorRef for dismissal"
|
||||
)
|
||||
assert "controlsPanelRef" in source_code, (
|
||||
"SearchBar should track controlsPanelRef for dismissal"
|
||||
# The options menu stays open for clicks inside selectorRef or optionsPanelRef.
|
||||
# AdvancedFilters renders beside SearchBar, outside both, which is why it stops
|
||||
# mousedown from reaching this listener.
|
||||
assert "useDismiss(isOptionsOpen, [selectorRef, optionsPanelRef]" in source_code, (
|
||||
"SearchBar should dismiss the options menu only outside selectorRef and optionsPanelRef"
|
||||
)
|
||||
Reference in new issue
Block a user