From 611874e63ee65fb5fce24ba639a8603d8b31c4c6 Mon Sep 17 00:00:00 2001 From: Zohaib Hassnain <109234410+ZohaibHassan16@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:37:35 +0500 Subject: [PATCH] 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 Co-authored-by: Mohd Kaif <98801504+KaifAhmad1@users.noreply.github.com> Co-authored-by: KaifAhmad1 --- CHANGELOG.md | 7 +++++ semantica/ingest/feed_ingestor.py | 42 ++++++++++++++++++++++++---- tests/ingest/test_feed_ingestor.py | 45 +++++++++++++++++++++--------- 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 717cc1d9..023332ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` diff --git a/semantica/ingest/feed_ingestor.py b/semantica/ingest/feed_ingestor.py index 0dc40559..7a1e50f7 100644 --- a/semantica/ingest/feed_ingestor.py +++ b/semantica/ingest/feed_ingestor.py @@ -42,6 +42,7 @@ from bs4 import BeautifulSoup from ..utils.exceptions import ProcessingError, ValidationError from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker +from .ssrf import parse_bool, request_with_ssrf_guard @dataclass @@ -425,6 +426,9 @@ class FeedMonitor: self.thread: Optional[threading.Thread] = None self.update_callback: Optional[callable] = None self.check_interval = config.get("check_interval", 3600) # Default 1 hour + self.allow_private_ips = parse_bool( + config.get("allow_private_ips"), default=False + ) def add_feed(self, feed_url: str, **options): """ @@ -485,7 +489,12 @@ class FeedMonitor: try: # Fetch feed - response = requests.get(feed_url, timeout=30) + response = request_with_ssrf_guard( + "GET", + feed_url, + allow_private_ips=self.allow_private_ips, + timeout=30, + ) response.raise_for_status() # Parse feed @@ -575,6 +584,9 @@ class FeedIngestor: self.logger = get_logger("feed_ingestor") self.config = config or {} self.config.update(kwargs) + self.allow_private_ips = parse_bool( + self.config.get("allow_private_ips"), default=False + ) # Initialize feed parser self.parser = FeedParser(**self.config) @@ -638,7 +650,12 @@ class FeedIngestor: request_timeout = timeout or options.get( "timeout", self.config.get("timeout", 30) ) - response = requests.get(feed_url, timeout=request_timeout) + response = request_with_ssrf_guard( + "GET", + feed_url, + allow_private_ips=self.allow_private_ips, + timeout=request_timeout, + ) response.raise_for_status() self.logger.debug( f"Fetched feed from {feed_url}: {len(response.text)} bytes" @@ -693,7 +710,12 @@ class FeedIngestor: try: # Fetch website content - response = requests.get(website_url, timeout=30) + response = request_with_ssrf_guard( + "GET", + website_url, + allow_private_ips=self.allow_private_ips, + timeout=30, + ) response.raise_for_status() # Parse HTML @@ -723,7 +745,12 @@ class FeedIngestor: for path in common_paths: try: feed_url = urljoin(website_url, path) - test_response = requests.head(feed_url, timeout=10) + test_response = request_with_ssrf_guard( + "HEAD", + feed_url, + allow_private_ips=self.allow_private_ips, + timeout=10, + ) if test_response.status_code == 200: content_type = test_response.headers.get("Content-Type", "") if ( @@ -741,7 +768,12 @@ class FeedIngestor: for feed_url in feed_urls: try: # Quick validation by fetching feed - test_response = requests.get(feed_url, timeout=10) + test_response = request_with_ssrf_guard( + "GET", + feed_url, + allow_private_ips=self.allow_private_ips, + timeout=10, + ) if test_response.status_code == 200: validated_feeds.append(feed_url) except Exception: diff --git a/tests/ingest/test_feed_ingestor.py b/tests/ingest/test_feed_ingestor.py index 244cc358..957dedbf 100644 --- a/tests/ingest/test_feed_ingestor.py +++ b/tests/ingest/test_feed_ingestor.py @@ -85,11 +85,15 @@ def test_ingest_feed_errors() -> None: ingestor.ingest_feed("not_a_url") with patch( - "requests.get", - side_effect=requests.exceptions.RequestException("Fail"), + "semantica.ingest.ssrf.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("93.184.216.34", 0))], ): - with pytest.raises(ProcessingError): - ingestor.ingest_feed("http://valid.com") + with patch( + "requests.request", + side_effect=requests.exceptions.RequestException("Fail"), + ): + with pytest.raises(ProcessingError): + ingestor.ingest_feed("http://valid.com") def test_monitor_loop_lifecycle() -> None: @@ -204,8 +208,22 @@ def test_discover_feeds_empty() -> None: ingestor = FeedIngestor() html = "No feeds here" - with patch("requests.get", return_value=MagicMock(text=html)): - feeds = ingestor.discover_feeds("http://site.com") + mock_response = MagicMock() + mock_response.text = html + mock_response.status_code = 200 + mock_response.headers = {} + + def fake_request(method, url, **kwargs): + if url == "http://site.com": + return mock_response + raise requests.exceptions.RequestException("not found") + + with patch( + "semantica.ingest.ssrf.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("93.184.216.34", 0))], + ): + with patch("requests.request", side_effect=fake_request): + feeds = ingestor.discover_feeds("http://site.com") assert len(feeds) == 0 @@ -227,15 +245,16 @@ def test_discover_feeds_found() -> None: mock_response = MagicMock() mock_response.text = html mock_response.status_code = 200 + mock_response.headers = {"Content-Type": "application/rss+xml"} - with patch("requests.get", return_value=mock_response): - with patch("requests.head") as mock_head: - # Mock HEAD request headers for the verification step - mock_head.return_value.headers = { - "Content-Type": "application/rss+xml", - } - mock_head.return_value.status_code = 200 + def fake_request(method, url, **kwargs): + return mock_response + with patch( + "semantica.ingest.ssrf.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("93.184.216.34", 0))], + ): + with patch("requests.request", side_effect=fake_request): feeds = ingestor.discover_feeds("http://site.com") assert "http://site.com/rss.xml" in feeds