Let a failed update check say so, instead of reporting good news

The phone reported every build as current because the release repository is
private. Gitea answers 404 rather than 403 for a repo you cannot see, the client
reads that address anonymously, and AndroidUpdateChannel caught the failure and
returned null — which IUpdateChannel documented as meaning "this build is the
latest". The check had never once succeeded on any phone and nothing anywhere
said so.

Two faults, and the second is why the first lasted.

The seam said null was the honest answer for an unreachable channel, on the
reasoning that the caller does the same thing either way. That is true of the
six-hourly pass and false of CHECK NOW. UpdateViewModel already draws the line
correctly — silent on the timer, the exception's message on the button — and it
could only ever draw the first half, because nothing was ever thrown at it. The
desktop's channel does not catch, so the interface described neither
implementation.

So CheckAsync throws now, and null means one thing. A release that is reachable
but missing its manifest or the APK it names throws too: "you are up to date"
about a half-published feed is the same lie in a smaller costume, and the
self-healing that argument protected is untouched, since the timer still swallows
everything.

The precondition is written down where somebody would look, rather than left as a
sentence about where a token could live. ADR 0013 §4 already said a private
release repository was incompatible with this design; nobody checked which side
of it this repository was on. It is one curl, and manual-checks phase 16 now
opens with it — pointedly not against /api/v1/version, which answers 200 from a
forge that is up whatever is readable on it, and which is what made this look
like nothing was wrong.

