feat(credentials): optional OS-keychain read fallback for CredentialStore - #246
Merged
Zongwei9888 merged 1 commit intoSep 27, 2026
Merged
Conversation
The credential store resolves from a 0600 JSON file only, so users who keep API keys in the OS keychain (Windows Credential Manager, macOS Keychain, the Linux Secret Service) have to duplicate them into a plaintext file. Add a read-only keychain fallback behind the DEEPCODE_KEYRING knob, consulted only when the file holds no value for a connection. keyring stays an optional import: the package is not a declared dependency, so a default install is unaffected and every import/backend failure degrades to the file. The file remains the source of truth and the only write target -- set/clear never touch the keychain -- which keeps revision()'s mtime/size fingerprint meaningful. The resolver chain is unchanged: a keychain-backed credential surfaces as credential_source="credential_store", because the keychain is that store's backend, not a fifth tier. (cherry picked from commit 6466dbd187814623029fc55521f391df3b930da4)
Zongwei9888
added a commit
that referenced
this pull request
Sep 27, 2026
…entialStore With DEEPCODE_KEYRING=1 and the optional keyring package, CredentialStore.get falls back to the OS keychain (service deepcode, user = connection id) when the credential file has no key; writes still go only to the file and every keychain failure degrades to the previous behaviour. Repair: document the setup. Contributed by raymondginger2018-sudo.
Collaborator
|
Merged into One addition on our side so the feature is discoverable: |
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
2 participants
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.
Description
Adds an opt-in OS-keychain read fallback to
CredentialStore, in the shape suggested when #188 was closed: the existing resolution order (env → credential store → legacy config) gains keychain support without a second store.keyringstays an undeclared optional import, and the JSON credential file remains the source of truth and the only write target.Related Issues
Refs #188 (closed) — reworked per review.
Changes Made
core/providers/credentials.py(+49):KEYRING_ENV_VAR = "DEEPCODE_KEYRING"(truthy spellings1/true/yes/on) gates a lazily importedkeyring.CredentialStore.get()falls back tokeyring.get_password(KEYRING_SERVICE, connection_id)only when the file has no value for that connection.strreturn — all returnNone, so the resolver falls through tolegacy_config. Turning the knob on cannot break a working setup.set/clearnever touch the keychain, which keepsrevision's mtime/size fingerprint meaningful and leaves ownership of the secret exactly where it is today.tests/test_provider_credentials.py(+216, 10 tests): not consulted by default; falsey knob spellings; keychain supplies an absent credential; the file still wins when both exist; missing package degrades; backend failure never escapes; non-string file value falls through; the store never writes to the keychain; a keychain-only credential leaves no revision trace; end-to-end resolution order.Checklist
ruff check+ruff format --checkclean)Additional Notes
DEEPCODE_KEYRINGis unset.setstoring into the keychain) is deliberately out of scope: it needs a decision on who owns the secret and on howrevisiontracks it. Happy to follow up separately if that is wanted.