fix(serverless): block SSRF in job-input downloads by deanq · Pull Request #533 · runpod/runpod-python

deanq · 2026-07-08T05:08:44Z

Summary

Serverless job input carries arbitrary user-supplied URLs that the SDK downloads with no restriction on scheme or destination address. A job can therefore make the worker fetch cloud instance-metadata (http://169.254.169.254/...) or other non-public addresses — server-side request forgery (CWE-918). Affected sites: download_files_from_urls and file() in runpod/serverless/utils/rp_download.py.

The host network layer already blocks RFC1918 egress from worker pods, but not link-local (169.254.0.0/16), so cloud-metadata credential theft is the live unmitigated vector.

Changes

New runpod/serverless/utils/rp_ssrf.py, routed through by both download sites:

Scheme allowlist — http/https only.
Destination validation — resolves the host and fails closed if any resolved IP is non-globally-routable (link-local incl. 169.254.169.254, loopback, RFC1918, CGNAT 100.64/10, reserved, multicast, IPv6 ULA/link-local, IPv4-mapped IPv6).
DNS-rebind defense — PinnedIPAdapter dials the pre-validated IP while keeping TLS SNI, certificate verification, and the Host header bound to the original hostname (never alters cert verification).
Redirects — disabled transparently; each hop is re-validated.
Size cap — rejects an oversized Content-Length and aborts mid-stream past the cap; file() now streams to disk instead of buffering the whole body in memory.

Blocks raise SSRFError (a ValueError, not a RequestException) so they fail loudly rather than being retried or silently returned as a failed download.

Configuration (secure by default)

RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS=true — escape hatch for same-VPC / self-hosted fetches (default off).
RUNPOD_MAX_DOWNLOAD_BYTES — per-download byte cap (default 5 GiB).

Public API signatures are unchanged; only non-public destinations and oversized downloads change behavior.

Review follow-ups

Addressed after the initial review:

Multi-address fallback — pinning to the first resolved IP turned a multi-address host into a hard failure when that address is unroutable from the worker (dual-stack host whose AAAA sorts first, on a pod with no IPv6 route). Each validated address is now tried in resolution order, advancing only on requests.ConnectionError; every candidate stays pre-validated.
Malformed Content-Length — the header is remote-controlled, so int() on it could raise ValueError and abort a valid download. In download_files_from_urls that error escaped both the backoff retry and the except RequestException handler, failing the whole batch. Parsed defensively now; the size limit is unchanged (enforced while streaming).
Proxied fetches refused — pinning only holds when this process opens the socket; through a proxy the hostname is re-resolved on the proxy side and the rebind protection silently stops applying. Such a URL now raises SSRFError. NO_PROXY exclusions are honored, and RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS permits the proxied fetch. Chosen over trust_env = False, which would silently ignore a needed proxy and also drop REQUESTS_CA_BUNDLE.
IPv6 Host header — urlparse strips the brackets from an IPv6-literal host, so the pinned adapter emitted a malformed Host (2606::1:443 rather than the [2606::1]:443 RFC 7230 requires).
file() now raises on HTTP errors — it previously saved whatever body came back, so a 4xx/5xx error page was written as the file and, for a .zip, handed to the extractor. This is a behavior change: file() raises requests.RequestException on an error status instead of returning a dict. Pre-existing on main; no in-tree callers and not re-exported from runpod.serverless.utils, so it affects only direct importers.

Test plan

New tests/test_serverless/test_utils/test_ssrf.py: address classification (metadata/RFC1918/CGNAT/IPv6/mapped blocked, public allowed), resolve fail-closed on mixed public+private, scheme rejection, redirect-to-metadata blocked, too-many-redirects, size cap (header + mid-stream), escape hatch, and a real-socket test asserting the adapter dials the pinned IP while preserving the Host header.
Updated test_download.py to the new fetch path; made the concurrent-download assertion order-independent.
Full suite green (495 passed); ruff check clean.
Manual: safe_get against a literal metadata IP, file://, and an RFC1918 address all raise SSRFError; a real public host validates.

Follow-up (out of scope)

Adding 169.254.0.0/16 (and IPv6) to the host daemon egress rules would close the metadata vector at the network layer as additional defense-in-depth — separate host-team ticket.

capy-ai · 2026-07-08T05:08:55Z

Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

promptless · 2026-07-08T05:17:10Z

Promptless prepared a documentation update related to this change.

Triggered by PR #533 (block SSRF in job-input downloads)

Adds a new docs page for the Serverless SDK download helper (download_files_from_urls) covering the default-on SSRF safeguards (only http/https, non-public destinations blocked, size cap) and the new RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS and RUNPOD_MAX_DOWNLOAD_BYTES environment variables, with a warning about the breaking change for handlers fetching from private/same-VPC addresses.

Review: Document SSRF safeguards and env vars for Serverless job-input downloads

Copilot

Pull request overview

This PR hardens serverless job-input URL downloads against SSRF by introducing a centralized “safe fetch” path and routing existing download utilities through it, preventing access to link-local/loopback/private/reserved destinations (notably instance metadata at 169.254.169.254) while also enforcing download size limits.

Changes:

Added runpod/serverless/utils/rp_ssrf.py providing scheme allowlisting, DNS resolution + global-routability validation, DNS-rebind-resistant pinned-IP dialing, redirect re-validation, and download size caps.
Updated runpod/serverless/utils/rp_download.py to use safe_get() and to stream responses to disk with iter_content_capped() rather than buffering full bodies in memory.
Added SSRF-focused test coverage and updated download tests to mock safe_get() and make concurrent assertions order-independent.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`runpod/serverless/utils/rp_ssrf.py`	New SSRF-safe fetch implementation (validation, pinning, redirects, size caps).
`runpod/serverless/utils/rp_download.py`	Routes downloads through `safe_get()` and streams to disk with size enforcement.
`tests/test_serverless/test_utils/test_ssrf.py`	New unit tests for address validation, redirects, size caps, and pinned-IP behavior.
`tests/test_serverless/test_utils/test_download.py`	Updates tests to mock `safe_get()` and handle concurrent call ordering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address PR #533 review feedback. - Tie each single-use SyncClientSession's lifecycle to its response via _bind_session_to_response, so closing the response (with-block or explicit) also closes the session; without it the session's pools/FDs leaked once per download in long-lived workers. - Close the session on redirect hops and on request exceptions; make teardown best-effort so a cleanup failure cannot mask the caller's real error. - Close response and session deterministically in the pinned-IP adapter test. - Add tests for the request-exception path and the error-masking guarantee.

KAJdev · 2026-07-10T02:16:55Z

doesn't the container runtime already do this?

Address PR #533 review feedback. - Tie each single-use SyncClientSession's lifecycle to its response via _bind_session_to_response, so closing the response (with-block or explicit) also closes the session; without it the session's pools/FDs leaked once per download in long-lived workers. - Close the session on redirect hops and on request exceptions; make teardown best-effort so a cleanup failure cannot mask the caller's real error. - Close response and session deterministically in the pinned-IP adapter test. - Add tests for the request-exception path and the error-masking guarantee.

KAJdev · 2026-07-13T18:18:40Z

plus any attacker could just simply not use this library or use any other library of their choice so if this was a security hole this wouldn't be possible to patch in runpod-python.

Address PR #533 review feedback. - Tie each single-use SyncClientSession's lifecycle to its response via _bind_session_to_response, so closing the response (with-block or explicit) also closes the session; without it the session's pools/FDs leaked once per download in long-lived workers. - Close the session on redirect hops and on request exceptions; make teardown best-effort so a cleanup failure cannot mask the caller's real error. - Close response and session deterministically in the pinned-IP adapter test. - Add tests for the request-exception path and the error-masking guarantee.

Job input carries arbitrary URLs that the worker downloads. Without restriction a job can point the worker at cloud instance-metadata (169.254.169.254) or other non-public addresses (CWE-918). Add runpod/serverless/utils/rp_ssrf.py: an http(s) scheme allowlist, DNS resolution that fails closed on any non-global IP (link-local, loopback, RFC1918, CGNAT, reserved, multicast, IPv6 ULA), a connection adapter that pins the socket to the validated IP to defeat DNS rebinding, per-hop redirect re-validation, and a streamed size cap. Route both rp_download fetch sites through it, and stream file() to disk instead of buffering the whole untrusted body in memory. Blocks raise SSRFError (a ValueError, not a RequestException) so they fail loudly rather than being retried or silently dropped. Configurable via RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS (escape hatch, off by default) and RUNPOD_MAX_DOWNLOAD_BYTES (default 5 GiB).

Address PR #533 review feedback. - Tie each single-use SyncClientSession's lifecycle to its response via _bind_session_to_response, so closing the response (with-block or explicit) also closes the session; without it the session's pools/FDs leaked once per download in long-lived workers. - Close the session on redirect hops and on request exceptions; make teardown best-effort so a cleanup failure cannot mask the caller's real error. - Close response and session deterministically in the pinned-IP adapter test. - Add tests for the request-exception path and the error-masking guarantee.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

runpod/serverless/utils/rp_download.py:145

Parsing Content-Length with int(...) can raise ValueError/TypeError for missing or non-integer headers, which would abort file() even though streaming + capped reads can otherwise proceed safely. Parse defensively and fall back to 0 for chunk sizing.

        file_size = int(download_response.headers.get("Content-Length", 0))
        chunk_size = calculate_chunk_size(file_size)

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

tests/test_serverless/test_utils/test_download.py:178

These file() tests call runpod.serverless.utils.rp_download.file(), which unconditionally creates the "job_files" directory via os.makedirs(). Because os.makedirs is not patched here, the unit test suite writes to the real filesystem, which can make tests less hermetic/flaky. Patch os.makedirs in this test (or wrap the call in a patch context) to avoid side effects.

    @patch("runpod.serverless.utils.rp_download.safe_get")
    @patch("builtins.open", new_callable=mock_open)
    def test_download_file_with_content_disposition(self, mock_file, mock_get):
        """
        Tests file() using filename from Content-Disposition
        """
        mock_get.return_value = MockResponse(
            b"file content", 200, {"Content-Disposition": 'inline; filename="test_file.txt"'}
        )

        result = file("http://test.com/file_without_extension")

Content-Length is remote-controlled, so int() on it can raise ValueError and abort an otherwise valid download. In file() this was a new failure path (main never parsed the header there); in download_files_from_urls the ValueError also escaped both the backoff retry and the RequestException handler, failing the whole batch. Parse defensively via parse_content_length(), falling back to 0 so chunk sizing still works. The size limit is unaffected: it is enforced while streaming by iter_content_capped().

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Pinning to ips[0] turned a multi-address host into a hard failure when the first address is unroutable from the worker: a dual-stack host whose AAAA sorts first is unreachable on a pod with no IPv6 route, where plain requests would have tried the A record. Walk the validated addresses in resolution order, advancing only on connection errors, so every candidate stays pre-validated while the fallback behavior is preserved. Also patch os.makedirs in the file() tests, which were creating a real job_files/ directory in the working tree.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

runpod/serverless/utils/rp_ssrf.py:162

PinnedIPAdapter.send() builds the Host header from parsed.hostname, but urllib.parse strips IPv6 brackets. For URLs with an IPv6 literal host (e.g. http://[2001:db8::1]:8080/), this code would emit an invalid Host header like "2001:db8::1:8080" instead of "[2001:db8::1]:8080", which can break requests and any downstream host parsing.

        parsed = urlparse(request.url)
        authority = parsed.hostname or ""
        if parsed.port is not None:
            authority = f"{authority}:{parsed.port}"
        request.headers["Host"] = authority

deanq · 2026-07-28T04:25:17Z

Two different threat models, and I think we're talking past each other on which one this patch is for.

On the container runtime: it blocks RFC1918 egress from worker pods, but not link-local. 169.254.169.254 is reachable from a worker today, which is why cloud-metadata credential theft is the live unmitigated vector here.

On "an attacker could just not use this library": agreed, and that case is out of reach of any SDK change. But the attacker in this threat model isn't the handler author — it's whoever submits the job. A third-party handler (Hub template, community worker) calls download_files_from_urls() on a URL that came from job input and trusts the SDK to fetch it. That handler author isn't the adversary; they're the one being used as the confused deputy. This patch protects them.

The malicious-container case you're describing is real and only the network layer can close it — that's the follow-up host-team ticket for adding 169.254.0.0/16 to the egress rules. Defense in depth, not a substitute.

file() saved whatever body came back, so a 4xx/5xx error page was written as the downloaded file and, when the URL ended in .zip, handed to the extractor. download_files_from_urls already called raise_for_status(); file() never did. Behavior change: file() now raises requests.RequestException on an error status instead of returning a dict pointing at the error page. It has no in-tree callers and is not re-exported from serverless.utils, so this is limited to direct importers.

urlparse strips the brackets from an IPv6-literal URL host, so the adapter built a malformed Host header ("2606::1:443" instead of "[2606::1]:443"), which RFC 7230 requires and some servers reject. Extracted the authority construction into _host_header_for() and covered every host form.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

runpod/serverless/utils/rp_download.py:137

safe_get() implicitly uses max_download_bytes() when max_bytes is omitted, but the streaming loop later calls max_download_bytes() again. Capture max_bytes once and pass it into safe_get(max_bytes=...) so the declared Content-Length check and the streaming cap are guaranteed to use the same value.

    os.makedirs("job_files", exist_ok=True)

    with safe_get(file_url, stream=True, timeout=30, headers=HEADERS) as download_response:
        # Fail on an error response rather than saving the error page as the

runpod/serverless/utils/rp_download.py:164

The streaming cap in iter_content_capped() should use the same max_bytes value that was passed to safe_get(), rather than re-reading RUNPOD_MAX_DOWNLOAD_BYTES mid-function.

        with open(output_file_path, "wb") as output_file:
            for chunk in iter_content_capped(download_response, chunk_size, max_download_bytes()):
                output_file.write(chunk)

runpod/serverless/utils/rp_download.py:80

max_download_bytes() is evaluated separately for safe_get() (implicit default) and iter_content_capped(). If RUNPOD_MAX_DOWNLOAD_BYTES changes at runtime (or callers later pass a custom max_bytes), the header check and streaming cap can disagree. Capture the cap once and pass it to both safe_get(max_bytes=...) and iter_content_capped(...).

This issue also appears in the following locations of the same file:

line 134
line 162

    @backoff.on_exception(backoff.expo, RequestException, max_tries=3)
    def download_file(url: str, path_to_save: str) -> str:
        with safe_get(url, stream=True, timeout=5, headers=HEADERS) as response:
            response.raise_for_status()
            content_disposition = response.headers.get("Content-Disposition")

Pinning only holds when this process opens the socket. Through a proxy the request carries the hostname and the proxy resolves it, so the validated IP is not what gets dialed and the DNS-rebind protection silently stops applying while still appearing to be in force. Refuse such a URL with SSRFError instead. NO_PROXY exclusions are honored, since those hosts are fetched directly, and RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS allows the proxied fetch for deployments that accept the trade-off. Chosen over trust_env=False, which would silently ignore a needed proxy and also drop REQUESTS_CA_BUNDLE.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/test_serverless/test_utils/test_download.py:117

The updated download path is intended to fail loudly on SSRF blocks (SSRFError bypasses RequestException/backoff), but test_download.py doesn't currently assert this propagation. Adding a unit test where safe_get raises SSRFError and download_files_from_urls raises will protect this behavior from regressions.

        # Test requests exception
        mock_get.side_effect = RequestException("Error")
        self.assertEqual(
            download_files_from_urls(JOB_ID, ["https://example.com/picture.jpg"]),
            [None],
        )

yourfinancedomain-star · 2026-07-28T06:07:52Z

***@***.***

…

On July 28, 2026 5:05:26 AM UTC, Copilot ***@***.***> wrote: @Copilot commented on this pull request. ## Pull request overview Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments. <details> <summary>Comments suppressed due to low confidence (1)</summary> **tests/test_serverless/test_utils/test_download.py:117** * The updated download path is intended to fail loudly on SSRF blocks (SSRFError bypasses RequestException/backoff), but test_download.py doesn't currently assert this propagation. Adding a unit test where safe_get raises SSRFError and download_files_from_urls raises will protect this behavior from regressions. ``` # Test requests exception mock_get.side_effect = RequestException("Error") self.assertEqual( download_files_from_urls(JOB_ID, ["https://example.com/picture.jpg"]), [None], ) ``` </details> -- Reply to this email directly or view it on GitHub: #533 (review) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

deanq requested a review from Copilot July 8, 2026 07:12

Copilot started reviewing on behalf of deanq July 8, 2026 07:13 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Comment thread runpod/serverless/utils/rp_ssrf.py Outdated

Comment thread tests/test_serverless/test_utils/test_ssrf.py Outdated

deanq force-pushed the deanq/sls-375-ssrf-download-guard branch from 64596af to 6d4098b Compare July 8, 2026 18:47

deanq requested review from KAJdev and jhcipar July 9, 2026 01:50

deanq force-pushed the deanq/sls-375-ssrf-download-guard branch from 6d4098b to 8382a63 Compare July 11, 2026 00:13

deanq force-pushed the deanq/sls-375-ssrf-download-guard branch from 8382a63 to cd5a130 Compare July 14, 2026 20:13

deanq force-pushed the deanq/sls-375-ssrf-download-guard branch from 66ed95d to 34d5558 Compare July 16, 2026 02:59

deanq added 2 commits July 24, 2026 12:59

Copilot AI review requested due to automatic review settings July 24, 2026 19:59

deanq force-pushed the deanq/sls-375-ssrf-download-guard branch from 34d5558 to f0ca9f9 Compare July 24, 2026 19:59

Copilot started reviewing on behalf of deanq July 24, 2026 20:00 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread runpod/serverless/utils/rp_download.py

Comment thread runpod/serverless/utils/rp_download.py Outdated

jhcipar approved these changes Jul 24, 2026

View reviewed changes

Merge branch 'main' into deanq/sls-375-ssrf-download-guard

a96d5a5

Copilot AI review requested due to automatic review settings July 27, 2026 16:32

Copilot started reviewing on behalf of deanq July 27, 2026 16:33 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread runpod/serverless/utils/rp_ssrf.py Outdated

Comment thread tests/test_serverless/test_utils/test_download.py

Comment thread tests/test_serverless/test_utils/test_download.py

Copilot AI review requested due to automatic review settings July 27, 2026 16:39

Copilot started reviewing on behalf of deanq July 27, 2026 16:40 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread runpod/serverless/utils/rp_ssrf.py

Copilot AI review requested due to automatic review settings July 27, 2026 16:45

Copilot started reviewing on behalf of deanq July 27, 2026 16:45 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 04:28

Copilot started reviewing on behalf of deanq July 28, 2026 04:28 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Comment thread runpod/serverless/utils/rp_ssrf.py

Copilot AI review requested due to automatic review settings July 28, 2026 04:32

Copilot started reviewing on behalf of deanq July 28, 2026 04:33 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 04:59

Copilot started reviewing on behalf of deanq July 28, 2026 05:00 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

deanq merged commit b510d2a into main Jul 28, 2026
23 of 27 checks passed

deanq deleted the deanq/sls-375-ssrf-download-guard branch July 28, 2026 08:10

github-actions Bot mentioned this pull request Jul 28, 2026

chore(main): release 1.11.1 #556

Open

Uh oh!

Conversation

deanq commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Configuration (secure by default)

Review follow-ups

Test plan

Follow-up (out of scope)

Uh oh!

capy-ai Bot commented Jul 8, 2026

Uh oh!

promptless Bot commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

KAJdev commented Jul 10, 2026

Uh oh!

KAJdev commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

deanq commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

yourfinancedomain-star commented Jul 28, 2026 via email

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

deanq commented Jul 8, 2026 •

edited

Loading