iOS Security Stack 1/5: Keychain Migrations + Tests (#33029)

Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: da2f8f6141 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky
2026-03-30 02:20:38 +00:00 · 2026-03-03 16:15:20 +00:00
parent 606cd0d591
commit ec0eb9f8c3
7 changed files with 268 additions and 108 deletions
--- a/apps/ios/Sources/Gateway/GatewaySettingsStore.swift
+++ b/apps/ios/Sources/Gateway/GatewaySettingsStore.swift
@@ -25,6 +25,7 @@ enum GatewaySettingsStore {
    private static let instanceIdAccount = "instanceId"
    private static let preferredGatewayStableIDAccount = "preferredStableID"
    private static let lastDiscoveredGatewayStableIDAccount = "lastDiscoveredStableID"
+    private static let lastGatewayConnectionAccount = "lastConnection"
    private static let talkProviderApiKeyAccountPrefix = "provider.apiKey."

    static func bootstrapPersistence() {
@@ -140,11 +141,20 @@ enum GatewaySettingsStore {
        }
    }

-    private enum LastGatewayKind: String {
+    private enum LastGatewayKind: String, Codable {
        case manual
        case discovered
    }

+    /// JSON-serializable envelope stored as a single Keychain entry.
+    private struct LastGatewayConnectionData: Codable {
+        var kind: LastGatewayKind
+        var stableID: String
+        var useTLS: Bool
+        var host: String?
+        var port: Int?
+    }
+
    static func loadTalkProviderApiKey(provider: String) -> String? {
        guard let providerId = self.normalizedTalkProviderID(provider) else { return nil }
        let account = self.talkProviderApiKeyAccount(providerId: providerId)
@@ -168,47 +178,93 @@ enum GatewaySettingsStore {
    }

    static func saveLastGatewayConnectionManual(host: String, port: Int, useTLS: Bool, stableID: String) {
-        let defaults = UserDefaults.standard
-        defaults.set(LastGatewayKind.manual.rawValue, forKey: self.lastGatewayKindDefaultsKey)
-        defaults.set(host, forKey: self.lastGatewayHostDefaultsKey)
-        defaults.set(port, forKey: self.lastGatewayPortDefaultsKey)
-        defaults.set(useTLS, forKey: self.lastGatewayTlsDefaultsKey)
-        defaults.set(stableID, forKey: self.lastGatewayStableIDDefaultsKey)
+        let payload = LastGatewayConnectionData(
+            kind: .manual, stableID: stableID, useTLS: useTLS, host: host, port: port)
+        self.saveLastGatewayConnectionData(payload)
    }

    static func saveLastGatewayConnectionDiscovered(stableID: String, useTLS: Bool) {
-        let defaults = UserDefaults.standard
-        defaults.set(LastGatewayKind.discovered.rawValue, forKey: self.lastGatewayKindDefaultsKey)
-        defaults.removeObject(forKey: self.lastGatewayHostDefaultsKey)
-        defaults.removeObject(forKey: self.lastGatewayPortDefaultsKey)
-        defaults.set(useTLS, forKey: self.lastGatewayTlsDefaultsKey)
-        defaults.set(stableID, forKey: self.lastGatewayStableIDDefaultsKey)
+        let payload = LastGatewayConnectionData(
+            kind: .discovered, stableID: stableID, useTLS: useTLS)
+        self.saveLastGatewayConnectionData(payload)
    }

    static func loadLastGatewayConnection() -> LastGatewayConnection? {
+        // Migrate legacy UserDefaults entries on first access.
+        self.migrateLastGatewayFromUserDefaultsIfNeeded()
+
+        guard let json = KeychainStore.loadString(
+            service: self.gatewayService, account: self.lastGatewayConnectionAccount),
+            let data = json.data(using: .utf8),
+            let stored = try? JSONDecoder().decode(LastGatewayConnectionData.self, from: data)
+        else { return nil }
+
+        let stableID = stored.stableID.trimmingCharacters(in: .whitespacesAndNewlines)
+        guard !stableID.isEmpty else { return nil }
+
+        if stored.kind == .discovered {
+            return .discovered(stableID: stableID, useTLS: stored.useTLS)
+        }
+
+        let host = (stored.host ?? "").trimmingCharacters(in: .whitespacesAndNewlines)
+        let port = stored.port ?? 0
+        guard !host.isEmpty, port > 0, port <= 65535 else { return nil }
+        return .manual(host: host, port: port, useTLS: stored.useTLS, stableID: stableID)
+    }
+
+    static func clearLastGatewayConnection(defaults: UserDefaults = .standard) {
+        _ = KeychainStore.delete(
+            service: self.gatewayService, account: self.lastGatewayConnectionAccount)
+        // Clean up any legacy UserDefaults entries.
+        defaults.removeObject(forKey: self.lastGatewayKindDefaultsKey)
+        defaults.removeObject(forKey: self.lastGatewayHostDefaultsKey)
+        defaults.removeObject(forKey: self.lastGatewayPortDefaultsKey)
+        defaults.removeObject(forKey: self.lastGatewayTlsDefaultsKey)
+        defaults.removeObject(forKey: self.lastGatewayStableIDDefaultsKey)
+    }
+
+    @discardableResult
+    private static func saveLastGatewayConnectionData(_ payload: LastGatewayConnectionData) -> Bool {
+        guard let data = try? JSONEncoder().encode(payload),
+              let json = String(data: data, encoding: .utf8)
+        else { return false }
+        return KeychainStore.saveString(
+            json, service: self.gatewayService, account: self.lastGatewayConnectionAccount)
+    }
+
+    /// Migrate legacy UserDefaults gateway.last.* keys into a single Keychain entry.
+    private static func migrateLastGatewayFromUserDefaultsIfNeeded() {
        let defaults = UserDefaults.standard
        let stableID = defaults.string(forKey: self.lastGatewayStableIDDefaultsKey)?
            .trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
-        guard !stableID.isEmpty else { return nil }
+        guard !stableID.isEmpty else { return }
+
+        // Already migrated if Keychain entry exists.
+        if KeychainStore.loadString(
+            service: self.gatewayService, account: self.lastGatewayConnectionAccount) != nil
+        {
+            // Clean up legacy keys.
+            self.removeLastGatewayDefaults(defaults)
+            return
+        }
+
        let useTLS = defaults.bool(forKey: self.lastGatewayTlsDefaultsKey)
        let kindRaw = defaults.string(forKey: self.lastGatewayKindDefaultsKey)?
            .trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
        let kind = LastGatewayKind(rawValue: kindRaw) ?? .manual
-
-        if kind == .discovered {
-            return .discovered(stableID: stableID, useTLS: useTLS)
-        }
-
        let host = defaults.string(forKey: self.lastGatewayHostDefaultsKey)?
-            .trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
-        let port = defaults.integer(forKey: self.lastGatewayPortDefaultsKey)
+            .trimmingCharacters(in: .whitespacesAndNewlines)
+        let port = defaults.object(forKey: self.lastGatewayPortDefaultsKey) as? Int

-        // Back-compat: older builds persisted manual-style host/port without a kind marker.
-        guard !host.isEmpty, port > 0, port <= 65535 else { return nil }
-        return .manual(host: host, port: port, useTLS: useTLS, stableID: stableID)
+        let payload = LastGatewayConnectionData(
+            kind: kind, stableID: stableID, useTLS: useTLS,
+            host: kind == .manual ? host : nil,
+            port: kind == .manual ? port : nil)
+        guard self.saveLastGatewayConnectionData(payload) else { return }
+        self.removeLastGatewayDefaults(defaults)
    }

-    static func clearLastGatewayConnection(defaults: UserDefaults = .standard) {
+    private static func removeLastGatewayDefaults(_ defaults: UserDefaults) {
        defaults.removeObject(forKey: self.lastGatewayKindDefaultsKey)
        defaults.removeObject(forKey: self.lastGatewayHostDefaultsKey)
        defaults.removeObject(forKey: self.lastGatewayPortDefaultsKey)
@@ -355,9 +411,15 @@ enum GatewayDiagnostics {
    private static let maxLogBytes: Int64 = 512 * 1024
    private static let keepLogBytes: Int64 = 256 * 1024
    private static let logSizeCheckEveryWrites = 50
-    nonisolated(unsafe) private static var logWritesSinceCheck = 0
+    private static let logWritesSinceCheck = OSAllocatedUnfairLock(initialState: 0)
+    private static let isoFormatter: ISO8601DateFormatter = {
+        let f = ISO8601DateFormatter()
+        f.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
+        return f
+    }()
+
    private static var fileURL: URL? {
-        FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first?
+        FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask).first?
            .appendingPathComponent("openclaw-gateway.log")
    }

@@ -404,32 +466,41 @@ enum GatewayDiagnostics {
        }
    }

+    private static func applyFileProtection(url: URL) {
+        try? FileManager.default.setAttributes(
+            [.protectionKey: FileProtectionType.completeUntilFirstUserAuthentication],
+            ofItemAtPath: url.path)
+    }
+
    static func bootstrap() {
        guard let url = fileURL else { return }
        queue.async {
            self.truncateLogIfNeeded(url: url)
-            let formatter = ISO8601DateFormatter()
-            formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
-            let timestamp = formatter.string(from: Date())
+            let timestamp = self.isoFormatter.string(from: Date())
            let line = "[\(timestamp)] gateway diagnostics started\n"
            if let data = line.data(using: .utf8) {
                self.appendToLog(url: url, data: data)
+                self.applyFileProtection(url: url)
            }
        }
    }

    static func log(_ message: String) {
-        let formatter = ISO8601DateFormatter()
-        formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
-        let timestamp = formatter.string(from: Date())
+        let timestamp = self.isoFormatter.string(from: Date())
        let line = "[\(timestamp)] \(message)"
        logger.info("\(line, privacy: .public)")

        guard let url = fileURL else { return }
        queue.async {
-            self.logWritesSinceCheck += 1
-            if self.logWritesSinceCheck >= self.logSizeCheckEveryWrites {
-                self.logWritesSinceCheck = 0
+            let shouldTruncate = self.logWritesSinceCheck.withLock { count in
+                count += 1
+                if count >= self.logSizeCheckEveryWrites {
+                    count = 0
+                    return true
+                }
+                return false
+            }
+            if shouldTruncate {
                self.truncateLogIfNeeded(url: url)
            }
            let entry = line + "\n"
--- a/apps/ios/Sources/Gateway/KeychainStore.swift
+++ b/apps/ios/Sources/Gateway/KeychainStore.swift
@@ -1,48 +1,16 @@
 import Foundation
-import Security
+import OpenClawKit

 enum KeychainStore {
    static func loadString(service: String, account: String) -> String? {
-        let query: [String: Any] = [
-            kSecClass as String: kSecClassGenericPassword,
-            kSecAttrService as String: service,
-            kSecAttrAccount as String: account,
-            kSecReturnData as String: true,
-            kSecMatchLimit as String: kSecMatchLimitOne,
-        ]
-
-        var item: CFTypeRef?
-        let status = SecItemCopyMatching(query as CFDictionary, &item)
-        guard status == errSecSuccess, let data = item as? Data else { return nil }
-        return String(data: data, encoding: .utf8)
+        GenericPasswordKeychainStore.loadString(service: service, account: account)
    }

    static func saveString(_ value: String, service: String, account: String) -> Bool {
-        let data = Data(value.utf8)
-        let query: [String: Any] = [
-            kSecClass as String: kSecClassGenericPassword,
-            kSecAttrService as String: service,
-            kSecAttrAccount as String: account,
-        ]
-
-        let update: [String: Any] = [kSecValueData as String: data]
-        let status = SecItemUpdate(query as CFDictionary, update as CFDictionary)
-        if status == errSecSuccess { return true }
-        if status != errSecItemNotFound { return false }
-
-        var insert = query
-        insert[kSecValueData as String] = data
-        insert[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
-        return SecItemAdd(insert as CFDictionary, nil) == errSecSuccess
+        GenericPasswordKeychainStore.saveString(value, service: service, account: account)
    }

    static func delete(service: String, account: String) -> Bool {
-        let query: [String: Any] = [
-            kSecClass as String: kSecClassGenericPassword,
-            kSecAttrService as String: service,
-            kSecAttrAccount as String: account,
-        ]
-        let status = SecItemDelete(query as CFDictionary)
-        return status == errSecSuccess || status == errSecItemNotFound
+        GenericPasswordKeychainStore.delete(service: service, account: account)
    }
 }
--- a/apps/ios/Tests/GatewayConnectionControllerTests.swift
+++ b/apps/ios/Tests/GatewayConnectionControllerTests.swift
@@ -71,18 +71,37 @@ import UIKit
    }

    @Test @MainActor func loadLastConnectionReadsSavedValues() {
-        withUserDefaults([:]) {
-            GatewaySettingsStore.saveLastGatewayConnectionManual(
-                host: "gateway.example.com",
-                port: 443,
-                useTLS: true,
-                stableID: "manual|gateway.example.com|443")
-            let loaded = GatewaySettingsStore.loadLastGatewayConnection()
-            #expect(loaded == .manual(host: "gateway.example.com", port: 443, useTLS: true, stableID: "manual|gateway.example.com|443"))
+        let prior = KeychainStore.loadString(service: "ai.openclaw.gateway", account: "lastConnection")
+        defer {
+            if let prior {
+                _ = KeychainStore.saveString(prior, service: "ai.openclaw.gateway", account: "lastConnection")
+            } else {
+                _ = KeychainStore.delete(service: "ai.openclaw.gateway", account: "lastConnection")
+            }
        }
+        _ = KeychainStore.delete(service: "ai.openclaw.gateway", account: "lastConnection")
+
+        GatewaySettingsStore.saveLastGatewayConnectionManual(
+            host: "gateway.example.com",
+            port: 443,
+            useTLS: true,
+            stableID: "manual|gateway.example.com|443")
+        let loaded = GatewaySettingsStore.loadLastGatewayConnection()
+        #expect(loaded == .manual(host: "gateway.example.com", port: 443, useTLS: true, stableID: "manual|gateway.example.com|443"))
    }

    @Test @MainActor func loadLastConnectionReturnsNilForInvalidData() {
+        let prior = KeychainStore.loadString(service: "ai.openclaw.gateway", account: "lastConnection")
+        defer {
+            if let prior {
+                _ = KeychainStore.saveString(prior, service: "ai.openclaw.gateway", account: "lastConnection")
+            } else {
+                _ = KeychainStore.delete(service: "ai.openclaw.gateway", account: "lastConnection")
+            }
+        }
+        _ = KeychainStore.delete(service: "ai.openclaw.gateway", account: "lastConnection")
+
+        // Plant legacy UserDefaults with invalid host/port to exercise migration + validation.
        withUserDefaults([
            "gateway.last.kind": "manual",
            "gateway.last.host": "",
--- a/apps/ios/Tests/GatewaySettingsStoreTests.swift
+++ b/apps/ios/Tests/GatewaySettingsStoreTests.swift
@@ -27,6 +27,7 @@ private let lastGatewayDefaultsKeys = [
    "gateway.last.tls",
    "gateway.last.stableID",
 ]
+private let lastGatewayKeychainEntry = KeychainEntry(service: gatewayService, account: "lastConnection")

 private func snapshotDefaults(_ keys: [String]) -> [String: Any?] {
    let defaults = UserDefaults.standard
@@ -84,9 +85,13 @@ private func withBootstrapSnapshots(_ body: () -> Void) {
    body()
 }

-private func withLastGatewayDefaultsSnapshot(_ body: () -> Void) {
-    let snapshot = snapshotDefaults(lastGatewayDefaultsKeys)
-    defer { restoreDefaults(snapshot) }
+private func withLastGatewaySnapshot(_ body: () -> Void) {
+    let defaultsSnapshot = snapshotDefaults(lastGatewayDefaultsKeys)
+    let keychainSnapshot = snapshotKeychain([lastGatewayKeychainEntry])
+    defer {
+        restoreDefaults(defaultsSnapshot)
+        restoreKeychain(keychainSnapshot)
+    }
    body()
 }

@@ -135,7 +140,7 @@ private func withLastGatewayDefaultsSnapshot(_ body: () -> Void) {
    }

    @Test func lastGateway_manualRoundTrip() {
-        withLastGatewayDefaultsSnapshot {
+        withLastGatewaySnapshot {
            GatewaySettingsStore.saveLastGatewayConnectionManual(
                host: "example.com",
                port: 443,
@@ -147,28 +152,24 @@ private func withLastGatewayDefaultsSnapshot(_ body: () -> Void) {
        }
    }

-    @Test func lastGateway_discoveredDoesNotPersistResolvedHostPort() {
-        withLastGatewayDefaultsSnapshot {
-            // Simulate a prior manual record that included host/port.
-            applyDefaults([
-                "gateway.last.host": "10.0.0.99",
-                "gateway.last.port": 18789,
-                "gateway.last.tls": true,
-                "gateway.last.stableID": "manual|10.0.0.99|18789",
-                "gateway.last.kind": "manual",
-            ])
+    @Test func lastGateway_discoveredOverwritesManual() {
+        withLastGatewaySnapshot {
+            GatewaySettingsStore.saveLastGatewayConnectionManual(
+                host: "10.0.0.99",
+                port: 18789,
+                useTLS: true,
+                stableID: "manual|10.0.0.99|18789")

            GatewaySettingsStore.saveLastGatewayConnectionDiscovered(stableID: "gw|abc", useTLS: true)

-            let defaults = UserDefaults.standard
-            #expect(defaults.object(forKey: "gateway.last.host") == nil)
-            #expect(defaults.object(forKey: "gateway.last.port") == nil)
            #expect(GatewaySettingsStore.loadLastGatewayConnection() == .discovered(stableID: "gw|abc", useTLS: true))
        }
    }

-    @Test func lastGateway_backCompat_manualLoadsWhenKindMissing() {
-        withLastGatewayDefaultsSnapshot {
+    @Test func lastGateway_migratesFromUserDefaults() {
+        withLastGatewaySnapshot {
+            // Clear Keychain entry and plant legacy UserDefaults values.
+            applyKeychain([lastGatewayKeychainEntry: nil])
            applyDefaults([
                "gateway.last.kind": nil,
                "gateway.last.host": "example.org",
@@ -179,6 +180,11 @@ private func withLastGatewayDefaultsSnapshot(_ body: () -> Void) {

            let loaded = GatewaySettingsStore.loadLastGatewayConnection()
            #expect(loaded == .manual(host: "example.org", port: 18789, useTLS: false, stableID: "manual|example.org|18789"))
+
+            // Legacy keys should be cleaned up after migration.
+            let defaults = UserDefaults.standard
+            #expect(defaults.object(forKey: "gateway.last.stableID") == nil)
+            #expect(defaults.object(forKey: "gateway.last.host") == nil)
        }
    }