Commit Graph
3 Commits
Author SHA1 Message Date
jaap-jan 5a899afd78 Decide what Lock does to a running shell, and say it
Pressing Lock nulled and disposed the vault view model and touched nothing else.
TerminalWorkspace is injected from App.axaml.cs and outlives every lock, so the SSH
connection, the pty and the pump all kept running while the window said "Unlock your
vault" — and since 0500e43 collapsed the WebView while locked, that live session was
invisible as well as unstopped. CloseSessionAsync was reachable in production only from
DisposeAsync, i.e. shutdown. None of this was written down anywhere, so it was neither a
policy nor a bug, which is the actual problem.

Shells now deliberately outlive the lock, and every layer says so.

The reason to prefer this over making Lock a disconnect: locking is what a person does
when they walk away from the machine, which is exactly when a long upgrade, build or
transfer is most likely to be in flight. Ending every shell would make Lock a button that
destroys work, and the predictable response is to stop pressing it and leave the vault
open instead. The idle auto-lock this will grow decides it outright — an unattended
timeout that killed a running job would be worse than the exposure it removes. Closing
the channel also buys less than it looks: the session was authorised at connect time by a
credential the remote verified itself, and no vault key participates in keeping it alive,
so locking cannot retroactively un-authorise it any more than removing a member can.

Stated honestly rather than implied, because the lock screen is what hides it:

- The unlock screen shows how many shells are still connected, and that locking closes
  the vault and not the connections — so a machine still holding authenticated SSH
  channels does not present itself as merely "locked". Shown only when there is something
  to disclose. Quitting is what ends them, and the text admits that.
- The Lock button carries the same thing in a tooltip, since its name implies the
  opposite of what it does to a shell.
- README lists it as a third architecture consequence beside non-retroactive revocation,
  which is the same shape of honest limit; docs/crypto.md §10 records it as a threat-model
  boundary; TerminalWorkspace and LockAsync carry the argument next to the code.

LiveSessionCount deliberately does not count dictionary entries. Nothing removes a
session when the remote closes the channel by itself — RunSessionAsync only drops the
renderer registration — so sessions.Count would report a shell that exited half an hour
ago as still running, on the one screen where a user is deciding whether it is safe to
walk away. A completed Run task is what "the shell is gone" actually looks like. While
locked the number can only fall, since opening a session needs the vault, so a stale
value over-reports rather than under-reports.

Both new tests fail when the policy is reverted: the count test times out against
sessions.Count, and the shell test reports "workspace.LiveSessionCount should be 1 but was
0" when Lock closes sessions. ShellFlowTests also stops building its workspace with a real
SshNetConnectionFactory that nothing ever called, which had made the suite's independence
from the network a coincidence rather than a property.

Verified by hand with a live shell, which nothing had done: a harness mirroring
MainWindow.axaml's 340,* grid with a real NativeWebView, the shipped WebAssets, a real
sshd in a container, and an ISshShellSession decorator recording every window-change the
remote is actually told about. Across lock and unlock, no window-change reached the remote
at all, stty size answered 50 118 before and after, the renderer's own buffer came back
byte for byte with the wrapped line intact, and the session stayed live throughout. A
control run that never hides the WebView behaves identically, so nothing above is startup
or idle behaviour. Keystrokes injected while locked reach nothing: twelve of twelve
SendInput events accepted with the harness confirmed as the foreground window, no probe
character in the remote's output, and a following Ctrl-U answered BEL, so nothing was
queued in the line editor either. A hidden WS_CHILD window is not eligible for keyboard
focus, which is what makes surviving the lock defensible rather than merely convenient.

Correction to a claim made in f80b3d4: terminal.js's guard comment listed "a host that
hides the WebView while the vault is locked" among the paths that reach a degenerate fit.
It does not. Collapsing the control hides a native child window without resizing it, so
the page still reports paneWidth 840 and paneHeight 760 with unchanged cols and rows, no
ResizeObserver callback fires and the fit never runs. Establishing that rather than
assuming it: the same cycle with MINIMUM_FITTABLE_PIXELS patched to 0 — the guard fully
disabled — is equally clean. The guard is still right for minimising and for a splitter
dragged to the edge; it is simply not what makes locking safe, and must not be cited as
though it were.

Recorded, not fixed:

- Nothing closes one terminal from the interface, so a user reading "1 shell is still
  connected" can only act on it by quitting. CloseSessionAsync is tested and correct;
  VaultViewModel discards the session id it would need.
- A session whose remote exits keeps its ISshConnection, and the thread ShellStream parks,
  until the process ends.
- Suspected and seen once: before the harness waited for the window's scale to settle, a
  DPI settle pushed a 2202x1328 pane for a window 1180 logical units wide and a later
  re-push reflowed the wrapped line. Three later runs at RenderScaling 1.00 never showed
  it, so it is filed as a lead, not a finding.
- WebView2 fails to initialise with CO_E_SERVER_EXEC_FAILURE when the host executable
  sits under a very long path. Cost an hour on the harness; relevant to packaging.
