4d1f07f253bca792febcd008d742db98c28b3adf
30
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 jaap-jan
|
174ef7c420 | Merge branch 'claude/android-release' | ||
|
jaap-jan
|
af0e29a98b |
Give the desktop a nightly channel, the way the phone has one
ADR 0014 gave the phone a nightly and ADR 0013 rule 3 gave the desktop none, so the two heads had different answers to the same question — how does somebody try what is on main? — for no reason except the order the work happened in. This is the desktop's answer: CI publishes a build from main on every push, and it installs beside the release one rather than over it. The phone gets its separation from the platform. Android refuses an update signed by a different key, so its two channels cannot replace one another whatever anybody does. Nothing refuses anything here: Velopack applies what its feed serves and verifies no signature. So all of it is construction, and there are four separations because each closes a different door. A pack id each, so the two install in different directories and neither feed's package can be applied to the other's install. A Velopack channel each — win and win-nightly — so neither build ever reads the other's release index; the name reaches the wire as releases.win-nightly.json, which is why the constant in VelopackUpdateChannel and the argument in ci.yml have to agree or the channel answers nothing forever with no error. A prerelease flag, so the release channel cannot see the nightly even by accident. And a profile directory each, which is the one that is easy to skip and would hurt most: the cache schema is migrated on every launch, before unlock, so a shared profile means a nightly quietly upgrading a database the release build then opens. Both are installed at once by design, so that is an ordinary Tuesday rather than a corner case. The prerelease flag turns out to be load-bearing across heads as well. The phone's release channel reads releases/latest, which skips prereleases — so a desktop nightly published as a stable release would become the newest release in this repository and every phone on the release channel would start failing its check against a release carrying no Android manifest. Which build this is arrives as assembly metadata, the same mechanism and the same reasoning as the Android head: the updater needs the string rather than a branch, and a value baked into the assembly is one a crash report can be asked for. Three things read it — the feed, the prerelease flag, and the profile — and one more shows it: the titlebar says DodoSSH Nightly. Everything else that distinguishes the two is somewhere nobody is looking while typing a passphrase into one of them. The version needed a floor and it is applied to the whole build rather than to the packaging. MinVer answers 0.0.0-alpha.0.N until the first v* tag and vpk refuses anything below 0.0.1, so the job lifts the patch digit and keeps the height — through MinVerVersionOverride, so the assemblies carry the same number the installer does. Packing a version the assembly disagreed with would put one string on the preferences screen and another in the feed, which is the screen somebody reads when asked which nightly they are on. Two things found by running it rather than reading it. -t:MinVer needs a restore first, because the target arrives with the package and MSB4057 on a clean checkout reads like a typo in the workflow rather than a missing restore; the release script had the same gap and now restores before it reads. And vpk rejects an empty --packVersion loudly, which is how a broken version handoff announces itself rather than shipping a package called 1.0.0. Rule 3 is untouched. The release channel still has no job, no token and no runner, and the two channels cannot see each other. What a nightly costs is written where somebody reads it before installing one: whoever can write a release here can put a build on every nightly machine, which is fine for a build being tried and is not fine for a build holding somebody's infrastructure credentials. Verified by running the job's own steps against a clone in a Linux container: DodoSSH.Desktop.Nightly-win-nightly-Setup.exe, and an index naming pack id DodoSSH.Desktop.Nightly at 0.0.1-alpha.0.144. The upload itself is the one step not exercised — it needs a real forge and a write token, and check 16.10 is what walks the half no runner can. |
||
|
jaap-jan
|
8591035170 | Merge branch 'claude/pipeline-curl-not-found-97e70f' | ||
|
jaap-jan
|
ca7fee2358 |
Start the confirmation Android hands back, so an update can install
Pressing INSTALL closed the application, installed nothing and said nothing. That is two independent faults in one method, either of which breaks it on its own, and they hid each other: the first kills the process before the second can be observed, and the second is silent by construction. The pending intent handed to commit was implicit — an action string with no component behind it. A mutable pending intent may not wrap one of those from API 34, and this head targets 36, so every current phone threw IllegalArgumentException before commit was reached. Nothing caught it, so it left the command handler, passed the dispatcher and took the process with it. That is the closing. Below 34, where it did not throw, it still installed nothing. An application holding REQUEST_INSTALL_PACKAGES rather than the privileged INSTALL_PACKAGES gets no verdict back from a commit: what the platform answers first is STATUS_PENDING_USER_ACTION, carrying the activity that draws the dialogue in EXTRA_INTENT for the application to start. Android does not draw it on its own. The comment here asserted the opposite — that a pending intent is required whether or not anything listens, and that nothing needed to — so no receiver was ever written, and the session was written, committed and left staged forever. So there is a receiver now, not exported because the only sender is this application's own commit, and the intent naming it is explicit, which is the same change that stops the throw. Sessions are abandoned when anything fails, since one created and neither committed nor abandoned stays staged against a per-application cap — a repeating fault would have started failing at CreateSession instead, which is the same bug wearing a completely unrelated face. The reporting is the part worth keeping even after the cause is gone. Where applying ends the process an exception has nowhere to go; where it does not, which is this head's whole shape, it goes out through the dispatcher. RestartNowAsync now answers the way CheckNowAsync already did, and the regression test asserts the absence of a throw rather than the presence of one. ADR 0014 rule 6 gets the correction in place: "asks Android to ask" is one step longer than it reads. Check 17.5 needed no rewording — it asks for the installer appearing by name, which is exactly the thing that never happened — so what it gets instead is the two symptoms named, because both present as a dead button. It is the only thing in the project that can catch either, and it plainly was never run against a real pair of builds. Note for whoever takes the next nightly: a broken updater cannot install its own fix. The phone is running the code this commit replaces, so the first build carrying it has to be sideloaded by hand; the ones after that install normally. Compile-verified and manifest-verified — the receiver reaches the generated manifest — and 321 tests pass. Not run on a device, which is what 17.5 is for. |
||
|
jaap-jan
|
3d3d0bc95f |
Package the Windows client in CI, on the runner that could not
The build job published a win-x64 tree and stopped there, so the half of a release that fails in ways a compile cannot see was proved by nobody until a person was midway through cutting one. It now packs as well: vpk opens the published binaries and verifies the main executable really calls VelopackApp.Build().Run(), which is the check worth having — a refactor that drops that call compiles, tests green, and produces an application that silently never updates itself. The file said this was impossible on Linux, and also said it was fine, in comments forty lines apart. The claim that vpk needs Windows tooling to stamp the Setup.exe stub is the one that was wrong: vpk cross-compiles when told to, and the telling is a bracketed directive before the verb rather than a flag. Plain `vpk pack --runtime win-x64` on a Linux host refuses outright and says so in the message that names the fix. `[win]` must be quoted, or the shell reads it as a glob matching any one of w, i and n. Only signing needs Windows, and nothing here is signed yet. Fixing that does not move ADR 0013 rule 3 an inch, which is why the two reasons were recorded separately in the first place. What may not live on a runner is the token, not the build: Velopack clients apply what their feed serves without verifying a signature, so whoever can write a release can ship an update every install runs. The packages go to RUNNER_TEMP and die with the job. They are not offered as workflow artefacts either — an installer nobody has run should not sit somewhere that invites passing it on. Written out as shell rather than by calling scripts/release-windows.ps1. That script is a person's procedure and holds things a runner must not have and must not skip: it refuses a dirty tree, insists HEAD is tagged, downloads the previous release for deltas, and asks for the forge token. Calling it would mean either weakening it with CI switches or having CI satisfy conditions that only make sense at a desk. The constants the two now share — pack id, title, authors, channel, icon — are a contract with VelopackUpdateChannel and with every installed client, and both sides say so. Two things fell out of running the steps rather than reading them, and both were in code nothing had ever executed: dotnet msbuild -getProperty:Version answers 1.0.0. Without a target named it evaluates the project and runs nothing, and MinVer computes inside a target — so the read comes back as the SDK default on a full checkout with every tag present. That line is the tag check in this file, which is `if:` a tag ref, and there are no tags yet: the first release ever cut would have been refused by its own guard, which would then have blamed fetch-depth. release-windows.ps1 had the same line and would have demanded HEAD be tagged v1.0.0. Both now pass -t:MinVer. And MinVer answers 0.0.0-alpha.0.N until that first tag exists, which vpk rejects outright as below 0.0.1 — so packing the true version could not have worked on any build made today. The patch digit is lifted for the throwaway package only. The release script gets no such floor and must not: its version is the one users compare against, and there the refusal is the right outcome. Verified by extracting both steps from this file and running them against a real clone in a dotnet SDK container: Setup.exe, the portable zip, the .nupkg and releases.win.json, from a machine that is not Windows. |
||
|
jaap-jan
|
253c72d2b7 |
Name the organisation the repository actually lives in
It moved to DodoTech-Public, and every address in the product still said DodoTech. That looked like it worked, which is the part worth writing down: Gitea leaves a 301 at the old path and HttpClient follows a redirect on a GET, so both update channels would have kept polling through it. What a 301 does not survive is a POST. `vpk upload gitea` publishes the desktop release by POSTing to that URL, so the stale address would have failed at the one step the whole feature depends on — and a redirect is a thing an operator can delete, which turns "works today" into the same silent outage this session has already spent two commits on. So both channel constants, both release scripts, the workflow's REPO, the image's source label and the curl in phase 16 all name the live path. The local remote too, which had been printing a redirect warning on every push. Measured after the move: the org, the repo and the nightly release all answer 200 anonymously, and that release now carries both assets — the manifest and a 54 MB APK. The upload going through also answers the open question about the reverse proxy's body-size limit, which nothing local could test. |
||
|
jaap-jan
|
9a7e3bbd5c |
Let a failed update check say so, instead of reporting good news
The phone reported every build as current because the release repository is private. Gitea answers 404 rather than 403 for a repo you cannot see, the client reads that address anonymously, and AndroidUpdateChannel caught the failure and returned null — which IUpdateChannel documented as meaning "this build is the latest". The check had never once succeeded on any phone and nothing anywhere said so. Two faults, and the second is why the first lasted. The seam said null was the honest answer for an unreachable channel, on the reasoning that the caller does the same thing either way. That is true of the six-hourly pass and false of CHECK NOW. UpdateViewModel already draws the line correctly — silent on the timer, the exception's message on the button — and it could only ever draw the first half, because nothing was ever thrown at it. The desktop's channel does not catch, so the interface described neither implementation. So CheckAsync throws now, and null means one thing. A release that is reachable but missing its manifest or the APK it names throws too: "you are up to date" about a half-published feed is the same lie in a smaller costume, and the self-healing that argument protected is untouched, since the timer still swallows everything. The precondition is written down where somebody would look, rather than left as a sentence about where a token could live. ADR 0013 §4 already said a private release repository was incompatible with this design; nobody checked which side of it this repository was on. It is one curl, and manual-checks phase 16 now opens with it — pointedly not against /api/v1/version, which answers 200 from a forge that is up whatever is readable on it, and which is what made this look like nothing was wrong. Phone check 17.4 was the one that passed all along. It now presses CHECK NOW with the network off as well as on, because two different answers are the whole of what makes the first one worth reading. |
||
|
jaap-jan
|
69bc9e270b |
Let a team be joined only by somebody who is already here
An invitation decided access from an assertion about an address. Everything else
in this model decides it from something a person did — an admin naming an
account, a key holder wrapping a vault key to a key they verified — and this was
the one place a token's email claim was the thing that let somebody in.
It was guarded as tightly as that can be guarded: the claim was refused outright
on an unverified or absent `email_verified`, with no setting to relax it. But the
guard and the risk were the same shape. The whole defence was one boolean sent by
a system the deployment does not control.
So `POST /teams/{id}/members` is the only way in, and an address with no account
is refused with `no-such-account` — which is now the end of the road rather than
the signal to invite. Both clients say the remedy: that person signs in here
once, which is what creates the account, and then they can be added. The desktop
leaves the address in the box, because a message telling you to come back later
is one you act on later.
Gone with it: the `team_invitation` table, the claim hook in the sign-in path,
and `Oidc:EmailVerifiedClaim`, which that hook was the only reader of. Nothing in
the server now reads the email claim to decide anything.
Pending invitations are dropped rather than converted. Converting one would mean
creating a membership because an address matched, which is the property being
removed — and an invitation to an address that did have an account here had
already been claimed by the hourly sweep, so what is left is offers to people who
never arrived.
Two tests carry the property rather than the feature: the endpoint inventory
asserts the three routes are absent, and the API suite adds an address that has
no account, watches the refusal, then signs that address in and checks it joined
nothing. Without the second half, a server that merely renamed the deferred path
would pass.
|
||
|
jaap-jan
|
b4a6c19ac1 |
Let the phone replace itself, and give CI a channel it may sign
The Android head had no updater and no release path, and the two are one problem: Android refuses an update signed by a different key, and CI generates a fresh debug key in every container. An APK released from a workflow could be installed once and never updated again — each new one an uninstall, which on this product means losing the cache, the outbox and the device key. So there are two channels, and they are two applications because the platform gives no third option. dev.dodotech.dodossh is cut from a v* tag by a person running scripts/release-android.ps1 with the key ADR 0011 rule 1 keeps off runners. dev.dodotech.dodossh.nightly is cut from main by CI and signed with a keystore committed here in the open — a key everybody has cannot be stolen and grants nothing by being held, which is why putting it in CI does not touch the rule. Neither can update the other, by construction. See ADR 0014. The android job assumed an image with a JDK and an Android SDK on it, which is what a GitHub runner is and what this project's is not. It now installs a JDK, fetches Google's command-line tools, accepts the licences and installs API 36 — each a no-op where it is already satisfied, and each cached by the persistent runner's own disk rather than by an action that would move a quarter of a gigabyte to rebuild a directory that never left. The client reads a small JSON manifest beside the APK, the counterpart of releases.win.json, and compares Android's versionCode rather than a version name: that integer is what the platform itself uses to accept or refuse an install, so comparing anything else would offer updates the phone then rejects. It fetches, and then asks Android to ask — the system draws its own confirmation, and from API 26 will not draw even that until unknown sources is on for this application. IUpdateChannel gained ApplyingEndsTheProcess. On Windows applying replaces the files and restarts, so the shell disposes the vault first and that is what zeroes the keys. On the phone the install is a request and the answer may be no, so disposing first would answer "not now" with a locked keychain and every shell closed — a punishment for declining an update. Two measured bugs found on the way, both older than this work and both invisible to a -getProperty check. ApplicationDisplayVersion is read by the Android targets in a top-level PropertyGroup, so the target setting it from MinVer ran after the only thing that reads it: every APK ever built here said versionName 1.0.0. And nothing found so far varies the launcher name per channel — four mechanisms tried, all of them recorded in platform-flags, none of them reaching the label the launcher shows. The two channels share an icon name for now and are told apart by package name, version, and what the preferences screen says. |
||
|
jaap-jan
|
3ead865f01 |
Merge branch 'main' into the desktop updater, and give way on two numbers
Main landed a realtime push feature while this branch was building the updater, and the two collided in three places. Every one of them resolves the same way: main got there first, so this branch moves. **Two ADRs were both numbered 0012.** Main's is realtime push; this one is now [ADR 0013](docs/adr/0013-desktop-distribution-and-updates.md). Git did not call this a conflict — the filenames differ — so it would have merged quietly and left the directory with two 0012s and every cross-reference ambiguous. Renumbered here along with the nine places that point at it. **Two manual-check phases were both numbered 15**, and that one git did catch. Main's "Changes that arrive without a timer" keeps 15; installing and updating the desktop client becomes Phase 16, with its checks and every reference to them renumbered. The file's own rule is that a number is for life, which is exactly why the one that had not been pushed is the one that gives way. **The merge rewrote several files with CRLF**, and `.editorconfig` asks for LF on everything except `*.ps1`. That is not cosmetic here: IDE0055 is an error and `EnforceCodeStyleInBuild` is on, so it failed the build on three lines of App.axaml.cs whose only change in this branch was an ADR number in a comment. Forty-six files normalised back to LF; the release script keeps CRLF, which is what `.gitattributes` and `.editorconfig` both already say for a PowerShell file. Nothing else conflicted. The updater does not touch the sync loop or the event stream, and the one file both sides edited heavily — MainWindowViewModel — merged without a hunk in common. Verified after merging: the solution restores locked and builds clean, and 304 shell, 100 layout, 54 session, 28 client-api and 25 contracts tests pass. The first two counts are higher than before the merge because main's own tests came with it and pass alongside these. |
||
|
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.
|
||
|
jaap-jan
|
4b706bc3c3 |
Say when a vault has moved, so nobody waits out the minute
The delta pull was cheap enough to run on a timer and the client did, once a minute. That is fine for a machine and wrong for two people: an edit a colleague makes is up to a minute stale, which is long enough for both of them to make it and produce a conflict neither needed to have. Shortening the interval is the obvious answer and the wrong one — it costs a request per client per interval whether or not anything happened, and it converges on a busier server that is still late. So the server now says so. A client holds a WebSocket open at GET /api/v1/events, subprotocol dodossh.events.v1, and gets a line down it when something it can read has changed. ADR 0012 has the reasoning; three parts of it are worth repeating here, because they are what everything else rests on. **What crosses the socket is a notice, never data.** A frame names a vault and how far its change log has got. No item, no ciphertext, not even which item it was. The client's answer is the delta pull it would have run anyway, so there is still exactly one code path that applies a change to a keychain, and it is not this one. Pushing the items themselves would save a round trip and fork that path in two, with the cursor, the merge and the tombstone rules duplicated across both — ADR 0003 put every mutation through one write path for that reason, and this keeps every read on one for the same one. It also makes a dropped notice harmless, which is what lets the fan-out below be as simple as it is. **Polling stays, and is what guarantees a pass.** The minute timer is unchanged. A network that eats WebSockets, a server with Events:Enabled off, an older server, a proxy that will not upgrade, a notice dropped under backpressure — every one of those leaves a client behaving exactly as it did before this commit. Nothing is reachable only over the socket and nothing is meant to become so; VaultViewModel's AutoSyncInterval remark now says that where somebody changing it will read it. **The bearer token authorises the upgrade, unlike the relay's ticket.** Not an inconsistency with ADR 0004: the relay's socket is a byte pipe whose whole authorization decision — which host, which IPs, which port — is made before it opens and never revisited, and it is the extraction seam for a process that must hold no ACL code. This one is a view of the caller's own vault list and has to keep answering "what may this account read" for as long as it is held. A ticket would carry that answer in a token and be wrong the moment the account's access changed. The two bounds that arrangement needs are met rather than waved at: the socket is closed at the token's exp with close code 4401 and the client comes straight back with a fresh one, and the vault set is re-resolved every few minutes as well as on the changes known to affect it. Both bound *metadata*, because a notice contains nothing else and reading a vault still needs a key this server has never held. **The fan-out.** VaultEventHub is a singleton holding the sockets this node accepted; publishing walks them and asks each whether it cares, rather than keeping a vault-to-subscriber index that every re-subscription would have to move entries between under a lock publishing also takes. At a few hundred sockets per node and an event rate bounded by how often people edit keychains, the walk is not measurable and its races are obvious. Per-connection queues are bounded and drop the *oldest*: a notice means "pull vault X, which is at least at sequence N", so the newest subsumes what it displaces and the client's answer is identical either way — which is what lets the publish path be void, never block, and never fail. Announced from the endpoint rather than from SyncService, and that placement is the point: by then the push has committed and released the per-vault advisory lock. From inside it would name a sequence no reader can see yet and would hold the lock that serialises writers across a socket write. Only the highest *applied* sequence, so a batch of pure conflicts announces nothing, and a duplicate — already announced when it first landed — announces nothing either. Grants and membership publish too, and those take the *recipient* rather than the actor. This is what AdmitNewVaultsAsync has been apologising for since sharing shipped — "the recipient is handed nothing, there is no push channel" — and the README with it. A vault shared with somebody now turns up as it is shared. The comment and the README paragraph both say what is true now, and both keep saying that the pass is what *discovers* the vault, because a client with no socket has to arrive at the same place. **On the client**, VaultEventStream is really a reconnection policy wrapped round a ClientWebSocket: a dropped socket is the ordinary case here — laptops sleep, proxies time out, tokens expire, servers are redeployed — so nothing in it treats a failure as exceptional, and every path ends in "wait, then dial again". A connection that lived long enough to say hello resets the backoff, so a laptop that woke, worked, and lost its network an hour later does not inherit a minute-long wait it has already proved it need not take. A 4401 close skips the backoff entirely and asks the token provider again, which is the whole reason that close code is distinct. A server that does not advertise the events feature gets IdleVaultEventStream, which never delivers — so IVaultServer.Events is never null and every caller stays on one shape, because the correct behaviour without a socket is the behaviour with a silent one. The shell's background loop now selects between the timer and a notice, and both waits are held across iterations. That is load-bearing rather than tidy: PeriodicTimer permits one outstanding WaitForNextTickAsync and throws on a second, and an abandoned channel read stays registered and consumes the next notice written. Either defect leaves the first notice working and every one after it silently lost, which is why NoticesKeepWakingTheLoop_NotJustTheFirst pushes three and not one. Notices are coalesced over a quarter of a second, so one person's save — a host and its log entry are two items — and a colleague clearing a folder each cost one pass rather than a dozen. **The kind is a string, not an enum**, and that is a compatibility decision. UseStringEnumConverter throws on a value it does not know, so a newer server sending a kind an older client had never heard of would not add an unreadable frame — it would break that client's socket outright. A string is ignored instead. ProblemCodes is the same shape for the same reason. **Tested on both sides, through the real pipeline.** The endpoint suite opens a genuine socket against TestServer and proves a push produces a notice, that another account's push does not reach it, that a ping is answered, and that a frame this server cannot parse does not end the connection. Two of those assert on *ordering* rather than on absence within a timeout — the stranger's write goes first, so a socket that leaked would have announced it before the one the test waits for — because "nothing arrived in two seconds" is a test that passes on a slow machine for the wrong reason. And ANoticeCarriesNoCiphertext asserts on the bytes that crossed the wire rather than on the record's fields, since the latter would only prove that this type has no payload member, which is a tautology; the former is what catches a field added later without anybody thinking about disclosure. The client suite drives VaultEventStream through an injected connector, because the one thing a test cannot do to a real network is make it fail on cue — and failure is the entire subject. The shell suite proves a notice produces a pull inside ten seconds against a sixty-second timer, so the timer cannot be what caused it. **Two limits, stated rather than left to be discovered.** Fan-out is in-process, so a deployment running more than one API replica only pushes for writes its own replica handled and the rest arrive on the timer. IVaultEventPublisher is the seam a PostgreSQL LISTEN/NOTIFY backplane implements and it is deliberately not implemented: an untested backplane is worse than a documented gap, and multiple replicas degrade to the behaviour before this commit rather than breaking. And a client is notified of its own writes; it pushed, so it already pulled, and the extra pass finds nothing. Suppressing that echo correctly needs a per-device identity on the socket, and the same user's other machines must still be told. Manual checks phase 15 covers what no test here can reach, which is the network in between: a proxy that will not upgrade, one that drops an idle socket without telling either end, a laptop lid, a token expiring. Every one of those is invisible inside a test host, and every check there passes only if the change arrives quickly *and* still arrives with the socket taken away. ADR 0012 also fixes one thing about the shared terminal session this is the transport for, so it need not be renegotiated later: session data will be binary frames on this same socket, because base64 in a JSON envelope is the wrong shape for the one payload here that is continuous rather than occasional. Two questions it explicitly does not answer by implication — whether those bytes go through the API at all, and what end-to-end encryption means when the second party watches a stream rather than holding a key — are ADR 0001 questions and get their own decision. 1512 tests pass. DodoSSH.SystemTests was not run — it needs the whole compose stack — so the end-to-end path is unverified for this change beyond what the manual checks describe. |
||
|
jaap-jan
|
e9cea2ccbc |
Let a shared vault arrive, a bucket be found, and a vault be deleted
Three things a user reported, one of which was a real bug and one of which was
not the bug it looked like.
**A vault shared with somebody never reached their machine.** The grant was
correct at both ends: the sharing client verified the recipient's key against the
key log and wrapped every generation to it, the server stored it, and /me would
have returned it. Nothing asked. VaultSession.RefreshVaultsAsync — the method
whose own summary says it is "called after a share and on a periodic pass" — had
no caller anywhere in the application, so the vault list was whatever the last
browser sign-in cached. A restart did not help: an offline unlock reads that same
cache. The vault appeared only if the recipient happened to sign in through the
browser again, which is why this looked like sharing being broken rather than
like a list that was never re-read.
So every synchronisation pass now re-reads it, before it syncs. SyncOnceAsync
takes the whole server rather than its sync half for that reason, and the order
matters: a vault admitted by the refresh is one that same pass then pulls, where
the other order would show a newly shared vault as an empty one until the minute
after. The shell is told only when the set actually changed — it rebuilds the tab
strip's vault menu from the session's list, and doing that on every quiet pass
would rebuild a menu once a minute for nothing.
The test needed the fake server to be able to do something no test here had
needed before: hand this account a vault it did not make. ShareVaultWithMe wraps
a real key to the encryption key this account enrolled, so the keyring opens it
exactly as it opens a real colleague's — a helper that filled the field with
bytes would let a vault appear in the list and never prove it could be read.
**Adding an S3 bucket on the desktop works, and could not be found.** The report
was that it is not possible; driving the real XAML headlessly says otherwise —
Keychain, + BUCKET, and the editor saves. What is true is that S3 is where
somebody goes looking, and from there SELECT BUCKET opened a combo box with
nothing in it and no sentence anywhere saying that a bucket is a keychain item.
From where the user was standing that is indistinguishable from an application
with no way to add one.
The empty state now says what a bucket is and offers a button that lands on the
keychain with the editor already open — navigating to the screen and leaving
+ BUCKET to be found among five buttons would be most of the same problem. The
phone gets the sentence and no button: its keychain screen reads and deletes and
edits nothing, so there is no editor to send anybody to, and naming the machine
that has one beats an empty control that reads as a screen still loading.
The keychain screen's layout test grew the two categories it never covered.
Tags and buckets arrived after it was written, and the header strip it measures
is one that has overflowed twice before.
**A vault can now be deleted.** DELETE /api/v1/vaults/{id}, gated on Admin —
the line the rename already drew, for a stronger version of its reason, since
this takes the vault from everybody in it at once. The row is soft-deleted and
every grant to it withdrawn in one write; VaultAccessService filters on the stamp
at both ends, so from that moment the vault is absent from every member's /me and
every call naming it answers 404. Their clients notice on the pass described
above.
The team behind it is archived when it owned nothing else, which is the mirror of
renaming it: a vault made from the vaults screen gets a team named after it that
nobody was ever shown, and leaving that behind would leave a membership list no
screen has a row for. That is a second call rather than one transaction —
archiving is TeamService's, it refuses while a team owns vaults, and it can only
tell that this one no longer does once the deletion is committed. A crash between
the two leaves an empty team: invisible, archivable afterwards, harmless, and a
better failure than a vault that could not be deleted because tidying up after it
did not work.
Two refusals worth stating. The personal vault cannot be deleted at either end:
it is created by enrollment, everything filed nowhere else lives in it, and no
call would make another. And the items are kept — ciphertext behind a vault
nothing will resolve, so deleting them buys no confidentiality while destroying
what an operator undoing a mistake would need.
The client drops the key from the keyring and the row from the cache rather than
waiting for a refresh, so the list is right immediately; the items stay, as they
stay for a vault whose grant was withdrawn, because a copy is on every other
member's machine too and removing these rows would be the client pretending to a
reach it does not have. The confirmation says that out loud before it is
answered. It is the one sentence this screen must not leave implied: deletion is
no more retroactive than revocation is. See ADR 0001.
Desktop only, deliberately. The Android vaults screen offers no rename and no
hand-over either, so adding delete alone there would be the one destructive vault
operation on a screen with no other.
Three places asserted that a vault can never be deleted — TeamService's refusal
message, the TeamNotEmpty problem code, and ADR 0009 — and each now names the
route instead.
|
||
|
jaap-jan
|
a0568d4c35 |
Merge branch 'main' into the vaults screen, and let it rotate keys too
Main built vault key rotation while this branch was reshaping the screen that would drive it, so the two met in the same three files. Every other conflict was textual and resolved by taking both; these are the ones where a decision had to be made. **The view model.** Main taught TeamsViewModel three things and this branch had renamed and rewritten it into VaultsViewModel. All three are ported rather than dropped, because each is a behaviour rather than wording: adding somebody now wraps the vault to them on the spot instead of leaving SHARE KEY to be pressed, removing somebody rotates the vault and hands the new key to whoever is left, and a share reports how many generations were wrapped. The session calls they reach — ShareTeamVaultsAsync and RekeyTeamVaultsAsync — are scoped to a membership list rather than to one vault, and they are called that way here rather than narrowed: adding somebody is a change to the list, so every vault the list carries is one they can now fetch. This screen makes lists that carry one vault, so the sentences name one; where a list carries several, naming them all is the honest report, and the members section already says the list is shared. AddMemberAsync ran two lines over the length limit once the sharing was in it, so the calls behind it moved to AddOrInviteAsync and the three-way refusal to WhyNobodyCanBeAdded — the command reads as its guards now, which is what it was before the sharing arrived. **The tests.** Main's four new cases are ported to the vault-first API, including the one that matters most: the tampered key log is corrupted *before* the add, because the add is now a route to a wrap and a test that corrupted it afterwards would be asserting about the manual route only. SelectingAVault_ListsWhoHoldsAKey now expects two holders rather than one — main's fake records the creator's own self-grant, and a key-holder list that omitted it would show the one person who can certainly open a new vault as somebody who cannot. **The README.** The limits list is six rather than four or five: main's rotation entries and this branch's "a vault cannot be deleted" describe different things and both are true. "The rekey is flagged, never performed" is gone, since it is now performed, and M3 reads *Done* rather than *Done, except rekey*. One thing worth writing down that neither side had. An invitation claimed at sign-in still leaves the key owed, where an add does not: at the moment an invitation is issued there is no account and no published key to wrap to, and the claim happens on the invitee's machine, which holds nothing. Manual check 12.1 says so, because a reader who knows adding shares would otherwise read that step as stale. 1561 tests pass. |
||
|
jaap-jan
|
8707629a6c |
Make the vault the thing you share, and ask a host which one it lives in
The teams screen listed teams that owned vaults, so sharing four servers with two
colleagues meant creating a team, then a vault inside it, then wrapping a key.
Two of those three steps are about a concept nobody arrives wanting. The screen
now lists vaults: naming one creates the membership list that carries it, named
after the vault and owned by you, and members, invitations, roles, hand-over and
key holders all hang off the vault they apply to.
Nothing on the server moved. VaultAccessService still resolves a shared vault
through team_membership and every membership call still names a team id — what
went is the requirement that anybody make one. The split the whole design rests
on is untouched and is still what the screen is built around: adding somebody
authorises the server to serve them, and only a machine holding the key can make
the vault readable. ADR 0009 keeps its decision and gains an addendum recording
which half of it a person is now asked about.
The one place the team resurfaces is a membership list carrying several vaults,
which this screen cannot produce and does not hide: the members section says so,
because "adding somebody here adds them there" is precisely the fact a
vault-shaped screen is in a position to conceal.
Two things left the interface and one arrived. Creating a team is gone, and so is
archiving one — it was only ever possible for a team owning no vaults, and a
screen whose rows are vaults has no row for one, so the button would have been
unreachable or always refused. The endpoint is unchanged and the screen states
the limit instead, since a vault cannot be deleted at all. The exception is a
create whose second call failed: cancelling that form archives the membership
list it left behind, which is a deliberate departure from this client's rule
against tidying up on the user's behalf, made because nothing else can reach it.
What arrived is PUT /api/v1/vaults/{id}. Without it the screen loses its only
editing action, since renaming the team behind a vault is invisible to everybody
who was never shown the team. It is gated on PermissionFlags.Admin — the line
UpdateTeamEndpoint already draws, because a name is what everybody in the vault
sees it called rather than part of its contents — and it renames the owning team
with it when that team carries nothing else, so the row an operator reads and the
name a user says cannot drift apart. The slug never moves, for the reason it does
not move on a team rename. The session edits its cached vault row rather than
replacing it with the response, which deliberately carries no wrapped key.
The host editor now asks which vault a host goes into, beside the name, while
adding and only where there is more than one vault to write to. It is a second
picker rather than the keychain screen's reused, and the two selections are
separate on purpose: that one is a standing preference about where new items go,
this is a field of the host in front of you, and binding both to one selection
would mean a click on the other screen could move a half-typed host. An existing
host is not offered it at all rather than offered it disabled — the two vaults
are encrypted under different keys, so moving an item is a delete and a retype.
That forced a fix worth naming. The group picker was built from the active
vault's groups whatever vault the host was being filed into, so a host put in a
shared vault could be filed under a group only its author can resolve — a
colleague would see it filed under nothing, which is the quietest kind of wrong.
Groups are now kept per vault and the picker follows the vault choice.
Two renames, because the pair they would otherwise have made is a bug farm:
ShellScreen.Vault became Keychain and VaultScreen became KeychainScreen, which is
what the rail has always labelled that screen, leaving Vault for one vault's
contents and Vaults for the vaults themselves. The enum values are unchanged;
NavRail.axaml writes them as x:Static literals.
1536 tests pass, seven more than before. Five are new on the server — the rename
endpoint's success, the team it does and does not take with it, the two refusals
and the empty name — and the client suite gains six and folds four together,
having lost the two about archiving a team.
|
||
|
jaap-jan
|
805d81dbcc |
Merge branch 'claude/team-key-share-rotate-4b2619'
Two conflicts, and the second is worth recording. main's M4 bullet gained the Android signing decision while this branch rewrote the M5 line either side of it; both are kept. The other is an ADR number collision: two sessions each took 0010, one for vault key rotation and one for Android distribution, and both are now on main. ADR numbers are identifiers — "see ADR 0010" appears in code comments as well as in prose — so leaving two would make every such reference ambiguous. The rotation ADR landed first and is referenced from crypto.md, the gaps document, ADR 0009 and the sync code; the Android one is referenced from README and android-port.md. So the later and cheaper one moves: 0010-android-distribution.md is now ADR 0011, with its title and both references updated. Nothing about either decision changes. |
||
|
jaap-jan
|
5d447da532 |
Take a rotated vault's contents onto the new key as well
Rotating a vault re-keyed the vault and not its contents, which was the deal struck last time: everything already stored stayed sealed under the generation it was written with, every remaining member kept the older keys, and the guarantee was narrowed to "nothing written from now on". That left one gap worth closing — somebody who walked off with the old key could still open old ciphertext they later got hold of — and the reason it was safe to defer is the reason it was cheap to add. A vault at mixed generations reads perfectly well, so the pass that moves items across can stop half way and be run again. VaultResealer walks the vault and rewrites each item as an ordinary upsert against the version the server holds. It never decodes the plaintext: an item is opened and the same bytes are sealed again under a fresh data key, so an item written by a newer client crosses a rotation untouched rather than being re-encoded through this build's codec and quietly losing the fields this build has no concept of. It also means nothing in the pass knows what an item is, which is why one loop covers every type including the ones added after it. A conflict is counted and skipped rather than merged — there is nothing to merge, since no content changes — and the next pass picks the item up at the version the other client left. The half that a pass over stored items cannot see is a change queued before the rotation and pushed after it, which would put a brand-new item into the vault under the key the person who just left still holds. So the push path re-seals a stale payload as it dispatches it, writing the revision back to the outbox first so that a retry sends the same bytes rather than a fresh envelope. Between the two, nothing reaches the server under a superseded generation at all. Queued items are therefore deliberately left alone by the pass: rewriting one there would overwrite the user's unpushed work with the version the server holds, which is the one thing a re-keying pass must never do. Removal runs it last, after a sync — a mirror that is behind produces a batch of conflicts instead of a re-sealed vault — and the status line distinguishes the two guarantees, because they are not the same: a vault fully re-sealed is closed to the person who left, and one with items outstanding is closed only to what happens next. Six tests, and three mutations run against them: making the re-seal return the payload unchanged fails five of the six, making the push path skip re-sealing fails the queued-edit test and only that one, and counting conflicts as applied fails the write-elsewhere test. One of the six was wrong before it was right — it modelled a third-party write by re-pushing an existing payload at a bumped version, which no real client would do, and it took reading the AAD to see that the test was lying rather than the code. |
||
|
jaap-jan
|
cba6f435e9 |
Merge branch 'claude/vault-creation-sharing-62c0b6'
# Conflicts: # README.md |
||
|
jaap-jan
|
ebb88c8ae4 |
Give the phone both pickers, and settle who signs the APK
The files screen could browse a remote and delete on it, and that was all: there is no browsable local filesystem on Android for a second pane to show, so the gesture the desktop is built around — choose on the left, press the arrow — has nothing to stand on. What replaces it is the platform's own two pickers. ADD FILES is ACTION_OPEN_DOCUMENT, so a document is pointed at wherever it lives and goes to the directory showing; SAVE FILE is ACTION_CREATE_DOCUMENT for the selected row. Both stage through the application's cache, and that copy is a requirement rather than a shortcut. android-port.md predicted a picked document would be a third IRemoteFileStore beside SFTP and S3; it cannot be. FileTransferQueue seeks, because an upload resumes from the byte the last attempt reached, and a content:// URI has no path behind it, no length worth trusting, no promised seek and no grant that survives the document being edited underneath it. Copying first costs one class in the head and nothing at all in the shared layers, where the alternative was every resume rule rewritten around a stream that cannot rewind. The copy is deleted when the transfer completes, kept while it is stopped so RESUME still has something to read, and swept at the next launch — which is the one moment emptying that directory is provably safe, since nothing has queued anything yet. Coming out had a decision going in did not: when to ask where it goes. The save picker is raised before the transfer, so the download runs into the same staging directory and hands its bytes to a callback the head supplied, held against the transfer id so a RETRY still lands where the person pointed. Asking afterwards would put the picker minutes from the button that caused it and, on a phone, usually while the application is backgrounded and Android will not show one at all. The cost is that the picker creates its file when it is dismissed, so a download that then fails leaves an empty one there; that is said on the screen, in the README and in the manual checks rather than left to be discovered. A delivery that fails keeps the staged bytes for the sweep instead of throwing away the one copy of something just fetched over somebody's network. The foreground service counts transfers now, which is the half of it that matters most here: a shell survives backgrounding because somebody is looking at it, and an upload has to survive precisely when nobody is. Queued counts as active, so putting five files in and locking the phone moves five files. The seam was built for this and wired to () => 0 because nothing could fill the queue. Alongside it, ADR 0010 answers the second question android-port.md left open, and it had to be answered before the first release rather than at upload time: a new Play app must use App Bundles and therefore Play App Signing, and an installed app can only be updated by a package signed with the same key, so the first release picks an identity for good. The project holds the key, offline and never in CI — the workflow's package step now says so where somebody would break it — and a DodoSSH deployment never serves the client, because a download link on your own server hands the binary that holds the plaintext to the party the whole threat model is about. The README's M1 gap note was stale in both halves and is replaced by what is actually true: credentials have an editor and a REMEMBER tick, and the device key registers into the TPM under a CNG policy that makes the consent dialog a condition of using it. What is left is the floor rather than a gap — no TPM, or no Windows, means the passphrase on every launch. |
||
|
jaap-jan
|
d5b1a73182 |
Move the keys when a membership changes, not just the flag
Adding somebody to a team granted them nothing readable and removing them
rotated nothing. Both were honest — the interface said so in as many words — and
both left the actual work to a button somebody had to remember to press, on a
machine that happened to hold the key. Adding now wraps every team vault this
machine can open to the new member, and removing revokes their grants and moves
each of those vaults to a fresh key that goes to whoever is left.
The rotation is where the design had to be decided rather than written. A vault
key is per generation and an item carries the generation it was sealed under, so
advancing the vault and withdrawing the old grants would make everything already
stored unreadable to everybody, including whoever pressed the button. So earlier
grants are kept: a member holds one per generation, /me serves them as
PriorKeyWraps, and VaultKeyring holds a key per generation — the newest for
writing, the item's own for reading, chosen per item on every read path. Sharing
issues one grant per generation held, because a recipient handed only the current
key would open the vault to find most of it undecryptable; revocation takes every
generation, because leaving the history behind leaves them able to read
everything written before the rotation.
The bump itself is one server transaction. POST /vaults/{id}/rekey must name
exactly current + 1 and the vault's xmin token makes that binding, so two admins
rotating at once do not both walk away believing they succeeded — the second is
refused and told to read the vault again. The server contributes the moment and
no cryptography: it cannot generate the key, cannot tell that the one it is
handed differs from the old one, and checks that the caller held the old one the
only way it can, by requiring a live grant at the current generation.
What this does not do is re-encrypt what is already stored, and the product says
so rather than the reassuring version: everything written from the rotation
onwards is unreadable to the person who left, and nothing about the past changes.
That half is deferred and is safe to add incrementally precisely because a vault
at mixed generations stays readable. ADR 0010 records the alternatives — revoking
the old grants, chaining each key under its successor, re-sealing every item in
one request against a server that caps a push at 500 operations — and why each
was rejected.
Two things fell out of the change rather than being asked for. The grant listing
would have shown a member once per generation, so it now returns one row per
holder carrying the best key they hold, which is what makes a row below the
vault's generation mean "still owed the new key". And MarkUnreadable gives up the
write target as well as reporting: a client whose vault was rotated elsewhere
would otherwise have gone on sealing items under its superseded key — readable to
its author, unreadable to everybody else, with nothing to show for it.
|
||
 jaap-janandClaude Opus 5
