Make keyring availability checks read-only

[code-monger-givenall]

What changed

Credential-store initialization now checks Keychain availability with a read-only lookup. A missing availability entry means the keyring is working. Other errors keep the existing plaintext-file fallback.

The keyring interface lets the tests verify that initialization calls Get once and never calls Set or Delete.

Why

Every CLI process previously wrote and deleted the same hey::test item before loading the real credential. Parallel processes could make that probe fail. The affected process then switched to an empty plaintext store and reported not logged in, even though valid credentials were still in Keychain.

I reproduced this with 60 read-only hey auth status --json calls at concurrency 6. Before the change, 25 printed the keyring warning and reported unauthenticated. After the change, all 60 completed without either failure.

Checks

• env GOWORK=off mise x golangci-lint@2.10.1 -- make check
• env GOWORK=off mise x -- go test -race -count=1 ./internal/auth
• 60 concurrent installed CLI auth checks, with zero keyring warnings and zero false unauthenticated results

---

Summary by cubic

Makes keyring availability checks read-only to remove concurrency races that caused false plaintext fallbacks. Previously the CLI wrote/deleted hey::test; now it does a single Get on hey::availability, where ErrNotFound means the keyring is available and only other errors fall back to the file store.

• Introduces a credentialKeyring interface with a default systemCredentialKeyring wrapper around github.com/zalando/go-keyring; Store now depends on this interface so tests assert exactly one Get and zero Set/Delete during init.
• Replaces direct calls to github.com/zalando/go-keyring with s.keyring across load/save/delete paths. External behavior is unchanged except fewer keyring warnings and no spurious unauthenticated states under concurrency.

^{Written for commit 4374b62. Summary will update on new commits.}

[Copilot]

Pull request overview

Makes credential-store initialization read-only, preventing concurrent keyring probes from causing false plaintext fallbacks.

Changes:

• Adds an injectable keyring interface.
• Replaces write/delete availability probes with one read.
• Tests read-only probing and fallback behavior.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
internal/auth/store.go	Implements read-only keyring availability checks.
internal/auth/store_test.go	Verifies probe calls and fallback behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jr-lillard]

I independently confirmed this race on macOS with the same shared hey::test probe that remains in current main at 58c83f1.

Two local CLI processes can overlap their probe writes. Keychain then returns a duplicate unique-index error for one process; that process treats Keychain as unavailable, switches to an empty plaintext fallback, and reports Not logged in even though the real credential remains present.

I tested a current-code variant that gives every process and Store a unique probe account:

• focused forced-overlap regression: 100/100 passes
• full auth race suite: 20/20 passes
• 64 simultaneous local auth status checks: zero Keychain warnings and zero false unauthenticated results
• full CLI release, race, security, and coverage gates passed

The read-only lookup in this PR is conceptually cleaner because it removes temporary probe writes entirely. This PR now conflicts with current main; I can prepare a clean current-main rebase and add the deterministic overlap regression if that would help. I have not opened a competing PR.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[robzolkos]

Maintainer validation complete. Thanks for the original fix.

We rebased this onto current main and added deterministic concurrent regression coverage. The tests failed against the previous implementation with 16 Set and 16 Delete calls against the shared probe, then passed with one read-only Get per store and no keyring mutations.

We also installed and tested the branch on Omarchy with GNOME Keyring 50.0:

• existing OAuth credentials remained authenticated in the system keyring
• 24/24 concurrent auth checks succeeded without fallback warnings
• CreateItem and Delete calls dropped from one each to zero
• a fresh bar-plugin hey watch started successfully and maintained its connections
• no duplicate-item warnings, Secret Service assertions, or daemon restarts
• full local checks, race tests, and GitHub CI passed

An independent security review found no blocking concerns. The change does not delete, overwrite, expose, or silently copy credentials to plaintext.

GNOME Keyring’s ability to abort during a legitimate OpenSession remains an upstream daemon defect. This fixes HEY’s avoidable shared write/delete probe and materially reduces the exposure described in #352.

Ready to merge.