feat(seed): allow_private_ips opt-in for trusted internal API sources

[yunaremaia]

Closes #943

Summary

Adds the documented opt-in for trusted internal deployments: allow_private_ips on SeedDataManager, without weakening the secure default.

Changes (semantica/seed/seed_manager.py):

• load_from_api() now delegates to the shared SSRF guard (semantica/ingest/ssrf.py, introduced in fix(ingest): SSRF protection for Web and API ingestors (#867) #906) via request_with_ssrf_guard() instead of raw requests.get() — this also gains redirect-chain validation and bounded DNS resolution for free
• New config option allow_private_ips, parsed through the shared parse_bool() helper (avoids the bool("false") == True pitfall), defaulting to False
• Docstring documents that it should be enabled only for trusted internal deployments

Tests (tests/test_seed_manager.py):

• test_load_from_api_blocks_private_by_default — 127.0.0.1 rejected, no request made
• test_load_from_api_allows_private_when_configured — with config={"allow_private_ips": True} the request proceeds and the flag reaches the guard
• Existing test_load_from_api updated to mock request_with_ssrf_guard (the call path changed)
• 19/19 green in test_seed_manager.py, 25/25 across both seed suites

The follow-up in #936 (SSRF guard for load_from_api) is complementary: this PR plugs the same function into the repo's shared guard with the internal-deployment escape hatch requested here.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

SeedDataManager: opt-in allow_private_ips with shared SSRF guard for API seeds

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Route SeedDataManager API loading through the shared SSRF guard for safer outbound requests.
• Add allow_private_ips config flag (default false) for trusted internal deployments.
• Update and extend tests to cover block-by-default and explicit opt-in behavior.

Diagram

graph TD
  A["SeedDataManager.load_from_api"] --> B["parse_bool(config.allow_private_ips)"] --> C["request_with_ssrf_guard"] --> D["validate_url_for_request"] --> E{"URL allowed?"}
  E -->|"Yes"| F["requests.request()"]
  E -->|"No"| G["Raise ProcessingError"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Host/CIDR allowlist instead of blanket allow_private_ips

• ➕ More precise control than allowing all private/loopback/link-local ranges
• ➕ Reduces risk of accidentally enabling broad internal network access
• ➖ More configuration complexity (parsing/validation and documentation)
• ➖ More edge cases to test (CIDRs, IPv6, DNS-based allowlists)

2. Per-call parameter instead of manager config

• ➕ Makes security posture explicit at call sites
• ➕ Avoids long-lived manager instances carrying a permissive flag unintentionally
• ➖ More API surface area and potential churn for downstream callers
• ➖ Harder to set globally in deployment config compared to a single manager option

Recommendation: The PR’s approach is appropriate: it keeps the secure default, reuses the central SSRF guard (including redirect-chain validation), and adds a clearly documented opt-in for trusted internal deployments. If future requirements demand finer-grained control, consider adding an allowlist (host/CIDR) on top of the existing boolean flag.

Files changed (2) +48 / -6

Enhancement (1) +20 / -2

seed_manager.pyUse shared SSRF guard in load_from_api with allow_private_ips opt-in +20/-2

Use shared SSRF guard in load_from_api with allow_private_ips opt-in

• Switches SeedDataManager.load_from_api from raw requests.get() to request_with_ssrf_guard() to enforce SSRF protections (including redirect validation). Adds allow_private_ips config parsing via parse_bool() with a default of False and documents the security implications in the method docstring.

semantica/seed/seed_manager.py

Tests (1) +28 / -4

test_seed_manager.pyUpdate API seed tests to mock SSRF guard and cover private-IP behavior +28/-4

Update API seed tests to mock SSRF guard and cover private-IP behavior

• Updates the existing API load test to patch request_with_ssrf_guard instead of requests.get. Adds coverage ensuring private/loopback targets are blocked by default and that allow_private_ips=True propagates to the guard when configured.

tests/test_seed_manager.py

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Auth leaks on redirect ✓ Resolved 🐞 Bug ⛨ Security

Description

SeedDataManager.load_from_api() passes Authorization: Bearer ... headers into
request_with_ssrf_guard(), which manually follows redirects while reusing the same headers,
potentially sending the API key to a different redirect target host. This is a credential-disclosure
risk when the initial API (or an intermediary/open-redirect) returns a cross-host redirect that
still passes SSRF validation.

Code

semantica/seed/seed_manager.py[R509-512]

+                full_url,
+                headers=request_headers,
+                timeout=30,
+                allow_private_ips=allow_private,

Evidence

The seed manager builds request_headers (including a Bearer token) and passes them directly to the
SSRF-guarded request. The SSRF helper then performs redirect handling in a loop and reuses the same
kwargs (including headers) for subsequent hops without stripping credentials when Location
changes the host, so sensitive headers can be forwarded to a different host.

semantica/seed/seed_manager.py[486-556]
semantica/ingest/ssrf.py[241-257]
semantica/ingest/ssrf.py[269-293]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`request_with_ssrf_guard()` manually follows redirects but does not implement the same safety behavior as `requests` for sensitive headers. Because it reuses the original `headers` kwargs on every hop, a cross-origin redirect can receive the original `Authorization` header (e.g., a Bearer token from `SeedDataManager.load_from_api`).
### Issue Context
- `SeedDataManager.load_from_api()` adds `Authorization: Bearer {api_key}` and calls `request_with_ssrf_guard(..., headers=request_headers, ...)`.
- `request_with_ssrf_guard()` disables automatic redirects and loops, issuing subsequent requests using the same `kwargs` without removing `Authorization` when the redirect target host changes.
### Fix Focus Areas
- semantica/ingest/ssrf.py[225-293]
- semantica/seed/seed_manager.py[487-556]
### Suggested fix
1. In `request_with_ssrf_guard()`, detect whether the redirect target changes **origin** (at least `hostname`/`netloc`; ideally scheme+host+port).
2. If the origin changes, remove sensitive headers from the next request, at minimum:
- `Authorization`
- `Proxy-Authorization`
(Optionally also consider `Cookie` depending on desired semantics.)
Make sure to copy the headers dict so you don’t mutate the caller’s dict.
3. Add/extend a unit test (preferably in an ssrf-focused test module) that simulates a redirect to a different host and asserts the second hop request does **not** include `Authorization`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Docs omit redirect/metadata SSRF ✓ Resolved 📎 Requirement gap ⛨ Security

Description

The new load_from_api() docstring documents private/loopback/link-local blocking and the
allow_private_ips opt-in, but it does not mention cloud metadata endpoints or that redirects are
always validated before each hop. This falls short of the Seed API SSRF documentation requirement
and can mislead users about the remaining SSRF protections.

Code

semantica/seed/seed_manager.py[R457-460]

+        SSRF protection is enabled by default: URLs resolving to private,
+        loopback, or link-local addresses are rejected. For trusted internal
+        deployments, pass ``allow_private_ips=True`` in the manager config to
+        opt in (documented for internal use only).

Evidence

PR Compliance ID 6 requires Seed API documentation to include an SSRF warning and to explicitly
cover default blocking (including metadata endpoints) and redirect protections. The newly added
docstring lines mention private/loopback/link-local blocking and trusted-only opt-in, but do not
mention metadata endpoints or redirect-chain validation; the existing Seed usage documentation for
API loading also lacks an SSRF warning or mention of allow_private_ips.

Document seed API SSRF security warning and trusted-only allow_private_ips usage
semantica/seed/seed_manager.py[457-460]
semantica/seed/seed_usage.md[631-640]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Seed API documentation does not fully describe SSRF protections: it omits cloud metadata endpoint blocking and does not state that redirect chains are validated/enforced.
## Issue Context
The PR adds/uses SSRF guarding for `SeedDataManager.load_from_api()` and introduces the `allow_private_ips` opt-in. The compliance checklist requires documentation to warn about SSRF risks and accurately describe default-deny behavior, redirect protections, and trusted-only usage.
## Fix Focus Areas
- semantica/seed/seed_manager.py[457-460]
- semantica/seed/seed_usage.md[631-640]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[yunaremaia]

Both findings addressed in 9288d847 — and thank you, the first one was a real credential-disclosure bug in request_with_ssrf_guard:

1. Auth leaks on redirect ✅ — request_with_ssrf_guard now strips Authorization and Proxy-Authorization from the headers when a redirect changes origin (netloc), copying the headers dict so the caller's dict is never mutated. Same-host redirects keep the credential (matching requests semantics). 2 new tests: cross-host redirect drops the bearer token, same-host keeps it. 37/37 green in test_ssrf_protection.py.

2. Docs omit redirect/metadata SSRF ✅ — load_from_api docstring now documents cloud-metadata blocking (169.254.169.254) and that every redirect hop is re-validated before being followed.

Full suite: 62/62 green across the seed + ssrf test modules.

[ZohaibHassan16]

One thing of note: auth stripping fix from Qodo round only covers half the case it needs to. It compares urlparse(url).netloc before/after a redfirect and strips Authorization/proxy-Authorization if the host changes. But netloc doesn't include the scheme so a same host https -> http downgrade redirect doesn't trip it. The bearer token gets replayed in cleartext.

Repro:

request_with_ssrf_guard(
          "GET", "https://example.com/start",
           session=session,
            headers={"Authorization": "Bearer secret-token"},
}

Headers sent on downgraded hop: {'Authorization': 'Bearer secret-token'}

reuqests' own should_strip_auth() handles the very case (strips on hostname change or https -> http downgrade) which is presumably what "mirroring reuqests' cross host credential stripping" was going for , it just landed the host half and missed the scheme half. Neither of the two new tests (test_strips_autorization_on_cross_host_redirect, test_keeps_authorization_on_same_host_redirect_) exercises a downgrade on the same host, so it slipped through.

Given this PR's whole point is sending Authorization: Bearer <api_key> to external APIs, I had want this closed before merging.

Fix should be a one liner: compare scheme too, or just check old_scheme == "https" and new_scheme == "http" alongisde netloc check.

Everything else is good once this is in.

[yunaremaia]

Excellent catch @ZohaibHassan16 — you are right, the netloc-only comparison missed the https -> http downgrade on the same host, and the Authorization header would have been replayed in cleartext. That is exactly the kind of leak this PR must not ship with.

Fixed in 0abea559:

1. New _should_strip_auth() helper in semantica/ingest/ssrf.py that mirrors requests.utils.should_strip_auth semantics:
   • strips on hostname change (as before),
   • strips on port change,
   • strips on https -> http scheme downgrade (the gap you found),
   • keeps the credential only for the safe http -> https upgrade on default ports (requests own behavior, so upgrades keep working).
2. Two new regression tests in tests/ingest/test_ssrf_protection.py:
   • test_strips_authorization_on_scheme_downgrade — same host, https -> http redirect: second hop has no Authorization (this test fails on the previous code),
   • test_keeps_authorization_on_scheme_upgrade — http -> https on default ports: credential survives, matching requests.

Verified locally: 39/39 tests in test_ssrf_protection.py pass, plus 19/19 in test_seed_manager.py. No behavior change for cross-host stripping.

Thanks again for the sharp review — exactly the kind of gap that is hard to see from the inside.

[ZohaibHassan16]

All tests passing. Approved.