Skip to content
Merged

pr #14

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 7 additions & 45 deletions dhavnii/Core/Security/SecureStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,35 +41,14 @@ internal enum SecureStorage {
throw KeychainError.conversionFailed
}

// Create an Access object with no trusted-application list.
//
// When a keychain item is stored WITHOUT an explicit SecAccess, macOS
// automatically creates an ACL tied to the storing app's code signature.
// For an unsigned (or ad-hoc-signed) app this means every new build gets
// a different signature, and macOS prompts the user for their system
// password on every launch because it sees a "different" app trying to
// read the item.
//
// Passing a SecAccess with an empty trusted-application array and the
// kSecACLAuthorizationAny flag tells the keychain to allow any
// application to read this item without prompting. This is the correct
// approach for unsigned, non-sandboxed apps that distribute outside the
// Mac App Store.
var access: SecAccess?
let accessStatus = SecAccessCreate("openwispher_api_keys" as CFString, [] as CFArray, &access)
guard accessStatus == errSecSuccess, let secAccess = access else {
throw KeychainError.invalidStatus(accessStatus)
}

let query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrAccount as String: key,
kSecAttrService as String: "openwispher_api_keys",
kSecValueData as String: data,
// AfterFirstUnlock: accessible after the user logs in once per boot,
// without prompting for the system password on every app launch.
kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
kSecAttrAccess as String: secAccess
kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve explicit keychain ACL when adding API keys

The new SecItemAdd query no longer includes kSecAttrAccess, so macOS falls back to a default ACL bound to the app’s current code signature; in this repo’s unsigned distribution context (dhavnii/UpdateManager.swift:5), that can cause existing saved keys to trigger password prompts again after rebuilds/updates because the signer identity changes. The removed SecAccessCreate(..., [] ...) path was the compatibility mechanism for that case, so this change reintroduces a launch-time keychain access regression for users who persist API keys.

Useful? React with 👍 / 👎.

]

let status = SecItemAdd(query as CFDictionary, nil)
Expand Down Expand Up @@ -144,32 +123,15 @@ internal enum SecureStorage {
}


/// Re-write any existing keychain items to use the AfterFirstUnlock accessibility level.
/// Run once on launch to silently upgrade keys stored under the old WhenUnlocked attribute,
/// which caused a system-password prompt on every fresh app launch.
/// Legacy migration entry point kept for compatibility.
///
/// Earlier versions attempted to migrate all providers on startup, which could trigger
/// repeated system-password prompts for each key. We now avoid any keychain I/O here and
/// only mark migration as completed.
internal static func migrateKeychainAccessibility() {
let migrationKey = "com.openwispher.keychainAccessibilityMigrated.v1"
guard !UserDefaults.standard.bool(forKey: migrationKey) else { return }

let providers: [TranscriptionProviderType] = [.groq, .elevenLabs, .deepgram, .sarvam]
var allSucceeded = true
for provider in providers {
// Read the existing value (if any) — this may still prompt once
// during this single migration run, but never again afterwards.
guard let existingKey = retrieveAPIKey(for: provider), !existingKey.isEmpty else { continue }
// Re-store with the new accessibility attribute (delete-then-add inside storeAPIKey)
do {
try storeAPIKey(existingKey, for: provider)
} catch {
print("⚠️ SecureStorage: failed to migrate keychain accessibility for \(provider.rawValue): \(error)")
allSucceeded = false
}
}

// Only mark migration complete if every present key was successfully re-written.
if allSucceeded {
UserDefaults.standard.set(true, forKey: migrationKey)
}
UserDefaults.standard.set(true, forKey: migrationKey)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not mark keychain migration complete before migrating

migrateKeychainAccessibility() now sets the migration flag immediately without rewriting any stored items, so users who still have legacy keychain entries will never be upgraded because later launches exit early on this flag. The previous implementation only marked completion after per-provider rewrite attempts; with this change, affected upgrades can remain permanently on legacy keychain settings unless users manually delete/re-enter keys.

Useful? React with 👍 / 👎.

}

/// Migrate existing UserDefaults keys to Keychain (one-time migration)
Expand Down
4 changes: 2 additions & 2 deletions openwispher/openwispherApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ private struct AppContentView: View {
appState.hasCompletedOnboarding = hasCompleted

if hasCompleted {
// Migrate existing keychain items to AfterFirstUnlock accessibility (one-time, silent).
// This also warms up the in-memory cache for all providers, so no extra read is needed.
// Mark legacy migration as completed without touching keychain items on launch.
// Startup keychain scans were causing repeated password prompts.
SecureStorage.migrateKeychainAccessibility()
}

Expand Down