fix(web_fetch): pin SSRF guard connect target in fetcher.py + link/pipeline.py (#13019)

[mrveiss]

Closes #13019

Thinking Path

Phase 1 verified each claim in #13019 against source and corrected two of them before touching code:

1. web_fetch/fetcher.py — confirmed, but only for _fetch_bs4. _fetch_bs4 really does check _is_public_url(url) then connect separately (previously via the shared pooled get_http_client() session) — a genuine DNS-rebind TOCTOU. Correction: _fetch_jina_impl does NOT share this defect. Its actual TCP connect target is always the fixed, trusted r.jina.ai host — the caller's url is only embedded in the request path (https://r.jina.ai/{url}), never the connect target — so pinning it would be a no-op. Same for _fetch_playwright (connects to our own in-cluster Playwright service, not url, mirroring PR fix(content_reach): pin SSRF guard connect target to close robots.txt redirect + TOCTOU gaps (#13017) #13020's Playwright/yt-dlp scope-narrowing). Reachability: WebFetcher.fetch/fetch_raw_html is called from chat_workflow/tool_handler.py, api/knowledge_scrape.py, api/knowledge_mcp.py, api/knowledge_extract.py, tools/tool_registry.py, services/research/orchestrator.py, knowledge/connectors/web_crawler.py — broad, caller-controlled-URL exposure, not admin-only.
2. media/link/pipeline.py — confirmed, and WORSE than reported. _fetch_and_parse's _is_public_url_async(url) check only ever gated the Jina attempt — if it returned False, the code fell through unconditionally to the BeautifulSoup fallback fetch with zero SSRF enforcement (an existing test, test_fetch_skips_jina_when_hostname_resolves_to_private_ip, explicitly asserted this: it mocked DNS to a private IP and still expected the BS4 fetch to proceed). This isn't merely a TOCTOU window — it's a missing gate. Fixed by making the pinned fetch unconditional, which closes both issues in one change. Reachability correction: LinkPipeline/MediaPipelineManager currently has no production caller — api/knowledge_scrape.py and friends call web_fetch.WebFetcher directly; process_url() is referenced only from its own test file. So severity here is "latent, not actively exploitable" rather than the issue's "Medium" — fixed anyway per the project's "never delete code — wire it in" stance, since perf(http): 134 raw aiohttp.ClientSession constructions remain (127 per-request) despite the shared pool at 151 sites #12979 already actively maintains this file.
3. HTTP stack: both modules are aiohttp, unlike content_reach (httpx). media/link/pipeline.py is also on the shared pooled client from perf(http): 134 raw aiohttp.ClientSession constructions remain (127 per-request) despite the shared pool at 151 sites #12979 for Jina, but Jina's fixed connect target means pooling there is unaffected by this fix. The existing aiohttp ssrf_guard.pinned_connector is reused as-is (aiohttp's own resolver only maps one hostname → one pinned IP, so a naive pinned_connector(url) + allow_redirects=True would silently misroute any cross-host redirect) — no httpx helper was needed, so nothing was promoted from content_reach/_http.py.
4. Both fetch paths need genuine redirect-following (BS4 defaults follow up to 5/10 hops today — real-world URLs redirect constantly); simply disabling redirects (the fetch_safe_url/api/knowledge.py precedent) would be a real functionality regression, so a new shared primitive, ssrf_guard.pinned_request_with_redirects, independently resolves + pins every hop (including redirects), rejecting a redirect to a private/rebound address outright instead of following it.

What Changed

autobot_shared/security/ssrf_guard.py — new pinned_request_with_redirects(method, url, ...): an async context manager that issues each hop with allow_redirects=False through its own fresh pinned_connector, re-validating the Location before ever connecting to it. Bounded by max_redirects (SSRFError on exhaustion, not silent truncation).

autobot_shared/ssot_config.py — new web_fetch_max_redirects field (AUTOBOT_WEB_FETCH_MAX_REDIRECTS, default 5) shared by both call sites.

web_fetch/fetcher.py — _fetch_bs4 now routes through pinned_request_with_redirects instead of the shared pooled client; SSRFError is logged distinctly ("blocked by SSRF guard") from a natural fetch failure. _MAX_REDIRECTS is now config-sourced instead of a hardcoded literal.

media/link/pipeline.py — _fetch_and_parse's fallback now routes through pinned_request_with_redirects unconditionally (closing the missing-gate bug), preserving the existing ssl= passthrough for allow_self_signed. New _MAX_REDIRECTS module constant, same config field.

Not changed (by design): _fetch_jina_impl/_try_jina and _fetch_playwright — see correction #1 above.

Verification

python3 -m py_compile <7 changed files>                      # OK
python3 -m flake8 --max-line-length=120 <7 changed files>    # 0
python3 -m black --check --line-length=120 <7 changed files> # unchanged

pytest tests/content_reach/ tests/unit/web_fetch/ media/link/pipeline_test.py \
       autobot_shared/security/ssrf_guard_test.py tests/test_startup_imports.py -q
  743 passed (393 non-startup + 350 startup-import-smoke)
  cp-swapped baseline (origin/Dev_new_gui content restored via cp, not stash):
    startup-import-smoke: 350/350 both sides, unchanged
    PR #13020's content_reach tests: still green, unchanged

New/changed hostile-path tests (mock real DNS at autobot_shared.url_safety.socket.getaddrinfo, never the guard — mirrors api/tests/test_provider_auth_ssrf.py / PR #13020's test_http_pinned_get.py):

• autobot_shared/security/ssrf_guard_test.py — pinned_request_with_redirects: blocks private IP / localhost / link-local metadata pre-connect; DNS-rebind (public hostname resolving private at connect); follows a legitimate cross-host redirect with independent per-hop pinning (asserts each hop gets its OWN connector); rejects a redirect to a private IP literal outright; exceeds-max_redirects raises SSRFError rather than truncating silently; verifies the connector's pinned resolver targets the IP literal while hostname (Host/SNI) preserves the original name.
• tests/unit/web_fetch/test_fetcher.py (TestFetchBS4SSRFPinning, 5 tests) — private IP literal, link-local metadata, DNS-rebind (public at _is_public_url pre-check / private at pinned connect), redirect-to-internal rejected outright, and a genuinely public URL still succeeds end-to-end.
• media/link/pipeline_test.py (TestLinkPipelineFetchSSRFPinning, 3 tests) — same private-IP/DNS-rebind/redirect-to-internal cases exercised through _fetch_and_parse itself, proving the previously-missing gate now blocks the fallback fetch unconditionally, not just Jina. The 7 pre-existing functional tests for the old get_http_client().tracked_request() shape were rewired onto the new pinned-fetch factory (mocked directly for pure functional assertions — TLS passthrough, HTTP-error status, success) rather than duplicating DNS-mocking plumbing into every test.

Discovered (filed separately, not in scope here)

• media/link/pipeline.py: _MAX_CONTENT_LENGTH (1MB cap) declared but never enforced in _fetch_and_parse #13021 — media/link/pipeline.py:67's _MAX_CONTENT_LENGTH (1 MB cap) is declared but never enforced; unbounded body read in the same fallback fetch (resource-exhaustion class, distinct from SSRF).

Model Used

Claude Sonnet 5

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!