From 64472213e964b52654607211220f351fdb596073 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 16 Sep 2026 00:26:45 +0000 Subject: [PATCH] Preflight /fetch URLs with the shared SSRF hostname check. cmd_fetch and cmd_fetch_bytes only blocked private peers at connect time. Metadata hostnames that resolve public, and private/metadata targets reached through HTTP_PROXY (opensocket sees the proxy), skipped the denylist /browse already runs. Call ssrf_preflight_url before libcurl; opensocket still re-checks every redirect connect. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 8 ++++++++ src/commands.cpp | 24 ++++++++++++++++++------ tests/test_commands.cpp | 24 ++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..671f4556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **`/fetch` uses the same SSRF hostname preflight as `/browse`.** + `cmd_fetch` and `cmd_fetch_bytes` already refused private/loopback + peers at connect time, but skipped `ssrf_preflight_url`. Metadata + hostnames that resolve public, and private/metadata URLs reached + through `HTTP_PROXY` (opensocket sees the proxy), were not blocked. + Both paths now run the shared preflight before libcurl. + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/src/commands.cpp b/src/commands.cpp index 72b5e411..79984647 100644 --- a/src/commands.cpp +++ b/src/commands.cpp @@ -1139,6 +1139,17 @@ FetchedResource cmd_fetch_bytes(const std::string& url, int64_t max_bytes) { return r; } + // Same hostname denylist / private-range resolve check /browse uses. + // CURLOPT_OPENSOCKETFUNCTION still re-validates every connect + // (including redirects). Preflight additionally blocks metadata + // hostnames that resolve public, and private/metadata targets that + // would otherwise reach a public HTTP(S)_PROXY (opensocket sees the + // proxy, not the URL host). + if (auto reject = preflight_ssrf_check(url); !reject.empty()) { + r.error = "refused — " + reject + " (SSRF guard)"; + return r; + } + CURL* curl = curl_easy_init(); if (!curl) { r.error = "failed to initialize curl"; return r; } @@ -1271,6 +1282,9 @@ std::string cmd_fetch(const std::string& url) { if (!is_http && !is_https) return "ERR: URL must start with http:// or https://"; + if (auto reject = preflight_ssrf_check(url); !reject.empty()) + return "ERR: refused — " + reject + " (SSRF guard)"; + CURL* curl = curl_easy_init(); if (!curl) return "ERR: failed to initialize curl"; @@ -2276,12 +2290,10 @@ std::string execute_agent_commands(const std::vector& cmds, << "[END BROWSE]\n\n"; cache_result = false; } else if (auto reject = preflight_ssrf_check(url); !reject.empty()) { - // Pre-flight the URL against the same SSRF blocklist - // /fetch enforces. Unlike libcurl-driven fetches, - // /browse hands the URL off to Playwright via MCP, - // which does its own DNS + connect with no hook for - // us to intervene — without this check the SSRF guard - // is bypassed entirely. + // Same ssrf_preflight_url /fetch and cmd_fetch_bytes + // run before libcurl. /browse still needs this because + // Playwright via MCP does its own DNS + connect with + // no opensocket hook. block << "[/browse " << url << "]\n" << "ERR: refused — " << reject << " (SSRF guard)\n" << "[END BROWSE]\n\n"; diff --git a/tests/test_commands.cpp b/tests/test_commands.cpp index 5e741ebc..5319378a 100644 --- a/tests/test_commands.cpp +++ b/tests/test_commands.cpp @@ -205,6 +205,30 @@ TEST_CASE("cmd_fetch accepts http and https") { CHECK(https_result.find("URL must start with") == std::string::npos); } +TEST_CASE("cmd_fetch and cmd_fetch_bytes preflight metadata and private literals") { + // Hostname denylist — no DNS required. + CHECK(cmd_fetch("http://metadata.google.internal/").find("ERR:") == 0); + CHECK(cmd_fetch("http://metadata.google.internal/").find("SSRF") + != std::string::npos); + CHECK(cmd_fetch("http://metadata.google.internal/").find("denylist") + != std::string::npos); + + // Literal blocked ranges — inet_pton, no DNS. + CHECK(cmd_fetch("http://127.0.0.1/").find("ERR:") == 0); + CHECK(cmd_fetch("http://127.0.0.1/").find("SSRF") != std::string::npos); + CHECK(cmd_fetch("http://169.254.169.254/latest/meta-data/").find("ERR:") == 0); + CHECK(cmd_fetch("http://[::1]/").find("ERR:") == 0); + + auto meta = cmd_fetch_bytes("http://metadata.google.internal/", 1024); + CHECK_FALSE(meta.ok); + CHECK(meta.error.find("SSRF") != std::string::npos); + CHECK(meta.error.find("denylist") != std::string::npos); + + auto loopback = cmd_fetch_bytes("http://127.0.0.1/", 1024); + CHECK_FALSE(loopback.ok); + CHECK(loopback.error.find("SSRF") != std::string::npos); +} + // --------------------------------------------------------------------------- // cmd_write — path traversal protection // ---------------------------------------------------------------------------