Skip to content

fix/(mcp/condor): honor full WEB_URL instead of hard-coded 127.0.0.1 - #160

Open
lgrawet wants to merge 1 commit into
hummingbot:mainfrom
lgrawet:fix/mcp-condor-use-web-url
Open

fix/(mcp/condor): honor full WEB_URL instead of hard-coded 127.0.0.1#160
lgrawet wants to merge 1 commit into
hummingbot:mainfrom
lgrawet:fix/mcp-condor-use-web-url

Conversation

@lgrawet

@lgrawet lgrawet commented Jul 20, 2026

Copy link
Copy Markdown

call_main_api built the URL as http://127.0.0.1:{WEB_PORT}/..., ignoring the host and scheme from WEB_URL. When WEB_URL points at a remote or reverse-proxied endpoint (e.g. https://hbot.example.com:8043), the MCP subprocess still hit 127.0.0.1: and failed. Use WEB_URL directly so scheme, host, and port are all respected.

call_main_api built the URL as `http://127.0.0.1:{WEB_PORT}/...`, ignoring
the host and scheme from WEB_URL. When WEB_URL points at a remote or
reverse-proxied endpoint (e.g. https://hbot.example.com:8043), the MCP
subprocess still hit 127.0.0.1:<port> and failed. Use WEB_URL directly
so scheme, host, and port are all respected.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fengtality

Copy link
Copy Markdown
Contributor

Thanks for tracking this down. Before merging I'd push back a bit — this changes an internal loopback call into an outbound call through the public endpoint, which has security and reliability implications.

The loopback was intentional

call_main_api's docstring notes the MCP server runs as a subprocess of the main Condor process — same host, same network namespace. The main API binds 0.0.0.0:WEB_PORT (main.py), and WEB_PORT is derived from WEB_URL's port when set (utils/config.py). So http://127.0.0.1:{WEB_PORT} already reaches the co-located API in the normal topology.

Could you share the exact deployment where 127.0.0.1 failed? In the shipped subprocess architecture it shouldn't, and that detail decides whether a change is needed and which one.

Security concern (the main one)

call_main_api attaches a user-scoped JWT bearer token (create_jwt(settings.user_id, ...)) that authorizes consult / delegate / agent-lifecycle calls — i.e. it can drive trades. Today that token never leaves loopback. After this change, when WEB_URL is public/reverse-proxied:

  • the JWT egresses to the public endpoint on every call, passing through (and loggable by) the reverse proxy and any hop;
  • WEB_URL may be http:// (the documented default scheme — .env.example shows http://localhost:8088), so the token can travel in cleartext;
  • internal control-plane traffic is now exposed at the public edge — a WAF / Cloudflare / IP allowlist / proxy auth could observe or block it, none of which applied to a loopback call.

Reliability

Every consult/delegate/tick now pays a TLS handshake + proxy round-trip instead of loopback, and depends on the box reaching its own public hostname (hairpin-NAT / split-horizon DNS often break this) and on the proxy being up.

Suggested direction

If there's a real failure, it's most likely the hardcoded http:// scheme (breaks an https endpoint) or WEB_PORT resolving to a public port that differs from the local bind — fix the local target and keep it on loopback:

url = f"http://127.0.0.1:{LOCAL_BIND_PORT}/api/v1{path}"

If the MCP genuinely runs on a different host, gate that behind an explicit, separate var (e.g. CONDOR_INTERNAL_API_URL) that defaults to loopback and requires https — rather than routing internal auth through the public WEB_URL by default.

Minor

  • No tests — if any version lands, add URL-construction cases: no WEB_URL, http, https, trailing slash, non-default port.
  • Worth confirming the MCP subprocess's utils.config resolves the same WEB_URL as the main process (the split branch in config.py is a good thing to pin with a test).

@fengtality

Copy link
Copy Markdown
Contributor

Closing due to the security concerns raised above: this routes internal, co-located loopback traffic — including the user-scoped JWT that can drive trades — out through the public WEB_URL (potentially over cleartext http:// and exposed at the reverse-proxy edge), rather than keeping it on loopback.

Happy to reopen with a revised approach: keep the call on loopback and fix the local target (scheme/port), or, for a genuine split-host setup, add a dedicated https-only CONDOR_INTERNAL_API_URL that defaults to loopback. If you can share the exact deployment where 127.0.0.1 failed, we can land the right fix. Thanks again for flagging the underlying issue.

@fengtality fengtality closed this Jul 20, 2026
@lgrawet

lgrawet commented Jul 21, 2026

Copy link
Copy Markdown
Author

Hi @fengtality,

I should have been more specific on this one. The context is https reverse proxy usage (caddy).

The goal is to achieve :

                                         Host                                     
┌────────────────────────────────────────────────────────────────────────────────┐
│              Caddy                                         Condor              │
│  ┌────────────────────────────────┐              ┌────────────────────────┐    │
│  │ https://hbot.example.com:8043  │ ───────────► │ http://127.0.0.1:8088  │    │
│  └────────────────────────────────┘              └────────────────────────┘    │
└────────────────────────────────────────────────────────────────────────────────┘

Here are the relevant vars from my .env

WEB_URL=https://hbot.example.com:8043
WEB_PORT=8088

I was also planning to send another patch for main API binds 0.0.0.0:WEB_PORT (main.py). To me, binding Condor to 0.0.0.0.0:8088 is a major security issue so I decided to restrict it to 127.0.0.1:8088. I want to prevent any external clear text access to condor. This is a first security measure before iptables.

I have already reported this on Discord some time ago. I made a quick hack in main.py in the meantime, it needs new vars of course.

I could not use the WEB_PORT var because it is derived from WEB_URL if defined (so 8043) and it is potentially used by Caddy for https.

--- main.py.ori 2026-07-03 23:36:23.703814281 +0200
+++ main.py     2026-07-04 07:17:10.055239716 +0200
@@ -716,8 +716,8 @@
     web_app = create_app()
     config = uvicorn.Config(
         web_app,
-        host="0.0.0.0",
-        port=WEB_PORT,
+        host="127.0.0.1",
+        port=8088,
         log_level="info",
         access_log=False,
     )

In this case url = f"http://127.0.0.1:{WEB_PORT}/api/v1{path} in condor_client.py will fail because it will translate to http://127.0.0.1:8043/api/v1{path} which currently not binded to caddy (I could do it) but https, not http.

@lgrawet

lgrawet commented Jul 21, 2026

Copy link
Copy Markdown
Author

I think the solution would be to use the WEB_PORT var when defined and not override it with the port from WEB_URL.
A new BIND_HOST var is also required for main.py that would become

--- main.py.ori 2026-07-03 23:36:23.703814281 +0200
+++ main.py     2026-07-21 10:13:19.255916328 +0200
@@ -716,7 +716,7 @@
     web_app = create_app()
     config = uvicorn.Config(
         web_app,
-        host="0.0.0.0",
+        host=BIND_HOST,
         port=WEB_PORT,
         log_level="info",
         access_log=False,

If I set BIND_HOST=127.0.0.1 instead of 0.0.0.0 and WEB_PORT=8088, url = f"http://127.0.0.1:{WEB_PORT}/api/v1{path} will work

@fengtality fengtality reopened this Jul 22, 2026
@fengtality

Copy link
Copy Markdown
Contributor

@lgrawet can you add more details on your use case and why you think it's needed?

@lgrawet

lgrawet commented Jul 23, 2026

Copy link
Copy Markdown
Author

Hi @fengtality, I've explained it in detail in this comment. Do you need more information?

@fengtality

Copy link
Copy Markdown
Contributor

Hi @fengtality, I've explained it in detail in this comment. Do you need more information?

Yes, why do you need to use it with Caddy? What's the use case?

@lgrawet

lgrawet commented Jul 25, 2026

Copy link
Copy Markdown
Author

To secure it with https and use automated certificate renewal. No clear text access even on local network.

@fengtality

fengtality commented Jul 25, 2026 via email

Copy link
Copy Markdown
Contributor

@lgrawet

lgrawet commented Jul 25, 2026

Copy link
Copy Markdown
Author

Condor network is currently only accessed via OpenVPN tunnel but I'm thinking about allowing Condor access from some limited trusted networks.

mTLS (Mutual TLS) could also be used for this use case thanks to reverse proxy support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants