clients/java: apply audit findings — true streaming upload + token redaction (0d3ee26 → next)
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
This commit is contained in:
parent
7745c5eb5c
commit
9866e97977
9 changed files with 443 additions and 105 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<AppToken> 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<AppToken> tokens = client.listTokens();
|
||||
for (AppToken t : tokens) {
|
||||
// AppToken.toString() redacts the plaintext bearer.
|
||||
System.out.println("token " + t);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@
|
|||
<maven.compiler.source>17</maven.compiler.source>
|
||||
<maven.compiler.target>17</maven.compiler.target>
|
||||
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
|
||||
<jackson.version>2.17.2</jackson.version>
|
||||
<jackson.version>2.18.2</jackson.version>
|
||||
<junit.version>5.10.2</junit.version>
|
||||
</properties>
|
||||
|
||||
|
|
@ -89,9 +89,6 @@
|
|||
<source>17</source>
|
||||
<doclint>all,-missing</doclint>
|
||||
<quiet>true</quiet>
|
||||
<sourceFileExcludes>
|
||||
<exclude>**/exception/package-info.java</exclude>
|
||||
</sourceFileExcludes>
|
||||
</configuration>
|
||||
</plugin>
|
||||
</plugins>
|
||||
|
|
|
|||
|
|
@ -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 <strong>without</strong> the plaintext bearer.
|
||||
*
|
||||
* <p>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 + "]";
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
|||
*
|
||||
* <p>Example:
|
||||
* <pre>{@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());
|
||||
* }
|
||||
* }</pre>
|
||||
*
|
||||
* <p>{@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.
|
||||
*
|
||||
* <p>The file is read from disk in chunks via
|
||||
* {@link BodyPublishers#ofByteArrays} rather than slurped fully into
|
||||
* memory.
|
||||
* <p>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.
|
||||
*
|
||||
* <p><strong>Path safety.</strong> {@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+.
|
||||
*
|
||||
* <p>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<byte[]> 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<InputStream> 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) {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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;
|
|||
* }
|
||||
* }</pre>
|
||||
*
|
||||
* <p><strong>Result-field absence.</strong> 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 <em>and</em> 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.
|
||||
*
|
||||
* <p>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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
/**
|
||||
* Exception hierarchy for the clawdforge Java SDK.
|
||||
*
|
||||
* <p>All SDK errors extend {@link com.clawdforge.exception.ForgeException},
|
||||
* which extends {@link java.lang.RuntimeException}. The hierarchy:
|
||||
*
|
||||
* <pre>
|
||||
* ForgeException (RuntimeException)
|
||||
* ├─ ApiException (any non-2xx HTTP)
|
||||
* │ └─ AuthException (401 / 403)
|
||||
* └─ TransportException (DNS, connect, IO, decode, timeout)
|
||||
* </pre>
|
||||
*
|
||||
* @since 0.1.0
|
||||
*/
|
||||
package com.clawdforge.exception;
|
||||
|
|
@ -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<byte[]> 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<String> 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<String> 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());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue