fix(security): add SSRF guard to save_url_image for provider-returned URLs by liuhao1024 · Pull Request #44743 · NousResearch/hermes-agent

liuhao1024 · 2026-06-12T07:34:24Z

What does this PR do?

Adds an SSRF guard to save_url_image() in agent/image_gen_provider.py. Before downloading a provider-returned URL, the function now checks it against the existing is_safe_url() private-network policy. This prevents a compromised or malicious image generation provider (xAI, OpenAI, Krea) from causing the Hermes host to fetch internal resources such as loopback services, cloud metadata endpoints, or private network hosts.

Related Issue

Fixes #44728

Type of Change

🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

agent/image_gen_provider.py: Added is_safe_url() check before requests.get() in save_url_image(). Raises ValueError with descriptive message if the URL targets a private/internal address.
tests/agent/test_image_gen_ssrf_guard.py: Added 4 tests — blocks loopback (127.0.0.1), cloud metadata (169.254.169.254), internal network (10.0.0.1), and allows valid public URLs.

How to Test

Run pytest tests/agent/test_image_gen_ssrf_guard.py -v — all 4 tests pass
Verify test_save_url_image_blocks_cloud_metadata passes (the key SSRF regression test)
Verify test_save_url_image_allows_public_url passes (public URLs still work)

Checklist

Code

I've read the Contributing Guide
My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
I searched for existing PRs to make sure this isn't a duplicate
My PR contains only changes related to this fix/feature (no unrelated commits)
I've run pytest tests/ -q and all tests pass
I've added tests for my changes (required for bug fixes, strongly encouraged for features)
I've tested on my platform: macOS

Documentation & Housekeeping

I've updated relevant documentation (README, docs/, docstrings) — or N/A
I've updated cli-config.yaml.example if I added/changed config keys — or N/A
I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

Analyzed: agent/image_gen_provider.py::save_url_image() — shared by xAI, OpenAI, and Krea providers
Blast radius: LOW — single function change, all 3 providers benefit from the guard
Related patterns: Same is_safe_url check used in tools/vision_tools.py and tools/web_tools.py

… URLs The `save_url_image()` function in `agent/image_gen_provider.py` performs a raw `requests.get()` on URLs returned by image generation providers (xAI, OpenAI, Krea). A compromised or malicious provider could return a URL targeting loopback addresses, internal hosts, or cloud metadata endpoints, causing the Hermes host to make an unintended request. Add `is_safe_url()` check before the fetch, using the same private-network policy already enforced for user-supplied URLs in browser/vision tools. Fixes NousResearch#44728

…mpatibility The SSRF guard added to save_url_image blocks 127.0.0.1 by default, which breaks the existing test suite that spins up a local HTTP server for integration testing. Add autouse fixture to bypass the guard in those tests (SSRF behavior is tested separately in test_image_gen_ssrf_guard.py).

egilewski

Recommendation: request changes

I reviewed this in security mode against current GitHub main 0db5cb8e7541c3713c0e14e09e9fcc5d99193ca7, PR base d810f2b2620bff54262e13c3e3239e771c11f342, and PR head ed75554c7741699504fc4fe44d80276681b40c0c.

Validation:

gh pr checks 44743 --repo NousResearch/hermes-agent: all visible checks passed or were skipped; no completed failing PR-specific check.
git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr/44743: passed with tree 4c6a8d01c1daf3216482ed2a1e7bef13715cc2f6.
git diff --check refs/remotes/upstream/main...refs/remotes/upstream/pr/44743: passed.
pytest -q tests/agent/test_image_gen_ssrf_guard.py tests/agent/test_save_url_image.py -p no:cacheprovider: passed, 12 passed.
py_compile agent/image_gen_provider.py tests/agent/test_image_gen_ssrf_guard.py tests/agent/test_save_url_image.py: passed.
Synthetic local-server probe: current main fetched loopback content through save_url_image(), while the PR blocked direct loopback URLs. The same PR head still saved loopback content after a public-looking provider URL redirected to loopback: redirect_private_routed=saved provider_dns_calls=1 private_hits=1 leaked_marker=True.

Finding:
agent/image_gen_provider.py validates only the original provider URL before calling requests.get(..., stream=True), but requests follows redirects by default. A malicious or compromised provider can return a public URL that passes is_safe_url() and then redirects to loopback, cloud metadata, or another private address; no redirect hook or final-URL validation runs before bytes are cached. The new tests cover direct private URLs, but they do not cover this provider-redirect source-to-sink path.

Please either disable redirects for this fetch or revalidate every redirect/final URL before streaming and caching image bytes.

Signed: GPT-5.5-xhigh in Codex

A compromised or malicious provider can return a public URL that passes is_safe_url() but redirects (HTTP 302) to a private/internal address. requests.get() follows redirects by default, so the initial SSRF check is bypassed. Add post-redirect re-validation: if response.url differs from the original URL, check the final destination with is_safe_url() before streaming bytes. This blocks the redirect-based SSRF attack vector identified in review. Also add 3 tests: redirect-to-private (blocked), redirect-to-public (allowed), and no-redirect (single is_safe_url call).

