4300d917a852027539c84be48ade4df73bb0fa4d
80
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 jaap-jan
|
4300d917a8 |
Stop making people wait for a handshake, and give the host list a pointer
Connecting held the vault's busy gate, which meant a window that did nothing visible for as long as a machine took to answer — and against one that is merely asleep, that is the whole timeout. The gate is gone from that one command. A tab now appears in the strip in the same turn as the click, carrying "connecting…" rather than a pane, and the terminal's rectangle draws a card naming the host and the address being dialled. Every other screen stays usable, and two connections can be in flight at once. That splits the vault's one connection event into three, carrying an attempt id, because "which tab is this about" can no longer be answered by "the most recent one". The id also buys the two kinds of not-connecting their different endings: a refusal stays in the strip as a tab holding its reason, since by then the user is quite likely three screens away and a status line they are not looking at is not where a failure should end; a host key question takes the tab away and puts the window back on HOSTS, because the prompt is drawn there and a tab claiming failure would be competing with the thing about to resume it. ConnectAsync takes no CancellationToken any more, and that is load-bearing rather than tidying. A [RelayCommand] over a method that takes one generates a command that cancels the previous execution's token on every invocation — so asking for a second machine silently abandoned the first, measured as the first tab disappearing with "Cancelled." the instant the second was asked for. Giving up on a connection is closing its tab, and a session that lands after that is adopted rather than dropped: a shell running with nothing naming it cannot be closed at all. A tab is marked active on IsShowing rather than IsSelected. The selection survives navigating away — that is what makes the strip a way back to a terminal instead of a way to lose one — so a tab lit while preferences filled the window was a second "you are here" mark pointing at something nobody could see. The nav rail's own entries have always made this distinction. The host list grows the two gestures it looked like it already had. A right click selects the row under the pointer before opening a menu of Connect, Edit and Delete — the menu is on the list rather than in the item template, so its entries are the vault's own commands and not a row's, and it is cancelled outright over a group heading. Dragging a host onto a heading files it there, onto a host files it beside that one, and onto UNGROUPED takes it out of a group; the write is one field of one host through the same repository a save uses, refused while the editor is open because a drop is a gesture on the list and not on a half-typed form. Clicking a result in the palette connects, which is what a list of hosts under a search box looks like it does. It went through the shell's own command, so the pointer and Enter take one path. And the files screen's two pickers followed the vault's lists once, at unlock: a host or a bucket created afterwards could not be picked until the keychain had been locked and opened again, with nothing on screen explaining why the machine plainly in the host list was missing. They follow the collections now, re-finding the selection by id across the rebuild a sync pass causes every minute. 165 shell tests and 69 layout tests green, including the connecting tab, both failure endings, two connections at once, a connection in flight across a lock, and the right click acting on the row under the pointer rather than on the selection. The drag itself is in docs/manual-checks.md with the rest of phase 7 — headless Avalonia has no platform drag, and a test that claimed to have dropped something would pass while confirming nothing. |
||
|
jaap-jan
|
7a3a521c59 |
Give the phone the rest of its screens, and a way in
All seven screens of the design, plus the two it does not draw because it starts at an enrolled phone: naming a server, and choosing a passphrase. The five states docs/android-port.md worried about losing at 360dp are all here and none of them softened. The changed-key refusal is a full-screen panel rather than a bottom sheet, because a sheet is swipe-to-dismiss by convention and that screen must have no way forward. The recovery code raises FLAG_SECURE for its own state and lowers it afterwards, so the sentence about screenshots is true rather than decorative. The delete confirmations keep their counts and replace the row in place. Signing in works, and the seam it needed is worth more than the implementation: IAuthorizationCallback now sits between OidcClient and the loopback listener, so the two heads differ in where the response arrives and in nothing else. PKCE, the state check, discovery, the token exchange and the key binding stay one implementation — a second OIDC client would be a second place for a security bug to live. The phone registers a private-use scheme with the system rather than binding a loopback port, which on a shared device any other app can do first. The accessory key row needed TerminalWorkspace.SendInputAsync: ordinary typing goes from the renderer straight down the socket, and there was no way in for the keys a software keyboard does not have. Ctrl latches, because one thumb cannot chord, and the latch is drawn — a modifier that is on and does not look on is how somebody sends ^L to a database prompt believing they typed an l. 597 client tests green, including two new ones for the input path and one for the terminal surface command. Nothing has run on a device. |
||
|
jaap-jan
|
81e7e6d939 |
Write down what the phone found, and stop it rotting
docs/android-port.md was an audit of work not started; it now says what is built. Three of its statements needed correcting rather than extending, and they are marked where they sit: the Android version question is settled and was never as open as it looked, because Avalonia.Controls.WebView ships only a net10.0-android36.0 assembly and nothing lower can resolve it; cleartext to loopback has to be permitted explicitly, which the audit missed entirely; and the spike produced a structural change it did not anticipate, in DodoSSH.Client.Shell. A CI job of its own, because the head is deliberately not in DodoSSH.slnx and a project outside the solution is a project nobody notices breaking. It packages as well as builds: a native library with no Android ABI and an assembly that will not dex are both invisible to a compile, and both are exactly what this head is exposed to. The README says plainly that signing in is not built, that a fingerprint re-enrolment destroys the device key, that a notification appears while a shell is open, and that none of it has run on a device. |
||
|
jaap-jan
|
2caedd93ff |
Merge branch 'main' into the Android head
Main grew the screens the host-management plan called for — hosts, pins, snippets, logs, import, teams — plus the ObjectStore and Import projects behind two of them, and moved WindowsDeviceKeyStore into the desktop head's Platform folder. Five of those view models landed in a directory this branch had already moved, so they join the rest in DodoSSH.Client.Shell: git spotted the rename and put them there, and the namespaces followed. Shell picks up ObjectStore and Import as a result, which the Android head then gets transitively and will use neither of at first — scoped storage means there is no ~/.ssh/config to import, and file transfer is out of its first scope. Desktop suites green at 155 and 64. |
||
|
jaap-jan
|
fe9d7fc289 |
Give DodoSSH a phone, and a shared shell for both heads to drive
The Android head from docs/android-port.md, taken as far as its step 6. Step 3, the spike, is answered and its throwaway screen is gone: libsodium.so and libe_sqlite3.so are both in the arm64 APK, so NSec resolves its native half on Android despite shipping no Android build, and the local cache opens. Two findings the audit could not have had: Avalonia.Controls.WebView only ships net10.0-android36.0, which settles the open "which Android versions" question at targetSdk 36; and Android has blocked cleartext HTTP since API 28, so the terminal renderer needs a network security config scoped to 127.0.0.1 or the WebView loads nothing. DodoSSH.Client.Shell is new and is why the phone can exist: the view models, the terminal renderer files and the palette moved there so both heads drive one state machine and draw from one set of tokens. The desktop head is otherwise untouched and its 144 tests still pass. The platform pieces behind interfaces that already existed: the profile directory from filesDir, a device key wrapped by a StrongBox-backed key that a fingerprint releases, and a foreground service so a shell outliving a vault lock stays true on a platform that stops backgrounded processes. Sign-in is deliberately absent rather than approximated. It needs an app link, because reusing the desktop loopback listener is the attack RFC 8252 section 8.3 names. |
||
|
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
|
23eca3a21b |
Merge branch 'main' into claude/m3-implementation-57f9d7
ci / build and test (push) Failing after 2s
Three files conflicted, and two of the resolutions are more than a choice of side. QuickConnectTests had both branches fixing the same build break — main's M2 merge left the shell's constructor with an ISftpSessionFactory nobody passed. Main's version wins because it carries a comment saying why the palette never needs a session. VaultSession's conflict is adjacent edits: main added the remembered sign-in members and this branch changed SyncAsync's summary from "the active vault" to "one vault". Both kept. VaultViewModel is the one that matters. Main taught the background pass to report a sync that had to start over, on the grounds that a machine which silently re-read a whole vault has had something happen to it; this branch turned a pass into one report per readable vault. Taking either side alone would have lost the other, so ResyncedFromStart is now one of the conditions IsWorthReporting checks, per vault. Merging also broke something neither branch could have caught alone, and the build would not have said a word. SyncOnceAsync cleared LastSyncFailed unconditionally, which was right while a pass was one vault and a failure was an exception that never reached that line. A failure is now a report — one unreachable team vault must not stop the others syncing — so the flag was being cleared over a vault that had just failed, lighting the titlebar SYNCED. It is computed from the report instead, in the one place both callers go through, so the manual command gets it as well as the loop. The background pass still swallows the message and keeps the fact, which is what AnAutomaticPassThatFails_LeavesTheStatusAlone is there to hold it to. Two comments the auto-merge left describing a world with one vault in it: the SCOPES rail's, which said team vaults are refused by the access service, and the host sidebar's "One heading, for one vault". |
||
|
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
|
03e902a2d2 |
Colour the host's file rows by what their mode says
ci / build and test (push) Failing after 2s
The remote pane's NAME column was blue for a directory and plain for everything else, and the PERMS column was faint whatever it said. Two colours now come off the mode, split across those two columns on purpose: NAME says what a row is, so a file with an execute bit is green there, and PERMS says what is notable about how it is set, so a file anyone may write to is amber over the characters that actually say so. Because the two never compete for one TextBlock, a world-writable executable shows both facts instead of one winning an argument. No new blue is spent, which is what App.axaml asks for: it reserves blue for a directory, a distinct scope, and calls it deliberately rare. Both are files only, and each exclusion is a wrong answer avoided rather than a case not got to. Every symbolic link is lrwxrwxrwx by convention and its mode governs nothing — what may be written is the target, whose mode an lstat listing never fetched — so amber there would fire on every link on the host. A world-writable directory is /tmp, made safe by a sticky bit PosixMode does not render, and warning about it would be warning about the half of the mode that is on screen while the half that answers the warning is not. And the execute bit on a directory means "may be searched", which is true of very nearly every directory a host has, so green there would paint the whole pane and mark nothing. The two questions read back the string PosixMode wrote rather than carrying its nine booleans through SftpEntry as well. That is the point rather than a shortcut: two representations of one fact is how a row ends up coloured for a bit the column beside it does not show. A mode of the wrong length answers false rather than throwing, since these decide a colour and a listing is not worth failing over one. The amber is Warn rather than WarnText, which is the muted amber a warning card writes its sentences in. At 9.5px against TextFaint that one is a shade rather than a signal, and a marker nobody notices is the same as no marker. The local pane is untouched, on the grounds it already gives for having no PERMS column at all: a POSIX mode is not a fact about a file on Windows, and colouring one there would invent exactly what the column declines to print. Twenty cases in RemotePathTests, which needs no container — the execute bit in any of the three triples rather than only the owner's, the others-write bit alone, a mode of the wrong length, and the file-only rule for both questions from all three kinds. dotnet format is clean and the app and layout suites pass at 109 and 35. |
||
|
jaap-jan
|
1292084af9 |
Merge branch 'claude/delete-confirmations-becf4a'
ci / build and test (push) Failing after 2s
|
||
|
jaap-jan
|
91438fb382 |
Ask before deleting, and connect a host by double-clicking it
DELETE on a host, an SSH key, a stored password or a file on the host now puts a question where the button was, and only answering it deletes anything. It is a state rather than a dialog, which is the arrangement signing out already had and for the same reason: this is the moment that has to be able to say what is about to go before it goes. What the question says is counted rather than generic, because a confirmation that only asks whether you are sure is a click to train people out of. A key names the hosts that authenticate with it and says they will refuse to connect afterwards rather than falling back to a typed password, which is what the connect path actually does. A host discloses a terminal open on it, because deleting the host does not close the session. Every vault deletion says how far it travels and whether this machine can push the tombstone yet or is queuing it. Deleting on the host carries the strongest warning of the four on purpose: everything else here is a tombstone against a copy the server still holds, and a file on somebody's machine is bytes with nothing behind them — so that one names the full path, since a bare name identifies nothing. The armed request carries the item's entity id, so nothing that moves the selection between the question and the answer can redirect it, and answering about something that has since gone says so instead of doing nothing quietly. Disarming compares ids rather than rows, which is the subtle half: a reload replaces every row object, so the naive rule would have let the pass that runs every minute take the card away from somebody halfway through reading it. Forgetting a pinned host key is deliberately still unguarded. It costs one fingerprint check on the next connection and it is the safe direction to be wrong in — the dangerous button there is the one that adds trust, and that one is already a prompt at connect time. Discarding a stopped transfer is likewise unguarded: it removes a resumable part file and leaves the source alone. Double-clicking a host in the sidebar connects to it, wired as a gesture in the control exactly as the transfers screen opens a directory. CONNECT stays, since it is the button with the password box beside it. Ten existing delete call sites now go through arm-and-confirm helpers, and eight new flow tests cover asking first, cancelling, the counted warning, disarming on a selection change and on an editor opening, surviving a sync, and the stale-item guard. Three layout tests measure the new shapes — the sidebar card is the one card in the application a user cannot scroll — and one of them also asserts the card renders its text, because a card whose compiled bindings did not resolve would lay out perfectly as empty rows. The double-click test performs the real gesture and proves it reached the connect command through a refusal that never touches a network. dotnet build, dotnet test and dotnet format --verify-no-changes are all clean: 853 tests, including the end-to-end suite against real containers. |
||
|
jaap-jan
|
9608d73747 |
Come back from a sync position the server will not accept
ci / build and test (push) Failing after 2s
"The server returned 400: The sync cursor is not valid for this vault. Resync from the beginning." told the user exactly what to do and gave them no way to do it. The cursor is the only thing a pull sends, so the refusal was permanent: the next pass read the same stored cursor and was told the same thing, once a minute, for ever. And because the pull runs first, the exception ended the pass before it reached the outbox — so the vault stopped receiving other machines' changes and stopped sending its own. A machine that met this went quietly read-only until somebody deleted its cache. The engine now does what the message asks. A pull refused with the invalid-cursor problem code — the code, never the prose, which is free to change — drops this vault's position, writes that down, and reads the log again from the beginning. The restarted request carries no cursor, which is the one position a server cannot reject, so the retry cannot loop; a refusal of that is rethrown rather than retried, and a restart is allowed once per pull. The position is saved before the replay starts, so a process that dies halfway through begins the next one from the beginning too rather than meeting the same refusal again. The mirror is deliberately kept. Replaying rewrites every row the server still has and applying a change is a blind overwrite, so the re-pull repairs the mirror on its way past; clearing it first would claim more than the evidence supports — the position was refused, not the contents — and would leave a machine that lost its connection mid-replay with less than it started with. That leaves one gap, named in the remarks rather than left to be discovered: once tombstone collection exists, a replay stops carrying deletions older than the retention window. None of the causes are the user's doing — a rotated cursor signing key, a vault served from a restored database, a cache copied between machines — so nothing asks them to decide anything. The report carries ResyncedFromStart and the status line says the position was not recognised and the vault was read again. It is kept out of NeedsAttention, because nothing is outstanding, but the background pass breaks its usual silence for it: a sync that pulled the whole vault on a day nobody changed anything otherwise reads as a fault. The fake server grew a switch that refuses cursors the way a rotated signing key does, including ones it minted itself. Three cases: the vault is re-read and the change on the far side of the refused position arrives; the edits waiting in the outbox are still pushed in that same pass, which is the half that made this worth recovering from rather than merely reporting; and a server that refuses the beginning itself is surfaced instead of replayed against. dotnet build is clean at zero warnings, dotnet format is clean, and the sync and app suites pass — 109 and 101. |
||
|
jaap-jan
|
240aadb746 |
Merge branch 'main' into claude/vault-unlock-logout-autosync-a84c35
ci / build and test (push) Failing after 3s
Four files needed a hand, and all four were two branches adding something in the same place rather than either changing what the other did. The shell's constructor now takes both new parameters: main's SFTP session factory, which it must have because it builds the transfers view model, and this branch's optional resume handler, which stays last so every existing test that constructs a shell without one still gets a shell that can only be online because somebody signed in during this run. App.axaml.cs, ShellFlowTests and QuickConnectTests pass the pair; the layout suite keeps both of its new fields. Signing out now detaches the transfers screen exactly as locking does, and the confirmation says that an open transfer session survives it. That is the same policy both sides already argue for their own case: signing out destroys this machine's copy of the vault, not work that authenticated before it. QuickConnectTests did not compile on main — the SFTP commit added a constructor parameter and the quick-connect suite, merged from a parallel branch just before it, was still calling the old one. Fixed here rather than worked around, since the merged tree has to build. dotnet build, dotnet test and dotnet format --verify-no-changes are all clean: 980 tests, including the end-to-end suite against real containers. |
||
|
jaap-jan
|
d1700f5a34 |
Merge branch 'claude/m2-file-transfer-1b9951'
ci / build and test (push) Failing after 3s
|
||
|
jaap-jan
|
0b261c4d39 |
Stay signed in, come back online by itself, and let a machine be given up
Three things a machine that has been set up could not do. Unlock now takes Enter, which is the gesture everybody makes after typing a password and which did nothing until they found the button. Signing in survives a relaunch. The refresh token is kept in the local cache, sealed under the vault's own cache key, so a later launch resumes the session through the refresh grant with no browser and nobody present — and because it is sealed under that key, only an unlocked vault can resume it. A locked client therefore cannot reach the server at all, which is a consequence worth stating rather than working around; docs/crypto.md §3.2 records it. Every sync pass asks the shell for a connection rather than reading one captured at unlock, so a laptop that unlocked on a train is online within a minute of finding a network, with nothing pressed. Unlocking itself still never waits on a socket. Signing out empties this machine: the profile, the cached items, the outbox and this machine's device key, with the account's row withdrawn when the server can be reached. It asks first and says what it costs — the outbox count when the vault is open, an admission that it cannot be counted when it is not, and the shells that keep running either way. The vault is on the server and is untouched, which is what makes the same button the only honest answer to a forgotten passphrase, so it is on the unlock screen as well as in preferences. It cannot end the session at the identity provider, and says so. Two defects surfaced on the way. The synchronisation pass that runs when the vault opens never ran at all: the loop is started from inside the unlock command, so the busy flag it yields to was raised by that command — the first sync was a minute late on every launch. And signing in from preferences while unlocked threw an unlock screen over an open vault whose keys were still in memory. The unlock card and the new confirmation live in their own controls because MainWindow cannot be laid out headless, so markup left inside it is markup no test can measure; both are now measured at the window's minimum size in the shapes that grow. What is still unverified is the composed window itself. |
||
|
jaap-jan
|
04faef6597 |
Move files to and from a host over SFTP
M2's file transfer, built bottom-up: an SFTP session on the SSH layer, a transfer queue in a project of its own, and the two-pane browser the design asked for replacing the screen that said it did not exist. Remote listings carry names, sizes, modification times and a real drwxr-xr-x — nothing in this repository could render a POSIX mode before — and the queue moves one file at a time with progress, throughput and resume. The design import assumed this would be an SFTP subsystem channel on ISshConnection, beside the shell on a transport that is already up. SSH.NET does not offer that: SftpClient derives from BaseClient and owns its own transport, and there is no supported way to hand it an SshClient's session. So file transfer opens a second authenticated connection, and it is named for that rather than dressed up as a channel — OpenSftpAsync is on ISftpSessionFactory, not on a connection. The difference is visible to a user: the host records a second login, and a host whose password is typed each time asks for it again on this screen. It goes through the same host key gate, the same pin and the same two refusals a shell does, so a fingerprint approved for a terminal is approved here and one approved here reaches the other machines with the next sync. docs/design-import-gaps.md is corrected, and marked as the one row where what shipped differs from what it predicted. Nothing is written at its final name until it is complete. Every transfer goes to a .dodossh-part file beside its destination and is renamed into place at the end, so an interrupted transfer can never be mistaken for a finished one — which matters most for what this screen is actually for, which is copying a build artefact onto a server and then running it. A destination that already exists is refused outright rather than overwritten: the queue has no way to ask, and silently replacing a file somebody's process is serving is the worse of the two failures. The remote pane has DELETE and MKDIR so that refusal is not a dead end. A test against the container pins the assumption underneath all of this — that SFTP's rename does not clobber. Resume works within a run of the application and not across a restart, and the limit is deliberate rather than unfinished. Nothing records which source wrote a part file, and resuming one on the strength of its name matching is how a corrupt artefact gets delivered with nothing reporting a failure; a part file found at startup is started over. Making it survive a restart needs the preferences store this client still has not got. The offset a resume starts at is the part file's own length rather than the transfer's recorded progress: a cancellation can land between a write completing and the counter moving, and only one of those two is a fact about the bytes that are there. The queue and its connection outlive a lock, as shells do. LockAsync already argues that locking must not destroy work in flight — it is what somebody does when they walk away from the machine, which is exactly when a long transfer is most likely to be running — so TransfersViewModel is created once and the vault is attached on unlock and detached on lock. What locking takes is the host list, and it has to: those rows carry decrypted secrets. DodoSSH.Client.Transfer is a new project rather than more of Client.Ssh. The two answer different questions — one is about reaching a host, the other about moving bytes and what to do when moving them stops halfway — and this is the only client project that deliberately touches the local filesystem. Three defects the tests found, none of which review would have. SftpPath.Name answered an empty string for the root. NavigateRemoteAsync wrapped itself in the busy guard, so navigating from inside another command did nothing at all and the remote pane simply stayed empty after connecting, with no failure anywhere to explain it. And opening an SFTP session per test made two handshakes per test — this client learns a host key by being refused — which pushed the SSH assembly past sshd's MaxStartups and failed a different few unrelated tests each run; the session is shared through the fixture now, with the reason written where the next person will hit it. 1004 tests green across 18 projects, 24 of them new: the SFTP subsystem against the OpenSSH container, the queue against a real temporary directory and a fake host, and three more layout measurements because a screen this window has never laid out is a screen never checked. Not verified: the screen has not been looked at running. The layout harness measures it at the window's minimum in three shapes, which is the class of defect that has shipped here before, but reaching it in the application needs the compose stack, the migrations, the API and a browser sign-in. What is still absent — the status bar's transfer count, dragging between the panes, transferring a directory, and sftp over a bastion — is in docs/design-import-gaps.md. |
||
|
jaap-jan
|
f7c5096bc6 |
Keep the stub servers on loopback
Running the tests raised a Windows Firewall prompt, and raised it again from every worktree. WireMockServer.Start() with no settings listens on 0.0.0.0 and [::], and the prompt is keyed to the binary that opened the socket — so each test executable asks once per bin path, which a new worktree or a switch between Debug and Release makes new again. The three suites that hold a firewall rule on this machine are exactly the three that use WireMock; every other listener in the repository already binds 127.0.0.1. The stubs now say so explicitly. Port 0 is still WireMock's own free-port search and still comes back on server.Url, which is what each stub builds its base URL from, so the authority the API validates against and the issuer its tokens claim follow the binding rather than being pinned to a host name. Sampling the listening sockets of a full DodoSSH.Api.Tests run afterwards finds one, 127.0.0.1, where there were previously three. |
||
|
jaap-jan
|
4eaa8eae6d |
Merge branch 'claude/search-modal-closing-cd05c4'
ci / build and test (push) Failing after 2s
|
||
|
jaap-jan
|
66271faaae |
Update .github/workflows/ci.yml
ci / build and test (push) Failing after 16s
|
||
|
jaap-jan
|
9c3edb078e |
Update .github/workflows/ci.yml
ci / build and test (push) Canceled after 0s
|
||
|
jaap-jan
|
312d766c30 |
Let the quick-connect palette answer for itself
Clicking outside the palette did nothing, because nothing was listening: the wash took no pointer input at all, so the only ways out were a key and the button that opened it. It now closes on a press whose source is the wash itself, which is what separates outside from inside — a press on the card bubbles through the same handler on its way to the window, and closing on those would make the palette impossible to click into. The caret never reached the query box either. The window focused it from the view model's PropertyChanged, and that handler runs before the binding which reveals the control — measured, with the same wiring, in a replica window. So it focused a control that was still collapsed, which Avalonia treats as a no-op and does not replay when the control is revealed, and the keyboard stayed wherever the click that opened the palette had left it. Becoming visible is now what triggers it, posted rather than called: a control that has never been laid out has no visual children, and at the instant IsVisible turns true the box still reports IsAttachedToVisualTree() == false. Escape, Enter and the arrows move to the palette as a tunnelled handler. Answering them only on the window was fragile in the way that matters here: anything on the route that took a key first would silence them, and with the focus never landing in the palette the key was being pressed at whatever the opening click had focused — a focused Button eats Enter. The window keeps Ctrl+K, which has to work when the palette is not showing, and forwards the rest as the net for a press that arrives from outside the palette. Which is also why this moved out of MainWindow rather than being fixed there. Showing MainWindow initialises WebView2 on a thread it refuses, so nothing on that window can be tested — the palette shipped with no test of any kind. As a UserControl it hosts in a bare window and takes real key and pointer input, and there are now six: press on the wash closes, press on the card does not, Escape closes, the arrows move the selection without taking the caret out of the box, Enter takes the highlighted host, and the palette takes the keyboard when it appears. |
||
|
jaap-jan
|
f0002b683c |
Update .github/workflows/ci.yml
ci / build and test (push) Canceled after 0s
|
||
|
jaap-jan
|
94e11f5e38 | update packages | ||
|
jaap-jan
|
0b49cfb3c6 | Merge branch 'claude/api-fastendpoints-migration-020431' | ||
|
jaap-jan
|
19dcd4c8e3 |
Merge pull request 'Give hosts and terminals their own screen, and the rest of the vault another' (#1) from claude/dodo-ssh-design-02b8b9 into main
Reviewed-on: DodoTech/DodoSSH#1 |
||
|
jaap-jan
|
9a76eced14 |
Give hosts and terminals their own screen, and the rest of the vault another
Rebuilds the client's shell from an imported design: a titlebar and nav rail it draws itself, real multi-session tabs over the one WebView, a Ctrl+K host search, and a vault screen that merges keys, passwords and pinned host keys into one table. Hosts left the vault column for their own screen beside the terminal, which is what the design asks for and turned out to be the better split anyway. Two screens the design shows have nothing behind them yet — file transfer and teams — and say so plainly rather than rendering invented data; every other gap between the design and this build is recorded in docs/design-import-gaps.md. |
||
|
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
|
d162271a45 |
Show the host keys this vault has approved
Trust was created by the connect prompt and withdrawn from one host's editor, so a pin for a host that had since been deleted or re-addressed was unreachable from the interface entirely. It went on refusing connections and nothing in the application would admit it was there. Two of the four recorded debts were really this one: leftover pins, and no list to see them in. A fourth section in the vault column, and the first that adding one has been cheap for — three edits and two layout tests, which is what #8 and #9 were for. No editor and no Add, which makes it the only section with neither. A pin is not something anybody writes: it appears when somebody approves a fingerprint at the moment of connecting, which is the one place a person can actually check it against what the operator published. A form for typing one in would be a form for pasting whatever a man in the middle just offered. So the section exists to show and to withdraw, which is exactly what was missing. The fingerprint is shown in full, wrapped, in a monospace line. The only thing anybody does with one is compare it against a fingerprint an operator published, and half of one cannot be compared — it can only be glanced at, which is the habit pinning exists to replace. Nothing here is secret; a host key fingerprint is published on purpose. A pin no host in this vault dials is badged rather than hidden or deleted. That is the leftover the debt was about, and keeping it is still right: the address may be reached by something without a bookmark, and trust is about the endpoint rather than the bookmark. The badge is a hint and not a verdict, which is why nothing acts on it. Matched case-insensitively, because a host name is, and because a list that called DB.internal unused next to a host saved as db.internal would be inviting somebody to delete trust they rely on. Forgetting goes through the same ForgetAsync as the host editor's button, which withdraws every pin for the address rather than the selected row. Deliberate: somebody who has stopped trusting a machine has not decided to keep trusting one of its keys, and a second pin under another algorithm would go on being offered at the next handshake — which reads as a withdrawal that did not work. The status line says how many went, and the change is pushed immediately, because the other machines are the ones still refusing to connect to a rebuilt server. The list is read through the repository rather than through VaultKnownHostStore, whose snapshot is shaped for the SSH handshake: one pin per endpoint, deduplicated, no entity ids. This list has to show duplicates, because a duplicate is one of the things worth seeing. Two mutations, both caught: calling every pin dialled (3 tests), and defaulting the selection to the first row (1) — the same hazard as the credential list, since Forget acts on the selection. The selector now holds four buttons in 340 pixels, and TheSelectorIsBigEnoughToClick measures how much of that they use rather than leaving a fifth section to discover it as "a button falls outside the window". 936 tests green across 16 projects, 6 of them new. Zero warnings, format clean. Not verified: how the section looks. It joins the list in outstanding item #7. |
||
|
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
|
d17a60e7c3 |
Stop asking the server to delete things it has never seen
Add a host on a laptop with no network, change your mind, delete it: the outbox holds a tombstone for a row the server has never heard of, the push answers Invalid, the change is parked, and the user is left looking at a rejected change for an item they already deleted and a pending count that will never reach zero. It applies to all four item types, because they all go through the one generic repository — the known-host path is only the likeliest way to meet it, since trust is pinned by connecting and withdrawn from the host editor. DeleteAsync now drops the queued create instead, when the server cannot be holding the item. A null expected version means the row is a create — including a create that has since been edited, because coalescing keeps the original expected version — so there is no server row and no mirror row, and dropping the queued change makes the item genuinely gone. The attempt count is what makes that safe rather than merely convenient. Nothing sent cannot have landed. A parked row cannot have landed either, because parking is what the pusher does when the server has refused, so the refusal is the evidence — and a parked create that the user then deletes could not be got rid of at all before this: the tombstone replacing it was parked in its turn. What is left is a create that went out and whose answer was never seen. That one still gets a tombstone, because the server may be holding the item and a local drop would strand it there for ever. A refused tombstone is recoverable; an orphan nobody can see and nobody can delete is not. Eight tests, and the interesting half is the other direction. A repository that quietly dropped tombstones would pass a suite written only around the bug and would lose data on every machine but the one that pressed the button. Which is not hypothetical, because the mutation pass found exactly that hole in the first draft of these tests. Removing the expected-version guard left every test passing: after a sync there is no queued row at all, so deleting a synced item never reaches the shortcut and proves nothing about it. The way to hold an unpushed Upsert over an item the server holds is to edit it offline, and EditingASyncedItemOfflineAndThenDeletingIt_StillQueuesATombstone is the test that was missing. Without the guard it deletes the item here, leaves it on the server, and the next pull brings it back. Three mutations, all caught now: removing the shortcut (5 tests), removing the expected-version guard (1), removing the attempt-count guard (1). The Upsert check itself is conservative rather than load-bearing — a queued Delete with no expected version is not reachable from the interface, and completing one locally would discard a tombstone that might be needed, so it stays and is not independently covered. 106 tests green in Client.Sync, 8 of them new. Zero warnings, format clean. |
||
|
jaap-jan
|
da7462e41f |
Show one kind of vault item at a time, and let the vault hold passwords
Outstanding items #8 and #9, in one commit rather than two. They are separable as work and were built in that order, but not as a diff: the section enum has three members, the one-editor guard has three arms, and the picker offers keys and credentials from the same list. Reconstructing an #8-only state would mean hand-writing an intermediate version of VaultViewModel that never existed and that no test has ever run. One honest commit beats two invented ones. --- #8, the type selector --- The column showed two lists and two editors stacked in 340 pixels, and only just: the key list needed a MaxHeight and had to hide itself whenever its editor opened, both to stop the host list above it pushing the buttons off the bottom edge. Credentials would not have fitted at all. It now shows one kind at a time, chosen by a selector at the top, and both workarounds are gone because a section owns the whole column. Three departures from the plan, each with a reason found while building it. The selector is plain Buttons and a parameterised command, not a TabControl, a TabStrip or a ListBox. All three of those hold the selection themselves, so a click moves the highlight before the view model can refuse it — and this column does refuse, while an editor is open. A selector lit on a section the column is not showing is worse than the refusal it would be hiding. Buttons carry no state and cannot disagree with the vault. The one-editor-at-a-time rule survives with its justification replaced. That rule was a workaround for the sizing problem above, and sections dissolved it: the editors are in different sections and only one section is ever laid out. BothEditorsAtOnce_DoNotFit_WhichIsWhyTheRuleExists is now BothEditorsOpen_NowFit_BecauseOnlyOneSectionIsLaidOut — the same test, inverted, because its own comment said that if it ever started passing the rule had become unnecessary. It has. The rule stays for a better reason: an open key editor holds a pasted private key in a bound string, and letting the column move on would leave key material in a form nobody can see, with nothing on screen to say it is there. A sizing hack became a rule about not hiding a secret from the person holding it. KeyEditorIsInTheWay and HostEditorIsInTheWay are one AnEditorIsInTheWay, called by the section switch and by every editor-opening command. And releasing the keyboard from the terminal has never worked. MainWindow takes Win32 focus off the WebView's child window and then calls Focus() on VaultColumn.KeyboardTarget — and a ListBox is not focusable by default in Avalonia, which leaves focus to its items. So the call returned false, the window ended up with nothing focused, and the keystrokes went nowhere: exactly the state that method's own comment says its second half exists to prevent. Found by writing the test to assert focus was taken rather than that the right control was named — the cheap assertion was already passing. Fixed with Focusable="True" on every list. --- #9, credentials --- Credentials have synced since they were added and could not be created. They can now, and the sync layer needed no change at all: fourth item type, same result, which is the item-kind seam working as intended. One picker for all three ways a host authenticates, which is what makes the illegal combination unrepresentable rather than merely invalid. SshKeyChoice became AuthenticationChoice carrying an AuthenticationKind, and BuildHost reads both SshKeyId and CredentialId off that single selection, so a host naming a key and a credential — which HostSecret.TryValidate refuses — cannot be expressed. Two pickers would have expressed it and then rejected it at save time. The kind travels with the id in three places and none is padding: Missing takes it, the placeholder lookup matches on kind as well as id, and Bound(kind) returns null unless the selection is that kind. Drop any one and a dangling credential comes back as a dangling key, which saves as a key binding to an id no key has. A credential's username had to reach the SSH request, not just its password. TryBuildCredential returned only the secret and the connect path read the username off the host, so a stored credential would have gone out under the wrong account — wrong in a way a server only reports as "authentication failed". It is now TryBuildAuthentication returning a (Username, Credential) pair. The no-username refusal moved, and had to. It ran before anything looked at the binding, which made a credential's username unreachable in the one case it is most useful: a host somebody never filled a username in for. It is now the last thing every branch agrees on, so such a host is perfectly usable through a credential that carries one, and a host with neither still refuses and now says where to put one. --- What the measurements cost --- Ten mutations, all caught. Two are worth naming. Removing a section's IsVisible is caught by OnlyOneSectionIsOnScreenAtOnce and by nothing else: two visible sections overlap in the row they share rather than clip, so every fit test still passes while the column shows one list through another. Defaulting the credential selection to the first row is caught by ReloadingKeepsACredentialSelectionButNeverInventsOne, and the property is a safety one rather than tidiness — Delete acts on the selection, so a list that picked a row on every background sync would aim a one-click password deletion at something nobody chose. The key list has the same property, and its comment cited a method that has not existed for some time; both now name the delete command they actually protect. One test of mine could not fail, and the mutation pass is what found it. AHostBoundToACredential_SendsItsPasswordAndItsUsername gave the credential and the host the same username, so it passed whichever one the code read. An override is only tested when the two values differ. Two shipped statements went false and were corrected rather than left: the class remark saying passwords were "not yet" in the vault, and the terminal column's "Keys are in the vault; passwords are not yet." That column's hint is now a tooltip on the password box rather than a sentence in the row, which was measured the hard way — by looking. At the window's 820px minimum the column gets 480, and a 220px box plus Connect plus any sentence does not fit; the row has shipped clipped for as long as it has had a hint in it. That strip is the one part of the window nothing can measure, because MainWindow cannot be laid out headlessly at all. Extracting it into its own control, as the vault column was extracted for exactly this reason, is what would fix that, and is not done here. 911 tests green, 30 of them new. Zero warnings, dotnet format clean. Seen by a person, which is how the two defects above were found. Still open from that pass: unlocking with the device key raises its consent dialog and then never returns, while registering one works — the difference is which thread the CNG call lands on, and diagnosing it properly is its own change. |
||
|
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
|
1faea42b94 |
Unlock with this machine's device key, without a passphrase or a network
The second of ADR 0007's three pieces: the seam a keystore plugs into, the wrap
cached where an offline unlock can reach it, and the unlock path itself. What is
still missing is the keystore — UnavailableDeviceKeyStore is what the application
composes for now, so behaviour is unchanged until piece three lands.
IDeviceKeyStore holds exactly 32 bytes, and only because the cache key moved
first. It would have had to hold the local cache key alongside the X25519 scalar —
a second live secret at rest, going stale on every passphrase change — had
|
||
|
jaap-jan
|
db4a8ed3d3 |
Let an already-enrolled account register a device key
The first of the three pieces ADR 0007 needs, and the one that was a discovery rather than a plan. EnrollmentService.AddDevice runs only during enrollment, so without an endpoint the device-unlock feature would have reached accounts created after it shipped and no others — which is to say none of the ones that exist. The code even said so: "the devices endpoint sets it properly when it lands." POST /api/v1/me/devices takes a name, an X25519 public key and the bundle sealed to it, and writes a device row plus a UserKeyWrapKind.Device wrap. Possession is proved by construction, so there is no challenge. The wrap is the secret bundle sealed to the supplied public key, and only something that has opened that bundle can produce it. A caller who seals the wrong bytes registers a device that cannot unlock, which harms nobody else; the server cannot tell the difference and must not pretend to, because it holds no key that opens either. That is also why the client must be unlocked to call this at all. It is the one endpoint in the /me group that requires enrollment, and it says so itself rather than relying on the group. The group deliberately does not: GET / and POST /enrollment are how a client discovers it needs to enroll and then does so, and gating those on enrollment would make enrollment unreachable. Adding the stricter policy to this route alone means an unenrolled caller is told "enrollment-required" by the authorization handler rather than getting a 400 about the shape of a request that was fine. Idempotent on the public key, and 200 rather than 201 for the reason enrollment gives: a retry of an identical request returns the same body, so there is no single moment of creation to point a Location header at. A second row for one key would mean a device list with a duplicate in it and two wraps to revoke instead of one. Mutation tested — removing the lookup fails RegisterDevice_TwiceWithTheSameKey_ReturnsTheSameDeviceAndAddsNoSecondWrap and nothing else. That test also found a real defect, in the way these usually surface: two timestamps that print identically and are not equal. TimeProvider reports 100-nanosecond ticks and PostgreSQL's timestamp with time zone keeps microseconds, so the first call returned a value that no later read of the row would ever produce, and the idempotent retry answered with a different timestamp for the same device. Nothing breaks, which is what makes it worth fixing: the service now truncates to the precision the column actually holds, so the response is the same value every time it is asked for. The repo already had a precedent for this class of thing in KeyLogChain.TruncateTimestamp; it just had not been applied here. The platform is deliberately not carried on the wire, which leaves Device.Platform unreported and the stale comment corrected rather than fulfilled. It would be a display-only field, and a Contracts enum mirroring the domain's DevicePlatform is exactly the shape of duplication that has produced three self-consistent bugs in this repository. A device list that wants it can add a mapping table and a test pinning the two together, which is what the sync entity types already do. Its own problem code and exception rather than reusing enrollment's, whose rules it largely shares. Registering a device is not enrolling, and a client showing "your enrollment was rejected" because somebody set up a fingerprint reader would be describing the wrong thing. The validation shares the limit constants — MaximumWrapBytes, MaximumDeviceNameLength, PublicKeySize — and not the four-line guards, which would have had to be parameterised over which exception to throw for less than they cost. Both in-memory fakes implement it properly rather than throwing: they record the wrap so a test can assert it arrived, and refuse before enrollment as the real endpoint's policy does. A fake that answered where the server refuses is a fake that can make a real bug pass. 866 tests green, 8 of them new. Zero warnings, dotnet format clean. Still to come: the protector seam with the wrap cached locally so device unlock works offline, then the Windows Hello implementation and the unlock-screen UI — which is where the Windows target framework lands and where automated testing stops. |
||
|
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
|
c5dec2d68e |
Measure the vault column instead of arguing about it
Nothing in this repository loaded a .axaml, so the one class of defect this window has actually shipped — a control arranged past the edge of its container, where it cannot be clicked — was the one class nothing could catch. The setup screens rendered sliced once, with their buttons unreachable. The vault column is the next candidate: 340 pixels wide, two lists and two editors, and the only thing keeping it from clipping its own Save button at the window's 520-pixel minimum is a state rule that one editor may be open at a time. That rule was added on the strength of an argument. This adds an Avalonia.Headless project that lays real XAML out at a real size and reports what a user could not reach, and the argument is now a number: with both editors open the column overflows, so the rule is load-bearing rather than defensive. BothEditorsAtOnce_DoNotFit_WhichIsWhyTheRuleExists is the test, and it says what to do if it ever starts passing — the column has room, so delete the rule, not the test. Two findings arrived by measuring rather than by reasoning, and the first one changed the design. MainWindow cannot be shown headlessly at all. Showing it attaches the terminal's NativeWebView, whose Win32 adapter initialises WebView2 on attach, and WebView2 refuses a non-STA thread — which is exactly why Program.Main carries [STAThread] and is written down in that comment. A HeadlessUnitTestSession owns its dispatcher thread and offers no apartment choice, so the whole window is out of reach at any size. That is pinned as a test asserting RPC_E_CHANGED_MODE by HResult rather than by message, so a future Avalonia that makes the adapter lazy will fail it and the harness can be widened. So the column had to become its own control to be measurable, which is the extraction the type-selector rework wanted anyway. Keyboard release moved with it: MainWindow used to call Focus() on HostList by name, and now asks VaultColumn.KeyboardTarget. The window decides that the keyboard should leave the terminal and the column decides where it lands — which is the seam the rework needs, because once the column shows one list at a time, "which list owns the keyboard" is a question only the column can answer. The second finding is the way this kind of test lies quietly. The hint class lived in MainWindow.Styles and carries TextWrapping. A Window's styles reach its whole tree, so nothing about the application depended on where it lived — but a control laid out on its own loses them, and every hint paragraph would have measured as a single line. The harness would have passed while measuring heights that were all too small. The three shared classes now live in App.axaml, which changes no rendering and makes the measurement honest. The detector is calibrated in both directions, because a clipping detector that never fires reads as a guarantee: a deliberately clipped Save button is caught by name, and a list longer than its viewport is exempt. Scrolling is how a list is supposed to handle more rows than fit, and without that exemption the host list would fail the moment it had content. It also mis-fired once and the rule is narrower for it — an empty ListBox is zero pixels tall and correct, so "arranged with no size" now applies only to controls the theme gives a height to. Skia rather than the headless drawing stub, deliberately. The stub's font manager invents glyph metrics, and text height is an input to every stacked panel in this column, so measuring against it would produce numbers that are self-consistent and unrelated to the application. A separate test project rather than more tests in DodoSSH.Client.App.Tests. Avalonia's application, dispatcher and platform are process-global singletons initialised once, and that project's identity is the shell's state machine without Avalonia — the whole reason sign-in is a delegate. The fakes needed to reach a real unlocked vault are shared from DodoSSH.Client.Session.Tests by source link: a project reference would make one test project a library of another, and a copy would be a third implementation of the same decision table drifting from the other two. 855 tests green, 10 of them new. Zero warnings, dotnet format clean. Not done, and this is groundwork rather than the item itself: the type selector. The column still holds both lists at once, so a third item type would still recreate the defect the one-editor rule works around. What is different is that the rework can now be checked instead of eyeballed — including the claim it is being made for, that one editor at a time stops being a runtime rule and becomes a fact about what is in the visual tree. What this harness will never catch is the terminal's native child window compositing over Avalonia content. That is a Win32 property of a real window, no headless surface reproduces it, and it is the reason the WebView is collapsed rather than covered. |
||
|
jaap-jan
|
211eba0666 |
Keep host key trust in the vault, and make it withdrawable
A fingerprint approved once is now approved on every machine and survives a
restart, because host key trust is a vault item type rather than a dictionary
that dies with the process. InMemoryKnownHostStore was what shipped, so the user
was asked to verify a fingerprint on every single connection — which is the gap
most likely to train somebody to click through the one warning that actually
matters. A warning that appears when nothing is wrong teaches that nothing is
ever wrong.
The fourth item type, and like the third it cost no sync logic: a row, an EF
configuration, a migration, a server kind; a secret, a codec, a merge, a cipher,
a repository facade and a session property. One row in the client registry. The
reconciler, the mirror, the repository, the outbox and the pull filter were not
touched. SyncEntityType.KnownHostKey and AadResourceType.KnownHostKey were
already reserved, so neither the contract nor docs/crypto.md changed.
One item per (host, port, algorithm), because a server legitimately offers
several host keys and which one gets negotiated is not ours to predict. Pinning
per endpoint would make an algorithm change indistinguishable from an attack.
The label is derived rather than stored, which is the one place this type
departs from the other three. A user never names a pin — there is nothing to
name it after but the three fields it already has — and a stored label is a
second copy of data that can disagree with the first after a merge. Relabel
returns the secret unchanged, and says why.
The store answers the handshake without touching the disk. SshNetConnectionFactory
calls FindAsync from inside SSH.NET's synchronous HostKeyReceived event, over
.GetAwaiter().GetResult(), which cannot be avoided; doing SQLite I/O plus an AEAD
open per lookup there would put the handshake behind the cache. So decryption
happens in OpenAsync and RefreshAsync — on unlock and after each sync pass,
exactly where the host and key lists already reload — and FindAsync is a
dictionary read under a lock with no await inside it.
That snapshot is where the one real bug in this change lived. Install originally
merged the live pins over the freshly loaded snapshot, to protect a TrustAsync
that had landed while the read was in flight. It would also have resurrected
every pin the user had just forgotten, and stopped a withdrawal made on another
machine from ever taking effect — the store would have healed the deletion back
into existence on every refresh. Replacing wholesale and discarding the read
instead is correct because writes are the rare case: every write bumps a
generation counter, and a refresh whose stamp is stale throws itself away rather
than winning. Nothing found this but reading the method again; it is the kind of
mistake that passes every test written before it, because the test that catches
it is the one the bug tells you to write.
Forgetting is new, and persistence is what made it mandatory rather than
convenient. A mismatch is a hard refusal with no way to continue — deliberately,
and that stays — so pinning a key permanently is also a way to make a
legitimately rebuilt server permanently unreachable. Before this change the pin
died at exit and the problem solved itself; now it does not.
ForgetAsync drops every algorithm for an endpoint, and it is reachable from the
host editor rather than from the warning. Putting it on the mismatch banner would
have made it two clicks from "this may be an attack" to "connect anyway", which
is the affordance the hard refusal exists to deny. The banner already promised
the key could be removed in the host's settings; that promise is now true and
points at the button.
Trust recorded on another machine becomes visible at the next sync pass, not
immediately, and that is a decision rather than an oversight. The failure it
produces is a first-contact prompt for a host a colleague approved a minute ago:
answerable, and self-correcting on the next pass. The opposite trade — polling
the vault on the handshake thread to close a one-minute window — buys nothing
and costs the property above. The dangerous direction is not reachable at all: a
pin recorded here enters the snapshot as part of recording it, so a refresh can
never discard a local trust decision.
The server learns nothing, and this is the item type where the temptation was
real. A plaintext host column would let a known-hosts screen sort and page
without decrypting anything, and it would hand the operator the map of every
user's estate — assembled, as these things are, out of facts that are each
individually harmless. A host row concedes an address only when relay is
switched on and the database refuses to store one otherwise (ADR 0004); there is
no equivalent excuse here. The table has no column to put one in, and the EF
configuration says so where somebody adding it would be standing.
Two things about the migration in this commit are worth knowing, because both
came out of getting it wrong.
It was hand-written first, including its .Designer.cs, and that version is not
what is here. Verifying it turned up something that had been quietly assumed:
Migration_AppliedCleanly_WithNoPendingModelChanges does not check the model
snapshot. It asserts that migrations applied and that none are pending, which a
wrong snapshot satisfies perfectly — the snapshot only matters as the diff base
for the *next* migrations add, so an incorrect one passes the whole suite and
corrupts the following migration instead. The real check is to generate a
throwaway migration and confirm its Up and Down come out empty. They did, and
the generated designer was byte-identical to the transcribed one across all 1255
lines, so the hand-written work was in fact correct.
Then dotnet ef migrations remove --no-build deleted the wrong migration. With
--no-build the tool reads the previously compiled assembly rather than the files
on disk, and the probe had just changed which migration was last, so it removed
AddKnownHostKeyItem and reverted the snapshot. That turned out to leave exactly
the right diff base, so the migration here is EF's own output rather than a
transcription — a better outcome than the one that was interrupted, arrived at
by accident. Never pass --no-build to migrations remove.
Mutation tested, all three sabotages detected: dropping the algorithm from
KnownHostIdentity.For, merging instead of replacing in Install, and pointing
KnownHostKeyCipher at PortForward — which is what a cast from the wire enum's 10
would silently produce. Each is caught both by an assertion about the mechanism
and by a behavioural test that never mentions it; the resource-type sabotage is
caught by the table from
|
||
|
jaap-jan
|
d10a38d8e6 |
Pin every cipher's AAD resource type from one table, not one test each
Mutation testing found that pointing CredentialCipher at AadResourceType.Vault passed the entire suite. Every credential test compared the cipher against itself — round trips, cross-type refusals, two-machine sync — and all of those stay true when both halves of one cipher are wrong together, because Seal and TryOpen share the constant. A password sealed under the resource type for a vault encrypts cleanly, decrypts cleanly, syncs cleanly, and violates docs/crypto.md in a way nothing surfaces until another implementation refuses the item. By then the AAD is frozen into stored ciphertext and only clients can re-encrypt it. This is the third time that hole has appeared in this file, and the second time mutation testing rather than review is what found it. So the fix is structural rather than another hand-written test: one table of wire type to resource type, a theory that seals a sample through each cipher and opens it with the resource type the table names — never the one the cipher holds — and a guard asserting the table covers ItemKinds.SyncedTypes. A fourth item type can no longer be added without pinning its resource type: the coverage test fails, and the sample switch throws with an explanation. The two per-cipher tests it replaces said the same thing for hosts and keys, so nothing is lost and the credential row is no longer something someone has to remember. Verified by re-running the mutation matrix. All seven sabotages are now detected: the credential merge dropping its redaction, the key/credential exclusivity check disabled, the schema version ladder flattened so a key-bound host claims the credential version, a credential sending the server an empty fields record instead of none, CredentialKind claiming to be a host, CredentialCipher sealing under the wrong resource type, and the credential noun reading "host". Two of those were unproven before this run — one because the earlier sabotage did not compile, and one because it was genuinely undetected. Sync.Tests 88/88, Domain.Tests 117/117. Zero warnings, dotnet format clean. |
||
|
jaap-jan
|
e24012b039 |
Sync credentials as a vault item type, and bind one to a host
Closes the largest remaining M1 gap in the data layer: a username and password can live in the vault, sync between machines, and be named by a host as how it authenticates. What is not here is the interface for creating one — see the end of this message. The third item type, and the first one that cost almost nothing to add. Server: a VaultCredential row, an EF configuration, a migration, and a CredentialKind. Client: a secret, a codec, a merge, a cipher, a kind, a repository facade and a session property. No new reconciliation logic, no change to the sync engine, no client cache migration. That was the whole point of the item-kind seam, and this is the evidence it holds. The narrowest type of the three on plaintext, and not for symmetry. A host has a deliberate concession — the relay needs an address it can resolve. A key has a fingerprint, public by nature, which this client still declines to send. A password has no part that is safe to expose: not its length, not a hash, not a hint. So CredentialKind refuses every plaintext field there is, hydrates none, and the table has no column to put one in. HostSecret.CredentialId is the password counterpart of SshKeyId, and the two are mutually exclusive. SSH itself would happily try a key and fall back to a password, but a host naming both leaves "how does this authenticate?" without a single answer — the interface, the connect path and the user would each be free to guess differently. TryValidate refuses it. One consequence was not anticipated: "a full host" stops being a coherent idea, which is what broke AFullHost_RoundTrips and is now written into that test. The schema version became a ladder rather than a maximum: credential-bound is 3, key-bound is 2, neither is still 1. Adding credentials therefore does not drag every key-bound host in every vault onto a version that clients understanding keys perfectly well would refuse to edit. A test pins exactly that, because it is the property the whole content-dependent-version rule exists to provide, and the obvious implementation would quietly lose it. Two tests had become false and said so: - Push_AnUnsupportedEntityType_IsInvalidNotAFailedBatch used Credential as its example of a type this server does not implement. It now asks the server's own registry what is still missing, so it cannot go stale again, and skips with a reason if that set ever empties. - ThePullFilterNamesEveryTypeThisBuildSynchronises pinned the exact list, which is what it is for. Also fixes ten nullable warnings — eight in SyncEndpointTests, two in a test file added earlier today. Neither set was introduced here; both were invisible until an unrelated change forced their project to recompile, which means the zero-warning claims made earlier in this work only ever covered what happened to be rebuilt. 777 tests green. Zero warnings, dotnet format clean. Not done, and deliberately: the credential interface. The vault column is 340 pixels wide and already holds two lists and two editors, kept from clipping its own buttons at the window's minimum height only by the one-editor-at-a-time rule added earlier today. A third list and a third editor would recreate that defect rather than avoid it, so the column needs a shape decision first. Credentials sync; they cannot yet be created in the interface. |
||
|
jaap-jan
|
70b3290a77 |
Bind an SSH key to a host instead of picking one per connection
A host now names the key it authenticates with, or none, as a field in its encrypted payload — so the choice follows the host to every machine rather than being made again each time somebody connects. The per-connection "Use key" switch it replaces was a stopgap for not having this, and keeping both would have left two mechanisms answering one question. This is the first payload schema version bump, and it does not work the obvious way. A host is written at the *lowest* schema version that can represent it: one that binds a key is written at 2, one that does not is still written at 1, byte for byte as it was before the field existed. The version is what makes an older client refuse to edit an item, so stamping 2 unconditionally would mean upgrading a single machine and renaming a single host made that host uneditable on every machine that had not upgraded yet. Confining the cost to the hosts that actually use the field is the difference between a team noticing a bump and a team being blocked by one. HostSecretCodec states the rule so the next field added follows it, and a test pins the version-1 bytes against a literal rather than against the codec, because the claim is about history: every host already in every vault has to re-encode to what it encoded before, or the first sync after an upgrade would push the whole vault as changed. A binding is an item id, not a copy of the key — a second copy of a private key is one that goes stale — which means the reference can dangle when the key is deleted on another machine. Both places that meets are handled the same way, by refusing rather than falling back: - Connecting to a host whose key is gone is refused outright. A host somebody deliberately set up for key-only access must not quietly start offering a password. - Opening such a host in the editor keeps the binding, selected, labelled as missing. The quieter version of the same failure is someone editing the port and saving, silently converting the host to password authentication with nothing ever having said so. Two things this found by being falsified: - The merge was untested for the new field, and "just take the server's value" passed the entire suite — a local binding change would have been discarded with no conflict recorded. HostSecretMergeTests already had a test written for exactly this class of omission; it simply had not been extended. - Adding a nullable field exposed a defect in HostSecretMerge.Field: it short-circuited when the discarded value was null, so the formatter never ran for the one case where null is a value rather than an absence, and a field whose absence has a name could not report it. Now the formatter always runs, and "no key" appears in the conflict log where an empty string used to. Also fixes eight nullable warnings in SyncEndpointTests left by the server-side SSH key commit, which had omitted the null-forgiving operator the rest of that file uses. They were invisible until an unrelated change forced the project to recompile. The end-to-end slice now binds its host to its key, so a schema-version-2 payload goes through the real API, the real PostgreSQL and back out on a second machine. 745 tests green. Zero warnings, dotnet format clean. |
||
|
jaap-jan
|
e3fd3e1728 |
Sync and authenticate with SSH keys on the client
Completes the client half of SSH keys: they sync alongside hosts, appear in their own list, and can be selected to authenticate a connection instead of typing a password. The reconciler and the repository were Host-typed throughout, so the choice was to generalise them or to keep a second copy per item type. Generalised, because ItemReconciler's whole premise is that the pull and the push paths must answer the same collision the same way — two copies would drift the first time one of them was fixed. What is genuinely per-type now arrives through IItemKind<TSecret>: the cipher, the merge, the plaintext columns, and the noun to use when telling a person what happened to their item. Generic where the server's IItemKind is not, and for the reason that reverses there — the client needs the concrete type, because it merges field by field. The pull filter is derived from the same registry that builds the reconcilers. That is the specific failure being designed out: an item type that encrypts, merges and lists perfectly and is never once requested from the server, so it works on the machine that made it and exists nowhere else. No client cache migration. The item table's primary key and the outbox's unique index already carry the entity type, and AadResourceTypes already mapped SshKey — so a host and a key may share an id and never see each other's rows, which SshKeySyncTests now arranges deliberately. A key hands the server nothing in plaintext. There is a public_key_fingerprint column and it would be accepted; leaving it null is deliberate. A fingerprint is not secret but it is a stable identifier for a key pair, so filling it would let an operator tell which of their users hold the same key and correlate one across vaults, for a column nothing reads. The design allows itself one plaintext concession — the relay address, which the relay cannot work without — and this is not that. A key is chosen per connection rather than bound to a host, which works the way ssh -i does. Binding one needs a field on HostSecret and therefore a payload schema bump, which makes every host written afterwards read-only on an older build; worth doing deliberately rather than as a side effect of adding keys. Three things this found, all of them by being falsified rather than by review: - Making the reconciler generic silently turned a record comparison into reference equality, because == on a type parameter is not value equality. The effect would have been a conflict recorded on every pass for an unacknowledged create that had in fact landed. Sabotaging the fix left all 73 tests passing — nothing covered that branch — so ConflictMatrixTests now has AnUnacknowledgedCreateThatDidLand_IsDroppedQuietly, which fails without it. - A test asserting that a blank passphrase reaches SSH.NET as null was vacuous: it exercised the editor, not the credential path, and passed with the guard deleted. Resolved by making SshKeySecret.Passphrase normalise an empty string to null, so there is one spelling of one state — which also keeps two clients from producing different payload bytes for an identical key. That exposed a wider gap: SshKeySecret, its codec and its merge had no direct unit tests at all. They have 25 now. - The reason first given for that normalisation was false. It claimed SSH.NET rejects a passphrase supplied for an unprotected key; measured against a real sshd it ignores it and authenticates anyway. Corrected everywhere it was stated and recorded in docs/platform-flags.md. The same test file also closes a real hole: SshPrivateKeyCredential had never been exercised against a server, because the existing key test builds SSH.NET's auth method directly and bypasses the path a vault-held key actually takes. Only one editor may be open at a time. Both sit in the same 340-pixel column as Auto rows and their heights together exceed it at the window's minimum size, so two open editors put the lower one's Save and Cancel past the bottom edge — the same failure this window already shipped once with the setup screens. Expressed as a state rule because that is the only form of it this repository can check: nothing here loads a .axaml. The refusal keeps what was typed, since in the key editor that is a pasted private key the user may have nowhere else. The end-to-end slice now carries a key as well as a host, so both item types go through the real API, the real PostgreSQL and the real crypto in one pass — the three hand-kept mappings between enums that do not line up are the reason that is worth doing rather than trusting the unit suites. 735 tests green, including the container-backed SSH and end-to-end suites. Zero warnings, dotnet format clean. |
||
|
jaap-jan
|
586cb303d5 |
Merge branch 'claude/gallant-brahmagupta-1f8244'
Writes down that locking the vault leaves shells running, and shows the count on the unlock screen rather than leaving it to be inferred. Conflict resolution: - ShellFlowTests' fixture keeps main's FakeSshConnectionFactory. The branch added an IdleSshConnectionFactory for exactly what main's fake already does — a shell that is open, silent and never closes on its own — so FakeSshConnections.cs is dropped rather than merged, leaving one fake SSH stack in the suite instead of two that would drift apart. - MainWindowViewModel and TerminalWorkspace: both sides added their own members, so both are kept. - TerminalWorkspaceTests was added by both branches, with the renderer gate on one side and session lifetime on the other. Merged into one class over one set of helpers; the gate tests now use FakeConnectionFactory rather than an NSubstitute stub, since the suite already has the fake. gallant's polling Timeout constant is PollTimeout, which no longer reads as the renderer's. - platform-flags.md keeps main's measured focus section and drops the short "nothing hands the terminal keyboard focus" entry the branch still carried, which that section supersedes. One genuine disagreement between the branches, left visible rather than flattened: this branch measured that a collapsed WebView cannot be typed into and attributed it to a hidden WS_CHILD window being ineligible for keyboard focus, while main's focus work measured Win32 focus still held by that hidden window and added a lock path that moves the keyboard off it. Both results stand; the mechanism sentence now defers to the focus entry, which makes the input barrier something the lock path maintains rather than something the platform guarantees. Full suite green, including the container-backed SSH tests. |
||
|
jaap-jan
|
74341d41e0 |
Merge branch 'claude/distracted-ritchie-53fc70'
Bounds the renderer wait, so a WebView2 that never initialises reports itself instead of hanging Connect with the busy flag stuck. Conflict resolution, all of it in the App test suite, which main had changed under the branch when sleepy-chebyshev landed: - The workspace fixture keeps main's fake SSH factory and its FakeRenderer-aware page, and takes the branch's RendererTimeout on top. One second rather than the branch's 250 ms, because the timeout now also bounds FakeRenderer's own wait for the attach it just made. - FakeRenderer arrived on main after the branch was cut and still called the no-argument WaitForRendererAsync. Both sides merged cleanly and left the build broken; it now passes its own token. - ConnectingWithNoRenderer's remark claimed the suite never starts the workspace and never attaches a renderer. Both are false here, so it now says what is true of the test: it is the one connect test that attaches no renderer. |
||
|
jaap-jan
|
9270d0cba5 | Merge branch 'claude/sleepy-chebyshev-cda68d' | ||
|
jaap-jan
|
0331ce8f33 |
Commit Rider's VCS directory mapping
The one file left untracked in the working tree, and it belongs in the repo rather than in .gitignore: the ignore rules here already follow JetBrains' own pattern, which excludes per-user state — workspace.xml, tasks.xml, dataSources, shelf — and keeps shareable project configuration. vcs.xml is the latter. All it says is that the project root is a Git checkout, which is true for everyone who clones it, and having it present stops Rider prompting each new checkout to add the mapping by hand. |
||
|
jaap-jan
|
c4dbd85da0 |
Add the client's SSH key model, codec, merge and cipher
The client can now seal and open an SSH key item. Nothing consumes it yet — the repository, the sync engine's per-type handling and the UI come next — but this is the layer everything above it depends on, and it is the layer where the crypto has to be right. SshKeySecret holds the private key as an ordinary string, deliberately, and says so: a .NET string cannot be wiped, so the material lives until the GC reuses the memory. libsodium's guarded memory was considered and rejected because the passphrase protecting the key, the password on the next item and the JSON the codec just parsed are all strings on the same heap — protecting one field among them reads as security and buys nothing. What the design does give is that the key never reaches the disk in plaintext, never reaches the server at all, and is handed to SSH.NET through a MemoryStream so there is no temporary key file to leak. Validation refuses a public key by name. ssh-keygen writes two files whose names differ by four characters, and pasting the wrong one otherwise produces a vault item that looks fine and fails at connection time with an authentication error that says nothing about which file you chose. The merge redacts the private key and its passphrase from the conflict log. A host conflict shows both values so the loser can be put back; doing that for a private key would write the discarded key into a log that is designed to be read rather than used and is deliberately retained after acknowledgement. Two different private keys are not something anyone reconciles by reading them side by side. And the lesson worth recording, because it nearly shipped: the first version of AadResourceTypeTests proved nothing. It checked that a key payload does not open as a host and vice versa — true however both ciphers are misconfigured, because Seal and TryOpen share one constant, so changing it changes both and the round trip still works. Sealing every private key as if it were a vault passed all twelve tests. The tests now open a sealed payload independently through ItemKeys with the resource type named out of band, and that does fail under the same sabotage. A test that only compares an implementation against itself cannot catch a self-consistent mistake. The trap it defends: SyncEntityType.SshKey is 3, AadResourceType.SshKey is 6, because the crypto enum also carries None, User, Device and Vault ahead of the item types. A cast between them is a specification violation that encrypts cleanly and would only surface when another implementation refused the item. |
||
|
jaap-jan
|
d459dac600 |
Stop a dead WebView2 hanging Connect with the busy flag stuck
VaultViewModel.ConnectAsync awaited TerminalWorkspace.WaitForRendererAsync
with no timeout and no token, and RunAsync clears IsBusy only after the
work returns. Whether the renderer attaches at all depends on a runtime
this application does not install: with a missing or policy-blocked
Evergreen runtime, or an AppContainer that cannot reach loopback, the
socket never arrives — so Connect never returned, the window stayed
disabled on "Connecting…" for the rest of the session, and nothing on
screen said why. Left out of
|
||
|
jaap-jan
|
e93acc856f |
Sync SSH keys as a vault item type, over a shared write path
The private key now lives in the vault as ciphertext, syncs between a user's machines, and is stored on the server so it can later be shared — sharing itself needs M3's signed grants; this is the storage that makes it possible. More was already reserved than expected: SyncEntityType.SshKey, CryptoSpec.AadResourceType.SshKey, ChangeEntityType.SshKey, SyncPlaintextFields.PublicKeyFingerprint, and SshPrivateKeyCredential wired through PrivateKeyFile over a MemoryStream so a key never touches disk. The frozen contract and crypto spec needed no change at all. What was missing was the server. Rather than copy the push path per item type — version check, change-log append, exactly-once receipt, advisory lock — it is now written once over IVaultItem, with everything type-specific behind IItemKind: which table, which plaintext columns, and what those columns must satisfy. Ten copies of that logic by M5, with a fix applied to nine, is the outcome this avoids. The refactor landed first with no behaviour change, so all 66 existing Host tests were the regression net, and they stayed green. An interface rather than a base class, deliberately: EF Core maps an inheritance hierarchy when it can see one, so a mapped base would quietly become a table-per-hierarchy discriminator across item types — the very arrangement per-type tables exist to avoid. ssh_key mirrors host and pointedly has no relay trio. That is the argument for separate tables rather than one wide item table: the columns a host needs are columns a key must never have, and a shared table could only make them nullable and trust the code. A key carrying a relay target is refused with a reason rather than silently dropped. A key hydrates PlaintextFields as null, not an empty instance — the difference is visible on the wire, because an all-defaults instance still serialises "relayEnabled": false and invites a reader to believe the setting exists and is off. It has none. Two things now defended by tests rather than by comments. Each kind states its own ChangeEntityType instead of casting: the two enums agree numerically but do not even share member names (Host against SshHost), and filing key changes under the host type is silent sync corruption — sabotaging it fails three tests. And EntityTypeAlignmentTests asserts the two enums stay aligned in both directions and in count, which nothing did before. The client half is next: SshKeySecret, its codec and merge, the cipher, a repository, and the UI. Note for that work — SyncEntityType.SshKey is 3 while AadResourceType.SshKey is 6, so a cast between them would seal key ciphertext as a vault and nothing would fail. |
||
|
jaap-jan
|
c6fc19bbbd |
Sync the vault automatically instead of only on a button press
Three triggers: once when the vault opens, straight after any local change, and every minute while it stays open. The Sync button stays, because someone just handed a credential wants to know now rather than within the minute, but nothing depends on it being pressed any more. A background pass is deliberately not the button's code path. Routing it through RunAsync would raise the busy flag every minute — disabling Connect and Save for the duration — and repaint the status line over whatever the user was reading. So it is quiet: the status changes only when a pass actually moved an item or produced something needing attention, and a pass is skipped outright while a command is running rather than queueing behind it. Both guards are covered; removing either fails a test. A shared semaphore serialises every pass, taken with a zero timeout rather than awaited — a pass arriving while another runs has nothing to add by waiting, and queueing them would turn a slow server into a backlog of identical work. Failures are swallowed, which is right in exactly this one place: a laptop closed all afternoon would otherwise replace the status line with a socket error once a minute. It is quiet rather than hidden — the account bar already shows when there is no connection, and pressing Sync reports the real reason. What earns that is the outbox: a test proves a change left queued by a failed pass is still sent by the next sync, so quiet never means lost. Two existing tests asserted the opposite behaviour — that a save queued and pushed nothing until Sync was pressed — and were rewritten rather than deleted; the local-first guarantee they were really protecting is that the list updates with no server, which the offline test still covers. Two things the tests caught in my own work. ReloadAsync had to be split out of LoadAsync because rebuilding the list repainted the status line unconditionally, which made "the background pass is quiet" false on the one path that mattered. And the yields-to-a-command test was vacuous as first written: saving pushes, so there was no pending change left and the assertion held with the guard deleted. It now fails the automatic push first to arrange a real queue. |