feat(keychain): refactor credential management and enhance API key retrieval#54
Merged
cooper (czxtm) merged 3 commits intomainfrom Apr 29, 2026
Merged
feat(keychain): refactor credential management and enhance API key retrieval#54cooper (czxtm) merged 3 commits intomainfrom
cooper (czxtm) merged 3 commits intomainfrom
Conversation
Scott McMaster (scottmcmaster)
requested a review
from Farhan (fkb032)
as a code owner
Contributor
📋 PR Overview
|
Copilot started reviewing on behalf of
Scott McMaster (scottmcmaster)
April 26, 2026 07:29
View session
There was a problem hiding this comment.
Pull request overview
Refactors native credential handling to move API keys out of settings.json and into an OS keychain-backed store, while adding env-var-first resolution to improve local dev and CI workflows.
Changes:
- Introduces a
credential_storeabstraction with lazy migration/cleanup from legacy plaintext settings into the keychain. - Updates provider construction (summary + evolve paths) to use “effective” credentials with environment-variable precedence.
- Enhances provider error classification by normalizing certain misleading 5xx auth-like failures to 401.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/native/src-tauri/src/store.rs | Adds env-first “effective” credential getters and keychain-backed secret preference helpers with lazy migration. |
| apps/native/src-tauri/src/credential_store.rs | New module implementing keychain + legacy settings stores and migration/cleanup helpers. |
| apps/native/src-tauri/src/providers/mod.rs | Switches provider creation to use effective credentials and centralized base URL constants. |
| apps/native/src-tauri/src/evolve/mod.rs | Uses effective credentials for evolve provider selection and removes duplicated base URL constants. |
| apps/native/src-tauri/src/commands.rs | Updates UI prefs retrieval to use effective (env-first) API key getters. |
| apps/native/src-tauri/src/provider_errors.rs | Adds status normalization to remap auth-like 5xx responses to 401, with tests. |
| apps/native/src-tauri/src/main.rs | Initializes tauri-plugin-keyring in both CLI and GUI modes. |
| apps/native/src-tauri/capabilities/default.json | Grants keyring permissions (get/set/delete/ping). |
| apps/native/src-tauri/Cargo.toml | Adds tauri-plugin-keyring dependency. |
| Cargo.lock | Locks transitive deps introduced by tauri-plugin-keyring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Scott McMaster (scottmcmaster)
requested review from
Cas Linden (CasLinden) and
cooper (czxtm)
Cas Linden (CasLinden)
approved these changes
Apr 28, 2026
Contributor
Cas Linden (CasLinden)
left a comment
Cas Linden (CasLinden)
left a comment
There was a problem hiding this comment.
I don't have comments on the code but I pulled this and tested this:
-
Plant dummy key in settings.json
-
Migration put it in keychain.
-
Then I tried to set a different dummy key and validation was rejected.
-
A free openrouter key was accepted,
-
The free model failed but tried an evolve
-
Couldn't verify the free key in keychain (security thing doing its job?)
-
I could however verify the dummy key (from the migration) in keychain. And this also passed the validation, unlike if I had put it into the input. But I am not sure if that is important.
Contributor
Merge activity
|
- Added a new `credential_store` module to handle API key storage and retrieval. - Introduced functions to get effective API keys with environment variable precedence for OpenRouter, OpenAI, and vLLM. - Updated existing functions in `store.rs` to utilize the new credential management system. - Refactored `generate_evolution` and `create_provider` functions to use the new API key retrieval methods. - Removed legacy API key retrieval logic and constants for OpenRouter and OpenAI from various modules. - Improved error handling and logging for credential access and migration. Co-authored-by: Copilot <copilot@github.com>
… management - Updated Cargo.toml to replace `keyring` with `tauri-plugin-keyring`. - Modified default.json to include keyring capabilities for password management. - Refactored credential_store.rs to utilize tauri-plugin-keyring for getting, setting, and deleting passwords. - Enhanced main.rs to initialize tauri-plugin-keyring in both CLI and GUI modes. - Improved error handling in provider_errors.rs to remap misleading 500 errors to 401 for authentication issues. - Simplified store.rs to resolve secrets with environment variable overrides, ensuring smoother integration with keychain. - Added tests for new functionality and ensured existing tests are updated accordingly. Co-authored-by: Copilot <copilot@github.com>
…alues in credential management per AI code review comments Co-authored-by: Copilot <copilot@github.com>
cooper (czxtm)
force-pushed
the
scott/keychain
branch
from
05eb3e9 to
0be60f2
Compare
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
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.
Summary
Refactor credential management by introducing a new
credential_storemodule that enhances API key retrieval with environment variable precedence for better local-dev and CI experience. Migrate fromkeyringtotauri-plugin-keyringfor improved password management and error handling, i.e. no longer store plaintext API keys in settings.json. Migrate legacy settings API keys to the Keychain.Also fixes another issue I found where vLLM API key errors throw back 500's instead of 400's which cause us to do a misleading error message.
For local development
Keychain access control is based on application identity, which for a production build is app identity and signing.
But for local dev, you're always rebuilding and not signing and therefore your keychain access needs to be refreshed all the time. If you prefer to not have to frequently type your password, then you can set one of the environment variables:
VLLM_API_KEYorOPENROUTER_API_KEYorOPENAI_API_KEYas appropriateand those take priority so that we can skip keychain access. Note that we previously supported env vars (albeit via some code that was badly in need of refactoring) but they were the last priority location for API keys. Now they are the first priority.
Test Plan
Added unit tests for new functionality, and ran through manual tests of various scenarios including:
Docs