feat: allow selecting the Linux credential store explicitly - #144
Merged
Merged
Conversation
Add an options bag to Entry/AsyncEntry constructors and withTarget:
new Entry(svc, user, { linux: { store: 'secret-service' } })
new Entry(svc, user, { linux: { store: 'keyutils' } })
When a store is pinned, only that store is constructed and any failure
propagates, so requiring an unavailable store throws instead of silently
falling back to the kernel keyring. Without the option the existing
auto-fallback selection is unchanged. The option is accepted on every
platform but only meaningful on Linux; for the keyutils store a
withTarget target maps to the kernel key description.
Also regenerate the napi-js binding loader and type definitions with the
current @napi-rs/cli and emit string enums as union types
(napi.constEnum=false) so plain string literals typecheck.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8d29fecf-1e0a-42e3-a8bd-067b2043361d) |
Owner
Author
|
@codex review |
1 task
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.
Closes #142 (option 1 of the request).
Problem
On Linux,
Entry/AsyncEntrypick their credential store through an automatic fallback: the D-Bus Secret Service is tried first, and any error silently falls back to the kernel keyutils keyring. The two stores give different guarantees — keyutils is in-memory only and credentials vanish on reboot — and the public API offers no way to require one store or even learn which one won. An app that needs durable storage can report a successful login and then "forget" the credential after a reboot, with no visible cause.What this PR adds
An options bag on the
Entry/AsyncEntryconstructors andwithTarget:CredentialStoreApi::build, leaving the process-global default store untouched.withTarget+ keyutils: the target maps to the kernel keydescriptionmodifier, so distinct targets keep distinct credentials (added a Linux regression test for this).Also in this PR
entry.rs/async_entry.rsintosrc/entry_builder.rs— no behavior change on non-Linux platforms; the WindowswithTargetplaceholder hack is preserved.@napi-rs/cli(template refresh), and setnapi.constEnum: falsesoLinuxStoreemits as the union'secret-service' | 'keyutils'— plain string literals typecheck.Verification
cargo fmt --check,cargo clippy --all-targets,cargo test— clean.tsc --noEmitover the test suite confirms the literal syntax typechecks and invalid values don't.secret-service/keyutilsround-trips forEntryandAsyncEntry, and keyutils target isolation. The pinned paths could not be exercised on a real Linux machine during development — they are compiler-checked code paths gated behindcfg(target_os = "linux")plus the existing keyring-core store APIs, so CI is the first real run.Out of scope (pre-existing, noted for follow-up)
findCredentials/findCredentialsAsyncalways talk to the Secret Service directly and never see the keyutils fallback.targetmodifiers to whichever store won, which the keyutils store rejects — unchanged here.@napi-rs/cli3.1.5 loader template no longerreturns theNAPI_RS_NATIVE_LIBRARY_PATHbinding before falling through to the WASI fallback; fixing that belongs in the CLI/template, not this feature.Note
Medium Risk
Changes how Linux credentials are stored and when constructors throw; mis-pinned stores or expecting keyutils durability could break apps, though defaults are unchanged and the option is ignored off Linux.
Overview
Adds an optional
EntryOptionsbag onEntry/AsyncEntryconstructors andwithTarget, so Linux callers can pin credentials tosecret-serviceorkeyutilsinstead of relying on the default Secret Service → keyutils auto-fallback. When a store is pinned, construction fails if that backend is unavailable (no silent fallback); omitting the option keeps existing behavior. Pinned entries are built viaCredentialStoreApi::buildwithout touching the process-global default store.Rust entry creation is centralized in
entry_builder.rs(store setup and WindowswithTargetusername preservation moved out ofentry.rs/async_entry.rs). For keyutils +withTarget, the target maps to the kernel keydescriptionmodifier so targets stay isolated.Ships updated TypeScript types (
LinuxStoreas string literals vianapi.constEnum: false), README guidance on Linux backends and keyutils non-persistence, AVA tests for pinned stores / cross-platform ignore / invalid store values, and a regenerated napi-rsindex.jsloader template.Reviewed by Cursor Bugbot for commit 69063f7. Bugbot is set up for automated code reviews on this repo. Configure here.