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>
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 and before every tool request.
utcp_http._securitywithis_secure_urlandensure_secure_urlfor strict URL parsing and loopback-only HTTP.http,streamable_http, andssemanual registration withensure_secure_url(..., context="manual discovery").servers[0].url(e.g., cloud metadata and internal services).http://localhost.evil.com; allowhttp://localhost,http://127.0.0.1, andhttp://[::1].utcp-http1.1.2.Written for commit de2c3a8. Summary will update on new commits.