fix(vault): wire OS keyring (libsecret/Keychain/Credential Manager)#3180
Merged
Conversation
…3178) The vault docs promised "OS keyring (macOS Keychain / Windows Credential Manager / Linux Secret Service)" support but store_keyring_key / load_keyring_key only ever exercised the file-based AES-256-GCM fallback. Every `librefang init` on Linux therefore printed "OS keyring unavailable" even when gnome-keyring + libsecret were running fine, contradicting the documentation and unnerving users. The previous developer had even reserved a KEYRING_USER constant for this — the rest of the wiring just never landed. This commit lights up the keyring crate (apple-native / windows-native / sync-secret-service features) and restructures both sides: - store_keyring_key tries keyring::Entry::set_password first; only falls back to the file-based wrapped store on failure (with the existing WARN scoped to that real-fallback path). - load_keyring_key tries keyring::Entry::get_password first; NoEntry drops through silently to the file path so existing installs that already wrote a fallback keep loading; other errors are debug-logged and the file path is tried as a last resort. Closes #3178.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
added
area/security
This was referenced Apr 25, 2026
Merged
houko
added a commit
that referenced
this pull request
Apr 27, 2026
* fix(release): install libdbus-1-dev for glibc Linux CLI builds Beta 6 shipped without all 10 Linux CLI assets because the keyring crate (wired in #3180) pulls libdbus-sys transitively on glibc Linux, but the release workflow only apt-installed pkg-config + libssl-dev. #3265 patched the Dockerfile and #3267 made keyring target-conditional (unblocking musl + android), but release.yml + Cross.toml were never updated, so the four glibc CLI tarballs would still fail on the next release. Adds libdbus-1-dev to: - cli_linux job (x86_64 native build) - cli_linux_mini job (x86_64 native build) - Cross.toml aarch64-unknown-linux-gnu pre-build (cross sysroot) * fix(release): install libdbus-1-dev for desktop Linux build too Same root cause as the CLI fix in this PR — `librefang-desktop` depends on `librefang-extensions`, which on glibc Linux pulls the keyring crate -> libdbus-sys, which needs libdbus-1-dev at link time. The desktop linux job has been getting away with it because webkit2gtk transitively pulls libdbus-1-dev today, but relying on that is brittle: an apt metadata change on the runner image could silently break the desktop linux release. Make it explicit alongside the CLI deps already added in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #3178.
The vault docs (
docs/src/app/security/network-api/page.mdx,docs/src/app/architecture/security/page.mdx) promised:But
store_keyring_key/load_keyring_keyinvault.rsonly ever exercised the file-based AES-256-GCM fallback path. Everylibrefang initon Linux printed:…even when
gnome-keyring+libsecretwere running fine. Contradicts the docs; unnerving for security-conscious users.The previous developer had even reserved a
KEYRING_USER = "master-key"constant gated under#[allow(dead_code)]— the rest of the OS-keyring wiring just never landed.Fix
keyring = { version = "3", features = ["apple-native", "windows-native", "sync-secret-service"] }to the workspace and tolibrefang-extensions. Covers macOS Keychain, Windows Credential Manager, and libsecret-based Linux Secret Service implementations.store_keyring_key: trykeyring::Entry::set_passwordfirst; only fall back to the file-based wrapped store on failure. The existingWARN OS keyring unavailableline is now scoped to that real-fallback path so it no longer fires on healthy hosts.load_keyring_key: trykeyring::Entry::get_passwordfirst.NoEntrydrops through silently to the file path so existing installs that previously wrote a fallback keep loading without WARN noise; other errors aredebug!-logged and the file path is tried as a last resort.Test plan
librefang init→ no fallback WARN → master key visible in Keychain Access under servicelibrefang-vault, accountmaster-key. Subsequent runs unlock vault from Keychain.gnome-keyring: freshlibrefang init→ no fallback WARN → key visible viasecret-tool lookup service librefang-vault username master-key. Headless install without Secret Service still falls back to file with the existing WARN.unset DBUS_SESSION_BUS_ADDRESS): WARN still fires, file fallback works.