liuhao1024 · 2026-06-12T22:32:18Z

Thanks for the thorough security review, @egilewski. Great catch on the redirect bypass — requests.get() follows 302s by default, so a malicious provider could redirect a safe initial URL to 127.0.0.1 or 169.254.169.254 after the guard passes.

Fix applied: Added post-redirect re-validation — if response.url differs from the original URL, the final destination is checked with is_safe_url() before any bytes are streamed or cached. This blocks the provider-redirect source-to-sink path.

Changes:

agent/image_gen_provider.py: Re-validate response.url after requests.get() returns; raise ValueError if redirect target is private/internal
tests/agent/test_image_gen_ssrf_guard.py: +3 tests — redirect-to-private (blocked), redirect-to-public (allowed), no-redirect (single guard call)

All 15 tests pass (7 SSRF guard + 8 existing save_url_image).

egilewski

Recommendation: request changes

I reviewed this rework in security mode against current GitHub main a86b7b314b6c381204ebdcdd7ed79917e6eb3f9b, PR base d810f2b2620bff54262e13c3e3239e771c11f342, and PR head 797ad96f9d55db23fb35c34a711b2ab4d89f544b.

Validation:

gh pr checks 44743 --repo NousResearch/hermes-agent: all visible checks passed or were skipped; no completed failing PR-specific check.
git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr/44743: passed, tree 4ab47b2aa3c13aae633abe94e5fa47469c88de6d.
git diff --check refs/remotes/upstream/main...refs/remotes/upstream/pr/44743: passed.
/home/mac/hermes-agent/.venv/bin/python -B redirect_probe.py <current-main-worktree>: reproduced the original redirect issue; the helper saved the redirected private response and the private test endpoint recorded private_hits=1.
/home/mac/hermes-agent/.venv/bin/python -B redirect_probe.py <pr-head-worktree>: the helper now raises ValueError: Blocked: redirect target is a private or internal address, but the private test endpoint still recorded private_hits=1.
/home/mac/hermes-agent/.venv/bin/python -B -m pytest -q tests/agent/test_image_gen_ssrf_guard.py tests/agent/test_save_url_image.py -p no:cacheprovider: passed, 15 tests.
/home/mac/hermes-agent/.venv/bin/python -B -m py_compile agent/image_gen_provider.py tests/agent/test_image_gen_ssrf_guard.py tests/agent/test_save_url_image.py: passed.

Finding:
The redirect fix still validates too late. save_url_image() calls requests.get(url, timeout=timeout, stream=True) with the default redirect behavior, so requests has already followed the provider URL to the private/internal target before this new response.url check runs. The PR now avoids caching the private response, but a malicious provider-controlled URL can still cause Hermes to make the private network request itself, including to loopback or metadata-style endpoints. The redirect safety check needs to happen before following or fetching the redirect target, not after the redirected response has already been obtained.

Signed: GPT-5.5-xhigh in Codex

… SSRF guard Address review feedback from egilewski: requests.get() follows redirects by default, meaning the request to a private/internal address has already been made by the time response.url is inspected. Fix: use allow_redirects=False and manually follow each redirect hop, validating the Location header URL with is_safe_url() before proceeding. This ensures no request is made to a private address even when a compromised provider returns a public URL that redirects internally. Also adds multi-hop redirect test to verify each hop is validated.

egilewski

Recommendation: request changes

I reviewed this rework in security mode against current GitHub main a86b7b314b6c381204ebdcdd7ed79917e6eb3f9b, PR base d810f2b2620bff54262e13c3e3239e771c11f342, and PR head 65d62942d03f90990c664756262c25d758fb2cf6.

Validation:

gh pr checks 44743 --repo NousResearch/hermes-agent: all visible checks passed or were skipped; no completed failing PR-specific check.
git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr/44743: passed, tree 723181eba80b475ae0f88cd8181a3349c8a39edf.
git diff --check refs/remotes/upstream/main...refs/remotes/upstream/pr/44743: passed.
redirect_probe.py <current-main-worktree>: reproduced the old redirect SSRF path; /single and /hop1 both reached /private and cached the private marker.
redirect_probe.py <pr-head-worktree>: the PR now raises before requesting /private for both direct and multi-hop private redirects, while a public redirect still saves successfully.
/home/mac/hermes-agent/.venv/bin/python -B -m pytest -q tests/agent/test_image_gen_ssrf_guard.py tests/agent/test_save_url_image.py -p no:cacheprovider: passed, 15 passed.
/home/mac/hermes-agent/.venv/bin/python -B -m py_compile agent/image_gen_provider.py tests/agent/test_image_gen_ssrf_guard.py tests/agent/test_save_url_image.py: passed.
coderabbit review --plain --base upstream/main --type committed: completed; it reported the redirect-limit issue below, which I independently reproduced with a live local-server probe.

