feat(ios-keychain): opt-in iCloud Keychain sync for SSH keys (#52)

Reddit-reported friction: every iOS device needed its own SSH key
because Scarf hardcoded
kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly +
kSecAttrSynchronizable=false on every Keychain write. Pairing iPhone
+ iPad meant onboarding twice and editing authorized_keys per device.

Add an opt-in toggle in System tab → Security:

- New SSHKeyICloudPreference (UserDefaults wrapper, default false so
  existing installs see no change on update).
- KeychainSSHKeyStore.writeBundle now consults the preference: when
  on, items use kSecAttrAccessibleAfterFirstUnlock (no ThisDeviceOnly
  suffix — required for iCloud Keychain sync) +
  kSecAttrSynchronizable=true.
- All read / list / delete queries unconditionally pass
  kSecAttrSynchronizable=kSecAttrSynchronizableAny so they match
  items regardless of sync state. Without this a flipped write would
  orphan items at the next read.
- Public migrateAllItems(toICloudSync:) reads every stored bundle,
  deletes with Any, re-saves with target attributes. Idempotent.

System tab Security section toggle:
- Live migration on flip with a "Updating Keychain..." progress row.
- Failure path reverts the toggle + surfaces the error inline rather
  than silently leaving the state inconsistent.
- Footer copy explains the tradeoff (E2EE via iCloud Keychain;
  Advanced Data Protection keeps encryption keys on device).

Out of scope: per-server-key sync override (M9 multi-server keys
all sync or none); in-app key export.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

scarf/Packages/ScarfIOS/Sources/ScarfIOS/KeychainSSHKeyStore.swift

@@ -17,9 +17,18 @@ import ScarfCore
 ///   go here; v1 item is migrated into v2 on first `listAll()` after
 ///   the upgrade, then removed.
 ///
-/// All items use `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`
+/// **Accessibility / sync attributes.** Default behavior pins items
-/// so they're reachable after a single device unlock (background
+/// to this device with `kSecAttrAccessibleAfterFirstUnlockThisDevice
-/// tasks, notification actions) but never sync to iCloud Keychain.
+/// Only` + `kSecAttrSynchronizable=false`. Users can opt into iCloud
+/// Keychain sync via `SSHKeyICloudPreference` (issue #52); when
+/// enabled, writes use `kSecAttrAccessibleAfterFirstUnlock` (no
+/// `ThisDeviceOnly` suffix) + `kSecAttrSynchronizable=true` so the
+/// key is picked up by iCloud Keychain on every signed-in device.
+///
+/// All read / list / delete queries pass `kSecAttrSynchronizable =
+/// kSecAttrSynchronizableAny` so they match items regardless of
+/// sync state — load-bearing during the migration window when
+/// device-only and synced items can briefly coexist.
 public struct KeychainSSHKeyStore: SSHKeyStore {
     public static let defaultService = "com.scarf.ssh-key"
     public static let legacyV1Account = "primary"
@@ -56,10 +65,12 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
 
     public func delete() async throws {
         // Wipe every v2 entry + the legacy v1 entry. Single-query delete
-        // that matches any account under our service.
+        // that matches any account under our service. Pass `Any` so the
+        // wipe catches synced + device-only items uniformly (issue #52).
         let query: [String: Any] = [
-            kSecClass as String:        kSecClassGenericPassword,
+            kSecClass as String:                   kSecClassGenericPassword,
-            kSecAttrService as String:  service,
+            kSecAttrService as String:             service,
+            kSecAttrSynchronizable as String:      kSecAttrSynchronizableAny,
         ]
         let status = SecItemDelete(query as CFDictionary)
         if status != errSecSuccess && status != errSecItemNotFound {
@@ -74,10 +85,13 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
     public func listAll() async throws -> [ServerID] {
         migrateLegacyIfNeeded()
         let query: [String: Any] = [
-            kSecClass as String:             kSecClassGenericPassword,
+            kSecClass as String:                   kSecClassGenericPassword,
-            kSecAttrService as String:       service,
+            kSecAttrService as String:             service,
-            kSecReturnAttributes as String:  true,
+            kSecReturnAttributes as String:        true,
-            kSecMatchLimit as String:        kSecMatchLimitAll,
+            kSecMatchLimit as String:              kSecMatchLimitAll,
+            // Match items regardless of sync state (issue #52). Without
+            // this the listing silently misses synced items.
+            kSecAttrSynchronizable as String:      kSecAttrSynchronizableAny,
         ]
         var items: CFTypeRef?
         let status = SecItemCopyMatching(query as CFDictionary, &items)
@@ -115,15 +129,60 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
         try deleteBundle(account: Self.multiAccountPrefix + id.uuidString)
     }
 
+    // MARK: - iCloud sync migration (issue #52)
+
+    /// Migrate every stored key bundle to the requested sync state and
+    /// persist the user's preference for future writes.
+    ///
+    /// Idempotent: if the user enables sync twice in a row the second
+    /// call simply re-saves with the same attributes. Safe to call
+    /// from a UI toggle handler. Errors thrown by individual key
+    /// re-writes propagate; partial migrations are tolerable because
+    /// the read paths use `kSecAttrSynchronizableAny` and pick up
+    /// either copy on the next read.
+    ///
+    /// Side effects:
+    /// - Each stored key is read with `Any`, deleted with `Any`, then
+    ///   re-saved with the target sync attributes via `writeBundle(_:account:syncToICloud:)`.
+    /// - The legacy v1 entry (if present) is migrated to the v2 layout
+    ///   with the new attributes in passing.
+    /// - `SSHKeyICloudPreference.isEnabled` is set BEFORE the rewrite
+    ///   loop so any concurrent `save(_:)` call from another path
+    ///   already uses the right attributes.
+    public func migrateAllItems(toICloudSync enabled: Bool) async throws {
+        SSHKeyICloudPreference.isEnabled = enabled
+
+        // Pull every v2 + v1 bundle into memory first. We can't iterate
+        // and rewrite simultaneously: deleting an item we're about to
+        // re-add would race with the listing query.
+        var bundles: [(account: String, bundle: SSHKeyBundle)] = []
+        for id in try await listAll() {
+            if let bundle = try await load(for: id) {
+                bundles.append((Self.multiAccountPrefix + id.uuidString, bundle))
+            }
+        }
+        if let legacy = try? readLegacy() {
+            bundles.append((Self.legacyV1Account, legacy))
+        }
+
+        for (account, bundle) in bundles {
+            try writeBundle(bundle, account: account, syncToICloud: enabled)
+        }
+    }
+
     // MARK: - Private — Keychain plumbing per-account
 
     private func readBundle(account: String) throws -> SSHKeyBundle? {
         let query: [String: Any] = [
-            kSecClass as String:            kSecClassGenericPassword,
+            kSecClass as String:                   kSecClassGenericPassword,
-            kSecAttrService as String:      service,
+            kSecAttrService as String:             service,
-            kSecAttrAccount as String:      account,
+            kSecAttrAccount as String:             account,
-            kSecReturnData as String:       true,
+            kSecReturnData as String:              true,
-            kSecMatchLimit as String:       kSecMatchLimitOne,
+            kSecMatchLimit as String:              kSecMatchLimitOne,
+            // Match items regardless of sync state (issue #52). Without
+            // this the query implicitly defaults to false and orphans
+            // any items that have been migrated to iCloud sync.
+            kSecAttrSynchronizable as String:      kSecAttrSynchronizableAny,
         ]
         var item: CFTypeRef?
         let status = SecItemCopyMatching(query as CFDictionary, &item)
@@ -149,6 +208,13 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
     }
 
     private func writeBundle(_ bundle: SSHKeyBundle, account: String) throws {
+        try writeBundle(bundle, account: account, syncToICloud: SSHKeyICloudPreference.isEnabled)
+    }
+
+    /// Write path with explicit sync control. Used by the public
+    /// migration helper to force a target sync state regardless of
+    /// the current preference.
+    private func writeBundle(_ bundle: SSHKeyBundle, account: String, syncToICloud: Bool) throws {
         let data: Data
         do {
             data = try JSONEncoder().encode(bundle)
@@ -157,17 +223,34 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
                 message: "Encode failed: \(error.localizedDescription)", osStatus: nil
             )
         }
-        let baseQuery: [String: Any] = [
+        // Delete with kSecAttrSynchronizableAny to clear out any prior
+        // copy regardless of its sync state — without this a flip from
+        // synced → device-only could leave the synced copy behind and
+        // create two competing items at the same (service, account).
+        let deleteQuery: [String: Any] = [
+            kSecClass as String:                   kSecClassGenericPassword,
+            kSecAttrService as String:             service,
+            kSecAttrAccount as String:             account,
+            kSecAttrSynchronizable as String:      kSecAttrSynchronizableAny,
+        ]
+        SecItemDelete(deleteQuery as CFDictionary)
+
+        var attributes: [String: Any] = [
             kSecClass as String:        kSecClassGenericPassword,
             kSecAttrService as String:  service,
             kSecAttrAccount as String:  account,
         ]
-        SecItemDelete(baseQuery as CFDictionary)
-
-        var attributes = baseQuery
         attributes[kSecValueData as String] = data
-        attributes[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
+        if syncToICloud {
-        attributes[kSecAttrSynchronizable as String] = kCFBooleanFalse
+            // iCloud Keychain requires the non-`ThisDeviceOnly` accessible
+            // class — items with the `ThisDeviceOnly` suffix are silently
+            // skipped by the sync engine.
+            attributes[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock
+            attributes[kSecAttrSynchronizable as String] = kCFBooleanTrue
+        } else {
+            attributes[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
+            attributes[kSecAttrSynchronizable as String] = kCFBooleanFalse
+        }
 
         let addStatus = SecItemAdd(attributes as CFDictionary, nil)
         guard addStatus == errSecSuccess else {
@@ -179,9 +262,10 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
 
     private func deleteBundle(account: String) throws {
         let query: [String: Any] = [
-            kSecClass as String:        kSecClassGenericPassword,
+            kSecClass as String:                   kSecClassGenericPassword,
-            kSecAttrService as String:  service,
+            kSecAttrService as String:             service,
-            kSecAttrAccount as String:  account,
+            kSecAttrAccount as String:             account,
+            kSecAttrSynchronizable as String:      kSecAttrSynchronizableAny,
         ]
         let status = SecItemDelete(query as CFDictionary)
         if status != errSecSuccess && status != errSecItemNotFound {
@@ -217,10 +301,13 @@ public struct KeychainSSHKeyStore: SSHKeyStore {
     /// triggering a recursive migration.
     private func listAllInternal(skipMigration: Bool) throws -> [ServerID] {
         let query: [String: Any] = [
-            kSecClass as String:             kSecClassGenericPassword,
+            kSecClass as String:                   kSecClassGenericPassword,
-            kSecAttrService as String:       service,
+            kSecAttrService as String:             service,
-            kSecReturnAttributes as String:  true,
+            kSecReturnAttributes as String:        true,
-            kSecMatchLimit as String:        kSecMatchLimitAll,
+            kSecMatchLimit as String:              kSecMatchLimitAll,
+            // Match items regardless of sync state (issue #52). Without
+            // this the listing silently misses synced items.
+            kSecAttrSynchronizable as String:      kSecAttrSynchronizableAny,
         ]
         var items: CFTypeRef?
         let status = SecItemCopyMatching(query as CFDictionary, &items)

scarf/Packages/ScarfIOS/Sources/ScarfIOS/SSHKeyICloudPreference.swift

@@ -0,0 +1,39 @@
+// Apple-only: Security.framework + UserDefaults are iOS/Mac only.
+// On Linux this file is skipped; tests don't exercise it.
+#if canImport(Security)
+
+import Foundation
+
+/// Device-local preference: should the SSH key bundle stored in the
+/// iOS Keychain sync to iCloud Keychain (issue #52)?
+///
+/// **Default `false`.** Existing installs see no change on update; the
+/// key remains pinned to the device with `kSecAttrAccessibleAfter
+/// FirstUnlockThisDeviceOnly` + `kSecAttrSynchronizable=false`. Users
+/// who opt in via Settings → Security trigger a one-shot migration
+/// that re-saves all stored keys with `kSecAttrAccessibleAfterFirst
+/// Unlock` + `kSecAttrSynchronizable=true` so iCloud Keychain picks
+/// them up.
+///
+/// **Trade-off the UI must surface clearly.**
+/// - On: convenient multi-device — iPhone + iPad + Mac all see the
+///   same key. End-to-end encrypted by iCloud Keychain (Apple-managed
+///   keys without ADP, user-managed keys with ADP). Requires iCloud
+///   Keychain enabled on every device.
+/// - Off (default): key never leaves this device. Each device must
+///   onboard separately (generate its own key, append its pubkey to
+///   `authorized_keys`).
+public enum SSHKeyICloudPreference {
+
+    /// UserDefaults key. Stable string so a v2 future fix can read
+    /// existing values without migration.
+    public static let key = "scarf.icloud.syncSSHKey"
+
+    /// Read the current preference. Defaults to `false`.
+    public static var isEnabled: Bool {
+        get { UserDefaults.standard.bool(forKey: key) }
+        set { UserDefaults.standard.set(newValue, forKey: key) }
+    }
+}
+
+#endif // canImport(Security)

scarf/Scarf iOS/App/ScarfGoTabRoot.swift

@@ -138,6 +138,13 @@ private struct SystemTab: View {
     @State private var showForgetConfirmation = false
     @State private var isForgetting = false
     @State private var isDisconnecting = false
+    /// Mirror of `SSHKeyICloudPreference.isEnabled` — drives the iCloud
+    /// Keychain sync toggle (issue #52). Initial value is read on view
+    /// init so the toggle reflects today's preference before the user
+    /// taps anything; flipping triggers `migrateAllItems(toICloudSync:)`.
+    @State private var iCloudSyncEnabled: Bool = SSHKeyICloudPreference.isEnabled
+    @State private var iCloudMigrationInFlight = false
+    @State private var iCloudMigrationError: String?
 
     var body: some View {
         List {
@@ -178,6 +185,67 @@ private struct SystemTab: View {
                 .listRowBackground(ScarfColor.backgroundSecondary)
             }
 
+            Section {
+                Toggle(isOn: $iCloudSyncEnabled) {
+                    HStack(spacing: 10) {
+                        Image(systemName: "key.icloud.fill")
+                            .foregroundStyle(.tint)
+                        VStack(alignment: .leading, spacing: 2) {
+                            Text("Sync SSH key with iCloud Keychain")
+                            Text(iCloudSyncEnabled
+                                 ? "Synced — your other Apple devices with iCloud Keychain will see this key."
+                                 : "This device only — generate a separate key on each device.")
+                                .font(.caption)
+                                .foregroundStyle(ScarfColor.foregroundMuted)
+                        }
+                    }
+                }
+                .tint(ScarfColor.accent)
+                .disabled(iCloudMigrationInFlight)
+                .onChange(of: iCloudSyncEnabled) { _, newValue in
+                    Task {
+                        iCloudMigrationInFlight = true
+                        iCloudMigrationError = nil
+                        defer { iCloudMigrationInFlight = false }
+                        do {
+                            try await KeychainSSHKeyStore().migrateAllItems(toICloudSync: newValue)
+                        } catch {
+                            // Revert the toggle on failure so the UI
+                            // reflects what's actually in the Keychain;
+                            // surface the error inline so the user can
+                            // retry / report. Keychain failures here are
+                            // rare (typically `errSecDuplicateItem` if a
+                            // prior migration was interrupted — the
+                            // delete-with-Any in writeBundle prevents
+                            // that, but we still belt-and-brace).
+                            iCloudMigrationError = error.localizedDescription
+                            iCloudSyncEnabled = !newValue
+                            SSHKeyICloudPreference.isEnabled = !newValue
+                        }
+                    }
+                }
+                if iCloudMigrationInFlight {
+                    HStack(spacing: 8) {
+                        ProgressView()
+                            .controlSize(.small)
+                        Text("Updating Keychain…")
+                            .font(.caption)
+                            .foregroundStyle(ScarfColor.foregroundMuted)
+                    }
+                }
+                if let err = iCloudMigrationError {
+                    Label(err, systemImage: "exclamationmark.triangle.fill")
+                        .font(.caption)
+                        .foregroundStyle(ScarfColor.warning)
+                }
+            } header: {
+                Text("Security")
+            } footer: {
+                Text("End-to-end encrypted via iCloud Keychain. With Advanced Data Protection on, the encryption keys never leave your devices. Toggle off to keep the key device-only — each new device must onboard separately.")
+                    .font(.caption)
+            }
+            .listRowBackground(ScarfColor.backgroundSecondary)
+
             Section {
                 Button {
                     Task {