fix(bypass): discard rejected DDoS-Guard cookies instead of replaying them (#1221)

A cookie that has been rejected was kept and presented again on every
later
request, so a single bad clearance could re-arm the challenge
indefinitely.

Cookie storage:
- Enforce expiry for every stored cookie, not just cf_clearance.
DDoS-Guard
domains have no cf_clearance, so the existing check never fired for them
and
  expired cookies were replayed forever.
- Stop storing the per-check cookies __ddg8_/__ddg9_/__ddg10_ and
ddg_last_challenge. Captured live from Anna's Archive, these carry the
client
IP and the timestamp the check was issued (~40 min), versus ~1 year for
the
  __ddg1_/__ddg2_/__ddgid_ clearance. Replaying an IP-bound token stops
describing the caller as soon as the egress IP changes, which is routine
  behind a VPN.

Failure handling — every path that is rejected while carrying cookies
now
purges them, not just the redirect loop:
- 403 returned while presenting cookies.
- Cached-cookie attempt rejected, whether by status or by redirect loop.
- Factored the purge into _purge_clearance, guarded on a non-empty
hostname
since clear_cf_cookies("") means "every host" and would wipe clearance
for
  sites that are working fine.

Also fix the search warm-up switches shipped inert in v1.3.8:
SEARCH_WARMUP_ENABLED and SEARCH_WARMUP_QUERY are not in the settings
registry, and config.get only consults the environment for keys it
knows, so
both always returned their defaults — the warm-up could not be turned
off or
retargeted. Read os.environ first.

Refs #1220. Deliberately not "Fixes": the reported failure could not be
reproduced on v1.3.8 from a stable IP (the reporter's own queries all
returned
200 on both the pre- and post-change builds), and the new purge paths
did not
fire in live testing because the failures arrive as redirect loops,
which were
already purged. These are correctness fixes with no measured effect on
that
issue. The underlying problem remains that Chrome-obtained cookies never
satisfy DDoS-Guard when replayed by requests, so every search still
re-solves.

Verified: 2542 unit tests pass; ruff, basedpyright and vulture clean;
e2e
platform baseline (10), full (6) and bypasser-external (5) all pass;
five
sequential live searches against Anna's Archive all returned 200 with
zero
"Exceeded 30 redirects".

shelfmark/bypass/internal_bypasser.py

@@ -253,6 +253,26 @@ DDG_COOKIE_NAMES = {
     "ddg_last_challenge",
 }
 
+# DDoS-Guard cookies that describe *one* check rather than granting clearance, and so
+# must never be replayed on a later request. Observed live on Anna's Archive:
+#
+#   __ddg9_   the client IP address
+#   __ddg10_  the unix timestamp the check was issued
+#   __ddg8_   an opaque token issued with them, same ~40 minute expiry
+#
+# Clearance itself lives in __ddg1_/__ddg2_/__ddgid_ (roughly a year) and __ddg5_.
+# Replaying the trio is actively harmful: once the timestamp ages out - or the egress
+# IP changes, which happens routinely behind a VPN - the values no longer describe the
+# caller, DDoS-Guard re-arms its check and answers every request with a ?check=1
+# redirect. That is the redirect loop, and it is self-inflicted. Dropping them simply
+# lets DDoS-Guard issue a fresh set, exactly as it does for a browser.
+DDG_EPHEMERAL_COOKIE_NAMES = {
+    "__ddg8_",
+    "__ddg9_",
+    "__ddg10_",
+    "ddg_last_challenge",
+}
+
 
 def _get_base_domain(domain: str) -> str:
     """Extract base domain from hostname (e.g., 'www.example.com' -> 'example.com')."""
@@ -268,6 +288,10 @@ def _get_full_cookie_domains() -> set[str]:
 
 def _should_extract_cookie(name: str, *, extract_all: bool) -> bool:
     """Determine if a cookie should be extracted based on its name."""
+    # Checked before extract_all: a per-check token is wrong to replay for every
+    # domain, including the full-session ones.
+    if name in DDG_EPHEMERAL_COOKIE_NAMES:
+        return False
     if extract_all:
         return True
     is_cf = name in CF_COOKIE_NAMES or name.startswith("cf_")
@@ -342,6 +366,16 @@ async def _extract_cookies_from_cdp(driver: Any, page: Any, url: str) -> None:
         logger.debug("Failed to extract cookies: %s", e)
 
 
+def _is_cookie_expired(cookie: dict[str, Any]) -> bool:
+    """Whether a stored cookie's expiry has passed. Session cookies never expire here."""
+    expiry = cookie.get("expiry")
+    if expiry is None:
+        expiry = cookie.get("expires")
+    if not expiry or expiry <= 0:
+        return False
+    return time.time() > expiry
+
+
 def get_cf_cookies_for_domain(domain: str) -> dict[str, str]:
     """Get stored cookies for a domain. Returns empty dict if none available."""
     if not domain:
@@ -355,16 +389,25 @@ def get_cf_cookies_for_domain(domain: str) -> dict[str, str]:
             return {}
 
         cf_clearance = cookies.get("cf_clearance", {})
-        if cf_clearance:
+        if cf_clearance and _is_cookie_expired(cf_clearance):
-            expiry = cf_clearance.get("expiry")
-            if expiry is None:
-                expiry = cf_clearance.get("expires")
-            if expiry and expiry > 0 and time.time() > expiry:
             logger.debug("CF cookies expired for %s", base_domain)
             _cf_cookies.pop(base_domain, None)
             return {}
 
-        return {name: c["value"] for name, c in cookies.items()}
+        # Expiry applies to every cookie, not just Cloudflare's. DDoS-Guard domains
+        # have no cf_clearance, so the check above never fired for them and dead
+        # cookies were replayed indefinitely - the server answers those with a
+        # challenge, which is indistinguishable from having sent nothing at all.
+        live = {name: c for name, c in cookies.items() if not _is_cookie_expired(c)}
+        if len(live) != len(cookies):
+            expired = sorted(set(cookies) - set(live))
+            logger.debug("Dropping expired cookies for %s: %s", base_domain, expired)
+            if live:
+                _cf_cookies[base_domain] = live
+            else:
+                _cf_cookies.pop(base_domain, None)
+
+        return {name: c["value"] for name, c in live.items()}
 
 
 def has_valid_cf_cookies(domain: str) -> bool:
@@ -1263,9 +1306,23 @@ def _try_with_cached_cookies(url: str, hostname: str) -> str | None:
         if response.status_code == HTTPStatus.OK:
             logger.debug("Cached cookies worked, skipped Chrome bypass")
             return response.text
+        logger.debug(
+            "Cached cookies rejected (%s) for %s; discarding them",
+            response.status_code,
+            url,
+        )
     except _REQUEST_OPERATION_ERRORS as exc:
+        # A redirect loop lands here too: DDoS-Guard answers a dead clearance cookie
+        # with an endless ?check=1 bounce rather than a status we can read.
         logger.debug("Cached cookie retry failed for %s: %s", url, exc)
 
+    # Reached only when the cached cookies did not produce a page, so they are no
+    # longer clearance. Dropping them now means the imminent Chrome solve starts from
+    # a clean slate and later requests cannot re-present the same rejected cookie.
+    # Guarded because clear_cf_cookies("") means "every host", which would wipe
+    # clearance for sites that are working fine.
+    if hostname:
+        clear_cf_cookies(hostname)
     return None
 
 

shelfmark/download/http.py

@@ -369,20 +369,29 @@ def html_get_page(
         """
         return allow_bypasser_fallback and _is_cf_bypass_enabled() and not use_bypasser_now
 
+    def _purge_clearance(target_url: str) -> None:
+        """Drop the host's stored clearance cookies.
+
+        Called whenever the protection answered a request that *carried* cookies:
+        being challenged while presenting them proves they no longer work, so keeping
+        them only guarantees the same rejection on every later request. Purging is
+        internal-bypasser only; with an external one get_cf_cookies_for_domain()
+        already returns {}.
+        """
+        hostname = urlparse(target_url).hostname or ""
+        # An empty domain means "clear every host" to the bypasser, so skip the purge
+        # rather than wipe clearance for sites that are working fine.
+        if hostname and not _is_using_external_bypasser():
+            _get_internal_bypasser().clear_cf_cookies(hostname)
+
     def _redirect_loop_handoff(bypass_url: str) -> str | tuple[str, str]:
         """Drop the host's stale clearance cookies, then bypass `bypass_url`.
 
         A `?check=1` loop is how DDoS-Guard answers a clearance cookie that has gone
         stale, so the dead cookie has to go before the solve — otherwise it is merged
-        back over the fresh one on the next request and the loop simply resumes. Purging
+        back over the fresh one on the next request and the loop simply resumes.
-        is internal-bypasser only; with an external one get_cf_cookies_for_domain()
-        already returns {}.
         """
-        hostname = urlparse(bypass_url).hostname or ""
+        _purge_clearance(bypass_url)
-        # An empty domain means "clear every host" to the bypasser, so skip the purge
-        # rather than wipe clearance for sites that are working fine.
-        if hostname and not _is_using_external_bypasser():
-            _get_internal_bypasser().clear_cf_cookies(hostname)
         return _run_bypasser(bypass_url)
 
     configured_retry = normalize_positive_int(app_config.MAX_RETRY)
@@ -574,6 +583,14 @@ def html_get_page(
                             current_url,
                         )
                         continue
+                    if cookies:
+                        # Challenged *while presenting* clearance: those cookies are
+                        # dead. Without this they survive the solve and get merged back
+                        # over the fresh ones, so every later request re-presents a
+                        # known-rejected cookie and is challenged again - the stale
+                        # retry that never ends.
+                        logger.debug("403 with cookies presented; purging: %s", current_url)
+                        _purge_clearance(current_url)
                     logger.info("403 detected; switching to bypasser: %s", current_url)
                     # Invoke it here rather than setting use_bypasser_now and continuing.
                     # The branch that acts on that flag runs at the top of the *next* retry

shelfmark/download/warmup.py

@@ -15,6 +15,7 @@ source that is down at boot must not affect startup or health.
 
 from __future__ import annotations
 
+import os
 import threading
 
 from shelfmark.core.config import config
@@ -43,13 +44,27 @@ def _as_bool(value: object, *, default: bool) -> bool:
     return bool(value)
 
 
+def _setting(key: str, default: object) -> object:
+    """Read a warm-up setting, preferring the deployment environment.
+
+    These keys are not in the settings registry, and ``config.get`` only consults the
+    environment for keys it knows about - so reading config alone silently ignored
+    SEARCH_WARMUP_ENABLED and always returned the default. Check os.environ first so
+    the documented switches actually work.
+    """
+    raw = os.environ.get(key)
+    if raw is not None and raw.strip():
+        return raw
+    return config.get(key, default)
+
+
 def is_enabled() -> bool:
     """Whether the boot-time warm-up search should run."""
-    if not _as_bool(config.get("SEARCH_WARMUP_ENABLED", True), default=True):
+    if not _as_bool(_setting("SEARCH_WARMUP_ENABLED", True), default=True):
         return False
     # Nothing to warm if the source is off, and no challenge to pre-solve without
     # the bypasser - a plain search is fast enough not to need this.
-    if not _as_bool(config.get("DIRECT_DOWNLOAD_ENABLED", True), default=True):
+    if not _as_bool(_setting("DIRECT_DOWNLOAD_ENABLED", True), default=True):
         logger.debug("Search warm-up skipped: direct download disabled")
         return False
     return True
@@ -57,7 +72,7 @@ def is_enabled() -> bool:
 
 def warmup_query() -> str:
     """The query used to warm the source."""
-    raw = config.get("SEARCH_WARMUP_QUERY", _DEFAULT_QUERY)
+    raw = _setting("SEARCH_WARMUP_QUERY", _DEFAULT_QUERY)
     query = str(raw).strip() if raw else ""
     return query or _DEFAULT_QUERY
 

tests/bypass/test_ddg_cookie_reuse.py

@@ -0,0 +1,221 @@
+"""DDoS-Guard cookie reuse between requests.
+
+Anna's Archive issues nine cookies after a solve, and they are not equivalent:
+
+    __ddg1_/__ddg2_/__ddgid_   ~1 year   clearance
+    __ddgmark_                 ~1 day
+    __ddg5_                    session
+    __ddg8_/__ddg9_/__ddg10_   ~40 min   one check: token, CLIENT IP, TIMESTAMP
+
+Replaying the last three is what produces the ?check=1 redirect loop. They describe a
+single check, so once the timestamp ages out - or the egress IP changes, routine
+behind a VPN - DDoS-Guard stops recognising the caller and re-arms the challenge on
+every request. Storing an expired cookie and sending it forever has the same effect.
+"""
+
+import time
+
+import pytest
+
+import shelfmark.bypass.internal_bypasser as ib
+
+
+@pytest.fixture(autouse=True)
+def _clean_cookie_store(monkeypatch):
+    monkeypatch.setattr(ib, "_cf_cookies", {})
+    monkeypatch.setattr(ib, "_cf_user_agents", {})
+
+
+class _Cookie:
+    """Stand-in for the CDP cookie objects the bypasser extracts."""
+
+    def __init__(self, name, value="v", expires=None, domain="annas-archive.gl"):
+        self.name = name
+        self.value = value
+        self.expires = expires
+        self.domain = domain
+        self.path = "/"
+        self.secure = True
+
+
+def _store(cookies, url="https://annas-archive.gl/search"):
+    ib._store_extracted_cookies(url=url, cookies=cookies, user_agent="UA/1.0")
+
+
+# --------------------------------------------------------------------------- #
+# Per-check cookies must not be persisted for replay
+# --------------------------------------------------------------------------- #
+def test_per_check_cookies_are_not_stored():
+    """The IP/timestamp trio describes one check and must not outlive it."""
+    _store(
+        [
+            _Cookie("__ddg1_", "clearance"),
+            _Cookie("__ddg2_", "clearance2"),
+            _Cookie("__ddg8_", "opaque"),
+            _Cookie("__ddg9_", "203.0.113.7"),
+            _Cookie("__ddg10_", "1786826304"),
+            _Cookie("ddg_last_challenge", "1786826304"),
+        ]
+    )
+
+    stored = ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+    assert set(stored) == {"__ddg1_", "__ddg2_"}
+    for ephemeral in ("__ddg8_", "__ddg9_", "__ddg10_", "ddg_last_challenge"):
+        assert ephemeral not in stored
+
+
+def test_clearance_cookies_survive():
+    _store([_Cookie("__ddg1_", "a"), _Cookie("__ddg2_", "b"), _Cookie("__ddgid_", "c")])
+
+    stored = ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+    assert stored == {"__ddg1_": "a", "__ddg2_": "b", "__ddgid_": "c"}
+
+
+def test_cloudflare_cookies_are_unaffected():
+    _store([_Cookie("cf_clearance", "token"), _Cookie("__cf_bm", "bm")])
+
+    stored = ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+    assert stored == {"cf_clearance": "token", "__cf_bm": "bm"}
+
+
+def test_per_check_cookies_are_excluded_even_for_full_session_domains(monkeypatch):
+    """extract_all exists for Z-Library sessions; it must not resurrect the trio."""
+    monkeypatch.setattr(ib, "_get_full_cookie_domains", lambda: {"annas-archive.gl"})
+    _store([_Cookie("sessionid", "s"), _Cookie("__ddg9_", "203.0.113.7")])
+
+    stored = ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+    assert "sessionid" in stored
+    assert "__ddg9_" not in stored
+
+
+# --------------------------------------------------------------------------- #
+# Expiry must be honoured for every cookie, not only cf_clearance
+# --------------------------------------------------------------------------- #
+def test_expired_ddg_cookies_are_dropped():
+    """The old code only expiry-checked cf_clearance, so DDoS-Guard domains - which
+    have none - replayed dead cookies forever."""
+    past = int(time.time()) - 60
+    _store([_Cookie("__ddg1_", "live"), _Cookie("__ddgmark_", "dead", expires=past)])
+
+    stored = ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+    assert stored == {"__ddg1_": "live"}
+
+
+def test_all_cookies_expired_returns_empty_so_caller_re_solves():
+    past = int(time.time()) - 60
+    _store([_Cookie("__ddg1_", "dead", expires=past)])
+
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {}
+    assert ib.has_valid_cf_cookies("annas-archive.gl") is False
+
+
+def test_unexpired_cookies_are_kept():
+    future = int(time.time()) + 3600
+    _store([_Cookie("__ddg1_", "live", expires=future)])
+
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {"__ddg1_": "live"}
+
+
+def test_session_cookies_never_expire():
+    """expires<=0 means a session cookie, not an already-expired one."""
+    _store([_Cookie("__ddg5_", "s", expires=0), _Cookie("__ddg1_", "a", expires=None)])
+
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {"__ddg5_": "s", "__ddg1_": "a"}
+
+
+def test_expired_cf_clearance_still_drops_the_whole_domain():
+    """Pre-existing Cloudflare behaviour must not regress."""
+    past = int(time.time()) - 60
+    _store([_Cookie("cf_clearance", "dead", expires=past), _Cookie("__cf_bm", "bm")])
+
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {}
+
+
+def test_expired_cookies_are_pruned_from_the_store():
+    """A dropped cookie must not linger and be re-evaluated on every request."""
+    past = int(time.time()) - 60
+    _store([_Cookie("__ddg1_", "live"), _Cookie("__ddgmark_", "dead", expires=past)])
+
+    ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+    assert set(ib._cf_cookies["annas-archive.gl"]) == {"__ddg1_"}
+
+
+def test_solve_that_yields_only_per_check_cookies_stores_nothing():
+    """No clearance means no reuse - the caller must go back to the bypasser rather
+    than believe it holds a valid session."""
+    _store([_Cookie("__ddg9_", "203.0.113.7"), _Cookie("__ddg10_", "1786826304")])
+
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {}
+
+
+# --------------------------------------------------------------------------- #
+# Rejected cookies are discarded, never retried forever
+# --------------------------------------------------------------------------- #
+class _Resp:
+    def __init__(self, status_code, text="page"):
+        self.status_code = status_code
+        self.text = text
+
+
+def _seed(monkeypatch):
+    _store([_Cookie("__ddg1_", "clearance"), _Cookie("__ddg2_", "c2")])
+    monkeypatch.setattr(ib, "get_proxies", lambda _url: None)
+    monkeypatch.setattr(ib, "get_ssl_verify", lambda _url: True)
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl")
+
+
+def test_rejected_cached_cookies_are_discarded(monkeypatch):
+    """A 403 while presenting cookies proves they are dead - keep them and every
+    later request re-presents a known-rejected cookie."""
+    _seed(monkeypatch)
+    monkeypatch.setattr(ib.requests, "get", lambda *a, **k: _Resp(403))
+
+    assert (
+        ib._try_with_cached_cookies("https://annas-archive.gl/search", "annas-archive.gl") is None
+    )
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {}
+
+
+def test_redirect_loop_on_cached_cookies_discards_them(monkeypatch):
+    """DDoS-Guard answers dead clearance with an endless ?check=1 bounce, which
+    surfaces as an exception rather than a status code."""
+    _seed(monkeypatch)
+
+    def boom(*_a, **_k):
+        raise ib.requests.exceptions.TooManyRedirects("Exceeded 30 redirects")
+
+    monkeypatch.setattr(ib.requests, "get", boom)
+
+    assert (
+        ib._try_with_cached_cookies("https://annas-archive.gl/search", "annas-archive.gl") is None
+    )
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {}
+
+
+def test_working_cookies_are_kept(monkeypatch):
+    _seed(monkeypatch)
+    monkeypatch.setattr(ib.requests, "get", lambda *a, **k: _Resp(200, "the page"))
+
+    result = ib._try_with_cached_cookies("https://annas-archive.gl/search", "annas-archive.gl")
+
+    assert result == "the page"
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") != {}
+
+
+def test_failure_only_clears_the_failing_host(monkeypatch):
+    """clear_cf_cookies('') means every host - a blank hostname must not wipe
+    clearance for sites that are working fine."""
+    _seed(monkeypatch)
+    _store([_Cookie("__ddg1_", "other")], url="https://other-site.test/x")
+    monkeypatch.setattr(ib.requests, "get", lambda *a, **k: _Resp(403))
+
+    ib._try_with_cached_cookies("https://annas-archive.gl/search", "annas-archive.gl")
+
+    assert ib.get_cf_cookies_for_domain("annas-archive.gl") == {}
+    assert ib.get_cf_cookies_for_domain("other-site.test") == {"__ddg1_": "other"}

tests/download/test_search_warmup.py

@@ -144,3 +144,27 @@ def test_start_does_not_run_the_search_inline(monkeypatch, warmup):
 
     if warmup._warmup_thread:
         warmup._warmup_thread.cancel()
+
+
+def test_env_var_can_disable_the_warmup(monkeypatch, warmup):
+    """SEARCH_WARMUP_ENABLED is not in the settings registry, so config.get never
+    sees it - the documented off-switch only works if os.environ is consulted."""
+    _patch_config(monkeypatch, warmup, {})  # config knows nothing about the key
+    monkeypatch.setenv("SEARCH_WARMUP_ENABLED", "false")
+
+    assert warmup.is_enabled() is False
+    assert warmup.start() is False
+
+
+def test_env_var_can_set_the_query(monkeypatch, warmup):
+    _patch_config(monkeypatch, warmup, {})
+    monkeypatch.setenv("SEARCH_WARMUP_QUERY", "Moby Dick")
+
+    assert warmup.warmup_query() == "Moby Dick"
+
+
+def test_env_var_absent_falls_back_to_config(monkeypatch, warmup):
+    monkeypatch.delenv("SEARCH_WARMUP_QUERY", raising=False)
+    _patch_config(monkeypatch, warmup, {"SEARCH_WARMUP_QUERY": "From Config"})
+
+    assert warmup.warmup_query() == "From Config"