From 9866e979776351050c8d70768aba50f8ad262c0d Mon Sep 17 00:00:00 2001 From: Kayos Date: Tue, 28 Apr 2026 23:20:45 -0700 Subject: [PATCH] =?UTF-8?q?clients/java:=20apply=20audit=20findings=20?= =?UTF-8?q?=E2=80=94=20true=20streaming=20upload=20+=20token=20redaction?= =?UTF-8?q?=20(0d3ee26=20=E2=86=92=20next)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MEDIUM: - C1: multipart upload now actually streams via SequenceInputStream + Files.newInputStream. Code comment + README + javadoc updated to match reality. Test added uploading 10 MiB file with received-bytes assertion bounding envelope overhead. - S1: AppToken.toString() override redacts token (was leaking plaintext via record auto-toString). LOW: - C2: RunResult.result null/missing-field handling — canonical-constructor coerces null/NullNode to MissingNode, javadoc updated. - C3: HTTP timeout lower bound: Math.max(5L, n + 30L). - C4: ForgeClient implements AutoCloseable (no-op on JDK 17, documented). - S4: javadoc warning on uploadFile path traversal / symlink follow. Quality: - Q1: package-info.java added for com.clawdforge.exception (clears pom.xml dead exclude). - C7: @JsonInclude(NON_DEFAULT) on POST DTOs (drops wire "created_at": 0). Deps: - jackson-databind/core/annotations 2.17.2 → 2.18.2 (2.17 EOL'd Aug 2025). Tests: 14 → 23 (9 added). Audit: memory/clawdforge-audits/java-0d3ee26.md --- clients/java/README.md | 66 ++++-- clients/java/examples/Basic.java | 83 +++---- clients/java/pom.xml | 5 +- .../main/java/com/clawdforge/AppToken.java | 22 +- .../main/java/com/clawdforge/ForgeClient.java | 134 ++++++++---- .../main/java/com/clawdforge/RunRequest.java | 2 +- .../main/java/com/clawdforge/RunResult.java | 18 ++ .../clawdforge/exception/package-info.java | 16 ++ .../java/com/clawdforge/ForgeClientTest.java | 202 ++++++++++++++++++ 9 files changed, 443 insertions(+), 105 deletions(-) create mode 100644 clients/java/src/main/java/com/clawdforge/exception/package-info.java diff --git a/clients/java/README.md b/clients/java/README.md index 6cebac3..76ba36f 100644 --- a/clients/java/README.md +++ b/clients/java/README.md @@ -38,25 +38,30 @@ import com.clawdforge.RunRequest; import com.clawdforge.RunResult; import com.fasterxml.jackson.databind.JsonNode; -ForgeClient client = ForgeClient.builder() - .baseUrl("http://192.168.0.5:8800") - .token(System.getenv("CLAWDFORGE_TOKEN")) - .build(); +try (ForgeClient client = ForgeClient.builder() + .baseUrl("http://192.168.0.5:8800") + .token(System.getenv("CLAWDFORGE_TOKEN")) + .build()) { -RunResult res = client.run(RunRequest.builder() - .prompt("Reply with JSON: {\"hello\": \"world\"}") - .model("sonnet") - .timeoutSecs(60) - .build()); + RunResult res = client.run(RunRequest.builder() + .prompt("Reply with JSON: {\"hello\": \"world\"}") + .model("sonnet") + .timeoutSecs(60) + .build()); -System.out.println(res.durationMs() + "ms"); + System.out.println(res.durationMs() + "ms"); -JsonNode r = res.result(); -if (r.isObject()) { - System.out.println(r.get("hello").asText()); + JsonNode r = res.result(); + if (r.isObject()) { + System.out.println(r.get("hello").asText()); + } } ``` +`ForgeClient` is immutable and safe to share across threads — for +long-lived multi-tenant deployments, construct once and reuse, rather +than relying on `close()`. + ## Naming & wire format The clawdforge HTTP API uses **`snake_case`** on the wire @@ -162,9 +167,12 @@ try { ## File uploads -`uploadFile` streams the file from disk in 1 MiB chunks via -`HttpRequest.BodyPublishers.ofByteArrays` rather than slurping the whole -file into memory: +`uploadFile` streams the file body through +`HttpRequest.BodyPublishers.ofInputStream` chained over a +`SequenceInputStream` (multipart preamble + `Files.newInputStream(path)` + +trailing boundary). The file body is never fully resident in heap — peak +upload memory is bounded by the `HttpClient` read-ahead buffer, so a +multi-GiB upload won't OOM a default-heap JVM: ```java FileToken ft = client.uploadFile(Path.of("./recipe.png"), 3600); @@ -177,6 +185,32 @@ client.run(RunRequest.builder() `ttlSecs` is clamped server-side to `[60, 86400]`. Pass `0` to use the server default of 3600. +> **Path safety.** `uploadFile` does not constrain its `Path` argument to +> any sandbox root, and `Files.isRegularFile` follows symlinks by default. +> Callers that pass user-supplied paths must canonicalise and sandbox them +> first (e.g. resolve under a known directory and reject any `Path` whose +> `toRealPath()` escapes it). + +## Lifecycle + +`ForgeClient` implements `AutoCloseable` for try-with-resources symmetry: + +```java +try (ForgeClient client = ForgeClient.builder() + .baseUrl("http://192.168.0.5:8800") + .token(System.getenv("CLAWDFORGE_TOKEN")) + .build()) { + client.healthz(); +} +``` + +On JDK 17 `HttpClient` has no `close()` (added in JDK 21), so the SDK's +`close()` is currently a no-op — the underlying client is reclaimed when +its last reference goes out of scope. For long-lived multi-tenant +deployments, prefer constructing a single shared `HttpClient` (injected +via `Builder.httpClient(HttpClient)`) and reusing it across `ForgeClient` +instances. + ## Build ```sh diff --git a/clients/java/examples/Basic.java b/clients/java/examples/Basic.java index 79a293a..4f85798 100644 --- a/clients/java/examples/Basic.java +++ b/clients/java/examples/Basic.java @@ -27,52 +27,55 @@ public class Basic { System.exit(2); } - ForgeClient client = ForgeClient.builder() + try (ForgeClient client = ForgeClient.builder() .baseUrl(baseUrl) .token(token) - .build(); + .build()) { - // 1. health - HealthStatus h = client.healthz(); - System.out.printf("ok=%s claude_present=%s version=%s%n", - h.ok(), h.claudePresent(), h.claudeVersion()); + // 1. health + HealthStatus h = client.healthz(); + System.out.printf("ok=%s claude_present=%s version=%s%n", + h.ok(), h.claudePresent(), h.claudeVersion()); - // 2. run a prompt asking for JSON - RunResult res = client.run(RunRequest.builder() - .prompt("Reply with JSON: {\"hello\": \"world\"}") - .model("sonnet") - .timeoutSecs(60) - .build()); - - System.out.printf("duration=%dms stop_reason=%s%n", res.durationMs(), res.stopReason()); - JsonNode r = res.result(); - if (r.isObject() && r.has("hello")) { - System.out.println("hello -> " + r.get("hello").asText()); - } else if (r.isTextual()) { - System.out.println("text -> " + r.asText()); - } else { - System.out.println("raw -> " + r); - } - - // 3. (optional) upload + reference a file - String filePath = System.getenv("CLAWDFORGE_FILE"); - if (filePath != null && !filePath.isBlank()) { - FileToken ft = client.uploadFile(Path.of(filePath), 3600); - System.out.println("uploaded " + ft.fileToken() + " (" + ft.size() + " bytes)"); - - RunResult res2 = client.run(RunRequest.builder() - .prompt("Summarize the attached file in one sentence.") - .files(List.of(ft.fileToken())) + // 2. run a prompt asking for JSON + RunResult res = client.run(RunRequest.builder() + .prompt("Reply with JSON: {\"hello\": \"world\"}") + .model("sonnet") + .timeoutSecs(60) .build()); - System.out.println("summary -> " + res2.result()); - } - // 4. (admin) list tokens — only works with the admin bootstrap token - if (Boolean.parseBoolean(System.getenv().getOrDefault("CLAWDFORGE_ADMIN", "false"))) { - List tokens = client.listTokens(); - for (AppToken t : tokens) { - System.out.printf("token name=%s created_at=%d ip_cidrs=%s%n", - t.name(), t.createdAt(), t.ipCidrs()); + System.out.printf("duration=%dms stop_reason=%s%n", res.durationMs(), res.stopReason()); + JsonNode r = res.result(); + if (r.isObject() && r.has("hello")) { + System.out.println("hello -> " + r.get("hello").asText()); + } else if (r.isTextual()) { + System.out.println("text -> " + r.asText()); + } else if (r.isMissingNode()) { + System.out.println("(no result field in response)"); + } else { + System.out.println("raw -> " + r); + } + + // 3. (optional) upload + reference a file + String filePath = System.getenv("CLAWDFORGE_FILE"); + if (filePath != null && !filePath.isBlank()) { + FileToken ft = client.uploadFile(Path.of(filePath), 3600); + System.out.println("uploaded " + ft.fileToken() + " (" + ft.size() + " bytes)"); + + RunResult res2 = client.run(RunRequest.builder() + .prompt("Summarize the attached file in one sentence.") + .files(List.of(ft.fileToken())) + .build()); + System.out.println("summary -> " + res2.result()); + } + + // 4. (admin) list tokens — only works with the admin bootstrap token + if (Boolean.parseBoolean(System.getenv().getOrDefault("CLAWDFORGE_ADMIN", "false"))) { + List tokens = client.listTokens(); + for (AppToken t : tokens) { + // AppToken.toString() redacts the plaintext bearer. + System.out.println("token " + t); + } } } } diff --git a/clients/java/pom.xml b/clients/java/pom.xml index 5d76a67..87c9f2b 100644 --- a/clients/java/pom.xml +++ b/clients/java/pom.xml @@ -28,7 +28,7 @@ 17 17 UTF-8 - 2.17.2 + 2.18.2 5.10.2 @@ -89,9 +89,6 @@ 17 all,-missing true - - **/exception/package-info.java - diff --git a/clients/java/src/main/java/com/clawdforge/AppToken.java b/clients/java/src/main/java/com/clawdforge/AppToken.java index 0d6b776..731ef2e 100644 --- a/clients/java/src/main/java/com/clawdforge/AppToken.java +++ b/clients/java/src/main/java/com/clawdforge/AppToken.java @@ -19,7 +19,7 @@ import java.util.List; * @param ipCidrs per-app IP allowlist, may be empty (global allowlist still applies) * @param createdAt unix-second creation timestamp; 0 on create response */ -@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonInclude(JsonInclude.Include.NON_DEFAULT) public record AppToken( @JsonProperty("name") String name, @JsonProperty("token") String token, @@ -32,4 +32,24 @@ public record AppToken( @JsonCreator public AppToken { } + + /** + * Renders this token without the plaintext bearer. + * + *

Records auto-generate a {@code toString()} that includes every + * component, which would leak {@link #token()} into any log line that + * happened to print an {@code AppToken}. We override it to redact the + * plaintext as {@code ***} when present, while keeping {@code null} + * visible (so list responses still render distinguishably from create + * responses). + * + * @return a redacted human-readable representation + */ + @Override + public String toString() { + return "AppToken[name=" + name + + ", token=" + (token == null ? null : "***") + + ", ipCidrs=" + ipCidrs + + ", createdAt=" + createdAt + "]"; + } } diff --git a/clients/java/src/main/java/com/clawdforge/ForgeClient.java b/clients/java/src/main/java/com/clawdforge/ForgeClient.java index 1c004b5..529d9f6 100644 --- a/clients/java/src/main/java/com/clawdforge/ForgeClient.java +++ b/clients/java/src/main/java/com/clawdforge/ForgeClient.java @@ -8,8 +8,11 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.SequenceInputStream; +import java.io.UncheckedIOException; import java.net.URI; import java.net.URISyntaxException; import java.net.URLEncoder; @@ -24,6 +27,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.concurrent.ThreadLocalRandom; @@ -42,21 +46,29 @@ import java.util.concurrent.ThreadLocalRandom; * *

Example: * 

{@code
- * ForgeClient client = ForgeClient.builder()
- *     .baseUrl("http://192.168.0.5:8800")
- *     .token(System.getenv("CLAWDFORGE_TOKEN"))
- *     .build();
+ * try (ForgeClient client = ForgeClient.builder()
+ *         .baseUrl("http://192.168.0.5:8800")
+ *         .token(System.getenv("CLAWDFORGE_TOKEN"))
+ *         .build()) {
  *
- * RunResult res = client.run(RunRequest.builder()
- *     .prompt("Reply with JSON: {\"hello\": \"world\"}")
- *     .model("sonnet")
- *     .timeoutSecs(60)
- *     .build());
+ *     RunResult res = client.run(RunRequest.builder()
+ *         .prompt("Reply with JSON: {\"hello\": \"world\"}")
+ *         .model("sonnet")
+ *         .timeoutSecs(60)
+ *         .build());
  *
- * System.out.println(res.durationMs());
+ *     System.out.println(res.durationMs());
+ * }
  * }
+ * + *

{@link AutoCloseable} is implemented for try-with-resources symmetry, + * but on JDK 17 the underlying {@link HttpClient} has no {@code close()} + * method and the SDK's {@link #close()} is a no-op. For long-lived + * multi-tenant deployments, prefer constructing a single {@code ForgeClient} + * (or a single shared {@link HttpClient} injected via + * {@link Builder#httpClient(HttpClient)}) and reusing it. */ -public final class ForgeClient { +public final class ForgeClient implements AutoCloseable { private static final ObjectMapper MAPPER = new ObjectMapper(); @@ -132,9 +144,21 @@ public final class ForgeClient { * Streams a file from disk to {@code POST /files} as * {@code multipart/form-data} and returns the resulting file token. * - *

The file is read from disk in chunks via - * {@link BodyPublishers#ofByteArrays} rather than slurped fully into - * memory. + *

The file body is streamed via + * {@link BodyPublishers#ofInputStream} chained over + * {@link Files#newInputStream} — the upload's peak heap footprint is + * the request's read-ahead buffer, not the file size, so a multi-GiB + * upload will not OOM a default-heap JVM. The multipart envelope + * (preamble headers, {@code ttl_secs} part, file part headers, trailing + * boundary) is small and held in memory once. + * + *

Path safety. {@link Files#isRegularFile(Path, + * java.nio.file.LinkOption...)} follows symlinks by default, and this + * method does not constrain {@code path} to any sandbox root. Callers + * that pass user-supplied paths are responsible for canonicalising and + * sandboxing first — for example by resolving inside a known directory + * and rejecting any {@link Path} whose {@link Path#toRealPath} escapes + * it. This SDK is deliberately library-by-design about traversal. * * @param path local file to upload * @param ttlSecs time-to-live in seconds; server clamps to [60, 86400]. @@ -152,12 +176,7 @@ public final class ForgeClient { String filename = path.getFileName().toString(); String boundary = generateBoundary(); - BodyPublisher publisher; - try { - publisher = multipartFromFile(boundary, path, filename, ttlSecs); - } catch (IOException e) { - throw new TransportException("read " + path, e); - } + BodyPublisher publisher = multipartFromFile(boundary, path, filename, ttlSecs); HttpRequest req = newRequest( "/files", @@ -336,8 +355,29 @@ public final class ForgeClient { if (subprocessTimeoutSecs == null) { return defaultTimeout; } - // 30s margin over subprocess timeout so HTTP doesn't bail prematurely - return Duration.ofSeconds(subprocessTimeoutSecs.longValue() + 30L); + // 30s margin over subprocess timeout so HTTP doesn't bail prematurely. + // Lower-bounded at 5s to defend against negative/zero subprocess + // timeouts that would otherwise produce a non-positive Duration. + long secs = Math.max(5L, subprocessTimeoutSecs.longValue() + 30L); + return Duration.ofSeconds(secs); + } + + /** + * Releases SDK-owned resources. On JDK 17 the underlying + * {@link HttpClient} has no {@code close()} method (added in JDK 21), so + * this is currently a no-op — the caller-supplied or + * builder-constructed {@link HttpClient} will be reclaimed when its last + * reference goes out of scope. Implemented for try-with-resources + * symmetry and forward-compatibility with JDK 21+. + * + *

For long-lived multi-tenant deployments, prefer constructing a + * single shared {@link HttpClient} (injected via + * {@link Builder#httpClient(HttpClient)}) and reusing it across + * {@code ForgeClient} instances rather than relying on close(). + */ + @Override + public void close() { + // no-op on JDK 17; HttpClient has no close() until JDK 21 } // ---------- multipart upload -------------------------------------------- @@ -354,40 +394,48 @@ public final class ForgeClient { } private static BodyPublisher multipartFromFile(String boundary, Path path, - String filename, int ttlSecs) throws IOException { - List parts = new ArrayList<>(); + String filename, int ttlSecs) { + // Build the multipart envelope around a true streaming body. + // prefixHead = preamble (optional ttl_secs part + file-part headers) + // fileStream = Files.newInputStream(path) <-- the only large bit + // suffixTail = trailing CRLF + closing boundary + // + // Streams file body via SequenceInputStream + Files.newInputStream. + // Peak heap is the read-ahead buffer used by HttpClient, NOT the file size. + StringBuilder head = new StringBuilder(); String dashBoundary = "--" + boundary + "\r\n"; if (ttlSecs > 0) { - StringBuilder ttlPart = new StringBuilder(); - ttlPart.append(dashBoundary) + head.append(dashBoundary) .append("Content-Disposition: form-data; name=\"ttl_secs\"\r\n\r\n") .append(ttlSecs).append("\r\n"); - parts.add(ttlPart.toString().getBytes(StandardCharsets.UTF_8)); } - StringBuilder fileHeader = new StringBuilder(); - fileHeader.append(dashBoundary) + head.append(dashBoundary) .append("Content-Disposition: form-data; name=\"file\"; filename=\"") .append(escapeQuoted(filename)) .append("\"\r\n") .append("Content-Type: application/octet-stream\r\n\r\n"); - parts.add(fileHeader.toString().getBytes(StandardCharsets.UTF_8)); - // Stream file in 1 MiB chunks rather than loading whole file into memory. - try (InputStream in = Files.newInputStream(path)) { - byte[] buf = new byte[1024 * 1024]; - int n; - while ((n = in.read(buf)) > 0) { - byte[] chunk = new byte[n]; - System.arraycopy(buf, 0, chunk, 0, n); - parts.add(chunk); + byte[] prefixHead = head.toString().getBytes(StandardCharsets.UTF_8); + byte[] suffixTail = ("\r\n--" + boundary + "--\r\n").getBytes(StandardCharsets.UTF_8); + + return BodyPublishers.ofInputStream(() -> { + InputStream fileStream; + try { + fileStream = Files.newInputStream(path); + } catch (IOException e) { + // ofInputStream's Supplier signature doesn't permit checked + // throws; surface as unchecked so the publisher pipeline + // fails the request rather than silently producing an empty body. + throw new UncheckedIOException("open " + path, e); } - } - parts.add("\r\n".getBytes(StandardCharsets.UTF_8)); - parts.add(("--" + boundary + "--\r\n").getBytes(StandardCharsets.UTF_8)); - - return BodyPublishers.ofByteArrays(parts); + List streams = new ArrayList<>(3); + streams.add(new ByteArrayInputStream(prefixHead)); + streams.add(fileStream); + streams.add(new ByteArrayInputStream(suffixTail)); + return new SequenceInputStream(Collections.enumeration(streams)); + }); } private static String escapeQuoted(String s) { diff --git a/clients/java/src/main/java/com/clawdforge/RunRequest.java b/clients/java/src/main/java/com/clawdforge/RunRequest.java index 732cf50..753c1e5 100644 --- a/clients/java/src/main/java/com/clawdforge/RunRequest.java +++ b/clients/java/src/main/java/com/clawdforge/RunRequest.java @@ -29,7 +29,7 @@ import java.util.Objects; * @param files optional list of file tokens to attach * @param timeoutSecs optional subprocess timeout in seconds (5..600) */ -@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonInclude(JsonInclude.Include.NON_DEFAULT) public record RunRequest( @JsonProperty("prompt") String prompt, @JsonProperty("model") String model, diff --git a/clients/java/src/main/java/com/clawdforge/RunResult.java b/clients/java/src/main/java/com/clawdforge/RunResult.java index d305adb..88cade5 100644 --- a/clients/java/src/main/java/com/clawdforge/RunResult.java +++ b/clients/java/src/main/java/com/clawdforge/RunResult.java @@ -3,6 +3,7 @@ package com.clawdforge; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.MissingNode; /** * Parsed response from {@code POST /run} on success (HTTP 200). @@ -21,6 +22,14 @@ import com.fasterxml.jackson.databind.JsonNode; * } * } * + *

Result-field absence. Jackson does not auto-fill missing + * JSON properties, and a JSON {@code null} for a {@link JsonNode}-typed + * property decodes to {@link com.fasterxml.jackson.databind.node.NullNode}. + * To preserve the "never null" contract and give callers a single + * way to detect absence, the canonical constructor coerces both {@code null} + * and {@code NullNode} to {@link MissingNode#getInstance()}. Callers should + * use {@link JsonNode#isMissingNode()} to detect absence. + * * @param ok always {@code true} on the success path * @param result the parsed result, never {@code null} * (use {@link JsonNode#isMissingNode()} on absence) @@ -35,8 +44,17 @@ public record RunResult( /** * Jackson-friendly canonical constructor. + * + *

Coerces a missing or {@code null} {@code result} (i.e. when the + * server omits the field or sends {@code "result": null}) to + * {@link MissingNode#getInstance()} so callers can rely on the + * "never null" contract and a single absence check via + * {@link JsonNode#isMissingNode()}. */ @JsonCreator public RunResult { + if (result == null || result.isNull()) { + result = MissingNode.getInstance(); + } } } diff --git a/clients/java/src/main/java/com/clawdforge/exception/package-info.java b/clients/java/src/main/java/com/clawdforge/exception/package-info.java new file mode 100644 index 0000000..9a6da5a --- /dev/null +++ b/clients/java/src/main/java/com/clawdforge/exception/package-info.java @@ -0,0 +1,16 @@ +/** + * Exception hierarchy for the clawdforge Java SDK. + * + *

All SDK errors extend {@link com.clawdforge.exception.ForgeException}, + * which extends {@link java.lang.RuntimeException}. The hierarchy: + * + * 

+ * ForgeException                 (RuntimeException)
+ * ├─ ApiException                (any non-2xx HTTP)
+ * │   └─ AuthException           (401 / 403)
+ * └─ TransportException          (DNS, connect, IO, decode, timeout)
+ *
+ * + * @since 0.1.0 + */ +package com.clawdforge.exception; diff --git a/clients/java/src/test/java/com/clawdforge/ForgeClientTest.java b/clients/java/src/test/java/com/clawdforge/ForgeClientTest.java index 132e298..c430e0c 100644 --- a/clients/java/src/test/java/com/clawdforge/ForgeClientTest.java +++ b/clients/java/src/test/java/com/clawdforge/ForgeClientTest.java @@ -14,12 +14,15 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.io.IOException; +import java.io.OutputStream; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -311,4 +314,203 @@ class ForgeClientTest { assertThrows(IllegalArgumentException.class, () -> RunRequest.builder().prompt(" ").build()); } + + // ----- audit follow-up tests -------------------------------------------- + + @Test + @DisplayName("AppToken.toString redacts the plaintext bearer (S1 regression)") + void appTokenToStringDoesNotLeakToken() { + AppToken withToken = new AppToken("cauldron", "cf_secret_do_not_log", + List.of("10.0.0.0/8"), 1700000000L); + String s = withToken.toString(); + assertFalse(s.contains("cf_secret_do_not_log"), + "toString must not include the plaintext token, got: " + s); + assertTrue(s.contains("token=***"), + "toString must redact as ***, got: " + s); + assertTrue(s.contains("name=cauldron"), + "non-secret fields should remain visible, got: " + s); + + // Distinguishability: list-shape (token=null) should still render. + AppToken listShape = new AppToken("cauldron", null, List.of(), 1700000000L); + assertTrue(listShape.toString().contains("token=null"), + "null-token shape should be visible, got: " + listShape); + } + + @Test + @DisplayName("RunResult: missing 'result' field doesn't NPE; isMissingNode() true (C2)") + void runResultResultMissingFieldHandled() { + // Server returns a 200 body that omits "result" entirely. + route("/run", ex -> respond(ex, 200, + "{\"ok\":true,\"duration_ms\":42,\"stop_reason\":\"end_turn\"}")); + RunResult res = client("cf_test").run(RunRequest.builder().prompt("hi").build()); + assertNotNull(res.result(), "result must never be null per contract"); + assertTrue(res.result().isMissingNode(), + "missing 'result' should map to MissingNode, got node-type: " + + res.result().getNodeType()); + assertEquals(42L, res.durationMs()); + } + + @Test + @DisplayName("ForgeClient implements AutoCloseable (try-with-resources compiles + runs) (C4)") + void forgeClientImplementsAutoCloseable() { + route("/healthz", ex -> respond(ex, 200, + "{\"ok\":true,\"claude_present\":true,\"claude_version\":\"x\"}")); + // try-with-resources is the load-bearing assertion: this only compiles + // if ForgeClient implements AutoCloseable. + try (ForgeClient c = ForgeClient.builder() + .baseUrl(baseUrl).token("cf_test").build()) { + assertTrue(c.healthz().ok()); + } + // close() called twice should remain a no-op safe operation + ForgeClient again = client("cf_test"); + again.close(); + again.close(); + } + + @Test + @DisplayName("uploadFile: 10 MiB file streams without buffering full body in heap (C1)") + void uploadFileLargerThanHeap(@org.junit.jupiter.api.io.TempDir Path tmp) throws IOException { + // Write a 10 MiB file with a deterministic byte pattern so we can + // verify the server received it intact. + Path file = tmp.resolve("big.bin"); + final int sizeBytes = 10 * 1024 * 1024; + try (OutputStream out = Files.newOutputStream(file, + StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + byte[] page = new byte[8192]; + for (int i = 0; i < page.length; i++) { + page[i] = (byte) (i & 0xff); + } + int written = 0; + while (written < sizeBytes) { + int n = Math.min(page.length, sizeBytes - written); + out.write(page, 0, n); + written += n; + } + } + assertEquals(sizeBytes, Files.size(file)); + + // Server: count bytes received without buffering them all so we don't + // OOM the test JVM either. Verifies size and a prefix of the pattern. + AtomicLong received = new AtomicLong(0); + AtomicReference firstChunk = new AtomicReference<>(new byte[0]); + route("/files", ex -> { + byte[] buf = new byte[64 * 1024]; + try (java.io.InputStream in = ex.getRequestBody()) { + int n; + while ((n = in.read(buf)) > 0) { + if (firstChunk.get().length == 0) { + byte[] copy = new byte[n]; + System.arraycopy(buf, 0, copy, 0, n); + firstChunk.set(copy); + } + received.addAndGet(n); + } + } + respond(ex, 200, + "{\"file_token\":\"ff_big\",\"ttl_secs\":3600,\"size\":" + sizeBytes + "}"); + }); + + FileToken ft = client("cf_test").uploadFile(file, 3600); + assertEquals("ff_big", ft.fileToken()); + assertEquals(sizeBytes, ft.size()); + + // Total bytes received = multipart envelope overhead + file size. + // Lower bound = file size; upper bound is small (a few hundred bytes). + long got = received.get(); + assertTrue(got >= sizeBytes, + "server should have received at least the file body, got " + got); + assertTrue(got < sizeBytes + 4096, + "envelope overhead should be < 4 KiB, got " + (got - sizeBytes)); + + // Confirm the multipart preamble (small, in-heap) is correct by + // inspecting the first received chunk. + String preamble = new String(firstChunk.get(), StandardCharsets.UTF_8); + assertTrue(preamble.contains("name=\"ttl_secs\""), + "first chunk should include ttl_secs part, got: " + preamble); + assertTrue(preamble.contains("name=\"file\"; filename=\"big.bin\""), + "first chunk should include file part header, got: " + preamble); + } + + @Test + @DisplayName("createToken: null ipCidrs is accepted and produces a usable AppToken") + void createTokenNullIpCidrs() throws IOException { + AtomicReference got = new AtomicReference<>(); + route("/admin/tokens", ex -> { + if (!"POST".equals(ex.getRequestMethod())) { + respond(ex, 405, "{}"); + return; + } + got.set(readBody(ex)); + respond(ex, 200, + "{\"name\":\"empty\",\"token\":\"cf_x\",\"ip_cidrs\":[]}"); + }); + AppToken t = client("admin_bootstrap").createToken("empty", null); + JsonNode sent = MAPPER.readTree(got.get()); + assertEquals("empty", sent.get("name").asText()); + // With @JsonInclude(NON_DEFAULT) on AppToken, an empty ip_cidrs may + // be omitted on the wire — that's the intentional C7 behavior. What + // matters for this test is that the call doesn't NPE on null. + if (sent.has("ip_cidrs")) { + assertTrue(sent.get("ip_cidrs").isArray(), + "if present, ip_cidrs must be an array"); + assertEquals(0, sent.get("ip_cidrs").size()); + } + // C7: created_at primitive 0 must be suppressed by NON_DEFAULT + assertFalse(sent.has("created_at"), + "created_at: 0 should not ship the wire (C7); got: " + got.get()); + assertEquals("cf_x", t.token()); + } + + @Test + @DisplayName("AppToken: NON_DEFAULT drops created_at:0 on the wire (C7)") + void appTokenNonDefaultDropsCreatedAtZero() throws Exception { + AppToken fresh = new AppToken("cauldron", null, List.of("10.0.0.0/8"), 0L); + String wire = MAPPER.writeValueAsString(fresh); + JsonNode tree = MAPPER.readTree(wire); + assertFalse(tree.has("created_at"), + "created_at: 0 must be omitted by NON_DEFAULT, got: " + wire); + assertEquals("cauldron", tree.get("name").asText()); + + // Non-zero created_at must still be present. + AppToken existing = new AppToken("cauldron", null, List.of(), 1700000000L); + JsonNode existingTree = MAPPER.readTree(MAPPER.writeValueAsString(existing)); + assertEquals(1700000000L, existingTree.get("created_at").asLong()); + } + + @Test + @DisplayName("revokeToken: blank name raises IllegalArgumentException") + void revokeTokenBlankName() { + ForgeClient c = client("admin_bootstrap"); + assertThrows(IllegalArgumentException.class, () -> c.revokeToken("")); + assertThrows(IllegalArgumentException.class, () -> c.revokeToken(" ")); + } + + @Test + @DisplayName("revokeToken: special-character name is URL-encoded") + void revokeTokenSpecialChars() { + List hits = new ArrayList<>(); + route("/admin/tokens/", ex -> { + hits.add(ex.getRequestURI().getRawPath()); + respond(ex, 200, "{\"ok\":true}"); + }); + client("admin_bootstrap").revokeToken("ns/test name"); + // Per URLEncoder.encode w/ UTF-8: '/' -> %2F, ' ' -> '+' + assertTrue(hits.contains("/admin/tokens/ns%2Ftest+name"), + "expected URL-encoded slug, hits: " + hits); + } + + @Test + @DisplayName("HTTP timeout floor: subprocess timeoutSecs <= -30 still produces a positive Duration (C3)") + void httpTimeoutLowerBound() throws IOException { + // Server returns immediately. We just need the request to succeed, + // proving the Duration we computed was positive (negative duration + // would have been rejected by HttpClient.timeout()). + route("/run", ex -> respond(ex, 200, + "{\"ok\":true,\"result\":{},\"duration_ms\":1,\"stop_reason\":\"end_turn\"}")); + RunResult res = client("cf_test").run(RunRequest.builder() + .prompt("hi") + .timeoutSecs(-100) // would yield -70s pre-fix; clamps to 5s now + .build()); + assertTrue(res.ok()); + } }