From 95ca1aa2c797f4aeafc29dd4e2b82b56cbaeb5d8 Mon Sep 17 00:00:00 2001 From: sudoHG Date: Thu, 8 Oct 2026 23:36:30 +0800 Subject: [PATCH 1/2] Fix agent organization privacy, transactions and cancellation --- .../FrozenOrganizationApprovalContent.swift | 9 ++ .../Resources/Localizable.xcstrings | 24 +++++ .../BrokerApprovalStateMachine+Requests.swift | 2 +- .../BrokerOrganizationSummary.swift | 2 + .../AskKeyVault/FrozenAgentOrganization.swift | 12 ++- .../Storage/VaultStoreConfigQueries.swift | 36 ++++--- .../AskKeyVault/Vault+AgentOrganization.swift | 6 +- .../Vault+AgentOrganizationFreeze.swift | 42 ++++++-- .../AskKeyVault/Vault+AgentWriteCommit.swift | 8 +- .../AskKeyVault/Vault+AgentWriteRequest.swift | 8 +- .../AskKeyVault/Vault+CredentialGroups.swift | 35 ++---- .../VaultStore+AgentOrganization.swift | 10 +- .../OrganizationApprovalContentTests.swift | 31 +++++- Tests/AskKeyE2ETests/ScreenshotE2ETests.swift | 9 ++ .../AskKeyTestSupport/E2EBrokerScenario.swift | 15 ++- .../AgentGroupTransactionTests.swift | 84 +++++++++++++++ ...AgentOrganizationCommitBoundaryTests.swift | 96 +++++++++++++++++ ...gentOrganizationInvisibleTargetTests.swift | 101 ++++++++++++++++++ .../AgentOrganizationTests.swift | 10 +- ...AgentTextWriteApprovalLifecycleTests.swift | 4 +- .../0010-agent-credential-metadata-writes.md | 23 ++-- docs/features.md | 2 +- 22 files changed, 483 insertions(+), 86 deletions(-) create mode 100644 Tests/AskKeyVaultTests/AgentGroupTransactionTests.swift create mode 100644 Tests/AskKeyVaultTests/AgentOrganizationCommitBoundaryTests.swift create mode 100644 Tests/AskKeyVaultTests/AgentOrganizationInvisibleTargetTests.swift diff --git a/Sources/AskKeyAppKit/App/FrozenOrganizationApprovalContent.swift b/Sources/AskKeyAppKit/App/FrozenOrganizationApprovalContent.swift index aa6ad02..cfd888f 100644 --- a/Sources/AskKeyAppKit/App/FrozenOrganizationApprovalContent.swift +++ b/Sources/AskKeyAppKit/App/FrozenOrganizationApprovalContent.swift @@ -21,9 +21,18 @@ struct FrozenOrganizationSummaryContent { case .createGroup(let name): title = appLocalizedFormat("Create group “%@”", name) detail = appLocalized("New group — created when approved") + case .existingGroup(let name, let members, let nonvisible): + title = appLocalizedFormat("Create group “%@”", name) + detail = appLocalizedFormat("Group already exists — no changes. %@", + Self.memberCountDescription(members: members, nonvisible: nonvisible)) case .renameGroup(let from, let to, let members, let nonvisible): title = appLocalizedFormat("Rename group “%@” → “%@”", from, to) detail = Self.memberCountDescription(members: members, nonvisible: nonvisible) + case .mergeGroup(let from, let to, let members, let nonvisible, let targetMembers, let targetNonvisible): + title = appLocalizedFormat("Merge group “%@” into existing group “%@”", from, to) + detail = appLocalizedFormat("Source: %@. Existing group: %@.", + Self.memberCountDescription(members: members, nonvisible: nonvisible), + Self.memberCountDescription(members: targetMembers, nonvisible: targetNonvisible)) case .deleteGroup(let name, let members, let nonvisible): title = appLocalizedFormat("Delete group “%@”", name) detail = Self.memberCountDescription(members: members, nonvisible: nonvisible) diff --git a/Sources/AskKeyAppKit/Resources/Localizable.xcstrings b/Sources/AskKeyAppKit/Resources/Localizable.xcstrings index 610c985..fc533f1 100644 --- a/Sources/AskKeyAppKit/Resources/Localizable.xcstrings +++ b/Sources/AskKeyAppKit/Resources/Localizable.xcstrings @@ -1,6 +1,30 @@ { "sourceLanguage": "en", "strings": { + "Group already exists — no changes. %@": { + "extractionState": "manual", + "localizations": { + "en": { "stringUnit": { "state": "translated", "value": "Group already exists — no changes. %@" } }, + "zh-Hans": { "stringUnit": { "state": "translated", "value": "分组已存在，不会更改。%@" } }, + "qps-ploc": { "stringUnit": { "state": "translated", "value": "[!!Group already exists — no changes. %@!!]" } } + } + }, + "Merge group “%@” into existing group “%@”": { + "extractionState": "manual", + "localizations": { + "en": { "stringUnit": { "state": "translated", "value": "Merge group “%@” into existing group “%@”" } }, + "zh-Hans": { "stringUnit": { "state": "translated", "value": "将分组“%@”合并到已有分组“%@”" } }, + "qps-ploc": { "stringUnit": { "state": "translated", "value": "[!!Merge group “%@” into existing group “%@”!!]" } } + } + }, + "Source: %@. Existing group: %@.": { + "extractionState": "manual", + "localizations": { + "en": { "stringUnit": { "state": "translated", "value": "Source: %@. Existing group: %@." } }, + "zh-Hans": { "stringUnit": { "state": "translated", "value": "来源：%@；已有分组：%@。" } }, + "qps-ploc": { "stringUnit": { "state": "translated", "value": "[!!Source: %@. Existing group: %@.!!]" } } + } + }, "“%@” wants to organize credentials": { "extractionState": "manual", "localizations": { diff --git a/Sources/AskKeyBroker/BrokerApprovalStateMachine+Requests.swift b/Sources/AskKeyBroker/BrokerApprovalStateMachine+Requests.swift index b14c368..c5bed9b 100644 --- a/Sources/AskKeyBroker/BrokerApprovalStateMachine+Requests.swift +++ b/Sources/AskKeyBroker/BrokerApprovalStateMachine+Requests.swift @@ -166,7 +166,7 @@ extension BrokerApprovalStateMachine { guard entry.request.matchesForConsumption(operationRequest) else { throw BrokerApprovalError.payloadMismatch } - if entry.state == .pending || entry.state == .approved { entry.state = .cancelled } + if !entry.committing, entry.state == .pending || entry.state == .approved { entry.state = .cancelled } }.state } diff --git a/Sources/AskKeyBroker/BrokerOrganizationSummary.swift b/Sources/AskKeyBroker/BrokerOrganizationSummary.swift index b4372f0..2fbbc06 100644 --- a/Sources/AskKeyBroker/BrokerOrganizationSummary.swift +++ b/Sources/AskKeyBroker/BrokerOrganizationSummary.swift @@ -5,7 +5,9 @@ public struct BrokerOrganizationSummary: Codable, Equatable, Sendable { public enum Operation: Codable, Equatable, Sendable { case move(credential: String, from: String?, to: String?) case createGroup(String) + case existingGroup(name: String, members: Int, nonvisible: Int) case renameGroup(from: String, to: String, members: Int, nonvisible: Int) + case mergeGroup(from: String, to: String, members: Int, nonvisible: Int, targetMembers: Int, targetNonvisible: Int) case deleteGroup(name: String, members: Int, nonvisible: Int) } diff --git a/Sources/AskKeyVault/FrozenAgentOrganization.swift b/Sources/AskKeyVault/FrozenAgentOrganization.swift index 4993079..47ad909 100644 --- a/Sources/AskKeyVault/FrozenAgentOrganization.swift +++ b/Sources/AskKeyVault/FrozenAgentOrganization.swift @@ -20,6 +20,7 @@ struct AgentOrganizationGroupState: Codable, Equatable { let names: [String] let storedNames: [String] let memberIDs: [String] + let memberDigests: [String] } struct AgentOrganizationSnapshot { @@ -47,11 +48,16 @@ struct AgentOrganizationSnapshot { func groupStates(names: Set, key: SymmetricKey) throws -> [AgentOrganizationGroupState] { let assignments = try assignments(key: key) let allNames = Set(storedGroups).union(assignments.values) - return names.sorted().map { normalized in - .init(normalizedName: normalized, + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + return try names.sorted().map { normalized in + let members = records.filter { assignments[$0.id].map(CredentialName.normalized) == normalized } + .sorted { $0.id < $1.id } + return .init(normalizedName: normalized, names: allNames.filter { CredentialName.normalized($0) == normalized }.sorted(), storedNames: storedGroups.filter { CredentialName.normalized($0) == normalized }.sorted(), - memberIDs: assignments.filter { CredentialName.normalized($0.value) == normalized }.keys.sorted()) + memberIDs: members.map(\.id), + memberDigests: try members.map { Vault.componentDigest(try encoder.encode($0)) }) } } } diff --git a/Sources/AskKeyVault/Storage/VaultStoreConfigQueries.swift b/Sources/AskKeyVault/Storage/VaultStoreConfigQueries.swift index e51c2d6..99099af 100644 --- a/Sources/AskKeyVault/Storage/VaultStoreConfigQueries.swift +++ b/Sources/AskKeyVault/Storage/VaultStoreConfigQueries.swift @@ -1,4 +1,5 @@ import Foundation +import CryptoKit import GRDB extension VaultStore { @@ -21,24 +22,35 @@ extension VaultStore { } } - func replaceCredentialGroupsConfig( - key: String, - value: String, - clearingCredentialIDs: [String], + func editCredentialGroup( + _ name: String, + key: SymmetricKey, + deleting: Bool, updatedAt: String - ) throws { + ) throws -> [String] { try db.write { db in + let configKey = Vault.credentialGroupsConfigKey + let current = try ConfigRecord.fetchOne(db, key: configKey)?.value + var groups = Set(try AgentOrganizationSnapshot.storedGroups(current, key: key)) + if deleting { groups.remove(name) } else { groups.insert(name) } + let value = try VaultCrypto.encrypt(JSONEncoder().encode(groups.sorted()), using: key).base64EncodedString() try db.execute( sql: "INSERT OR REPLACE INTO config (key, value) VALUES (?, ?)", - arguments: [key, value] + arguments: [configKey, value] ) - for id in clearingCredentialIDs { - guard let fetched = try CredentialRecord.fetchOne(db, key: id) else { continue } - var record = try authenticatedCredential(fetched) - record.encryptedGroupName = nil - record.updatedAt = updatedAt - try credentialForPersistence(record).update(db) + var clearedIDs: [String] = [] + if deleting { + for fetched in try CredentialRecord.fetchAll(db) { + var record = try authenticatedCredential(fetched) + guard let encrypted = record.encryptedGroupName, + try VaultCrypto.decrypt(encrypted, using: key) == name else { continue } + record.encryptedGroupName = nil + record.updatedAt = updatedAt + try credentialForPersistence(record).update(db) + clearedIDs.append(record.id) + } } + return clearedIDs } } } diff --git a/Sources/AskKeyVault/Vault+AgentOrganization.swift b/Sources/AskKeyVault/Vault+AgentOrganization.swift index eec311b..42a6b6e 100644 --- a/Sources/AskKeyVault/Vault+AgentOrganization.swift +++ b/Sources/AskKeyVault/Vault+AgentOrganization.swift @@ -33,8 +33,10 @@ extension Vault { } func cancelAgentOrganization(operationID: String, requestID: String, capability: String) throws -> BrokerRequestState { - guard let frozen = agentOrganizations.entry(operationID: operationID) else { throw BrokerApprovalError.requestNotFound } - return try approvalRequests.cancel(requestID: requestID, capability: capability, operationRequest: frozen.approvalRequest) + try agentOrganizations.synchronized { + guard let frozen = agentOrganizations.entry(operationID: operationID) else { throw BrokerApprovalError.requestNotFound } + return try approvalRequests.cancel(requestID: requestID, capability: capability, operationRequest: frozen.approvalRequest) + } } public func frozenAgentOrganizationSummary(operationID: String, requestID: String, diff --git a/Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift b/Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift index eb6b62c..1bd86aa 100644 --- a/Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift +++ b/Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift @@ -10,6 +10,7 @@ extension Vault { } let snapshot = try store.agentOrganizationSnapshot(key: key) var assignments = try snapshot.assignments(key: key) + var storedGroupNames = Set(snapshot.storedGroups) var groupNames = Set(snapshot.storedGroups).union(assignments.values) var knownGroups = Set(groupNames.filter { group in let members = snapshot.records.filter { assignments[$0.id].map(CredentialName.normalized) == CredentialName.normalized(group) } @@ -39,6 +40,9 @@ extension Vault { func members(_ group: String) -> [CredentialRecord] { records.values.filter { assignments[$0.id].map(normalized) == normalized(group) }.sorted { $0.id < $1.id } } + func nonvisible(_ members: [CredentialRecord]) -> Int { + members.filter { $0.deletedAt != nil || $0.permission == CredentialPermission.hidden.rawValue }.count + } func assign(_ id: String, to group: String?) throws { guard var record = records[id] else { throw VaultError.credentialUnavailable } affected.insert(id) @@ -73,24 +77,41 @@ extension Vault { case .createGroup(let raw): let display = try name(raw) namedGroups.insert(normalized(display)) - guard !groupNames.contains(where: { normalized($0) == normalized(display) }) else { + guard !knownGroups.contains(normalized(display)) else { throw VaultError.credentialUnavailable } - groupNames.insert(display) + if let existing = groupNames.sorted().first(where: { normalized($0) == normalized(display) }) { + let existingMembers = members(existing) + summary.append(.existingGroup(name: existing, members: existingMembers.count, nonvisible: nonvisible(existingMembers))) + } else { + groupNames.insert(display) + storedGroupNames.insert(display) + summary.append(.createGroup(display)) + } knownGroups.insert(normalized(display)) - summary.append(.createGroup(display)) case .renameGroup(let rawFrom, let rawTo): let from = try existingGroup(rawFrom) - let to = try name(rawTo) - namedGroups.insert(normalized(to)) - guard !groupNames.contains(where: { normalized($0) == normalized(to) }) else { + let proposed = try name(rawTo) + namedGroups.insert(normalized(proposed)) + guard !knownGroups.contains(normalized(proposed)) else { throw VaultError.credentialUnavailable } + let existing = groupNames.sorted().first { normalized($0) == normalized(proposed) } + let to = existing ?? proposed let affectedMembers = members(from) - summary.append(.renameGroup(from: from, to: to, members: affectedMembers.count, - nonvisible: affectedMembers.filter { $0.deletedAt != nil || $0.permission == CredentialPermission.hidden.rawValue }.count)) + if let existing { + let targetMembers = members(existing) + summary.append(.mergeGroup(from: from, to: to, members: affectedMembers.count, + nonvisible: nonvisible(affectedMembers), targetMembers: targetMembers.count, + targetNonvisible: nonvisible(targetMembers))) + } else { + summary.append(.renameGroup(from: from, to: to, members: affectedMembers.count, + nonvisible: nonvisible(affectedMembers))) + storedGroupNames.insert(to) + } for record in affectedMembers { try assign(record.id, to: to) } groupNames = Set(groupNames.filter { normalized($0) != normalized(from) }) + storedGroupNames = Set(storedGroupNames.filter { normalized($0) != normalized(from) }) groupNames.insert(to) knownGroups.remove(normalized(from)) knownGroups.insert(normalized(to)) @@ -98,16 +119,17 @@ extension Vault { let group = try existingGroup(raw) let affectedMembers = members(group) summary.append(.deleteGroup(name: group, members: affectedMembers.count, - nonvisible: affectedMembers.filter { $0.deletedAt != nil || $0.permission == CredentialPermission.hidden.rawValue }.count)) + nonvisible: nonvisible(affectedMembers))) for record in affectedMembers { try assign(record.id, to: nil) } groupNames = Set(groupNames.filter { normalized($0) != normalized(group) }) + storedGroupNames = Set(storedGroupNames.filter { normalized($0) != normalized(group) }) knownGroups.remove(normalized(group)) } } let before = affected.sorted().compactMap { original[$0] } let after = affected.sorted().compactMap { records[$0] } let groups = try snapshot.groupStates(names: namedGroups, key: key) - let finalNames = groupNames.filter { namedGroups.contains(normalized($0)) }.sorted() + let finalNames = storedGroupNames.filter { namedGroups.contains(normalized($0)) }.sorted() let presentation = BrokerOrganizationSummary(operations: summary) struct ApprovalContents: Encodable { let requestDigest: String diff --git a/Sources/AskKeyVault/Vault+AgentWriteCommit.swift b/Sources/AskKeyVault/Vault+AgentWriteCommit.swift index eae03a8..f33de4e 100644 --- a/Sources/AskKeyVault/Vault+AgentWriteCommit.swift +++ b/Sources/AskKeyVault/Vault+AgentWriteCommit.swift @@ -67,7 +67,7 @@ extension Vault { throw BrokerApprovalError.payloadMismatch } if let expiresAt = frozen.credentialExpiresAt, expiresAt <= currentDate { - cancelAgentWrites(credentialID: frozen.approvalRequest.credentialID) + cancelAgentWritesForExpiry(credentialID: frozen.approvalRequest.credentialID) throw VaultError.credentialUnavailable } do { @@ -90,7 +90,7 @@ extension Vault { notifySnapshotRelevantChange() return result } catch VaultError.credentialUnavailable { - cancelAgentWrites(credentialID: frozen.approvalRequest.credentialID) + cancelAgentWritesForExpiry(credentialID: frozen.approvalRequest.credentialID) throw VaultError.credentialUnavailable } catch BrokerApprovalError.alreadyConsumed { guard let committed = try completedAgentTextWrite( @@ -115,9 +115,9 @@ extension Vault { } } - private func cancelAgentWrites(credentialID: String) { + private func cancelAgentWritesForExpiry(credentialID: String) { brokerRequests.cancelPending(credentialID: credentialID) - approvalRequests.cancelPending(credentialID: credentialID) + approvalRequests.cancelPendingForExpiry(credentialID: credentialID) } private func completedAgentTextWrite( diff --git a/Sources/AskKeyVault/Vault+AgentWriteRequest.swift b/Sources/AskKeyVault/Vault+AgentWriteRequest.swift index 3bca2ef..109f36a 100644 --- a/Sources/AskKeyVault/Vault+AgentWriteRequest.swift +++ b/Sources/AskKeyVault/Vault+AgentWriteRequest.swift @@ -97,10 +97,10 @@ extension Vault { requestID: String, capability: String ) throws -> BrokerRequestState { - try agentTextWrites.synchronized { - if agentOrganizations.entry(operationID: operationID) != nil { - return try cancelAgentOrganization(operationID: operationID, requestID: requestID, capability: capability) - } + if agentOrganizations.entry(operationID: operationID) != nil { + return try cancelAgentOrganization(operationID: operationID, requestID: requestID, capability: capability) + } + return try agentTextWrites.synchronized { guard let frozen = agentTextWrites.entry(operationID: operationID) else { throw BrokerApprovalError.requestNotFound } diff --git a/Sources/AskKeyVault/Vault+CredentialGroups.swift b/Sources/AskKeyVault/Vault+CredentialGroups.swift index 9c186eb..99c97a9 100644 --- a/Sources/AskKeyVault/Vault+CredentialGroups.swift +++ b/Sources/AskKeyVault/Vault+CredentialGroups.swift @@ -47,9 +47,10 @@ extension Vault { try authorizeManagement(authenticator, reason: CredentialManagementCopy.manageReason) let key = try requireKey() let name = try CredentialName.displayName(from: rawName) - var groups = Set(try storedCredentialGroups(key: key)) - groups.insert(name) - try persistCredentialGroups(groups.sorted(), key: key) + let wasPaused = try agentAccessGate.beginExclusiveChange() + defer { agentAccessGate.endExclusiveChange(paused: wasPaused) } + _ = try store.editCredentialGroup(name, key: key, deleting: false, + updatedAt: sharedDateFormatter.string(from: currentDate)) notifySnapshotRelevantChange() } @@ -64,21 +65,8 @@ extension Vault { let deliveryManager = try fileDeliveryManager.get() let wasPaused = try agentAccessGate.beginExclusiveChange() defer { agentAccessGate.endExclusiveChange(paused: wasPaused) } - var groups = Set(try storedCredentialGroups(key: key)) - groups.remove(name) - let matchingIDs = try (store.fetchAllCredentials() + store.fetchRecycledCredentials()) - .compactMap { record -> String? in - guard let encrypted = record.encryptedGroupName, - try VaultCrypto.decrypt(encrypted, using: key) == name else { return nil } - return record.id - } - let encrypted = try encryptedCredentialGroups(groups.sorted(), key: key) - try store.replaceCredentialGroupsConfig( - key: Self.credentialGroupsConfigKey, - value: encrypted, - clearingCredentialIDs: matchingIDs, - updatedAt: sharedDateFormatter.string(from: currentDate) - ) + let matchingIDs = try store.editCredentialGroup(name, key: key, deleting: true, + updatedAt: sharedDateFormatter.string(from: currentDate)) for id in matchingIDs { brokerRequests.cancelPending(credentialID: id) approvalRequests.cancelPending(credentialID: id) @@ -97,15 +85,4 @@ extension Vault { from: VaultCrypto.decryptData(encrypted, using: key) ) } - - private func persistCredentialGroups(_ groups: [String], key: SymmetricKey) throws { - try store.setConfigValue( - key: Self.credentialGroupsConfigKey, - value: try encryptedCredentialGroups(groups, key: key) - ) - } - - func encryptedCredentialGroups(_ groups: [String], key: SymmetricKey) throws -> String { - try VaultCrypto.encrypt(JSONEncoder().encode(groups), using: key).base64EncodedString() - } } diff --git a/Sources/AskKeyVault/VaultStore+AgentOrganization.swift b/Sources/AskKeyVault/VaultStore+AgentOrganization.swift index 64e2421..ff3d590 100644 --- a/Sources/AskKeyVault/VaultStore+AgentOrganization.swift +++ b/Sources/AskKeyVault/VaultStore+AgentOrganization.swift @@ -49,13 +49,15 @@ extension VaultStore { } // Merge only named groups into transaction-current state; unrelated // App group edits survive this batch. - var groups = Set(snapshot.storedGroups).union(try snapshot.assignments(key: key).values) + var groups = Set(snapshot.storedGroups) groups = Set(groups.filter { !names.contains(CredentialName.normalized($0)) }) groups.formUnion(frozen.finalGroupNames) - let value = try VaultCrypto.encrypt(JSONEncoder().encode(groups.sorted()), using: key).base64EncodedString() for record in frozen.after { try credentialForPersistence(record).update(db) } - try db.execute(sql: "INSERT OR REPLACE INTO config (key, value) VALUES (?, ?)", - arguments: [Vault.credentialGroupsConfigKey, value]) + if groups != Set(snapshot.storedGroups) { + let value = try VaultCrypto.encrypt(JSONEncoder().encode(groups.sorted()), using: key).base64EncodedString() + try db.execute(sql: "INSERT OR REPLACE INTO config (key, value) VALUES (?, ?)", + arguments: [Vault.credentialGroupsConfigKey, value]) + } try AgentWriteOperationRecord(operationId: frozen.request.operationID, payloadDigest: frozen.digest, credentialId: "", operation: BrokerApprovalOperation.organize.rawValue, committedAt: sharedDateFormatter.string(from: committedAt), requestId: requestID, diff --git a/Tests/AskKeyAppTests/OrganizationApprovalContentTests.swift b/Tests/AskKeyAppTests/OrganizationApprovalContentTests.swift index ed7d2ea..3193b6a 100644 --- a/Tests/AskKeyAppTests/OrganizationApprovalContentTests.swift +++ b/Tests/AskKeyAppTests/OrganizationApprovalContentTests.swift @@ -5,6 +5,30 @@ import AskKeyBroker @testable import AskKeyAppKit final class OrganizationApprovalContentTests: AskKeyAppTestCase { + func testInvisibleExistingTargetsShowNoOpAndMergeTruthInBothLanguages() { + let previous = AppLanguage.current + defer { AppLanguage.current = previous } + let summary = BrokerOrganizationSummary(operations: [ + .existingGroup(name: "Target", members: 1, nonvisible: 1), + .mergeGroup(from: "Source", to: "Target", members: 4, nonvisible: 1, targetMembers: 2, targetNonvisible: 2), + ]) + for language in ["en", "zh-Hans"] { + AppLanguage.current = language + let rows = FrozenOrganizationSummaryContent(summary: summary).rows + if language == "en" { + XCTAssertEqual(rows[0].title, "1. Create group “Target”") + XCTAssertEqual(rows[0].detail, "Group already exists — no changes. 1 credential, 1 not visible to agents") + XCTAssertEqual(rows[1].title, "2. Merge group “Source” into existing group “Target”") + XCTAssertEqual(rows[1].detail, "Source: 4 credentials, 1 not visible to agents. Existing group: 2 credentials, 2 not visible to agents.") + } else { + XCTAssertEqual(rows[0].title, "1. 创建分组“Target”") // i18n-literal: Assert reviewed Simplified Chinese no-op copy. + XCTAssertEqual(rows[0].detail, "分组已存在，不会更改。1 个凭证，其中 1 个 Agent 看不到") // i18n-literal: Assert reviewed Simplified Chinese no-op copy. + XCTAssertEqual(rows[1].title, "2. 将分组“Source”合并到已有分组“Target”") // i18n-literal: Assert reviewed Simplified Chinese merge copy. + XCTAssertEqual(rows[1].detail, "来源：4 个凭证，其中 1 个 Agent 看不到；已有分组：2 个凭证，其中 2 个 Agent 看不到。") // i18n-literal: Assert reviewed Simplified Chinese merge counts. + } + } + } + func testOrderedRowsShowGroupEffectsAndNonvisibleCountsInBothLanguages() { let previous = AppLanguage.current defer { AppLanguage.current = previous } @@ -66,8 +90,11 @@ final class OrganizationApprovalContentTests: AskKeyAppTestCase { let request = BrokerApprovalOperationRequest(operationID: "synthetic", credentialID: "", targetID: "credential-library", operation: .organize, payloadDigest: String(repeating: "a", count: 64), callerName: "Synthetic Agent", callerPurpose: String(repeating: "Synthetic purpose ", count: 200), organizationCredentialIDs: []) - let summary = BrokerOrganizationSummary(operations: (0..<64).map { - .move(credential: "API \($0)", from: String(repeating: "Original group ", count: 15), to: "Staging") + let summary = BrokerOrganizationSummary(operations: (0..<64).map { index in + index.isMultiple(of: 2) + ? .existingGroup(name: String(repeating: "Existing group ", count: 15), members: 1, nonvisible: 1) + : .mergeGroup(from: String(repeating: "Original group ", count: 15), to: String(repeating: "Existing group ", count: 15), + members: 4, nonvisible: 1, targetMembers: 2, targetNonvisible: 2) }) var reveals = 0 for language in ["en", "zh-Hans"] { diff --git a/Tests/AskKeyE2ETests/ScreenshotE2ETests.swift b/Tests/AskKeyE2ETests/ScreenshotE2ETests.swift index 10075ab..8996710 100644 --- a/Tests/AskKeyE2ETests/ScreenshotE2ETests.swift +++ b/Tests/AskKeyE2ETests/ScreenshotE2ETests.swift @@ -118,6 +118,15 @@ final class ScreenshotE2ETests: E2EBaseCase { XCTAssertFalse(app.buttons["approval-reveal-frozen-material"].exists) XCTAssertFalse(app.buttons["approval-allow-timed"].exists) capture("16-approval-organization", approval: true) + let operations = app.scrollViews["approval-organization-operations"] + XCTAssertTrue(operations.exists) + operations.scroll(byDeltaX: 0, deltaY: -600) + waitForText("Group already exists — no changes. 1 credential, 1 not visible to agents", containing: true) + waitForText("Merge group “Merge Source” into existing group “Existing Merge Services”", containing: true) + waitForText("Source: 2 credentials, 1 not visible to agents. Existing group: 2 credentials, 2 not visible to agents.", containing: true) + XCTAssertTrue(app.buttons["approval-allow-once"].isHittable) + XCTAssertTrue(app.buttons["approval-deny"].isHittable) + capture("17-approval-organization-existing-targets", approval: true) click("approval-deny") let result = try waitForEvidence("approval-result.json") XCTAssertEqual(result["outcome"] as? String, "denied") diff --git a/Tests/AskKeyTestSupport/E2EBrokerScenario.swift b/Tests/AskKeyTestSupport/E2EBrokerScenario.swift index e4d4230..5e7a604 100644 --- a/Tests/AskKeyTestSupport/E2EBrokerScenario.swift +++ b/Tests/AskKeyTestSupport/E2EBrokerScenario.swift @@ -127,6 +127,17 @@ final class E2EBrokerScenario { groupName: "Old Services", permission: permission), using: .allow) if name == "Retired CI Token" { try Vault.shared.deleteTextCredential(id: credential.id, using: .allow) } } + for (name, group, permission, recycled) in [ + ("Private Service Token", "Existing Private Services", CredentialPermission.hidden, false), + ("Merge API", "Merge Source", .ask, false), + ("Hidden Merge Source", "Merge Source", .hidden, false), + ("Hidden Merge Target", "Existing Merge Services", .hidden, false), + ("Retired Merge Target", "Existing Merge Services", .ask, true), + ] { + let credential = try Vault.shared.createTextCredential(.init(name: name, value: "synthetic-organization-token", + groupName: group, permission: permission), using: .allow) + if recycled { try Vault.shared.deleteTextCredential(id: credential.id, using: .allow) } + } } catch { Vault.shared.endManagementSession() throw error @@ -138,7 +149,9 @@ final class E2EBrokerScenario { "operations": [["create_group": "Staging Services"], ["move": ["credential": "Staging API", "group": "Staging Services"]], ["rename_group": ["from": "Old Services", "to": "Renamed Services"]], - ["delete_group": "Renamed Services"]] + ["delete_group": "Renamed Services"], + ["create_group": "existing private services"], + ["rename_group": ["from": "Merge Source", "to": "existing merge services"]]] ], evidence: "organization-write-pending.json") guard !reply.isError, case .success(.textWriteRequest(.submitted(let ticket))) = try JSONDecoder().decode(BrokerResponse.self, from: reply.data), diff --git a/Tests/AskKeyVaultTests/AgentGroupTransactionTests.swift b/Tests/AskKeyVaultTests/AgentGroupTransactionTests.swift new file mode 100644 index 0000000..f07b365 --- /dev/null +++ b/Tests/AskKeyVaultTests/AgentGroupTransactionTests.swift @@ -0,0 +1,84 @@ +import XCTest +import GRDB +import AskKeyBroker +@testable import AskKeyVault + +final class AgentGroupTransactionTests: AgentOrganizationTestSupport { + func testAppCreateAndDeleteHoldOneTransactionAcrossReadMergeWriteAgainstBothAgentWriters() throws { + for deleting in [false, true] { + for organization in [false, true] { + let harness = try makeHarness { _ in true } + try harness.vault.createCredentialGroup("Keep", using: .allow) + try harness.vault.createCredentialGroup("Obsolete", using: .allow) + let batch = organization + ? request([.deleteGroup("Obsolete"), .createGroup("Agent")]) + : AgentTextWriteRequest(operationID: UUID().uuidString, action: .createBundle(name: "Synthetic", + components: [.init(name: "token", value: .text("synthetic-token"), delivery: .none)], group: "Agent")) + let ticket = try approve(batch, harness: harness) + let appRead = DispatchSemaphore(value: 0) + let releaseApp = DispatchSemaphore(value: 0) + let appFinished = DispatchSemaphore(value: 0) + let agentStarted = DispatchSemaphore(value: 0) + let agentFinished = DispatchSemaphore(value: 0) + let appResults = ConcurrentResults() + let agentResults = ConcurrentResults() + let transactionBoundaries = ConcurrentResults() + try harness.store.db.write { db in + var stopped = false + var crossedBoundary = false + var written = false + db.trace(options: [.statement, .profile]) { event in + if case .statement(let statement) = event, stopped, !written { + if statement.sql == "COMMIT" || statement.sql == "ROLLBACK" { crossedBoundary = true } + if statement.sql.hasPrefix("INSERT OR REPLACE INTO config") { + written = true + transactionBoundaries.append(.success(crossedBoundary)) + } + } + guard case .profile(let statement, _) = event, !stopped, + statement.sql.hasPrefix("SELECT"), statement.expandedSQL.contains("credential_groups") else { return } + stopped = true + appRead.signal() + _ = releaseApp.wait(timeout: .now() + 5) + } + } + defer { + releaseApp.signal() + try? harness.store.db.write { $0.trace(nil) } + } + DispatchQueue.global().async { + appResults.append(Result { + if deleting { try harness.vault.deleteCredentialGroup("Keep", using: .allow) } + else { try harness.vault.createCredentialGroup("App", using: .allow) } + }) + appFinished.signal() + } + XCTAssertEqual(appRead.wait(timeout: .now() + 2), .success) + XCTAssertTrue(try harness.vault.agentAccessGate.isPaused(), "App group edits hold the exclusive gate") + DispatchQueue.global().async { + agentStarted.signal() + agentResults.append(Result { + try harness.vault.commitAgentTextWrite(batch, requestID: ticket.requestID, capability: ticket.capability) + }) + agentFinished.signal() + } + XCTAssertEqual(agentStarted.wait(timeout: .now() + 2), .success) + let reservationBlocked = agentFinished.wait(timeout: .now() + 0.1) + releaseApp.signal() + XCTAssertEqual(reservationBlocked, .timedOut, "The App retains the exclusive gate after reading groups") + XCTAssertEqual(appFinished.wait(timeout: .now() + 5), .success) + if reservationBlocked == .timedOut { XCTAssertEqual(agentFinished.wait(timeout: .now() + 5), .success) } + try appResults.values.forEach { try $0.get() } + XCTAssertEqual(try transactionBoundaries.values.map { try $0.get() }, [false], + "No transaction may end between reading and writing the stored group list") + XCTAssertEqual(agentResults.values.count, 1) + _ = try agentResults.values.first?.get() + var expected = ["Agent"] + if !organization { expected.append("Obsolete") } + if !deleting { expected += ["App", "Keep"] } + XCTAssertEqual(try harness.vault.listCredentialGroups(), expected.sorted()) + XCTAssertNotNil(try harness.store.fetchAgentWriteOperation(operationID: batch.operationID)) + } + } + } +} diff --git a/Tests/AskKeyVaultTests/AgentOrganizationCommitBoundaryTests.swift b/Tests/AskKeyVaultTests/AgentOrganizationCommitBoundaryTests.swift new file mode 100644 index 0000000..c962c6e --- /dev/null +++ b/Tests/AskKeyVaultTests/AgentOrganizationCommitBoundaryTests.swift @@ -0,0 +1,96 @@ +import XCTest +import GRDB +import AskKeyBroker +@testable import AskKeyVault + +final class AgentOrganizationCommitBoundaryTests: AgentOrganizationTestSupport { + func testSingleWriteExpiryRejectionPreservesRenameAndDeleteOnlyApprovalsAtBothChecks() throws { + for transactionExpiry in [false, true] { + for deleting in [false, true] { + let start = Date(timeIntervalSince1970: 2_000_000_000) + let clock = MutableClock(start) + let harness = try makeHarness(now: { clock.now }) { _ in true } + let member = try credential("Member", group: "Old", expiresAt: start.addingTimeInterval(2), harness: harness) + let single = AgentTextWriteRequest(operationID: "single", action: .modify(name: "Member", value: "synthetic-updated")) + let singleTicket = try approve(single, harness: harness) + let batch = request([deleting ? .deleteGroup("Old") : .renameGroup(from: "Old", to: "New")]) + let batchTicket = try approve(batch, harness: harness) + if transactionExpiry { + try harness.store.db.write { db in + db.trace { event in + if case .statement(let statement) = event, statement.sql.hasPrefix("BEGIN") { + clock.now = start.addingTimeInterval(3) + } + } + } + } else { clock.now = start.addingTimeInterval(3) } + XCTAssertThrowsError(try commit(single, harness: harness, ticket: singleTicket)) { + guard case VaultError.credentialUnavailable = $0 else { return XCTFail("Expected expiry rejection, got \($0)") } + } + try harness.store.db.write { $0.trace(nil) } + XCTAssertEqual(clock.now, start.addingTimeInterval(3)) + XCTAssertNil(try harness.store.fetchAgentWriteOperation(operationID: single.operationID)) + XCTAssertEqual(try harness.vault.approvalRequests.status(requestID: batchTicket.requestID, + capability: batchTicket.capability), .approved) + _ = try commit(batch, harness: harness, ticket: batchTicket) + XCTAssertEqual(try group(member.id, harness: harness), deleting ? nil : "New") + } + } + } + + func testCancellationDuringReservedOrganizationTransactionCannotCancelOrRecordDenial() throws { + let harness = try makeHarness { _ in true } + let member = try credential("Member", group: "Old", harness: harness) + let batch = request([.renameGroup(from: "Old", to: "New")]) + let ticket = try approve(batch, harness: harness) + let transactionEntered = DispatchSemaphore(value: 0) + let releaseTransaction = DispatchSemaphore(value: 0) + let commitFinished = DispatchSemaphore(value: 0) + let cancelStarted = DispatchSemaphore(value: 0) + let cancelFinished = DispatchSemaphore(value: 0) + let commits = ConcurrentResults() + let cancels = ConcurrentResults() + try harness.store.db.write { db in + db.add(function: DatabaseFunction("block_receipt", argumentCount: 0) { _ in + transactionEntered.signal() + _ = releaseTransaction.wait(timeout: .now() + 5) + return 1 + }) + try db.execute(sql: "CREATE TEMP TRIGGER block_commit BEFORE INSERT ON agent_write_operations BEGIN SELECT block_receipt(); END") + } + defer { releaseTransaction.signal() } + let frozen = try XCTUnwrap(harness.vault.agentOrganizations.entry(operationID: batch.operationID)) + DispatchQueue.global().async { + commits.append(Result { try harness.vault.commitAgentTextWrite(batch, requestID: ticket.requestID, capability: ticket.capability) }) + commitFinished.signal() + } + XCTAssertEqual(transactionEntered.wait(timeout: .now() + 2), .success) + // The operation-bound overload must independently honor the reservation. + XCTAssertEqual(try harness.vault.approvalRequests.cancel(requestID: ticket.requestID, + capability: ticket.capability, operationRequest: frozen.approvalRequest), .approved) + DispatchQueue.global().async { + cancelStarted.signal() + cancels.append(Result { try harness.vault.cancelAgentTextWrite(operationID: batch.operationID, + requestID: ticket.requestID, capability: ticket.capability) }) + cancelFinished.signal() + } + XCTAssertEqual(cancelStarted.wait(timeout: .now() + 2), .success) + let cancellationBlocked = cancelFinished.wait(timeout: .now() + 0.1) + releaseTransaction.signal() + XCTAssertEqual(cancellationBlocked, .timedOut) + XCTAssertEqual(commitFinished.wait(timeout: .now() + 5), .success) + if cancellationBlocked == .timedOut { XCTAssertEqual(cancelFinished.wait(timeout: .now() + 5), .success) } + XCTAssertEqual(commits.values.count, 1) + _ = try commits.values.first?.get() + XCTAssertEqual(cancels.values.count, 1) + switch try XCTUnwrap(cancels.values.first) { + case .success(let state): XCTAssertNotEqual(state, .cancelled) + case .failure(let error): XCTAssertEqual(error as? BrokerApprovalError, .requestNotFound) + } + XCTAssertEqual(try harness.vault.approvalRequests.status(requestID: ticket.requestID, capability: ticket.capability), .consumed) + XCTAssertEqual(try group(member.id, harness: harness), "New") + XCTAssertNotNil(try harness.store.fetchAgentWriteOperation(operationID: batch.operationID)) + let records = try harness.vault.listCredentialAccessRecords().filter { $0.credentialID == member.id && $0.operation == .modify } + XCTAssertEqual(records.map(\.result), [.allowed]) + } +} diff --git a/Tests/AskKeyVaultTests/AgentOrganizationInvisibleTargetTests.swift b/Tests/AskKeyVaultTests/AgentOrganizationInvisibleTargetTests.swift new file mode 100644 index 0000000..8257370 --- /dev/null +++ b/Tests/AskKeyVaultTests/AgentOrganizationInvisibleTargetTests.swift @@ -0,0 +1,101 @@ +import XCTest +import AskKeyBroker +@testable import AskKeyVault + +final class AgentOrganizationInvisibleTargetTests: AgentOrganizationTestSupport { + private func canonical(_ value: T) throws -> Data { + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + return try encoder.encode(value) + } + + func testInvisibleAndAbsentTargetsHaveIdenticalAgentOutcomesForCreateAndRename() throws { + for operation: BrokerOrganizationOperation in [.createGroup("target"), .renameGroup(from: "Source", to: "target")] { + var observable: [Data] = [] + for state in ["hidden", "recycled", "stored-invisible", "absent"] { + let harness = try makeHarness { _ in true } + try credential("Source Member", group: "Source", harness: harness) + if state != "absent" { + let member = try credential("Invisible", group: "Target", + permission: state == "recycled" ? .ask : .hidden, harness: harness) + if state == "recycled" { try harness.vault.deleteTextCredential(id: member.id, using: .allow) } + if state == "stored-invisible" { try harness.vault.createCredentialGroup("Target", using: .allow) } + } + XCTAssertEqual(try harness.vault.brokerCredentialGroups(), ["Source"]) + let batch = request([operation], id: "identical-batch") + let ticket = try submitted(harness.vault.requestAgentTextWrite(batch)) + XCTAssertFalse(ticket.requestID.isEmpty) + XCTAssertFalse(ticket.capability.isEmpty) + let normalized = AgentTextWriteSubmission(operationID: ticket.operationID, requestID: "random-request-id", + capability: "random-capability", state: ticket.state, retryCount: ticket.retryCount) + observable.append(try canonical(normalized)) + let retransmission = try submitted(harness.vault.requestAgentTextWrite(batch)) + XCTAssertEqual(retransmission.requestID, ticket.requestID) + XCTAssertEqual(retransmission.capability, ticket.capability) + XCTAssertEqual(retransmission.retryCount, 1) + XCTAssertEqual(try harness.vault.approvalRequests.status(requestID: ticket.requestID, capability: ticket.capability), .pending) + XCTAssertThrowsError(try commit(batch, harness: harness, ticket: ticket)) { + XCTAssertEqual($0 as? BrokerApprovalError, .invalidDecision) + } + XCTAssertEqual(try harness.vault.cancelAgentTextWrite(operationID: batch.operationID, + requestID: ticket.requestID, capability: ticket.capability), .cancelled) + } + XCTAssertEqual(Set(observable).count, 1) + } + } + + func testInvisibleCreationIsNoOpAndRenameMergesUsingExistingSpelling() throws { + for stored in [false, true] { + let harness = try makeHarness { _ in true } + let source = try credential("Source", group: "Old", harness: harness) + let hidden = try credential("Hidden", group: "Café", permission: .hidden, harness: harness) + let recycled = try credential("Recycled", group: "Café", harness: harness) + try harness.vault.deleteTextCredential(id: recycled.id, using: .allow) + if stored { try harness.vault.createCredentialGroup("Café", using: .allow) } + let before = try canonical(harness.store.fetchAllCredentialsIncludingRecycled().sorted { $0.id < $1.id }) + let configBefore = try harness.store.configValue(key: Vault.credentialGroupsConfigKey) + let creation = request([.createGroup(" CAFE\u{301} ")]) + let createTicket = try approve(creation, harness: harness) + XCTAssertEqual(try harness.vault.frozenAgentOrganizationSummary(operationID: creation.operationID, + requestID: createTicket.requestID, capability: createTicket.capability).operations, + [.existingGroup(name: "Café", members: 2, nonvisible: 2)]) + _ = try commit(creation, harness: harness, ticket: createTicket) + XCTAssertEqual(try canonical(harness.store.fetchAllCredentialsIncludingRecycled().sorted { $0.id < $1.id }), before) + XCTAssertEqual(try harness.store.configValue(key: Vault.credentialGroupsConfigKey), configBefore) + let rename = request([.renameGroup(from: "Old", to: "CAFE\u{301}")]) + let renameTicket = try approve(rename, harness: harness) + XCTAssertEqual(try harness.vault.frozenAgentOrganizationSummary(operationID: rename.operationID, + requestID: renameTicket.requestID, capability: renameTicket.capability).operations, + [.mergeGroup(from: "Old", to: "Café", members: 1, nonvisible: 0, targetMembers: 2, targetNonvisible: 2)]) + _ = try commit(rename, harness: harness, ticket: renameTicket) + for id in [source.id, hidden.id, recycled.id] { XCTAssertEqual(try group(id, harness: harness), "Café") } + XCTAssertEqual(try harness.vault.listCredentialGroups(), ["Café"]) + XCTAssertEqual(try harness.vault.brokerCredentialGroups(), ["Café"]) + } + } + + func testAnyInvisibleTargetStateChangeInvalidatesCreationAndMerge() throws { + for operation: BrokerOrganizationOperation in [.createGroup("Target"), .renameGroup(from: "Old", to: "Target")] { + for change in ["permission", "notes", "membership", "stored-name"] { + let harness = try makeHarness { _ in true } + let source = try credential("Source", group: "Old", harness: harness) + let target = try credential("Target Member", group: "Target", permission: .hidden, harness: harness) + let batch = request([operation]) + let ticket = try approve(batch, harness: harness) + if change == "membership" { try credential("Joined", group: "Target", permission: .hidden, harness: harness) } + else if change == "stored-name" { try harness.vault.createCredentialGroup("Target", using: .allow) } + else { + var record = try XCTUnwrap(harness.store.fetchCredential(id: target.id)) + if change == "permission" { record.permission = CredentialPermission.ask.rawValue } + else { record.encryptedPrivateNotes = try VaultCrypto.encrypt("synthetic-changed", using: harness.key) } + try harness.store.updateCredential(record) + } + XCTAssertThrowsError(try commit(batch, harness: harness, ticket: ticket)) { + guard case VaultError.credentialChanged = $0 else { return XCTFail("Expected stale hidden state, got \($0)") } + } + XCTAssertEqual(try group(source.id, harness: harness), "Old") + XCTAssertNil(try harness.store.fetchAgentWriteOperation(operationID: batch.operationID)) + } + } + } +} diff --git a/Tests/AskKeyVaultTests/AgentOrganizationTests.swift b/Tests/AskKeyVaultTests/AgentOrganizationTests.swift index a9a0331..ae075db 100644 --- a/Tests/AskKeyVaultTests/AgentOrganizationTests.swift +++ b/Tests/AskKeyVaultTests/AgentOrganizationTests.swift @@ -98,18 +98,20 @@ final class AgentOrganizationTests: AgentOrganizationTestSupport { } } - func testOccupiedRenameAndDuplicateCreationRejectWithGenericError() throws { + func testVisibleOccupiedRenameAndDuplicateCreationRejectWithGenericError() throws { let harness = try makeHarness { _ in true } try credential("Visible", group: "Source", harness: harness) - try credential("Hidden", group: "Occupied", permission: .hidden, harness: harness) - for operation: BrokerOrganizationOperation in [.renameGroup(from: "Source", to: "OCCUPIED"), .createGroup("occupied")] { + try credential("Visible Target", group: "Occupied", harness: harness) + try harness.vault.createCredentialGroup("Empty", using: .allow) + for operation: BrokerOrganizationOperation in [.renameGroup(from: "Source", to: "OCCUPIED"), .createGroup("occupied"), + .createGroup("empty"), .renameGroup(from: "Source", to: "EMPTY")] { XCTAssertThrowsError(try harness.vault.requestAgentTextWrite(request([operation]))) { error in guard case VaultError.credentialUnavailable = error else { return XCTFail("Expected generic unavailable, got \(error)") } } } let batch = request([.renameGroup(from: "source", to: " Destination ")]) _ = try commit(batch, harness: harness, ticket: approve(batch, harness: harness)) - XCTAssertEqual(try harness.vault.brokerCredentialGroups(), ["Destination"]) + XCTAssertEqual(try harness.vault.brokerCredentialGroups(), ["Destination", "Empty", "Occupied"]) } func testAllowAndTimedReadAllowanceNeverSkipOrganizationApproval() throws { diff --git a/Tests/AskKeyVaultTests/AgentTextWriteApprovalLifecycleTests.swift b/Tests/AskKeyVaultTests/AgentTextWriteApprovalLifecycleTests.swift index 9c08cff..dba4d1a 100644 --- a/Tests/AskKeyVaultTests/AgentTextWriteApprovalLifecycleTests.swift +++ b/Tests/AskKeyVaultTests/AgentTextWriteApprovalLifecycleTests.swift @@ -102,11 +102,11 @@ final class AgentTextWriteApprovalLifecycleTests: AgentTextWriteTestSupport { } XCTAssertEqual( try machine.status(requestID: modifyTicket.requestID, capability: modifyTicket.capability), - .cancelled + .expired ) XCTAssertEqual( try machine.status(requestID: deleteTicket.requestID, capability: deleteTicket.capability), - .cancelled + .expired ) XCTAssertEqual( try harness.vault.revealTextCredential(id: existing.id, using: .allow).value, diff --git a/docs/adr/0010-agent-credential-metadata-writes.md b/docs/adr/0010-agent-credential-metadata-writes.md index 4472297..712631c 100644 --- a/docs/adr/0010-agent-credential-metadata-writes.md +++ b/docs/adr/0010-agent-credential-metadata-writes.md @@ -14,9 +14,9 @@ specified in [ADR 0001](0001-domain-storage-and-schema.md). ## Decision - Agents may propose usage-instruction changes, group assignment, and group - creation, rename and deletion as approved writes. Group rename and deletion - through a batch tool are deferred to #162; #161 implements instructions, - assignment and creation only. Permission changes, access records, stored-value + creation, rename and deletion as approved writes. #161 added instructions, + assignment and creation; #162 added batch organization, including group rename + and deletion. Permission changes, access records, stored-value reveal and local erase remain authenticated App operations. - Every metadata write requires explicit write approval and separate system authentication. Allow permission and timed read allowances never authorize it. @@ -52,7 +52,10 @@ remain organizational data rather than grants of access. `organize_credentials` accepts 1–64 ordered operations: `move` (visible credential name and a visible group name or `null`), `create_group`, `rename_group` (`from` and `to`), and `delete_group`. A move may also name a group created earlier in -the batch. Rename rejects an occupied normalized name; merging uses moves. +the batch. Create and rename reject catalog-visible occupied names, including +empty stored groups. Agent-invisible names freeze like absent names. The App +shows existing-group creation as a no-op and rename as a merge, with source and +target counts; renamed members reuse the target's existing spelling. Deletion removes only the group and ungroups all members, including Hidden and recycled members. Neither permissions nor credential deletion is available. @@ -67,9 +70,12 @@ successful wire result contains only the operation ID. The approval digest binds the ordered request and caller data, full before and after states of affected records, and the spelling, existence and membership of -named groups. Commit revalidates these snapshots inside one SQL transaction that +named groups, including full member states of an invisible target. Commit +revalidates these snapshots inside one SQL transaction that persists all records, the group changes and the idempotency receipt. Unrelated -App group edits are preserved. A changed affected record or named group rejects +App group edits are preserved. Every stored-group writer reads, merges and +writes inside one SQL transaction; App create/delete also take the exclusive +agent-access gate. A changed affected record or named group rejects the whole transaction. Existing Hidden members remain included; newly Hidden members invalidate the frozen state. Only explicitly moved credentials must be unexpired and can shorten the approval deadline. Rename/delete-only members, @@ -90,5 +96,8 @@ remain outside the scrolling area within the 680-point budget in English and Simplified Chinese. The catalog returns a top-level `credentials` list and `groups`: empty groups and groups with an active visible member. Hidden-only and recycled-only groups are absent and use the existing generic not-found -response. This additive tool keeps Broker protocol version 1 and leaves runtime +response for operations requiring an existing visible group. Create and rename +targets reveal no invisible-group existence before approval. Cancellation is +serialized with organization commit and cannot cancel reserved consumption. +This additive tool keeps Broker protocol version 1 and leaves runtime delivery and the App's group editor unchanged. diff --git a/docs/features.md b/docs/features.md index fa93d00..69f3612 100644 --- a/docs/features.md +++ b/docs/features.md @@ -20,7 +20,7 @@ Saved plaintext never belongs in Agent responses. A target that has received an | Credential templates and editor | Provide API, GitHub App, Apple, SSH, cloud, database and custom templates, optional fields, reveal controls and validated save actions. | **Keep** | `Sources/AskKeyApp/Views/CredentialManagementView.swift`
`Sources/AskKeyApp/CredentialComponentDeliveryEditor.swift` | `Tests/AskKeyAppTests/ReviewEditorValidationTests.swift`
`Tests/AskKeyAppTests/AgentClientConnectorTests.swift`
`Tests/AskKeyAppTests/WorkspaceVisualContractTests.swift` | | Names and field validation | Trim display names, use NFC/case-folded global uniqueness, reject control or oversize fields, and validate environment-variable mappings. | **Keep** | `Sources/AskKeyCore/Models/CredentialName.swift`
`Sources/AskKeyCore/CredentialFieldValidation.swift`
`Sources/AskKeyCore/Vault+TextCredentials.swift` | `Tests/AskKeyCoreTests/HumanTextCredentialTests.swift`
`Tests/AskKeyCoreTests/ReviewCatalogLimitTests.swift`
`Tests/AskKeyAppTests/ReviewEditorValidationTests.swift` | | Usage instructions and private notes | Edit catalog-visible usage instructions separately from encrypted private notes; preserve unrevealed payload and notes during metadata edits. | **Keep** | `Sources/AskKeyCore/Models/CredentialName.swift`
`Sources/AskKeyCore/Vault+TextCredentials.swift`
`Sources/AskKeyApp/Views/CredentialManagementView.swift` | `Tests/AskKeyCoreTests/HumanTextCredentialTests.swift`
`Tests/AskKeyCoreTests/BrokerCatalogTests.swift`
`Tests/AskKeyAppTests/ReviewPrivateNotesTests.swift` | -| Credential groups | Organize credentials without granting authority; metadata writes assign/create groups and `organize_credentials` validates and freezes up to 64 ordered moves, creations, renames and deletions under one approval and one system authentication. Group rename/delete includes Hidden and recycled members; deletion ungroups credentials. Catalog `groups` lists empty groups and groups with a visible member. | **Keep** | `Sources/AskKeyVault/Vault+CredentialGroups.swift`
`Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift`
`Sources/AskKeyVault/VaultStore+AgentOrganization.swift`
`Sources/AskKeyBroker/BrokerOrganizationSummary.swift`
`docs/adr/0010-agent-credential-metadata-writes.md` | `Tests/AskKeyVaultTests/AgentOrganizationTests.swift`
`Tests/AskKeyVaultTests/AgentOrganizationConcurrencyTests.swift`
`Tests/AskKeyAppTests/OrganizationApprovalContentTests.swift` | +| Credential groups | Organize credentials without granting authority; metadata writes assign/create groups and `organize_credentials` freezes up to 64 ordered operations under one approval and system authentication. Agent-invisible targets freeze like absent names; the App shows existing-group no-ops or merges with both member counts, and commit rechecks the full target state. Group rename/delete includes Hidden and recycled members; deletion ungroups credentials. All stored-group writers merge inside one SQL transaction, and App edits take the exclusive gate. Catalog `groups` lists empty groups and groups with a visible member. | **Keep** | `Sources/AskKeyVault/Vault+CredentialGroups.swift`
`Sources/AskKeyVault/Vault+AgentOrganizationFreeze.swift`
`Sources/AskKeyVault/VaultStore+AgentOrganization.swift`
`Sources/AskKeyBroker/BrokerOrganizationSummary.swift`
`docs/adr/0010-agent-credential-metadata-writes.md` | `Tests/AskKeyVaultTests/AgentOrganizationTests.swift`
`Tests/AskKeyVaultTests/AgentOrganizationInvisibleTargetTests.swift`
`Tests/AskKeyVaultTests/AgentGroupTransactionTests.swift`
`Tests/AskKeyVaultTests/AgentOrganizationCommitBoundaryTests.swift`
`Tests/AskKeyVaultTests/AgentOrganizationConcurrencyTests.swift`
`Tests/AskKeyAppTests/OrganizationApprovalContentTests.swift` | | Library navigation and search | Navigate all credentials, named groups and ungrouped items; search names, groups and instructions in the all-credentials view and clear search on scope changes. | **Keep** | `Sources/AskKeyApp/Views/CredentialManagementView.swift`
`Sources/AskKeyApp/Views/SettingsView.swift`
`Sources/AskKeyApp/WorkspaceVisualContract.swift` | `Tests/AskKeyAppTests/ManagementWorkspaceNavigationTests.swift`
`Tests/AskKeyAppTests/WorkspaceVisualContractTests.swift`
`Tests/AskKeyAppTests/AppLanguageExperienceTests.swift` | | Editor lifecycle | Keep create and edit routes distinct and stop saving an editor whose credential has disappeared, including after locking or reopening. | **Keep** | `Sources/AskKeyApp/Views/CredentialManagementView.swift`
`Sources/AskKeyApp/Views/SettingsView.swift` | `Tests/AskKeyAppTests/ReviewEditorMissingCredentialTests.swift` | | Trash, restore and permanent deletion | Recycle deleted credentials for 30 days, restore them from the App, and require explicit confirmation and authentication for permanent deletion. | **Keep** | `Sources/AskKeyCore/Storage/VaultStoreCredentialQueries.swift`
`Sources/AskKeyCore/Vault+TextCredentials.swift`
`Sources/AskKeyCore/AgentTextWriteGovernance.swift`
`Sources/AskKeyApp/VaultViewModel+Credentials.swift`
`Sources/AskKeyApp/Views/CredentialManagementView.swift` | `Tests/AskKeyCoreTests/AgentTextWriteGovernanceTests.swift`
`Tests/AskKeyCoreTests/CredentialStorageAuthenticationTests.swift`
`Tests/AskKeyCoreTests/ICloudBackupRecycleTests.swift`
`Tests/AskKeyAppTests/WorkspaceVisualContractTests.swift`
`Tests/AskKeyE2ETests/CredentialE2ETests.swift` | From f00bf14b3286b41331c9cd27c440793168b0a727 Mon Sep 17 00:00:00 2001 From: sudoHG Date: Thu, 8 Oct 2026 23:47:16 +0800 Subject: [PATCH 2/2] Export the existing-target organization approval screenshot --- scripts/e2e-report.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/e2e-report.py b/scripts/e2e-report.py index 8484981..3c3d0b6 100644 --- a/scripts/e2e-report.py +++ b/scripts/e2e-report.py @@ -16,6 +16,7 @@ "12-approval-default", "13-approval-details", "14-approval-cancelled-authentication", "15-approval-credential-metadata", "16-approval-organization", + "17-approval-organization-existing-targets", ]