fix(vault): cache CredsRoot in process to stop repeated Keychain prompts#394
Merged
Conversation
Without an in-process cache, every CredentialsVault::get_* /
get_active_asr / snapshot call drove load_credentials() →
load_keyring_credentials(), which reads the manifest entry plus every
chunk entry from the OS keyring. On macOS each distinct keychain entry
has its own ACL — so an ad-hoc-signed binary (or any binary whose ACL
grants haven't yet been "Always Allow"-ed) prompts the user on every
read of every entry. A single dictation cycle reads credentials 5–10
times, multiplied by (1 manifest + N chunks), produces tens of
"OpenLess wants to use the keychain" prompts per recording.
Add a process-wide CREDENTIALS_CACHE: OnceLock<Mutex<Option<CredsRoot>>>:
- load_credentials / load_credentials_for_update consult the cache
before falling through to Keychain. First read populates the cache;
every subsequent read in the same process is silent.
- save_credentials populates the cache with the cleaned root after
successful Keychain write, keeping Settings → Recording credential
edits visible immediately to dictation.
- migrate_legacy_sources{,_for_update} call save_credentials internally
so legacy-migration paths inherit the cache update for free.
Trade-off: cross-process changes (e.g. user runs `security` CLI
manually, or a second instance of the app — single-instance is enforced
but defense in depth) are invisible until next launch. Acceptable per
the credential vault contract: the keyring is owned by this app.
Note: this does NOT fix the underlying problem that ad-hoc-signed
builds change cdhash on every rebuild and lose "Always Allow" grants.
For that the build must use a stable Apple Developer ID signature
(APPLE_CERTIFICATE / APPLE_CERTIFICATE_PASSWORD / APPLE_TEAM_ID env
vars). The cache only ensures that within a single process lifetime,
the user is prompted at most once per Keychain entry.
185/185 lib tests pass. cargo check clean.
PR Reviewer Guide 🔍(Review updated until commit 8a9fb4f)Here are some key observations to aid the review process:
|
|
Persistent review updated to latest commit 2dc7ac5 |
pr_agent on PR #394 flagged a "Stale cache on read error" issue: both load_credentials / load_credentials_for_update were unconditionally writing the result into CREDENTIALS_CACHE, including the legacy-file fallback returned when load_keyring_credentials() hit an Err (e.g. user hasn't yet clicked "Always Allow" on the first keychain dialog, login keychain locked, DataProtection error). Effect: after a transient keyring failure the process pinned the fallback (or default) creds in cache for its entire lifetime — even after the user granted access, subsequent reads would never retry the keyring and would keep returning stale defaults / legacy content until the app is restarted. Fix: cache only on the Ok(Some) and Ok(None)→migrate paths. The Err path now returns the legacy fallback without populating the cache, so the next call retries the keyring and picks up the user's just-granted access. 185/185 lib tests pass; cargo check clean.
|
Persistent review updated to latest commit 8a9fb4f |
5 tasks
pull Bot
pushed a commit
to yimmy23/openless
that referenced
this pull request
May 10, 2026
10 PRs landed on beta this cycle: - Open-Less#377 paste shortcut configurable (issue Open-Less#360) - Open-Less#386 TS UserPreferences updateChannel alignment - Open-Less#387 focus_target leak on Processing-phase cancel - Open-Less#388 [严重] MacHotkeyAdapter::shutdown stops CFRunLoop + tap - Open-Less#389 emit_capsule window.show/hide off audio thread - Open-Less#390 QA / dictation hotkey routing race - Open-Less#391 audio-mute spawn_blocking (async hygiene) - Open-Less#392 hotkey supervisor + global dispatcher exit signal - Open-Less#393 post-audit logic-review hotfixes (QA mute .await + focus_target Processing branch) - Open-Less#394 in-process credentials cache (kills repeated Keychain prompts) Bump 4 files: package.json, tauri.conf.json, Cargo.toml, Cargo.lock.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Symptom
User reports macOS keychain prompts firing repeatedly during normal use, even after granting "Always Allow". Previously this was 2 prompts on first launch then silent; now it's many prompts per recording.
Root cause analysis
Two independent issues compound:
No in-process cache — `load_credentials()` re-reads from OS keyring on every `CredentialsVault::get_*` / `snapshot` call. The credentials are stored as a manifest entry + N chunks, each with its own macOS Keychain ACL. A single dictation reads credentials ~5-10 times → ~5-10 × (1 + N) prompts if ACL hasn't been granted yet.
Ad-hoc signing instability (out of scope for this PR) — every fresh build produces a new `cdhash`, so previous "Always Allow" grants don't bind to the new binary. Stable Apple Developer ID signing fixes this; the cache fix mitigates per-session impact but can't restore cross-build "Always Allow" persistence.
Fix (this PR)
Add a process-wide cache via `static CREDENTIALS_CACHE: OnceLock<Mutex<Option>>`:
Trade-off
Cross-process credential changes (user editing via `security` CLI, or another instance) are invisible until next app launch. Acceptable: the keyring is owned by this app, single-instance enforced.
Test plan
PR Type
Bug fix
Description
Add process-wide credentials cache
Reuse cached creds for reads
Refresh cache after successful saves
Avoid caching keyring error fallbacks
Diagram Walkthrough
File Walkthrough
persistence.rs
Add in-process credentials cachingopenless-all/app/src-tauri/src/persistence.rs
CREDENTIALS_CACHEas aOnceLock>>.load_credentialsandload_credentials_for_updatebefore hitting Keychain.credentials.