Finding:
save_url_image() still needs a too-many-redirects failure path. The new manual redirect loop iterates range(_MAX_REDIRECTS + 1), closes each redirect response, and continues. If every hop is a safe redirect, the loop can exhaust without hitting the non-redirect break; execution then falls through to response.raise_for_status() and streams the last redirect response body as if it were the final image. My too_many_redirects_probe.py <pr-head-worktree> test served 11 safe redirects with Content-Type: image/png, and the PR saved the redirect body: status=saved, redirect_body_saved=true, requests_seen=/r0.../r10.

This does not reintroduce the private-target SSRF request from the previous review, but it is a concrete regression in the new redirect-following implementation. Please add an explicit redirect-budget error, for example with a for ... else that raises before response.raise_for_status() when the loop exhausts, and clean up the unused final variable in the multi-hop test.

Signed: GPT-5.5-xhigh in Codex

…rl_image When every hop in the redirect chain is a safe redirect, the loop exhausts without hitting the non-redirect break. Previously, execution fell through to response.raise_for_status() and streamed the last redirect body as the image. Add a for...else clause that closes the response and raises ValueError when the redirect budget is exceeded. Also remove unused `final` variable in multi-hop test.

Remove unused `_CallList` and `call` imports from the multi-hop redirect test, and add the missing assertion that verifies the private URL was never fetched (addresses egilewski review feedback on PR NousResearch#44743).

teknium1

Thanks for the security hardening. The premise is confirmed on current main: agent/image_gen_provider.py:293 still sends the provider-returned URL directly to requests.get(). The PR's allow_redirects=False loop and per-hop is_safe_url() validation at PR-head agent/image_gen_provider.py:239-272 address the direct and redirect-to-private paths discussed in the review history.

Problems

agent/image_gen_provider.py:252 only enters the redirect path when Location is present. A malformed 3xx without Location reaches the normal cache path after response.raise_for_status() rather than failing closed. Add an explicit malformed-redirect error and regression coverage.
A separate URL-fetch sibling remains at plugins/image_gen/openai/__init__.py:137 (_load_image_bytes). The PR timeline cross-references #56035 for that path; it should remain independently tracked rather than treated as covered here.

Suggested changes

Close any 3xx response and raise if it lacks Location; test that behavior against a real HTTP response.

Automated hermes-sweeper review.

teknium1 · 2026-07-14T12:02:24Z

+            _current_url, timeout=timeout, stream=True, allow_redirects=False
+        )
+        # 3xx with Location → validate before following
+        if response.is_redirect and response.headers.get("Location"):


Please fail closed for every 3xx response that lacks Location. This branch only handles redirects with a location, so a malformed 3xx can fall through to raise_for_status() and be cached as an image. Close the response and raise, with a regression test using an actual 3xx/no-Location response.

## Summary - Apply `is_safe_url` and manual redirect re-validation to `save_url_video` (sibling of the image path). - Prevent cloud-metadata / private-target pivots via redirected CDN URLs when downloading generated video assets. - Add focused regression tests. ## Salvage / credit Sibling coverage for the incomplete image-side work tracked around NousResearch#44743 / NousResearch#44728 (video download path was still unprotected).

alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/vision Vision analysis and image generation labels Jun 12, 2026

liuhao1024 mentioned this pull request Jun 12, 2026

[Security] xAI image generation provider performs an unguarded host-side fetch of provider-controlled image URLs #44728

Open

egilewski suggested changes Jun 12, 2026

View reviewed changes

egilewski suggested changes Jun 13, 2026

View reviewed changes

zapabob mentioned this pull request Jun 13, 2026

fix(security): guard provider image URL downloads #45537

Open

liuhao1024 added 2 commits June 13, 2026 20:00

alt-glitch mentioned this pull request Jul 1, 2026

fix(security): add URL safety check to image_ref fetch in openai image_gen (SSRF) #56035

Open

teknium1 reviewed Jul 14, 2026

View reviewed changes

teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026

zapabob mentioned this pull request Jul 23, 2026

fix(security): SSRF-guard save_url_video redirects #70349

Open

1 task

Navigation Menu

Uh oh!

fix(security): add SSRF guard to save_url_image for provider-returned URLs - #44743

fix(security): add SSRF guard to save_url_image for provider-returned URLs#44743
liuhao1024 wants to merge 6 commits into
NousResearch:mainfrom
liuhao1024:fix/image-gen-ssrf-guard

liuhao1024 commented Jun 12, 2026

Uh oh!

egilewski left a comment

Uh oh!

liuhao1024 commented Jun 12, 2026

Uh oh!

egilewski left a comment

Uh oh!

egilewski left a comment

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

liuhao1024 commented Jun 12, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Checklist

Code

Documentation & Housekeeping

Code Intelligence

Uh oh!

egilewski left a comment

Choose a reason for hiding this comment

Uh oh!

liuhao1024 commented Jun 12, 2026

Uh oh!

egilewski left a comment

Choose a reason for hiding this comment

Uh oh!

egilewski left a comment

Choose a reason for hiding this comment

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Suggested changes

Uh oh!

teknium1 Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants