Commit Graph
7 Commits
Author SHA1 Message Date
jaap-jan 6728a0a597 Let the desktop client replace itself, and give the repository one version
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%\<packId>
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-<short sha> 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.
2026-08-04 17:04:41 +02:00
jaap-jan 94e11f5e38 update packages
ci / build and test (ubuntu) (push) Canceled after 0s
ci / build (windows) (push) Canceled after 0s
2026-07-31 10:12:05 +02:00
jaap-jan 1d262b7ccc Run M1's end-to-end slice, and fix the two bugs it found
The whole vertical slice now runs against a real Keycloak, a real API, a
real PostgreSQL and a real sshd: sign in through the browser flow, enroll
with the identity-provider key binding, unlock, create a host, sync it,
read it back on a second machine, unlock again with no network, accept an
unseen host key, and open an interactive shell. Opt-in, because it needs
the development stack; skipped with a message naming the commands.

It found two bugs on its first run, and both are the same class: two
sides of a stub agreeing with each other about something the
specification never said.

**The API never applied DodoSshJsonContext to its HTTP JSON options.**
Minimal APIs therefore used the framework's web defaults, which write an
enum as a number. Every request DTO carrying one failed to bind against a
client writing the specified string form — which is the entire sync
surface, unreachable from the real client, with a 400 naming only the
parameter. The documented guarantee that request bodies reject unmapped
members was likewise not in effect anywhere.

Nothing caught it because the API tests posted with PostAsJsonAsync's
defaults, so they and the server had independently settled on integers.
Those tests now serialise through the contract, which is the deeper fix:
removing the new configuration fails 13 of them. Copying settings into
options a host owns is itself the hazard the context warns about, so
ApplyTo lives beside the settings it mirrors and ApplyToTests pins the
transformation, including that inserting the resolver leaves the caller's
own in place.

**The realm registered a loopback redirect URI Keycloak rejects.**
`http://127.0.0.1:*/callback` looks more explicit than the RFC 8252 form
and is broken: Keycloak's wildcards are trailing-only, so the `*` parses
as a literal port and every authorization request came back "Invalid
parameter: redirect_uri". Providers ignore the port for loopback hosts,
which is the whole mechanism, so the correct registration is
`http://127.0.0.1/callback` — path pinned, port free. The value the
server advertises through the discovery document said the same wrong
thing and now says the right one.

Two smaller things, both documented in docs/platform-flags.md:

- --import-realm skips a realm that already exists, so editing the realm
  file and restarting Keycloak changes nothing and serves stale
  configuration. The container has to be recreated. The compose comment
  claimed the opposite.
- Keycloak marks its session cookies Secure even over plain HTTP, because
  SameSite=None requires it. A spec-conformant client drops them and the
  login POST answers 400 with no message; browsers complete the flow only
  because they exempt loopback. Harmless for the product, fatal for
  automation, so ScriptedBrowser carries the cookies by hand and says why.

Also: the server enforces a 64 MiB floor on the passphrase KDF, so this
suite cannot use the 8 MiB profile the other client suites take for
speed. Those only get away with it because their in-memory servers have
no policy — worth knowing rather than rediscovering.

638 tests. The solution-wide run stays green with the stack down: exit
code 8 means "no tests ran", which the platform reports as failure, so
the opt-in project ignores exactly that code.
2026-07-29 11:37:49 +02:00
jaap-jan 8d2416a602 Add the encrypted local cache and the sync client
Three new client projects, and the wire-contract fix they needed.

DodoSSH.Client.Domain holds the decrypted item model and the three-way
merge, with no I/O at all — so the suite that decides whether a
credential can be lost runs in milliseconds with nothing to mock.
Scalars defer to the server on a genuine clash so every replica resolves
the same triple identically and two clients cannot ping-pong; directives
merge per name so two people each adding one both keep theirs; the jump
chain merges as a whole value because its order is the route. Whatever
loses is returned rather than dropped.

DodoSSH.Client.Storage is EF Core on SQLite, no SQLCipher: the rows are
already ciphertext, so an encrypted file would protect protected bytes
at the cost of a native dependency. It keeps the server's state and the
outbox in separate tables, which is what preserves the common ancestor a
merge needs. One pending operation per item, enforced by a unique index.