|
a43286ece8 |
Let a team change hands, and be joined by somebody with no account yet
M3 built teams and stopped short of the two operations that decide who controls one. Both were written down as refusals rather than omissions: ADR 0009 listed ownership transfer under "deliberately not built", and design-import-gaps said an invitation needed "a token with a lifetime and an outbound mail path". One of those reasons had expired and the other never applied — an invitation does not need a token if it is not a thing anybody presents. Handing a team over is one write. The member you name becomes owner and you become an admin, in a single transaction, because ownership is sole: promoting first leaves the team owned twice, demoting first leaves it owned by nobody, and there is nobody left with the authority to finish a transfer that stopped in the middle. That is also why it is not two calls to the role endpoint, which refuses Owner outright. The outgoing owner is demoted rather than removed — removing them would revoke their vault key grants and flag every team vault for rekey, which is a far larger act than the one asked for, and somebody handing over a team is usually staying in it. It unblocks the thing that was impossible before: an owner can now leave, by handing the team on first. An invitation is a standing instruction rather than a message. This server has no outbound mail path, so nothing is sent and there is nothing for the invitee to present. The row says the next account signing in with that address joins this team at this role, and telling them to sign in is the caller's job over a channel this server does not carry. A link nobody can deliver would be worse than none. It lives in its own table rather than becoming a membership with MembershipStatus.Invited, and that member stays unwritten for the reason it always was: team_membership.user_id is not nullable and carries a foreign key, so somebody who has never signed in has nothing for that row to point at. Widening it would make the unique index on (team, user) meaningless, because PostgreSQL counts every NULL as distinct. Verification is the security boundary, and nothing in this server read it before. A claim requires the access token to assert email_verified. An invitation decides what the server will serve, so one claimable by anybody able to obtain a token carrying somebody else's address is a way into a team — which is precisely the attack OidcOptions.AllowEmailLinking exists to refuse, and it would have been reintroduced by the back door. There is deliberately no setting that relaxes it: a flag that exists is one somebody turns on for the afternoon their provider is misconfigured. Absence is refused rather than trusted, and logged, because a provider that never sends the claim otherwise leaves every invitation pending with nothing anywhere saying why. Claiming happens at just-in-time provisioning and again on an hourly sweep. The sweep is what makes it recoverable rather than one-shot — an invitation issued between an account being created and that person next signing in would otherwise be stranded for ever — and it shares its rate with the last-seen write because both are housekeeping nobody is waiting on. Archiving is refused while a team owns a vault, and that refusal is the end of the road rather than a step on it. A team vault is readable because of membership, so archiving one that still owned vaults would take them away from everybody holding a key, including the caller, quietly and all at once. Nothing in this product deletes a vault, so no order of operations gets past it today — which is stated with a count of what is in the way, for the reason the SFTP layer refuses a recursive delete: a refusal is visible and a quiet removal is not. It is owner-only, as handing over is; renaming is not, because a rename is visible to everybody and reversible by anybody who can do it. The slug is not renameable at all: it is unique only among live teams, so a rename could take one an archived team is still holding, and that team could then never be restored. LAST ACTIVE is real and coarse on purpose. UserAccount.LastSeenAtUtc is refreshed on ordinary authenticated requests, at most once per account per hour, through ExecuteUpdateAsync — user_account carries the xmin concurrency token, so a read-then-write on the hot path would start losing races between one user's own overlapping requests. An hour is the granularity the question is actually asked at, and the interface draws it to the day rather than the minute so it does not read as a precision that is not there. The remarks in Contracts and in the view model that argued at length for the column's absence are rewritten rather than extended; both had become false. Two endpoints already existed and nothing called them. ChangeTeamMemberRole and ListVaultGrants have been reachable since M3. The role picker refuses Owner itself rather than letting the server do it, since the interface already knew the rule; the key-holder list sits under the vault rather than beside the member, because a grant is per vault and a count on a member row would imply per-item sharing, which is M5. It lists withdrawn and stale grants and says which they are — a list that dropped them would show a departed colleague as merely absent rather than as somebody whose key was taken away — and staleness is decided by comparing generations, since a grant can be Active and still open nothing. ADD MEMBER stopped being a dead end. An address the directory did not know used to end at a sentence telling the user their colleague had to sign in first. It invites them instead, from the same button, because which of the two applies is a fact about the server's account table rather than about what the user is doing; which one happened is reported afterwards, because that decides what they do next. An address that merely has an account is invited rather than refused: refusing would have made the endpoint an oracle for which addresses have accounts here, answerable by anybody willing to create a team first. The phone has a TEAMS screen, behind MORE, and it is the reverse of every other row in design-import-gaps: a shipped screen the design had no slot for. It is there because an invitation is claimed by signing in, so somebody told they are now in a team is at least as likely to be holding a phone — and a membership visible only on a head they never installed is one they cannot see. It draws SHARE KEY and nothing that takes something away: wrapping a key is the one act on that screen a server cannot perform at all, and the desktop guards its revocations with a tooltip, which is a control a touch screen cannot show. Two defects were found by an adversarial pass and both were green against the whole suite at the time. The owner-only check on archiving and handing over had been weakened to the admin check while their messages and comments still said owner — and since nothing behind the archive endpoint re-checks it, an admin the owner had promoted could have archived the team out from under them. And the rename endpoint built its response with a hardcoded Owner role, so an admin who renamed a team was handed a summary claiming they owned it, and a client trusting that instead of re-listing would have offered them the two owner-only buttons the server then refuses. The new table gets its constraints tested rather than merely migrated: live uniqueness per (team, address), the citext proof that an address typed by a person matches one cased by a provider, and reissue after both revocation and acceptance. The teams screen gets its first entries in the layout suite, at the minimum window with every list populated and with each of the two states that cover half of it — it had none, and it just grew four sections and a second line in the member row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
jaap-jan
|
5cbda59a34 |
Merge branch 'main' into claude/host-management-ui-plan-7f20ab
Seven files needed a hand. Most were two branches adding something in the same place, but three were one branch changing what the other had moved or renamed, and those are the ones worth reading. The shell keeps both new fields and both constructor lines: the connection recorder this branch built and the teams view model main did. Where main put a teams load inside OnScreenChanged, it now sits beside the logs refresh rather than inside RaiseSurfaceState — this branch extracted that notification block and it is called from two properties, so a screen-specific side effect in there would fire on every terminal switch as well. Main gave four row types a vault id and a vault name, and this branch had moved one of them — KnownHostRowViewModel — into its own file when the pinned keys became a screen. Git resolved that as "deleted here, modified there" and took the delete, which compiles as long as nobody looks: the moved copy still had the two-argument constructor and the call site had grown to four. Carried over by hand, along with the ordering the pins list now does on them. The status line's quiet rule was the subtle one. Main extracted it into IsWorthReporting; this branch had changed the same condition to read item counts rather than raw ones, because every user action queues a log entry a moment later and this machine reads its own entries back on the next pull. Take main's structure and the merge builds, passes, and silently restores a bug this branch existed partly to fix — every save's message overwritten a second after it appears. The method now reads PulledItems and PushedItems, with the reason in its remarks. Two conflicts were prose that had gone stale rather than code. The keychain screen's comment said team vaults are refused by the server's access service, which was true when it was written and is not now; main's replacement stands, in this branch's vocabulary. The design-gaps row for groups was claimed by both — real host groups here, per-vault headings there — and they are different things, so both rows stay and the difference is stated: a group is a shelf the user chose, a vault is who can read the item. One defect the tests found and the compiler could not. Generating a key opens the same editor as pasting one, but not through NewKey — so it never set the target vault main added, and a generated key was filed into whatever vault was edited last, or none. Both key-generation tests failed on it. Fixed where the editor opens, with the reason recorded there. One gap is left deliberately and is written down rather than half-built. Hosts, keys, credentials and pins are read across every vault this session holds a key for; groups are read from the active vault alone, so a host a teammate filed shows under UNGROUPED. Nothing is lost or misfiled — it is what the sidebar already shows for a group that has been deleted — but closing it needs a vault id on every group row for rename and delete, and a way to tell two vaults' identically-named groups apart under a layout with one heading per group. Both are worth doing and neither is a merge's business. It is in the remarks on ReloadGroupsAsync and in docs/design-import-gaps.md. dotnet build, dotnet test and dotnet format --verify-no-changes are all clean: 1282 tests, including the end-to-end suite against real containers. |
||
|
jaap-jan
|
d07b336868 |
Free the terminal from the Hosts screen, and fill the room it left
The WebView sat inside the Hosts grid, so navigating to Files or the keychain hid every open terminal and the strip that named them. A connection you had opened was invisible from four of the five screens. The window now has two surfaces rather than one: a nav rail that says which page you are on, and a terminal strip that is always there and switches the whole content area to a shell. Screen keeps meaning "which page" and never becomes a sixth kind of page, which is why this is two properties instead of one enum with a terminal member in it. Every screen lives inside one wrapper panel that collapses when a terminal is showing. That is not tidiness — the WebView hosts a Win32 child window that composites above everything Avalonia draws, so a screen left visible over its rectangle is a screen sliced in half, and this window has shipped that defect once already. One decision point, IsTerminalShowing, and a nested panel rather than five compound bindings nobody would remember to extend. The focus choreography is the part no test in this repo can see. Every reveal path now focuses in the same turn the WebView appeared, so all three of them post at DispatcherPriority.Loaded and let the native control re-push its bounds first. Going the other way had a real bug: the screen-changed branch called a bare Focus() where it had to release the keyboard from the native child, so switching from a terminal to Files silently ate the first keystrokes. Rare before this commit and the primary gesture after it. The tab strip grew a cross inside each tab, a plus that opens the quick-connect palette, and middle-click close. Nested buttons are correct here: Avalonia handles a left press on the cross and deliberately does not handle other buttons, which is exactly what lets middle-click bubble up from the cross as well as the tab. The test is PointerUpdateKind rather than IsMiddleButtonPressed, because the latter reports button state and is also true for a left press made while the middle button happens to be held. The handler is on the tab and not the strip, so the background closes nothing by construction. Plus opens the palette rather than a flyout, since a menu dropping into the WebView's rectangle may or may not composite above a child HWND and this repo does not make rendering claims it has not photographed. Everything a user reads now says keychain. The wire, the database and the cryptographic spec still say vault, deliberately: renaming those is a migration and a protocol change for a word. That split is written down rather than left to be rediscovered as an inconsistency. Four things that were squeezed into the keychain's category rail, or into nothing at all, now have screens. Pinned host keys get one, with fingerprints never truncated and a filter that matches them, because comparing what you have against what the operator published is the whole workflow; the approved date is read out of the item's UUIDv7 rather than added as a column, and says so, since it means first approval and not last use. Keys can be generated in the client, which needed the openssh-key-v1 container written by hand — there is no BCL or NSec helper, and the PKCS#8 route is unverified in the SSH library this uses. The armour carries no passphrase: encrypting it needs bcrypt_pbkdf, which is Blowfish with a swizzle, in a project whose crypto is otherwise entirely libsodium, for a protection the key's own remarks argue is redundant inside a vault. Generation fills the existing editor and stops, so SAVE stays the one thing that writes. ~/.ssh/config can be imported behind a preview that is ticked per row and writes nothing until the button; IdentityFile records the path and imports the key material only on an explicit opt-in, because reading somebody's private key into a vault is precisely the act this product exists to make deliberate. Match blocks and ProxyJump are reported rather than obeyed — one cannot be evaluated statically and the other has nothing behind it to route with, and a preview that implied otherwise would be worse than one that admits it. Files can be dragged in all four directions that are honestly available. Remote to Explorer does not ship and is not pretended to: the shell wants the bytes during the drop, which needs a virtual file and a native COM data object, outside what Avalonia offers. Note for the next person that Avalonia 12 replaced the drag model outright — DataObject and DataFormats are no-op stubs and IDataObject is not in the reference assembly, so every tutorial written for 11 does not compile here. Hosts can be grouped, flat and never nested. A parent id merged as a scalar lets two offline clients each re-parent A under B and B under A, producing a cycle inside an encrypted payload that no server can police and every reader would have to detect for ever. Membership lives in that payload rather than in the one plaintext concession ADR 0001 allows, whose test is that the relay cannot function without it — nothing on the server reads a group, so what plaintext would hand over is a clustering of the estate for nothing. The plaintext column reserved for it is dropped, provably always null, and the server now refuses a client that sends one; it was never populated, was copied on apply, and was not cleared on delete, so a group id would have outlived the host it described. Snippets insert through xterm rather than through the pump, because xterm is the only thing that knows whether the remote has bracketed paste on, and that is what makes a shell treat embedded newlines as text instead of as execute. The host process moves opaque bytes and never parses output, so it would have to guess, and guessing wrong runs every line. Running is off by default and the copy says the text goes into whatever is there — the terminal has no notion of being at a prompt, and may be in vi or at a password prompt with echo off, so the Enter the user presses themselves is the entire safety property. Connections and keychain changes are recorded as synced encrypted items, which is what makes them auditable by a team later and costs the server knowledge of connection rate and timing from row counts alone. ADR 0001 already concedes it cannot hide that class of metadata; the trade is now written into it rather than left implicit. A connection entry is written once, at close, which is what makes a synced log tractable: nothing to merge, one outbox row, no chance of colliding with itself. Live sessions come from memory, not from the log. The write is void by contract and posts to a bounded channel, because putting an encrypt-and-write on the teardown path of every session is how closing the application comes to take four seconds. A ticket opened before a lock still closes afterwards, since a shell outlives the vault. The activity log hooks the one generic repository every kind writes through, so it cannot miss a caller — which is also why the log kinds themselves declare they are not audited, or the first entry would write an entry about writing an entry. It records the names of the fields that changed and never their values; a log with an old password in it would be a plaintext credential store with no vault around it. Retention is 90 days or 5,000 entries, whichever bites first, pruned on the sync loop rather than on a second timer. That log traffic then broke the status line, which is worth recording because the fix is a shape and not a patch: background sync counted its own log rows as pushed items, so the quiet rule stopped being quiet and every action's message was overwritten a second later by a sync report. The report now separates log rows from user items and the rule reads the latter. S3 buckets appear as a remote in the file browser, behind the same interface an SFTP session implements, so the queue and both panes did not have to learn what they are talking to. Uploads go through a pipe, because the queue wants to write and the SDK wants to read; memory is then bounded by the part size instead of buffering a file to disk twice. Finally, the Windows device key store moved out of the session project, which was the one thing keeping it from being portable — everything else in it is platform-neutral, and a Windows CNG dependency in the middle of the vault code meant a second head could not reference it without dragging Windows along. The seam that made the move free was already there. docs/android-port.md is the audit behind that: what ports, what does not, in order of cost, the four decisions taken, and an inventory of every screen and state the interface has to carry, written so a design can be made from it directly. dotnet build, dotnet test and dotnet format --verify-no-changes are all clean: 1240 tests at zero warnings, including the end-to-end suite against real containers. The manual checks that headless Avalonia cannot make — the drag from Explorer, a generated key against a real host, twelve tabs at the minimum window width — are listed in docs/manual-checks.md and are still outstanding. |
||
|
jaap-jan
|
95816de0c5 |
Share a vault with a team, without the server holding a key
M3's teams, sharing and ACLs. Teams with roles, a public-key directory, the append-only key log served for clients to check it against, team-owned vaults, and vault key grants wrapped by a client and stored opaquely by the server. VaultAccessService resolves team membership to PermissionFlags, so a viewer may pull and may not push; the desktop client reads and syncs every vault it holds a key for, and a real TEAMS screen replaces the one that said it did not exist. No migration: team, team_membership, vault.team_id and vault_key_grant have all been there since the first one, which is what carrying two unused tables bought. Membership is authorisation. A grant is access. The obvious model is one concept — "access", with a role attached, handed out by the server — and this architecture cannot implement it: a vault key is sealed to each member's X25519 key, and only a client holding the plaintext can seal it for somebody else. So "give Bob access" decomposes into a database write and a wrap, which happen on different machines. Adding a member makes the server serve them the vault; it cannot make it readable. VaultSummary.WrappedVaultKey is null in the meantime and the vault appears in their list saying it is waiting for a key, because hiding it until a grant existed would have been tidier and would have implied the server was the thing granting access. The screen says the same thing after every add, in the status line. ADR 0009 records the whole decision. Sharing verifies or refuses. A directory lookup is a claim by the server about a third party's public key, and wrapping to an unverified claim hands the vault to whoever made it — no amount of transport security helps, because the server is inside the threat model. KeyLogAudit reads the whole log, recomputes every entry's hash from its own contents, checks the chain from genesis, and refuses unless the offered key appears in it unchanged. There is no override flag: one that exists gets used on the day the log is briefly unreachable, and the resulting grant is indistinguishable from a correct one afterwards. What it still cannot promise is that the key is the right person's, so the fingerprint comes back for an out-of-band comparison and the success message says so every time. A test corrupts the fake server's log by one byte and watches the client refuse rather than warn. The roles are only the ones that are enforceable. There is no ConnectOnly, despite the design asking for one and TeamRole having room: SSH terminates on the client, so a session needs the credential's plaintext on that machine, and "may connect but may not read the key" cannot be enforced here. Shipping it as an option in a dropdown would have been a lie. Connect rides along with Read and is documented as an interface hint. Removal is named for what it does — it revokes grants and flags the vault for rekey, and claims nothing about what is already on somebody's laptop. Three things are deliberately absent, and each is a refusal rather than an omission. The rekey itself, because re-wrapping every item's data key under a new vault key needs a client holding the current one; the server records that a rotation is owed and the interface reports it, which is more honest than a button that only appears to do it. Ownership transfer, because allowing an owner to be removed without one leaves a team nobody can administer. And cross-vault host key trust: a pin in a team vault is listed but not consulted at connect time, because any member with Write could otherwise pre-approve a fingerprint another member's client then trusts silently for a host in their own vault. Scoping trust properly needs a scope on the SSH connect path, which IKnownHostStore has not got; until then the narrow direction is the safe one and the cost is in the README rather than hidden. Reading now spans vaults and writing still does not. Every list on the vault and hosts screens covers each vault the keyring opened, rows carry the vault they came from, and an edit goes back to that vault rather than to the active one — writing it to the active vault would fork the item and only show up when a colleague wondered why their change never arrived. A new item goes wherever a picker says, defaulting to the personal vault and never moving on its own, because an item filed into a team's vault is visible to that team and moving it back means deleting and retyping. The sidebar heading stops naming one vault once there are two, and each row names its own. The server checks what it can and nothing it cannot. It will not record a grant for a key its recipient no longer holds, for a superseded generation, or for somebody who is not in the team — each of those would otherwise surface days later at the far end as a tag failure indistinguishable from corruption. It does not verify the wrap or the signature, and the grant service says so: that would be a convenience and never the boundary, and would put an asymmetric implementation on a machine that is supposed to hold no keys. Two bugs the tests found. TeamsViewModel's busy gate blocked its own reload, so a team created a moment earlier was missing from the list it had just been added to. And syncing every vault turned a failure from an exception into a report, which made a background pass announce an unreachable vault once a minute — the exact behaviour AnAutomaticPassThatFails_LeavesTheStatusAlone exists to prevent. The fact is recorded and the message swallowed, as it was before; pressing Sync still names the vault and the reason. Also fixes a build break this branch started with: QuickConnectTests was never updated when M2 added ISftpSessionFactory to the shell's constructor, so nothing built at all. |
||
|
jaap-jan
|
9bc28f1c0f |
Move the API onto FastEndpoints, without moving the wire
Eight endpoints today, around sixty planned. The minimal-API shape — a static
class per area holding static local functions, route and policy and name
asserted in one fluent chain with the handler somewhere below it — has not hurt
yet, and would. A handler's dependencies are parameters rather than injected, a
group's RequireAuthorization sits far from the handler it governs, and there is
no type to hang an endpoint's own documentation on. FastEndpoints is one class
per endpoint, its route and authorization in Configure(), its handler a method
on the same type.
Nothing about the wire moves, and the evidence is that the 94 existing HTTP
tests pass with zero edits to any of them. Same routes, verbs, route
constraints, status codes, operation ids, and the same RFC 9457 bodies with the
same code values. Every place the idiomatic FastEndpoints answer would have
changed one of those, it was refused:
Endpoints are registered from an explicit List<Type>, not found by scanning.
ADR 0002 rejected reflection discovery by name, and the reason it gave is
sharper here than in general — under WebApplicationFactory the scan reaches the
test assembly, so an endpoint written in a test would be registered into the
host under test. The cost is a line per endpoint that can be forgotten, which is
what the endpoint-inventory test is for. That test is the one ADR 0002 promised
and never got.
Handlers still return Results<Ok<T>, NotFound, ProblemHttpResult> from
ExecuteAsync. The union executes as an ordinary IResult, which is what keeps
problem bodies going through the host's serialiser and IProblemDetailsService,
and what keeps the compile-time record of which statuses an endpoint can
produce. No Send.* call appears anywhere; the moment one does, a response has
left the host's serialiser.
Validation stays in the feature services. A Validator<T> short-circuits before
the handler and answers with FastEndpoints' own envelope, which carries no code
— and the code is the only part of an error the client branches on. Twenty-odd
tests assert a specific code on a 400. It is banned in BannedSymbols.txt rather
than merely avoided, because the framework's documentation leads straight to it
and it looks like an improvement.
Three defects arrived with the framework and were caught in review. All three
were green at the time, which is the part worth remembering. FastEndpoints maps
GET /_test_url_cache_ unconditionally, in every environment, with no policy and
no way to opt out; it answers with the whole endpoint-name-to-route table. It is
short-circuited to 404 — by asking routing which endpoint it selected, after the
first attempt compared the request path with Ordinal and was therefore bypassable
at /_TEST_URL_CACHE_, certified by a test that only ever tried one spelling. The
default request binder writes query-string values over the deserialised body,
which would have let ?identityProviderToken=... put an ID token in a URL and from
there into every proxy log on the path; every endpoint now binds from the body
alone. And a route value read with Route<T>() is invisible to ApiExplorer, so the
generated document named {vaultId} in a path template with nothing declaring it —
invalid OpenAPI, and unusable by the client generators the document exists for.
Two changes to the surface, both deliberate. A body that cannot be deserialised
now answers with a problem document carrying malformed-request, rather than an
empty 400: FastEndpoints' default announces application/problem+json while
sending something else, and names the failing .NET type on the wire, in a
codebase that sets IncludeErrorDetails = false to prevent exactly that. And the
route table above returns 404 where it would otherwise have answered any
authenticated caller.
Each of the three fixes has a regression test that was checked by reverting the
fix and watching it fail — four failures for the route table and the binder, four
for the document. That check is the whole reason to trust them, since all three
defects passed a full green suite on the way in.
950 tests green across 16 projects, 14 of them new and no existing test edited.
Zero warnings, format clean, locked restore clean. FluentValidation, JobQueues
and Messaging are in the graph now and none is used.
Not verified: the generated document's response schemas, which differ from
before — FastEndpoints contributes its own Produces metadata. Nothing consumes
the document yet, and MapOpenApi runs only in Development behind the fallback
policy. It needs pinning if ADR 0002's build-time artifacts/openapi/v1.json is
ever built.
|
||
|
jaap-jan
|
f86791e817 |
Finish revoking a device, instead of half of it
ForgetDeviceAsync stopped this machine unlocking without a passphrase and left
the server's row exactly where it was, so the account went on listing a device
nobody could account for. ADR 0007 recorded that as a deliberate gap needing an
endpoint. This is the endpoint, and the two things that turned up behind it.
DELETE /api/v1/me/devices/{id}. The device row is not the dangerous half: a
kind=device wrap is the user's identity bundle sealed to a key somebody may be
holding, and that is what has to go. It goes on the foreign key's cascade rather
than a second statement, and RevokeDevice_TakesItsWrapWithIt asserts the cascade
rather than trusting the configuration to keep saying so.
Scoped to the caller's own account, which is the only authorisation check there
is. The id is an unguessable v7 GUID, but unguessable is not a permission —
without the scope one user could withdraw another's device key by pasting an id
they saw once, and the victim's next launch would ask for a passphrase with no
explanation. 404 rather than 403 for somebody else's device, so a stranger does
not learn the id exists.
Never refused for being the last device. ADR 0001 makes an enrolled device a
recovery path, so removing the last one does cost the user something — but the
machine being revoked is most likely the one they have just lost, and a server
that argued about it would be refusing the one request that has to work
immediately. The passphrase wrap is untouched either way, which
RevokeDevice_LeavesThePassphraseWrapAlone pins.
--- Two things found on the way ---
Registering twice from one machine left two devices on the account. The server
is idempotent on the public key, but the client generates a fresh key pair every
call and the keystore holds one — so the second registration orphaned a wrap
whose private half had just been overwritten, which is precisely the leftover
this change exists to remove. Registering now withdraws the previous device.
Found by a test that asserted the property and failed.
And the fakes were lying about it. FakeAccountServer's comment claimed the real
service's idempotence while handing back a fresh Guid on every call, which is
invisible until something revokes by id — at which point a test would be
revoking an id the server never issued, and passing. Both fakes now issue one id
per public key and drop the wrap with the device, as the cascade does.
--- Reachable at all ---
ForgetDeviceAsync had exactly one caller and it was a test, so "Stop unlocking
here" now sits in the account bar where "Use Windows Hello here" was. Its own
flag rather than the negation of that one: a machine with no TPM and a machine
that is already registered are both "cannot register", and only the second has
anything to take back.
No confirmation prompt, deliberately. The cost of pressing it by accident is one
passphrase and one re-registration; the cost of a dialog is a moment's
hesitation at the point somebody has realised a machine is in the wrong hands.
Offline it does the local half and says so rather than refusing. Whether this
machine may unlock itself is decided entirely by the local cache and the local
keystore — the unlock path never asks the server — so forgetting here is what
actually revokes, and "you are offline, so this machine will go on unlocking
itself" would be the worst available answer. DeviceRevocation.LocalOnly is what
the interface reports and the status line explains what is left to do.
The local half runs first for the same reason, and the keystore call is the
first thing in the method that can yield: on Windows it raises a consent dialog,
and a dialog wants the thread it was called from. That ordering is currently
load-bearing and shakier than it looks — see the open device-unlock hang.
Four mutations, all caught: dropping the user scope from the server query
(1 test), skipping the stale-device revoke on re-registration (2), skipping the
server call in ForgetDeviceAsync (2), and the earlier version of the client that
never called it at all.
930 tests green across 16 projects, 13 of them new. Zero warnings, format clean.
|
||
|
jaap-jan
|
573f5d5668 |
Keep the device key in the TPM, behind a consent Windows enforces
The last of ADR 0007's three pieces, and it does not implement what that ADR originally decided — because writing it exposed a flaw in the decision. The ADR said "a Windows Hello gesture gating a protected blob". That does not deliver what the rest of the document claims for it: a gate inside the process is not a gate. A store that showed a prompt and then read a DPAPI blob would be bypassed by malware that skipped the prompt, read the file and called CryptUnprotectData itself — which is exactly the attacker the whole decision was made against, and exactly the reason DPAPI alone was rejected. The presence requirement has to be a condition of using the key, enforced below the application, or it is decoration. So the device key is encrypted to an RSA key created in the Microsoft Platform Crypto Provider — the TPM — under CngUIProtectionLevels.ProtectKey. Windows requires consent to use that key, so the prompt is not something this code can be talked out of showing. Malware can ask for the key; it cannot answer the dialog. That is strictly stronger than the ADR described, and most of what option D was being saved for: the wrapping key genuinely never leaves hardware. The X25519 device key still lands in memory to open the wrap, because DSH1 fixes that wrap at a curve the TPM cannot do — the remaining gap, and now a smaller step than it was. CngKey is in-box, so this needed no WinRT projection and no Windows target framework. Which is worth stating plainly because the opposite was planned: the piece was scoped as "where the Windows TFM lands", and it turned out a platform guard on one class was enough. Client.App and its two test projects stay on net10.0. Two things were measured on real hardware rather than assumed, and the second changed the shape of the work. The platform provider works here and holds an RSA key — confirmed by creating and deleting one before writing anything that depended on it. And ProtectKey prompts at key *creation*, not only at use. The comment in the first draft of this file said the opposite, with a confident explanation: sealing uses only the public half, so it should be silent. It is not. CngKey.Create blocks on a dialog, because the policy means "protect this key with a PIN" and Windows asks the user to set that up there and then. Found by writing tests around save and forget and watching the suite hang for ten minutes waiting for somebody to type one. That has two consequences worth knowing before touching this file. SaveAsync is user-facing code — it belongs on a UI thread, behind a button somebody pressed, never on a background pass. And almost nothing in the store can be covered automatically: two tests remain, availability and the empty-blob case, both of which provably reach no dialog. Disabling the UI policy to make the rest testable would remove the one property worth having. The interface offers two things and hides both where they cannot work. "Use Windows Hello" appears on the unlock screen only when this machine has a cached wrap and a keystore still willing to release the key; "Use Windows Hello here" appears in the account bar only when the machine can keep a key and has not already registered one, so it is spent once used. Absent rather than disabled, in both cases: a greyed-out button on a machine that never had a TPM reads as something broken, and the passphrase box beside it is not a fallback — it is the ordinary way in. Both unlock paths now share AdoptAsync rather than each opening the known-host store, building the vault and starting auto-sync. The ordering in there is load-bearing and a second copy would be a second chance to get it wrong. The shell's tests drive a fake keystore. Not for speed: the real one prompts on every save and load, so a suite using it would block forever. What the shell has to get right is which buttons appear and what happens when one is pressed, and a fake answers exactly that. It is shared from Client.Session.Tests by source link rather than reimplemented. 882 tests green, 6 of them new. Zero warnings, dotnet format clean. Not verified, and not verifiable here: the dialogs. Whether the consent prompt appears at the right moments, reads sensibly, and returns to a usable window when declined needs the application run by a person on a machine with a TPM. That is the remaining half of outstanding item #7, and it is now the only thing between this feature and being finished. |
||
|
jaap-jan
|
7016ce36f1 |
Key the local cache to the identity, not to the door it was opened through
Groundwork for a device key, and a spec change rather than a feature. ADR 0007 records the decision it clears the way for: a Windows Hello gesture guarding a protected blob, with the passphrase kept as a permanent fallback. The reason that decision needed this first is that a device key cannot open a session on its own. SessionOpener derived two things from the passphrase master key — the bundle, and the local cache key — and a device wrap is SealTo(device_x25519_pk), which yields the bundle and never computes a master key at all. A device unlock could therefore have opened the identity and still not read the cache it had itself written. So LocalCacheKey now derives from the bundle: dsh1/localcache/v1 → v2, specified in crypto.md §3.2. Every wrap that opens a vault ends up holding the bundle, so every door reaches the same cache. Extract-and-expand, not expand alone. Everything derived from the master key uses HKDF-Expand directly, which is sound because an Argon2id output is uniformly random over its whole length. The bundle's encoding is not — it opens with a fixed 14-byte label and carries a version, a generation and a timestamp before reaching any key material — so it needs the extract step to become a pseudorandom key first. Two consequences fell out, both improvements and neither the point: - A passphrase change no longer discards the local cache. The bundle is unchanged by a re-wrap, so the cache key is too. Under v1 changing a passphrase silently orphaned every cached row and the next launch re-pulled the whole vault. - Recovery-code unlock is fixed before it ships. It derives a different master key from a different secret and a different salt, so under v1 it would have had the same defect as the device path, and nobody would have noticed until it landed. The cache becomes unreadable exactly when the identity is rotated, which is the correct moment to discard it. Existing caches are discarded and re-pulled on upgrade — already the specified behaviour for a stale cache, and the reason the label is versioned rather than reused: a v1 cache must fail to open rather than decrypt to nonsense. One stated guarantee got weaker and now says so. crypto.md §10 claimed locking meant "nothing on disk can be read again without the passphrase." Where a device wrap exists that is no longer true, and it would have been untrue under either candidate design — the alternative was storing a copy of the cache key in the device blob, which is the same door with an extra key lying next to it. The wording now points at ADR 0007, because what guards the device key is a platform decision and not a property of this specification. A golden vector was quietly lying, which is the part worth reading twice. The "local-cache" entry pinned HKDF-SHA512-Expand over a fixed PRK — a construction the cache key no longer uses. Regenerating it would have produced a green suite describing a derivation this code does not perform. It is replaced by a vector over a bundle whose every byte is pinned: the label, version 1, generation 1, a fixed timestamp and two recognisable key scalars, all visible in the fixture so a second implementation can check itself against it. UserSecretBundle.TryDecode is internal for this, because Create draws fresh randomness and so can never produce a reproducible input. Mutation tested, and this one earns its keep: dropping the extract step now fails CommittedVectors_MatchCurrentImplementation. The vector it replaced could not have caught that, because it never touched the bundle at all. One test became false and says so. ARecordSealedUnderAnotherPassphrase is now ARecordSealedByAnotherIdentity: a different passphrase deliberately no longer changes the cache key, and TheLocalCacheKey_SurvivesAPassphraseChange pins that. What must still be unreadable is another user's cache. CacheHarness therefore generates an identity rather than deriving from a passphrase, and has no passphrase parameter left — the cache key is not a question about passphrases any more. SyncHarness's two simulated machines now derive the same cache key, which is what keying on the bundle means: they are the same user holding the same identity. They still have separate cache databases, so nothing is shared between them but the key that would open either. Both harnesses lost a MasterKey field that existed only to make a protector. 858 tests green. Zero warnings, dotnet format clean. Not done: the device key itself. Three pieces remain, and the middle one was a discovery rather than a plan — EnrollmentService.AddDevice runs only during enrollment, so every already-enrolled account, which is all of them, needs an endpoint to add a device wrap while unlocked. The client proves possession by producing the wrap, so that shape falls out of the crypto. After that: the protector seam with the wrap cached locally for offline unlock, then the Hello implementation and the unlock-screen UI, which is where the Windows TFM lands and where automated testing stops. |
||
|
jaap-jan
|
b15af836a3 |
Freeze DSH1 crypto specification and implement the core (M1)
docs/crypto.md is now the normative, frozen specification. This had to land before anything else in M1: the server holds ciphertext and no keys, so it can never re-encrypt, and a format change after users hold data is a coordinated client rewrite with no rollback. Specification: - DSH1 envelope layout, canonical 64-byte AAD encoding, SealTo construction, key hierarchy, Argon2id profiles, fingerprints, and the change rules for each version field. - AAD encoding is fixed-width binary rather than delimited string concatenation, so no field value can forge a field boundary. This supersedes the illustrative form sketched in ADR 0001, which now points here. - UUIDs are RFC 4122 big-endian. Guid.ToByteArray() emits the first three groups little-endian and would have made our ciphertext unreadable by any other implementation of this spec, failing only at a cross-implementation boundary. Verified rather than assumed: - PrimitiveAvailabilityTests proves X25519, Ed25519, XChaCha20-Poly1305, Argon2id and HKDF-SHA512 all function on net10.0. NSec 26.4.0 targets net9.0 and is consumed by forward compatibility; this closes one of the two package questions the plan flagged. - Argon2Profile exists because NSec's MemorySize is in KIBIBYTES, not bytes. Passing bytes gives either a 256 GiB allocation or a 256 KiB KDF that cracks instantly. The type takes mebibytes so the unit cannot be got wrong at a call site. Found by benchmarking: the first measurements were ~1000x too slow, which turned out to be 19 GiB of work. - Parameters measured, not guessed: 256 MiB/t=4 is 323 ms on this machine; the table of candidates is in the spec. Implementation and tests (83 total, up from 17): - AadDescriptor, DshEnvelope, DshCrypto (Seal/Open/SealTo/OpenSealed/fingerprints). - Decryption returns null rather than throwing: ciphertext comes from a server that is explicitly not trusted, so a failed tag is an expected outcome. - Envelope readers reject unknown algorithms and any non-zero flag bit, so an envelope that is not fully understood fails closed. - Executable form of the spec's substitution claims: a server cannot move ciphertext between resources, roll back a key generation or item version, repurpose a payload as metadata, or confuse the two constructions. - Golden vectors in tests/fixtures/crypto/vectors.json guard the format. Mutation-checked: a one-byte schema version change trips four tests including the guard. Two build-infrastructure bugs found and fixed along the way: - .editorconfig forced camelCase on const and static readonly fields. PascalCase is the .NET convention for both; the config was wrong, not the code. - The golden fixture was resolved with [CallerFilePath], which ContinuousIntegrationBuild rewrites to /_/... under deterministic source paths. It passed locally and would have failed only in CI. Now copied to the output directory and read from there. |
||
|
jaap-jan
|
ce43f397a6 |
Add ADRs 0001-0006 and README (M0)
Records the decisions the milestone plan already made, with their costs stated rather than only their benefits: - 0001 e2ee-trust-model: key hierarchy, the AAD-to-row binding that stops the server moving ciphertext between rows, and the four-layer public-key trust story. States plainly that revocation is not retroactive, that Connect cannot be a security boundary, and that the IdP becomes a key-distribution trust root. - 0002 minimal-apis: feature modules with explicit registration; capability negotiation instead of Asp.Versioning, since client and server upgrade independently when self-hosted. - 0003 sync-protocol: single write path, revision cursors, and the bigserial pre-commit sequence gap that silently corrupts sync — plus the per-vault advisory lock that fixes it and the test that must prove it. - 0004 relay-authorization: relay forwards bytes rather than terminating SSH, so zero-knowledge survives; server-resolved target IPs in the ticket to defeat DNS rebinding; why host addresses must be plaintext when relay is enabled. - 0005 no-application-layer: why the usual Application/mediator layer earns nothing here, with the trigger that would make us revisit it. - 0006 observability-stack: OTel plus built-in ILogger; liveness excludes dependencies so a database blip cannot restart the container and kill live SSH sessions. Also adds a README covering layout, build, enforced conventions and milestones. |