Conversation
…83) Root cause: register_manual() validated the manual-discovery URL against an HTTPS / loopback allowlist, but call_tool() and call_tool_streaming() re-used `tool_call_template.url` directly without revalidating. An attacker who hosts a malicious OpenAPI spec on a legitimate HTTPS endpoint can declare ``servers: [{ url: "http://169.254.169.254" }]`` (or any internal address) in the spec; the OpenAPI converter blindly trusts that value, and the tool becomes a blind SSRF primitive that hands cloud-metadata credentials and other internal-only responses back to the LLM caller. Same gap existed in all three HTTP-class protocols (``utcp_http.http``, ``utcp_http.streamable_http``, ``utcp_http.sse``); each had a copy of the prefix-based check that was also bypassable via ``http://localhost.evil.com`` because of how ``str.startswith`` matches. Fix: * Add ``utcp_http._security`` with ``is_secure_url`` / ``ensure_secure_url`` helpers that parse the URL with ``urlparse`` and check the hostname (not the prefix) against the loopback set, closing the ``localhost.evil.com`` bypass. * Call ``ensure_secure_url(url, context="manual discovery")`` in each of the three ``register_manual`` paths (replacing the duplicated prefix check) and ``ensure_secure_url(url, context="tool invocation")`` immediately before each aiohttp request in the three ``call_tool`` / ``call_tool_streaming`` paths. The runtime check is the actual SSRF fix; the rewrite of the discovery check just closes a related hostname-prefix bypass. * Tests in ``test_security.py`` pin the accept/reject decisions and explicitly cover the historical bypass cases (``http://localhost.evil.com``, ``http://127.0.0.1.attacker.example``, cloud-metadata IP, ``file://``, etc.). 89/89 HTTP-plugin tests pass. Reported by @YLChen-007 in #83. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ships the SSRF fix from 5b16e43 (#83): tool invocation now revalidates the resolved URL against the same HTTPS / loopback allowlist that manual discovery uses, and the allowlist itself is now hostname-based instead of prefix-based so `http://localhost.evil.com` is rejected.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py">
<violation number="1" location="plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py:281">
P1: The SSRF check allows loopback addresses (`http://127.0.0.1:*`), which contradicts the threat model documented in its own comment. An attacker-controlled OpenAPI spec fetched over a legitimate HTTPS endpoint can set `servers[0].url` to `http://127.0.0.1:9200` (or any other local service), and `ensure_secure_url` will pass it through because `127.0.0.1` is in the loopback allowlist.
Consider using a stricter variant for the tool-invocation context that only allows HTTPS (no loopback HTTP), or at least remove the misleading comment about blocking `http://127.0.0.1:9200`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # (e.g. http://169.254.169.254 for cloud metadata, http://127.0.0.1:9200 | ||
| # for an unauthenticated Elasticsearch). Without this re-check, tool | ||
| # invocation is a blind SSRF primitive — see GHSA / issue #83. | ||
| ensure_secure_url(url, context="tool invocation") |
Contributor
There was a problem hiding this comment.
P1: The SSRF check allows loopback addresses (http://127.0.0.1:*), which contradicts the threat model documented in its own comment. An attacker-controlled OpenAPI spec fetched over a legitimate HTTPS endpoint can set servers[0].url to http://127.0.0.1:9200 (or any other local service), and ensure_secure_url will pass it through because 127.0.0.1 is in the loopback allowlist.
Consider using a stricter variant for the tool-invocation context that only allows HTTPS (no loopback HTTP), or at least remove the misleading comment about blocking http://127.0.0.1:9200.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py, line 281:
<comment>The SSRF check allows loopback addresses (`http://127.0.0.1:*`), which contradicts the threat model documented in its own comment. An attacker-controlled OpenAPI spec fetched over a legitimate HTTPS endpoint can set `servers[0].url` to `http://127.0.0.1:9200` (or any other local service), and `ensure_secure_url` will pass it through because `127.0.0.1` is in the loopback allowlist.
Consider using a stricter variant for the tool-invocation context that only allows HTTPS (no loopback HTTP), or at least remove the misleading comment about blocking `http://127.0.0.1:9200`.</comment>
<file context>
@@ -274,7 +271,15 @@ async def call_tool(self, caller, tool_name: str, tool_args: Dict[str, Any], too
+ # (e.g. http://169.254.169.254 for cloud metadata, http://127.0.0.1:9200
+ # for an unauthenticated Elasticsearch). Without this re-check, tool
+ # invocation is a blind SSRF primitive — see GHSA / issue #83.
+ ensure_secure_url(url, context="tool invocation")
+
# The rest of the arguments are query parameters
</file context>
Defense in depth on top of the runtime URL revalidation that landed in 5b16e43 (GHSA-39j6-4867-gg4w). The runtime check rejects the request once it's already on its way to the loopback interface, but the malicious tools are still registered, still surfaced to the LLM, and still try to fire on every invocation. Better to refuse them at conversion time so they never enter the registry in the first place. Rule: when an OpenAPI spec is fetched from a non-loopback URL, its ``servers[0].url`` must not be a literal loopback address. Anyone running their own UTCP agent locally pointing at a localhost OpenAPI spec stays unaffected — the source URL is itself loopback in that case. And an operator who explicitly trusts a remote spec's loopback server can still override via the call template's ``base_url`` field (handled by the existing override-takes-precedence branch). Added ``is_loopback_url`` helper to ``_security.py`` and four new converter test cases covering: rejection of remote→loopback, allowance of loopback→loopback, allowance of explicit override, and the normal remote→remote case. 106/106 utcp-http tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ships the OpenAPI converter defense-in-depth from e356ea7: a remote spec can no longer declare a loopback ``servers[0].url`` to redirect tool invocation at the agent's loopback interface. The runtime check already shipped in 1.1.2; this just refuses the malicious tools at conversion time so they never enter the registry.
Earlier comment claimed the runtime ``ensure_secure_url`` in ``call_tool`` "already blocks the request" for loopback-redirect SSRF. That is wrong. ``ensure_secure_url`` allows ``http://`` to literal loopback hosts as a legitimate localhost-dev case, so a remote spec declaring ``servers[0].url = http://127.0.0.1:9090`` slips through the runtime gate and reaches the loopback service. Reword the comment to make the actual coverage split explicit: the runtime check catches non-loopback internal addresses (cloud metadata, RFC1918 ranges); the conversion-time check is the *only* defense for the attacker-controlled-loopback case because the spec's origin is the sole signal that distinguishes it from a legitimate localhost call, and that information only exists at conversion time. No behavior change.
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.
Summary by cubic
Centralized URL security validation to prevent SSRF and MITM across all HTTP-based protocols. Only HTTPS is allowed, with HTTP limited to loopback; checks now run at manual discovery, before every tool request, and during OpenAPI conversion.
utcp_http._securitywithis_secure_url,is_loopback_url, andensure_secure_url(hostname-based parsing; loopback-only HTTP; blockslocalhost.evil.com-style spoofs).http,streamable_http, andssemanual registration withensure_secure_url(..., context="manual discovery").ensure_secure_url(..., context="tool invocation")to block SSRF to non-loopback internal hosts via attacker-controlled OpenAPIservers[0].url(loopback handled below).base_urloverride is set.utcp-httpto1.1.3.Written for commit f873ed6. Summary will update on new commits.