feat(core): store the Mergify credential in the OS keychain - #1803
Conversation
sileht
had a problem deploying
to
func-tests-live
September 4, 2026 23:20 — with 
GitHub Actions
Failure
GitHub Actions
Failure
Member
Author
|
This pull request is part of a Mergify stack:
|
Contributor
Merge Protections🔴 3 of 7 protections blocking · waiting on 👀 reviews and ⛓️ dependency
🔴 ⛓️ Depends-On RequirementsWaiting for
This rule is failing.Requirement based on the presence of
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
`mergify auth login` has to put the token it mints somewhere. This adds
that somewhere, keyed by API URL and nothing else -- one machine can
legitimately hold a credential for the hosted service and one for an
on-premise install, and each has to survive the other's `logout`.
Two backends. The OS keychain first: macOS Keychain, the Windows
credential manager, or a freedesktop Secret Service. Then a `0600` JSON
file under the platform configuration directory, used whenever the
keychain is absent or refuses -- a container, a CI runner, or an SSH
session on a headless box has no D-Bus session at all, and `auth login`
still has to work there. So no keychain failure is fatal; each one is a
debug line and a fallback.
Two details worth the reviewer's attention:
- `keyring`'s default feature set resolves to
`zbus-secret-service-keyring-store` on Linux, which speaks D-Bus in
pure Rust. The libsecret-backed alternative would have added a C
build dependency inside the manylinux container and a runtime `.so`
on the user's machine, for the same functionality. The bill for
avoiding that is 63 transitive crates, nearly all of them zbus and
its async-io runtime, and nearly all of them Linux-only.
- `keyring` 4.2's MSRV is 1.88.0, which is exactly this workspace's
floor. A patch release that raises it reddens the msrv job.
`discover()` is infallible on purpose. A machine with no configuration
directory has no *fallback*, which is not the same as having no store --
and that machine, a Windows service account or a systemd unit with
`ProtectHome=`, is often exactly the one whose OS credential manager
works. Failing there would refuse to read an entry sitting right in
front of it. Writing when neither backend is available is the one case
that errors, and it says which of the two is missing.
Two backends mean two ways to disagree, and both are handled by
reading back rather than trusting a write. A keychain that refuses a
delete looks exactly like one that had nothing, so `delete` checks
whether the entry is still there and says so if it is -- `logout`
promising a clean machine cannot report success it did not achieve.
Same check before the fallback write: an older keychain entry that
cannot be removed would shadow the file copy forever, since `get`
reads the keychain first.
A write goes to a sibling temp file and is renamed, so a crash cannot
truncate a store holding a second deployment's credential, and the
backend that did not take the credential is cleared, so logging in once
without a keychain and once with it leaves no readable copy behind. An
emptied store is deleted rather than left as `{}`: an empty object on
disk reads, to anyone auditing the machine, like a credential that
failed to load.
Nothing is wired to this yet.
Fixes MRGFY-8703
Change-Id: Ibd91cd6db5a6c3c5863e40538d9641f97c0b5162
sileht
force-pushed
the
devs/sileht/mrgfy-8703-cli-auth-commands/split-mergify-github-token-resolution--32c37b21
branch
from
September 4, 2026 23:40
3d3e49d to
a02e23a
Compare
sileht
force-pushed
the
devs/sileht/mrgfy-8703-cli-auth-commands/store-mergify-credential-os-keychain--bd91cd6d
branch
from
September 4, 2026 23:40
7273861 to
e0609e3
Compare
sileht
had a problem deploying
to
func-tests-live
September 4, 2026 23:40 — with
GitHub Actions
Error
GitHub Actions
Error
sileht
had a problem deploying
to
func-tests-live
September 4, 2026 23:40 — with
GitHub Actions
Failure
GitHub Actions
Failure
Member
Author
Revision history
|
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.
mergify auth loginhas to put the token it mints somewhere. This addsthat somewhere, keyed by API URL and nothing else -- one machine can
legitimately hold a credential for the hosted service and one for an
on-premise install, and each has to survive the other's
logout.Two backends. The OS keychain first: macOS Keychain, the Windows
credential manager, or a freedesktop Secret Service. Then a
0600JSONfile under the platform configuration directory, used whenever the
keychain is absent or refuses -- a container, a CI runner, or an SSH
session on a headless box has no D-Bus session at all, and
auth loginstill has to work there. So no keychain failure is fatal; each one is a
debug line and a fallback.
Two details worth the reviewer's attention:
keyring's default feature set resolves tozbus-secret-service-keyring-storeon Linux, which speaks D-Bus inpure Rust. The libsecret-backed alternative would have added a C
build dependency inside the manylinux container and a runtime
.soon the user's machine, for the same functionality. The bill for
avoiding that is 63 transitive crates, nearly all of them zbus and
its async-io runtime, and nearly all of them Linux-only.
keyring4.2's MSRV is 1.88.0, which is exactly this workspace'sfloor. A patch release that raises it reddens the msrv job.
discover()is infallible on purpose. A machine with no configurationdirectory has no fallback, which is not the same as having no store --
and that machine, a Windows service account or a systemd unit with
ProtectHome=, is often exactly the one whose OS credential managerworks. Failing there would refuse to read an entry sitting right in
front of it. Writing when neither backend is available is the one case
that errors, and it says which of the two is missing.
Two backends mean two ways to disagree, and both are handled by
reading back rather than trusting a write. A keychain that refuses a
delete looks exactly like one that had nothing, so
deletecheckswhether the entry is still there and says so if it is --
logoutpromising a clean machine cannot report success it did not achieve.
Same check before the fallback write: an older keychain entry that
cannot be removed would shadow the file copy forever, since
getreads the keychain first.
A write goes to a sibling temp file and is renamed, so a crash cannot
truncate a store holding a second deployment's credential, and the
backend that did not take the credential is cleared, so logging in once
without a keychain and once with it leaves no readable copy behind. An
emptied store is deleted rather than left as
{}: an empty object ondisk reads, to anyone auditing the machine, like a credential that
failed to load.
Nothing is wired to this yet.
Fixes MRGFY-8703
Depends-On: #1802