DodoSSH.Client.Sync is the pull/apply/push loop. Pulling never decrypts
— a change with no local work pending is plumbed as ciphertext — so a
first sync of thousands of items does not run twice as many AEAD
operations for nothing.

Contracts: EncryptedPayload gains WrappedDataKey and DataKeyId. The
specification has required a per-item data key since crypto.md §3, the
columns have existed since the first migration and DshAad.ItemPayload
binds the id, but this record had nowhere to put either — so a
spec-compliant item could not be transmitted at all. Found by writing
the client that has to produce one. Also closes a hole in
AadResourceType, which had no value for the HostTag and HostCredential
that SyncEntityType has always listed.

Four bugs the tests found, not review:

- SQLite refuses to order or compare its own DateTimeOffset mapping, and
  throws at execution rather than model build. Collecting tombstones and
  listing conflicts are both that shape, so this was a crash waiting for
  the first user with a deleted host. Timestamps are integers now, by
  convention so a later field cannot be the one left unconverted.
- SQLitePCLRaw 2.1.11, which EF resolves, is covered by
  GHSA-2m69-gcr7-jv3q. Pinned forward as a family.
- Resurrecting content from a remote deletion cleared the original
  before queueing the copy. Two transactions, so a crash between them
  lost the work; reversed, and the rescued id is derived from the
  tombstone so a replay coalesces instead of duplicating.
- Several equality assertions went through Shouldly's ShouldBe, which
  compares IEnumerable element-wise and so tested nothing about the
  Equals these types exist to provide. Corrected; the falsification that
  caught it went from 2 failures to 6.

The push response's cursor is deliberately ignored. It sits after this
client's own writes, so adopting it skips anything another client
committed at a lower sequence in the window between a pull and a push —
permanently. Re-reading one's own writes is idempotent and costs a page.
The Contracts doc that invited the shortcut now says so.

593 tests, up from 448. The delete-versus-edit rules, the ancestor
retention, the fresh operation id on coalesce and the cursor safeguard
were each verified by breaking them and watching the right test fail.
2026-07-29 10:27:37 +02:00
jaap-jan d2a2ed8a29 Specify the key statement encoding and key log chain (crypto.md 7.1, 7.2)
Section 7 always required "a canonical, length-prefixed encoding" for
signatures without ever specifying one. That gap had to be closed before
enrollment could exist: the client hashes the key statement and uses the
result as an OIDC nonce, so the provider signs over those exact bytes. Two
implementations disagreeing by one byte produce two nonces and an
enrollment nobody can verify -- and it only shows up against a real
provider, never in a local test.

JSON cannot be the hashed form. Property order, number formatting, Unicode
escaping and whitespace all vary between serialisers. So the statement is
transmitted as JSON and hashed as a fixed binary encoding, and the two are
independent by construction.

Three details are load-bearing rather than stylistic:

- The presence byte before each string is what makes the encoding
  injective. Without it an absent email and an empty one encode
  identically, and two different statements share a binding.
- Timestamps truncate to milliseconds. PostgreSQL stores microseconds, so
  a statement that has been through the database must still hash to what
  the client hashed. The same applies to the key log, where an entry that
  cannot reproduce its own hash after being read back makes the chain
  unverifiable.
- The key log entry hash deliberately excludes the database sequence. It
  is unknown until the insert runs, and order already follows the hash
  links -- so a renumbered or gapped sequence column cannot silently
  reorder history.

KeyStatementFields is separate from Contracts.KeyStatement on purpose: one
may gain JSON fields freely, the other cannot change without invalidating
every stored binding, and Crypto must not depend on the contract assembly.
KeyStatementDriftTests makes a field added to one and not the other a
build failure, because a wire field outside the binding is unauthenticated
data the server can change undetected.

54 new tests and two new golden vector sections. The vectors pin the
absent-versus-empty email case and confirm that an offset-bearing
sub-millisecond timestamp encodes identically to its truncated UTC form.
Only additions to vectors.json; nothing existing moved.
2026-07-28 16:06:11 +02:00
jaap-jan 06d04b490b Freeze DodoSSH.Contracts v0.1 (M1)
The second M1 gate. This assembly, not the OpenAPI document, is the client's contract,
so PublicApiAnalyzers now tracks all 540 public members: a renamed DTO property becomes
a build error rather than a runtime deserialisation failure on someone's laptop.

