Skip to content

Commit 76a679f

Browse files
bokelleyclaude
andauthored
fix(adagents): disable follow_redirects on ads.txt MANAGERDOMAIN fetch (#754)
Mirrors the adagents.json streaming-fetch posture (`_stream_capped` uses `follow_redirects=False`): ads.txt is a publisher-controlled URL and a transparent 30x by httpx would bypass `_check_safe_host` on the resolved Location, re-opening the same SSRF surface the main path already closed. Surfaced as a defense-in-depth follow-up by the round-2 security review on PR #753 — was pre-existing behavior, not regressed by that PR, but worth closing. ads.txt fetch is best-effort by construction (any non-200 returns an empty MANAGERDOMAIN list); publishers who 30x their ads.txt now fall through to "no managerdomain found", and the SDK surfaces the publisher's original 404 — the same outcome as if the publisher had no ads.txt at all. Acceptable for a fallback path. Adds `test_ads_txt_30x_is_not_followed` covering the new behavior. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 352d1bb commit 76a679f

2 files changed

Lines changed: 40 additions & 2 deletions

File tree

src/adcp/adagents.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,18 +625,26 @@ async def _fetch_ads_txt_managerdomains(
625625
or oversized body) — the fallback is best-effort and absence is not
626626
an error. Bodies larger than :data:`MAX_ADS_TXT_BYTES` are discarded
627627
so a hostile publisher can't force the SDK to buffer arbitrary data.
628+
629+
``follow_redirects=False`` matches the adagents.json streaming path:
630+
HTTP 30x is not a sanctioned cross-host delegation mechanism for
631+
ads.txt either, and following one transparently would bypass the
632+
SSRF gate on the resolved Location host. Publishers who serve
633+
ads.txt behind a redirect will fall through to "no MANAGERDOMAIN
634+
found" — the SDK then surfaces the publisher's original 404,
635+
which is the correct outcome for a best-effort fallback.
628636
"""
629637
url = f"https://{publisher_domain}/ads.txt"
630638
headers = {"User-Agent": user_agent, "Accept": "text/plain"}
631639
try:
632640
if client is not None:
633641
response = await client.get(
634-
url, headers=headers, timeout=timeout, follow_redirects=True
642+
url, headers=headers, timeout=timeout, follow_redirects=False
635643
)
636644
else:
637645
async with httpx.AsyncClient() as new_client:
638646
response = await new_client.get(
639-
url, headers=headers, timeout=timeout, follow_redirects=True
647+
url, headers=headers, timeout=timeout, follow_redirects=False
640648
)
641649
if response.status_code != 200:
642650
return []

tests/test_adagents.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2300,6 +2300,36 @@ def handler(url):
23002300
assert result.valid is False
23012301
assert result.manager_domain is None
23022302

2303+
@pytest.mark.asyncio
2304+
async def test_ads_txt_30x_is_not_followed(self):
2305+
# ads.txt fetch uses follow_redirects=False to match adagents.json;
2306+
# a 30x response from the publisher therefore falls through to
2307+
# "no MANAGERDOMAIN parsed" rather than transparently chasing the
2308+
# Location header (which would bypass the SSRF gate).
2309+
from adcp.adagents import validate_adagents_domain
2310+
2311+
def handler(url):
2312+
if url == "https://publisher.example/.well-known/adagents.json":
2313+
return self._not_found()
2314+
if url == "https://publisher.example/ads.txt":
2315+
response = MagicMock()
2316+
response.status_code = 302
2317+
response.headers = httpx.Headers({"location": "https://127.0.0.1/ads.txt"})
2318+
response.text = ""
2319+
response.content = b""
2320+
response.json.return_value = {}
2321+
return response
2322+
raise AssertionError(f"unexpected url {url}")
2323+
2324+
result = await validate_adagents_domain(
2325+
"publisher.example", client=self._build_mock_client(handler)
2326+
)
2327+
2328+
# A 30x ads.txt is treated as "no managerdomain", so the result
2329+
# is the publisher's original 404 with no manager fallback.
2330+
assert result.valid is False
2331+
assert result.manager_domain is None
2332+
23032333
@pytest.mark.asyncio
23042334
async def test_redirect_target_404_does_not_trigger_managerdomain_fallback(self):
23052335
# A 404 on a publisher-named authoritative_location target is a

0 commit comments

Comments
 (0)