Phone check 17.4 was the one that passed all along. It now presses CHECK NOW with
the network off as well as on, because two different answers are the whole of
what makes the first one worth reading.
This commit is contained in:
2026-08-05 11:17:44 +02:00
parent ca07d63585
commit 9a7e3bbd5c
6 changed files with 168 additions and 46 deletions
+13
View File
@@ -134,6 +134,19 @@ operator of a deployment is precisely the party the trust model is about. An ope
get it. They are not where it comes from, and the update check inside the application points at the get it. They are not where it comes from, and the update check inside the application points at the
project's own forge and nowhere else. See [ADR 0011](docs/adr/0011-android-distribution.md) rule 2. project's own forge and nowhere else. See [ADR 0011](docs/adr/0011-android-distribution.md) rule 2.
**That address is read anonymously, so the repository behind it has to be public** — on both heads. There is
no token and there is deliberately nowhere to put one: it would have to be readable before the vault is
unlocked, and the only file readable then is the plaintext settings file, which is the one place a token may
not go. If the repository is private, every check answers `404`, and the honest thing the client can do
about that is say so. It is one line to check:
```bash
curl -so /dev/null -w '%{http_code}\n' https://git.dodotech.cloud/api/v1/repos/DodoTech/DodoSSH
```
`200` and updates work. `404` and they cannot. `/api/v1/version` answering `200` proves only that the forge
is up, which is what made this look like nothing was wrong for as long as it did.
**Uninstalling removes the application and leaves your vault cache** at `%LOCALAPPDATA%\DodoSSH`, so **Uninstalling removes the application and leaves your vault cache** at `%LOCALAPPDATA%\DodoSSH`, so
reinstalling asks for your passphrase rather than starting over. Use **Sign out** inside the application if reinstalling asks for your passphrase rather than starting over. Use **Sign out** inside the application if
you want the machine to genuinely forget everything — an uninstall is not a sign-out, and does not withdraw you want the machine to genuinely forget everything — an uninstall is not a sign-out, and does not withdraw
@@ -110,6 +110,23 @@ private release repository is incompatible with this design and that is worth kn
discovering, because the token would have to be readable before the vault is unlocked, and this file is the discovering, because the token would have to be readable before the vault is unlocked, and this file is the
one place that can be read then — which is the one place a token may not go. one place that can be read then — which is the one place a token may not go.
**It was discovered.** The paragraph above was written and the repository was private anyway, so every
check on every client asked an address that answers `404` — Gitea does not distinguish "not there" from
"not yours" — and both heads reported the running build as current. It had never once worked, on either
platform, and nothing anywhere said so. Two things came out of that and both are now written down rather
than assumed:
- **The feed repository must be readable with no credentials, and that is a deployment precondition** rather
than a property of the code. It is checkable in one line, which is the only reason it is worth stating:
`curl -so /dev/null -w '%{http_code}' https://git.dodotech.cloud/api/v1/repos/DodoTech/DodoSSH` answers
`200` when the design holds and `404` when it does not. `/api/v1/version` answering `200` proves only that
the forge is reachable, which is what made this look like nothing was wrong.
- **An unreachable channel must be distinguishable from a current build.** `IUpdateChannel.CheckAsync` used
to document null as the honest answer for both, on the reasoning that the caller does the same thing
either way. That is true of the six-hourly pass and false of CHECK NOW, and it is what turned a total
outage into a reassuring sentence. It throws now, and `UpdateViewModel` keeps the distinction it always
had: silent on the timer, the exception's own message on the button.
### 5. `MinClientVersion` stays unread, and if it is ever read it may not fetch ### 5. `MinClientVersion` stays unread, and if it is ever read it may not fetch
`MetaResponse.MinClientVersion` has existed since M1, is served, is fetched on every sign-in, and is read `MetaResponse.MinClientVersion` has existed since M1, is served, is fetched on every sign-in, and is read
+14
View File
@@ -59,6 +59,20 @@ losing the local cache, the outbox and the device key.
no setting for it — a configurable update URL is exactly the knob that would let an operator, or a no setting for it — a configurable update URL is exactly the knob that would let an operator, or a
stray edit to a plaintext file, point the update path at the party ADR 0001 models as the adversary. stray edit to a plaintext file, point the update path at the party ADR 0001 models as the adversary.
**Which means the repository has to be public, and it was not.** A constant address with no token is
an anonymous read; the repository was private, so every check answered `404` and every phone reported
itself current. See ADR 0013 §4, which had written the incompatibility down as a fact about where a
token could live and not as something to check on the deployment. The check is one `curl` and it is
there.
The Android channel made it invisible rather than merely broken: it caught every network and parse
failure and answered null, which `IUpdateChannel` then documented as meaning "this build is current". So
a pressed CHECK NOW reported the latest build for an outage that had lasted since the feature shipped.
It throws now. The timer still swallows — a phone on a train is not news, and a half-published release
still fixes itself in six hours without anybody being told — and the button reports what happened,
which is the distinction `UpdateViewModel` was written to make and was never given anything to make it
with.
5. **The comparison is Android's `versionCode` and not the version name.** That integer is what the 5. **The comparison is Android's `versionCode` and not the version name.** That integer is what the
platform itself uses to accept or refuse an install, so comparing anything else would let the client platform itself uses to accept or refuse an install, so comparing anything else would let the client
offer an update the platform then rejects. The feed publishes it in a small JSON manifest beside the offer an update the platform then rejects. The feed publishes it in a small JSON manifest beside the
+39 -5
View File
@@ -1766,6 +1766,25 @@ Walk it once per release, and in order — 16.6 onwards needs 16.1 to have happe
Run `pwsh -File scripts/release-windows.ps1` first. It stops after packing, on purpose, so that everything Run `pwsh -File scripts/release-windows.ps1` first. It stops after packing, on purpose, so that everything
below happens before anything reaches a user. below happens before anything reaches a user.
### 16.0 The feed is readable without credentials · **do this one first, on either platform**
Nothing else in this phase or the next means anything until this passes. Run it from anywhere, signed in to
nothing:
```bash
curl -so /dev/null -w '%{http_code}\n' https://git.dodotech.cloud/api/v1/repos/DodoTech/DodoSSH
```
**Pass:** `200`.
**Failure means:** `404` is a private repository — Gitea does not distinguish "not there" from "not yours" —
and the clients read this address anonymously, because there is no token and deliberately nowhere to put
one. Every check on every machine will answer 404 and the feature cannot work. See
[ADR 0013 §4](adr/0013-desktop-distribution-and-updates.md).
**Do not check `/api/v1/version` instead.** It answers `200` from a forge that is up regardless of what is
readable on it, and that is exactly what made this look fine while nothing worked.
### 16.1 The installer needs no administrator, and lands beside the vault rather than on it · **the one that would destroy data** ### 16.1 The installer needs no administrator, and lands beside the vault rather than on it · **the one that would destroy data**
Run `Releases\DodoSSH.Desktop-win-Setup.exe` from an ordinary account. Then look at `%LOCALAPPDATA%`. Run `Releases\DodoSSH.Desktop-win-Setup.exe` from an ordinary account. Then look at `%LOCALAPPDATA%`.
@@ -1917,15 +1936,30 @@ height. Never `1.0.0`.
**Failure means:** `1.0.0` is the `_AndroidVersionName` fix having come undone; see platform-flags. **Failure means:** `1.0.0` is the `_AndroidVersionName` fix having come undone; see platform-flags.
### 17.4 A check that finds nothing says so, and one on a timer does not ### 17.4 A check that finds nothing says so, a check that fails says something else, and a timer says neither
**Walk 16.0 first.** This check is the one that used to pass while the whole feature was broken: with the
repository private, every request answered 404, the channel swallowed it, and CHECK NOW reported the running
build as the latest. It read exactly like success.
Press CHECK NOW on the newest build there is. Press CHECK NOW on the newest build there is.
**Pass:** it answers — "DodoSSH x.y.z is the latest build." Then leave the app open and do nothing for **Pass:** it answers — "DodoSSH x.y.z is the latest build."
several minutes.
**Pass:** no message appears on its own. A background pass that found nothing is silent, which is what makes Now turn the phone's network off and press CHECK NOW again.
the feature tolerable.
**Pass:** it says it could not reach the forge, in whatever words the failure came with, and the section
shows the failed state rather than a version number. **It must not say the build is the latest.** Those two
answers being different is the whole of what makes the first one worth reading.
Turn the network back on, and leave the app open doing nothing for several minutes.
**Pass:** no message appears on its own, either way. A background pass that found nothing — or that could
not ask — is silent, which is what makes the feature tolerable.
**Failure means:** if the offline press reports the latest build, `AndroidUpdateChannel.CheckAsync` is
catching again. Every outage then looks like good news, which is how this went unnoticed from the day it
shipped.
### 17.5 An update is found, fetched without being asked, and installed only when asked · **the whole point** ### 17.5 An update is found, fetched without being asked, and installed only when asked · **the whole point**
@@ -132,51 +132,75 @@ internal sealed class AndroidUpdateChannel : IUpdateChannel
/// <summary>What Android thinks is installed, which is the number the comparison is made on.</summary> /// <summary>What Android thinks is installed, which is the number the comparison is made on.</summary>
private long InstalledVersionCode { get; } private long InstalledVersionCode { get; }
/// <inheritdoc /> /// <summary>
/// Asks the channel's release what it is publishing.
/// </summary>
/// <remarks>
/// <para>
/// ◆ <b>Nothing here is caught, and it used to catch everything.</b> Every network and parse failure
/// resolved to null on the argument that an unreachable forge is a phone on a train — which is a good
/// argument for the timer and no argument at all for the button. <c>UpdateViewModel</c> already draws
/// that line correctly: a background pass swallows and goes back to Idle, a pressed CHECK NOW reports
/// the message. Swallowing here took the second half away and answered every failure with "you are on
/// the latest build".
/// </para>
/// <para>
/// <b>What that hid was the whole feature not working.</b> The release repository was private, so every
/// request answered 404 — Gitea does not distinguish "not there" from "not yours" — and every phone
/// reported itself current. The check had never once succeeded and nothing anywhere said so. The feed
/// has to be readable without credentials; see the interface, which now records that as a precondition.
/// </para>
/// <para>
/// A release that is <em>reachable</em> but missing the manifest or the APK it names throws too, and
/// that is deliberate rather than incidental. It means CI published half a release, which is a fact
/// worth an answer — "you are up to date" about a broken feed is the same lie in a smaller costume. The
/// self-healing that argument protected is unaffected: the timer swallows it, so a half-published
/// release still fixes itself without anybody being told.
/// </para>
/// </remarks>
/// <exception cref="HttpRequestException">The forge could not be reached, or refused.</exception>
/// <exception cref="InvalidOperationException">The release is there and does not carry a usable build.</exception>
public async Task<AvailableUpdate?> CheckAsync(CancellationToken cancellationToken) public async Task<AvailableUpdate?> CheckAsync(CancellationToken cancellationToken)
{ {
// Every failure below resolves to null rather than throwing, and the caller's remark says why: an var release = await ReadAsync(ReleaseUrl(), ForgeJsonContext.Default.ForgeRelease, cancellationToken)
// unreachable forge is a phone on a train. It is not news and it heals itself in six hours. .ConfigureAwait(false);
try
if (Asset(release, $"android-{channel}.json") is not { } manifestAsset)
{ {
var release = await ReadAsync(ReleaseUrl(), ForgeJsonContext.Default.ForgeRelease, cancellationToken) throw new InvalidOperationException(
.ConfigureAwait(false); $"The {channel} release carries no android-{channel}.json, so there is nothing saying "
+ "what it publishes.");
if (Asset(release, $"android-{channel}.json") is not { } manifestAsset)
{
return null;
}
var manifest = await ReadAsync(
manifestAsset,
ForgeJsonContext.Default.AndroidChannelManifest,
cancellationToken)
.ConfigureAwait(false);
if (manifest is null || manifest.VersionCode <= InstalledVersionCode)
{
return null;
}
if (Asset(release, manifest.Apk) is not { } apk)
{
// A manifest naming an APK the release does not carry. CI uploads the package before the
// manifest precisely so this window is short, and answering null rather than throwing is
// what makes a half-published release a thing that fixes itself.
return null;
}
found = (manifest.VersionName, apk);
return new AvailableUpdate(manifest.VersionName);
} }
catch (Exception exception) when (exception is HttpRequestException
or JsonException var manifest = await ReadAsync(
or TaskCanceledException manifestAsset,
or IOException) ForgeJsonContext.Default.AndroidChannelManifest,
cancellationToken)
.ConfigureAwait(false);
if (manifest is null)
{
throw new InvalidOperationException(
$"The {channel} release's android-{channel}.json could not be read as a manifest.");
}
// The one place null is returned, and it means what null is documented to mean: this build is
// current. Compared on the version code because that is the number Android itself accepts or
// refuses an install on — see AndroidChannelManifest.
if (manifest.VersionCode <= InstalledVersionCode)
{ {
return null; return null;
} }
if (Asset(release, manifest.Apk) is not { } apk)
{
throw new InvalidOperationException(
$"The {channel} release advertises {manifest.VersionName} but carries no {manifest.Apk}.");
}
found = (manifest.VersionName, apk);
return new AvailableUpdate(manifest.VersionName);
} }
/// <inheritdoc /> /// <inheritdoc />
+23 -3
View File
@@ -58,10 +58,30 @@ public interface IUpdateChannel
/// <summary>Asks the release channel whether there is anything newer.</summary> /// <summary>Asks the release channel whether there is anything newer.</summary>
/// <returns> /// <returns>
/// The newer build, or <see langword="null"/> if this one is current. Null is also the honest answer /// The newer build, or <see langword="null"/> if this one is current. <b>Null means exactly that and
/// when the channel cannot be reached at all: the caller does the same thing either way, and an /// nothing else.</b>
/// unreachable forge is not a state a user can act on.
/// </returns> /// </returns>
/// <remarks>
/// <para>
/// ◆ <b>A channel that cannot be reached throws, and this used to say the opposite.</b> The reasoning
/// was that the caller does the same thing either way, which is true of a background pass and false of
/// a pressed button: <c>UpdateViewModel</c> swallows a failure on the timer and reports one on
/// CHECK NOW, and it can only do the second if there is something to report. Answering null for an
/// unreachable forge turned every failure into "you are on the latest build".
/// </para>
/// <para>
/// That was not merely a bad answer, it was the wrong one about a real outage: the release repository
/// was private, so every check on every phone got a 404 and every check said the build was current.
/// Nobody could see it, which is the whole argument for this direction — the caller decides what
/// silence is worth, and it cannot decide about a failure it was never told about.
/// </para>
/// <para>
/// <b>The feed must be readable without credentials, and that is a precondition rather than an
/// implementation detail.</b> A token cannot live anywhere useful here: it would be needed before
/// unlock, so it cannot be in the encrypted cache, and putting it in plaintext beside the cache is
/// exactly what <c>ClientSettings</c> refuses. See ADR 0013 and ADR 0014.
/// </para>
/// </remarks>
Task<AvailableUpdate?> CheckAsync(CancellationToken cancellationToken); Task<AvailableUpdate?> CheckAsync(CancellationToken cancellationToken);
/// <summary>Fetches an update a previous <see cref="CheckAsync"/> found.</summary> /// <summary>Fetches an update a previous <see cref="CheckAsync"/> found.</summary>