Skip to content

fix(auth): enable real OS keyring backends for secure-storage#40

Merged
Finesssee merged 4 commits into
masterfrom
fix/33-secure-storage-platform-backends
Jul 24, 2026
Merged

fix(auth): enable real OS keyring backends for secure-storage#40
Finesssee merged 4 commits into
masterfrom
fix/33-secure-storage-platform-backends

Conversation

@Finesssee

@Finesssee Finesssee commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #33: --features secure-storage was enabling keyring without platform credential-store features. keyring 3.x then uses its in-memory mock backend, so OAuth/API-key writes appeared to succeed and immediate readback failed (misreported as a macOS code-signing issue).

Changes

  • Wire keyring platform features for all advertised OS backends:
    • apple-native (Keychain)
    • windows-native (Credential Manager)
    • linux-native-sync-persistent + crypto-rust (keyutils + Secret Service)
  • Fail closed on mock backend for writes; is_available() is false for mock
  • Honest error text on auth oauth --secure
  • Hard unit test: default_backend_is_not_mock
  • CI/release: build & test with --features secure-storage; install libdbus-1-dev on Linux; Cross.toml for aarch64 dbus
  • README + manual-release notes aligned

Test plan

  • cargo tree --features secure-storage -i keyring -e features shows platform natives
  • cargo test --features secure-storage default_backend_is_not_mock
  • CI matrix green (ubuntu/windows/macos) with secure-storage
  • macOS: auth oauth --secure + auth status --validate
  • Windows: auth login --secure round-trip

Closes #33

keyring 3.x has no default platform stores and falls back to an
in-memory mock, so --features secure-storage did not persist secrets
on macOS/Windows/Linux (issue #33).

Wire apple-native, windows-native, and linux-native-sync-persistent,
reject the mock backend on writes, and stop blaming code-signing for
mock readback failures.

Closes #33
CI and release never enabled --features secure-storage, so the mock-backend
regression guard could not run and official binaries lacked OS keyring support.
Install libdbus on Linux, add Cross.toml for aarch64, and enable the feature
on test/clippy/release builds.
@Finesssee

Copy link
Copy Markdown
Collaborator Author

Thermo-nuclear review — PR #40

Scope: full branch vs master (including CI/release follow-up).
Note: GitHub blocks self-approve on this account; verdict below is the review bar anyway.

Verdict: merge when CI is green (no structural blockers)

Fixes the real #33 defect. Not an apple-only one-liner. CI/release follow-up makes the regression guard real.


1. Structural regressions

None. Net improvement:

  • Feature matrix matches advertised OS backends (Keychain / Credential Manager / Secret Service)
  • Keyring get/set/delete de-duplicated
  • Mock fail-closed on write; is_available() no longer true for mock
  • CI + release enable --features secure-storage with Linux dbus deps + Cross.toml for aarch64

2. Code judo

Landed: one matrix, shared secret helpers, hard default_backend_is_not_mock test, honest errors.

Follow-up (non-blocking):

  • Collapse remaining #[cfg(feature = "secure-storage")] forks in auth/config/api/doctor behind one storage API
  • Explicit oauth storage mode in metadata (replace empty-secret heuristic)
  • Soft Ok(None) on non-NoEntry reads still blurs missing vs broken

3. Spaghetti

No new ad-hoc branches in unrelated flows. Auth error demotes signing to macOS Keychain-only secondary hint. require_persistent_backend kept private.

4. Contract checklist

Item Status
secure-storage ⇒ durable OS backend fixed
mock ≠ available / not writable fixed
errors name real failure fixed
release binaries include feature fixed
CI runs mock guard fixed
backend_name() aspirational via target_os acceptable post-matrix; runtime type later

5. File size

keyring.rs ~250 lines. No file pushed over 1k. Lockfile growth expected.

6. Residual risks

  1. Headless Linux: backend ≠ mock even if Secret Service daemon absent — confirm CI ubuntu job stays green
  2. cargo install without feature still has no keyring (documented); binstall/release path is the product path
  3. Older keyring tests still skip/warn — only the new mock test is hard

Approval bar

  • no structural regression
  • no 1k-line explosion
  • no spaghetti growth
  • cross-platform root cause (not apple-only)
  • mock not success
  • CI/release wire-up
  • hard regression test
  • full storage-boundary collapse → follow-up

Ship when the OS matrix is green.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Thermo-nuclear review (final) — PR #40

CI is flaky/slow on the runner side right now; this is a local + diff review, not a green-check stamp.

Verdict: approve to merge for #33

No structural blockers in the branch diff. Root cause is fixed cross-platform; mock is fail-closed; CI/release wiring is correct in source even if runners are currently unhappy.

What landed (branch vs master)

Area Change
Cargo.toml apple-native + windows-native + linux-native-sync-persistent + crypto-rust
src/keyring.rs shared get/set/delete, mock detect, fail-closed writes, missing-delete tolerance
auth.rs honest backend-first error; signing demoted to macOS Keychain hint
CI/release --features secure-storage, Linux libdbus-1-dev, Cross.toml for aarch64
Test default_backend_is_not_mock (hard fail)
Docs README + manual-release aligned

Local verification (this machine)

  • cargo tree --features secure-storage -i keyring -e features → all platform natives present
  • cargo test --features secure-storage keyring::tests5/5 ok including mock guard + delete-missing

Approval bar

Non-blocking follow-ups

  1. Collapse remaining #[cfg(feature = "secure-storage")] forks behind one storage API
  2. Explicit oauth storage mode in metadata (replace empty-secret heuristic)
  3. Soft Ok(None) on non-NoEntry reads still blurs missing vs broken

Ship when you're ready; re-check CI when the runner issue clears. Cannot self-approve on GitHub (own PR).

@Finesssee
Finesssee merged commit 80fd309 into master Jul 24, 2026
1 of 4 checks passed
@Finesssee
Finesssee deleted the fix/33-secure-storage-platform-backends branch July 24, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

secure-storage uses mock keyring on macOS because apple-native is not enabled

1 participant