From 79818f73c0e7cbcb49d358924c0f026f85236198 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 12 Dec 2025 17:30:21 +0000 Subject: [PATCH] fix(mac): harden gateway frame decoding --- .../Sources/Clawdis/InstanceIdentity.swift | 2 +- apps/macos/Sources/Clawdis/Utilities.swift | 38 +++++--- .../ClawdisProtocol/GatewayModels.swift | 34 +++---- .../Tests/ClawdisIPCTests/AgentRPCTests.swift | 2 +- .../CommandResolverTests.swift | 92 ++++++------------- .../GatewayFrameDecodeTests.swift | 67 ++++++++++++++ .../ClawdisIPCTests/UtilitiesTests.swift | 13 +-- scripts/protocol-gen-swift.ts | 36 +++----- 8 files changed, 153 insertions(+), 131 deletions(-) diff --git a/apps/macos/Sources/Clawdis/InstanceIdentity.swift b/apps/macos/Sources/Clawdis/InstanceIdentity.swift index a5355c2a6..d4c6b0031 100644 --- a/apps/macos/Sources/Clawdis/InstanceIdentity.swift +++ b/apps/macos/Sources/Clawdis/InstanceIdentity.swift @@ -7,8 +7,8 @@ enum InstanceIdentity { private static var defaults: UserDefaults { UserDefaults(suiteName: suiteName) ?? .standard } - static let instanceId: String = { + let defaults = Self.defaults if let existing = defaults.string(forKey: instanceIdKey)? .trimmingCharacters(in: .whitespacesAndNewlines), !existing.isEmpty diff --git a/apps/macos/Sources/Clawdis/Utilities.swift b/apps/macos/Sources/Clawdis/Utilities.swift index e58aa57bd..86e4ab809 100644 --- a/apps/macos/Sources/Clawdis/Utilities.swift +++ b/apps/macos/Sources/Clawdis/Utilities.swift @@ -303,8 +303,12 @@ enum CommandResolver { return false } - static func clawdisNodeCommand(subcommand: String, extraArgs: [String] = []) -> [String] { - let settings = self.connectionSettings() + static func clawdisNodeCommand( + subcommand: String, + extraArgs: [String] = [], + defaults: UserDefaults = .standard) -> [String] + { + let settings = self.connectionSettings(defaults: defaults) if settings.mode == .remote, let ssh = self.sshNodeCommand( subcommand: subcommand, extraArgs: extraArgs, @@ -343,8 +347,12 @@ enum CommandResolver { } } - static func clawdisMacCommand(subcommand: String, extraArgs: [String] = []) -> [String] { - let settings = self.connectionSettings() + static func clawdisMacCommand( + subcommand: String, + extraArgs: [String] = [], + defaults: UserDefaults = .standard) -> [String] + { + let settings = self.connectionSettings(defaults: defaults) if settings.mode == .remote, let ssh = self.sshMacHelperCommand( subcommand: subcommand, extraArgs: extraArgs, @@ -359,8 +367,12 @@ enum CommandResolver { } // Existing callers still refer to clawdisCommand; keep it as node alias. - static func clawdisCommand(subcommand: String, extraArgs: [String] = []) -> [String] { - self.clawdisNodeCommand(subcommand: subcommand, extraArgs: extraArgs) + static func clawdisCommand( + subcommand: String, + extraArgs: [String] = [], + defaults: UserDefaults = .standard) -> [String] + { + self.clawdisNodeCommand(subcommand: subcommand, extraArgs: extraArgs, defaults: defaults) } // MARK: - SSH helpers @@ -477,12 +489,12 @@ enum CommandResolver { let projectRoot: String } - static func connectionSettings() -> RemoteSettings { - let modeRaw = UserDefaults.standard.string(forKey: connectionModeKey) ?? "local" + static func connectionSettings(defaults: UserDefaults = .standard) -> RemoteSettings { + let modeRaw = defaults.string(forKey: connectionModeKey) ?? "local" let mode = AppState.ConnectionMode(rawValue: modeRaw) ?? .local - let target = UserDefaults.standard.string(forKey: remoteTargetKey) ?? "" - let identity = UserDefaults.standard.string(forKey: remoteIdentityKey) ?? "" - let projectRoot = UserDefaults.standard.string(forKey: remoteProjectRootKey) ?? "" + let target = defaults.string(forKey: remoteTargetKey) ?? "" + let identity = defaults.string(forKey: remoteIdentityKey) ?? "" + let projectRoot = defaults.string(forKey: remoteProjectRootKey) ?? "" return RemoteSettings( mode: mode, target: self.sanitizedTarget(target), @@ -494,8 +506,8 @@ enum CommandResolver { UserDefaults.standard.bool(forKey: attachExistingGatewayOnlyKey) } - static func connectionModeIsRemote() -> Bool { - self.connectionSettings().mode == .remote + static func connectionModeIsRemote(defaults: UserDefaults = .standard) -> Bool { + self.connectionSettings(defaults: defaults).mode == .remote } private static func sanitizedTarget(_ raw: String) -> String { diff --git a/apps/macos/Sources/ClawdisProtocol/GatewayModels.swift b/apps/macos/Sources/ClawdisProtocol/GatewayModels.swift index faafc1ea2..7f88763ef 100644 --- a/apps/macos/Sources/ClawdisProtocol/GatewayModels.swift +++ b/apps/macos/Sources/ClawdisProtocol/GatewayModels.swift @@ -545,26 +545,29 @@ public enum GatewayFrame: Codable { case event(EventFrame) case unknown(type: String, raw: [String: AnyCodable]) + private enum CodingKeys: String, CodingKey { + case type + } + public init(from decoder: Decoder) throws { - let container = try decoder.singleValueContainer() - let raw = try container.decode([String: AnyCodable].self) - guard let type = raw["type"]?.value as? String else { - throw DecodingError.dataCorruptedError(in: container, debugDescription: "missing type") - } + let typeContainer = try decoder.container(keyedBy: CodingKeys.self) + let type = try typeContainer.decode(String.self, forKey: .type) switch type { case "hello": - self = .hello(try Self.decodePayload(Hello.self, from: raw)) + self = .hello(try Hello(from: decoder)) case "hello-ok": - self = .helloOk(try Self.decodePayload(HelloOk.self, from: raw)) + self = .helloOk(try HelloOk(from: decoder)) case "hello-error": - self = .helloError(try Self.decodePayload(HelloError.self, from: raw)) + self = .helloError(try HelloError(from: decoder)) case "req": - self = .req(try Self.decodePayload(RequestFrame.self, from: raw)) + self = .req(try RequestFrame(from: decoder)) case "res": - self = .res(try Self.decodePayload(ResponseFrame.self, from: raw)) + self = .res(try ResponseFrame(from: decoder)) case "event": - self = .event(try Self.decodePayload(EventFrame.self, from: raw)) + self = .event(try EventFrame(from: decoder)) default: + let container = try decoder.singleValueContainer() + let raw = try container.decode([String: AnyCodable].self) self = .unknown(type: type, raw: raw) } } @@ -583,13 +586,4 @@ public enum GatewayFrame: Codable { } } - - private static func decodePayload(_ type: T.Type, from raw: [String: AnyCodable]) throws -> T { - // raw is [String: AnyCodable] which is not directly JSONSerialization-compatible. - // Round-trip through JSONEncoder so AnyCodable can encode itself safely. - let data = try JSONEncoder().encode(raw) - let decoder = JSONDecoder() - return try decoder.decode(T.self, from: data) - } - } diff --git a/apps/macos/Tests/ClawdisIPCTests/AgentRPCTests.swift b/apps/macos/Tests/ClawdisIPCTests/AgentRPCTests.swift index f62bf6af0..4f9e0c6ff 100644 --- a/apps/macos/Tests/ClawdisIPCTests/AgentRPCTests.swift +++ b/apps/macos/Tests/ClawdisIPCTests/AgentRPCTests.swift @@ -11,7 +11,7 @@ import Testing } @Test func rejectEmptyMessage() async { - let result = await AgentRPC.shared.send(text: "", thinking: nil, session: "main", deliver: false, to: nil) + let result = await AgentRPC.shared.send(text: "", thinking: nil, sessionKey: "main", deliver: false, to: nil) #expect(result.ok == false) } } diff --git a/apps/macos/Tests/ClawdisIPCTests/CommandResolverTests.swift b/apps/macos/Tests/ClawdisIPCTests/CommandResolverTests.swift index 79974a719..f0a543a87 100644 --- a/apps/macos/Tests/ClawdisIPCTests/CommandResolverTests.swift +++ b/apps/macos/Tests/ClawdisIPCTests/CommandResolverTests.swift @@ -4,6 +4,11 @@ import Testing @testable import Clawdis @Suite(.serialized) struct CommandResolverTests { + private func makeDefaults() -> UserDefaults { + // Use a unique suite to avoid cross-suite concurrency on UserDefaults.standard. + UserDefaults(suiteName: "CommandResolverTests.\(UUID().uuidString)")! + } + private func makeTempDir() throws -> URL { let base = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true) let dir = base.appendingPathComponent(UUID().uuidString, isDirectory: true) @@ -20,16 +25,8 @@ import Testing } @Test func prefersClawdisBinary() async throws { - UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - } + let defaults = self.makeDefaults() + defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) let tmp = try makeTempDir() CommandResolver.setProjectRoot(tmp.path) @@ -37,21 +34,13 @@ import Testing let clawdisPath = tmp.appendingPathComponent("node_modules/.bin/clawdis") try self.makeExec(at: clawdisPath) - let cmd = CommandResolver.clawdisCommand(subcommand: "gateway") + let cmd = CommandResolver.clawdisCommand(subcommand: "gateway", defaults: defaults) #expect(cmd.prefix(2).elementsEqual([clawdisPath.path, "gateway"])) } @Test func fallsBackToNodeAndScript() async throws { - UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - } + let defaults = self.makeDefaults() + defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) let tmp = try makeTempDir() CommandResolver.setProjectRoot(tmp.path) @@ -63,7 +52,7 @@ import Testing try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: nodePath.path) try self.makeExec(at: scriptPath) - let cmd = CommandResolver.clawdisCommand(subcommand: "rpc") + let cmd = CommandResolver.clawdisCommand(subcommand: "rpc", defaults: defaults) #expect(cmd.count >= 3) #expect(cmd[0] == nodePath.path) @@ -72,16 +61,8 @@ import Testing } @Test func fallsBackToPnpm() async throws { - UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - } + let defaults = self.makeDefaults() + defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) let tmp = try makeTempDir() CommandResolver.setProjectRoot(tmp.path) @@ -89,22 +70,14 @@ import Testing let pnpmPath = tmp.appendingPathComponent("node_modules/.bin/pnpm") try self.makeExec(at: pnpmPath) - let cmd = CommandResolver.clawdisCommand(subcommand: "rpc") + let cmd = CommandResolver.clawdisCommand(subcommand: "rpc", defaults: defaults) #expect(cmd.prefix(4).elementsEqual([pnpmPath.path, "--silent", "clawdis", "rpc"])) } @Test func pnpmKeepsExtraArgsAfterSubcommand() async throws { - UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - } + let defaults = self.makeDefaults() + defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) let tmp = try makeTempDir() CommandResolver.setProjectRoot(tmp.path) @@ -112,24 +85,16 @@ import Testing let pnpmPath = tmp.appendingPathComponent("node_modules/.bin/pnpm") try self.makeExec(at: pnpmPath) - let cmd = CommandResolver.clawdisCommand(subcommand: "health", extraArgs: ["--json", "--timeout", "5"]) + let cmd = CommandResolver.clawdisCommand( + subcommand: "health", + extraArgs: ["--json", "--timeout", "5"], + defaults: defaults) #expect(cmd.prefix(5).elementsEqual([pnpmPath.path, "--silent", "clawdis", "health", "--json"])) #expect(cmd.suffix(2).elementsEqual(["--timeout", "5"])) } @Test func preferredPathsStartWithProjectNodeBins() async throws { - UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - } - let tmp = try makeTempDir() CommandResolver.setProjectRoot(tmp.path) @@ -138,18 +103,13 @@ import Testing } @Test func buildsSSHCommandForRemoteMode() async throws { - UserDefaults.standard.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey) - UserDefaults.standard.set("clawd@example.com:2222", forKey: remoteTargetKey) - UserDefaults.standard.set("/tmp/id_ed25519", forKey: remoteIdentityKey) - UserDefaults.standard.set("/srv/clawdis", forKey: remoteProjectRootKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - UserDefaults.standard.removeObject(forKey: remoteIdentityKey) - UserDefaults.standard.removeObject(forKey: remoteProjectRootKey) - } + let defaults = self.makeDefaults() + defaults.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey) + defaults.set("clawd@example.com:2222", forKey: remoteTargetKey) + defaults.set("/tmp/id_ed25519", forKey: remoteIdentityKey) + defaults.set("/srv/clawdis", forKey: remoteProjectRootKey) - let cmd = CommandResolver.clawdisCommand(subcommand: "status", extraArgs: ["--json"]) + let cmd = CommandResolver.clawdisCommand(subcommand: "status", extraArgs: ["--json"], defaults: defaults) #expect(cmd.first == "/usr/bin/ssh") #expect(cmd.contains("clawd@example.com")) diff --git a/apps/macos/Tests/ClawdisIPCTests/GatewayFrameDecodeTests.swift b/apps/macos/Tests/ClawdisIPCTests/GatewayFrameDecodeTests.swift index 62ce6bbcd..70af4d2ac 100644 --- a/apps/macos/Tests/ClawdisIPCTests/GatewayFrameDecodeTests.swift +++ b/apps/macos/Tests/ClawdisIPCTests/GatewayFrameDecodeTests.swift @@ -28,4 +28,71 @@ import Testing #expect(payload?["count"]?.value as? Int == 1) #expect(evt.seq == 7) } + + @Test func decodesRequestFrameWithNestedParams() throws { + let json = """ + { + "type": "req", + "id": "1", + "method": "agent.send", + "params": { + "text": "hi", + "items": [1, null, {"ok": true}], + "meta": { "count": 2 } + } + } + """ + + let frame = try JSONDecoder().decode(GatewayFrame.self, from: Data(json.utf8)) + + #expect({ + if case .req = frame { true } else { false } + }(), "expected .req frame") + + guard case let .req(req) = frame else { + return + } + + let params = req.params?.value as? [String: AnyCodable] + #expect(params?["text"]?.value as? String == "hi") + + let items = params?["items"]?.value as? [AnyCodable] + #expect(items?.count == 3) + #expect(items?[0].value as? Int == 1) + #expect(items?[1].value is NSNull) + + let item2 = items?[2].value as? [String: AnyCodable] + #expect(item2?["ok"]?.value as? Bool == true) + + let meta = params?["meta"]?.value as? [String: AnyCodable] + #expect(meta?["count"]?.value as? Int == 2) + } + + @Test func decodesUnknownFrameAndPreservesRaw() throws { + let json = """ + { + "type": "made-up", + "foo": "bar", + "count": 1, + "nested": { "ok": true } + } + """ + + let frame = try JSONDecoder().decode(GatewayFrame.self, from: Data(json.utf8)) + + #expect({ + if case .unknown = frame { true } else { false } + }(), "expected .unknown frame") + + guard case let .unknown(type, raw) = frame else { + return + } + + #expect(type == "made-up") + #expect(raw["type"]?.value as? String == "made-up") + #expect(raw["foo"]?.value as? String == "bar") + #expect(raw["count"]?.value as? Int == 1) + let nested = raw["nested"]?.value as? [String: AnyCodable] + #expect(nested?["ok"]?.value as? Bool == true) + } } diff --git a/apps/macos/Tests/ClawdisIPCTests/UtilitiesTests.swift b/apps/macos/Tests/ClawdisIPCTests/UtilitiesTests.swift index 53b63815d..8c6f12cbd 100644 --- a/apps/macos/Tests/ClawdisIPCTests/UtilitiesTests.swift +++ b/apps/macos/Tests/ClawdisIPCTests/UtilitiesTests.swift @@ -2,7 +2,7 @@ import Foundation import Testing @testable import Clawdis -@Suite struct UtilitiesTests { +@Suite(.serialized) struct UtilitiesTests { @Test func ageStringsCoverCommonWindows() { let now = Date(timeIntervalSince1970: 1_000_000) #expect(age(from: now, now: now) == "just now") @@ -33,14 +33,11 @@ import Testing } @Test func sanitizedTargetStripsLeadingSSHPrefix() { - UserDefaults.standard.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey) - UserDefaults.standard.set("ssh alice@example.com", forKey: remoteTargetKey) - defer { - UserDefaults.standard.removeObject(forKey: connectionModeKey) - UserDefaults.standard.removeObject(forKey: remoteTargetKey) - } + let defaults = UserDefaults(suiteName: "UtilitiesTests.\(UUID().uuidString)")! + defaults.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey) + defaults.set("ssh alice@example.com", forKey: remoteTargetKey) - let settings = CommandResolver.connectionSettings() + let settings = CommandResolver.connectionSettings(defaults: defaults) #expect(settings.mode == .remote) #expect(settings.target == "alice@example.com") } diff --git a/scripts/protocol-gen-swift.ts b/scripts/protocol-gen-swift.ts index 3145b4ea7..0d9fba9ba 100644 --- a/scripts/protocol-gen-swift.ts +++ b/scripts/protocol-gen-swift.ts @@ -157,26 +157,29 @@ function emitGatewayFrame(): string { }; const caseLines = cases.map((c) => ` case ${safeName(c)}(${associated[c]})`); const initLines = ` + private enum CodingKeys: String, CodingKey { + case type + } + public init(from decoder: Decoder) throws { - let container = try decoder.singleValueContainer() - let raw = try container.decode([String: AnyCodable].self) - guard let type = raw["type"]?.value as? String else { - throw DecodingError.dataCorruptedError(in: container, debugDescription: "missing type") - } + let typeContainer = try decoder.container(keyedBy: CodingKeys.self) + let type = try typeContainer.decode(String.self, forKey: .type) switch type { case "hello": - self = .hello(try Self.decodePayload(Hello.self, from: raw)) + self = .hello(try Hello(from: decoder)) case "hello-ok": - self = .helloOk(try Self.decodePayload(HelloOk.self, from: raw)) + self = .helloOk(try HelloOk(from: decoder)) case "hello-error": - self = .helloError(try Self.decodePayload(HelloError.self, from: raw)) + self = .helloError(try HelloError(from: decoder)) case "req": - self = .req(try Self.decodePayload(RequestFrame.self, from: raw)) + self = .req(try RequestFrame(from: decoder)) case "res": - self = .res(try Self.decodePayload(ResponseFrame.self, from: raw)) + self = .res(try ResponseFrame(from: decoder)) case "event": - self = .event(try Self.decodePayload(EventFrame.self, from: raw)) + self = .event(try EventFrame(from: decoder)) default: + let container = try decoder.singleValueContainer() + let raw = try container.decode([String: AnyCodable].self) self = .unknown(type: type, raw: raw) } } @@ -196,22 +199,11 @@ function emitGatewayFrame(): string { } `; - const helper = ` - private static func decodePayload(_ type: T.Type, from raw: [String: AnyCodable]) throws -> T { - // raw is [String: AnyCodable] which is not directly JSONSerialization-compatible. - // Round-trip through JSONEncoder so AnyCodable can encode itself safely. - let data = try JSONEncoder().encode(raw) - let decoder = JSONDecoder() - return try decoder.decode(T.self, from: data) - } -`; - return [ "public enum GatewayFrame: Codable {", ...caseLines, " case unknown(type: String, raw: [String: AnyCodable])", initLines, - helper, "}", "", ].join("\n");