mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-06 06:52:20 +02:00
The R09 scholarly hardening routes every hop through check_outbound_url with block_private=True. On a DNS64/NAT64 network, an IPv4-only host can resolve through the RFC 6052 Well-Known Prefix. For example, export.arxiv.org resolved to 64:ff9b::924b:5b2a in the reproduced environment. CPython classifies that outer IPv6 prefix as reserved, so the URL guard rejected the request before examining the effective IPv4 destination. Decode addresses in exactly 64:ff9b::/96 to their embedded IPv4 destination and evaluate that destination under the strict outbound policy. The translated target is always checked with private-address blocking enabled. This prevents the NAT64 prefix from becoming a path to loopback, private, shared/CGNAT, link-local, multicast, unspecified, or other non-global IPv4 space. Network-specific translation prefixes are not decoded. In particular, 64:ff9b:1::/48 remains subject to the existing IPv6 policy. Coverage includes: - public IPv4 destinations embedded through the RFC 6052 prefix - loopback, link-local, private, CGNAT, multicast, unspecified, benchmark, and TEST-NET rejection - network-specific NAT64 prefixes remaining undecoded - deterministic scholarly provider tests without live DNS dependence - redirect query parameter isolation - relative redirect resolution - HTTP error fallback behavior - editor draft GET and DELETE behavior remaining unaffected R09, R11, and R01 were independently audited and otherwise left unchanged.
523 lines
18 KiB
Python
523 lines
18 KiB
Python
"""Regressions for the 2026-09-23 review fixes.
|
|
|
|
Two findings, both about bounding work the server does on someone else's behalf:
|
|
|
|
* the scholarly metadata lookups called two hardcoded third-party endpoints with
|
|
a stale hand-written User-Agent, no outbound-URL policy, and a full timeout per
|
|
hop, so one query could hold a user-facing search open for the sum of all three;
|
|
* editor drafts were only size-checked after the body had been parsed and
|
|
re-serialised, so the ceiling rejected an allocation it had already paid for.
|
|
|
|
Each test fails on the pre-fix tree.
|
|
"""
|
|
|
|
import time
|
|
|
|
import httpx
|
|
import pytest
|
|
from fastapi import FastAPI
|
|
from fastapi.testclient import TestClient
|
|
|
|
from services.search import core as search_core
|
|
from src.constants import (
|
|
APP_VERSION,
|
|
ARXIV_API_URL,
|
|
OPENALEX_API_URL,
|
|
SCHOLARLY_LOOKUP_TIMEOUT,
|
|
)
|
|
from src.upload_limits import EDITOR_DRAFT_MAX_BYTES
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# ODY-R09 — scholarly lookups
|
|
# --------------------------------------------------------------------------
|
|
|
|
|
|
def test_scholarly_endpoints_use_configured_constants(monkeypatch):
|
|
"""Call sites must route through configured endpoints, not hardcoded URLs."""
|
|
requested_urls = []
|
|
|
|
def fake_scholarly_api_get(url: str, params: dict):
|
|
requested_urls.append(url)
|
|
if "arxiv" in url:
|
|
class FakeArxivResponse:
|
|
text = "<feed xmlns='http://www.w3.org/2005/Atom'></feed>"
|
|
|
|
return FakeArxivResponse()
|
|
elif "openalex" in url:
|
|
class FakeOpenAlexResponse:
|
|
def json(self):
|
|
return {"results": []}
|
|
|
|
return FakeOpenAlexResponse()
|
|
return None
|
|
|
|
monkeypatch.setattr(search_core, "_scholarly_api_get", fake_scholarly_api_get)
|
|
|
|
custom_arxiv = "https://custom.arxiv.test/api/query"
|
|
custom_openalex = "https://custom.openalex.test/works"
|
|
monkeypatch.setattr(search_core, "ARXIV_API_URL", custom_arxiv)
|
|
monkeypatch.setattr(search_core, "OPENALEX_API_URL", custom_openalex)
|
|
|
|
search_core._arxiv_title_results("Attention Is All You Need")
|
|
search_core._openalex_title_results("Attention Is All You Need")
|
|
|
|
assert custom_arxiv in requested_urls
|
|
assert custom_openalex in requested_urls
|
|
|
|
|
|
def test_user_agent_tracks_app_version():
|
|
"""A hand-written version string drifts; APP_VERSION cannot."""
|
|
agent = search_core._scholarly_user_agent()
|
|
assert APP_VERSION in agent
|
|
# The pre-fix tree hardcoded 0.20 while APP_VERSION was already 1.0.3.
|
|
assert "Odysseus/0.20 " not in agent
|
|
|
|
|
|
def test_outbound_policy_rejection_skips_the_request(monkeypatch):
|
|
"""A URL the outbound policy refuses must never reach httpx."""
|
|
calls = []
|
|
monkeypatch.setattr(
|
|
httpx, "get", lambda *a, **k: calls.append(a) or pytest.fail("request sent")
|
|
)
|
|
monkeypatch.setattr(
|
|
"src.url_safety.check_outbound_url", lambda url, **kw: (False, "blocked")
|
|
)
|
|
|
|
assert search_core._scholarly_api_get(ARXIV_API_URL, {}) is None
|
|
assert calls == []
|
|
|
|
|
|
def test_redirect_to_prohibited_destination_is_blocked_and_never_requested(monkeypatch):
|
|
"""An allowed initial URL must not be permitted to redirect into a prohibited destination."""
|
|
from src.url_safety import check_outbound_url as real_check
|
|
|
|
called_urls = []
|
|
|
|
def fake_get(url, **kwargs):
|
|
called_urls.append(url)
|
|
req = httpx.Request("GET", url)
|
|
return httpx.Response(
|
|
302,
|
|
headers={"Location": "http://127.0.0.1:8080/internal-admin"},
|
|
request=req,
|
|
)
|
|
|
|
def mock_check(url, **kwargs):
|
|
if url == OPENALEX_API_URL:
|
|
return (True, "")
|
|
return real_check(url, **kwargs)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", mock_check)
|
|
|
|
result = search_core._scholarly_api_get(OPENALEX_API_URL, {})
|
|
|
|
assert result is None
|
|
# Only the initial allowed URL was contacted; the prohibited redirect destination was never requested
|
|
assert called_urls == [OPENALEX_API_URL]
|
|
|
|
|
|
def test_redirect_to_link_local_metadata_is_blocked_and_never_requested(monkeypatch):
|
|
"""Redirects to cloud metadata or link-local addresses must be refused before connection."""
|
|
from src.url_safety import check_outbound_url as real_check
|
|
|
|
called_urls = []
|
|
|
|
def fake_get(url, **kwargs):
|
|
called_urls.append(url)
|
|
req = httpx.Request("GET", url)
|
|
return httpx.Response(
|
|
301,
|
|
headers={"Location": "http://169.254.169.254/latest/meta-data"},
|
|
request=req,
|
|
)
|
|
|
|
def mock_check(url, **kwargs):
|
|
if url == ARXIV_API_URL:
|
|
return (True, "")
|
|
return real_check(url, **kwargs)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", mock_check)
|
|
|
|
result = search_core._scholarly_api_get(ARXIV_API_URL, {})
|
|
|
|
assert result is None
|
|
assert called_urls == [ARXIV_API_URL]
|
|
|
|
|
|
def test_allowed_redirect_is_followed_safely(monkeypatch):
|
|
"""A safe redirect destination passing outbound checks is followed to completion."""
|
|
called_urls = []
|
|
canonical_url = "https://api.openalex.org/canonical-works"
|
|
|
|
def fake_get(url, **kwargs):
|
|
called_urls.append(url)
|
|
req = httpx.Request("GET", url)
|
|
if url == OPENALEX_API_URL:
|
|
return httpx.Response(301, headers={"Location": "/canonical-works"}, request=req)
|
|
return httpx.Response(200, json={"results": []}, request=req)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", lambda url, **kw: (True, ""))
|
|
|
|
result = search_core._scholarly_api_get(OPENALEX_API_URL, {})
|
|
|
|
assert result is not None
|
|
assert result.status_code == 200
|
|
assert called_urls == [OPENALEX_API_URL, canonical_url]
|
|
|
|
|
|
def test_redirect_limit_is_bounded(monkeypatch):
|
|
"""Redirects exceeding MAX_SCHOLARLY_REDIRECTS must fail safely without looping."""
|
|
called_urls = []
|
|
|
|
def fake_get(url, **kwargs):
|
|
called_urls.append(url)
|
|
req = httpx.Request("GET", url)
|
|
return httpx.Response(302, headers={"Location": f"{url}/next"}, request=req)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", lambda url, **kw: (True, ""))
|
|
|
|
result = search_core._scholarly_api_get("https://export.arxiv.org/api/query", {})
|
|
|
|
assert result is None
|
|
assert len(called_urls) == search_core.MAX_SCHOLARLY_REDIRECTS + 1
|
|
|
|
|
|
def test_redirect_does_not_replay_query_parameters_to_the_new_destination(monkeypatch):
|
|
"""The original query must not be re-sent to a redirect target.
|
|
|
|
The params belong to the endpoint that was asked for. Replaying them across
|
|
a redirect would hand the search terms to whatever host the redirect names.
|
|
"""
|
|
calls = []
|
|
|
|
def fake_get(url, **kwargs):
|
|
params = kwargs.get("params")
|
|
calls.append((url, params))
|
|
# Mirror real httpx: response.url carries the query that was sent.
|
|
req = httpx.Request("GET", httpx.URL(url, params=params or {}))
|
|
if len(calls) == 1:
|
|
return httpx.Response(
|
|
302, headers={"Location": "https://redirect.openalex.test/v2"}, request=req
|
|
)
|
|
return httpx.Response(200, json={"results": []}, request=req)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", lambda url, **kw: (True, ""))
|
|
|
|
result = search_core._scholarly_api_get(
|
|
OPENALEX_API_URL, {"search": "confidential-title"}
|
|
)
|
|
|
|
assert result is not None
|
|
assert len(calls) == 2
|
|
second_url, second_params = calls[1]
|
|
assert second_url == "https://redirect.openalex.test/v2"
|
|
assert second_params is None
|
|
assert "confidential-title" not in second_url
|
|
|
|
|
|
def test_relative_redirect_resolves_against_the_responding_url(monkeypatch):
|
|
"""A relative Location resolves against the URL that answered, not the origin."""
|
|
calls = []
|
|
|
|
def fake_get(url, **kwargs):
|
|
calls.append(url)
|
|
req = httpx.Request("GET", httpx.URL(url, params=kwargs.get("params") or {}))
|
|
if len(calls) == 1:
|
|
return httpx.Response(
|
|
302, headers={"Location": "https://mirror.arxiv.test/api/v1/query"}, request=req
|
|
)
|
|
if len(calls) == 2:
|
|
# Relative hop: must resolve against mirror.arxiv.test, not arxiv.
|
|
return httpx.Response(302, headers={"Location": "../v2/query"}, request=req)
|
|
return httpx.Response(200, text="<feed/>", request=req)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", lambda url, **kw: (True, ""))
|
|
|
|
result = search_core._scholarly_api_get(ARXIV_API_URL, {"search_query": "x"})
|
|
|
|
assert result is not None
|
|
assert calls[2] == "https://mirror.arxiv.test/api/v2/query"
|
|
|
|
|
|
def test_http_error_status_degrades_to_no_results(monkeypatch):
|
|
"""A 500 from a scholarly API must degrade to no results, not raise."""
|
|
|
|
def fake_get(url, **kwargs):
|
|
req = httpx.Request("GET", url)
|
|
return httpx.Response(500, text="upstream exploded", request=req)
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr("src.url_safety.check_outbound_url", lambda url, **kw: (True, ""))
|
|
|
|
assert search_core._arxiv_title_results("Attention Is All You Need") == []
|
|
assert search_core._openalex_title_results("Attention Is All You Need") == []
|
|
|
|
|
|
def test_exhausted_budget_skips_the_request(monkeypatch):
|
|
"""Once the chain's budget is spent, later hops are skipped, not retried."""
|
|
calls = []
|
|
monkeypatch.setattr(
|
|
httpx, "get", lambda *a, **k: calls.append(a) or pytest.fail("request sent")
|
|
)
|
|
monkeypatch.setattr(
|
|
"src.url_safety.check_outbound_url", lambda url, **kw: (True, "")
|
|
)
|
|
|
|
token = search_core._scholarly_deadline.set(time.monotonic() - 1)
|
|
try:
|
|
assert search_core._scholarly_api_get(OPENALEX_API_URL, {}) is None
|
|
finally:
|
|
search_core._scholarly_deadline.reset(token)
|
|
assert calls == []
|
|
|
|
|
|
def test_remaining_budget_caps_the_per_request_timeout(monkeypatch):
|
|
"""A hop cannot wait longer than the budget the chain has left."""
|
|
seen = {}
|
|
|
|
class _Response:
|
|
def raise_for_status(self):
|
|
return None
|
|
|
|
def fake_get(url, **kwargs):
|
|
seen["timeout"] = kwargs.get("timeout")
|
|
return _Response()
|
|
|
|
monkeypatch.setattr(httpx, "get", fake_get)
|
|
monkeypatch.setattr(
|
|
"src.url_safety.check_outbound_url", lambda url, **kw: (True, "")
|
|
)
|
|
|
|
token = search_core._scholarly_deadline.set(time.monotonic() + 2)
|
|
try:
|
|
search_core._scholarly_api_get(ARXIV_API_URL, {})
|
|
finally:
|
|
search_core._scholarly_deadline.reset(token)
|
|
|
|
assert seen["timeout"] <= 2.0
|
|
assert seen["timeout"] < SCHOLARLY_LOOKUP_TIMEOUT
|
|
|
|
|
|
def test_budget_is_shared_across_the_whole_chain(monkeypatch):
|
|
"""Both hops of one lookup draw on a single deadline."""
|
|
observed = []
|
|
|
|
monkeypatch.setattr(
|
|
"src.url_safety.check_outbound_url", lambda url, **kw: (True, "")
|
|
)
|
|
monkeypatch.setattr(
|
|
search_core,
|
|
"_openalex_title_results",
|
|
lambda title, count=3: observed.append(search_core._scholarly_deadline.get())
|
|
or [],
|
|
)
|
|
monkeypatch.setattr(
|
|
search_core,
|
|
"_arxiv_title_results",
|
|
lambda title, count=3: observed.append(search_core._scholarly_deadline.get())
|
|
or [],
|
|
)
|
|
|
|
search_core._direct_scholarly_title_results("a paper title")
|
|
|
|
assert len(observed) == 2
|
|
assert observed[0] is not None
|
|
assert observed[0] == observed[1]
|
|
|
|
|
|
def test_budget_does_not_leak_out_of_the_chain():
|
|
"""The deadline is scoped to the lookup, not left set on the context."""
|
|
assert search_core._scholarly_deadline.get() is None
|
|
with search_core._scholarly_budget():
|
|
assert search_core._scholarly_deadline.get() is not None
|
|
assert search_core._scholarly_deadline.get() is None
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# ODY-R11 — editor draft size ceiling
|
|
# --------------------------------------------------------------------------
|
|
|
|
|
|
def _draft_client() -> TestClient:
|
|
from routes.editor_draft_routes import setup_editor_draft_routes
|
|
|
|
app = FastAPI()
|
|
app.include_router(setup_editor_draft_routes())
|
|
return TestClient(app)
|
|
|
|
|
|
def test_oversized_declared_body_is_refused_before_it_is_parsed():
|
|
"""An over-ceiling Content-Length is rejected without reading the payload."""
|
|
client = _draft_client()
|
|
response = client.post(
|
|
"/api/editor-drafts",
|
|
content=b"{}",
|
|
headers={
|
|
"content-type": "application/json",
|
|
"content-length": str(EDITOR_DRAFT_MAX_BYTES + 1),
|
|
},
|
|
)
|
|
assert response.status_code == 413
|
|
assert "safety limit" in response.text
|
|
|
|
|
|
def test_oversized_declared_body_guard_runs_before_body_consumption():
|
|
"""An oversized Content-Length must reject the request without reading or consuming the body."""
|
|
body_consumed = False
|
|
|
|
def body_stream():
|
|
nonlocal body_consumed
|
|
body_consumed = True
|
|
yield b'{"layers": []}'
|
|
|
|
client = _draft_client()
|
|
response = client.post(
|
|
"/api/editor-drafts",
|
|
content=body_stream(),
|
|
headers={
|
|
"content-type": "application/json",
|
|
"content-length": str(EDITOR_DRAFT_MAX_BYTES + 1),
|
|
},
|
|
)
|
|
assert response.status_code == 413
|
|
assert "safety limit" in response.text
|
|
# Proves the body stream was never read or consumed before rejection
|
|
assert body_consumed is False
|
|
|
|
|
|
def test_oversized_declared_body_guard_runs_before_body_consumption_on_put():
|
|
"""Update route also rejects oversized Content-Length without consuming body."""
|
|
body_consumed = False
|
|
|
|
def body_stream():
|
|
nonlocal body_consumed
|
|
body_consumed = True
|
|
yield b'{"layers": []}'
|
|
|
|
client = _draft_client()
|
|
response = client.put(
|
|
"/api/editor-drafts/some-draft-id",
|
|
content=body_stream(),
|
|
headers={
|
|
"content-type": "application/json",
|
|
"content-length": str(EDITOR_DRAFT_MAX_BYTES + 1),
|
|
},
|
|
)
|
|
assert response.status_code == 413
|
|
assert body_consumed is False
|
|
|
|
|
|
def test_oversized_declared_body_rejects_without_json_parsing():
|
|
"""Even malformed or invalid JSON is rejected with 413 rather than 422 if Content-Length exceeds ceiling."""
|
|
client = _draft_client()
|
|
response = client.post(
|
|
"/api/editor-drafts",
|
|
content=b"this is completely invalid json {[[",
|
|
headers={
|
|
"content-type": "application/json",
|
|
"content-length": str(EDITOR_DRAFT_MAX_BYTES + 1),
|
|
},
|
|
)
|
|
assert response.status_code == 413
|
|
|
|
|
|
def test_update_route_carries_the_same_guard():
|
|
client = _draft_client()
|
|
response = client.put(
|
|
"/api/editor-drafts/whatever",
|
|
content=b"{}",
|
|
headers={
|
|
"content-type": "application/json",
|
|
"content-length": str(EDITOR_DRAFT_MAX_BYTES + 1),
|
|
},
|
|
)
|
|
assert response.status_code == 413
|
|
|
|
|
|
def test_absent_content_length_still_reaches_the_exact_check():
|
|
"""The header guard is an optimisation; it must not become the only check."""
|
|
from routes.editor_draft_routes import _dump_payload, reject_oversized_draft_body
|
|
|
|
class _NoLengthRequest:
|
|
headers: dict = {}
|
|
|
|
# No header: the guard abstains rather than rejecting or accepting outright.
|
|
assert reject_oversized_draft_body(_NoLengthRequest()) is None
|
|
|
|
# The authoritative byte count still refuses an over-ceiling payload.
|
|
with pytest.raises(Exception) as excinfo:
|
|
_dump_payload({"blob": "x" * (EDITOR_DRAFT_MAX_BYTES + 1)})
|
|
assert getattr(excinfo.value, "status_code", None) == 413
|
|
|
|
|
|
def test_malformed_content_length_does_not_crash_the_route():
|
|
from routes.editor_draft_routes import reject_oversized_draft_body
|
|
|
|
class _BadLengthRequest:
|
|
headers = {"content-length": "not-a-number"}
|
|
|
|
assert reject_oversized_draft_body(_BadLengthRequest()) is None
|
|
|
|
|
|
def test_ordinary_draft_is_unaffected():
|
|
"""The guard must not change behaviour for normal payloads."""
|
|
from routes.editor_draft_routes import _dump_payload
|
|
|
|
assert _dump_payload({"layers": []}) == '{"layers":[]}'
|
|
|
|
|
|
def test_route_class_guard_applies_only_to_body_bearing_methods():
|
|
"""The route class must not change GET/DELETE semantics.
|
|
|
|
``EditorDraftRoute`` is attached to the whole editor-draft router, so the
|
|
bodyless routes run through it too. They must reach their handler
|
|
untouched — the early ceiling belongs to the methods that carry a draft.
|
|
"""
|
|
from fastapi import APIRouter
|
|
|
|
from routes.editor_draft_routes import EditorDraftRoute
|
|
|
|
router = APIRouter(route_class=EditorDraftRoute)
|
|
|
|
@router.get("/probe")
|
|
async def _get_probe():
|
|
return {"reached": "get"}
|
|
|
|
@router.delete("/probe")
|
|
async def _delete_probe():
|
|
return {"reached": "delete"}
|
|
|
|
@router.post("/probe")
|
|
async def _post_probe():
|
|
return {"reached": "post"}
|
|
|
|
app = FastAPI()
|
|
app.include_router(router)
|
|
client = TestClient(app)
|
|
|
|
oversized = {
|
|
"content-type": "application/json",
|
|
"content-length": str(EDITOR_DRAFT_MAX_BYTES + 1),
|
|
}
|
|
|
|
# Bodyless methods are unaffected even when a bogus huge length is declared.
|
|
for method, expected in (("GET", "get"), ("DELETE", "delete")):
|
|
response = client.request(method, "/probe", content=b"{}", headers=oversized)
|
|
assert response.status_code == 200, (method, response.text)
|
|
assert response.json() == {"reached": expected}
|
|
|
|
# The same declaration on the body-bearing method is refused.
|
|
assert client.post("/probe", content=b"{}", headers=oversized).status_code == 413
|
|
|
|
# ...and an ordinary POST still reaches the handler.
|
|
ordinary = client.post("/probe", json={"layers": []})
|
|
assert ordinary.status_code == 200
|
|
assert ordinary.json() == {"reached": "post"}
|