Authenticate independent key profiles and revoke before policy writes

Sources/ClavisCore/Crypto/KeychainManager.swift

@@ -78,12 +78,13 @@ public class KeychainManager {
         algorithm: String = "ECDSA P-256",
         storageType: KeyStorageType = .secureEnclave,
         biometricPolicy: BiometricPolicy? = nil,
-        keyPurpose: KeyPurpose = .general
+        keyPurpose: KeyPurpose = .general,
+        accessProfile: KeyAccessProfile? = nil
     ) throws -> SSHKeyInfo {
         #if DEBUG
         if let context = self.testContext, TestContextScope.current == nil {
             return try TestContextScope.$current.withValue(context) {
-                try self.generateKey(label: label, algorithm: algorithm, storageType: storageType, biometricPolicy: biometricPolicy, keyPurpose: keyPurpose)
+                try self.generateKey(label: label, algorithm: algorithm, storageType: storageType, biometricPolicy: biometricPolicy, keyPurpose: keyPurpose, accessProfile: accessProfile)
             }
         }
         #endif
@@ -113,6 +114,7 @@ public class KeychainManager {
             storageType: storageType,
             biometricPolicy: policy,
             keyPurpose: keyPurpose,
+            accessProfile: accessProfile,
             keyData: keyPair.keyData
         )
         defer { record.wipe() }
@@ -144,7 +146,8 @@ public class KeychainManager {
             algorithmName: algorithm,
             storage: storageType,
             biometricPolicy: policy,
-            keyPurpose: keyPurpose
+            keyPurpose: keyPurpose,
+            accessProfile: record.accessProfile
         )
         do {
             try PublicKeyStore.saveChecked(keyInfo)
@@ -158,7 +161,7 @@ public class KeychainManager {
                 type: .keyCreate,
                 result: .allowed,
                 keyFingerprint: keyInfo.fingerprint,
-                keyKind: .personal,
+                keyKind: record.accessProfile.type == .bot ? .agent : .personal,
                 sensitive: AuditSensitive(keyLabel: keyInfo.label, processChain: [], host: nil)
             )
         )
@@ -825,62 +828,8 @@ public class KeychainManager {
                 throw KeyPurposeError.kindChangeNotAllowed
             }
 
-            var updatedRecord = StoredPrivateKeyRecord(
-                label: record.label,
-                algorithm: record.algorithm,
-                storageType: record.storageType,
-                biometricPolicy: record.biometricPolicy,
-                keyPurpose: newPurpose,
-                accessProfile: record.accessProfile.changingType(to: newPurpose == .agent ? .bot : .personal),
-                keyData: record.keyData,
-                createdAt: record.createdAt
-            )
-            defer { updatedRecord.wipe() }
-
-            let hasVaultRecord = vaultStore.hasRecord(label: label)
-            if hasVaultRecord {
-                try vaultStore.saveRecord(updatedRecord)
-            }
-
-            var recordData = try updatedRecord.encode()
-            defer { Self.wipeData(&recordData) }
-
-            do {
-                try privateKeyStore.save(label: label, data: recordData, accessControlFlags: [])
-            } catch {
-                if hasVaultRecord {
-                    try? vaultStore.saveRecord(record)
-                }
-                throw error
-            }
-
-            let updatedKeyInfo = SSHKeyInfo(
-                label: key.label,
-                publicKeyOpenSSH: key.publicKeyOpenSSH,
-                publicKeyBlob: key.publicKeyBlob,
-                fingerprint: key.fingerprint,
-                createdAt: key.createdAt,
-                algorithmName: key.algorithmName,
-                storage: key.storageType,
-                biometricPolicy: key.biometricPolicy,
-                keyPurpose: newPurpose,
-                accessProfile: updatedRecord.accessProfile
-            )
-
-            do {
-                try PublicKeyStore.saveChecked(updatedKeyInfo)
-            } catch {
-                if var originalRecordData = try? record.encode() {
-                    defer { Self.wipeData(&originalRecordData) }
-                    try? privateKeyStore.save(label: label, data: originalRecordData, accessControlFlags: [])
-                }
-                if hasVaultRecord {
-                    try? vaultStore.saveRecord(record)
-                }
-                throw error
-            }
-
-            try revokeKeyCapabilities(label: label)
+            try persistAccessProfile(record.accessProfile.changingType(to: newPurpose == .agent ? .bot : .personal),
+                                     purpose: newPurpose, key: key, record: record)
 
             auditRecorder.record(
                 AuditEvent(
@@ -895,6 +844,94 @@ public class KeychainManager {
         }
     }
 
+    private func persistAccessProfile(_ profile: KeyAccessProfile, purpose newPurpose: KeyPurpose,
+                                      key: SSHKeyInfo, record: StoredPrivateKeyRecord) throws {
+        let label = key.label
+        // Never persist a broader policy while the daemon can still reuse old approvals.
+        // Revocation failure leaves all stored copies untouched.
+        try revokeKeyCapabilities(label: label)
+        var updatedRecord = StoredPrivateKeyRecord(
+            label: record.label,
+            algorithm: record.algorithm,
+            storageType: record.storageType,
+            biometricPolicy: record.biometricPolicy,
+            keyPurpose: newPurpose,
+            accessProfile: profile,
+            keyData: record.keyData,
+            createdAt: record.createdAt
+        )
+        defer { updatedRecord.wipe() }
+
+        let hasVaultRecord = vaultStore.hasRecord(label: label)
+        if hasVaultRecord {
+            try vaultStore.saveRecord(updatedRecord)
+        }
+
+        var recordData = try updatedRecord.encode()
+        defer { Self.wipeData(&recordData) }
+
+        do {
+            try privateKeyStore.save(label: label, data: recordData, accessControlFlags: [])
+        } catch {
+            if hasVaultRecord {
+                try? vaultStore.saveRecord(record)
+            }
+            throw error
+        }
+
+        let updatedKeyInfo = SSHKeyInfo(
+            label: key.label,
+            publicKeyOpenSSH: key.publicKeyOpenSSH,
+            publicKeyBlob: key.publicKeyBlob,
+            fingerprint: key.fingerprint,
+            createdAt: key.createdAt,
+            algorithmName: key.algorithmName,
+            storage: key.storageType,
+            biometricPolicy: key.biometricPolicy,
+            keyPurpose: newPurpose,
+            accessProfile: updatedRecord.accessProfile
+        )
+
+        do {
+            try PublicKeyStore.saveChecked(updatedKeyInfo)
+        } catch {
+            if var originalRecordData = try? record.encode() {
+                defer { Self.wipeData(&originalRecordData) }
+                try? privateKeyStore.save(label: label, data: originalRecordData, accessControlFlags: [])
+            }
+            if hasVaultRecord {
+                try? vaultStore.saveRecord(record)
+            }
+            throw error
+        }
+
+    }
+
+    /// A type-only change never changes operation capabilities or legacy Git restrictions.
+    /// Authentication cannot be supplied by the caller or reused from a previous unlock.
+    public func changeAccessProfile(label: String, type: KeyType? = nil,
+                                    capabilities: KeyCapabilities? = nil) throws {
+        guard let key = try fetchKeyInfo(label: label) else {
+            throw KeychainManagerError.keyNotFound(label: label)
+        }
+        let profile = KeyAccessProfile(type: type ?? key.effectiveAccessProfile.type,
+            capabilities: capabilities ?? key.effectiveAccessProfile.capabilities)
+        guard profile != key.effectiveAccessProfile else { return }
+        try withVerifiedPrivateKeyRecord(for: key,
+            prompt: "Change access policy for '\(label)'",
+            auditOnAuthFailure: true, auditKeyKind: profile.type == .bot ? .agent : .personal
+        ) { context, record in
+            defer { context.invalidate() }
+            // Retain Git-only restrictions unless the user explicitly enables SSH.
+            let purpose: KeyPurpose = record.purpose == .gitSigningOnly && !profile.capabilities.ssh
+                ? .gitSigningOnly : .general
+            try persistAccessProfile(profile, purpose: purpose, key: key, record: record)
+            auditRecorder.record(AuditEvent(type: .keyKindChange, result: .allowed,
+                keyFingerprint: key.fingerprint, keyKind: profile.type == .bot ? .agent : .personal,
+                sensitive: .init(keyLabel: label, processChain: [], host: nil)))
+        }
+    }
+
     /// Rename an existing key label across the private key store, shadow vault, and public key store.
     @discardableResult
     public func renameKey(from oldLabel: String, to newLabel: String) throws -> SSHKeyInfo {

Tests/ClavisCoreTests/KeyProfileMutationTests.swift

@@ -0,0 +1,143 @@
+import Foundation
+import LocalAuthentication
+import Testing
+@testable import ClavisCore
+@testable import ClavisTestSupport
+
+@Suite(.serialized) struct KeyProfileMutationTests {
+    private struct Rejecting: UserAuthenticating {
+        func authenticate(reason: String, policy: LAPolicy) throws -> LAContext { throw LAError(.userCancel) }
+        func authenticate(reason: String, policy: LAPolicy) async throws -> LAContext { throw LAError(.userCancel) }
+    }
+
+    @Test(arguments: KeyType.allCases)
+    func generationPersistsIndependentProfile(type: KeyType) async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let store = InMemoryPrivateKeyStore()
+            let manager = fixture.makeKeyManager(privateKeyStore: store)
+            let profile = KeyAccessProfile(type: type, capabilities: .init(ssh: false, signature: true))
+            let key = try manager.generateKey(label: "new-profile", accessProfile: profile)
+            let raw = try #require(try store.load(label: key.label, context: LAContext(), prompt: ""))
+            let record = try StoredPrivateKeyRecord.decode(from: raw)
+            #expect(record.accessProfile == profile)
+            #expect(key.effectiveAccessProfile == profile)
+            #expect(try manager.fetchKeyInfo(label: key.label)?.effectiveAccessProfile == profile)
+            #expect(throws: KeyAccessError.operationDisabled(.ssh)) {
+                try manager.signSSH(key: key, data: Data("ssh".utf8), prompt: "")
+            }
+        }
+    }
+
+    @Test(arguments: KeyPurpose.allCases)
+    func typeChangePreservesCapabilitiesAndMaterial(purpose: KeyPurpose) async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let store = InMemoryPrivateKeyStore()
+            let auth = CountingAuthenticator()
+            let manager = fixture.makeKeyManager(authenticator: auth, privateKeyStore: store)
+            let key = try manager.generateKey(label: "type-change", keyPurpose: purpose)
+            let raw = try #require(try store.load(label: key.label, context: LAContext(), prompt: ""))
+            let original = try StoredPrivateKeyRecord.decode(from: raw)
+            let target: KeyType = key.effectiveAccessProfile.type == .bot ? .personal : .bot
+            try manager.changeAccessProfile(label: key.label, type: target)
+            let changed = try #require(try manager.fetchKeyInfo(label: key.label))
+            let updated = try StoredPrivateKeyRecord.decode(from: #require(try store.load(label: key.label, context: LAContext(), prompt: "")))
+            #expect(auth.authenticationCount == 1)
+            #expect(changed.effectiveAccessProfile.type == target)
+            #expect(changed.effectiveAccessProfile.capabilities == key.effectiveAccessProfile.capabilities)
+            #expect(updated.accessProfile == changed.effectiveAccessProfile)
+            #expect(updated.keyData == original.keyData)
+            #expect(updated.createdAt == original.createdAt)
+            #expect(changed.fingerprint == key.fingerprint)
+            #expect(changed.publicKeyBlob == key.publicKeyBlob)
+            if purpose == .gitSigningOnly { #expect(changed.purpose == .gitSigningOnly) }
+        }
+    }
+
+    @Test func everyActualChangeAuthenticatesAndNoOpDoesNot() async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let auth = CountingAuthenticator()
+            let manager = fixture.makeKeyManager(authenticator: auth)
+            let key = try manager.generateKey(label: "fresh-auth")
+            try manager.changeAccessProfile(label: key.label, type: .personal)
+            #expect(auth.authenticationCount == 0)
+            try manager.changeAccessProfile(label: key.label, type: .bot)
+            try manager.changeAccessProfile(label: key.label, capabilities: .init(ssh: true, signature: false))
+            try manager.changeAccessProfile(label: key.label, type: .personal)
+            #expect(auth.authenticationCount == 3)
+            let fresh = try #require(try manager.fetchKeyInfo(label: key.label))
+            #expect(fresh.effectiveAccessProfile.type == .personal)
+            #expect(!fresh.effectiveAccessProfile.capabilities.signature)
+        }
+    }
+
+    @Test func cancellationPreservesAllStoredPolicy() async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let store = InMemoryPrivateKeyStore()
+            let manager = fixture.makeKeyManager(privateKeyStore: store)
+            let key = try manager.generateKey(label: "cancel-policy")
+            let raw = try store.load(label: key.label, context: LAContext(), prompt: "")
+            let failing = fixture.makeKeyManager(authenticator: Rejecting(), privateKeyStore: store)
+            #expect(throws: (any Error).self) {
+                try failing.changeAccessProfile(label: key.label, type: .bot, capabilities: .init(ssh: false, signature: false))
+            }
+            #expect(try manager.fetchKeyInfo(label: key.label) == key)
+            #expect(try store.load(label: key.label, context: LAContext(), prompt: "") == raw)
+        }
+    }
+
+    @Test func disablingBothOperationsIsARealPersistedPolicy() async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let manager = fixture.makeKeyManager()
+            let key = try manager.generateKey(label: "disabled")
+            try manager.changeAccessProfile(label: key.label, capabilities: .init(ssh: false, signature: false))
+            let fresh = try #require(try manager.fetchKeyInfo(label: key.label))
+            for operation in KeyOperation.allCases {
+                #expect(throws: KeyAccessError.operationDisabled(operation)) {
+                    try fresh.effectiveAccessProfile.require(operation)
+                }
+            }
+        }
+    }
+
+    @Test func indexFailureRollsBackPrivateRecord() async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let store = InMemoryPrivateKeyStore()
+            let manager = fixture.makeKeyManager(privateKeyStore: store)
+            let key = try manager.generateKey(label: "rollback-profile")
+            let raw = try store.load(label: key.label, context: LAContext(), prompt: "")
+            PublicKeyStore.forcedSaveErrorForTesting = NSError(domain: "test", code: 1)
+            defer { PublicKeyStore.forcedSaveErrorForTesting = nil }
+            #expect(throws: (any Error).self) { try manager.changeAccessProfile(label: key.label, type: .bot) }
+            #expect(try store.load(label: key.label, context: LAContext(), prompt: "") == raw)
+            #expect(try manager.fetchKeyInfo(label: key.label) == key)
+        }
+    }
+    @Test func revocationFailureDoesNotCommitNewPolicy() async throws {
+        let fixture = try ClavisTestFixture()
+        defer { fixture.cleanup() }
+        try await fixture.run {
+            let store = InMemoryPrivateKeyStore()
+            let initial = fixture.makeKeyManager(privateKeyStore: store)
+            let key = try initial.generateKey(label: "revoke-failure")
+            let raw = try store.load(label: key.label, context: LAContext(), prompt: "")
+            let manager = fixture.makeKeyManager(privateKeyStore: store,
+                agentGrantRevoker: { _ in throw NSError(domain: "revocation", code: 1) })
+            #expect(throws: (any Error).self) { try manager.changeAccessProfile(label: key.label, type: .bot) }
+            #expect(try store.load(label: key.label, context: LAContext(), prompt: "") == raw)
+            #expect(try initial.fetchKeyInfo(label: key.label) == key)
+        }
+    }
+
+}