diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a946f9..b2d68bdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,20 +7,17 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] -- **MCP registry writes do not follow a planted `.tmp` symlink.** - `save_server_registry` opened `.tmp` with `O_CREAT|O_TRUNC`, so - a symlink at that staging name redirected the write (including - registry `env` secrets) into the link target before `rename` replaced - only the symlink. Open the staging file with `O_NOFOLLOW` / `O_EXCL` - after `unlink` (which does not follow). Dest-symlink `rename` already - replaced the link, not its target. -- **`/schedule` calendar dates.** `on YYYY-MM-DD` now rejects impossible -- **MCP string JSON-RPC ids.** `parse_response` now accepts a decimal-string +### 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. - **Reconcile rollback no longer wipes the workspace on a failed restore.** - **LaTeX math recursion depth.** `latex_math_to_plain` now stops converting - **Secret key/token writes do not follow a planted dest symlink.** - **Remote `--connect` base URL query/userinfo.** `normalize_api_base_url` -- **MCP registry `env` overrides parent keys.** Subprocess spawn skipped - **Intent reconcile Phase B (JIT ΔS waves).** `POST /v1/reconcile` `mode=ensure` - **A2A unary HTTP errors stay bounded.** `Client::rpc` no longer concatenates - **Remote `--connect` DELETE/PATCH body cap.** Conversation delete and @@ -28,6 +25,10 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). - **Remote TUI: recoverable SSE `error` is not a failed turn.** `RemoteSseTurnConsumer::finish` copied accumulated `error` event text even when the terminal `done` event had `ok: true` (e.g. catalog skip of a stored agent whose JSON failed validation). `done` is authoritative: success clears the result error; failure still prefers `done.error` and falls back to prior `error` events when that field is empty. - **Unreadable TUI sessions are not empty.** `session_json_is_empty` no - **Advise-gate cancel is not a bad bearer.** `POST /v1/advise/gate` +- **MCP string JSON-RPC ids.** `parse_response` now accepts a decimal-string +- **MCP registry writes do not follow a planted `.tmp` symlink.** +- **`/schedule` calendar dates.** `on YYYY-MM-DD` now rejects impossible +- **MCP registry `env` overrides parent keys.** Subprocess spawn skipped ## [0.13.7] — 2026-09-21 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 // ---------------------------------------------------------------------------