From 47b8e1e7e3f11827c12c8b687f1760f099ea1432 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:30:35 +0200 Subject: [PATCH 1/8] feat(keychain): classify locked collections with ErrCollectionLocked and bound the unlock prompt On headless Linux hosts (SSH key-only logins), PAM has no password to auto-unlock the login keyring with, so the Secret Service collection comes up locked after every keyring-daemon restart. The store's only reaction to a locked collection was Service.Unlock -> PromptAndWait, whose failures surfaced as opaque strings ("failed to prompt: prompt dismissed" / "prompt timed out") that callers cannot classify, and whose wait was a hardcoded 30s regardless of the caller's deadline. - Export ErrCollectionLocked in the cross-platform keychain.go (mirroring ErrKeychainUnavailable / ErrNoDefaultCollection; Linux-only match). Every path that fails because the collection is locked and could not be unlocked now wraps it, naming the collection and preserving the prompt failure as the cause: the up-front unlock (new ensureCollectionUnlocked helper, replacing five duplicated blocks), the re-unlock inside withRelockRetry, and a collection still locked after the bounded retries. - Bound the prompt wait by the operation's context: PromptAndWait (and the prompt-capable Unlock/LockItems/CreateItem/DeleteItem) now take a ctx. Store operations pass their original ctx - deliberately not the context.WithoutCancel connection ctx - so a caller deadline bounds the human-wait while in-flight D-Bus calls stay protected from teardown. The internal 30s cap remains as an upper bound and is now created once outside the receive loop, so unrelated bus signals no longer reset it. A null prompt still returns before the ctx check, keeping best-effort cleanup calls working on passwordless keyrings. - Add fake-seam tests for every operation against a locked collection, and a new ubuntu-24-gnome-keyring-locked CI target that runs an env-gated live test against a password-protected keyring (created PAM-style via gnome-keyring-daemon --login) with the collection locked, asserting the operation fails fast with ErrCollectionLocked. Validated live on an Ubuntu 24.04 VM: headless locked operations fail in ~15ms with the classified error (gnome-keyring dismisses the prompt immediately when no prompter can be shown), an interactive unlock prompt on a display still completes, and a 2s caller deadline aborts an unanswered prompt at 2s. Deliberately not added (see the decision log): prompter-presence probes, password callbacks, programmatic master-password unlock, and library TTY prompting - the caller owns remediation, and a locked collection must not be treated as "unavailable, fall back", which would split credentials across stores. Co-Authored-By: Claude Fable 5 --- .github/workflows/keychain.yml | 4 + store/Dockerfile | 9 ++ store/docker-bake.hcl | 12 +- store/docs/keychain/decision-logs.md | 52 ++++++ store/docs/keychain/design.md | 45 ++++++ store/keychain/README.md | 33 ++++ .../secretservice/secretservice.go | 56 +++++-- .../secretservice/secretservice_test.go | 14 +- store/keychain/keychain.go | 28 ++++ store/keychain/keychain_linux.go | 122 +++++++------- store/keychain/keychain_linux_test.go | 150 ++++++++++++++++-- store/scripts/gnome-keyring-locked | 91 +++++++++++ 12 files changed, 531 insertions(+), 85 deletions(-) create mode 100755 store/scripts/gnome-keyring-locked diff --git a/.github/workflows/keychain.yml b/.github/workflows/keychain.yml index bb8bd0c1..7beae835 100644 --- a/.github/workflows/keychain.yml +++ b/.github/workflows/keychain.yml @@ -21,6 +21,10 @@ jobs: subtest: - fedora-43-gnome-keyring - ubuntu-24-gnome-keyring + # password-protected keyring, locked collection: asserts store ops + # fail fast with ErrCollectionLocked instead of hanging on a prompt + # nothing can answer + - ubuntu-24-gnome-keyring-locked # disabled kdewallet tests since it prompts for a password in a # headless environment... need to still fix this # - fedora-43-kdewallet diff --git a/store/Dockerfile b/store/Dockerfile index 892ce5d3..345f8f61 100644 --- a/store/Dockerfile +++ b/store/Dockerfile @@ -61,6 +61,15 @@ RUN --mount=type=bind,target=. \ --mount=type=cache,target=/root/.cache/go-build \ bash -c "set -euxo pipefail; /app/store/scripts/gnome-keyring" +FROM ubuntu24 AS ubuntu-24-gnome-keyring-locked +ENV CGO_ENABLED=0 +USER user +WORKDIR /app +RUN --mount=type=bind,target=. \ + --mount=type=cache,target=/go/pkg/mod \ + --mount=type=cache,target=/root/.cache/go-build \ + bash -c "set -euxo pipefail; bash /app/store/scripts/gnome-keyring-locked" + FROM ubuntu24 AS ubuntu-24-kdewallet ENV CGO_ENABLED=0 USER user diff --git a/store/docker-bake.hcl b/store/docker-bake.hcl index ca409c27..4440645e 100644 --- a/store/docker-bake.hcl +++ b/store/docker-bake.hcl @@ -19,7 +19,8 @@ group "default" { # it just prompts anyway... # "fedora_43_kdewallet", # "ubuntu_24_kdewallet", - "ubuntu_24_gnome_keyring" + "ubuntu_24_gnome_keyring", + "ubuntu_24_gnome_keyring_locked" ] } @@ -62,3 +63,12 @@ target "ubuntu_24_gnome_keyring" { GO_VERSION = GO_VERSION } } + +target "ubuntu_24_gnome_keyring_locked" { + dockerfile = "store/Dockerfile" + target = "ubuntu-24-gnome-keyring-locked" + context = "." + args = { + GO_VERSION = GO_VERSION + } +} diff --git a/store/docs/keychain/decision-logs.md b/store/docs/keychain/decision-logs.md index 00b52c24..ca2989a1 100644 --- a/store/docs/keychain/decision-logs.md +++ b/store/docs/keychain/decision-logs.md @@ -135,3 +135,55 @@ every store operation (not only the probe) benefits. thread it into the dial (via `operationService`) like the other operations. --- + +2026-08-25 Locked collections fail fast with ErrCollectionLocked; unlock prompt bounded + +On headless Linux hosts (SSH key-only login, gnome-keyring) the login +collection comes up locked after every keyring-daemon restart — PAM has no +password to auto-unlock it with. The store's only reaction to a locked +collection was `Service.Unlock` → `PromptAndWait`, whose failures surfaced as +opaque strings ("failed to prompt: prompt dismissed" / "prompt timed out") +that downstream consumers could not classify. + +Decisions: + +- **One exported sentinel, `ErrCollectionLocked`**, declared in the + cross-platform `keychain.go` (mirroring `ErrKeychainUnavailable` / + `ErrNoDefaultCollection`), Linux-only behavior. Every path that fails + because the collection is locked and could not be unlocked wraps it: the + up-front unlock in `ensureCollectionUnlocked` (which also names the + collection in the message), the re-unlock inside `withRelockRetry`, and a + collection still locked once the bounded retries are exhausted. The + underlying prompt failure is preserved as the wrapped cause; no exported + prompt-dismissed/timed-out sentinels (unexported-cause promotion pattern, + same as `errSessionBusUnavailable`). +- **The prompt wait is ctx-bounded.** `PromptAndWait` (and the prompt-capable + calls `Unlock`, `LockItems`, `CreateItem`, `DeleteItem`) now take a + `context.Context`; store operations pass their ORIGINAL operation ctx — + deliberately not the `context.WithoutCancel` connection ctx — so a caller + deadline bounds the human-wait while in-flight D-Bus calls stay protected + from teardown. The internal 30s cap remains as an upper bound, created once + outside the receive loop (previously `time.After` inside the loop was reset + by every unrelated bus signal). A null prompt returns before the ctx check, + so best-effort cleanup calls with cancelled contexts still succeed on + passwordless keyrings. +- **Nothing else was added, deliberately.** Evaluated and rejected: + prompter-presence probes (`org.gnome.keyring.SystemPrompter` is + activatable-but-unstartable on headless hosts with gcr installed, and + KWallet/KeePassXC never own that name — both directions misclassify); + password callbacks and programmatic master-password unlock via + `org.gnome.keyring.InternalUnsupportedGuiltRiddenInterface` (gnome-only, + and hands the library a UX/credential-handling responsibility the caller + owns); library TTY prompting; and a New-time lock check (lock state is + per-operation and mutable; locked ≠ unavailable). The caller detects + `ErrCollectionLocked` via `errors.Is` and owns remediation messaging — and + must NOT fall back to another store, which would split credentials. +- Validated live (Ubuntu 24.04 VM, gnome-keyring): headless + locked fails in + ~15ms with "prompt dismissed" (gnome-keyring dismisses immediately when no + prompter can be shown, with or without gcr installed); a real prompt on a + display still completes and unlocks; a 2s caller deadline aborts an + unanswered prompt at 2.003s. CI: new `ubuntu-24-gnome-keyring-locked` + target runs `TestKeychainLiveLockedCollection` against a password-protected + keyring (`gnome-keyring-daemon --login`) with the collection locked. + +--- diff --git a/store/docs/keychain/design.md b/store/docs/keychain/design.md index b095e223..1afe5a44 100644 --- a/store/docs/keychain/design.md +++ b/store/docs/keychain/design.md @@ -92,3 +92,48 @@ collection exists, so a reachable-but-uninitialized keyring still passes `New` and surfaces `ErrNoDefaultCollection` lazily on the first operation, as before. On macOS and Windows the check is a no-op (`New` never returns `ErrKeychainUnavailable` there). + +### Locked collections and the bounded unlock prompt + +A reachable backend can still hold a **locked** collection — the default state +on headless hosts with SSH key-only logins, where PAM has no password to +auto-unlock the login keyring with, so it relocks on every keyring-daemon +restart. + +Every store operation checks the collection's lock state up front +(`ensureCollectionUnlocked`) and, when locked, issues a Secret Service +`Unlock`. On a passwordless keyring that completes silently via the null +prompt. On a password-protected keyring it opens the backend's unlock prompt, +and that prompt wait is **bounded twice**: + +- by the operation's own `ctx` (deliberately the caller's original context, + not the `context.WithoutCancel` connection context: in-flight D-Bus calls + are protected from teardown, but waiting on a human is bounded by the + caller); and +- by an internal 30s cap (`promptTimeout`), so a prompt nobody can ever answer + cannot block an operation forever even without a caller deadline. + +When the unlock fails — prompt dismissed, timed out, or ctx expired — the +operation fails with an error wrapping the exported `ErrCollectionLocked` +sentinel and naming the collection, with the underlying prompt failure +preserved as the cause. The same wrapping applies inside the relock-retry loop +(`withRelockRetry`) and when a collection is still locked after the bounded +retries. + +Empirically (validated live on Ubuntu 24.04, gnome-keyring): on a headless +host the unlock prompt does not hang — gnome-keyring completes it as +*dismissed* within milliseconds when no prompter can be shown (whether or not +the gcr prompter is installed and D-Bus-activatable), so the locked error +surfaces in ~15ms. The 30s cap and ctx bound cover the remaining case of a +live prompter with an absent user. + +Deliberately **not** built (see the decision log): prompter-presence probes +(`org.gnome.keyring.SystemPrompter` is activatable-but-unstartable on headless +hosts with gcr installed, and KWallet/KeePassXC never own that name — both +directions misclassify), password callbacks, programmatic master-password +unlock (`org.gnome.keyring.InternalUnsupportedGuiltRiddenInterface`), and +library-owned TTY prompting. The library reports the locked state reliably; +the caller owns remediation. `ErrCollectionLocked` also must not be treated as +"unavailable, fall back": the locked collection still holds the user's +credentials, and silently writing new ones to a fallback store would split +credentials across two stores. diff --git a/store/keychain/README.md b/store/keychain/README.md index 680d1e9b..38549a30 100644 --- a/store/keychain/README.md +++ b/store/keychain/README.md @@ -64,6 +64,39 @@ daemon whether the Secret Service is registered and never touches your stored secrets. On macOS and Windows the check is a no-op (and `ctx` is unused). See [../docs/keychain/design.md](../docs/keychain/design.md) for details. +### Locked collections (Linux) + +A reachable keychain can still hold a **locked** collection. This is the +default state on headless Linux hosts with SSH key-only logins: there is no +password for PAM to auto-unlock the login keyring with, so the collection comes +up locked after every keyring-daemon restart. + +When a store operation encounters a locked collection it asks the Secret +Service to unlock it. On a passwordless keyring that succeeds silently; on a +password-protected keyring it opens the backend's unlock prompt. If that prompt +cannot complete — it is dismissed (gnome-keyring does this immediately when no +prompter can be shown, e.g. headless), times out, or the operation's context +expires — the operation fails with an error matching +`keychain.ErrCollectionLocked`: + +```go +_, err := st.Get(ctx, id) +if errors.Is(err, keychain.ErrCollectionLocked) { + // The collection still holds the user's credentials; it just needs to be + // unlocked. Tell the user how (e.g. log in to the desktop session, or + // `gnome-keyring-daemon --unlock`). Do NOT fall back to another store: + // writing new credentials elsewhere while the locked collection keeps the + // old ones would split credentials across two stores. +} +``` + +The operation's `ctx` bounds the unlock-prompt wait, so a caller can put its +own deadline on the "waiting for the user to type the keyring password" case; +an internal cap (30s) always applies as an upper bound. `ErrCollectionLocked` +is deliberately distinct from `ErrKeychainUnavailable`: unavailable means no +keychain exists to use (fall back), locked means the keychain and credentials +exist but need the user's help (surface remediation, don't fall back). + ### Secrets The `keychain` assumes that any secret stored would conform to the `store.Secret` diff --git a/store/keychain/internal/go-keychain/secretservice/secretservice.go b/store/keychain/internal/go-keychain/secretservice/secretservice.go index 81cc2bf2..ce73d1aa 100644 --- a/store/keychain/internal/go-keychain/secretservice/secretservice.go +++ b/store/keychain/internal/go-keychain/secretservice/secretservice.go @@ -412,8 +412,16 @@ const ReplaceBehaviorDoNotReplace = 0 // ReplaceBehaviorReplace const ReplaceBehaviorReplace = 1 -// CreateItem -func (s *SecretService) CreateItem(collection dbus.ObjectPath, properties map[string]dbus.Variant, secret Secret, replaceBehavior ReplaceBehavior) (item dbus.ObjectPath, err error) { +// CreateItem creates an item in the collection. The call can open a prompt +// (e.g. when the collection relocked in the meantime); ctx bounds that prompt +// wait (see [SecretService.PromptAndWait]). +func (s *SecretService) CreateItem( + ctx context.Context, + collection dbus.ObjectPath, + properties map[string]dbus.Variant, + secret Secret, + replaceBehavior ReplaceBehavior, +) (item dbus.ObjectPath, err error) { var replace bool switch replaceBehavior { case ReplaceBehaviorDoNotReplace: @@ -431,15 +439,16 @@ func (s *SecretService) CreateItem(collection dbus.ObjectPath, properties map[st if err != nil { return "", fmt.Errorf("failed to create item: %w", err) } - _, err = s.PromptAndWait(prompt) + _, err = s.PromptAndWait(ctx, prompt) if err != nil { return "", err } return item, nil } -// DeleteItem -func (s *SecretService) DeleteItem(item dbus.ObjectPath) (err error) { +// DeleteItem deletes an item. The call can open a prompt; ctx bounds that +// prompt wait (see [SecretService.PromptAndWait]). +func (s *SecretService) DeleteItem(ctx context.Context, item dbus.ObjectPath) (err error) { var prompt dbus.ObjectPath err = s.Obj(item). Call("org.freedesktop.Secret.Item.Delete", NilFlags). @@ -447,7 +456,7 @@ func (s *SecretService) DeleteItem(item dbus.ObjectPath) (err error) { if err != nil { return fmt.Errorf("failed to delete item: %w", err) } - _, err = s.PromptAndWait(prompt) + _, err = s.PromptAndWait(ctx, prompt) if err != nil { return err } @@ -501,8 +510,10 @@ func (s *SecretService) GetSecret(item dbus.ObjectPath, session Session) (secret // NullPrompt const NullPrompt = "/" -// Unlock -func (s *SecretService) Unlock(items []dbus.ObjectPath) (err error) { +// Unlock unlocks the given collections or items. On a password-protected +// keyring this opens the backend's unlock prompt; ctx bounds that prompt wait +// (see [SecretService.PromptAndWait]). +func (s *SecretService) Unlock(ctx context.Context, items []dbus.ObjectPath) (err error) { var dummy []dbus.ObjectPath var prompt dbus.ObjectPath err = s.ServiceObj(). @@ -511,15 +522,16 @@ func (s *SecretService) Unlock(items []dbus.ObjectPath) (err error) { if err != nil { return fmt.Errorf("failed to unlock items: %w", err) } - _, err = s.PromptAndWait(prompt) + _, err = s.PromptAndWait(ctx, prompt) if err != nil { return fmt.Errorf("failed to prompt: %w", err) } return nil } -// LockItems -func (s *SecretService) LockItems(items []dbus.ObjectPath) (err error) { +// LockItems locks the given collections or items. The call can open a prompt; +// ctx bounds that prompt wait (see [SecretService.PromptAndWait]). +func (s *SecretService) LockItems(ctx context.Context, items []dbus.ObjectPath) (err error) { var dummy []dbus.ObjectPath var prompt dbus.ObjectPath err = s.ServiceObj(). @@ -528,7 +540,7 @@ func (s *SecretService) LockItems(items []dbus.ObjectPath) (err error) { if err != nil { return fmt.Errorf("failed to lock items: %w", err) } - _, err = s.PromptAndWait(prompt) + _, err = s.PromptAndWait(ctx, prompt) if err != nil { return fmt.Errorf("failed to prompt: %w", err) } @@ -545,8 +557,19 @@ func (p PromptDismissedError) Error() string { return p.err.Error() } +// promptTimeout caps how long PromptAndWait waits for the user to answer a +// prompt when the caller's ctx carries no (earlier) deadline of its own, so a +// prompt that nobody will ever answer cannot block an operation forever. +const promptTimeout = 30 * time.Second + +// PromptAndWait displays the given prompt and blocks until the user answers +// it, the prompt is dismissed, ctx is done, or promptTimeout elapses — +// whichever comes first. ctx lets a caller bound the human-wait with its own +// deadline or cancellation; the promptTimeout cap always applies as an upper +// bound. A NullPrompt returns immediately with no error. +// // PromptAndWait is NOT thread-safe. -func (s *SecretService) PromptAndWait(prompt dbus.ObjectPath) (paths *dbus.Variant, err error) { +func (s *SecretService) PromptAndWait(ctx context.Context, prompt dbus.ObjectPath) (paths *dbus.Variant, err error) { if prompt == NullPrompt { return nil, nil } @@ -554,6 +577,9 @@ func (s *SecretService) PromptAndWait(prompt dbus.ObjectPath) (paths *dbus.Varia if call.Err != nil { return nil, fmt.Errorf("failed to prompt: %w", call.Err) } + // The timer is created once, outside the receive loop, so unrelated bus + // signals cannot keep resetting the timeout. + timeout := time.After(promptTimeout) for { var result PromptCompletedResult select { @@ -575,7 +601,9 @@ func (s *SecretService) PromptAndWait(prompt dbus.ObjectPath) (paths *dbus.Varia return nil, PromptDismissedError{errors.New("prompt dismissed")} } return &result.Paths, nil - case <-time.After(30 * time.Second): + case <-ctx.Done(): + return nil, fmt.Errorf("prompt aborted: %w", ctx.Err()) + case <-timeout: return nil, errors.New("prompt timed out") } } diff --git a/store/keychain/internal/go-keychain/secretservice/secretservice_test.go b/store/keychain/internal/go-keychain/secretservice/secretservice_test.go index 634d2999..a9899ea2 100644 --- a/store/keychain/internal/go-keychain/secretservice/secretservice_test.go +++ b/store/keychain/internal/go-keychain/secretservice/secretservice_test.go @@ -38,10 +38,10 @@ func testKeyring(t *testing.T, mode AuthenticationMode) { secret, err := session.NewSecret([]byte("secret")) require.NoError(t, err) - err = srv.Unlock([]dbus.ObjectPath{collection}) + err = srv.Unlock(t.Context(), []dbus.ObjectPath{collection}) require.NoError(t, err) - _, err = srv.CreateItem(collection, NewSecretProperties("testlabel", map[string]string{"foo": "bar"}), secret, ReplaceBehaviorReplace) + _, err = srv.CreateItem(t.Context(), collection, NewSecretProperties("testlabel", map[string]string{"foo": "bar"}), secret, ReplaceBehaviorReplace) require.NoError(t, err) items, err = srv.SearchCollection(collection, map[string]string{"foo": "bar"}) @@ -52,10 +52,10 @@ func testKeyring(t *testing.T, mode AuthenticationMode) { require.NoError(t, err) require.Equal(t, secretPlaintext, []byte("secret")) - err = srv.DeleteItem(gotItem) + err = srv.DeleteItem(t.Context(), gotItem) require.NoError(t, err) - err = srv.LockItems([]dbus.ObjectPath{collection}) + err = srv.LockItems(t.Context(), []dbus.ObjectPath{collection}) require.NoError(t, err) } @@ -71,16 +71,16 @@ func TestGetAll(t *testing.T) { secret, err := session.NewSecret([]byte("secret")) require.NoError(t, err) - err = srv.Unlock([]dbus.ObjectPath{collection}) + err = srv.Unlock(t.Context(), []dbus.ObjectPath{collection}) require.NoError(t, err) - item, err := srv.CreateItem(collection, NewSecretProperties("testlabel", map[string]string{"username": "testuser"}), secret, ReplaceBehaviorReplace) + item, err := srv.CreateItem(t.Context(), collection, NewSecretProperties("testlabel", map[string]string{"username": "testuser"}), secret, ReplaceBehaviorReplace) require.NoError(t, err) attrs, err := srv.GetAttributes(item) require.NoError(t, err) require.Equal(t, attrs["username"], "testuser") - err = srv.DeleteItem(item) + err = srv.DeleteItem(t.Context(), item) require.NoError(t, err) } diff --git a/store/keychain/keychain.go b/store/keychain/keychain.go index 7fe48bf9..6eca50c0 100644 --- a/store/keychain/keychain.go +++ b/store/keychain/keychain.go @@ -71,6 +71,34 @@ var ErrNoDefaultCollection = errors.New("no default keychain collection availabl // ErrNoDefaultCollection lazily on the first operation, exactly as before. var ErrKeychainUnavailable = errors.New("keychain backend unavailable") +// ErrCollectionLocked is returned by store operations when the keychain +// collection is locked and could not be unlocked: the backend's unlock prompt +// was dismissed (which is what gnome-keyring does immediately when no prompter +// can be shown, e.g. on a headless host), timed out, or was aborted by the +// operation's context. +// +// This is DISTINCT from [ErrKeychainUnavailable]: the backend is reachable and +// the collection exists — it still holds the user's credentials, but they +// cannot be read or written until the user unlocks the keyring (for example +// via their desktop session, or `gnome-keyring-daemon --unlock`). Callers +// should surface that remediation to the user rather than fall back to a +// different store: writing new credentials elsewhere while the locked +// collection still holds the old ones would split credentials across stores. +// +// A common cause on headless hosts: with SSH key-only logins there is no +// password for PAM to auto-unlock the login keyring with, so the collection is +// locked after every keyring-daemon restart. +// +// NOTE: like the sentinels above this condition is currently specific to the +// Linux keyring (the freedesktop Secret Service). It is declared here, in the +// cross-platform file, so platform-agnostic callers can reference it on every +// platform without build tags; on non-Linux platforms it simply never matches. +// +// It is exported so callers can use [errors.Is] to detect the locked state and +// present an actionable message, rather than relying on fragile error message +// comparisons. +var ErrCollectionLocked = errors.New("keychain collection is locked") + type ( Option interface{ apply(any) error } optionFunc[K any] func(K) error diff --git a/store/keychain/keychain_linux.go b/store/keychain/keychain_linux.go index cbf67e7e..c9d96424 100644 --- a/store/keychain/keychain_linux.go +++ b/store/keychain/keychain_linux.go @@ -60,10 +60,10 @@ type secretService interface { IsLocked(collection dbus.ObjectPath) (bool, error) OpenSession(mode kc.AuthenticationMode) (*kc.Session, error) CloseSession(session *kc.Session) - Unlock(items []dbus.ObjectPath) error + Unlock(ctx context.Context, items []dbus.ObjectPath) error SearchCollection(collection dbus.ObjectPath, attributes kc.Attributes) ([]dbus.ObjectPath, error) - CreateItem(collection dbus.ObjectPath, properties map[string]dbus.Variant, secret kc.Secret, replaceBehavior kc.ReplaceBehavior) (dbus.ObjectPath, error) - DeleteItem(item dbus.ObjectPath) error + CreateItem(ctx context.Context, collection dbus.ObjectPath, properties map[string]dbus.Variant, secret kc.Secret, replaceBehavior kc.ReplaceBehavior) (dbus.ObjectPath, error) + DeleteItem(ctx context.Context, item dbus.ObjectPath) error GetAttributes(item dbus.ObjectPath) (kc.Attributes, error) GetSecret(item dbus.ObjectPath, session kc.Session) ([]byte, error) SetItemSecret(item dbus.ObjectPath, secret kc.Secret) error @@ -224,11 +224,9 @@ func resolveDefaultCollection(collections []dbus.ObjectPath, aliasPath dbus.Obje return aliasPath, nil } -var errCollectionLocked = errors.New("collection is locked") - // isCollectionUnlocked verifies if the collection is unlocked. // -// It returns the errCollectionLocked error by default if the collection is locked. +// It returns [ErrCollectionLocked] by default if the collection is locked. // On any other error, it returns the underlying error instead. func isCollectionUnlocked(collectionPath dbus.ObjectPath, service secretService) error { locked, err := service.IsLocked(collectionPath) @@ -238,7 +236,45 @@ func isCollectionUnlocked(collectionPath dbus.ObjectPath, service secretService) if !locked { return nil } - return errCollectionLocked + return ErrCollectionLocked +} + +// lockedError wraps cause under the exported [ErrCollectionLocked] sentinel, +// naming the collection so the message is actionable on its own. +func lockedError(collectionPath dbus.ObjectPath, cause error) error { + return fmt.Errorf("%w: could not unlock collection %q: %w", ErrCollectionLocked, collectionPath, cause) +} + +// ensureCollectionUnlocked checks the collection's lock state and, when +// locked, asks the secret service to unlock it. On a passwordless keyring +// (e.g. the PAM-unlocked login keyring) that unlock completes silently via the +// null prompt; on a password-protected keyring it opens the backend's unlock +// prompt. +// +// ctx bounds the prompt wait — deliberately the caller's ORIGINAL operation +// context, not the [context.WithoutCancel] connection context from +// [operationService]: in-flight D-Bus operations are protected from teardown, +// but waiting on a human is bounded by the caller's deadline or cancellation +// (and by the backstop timeout in the secretservice package). A null prompt is +// unaffected by ctx, so best-effort cleanup calls with an already-cancelled +// ctx still succeed on passwordless keyrings. +// +// When the unlock fails — the prompt was dismissed (gnome-keyring does this +// immediately when no prompter can be shown, e.g. headless), timed out, or ctx +// expired — the error wraps [ErrCollectionLocked] so callers can detect the +// locked state with errors.Is. +func ensureCollectionUnlocked(ctx context.Context, service secretService, collectionPath dbus.ObjectPath) error { + err := isCollectionUnlocked(collectionPath, service) + if err == nil { + return nil + } + if !errors.Is(err, ErrCollectionLocked) { + return err + } + if err := service.Unlock(ctx, []dbus.ObjectPath{collectionPath}); err != nil { + return lockedError(collectionPath, err) + } + return nil } // secretServiceIsLockedError is the D-Bus error name the secret service returns @@ -304,22 +340,30 @@ var sleepFn = time.Sleep // authentication prompt; the bounded retry count and backoff keep that to a // handful of spaced-out prompts at worst, and a dismissed prompt makes Unlock // return an error that aborts the loop immediately rather than re-prompting. -func withRelockRetry(service secretService, collectionPath dbus.ObjectPath, op func() error, itemPaths ...dbus.ObjectPath) error { +// +// ctx bounds each retry's unlock-prompt wait (see [ensureCollectionUnlocked] +// for why the original operation context is used). Failures to unlock — and a +// collection that is still locked once the retries are exhausted — are wrapped +// under [ErrCollectionLocked]. +func withRelockRetry(ctx context.Context, service secretService, collectionPath dbus.ObjectPath, op func() error, itemPaths ...dbus.ObjectPath) error { err := op() delay := relockRetryBaseDelay unlockPaths := append([]dbus.ObjectPath{collectionPath}, itemPaths...) for attempt := 0; attempt < maxRelockRetries && isLockedDBusError(err); attempt++ { sleepFn(delay) delay = min(delay*2, relockRetryMaxDelay) - if unlockErr := service.Unlock(unlockPaths); unlockErr != nil { + if unlockErr := service.Unlock(ctx, unlockPaths); unlockErr != nil { // Surface why the retry stopped while preserving errors.Is on the // underlying Unlock error (e.g. a dismissed prompt). The original // locked error is intentionally dropped: the failed unlock is the // actionable cause once we have decided to stop retrying. - return fmt.Errorf("unlock after relock: %w", unlockErr) + return lockedError(collectionPath, fmt.Errorf("unlock after relock: %w", unlockErr)) } err = op() } + if isLockedDBusError(err) { + return lockedError(collectionPath, err) + } return err } @@ -350,15 +394,9 @@ func (k *keychainStore[T]) Delete(ctx context.Context, id store.ID) error { return err } - err = isCollectionUnlocked(objectPath, service) - if err != nil && !errors.Is(err, errCollectionLocked) { + if err := ensureCollectionUnlocked(ctx, service, objectPath); err != nil { return err } - if errors.Is(err, errCollectionLocked) { - if err := service.Unlock([]dbus.ObjectPath{objectPath}); err != nil { - return err - } - } attributes := make(map[string]string) safelySetMetadata(k.serviceGroup, k.serviceName, attributes) @@ -373,8 +411,8 @@ func (k *keychainStore[T]) Delete(ctx context.Context, id store.ID) error { return nil } - return withRelockRetry(service, objectPath, func() error { - return service.DeleteItem(items[0]) + return withRelockRetry(ctx, service, objectPath, func() error { + return service.DeleteItem(ctx, items[0]) }, items[0]) } @@ -399,15 +437,9 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret, return nil, err } - err = isCollectionUnlocked(objectPath, service) - if err != nil && !errors.Is(err, errCollectionLocked) { + if err := ensureCollectionUnlocked(ctx, service, objectPath); err != nil { return nil, err } - if errors.Is(err, errCollectionLocked) { - if err := service.Unlock([]dbus.ObjectPath{objectPath}); err != nil { - return nil, err - } - } searchMetadata := make(map[string]string) safelySetMetadata(k.serviceGroup, k.serviceName, searchMetadata) @@ -429,7 +461,7 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret, safelyCleanMetadata(attributes) var value []byte - err = withRelockRetry(service, objectPath, func() error { + err = withRelockRetry(ctx, service, objectPath, func() error { var getErr error value, getErr = service.GetSecret(items[0], *session) return getErr @@ -471,15 +503,9 @@ func (k *keychainStore[T]) GetAllMetadata(ctx context.Context) (map[store.ID]sto return nil, err } - err = isCollectionUnlocked(objectPath, service) - if err != nil && !errors.Is(err, errCollectionLocked) { + if err := ensureCollectionUnlocked(ctx, service, objectPath); err != nil { return nil, err } - if errors.Is(err, errCollectionLocked) { - if err := service.Unlock([]dbus.ObjectPath{objectPath}); err != nil { - return nil, err - } - } searchMetadata := make(map[string]string) safelySetMetadata(k.serviceGroup, k.serviceName, searchMetadata) @@ -542,15 +568,9 @@ func (k *keychainStore[T]) Save(ctx context.Context, id store.ID, secret store.S return err } - err = isCollectionUnlocked(objectPath, service) - if err != nil && !errors.Is(err, errCollectionLocked) { + if err := ensureCollectionUnlocked(ctx, service, objectPath); err != nil { return err } - if errors.Is(err, errCollectionLocked) { - if err := service.Unlock([]dbus.ObjectPath{objectPath}); err != nil { - return err - } - } value, err := secret.Marshal() if err != nil { @@ -587,8 +607,8 @@ func (k *keychainStore[T]) Save(ctx context.Context, id store.ID, secret store.S // Nothing stored yet: create a fresh item. if len(items) == 0 { properties := kc.NewSecretProperties(label, attributes) - return withRelockRetry(service, objectPath, func() error { - _, createErr := service.CreateItem(objectPath, properties, sessSecret, kc.ReplaceBehaviorReplace) + return withRelockRetry(ctx, service, objectPath, func() error { + _, createErr := service.CreateItem(ctx, objectPath, properties, sessSecret, kc.ReplaceBehaviorReplace) return createErr }) } @@ -599,7 +619,7 @@ func (k *keychainStore[T]) Save(ctx context.Context, id store.ID, secret store.S // the attributes and label and collapsing any pre-existing duplicates are // best-effort (the secret is already stored) and must not flip the result. primary := items[0] - if err := withRelockRetry(service, objectPath, func() error { + if err := withRelockRetry(ctx, service, objectPath, func() error { return service.SetItemSecret(primary, sessSecret) }, primary); err != nil { return err @@ -610,8 +630,8 @@ func (k *keychainStore[T]) Save(ctx context.Context, id store.ID, secret store.S // Best-effort, but still relock-aware: a collection that relocks // mid-collapse would otherwise leave the duplicates the whole feature // exists to drain (see withRelockRetry and issue #446). - _ = withRelockRetry(service, objectPath, func() error { - return service.DeleteItem(dup) + _ = withRelockRetry(ctx, service, objectPath, func() error { + return service.DeleteItem(ctx, dup) }, dup) } @@ -634,7 +654,7 @@ func (k *keychainStore[T]) loadSecret( attributes map[string]string, ) (store.Secret, error) { var value []byte - err := withRelockRetry(svc, collectionPath, func() error { + err := withRelockRetry(ctx, svc, collectionPath, func() error { var getErr error value, getErr = svc.GetSecret(itemPath, *session) return getErr @@ -675,15 +695,9 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m return nil, err } - err = isCollectionUnlocked(objectPath, service) - if err != nil && !errors.Is(err, errCollectionLocked) { + if err := ensureCollectionUnlocked(ctx, service, objectPath); err != nil { return nil, err } - if errors.Is(err, errCollectionLocked) { - if err := service.Unlock([]dbus.ObjectPath{objectPath}); err != nil { - return nil, err - } - } attributes := make(map[string]string) // add our pattern to the attributes so we can match against items that diff --git a/store/keychain/keychain_linux_test.go b/store/keychain/keychain_linux_test.go index 774688b8..161c3ec5 100644 --- a/store/keychain/keychain_linux_test.go +++ b/store/keychain/keychain_linux_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "os" "sync/atomic" "testing" "time" @@ -74,6 +75,11 @@ type fakeService struct { unlockCalls int unlockErr error + // locked is returned by IsLocked, so a test can present the collection as + // locked up front and drive the ensureCollectionUnlocked paths. The zero + // value (unlocked) keeps every existing test passing. + locked bool + lastUnlockPaths []dbus.ObjectPath // availableErr, when set, is returned by Available so a test can drive the @@ -105,14 +111,14 @@ func (f *fakeService) Collections() ([]dbus.ObjectPath, error) { return []dbus.ObjectPath{loginKeychainObjectPath}, nil } func (f *fakeService) ReadAlias(string) (dbus.ObjectPath, error) { return loginKeychainObjectPath, nil } -func (f *fakeService) IsLocked(dbus.ObjectPath) (bool, error) { return false, nil } +func (f *fakeService) IsLocked(dbus.ObjectPath) (bool, error) { return f.locked, nil } func (f *fakeService) OpenSession(kc.AuthenticationMode) (*kc.Session, error) { // plain mode so Session.NewSecret works without a negotiated AES key, which // lets the Save path run end-to-end against the fake. return &kc.Session{Mode: kc.AuthenticationInsecurePlain}, nil } func (f *fakeService) CloseSession(*kc.Session) {} -func (f *fakeService) Unlock(items []dbus.ObjectPath) error { +func (f *fakeService) Unlock(_ context.Context, items []dbus.ObjectPath) error { f.unlockCalls++ f.lastUnlockPaths = items return f.unlockErr @@ -122,7 +128,7 @@ func (f *fakeService) SearchCollection(dbus.ObjectPath, kc.Attributes) ([]dbus.O return f.items, nil } -func (f *fakeService) CreateItem(dbus.ObjectPath, map[string]dbus.Variant, kc.Secret, kc.ReplaceBehavior) (dbus.ObjectPath, error) { +func (f *fakeService) CreateItem(context.Context, dbus.ObjectPath, map[string]dbus.Variant, kc.Secret, kc.ReplaceBehavior) (dbus.ObjectPath, error) { f.createCalls++ if f.createCalls <= f.createItemLockedErrs { return "", lockedErr("create item") @@ -130,7 +136,7 @@ func (f *fakeService) CreateItem(dbus.ObjectPath, map[string]dbus.Variant, kc.Se return "/created", nil } -func (f *fakeService) DeleteItem(item dbus.ObjectPath) error { +func (f *fakeService) DeleteItem(_ context.Context, item dbus.ObjectPath) error { f.deleteCalls++ if f.deleteCalls <= f.deleteItemLockedErrs { return lockedErr("delete item") @@ -377,9 +383,95 @@ func TestKeychainSaveStopsRetryingAfterMaxRelocks(t *testing.T) { &mocks.MockCredential{Username: "bob", Password: "bob-password"}) require.Error(t, err) assert.True(t, isLockedDBusError(err), "the persistent locked error must reach the caller") + assert.ErrorIs(t, err, ErrCollectionLocked, + "a collection still locked after the bounded retries must be detectable via the exported sentinel") assert.Equal(t, maxRelockRetries+1, fake.createCalls, "initial attempt plus the bounded retries") } +// TestKeychainLockedCollectionSurfacesErrCollectionLocked is the unit-level +// regression test for a locked collection that cannot be unlocked (e.g. a +// headless host where gnome-keyring immediately dismisses the unlock prompt +// because no prompter can be shown): every store operation must fail fast with +// an error matching the exported ErrCollectionLocked sentinel and naming the +// collection, instead of surfacing an opaque prompt error. +func TestKeychainLockedCollectionSurfacesErrCollectionLocked(t *testing.T) { + ops := map[string]func(store.Store) error{ + "get": func(ks store.Store) error { + _, err := ks.Get(t.Context(), store.MustParseID("com.test.test/test/bob")) + return err + }, + "save": func(ks store.Store) error { + return ks.Save(t.Context(), store.MustParseID("com.test.test/test/bob"), + &mocks.MockCredential{Username: "bob", Password: "bob-password"}) + }, + "delete": func(ks store.Store) error { + return ks.Delete(t.Context(), store.MustParseID("com.test.test/test/bob")) + }, + "get all metadata": func(ks store.Store) error { + _, err := ks.GetAllMetadata(t.Context()) + return err + }, + "filter": func(ks store.Store) error { + _, err := ks.Filter(t.Context(), store.MustParsePattern("**")) + return err + }, + } + for name, op := range ops { + t.Run(name, func(t *testing.T) { + fake := &fakeService{locked: true} + fake.unlockErr = errors.New("failed to prompt: prompt dismissed") + withFakeService(t, fake) + + err := op(setupKeychain(t, nil)) + require.Error(t, err) + assert.ErrorIs(t, err, ErrCollectionLocked) + assert.ErrorContains(t, err, string(loginKeychainObjectPath), + "the error must name the locked collection") + assert.ErrorContains(t, err, "prompt dismissed", + "the unlock failure cause must be preserved") + assert.Equal(t, 1, fake.unlockCalls, "exactly one unlock attempt before failing fast") + }) + } +} + +// TestKeychainLockedCollectionUnlocksAndProceeds pins the interactive happy +// path: a locked collection whose unlock succeeds (null prompt on a +// passwordless keyring, or the user answering the prompt) must not fail the +// operation. +func TestKeychainLockedCollectionUnlocksAndProceeds(t *testing.T) { + fake := &fakeService{ + locked: true, + items: []dbus.ObjectPath{"/org/freedesktop/secrets/collection/login/1"}, + } + withFakeService(t, fake) + + ks := setupKeychain(t, nil) + secret, err := ks.Get(t.Context(), store.MustParseID("com.test.test/test/bob")) + require.NoError(t, err) + require.NotNil(t, secret) + assert.Equal(t, 1, fake.unlockCalls, "the locked collection must be unlocked before the read") +} + +// TestKeychainRelockRetryUnlockFailureWrapsErrCollectionLocked covers the +// retry loop's unlock: when the collection relocks mid-operation and the +// re-unlock fails (e.g. a dismissed prompt), the surfaced error must match the +// exported sentinel too. +func TestKeychainRelockRetryUnlockFailureWrapsErrCollectionLocked(t *testing.T) { + stubRelockSleep(t) + fake := &fakeService{items: []dbus.ObjectPath{"/item/a"}} + fake.setSecretLockedErrs = 1 << 30 // never recovers + fake.unlockErr = errors.New("failed to prompt: prompt dismissed") + withFakeService(t, fake) + + ks := setupKeychain(t, nil) + err := ks.Save(t.Context(), store.MustParseID("com.test.test/test/bob"), + &mocks.MockCredential{Username: "bob", Password: "bob-password"}) + require.Error(t, err) + assert.ErrorIs(t, err, ErrCollectionLocked) + assert.ErrorContains(t, err, "unlock after relock") + assert.Equal(t, 1, fake.unlockCalls, "a failed unlock must abort the retry loop immediately") +} + // TestKeychainGetRetriesWhenCollectionRelocks covers the read path: GetSecret can // hit org.freedesktop.Secret.Error.IsLocked if the collection relocks between the // unlock and the read, so Get wraps it in withRelockRetry too. @@ -467,7 +559,7 @@ const ( // stays unlocked once any earlier operation has unlocked it.) func ensureUnlocked(t *testing.T, svc *kc.SecretService, collection dbus.ObjectPath) { t.Helper() - require.NoError(t, svc.Unlock([]dbus.ObjectPath{collection})) + require.NoError(t, svc.Unlock(context.Background(), []dbus.ObjectPath{collection})) require.Eventually(t, func() bool { locked, err := svc.IsLocked(collection) return err == nil && !locked @@ -570,8 +662,8 @@ func seedRealDuplicates(t *testing.T, serviceGroup, serviceName string, id store // closing connection can relock the collection between the unlock above // and this create (see withRelockRetry), which would otherwise fail the // seed with "Cannot create an item in a locked collection". - err = withRelockRetry(svc, collection, func() error { - _, createErr := svc.CreateItem(collection, kc.NewSecretProperties(label, attrs), sessSecret, kc.ReplaceBehaviorDoNotReplace) + err = withRelockRetry(context.Background(), svc, collection, func() error { + _, createErr := svc.CreateItem(context.Background(), collection, kc.NewSecretProperties(label, attrs), sessSecret, kc.ReplaceBehaviorDoNotReplace) return createErr }) require.NoError(t, err) @@ -601,8 +693,8 @@ func purgeRealItems(t *testing.T, serviceGroup, serviceName string, id store.ID) items, err := svc.SearchCollection(collection, attrs) require.NoError(t, err) for _, item := range items { - require.NoError(t, withRelockRetry(svc, collection, func() error { - return svc.DeleteItem(item) + require.NoError(t, withRelockRetry(context.Background(), svc, collection, func() error { + return svc.DeleteItem(context.Background(), item) })) } } @@ -673,6 +765,46 @@ func TestKeychainSaveDoesNotAccumulate(t *testing.T) { "the surviving item's metadata must be refreshed in place") } +// TestKeychainLiveLockedCollection is the live regression test for +// headless locked keyrings: against a password-protected keyring with no way to +// answer the unlock prompt (headless, no prompter), a store operation on a +// locked collection must fail quickly with an error matching the exported +// ErrCollectionLocked sentinel — not hang and not surface an opaque prompt +// error. +// +// It is gated behind TEST_KEYCHAIN_LOCKED_COLLECTION because it only makes +// sense against a PASSWORD-PROTECTED keyring (see the gnome-keyring-locked +// script): on the passwordless keyring the regular suite runs against, the +// store's unlock succeeds via the null prompt and the operation would simply +// succeed. +func TestKeychainLiveLockedCollection(t *testing.T) { + if os.Getenv("TEST_KEYCHAIN_LOCKED_COLLECTION") == "" { + t.Skip("TEST_KEYCHAIN_LOCKED_COLLECTION not set; needs a live password-protected keyring") + } + + svc, err := kc.NewService(context.Background()) + require.NoError(t, err) + defer func() { _ = svc.Close() }() + + collection, err := getDefaultCollection(svc) + require.NoError(t, err) + require.NoError(t, svc.LockItems(context.Background(), []dbus.ObjectPath{collection})) + locked, err := svc.IsLocked(collection) + require.NoError(t, err) + require.True(t, locked, "collection must be locked; is the keyring password-protected?") + + ks := setupKeychain(t, nil) + start := time.Now() + _, err = ks.Get(t.Context(), store.MustParseID("com.test.test/test/bob")) + elapsed := time.Since(start) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrCollectionLocked) + assert.ErrorContains(t, err, string(collection), "the error must name the locked collection") + assert.Less(t, elapsed, 15*time.Second, + "a locked collection must fail fast, not sit out the full prompt timeout") +} + // TestNewProbeSucceeds asserts the eager availability probe passes for a // reachable backend: New returns a usable store, dialing exactly one connection // and closing it (honoring the leak contract). diff --git a/store/scripts/gnome-keyring-locked b/store/scripts/gnome-keyring-locked new file mode 100755 index 00000000..8a7e9789 --- /dev/null +++ b/store/scripts/gnome-keyring-locked @@ -0,0 +1,91 @@ +#!/bin/bash + +# Locked-collection variant of the gnome-keyring harness (see ./gnome-keyring). +# +# Instead of seeding a passwordless 'login' keyring, it creates a +# PASSWORD-PROTECTED one the way PAM does (gnome-keyring-daemon --login reads +# the password from stdin), so locking the collection makes it genuinely +# require the password to unlock. The container has no display and no prompter, +# so the unlock prompt can never be answered — exactly the headless situation +# hit by downstream consumers — and the gated live test asserts store operations +# fail fast with ErrCollectionLocked instead of hanging. + +set -euxo pipefail + +if test -z $(command -v gnome-keyring-daemon); then + echo "gnome-keyring-daemon is not installed" + exit 1 +fi + +if test -z $(command -v dbus-daemon); then + echo "dbus-daemon is not installed" + exit 1 +fi + +mkdir -p ~/.local/share/keyrings + +# Start D-Bus session (dbus must be installed) +export DBUS_SESSION_BUS_ADDRESS=$(dbus-daemon --session --print-address --fork) + +# PAM-style login mode: read the password from stdin and create the +# password-protected 'login' keyring (unlocked for now; the test locks it). +# The daemon prints GNOME_KEYRING_CONTROL=; eval + export it so the +# --start below attaches to THIS daemon instead of spawning a second one that +# never learned the login password (without XDG_RUNTIME_DIR each invocation +# would otherwise mint its own control directory under ~/.cache). +eval "$(echo -n 'test-keyring-password' | gnome-keyring-daemon --daemonize --login)" +export GNOME_KEYRING_CONTROL + +gnome-keyring-daemon --start --components=secrets + +# Wait up to 5 seconds for org.freedesktop.secrets to appear on D-Bus +timeout=5000 +interval=100 +elapsed=0 + +while ! gdbus call --session \ + --dest org.freedesktop.DBus \ + --object-path /org/freedesktop/DBus \ + --method org.freedesktop.DBus.GetNameOwner \ + org.freedesktop.secrets >/dev/null 2>&1; do + + sleep 0.1 + elapsed=$((elapsed + interval)) + if (( elapsed >= timeout )); then + echo "❌ Timeout waiting for gnome-keyring-daemon to register org.freedesktop.secrets" + exit 1 + fi +done + +owner=$(gdbus call --session \ + --dest org.freedesktop.DBus \ + --object-path /org/freedesktop/DBus \ + --method org.freedesktop.DBus.GetNameOwner \ + org.freedesktop.secrets | awk -F"'" '{print $2}') + +if test -v $owner; then + echo "there is no owner of the org.freedesktop.secrets API" + exit 1 +fi + +pid=$(gdbus call --session \ + --dest org.freedesktop.DBus \ + --object-path /org/freedesktop/DBus \ + --method org.freedesktop.DBus.GetConnectionUnixProcessID \ + "$owner" | awk '{print $2}' | tr -d ',)') + +if test -v $pid; then + echo "there is no registered org.freedesktop.secrets daemon" + exit 1 +fi + +exe=$(readlink -f /proc/$pid/exe) + +if [[ "$exe" == *gnome-keyring-daemon* ]]; then + echo "dbus org.freedesktop.secrets is using gnome-keyring-daemon" + TEST_KEYCHAIN_LOCKED_COLLECTION=1 go test -v -count=1 -run TestKeychainLiveLockedCollection ./store/keychain/ + exit 0 +fi + +echo "dbus org.freedesktop.secrets is not using gnome-keyring-daemon. Using ${exe}" +exit 1 From 4f7aadcb76e435549cfcd1aff586ce50d47bddb9 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:36:16 +0200 Subject: [PATCH 2/8] docs(keychain): plain-language decision log entry Co-Authored-By: Claude Fable 5 --- store/docs/keychain/decision-logs.md | 82 +++++++++++++--------------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/store/docs/keychain/decision-logs.md b/store/docs/keychain/decision-logs.md index ca2989a1..0bba161b 100644 --- a/store/docs/keychain/decision-logs.md +++ b/store/docs/keychain/decision-logs.md @@ -138,52 +138,48 @@ every store operation (not only the probe) benefits. 2026-08-25 Locked collections fail fast with ErrCollectionLocked; unlock prompt bounded -On headless Linux hosts (SSH key-only login, gnome-keyring) the login -collection comes up locked after every keyring-daemon restart — PAM has no -password to auto-unlock it with. The store's only reaction to a locked -collection was `Service.Unlock` → `PromptAndWait`, whose failures surfaced as -opaque strings ("failed to prompt: prompt dismissed" / "prompt timed out") -that downstream consumers could not classify. +On headless Linux hosts with SSH key-only logins, PAM has no password to +auto-unlock the login keyring, so the collection is locked after every +keyring-daemon restart. The store reacted to a locked collection by calling +Service.Unlock and waiting on the prompt. The failures surfaced as opaque +strings ("failed to prompt: prompt dismissed", "prompt timed out") that +callers could not classify, and the wait was a hardcoded 30 seconds. Decisions: -- **One exported sentinel, `ErrCollectionLocked`**, declared in the - cross-platform `keychain.go` (mirroring `ErrKeychainUnavailable` / - `ErrNoDefaultCollection`), Linux-only behavior. Every path that fails - because the collection is locked and could not be unlocked wraps it: the - up-front unlock in `ensureCollectionUnlocked` (which also names the - collection in the message), the re-unlock inside `withRelockRetry`, and a - collection still locked once the bounded retries are exhausted. The - underlying prompt failure is preserved as the wrapped cause; no exported - prompt-dismissed/timed-out sentinels (unexported-cause promotion pattern, - same as `errSessionBusUnavailable`). -- **The prompt wait is ctx-bounded.** `PromptAndWait` (and the prompt-capable - calls `Unlock`, `LockItems`, `CreateItem`, `DeleteItem`) now take a - `context.Context`; store operations pass their ORIGINAL operation ctx — - deliberately not the `context.WithoutCancel` connection ctx — so a caller - deadline bounds the human-wait while in-flight D-Bus calls stay protected - from teardown. The internal 30s cap remains as an upper bound, created once - outside the receive loop (previously `time.After` inside the loop was reset - by every unrelated bus signal). A null prompt returns before the ctx check, - so best-effort cleanup calls with cancelled contexts still succeed on +- One exported sentinel, `ErrCollectionLocked`, declared in the cross-platform + `keychain.go` like `ErrKeychainUnavailable` and `ErrNoDefaultCollection`. + Every path that fails because the collection stayed locked wraps it: the + up-front unlock in `ensureCollectionUnlocked` (which names the collection), + the re-unlock in `withRelockRetry`, and a collection still locked after the + bounded retries. The prompt failure is kept as the wrapped cause. No + exported prompt-dismissed or prompt-timeout sentinels; those stay unexported + causes, the same pattern as `errSessionBusUnavailable`. +- The prompt wait is bounded by the operation's context. `PromptAndWait`, + `Unlock`, `LockItems`, `CreateItem` and `DeleteItem` now take a ctx. Store + operations pass their original ctx, not the `context.WithoutCancel` + connection ctx, so a caller deadline bounds the wait for the user while + in-flight D-Bus calls stay protected from teardown. The internal 30s cap + remains as an upper bound and is created once outside the receive loop; + previously any unrelated bus signal reset it. A null prompt returns before + the ctx check, so cleanup calls with cancelled contexts still work on passwordless keyrings. -- **Nothing else was added, deliberately.** Evaluated and rejected: - prompter-presence probes (`org.gnome.keyring.SystemPrompter` is - activatable-but-unstartable on headless hosts with gcr installed, and - KWallet/KeePassXC never own that name — both directions misclassify); - password callbacks and programmatic master-password unlock via - `org.gnome.keyring.InternalUnsupportedGuiltRiddenInterface` (gnome-only, - and hands the library a UX/credential-handling responsibility the caller - owns); library TTY prompting; and a New-time lock check (lock state is - per-operation and mutable; locked ≠ unavailable). The caller detects - `ErrCollectionLocked` via `errors.Is` and owns remediation messaging — and - must NOT fall back to another store, which would split credentials. -- Validated live (Ubuntu 24.04 VM, gnome-keyring): headless + locked fails in - ~15ms with "prompt dismissed" (gnome-keyring dismisses immediately when no - prompter can be shown, with or without gcr installed); a real prompt on a - display still completes and unlocks; a 2s caller deadline aborts an - unanswered prompt at 2.003s. CI: new `ubuntu-24-gnome-keyring-locked` - target runs `TestKeychainLiveLockedCollection` against a password-protected - keyring (`gnome-keyring-daemon --login`) with the collection locked. +- Nothing else was added. Rejected: prompter-presence probes (the + `org.gnome.keyring.SystemPrompter` name is activatable but unstartable on + headless hosts with gcr installed, and KWallet/KeePassXC never own it, so a + probe misclassifies in both directions), password callbacks and + master-password unlock via the gnome-only + `InternalUnsupportedGuiltRiddenInterface`, TTY prompting in the library, and + a lock check in `New` (lock state is per-operation and mutable). The caller + detects `ErrCollectionLocked` with `errors.Is` and owns the remediation + message. A locked collection must not be treated as unavailable; falling + back to another store would split credentials across stores. +- Validated live on an Ubuntu 24.04 VM: a headless locked operation fails in + about 15ms with "prompt dismissed" (gnome-keyring dismisses immediately when + no prompter can start), an answered prompt on a display still works, and a + 2s caller deadline aborts an unanswered prompt at 2s. A new + `ubuntu-24-gnome-keyring-locked` CI target runs + `TestKeychainLiveLockedCollection` against a password-protected keyring with + the collection locked. --- From 1541824b6ed04ca153a2d1a2a58fdcf4410f4871 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:39:47 +0200 Subject: [PATCH 3/8] docs(keychain): trim godocs, drop test and workflow comments Co-Authored-By: Claude Fable 5 --- .github/workflows/keychain.yml | 3 -- .../secretservice/secretservice.go | 29 +++++----------- store/keychain/keychain.go | 32 +++++------------ store/keychain/keychain_linux.go | 34 +++++++------------ store/keychain/keychain_linux_test.go | 32 ++--------------- 5 files changed, 33 insertions(+), 97 deletions(-) diff --git a/.github/workflows/keychain.yml b/.github/workflows/keychain.yml index 7beae835..3a0ec9e7 100644 --- a/.github/workflows/keychain.yml +++ b/.github/workflows/keychain.yml @@ -21,9 +21,6 @@ jobs: subtest: - fedora-43-gnome-keyring - ubuntu-24-gnome-keyring - # password-protected keyring, locked collection: asserts store ops - # fail fast with ErrCollectionLocked instead of hanging on a prompt - # nothing can answer - ubuntu-24-gnome-keyring-locked # disabled kdewallet tests since it prompts for a password in a # headless environment... need to still fix this diff --git a/store/keychain/internal/go-keychain/secretservice/secretservice.go b/store/keychain/internal/go-keychain/secretservice/secretservice.go index ce73d1aa..5a416e9e 100644 --- a/store/keychain/internal/go-keychain/secretservice/secretservice.go +++ b/store/keychain/internal/go-keychain/secretservice/secretservice.go @@ -412,9 +412,7 @@ const ReplaceBehaviorDoNotReplace = 0 // ReplaceBehaviorReplace const ReplaceBehaviorReplace = 1 -// CreateItem creates an item in the collection. The call can open a prompt -// (e.g. when the collection relocked in the meantime); ctx bounds that prompt -// wait (see [SecretService.PromptAndWait]). +// CreateItem creates an item in the collection; ctx bounds the prompt wait. func (s *SecretService) CreateItem( ctx context.Context, collection dbus.ObjectPath, @@ -446,8 +444,7 @@ func (s *SecretService) CreateItem( return item, nil } -// DeleteItem deletes an item. The call can open a prompt; ctx bounds that -// prompt wait (see [SecretService.PromptAndWait]). +// DeleteItem deletes an item; ctx bounds the prompt wait. func (s *SecretService) DeleteItem(ctx context.Context, item dbus.ObjectPath) (err error) { var prompt dbus.ObjectPath err = s.Obj(item). @@ -510,9 +507,7 @@ func (s *SecretService) GetSecret(item dbus.ObjectPath, session Session) (secret // NullPrompt const NullPrompt = "/" -// Unlock unlocks the given collections or items. On a password-protected -// keyring this opens the backend's unlock prompt; ctx bounds that prompt wait -// (see [SecretService.PromptAndWait]). +// Unlock unlocks the given collections or items; ctx bounds the prompt wait. func (s *SecretService) Unlock(ctx context.Context, items []dbus.ObjectPath) (err error) { var dummy []dbus.ObjectPath var prompt dbus.ObjectPath @@ -529,8 +524,7 @@ func (s *SecretService) Unlock(ctx context.Context, items []dbus.ObjectPath) (er return nil } -// LockItems locks the given collections or items. The call can open a prompt; -// ctx bounds that prompt wait (see [SecretService.PromptAndWait]). +// LockItems locks the given collections or items; ctx bounds the prompt wait. func (s *SecretService) LockItems(ctx context.Context, items []dbus.ObjectPath) (err error) { var dummy []dbus.ObjectPath var prompt dbus.ObjectPath @@ -557,16 +551,12 @@ func (p PromptDismissedError) Error() string { return p.err.Error() } -// promptTimeout caps how long PromptAndWait waits for the user to answer a -// prompt when the caller's ctx carries no (earlier) deadline of its own, so a -// prompt that nobody will ever answer cannot block an operation forever. +// promptTimeout caps how long PromptAndWait waits for a prompt to complete. const promptTimeout = 30 * time.Second -// PromptAndWait displays the given prompt and blocks until the user answers -// it, the prompt is dismissed, ctx is done, or promptTimeout elapses — -// whichever comes first. ctx lets a caller bound the human-wait with its own -// deadline or cancellation; the promptTimeout cap always applies as an upper -// bound. A NullPrompt returns immediately with no error. +// PromptAndWait displays the prompt and blocks until it completes, is +// dismissed, ctx is done, or promptTimeout elapses. A NullPrompt returns +// immediately. // // PromptAndWait is NOT thread-safe. func (s *SecretService) PromptAndWait(ctx context.Context, prompt dbus.ObjectPath) (paths *dbus.Variant, err error) { @@ -577,8 +567,7 @@ func (s *SecretService) PromptAndWait(ctx context.Context, prompt dbus.ObjectPat if call.Err != nil { return nil, fmt.Errorf("failed to prompt: %w", call.Err) } - // The timer is created once, outside the receive loop, so unrelated bus - // signals cannot keep resetting the timeout. + // created once, outside the loop, so unrelated signals cannot reset it timeout := time.After(promptTimeout) for { var result PromptCompletedResult diff --git a/store/keychain/keychain.go b/store/keychain/keychain.go index 6eca50c0..a345a39f 100644 --- a/store/keychain/keychain.go +++ b/store/keychain/keychain.go @@ -72,31 +72,17 @@ var ErrNoDefaultCollection = errors.New("no default keychain collection availabl var ErrKeychainUnavailable = errors.New("keychain backend unavailable") // ErrCollectionLocked is returned by store operations when the keychain -// collection is locked and could not be unlocked: the backend's unlock prompt -// was dismissed (which is what gnome-keyring does immediately when no prompter -// can be shown, e.g. on a headless host), timed out, or was aborted by the -// operation's context. +// collection is locked and could not be unlocked: the unlock prompt was +// dismissed (gnome-keyring does this immediately when no prompter can be +// shown, e.g. on a headless host), timed out, or was aborted by the +// operation's context. Detect it with [errors.Is]. // -// This is DISTINCT from [ErrKeychainUnavailable]: the backend is reachable and -// the collection exists — it still holds the user's credentials, but they -// cannot be read or written until the user unlocks the keyring (for example -// via their desktop session, or `gnome-keyring-daemon --unlock`). Callers -// should surface that remediation to the user rather than fall back to a -// different store: writing new credentials elsewhere while the locked -// collection still holds the old ones would split credentials across stores. +// Unlike [ErrKeychainUnavailable], the collection exists and still holds the +// user's credentials. Tell the user how to unlock it; do not fall back to +// another store, which would split credentials across stores. // -// A common cause on headless hosts: with SSH key-only logins there is no -// password for PAM to auto-unlock the login keyring with, so the collection is -// locked after every keyring-daemon restart. -// -// NOTE: like the sentinels above this condition is currently specific to the -// Linux keyring (the freedesktop Secret Service). It is declared here, in the -// cross-platform file, so platform-agnostic callers can reference it on every -// platform without build tags; on non-Linux platforms it simply never matches. -// -// It is exported so callers can use [errors.Is] to detect the locked state and -// present an actionable message, rather than relying on fragile error message -// comparisons. +// Like the sentinels above, it is declared in the cross-platform file so +// callers need no build tags; it only matches on Linux. var ErrCollectionLocked = errors.New("keychain collection is locked") type ( diff --git a/store/keychain/keychain_linux.go b/store/keychain/keychain_linux.go index c9d96424..bfee6b7b 100644 --- a/store/keychain/keychain_linux.go +++ b/store/keychain/keychain_linux.go @@ -239,30 +239,22 @@ func isCollectionUnlocked(collectionPath dbus.ObjectPath, service secretService) return ErrCollectionLocked } -// lockedError wraps cause under the exported [ErrCollectionLocked] sentinel, -// naming the collection so the message is actionable on its own. +// lockedError wraps cause under [ErrCollectionLocked], naming the collection. func lockedError(collectionPath dbus.ObjectPath, cause error) error { return fmt.Errorf("%w: could not unlock collection %q: %w", ErrCollectionLocked, collectionPath, cause) } -// ensureCollectionUnlocked checks the collection's lock state and, when -// locked, asks the secret service to unlock it. On a passwordless keyring -// (e.g. the PAM-unlocked login keyring) that unlock completes silently via the -// null prompt; on a password-protected keyring it opens the backend's unlock -// prompt. +// ensureCollectionUnlocked unlocks the collection if it is locked. On a +// passwordless keyring the unlock completes silently via the null prompt; on a +// password-protected keyring it opens the backend's unlock prompt. // -// ctx bounds the prompt wait — deliberately the caller's ORIGINAL operation -// context, not the [context.WithoutCancel] connection context from -// [operationService]: in-flight D-Bus operations are protected from teardown, -// but waiting on a human is bounded by the caller's deadline or cancellation -// (and by the backstop timeout in the secretservice package). A null prompt is -// unaffected by ctx, so best-effort cleanup calls with an already-cancelled -// ctx still succeed on passwordless keyrings. +// ctx bounds the prompt wait. It is the caller's original operation context, +// not the detached connection context from [operationService]: waiting on the +// user is bounded by the caller. A null prompt ignores ctx, so cleanup calls +// with a cancelled ctx still work on passwordless keyrings. // -// When the unlock fails — the prompt was dismissed (gnome-keyring does this -// immediately when no prompter can be shown, e.g. headless), timed out, or ctx -// expired — the error wraps [ErrCollectionLocked] so callers can detect the -// locked state with errors.Is. +// A failed unlock (prompt dismissed, timed out, or ctx expired) wraps +// [ErrCollectionLocked]. func ensureCollectionUnlocked(ctx context.Context, service secretService, collectionPath dbus.ObjectPath) error { err := isCollectionUnlocked(collectionPath, service) if err == nil { @@ -341,10 +333,8 @@ var sleepFn = time.Sleep // handful of spaced-out prompts at worst, and a dismissed prompt makes Unlock // return an error that aborts the loop immediately rather than re-prompting. // -// ctx bounds each retry's unlock-prompt wait (see [ensureCollectionUnlocked] -// for why the original operation context is used). Failures to unlock — and a -// collection that is still locked once the retries are exhausted — are wrapped -// under [ErrCollectionLocked]. +// ctx bounds each retry's unlock-prompt wait. Unlock failures, and a +// collection still locked after the retries, wrap [ErrCollectionLocked]. func withRelockRetry(ctx context.Context, service secretService, collectionPath dbus.ObjectPath, op func() error, itemPaths ...dbus.ObjectPath) error { err := op() delay := relockRetryBaseDelay diff --git a/store/keychain/keychain_linux_test.go b/store/keychain/keychain_linux_test.go index 161c3ec5..76ec41d5 100644 --- a/store/keychain/keychain_linux_test.go +++ b/store/keychain/keychain_linux_test.go @@ -75,9 +75,7 @@ type fakeService struct { unlockCalls int unlockErr error - // locked is returned by IsLocked, so a test can present the collection as - // locked up front and drive the ensureCollectionUnlocked paths. The zero - // value (unlocked) keeps every existing test passing. + // locked is returned by IsLocked. locked bool lastUnlockPaths []dbus.ObjectPath @@ -388,12 +386,6 @@ func TestKeychainSaveStopsRetryingAfterMaxRelocks(t *testing.T) { assert.Equal(t, maxRelockRetries+1, fake.createCalls, "initial attempt plus the bounded retries") } -// TestKeychainLockedCollectionSurfacesErrCollectionLocked is the unit-level -// regression test for a locked collection that cannot be unlocked (e.g. a -// headless host where gnome-keyring immediately dismisses the unlock prompt -// because no prompter can be shown): every store operation must fail fast with -// an error matching the exported ErrCollectionLocked sentinel and naming the -// collection, instead of surfacing an opaque prompt error. func TestKeychainLockedCollectionSurfacesErrCollectionLocked(t *testing.T) { ops := map[string]func(store.Store) error{ "get": func(ks store.Store) error { @@ -434,10 +426,6 @@ func TestKeychainLockedCollectionSurfacesErrCollectionLocked(t *testing.T) { } } -// TestKeychainLockedCollectionUnlocksAndProceeds pins the interactive happy -// path: a locked collection whose unlock succeeds (null prompt on a -// passwordless keyring, or the user answering the prompt) must not fail the -// operation. func TestKeychainLockedCollectionUnlocksAndProceeds(t *testing.T) { fake := &fakeService{ locked: true, @@ -452,10 +440,6 @@ func TestKeychainLockedCollectionUnlocksAndProceeds(t *testing.T) { assert.Equal(t, 1, fake.unlockCalls, "the locked collection must be unlocked before the read") } -// TestKeychainRelockRetryUnlockFailureWrapsErrCollectionLocked covers the -// retry loop's unlock: when the collection relocks mid-operation and the -// re-unlock fails (e.g. a dismissed prompt), the surfaced error must match the -// exported sentinel too. func TestKeychainRelockRetryUnlockFailureWrapsErrCollectionLocked(t *testing.T) { stubRelockSleep(t) fake := &fakeService{items: []dbus.ObjectPath{"/item/a"}} @@ -765,18 +749,8 @@ func TestKeychainSaveDoesNotAccumulate(t *testing.T) { "the surviving item's metadata must be refreshed in place") } -// TestKeychainLiveLockedCollection is the live regression test for -// headless locked keyrings: against a password-protected keyring with no way to -// answer the unlock prompt (headless, no prompter), a store operation on a -// locked collection must fail quickly with an error matching the exported -// ErrCollectionLocked sentinel — not hang and not surface an opaque prompt -// error. -// -// It is gated behind TEST_KEYCHAIN_LOCKED_COLLECTION because it only makes -// sense against a PASSWORD-PROTECTED keyring (see the gnome-keyring-locked -// script): on the passwordless keyring the regular suite runs against, the -// store's unlock succeeds via the null prompt and the operation would simply -// succeed. +// Needs a live password-protected keyring (see scripts/gnome-keyring-locked); +// on a passwordless keyring the unlock succeeds and the operation passes. func TestKeychainLiveLockedCollection(t *testing.T) { if os.Getenv("TEST_KEYCHAIN_LOCKED_COLLECTION") == "" { t.Skip("TEST_KEYCHAIN_LOCKED_COLLECTION not set; needs a live password-protected keyring") From 7a0cf6b0588e0ac8cb01e6fcbda2636863af81f6 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:41:05 +0200 Subject: [PATCH 4/8] docs(keychain): clarify the 2s deadline was a test parameter, not a default Co-Authored-By: Claude Fable 5 --- store/docs/keychain/decision-logs.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/store/docs/keychain/decision-logs.md b/store/docs/keychain/decision-logs.md index 0bba161b..abac81aa 100644 --- a/store/docs/keychain/decision-logs.md +++ b/store/docs/keychain/decision-logs.md @@ -176,8 +176,11 @@ Decisions: back to another store would split credentials across stores. - Validated live on an Ubuntu 24.04 VM: a headless locked operation fails in about 15ms with "prompt dismissed" (gnome-keyring dismisses immediately when - no prompter can start), an answered prompt on a display still works, and a - 2s caller deadline aborts an unanswered prompt at 2s. A new + no prompter can start), and an answered prompt on a display still works. To + prove the caller's deadline is honored, a test probe with a deliberately + short 2 second context deadline aborted an unanswered prompt at 2 seconds. + Nothing changes for callers without a deadline: they still get the 30 second + cap, and gnome-keyring itself never times a prompt out. A new `ubuntu-24-gnome-keyring-locked` CI target runs `TestKeychainLiveLockedCollection` against a password-protected keyring with the collection locked. From 76fcd61e82df0b811e126ff55be6898505c6e93b Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:42:32 +0200 Subject: [PATCH 5/8] docs(keychain): drop fallback warning from ErrCollectionLocked godoc Co-Authored-By: Claude Fable 5 --- store/keychain/keychain.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/store/keychain/keychain.go b/store/keychain/keychain.go index a345a39f..3e92a12f 100644 --- a/store/keychain/keychain.go +++ b/store/keychain/keychain.go @@ -78,8 +78,7 @@ var ErrKeychainUnavailable = errors.New("keychain backend unavailable") // operation's context. Detect it with [errors.Is]. // // Unlike [ErrKeychainUnavailable], the collection exists and still holds the -// user's credentials. Tell the user how to unlock it; do not fall back to -// another store, which would split credentials across stores. +// user's credentials. Tell the user how to unlock it. // // Like the sentinels above, it is declared in the cross-platform file so // callers need no build tags; it only matches on Linux. From 56047d894e092f1b3c823df9aa3655dfa8aa9f81 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:43:24 +0200 Subject: [PATCH 6/8] docs(keychain): drop positional reference in ErrCollectionLocked godoc Co-Authored-By: Claude Fable 5 --- store/keychain/keychain.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/store/keychain/keychain.go b/store/keychain/keychain.go index 3e92a12f..9f33378f 100644 --- a/store/keychain/keychain.go +++ b/store/keychain/keychain.go @@ -80,8 +80,8 @@ var ErrKeychainUnavailable = errors.New("keychain backend unavailable") // Unlike [ErrKeychainUnavailable], the collection exists and still holds the // user's credentials. Tell the user how to unlock it. // -// Like the sentinels above, it is declared in the cross-platform file so -// callers need no build tags; it only matches on Linux. +// It is declared in the cross-platform file so callers can reference it +// without build tags; it only matches on Linux. var ErrCollectionLocked = errors.New("keychain collection is locked") type ( From d5864ed41f48f3517b9aef719ff74c7bc7bbd263 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:52:34 +0200 Subject: [PATCH 7/8] docs(keychain): plain-language locked-collection section in design.md Co-Authored-By: Claude Fable 5 --- store/docs/keychain/design.md | 74 +++++++++++++++++------------------ 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/store/docs/keychain/design.md b/store/docs/keychain/design.md index 1afe5a44..d77bc5b1 100644 --- a/store/docs/keychain/design.md +++ b/store/docs/keychain/design.md @@ -95,45 +95,41 @@ On macOS and Windows the check is a no-op (`New` never returns ### Locked collections and the bounded unlock prompt -A reachable backend can still hold a **locked** collection — the default state -on headless hosts with SSH key-only logins, where PAM has no password to -auto-unlock the login keyring with, so it relocks on every keyring-daemon +A reachable backend can still hold a locked collection. This is the default +state on headless hosts with SSH key-only logins: PAM has no password to +auto-unlock the login keyring, so it is locked after every keyring-daemon restart. -Every store operation checks the collection's lock state up front +Every store operation checks the lock state up front (`ensureCollectionUnlocked`) and, when locked, issues a Secret Service -`Unlock`. On a passwordless keyring that completes silently via the null -prompt. On a password-protected keyring it opens the backend's unlock prompt, -and that prompt wait is **bounded twice**: - -- by the operation's own `ctx` (deliberately the caller's original context, - not the `context.WithoutCancel` connection context: in-flight D-Bus calls - are protected from teardown, but waiting on a human is bounded by the - caller); and -- by an internal 30s cap (`promptTimeout`), so a prompt nobody can ever answer - cannot block an operation forever even without a caller deadline. - -When the unlock fails — prompt dismissed, timed out, or ctx expired — the -operation fails with an error wrapping the exported `ErrCollectionLocked` -sentinel and naming the collection, with the underlying prompt failure -preserved as the cause. The same wrapping applies inside the relock-retry loop -(`withRelockRetry`) and when a collection is still locked after the bounded -retries. - -Empirically (validated live on Ubuntu 24.04, gnome-keyring): on a headless -host the unlock prompt does not hang — gnome-keyring completes it as -*dismissed* within milliseconds when no prompter can be shown (whether or not -the gcr prompter is installed and D-Bus-activatable), so the locked error -surfaces in ~15ms. The 30s cap and ctx bound cover the remaining case of a -live prompter with an absent user. - -Deliberately **not** built (see the decision log): prompter-presence probes -(`org.gnome.keyring.SystemPrompter` is activatable-but-unstartable on headless -hosts with gcr installed, and KWallet/KeePassXC never own that name — both -directions misclassify), password callbacks, programmatic master-password -unlock (`org.gnome.keyring.InternalUnsupportedGuiltRiddenInterface`), and -library-owned TTY prompting. The library reports the locked state reliably; -the caller owns remediation. `ErrCollectionLocked` also must not be treated as -"unavailable, fall back": the locked collection still holds the user's -credentials, and silently writing new ones to a fallback store would split -credentials across two stores. +`Unlock`. On a passwordless keyring this completes silently via the null +prompt. On a password-protected keyring it opens the backend's unlock prompt. +The prompt wait is bounded twice: + +- by the operation's `ctx`. This is the caller's original context, not the + `context.WithoutCancel` connection context, so a caller deadline bounds the + wait for the user while in-flight D-Bus calls stay protected from teardown. +- by an internal 30 second cap (`promptTimeout`), so a prompt nobody can + answer cannot block an operation forever. + +When the unlock fails (prompt dismissed, timed out, or ctx expired), the +operation returns an error wrapping the exported `ErrCollectionLocked` +sentinel, naming the collection and keeping the prompt failure as the cause. +The same wrapping applies in the relock-retry loop (`withRelockRetry`) and +when a collection is still locked after the bounded retries. + +Validated live on Ubuntu 24.04 with gnome-keyring: on a headless host the +unlock prompt does not hang. gnome-keyring dismisses it within milliseconds +when no prompter can start, so the locked error surfaces in about 15ms. The +30 second cap and the ctx bound cover the remaining case of a live prompter +with nobody answering. + +Deliberately not built (see the decision log): prompter-presence probes (the +`org.gnome.keyring.SystemPrompter` name is activatable but unstartable on +headless hosts with gcr installed, and KWallet/KeePassXC never own it, so a +probe misclassifies in both directions), password callbacks, master-password +unlock via `InternalUnsupportedGuiltRiddenInterface`, and TTY prompting in +the library. The library reports the locked state; the caller owns the +remediation message. A locked collection must not be treated as unavailable; +it still holds the user's credentials, and falling back to another store +would split credentials across stores. From 8e57be6d683f98e7973d75167b0f8bd7449d75ce Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:53:11 +0200 Subject: [PATCH 8/8] docs(keychain): plain-language locked-collection section in README Co-Authored-By: Claude Fable 5 --- store/keychain/README.md | 43 +++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/store/keychain/README.md b/store/keychain/README.md index 38549a30..16b6e0df 100644 --- a/store/keychain/README.md +++ b/store/keychain/README.md @@ -66,36 +66,33 @@ secrets. On macOS and Windows the check is a no-op (and `ctx` is unused). See ### Locked collections (Linux) -A reachable keychain can still hold a **locked** collection. This is the -default state on headless Linux hosts with SSH key-only logins: there is no -password for PAM to auto-unlock the login keyring with, so the collection comes -up locked after every keyring-daemon restart. - -When a store operation encounters a locked collection it asks the Secret -Service to unlock it. On a passwordless keyring that succeeds silently; on a -password-protected keyring it opens the backend's unlock prompt. If that prompt -cannot complete — it is dismissed (gnome-keyring does this immediately when no -prompter can be shown, e.g. headless), times out, or the operation's context -expires — the operation fails with an error matching -`keychain.ErrCollectionLocked`: +A reachable keychain can still hold a locked collection. This is the default +state on headless Linux hosts with SSH key-only logins: PAM has no password to +auto-unlock the login keyring, so it is locked after every keyring-daemon +restart. + +A store operation that finds the collection locked asks the Secret Service to +unlock it. On a passwordless keyring this succeeds silently. On a +password-protected keyring it opens the backend's unlock prompt. If the prompt +is dismissed (gnome-keyring does this immediately when no prompter can be +shown), times out, or the operation's context expires, the operation fails +with an error matching `keychain.ErrCollectionLocked`: ```go _, err := st.Get(ctx, id) if errors.Is(err, keychain.ErrCollectionLocked) { - // The collection still holds the user's credentials; it just needs to be - // unlocked. Tell the user how (e.g. log in to the desktop session, or - // `gnome-keyring-daemon --unlock`). Do NOT fall back to another store: - // writing new credentials elsewhere while the locked collection keeps the - // old ones would split credentials across two stores. + // The collection still holds the user's credentials. Tell the user how + // to unlock it, for example by logging in to the desktop session or + // running gnome-keyring-daemon --unlock. Do not fall back to another + // store; that would split credentials across stores. } ``` -The operation's `ctx` bounds the unlock-prompt wait, so a caller can put its -own deadline on the "waiting for the user to type the keyring password" case; -an internal cap (30s) always applies as an upper bound. `ErrCollectionLocked` -is deliberately distinct from `ErrKeychainUnavailable`: unavailable means no -keychain exists to use (fall back), locked means the keychain and credentials -exist but need the user's help (surface remediation, don't fall back). +The operation's `ctx` bounds the prompt wait, so a caller can set its own +deadline; an internal 30 second cap always applies. Unavailable means there is +no keychain to use, so fall back. Locked means the keychain and credentials +exist but need the user's help, so surface the remediation and do not fall +back. ### Secrets