fix/(mcp/condor): honor BIND_HOST:BIND_PORT instead of hard-coded 127.0.0.1:WEB_PORT - #160
fix/(mcp/condor): honor BIND_HOST:BIND_PORT instead of hard-coded 127.0.0.1:WEB_PORT#160lgrawet wants to merge 9 commits into
Conversation
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>
|
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
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)
ReliabilityEvery 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 directionIf there's a real failure, it's most likely the hardcoded 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. Minor
|
|
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 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 |
|
Hi @fengtality, I should have been more specific on this one. The context is https reverse proxy usage (caddy). The goal is to achieve : Here are the relevant vars from my .env 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. In this case |
|
I think the solution would be to use the WEB_PORT var when defined and not override it with the port from WEB_URL. If I set BIND_HOST=127.0.0.1 instead of 0.0.0.0 and WEB_PORT=8088, |
|
@lgrawet can you add more details on your use case and why you think it's needed? |
|
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? |
|
To secure it with https and use automated certificate renewal. No clear text access even on local network. |
|
Are you hosting condor for external access?
…On Fri, Jul 24, 2026 at 11:00 PM Laurent Grawet ***@***.***> wrote:
*lgrawet* left a comment (hummingbot/condor#160)
<#160 (comment)>
To secure it with https and use automated certificate renewal. No clear
text access even on local network.
—
Reply to this email directly, view it on GitHub
<#160?email_source=notifications&email_token=AANWHVVEFBZ7TZRK7HRUQRL5GREJBA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBXG4ZDENJXGEYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5077225710>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANWHVVNYCUMLWJWVEHCHXL5GREJBAVCNFSNUABFKJSXA33TNF2G64TZHM3TGNRXGQZTOMJYHNEXG43VMU5TIOJSHAYTAOBYGU3KC5QC>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
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. |
|
@lgrawet Have you checked this against a Tailscale deployment? I think this PR might break it |
….0.0.1" This reverts commit 63c7702.
…D_PORT and honor it in condor_client.py
Greptile SummaryThe PR separates Condor’s externally advertised web endpoint from its internal listener and updates MCP requests to target the listener safely.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| utils/config.py | Introduces independent bind settings while retaining WEB_PORT as the default bind port. |
| main.py | Configures uvicorn with the new bind address and port. |
| mcp_servers/condor/condor_client.py | Targets the configured local listener and correctly normalizes wildcard and IPv6 bind addresses. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
ENV[Environment configuration] --> CFG[utils.config]
CFG -->|BIND_HOST and BIND_PORT| UV[Uvicorn listener]
CFG -->|BIND_HOST and BIND_PORT| NORM[MCP host normalization]
NORM -->|Wildcard to loopback; IPv6 bracketed| API[Main-process API URL]
CFG -->|WEB_URL and WEB_PORT| PUBLIC[Externally advertised dashboard URL]
Reviews (5): Last reviewed commit: "fix(mcp): prevent requests to 0.0.0.0 an..." | Re-trigger Greptile
|
I reverted the commit and made a new proposal/commit based on my comment which doesn't alter the default behavior. |
|
@lgrawet Can we set it so that |
@david-hummingbot Here it is + two more fixes |
call_main_api built the URL as
http://127.0.0.1:{WEB_PORT}/.... When WEB_URL/WEB_PORT points at a remote or reverse-proxied endpoint (e.g. https://hbot.example.com:8043), the local MCP subprocess still has to hit http://{BIND_HOST}:{BIND_PORT} (local bind).