fix(security): add SSRF guard for A2A peer and Gensui connect endpoints#18
Open
wstlima wants to merge 1 commit into
Open
fix(security): add SSRF guard for A2A peer and Gensui connect endpoints#18wstlima wants to merge 1 commit into
wstlima wants to merge 1 commit into
Conversation
CodeQL flagged 2 full-ssrf and 7 partial-ssrf findings. Investigated all
9: the ones in shogun/api/system.py (Ollama/LMStudio model provider
proxy) and shogun/services/channel_service.py (Telegram bot API, only
the path is user-controlled, not the host) are working as designed —
connecting to a local/self-hosted inference server is the actual
feature, and blocking private IPs there would break the documented use
case. Left those alone.
The two that are genuine unrestricted SSRF, with no legitimate reason
to reach internal network ranges:
- shogun/integrations/a2a_client.py `send()` / `ping()` — peer_url
comes straight from InvitePeerRequest.peer_url (shogun/api/a2a.py,
no visible auth dependency on that router) and is used verbatim to
build the outbound request URL. A2A peers are remote Shogun
instances reached over the public internet — there's no valid case
for an A2A peer resolving to 127.0.0.0/8, 169.254.169.254, or an
RFC 1918 address.
- shogun/api/gensui_config.py `/connect` and `/test` — req.server_url
is fully attacker/user-controlled and hits `{server_url}/api/gensui/
health` unchecked. Gensui commonly runs on the operator's own LAN
(this POC itself proxies http://127.0.0.1:8787), so this endpoint
needs to allow private/loopback ranges — but not the cloud metadata
endpoint or link-local space.
Added shogun/services/ssrf_guard.py: resolves the hostname via
socket.getaddrinfo and rejects the target if any resolved address is
private/loopback/link-local/reserved/multicast. `assert_safe_url()`
defaults to strict (blocks everything above — used for a2a_client.py);
`allow_private=True` permits RFC 1918 and loopback while still
blocking link-local (which covers 169.254.169.254 generically) and the
well-known metadata literal addresses explicitly — used for
gensui_config.py.
This is a resolve-time check, not a complete SSRF defense: it does not
protect against DNS rebinding after the check or against redirects to
a blocked target (httpx follows redirects by default in some call
sites here). Sufficient to close the most common/obvious vector;
documented as a known limitation in the module docstring.
Verified with 8 representative cases (public host, metadata endpoint
in both modes, private/loopback in both modes, bad scheme) — all pass
as expected. See shogun/services/ssrf_guard.py for the test snippet
used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9
Summary
CodeQL flagged 2
full-ssrfand 7partial-ssrffindings. Investigated all 9:Findings in
shogun/api/system.py(Ollama/LMStudio model provider proxy) andshogun/services/channel_service.py(Telegram bot API — only the path is user-controlled, not the host) are working as designed. Connecting to a local/self-hosted inference server is the actual feature; blocking private IPs there would break the documented use case. Left those alone.The two that are genuine unrestricted SSRF, with no legitimate reason to reach internal network ranges:
shogun/integrations/a2a_client.pysend()/ping()—peer_urlcomes straight fromInvitePeerRequest.peer_url(no visible auth dependency on that router) and is used verbatim to build the outbound request URL. A2A peers are remote Shogun instances reached over the public internet — no valid case for a peer resolving to127.0.0.0/8,169.254.169.254, or an RFC 1918 address.shogun/api/gensui_config.py/connectand/test—req.server_urlis fully user-controlled and hits{server_url}/api/gensui/healthunchecked. Gensui commonly runs on the operator's own LAN, so this endpoint needs to allow private/loopback ranges — but not the cloud metadata endpoint or link-local space.Fix
Added
shogun/services/ssrf_guard.py: resolves the hostname viasocket.getaddrinfoand rejects the target if any resolved address is private/loopback/link-local/reserved/multicast.assert_safe_url()defaults to strict (blocks everything above) — used fora2a_client.pyallow_private=Truepermits RFC 1918 and loopback while still blocking link-local (covers169.254.169.254) and the well-known metadata literal addresses — used forgensui_config.pyThis is a resolve-time check, not a complete SSRF defense: it doesn't protect against DNS rebinding after the check or redirects to a blocked target. Documented as a known limitation in the module docstring.
Test plan
127.0.0.1:8787(the documented local Gensui use case) still works withallow_private=True