diff --git a/apps/ios/Tests/GatewayConnectionControllerTests.swift b/apps/ios/Tests/GatewayConnectionControllerTests.swift index 3c140a0ab36f..30480f89638b 100644 --- a/apps/ios/Tests/GatewayConnectionControllerTests.swift +++ b/apps/ios/Tests/GatewayConnectionControllerTests.swift @@ -1913,6 +1913,66 @@ private func waitUntil( } } + @Test @MainActor func `share relay keeps credentials out of app group defaults`() throws { + let registryIsolation = GatewayRegistryTestIsolation() + defer { registryIsolation.restore() } + let token = "relay-token-\(UUID().uuidString)" + let password = "relay-password-\(UUID().uuidString)" + + #expect(ShareGatewayRelaySettings.saveConfig(ShareGatewayRelayConfig( + gatewayURLString: "wss://secure-relay.example.com", + gatewayStableID: "manual|secure-relay.example.com|443", + token: token, + password: password, + sessionKey: "main"))) + + let defaults = try #require(UserDefaults(suiteName: OpenClawAppGroup.identifier)) + let persisted = try #require(defaults.data(forKey: "share.gatewayRelay.config.v1")) + #expect(persisted.range(of: Data(token.utf8)) == nil) + #expect(persisted.range(of: Data(password.utf8)) == nil) + let loaded = try #require(ShareGatewayRelaySettings.loadConfig()) + #expect(loaded.token == token) + #expect(loaded.password == password) + + let otherRelay = ShareGatewayRelayConfig( + gatewayURLString: "wss://other-relay.example.com", + gatewayStableID: "manual|other-relay.example.com|443", + token: nil, + password: nil, + sessionKey: "main") + defaults.set(try JSONEncoder().encode(otherRelay), forKey: "share.gatewayRelay.config.v1") + let mismatched = try #require(ShareGatewayRelaySettings.loadConfig()) + #expect(mismatched.token == nil) + #expect(mismatched.password == nil) + } + + @Test @MainActor func `share relay migrates legacy defaults credentials into keychain`() throws { + let registryIsolation = GatewayRegistryTestIsolation() + defer { registryIsolation.restore() } + let token = "legacy-token-\(UUID().uuidString)" + let password = "legacy-password-\(UUID().uuidString)" + let defaults = try #require(UserDefaults(suiteName: OpenClawAppGroup.identifier)) + let legacy = try JSONSerialization.data(withJSONObject: [ + "gatewayURLString": "wss://legacy-relay.example.com", + "gatewayStableID": "manual|legacy-relay.example.com|443", + "token": token, + "password": password, + "sessionKey": "main", + ]) + defaults.set(legacy, forKey: "share.gatewayRelay.config.v1") + + let loaded = try #require(ShareGatewayRelaySettings.loadConfig()) + + #expect(loaded.token == token) + #expect(loaded.password == password) + let migrated = try #require(defaults.data(forKey: "share.gatewayRelay.config.v1")) + #expect(migrated.range(of: Data(token.utf8)) == nil) + #expect(migrated.range(of: Data(password.utf8)) == nil) + let reloaded = try #require(ShareGatewayRelaySettings.loadConfig()) + #expect(reloaded.token == token) + #expect(reloaded.password == password) + } + @Test @MainActor func `forget gateway clears matching share relay only`() async { let registryIsolation = GatewayRegistryTestIsolation() defer { registryIsolation.restore() } diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift index 54657fa1d1e0..63e9d6535092 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift @@ -24,8 +24,12 @@ public enum GenericPasswordKeychainStore { } } - public static func loadString(service: String, account: String) -> String? { - guard let data = self.loadData(service: service, account: account) else { return nil } + public static func loadString( + service: String, + account: String, + accessGroup: String? = nil) -> String? + { + guard let data = self.loadData(service: service, account: account, accessGroup: accessGroup) else { return nil } return String(data: data, encoding: .utf8) } @@ -34,9 +38,16 @@ public enum GenericPasswordKeychainStore { _ value: String, service: String, account: String, + accessGroup: String? = nil, accessible: CFString = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly) -> Bool { - switch self.saveStringResult(value, service: service, account: account, accessible: accessible) { + switch self.saveStringResult( + value, + service: service, + account: account, + accessGroup: accessGroup, + accessible: accessible) + { case .success: true case .failure: false } @@ -46,6 +57,7 @@ public enum GenericPasswordKeychainStore { _ value: String, service: String, account: String, + accessGroup: String? = nil, accessible: CFString = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly) -> Result { @@ -53,14 +65,19 @@ public enum GenericPasswordKeychainStore { Data(value.utf8), service: service, account: account, + accessGroup: accessGroup, accessible: accessible, updateItem: { SecItemUpdate($0, $1) }, addItem: { SecItemAdd($0, nil) }) } @discardableResult - public static func delete(service: String, account: String) -> Bool { - switch self.deleteResult(service: service, account: account) { + public static func delete( + service: String, + account: String, + accessGroup: String? = nil) -> Bool + { + switch self.deleteResult(service: service, account: account, accessGroup: accessGroup) { case .success: true case .failure: false } @@ -68,11 +85,13 @@ public enum GenericPasswordKeychainStore { public static func deleteResult( service: String, - account: String) -> Result + account: String, + accessGroup: String? = nil) -> Result { self.deleteResult( service: service, account: account, + accessGroup: accessGroup, deleteItem: { SecItemDelete($0) }) } @@ -86,8 +105,8 @@ public enum GenericPasswordKeychainStore { return status == errSecSuccess || status == errSecItemNotFound } - private static func loadData(service: String, account: String) -> Data? { - var query = self.baseQuery(service: service, account: account) + private static func loadData(service: String, account: String, accessGroup: String?) -> Data? { + var query = self.baseQuery(service: service, account: account, accessGroup: accessGroup) query[kSecReturnData as String] = true query[kSecMatchLimit as String] = kSecMatchLimitOne @@ -101,11 +120,12 @@ public enum GenericPasswordKeychainStore { _ data: Data, service: String, account: String, + accessGroup: String? = nil, accessible: CFString, updateItem: (CFDictionary, CFDictionary) -> OSStatus, addItem: (CFDictionary) -> OSStatus) -> Result { - let query = self.baseQuery(service: service, account: account) + let query = self.baseQuery(service: service, account: account, accessGroup: accessGroup) let updates: [String: Any] = [ kSecValueData as String: data, kSecAttrAccessible as String: accessible, @@ -141,20 +161,32 @@ public enum GenericPasswordKeychainStore { static func deleteResult( service: String, account: String, + accessGroup: String? = nil, deleteItem: (CFDictionary) -> OSStatus) -> Result { - let status = deleteItem(self.baseQuery(service: service, account: account) as CFDictionary) + let status = deleteItem(self.baseQuery( + service: service, + account: account, + accessGroup: accessGroup) as CFDictionary) guard status != errSecSuccess, status != errSecItemNotFound else { return .success(()) } return .failure(MutationError(operation: .delete, status: status)) } - private static func baseQuery(service: String, account: String) -> [String: Any] { - [ + private static func baseQuery( + service: String, + account: String, + accessGroup: String?) -> [String: Any] + { + var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, kSecAttrAccount as String: account, ] + if let accessGroup { + query[kSecAttrAccessGroup as String] = accessGroup + } + return query } } diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/ShareGatewayRelaySettings.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/ShareGatewayRelaySettings.swift index f7489b4cfb2c..bab9f91cf23f 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/ShareGatewayRelaySettings.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/ShareGatewayRelaySettings.swift @@ -34,15 +34,51 @@ public enum ShareGatewayRelaySettings { } private static let relayConfigKey = "share.gatewayRelay.config.v1" + // On iOS an App Group is also a Keychain access group. Reuse the existing + // group so the host and extension share only this credential bundle. + private static let relayCredentialService = "ai.openclawfoundation.app.share-gateway-relay" + private static let relayCredentialAccount = "credentials.v1" private static let lastEventKey = "share.gatewayRelay.event.v1" private static var defaults: UserDefaults { UserDefaults(suiteName: self.suiteName) ?? .standard } + private static var isAppExtension: Bool { + Bundle.main.object(forInfoDictionaryKey: "NSExtension") != nil + } + public static func loadConfig() -> ShareGatewayRelayConfig? { guard let data = self.defaults.data(forKey: self.relayConfigKey) else { return nil } - return try? JSONDecoder().decode(ShareGatewayRelayConfig.self, from: data) + guard let config = try? JSONDecoder().decode(ShareGatewayRelayConfig.self, from: data) else { return nil } + if config.token != nil || config.password != nil { + return self.resolveLegacyConfig( + config, + isAppExtension: self.isAppExtension, + migrate: { config in + self.commitConfig( + config, + saveCredentials: self.saveCredentials, + saveMetadata: self.saveMetadata) + }, + discard: { + self.defaults.removeObject(forKey: self.relayConfigKey) + self.saveLastEvent("Share unavailable after upgrade: open OpenClaw to reconnect securely.") + }) + } + // Keep relay identity in the Keychain bundle with its secrets. A partial + // route update must never bind one gateway's credentials to another. + let credentials = self.loadCredentials().flatMap { stored in + self.credentials(stored, match: config) ? stored : nil + } + return ShareGatewayRelayConfig( + gatewayURLString: config.gatewayURLString, + gatewayStableID: config.gatewayStableID, + token: credentials?.token, + password: credentials?.password, + sessionKey: config.sessionKey, + deliveryChannel: config.deliveryChannel, + deliveryTo: config.deliveryTo) } /// An endpoint is not a gateway identity. If the extension launches before the @@ -61,13 +97,23 @@ public enum ShareGatewayRelaySettings { return config } - public static func saveConfig(_ config: ShareGatewayRelayConfig) { - guard let data = try? JSONEncoder().encode(config) else { return } - self.defaults.set(data, forKey: self.relayConfigKey) + @discardableResult + public static func saveConfig(_ config: ShareGatewayRelayConfig) -> Bool { + let saved = self.commitConfig( + config, + saveCredentials: self.saveCredentials, + saveMetadata: self.saveMetadata) + guard saved else { + self.defaults.removeObject(forKey: self.relayConfigKey) + self.saveLastEvent("Share unavailable: reconnect OpenClaw to save gateway access securely.") + return false + } + return true } public static func clearConfig() { self.defaults.removeObject(forKey: self.relayConfigKey) + _ = self.deleteCredentials() } public static func saveLastEvent(_ message: String) { @@ -81,4 +127,88 @@ public enum ShareGatewayRelaySettings { .trimmingCharacters(in: .whitespacesAndNewlines) ?? "" return value.isEmpty ? nil : value } + + private static func saveMetadata(_ config: ShareGatewayRelayConfig) { + let metadata = ShareGatewayRelayConfig( + gatewayURLString: config.gatewayURLString, + gatewayStableID: config.gatewayStableID, + token: nil, + password: nil, + sessionKey: config.sessionKey, + deliveryChannel: config.deliveryChannel, + deliveryTo: config.deliveryTo) + guard let data = try? JSONEncoder().encode(metadata) else { return } + self.defaults.set(data, forKey: self.relayConfigKey) + } + + static func commitConfig( + _ config: ShareGatewayRelayConfig, + saveCredentials: (ShareGatewayRelayConfig) -> Bool, + saveMetadata: (ShareGatewayRelayConfig) -> Void) -> Bool + { + guard saveCredentials(config) else { return false } + saveMetadata(config) + return true + } + + static func resolveLegacyConfig( + _ config: ShareGatewayRelayConfig, + isAppExtension: Bool, + migrate: (ShareGatewayRelayConfig) -> Bool, + discard: () -> Void) -> ShareGatewayRelayConfig? + { + // Only the host may create shared credentials. An extension-first upgrade + // or failed Keychain write must scrub and reject the legacy auth record. + guard !isAppExtension, migrate(config) else { + discard() + return nil + } + return config + } + + private static func loadCredentials() -> ShareGatewayRelayConfig? { + guard let json = GenericPasswordKeychainStore.loadString( + service: self.relayCredentialService, + account: self.relayCredentialAccount, + accessGroup: self.suiteName), + let data = json.data(using: .utf8), + let credentials = try? JSONDecoder().decode(ShareGatewayRelayConfig.self, from: data) + else { return nil } + return credentials + } + + private static func saveCredentials(_ config: ShareGatewayRelayConfig) -> Bool { + guard config.token != nil || config.password != nil else { + return self.deleteCredentials() + } + guard let data = try? JSONEncoder().encode(config), + let json = String(data: data, encoding: .utf8), + GenericPasswordKeychainStore.saveString( + json, + service: self.relayCredentialService, + account: self.relayCredentialAccount, + accessGroup: self.suiteName) + else { + return false + } + return true + } + + private static func deleteCredentials() -> Bool { + GenericPasswordKeychainStore.delete( + service: self.relayCredentialService, + account: self.relayCredentialAccount, + accessGroup: self.suiteName) + } + + private static func credentials( + _ credentials: ShareGatewayRelayConfig, + match metadata: ShareGatewayRelayConfig) -> Bool + { + if let stableID = metadata.gatewayStableID, !stableID.isEmpty { + return credentials.gatewayStableID == stableID + } + return credentials.gatewayStableID?.isEmpty != false && + credentials.gatewayURLString == metadata.gatewayURLString + } } diff --git a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/GenericPasswordKeychainStoreTests.swift b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/GenericPasswordKeychainStoreTests.swift index 6cfbd77fd031..61d7e9a9755d 100644 --- a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/GenericPasswordKeychainStoreTests.swift +++ b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/GenericPasswordKeychainStoreTests.swift @@ -51,6 +51,34 @@ struct GenericPasswordKeychainStoreTests { #expect(addCalls == 1) } + @Test func `shared access group scopes keychain mutation queries`() { + let accessGroup = "group.ai.openclawfoundation.app.shared" + var updateQuery: [String: Any] = [:] + var addQuery: [String: Any] = [:] + + let result = GenericPasswordKeychainStore.saveDataResult( + Data("shared-value".utf8), + service: "test-service", + account: "test-account", + accessGroup: accessGroup, + accessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, + updateItem: { query, _ in + updateQuery = query as? [String: Any] ?? [:] + return errSecItemNotFound + }, + addItem: { query in + addQuery = query as? [String: Any] ?? [:] + return errSecSuccess + }) + + guard case .success = result else { + Issue.record("expected shared keychain add to succeed") + return + } + #expect(updateQuery[kSecAttrAccessGroup as String] as? String == accessGroup) + #expect(addQuery[kSecAttrAccessGroup as String] as? String == accessGroup) + } + @Test func `add race retries atomic update`() { var updateCalls = 0 diff --git a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ShareGatewayRelaySettingsTests.swift b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ShareGatewayRelaySettingsTests.swift new file mode 100644 index 000000000000..246c04eed96c --- /dev/null +++ b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ShareGatewayRelaySettingsTests.swift @@ -0,0 +1,87 @@ +import Testing +@testable import OpenClawKit + +struct ShareGatewayRelaySettingsTests { + private let config = ShareGatewayRelayConfig( + gatewayURLString: "wss://relay.example.com", + gatewayStableID: "manual|relay.example.com|443", + token: "token", + password: "password", + sessionKey: "main") + + @Test func `failed credential persistence leaves metadata unchanged`() { + var metadataWrites = 0 + + let saved = ShareGatewayRelaySettings.commitConfig( + self.config, + saveCredentials: { _ in false }, + saveMetadata: { _ in metadataWrites += 1 }) + + #expect(!saved) + #expect(metadataWrites == 0) + } + + @Test func `successful credential persistence commits metadata once`() { + var metadataWrites = 0 + + let saved = ShareGatewayRelaySettings.commitConfig( + self.config, + saveCredentials: { _ in true }, + saveMetadata: { _ in metadataWrites += 1 }) + + #expect(saved) + #expect(metadataWrites == 1) + } + + @Test func `extension-first upgrade discards and rejects legacy credentials`() { + var migrations = 0 + var discards = 0 + + let resolved = ShareGatewayRelaySettings.resolveLegacyConfig( + self.config, + isAppExtension: true, + migrate: { _ in + migrations += 1 + return true + }, + discard: { discards += 1 }) + + #expect(resolved == nil) + #expect(migrations == 0) + #expect(discards == 1) + } + + @Test func `host app owns legacy migration`() { + var migrated: ShareGatewayRelayConfig? + var discards = 0 + + let resolved = ShareGatewayRelaySettings.resolveLegacyConfig( + self.config, + isAppExtension: false, + migrate: { config in + migrated = config + return true + }, + discard: { discards += 1 }) + + #expect(resolved == self.config) + #expect(migrated == self.config) + #expect(discards == 0) + } + + @Test func `failed host migration discards and rejects legacy credentials`() { + var calls: [String] = [] + + let resolved = ShareGatewayRelaySettings.resolveLegacyConfig( + self.config, + isAppExtension: false, + migrate: { _ in + calls.append("migrate") + return false + }, + discard: { calls.append("discard") }) + + #expect(resolved == nil) + #expect(calls == ["migrate", "discard"]) + } +}