This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new b98faf4d1c fix(workflow-operator): re-validate redirect hops in HF
remote-URL fetches to close an SSRF bypass (#7013)
b98faf4d1c is described below
commit b98faf4d1c8ff4a60d8282fe906167568f178992
Author: Prateek Ganigi <[email protected]>
AuthorDate: Thu Jul 30 12:11:53 2026 -0700
fix(workflow-operator): re-validate redirect hops in HF remote-URL fetches
to close an SSRF bypass (#7013)
### What changes were proposed in this PR?
Closes an SSRF bypass in the HuggingFace inference operator's
`_fetch_remote_url` (generated Python in `PythonCodegenBase.scala`).
The helper hardens remote fetches (https-only, rejects
private/loopback/link-local/ reserved addresses incl. the
169.254.169.254 metadata endpoint, size cap) but validated only the
*original* URL, then called `requests.get(...)`, which follows redirects
by default. Redirects were never re-checked, so a 302 to
`http://169.254.169.254/...` or an internal host was fetched without
re-running the scheme/address checks. Every remote fetch in the operator
routes through this helper, user-provided image/audio URLs and
provider-returned media URLs, so a malicious input or hostile provider
response could
reach internal services and exfiltrate the content via the result
column.
The fix follows redirects manually so every hop gets the same scrutiny
as the original:
- Extracted the scheme + address checks into a `_validate_remote_url`
helper so it can run per hop.
- `_fetch_remote_url` sets `allow_redirects=False` and walks the chain
in a bounded loop (`MAX_REDIRECT_HOPS = 5`), validating before each
request; relative `Location` values are resolved via `urljoin` and
re-validated.
- Fails closed on a missing `Location` or an over-long chain;
intermediate responses are closed. Size cap and `raise_for_status`
unchanged.
- Hardened the address check to an allowlist stance: it now also
requires a globally-routable address (`not ip.is_global`) alongside the
existing predicates. This additionally blocks the CGNAT/shared range
(100.64.0.0/10) the predicate list missed and stays correct across
Python versions, while keeping the explicit predicates (e.g. multicast,
which CPython reports as global).
Net effect: a redirect can no longer downgrade the scheme or point the
worker at a non-public address; legitimate https→https (and relative)
redirects still work.
### Any related issues, documentation, discussions?
Closes #6967
### How was this PR tested?
Added a generated-code test to HuggingFaceInferenceOpDescSpec pinning
the fix:
`allow_redirects=False`, validation running before each request,
interception of all redirect statuses (301/302/303/307/308),
relative-Location resolution, the hop cap, the fail-closed errors, and
the globally-routable-only address check.
sbt "WorkflowOperator/testOnly
org.apache.texera.amber.operator.huggingFace.*"
Full HF package passes (123 tests), including
PythonCodeRawInvalidTextSpec, which py_compiles the generated Python of
all 117 operators. scalafmt clean.
Beyond the source-level assertions, the redirect logic was manually
verified by executing the generated operator Python against simulated
redirect scenarios (no network): http-downgrade, private/RFC1918,
169.254.169.254 metadata, CGNAT, multicast, IPv6 loopback/ULA,
IPv4-mapped, userinfo-trick, and mixed public+private hosts are each
blocked and never fetched, while legitimate https→https and
relative-Location redirects still succeed and the hop cap is enforced.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Fable 5 in compliance with ASF.
---
.../huggingFace/codegen/PythonCodegenBase.scala | 60 ++++++++++++++++++----
.../HuggingFaceInferenceOpDescSpec.scala | 35 +++++++++++++
2 files changed, 85 insertions(+), 10 deletions(-)
diff --git
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/huggingFace/codegen/PythonCodegenBase.scala
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/huggingFace/codegen/PythonCodegenBase.scala
index 7cd305bfb6..bac14f0a8a 100644
---
a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/huggingFace/codegen/PythonCodegenBase.scala
+++
b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/huggingFace/codegen/PythonCodegenBase.scala
@@ -128,6 +128,10 @@ object PythonCodegenBase {
| # Hard cap on bytes pulled from an external
(user/response-provided) URL.
| MAX_REMOTE_FETCH_BYTES = 25 * 1024 * 1024
|
+ | # Redirect-chain bound for _fetch_remote_url. Redirects are
followed
+ | # manually so every hop is re-validated (https-only, public
address).
+ | MAX_REDIRECT_HOPS = 5
+ |
| def open(self):
| # User-provided strings reach the operator via base64-encoded
| # decode expressions so they cannot break Python syntax or
@@ -704,13 +708,16 @@ object PythonCodegenBase {
| # branches of _call_provider).
| #
──────────────────────────────────────────────────────────────────
|
- | def _fetch_remote_url(self, url):
- | '''Fetch an external URL with SSRF hardening. Returns
(content_type, data).
- | Enforces https-only, rejects
private/loopback/link-local/reserved
- | addresses (covers the 169.254.169.254 cloud-metadata
endpoint), and
- | caps the response at MAX_REMOTE_FETCH_BYTES. The address check
runs
- | before the request, so it mitigates but does not fully prevent
DNS
- | rebinding (requests re-resolves on connect).
+ | def _validate_remote_url(self, url):
+ | '''Validate one URL before it is fetched: https-only, has a
host,
+ | and every address the host resolves to is a globally-routable
+ | public address (rejects private/loopback/link-local/reserved/
+ | multicast/CGNAT addresses, covering the 169.254.169.254
+ | cloud-metadata endpoint). Called on the original
+ | URL and again on every redirect hop, so a redirect can neither
+ | downgrade the scheme nor point at an internal address. The
+ | address check runs before the request, so it mitigates but does
+ | not fully prevent DNS rebinding (requests re-resolves on
connect).
| '''
| import ipaddress
| import socket
@@ -727,10 +734,43 @@ object PythonCodegenBase {
| raise ValueError(f"Could not resolve host '{host}': {e}")
| for info in addrinfos:
| ip = ipaddress.ip_address(info[4][0])
- | if (ip.is_private or ip.is_loopback or ip.is_link_local
- | or ip.is_reserved or ip.is_multicast or
ip.is_unspecified):
+ | # Allowlist stance: require a globally-routable address.
The
+ | # explicit predicates stay because is_global misses some
ranges
+ | # (CPython reports multicast as global) —
belt-and-suspenders,
+ | # and it also covers the CGNAT/shared range (100.64.0.0/10)
+ | # that the predicate list alone lets through.
+ | if (not ip.is_global or ip.is_private or ip.is_loopback
+ | or ip.is_link_local or ip.is_reserved or
ip.is_multicast
+ | or ip.is_unspecified):
| raise ValueError(f"Refusing to fetch from non-public
address {ip}.")
- | resp = requests.get(url, timeout=120, stream=True)
+ |
+ | def _fetch_remote_url(self, url):
+ | '''Fetch an external URL with SSRF hardening. Returns
(content_type, data).
+ | Redirects are never followed automatically: each hop is
re-validated
+ | by _validate_remote_url and the chain is bounded by
+ | MAX_REDIRECT_HOPS, so a redirect cannot escape the https-only /
+ | public-address checks. The body is capped at
MAX_REMOTE_FETCH_BYTES.
+ | '''
+ | from urllib.parse import urljoin as _urljoin
+ | current_url = url
+ | resp = None
+ | for _hop in range(self.MAX_REDIRECT_HOPS + 1):
+ | self._validate_remote_url(current_url)
+ | resp = requests.get(current_url, timeout=120, stream=True,
allow_redirects=False)
+ | if resp.status_code in (301, 302, 303, 307, 308):
+ | location = resp.headers.get("Location", "")
+ | resp.close()
+ | if not location:
+ | raise ValueError("Redirect response has no
Location header.")
+ | # Location may be relative; resolve it against the
current
+ | # URL. The result is re-validated at the top of the
loop.
+ | current_url = _urljoin(current_url, location)
+ | else:
+ | break
+ | else:
+ | raise ValueError(
+ | f"Too many redirects (more than
{self.MAX_REDIRECT_HOPS}) while fetching remote URL."
+ | )
| resp.raise_for_status()
| content_type = resp.headers.get("Content-Type", "")
| total = 0
diff --git
a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceInferenceOpDescSpec.scala
b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceInferenceOpDescSpec.scala
index 83f6239903..3db7576470 100644
---
a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceInferenceOpDescSpec.scala
+++
b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/huggingFace/HuggingFaceInferenceOpDescSpec.scala
@@ -304,6 +304,41 @@ class HuggingFaceInferenceOpDescSpec extends AnyFlatSpec
with Matchers {
code should not include "open(audio_input"
}
+ it should "re-validate every redirect hop in _fetch_remote_url instead of
following blindly" in {
+ // requests follows redirects by default, which would skip the scheme/IP
+ // checks on the redirect target: a 302 to http://169.254.169.254/... or an
+ // internal host would be fetched. The helper must disable automatic
+ // redirects and re-run _validate_remote_url on each hop.
+ val code = makeDesc(task = "image-to-image", inputImageColumn =
"img").generatePythonCode()
+ // Automatic redirect-following is off, and no redirect-following variant
+ // of the fetch remains anywhere in the helper.
+ code should include(
+ "resp = requests.get(current_url, timeout=120, stream=True,
allow_redirects=False)"
+ )
+ code should not include "resp = requests.get(url, timeout=120,
stream=True)"
+ // The per-hop validator exists and runs BEFORE the request inside the
loop.
+ code should include("def _validate_remote_url(self, url):")
+ val validateCall = code.indexOf("self._validate_remote_url(current_url)")
+ val fetchCall = code.indexOf("resp = requests.get(current_url")
+ validateCall should be > 0
+ fetchCall should be > validateCall
+ // Every redirect status is intercepted; relative Location values are
+ // resolved against the current URL before re-validation.
+ code should include("if resp.status_code in (301, 302, 303, 307, 308):")
+ code should include("current_url = _urljoin(current_url, location)")
+ // Degenerate redirects fail closed: missing Location and unbounded chains.
+ code should include("Redirect response has no Location header.")
+ code should include("MAX_REDIRECT_HOPS = 5")
+ code should include("Too many redirects")
+ // The validator keeps the full pre-existing checks (https-only + public
+ // address) so each hop gets the same scrutiny as the original URL, and
+ // takes an allowlist stance (globally-routable only) that also blocks the
+ // CGNAT/shared range the predicate list alone misses.
+ code should include("""if parsed.scheme != "https":""")
+ code should include("not ip.is_global")
+ code should include("ip.is_multicast")
+ }
+
it should "treat pandas NA sentinels (NaN, pd.NA, NaT) as missing in
_read_binary_value" in {
// isinstance(value, float) only catches float('nan'); pd.NA / NaT are not
// floats and previously fell through to be str()-ified into bytes. The