2026-07-29 14:44:02 +02:00
jaap-jan 0500e43e02 Stop the terminal's WebView painting over the setup screens
The shell layered its setup and unlock screens over the terminal, which does
not work: NativeWebView attaches a real Win32 child HWND through
NativeControlHost, and a child window composites above everything its parent
paints regardless of visual-tree z-order. The cards rendered sliced at the
terminal column's left edge; at the window's default width every one of their
buttons fell inside the WebView's rectangle, so the flow could only be
completed by keyboard, and a click in that region handed Win32 focus to
WebView2 so the text boxes silently stopped accepting keystrokes.

The WebView is now collapsed while the vault is not unlocked. The comment
that previously forbade this — hiding it means never realising it — was
wrong: NativeControlHost creates the native attachment on attach to the
visual tree, never consulting layout or visibility, and NativeWebView replays
a Source assigned before its adapter exists. A collapsed WebView still starts
WebView2, loads the page and lets the renderer attach. Confirmed: 35
msedgewebview2 processes with the control collapsed. What the first
connection after unlocking actually depends on is the existing await on
WaitForRendererAsync, since the data plane drops frames when no renderer is
attached.

Also fixes the second visible defect: the default server URL was
https://localhost:7217, the API's *second* launch profile, while the README,
its appsettings and a plain `dotnet run` all use http://localhost:5233 — so
nothing was listening, and an HTTPS client against a plaintext port reports
"The SSL connection could not be established", which reads as a certificate
problem. The default now matches, a missing scheme is rejected by name
instead of parsing as scheme "localhost", and that specific TLS failure now
suggests http://. Both new tests fail when the fixes are reverted.

Corrections to claims I made earlier and should not have:

- docs/platform-flags.md asserted the opposite of the mechanism above and
  cited an established msedgewebview2 connection as verification. That
  observation was taken while the overlay was showing but, because of this
  very bug, the WebView was uncovered and in plain view — so it confirmed
  only that a visible WebView is realised. A process-level check cannot
  verify a rendering claim. The entry was also filed under "Local cache".
- ITerminalHost was documented as the live seam the app plugs into, with a
  stub standing in for headless tests. It has no implementation anywhere and
  no test uses it; the view navigates the control directly. It also counted
  Avalonia.Controls.WebView and NativeWebView as two interchangeable
  backends when they are one component, with the Linux backend backwards.
- The README claimed the shell's whole path was covered by tests. Its state
  machine is; its layout is covered by nothing, and a headless test could
  not have caught this — headless has no native window, so it would have
  rendered correctly and confirmed the wrong belief.

Verified by screenshotting the running app: the card renders complete and
centred at the default size, with the button clickable.
2026-07-29 13:26:30 +02:00
jaap-jan 49f617b450 Wire the Avalonia shell to the vault
The host list now comes from the vault instead of from a form. A fresh
machine takes a server URL, signs in through the browser, enrolls, and
from then on opens with the passphrase alone.

DodoSSH.Client.Session is the composition layer: where a profile lives,
how it unlocks, and how a machine gets one. ClientPaths picks a
non-roaming per-OS directory — %LOCALAPPDATA% and never %APPDATA%,
because a SQLite cache that roams between two machines is a corrupt one,
and each machine's outbox is its own. SessionOpener needs no transport at
all and could not reach one if it wanted to; that is the offline unlock,
asserted rather than asserted about. A wrong passphrase, a stale KDF and a
grant revoked by a rekey are three different answers, because the remedies
are three different things and telling someone to retype a passphrase that
was never the problem is worse than saying nothing.

The shell's states are the onboarding story. The recovery code gets its
own state that cannot be clicked past: it exists for one moment, losing it
with the passphrase loses the vault, and there is no server-side reset by
design. It is dropped from memory on confirmation rather than merely
hidden.

Sign-in is a delegate over IVaultServer, so the whole state machine runs
in a test against an in-memory server — no browser, no identity provider,
no toolkit. The view models are plain observable objects, which is what
makes that possible. What it does not cover is whether the XAML binds to
the right names; that needs a rendered tree and Avalonia.Headless, and is
its own piece of work.

Three things found by doing it rather than by reading it:

- Pooled SQLite connections keep the database file open after the last
  context is disposed. On Windows that means locked, so the application
  could never replace its own cache — and a test could not clean up after
  itself, which is how it surfaced. Dispose now clears the pool.
- EF's SQLite provider puts the database in WAL mode, so the cache is
  three files. A comment in ClientCacheFactory claimed the opposite;
  reading PRAGMA journal_mode off a real launch settled it. WAL is the
  right mode here — a sync pass writes while the interface reads — so the
  comment was wrong on the merits as well as on the fact.
- Enrolling a device key with nowhere to keep the private half would put a
  wrap on the server nobody can open and make the device list claim this
  machine can unlock without a passphrase. Device binding is now optional
  and the shell declines it until the OS keystore is wired.

Verified on Windows: the client created %LOCALAPPDATA%\DodoSSH\cache.db
and migrated it on first launch, and msedgewebview2 held an established
connection to the data plane while the unlock overlay covered it — which
is the point of covering the WebView rather than collapsing it, since a
NativeWebView that is never laid out is never realised.

630 tests, up from 593. The recovery-code gate and the offline unlock were
each verified by breaking them and watching the right test fail.

Still to do for M1's actual definition of done: the manual run against the
real API and a real Keycloak. Credentials are not a synced entity type
yet, so a connection still asks for a password, and the interface says so
rather than implying otherwise.
2026-07-29 11:02:19 +02:00