mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-08-29 04:26:20 +00:00
security: apply SSRF guard to feed ingestion requests (#928)
* security: apply SSRF guard to feed ingestion requests FeedIngestor and FeedMonitor fetched feed and website URLs with plain requests.get/head calls, bypassing the SSRF validation already used by web_ingestor.py and api_ingestor.py. This allowed feed URLs pointing at loopback, link-local, or other private network addresses to be fetched directly. Route all outbound requests in feed_ingestor.py through request_with_ssrf_guard, gated by the same allow_private_ips config option the other ingestors expose. * test: mock the correct request boundary in test_discover_feeds_empty The test still patched requests.get after discover_feeds() moved to request_with_ssrf_guard(), which calls requests.request and performs real DNS resolution. That left the test hitting live network/DNS. * docs(changelog): document FeedIngestor SSRF guard fix (#928, closes #927) Records the SSRF guard applied to all 5 feed-ingestion request sites, the Qodo-flagged test-mock fix, independent PoC verification, and the carried-over exception-swallowing behavior in discover_feeds(). --------- Co-authored-by: Sameer Kadam <sskadam6305@gmail.com> Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com> Co-authored-by: KaifAhmad1 <kaifahmad087@gmail.com>
This commit is contained in:
co-authored by
Sameer Kadam
Mohd Kaif
KaifAhmad1
parent
43bac6170c
commit
611874e63e
@@ -97,6 +97,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Security
|
||||
|
||||
- **`FeedIngestor`/`FeedMonitor` (RSS/Atom feed ingestion) had no SSRF protection, allowing requests to internal/private network targets** (#928, closes #927) by @ZohaibHassan16
|
||||
- `FeedIngestor.ingest_feed()`, `discover_feeds()` (link-tag fetch, common-path HEAD probe, and feed-validation GET), and `FeedMonitor.check_updates()` all called `requests.get()`/`requests.head()` directly with default redirect-following and no scheme allowlist or private/loopback/link-local IP validation — despite `semantica/ingest/ssrf.py`'s `request_with_ssrf_guard()` already existing and being used by `web_ingestor.py`/`api_ingestor.py`. `ingest_feed()`'s own URL check only verified `urlparse(url).scheme`/`.netloc` were non-empty, never that the scheme was http/https or that the resolved target IP was safe. Reachable via the public `ingest_feed()`/`ingest()` entry points with any caller-supplied feed URL
|
||||
- All 5 call sites now route through `request_with_ssrf_guard()`, which validates scheme (http/https only) and resolved IP before the request, and re-validates every redirect `Location` before following it — closing both the direct-IP and redirect-chain SSRF paths. Added an `allow_private_ips` config option to both `FeedIngestor` and `FeedMonitor`, consistent with the other ingestors
|
||||
- **Fixed during review** (Qodo): `test_discover_feeds_empty` mocked `requests.get`, which no longer executes now that the code path goes through `request_with_ssrf_guard()` (backed by `requests.request`) — the test was passing without exercising the real code. Corrected to mock `requests.request` and `socket.getaddrinfo`
|
||||
- `pytest tests/ingest/test_feed_ingestor.py`: 12/12 passed. Independently reproduced the issue's own PoC (`FeedIngestor().ingest_feed("http://127.0.0.1:8765/feed.xml")` against a live local server) and confirmed it now raises `ValidationError` instead of succeeding
|
||||
- **Known limitation carried over from `discover_feeds()`'s pre-existing design**: its common-path and feed-validation loops use a blanket `except Exception: continue`, which now also silently absorbs `ValidationError` from a blocked candidate URL the same way it already absorbed network failures — the request is still correctly blocked before reaching the network, so this is not an SSRF bypass, just a missed opportunity to log "blocked as SSRF target" distinctly from "unreachable"
|
||||
|
||||
- **`RepoIngestor` clone surface hardened against GitPython URL/option injection** (#905, closes #868) by @pravit-amp
|
||||
- `RepoIngestor.ingest_repository()` passed the caller-supplied repository URL and arbitrary `**options` straight through to `git.Repo.clone_from()` on a `GitPython>=3.1.50` floor predating hardening for `ext::`-style transport helpers and `$VAR`/`${VAR}` environment-variable expansion in clone URLs — unvalidated clone options (`upload_pack`, `multi_options`, `template`, `config`, `env`, ...) could be abused for command execution, and unvalidated hostnames allowed SSRF against internal services (e.g. cloud metadata endpoints)
|
||||
- `GitPython` floor raised to `>=3.1.58`
|
||||
|
||||
Reference in New Issue
Block a user