credentials: distinguish "no backend" from real keyring.Open errors #589

New issue

Closed

opened 2026-08-07 18:36:21 +00:00 by the.auditor · 1 comment

the.auditor commented

2026-08-07 18:36:21 +00:00

Owner

In internal/tpagectl/credentials/credentials.go the open() helper maps every keyring.Open error to ErrUnavailable. A genuinely present-but-broken backend (Secret Service running but dbus disconnected, locked/failed keychain, permission denied) is therefore indistinguishable from "no keychain on this machine", which silently triggers the plaintext config-file fallback (0600).

Prefer checking keyring.ErrNoAvailBackend / the "No suitable keyring backend" condition and returning ErrUnavailable only for that; surface other open errors so callers don't quietly downgrade from secure storage on real failures.

See #588

In `internal/tpagectl/credentials/credentials.go` the `open()` helper maps *every* `keyring.Open` error to `ErrUnavailable`. A genuinely present-but-broken backend (Secret Service running but dbus disconnected, locked/failed keychain, permission denied) is therefore indistinguishable from "no keychain on this machine", which silently triggers the plaintext config-file fallback (0600). Prefer checking `keyring.ErrNoAvailBackend` / the "No suitable keyring backend" condition and returning `ErrUnavailable` only for that; surface other open errors so callers don't quietly downgrade from secure storage on real failures. See https://git.lan.thwap.org/thwap/thwap-pagesd/pulls/588

the.auditor added the

Kind/Security

Priority

High

labels

2026-08-07 18:36:35 +00:00

automation referenced this issue

2026-08-07 19:36:09 +00:00

feat(tpagectl): implement credential management #588

automation referenced this issue

2026-08-11 01:29:32 +00:00

fix(tpagectl): address CLI issues #634

automation closed this issue

2026-08-11 01:30:01 +00:00

automation commented

2026-08-11 01:30:02 +00:00

Owner

Already resolved in PR #588: credentials.open() maps only keyring.ErrNoAvailImpl to ErrUnavailable; genuine keyring Open errors now propagate as open keychain: ..., and Get() propagates non-ErrKeyNotFound read errors. Verified during the CLI-lump pass; closing without further change.

Already resolved in PR #588: `credentials.open()` maps only `keyring.ErrNoAvailImpl` to `ErrUnavailable`; genuine keyring `Open` errors now propagate as `open keychain: ...`, and `Get()` propagates non-`ErrKeyNotFound` read errors. Verified during the CLI-lump pass; closing without further change.