apple_kit

commit ba96eda9ff5d50bdb07d183690dca3676f1819f9
parent fe759666a3e6165330e492eb7320d0b74f894473
Author: triesap <tyson@radroots.org>
Date:   Fri, 19 Jun 2026 16:02:01 -0700

secure-store: harden keychain replacement semantics

- replace delete-before-add writes with add-or-update keychain mutation.
- prepare access-control attributes before mutating stored secrets.
- add regression coverage for failed replacement preserving old data.
- keep secure-store replacement behavior local to RadrootsKit.

Diffstat:
M
Sources/RadrootsKit/RadrootsAppleKeychainSecureStore.swift
 | 
58
+++++++++++++++++++++++++++++++++++++++++++++-------------
M
Tests/RadrootsKitTests/RadrootsSecureStoreTests.swift
 | 
33
+++++++++++++++++++++++++++++++++

2 files changed, 78 insertions(+), 13 deletions(-)
diff --git a/Sources/RadrootsKit/RadrootsAppleKeychainSecureStore.swift b/Sources/RadrootsKit/RadrootsAppleKeychainSecureStore.swift
@@ -3,9 +3,19 @@ import Security
 
 public final class RadrootsAppleKeychainSecureStore: RadrootsSecureStore, @unchecked Sendable {
     public let servicePrefix: String
+    private let accessControlFactory: (RadrootsKeychainSecretPolicyMapping) throws -> SecAccessControl
 
     public init(servicePrefix: String = "org.radroots.kit.secure-store") {
         self.servicePrefix = servicePrefix
+        self.accessControlFactory = Self.makeAccessControl(for:)
+    }
+
+    init(
+        servicePrefix: String = "org.radroots.kit.secure-store",
+        accessControlFactory: @escaping (RadrootsKeychainSecretPolicyMapping) throws -> SecAccessControl
+    ) {
+        self.servicePrefix = servicePrefix
+        self.accessControlFactory = accessControlFactory
     }
 
     public func put(
@@ -13,21 +23,23 @@ public final class RadrootsAppleKeychainSecureStore: RadrootsSecureStore, @unche
         for key: RadrootsSecureStoreKey,
         policy: RadrootsSecretAccessPolicy = .secureLocalSecret
     ) throws {
-        try delete(key)
-
-        var query = try baseQuery(for: key)
-        query[kSecValueData as String] = value
-
-        let mapping = keychainPolicyMapping(for: policy)
-        if mapping.usesAccessControl {
-            query[kSecAttrAccessControl as String] = try accessControl(for: mapping)
-        } else {
-            query[kSecAttrAccessible as String] = mapping.accessibilityConstant
+        let attributes = try mutationAttributes(value, policy: policy)
+        var addQuery = try baseQuery(for: key)
+        addQuery.merge(attributes) { _, new in new }
+
+        let addStatus = SecItemAdd(addQuery as CFDictionary, nil)
+        switch addStatus {
+        case errSecSuccess:
+            return
+        case errSecDuplicateItem:
+            break
+        default:
+            throw Self.mapStatus(addStatus, defaultMessage: "keychain write failed")
         }
 
-        let status = SecItemAdd(query as CFDictionary, nil)
-        guard status == errSecSuccess else {
-            throw Self.mapStatus(status, defaultMessage: "keychain write failed")
+        let updateStatus = SecItemUpdate(try baseQuery(for: key) as CFDictionary, attributes as CFDictionary)
+        guard updateStatus == errSecSuccess else {
+            throw Self.mapStatus(updateStatus, defaultMessage: "keychain update failed")
         }
     }
 
@@ -123,6 +135,26 @@ public final class RadrootsAppleKeychainSecureStore: RadrootsSecureStore, @unche
     }
 
     func accessControl(for mapping: RadrootsKeychainSecretPolicyMapping) throws -> SecAccessControl {
+        try accessControlFactory(mapping)
+    }
+
+    private func mutationAttributes(
+        _ value: Data,
+        policy: RadrootsSecretAccessPolicy
+    ) throws -> [String: Any] {
+        let mapping = keychainPolicyMapping(for: policy)
+        var attributes: [String: Any] = [
+            kSecValueData as String: value
+        ]
+        if mapping.usesAccessControl {
+            attributes[kSecAttrAccessControl as String] = try accessControl(for: mapping)
+        } else {
+            attributes[kSecAttrAccessible as String] = mapping.accessibilityConstant
+        }
+        return attributes
+    }
+
+    private static func makeAccessControl(for mapping: RadrootsKeychainSecretPolicyMapping) throws -> SecAccessControl {
         var error: Unmanaged<CFError>?
         guard let accessControl = SecAccessControlCreateWithFlags(
             nil,
diff --git a/Tests/RadrootsKitTests/RadrootsSecureStoreTests.swift b/Tests/RadrootsKitTests/RadrootsSecureStoreTests.swift
@@ -84,6 +84,39 @@ import Testing
     #expect(try store.contains(key) == false)
 }
 
+@Test func keychainStoreReplacesExistingSecret() throws {
+    let store = RadrootsAppleKeychainSecureStore(
+        servicePrefix: "org.radroots.tests.\(UUID().uuidString)"
+    )
+    let key = RadrootsSecureStoreKey(namespace: "replacement", name: "token")
+
+    try store.put(Data("old-secret".utf8), for: key)
+    try store.put(Data("new-secret".utf8), for: key)
+
+    #expect(try store.get(key) == Data("new-secret".utf8))
+    try store.delete(key)
+}
+
+@Test func keychainStorePreservesExistingSecretWhenReplacementPreparationFails() throws {
+    let servicePrefix = "org.radroots.tests.\(UUID().uuidString)"
+    let key = RadrootsSecureStoreKey(namespace: "replacement-failure", name: "token")
+    let store = RadrootsAppleKeychainSecureStore(servicePrefix: servicePrefix)
+    let failingStore = RadrootsAppleKeychainSecureStore(
+        servicePrefix: servicePrefix,
+        accessControlFactory: { _ in
+            throw RadrootsAppleSecurityError.invalidRequest("forced access control failure")
+        }
+    )
+
+    try store.put(Data("old-secret".utf8), for: key)
+    #expect(throws: RadrootsAppleSecurityError.self) {
+        try failingStore.put(Data("new-secret".utf8), for: key, policy: .userPresenceLocalSecret)
+    }
+
+    #expect(try store.get(key) == Data("old-secret".utf8))
+    try store.delete(key)
+}
+
 @Test func secureLocalSecretMapsToDeviceLocalWhenUnlockedKeychainPolicy() {
     let store = RadrootsAppleKeychainSecureStore()
     let mapping = store.keychainPolicyMapping(for: .secureLocalSecret)