Commit Graph
6 Commits
Author SHA1 Message Date
jaap-jan 586cb303d5 Merge branch 'claude/gallant-brahmagupta-1f8244'
ci / build and test (ubuntu) (push) Has been cancelled
ci / build (windows) (push) Has been cancelled
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.
2026-07-29 15:46:41 +02:00
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 0500e43 to keep that change focused, and
recorded in docs/platform-flags.md as worth fixing on its own merits.

The gate itself is unchanged and has to stay: TerminalDataPlane.SendAsync
drops frames when no renderer is attached rather than queueing them, so a
session opened before the renderer arrives loses its SessionOpened frame
and then streams output at a terminal that was never created. Only the
wait changed — RendererAttached.WaitAsync(timeout, cancellationToken),
with the command's own token threaded through.

Fifteen seconds, on TerminalWorkspaceOptions.RendererTimeout. Attaching is
normally near-instant, since WebView2 starts with the window and the page
has usually attached while the passphrase was still being typed, but a
first run on a cold profile creates a user-data directory and starts a
process tree of some thirty-five processes first, which on a loaded
machine is seconds rather than milliseconds. A renderer that will never
attach will not attach however long the wait is, so being generous costs
only how long a broken runtime takes to say so, while being tight costs
telling someone their runtime is broken when it was merely slow.
Injectable because both new tests would otherwise sit out that budget.

The timeout is caught in VaultViewModel rather than left to RunAsync's
generic handler, because TimeoutException.Message is "The operation has
timed out" — which sends someone looking at their network or their host.
The status now names the WebView2 runtime and says to install it.

TerminalWorkspaceTests covers the half that was missing: the wait gives up
(329 ms against a 250 ms budget) and obeys its token (2 ms against a
five-minute one). Before the bound, the first of those would have hung
rather than failed. ShellFlowTests never starts its workspace, which from
the view model's side is indistinguishable from a WebView2 that failed to
initialise, so it asserts that the status names WebView2 and that IsBusy
is cleared; changing the catch to another exception type makes it fail
with "The operation has timed out.", so neither assertion is vacuous. The
success path is untouched and still covered end to end by
TerminalEndToEndTests against a real sshd container, which now passes the
test's cancellation token.

One byproduct: the doc comment on WaitForRendererAsync carried two
double-encoded em dashes, fixed now that the block is rewritten.
2026-07-29 15:26:53 +02:00
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 5fccd53824 Add the Avalonia app and the xterm renderer, and fix two real bugs
The terminal works end to end. A new integration test drives a real sshd in
a container through a real PTY, the real pump, the real loopback WebSocket
with its token and origin checks, and a ClientWebSocket standing in for the
page: the login banner arrives, typed input round-trips, and `stty size`
reports the 100x30 the session asked for. The only untested link left is
xterm drawing bytes it was handed.

The WebView is de-risked on Windows, which was the plan's largest risk. Not
by assertion: with the app running there is an established TCP connection
from msedgewebview2 to the data plane port, so WebView2 launched, navigated
to the loopback page, executed terminal.js, and completed the WebSocket
handshake against the real token and origin checks. Linux remains unproven
and the package's own release notes now corroborate the concern -- Linux uses
a WPE backend, and it ships a NativeWebDialog described as useful where
embedded WebViews may be unavailable.

Two bugs found by building it, both of which would have shipped:

- ShellStream.Write buffers and needs an explicit Flush. Without one a
  keystroke is accepted, reported as written, and never reaches the remote:
  the terminal displays output perfectly and simply stops responding to
  input. SSH.NET's own WriteLine flushes, which is why the earlier spike
  never hit it. Found by isolating the pump against real SSH and reading
  BytesRead=51 -- banner and prompt through, nothing after.
- The Windows app manifest needs a supportedOS list, or Avalonia's native
  control host fails outright and the terminal never starts.

Also fixed a genuinely flaky test I happened to catch: SyncCursorTests
tampered with the *last* base64url character, whose low bits the decoder
ignores when the input length is not a multiple of three -- so a tampered
cursor sometimes decoded to identical bytes and verified. It failed roughly
one run in thirty, depending on a random key. Now tampers the penultimate
character, which is fully significant at every length; 40 consecutive runs
are clean.

xterm 6.0.0 plus the fit and webgl addons are vendored as UMD bundles rather
than built with npm, so a clean clone needs only the .NET SDK. Provenance
and licences are recorded next to them, along with the UMD global names
terminal.js depends on -- a bundle that switched to ES modules would load
without error and leave Terminal undefined.

The renderer acknowledges output from term.write's completion callback, not
on receipt. Acknowledging early would return flow-control credit for bytes
the screen has not caught up with, which is the one thing the credit window
exists to measure.

TerminalWorkspace moved into DodoSSH.Client.Terminal: it has no Avalonia
dependency, and having it there is what let the end-to-end test exist at all.

404 tests pass, zero warnings on a clean rebuild, format clean.
2026-07-28 22:30:42 +02:00
jaap-jan eb354bcdd9 Add the SSH session layer and the terminal data plane
The throughput harness the plan requires before any UI, plus the SSH
plumbing under it. 94 new tests, no WebView involved.

Credit-based flow control is what makes `yes` survivable. A terminal renders
at 60 Hz at best while a remote produces output as fast as the network
allows, and the difference has to accumulate somewhere or be refused.
Credit is reserved *before* reading, never after: because the pump cannot
read more than the renderer has room for, the coalescing buffer is bounded
by the window rather than by how fast the remote can talk. When credit runs
out the pump stops reading, SSH's own receive window closes, and the remote
sshd blocks -- backpressure to the source with no custom protocol.

Verified by falsification, not just by passing: with the credit gate removed
three tests fail, including the throughput harness's bounded-memory
assertion. Acknowledgements are clamped because they cross into JavaScript,
where a buggy or hostile page could otherwise claim to have rendered a
gigabyte and talk the host into an unbounded read.

Host key trust is enforced by *failing* the connection rather than
prompting inside the handshake. SSH.NET raises verification synchronously,
so consulting the user there would block the handshake on a UI round trip
and deadlock the first time the prompt needed the UI thread. Unknown host
and changed key become distinct exceptions the caller resolves
asynchronously. A mismatch has no retry path at all: a dialog offering to
continue is how users are trained to click through the one warning that
actually indicates interception. A legitimately rebuilt server is handled by
removing the pin in settings, away from the moment of connecting.

The data plane serves the renderer page from the same loopback listener as
the socket, which makes Origin predictable -- always http://127.0.0.1:{port}
-- where a WebView virtual-host mapping would give a different origin per
backend and nothing to validate. The token is substituted at serve time, so
it never touches disk and never appears in a URL. Being clear about what
that buys: not protection from a process running as this user, which can
read our memory anyway, but from a page in the user's browser attempting
WebSocket connections to loopback ports, which is a real and routine thing.

Two bugs the tests caught. The accept loop handled connections serially, so
an upgraded WebSocket parked it inside the receive loop and every later
request went unanswered -- the page's own script among them. The suite hung
rather than failed, which is how I found it. And SHA-1 is unavoidable here:
RFC 6455 mandates it for Sec-WebSocket-Accept, where it authenticates
nothing. Suppressed narrowly with that reasoning; the alternative,
HttpListener.AcceptWebSocketAsync, throws PlatformNotSupportedException off
Windows.
2026-07-28 21:58:55 +02:00