From 6728a0a5971aa855ed21d2e3d3b292e2b97b6fc4 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Tue, 4 Aug 2026 17:04:41 +0200 Subject: [PATCH] Let the desktop client replace itself, and give the repository one version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Packaging for Windows, and the updater that only exists once something is packaged. Velopack, win-x64, fed from the project's own forge — never from the deployment a client signs in to, which is ADR 0011 rule 2 carried over unchanged and is why the feed address is a constant in the code rather than a setting. See docs/adr/0012-desktop-distribution-and-updates.md. **Nothing is ever installed while somebody is using it.** A newer build is found on a six-hourly pass, downloaded in the background, and then waits — for a restart the user presses, or for the next launch they were going to do anyway. That is a policy rather than caution: this application argues at length that locking keeps shells running, because a lock that destroyed work would stop being used, and a restart does not keep them. Having taught that, it owes the user the choice at the one moment it stops being true, and the sentence saying so counts the shells it would close. **The version is now derived from the v* tag**, by MinVer, for everything. There was no version before this — no property anywhere, so every assembly reported the SDK's 1.0.0 and the API served that string as its serverVersion to every client that asked. The tag was already the version of record for the container image; this makes it the version of record full stop. MinVer's failure mode is answering plausibly rather than failing, and here a wrong version is a client that never updates, so it is guarded twice: fetch-depth 0 on every checkout, and a step that fails a tag build when the tag and the computed version disagree. **The pack id is DodoSSH.Desktop and not DodoSSH**, which is the one decision here that would have destroyed data. Velopack installs to %LOCALAPPDATA%\ and removes that whole directory on uninstall, and %LOCALAPPDATA%\DodoSSH is where ClientPaths keeps the encrypted cache, the outbox of changes not yet pushed, and the device key. The obvious id would have had the uninstaller silently delete work the server has never seen — the thing the application refuses to do without a counted confirmation. Velopack's own advice to move user data to roaming %APPDATA% is declined for the reason ClientPaths already gives. **Releases are cut by a person, and CI gains no job that could.** The tempting argument is that a forge write token is not a signing key. It does not survive contact with what the token does: Velopack clients trust their feed and do not verify a package signature when they apply one, so whoever can write a release can ship an update every install runs. That is the capability ADR 0011 rule 1 puts on a machine which is not a runner, reached through a different door. The mechanical objection — vpk needs Windows and the runners are Linux — is the smaller of the two and is recorded beside it, because somebody will fix one and believe they are done. Unsigned for now, deliberately and with the cost stated where a user reads it: SmartScreen warns once per person, on Setup.exe, because Mark-of-the-Web is applied by the browser that downloaded it. In-app updates are fetched by the application and applied from a local file, and never trip it. The banner is a fourth row of the window rather than an overlay. Anything drawn in the terminal's rectangle is sliced by the native child window that composites above it — the defect this window has shipped once — and a sibling row is the arrangement TitleBar and StatusBar already prove works. ---- Three defects surfaced on the way, none of them in the feature being built. **A settings key absent from the file came back as the CLR default, not the declared one.** The JSON source generator builds a record through a synthesised parameterised constructor and assigns every property from its argument array, so a property initializer runs and is then overwritten by a default for anything the file did not contain. A settings.json of {} read back a font size of 0, clamped up to the 8px floor rather than the 13px the renderer draws at. It could not bite while there was one setting, because that setting was written on every save and so was never absent; adding a second would have turned automatic update checks off for every existing profile, silently, the opposite of the documented default. Reflection-based deserialisation of the same JSON answers correctly, which is why every way of checking it by hand agrees except the one that ships. The defaults now live on the constructor parameters, which is the only place the generator reads them from. **Declaring a RuntimeIdentifier on the desktop head broke the server's image build.** It is the obvious way to let a self-contained publish restore under locked mode, and it writes a net10.0/win-x64 target into the lock file of every project the head references transitively — including DodoSSH.Contracts and DodoSSH.Crypto, which the API builds too. The Dockerfile restores those with no RID and fails NU1004. Found by running docker build rather than by reading. The RID stays out of the committed state; the two commands that need one ask for it unlocked, and the release script puts the lock files back. **A Docker ARG named VERSION silently sets MSBuild's Version.** An ARG is an environment variable for the rest of the stage, MSBuild reads environment variables as properties, and property names are case-insensitive. With the workflow passing main- on a main build the publish died with NETSDK1018 pointing at DodoSSH.Contracts, a project nobody had touched. The build stage's argument is ASSEMBLY_VERSION now, empty except on a tag build. All three are in docs/platform-flags.md, which is where the next person will look. ---- Verified: the whole solution builds and restores locked; 289 shell, 93 layout and 54 session tests pass, including the regression test for the settings defect and a measurement of the banner at the window's minimum width. vpk pack runs end to end and reports "Verified VelopackApp.Run()" against Program.Main. The API image builds correctly both as a main build and as a tag build, carrying 1.0.0 and 0.1.0 respectively. Not verified, and it needs a published release to be: installing, updating and uninstalling on a real machine. That is Phase 15 of docs/manual-checks.md, and the pack id and the WebView2 profile fix are reasoned and commented but only proved by walking it. Two things to watch at the first upload — the reverse proxy's body-size limit for a 64 MB asset, and whether vpk upload gitea is happy with Gitea 1.27.1. --- .config/dotnet-tools.json | 7 + .github/workflows/ci.yml | 114 ++++- Directory.Build.props | 17 + Directory.Packages.props | 34 ++ README.md | 54 +++ .../0012-desktop-distribution-and-updates.md | 246 +++++++++++ docs/manual-checks.md | 124 ++++++ docs/platform-flags.md | 88 +++- scripts/release-windows.ps1 | 282 ++++++++++++ src/DodoSSH.Api/Dockerfile | 27 ++ src/DodoSSH.Api/packages.lock.json | 6 + .../DodoSSH.Client.Android.csproj | 33 +- src/DodoSSH.Client.Api/packages.lock.json | 6 + src/DodoSSH.Client.App/App.axaml.cs | 37 +- .../DodoSSH.Client.App.csproj | 64 +++ .../Platform/VelopackUpdateChannel.cs | 194 +++++++++ src/DodoSSH.Client.App/Program.cs | 73 +++- src/DodoSSH.Client.App/Views/MainWindow.axaml | 21 +- .../Views/PreferencesScreen.axaml | 75 +++- .../Views/UpdateBanner.axaml | 82 ++++ .../Views/UpdateBanner.axaml.cs | 8 + src/DodoSSH.Client.App/app.manifest | 12 + src/DodoSSH.Client.App/packages.lock.json | 12 + src/DodoSSH.Client.Auth/packages.lock.json | 6 + src/DodoSSH.Client.Domain/packages.lock.json | 6 + src/DodoSSH.Client.Import/packages.lock.json | 6 + .../packages.lock.json | 6 + src/DodoSSH.Client.Session/ClientSettings.cs | 62 ++- src/DodoSSH.Client.Session/ClientUpdates.cs | 166 +++++++ src/DodoSSH.Client.Session/packages.lock.json | 6 + .../DodoSSH.Client.Shell.csproj | 8 +- .../ViewModels/MainWindowViewModel.cs | 71 ++- .../ViewModels/UpdateViewModel.cs | 412 ++++++++++++++++++ src/DodoSSH.Client.Shell/packages.lock.json | 6 + src/DodoSSH.Client.Ssh/packages.lock.json | 6 + src/DodoSSH.Client.Storage/packages.lock.json | 6 + src/DodoSSH.Client.Sync/packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + src/DodoSSH.Contracts/packages.lock.json | 6 + src/DodoSSH.Crypto/packages.lock.json | 6 + src/DodoSSH.Domain/packages.lock.json | 6 + src/DodoSSH.Infrastructure/packages.lock.json | 6 + tests/DodoSSH.Api.Tests/packages.lock.json | 6 + .../packages.lock.json | 6 + .../LayoutHarness.cs | 12 + .../UpdateBannerTests.cs | 145 ++++++ .../packages.lock.json | 33 +- .../FakeUpdateChannel.cs | 93 ++++ .../UpdateFlowTests.cs | 391 +++++++++++++++++ .../packages.lock.json | 33 +- .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + .../packages.lock.json | 6 + tests/DodoSSH.Crypto.Tests/packages.lock.json | 6 + tests/DodoSSH.Domain.Tests/packages.lock.json | 6 + .../packages.lock.json | 6 + tests/DodoSSH.SystemTests/packages.lock.json | 6 + 66 files changed, 3190 insertions(+), 44 deletions(-) create mode 100644 docs/adr/0012-desktop-distribution-and-updates.md create mode 100644 scripts/release-windows.ps1 create mode 100644 src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs create mode 100644 src/DodoSSH.Client.App/Views/UpdateBanner.axaml create mode 100644 src/DodoSSH.Client.App/Views/UpdateBanner.axaml.cs create mode 100644 src/DodoSSH.Client.Session/ClientUpdates.cs create mode 100644 src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs create mode 100644 tests/DodoSSH.Client.App.Layout.Tests/UpdateBannerTests.cs create mode 100644 tests/DodoSSH.Client.App.Tests/FakeUpdateChannel.cs create mode 100644 tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index f3cc8ba..f6dc1ff 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -8,6 +8,13 @@ "dotnet-ef" ], "rollForward": false + }, + "vpk": { + "version": "1.2.0", + "commands": [ + "vpk" + ], + "rollForward": true } } } diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25e1d47..5509fbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,7 +91,17 @@ jobs: "give the runner an image with node 20 or newer if actions misbehave." fi + # fetch-depth 0, and it is load-bearing rather than tidy. MinVer derives the version from the + # nearest v* tag, and checkout's default shallow clone has no tags at all — so it would not fail, + # it would quietly answer 0.0.0-alpha.0.N and every build would ship that. Velopack decides + # whether an installed client is out of date by comparing versions, which makes a plausible wrong + # answer here a client that never updates. + # + # Repeated in all three jobs, like the node preamble above and for the same reason. Change one + # copy, change all three. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 - uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 with: @@ -104,6 +114,31 @@ jobs: - name: restore run: dotnet restore DodoSSH.slnx --locked-mode + # The version comes from the tag, so on a tag build there are two ways to say the same number + # and they can disagree — a moved tag, a tag on the wrong commit, or a checkout that somehow + # still lost its history. What makes that worth a step of its own is that the disagreement is + # silent everywhere else: MinVer answers 0.0.0-alpha.0.N rather than failing, the build goes + # green, the package is cut, and the symptom arrives weeks later as clients that never update. + # + # -getProperty evaluates without building, so this costs a second and runs before the build. + - name: the tag and the version agree + if: startsWith(github.ref, 'refs/tags/v') + run: | + set -euo pipefail + tag="${GITHUB_REF#refs/tags/v}" + declared="$(dotnet msbuild src/DodoSSH.Client.App/DodoSSH.Client.App.csproj \ + -getProperty:Version -nologo | tr -d '[:space:]')" + + if [ "$tag" != "$declared" ]; then + echo "The tag says v$tag and MinVer computed $declared." >&2 + echo >&2 + echo "These come from the same place, so a mismatch means the checkout did not see the" >&2 + echo "tag it is building — most likely fetch-depth, which must stay 0 in every job here." >&2 + exit 1 + fi + + echo "v$declared" + # No `dotnet format --verify-no-changes` step. It re-analysed the whole solution before # the build did, for minutes, to check something the build already checks: IDE0055 is an # error in .editorconfig and TreatWarningsAsErrors is on, so a misformatted file fails @@ -150,6 +185,34 @@ jobs: - name: test run: dotnet test DodoSSH.slnx --no-build --configuration Release + # The one build shape nothing else here exercises: a self-contained RID-specific publish. Its + # failure mode is a restore graph or a native asset that resolves for net10.0 and not for + # net10.0/win-x64, which nobody would see until a person was halfway through cutting a release + # on a Windows machine. vpk can pack a Windows package from Linux; only signing needs Windows, + # and this repository signs nothing yet, so proving the publish here is worth the minutes. + # + # RestoreLockedMode=false for this command only, and it is not a loosened gate. The committed + # lock files are deliberately RID-free: declaring win-x64 on the desktop head writes a + # net10.0/win-x64 target into every project it references transitively, which includes + # DodoSSH.Contracts and DodoSSH.Crypto — and the API's Dockerfile restores those with no RID + # under locked mode, so the image job would fail NU1004. The gate is the locked solution + # restore at the top of this job, which is unchanged. + # + # It rewrites the lock files as it goes; nothing after this step reads them, and the runner's + # checkout is thrown away. The release script does the same thing and puts them back, because + # there the tree is somebody's working copy. + # + # After the tests rather than before them, so a red suite does not first spend a hundred + # megabytes pulling a win-x64 runtime pack. main and tags only, for the same reason: a break + # found by the person about to release is found early enough. + - name: the windows publish still resolves + if: github.event_name != 'pull_request' + run: > + dotnet publish src/DodoSSH.Client.App/DodoSSH.Client.App.csproj + --configuration Release --runtime win-x64 --self-contained true + -p:RestoreLockedMode=false + --output "$RUNNER_TEMP/win-x64-check" + # This includes the end-to-end suite, which starts PostgreSQL, Keycloak and an OpenSSH # server through Testcontainers and runs the API as a child process — so it needs a # Docker daemon and gets one here. That is why the tests run on ubuntu rather than @@ -241,7 +304,17 @@ jobs: "give the runner an image with node 20 or newer if actions misbehave." fi + # fetch-depth 0, and it is load-bearing rather than tidy. MinVer derives the version from the + # nearest v* tag, and checkout's default shallow clone has no tags at all — so it would not fail, + # it would quietly answer 0.0.0-alpha.0.N and every build would ship that. Velopack decides + # whether an installed client is out of date by comparing versions, which makes a plausible wrong + # answer here a client that never updates. + # + # Repeated in all three jobs, like the node preamble above and for the same reason. Change one + # copy, change all three. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 - uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 with: @@ -281,7 +354,7 @@ jobs: # resolves for net10.0 but has nothing to dex. Neither shows up in a compile. # # Debug-signed on purpose, and it has to stay that way: no keystore secret, no AndroidKeyStore=true. - # docs/adr/0010-android-distribution.md puts the release key on a machine that is not a runner, + # docs/adr/0011-android-distribution.md puts the release key on a machine that is not a runner, # because a signing key reachable from a workflow is a key held by everyone who can change one. # This APK is a build check. It is not something anybody installs. - name: package @@ -343,7 +416,17 @@ jobs: "give the runner an image with node 20 or newer if actions misbehave." fi + # fetch-depth 0, and it is load-bearing rather than tidy. MinVer derives the version from the + # nearest v* tag, and checkout's default shallow clone has no tags at all — so it would not fail, + # it would quietly answer 0.0.0-alpha.0.N and every build would ship that. Velopack decides + # whether an installed client is out of date by comparing versions, which makes a plausible wrong + # answer here a client that never updates. + # + # Repeated in all three jobs, like the node preamble above and for the same reason. Change one + # copy, change all three. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 # The daemon and the client are two different things to have, and this runner had only # one of them. Testcontainers reaches Docker straight over /var/run/docker.sock from a @@ -449,8 +532,18 @@ jobs: ;; esac + # The version MSBuild is allowed to see, which is not the same string as the one above. + # `version` is a docker tag and is `main-` on a main build; handing that to + # -p:Version fails the publish with NETSDK1018. So this is set only when it is a real + # version, and the Dockerfile leaves the SDK default alone when it is empty. + assembly_version="" + case "$GITHUB_REF" in + refs/tags/v*) assembly_version="${GITHUB_REF#refs/tags/v}" ;; + esac + echo "tags=$tags" >> "$GITHUB_OUTPUT" echo "version=$version" >> "$GITHUB_OUTPUT" + echo "assemblyVersion=$assembly_version" >> "$GITHUB_OUTPUT" echo "created=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$GITHUB_OUTPUT" echo "Tagging: $tags" @@ -462,6 +555,7 @@ jobs: env: TAGS: ${{ steps.tags.outputs.tags }} VERSION: ${{ steps.tags.outputs.version }} + ASSEMBLY_VERSION: ${{ steps.tags.outputs.assemblyVersion }} REVISION: ${{ github.sha }} CREATED: ${{ steps.tags.outputs.created }} run: | @@ -477,6 +571,7 @@ jobs: --pull \ --file src/DodoSSH.Api/Dockerfile \ --build-arg "VERSION=$VERSION" \ + --build-arg "ASSEMBLY_VERSION=$ASSEMBLY_VERSION" \ --build-arg "REVISION=$REVISION" \ --build-arg "CREATED=$CREATED" \ "${args[@]}" \ @@ -547,3 +642,20 @@ jobs: - name: log out if: always() && github.event_name != 'pull_request' run: docker logout registry-docker.dodotech.cloud + +# There is no job here that publishes the desktop client, and there is not going to be one. Two +# independent reasons, and both need saying because someone will fix one and think they are done. +# +# The smaller one is mechanical: vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows +# tooling, and every job in this file is runs-on: [linux]. A Windows runner would answer that. +# +# The larger one is that a Windows runner would not answer the other. Velopack clients fetch from the +# release feed and do not verify a package signature when they apply it, so whoever can write a release +# on this repository can publish an update that every installed client downloads and runs. That is the +# same capability as the signing key, reached through a different door — and docs/adr/0011 rule 1 puts +# that capability on a machine which is not a runner, because a workflow secret is held by everyone who +# can change a workflow file. See docs/adr/0012-desktop-distribution-and-updates.md. +# +# What cuts a release is scripts/release-windows.ps1, run by a person. What this file does is prove the +# thing still builds and packages, which is the same division of labour the android job above already +# has: it packages an APK nobody installs, so that a link-time break fails here rather than later. diff --git a/Directory.Build.props b/Directory.Build.props index 9300cc3..2a7309e 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -33,6 +33,15 @@ true + + + v + + DodoTech DodoSSH @@ -51,6 +60,14 @@ + + + + + diff --git a/Directory.Packages.props b/Directory.Packages.props index 440de5f..3c0d23d 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -145,6 +145,40 @@ terminal data plane — is Pipelines and channel code rather than view models. --> + + + + + + diff --git a/README.md b/README.md index bac43c1..924da91 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,7 @@ docs/design-import-gaps.md what the client's design asked for and this build has docs/platform-flags.md what differs off Windows, and the gotchas that have cost time docs/manual-checks.md what no test can reach, and what to look for when checking by hand docs/android-port.md the Android head: what was decided, what is built, what is left +scripts/ release-windows.ps1 — builds, packs and publishes the Windows client ``` Everything under `src/DodoSSH.Client.*` except the two heads and `Shell` is deliberately free of Avalonia. @@ -107,6 +108,40 @@ The tests need a Docker daemon. Everything that touches the database, the identi server uses Testcontainers rather than a stub or a shared instance, so there is nothing to start first and nothing to clean up after — but with no daemon those suites fail rather than skip. +## Installing on Windows + +The desktop client is published on the project's own release page as a `Setup.exe`. It installs per user, +under `%LOCALAPPDATA%\DodoSSH.Desktop`, and never asks for an administrator. + +**It will warn you, and here is exactly what the warning means.** The build is not yet signed with a code +signing certificate, so Windows SmartScreen shows *"Windows protected your PC"* the first time you run the +installer; **More info → Run anyway** gets past it. That is the honest state of things rather than something +to click through blindly — it is a statement that Microsoft has not seen this file before, and it will stop +appearing when the project buys a certificate. +[ADR 0012](docs/adr/0012-desktop-distribution-and-updates.md) says what that costs and when it happens. The +warning is once per person: updates from inside the application do not raise it. + +**Updates.** The client checks the project's release page every six hours, downloads a newer build in the +background, and then waits. Nothing is ever installed while you are using it — a downloaded update runs +after a restart you ask for, or the next time you start DodoSSH anyway. Restarting does end every shell you +have open, which locking deliberately does not, so the choice of when is left to you. You can turn the +checking off on PREFERENCES → UPDATES. + +**Where it comes from matters, and it is worth one paragraph.** A DodoSSH server will never offer you the +client, and one that does is not one to trust. Whoever hands you the binary can hand you a binary that +copies your passphrase — the client is where your credentials are in plaintext, by construction — and the +operator of a deployment is precisely the party the trust model is about. An operator may tell you where to +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. + +**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 +you want the machine to genuinely forget everything — an uninstall is not a sign-out, and does not withdraw +this machine's device key from your account. + +Cutting a release is `scripts/release-windows.ps1`, run by a person on a Windows machine. Deliberately not a +CI job; ADR 0012 decision 3 explains why, and it is not only that the runners are Linux. + ## Running it Three commands, in order. The first is once per machine. @@ -515,6 +550,11 @@ keychain plus a terminal — and the spike that gates all of it. ### Conventions the build enforces +- One version for the whole repository, derived from the nearest `v*` tag by MinVer. A tag build whose + computed version disagrees with the tag fails CI, and every checkout uses `fetch-depth: 0` — without it + MinVer answers `0.0.0-alpha.0.N` rather than failing, and a wrong version here is a client that never + updates. + - Warnings are errors, formatting included: `IDE0055` is an error in `.editorconfig`, so a misformatted file fails the build itself rather than a separate CI step. - Package versions are centralised in `Directory.Packages.props`; `packages.lock.json` is @@ -659,6 +699,20 @@ keychain plus a terminal — and the spike that gates all of it. serving the client binary, which hands it to the one party the whole trust model is about. An installed Android app can only ever be updated by a package signed with the same key, so this is the first release's decision to make and nobody else's afterwards. + + **The Windows desktop half is built.** Velopack packaging for `win-x64`, a `Setup.exe` that installs per + user with no administrator prompt, and a client that checks the project's own forge every six hours, + fetches a newer build in the background, and then waits for a restart the user presses — because a + restart ends every shell, and this application has gone to some trouble to make locking not do that. The + version is now one number for the whole repository, derived from the `v*` tag by MinVer, which is also + the first time the API has reported a true `serverVersion`. Releases are cut by a person rather than by + CI: the token that writes a release is, for an updater that trusts its feed, the same capability as the + signing key, which [ADR 0011](docs/adr/0011-android-distribution.md) rule 1 keeps off runners. See + [ADR 0012](docs/adr/0012-desktop-distribution-and-updates.md), and + [Installing on Windows](#installing-on-windows) for what a user sees. + + Still to do here: signing (the first release is unsigned, and the trigger for buying a certificate is the + first release aimed at strangers), and macOS and Linux packaging. - **M5 — multi-provider OIDC**, identity key rotation, per-item content keys. ## Licence diff --git a/docs/adr/0012-desktop-distribution-and-updates.md b/docs/adr/0012-desktop-distribution-and-updates.md new file mode 100644 index 0000000..1fc6a24 --- /dev/null +++ b/docs/adr/0012-desktop-distribution-and-updates.md @@ -0,0 +1,246 @@ +# ADR 0012 — Distributing the desktop client, and letting it replace itself + +- Status: accepted +- Date: 2026-08-04 +- Builds on: [ADR 0001](0001-e2ee-trust-model.md), [ADR 0011](0011-android-distribution.md) +- Settles: the desktop half of M4 packaging, which [ADR 0011](0011-android-distribution.md#consequences) + left open + +## Context + +[ADR 0011](0011-android-distribution.md) settled the phone and said explicitly what it was not settling: +"M4's desktop packaging inherits rule 2 and not the rest." This is that inheritance, plus the thing the +phone deliberately does not have. + +The desktop head was not packaged at all. There were no tags, no `Version` property anywhere in the +repository, and therefore no version: every assembly reported the SDK's default `1.0.0`, and the API had +been serving that string to every client that asked it for `serverVersion`. There was no installer and no +way for anybody holding a copy to learn that a newer one existed. + +That is tolerable while nobody has it. It stops being tolerable at the first release, and it stops in a +particular direction: this client holds the plaintext of a team's infrastructure credentials, by +construction (ADR 0001). For software like that, "the fix shipped and the user never got it" is a security +outcome. ADR 0011 named the cost of shipping outside a store — "no discovery, no automatic update" — and +accepted it for Android because the platform left no better option. The desktop leaves a better option, and +this ADR takes it. + +The new thing an updater introduces is a capability that did not exist before: **something that can replace +the binary on a user's machine, without the user choosing each build.** ADR 0001 models the party who wants +the secrets as attacking the client rather than the crypto, and ADR 0011 turns that into a question of +custody — *which parties can ship one person a build?* An updater is a second answer to that question, so +it needs the same treatment as the signing key, and it is easy to get wrong because the dangerous part does +not look like a key. + +## Decision + +### 1. Velopack, per-user, under `%LOCALAPPDATA%`, with no administrator prompt + +`docs/platform-flags.md` had already ruled out the platform-native option, and it is worth restating +because it is the kind of decision that gets reopened: **MSIX is not deprioritised, it is impossible +here.** A packaged application runs WebView2 in an AppContainer where loopback connections are blocked +without a `CheckNetIsolation` exemption, and the terminal data plane *is* a loopback WebSocket. MSIX would +not degrade the product, it would remove the terminal. + +Velopack's path was checked against that and reintroduces nothing: `Setup.exe` is an ordinary Win32 +executable that unpacks a directory and creates shortcuts, there is no package manifest and no package +identity, and the process therefore stays an ordinary desktop process. Manual check 15.4 is what would +notice if that ever changed, because it connects a real shell from the installed build. + +### 2. The pack id is `DodoSSH.Desktop`, and it is irreversible + +Velopack installs to `%LOCALAPPDATA%\` and **removes that entire directory on uninstall**. +`ClientPaths.DataDirectory` is `%LOCALAPPDATA%\DodoSSH`, and it holds `cache.db` with its `-wal` and `-shm` +companions, `settings.json`, and `device.key`. + +So the obvious pack id would have installed the application on top of the user's encrypted cache, and +uninstalling would have deleted the device key and the outbox — the changes this machine has made and not +yet pushed, which `MainWindowViewModel.SignOutWarning` already describes to users as existing "nowhere else +in the world". The application refuses to delete that without a counted confirmation; an uninstaller would +have done it silently. + +Velopack's own guidance is to keep persistent files in roaming `%APPDATA%`. **That guidance is declined**, +and `ClientPaths` already explains why: two machines writing one SQLite file through a file-sync client +corrupts it, and the outbox is per-machine by design. The install moves; the profile does not. + +Like the Android package id, this is a one-way door — it is the identity an installed client matches an +update against, so changing it later orphans every existing install: still running, never updated, and +invisible to the new one. + +### 3. The release is cut by a person, and the forge token never goes near CI + +This is the paragraph that will be argued with, so it is the one written most carefully. + +The tempting argument is that a Gitea write token is not a signing key, and so — unlike the Android +keystore ADR 0011 rule 1 keeps off runners — it could live in a CI secret and let a tag cut a release. + +**It does not survive contact with what the token can do.** Velopack clients fetch from the configured +source over TLS and do not verify a package signature when they apply it. So anyone who can write a release +on this repository can publish an update that every installed client downloads and runs. That is precisely +the capability ADR 0011 rule 1 places on a machine which is not a runner, reached through a different door — +and a workflow secret is held by everyone who can change a workflow file, which for a repository with any +contributors is a wider set than it looks. + +So: `scripts/release-windows.ps1`, run by a person on a Windows machine, in two phases. Phase one builds and +packs and stops. Phase two, a separate invocation, asks for the token and uploads. The split exists so that +what reaches users has been installed and started by a human first, and so that the credential is in memory +only for the minutes that need it. + +There is a second, smaller reason the release could not be a CI job here anyway: `vpk` stamps and embeds +the `Setup.exe` and `Update.exe` stubs with Windows tooling, and every job in `ci.yml` is +`runs-on: [linux]`. Both reasons are recorded because somebody will fix one and believe they are done. + +What CI does gain is the same thing the `android` job already does — it proves the artefact still builds. +A `win-x64` publish runs on main and on tags, so a restore graph that resolves for `net10.0` and not for +`net10.0/win-x64` fails there rather than under a person midway through a release. + +### 4. The update check points at the project's forge, and the address is a constant + +ADR 0011 rule 2 carries over unchanged, and its update clause carries with it: "If an update check is ever +added it points at the project's domain." Here that is `git.dodotech.cloud`, and it is a `const` in +`VelopackUpdateChannel` rather than a setting. + +**The constant is the mechanism, not a convention.** A configurable feed address is exactly the knob that +would let an operator — or an edit to a plaintext `settings.json` — point the update path at the +deployment, and an operator who can answer "is there a newer version" can answer "no" forever, pinning a +chosen user to a build with a known hole while holding no key at all. Making it unsettable is that rule +expressed in a way nobody has to remember. + +`ClientSettings` therefore stores whether to check, and nothing else: no address, no channel, no token. A +private release repository is incompatible with this design and that is worth knowing rather than +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. + +### 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 +by nothing. Hanging the updater off it would have been natural and would have been wrong — and note the +shape of the error, because it is not "inherits an existing risk". Today the field controls nothing at all. +The moment an update check is conditioned on it, rule 2's sentence applies verbatim and the risk is +*created*. + +The split to preserve: + +- A deployment may say **"I will not serve a client this old"** and draw a remediation screen. That is the + deployment describing itself, which is legitimate and is what the field's own documentation asks for. + That screen may carry a sentence and a link to the project's release page. +- It may **not** carry a button that triggers a check or a download. A link is the user going somewhere; a + fetch is the operator's answer steering this process. + +Under the design as built, an operator withholding or deflating the value achieves nothing — the check runs +on its own timer and never consults the deployment. Inflating it denies service to their own users, which +they can already do by turning the server off. + +### 6. Self-contained, and not single-file + +Self-contained because .NET 10 is recent enough that almost no machine has the runtime, and because the +usual objection — that runtime security patches then require an application update — is answered by the +feature this ADR is about. Velopack's deltas are per-file and the runtime files do not change between our +releases, so the runtime costs almost nothing per update; it is paid on first install. + +Not single-file, for four independent reasons: `platform-flags.md` records that libsodium ships native +binaries per RID and complicates single-file publishing, and `libe_sqlite3`, `libSkiaSharp` and +`libHarfBuzzSharp` do the same; a bundle changes wholly on every build, so deltas stop working; Velopack is +a directory-based updater by design; and a self-extracting bundle puts the executable under a temp path +deep enough to hit the WebView2 long-path failure that same document records. Not trimmed — EF Core is not +trim-safe and `TreatWarningsAsErrors` turns every `IL2xxx` into a build break, so trimming is a project +rather than a flag. + +**Native symbol files are excluded and ours are kept**, which is worth recording as a decision because the +numbers are so lopsided: `libSkiaSharp.pdb` and `libHarfBuzzSharp.pdb` are 100 MB of debug symbols for +third-party native code nobody here will step through, and all fifteen of our own managed PDBs together are +0.93 MB. Dropping the two took a publish from 227 MB to 127 MB. Keeping ours means an +`Exception.ToString()` carries file names and line numbers, which for a self-hosted product is the whole +diagnostic channel — the way a fault gets reported is a user pasting a stack into an issue. + +### 7. One version, derived from the tag + +MinVer, with `MinVerTagPrefix` of `v`, matching the tags CI already triggers on. The tag was already the +version of record for the API's container image; this makes it the version of record for everything, +including the client's own `AssemblyInformationalVersion`, which is what the preferences screen prints and +what Velopack compares. + +MinVer's one sharp edge is that it answers plausibly rather than failing: a shallow clone with no tags +yields `0.0.0-alpha.0.N`. Here a wrong version is a client that never updates, so it is guarded twice — +`fetch-depth: 0` on every checkout, and a step that fails a tag build when the computed version and the tag +disagree. + +The Windows application manifest's `assemblyIdentity` version is **deliberately** left at `1.0.0.0`. It is a +side-by-side activation field this application does not use and nothing reads; what a person sees comes from +the PE version resource, which MSBuild fills from `FileVersion`. + +### 8. Unsigned for now, with a named trigger + +Every installer will raise SmartScreen's "Windows protected your PC" until reputation accrues. The cost is +smaller and more precisely bounded than the reflex suggests, and the bound is worth knowing: Mark-of-the-Web +is applied by the *browser* that downloads `Setup.exe`, so the warning lands at first install only. In-app +updates are fetched by the application's own HTTP client and applied by `Update.exe` from a local file, and +never trip it. **One dialog per user per lifetime, not one per update.** + +The trigger for buying an OV certificate on a hardware token is the first release the README invites a +stranger to install. ADR 0011 already describes Authenticode as the easy custody case — the key stays with +the developer — and the post-2023 requirement that it live on FIPS 140-2 Level 2 hardware enforces "offline, +never in CI" physically rather than by policy. `--signParams` is the single line in the release script that +changes. + +**Azure Trusted Signing is not the quiet default and needs its own ADR.** It is cheaper and has no hardware +token, and it puts a compellable third party in the signing path — which is ADR 0011 rule 3's shape one +layer down, declined there for reasons that do not stop applying because the vendor changed. + +## Consequences + +**The desktop gets what ADR 0011 had to refuse the phone.** Discovery is still manual — somebody has to be +told where the release page is — but from the first install onward the client keeps itself current without +anybody deciding to go and look. That is the sharpest edge of the Android decision, blunted on the platform +where it could be. + +**Nothing is ever installed while the application is running.** A fetched update waits for a restart the +user presses, or for the next launch they were going to do anyway. This is not caution for its own sake: +this application deliberately keeps shells running across a lock, and argues in `LockAsync` that a lock +which destroyed work would simply stop being used. A restart does end every shell, so it is a decision that +belongs to the person whose job is running in one — and the interface says so, in the same words the close +button already uses. + +**The release is a manual step, and that will feel slow.** It is roughly ten minutes of a person's attention +per release, and the convenience being refused is the entire point of CI. Where the two collide the custody +argument wins, exactly as it does for the Android keystore. + +**The forge grows by about 125 MB per release, forever.** Measured on the first real pack rather than +estimated: a 127 MB publish directory compresses to a 60 MB full `.nupkg` and a 64 MB `Setup.exe`, and both +are uploaded. Velopack needs the previous full package present to build a delta against, so pruning old +releases has a real cost — a client several versions behind falls back to a full download. A 64 MB asset is +also large enough to meet whatever body-size limit fronts `git.dodotech.cloud` (nginx's +`client_max_body_size` defaults to 1 MB), which is worth checking before the first upload rather than +discovering after ten minutes of transfer. + +**Uninstalling leaves the vault cache behind, on purpose.** `%LOCALAPPDATA%\DodoSSH` survives, so a +reinstall finds an enrolled machine and asks for the passphrase — which is what somebody reinstalling wants. +The honest cost is that `device.key` is left too, and the account goes on listing a device that is no longer +installed. That is a wart rather than a hole: the blob is decryptable only by this machine's TPM, and Sign +Out is the button that withdraws the registration. An uninstall is not a sign-out and must not pretend to +be one. + +**Contracts and client now share a version number.** `DodoSSH.Contracts` is packable, so a contracts-only +change ships as a client version bump. One number is the point; this is the price of it. + +**The Android head's `versionName` now follows the same tag**, while its integer `versionCode` stays +hand-bumped, because Android needs a monotonic integer and SemVer does not provide one. Nothing else about +ADR 0011 changes. + +## Rejected + +- **MSIX.** Not a preference. It would break the terminal outright; see decision 1. +- **A CI job that cuts the release on a tag.** The convenience is the point of CI and the custody is the + point of ADR 0011; where they collide the custody wins. See decision 3, and note that a Windows runner + answers the mechanical objection while leaving the real one untouched. +- **A configurable update feed.** Named here because it is the change somebody will propose in good faith, + for a self-hosted product where making things configurable is usually right. It is the one setting that + would hand the operator the capability the whole trust model is about. +- **The deployment serving the installer.** Refused again, for the third ADR running, because it is + genuinely the nicest onboarding available and will be proposed again. +- **Conditioning updates on `MinClientVersion`.** See decision 5. It would create the attack rather than + inherit it. +- **Azure Trusted Signing**, for now, on custody grounds rather than on mechanics. See decision 8. +- **A hand-edited version property.** Considered seriously against MinVer, and it loses on the thing that + matters here: the tag is already the version of record for the container image, and two places to write + one number is a pair that can disagree. MinVer's silent-wrong-answer failure mode is real and is what the + two CI guards in decision 7 exist for. diff --git a/docs/manual-checks.md b/docs/manual-checks.md index a33fdd8..09deb57 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -1465,3 +1465,127 @@ delivery survives the failure because it is held against the transfer rather tha **Failure means:** a retry that succeeds but leaves the destination empty is the delivery having been dropped on the failure. An error saying the staged file is missing is the copy having been deleted at the stop, which is what `QueueDeliveredDownload` documents it does not do. + +--- + +## Phase 15 — Installing the desktop client, and being updated by it + +Nothing in this phase is reachable by a test, and not for the usual reason. There is no installed +application in CI, no `%LOCALAPPDATA%` worth inspecting, and the update path only exists across two builds +published minutes apart — so what is being checked is the shape of a release, which only exists once +somebody has cut one. What *is* automated is the machinery underneath: `UpdateFlowTests` drives every state +of the view model against a fake channel, and pins the one promise that matters +(`AReadyUpdate_IsNeverAppliedOnItsOwn`); `UpdateBannerTests` measures the banner at the window's minimum +width. Neither can install anything. + +Walk it once per release, and in order — 15.6 onwards needs 15.1 to have happened. + +Run `pwsh -File scripts/release-windows.ps1` first. It stops after packing, on purpose, so that everything +below happens before anything reaches a user. + +### 15.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%`. + +**Pass:** no UAC prompt; `%LOCALAPPDATA%\DodoSSH.Desktop\current\DodoSSH.exe` exists; a Start-menu entry +reading **DodoSSH**; and `%LOCALAPPDATA%\DodoSSH` either absent (a fresh machine) or **untouched**. + +**Failure means:** a UAC prompt is a per-machine install, which is not what was designed. Anything written +into `%LOCALAPPDATA%\DodoSSH` is the pack id having drifted back to `DodoSSH`, and that is the serious one — +the uninstaller removes its whole install root, so it would take the vault cache and the outbox with it. +See [ADR 0012](adr/0012-desktop-distribution-and-updates.md) decision 2. + +### 15.2 The installed path is short, measured rather than assumed + +```powershell +"$env:LOCALAPPDATA\DodoSSH.Desktop\current\DodoSSH.exe".Length +``` + +**Pass:** under 100. It is 64 on an ordinary profile. + +**Failure means:** folder redirection, or a very long profile path. The terminal is about to fail with +`CO_E_SERVER_EXEC_FAILURE` and name nothing — see the long-path entry in `platform-flags.md`, which is the +reason this check is a number rather than a shrug. + +### 15.3 The window opens and carries its own icon + +**Pass:** the Start-menu shortcut launches it, and the taskbar and Alt-Tab show the dodo mark rather than a +generic icon. + +**Failure means:** `--icon` or `ApplicationIcon` did not survive packaging. Cosmetic, and the first thing +anybody notices. + +### 15.4 A terminal connects from the installed build · **the one that would catch an AppContainer** + +Sign in, unlock, open a shell against a real host, and type. + +**Pass:** characters reach the remote and output comes back. + +**Failure means:** if it hangs and then reports the WebView2 message after about fifteen seconds, the +renderer never attached — check whether packaging has given the process a package identity, which would put +WebView2 in an AppContainer where the loopback data plane cannot connect. That is the failure MSIX was ruled +out for, and this is the check that would find it in the Velopack path. `RendererTimeout` is where the +fifteen seconds comes from. + +### 15.5 The version on screen is the version that was built + +Right-click `DodoSSH.exe` → Properties → Details, and open PREFERENCES → UPDATES. + +**Pass:** File version reads the tag (`0.1.0.0`), product **DodoSSH**, company **DodoTech**, and the +preferences screen prints the same number. + +**Failure means:** `0.0.0.0` is MinVer never seeing a tag — a shallow clone, or `fetch-depth` having been +dropped from a checkout. `1.0.0.0` is somebody having wired the app manifest's inert `assemblyIdentity` +version to the real one. A version on screen that differs from the file properties means the two are being +read from different places, which is the thing having one number was for. + +### 15.6 A second release produces a delta, not only a full package + +Tag `v0.1.1` and run the script again. + +**Pass:** `Releases\` holds both a `*-full.nupkg` and a `*-delta.nupkg`, and the delta is a small fraction +of the full. + +**Failure means:** no delta at all is `vpk download gitea` having found nothing to build one against — the +previous release did not come down, so every user is about to fetch a ~60 MB full package for a one-line +change. The +script warns rather than failing when that is legitimate, which is the first release only. + +### 15.7 The update arrives, and the restart lands in it · **the whole point of the work** + +With v0.1.0 installed and running, a vault unlocked, a host change made, and **a terminal open**, publish +v0.1.1 (`-Upload`). Then press CHECK NOW on PREFERENCES rather than waiting six hours. + +**Pass:** the progress bar moves, the banner appears above the status bar, and — the part to actually watch +— the terminal **reflows cleanly rather than being sliced**, with the remote seeing the smaller row count. +Press **RESTART NOW**: the application closes and reopens as 0.1.1, still enrolled, with the host change +intact. + +**Failure means:** no banner is a channel mismatch between `vpk pack --channel` and +`VelopackUpdateChannel.ReleaseChannel`, which fails silently by design — the check succeeds, finds nothing, +and reports the client up to date forever. A banner sliced at the terminal's left edge is the occlusion rule +having been broken, and the fallback is to move the offer into the titlebar instead. Coming back as 0.1.0 is +the swap having been blocked, usually by a process still holding a file under `current\`. Being asked to +enrol again means the profile directory did not survive, which is 15.1's failure arriving late. + +### 15.8 The first connect after an update is not a cold start + +Immediately after 15.7, connect to a host. + +**Pass:** the terminal appears about as quickly as it did before the update. + +**Failure means:** the WebView2 user data folder is back inside `current\` and was destroyed by the update — +see the entry in `platform-flags.md`. Slow but working, so it gets dismissed as a fluke unless somebody is +looking for it, which is why it is a numbered check rather than a note. + +### 15.9 Uninstalling removes the application and leaves the vault · **the data-loss check** + +Settings → Apps → DodoSSH → Uninstall. + +**Pass:** `%LOCALAPPDATA%\DodoSSH.Desktop` is gone, and `%LOCALAPPDATA%\DodoSSH` still holds `cache.db`, +`cache.db-wal` and `cache.db-shm` — all three, per the entry that says any routine touching only the first +is wrong. Reinstalling then asks for the passphrase rather than for a server. + +**Failure means:** the cache going with the application is the pack-id collision, and whoever ran this has +lost their offline unlock and any change that was still in the outbox. That is the failure ADR 0012 +decision 2 exists to prevent, and it is why 15.1 checks the same thing from the other end. diff --git a/docs/platform-flags.md b/docs/platform-flags.md index 56a0590..2e17cac 100644 --- a/docs/platform-flags.md +++ b/docs/platform-flags.md @@ -217,8 +217,33 @@ execution failed") and the terminal never appears. Hit while building the harnes that failed from a ~230-character directory ran first time from `%TEMP%\h`. The exact threshold was not established and the mechanism is unconfirmed — the user data folder is created beside the executable by default and the browser process is launched with paths derived from it, so `MAX_PATH` is the obvious -suspect. Relevant to packaging: an installer that lands under a deep per-user path would break the -terminal with an error that names nothing. +suspect. + +*Measured for the packaged layout, so this stops being a worry and becomes a number.* Velopack installs to +`%LOCALAPPDATA%\DodoSSH.Desktop\current\`, and `…\AppData\Local\DodoSSH.Desktop\current\DodoSSH.exe` is +**64 characters** against the ~230 that reproduced the failure — about 180 characters of headroom, and a +40-character corporate username adds 35 of them back. The shipped installer is not at risk. Two things +would reopen it and neither is in the plan: a self-extracting single-file publish, whose native libraries +land under a hashed temp path, and `%LOCALAPPDATA%` folder-redirected to a deep UNC path in a domain. +Manual check 15.2 measures it on the real machine rather than trusting this paragraph. + +**WebView2's user data folder must be kept out of the install directory.** It defaults to a directory +beside the host executable, which under Velopack is inside `current\` — and `current\` is *replaced* by +every update. Left alone, the browser profile would be destroyed on each one, so the first connect after +every update would pay a cold WebView2 start: a fresh user-data directory and a new process tree, which is +the slow path `RendererTimeout`'s fifteen seconds was sized for, arriving at the exact moment somebody is +most ready to believe the update broke the terminal. `Program.Main` sets `WEBVIEW2_USER_DATA_FOLDER` to +`%LOCALAPPDATA%\DodoSSH\WebView2` — under the profile directory, which Velopack never touches. Check 15.8 +is what would notice it regressing, and it is worth having because the symptom is "slow but working", which +gets dismissed as a fluke. + +**The install root and the profile directory must not be the same folder.** Velopack removes +`%LOCALAPPDATA%\` entirely on uninstall, and `ClientPaths` puts `cache.db` (plus `-wal` and `-shm`), +`settings.json` and `device.key` in `%LOCALAPPDATA%\DodoSSH`. So a pack id of `DodoSSH` — the obvious +choice — would have made the uninstaller delete the vault cache and the outbox of changes not yet pushed, +silently, which is the thing the application will not do without a counted confirmation. The pack id is +`DodoSSH.Desktop` for that reason and no other; `--packTitle` supplies the name people see, so nothing is +lost. Do not "tidy" it. See [ADR 0012](adr/0012-desktop-distribution-and-updates.md) and check 15.9. **The Windows app manifest must declare a `supportedOS` list.** Without it the process reports a downlevel Windows version and Avalonia's native control host fails outright — *"Unable to create child @@ -329,6 +354,15 @@ AppContainer where loopback connections are blocked without a `CheckNetIsolation terminal data plane *is* a loopback WebSocket, so MSIX would break the product outright. Velopack for Windows/macOS/AppImage; Flatpak and deb/rpm defer updates to the package manager. +*Checked rather than assumed, now that Velopack is actually wired up:* its Windows path does not +reintroduce the thing MSIX was ruled out for. `Setup.exe` is an ordinary Win32 executable that unpacks a +directory under `%LOCALAPPDATA%` and creates shortcuts — there is no `AppxManifest`, no package identity, +no `runFullTrust`, no elevation and no execution alias, so the process stays an ordinary desktop process +and WebView2 stays out of an AppContainer. That is reasoning, not measurement; manual check 15.4 is the +measurement, because if a package identity ever did appear the symptom would be the terminal hanging and +then reporting the WebView2 message after fifteen seconds, which reads like a broken runtime rather than +like packaging. + **Linux ships AppImage and Flatpak first**, specifically so the WebKit runtime is bundled rather than assumed present on the user's machine. @@ -486,6 +520,56 @@ so it cannot be discovered from Kestrel afterwards. The window for another proce few milliseconds; if the suite ever fails with an address-in-use, this is why, and a retry is the fix rather than a redesign. +**A RID must never reach the committed lock files, and the obvious fix for a RID-specific publish puts +one there.** `dotnet publish -r win-x64` resolves a graph the committed `packages.lock.json` files do not +describe — they carry a `net10.0` target and nothing else — so under locked mode it fails NU1004. The +obvious answer is `win-x64` on the desktop head plus a +`--force-evaluate` to regenerate. **That is wrong here, and it was tried and reverted.** + +A RID declared on one project flows to every project it references transitively while restoring, so the +regenerated lock files for `DodoSSH.Contracts` and `DodoSSH.Crypto` grew a `net10.0/win-x64` target as +well — and those two are built by the *server*. The API's Dockerfile restores them with no RID and +`--locked-mode`, so it failed: + +``` +error NU1004: The project's runtime identifiers have changed from. +Project's runtime identifiers: , lock file's runtime identifiers win-x64. +``` + +Packaging the desktop client had broken the server's image build, and nothing but the `image` job would +have caught it. Found by running `docker build` locally rather than by reading the lock files. + +So the RID stays out of the committed state, and the two commands that need one — the release script's +publish and the `windows publish still resolves` step in `ci.yml` — pass `-p:RestoreLockedMode=false` for +themselves alone. That restore rewrites the lock files as a side effect, which does not matter on a runner +whose checkout is discarded and does matter on a developer's machine, so the release script runs +`git checkout -- '*packages.lock.json'` afterwards. `-p:RestorePackagesWithLockFile=false` is not an +alternative: it fails NU1005 whenever a lock file already exists. + +**A Docker `ARG` named `VERSION` silently sets MSBuild's `Version`.** An `ARG` is an environment variable +for the rest of the stage, MSBuild reads environment variables as global properties, and MSBuild property +names are case-insensitive — so `ARG VERSION` in a build stage sets `Version` for every project built in +it, with no line anywhere saying so. The workflow passes `main-` on a main build, which is a +fine docker tag and not a version, and the publish died with `NETSDK1018: Invalid NuGet version string` +pointing at `DodoSSH.Contracts` — a project nobody had touched. The build stage's argument is therefore +`ASSEMBLY_VERSION`, passed empty except on a tag build; the `VERSION` arg in the final stage is only ever +an OCI label and never meets MSBuild. Renaming is the entire fix, and the reason it is written down is that +the symptom names the wrong project and the cause is invisible. + +**System.Text.Json's source generator does not honour property initializers on a record.** Defaults for a +`ClientSettings`-style record must live on the **constructor parameters**, not on property initializers, +and getting it wrong fails silently in the worst direction. The generator emits an +`ObjectWithParameterizedConstructorCreator` — it treats the init-only properties as constructor arguments +and builds `new ClientSettings() { A = (T)args[0], … }`, so the initializer runs and is then overwritten by +`args`, which for a member absent from the JSON is the CLR default. Measured: a `settings.json` of `{}` +read back `TerminalFontSize` 0 (clamped up to the 8px floor, not the 13px the renderer draws at) and, once +it existed, `AutomaticUpdateChecks` false. **Reflection-based deserialisation of the same JSON answers 13 +and true**, which is what makes it so easy to miss — every way of checking it by hand is right except the +one that ships. `JsonSourceGenerationMode.Metadata` does not help; it was tried. It stayed invisible while +there was one setting, because that setting was written on every save and so was never absent; it went live +the moment a second one was added, since every existing profile lacks the new key. +`ASettingAbsentFromTheFile_ComesBackAsItsDeclaredDefault` fails without the fix. + **`[CallerFilePath]` is rewritten to `/_/...` under `ContinuousIntegrationBuild`.** Any test that locates a fixture by source path passes locally and fails in CI. Copy fixtures to the output directory and read them via `AppContext.BaseDirectory` instead; `GoldenVectorTests` shows the diff --git a/scripts/release-windows.ps1 b/scripts/release-windows.ps1 new file mode 100644 index 0000000..dda0d95 --- /dev/null +++ b/scripts/release-windows.ps1 @@ -0,0 +1,282 @@ +<# +.SYNOPSIS + Builds, packages and publishes the Windows desktop client. + +.DESCRIPTION + Run by a person, on a Windows machine that is not a CI runner. That is not an accident of tooling — + docs/adr/0011-android-distribution.md rule 1 puts the capability to ship somebody a build on a machine + which is not a runner, and docs/adr/0012-desktop-distribution-and-updates.md explains why the token that + writes a Gitea release is that capability: Velopack clients trust their feed and do not verify a package + signature when they apply it, so whoever can write a release can ship an update every install runs. + + Two phases, and the split is the design rather than a convenience. + + 1. Without -Upload: builds, packs, and stops. Nothing has left this machine. + Install the Setup.exe it names, and walk Phase 15 of docs/manual-checks.md. + 2. With -Upload: asks for the forge token and publishes what phase 1 produced. It does not rebuild, + so the bytes that reach users are the bytes that were installed and checked. + + The token is prompted for rather than read from a file or an environment variable, and only in the phase + that needs it — the build does not, and the fewer minutes a credential that can publish an update spends + in a shell's memory the better. + +.PARAMETER Upload + Publish the packages already in Releases/ instead of building. + +.PARAMETER SkipTests + Skip the test run. For a re-pack of a tag CI has already gone green on. + +.EXAMPLE + pwsh -File scripts/release-windows.ps1 + pwsh -File scripts/release-windows.ps1 -Upload +#> +#Requires -Version 7.0 + +# PowerShell 7, and stated so the failure is a clear message rather than a confusing one: this script reads +# $IsWindows, which does not exist in Windows PowerShell 5.1 and under Set-StrictMode would throw about an +# unset variable — sending the reader after a typo rather than after the shell they are using. + +[CmdletBinding()] +param( + [switch] $Upload, + [switch] $SkipTests +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +# Velopack's identity for this application, and it is effectively irreversible. +# +# It is what an installed client matches an update against and the directory it installs into, so changing +# it later orphans every existing install — still running, never updated, invisible to the new one. +# +# DodoSSH.Desktop and not DodoSSH, for a specific reason worth keeping next to the value: Velopack installs +# to %LOCALAPPDATA%\ and removes that whole directory on uninstall, and %LOCALAPPDATA%\DodoSSH is +# where ClientPaths keeps the encrypted cache, the outbox of changes not yet pushed, and the device key. +# Sharing the directory would mean the uninstaller silently taking a user's un-synced work with it. +$PackId = 'DodoSSH.Desktop' + +# What a person sees, in the Start menu and in Add/Remove Programs. The distinct pack id costs nothing here. +$PackTitle = 'DodoSSH' +$PackAuthors = 'DodoTech' + +# The project's own forge. Never a DodoSSH deployment — ADR 0011 rule 2. The same URL is a constant in +# VelopackUpdateChannel, and the two have to agree or the client polls somewhere nothing is published. +$RepoUrl = 'https://git.dodotech.cloud/DodoTech/DodoSSH' + +# A contract with VelopackUpdateChannel.ReleaseChannel. It is Velopack's Windows default, so leaving it +# unsaid on both sides would work too — but unsaid here and stated there is how a feed goes quiet with no +# error at all: the client checks, finds nothing, and reports itself up to date forever. +$Channel = 'win' + +$RepoRoot = Split-Path -Parent $PSScriptRoot +$Project = Join-Path $RepoRoot 'src/DodoSSH.Client.App/DodoSSH.Client.App.csproj' +$PublishDir = Join-Path $RepoRoot 'publish/win-x64' +$ReleasesDir = Join-Path $RepoRoot 'Releases' + +function Write-Step([string] $Message) { + Write-Host '' + Write-Host "==> $Message" -ForegroundColor Cyan +} + +function Stop-With([string] $Message) { + Write-Host '' + Write-Host $Message -ForegroundColor Red + exit 1 +} + +if (-not $IsWindows) { + # vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows tooling. This is the smaller of + # the two reasons a runner cannot do this job; see the comment at the foot of .github/workflows/ci.yml + # for the larger one. + Stop-With 'This builds a Windows package and has to run on Windows.' +} + +Push-Location $RepoRoot +try { + # ---- What is being released ----------------------------------------------------------------------- + + $version = (& dotnet msbuild $Project -getProperty:Version -nologo) -replace '\s', '' + if ([string]::IsNullOrWhiteSpace($version)) { + Stop-With 'Could not read the version from MSBuild.' + } + + $tag = "v$version" + + Write-Step "DodoSSH $version ($PackId, channel $Channel)" + + if ($Upload) { + # ---- Phase 2: publish what phase 1 built ------------------------------------------------------ + + $setup = Get-ChildItem $ReleasesDir -Filter '*Setup*.exe' -ErrorAction SilentlyContinue | + Select-Object -First 1 + + if (-not $setup) { + Stop-With "Nothing to upload: $ReleasesDir has no Setup executable. Run this without -Upload first." + } + + Write-Host "About to publish the contents of $ReleasesDir to $RepoUrl as $tag." + Write-Host 'Only do this once you have installed it and walked Phase 15 of docs/manual-checks.md.' + + # Read-Host -AsSecureString so the token is never echoed and never lands in the shell's history. + $secure = Read-Host -Prompt 'Gitea token (write:repository)' -AsSecureString + $token = [System.Net.NetworkCredential]::new('', $secure).Password + + if ([string]::IsNullOrWhiteSpace($token)) { + Stop-With 'No token given.' + } + + # --merge because Gitea already has a release entry for the pushed tag, and without it the upload + # fails on a release that exists. --pre mirrors the rule the docker image job already applies to the + # same tag, so a release candidate is a prerelease in both channels or in neither. + $uploadArgs = @( + 'upload', 'gitea', + '--repoUrl', $RepoUrl, + '--token', $token, + '--outputDir', $ReleasesDir, + '--channel', $Channel, + '--releaseName', $tag, + '--tag', $tag, + '--merge', + '--publish' + ) + + if ($version -match '-') { + $uploadArgs += '--pre' + } + + Write-Step 'Uploading' + & dotnet vpk @uploadArgs + if ($LASTEXITCODE -ne 0) { Stop-With 'vpk upload failed.' } + + Write-Step "Published $tag." + return + } + + # ---- Phase 1: build and pack ---------------------------------------------------------------------- + + if ((git status --porcelain) -ne $null) { + Stop-With 'The working tree is not clean. A release is cut from a commit, not from a desk.' + } + + $headTag = git describe --exact-match --tags HEAD 2>$null + if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrWhiteSpace($headTag)) { + Stop-With "HEAD is not tagged. Tag it $tag first, or change the version and tag that." + } + + if ($headTag -ne $tag) { + # Cannot happen while MinVer is deriving the version from this very tag, and checked anyway: the + # day somebody pins a version by hand this is the guard that notices. + Stop-With "HEAD is tagged $headTag but the computed version is $version." + } + + Write-Step 'Restoring tools' + & dotnet tool restore + if ($LASTEXITCODE -ne 0) { Stop-With 'dotnet tool restore failed.' } + + Write-Step 'Restoring packages (locked, exactly as CI does)' + & dotnet restore (Join-Path $RepoRoot 'DodoSSH.slnx') --locked-mode + if ($LASTEXITCODE -ne 0) { Stop-With 'Restore failed. A lock file that only works on Linux fails here.' } + + Write-Step 'Building' + & dotnet build (Join-Path $RepoRoot 'DodoSSH.slnx') --no-restore --configuration Release + if ($LASTEXITCODE -ne 0) { Stop-With 'Build failed.' } + + if (-not $SkipTests) { + # The end-to-end suite starts containers and takes minutes. It is run here anyway rather than taken + # on trust from CI, because a tag is the one build nobody is watching — the same argument ci.yml + # already makes for running the whole workflow on a tag. + Write-Step 'Testing' + & dotnet test (Join-Path $RepoRoot 'DodoSSH.slnx') --no-build --configuration Release + if ($LASTEXITCODE -ne 0) { Stop-With 'Tests failed.' } + } + + Write-Step 'Publishing win-x64' + if (Test-Path $PublishDir) { Remove-Item $PublishDir -Recurse -Force } + + # Self-contained: .NET 10 is recent enough that almost no machine has the runtime, and the usual + # objection — that runtime patches then need an application update — is answered by the updater this + # very script exists to feed. Not single-file: the native libraries ship per RID, deltas would stop + # working, and a self-extracting bundle puts the executable under a temp path deep enough to break + # WebView2 (docs/platform-flags.md). + # RestoreLockedMode=false, and the lock files put back straight afterwards. Both halves need saying. + # + # A RID-specific publish resolves a graph the committed lock files do not describe, because they are + # deliberately kept RID-free: declaring win-x64 on the project writes a net10.0/win-x64 target into + # every project it references transitively, including DodoSSH.Contracts and DodoSSH.Crypto — and the + # API's Dockerfile then restores those with no RID under locked mode and fails NU1004. Packaging the + # desktop client would have broken the server's image build. See the comment in the head's csproj. + # + # So this one command restores unlocked. It is a supervised build, from a tag, run by a person; the + # gate that matters is the locked solution restore two steps above, which is untouched and is the same + # command CI runs. + & dotnet publish $Project ` + --configuration Release ` + --runtime win-x64 ` + --self-contained true ` + --output $PublishDir ` + -p:RestoreLockedMode=false + if ($LASTEXITCODE -ne 0) { Stop-With 'Publish failed.' } + + # An unlocked restore rewrites the lock files it walked, adding the win-x64 target. Left there, the + # next commit would carry exactly the change that breaks the image build — so they go back. Safe to do + # bluntly because this script refuses to run on a dirty tree, so anything modified here is its own. + & git checkout -- '*packages.lock.json' + if ($LASTEXITCODE -ne 0) { Stop-With 'Could not restore the lock files after publishing.' } + + # Checked rather than assumed. A publish directory without Velopack.dll would pack into an installer for + # an application that never checks for updates — which looks completely normal until the next release + # goes out and nobody receives it. + foreach ($required in @('DodoSSH.exe', 'Velopack.dll')) { + if (-not (Test-Path (Join-Path $PublishDir $required))) { + Stop-With "$required is missing from $PublishDir." + } + } + + $sizeMb = [math]::Round(((Get-ChildItem $PublishDir -Recurse -File | Measure-Object Length -Sum).Sum / 1MB), 1) + Write-Host " $sizeMb MB in $((Get-ChildItem $PublishDir -Recurse -File).Count) files" + + New-Item -ItemType Directory -Force -Path $ReleasesDir | Out-Null + + # The previous release, so a delta can be built against it. Tolerated when it finds nothing: the first + # release has no predecessor, and a hard failure here would make cutting it impossible. + Write-Step 'Fetching the previous release, for deltas' + & dotnet vpk download gitea --repoUrl $RepoUrl --outputDir $ReleasesDir --channel $Channel + if ($LASTEXITCODE -ne 0) { + Write-Host ' Nothing came down. This package will be full-only, which is right for a first release.' -ForegroundColor Yellow + } + + Write-Step 'Packing' + + # No --signParams. Every installer therefore raises SmartScreen's "Windows protected your PC" on first + # run, once per user — Mark-of-the-Web is applied by the browser that downloads Setup.exe, so in-app + # updates, which this application fetches itself and applies from a local file, never trip it. + # + # This is the one line that changes when a certificate is bought. See ADR 0012 for what it costs and + # what the trigger for buying one is. + & dotnet vpk pack ` + --packId $PackId ` + --packVersion $version ` + --packDir $PublishDir ` + --packTitle $PackTitle ` + --packAuthors $PackAuthors ` + --mainExe 'DodoSSH.exe' ` + --icon (Join-Path $RepoRoot 'src/DodoSSH.Client.App/Assets/dodossh.ico') ` + --channel $Channel ` + --outputDir $ReleasesDir + if ($LASTEXITCODE -ne 0) { Stop-With 'vpk pack failed.' } + + Write-Step 'Built, and deliberately not uploaded' + + Get-ChildItem $ReleasesDir -File | + Sort-Object Length -Descending | + Select-Object Name, @{ n = 'MB'; e = { [math]::Round($_.Length / 1MB, 1) } } | + Format-Table -AutoSize + + Write-Host 'Next:' + Write-Host " 1. Install the Setup executable above and walk Phase 15 of docs/manual-checks.md." + Write-Host ' 2. Then: pwsh -File scripts/release-windows.ps1 -Upload' +} +finally { + Pop-Location +} diff --git a/src/DodoSSH.Api/Dockerfile b/src/DodoSSH.Api/Dockerfile index eeeeb0d..4a583af 100644 --- a/src/DodoSSH.Api/Dockerfile +++ b/src/DodoSSH.Api/Dockerfile @@ -50,10 +50,37 @@ RUN dotnet restore src/DodoSSH.Api/DodoSSH.Api.csproj --locked-mode COPY BannedSymbols.txt .editorconfig ./ COPY src/ src/ +# The version, handed in rather than derived, because there is no repository in here to derive +# it from: MinVer reads git tags, and .dockerignore excludes .git/ deliberately — the context is +# the repository root and copying the whole history into every image build would be absurd. +# +# Without this the build still succeeds (MINVER1001 is a warning, and TreatWarningsAsErrors does +# not escalate a task warning), and that is the trap: the image would be built with the SDK's +# fallback version and GET /api/v1/meta would report 0.0.0-alpha.0 as its serverVersion, which is +# a lie told quietly. The tag is already parsed by the workflow for the image tags, so it is the +# same number, passed one step further. +# +# MinVerSkip because there is nothing here for it to do, and it should not warn about it either. +# +# ASSEMBLY_VERSION and emphatically not VERSION, which is the trap this block exists to avoid and +# which cost a build to find. An ARG is an environment variable for the rest of the stage, MSBuild +# reads environment variables as global properties, and property names are case-insensitive — so an +# `ARG VERSION` in a build stage silently sets MSBuild's `Version` for every project in it. With the +# workflow passing `main-` on a main build, that is not a version the SDK will accept, and +# the publish dies with NETSDK1018 "Invalid NuGet version string" pointing at DodoSSH.Contracts, a +# project nobody changed. The name is the whole fix; the ARG in the final stage below is only ever a +# label and never meets MSBuild. +# +# The workflow passes this empty except on a tag build, so a main image keeps the SDK default rather +# than carrying a version that is not one. +ARG ASSEMBLY_VERSION="" + RUN dotnet publish src/DodoSSH.Api/DodoSSH.Api.csproj \ --no-restore \ --configuration Release \ --output /app \ + -p:MinVerSkip=true \ + ${ASSEMBLY_VERSION:+-p:Version="$ASSEMBLY_VERSION"} \ -p:UseAppHost=false # --------------------------------------------------------------------------------------- diff --git a/src/DodoSSH.Api/packages.lock.json b/src/DodoSSH.Api/packages.lock.json index 91c08cd..c73ac63 100644 --- a/src/DodoSSH.Api/packages.lock.json +++ b/src/DodoSSH.Api/packages.lock.json @@ -44,6 +44,12 @@ "resolved": "5.6.0", "contentHash": "Kcobt3pnOdO0A+6CKiMHZdTEluJpsfxiV20axtZdmfBQnDmiWTKPJADlgAfdTuKNAnVarrkJa0UEGwuOo91muw==" }, + "MinVer": { + "type": "Direct", + "requested": "[7.0.0, )", + "resolved": "7.0.0", + "contentHash": "2lMTCQl5bGP4iv0JNkockPnyllC6eHLz+CoK2ICvalvHod+exXSxueu9hq+zNkU7bZBJf8wMfeRC/Edn8AGmEg==" + }, "FastEndpoints.Attributes": { "type": "Transitive", "resolved": "8.2.0", diff --git a/src/DodoSSH.Client.Android/DodoSSH.Client.Android.csproj b/src/DodoSSH.Client.Android/DodoSSH.Client.Android.csproj index 7dc1237..ca0c07f 100644 --- a/src/DodoSSH.Client.Android/DodoSSH.Client.Android.csproj +++ b/src/DodoSSH.Client.Android/DodoSSH.Client.Android.csproj @@ -20,8 +20,14 @@ 36 dev.dodotech.dodossh + + 1 - 0.1.0 + + + + $(MinVerMajor).$(MinVerMinor).$(MinVerPatch) + + + diff --git a/src/DodoSSH.Client.Api/packages.lock.json b/src/DodoSSH.Client.Api/packages.lock.json index 8d54521..2a7993d 100644 --- a/src/DodoSSH.Client.Api/packages.lock.json +++ b/src/DodoSSH.Client.Api/packages.lock.json @@ -14,6 +14,12 @@ "resolved": "5.6.0", "contentHash": "Kcobt3pnOdO0A+6CKiMHZdTEluJpsfxiV20axtZdmfBQnDmiWTKPJADlgAfdTuKNAnVarrkJa0UEGwuOo91muw==" }, + "MinVer": { + "type": "Direct", + "requested": "[7.0.0, )", + "resolved": "7.0.0", + "contentHash": "2lMTCQl5bGP4iv0JNkockPnyllC6eHLz+CoK2ICvalvHod+exXSxueu9hq+zNkU7bZBJf8wMfeRC/Edn8AGmEg==" + }, "dodossh.client.auth": { "type": "Project" }, diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index b17d950..b336613 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -72,6 +72,24 @@ internal sealed partial class DodoSshApp : Application } }; + /// The terminal workspace, with its loopback listener already up. + /// + /// Extracted so that constructing it and starting it cannot drift apart: the data plane's socket has to + /// be listening before the renderer attaches, and a workspace handed out un-started is one whose first + /// connect fails for a reason nothing on screen would explain. + /// + private static TerminalWorkspace StartedWorkspace(SshNetConnectionFactory connections) + { + var workspace = new TerminalWorkspace( + new AvaloniaTerminalAssetProvider(), + connections, + TimeProvider.System); + + workspace.Start(); + + return workspace; + } + private static void Compose(IClassicDesktopStyleApplicationLifetime desktop) { var paths = ClientPaths.Default; @@ -87,19 +105,17 @@ internal sealed partial class DodoSshApp : Application // and the same host key decision, and composing two would mean two snapshots of the pins. var connections = new SshNetConnectionFactory(knownHosts); - var workspace = new TerminalWorkspace( - new AvaloniaTerminalAssetProvider(), - connections, - TimeProvider.System); - - workspace.Start(); + var workspace = StartedWorkspace(connections); var browser = new SystemBrowserLauncher(); - // Chosen once, here, because it is a property of the machine and not of any session. A computer with - // a usable TPM gets the store that keeps a device key behind a Windows consent prompt; anything else - // gets one that reports itself unavailable, so unlock keeps asking for the passphrase. See ADR 0007. + // Both chosen once, here, because each is a property of the machine rather than of any session. A + // computer with a usable TPM gets the store that keeps a device key behind a Windows consent prompt; + // anything else gets one that reports itself unavailable, so unlock keeps asking for the passphrase + // (ADR 0007). The update channel answers the same shape of question about how this copy was + // installed, and a build run from a checkout likewise gets one that says so. See ADR 0012. var deviceKeys = DesktopDeviceKeyStores.ForThisMachine(paths); + var updates = UpdateChannels.ForThisMachine(); var viewModel = new MainWindowViewModel( paths, @@ -120,7 +136,8 @@ internal sealed partial class DodoSshApp : Application .ResumeAsync(url, refreshToken, TimeProvider.System, cancellationToken) .ConfigureAwait(false), - copyToClipboard: ClipboardWriter(desktop)); + copyToClipboard: ClipboardWriter(desktop), + updates: updates); desktop.MainWindow = new MainWindow { DataContext = viewModel }; diff --git a/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj b/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj index 38f9352..6b6cfc2 100644 --- a/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj +++ b/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj @@ -5,6 +5,35 @@ app.manifest true + + DodoSSH + + + + + + + + + @@ -38,6 +94,14 @@ + + diff --git a/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs b/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs new file mode 100644 index 0000000..8ea2a4b --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs @@ -0,0 +1,194 @@ +using DodoSSH.Client.Session; +using Velopack; +using Velopack.Sources; + +namespace DodoSSH.Client.App.Platform; + +/// +/// Chooses the update channel this machine can actually use. +/// +/// +/// Decided once, at composition, from a property of the machine — the same shape as +/// DesktopDeviceKeyStores.ForThisMachine, and for the same reason: whether this copy can replace +/// itself does not change while it runs, and a check repeated at each call site is a check somebody +/// eventually forgets. +/// +internal static class UpdateChannels +{ + /// The channel for this machine, or one that reports itself unavailable. + /// + /// + /// Two conditions, and the second is the one that matters in development. Velopack's + /// IsInstalled is false when the process is not running from an installed layout — which is + /// every dotnet run, every build started from an IDE, and every copy somebody extracted from + /// an archive by hand. Reaching into the updater from one of those does not fail politely. + /// + /// + /// Constructing an is what answers the question, and constructing one is + /// cheap — it reads the layout on disk and talks to nothing. The network is not touched until + /// somebody asks for a check. + /// + /// + internal static IUpdateChannel ForThisMachine() + { + if (!OperatingSystem.IsWindows()) + { + return new UnavailableUpdateChannel(); + } + + try + { + var manager = VelopackUpdateChannel.CreateManager(); + + return manager.IsInstalled + ? new VelopackUpdateChannel(manager) + : new UnavailableUpdateChannel(); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // A machine whose install layout cannot be read is a machine with no updater, which is a + // state this application already knows how to be in. Refusing to start an SSH client over + // it would be the wrong trade by a wide margin. + return new UnavailableUpdateChannel(); + } + } +} + +/// +/// The Windows update channel, backed by Velopack against the project's own forge. +/// +/// +/// +/// The one file in the repository that names Velopack. It lives beside WindowsDeviceKeyStore +/// rather than in a project of its own because it is the same kind of thing — a Windows-only +/// implementation of an interface declared in DodoSSH.Client.Session — and because +/// DodoSSH.Client.Shell is shared with the Android head, which must never acquire an updater. +/// +/// +/// See docs/adr/0012-desktop-distribution-and-updates.md. +/// +/// +internal sealed class VelopackUpdateChannel : IUpdateChannel +{ + /// + /// Where builds come from, and it is a constant on purpose. + /// + /// + /// This must never become a setting. ADR 0011 rule 2 says the deployment a client signs in to + /// is never where the client comes from, and it says the same about the update check: an operator who + /// can answer "is there a newer version" can answer "no" forever, and pin a chosen user to a build + /// with a known hole without holding any key. A configurable feed URL is exactly the knob that would + /// hand them that, whether through a settings screen or through somebody editing the plaintext + /// settings.json by hand. A constant is that rule expressed structurally rather than as a convention + /// somebody has to keep. + /// + private const string RepositoryUrl = "https://git.dodotech.cloud/DodoTech/DodoSSH"; + + /// + /// The release channel to read, and it is stated rather than left to the default. + /// + /// + /// A contract with scripts/release-windows.ps1, which passes the same word to vpk pack. + /// It happens to be Velopack's Windows default, so leaving it unsaid on both sides would work too — + /// but unsaid on one side and stated on the other is how a feed goes quiet with no error anywhere: + /// the check succeeds, finds nothing, and reports that the client is up to date forever. + /// + private const string ReleaseChannel = "win"; + + private readonly UpdateManager manager; + + /// + /// The last thing a check found, kept so that a download and an apply can name it. + /// + /// + /// Velopack's UpdateInfo carries the delta chain and the target asset, and none of that should + /// cross the seam — the shell has no use for it and a test would have to construct it. So the record + /// handed upwards is a version string, and this is where the real answer waits to be matched back up. + /// + private UpdateInfo? found; + + internal VelopackUpdateChannel(UpdateManager manager) => this.manager = manager; + + /// + public bool IsSupported => true; + + /// + /// + /// From the assembly rather than from manager.CurrentVersion, so that this and the version an + /// un-updatable build reports come from one place. Two ways of answering the same question is how + /// they come to disagree. + /// + public string CurrentVersion => ClientVersion.Current; + + internal static UpdateManager CreateManager() => + new( + new GiteaSource(RepositoryUrl, accessToken: null, prerelease: false), + new UpdateOptions { ExplicitChannel = ReleaseChannel }); + + /// + public async Task CheckAsync(CancellationToken cancellationToken) + { + // CheckForUpdatesAsync takes no token of its own, so cancellation is observed on either side of + // it rather than during. The call is one HTTPS request against a small JSON document; the worst + // case is a lock-up already bounded by the handler's own timeout. + cancellationToken.ThrowIfCancellationRequested(); + + var update = await manager.CheckForUpdatesAsync().ConfigureAwait(false); + + cancellationToken.ThrowIfCancellationRequested(); + + if (update is null) + { + found = null; + + return null; + } + + found = update; + + return new AvailableUpdate(update.TargetFullRelease.Version.ToString()); + } + + /// + public Task DownloadAsync( + AvailableUpdate update, + IProgress progress, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(update); + ArgumentNullException.ThrowIfNull(progress); + + // Velopack reports progress as an Action and the rest of this codebase speaks IProgress, + // so the adaptation happens here rather than leaking the older shape into the view models. + return manager.DownloadUpdatesAsync(Matched(update), progress.Report, cancellationToken); + } + + /// + public void ApplyAndRestart(AvailableUpdate update) + { + ArgumentNullException.ThrowIfNull(update); + + // Does not return: the process is replaced. Anything that needed to happen before the window + // closes has to have happened already — see the shell's restart command, which disposes first. + manager.ApplyUpdatesAndRestart(Matched(update).TargetFullRelease); + } + + /// + /// The guard exists because the seam narrows UpdateInfo down to a version string, so nothing in + /// the type system stops a caller inventing one. Every legitimate caller passes back exactly what + /// returned; a mismatch is a bug in this application rather than anything a + /// user did, which is why it throws rather than resolving to some safe-looking default. + /// + private UpdateInfo Matched(AvailableUpdate update) + { + if (found is not { } info + || !string.Equals(info.TargetFullRelease.Version.ToString(), update.Version, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + $"No update matching {update.Version} has been found by this channel. " + + "Call CheckAsync and pass back what it returned."); + } + + return info; + } +} diff --git a/src/DodoSSH.Client.App/Program.cs b/src/DodoSSH.Client.App/Program.cs index a7c8863..dc6d553 100644 --- a/src/DodoSSH.Client.App/Program.cs +++ b/src/DodoSSH.Client.App/Program.cs @@ -1,5 +1,7 @@ using Avalonia; using Avalonia.Media; +using DodoSSH.Client.Session; +using Velopack; namespace DodoSSH.Client.App; @@ -10,11 +12,78 @@ internal static class Program /// /// /// STAThread is required, not decorative: WebView2 checks the apartment state and refuses - /// to initialise on an MTA thread. Without it the terminal is simply blank on Windows. + /// to initialise on an MTA thread. Without it the terminal is simply blank on Windows. It applies to + /// everything below, which is why the Velopack call lives inside this method rather than in an entry + /// point of its own. /// [STAThread] - public static void Main(string[] args) => + public static void Main(string[] args) + { + // First, before Avalonia is even configured. + // + // The installer, the updater and the uninstaller all re-run this executable with arguments that + // mean "do the install bookkeeping and stop". Run() is what notices, does it, and exits — so on + // those runs nothing below happens at all, and that is the point rather than a side effect: + // DodoSshApp.Compose opens the SQLite cache and starts the terminal workspace's listening socket, + // and a silent installer run that reached either would be a background process holding the cache + // file open during the very file operations the installer is performing. + // + // There are deliberately no OnFirstRun or OnAfterUpdate hooks. A hook process has no passphrase, + // so the cache is bytes it cannot read, and the one thing that would want doing after an update — + // a schema migration — already runs on every ordinary launch from MainWindowViewModel.StartAsync, + // before unlock and touching no encrypted content. + VelopackApp.Build().Run(); + + KeepTheWebViewProfileOutOfTheInstallDirectory(); + BuildAvaloniaApp().StartWithClassicDesktopLifetime(args); + } + + /// + /// Puts WebView2's user data folder beside the vault cache instead of beside the executable. + /// + /// + /// + /// WebView2 defaults this to a directory next to the host executable. Under a Velopack install that is + /// %LOCALAPPDATA%\DodoSSH.Desktop\current\, and current\ is replaced by every + /// update — so the browser profile would be destroyed on each one, and the first connect afterwards + /// would pay a cold WebView2 start: a new user data directory and a fresh process tree, which is the + /// slow path TerminalWorkspaceOptions.RendererTimeout's fifteen seconds was sized for. It would + /// land at the exact moment somebody is most ready to believe the update broke the terminal. + /// + /// + /// The profile directory is the right home because Velopack never touches it — the pack id is + /// deliberately not DodoSSH, so the install root and ClientPaths.DataDirectory are + /// siblings rather than the same folder. See docs/adr/0012-desktop-distribution-and-updates.md. + /// + /// + /// An environment variable rather than the control's own options, because it is read by the WebView2 + /// loader before any of this application's UI exists, and because it needs no reference to whichever + /// WebView package the terminal happens to be hosted by. + /// + /// + private static void KeepTheWebViewProfileOutOfTheInstallDirectory() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + var folder = Path.Combine(ClientPaths.Default.DataDirectory, "WebView2"); + + try + { + Directory.CreateDirectory(folder); + + Environment.SetEnvironmentVariable("WEBVIEW2_USER_DATA_FOLDER", folder); + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // Left unset, which puts the profile back beside the executable. That is a slow first connect + // after each update, not a broken terminal, and refusing to start an SSH client over it would + // be the wrong trade. + } + } /// Used by the designer as well as by . /// diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml b/src/DodoSSH.Client.App/Views/MainWindow.axaml index 2bd26a6..5231498 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml @@ -70,7 +70,7 @@ fires only for its own IsVisible. --> - + @@ -334,7 +334,24 @@ - + + + + diff --git a/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml b/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml index 011adf4..406dd92 100644 --- a/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml +++ b/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml @@ -61,6 +61,79 @@ + + + + + + + + + + + +