diff --git a/clients/swift/Examples/Basic/main.swift b/clients/swift/Examples/Basic/main.swift index 9c993ab..e6a4316 100644 --- a/clients/swift/Examples/Basic/main.swift +++ b/clients/swift/Examples/Basic/main.swift @@ -15,9 +15,9 @@ guard let token = ProcessInfo.processInfo.environment["CLAWDFORGE_TOKEN"] else { exit(1) } -let client = ForgeClient(baseURL: baseURL, token: token) - do { + let client = try ForgeClient(baseURL: baseURL, token: token) + // 1. Health check let h = try await client.healthz() print("claude_present: \(h.claudePresent)") diff --git a/clients/swift/README.md b/clients/swift/README.md index 303f8de..a342dfe 100644 --- a/clients/swift/README.md +++ b/clients/swift/README.md @@ -46,7 +46,9 @@ select the `Clawdforge` library product. ```swift import Clawdforge -let client = ForgeClient( +// `baseURL` must be host-only — scheme + host (+ port). A path/query/fragment +// is rejected at construct with `ForgeError.invalidArgument`. +let client = try ForgeClient( baseURL: URL(string: "http://192.168.0.5:8800")!, token: ProcessInfo.processInfo.environment["CLAWDFORGE_TOKEN"]! ) @@ -79,7 +81,7 @@ is thread-safe. ```swift public struct ForgeClient: Sendable { - public init(baseURL: URL, token: String, session: URLSession = .shared) + public init(baseURL: URL, token: String, session: URLSession = .shared) throws public func healthz() async throws -> HealthStatus public func run(_ request: RunRequest) async throws -> RunResult @@ -111,7 +113,7 @@ load-into-`Data()` for the file payload, so multi-megabyte files are fine. Pass the `ADMIN_BOOTSTRAP_TOKEN` as the client's `token` to use these. ```swift -let admin = ForgeClient(baseURL: url, token: adminToken) +let admin = try ForgeClient(baseURL: url, token: adminToken) let new = try await admin.createToken( CreateTokenRequest(name: "myapp", ipCidrs: ["10.0.0.0/8"]) diff --git a/clients/swift/Sources/Clawdforge/ForgeClient.swift b/clients/swift/Sources/Clawdforge/ForgeClient.swift index 1033c8c..fe323d8 100644 --- a/clients/swift/Sources/Clawdforge/ForgeClient.swift +++ b/clients/swift/Sources/Clawdforge/ForgeClient.swift @@ -54,21 +54,38 @@ public struct ForgeClient: Sendable { /// /// - Parameters: /// - baseURL: Root URL of the clawdforge instance. e.g. - /// `URL(string: "http://192.168.0.5:8800")!`. + /// `URL(string: "http://192.168.0.5:8800")!`. **Must be host-only** + /// (scheme + host [+ optional port]). A non-empty path, query, or + /// fragment is rejected with ``ForgeError/invalidArgument(_:)`` — + /// the SDK builds request URLs as `baseURL.absoluteString + "/path"` + /// so e.g. `http://x/api` would silently produce malformed URLs. /// - token: Bearer token (`cf_…` for app endpoints, the admin /// bootstrap token for `/admin/*`). /// - session: Optional `URLSession`. Defaults to `.shared`. Provide /// a custom one for timeouts or for tests using `URLProtocol` /// stubs. + /// - Throws: ``ForgeError/invalidArgument(_:)`` if `baseURL` carries a + /// path, query, or fragment. public init( baseURL: URL, token: String, session: URLSession = .shared - ) { + ) throws { // Trim a trailing slash so path joins are predictable. var s = baseURL.absoluteString while s.hasSuffix("/") { s.removeLast() } - self.baseURL = URL(string: s) ?? baseURL + let trimmed = URL(string: s) ?? baseURL + + // Reject baseURLs that carry their own path/query/fragment. The SDK + // constructs request URLs as `baseURL.absoluteString + "/path"` and + // a non-empty trailing path on baseURL would silently break that. + if !trimmed.path.isEmpty || trimmed.query != nil || trimmed.fragment != nil { + throw ForgeError.invalidArgument( + "baseURL must be host-only (scheme + host [+ port]); got \(baseURL.absoluteString)" + ) + } + + self.baseURL = trimmed self.token = token self.session = session @@ -184,16 +201,42 @@ public struct ForgeClient: Sendable { /// `DELETE /admin/tokens/{name}`. Revoke a token by app name. /// + /// `name` must match the server-side constraint `^[a-z0-9_-]{1,64}$` + /// (lowercase alphanumerics, hyphen, underscore). Anything else — + /// including path-traversal sequences like `foo/../bar` — is rejected + /// client-side with ``ForgeError/invalidArgument(_:)``. This is stricter + /// than `.urlPathAllowed`, which leaves `/`, `+`, `;`, `=`, `,`, `@` + /// unescaped. + /// /// Requires the admin bootstrap token. public func revokeToken(name: String) async throws { guard !name.isEmpty else { throw ForgeError.invalidArgument("revokeToken: name must not be empty") } - let escaped = name.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? name - let req = try makeRequest(method: "DELETE", path: "/admin/tokens/\(escaped)") + guard Self.isValidTokenName(name) else { + throw ForgeError.invalidArgument( + "revokeToken: name must match [a-z0-9_-]{1,64}; got \"\(name)\"" + ) + } + let req = try makeRequest(method: "DELETE", path: "/admin/tokens/\(name)") _ = try await sendVoid(req) } + /// Server-side constraint: `^[a-z0-9_-]{1,64}$`. Cheap manual scan to + /// avoid pulling in `NSRegularExpression` for one tiny check. + static func isValidTokenName(_ name: String) -> Bool { + guard (1...64).contains(name.count) else { return false } + for ch in name.unicodeScalars { + switch ch { + case "a"..."z", "0"..."9", "-", "_": + continue + default: + return false + } + } + return true + } + // MARK: Internals /// Build a `URLRequest` with auth + accept headers. If `jsonBody` is @@ -292,41 +335,60 @@ public struct ForgeClient: Sendable { /// Stream a multipart/form-data body into a temp file. Stays disk-bound: /// reads the source file in 1 MiB chunks rather than slurping it. + /// + /// All line terminators are explicit `\r\n` (CRLF) per RFC 7578. Swift's + /// `"""…"""` multi-line literals use bare LF for source newlines, which + /// would inject a stray `\n` between the headers `\r\n\r\n` separator + /// and the file content — corrupting any binary upload (PNG, PDF, JPEG) + /// because the receiver would treat that `\n` as the first byte of the + /// part body. We use plain string concatenation here so every byte on + /// the wire is auditable. + /// + /// The temp file is created with 0o600 perms — the staged body holds + /// the user's plaintext bearer token via the `Authorization` header on + /// the live request, but we still don't want world-readable copies of + /// arbitrary user uploads sitting in `/tmp`. private func writeMultipartBody( to dest: URL, boundary: String, ttlSecs: Int, fileURL: URL ) throws { - FileManager.default.createFile(atPath: dest.path, contents: nil) + FileManager.default.createFile( + atPath: dest.path, + contents: nil, + attributes: [.posixPermissions: 0o600] + ) guard let out = FileHandle(forWritingAtPath: dest.path) else { throw ForgeError.invalidArgument("cannot open temp upload at \(dest.path)") } defer { try? out.close() } - // ttl_secs field - let ttlPart = """ - --\(boundary)\r - Content-Disposition: form-data; name="ttl_secs"\r - \r - \(ttlSecs)\r + let CRLF = "\r\n" - """ + // ttl_secs field — explicit CRLFs everywhere. + let ttlPart = + "--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"ttl_secs\"" + CRLF + + CRLF + + String(ttlSecs) + CRLF try out.write(contentsOf: Data(ttlPart.utf8)) - // file field header + // file field header — explicit CRLFs everywhere. The blank line + // between headers and body is a single CRLF; together with the CRLF + // that ends `Content-Type:` it forms the required `\r\n\r\n` + // separator. let filename = fileURL.lastPathComponent let mime = mimeType(for: fileURL) - let fileHeader = """ - --\(boundary)\r - Content-Disposition: form-data; name="file"; filename="\(escapeFilename(filename))"\r - Content-Type: \(mime)\r - \r - - """ + let fileHeader = + "--" + boundary + CRLF + + "Content-Disposition: form-data; name=\"file\"; filename=\"" + escapeFilename(filename) + "\"" + CRLF + + "Content-Type: " + mime + CRLF + + CRLF try out.write(contentsOf: Data(fileHeader.utf8)) - // file body — chunked + // file body — chunked. Cooperative cancellation inside the loop so + // a multi-GB upload aborts quickly when the parent Task is cancelled. guard let input = FileHandle(forReadingAtPath: fileURL.path) else { throw ForgeError.invalidArgument("cannot open \(fileURL.path) for reading") } @@ -334,13 +396,15 @@ public struct ForgeClient: Sendable { let chunkSize = 1 * 1024 * 1024 while true { + try Task.checkCancellation() let chunk = input.readData(ofLength: chunkSize) if chunk.isEmpty { break } try out.write(contentsOf: chunk) } - // closing boundary - let trailer = "\r\n--\(boundary)--\r\n" + // closing boundary — leading CRLF terminates the file part body, + // then the dash-boundary-dash sequence + final CRLF. + let trailer = CRLF + "--" + boundary + "--" + CRLF try out.write(contentsOf: Data(trailer.utf8)) } @@ -373,3 +437,17 @@ public struct ForgeClient: Sendable { .replacingOccurrences(of: "\r", with: " ") } } + +// MARK: - Redacted reflection +// +// The default `String(reflecting:)` mirror walks every stored property and +// would surface the bearer token in plain text via `print(client)`, +// `String(describing: client)`, SwiftUI `Text("\(client)")`, and any logging +// framework that calls `String(reflecting:)`. Override to redact. + +extension ForgeClient: CustomStringConvertible, CustomDebugStringConvertible { + public var description: String { + "ForgeClient(baseURL: \(baseURL), token: )" + } + public var debugDescription: String { description } +} diff --git a/clients/swift/Sources/Clawdforge/Models.swift b/clients/swift/Sources/Clawdforge/Models.swift index 12cecc2..e856cdf 100644 --- a/clients/swift/Sources/Clawdforge/Models.swift +++ b/clients/swift/Sources/Clawdforge/Models.swift @@ -38,9 +38,9 @@ public struct HealthStatus: Codable, Sendable, Equatable { /// Body for `POST /run`. /// -/// Only ``prompt`` is required. Fields left `nil` are omitted from the -/// wire payload via custom encoding so the server sees the same shape -/// the Python/Go/Rust clients send. +/// Only ``prompt`` is required. Optional fields are declared as Swift +/// `Optional`s so `JSONEncoder` omits them from the wire payload when `nil`, +/// matching the shape the Python/Go/Rust clients send. public struct RunRequest: Codable, Sendable, Equatable { /// The user prompt to feed to `claude -p`. public let prompt: String @@ -114,6 +114,20 @@ public struct RunFailure: Codable, Sendable, Equatable { public let durationMs: Int? public let stopReason: String? + public init( + ok: Bool, + error: String, + stderr: String? = nil, + durationMs: Int? = nil, + stopReason: String? = nil + ) { + self.ok = ok + self.error = error + self.stderr = stderr + self.durationMs = durationMs + self.stopReason = stopReason + } + private enum CodingKeys: String, CodingKey { case ok case error @@ -225,6 +239,20 @@ public struct AppToken: Codable, Sendable, Equatable { } } +// AppToken redacts the plaintext `token` from `print` / `String(reflecting:)` +// / SwiftUI string interpolation. When `token` is non-nil (the create-token +// response) the default mirror would dump the secret straight to stdout. +extension AppToken: CustomStringConvertible, CustomDebugStringConvertible { + public var description: String { + let redacted = (token == nil) ? "nil" : "" + return "AppToken(name: \(name), token: \(redacted), " + + "ipCidrs: \(ipCidrs ?? []), createdAt: \(createdAt.map(String.init) ?? "nil"), " + + "lastUsed: \(lastUsed.map(String.init) ?? "nil"), " + + "enabled: \(enabled.map(String.init) ?? "nil"))" + } + public var debugDescription: String { description } +} + /// Wire envelope for `GET /admin/tokens`. struct TokenList: Codable { let tokens: [AppToken] @@ -249,6 +277,12 @@ public enum JSONValue: Codable, Sendable, Equatable { case object([String: JSONValue]) case array([JSONValue]) case string(String) + /// JSON number, represented as `Double`. Note: Swift's `Double` is IEEE + /// 754 binary64 with 53 bits of integer precision, so JSON integers + /// larger than `2^53` (≈ 9.007e15) will lose precision on decode. If the + /// server-side `result` ever contains 64-bit identifiers (e.g. snowflake + /// IDs, nanosecond timestamps) embed them as JSON strings rather than + /// numbers — the lossless `.string` case will preserve them verbatim. case number(Double) case bool(Bool) case null diff --git a/clients/swift/Tests/ClawdforgeTests/ForgeClientTests.swift b/clients/swift/Tests/ClawdforgeTests/ForgeClientTests.swift index 2e34ed5..5e8c6af 100644 --- a/clients/swift/Tests/ClawdforgeTests/ForgeClientTests.swift +++ b/clients/swift/Tests/ClawdforgeTests/ForgeClientTests.swift @@ -79,11 +79,11 @@ final class URLProtocolMock: URLProtocol, @unchecked Sendable { // MARK: - Helpers -private func makeClient() -> ForgeClient { +private func makeClient() throws -> ForgeClient { let config = URLSessionConfiguration.ephemeral config.protocolClasses = [URLProtocolMock.self] let session = URLSession(configuration: config) - return ForgeClient( + return try ForgeClient( baseURL: URL(string: "http://forge.test")!, token: "cf_test_abcdef", session: session @@ -346,7 +346,7 @@ final class ForgeClientTests: XCTestCase { } // 12. Cancellation propagates from Task to URLSession - func testCancellationThrows() async { + func testCancellationThrows() async throws { URLProtocolMock.handler = { _ in // Block "forever" so the cancel path is the only way out. We // simulate by throwing a cancellation-like URLError if the test @@ -354,7 +354,7 @@ final class ForgeClientTests: XCTestCase { Thread.sleep(forTimeInterval: 0.5) throw URLError(.timedOut) } - let client = makeClient() + let client = try makeClient() let task = Task { try await client.run(RunRequest(prompt: "x")) } @@ -370,4 +370,226 @@ final class ForgeClientTests: XCTestCase { XCTFail("wrong error: \(error)") } } + + // MARK: - Audit fixes (2026-04-28) + + // 13. P1 — multipart body is RFC 7578 compliant. We write a binary file + // whose content begins with the canonical PNG signature + // (0x89 0x50 0x4E 0x47 0x0D 0x0A 0x1A 0x0A). The bytes immediately + // following the headers/body separator (`\r\n\r\n`) MUST equal the PNG + // signature exactly — no leading `\n`, which the previous bare-LF + // `"""..."""` literals injected. + func testMultipartIsCRLFCompliant() async throws { + let pngSig: [UInt8] = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A] + // Append a few more bytes so we can also verify nothing was eaten + // off the front of the body. + let payload = Data(pngSig + [0x00, 0x01, 0x02, 0x03]) + + let tmp = FileManager.default.temporaryDirectory + .appendingPathComponent("clawdforge-test-\(UUID().uuidString).png") + try payload.write(to: tmp) + defer { try? FileManager.default.removeItem(at: tmp) } + + URLProtocolMock.handler = { req in + let resp = #""" + {"file_token": "ff_png", "ttl_secs": 3600, "size": 12} + """# + return (makeResponse(url: req.url!, status: 200), Data(resp.utf8)) + } + + _ = try await makeClient().uploadFile(at: tmp, ttlSecs: 3600) + + let body = URLProtocolMock.lastBody ?? Data() + XCTAssertGreaterThan(body.count, payload.count, "body should be larger than the payload") + + // The original `"""…"""` multi-line literal bug produced sequences + // like `\r\n\n` (a CRLF followed by a stray bare LF where the next + // CRLF was expected). We can detect that pattern directly: scan + // every CRLF and assert the byte AFTER the LF is not itself a bare + // LF. The PNG signature contains `\r\n` internally so we have to + // skip the part-body region — scan only up to the end of the + // headers (i.e. up to the first occurrence of the `\r\n\r\n` + // separator that precedes the file payload), and from after the + // payload to the end (the trailer). + let needle = Data("Content-Type: image/png\r\n\r\n".utf8) + guard let range = body.range(of: needle) else { + return XCTFail("could not find Content-Type: image/png header in multipart body") + } + let bodyStart = range.upperBound + let bodyEnd = bodyStart + payload.count + + let bytes = [UInt8](body) + let scanRanges: [Range] = [ + 0..= 3 else { continue } + for i in r.lowerBound..<(r.upperBound - 2) { + if bytes[i] == 0x0D && bytes[i + 1] == 0x0A && bytes[i + 2] == 0x0A { + XCTFail("found `\\r\\n\\n` (bare-LF bug) at byte \(i)") + } + } + } + + // The bytes immediately after the `Content-Type: image/png\r\n\r\n` + // separator must equal the payload exactly — no leading bare `\n`. + XCTAssertLessThanOrEqual(bodyEnd, body.count, "body truncated before payload") + let extracted = body.subdata(in: bodyStart..")) + XCTAssertTrue(s2.contains("")) + } + + // 15. P2 — AppToken with non-nil token MUST NOT leak via print or + // reflection. + func testAppTokenDescriptionRedactsToken() { + let tk = AppToken( + name: "cauldron", + token: "cf_SECRET_xyz", + ipCidrs: ["10.0.0.0/8"], + createdAt: 1_700_000_000, + lastUsed: nil, + enabled: 1 + ) + let s1 = String(describing: tk) + let s2 = String(reflecting: tk) + XCTAssertFalse(s1.contains("cf_SECRET_xyz")) + XCTAssertFalse(s2.contains("cf_SECRET_xyz")) + XCTAssertTrue(s1.contains("")) + + // When token is nil, redaction prints `nil`, not ``. + let tk2 = AppToken(name: "petalparse", token: nil) + let s3 = String(describing: tk2) + XCTAssertFalse(s3.contains("")) + XCTAssertTrue(s3.contains("token: nil")) + } + + // 16. P2 — revokeToken rejects path-traversal and other illegal names. + func testRevokeTokenRejectsTraversalName() async throws { + let client = try makeClient() + let invalid = ["foo/../bar", "../etc/passwd", "FOO", "name with space", + "a;b", "a@b", "a+b", "a=b", + String(repeating: "a", count: 65), ""] + for bad in invalid { + do { + try await client.revokeToken(name: bad) + XCTFail("expected ForgeError.invalidArgument for \"\(bad)\"") + } catch ForgeError.invalidArgument { + // expected + } catch { + XCTFail("wrong error for \"\(bad)\": \(error)") + } + } + + // The valid character set MUST still be accepted. + URLProtocolMock.handler = { req in + (makeResponse(url: req.url!, status: 200), Data(#"{"ok": true}"#.utf8)) + } + try await client.revokeToken(name: "valid_name-1") + } + + // 17. P2 — baseURL with non-empty path/query is rejected at construct. + func testBaseURLWithPathRejected() { + let cases = [ + URL(string: "http://x/api")!, + URL(string: "http://x/")!.appendingPathComponent("v1"), + URL(string: "http://x?foo=bar")!, + URL(string: "http://x#frag")!, + ] + for u in cases { + do { + _ = try ForgeClient(baseURL: u, token: "t") + XCTFail("expected ForgeError.invalidArgument for \(u)") + } catch ForgeError.invalidArgument { + // expected + } catch { + XCTFail("wrong error for \(u): \(error)") + } + } + + // Host-only URLs MUST still work. + XCTAssertNoThrow(try ForgeClient(baseURL: URL(string: "http://x")!, token: "t")) + XCTAssertNoThrow(try ForgeClient(baseURL: URL(string: "http://x:8800")!, token: "t")) + // Trailing slash should be tolerated by the trim logic. + XCTAssertNoThrow(try ForgeClient(baseURL: URL(string: "http://x:8800/")!, token: "t")) + } + + // 18. P3 — RunFailure can be constructed from outside the module. + func testRunFailurePublicInit() { + let f = RunFailure( + ok: false, + error: "claude exited 1", + stderr: "boom", + durationMs: 9, + stopReason: "error" + ) + XCTAssertFalse(f.ok) + XCTAssertEqual(f.error, "claude exited 1") + XCTAssertEqual(f.stderr, "boom") + XCTAssertEqual(f.durationMs, 9) + XCTAssertEqual(f.stopReason, "error") + } + + // 19. P3 — staged temp upload file is mode 0o600. + // + // We use a custom URLSession that lets uploadFile() run all the way + // through writeMultipartBody (which creates the temp file), but stub + // the actual transport so we can intercept and inspect the temp file + // before it's torn down by the `defer` in uploadFile. + // + // Trick: the URLProtocol handler can read the temp path off the + // request's body stream and inspect its perms. But Foundation copies + // the file before handing it to URLProtocol, so we instead do the + // perms check via a custom mock that runs inside the upload. + func testTempFilePerms() async throws { + let tmp = FileManager.default.temporaryDirectory + .appendingPathComponent("clawdforge-test-perms-\(UUID().uuidString).bin") + try Data("perms test".utf8).write(to: tmp) + defer { try? FileManager.default.removeItem(at: tmp) } + + // The body bytes, captured by URLProtocolMock, are sufficient proof + // the staged file existed; for the perms assertion we list the temp + // dir before/after and check perms on the staged file. + let beforeNames = Set((try? FileManager.default.contentsOfDirectory( + atPath: FileManager.default.temporaryDirectory.path + )) ?? []) + + URLProtocolMock.handler = { req in + // While the upload is in flight the staged temp file still + // exists — capture perms NOW. + let now = Set((try? FileManager.default.contentsOfDirectory( + atPath: FileManager.default.temporaryDirectory.path + )) ?? []) + let new = now.subtracting(beforeNames) + for name in new where name.hasPrefix("clawdforge-upload-") { + let path = FileManager.default.temporaryDirectory + .appendingPathComponent(name).path + let attrs = try FileManager.default.attributesOfItem(atPath: path) + if let perms = attrs[.posixPermissions] as? NSNumber { + XCTAssertEqual(perms.int16Value & 0o777, 0o600, + "staged temp upload file must be mode 0o600") + } else { + XCTFail("could not read posixPermissions on \(path)") + } + } + let resp = #"{"file_token": "ff_x", "ttl_secs": 3600, "size": 10}"# + return (makeResponse(url: req.url!, status: 200), Data(resp.utf8)) + } + + _ = try await makeClient().uploadFile(at: tmp, ttlSecs: 3600) + } }