From 19fe299b3d08d56113178bc9d0c46e59d412e4a0 Mon Sep 17 00:00:00 2001 From: Kayos Date: Tue, 28 Apr 2026 23:41:15 -0700 Subject: [PATCH] =?UTF-8?q?clients/cpp:=20apply=20audit=20findings=20?= =?UTF-8?q?=E2=80=94=20protocol-error=20guard=20+=20libcurl=20redirect=20c?= =?UTF-8?q?lamp=20(bae34a7=20=E2=86=92=20next)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HIGH: - H1: nlohmann::json::exception wrapped as ProtocolError at 5 sites in client.cpp via with_protocol_guard helper. Preserves the documented clawdforge::Error catch-all base contract; nlohmann types never leak into the message (e.what() only). - H2: libcurl MAXREDIRS=5, REDIR_PROTOCOLS_STR="http,https" (CURLOPT_REDIR_PROTOCOLS bitmask fallback for libcurl < 7.85.0), UNRESTRICTED_AUTH=0L. Defense-in-depth on top of libcurl's automatic bearer strip on cross-host redirects (>=7.64.0). MEDIUM: - M1: upload_file resolves the path via std::filesystem::canonical up front. Closes broken-symlink, symlink-loop, and TOCTOU-on-target classes without a doc burden on callers. - M2: README "Linking" section documents the public-ABI nlohmann_json implication. v0.2 wrapper deferred. - M3: README "Threat model" section documents the parse-depth concern on the result field of /run replies. Runtime guard skipped for v0.1 per audit recommendation (low yield, complexity). LOW: - L1: cxx_std_20 → cxx_std_17 in CMakeLists.txt (no C++20-only features in the library source; broader downstream reach). Examples and tests still build via designated initializers (g++ accepts these in C++17 mode). - L2: RunResult struct doc clarifies that missing ok/duration_ms decode to defaults — opt-out forward-compat. - L3: Client class doc clarifies that moved-from instances must not have any non-special-member methods invoked (UB), with explicit callout on base_url() returning an internal reference. Test-only: - cpp-httplib 0.15.3 → 0.20.1. Optional backends (OpenSSL / zlib / brotli / zstd) forced off to keep the dep graph minimal. Test-only, never on the consumer wire path. README "Test deps" section added for transparency. Tests added (12 → 23 cases, 70 → 106 assertions): - protocol_error on malformed response for healthz, run, upload_file, create_token, list_tokens (H1 regression) - redirect_clamp_test (H2 regression — TransportError after 5+ hops) - redirect_protocol_clamp (H2 regression — ftp:// Location rejected) - upload_file_canonicalize: symlink→file works, broken symlink rejected, symlink loop rejected, directory rejected (M1 regression) Verified: - cmake --build build clean (-Wall -Wextra -Wpedantic -Wshadow -Wconversion -Wsign-conversion -Wold-style-cast -Werror) - ctest --output-on-failure all green (Release) - ASan + UBSan: 23/23 cases, 106/106 assertions, zero diagnostics Audit: memory/clawdforge-audits/cpp-bae34a7.md --- clients/cpp/CMakeLists.txt | 18 +- clients/cpp/README.md | 50 +++++- clients/cpp/include/clawdforge/client.hpp | 9 + clients/cpp/include/clawdforge/types.hpp | 7 + clients/cpp/src/client.cpp | 47 +++-- clients/cpp/src/http.cpp | 15 ++ clients/cpp/tests/test_client.cpp | 208 ++++++++++++++++++++++ 7 files changed, 336 insertions(+), 18 deletions(-) diff --git a/clients/cpp/CMakeLists.txt b/clients/cpp/CMakeLists.txt index 9529761..b146b2f 100644 --- a/clients/cpp/CMakeLists.txt +++ b/clients/cpp/CMakeLists.txt @@ -23,7 +23,7 @@ endif() # ---- standard / flags ------------------------------------------------------- -set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) @@ -80,7 +80,7 @@ target_link_libraries(clawdforge CURL::libcurl ) -target_compile_features(clawdforge PUBLIC cxx_std_20) +target_compile_features(clawdforge PUBLIC cxx_std_17) set_target_properties(clawdforge PROPERTIES CXX_VISIBILITY_PRESET hidden @@ -159,12 +159,22 @@ if(CLAWDFORGE_BUILD_TESTS) enable_testing() # cpp-httplib for an in-process mock server. Prefer system / installed copy. + # Test-only — never linked into the shipped library. Bumped from 0.15.3 to + # 0.20.1 for clean dep graphs (0.15.x has several published advisories; + # the SDK never exposes the mock to a network). find_package(httplib QUIET CONFIG) if(NOT httplib_FOUND) - message(STATUS "clawdforge: cpp-httplib not installed, fetching v0.15.3") + message(STATUS "clawdforge: cpp-httplib not installed, fetching v0.20.1") + # The mock server doesn't need TLS / compression — disable optional + # backends so we don't drag in zstd / brotli / openssl find_package + # requirements for a test-only dep. + set(HTTPLIB_USE_OPENSSL_IF_AVAILABLE OFF CACHE INTERNAL "") + set(HTTPLIB_USE_ZLIB_IF_AVAILABLE OFF CACHE INTERNAL "") + set(HTTPLIB_USE_BROTLI_IF_AVAILABLE OFF CACHE INTERNAL "") + set(HTTPLIB_USE_ZSTD_IF_AVAILABLE OFF CACHE INTERNAL "") FetchContent_Declare(cpp_httplib GIT_REPOSITORY https://github.com/yhirose/cpp-httplib.git - GIT_TAG v0.15.3 + GIT_TAG v0.20.1 GIT_SHALLOW TRUE ) FetchContent_MakeAvailable(cpp_httplib) diff --git a/clients/cpp/README.md b/clients/cpp/README.md index 0caa914..f6d4a41 100644 --- a/clients/cpp/README.md +++ b/clients/cpp/README.md @@ -4,7 +4,9 @@ Modern C++ client for the [clawdforge](https://github.com/Sulkta-Coop/clawdforge HTTP API. Wraps `claude -p` subprocess calls behind a bearer-token-gated REST service. -- C++17 minimum (C++20 lets you use designated initializers in examples). +- C++17 minimum. C++20 unlocks designated initializers in caller code (used + in the examples / tests for ergonomics) but the library itself compiles + cleanly at C++17. - libcurl + nlohmann/json. No Boost. - RAII, move-only `Client` (one libcurl handle per instance). - Throwing API; full exception hierarchy under `clawdforge::Error`. @@ -136,6 +138,43 @@ in-memory buffering of the payload. or wrap external accesses in a mutex. Multiple `Client` instances share the process-wide libcurl global state safely (refcounted internally). +## Linking + +`nlohmann_json` is a **public** dependency of the SDK (used in `types.hpp` for +on-wire structs). Consumers therefore compile against and link `nlohmann_json` +even when only calling, say, `healthz()`. If you already vendor a different +version of `nlohmann_json`, you'll want to pin a compatible version (3.10+) to +avoid ODR drift across the boundary. + +This is a known v0.1 limitation. A v0.2 refactor will hide nlohmann behind a +thin `clawdforge::JsonValue` wrapper to drop it from the public ABI. + +`libcurl` is private and not exposed across the public header. + +## Threat model + +The SDK is intended for clawdforge servers that the caller trusts (LAN-deployed, +bearer-gated). With that in mind: + +- The `result` field of a `POST /run` reply is **untrusted user-influenced + data** — it carries Claude's response, which can be shaped by whatever ended + up in the prompt. The SDK parses it as JSON via `nlohmann::json::parse`, + which is recursion-based. A pathologically deep object (thousands of nested + arrays) could push the stack arbitrarily far before the parser bails. The + SDK does **not** currently apply a max-depth guard. If your threat model + includes attacker-controlled prompts, either cap prompt size upstream or + pre-validate the response body before letting the SDK parse it. + +- TLS verification is **on by default**. Only disable via + `ClientOptions::insecure_tls = true` for self-signed LAN-internal certs. + Bearer tokens are passed via `Authorization: Bearer …`; libcurl strips this + on cross-host redirects automatically (≥ 7.64.0), and the SDK additionally + pins `MAXREDIRS=5` and `REDIR_PROTOCOLS=http,https`. + +- Bearer tokens never appear in exception messages, log lines, or + `truncate_for_log` output by design. If you observe one, that's a bug — + please file an issue. + ## Build options | Option | Default | Effect | @@ -155,6 +194,15 @@ ctest --test-dir build --output-on-failure The test suite spins up a `cpp-httplib` mock server in-process — no real clawdforge needed. +### Test deps + +| Dep | Version | Notes | +|---|---|---| +| `cpp-httplib` | `0.20.1` | Test-only; mock binds to `127.0.0.1:0`. Optional backends (OpenSSL / zlib / brotli / zstd) are forced off when fetched. | +| `doctest` | `2.4.11` | Test-only. | + +Test deps are NOT linked into the shipped `clawdforge::clawdforge` target. + ## License MIT. See `LICENSE` at the repo root. diff --git a/clients/cpp/include/clawdforge/client.hpp b/clients/cpp/include/clawdforge/client.hpp index 52beb44..3eb858d 100644 --- a/clients/cpp/include/clawdforge/client.hpp +++ b/clients/cpp/include/clawdforge/client.hpp @@ -54,6 +54,11 @@ struct ClientOptions { /// Move-only: each `Client` owns a libcurl easy handle, which doesn't share /// well between threads. Construct one per worker thread, or guard external /// access with a mutex. +/// +/// Moved-from state: a `Client` that has been moved from holds no resources +/// and **must not** have any of its non-special-member methods invoked. Doing +/// so (including `base_url()`) is undefined behaviour. Re-assign or destroy +/// the moved-from object before further use. class Client { public: explicit Client(ClientOptions opts); @@ -66,6 +71,10 @@ public: Client& operator=(Client&&) noexcept; /// Base URL the client was configured with (trailing slash trimmed). + /// + /// Returns a reference into the `Client`'s internal state; the reference + /// is valid until the `Client` is destroyed or moved from. Do not call + /// on a moved-from `Client` (UB — see class doc). [[nodiscard]] const std::string& base_url() const noexcept; // -- public API -------------------------------------------------------- diff --git a/clients/cpp/include/clawdforge/types.hpp b/clients/cpp/include/clawdforge/types.hpp index f843e5a..1e1cd24 100644 --- a/clients/cpp/include/clawdforge/types.hpp +++ b/clients/cpp/include/clawdforge/types.hpp @@ -52,6 +52,13 @@ struct RunRequest { /// `result` is a `variant` because clawdforge auto-parses the inner `claude` /// reply as JSON when possible (object/array/etc.) and falls back to a raw /// string otherwise. Use `std::get_if(&res.result)` to branch. +/// +/// Forward-compat note: `ok` and `duration_ms` decode to their default values +/// (`false` / `0`) when missing from the wire body — the SDK does not require +/// them. `result` is required; absence throws `ProtocolError`. If a future +/// server omits `ok`, callers will see `RunResult{ok=false}` despite a 2xx +/// HTTP status. This is opt-out forward-compat — branch on the HTTP layer +/// (no exception thrown) rather than on `ok` if you want strict semantics. struct RunResult { bool ok{false}; std::variant result; diff --git a/clients/cpp/src/client.cpp b/clients/cpp/src/client.cpp index ea97733..e431425 100644 --- a/clients/cpp/src/client.cpp +++ b/clients/cpp/src/client.cpp @@ -30,6 +30,19 @@ std::string default_user_agent() { } } +// Wraps any nlohmann/json shape error (out_of_range from `at()`, type_error +// from `get()`, etc.) as a `ProtocolError`. Without this, malformed but +// well-formed-as-JSON server responses bypass the documented `clawdforge::Error` +// catch-all base class. Keep `e.what()` only — never expose nlohmann types. +template +auto with_protocol_guard(Fn&& fn) -> decltype(fn()) { + try { + return fn(); + } catch (const nlohmann::json::exception& e) { + throw ProtocolError(std::string{"malformed response: "} + e.what()); + } +} + [[noreturn]] void throw_for_status(long status, std::string body) { const std::string trimmed = detail::truncate_for_log(body, kErrorBodyMax); if (status == 401 || status == 403) { @@ -120,7 +133,7 @@ HealthzResponse Client::healthz() { throw_for_status(resp.status, std::move(resp.body)); } auto j = parse_json_or_throw(resp.body, resp.status); - return j.get(); + return with_protocol_guard([&] { return j.get(); }); } RunResult Client::run(const RunRequest& body) { @@ -143,7 +156,7 @@ RunResult Client::run(const RunRequest& body) { throw_for_status(resp.status, std::move(resp.body)); } auto parsed = parse_json_or_throw(resp.body, resp.status); - return parsed.get(); + return with_protocol_guard([&] { return parsed.get(); }); } FileToken Client::upload_file(std::string_view path, std::int32_t ttl_secs) { @@ -151,10 +164,16 @@ FileToken Client::upload_file(std::string_view path, std::int32_t ttl_secs) { throw AuthError("Client::upload_file requires `token` in ClientOptions"); } namespace fs = std::filesystem; - const fs::path p{std::string{path}}; + const fs::path raw_path{std::string{path}}; std::error_code ec; - if (!fs::exists(p, ec) || ec) { - throw ProtocolError(std::string{"upload_file: file does not exist: "} + p.string()); + // Resolve symlinks up front. `canonical` requires the path to exist and + // refuses broken / loop symlinks via the error_code overload — closing a + // class of issues (TOCTOU on the symlink target, dangling symlink, loop) + // without a doc burden on callers. + const fs::path p = fs::canonical(raw_path, ec); + if (ec) { + throw ProtocolError(std::string{"upload_file: cannot resolve path: "} + + raw_path.string() + ": " + ec.message()); } if (!fs::is_regular_file(p, ec) || ec) { throw ProtocolError(std::string{"upload_file: not a regular file: "} + p.string()); @@ -182,7 +201,7 @@ FileToken Client::upload_file(std::string_view path, std::int32_t ttl_secs) { throw_for_status(resp.status, std::move(resp.body)); } auto j = parse_json_or_throw(resp.body, resp.status); - return j.get(); + return with_protocol_guard([&] { return j.get(); }); } AppToken Client::create_token(const TokenCreateRequest& body) { @@ -199,7 +218,7 @@ AppToken Client::create_token(const TokenCreateRequest& body) { throw_for_status(resp.status, std::move(resp.body)); } auto parsed = parse_json_or_throw(resp.body, resp.status); - return parsed.get(); + return with_protocol_guard([&] { return parsed.get(); }); } std::vector Client::list_tokens() { @@ -216,12 +235,14 @@ std::vector Client::list_tokens() { if (!parsed.contains("tokens") || !parsed.at("tokens").is_array()) { throw ProtocolError("list_tokens: missing or non-array `tokens`"); } - std::vector out; - out.reserve(parsed["tokens"].size()); - for (const auto& el : parsed["tokens"]) { - out.push_back(el.get()); - } - return out; + return with_protocol_guard([&] { + std::vector out; + out.reserve(parsed["tokens"].size()); + for (const auto& el : parsed["tokens"]) { + out.push_back(el.get()); + } + return out; + }); } void Client::revoke_token(std::string_view name) { diff --git a/clients/cpp/src/http.cpp b/clients/cpp/src/http.cpp index d7ed04f..80da335 100644 --- a/clients/cpp/src/http.cpp +++ b/clients/cpp/src/http.cpp @@ -168,6 +168,21 @@ Response CurlSession::perform(const Request& req) { curl_easy_setopt(easy_, CURLOPT_URL, req.url.c_str()); curl_easy_setopt(easy_, CURLOPT_FOLLOWLOCATION, 1L); + // Clamp redirects: bound the chain length, only allow http(s) on the + // redirect path (default also includes ftp/sftp/scp), and explicitly + // disable cross-host bearer leakage. libcurl >= 7.64.0 already strips + // header-mode auth on cross-host redirects; UNRESTRICTED_AUTH=0 is + // defense-in-depth. + curl_easy_setopt(easy_, CURLOPT_MAXREDIRS, 5L); + // CURLOPT_REDIR_PROTOCOLS_STR added in 7.85.0; older curl uses the + // bitmask form. Both spell the same allowlist: http + https only. +#if LIBCURL_VERSION_NUM >= 0x075500 /* 7.85.0 */ + curl_easy_setopt(easy_, CURLOPT_REDIR_PROTOCOLS_STR, "http,https"); +#else + curl_easy_setopt(easy_, CURLOPT_REDIR_PROTOCOLS, + static_cast(CURLPROTO_HTTP | CURLPROTO_HTTPS)); +#endif + curl_easy_setopt(easy_, CURLOPT_UNRESTRICTED_AUTH, 0L); curl_easy_setopt(easy_, CURLOPT_NOSIGNAL, 1L); // be thread-friendly curl_easy_setopt(easy_, CURLOPT_TIMEOUT, static_cast(timeout_.count())); curl_easy_setopt(easy_, CURLOPT_CONNECTTIMEOUT, static_cast(connect_timeout_.count())); diff --git a/clients/cpp/tests/test_client.cpp b/clients/cpp/tests/test_client.cpp index 91d120d..3a0bc43 100644 --- a/clients/cpp/tests/test_client.cpp +++ b/clients/cpp/tests/test_client.cpp @@ -333,3 +333,211 @@ TEST_CASE("invalid base_url scheme is rejected at construction") { CHECK_THROWS_AS(make_ftp(), cf::ProtocolError); CHECK_THROWS_AS(make_empty(), cf::ProtocolError); } + +// --------------------------------------------------------------------------- +// Regression: malformed JSON bodies must surface as ProtocolError, NOT as +// raw nlohmann::json::exception (which would bypass the documented +// `clawdforge::Error` catch-all base — H1 fix). +// --------------------------------------------------------------------------- + +TEST_CASE("healthz: malformed response surfaces as ProtocolError") { + MockServer mock{[](httplib::Server& s) { + s.Get("/healthz", [](const httplib::Request&, httplib::Response& res) { + // Well-formed JSON, but wrong shape — `claude_version` is a + // number, not a string. Triggers nlohmann type_error during + // from_json, which must be wrapped. + res.set_content(R"({"ok":true,"claude_version":42})", "application/json"); + }); + }}; + cf::Client c{cf::ClientOptions{.base_url = mock.base_url(), .token = "cf_test"}}; + CHECK_THROWS_AS(c.healthz(), cf::ProtocolError); + // Sanity: the catch-all base must also catch. + CHECK_THROWS_AS(c.healthz(), cf::Error); +} + +TEST_CASE("run: malformed response surfaces as ProtocolError") { + MockServer mock{[](httplib::Server& s) { + s.Post("/run", [](const httplib::Request&, httplib::Response& res) { + // Missing required `result` field — `from_json(RunResult)` calls + // `j.at("result")` which throws out_of_range. + res.set_content(R"({"ok":true,"duration_ms":1})", "application/json"); + }); + }}; + cf::Client c{cf::ClientOptions{.base_url = mock.base_url(), .token = "cf_test"}}; + CHECK_THROWS_AS(c.run(cf::RunRequest{.prompt = "hi"}), cf::ProtocolError); +} + +TEST_CASE("upload_file: malformed response surfaces as ProtocolError") { + const auto path = write_temp_file("payload"); + + MockServer mock{[](httplib::Server& s) { + s.Post("/files", [](const httplib::Request&, httplib::Response& res) { + // Missing required `file_token`. + res.set_content(R"({"ttl_secs":120,"size":7})", "application/json"); + }); + }}; + cf::Client c{cf::ClientOptions{.base_url = mock.base_url(), .token = "cf_test"}}; + CHECK_THROWS_AS(c.upload_file(path), cf::ProtocolError); + + std::filesystem::remove(path); +} + +TEST_CASE("create_token: malformed response surfaces as ProtocolError") { + MockServer mock{[](httplib::Server& s) { + s.Post("/admin/tokens", [](const httplib::Request&, httplib::Response& res) { + // Missing required `name` + `token`. + res.set_content(R"({"foo":"bar"})", "application/json"); + }); + }}; + cf::Client c{cf::ClientOptions{.base_url = mock.base_url(), + .token = "cf_test", + .admin_token = "admin_secret"}}; + CHECK_THROWS_AS( + c.create_token(cf::TokenCreateRequest{.name = "x"}), + cf::ProtocolError); +} + +TEST_CASE("list_tokens: malformed entries surface as ProtocolError") { + MockServer mock{[](httplib::Server& s) { + s.Get("/admin/tokens", [](const httplib::Request&, httplib::Response& res) { + // Array is present, but entries are missing `name` (required). + res.set_content(R"({"tokens":[{"ip_cidrs":["10.0.0.0/8"]}]})", + "application/json"); + }); + }}; + cf::Client c{cf::ClientOptions{.base_url = mock.base_url(), + .token = "cf_test", + .admin_token = "admin_secret"}}; + CHECK_THROWS_AS(c.list_tokens(), cf::ProtocolError); +} + +// --------------------------------------------------------------------------- +// Regression: libcurl redirect clamps (H2). Verify MAXREDIRS=5 caps the chain +// and REDIR_PROTOCOLS rejects ftp:// targets. +// --------------------------------------------------------------------------- + +TEST_CASE("redirect_clamp: more than 5 redirects -> TransportError") { + std::atomic hops{0}; + MockServer mock{[&](httplib::Server& s) { + s.Get("/healthz", [](const httplib::Request&, httplib::Response& res) { + res.status = 302; + res.set_header("Location", "/loop0"); + }); + s.Get(R"(/loop(\d+))", [&](const httplib::Request& req, httplib::Response& res) { + hops.fetch_add(1, std::memory_order_relaxed); + const int n = std::stoi(req.matches[1]); + res.status = 302; + res.set_header("Location", "/loop" + std::to_string(n + 1)); + }); + }}; + + cf::Client c{cf::ClientOptions{ + .base_url = mock.base_url(), + .token = "cf_test", + .timeout = std::chrono::seconds{5}, + .connect_timeout = std::chrono::seconds{2}, + }}; + CHECK_THROWS_AS(c.healthz(), cf::TransportError); + // libcurl follows up to MAXREDIRS+1 hops total before giving up. + CHECK(hops.load() <= 6); +} + +TEST_CASE("redirect_protocol_clamp: ftp:// Location is rejected") { + MockServer mock{[](httplib::Server& s) { + s.Get("/healthz", [](const httplib::Request&, httplib::Response& res) { + res.status = 302; + res.set_header("Location", "ftp://example.invalid/data"); + }); + }}; + + cf::Client c{cf::ClientOptions{ + .base_url = mock.base_url(), + .token = "cf_test", + .timeout = std::chrono::seconds{5}, + .connect_timeout = std::chrono::seconds{2}, + }}; + CHECK_THROWS_AS(c.healthz(), cf::TransportError); +} + +// --------------------------------------------------------------------------- +// Regression: upload_file canonicalizes its path argument (M1). A symlink to +// a real file must work; broken / loop symlinks must be rejected up front +// with ProtocolError, never reach libcurl. +// --------------------------------------------------------------------------- + +TEST_CASE("upload_file: symlink to regular file is accepted") { + namespace fs = std::filesystem; + const auto target = write_temp_file("symlinked-payload"); + const auto link = fs::temp_directory_path() / + ("clawdforge-link-" + + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count())); + std::error_code ec; + fs::create_symlink(target, link, ec); + REQUIRE(!ec); + + MockServer mock{[](httplib::Server& s) { + s.Post("/files", [](const httplib::Request& req, httplib::Response& res) { + REQUIRE(req.has_file("file")); + CHECK(req.get_file_value("file").content == "symlinked-payload"); + res.set_content( + R"({"file_token":"ff_link","ttl_secs":120,"size":17})", + "application/json"); + }); + }}; + + cf::Client c{cf::ClientOptions{.base_url = mock.base_url(), .token = "cf_test"}}; + const auto ft = c.upload_file(link.string(), 120); + CHECK(ft.file_token == "ff_link"); + + fs::remove(link); + fs::remove(target); +} + +TEST_CASE("upload_file: broken symlink is rejected with ProtocolError") { + namespace fs = std::filesystem; + const auto link = fs::temp_directory_path() / + ("clawdforge-broken-" + + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count())); + std::error_code ec; + fs::create_symlink("/no/such/path/clawdforge-target", link, ec); + REQUIRE(!ec); + + cf::Client c{cf::ClientOptions{.base_url = "http://127.0.0.1:1", .token = "cf_test"}}; + CHECK_THROWS_AS(c.upload_file(link.string()), cf::ProtocolError); + + fs::remove(link); +} + +TEST_CASE("upload_file: symlink loop is rejected with ProtocolError") { + namespace fs = std::filesystem; + const auto stamp = + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()); + const auto a = fs::temp_directory_path() / ("clawdforge-loopA-" + stamp); + const auto b = fs::temp_directory_path() / ("clawdforge-loopB-" + stamp); + std::error_code ec; + fs::create_symlink(b, a, ec); + REQUIRE(!ec); + fs::create_symlink(a, b, ec); + REQUIRE(!ec); + + cf::Client c{cf::ClientOptions{.base_url = "http://127.0.0.1:1", .token = "cf_test"}}; + CHECK_THROWS_AS(c.upload_file(a.string()), cf::ProtocolError); + + fs::remove(a); + fs::remove(b); +} + +TEST_CASE("upload_file: directory is rejected") { + namespace fs = std::filesystem; + const auto dir = fs::temp_directory_path() / + ("clawdforge-dir-" + + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count())); + std::error_code ec; + fs::create_directory(dir, ec); + REQUIRE(!ec); + + cf::Client c{cf::ClientOptions{.base_url = "http://127.0.0.1:1", .token = "cf_test"}}; + CHECK_THROWS_AS(c.upload_file(dir.string()), cf::ProtocolError); + + fs::remove(dir); +}