Contract surface:
- EncryptedPayload carries the envelope plus the KeyGeneration and AadVersion columns
  needed to recompute AAD, since AAD is derived from the row rather than transmitted.
- Sync: push with per-operation status (Applied/Conflict/Forbidden/Invalid/Duplicate) so
  one stale item cannot block a whole offline queue; a Conflict returns the server's row
  for client-side three-way merge, because the server cannot merge ciphertext.
- Enrollment: KeyStatement whose hash becomes the OIDC nonce, so the identity provider
  signs over the public keys and this server cannot fabricate a key for a user who never
  enrolled.
- Meta and .well-known configuration: capability negotiation instead of URL versioning,
  which is what a self-hosted product needs when client and server upgrade independently.
- SyncPlaintextFields deliberately has no label or name field. ACL admin runs client-side
  where names can be decrypted, so the server never needs a searchable title.

Two design problems found by writing the tests rather than assuming:
- Hand-constructing JsonSerializerOptions and merely pointing its resolver at the context
  silently discards every source-generated setting. JsonSerializerDefaults.Web replaces
  NumberHandling.Strict with AllowReadingFromString, so "1" would be accepted where 1 is
  meant — invisible until two implementations disagree. Callers now use ResponseOptions or
  StrictRequestOptions; StrictRequestOptions is derived by copying so it cannot drift.
- StrictRequestOptions had a static-initialisation cycle: it read the generated Default
  property from the same type's initialiser and got null. Now lazy.

Requests reject unmapped members so a client typo is a 400; responses tolerate them so an
older client can read a newer server. Enums cross the wire as strings, so reordering one
cannot silently reinterpret stored data.

Also: excluded source-generator output from PublicApiAnalyzers. The JSON generator emits a
public member per serialisable type, which would have added hundreds of mechanical entries
and drowned the ones describing the actual wire contract. And disabled MA0048's
one-type-per-file rule: splitting SyncPullRequest from SyncPullResponse makes a reviewer
open two files to understand one endpoint.

Verified: 0 warnings, 95 tests pass, format clean.
2026-07-28 13:28:02 +02:00
jaap-jan 3a81f3c90b Restructure into src/tests and add build foundation (M0)
Moves the scaffold to src/DodoSSH.Api and establishes the repo conventions the rest
of the milestones build on.

Structure:
- src/{Contracts,Crypto,Domain,Infrastructure,Api}, tests/{Contracts,Crypto,Domain}.Tests
- DodoSSH.slnx rewritten with src/ and tests/ solution folders

Build:
- Directory.Build.props centralises TFM, nullable, deterministic builds and
  TreatWarningsAsErrors; Directory.Packages.props pins every version centrally
- packages.lock.json committed so CI restores in locked mode
- NuGet.config clears machine-level sources, which both fixes NU1507 under central
  package management and makes restore reproducible off this machine
- Microsoft.OpenApi pinned to 2.11.0: ASP.NET Core 10.0.10 resolves 2.0.0, which is
  covered by GHSA-v5pm-xwqc-g5wc (high, patched in 2.7.5)

Analyzers:
- AnalysisLevel is Recommended, not All. With warnings-as-errors, All turns opinionated
  naming rules into build breaks and trains people to blanket-suppress.
- BannedSymbols.txt bans DateTime.UtcNow (TimeProvider), Guid.NewGuid (CreateVersion7),
  sync-over-async, MD5/SHA1, PBKDF2 and SecureString
- CA1711/CA1724 disabled: both are .NET Framework CAS-era naming rules
- PublicApiAnalyzers on Contracts only, since that assembly is the client's real contract

API:
- weather-forecast template removed
- UseHttpsRedirection removed; TLS terminates at the reverse proxy and redirecting
  behind one causes loops
- /healthz/{live,ready,startup}. Liveness deliberately checks no dependencies so a
  transient database outage cannot restart the container and kill live SSH sessions.

Notes:
- No coverage collector yet. Microsoft.Testing.Extensions.CodeCoverage pulls an MTP 1.x
  MSBuild extension that throws TypeLoadException against the MTP 2.3.x xunit.v3 brings.
  Coverage gates are an M3 concern; revisit with an MTP 2.x-aligned version then.

Verified: dotnet build (0 warnings), 17 tests pass, format check clean, API serves
health and OpenAPI endpoints.
2026-07-28 12:25:34 +02:00