From 8caac1d34d44411a9f52b12fec18833683e97bd8 Mon Sep 17 00:00:00 2001 From: leemour Date: Wed, 21 Oct 2026 00:58:56 +0200 Subject: [PATCH] fix(keyring): require the Secret Service on Linux instead of the kernel keyring @napi-rs/keyring 2.1.0 tries the Secret Service first and silently falls back to the kernel keyutils store when it fails. On 2026-10-21 a locked gnome-keyring answered "DBus error: Did not receive a reply": reads of stored secrets returned null with no hint, and a write went to kernel memory that a reboot clears. Nothing threw, so Credentials never used its file. systemKeyring now passes { linux: { store: "secret-service" } }, so the binding throws and Credentials' auto mode warns once and falls back to its 0600 file. The warning now says the keyring could not be reached (locked, or no keyring service) rather than "storing in", which was wrong on a read. Release 0.47.5. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 13 +++++++++ package.json | 2 +- src/credentials.test.ts | 2 +- src/credentials.ts | 5 ++-- src/keyring.test.ts | 63 ++++++++++++++++++++++++++++++++++++++++- src/keyring.ts | 19 ++++++++++--- 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b56b66..ef0f603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ Versions before 0.8.0 are in the [git tags](https://github.com/WireCatLabs/cli-b Every entry says what changed as a caller sees it, why, and what to watch for — the rules are [`docs/dev/CONVENTIONS.md`](docs/dev/CONVENTIONS.md#the-changelog). +## 0.47.5 — 21.10.2026 + +### Fixed + +- **On Linux the OS keyring now means the Secret Service, never the kernel keyring.** `@napi-rs/keyring` silently + fell back to the in-memory kernel keyring when the Secret Service did not answer (gnome-keyring locked, D-Bus + timeout): reads of stored secrets came back empty with no hint why, and writes went to memory that a reboot + clears. `systemKeyring` now pins every entry to the Secret Service, so such a failure throws and `Credentials` + in `auto` mode warns once and uses its file. What to watch for: a machine with no Secret Service but a working + kernel keyring now gets the credentials file instead; a secret written to the kernel keyring by an earlier + version is no longer read and has to be stored again. The warning now reads "could not reach the OS keyring — it + may be locked, or no keyring service is running (…); using instead, readable only by you". + ## 0.47.4 — 20.10.2026 ### Added diff --git a/package.json b/package.json index 759942d..beb6c5e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@wirecat/cli-base", - "version": "0.47.4", + "version": "0.47.5", "description": "The parts every command line tool needs: output modes, terminal rendering, error model, exit codes, credentials, clocks", "license": "Apache-2.0", "author": "Viacheslav Ptsarev", diff --git a/src/credentials.test.ts b/src/credentials.test.ts index b6dd210..3f4a029 100644 --- a/src/credentials.test.ts +++ b/src/credentials.test.ts @@ -46,7 +46,7 @@ describe("Credentials", () => { expect(credentials.write("default", "s3cret")).toBe("file") expect(credentials.read("default")).toEqual({ secret: "s3cret", source: "file" }) expect(warnings).toHaveLength(1) - expect(warnings[0]).toMatch(/keyring is unavailable/) + expect(warnings[0]).toMatch(/could not reach the OS keyring — it may be locked/) }) it("writes the fallback file with owner-only permissions", () => { diff --git a/src/credentials.ts b/src/credentials.ts index c106706..058a3a7 100644 --- a/src/credentials.ts +++ b/src/credentials.ts @@ -160,8 +160,9 @@ export class Credentials { if (!this.#warned) { this.#warned = true this.#warn( - `the OS keyring is unavailable (${error instanceof Error ? error.message : String(error)}); ` + - `storing in ${this.#path()} instead, readable only by you`, + `could not reach the OS keyring — it may be locked, or no keyring service is running ` + + `(${error instanceof Error ? error.message : String(error)}); ` + + `using ${this.#path()} instead, readable only by you`, ) } return undefined diff --git a/src/keyring.test.ts b/src/keyring.test.ts index 99404f7..740a12a 100644 --- a/src/keyring.test.ts +++ b/src/keyring.test.ts @@ -1,4 +1,8 @@ -import { describe, expect, it } from "vitest" +import { mkdtempSync } from "node:fs" +import { createRequire } from "node:module" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { afterEach, describe, expect, it, vi } from "vitest" import { brokenKeyring, memoryKeyring, systemKeyring } from "./keyring.js" describe("memoryKeyring", () => { @@ -42,3 +46,60 @@ describe("systemKeyring", () => { expect(() => systemKeyring.delete("cli-base-test", "default")).toThrow(/real OS keyring/) }) }) + +describe("systemKeyring with the binding replaced", () => { + const require = createRequire(import.meta.url) + const path = require.resolve("@napi-rs/keyring") + const guard = require.cache[path] + + afterEach(() => { + require.cache[path] = guard + }) + + const withBinding = async (Entry: unknown) => { + require.cache[path] = { ...guard, exports: { Entry } } as NodeJS.Module + vi.resetModules() + return import("./keyring.js") + } + + it("pins every entry to the Secret Service, so a locked keyring throws instead of using kernel memory", async () => { + const seen: unknown[] = [] + class RecordingEntry { + constructor(_service: string, _account: string, options: unknown) { + seen.push(options) + } + getPassword = () => null + setPassword = () => {} + deletePassword = () => false + } + const { systemKeyring: keyring } = await withBinding(RecordingEntry) + + expect(keyring.get("cli-base-test", "default")).toBeNull() + keyring.set("cli-base-test", "default", "x") + keyring.delete("cli-base-test", "default") + + expect(seen).toEqual(Array(3).fill({ linux: { store: "secret-service" } })) + }) + + it("lets Credentials warn and use its file when the pinned Secret Service does not answer", async () => { + class LockedEntry { + constructor() { + throw new Error("DBus error: Did not receive a reply") + } + } + const { systemKeyring: keyring } = await withBinding(LockedEntry) + const { Credentials } = await import("./credentials.js") + const warnings: string[] = [] + const credentials = new Credentials({ + configDir: mkdtempSync(join(tmpdir(), "cli-base-keyring-")), + service: "cli-base-test", + keyring, + env: {}, + warn: (message) => warnings.push(message), + }) + + expect(credentials.write("default", "s3cret")).toBe("file") + expect(credentials.read("default")).toEqual({ secret: "s3cret", source: "file" }) + expect(warnings).toEqual([expect.stringMatching(/could not reach the OS keyring .*Did not receive a reply/)]) + }) +}) diff --git a/src/keyring.ts b/src/keyring.ts index 8767a25..da696d2 100644 --- a/src/keyring.ts +++ b/src/keyring.ts @@ -26,7 +26,16 @@ interface Entry { deletePassword(): boolean } -type EntryConstructor = new (service: string, account: string) => Entry +type EntryConstructor = new ( + service: string, + account: string, + options?: { linux?: { store?: "secret-service" | "keyutils" } }, +) => Entry + +// Without a pinned store the binding silently falls back to the kernel keyring when the Secret +// Service fails (locked, not running): reads miss every stored secret and writes are lost on +// reboot, and nothing throws, so `Credentials` never falls back to its file. Ignored off Linux. +const ENTRY_OPTIONS = { linux: { store: "secret-service" } } as const const require = createRequire(import.meta.url) let cached: EntryConstructor | undefined @@ -38,14 +47,16 @@ const entryClass = (): EntryConstructor => { return cached } +const entry = (service: string, account: string): Entry => new (entryClass())(service, account, ENTRY_OPTIONS) + export const systemKeyring: KeyringStore = { // Returns null for an absent entry rather than throwing — measured against @napi-rs/keyring // on 2026-09-13, and the difference matters: a throw here means the keychain is unusable. - get: (service, account) => new (entryClass())(service, account).getPassword(), + get: (service, account) => entry(service, account).getPassword(), set: (service, account, secret) => { - ;new (entryClass())(service, account).setPassword(secret) + entry(service, account).setPassword(secret) }, - delete: (service, account) => new (entryClass())(service, account).deletePassword(), + delete: (service, account) => entry(service, account).deletePassword(), } /** An in-memory stand-in. Every test uses this; none can reach the real keychain. */