feat(platform): tell an absent secret service from one that refused (T15b) - #30
Merged
Merged
Conversation
…is wrong both ways on Linux
T15b was recorded as one mistake and is two. `keyring`'s secret-service backend maps `Locked`,
`NoResult` and `Prompt` to `NoStorageAccess` and everything else to `PlatformFailure`, so on this
system a session with no secret service arrives as a failure of ours — rule 4 of
platform-abstraction.md inverted — **and a keyring that is merely locked arrives as a machine with
no credential store**, which is the same rule inverted the other way. Only the first was written
down. Both are fixed by not consulting `keyring`'s judgement on Linux at all.
**What is read instead is the D-Bus error name, and the names were measured rather than looked up.**
A probe built against these exact crate versions, run on a real Linux, one environment each:
bus present, no provider -> org.freedesktop.DBus.Error.ServiceUnknown
no bus, no DISPLAY -> org.freedesktop.DBus.Error.NotSupported
bus address points nowhere -> org.freedesktop.DBus.Error.FileNotFound
Three shapes, not one. The task recorded only `ServiceUnknown`, which is what a CI runner answers
because a runner has a session bus; **the headless machine the task exists for never reaches that
name at all**, because it fails one step earlier, at the bus. A match on the recorded name would
have fixed CI and left the case it was written for exactly as broken.
**The name and not the message, and that is not a bet on the future.** `dbus::Error`'s `Display`
prints the message and never the name, so matching text means matching the bus implementation's own
wording — and it is already wrong today: Ubuntu 24.04 answers an unreachable bus with "Using X11 for
dbus-daemon autolaunch was disabled at compile time", not the "without a $DISPLAY for X11" everybody
quotes, because that distribution disabled X11 autolaunch when it built dbus. The names are in the
D-Bus specification, so `dbus-daemon` and `dbus-broker` must both use them.
The list is closed and the unmatched direction is the safe one: anything not on it stays
`Error::Secret`, so a fourth shape is reported as a store that refused rather than as a capability
this machine lacks. `AccessDenied` and `NoReply` both land there, and the second is not
hypothetical — measuring turned one up, a `gnome-keyring` the bus activated that then never answered.
**Three modules over one implementation, which is the opposite split from three wrappers over one
library.** `crate::secrets` stays a single `Secrets`; what is per-OS is one reading of one error,
because each backend encodes "there is nothing here" differently and `NoStorageAccess` is exactly
right on the other two — `errSecNoSuchKeychain` and friends on macOS, `ERROR_NO_SUCH_LOGON_SESSION`
on Windows, both of which MixEngine can genuinely meet. Each carries its own workaround now, so the
hint names what this machine is missing rather than reciting Linux advice on a Mac.
`dbus-secret-service` becomes a direct dependency on Linux, behind `host`, so it stays out of
`mixengine-elevate`'s closure — verified, that tree has no dbus in it. The one silent failure left
is two copies of the crate in one tree, which would make the downcast answer `None` forever without
failing to compile; the next commit makes CI count them.
… this job **The fix quietly turned four loud failures into eight quiet skips, and that had to be handled in the same change.** `secrets.rs` skips on `Error::UnsupportedPlatform` and fails on everything else — so before this, a Linux leg whose `gnome-keyring` never arrived produced a wall of red. Now it produces a green leg that judged no store at all. Three places in this job leaned on the old behaviour and said so in prose; each is corrected rather than left to be believed: - the missing-keyring case is an `::error` and an exit, not a `::warning`. It was a warning because the failures behind it were unmissable, and they are not unmissable any more. - the wait for `org.freedesktop.secrets` to answer stops being a convenience. A `gnome-keyring` that forks and dies leaves its name unowned, which is now read as a machine with no secret service, so this loop is the whole guard rather than five seconds saved on reading a failure. - the workflow's own comment claimed a bus with no provider "is a store that is there and refusing, because that is what it is". It is not, and was not — that sentence was the bug, written down. **And the branch itself is now walked on every run rather than trusted.** Every other leg hands the tests a store that works, on purpose, which leaves the answer a machine *without* one gets as a path no green run has ever taken — which is exactly how the bug survived to be found by a stack trace on somebody's console instead of by this suite. `test-absent-secret-service.sh` takes the store away in the three ways a Linux can lose it, one per D-Bus error name the capability matches, including the `ServiceUnknown` that opened T15b. A round that sabotages nothing must not pass, so it asserts rather than skips: `MIXENGINE_TEST_NO_KEYRING=1` makes *finding* a store a failure. Checked both ways here — with the variable set, a run against this machine's real gnome-keyring fails on that assertion, so the guard is known to bite rather than assumed to. **The remaining silent failure is counted in `lint`.** Two versions of `dbus-secret-service` in one tree would make the downcast in `linux/secrets.rs` answer `None` for ever: not a build failure, not a test failure, just every machine without a keyring quietly told its store refused again. The tree is asked how many there are, and one is the only right answer.
…tation `mixengine-platform` now depends on `keyring`'s own Linux backend, which the one-crate-per-concern table in standards/rust.md exists to prevent — so it is argued rather than imported, and the table carries the exception rather than being quietly untrue. The ADR records what was ruled out, because each rejection is a measurement rather than a preference. Waiting for `keyring` to fix it: 4.1.6 restructures into `keyring-core` plus per-store crates and carries the same four lines, so upgrading is a migration and not a fix. Reading the variant the backend already has: `Error::Unavailable` is documented for exactly this case and is constructed nowhere in 4.1.0. Matching the message text: wrong today rather than eventually — `dbus::Error` prints `message()` and never `name()`, and Ubuntu 24.04 already answers with wording no account of this failure quotes, because that distribution built dbus with X11 autolaunch off. It also records the two things a reader would otherwise have to rediscover: that the failure mode of this decision is silence rather than a broken build, which is why `lint` counts the crate rather than trusting it, and that a machine with no store still cannot run a service that needs a credential — ADR 0006 gives a root password exactly one home, and that is unchanged. What changes is that the refusal now names what is missing.
Phase 1 goes to 14 of 15 and the debt table loses a row. The entry is rewritten rather than ticked, because what the work found was mostly about the entry. It recorded one misreading and there were two — the same four lines in `keyring` that report a headless machine as our failure also report a locked keyring as a machine with no store. It recorded one D-Bus error name and there are three, and the one it recorded is the one a *CI runner* answers: a headless machine fails a step earlier, at the bus, so a match on the recorded name would have fixed CI and left the case the entry exists for exactly as broken. And it framed "read the name" and "match the text" as a live choice with a cost each; matching text is already wrong on Ubuntu 24.04, which answers an unreachable bus in wording no account of this failure quotes. The entry now also carries what the task deliberately did not do — no `mix doctor` check, which is its own task — and what it cost elsewhere, which is the CI hole the fix opened by turning loud failures into quiet skips. `first_run.rs` loses its "until T15b lands" note: the refusal there is unchanged, since ADR 0006 gives a root password exactly one home, and what changed is that it stopped reporting MixEngine as broken. The T33 spec and plan still describe T15b as open. Both are records of what was decided on 2026-08-20 and are left alone; editing them would be rewriting a decision rather than making one.
…rate as two The dbus-secret-service count failed on a tree that is fine. This job sets CARGO_TERM_COLOR: always, so a repeated subtree marker arrives as escape sequences around the (*) and the sed meant to strip it stops matching — two lines, one package. Worth recording rather than just fixing: the same command passed locally, and it passed for a reason that does not hold here. cargo drops colour by itself when it is not writing to a terminal, so a check run through a pipe on a developer machine never sees the input CI hands it. Reproduced by exporting the variable, which is what verifying it should have done the first time.
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.
T15b asked for one error to be reported as another. What it turned out to need was measurement, and
the measurements disagreed with the task in three places.
What was wrong, and it was not one thing
keyring's secret-service backend classifies in four lines:no_accessisNoStorageAccess, which on Windows and macOS means exactly "there is no store here".On Linux it means a keyring that is present and locked, and a session with no secret service at
all falls through to
PlatformFailure. So rule 4 ofplatform-abstraction.mdwas inverted in bothdirections from the same four lines: a headless machine was told MixEngine had failed, and a locked
keyring was told the machine had no store. The task recorded only the first.
Three names, and the recorded one is the wrong one to fix
Measured with a probe built against these exact crate versions, on a real Linux, one environment
each:
org.freedesktop.secretsServiceUnknownNotSupportedDBUS_SESSION_BUS_ADDRESSnames a socket that is not thereFileNotFoundServiceUnknownis what the task recorded, from a CI run — because a runner has a session bus. Aheadless machine never reaches that name. It fails a step earlier, at the bus. A match on the
recorded name alone would have turned CI green and left the case the task exists for exactly as
broken.
Why the name and not the message
The task framed this as a live choice with a cost on each side. It is not.
dbus::Error'sDisplayprintsmessage()and nevername(), so matching text means matching abus implementation's own wording — and it is already wrong: Ubuntu 24.04 answers an unreachable bus
with "Using X11 for dbus-daemon autolaunch was disabled at compile time", not the "without a
$DISPLAY for X11" that every account of this failure quotes, because that distribution built dbus
with X11 autolaunch off. Two bus implementations are deployed and are not obliged to agree. The
names are in the D-Bus specification.
Waiting for upstream was a fourth non-option:
keyring4.1.6 restructures intokeyring-coreplusper-store crates and carries the same four lines, and
dbus_secret_service::Error::Unavailable—documented for exactly this case — is constructed nowhere in 4.1.0.
So the direct edge onto
keyring's backend is taken and argued in ADR 0013. The list of names isclosed, and everything off it stays
Error::Secret— the safe direction to be wrong in, sincereporting a working machine as unsupported sends a person hunting a keyring they already own.
Three modules over one implementation
crate::secretsstays a singleSecrets; what is per-OS is one reading of one error. Each systemnow carries its own workaround too, so a Mac is not handed Linux advice.
The hole this opened, closed in the same change
secrets.rsskips onUnsupportedPlatformand fails on everything else — so the fix silentlyturned four loud CI failures into eight quiet skips. A Linux leg whose
gnome-keyringneverarrived used to produce a wall of red and now produces a green leg that judged nothing. Three places
leaned on the old behaviour in prose; the missing-keyring case is now an error rather than a warning,
and the wait for
org.freedesktop.secretsstopped being a convenience.And the absent branch is now walked on every run rather than trusted:
test-absent-secret-service.shtakes the store away in all three ways, withMIXENGINE_TEST_NO_KEYRING=1making finding a store a failure. Checked both ways — with thevariable set, a run against a real gnome-keyring fails on that assertion.
The one silent failure left is two copies of
dbus-secret-servicein one tree: not a build failure,not a test failure, just the downcast answering
Nonefor ever.lintcounts them.One thing this does not do
No
mix doctorcheck. Nothing there asks about the keyring today, and aNotefor a machine with nostore is a task of its own.
Phase 1 goes to 14 of 15; the debt table loses a row. CI green on all ten jobs.