Redoing locks in actors bc of observable

This commit is contained in:
Max Goedjen 2025-08-15 22:07:25 -05:00
parent c22e7d9ed8
commit c227c90fd4
No known key found for this signature in database
14 changed files with 93 additions and 90 deletions

View File

@ -27,16 +27,13 @@ let package = Package(
.library( .library(
name: "Brief", name: "Brief",
targets: ["Brief"]), targets: ["Brief"]),
.library(
name: "Common",
targets: ["Common"]),
], ],
dependencies: [ dependencies: [
], ],
targets: [ targets: [
.target( .target(
name: "SecretKit", name: "SecretKit",
dependencies: ["Common"], dependencies: [],
swiftSettings: swiftSettings swiftSettings: swiftSettings
), ),
.testTarget( .testTarget(
@ -46,17 +43,17 @@ let package = Package(
), ),
.target( .target(
name: "SecureEnclaveSecretKit", name: "SecureEnclaveSecretKit",
dependencies: ["Common", "SecretKit"], dependencies: ["SecretKit"],
swiftSettings: swiftSettings swiftSettings: swiftSettings
), ),
.target( .target(
name: "SmartCardSecretKit", name: "SmartCardSecretKit",
dependencies: ["Common", "SecretKit"], dependencies: ["SecretKit"],
swiftSettings: swiftSettings swiftSettings: swiftSettings
), ),
.target( .target(
name: "SecretAgentKit", name: "SecretAgentKit",
dependencies: ["Common", "SecretKit", "SecretAgentKitHeaders"], dependencies: ["SecretKit", "SecretAgentKitHeaders"],
swiftSettings: swiftSettings swiftSettings: swiftSettings
), ),
.systemLibrary( .systemLibrary(
@ -68,18 +65,13 @@ let package = Package(
, ,
.target( .target(
name: "Brief", name: "Brief",
dependencies: ["Common"], dependencies: [],
swiftSettings: swiftSettings swiftSettings: swiftSettings
), ),
.testTarget( .testTarget(
name: "BriefTests", name: "BriefTests",
dependencies: ["Brief"] dependencies: ["Brief"]
), ),
.target(
name: "Common",
dependencies: [],
swiftSettings: swiftSettings
),
] ]
) )

View File

@ -1,15 +1,18 @@
import Foundation import Foundation
import Observation import Observation
import os
import Common
/// A concrete implementation of ``UpdaterProtocol`` which considers the current release and OS version. /// A concrete implementation of ``UpdaterProtocol`` which considers the current release and OS version.
@Observable public final class Updater: UpdaterProtocol, Sendable { @Observable public final class Updater: UpdaterProtocol, Sendable {
public var update: Release? { private let state = State()
_update.lockedValue @MainActor @Observable public final class State {
var update: Release? = nil
nonisolated init() {}
} }
private let _update: OSAllocatedUnfairLock<Release?> = .init(uncheckedState: nil) public var update: Release? {
state.update
}
public let testBuild: Bool public let testBuild: Bool
/// The current OS version. /// The current OS version.
@ -23,7 +26,12 @@ import Common
/// - checkFrequency: The interval at which the Updater should check for updates. Subject to a tolerance of 1 hour. /// - checkFrequency: The interval at which the Updater should check for updates. Subject to a tolerance of 1 hour.
/// - osVersion: The current OS version. /// - osVersion: The current OS version.
/// - currentVersion: The current version of the app that is running. /// - currentVersion: The current version of the app that is running.
public init(checkOnLaunch: Bool, checkFrequency: TimeInterval = Measurement(value: 24, unit: UnitDuration.hours).converted(to: .seconds).value, osVersion: SemVer = SemVer(ProcessInfo.processInfo.operatingSystemVersion), currentVersion: SemVer = SemVer(Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "0.0.0")) { public init(
checkOnLaunch: Bool,
checkFrequency: TimeInterval = Measurement(value: 24, unit: UnitDuration.hours).converted(to: .seconds).value,
osVersion: SemVer = SemVer(ProcessInfo.processInfo.operatingSystemVersion),
currentVersion: SemVer = SemVer(Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "0.0.0")
) {
self.osVersion = osVersion self.osVersion = osVersion
self.currentVersion = currentVersion self.currentVersion = currentVersion
testBuild = currentVersion == SemVer("0.0.0") testBuild = currentVersion == SemVer("0.0.0")
@ -54,7 +62,7 @@ import Common
guard !release.critical else { return } guard !release.critical else { return }
defaults.set(true, forKey: release.name) defaults.set(true, forKey: release.name)
await MainActor.run { await MainActor.run {
_update.lockedValue = nil state.update = nil
} }
} }
@ -75,7 +83,8 @@ extension Updater {
let latestVersion = SemVer(release.name) let latestVersion = SemVer(release.name)
if latestVersion > currentVersion { if latestVersion > currentVersion {
await MainActor.run { await MainActor.run {
_update.lockedValue = release print("SET \(release)")
state.update = release
} }
} }
} }

View File

@ -5,7 +5,7 @@ import os
public protocol UpdaterProtocol: Observable, Sendable { public protocol UpdaterProtocol: Observable, Sendable {
/// The latest update /// The latest update
var update: Release? { get } @MainActor var update: Release? { get }
/// A boolean describing whether or not the current build of the app is a "test" build (ie, a debug build or otherwise special build) /// A boolean describing whether or not the current build of the app is a "test" build (ie, a debug build or otherwise special build)
var testBuild: Bool { get } var testBuild: Bool { get }

View File

@ -1,14 +0,0 @@
import os
public extension OSAllocatedUnfairLock where State: Sendable {
var lockedValue: State {
get {
withLock { $0 }
}
nonmutating set {
withLock { $0 = newValue }
}
}
}

View File

@ -22,8 +22,10 @@ public final class Agent: Sendable {
logger.debug("Agent is running") logger.debug("Agent is running")
self.storeList = storeList self.storeList = storeList
self.witness = witness self.witness = witness
Task { @MainActor in
certificateHandler.reloadCertificates(for: storeList.allSecrets) certificateHandler.reloadCertificates(for: storeList.allSecrets)
} }
}
} }
@ -60,7 +62,7 @@ extension Agent {
switch requestType { switch requestType {
case .requestIdentities: case .requestIdentities:
response.append(SSHAgent.ResponseType.agentIdentitiesAnswer.data) response.append(SSHAgent.ResponseType.agentIdentitiesAnswer.data)
response.append(identities()) response.append(await identities())
logger.debug("Agent returned \(SSHAgent.ResponseType.agentIdentitiesAnswer.debugDescription)") logger.debug("Agent returned \(SSHAgent.ResponseType.agentIdentitiesAnswer.debugDescription)")
case .signRequest: case .signRequest:
let provenance = requestTracer.provenance(from: reader) let provenance = requestTracer.provenance(from: reader)
@ -83,8 +85,8 @@ extension Agent {
/// Lists the identities available for signing operations /// Lists the identities available for signing operations
/// - Returns: An OpenSSH formatted Data payload listing the identities available for signing operations. /// - Returns: An OpenSSH formatted Data payload listing the identities available for signing operations.
func identities() -> Data { func identities() async -> Data {
let secrets = storeList.allSecrets let secrets = await storeList.allSecrets
certificateHandler.reloadCertificates(for: secrets) certificateHandler.reloadCertificates(for: secrets)
var count = secrets.count var count = secrets.count
var keyData = Data() var keyData = Data()
@ -123,7 +125,7 @@ extension Agent {
hash = payloadHash hash = payloadHash
} }
guard let (store, secret) = secret(matching: hash) else { guard let (store, secret) = await secret(matching: hash) else {
logger.debug("Agent did not have a key matching \(hash as NSData)") logger.debug("Agent did not have a key matching \(hash as NSData)")
throw AgentError.noMatchingKey throw AgentError.noMatchingKey
} }
@ -189,7 +191,7 @@ extension Agent {
/// Gives any store with no loaded secrets a chance to reload. /// Gives any store with no loaded secrets a chance to reload.
func reloadSecretsIfNeccessary() async { func reloadSecretsIfNeccessary() async {
for store in storeList.stores { for store in await storeList.stores {
if store.secrets.isEmpty { if store.secrets.isEmpty {
logger.debug("Store \(store.name, privacy: .public) has no loaded secrets. Reloading.") logger.debug("Store \(store.name, privacy: .public) has no loaded secrets. Reloading.")
await store.reloadSecrets() await store.reloadSecrets()
@ -200,8 +202,8 @@ extension Agent {
/// Finds a ``Secret`` matching a specified hash whos signature was requested. /// Finds a ``Secret`` matching a specified hash whos signature was requested.
/// - Parameter hash: The hash to match against. /// - Parameter hash: The hash to match against.
/// - Returns: A ``Secret`` and the ``SecretStore`` containing it, if a match is found. /// - Returns: A ``Secret`` and the ``SecretStore`` containing it, if a match is found.
func secret(matching hash: Data) -> (AnySecretStore, AnySecret)? { func secret(matching hash: Data) async -> (AnySecretStore, AnySecret)? {
storeList.stores.compactMap { store -> (AnySecretStore, AnySecret)? in await storeList.stores.compactMap { store -> (AnySecretStore, AnySecret)? in
let allMatching = store.secrets.filter { secret in let allMatching = store.secrets.filter { secret in
hash == writer.data(secret: secret) hash == writer.data(secret: secret)
} }

View File

@ -32,7 +32,7 @@ public final class OpenSSHCertificateHandler: Sendable {
/// - Parameter secret: The secret to check for a certificate. /// - Parameter secret: The secret to check for a certificate.
/// - Returns: A boolean describing whether or not the certificate handler has a certifiicate associated with a given secret /// - Returns: A boolean describing whether or not the certificate handler has a certifiicate associated with a given secret
public func hasCertificate<SecretType: Secret>(for secret: SecretType) -> Bool { public func hasCertificate<SecretType: Secret>(for secret: SecretType) -> Bool {
keyBlobsAndNames.lockedValue[AnySecret(secret)] != nil keyBlobsAndNames.withLock { $0[AnySecret(secret)] != nil }
} }
@ -64,7 +64,7 @@ public final class OpenSSHCertificateHandler: Sendable {
/// - Parameter secret: The secret to search for a certificate with /// - Parameter secret: The secret to search for a certificate with
/// - Returns: A (``Data``, ``Data``) tuple containing the certificate and certificate name, respectively. /// - Returns: A (``Data``, ``Data``) tuple containing the certificate and certificate name, respectively.
public func keyBlobAndName<SecretType: Secret>(for secret: SecretType) throws -> (Data, Data)? { public func keyBlobAndName<SecretType: Secret>(for secret: SecretType) throws -> (Data, Data)? {
keyBlobsAndNames.lockedValue[AnySecret(secret)] keyBlobsAndNames.withLock { $0[AnySecret(secret)] }
} }
/// Attempts to find an OpenSSH Certificate that corresponds to a ``Secret`` /// Attempts to find an OpenSSH Certificate that corresponds to a ``Secret``

View File

@ -1,50 +1,39 @@
import Foundation import Foundation
import Observation import Observation
import os
import Common
/// A "Store Store," which holds a list of type-erased stores. /// A "Store Store," which holds a list of type-erased stores.
@Observable public final class SecretStoreList: Sendable { @Observable @MainActor public final class SecretStoreList: Sendable {
/// The Stores managed by the SecretStoreList. /// The Stores managed by the SecretStoreList.
public var stores: [AnySecretStore] { public var stores: [AnySecretStore] = []
__stores.lockedValue
}
private let __stores: OSAllocatedUnfairLock<[AnySecretStore]> = .init(uncheckedState: [])
/// A modifiable store, if one is available. /// A modifiable store, if one is available.
public var modifiableStore: AnySecretStoreModifiable? { public var modifiableStore: AnySecretStoreModifiable? = nil
__modifiableStore.withLock { $0 }
}
private let __modifiableStore: OSAllocatedUnfairLock<AnySecretStoreModifiable?> = .init(uncheckedState: nil)
/// Initializes a SecretStoreList. /// Initializes a SecretStoreList.
public init() { public nonisolated init() {
} }
/// Adds a non-type-erased SecretStore to the list. /// Adds a non-type-erased SecretStore to the list.
public func add<SecretStoreType: SecretStore>(store: SecretStoreType) { public func add<SecretStoreType: SecretStore>(store: SecretStoreType) {
__stores.withLock { stores.append(AnySecretStore(store))
$0.append(AnySecretStore(store))
}
} }
/// Adds a non-type-erased modifiable SecretStore. /// Adds a non-type-erased modifiable SecretStore.
public func add<SecretStoreType: SecretStoreModifiable>(store: SecretStoreType) { public func add<SecretStoreType: SecretStoreModifiable>(store: SecretStoreType) {
let modifiable = AnySecretStoreModifiable(modifiable: store) let modifiable = AnySecretStoreModifiable(modifiable: store)
__modifiableStore.lockedValue = modifiable if modifiableStore == nil {
__stores.withLock { modifiableStore = modifiable
$0.append(modifiable)
} }
stores.append(modifiable)
} }
/// A boolean describing whether there are any Stores available. /// A boolean describing whether there are any Stores available.
public var anyAvailable: Bool { public var anyAvailable: Bool {
__stores.lockedValue.contains(where: \.isAvailable) stores.contains(where: \.isAvailable)
} }
public var allSecrets: [AnySecret] { public var allSecrets: [AnySecret] {
__stores.lockedValue.flatMap(\.secrets) stores.flatMap(\.secrets)
} }
} }

View File

@ -5,7 +5,20 @@ import CryptoKit
@preconcurrency import LocalAuthentication @preconcurrency import LocalAuthentication
import SecretKit import SecretKit
import os import os
import Common
public extension OSAllocatedUnfairLock where State: Sendable {
var lockedValue: State {
get {
withLock { $0 }
}
nonmutating set {
withLock { $0 = newValue }
}
}
}
extension SecureEnclave { extension SecureEnclave {

View File

@ -11,13 +11,13 @@ import Observation
@main @main
class AppDelegate: NSObject, NSApplicationDelegate { class AppDelegate: NSObject, NSApplicationDelegate {
private let storeList: SecretStoreList = { @MainActor private let storeList: SecretStoreList = {
let list = SecretStoreList() let list = SecretStoreList()
list.add(store: SecureEnclave.Store()) list.add(store: SecureEnclave.Store())
list.add(store: SmartCard.Store()) list.add(store: SmartCard.Store())
return list return list
}() }()
private let updater = Updater(checkOnLaunch: false) private let updater = Updater(checkOnLaunch: true)
private let notifier = Notifier() private let notifier = Notifier()
private let publicKeyFileStoreController = PublicKeyFileStoreController(homeDirectory: NSHomeDirectory()) private let publicKeyFileStoreController = PublicKeyFileStoreController(homeDirectory: NSHomeDirectory())
private lazy var agent: Agent = { private lazy var agent: Agent = {
@ -44,15 +44,15 @@ class AppDelegate: NSObject, NSApplicationDelegate {
} }
try? publicKeyFileStoreController.generatePublicKeys(for: storeList.allSecrets, clear: true) try? publicKeyFileStoreController.generatePublicKeys(for: storeList.allSecrets, clear: true)
notifier.prompt() notifier.prompt()
_ = withObservationTracking { // _ = withObservationTracking {
updater.update // updater.update
} onChange: { [updater, notifier] in // } onChange: { [updater, notifier] in
notifier.notify(update: updater.update!) { release in // notifier.notify(update: updater.update!) { release in
Task { // Task {
await updater.ignore(release: release) // await updater.ignore(release: release)
} // }
} // }
} // }
} }
} }

View File

@ -6,14 +6,19 @@ import SmartCardSecretKit
import Brief import Brief
extension EnvironmentValues { extension EnvironmentValues {
@Entry var secretStoreList: SecretStoreList = { private static let _secretStoreList: SecretStoreList = {
let list = SecretStoreList() let list = SecretStoreList()
Task { @MainActor in
list.add(store: SecureEnclave.Store()) list.add(store: SecureEnclave.Store())
list.add(store: SmartCard.Store()) list.add(store: SmartCard.Store())
}
return list return list
}() }()
@Entry var agentStatusChecker: any AgentStatusCheckerProtocol = AgentStatusChecker() @Entry var secretStoreList = _secretStoreList
@Entry var updater: any UpdaterProtocol = Updater(checkOnLaunch: false) private static let _agentStatusChecker = AgentStatusChecker()
@Entry var agentStatusChecker: any AgentStatusCheckerProtocol = _agentStatusChecker
private static let _updater: any UpdaterProtocol = Updater(checkOnLaunch: true)
@Entry var updater: any UpdaterProtocol = _updater
} }
@main @main
@ -29,7 +34,7 @@ struct Secretive: App {
WindowGroup { WindowGroup {
ContentView(showingCreation: $showingCreation, runningSetup: $showingSetup, hasRunSetup: $hasRunSetup) ContentView(showingCreation: $showingCreation, runningSetup: $showingSetup, hasRunSetup: $hasRunSetup)
// This one is explicitly injected via environment to support hasRunSetup. // This one is explicitly injected via environment to support hasRunSetup.
.environment(Updater(checkOnLaunch: hasRunSetup)) // .environment(Updater(checkOnLaunch: hasRunSetup))
.onAppear { .onAppear {
if !hasRunSetup { if !hasRunSetup {
showingSetup = true showingSetup = true

View File

@ -4,19 +4,21 @@ import AppKit
import SecretKit import SecretKit
import Observation import Observation
protocol AgentStatusCheckerProtocol: Observable { @MainActor protocol AgentStatusCheckerProtocol: Observable, Sendable {
var running: Bool { get } var running: Bool { get }
var developmentBuild: Bool { get } var developmentBuild: Bool { get }
func check() func check()
} }
@Observable class AgentStatusChecker: AgentStatusCheckerProtocol { @Observable @MainActor final class AgentStatusChecker: AgentStatusCheckerProtocol {
var running: Bool = false var running: Bool = false
init() { nonisolated init() {
Task { @MainActor in
check() check()
} }
}
func check() { func check() {
running = instanceSecretAgentProcess != nil running = instanceSecretAgentProcess != nil

View File

@ -2511,6 +2511,9 @@
} }
} }
} }
},
"No Update: %@" : {
}, },
"no_secure_storage_description" : { "no_secure_storage_description" : {
"localizations" : { "localizations" : {

View File

@ -104,7 +104,7 @@ extension Preview {
extension Preview { extension Preview {
static func storeList(stores: [Store] = [], modifiableStores: [StoreModifiable] = []) -> SecretStoreList { @MainActor static func storeList(stores: [Store] = [], modifiableStores: [StoreModifiable] = []) -> SecretStoreList {
let list = SecretStoreList() let list = SecretStoreList()
for store in stores { for store in stores {
list.add(store: store) list.add(store: store)

View File

@ -91,6 +91,8 @@ extension ContentView {
.popover(item: $selectedUpdate, attachmentAnchor: attachmentAnchor, arrowEdge: .bottom) { update in .popover(item: $selectedUpdate, attachmentAnchor: attachmentAnchor, arrowEdge: .bottom) { update in
UpdateDetailView(update: update) UpdateDetailView(update: update)
} }
} else {
Text("No Update: \(updater.update as Any)")
} }
} }