From f0002b683c1df12ee7ea935fa01c98496b4d7a3a Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit Date: Fri, 31 Jul 2026 08:39:25 +0000 Subject: [PATCH 1/6] Update .github/workflows/ci.yml --- .github/workflows/ci.yml | 26 +++----------------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83f7d4f..1354136 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,8 +23,8 @@ env: jobs: build: - name: build and test (ubuntu) - runs-on: ubuntu-latest + name: build and test + runs-on: [self-hosted, linux] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -52,24 +52,4 @@ jobs: # server through Testcontainers and runs the API as a child process — so it needs a # Docker daemon and gets one here. That is why the tests run on ubuntu rather than # macOS, whose runners have no daemon at all. Expect the Keycloak image pull to - # dominate a cold run. - - build-windows: - name: build (windows) - runs-on: windows-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 - with: - global-json-file: global.json - cache: true - cache-dependency-path: '**/packages.lock.json' - - - name: restore - run: dotnet restore DodoSSH.slnx --locked-mode - - # Build only. Day-to-day development happens in Rider on Windows, so a - # Windows-specific compile break must fail CI even though the tests run on Linux. - - name: build - run: dotnet build DodoSSH.slnx --no-restore --configuration Release + # dominate a cold run. \ No newline at end of file From 312d766c3047e2a51f1109955105dc10a830c595 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 31 Jul 2026 10:43:30 +0200 Subject: [PATCH 2/6] Let the quick-connect palette answer for itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Views/MainWindow.axaml.cs | 76 ++--- .../Views/QuickConnect.axaml | 12 +- .../Views/QuickConnect.axaml.cs | 167 +++++++++- .../QuickConnectTests.cs | 298 ++++++++++++++++++ 4 files changed, 489 insertions(+), 64 deletions(-) create mode 100644 tests/DodoSSH.Client.App.Layout.Tests/QuickConnectTests.cs diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs b/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs index 936a77e..312227a 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml.cs @@ -79,9 +79,15 @@ internal sealed partial class MainWindow : Window /// /// /// - /// A tunnelled handler rather than KeyBindings, because three of these four keys have to be - /// intercepted before the control under the pointer sees them: Escape and the arrows belong to the - /// palette while it is open, and the palette's own text box would otherwise eat them. + /// Ctrl+K is here rather than on the palette because it has to work when the palette is not showing, and + /// it is a plain handler rather than a KeyBinding so that toggling stays one code path with the + /// rest of the chord set. + /// + /// + /// The palette's own keys are forwarded rather than answered: intercepts them + /// on their way down while the focus is inside it, and this is the net for when it is not — a press that + /// arrives with nothing focused, or from a control on the screen behind, still has to close the palette + /// rather than fall through to whatever is underneath it. /// /// /// None of this reaches the terminal, and it does not need to. Once the WebView's child window holds @@ -104,61 +110,12 @@ internal sealed partial class MainWindow : Window } else if (viewModel.IsSearching) { - HandlePaletteKey(viewModel, e); + Palette.HandleKey(e); } base.OnKeyDown(e); } - /// - /// The selection is moved here rather than by letting the list take focus, because the list taking - /// focus is exactly what would stop the query box receiving the next character typed. - /// - private static void HandlePaletteKey(MainWindowViewModel viewModel, KeyEventArgs e) - { - switch (e.Key) - { - case Key.Escape: - viewModel.CloseSearchCommand.Execute(null); - e.Handled = true; - break; - - case Key.Enter: - viewModel.ConnectToSearchResultCommand.Execute(null); - e.Handled = true; - break; - - case Key.Down: - Move(viewModel, 1); - e.Handled = true; - break; - - case Key.Up: - Move(viewModel, -1); - e.Handled = true; - break; - - default: - break; - } - } - - /// Clamped rather than wrapped: a list that jumps from the last row to the first loses people. - private static void Move(MainWindowViewModel viewModel, int delta) - { - if (viewModel.SearchResults.Count == 0) - { - return; - } - - var current = viewModel.SelectedSearchResult is { } selected - ? viewModel.SearchResults.IndexOf(selected) - : -1; - - viewModel.SelectedSearchResult = - viewModel.SearchResults[Math.Clamp(current + delta, 0, viewModel.SearchResults.Count - 1)]; - } - private void Attach(MainWindowViewModel? viewModel) { if (shell is { } previous) @@ -216,11 +173,18 @@ internal sealed partial class MainWindow : Window return; } - // The palette is a text box somebody is expected to start typing into immediately, so opening it - // has to move the caret there — including out of the terminal, which needs the Win32 half as well. + // Closing only. Opening also has to move the keyboard — the palette is a text box somebody is + // expected to start typing into immediately — but the palette does that for itself when it becomes + // visible, which is a moment this handler is measurably ahead of: it runs from the view model's + // PropertyChanged, before the binding that reveals the control, and Focus() on a control that is + // still collapsed is a no-op that is not replayed when it is revealed. if (string.Equals(e.PropertyName, nameof(MainWindowViewModel.IsSearching), StringComparison.Ordinal)) { - ReleaseKeyboardTo(viewModel.IsSearching ? Palette.QueryBox : KeyboardHome); + if (!viewModel.IsSearching) + { + ReleaseKeyboardTo(KeyboardHome); + } + return; } diff --git a/src/DodoSSH.Client.App/Views/QuickConnect.axaml b/src/DodoSSH.Client.App/Views/QuickConnect.axaml index d9a07f6..c684a83 100644 --- a/src/DodoSSH.Client.App/Views/QuickConnect.axaml +++ b/src/DodoSSH.Client.App/Views/QuickConnect.axaml @@ -21,7 +21,13 @@ an overlay across the middle of this window would be painted underneath it and take no clicks. --> - + + @@ -39,8 +45,8 @@ The Ctrl+K host search, overlaid on the window. /// -/// The keys it responds to are handled by rather than here, because two of them — -/// Ctrl+K to open and Escape to close — have to work when this control does not exist yet or has just -/// stopped existing. Handling the arrows in the same place keeps the whole chord set in one method. +/// +/// Everything a palette does once it is open belongs here rather than in : the four +/// keys it answers to, the press outside it that dismisses it, and taking the keyboard the moment it appears. +/// Opening it is the window's business, because Ctrl+K has to work when this control is not showing. +/// +/// +/// The split used to fall the other way, and the cost was that none of it could be tested. Showing +/// initialises WebView2, which refuses the headless dispatcher's thread — see +/// LayoutHarnessTests.WhyTheWindowItselfIsNeverShown — so behaviour that lived on the window could only +/// be checked by hand. A hosts in a bare window and takes real key and pointer +/// input. +/// /// internal sealed partial class QuickConnect : UserControl { - public QuickConnect() => InitializeComponent(); + public QuickConnect() + { + InitializeComponent(); - /// The box, so the window can put the caret in it the moment the palette opens. + // Tunnelled, and deliberately: the query box below is on the route these keys take, and a text box + // that grows a use for Enter or the arrows — a multi-line box, a completion list — would take them + // before a bubbling handler here ever ran. The palette owns them while it is open, so it says so at + // the point on the route where nothing else has had a chance yet. + AddHandler(KeyDownEvent, OnPaletteKey, RoutingStrategies.Tunnel); + } + + /// The box, so the caret can be put in it the moment the palette opens. internal TextBox QueryBox => Query; + + private MainWindowViewModel? Shell => DataContext as MainWindowViewModel; + + /// + /// Answers one of the palette's keys, wherever in the window it was pressed. + /// + /// + /// Internal because calls it too, for the case this control's own tunnelled + /// handler cannot see: a key routes through here only while the focus is inside the palette, and the + /// window is what catches Escape when it is not. + /// + internal void HandleKey(KeyEventArgs e) + { + ArgumentNullException.ThrowIfNull(e); + + if (Shell is not { IsSearching: true } shell) + { + return; + } + + switch (e.Key) + { + case Key.Escape: + shell.CloseSearchCommand.Execute(null); + e.Handled = true; + break; + + case Key.Enter: + shell.ConnectToSearchResultCommand.Execute(null); + e.Handled = true; + break; + + case Key.Down: + Move(shell, 1); + e.Handled = true; + break; + + case Key.Up: + Move(shell, -1); + e.Handled = true; + break; + + default: + break; + } + } + + /// + /// Takes the keyboard, so the palette can be typed into the instant it appears. + /// + /// + /// + /// Both halves, for the reason gives: the terminal's WebView is a native + /// child window that keeps Win32 focus even after it is collapsed, so focusing an Avalonia control without + /// the Win32 call produces a box with a caret in it that silently receives nothing. + /// + /// + /// Done here rather than by the window, and that is the fix rather than a tidying. Focus() on a + /// collapsed control is measurably a no-op that is not replayed when the control is revealed, and the + /// window's own attempt ran from the view model's PropertyChanged — ahead of the binding that makes + /// this control visible, so it focused a control that was still collapsed and the keyboard stayed wherever + /// it was. + /// + /// + /// Becoming visible is still too early on its own, which is why this is posted rather than called. A + /// control that has never been laid out has no visual children — measured: at the instant + /// IsVisible turns true the query box reports IsAttachedToVisualTree() == false, and focus is + /// refused to anything not in the tree. Layout runs at a higher priority than this callback, so by the + /// time it is picked up the box exists. + /// + /// + private void TakeKeyboard() + { + // The palette can have been dismissed between the post and the callback — a press on the wash, or a + // second Ctrl+K — and stealing the keyboard back into a control nobody can see would be worse than + // arriving late. + if (!IsVisible) + { + return; + } + + if (TopLevel.GetTopLevel(this) is Window window) + { + NativeKeyboardFocus.ReturnTo(window); + } + + Query.Focus(); + } + + /// Clamped rather than wrapped: a list that jumps from the last row to the first loses people. + private static void Move(MainWindowViewModel shell, int delta) + { + if (shell.SearchResults.Count == 0) + { + return; + } + + var current = shell.SelectedSearchResult is { } selected + ? shell.SearchResults.IndexOf(selected) + : -1; + + shell.SelectedSearchResult = + shell.SearchResults[Math.Clamp(current + delta, 0, shell.SearchResults.Count - 1)]; + } + + /// + protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change) + { + // Base first, so the visibility this control has just been given has already reached its descendants: + // focus is refused to anything not effectively visible, and the box below is a descendant. + base.OnPropertyChanged(change); + + if (change.Property == IsVisibleProperty && change.GetNewValue()) + { + Dispatcher.UIThread.Post(TakeKeyboard, DispatcherPriority.Loaded); + } + } + + private void OnPaletteKey(object? sender, KeyEventArgs e) => HandleKey(e); + + /// + /// Only a press on the wash itself. Presses on the card bubble through here as well, and closing on those + /// would make the palette impossible to click into. + /// + private void OnBackdropPressed(object? sender, PointerPressedEventArgs e) + { + if (!ReferenceEquals(e.Source, Backdrop) || Shell is not { IsSearching: true } shell) + { + return; + } + + shell.CloseSearchCommand.Execute(null); + e.Handled = true; + } } diff --git a/tests/DodoSSH.Client.App.Layout.Tests/QuickConnectTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/QuickConnectTests.cs new file mode 100644 index 0000000..916a5e3 --- /dev/null +++ b/tests/DodoSSH.Client.App.Layout.Tests/QuickConnectTests.cs @@ -0,0 +1,298 @@ +using Avalonia; +using Avalonia.Controls; +using Avalonia.Headless; +using Avalonia.Input; +using Avalonia.Threading; +using Avalonia.VisualTree; +using DodoSSH.Client.App.ViewModels; +using DodoSSH.Client.App.Views; +using DodoSSH.Client.Session; +using DodoSSH.Client.Session.Tests; +using DodoSSH.Client.Ssh; +using DodoSSH.Client.Storage; +using DodoSSH.Client.Terminal; +using DodoSSH.Crypto; +using NSubstitute; + +namespace DodoSSH.Client.App.Layout.Tests; + +/// +/// How the quick-connect palette answers a keyboard and a pointer. +/// +/// +/// +/// This is the suite the palette shipped without, and the reason it shipped without one is that all of this +/// used to live on MainWindow — which cannot be shown here at all, because attaching the terminal's +/// WebView initialises WebView2 on a thread it refuses. See +/// . A UserControl hosts in a bare +/// window, takes real key and pointer input, and can therefore be held to what it promises. +/// +/// +/// Three things were wrong and each has a test here: nothing answered a press outside the palette, so the one +/// gesture everybody tries first did nothing; the caret never reached the query box, because the window +/// focused it from the view model's PropertyChanged — ahead of the binding that reveals the control, +/// and focus on a collapsed control is a no-op; and the keys were answered only by a handler on the window, +/// which anything on the route could have taken first. +/// +/// +/// A real over a real unlocked vault, for the same reason the layout suite +/// uses one: compiled bindings resolve against the declared type, and the palette's list is populated by the +/// vault's own hosts. Nothing here reaches a network — the connect the Enter test performs fails inside the +/// vault's own error handling, which is fine, because what Enter promises is to take the highlighted result +/// and close. +/// +/// +public sealed class QuickConnectTests : IAsyncLifetime +{ + private const string Passphrase = "a sufficiently long passphrase"; + private const string ServerUrl = "https://dodossh.example"; + + /// Far below the shipped profile: nothing here attacks a wrap. + private static readonly Argon2Profile CheapProfile = + Argon2Profile.FromStoredParameters(memoryKibibytes: 8 * 1024, passes: 1, parallelism: 1); + + private readonly FakeAccountServer server = new(); + private readonly StubKeyBinding keyBinding = new(); + private readonly VaultKnownHostStore knownHosts = new(); + + private ClientCacheFactory caches = null!; + private TerminalWorkspace workspace = null!; + private VaultSession session = null!; + private VaultViewModel vault = null!; + private MainWindowViewModel shell = null!; + + private static CancellationToken Token => TestContext.Current.CancellationToken; + + /// + public async ValueTask InitializeAsync() + { + caches = ClientCacheFactory.ForMemory($"palette-{Guid.CreateVersion7():N}"); + await caches.MigrateAsync(Token); + + await new AccountProvisioner(server, keyBinding, caches, TimeProvider.System, CheapProfile) + .EnrollAsync(ServerUrl, Passphrase, "laptop", "Personal", Token); + + var outcome = await new SessionOpener(caches, TimeProvider.System).UnlockAsync(Passphrase, Token); + outcome.IsUnlocked.ShouldBeTrue(outcome.Message); + session = outcome.Session!; + + workspace = new TerminalWorkspace( + new InMemoryTerminalAssetProvider(new Dictionary(StringComparer.Ordinal)), + Substitute.For(), + TimeProvider.System); + + await knownHosts.OpenAsync(session, Token); + + vault = new VaultViewModel(session, workspace, knownHosts, static () => null); + + await SeedAsync(); + + shell = new MainWindowViewModel( + ClientPaths.Default, + caches, + workspace, + knownHosts, + Substitute.For(), + (_, _) => throw new NotSupportedException("nothing here signs in"), + TimeProvider.System, + CheapProfile) + { + // The state the palette is only ever open in. Assigned rather than reached through the unlock + // path, which would be a second enrollment and a second Argon2 pass for no extra coverage. + State = ShellState.Unlocked, + Vault = vault, + }; + } + + /// + public async ValueTask DisposeAsync() + { + await shell.DisposeAsync(); + knownHosts.Close(); + await workspace.DisposeAsync(); + await session.DisposeAsync(); + caches.Dispose(); + } + + /// + /// The gesture everybody tries first, and the one that did nothing at all: the wash took no pointer input, + /// so the only ways out of the palette were a key and the button that opened it. + /// + [Fact] + public async Task APressOnTheWashClosesThePalette() + { + await OnThePaletteAsync((_, window) => + { + // The bottom-left corner: the card is 520 wide, centred, and starts 90 pixels down, so nothing + // here belongs to it. + window.MouseDown(new Point(12, 520), MouseButton.Left); + + shell.IsSearching.ShouldBeFalse(); + }); + } + + /// + /// The other half of the same rule, and the one that makes it worth a handler rather than a press anywhere + /// closing: a press on the card bubbles through the wash on its way out, so a handler that did not check + /// where the press started would close the palette the moment somebody clicked into the box. + /// + [Fact] + public async Task APressOnTheCardDoesNotClose() + { + await OnThePaletteAsync((palette, window) => + { + window.MouseDown(Centre(palette.QueryBox, window), MouseButton.Left); + + shell.IsSearching.ShouldBeTrue(); + }); + } + + [Fact] + public async Task EscapeClosesThePalette() + { + await OnThePaletteAsync((palette, window) => + { + palette.QueryBox.Focus().ShouldBeTrue(); + + window.KeyPressQwerty(PhysicalKey.Escape, RawInputModifiers.None); + + shell.IsSearching.ShouldBeFalse(); + }); + } + + /// + /// The second assertion is the whole reason the selection is moved by hand rather than by letting the list + /// take focus: a palette whose arrow keys moved the caret out of the query box would stop receiving the + /// next character typed. + /// + [Fact] + public async Task TheArrowsMoveTheSelectionAndLeaveTheKeyboardInTheBox() + { + await OnThePaletteAsync((palette, window) => + { + palette.QueryBox.Focus().ShouldBeTrue(); + + shell.SearchResults.Count.ShouldBeGreaterThan(2, "an empty list would prove nothing"); + shell.SelectedSearchResult.ShouldBe(shell.SearchResults[0]); + + window.KeyPressQwerty(PhysicalKey.ArrowDown, RawInputModifiers.None); + shell.SelectedSearchResult.ShouldBe(shell.SearchResults[1]); + + window.KeyPressQwerty(PhysicalKey.ArrowUp, RawInputModifiers.None); + shell.SelectedSearchResult.ShouldBe(shell.SearchResults[0]); + + // Clamped rather than wrapped, which is the palette's own rule. + window.KeyPressQwerty(PhysicalKey.ArrowUp, RawInputModifiers.None); + shell.SelectedSearchResult.ShouldBe(shell.SearchResults[0]); + + palette.QueryBox.IsFocused.ShouldBeTrue("the arrows must not move the caret out of the box"); + }); + } + + /// + /// What Enter promises is to take the highlighted row: the palette closes and the vault is pointed at that + /// host. The connection it then asks for fails in this suite — there is no server and no shell — and it + /// fails inside the vault's own handling, which is the point of connecting through the vault's command + /// rather than opening a session from the palette. + /// + [Fact] + public async Task EnterTakesTheHighlightedResult() + { + await OnThePaletteAsync((palette, window) => + { + palette.QueryBox.Focus().ShouldBeTrue(); + + window.KeyPressQwerty(PhysicalKey.ArrowDown, RawInputModifiers.None); + var highlighted = shell.SelectedSearchResult.ShouldNotBeNull(); + + window.KeyPressQwerty(PhysicalKey.Enter, RawInputModifiers.None); + + shell.IsSearching.ShouldBeFalse(); + vault.SelectedHost?.EntityId.ShouldBe(highlighted.EntityId); + }); + } + + /// + /// The palette is a box somebody is expected to start typing into, and for a while it was not: the window + /// focused it from the view model's PropertyChanged, which runs before the binding that reveals the + /// control, and Focus() on a collapsed control is a no-op that is never replayed. Becoming visible + /// is the moment that cannot be too early, so that is where the palette takes the keyboard — and this is + /// the test that says so. + /// + [Fact] + public async Task ThePaletteTakesTheKeyboardWhenItAppears() + { + await LayoutHarness.OnTheUiThreadAsync( + () => + { + var elsewhere = new TextBox(); + var palette = new QuickConnect { DataContext = shell, IsVisible = false }; + + var window = new Window { Content = new Panel { Children = { elsewhere, palette } } }; + LayoutHarness.Settle(window, 900, 600); + + try + { + elsewhere.Focus().ShouldBeTrue(); + + shell.ToggleSearchCommand.Execute(null); + palette.IsVisible = true; + + // The layout pass the application's dispatcher would run anyway. Without it the query box + // is not in the visual tree yet, which is the whole reason the palette defers this. + Dispatcher.UIThread.RunJobs(); + + palette.QueryBox.IsFocused.ShouldBeTrue(); + } + finally + { + window.Close(); + } + }, + Token); + } + + // ---- Helpers ---- + + /// Opens the palette in a window the size the application's is, and runs one body against it. + private Task OnThePaletteAsync(Action body) => + LayoutHarness.OnTheUiThreadAsync( + () => + { + shell.ToggleSearchCommand.Execute(null); + shell.IsSearching.ShouldBeTrue("every case here starts with the palette open"); + + var palette = new QuickConnect { DataContext = shell }; + var window = new Window { Content = palette }; + LayoutHarness.Settle(window, 900, 600); + + try + { + body(palette, window); + } + finally + { + window.Close(); + } + }, + Token); + + private static Point Centre(Visual control, Visual window) => + control.TranslatePoint(new Point(control.Bounds.Width / 2, control.Bounds.Height / 2), window) + ?? throw new InvalidOperationException("the control is not in this window's tree"); + + /// Enough hosts that the arrow keys have somewhere to go. + private async Task SeedAsync() + { + for (var i = 0; i < 6; i++) + { + vault.NewHostCommand.Execute(null); + vault.EditorLabel = $"host-{i}"; + vault.EditorHostname = $"host-{i}.internal"; + vault.EditorUsername = "deploy"; + await vault.SaveHostCommand.ExecuteAsync(null); + } + + await vault.LoadAsync(Token); + } +} From 9c3edb078e5d88004d1d12189642411f3ec393be Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit Date: Fri, 31 Jul 2026 08:43:39 +0000 Subject: [PATCH 3/6] Update .github/workflows/ci.yml --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1354136..df3fca5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ env: jobs: build: name: build and test - runs-on: [self-hosted, linux] + runs-on: [self-hosted] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 From 66271faaae713d5dc540da53b7f7937753cac943 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit Date: Fri, 31 Jul 2026 08:44:19 +0000 Subject: [PATCH 4/6] Update .github/workflows/ci.yml --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df3fca5..7deae0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ env: jobs: build: name: build and test - runs-on: [self-hosted] + runs-on: [linux] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 From f7c5096bc6a7c9b02433ed08d5f3700826d91421 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 31 Jul 2026 11:06:46 +0200 Subject: [PATCH 5/6] Keep the stub servers on loopback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/DodoSSH.Api.Tests/StubIdentityProvider.cs | 7 ++++++- tests/DodoSSH.Client.Api.Tests/StubServer.cs | 10 +++++++++- tests/DodoSSH.Client.Auth.Tests/StubProvider.cs | 6 +++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/DodoSSH.Api.Tests/StubIdentityProvider.cs b/tests/DodoSSH.Api.Tests/StubIdentityProvider.cs index b81edce..399bcc5 100644 --- a/tests/DodoSSH.Api.Tests/StubIdentityProvider.cs +++ b/tests/DodoSSH.Api.Tests/StubIdentityProvider.cs @@ -6,6 +6,7 @@ using Microsoft.IdentityModel.Tokens; using WireMock.RequestBuilders; using WireMock.ResponseBuilders; using WireMock.Server; +using WireMock.Settings; namespace DodoSSH.Api.Tests; @@ -30,7 +31,11 @@ public sealed class StubIdentityProvider : IDisposable var rsa = RSA.Create(2048); signingKey = new RsaSecurityKey(rsa) { KeyId = KeyId }; - server = WireMockServer.Start(); + // Loopback explicitly: WireMock's default listens on every interface, which makes Windows Firewall + // prompt the first time each test executable runs — per binary path, so a new worktree or + // configuration asks again. Port 0 still picks a free port and reports it on server.Url, which is + // what Authority below is built from, so the issuer the tokens claim follows the binding. + server = WireMockServer.Start(new WireMockServerSettings { Urls = ["http://127.0.0.1:0"] }); Authority = server.Url!.TrimEnd('/'); StubDiscovery(); diff --git a/tests/DodoSSH.Client.Api.Tests/StubServer.cs b/tests/DodoSSH.Client.Api.Tests/StubServer.cs index dd5f46d..cd69f49 100644 --- a/tests/DodoSSH.Client.Api.Tests/StubServer.cs +++ b/tests/DodoSSH.Client.Api.Tests/StubServer.cs @@ -4,6 +4,7 @@ using DodoSSH.Contracts; using WireMock.RequestBuilders; using WireMock.ResponseBuilders; using WireMock.Server; +using WireMock.Settings; namespace DodoSSH.Client.Api.Tests; @@ -15,7 +16,14 @@ namespace DodoSSH.Client.Api.Tests; /// internal sealed class StubServer : IDisposable { - private readonly WireMockServer server = WireMockServer.Start(); + /// + /// Bound to loopback explicitly. WireMock's default listens on every interface, which makes Windows + /// Firewall prompt the first time each test executable runs — and the prompt is per binary path, so a + /// new worktree or configuration asks again. Port 0 still picks a free port and reports it on + /// . + /// + private readonly WireMockServer server = WireMockServer.Start( + new WireMockServerSettings { Urls = ["http://127.0.0.1:0"] }); internal Uri BaseUrl => new(server.Url!, UriKind.Absolute); diff --git a/tests/DodoSSH.Client.Auth.Tests/StubProvider.cs b/tests/DodoSSH.Client.Auth.Tests/StubProvider.cs index b235770..c0aa700 100644 --- a/tests/DodoSSH.Client.Auth.Tests/StubProvider.cs +++ b/tests/DodoSSH.Client.Auth.Tests/StubProvider.cs @@ -4,6 +4,7 @@ using System.Text.Json.Nodes; using WireMock.RequestBuilders; using WireMock.ResponseBuilders; using WireMock.Server; +using WireMock.Settings; namespace DodoSSH.Client.Auth.Tests; @@ -16,7 +17,10 @@ internal sealed class StubProvider : IDisposable bool advertiseS256 = true, string? issuerOverride = null) { - server = WireMockServer.Start(); + // Loopback explicitly: WireMock's default listens on every interface, which makes Windows Firewall + // prompt the first time each test executable runs — per binary path, so a new worktree or + // configuration asks again. Port 0 still picks a free port and reports it on server.Url. + server = WireMockServer.Start(new WireMockServerSettings { Urls = ["http://127.0.0.1:0"] }); Authority = new Uri(server.Url!.TrimEnd('/'), UriKind.Absolute); StubDiscovery(advertiseS256, issuerOverride); From 04faef65974f1bd64da786c56d6d0d266fb15492 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Fri, 31 Jul 2026 11:07:29 +0200 Subject: [PATCH 6/6] Move files to and from a host over SFTP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- DodoSSH.slnx | 2 + README.md | 40 +- docs/design-import-gaps.md | 50 +- src/DodoSSH.Client.App/App.axaml.cs | 9 +- .../DodoSSH.Client.App.csproj | 1 + .../ViewModels/MainWindowViewModel.cs | 50 +- .../ViewModels/TransfersViewModel.cs | 935 ++++++++++++++++++ .../ViewModels/VaultViewModel.cs | 34 +- src/DodoSSH.Client.App/Views/MainWindow.axaml | 23 +- src/DodoSSH.Client.App/Views/NavRail.axaml | 12 +- .../Views/TransfersScreen.axaml | 404 ++++++++ .../Views/TransfersScreen.axaml.cs | 48 + src/DodoSSH.Client.App/packages.lock.json | 6 + src/DodoSSH.Client.Ssh/SftpSession.cs | 357 +++++++ .../SshNetConnectionFactory.cs | 72 +- src/DodoSSH.Client.Ssh/SshNetSftpSession.cs | 250 +++++ .../DodoSSH.Client.Transfer.csproj | 22 + .../FileTransferQueue.cs | 870 ++++++++++++++++ src/DodoSSH.Client.Transfer/LocalDirectory.cs | 126 +++ .../packages.lock.json | 54 + .../ScreenLayoutTests.cs | 126 +++ .../packages.lock.json | 9 +- tests/DodoSSH.Client.App.Tests/FakeSsh.cs | 85 +- .../ShellFlowTests.cs | 89 +- .../packages.lock.json | 9 +- .../RemotePathTests.cs | 99 ++ .../SftpSessionTests.cs | 262 +++++ .../SshServerFixture.cs | 63 ++ .../DodoSSH.Client.Transfer.Tests.csproj | 16 + .../FakeSftpSession.cs | 258 +++++ .../FileTransferQueueTests.cs | 365 +++++++ .../packages.lock.json | 240 +++++ 32 files changed, 4933 insertions(+), 53 deletions(-) create mode 100644 src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs create mode 100644 src/DodoSSH.Client.App/Views/TransfersScreen.axaml create mode 100644 src/DodoSSH.Client.App/Views/TransfersScreen.axaml.cs create mode 100644 src/DodoSSH.Client.Ssh/SftpSession.cs create mode 100644 src/DodoSSH.Client.Ssh/SshNetSftpSession.cs create mode 100644 src/DodoSSH.Client.Transfer/DodoSSH.Client.Transfer.csproj create mode 100644 src/DodoSSH.Client.Transfer/FileTransferQueue.cs create mode 100644 src/DodoSSH.Client.Transfer/LocalDirectory.cs create mode 100644 src/DodoSSH.Client.Transfer/packages.lock.json create mode 100644 tests/DodoSSH.Client.Ssh.Tests/RemotePathTests.cs create mode 100644 tests/DodoSSH.Client.Ssh.Tests/SftpSessionTests.cs create mode 100644 tests/DodoSSH.Client.Transfer.Tests/DodoSSH.Client.Transfer.Tests.csproj create mode 100644 tests/DodoSSH.Client.Transfer.Tests/FakeSftpSession.cs create mode 100644 tests/DodoSSH.Client.Transfer.Tests/FileTransferQueueTests.cs create mode 100644 tests/DodoSSH.Client.Transfer.Tests/packages.lock.json diff --git a/DodoSSH.slnx b/DodoSSH.slnx index efb273d..5d27ab4 100644 --- a/DodoSSH.slnx +++ b/DodoSSH.slnx @@ -25,6 +25,7 @@ + @@ -39,6 +40,7 @@ + diff --git a/README.md b/README.md index 4ebf4c4..f018ec1 100644 --- a/README.md +++ b/README.md @@ -65,8 +65,9 @@ src/ DodoSSH.Client.Storage the local cache: ciphertext mirror, outbox, offline unlock material DodoSSH.Client.Sync the pull/apply/push loop and the conflict policy DodoSSH.Client.Session where a profile lives, unlocking it, and getting one in the first place - DodoSSH.Client.Ssh connections, PTY shells, host key trust + DodoSSH.Client.Ssh connections, PTY shells, SFTP, host key trust DodoSSH.Client.Terminal the loopback data plane and credit-based flow control + DodoSSH.Client.Transfer the transfer queue, part files and resume, and the local file listing DodoSSH.Client.App Avalonia; the only project that knows about a UI toolkit tests/ one test project per source project docs/adr/ architecture decision records @@ -150,6 +151,37 @@ authentication asks for the password every time, because nothing in the interfac credential yet (they do sync — there is just no editor for one); and unlock asks for the passphrase on every launch, because no device key is registered. +### Moving files + +**FILES** in the nav rail is a two-pane browser: this machine on the left, the host on the right, and a +queue underneath. Choose a host, press **CONNECT**, then select a file in either pane and press the arrow +pointing the way you want it to go. + +Two things about it are worth expecting rather than discovering. + +**It is a second connection, not a second channel.** SSH itself would allow the SFTP subsystem to open +beside a shell on the transport that is already up; SSH.NET does not offer that — its `SftpClient` owns its +own transport — so pressing CONNECT here authenticates again. The host records a second login, and a host +whose password you type each time will ask for it again on this screen. Host key trust is shared: a +fingerprint approved for a terminal is approved here, and one approved here reaches your other machines with +the next sync. + +**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 people actually use this 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 remote pane has **DELETE** and **MKDIR** so that refusal is not a dead end. +**RESUME** on a stopped transfer carries on from what the part file already holds. + +Resume works within a run of the application and not across a restart, and that limit is deliberate: 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. + +What is not here: transferring a directory, dragging between the panes, and routing a transfer through a +bastion — the last needs jump hosts the connection layer has not got. All three are in +[`docs/design-import-gaps.md`](docs/design-import-gaps.md). + ### End-to-end verification One suite runs against a real server rather than a stub. It needs a Docker daemon and nothing else, so it @@ -230,7 +262,11 @@ off-Windows. a key are written at version 2 and become read-only on an older build. Hosts that do not are still written at version 1, byte-identically to before the field existed — which is what keeps upgrading one machine from making a team's whole vault uneditable everywhere else. -- **M2 — full personal vault**, robust sync, relay. +- **M2 — full personal vault**, robust sync, relay. *File transfer done:* an SFTP session, a two-pane file + browser with a real remote listing — names, sizes, modification times and `drwxr-xr-x` permission bits — + and a queue that moves one file at a time with progress, throughput and resume. See + [Moving files](#moving-files) for the two things about it worth knowing before you use it, both of which + are consequences rather than choices. - **M3 — teams**, sharing, ACLs. - **M4 — hardening and ops**, packaging, self-hosting guide. - **M5 — multi-provider OIDC**, key rotation, per-item content keys. diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index a1dd79a..0717a27 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -18,9 +18,13 @@ M2 and M3 arriving in a design before it arrives in the code. Three facts explain nearly every row below. **An SSH connection here opens exactly one channel.** `ISshConnection` offers `OpenShellAsync` and nothing -else (`src/DodoSSH.Client.Ssh/SshConnection.cs`). No SFTP subsystem, no port forwarding, no ProxyJump. That -one fact removes the whole file-transfer screen, the `FORWARDS` chip, the status bar's port list, and every -`via bastion-eu` in the design. +else (`src/DodoSSH.Client.Ssh/SshConnection.cs`). No port forwarding, no ProxyJump. That one fact removes +the `FORWARDS` chip, the status bar's port list, and every `via bastion-eu` in the design. + +It used to remove the file-transfer screen too. M2 did not lift the restriction — it worked around it: +file transfer is a *separate connection* rather than a second channel, because SSH.NET's `SftpClient` owns +its own transport. See [File transfer](#file-transfer-the-designs-sftp-screen), which is the one place in +this document where what shipped differs from what the row predicted. **Teams are schema and nothing else.** The `team` and `team_membership` tables exist from the first migration, with entities in `DodoSSH.Domain/Teams.cs` and a `TeamRole` enum — and no endpoint reads or @@ -50,7 +54,7 @@ protocol rather than a protocol change. | `⌘K` command palette running commands | client-domain | A snippet or saved-command item type (`SyncEntityType.Snippet = 8` is reserved). | Ctrl+K opens a real host search that connects on Enter. The box says "search hosts", not "search hosts · run command". | | Status bar `· via bastion-eu` | client-ssh | Jump-host execution. See below. | Omitted. | | Status bar port forwards | client-ssh | Port forwarding. See below. | Omitted. | -| Status bar `sftp · 2 transfers` | client-ssh | File transfer. See below. | Omitted. | +| Status bar `sftp · 2 transfers` | client-app | Nothing now — file transfer is built. What is missing is the count reaching the status bar, which is a screen away from where the queue lives. | Omitted from the status bar. The queue itself is on the FILES screen, with a row per transfer. | | Status bar `locks in 09:41` | client-app | An idle auto-lock. See preferences below. | Omitted. | | IBM Plex Mono / IBM Plex Sans | ui | Shipping the font files as `AvaloniaResource` and registering them. The design loads them from Google Fonts, which a desktop app cannot. | Inter (already embedded) for prose, and the system monospace stack the terminal already names. Named once in `App.axaml` as `MonoFont`, so the substitution is reversible in one place. | | `⌘K`, `⌥↵` | ui | Nothing; the design is Mac-flavoured. | `CTRL K`. Development is Windows-only today (`docs/platform-flags.md`). | @@ -86,18 +90,36 @@ caption buttons and window title drawn on top of the application's own — two s ## File transfer (the design's SFTP screen) -Nothing on this screen exists. It is listed in the nav rail and reaches a screen that says so, naming the -milestone and what is missing — see `ShellScreen` for why it is not simply dropped from the rail. +**Built in M2.** The screen ships: two directory panes, a breadcrumb trail on each, and a queue that moves +one file at a time with progress, throughput and resume. What follows is what it does *not* do, and one +thing this document got wrong before it was built. -| Design element | Layer | What it would take | +**The correction.** The row below used to say an SFTP subsystem channel on `ISshConnection` was what it +would take. SSH.NET does not offer that: `SftpClient` derives from `BaseClient` and owns its own transport, +and there is no supported way to hand it a session an `SshClient` already has. So the transfers screen +**opens a second authenticated connection** to the host rather than a second channel on the terminal's. That +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 — so it is named for what it is: `ISftpSessionFactory.OpenSftpAsync` is a +connect, and it goes through the same host key gate, the same pin and the same two refusals a shell does. + +| Design element | Layer | What ships | | --- | --- | --- | -| SFTP itself | client-ssh | An SFTP subsystem channel on `ISshConnection`. There is no `SftpClient`, `ScpClient` or transfer type anywhere in `src/`. | -| Remote listing with `NAME/SIZE/MODIFIED/PERMS` | client-ssh | The channel, plus a listing record and a POSIX mode formatter — nothing in the repo formats a `drwxr-xr-x`. | -| Local listing | client-app | The App project contains no `System.IO` usage at all. The only paths this client knows are its own two files. | -| Path breadcrumbs, per-host last directory | client-storage | Navigation state for two panes, and somewhere to persist it. There is no settings table. | -| Transfer queue, progress, throughput | client-ssh | A transfer engine. **Do not reach for `CreditWindow`** — that is a 256 KiB flow-control window for terminal output, not a transfer primitive. | -| `resume supported` | client-ssh | Offset-based reads and writes, plus partial-transfer bookkeeping that survives a restart. | -| `sftp over bastion-eu` | client-ssh | Jump hosts, as above. | +| SFTP itself | client-ssh | `ISftpSession` over SSH.NET's `SftpClient`: listing, stat, offset-based read and write, mkdir, delete and rename. Delete is deliberately **not** recursive. | +| Remote listing with `NAME/SIZE/MODIFIED/PERMS` | client-ssh | All four. `PosixMode` renders `drwxr-xr-x` from the bits SFTP hands over; setuid, setgid and sticky are not shown, because SSH.NET does not surface them and `rwx` where `rws` is true would be worse than nothing. | +| Local listing | client-transfer | `LocalDirectory`, which is where this client's `System.IO` now lives. `PERMS` is blank on the local side rather than filled with a plausible-looking POSIX mode that is not a fact about a file on Windows. | +| Transfer queue, progress, throughput | client-transfer | `FileTransferQueue`. One transfer at a time, so the rate on a row is the rate of the link rather than a share of it. Throughput is measured over a half-second window, not averaged since the start. | +| `resume supported` | client-transfer | **Within a run of the application.** Every transfer writes to a `.dodossh-part` file beside its destination and is renamed into place at the end, so an interrupted one can never be mistaken for a finished one, and `RESUME` carries on from the part file's own length. A part file found at startup is *not* resumed: nothing records what wrote it, and resuming on the strength of a name matching is how a corrupt artefact gets delivered with nothing reporting a failure. Making it survive a restart needs the preferences store this client has not got — see below. | + +| Design element | Layer | What it would take | What ships instead | +| --- | --- | --- | --- | +| Per-host last directory | client-storage | Somewhere to persist two panes' navigation state. There is still no settings table. | The remote pane opens on the account's home directory, which the server canonicalises during the handshake; the local pane opens on the user profile. | +| `sftp over bastion-eu` | client-ssh | Jump hosts, as above. `HostSecret.JumpHostIds` is still stored, synced, merged and read by nothing. | Omitted. | +| Overwriting a file that is already there | ui | A prompt, which means a modal this window has no idiom for. | Refused, with the name that is in the way. The remote pane has DELETE and MKDIR so the refusal is not a dead end. | +| Dragging between the panes | ui | Drag-and-drop between two `ListBox`es, plus a drop target that is a directory rather than a row. | Two arrow buttons between the panes, pointing at the pane the file is going to. | +| Transferring a directory | client-transfer | Recursive enumeration on both sides, and a policy for what a partial directory means. | One file at a time. A directory cannot be selected as a transfer source. | + +**Not to be reached for:** `CreditWindow` is a 256 KiB flow-control window for terminal output, not a +transfer primitive. The queue does its own 64 KiB copy loop and shares nothing with the terminal data plane. --- diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index 1b8496a..814305a 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -58,9 +58,13 @@ internal sealed partial class DodoSshApp : Application // for why the handshake is answered from a snapshot rather than by reading the vault per lookup. var knownHosts = new VaultKnownHostStore(); + // One factory for both kinds of connection. Shells and file transfers start with the same handshake + // and the same host key decision, and composing two would mean two snapshots of the pins. + var connections = new SshNetConnectionFactory(knownHosts); + var workspace = new TerminalWorkspace( new AvaloniaTerminalAssetProvider(), - new SshNetConnectionFactory(knownHosts), + connections, TimeProvider.System); workspace.Start(); @@ -81,7 +85,8 @@ internal sealed partial class DodoSshApp : Application async (url, cancellationToken) => await ServerConnection .SignInAsync(url, browser, TimeProvider.System, cancellationToken) .ConfigureAwait(false), - TimeProvider.System); + TimeProvider.System, + connections); desktop.MainWindow = new MainWindow { DataContext = viewModel }; diff --git a/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj b/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj index 5489fa2..ce05c36 100644 --- a/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj +++ b/src/DodoSSH.Client.App/DodoSSH.Client.App.csproj @@ -28,6 +28,7 @@ + diff --git a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs index c22a633..e5294ca 100644 --- a/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/MainWindowViewModel.cs @@ -7,6 +7,7 @@ using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; using DodoSSH.Client.Auth; using DodoSSH.Client.Session; +using DodoSSH.Client.Ssh; using DodoSSH.Client.Storage; using DodoSSH.Client.Terminal; using DodoSSH.Crypto; @@ -52,11 +53,10 @@ internal enum ShellState /// in you are, the other is what you are looking at once you are. /// /// -/// and are in this list without anything behind them, which is -/// stated on the screens themselves rather than hidden by dropping them from the rail. See -/// docs/design-import-gaps.md: file transfer is M2 and teams are M3, and a rail that quietly had -/// three entries would make the eventual arrival of the other two look like a new product rather than a -/// milestone. +/// is in this list without anything behind it, which is stated on the screen itself +/// rather than hidden by dropping it from the rail. See docs/design-import-gaps.md: teams are M3, and +/// a rail that quietly had four entries would make its eventual arrival look like a new product rather than +/// a milestone. was the other one until M2 built it. /// /// internal enum ShellScreen @@ -64,7 +64,7 @@ internal enum ShellScreen /// The host list and the terminals, which is where the application opens. Hosts = 0, - /// File transfer. Nothing implements it yet. + /// File transfer over SFTP: two directory panes and a queue. Transfers = 1, /// Everything in the vault that is not a host. @@ -118,6 +118,14 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp private readonly TimeProvider clock; private readonly Argon2Profile? passphraseProfile; + /// + /// Created once and kept for the life of the process, like and for the same + /// reason: file transfer opens its own authenticated connection, and locking the vault must not destroy + /// a transfer in flight any more than it closes a running shell. See . The vault + /// is attached to it on unlock and detached on lock, which is all the vault is for here — the host list. + /// + private readonly TransfersViewModel transfers; + private IVaultServer? connection; private bool disposed; @@ -132,6 +140,11 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// internal delegate Task SignInHandler(Uri serverUrl, CancellationToken cancellationToken); + /// + /// How file-transfer sessions are opened. The same object as the connection factory in the composed + /// application — one type implements both — and a separate parameter because it is a separate capability + /// and the tests that drive this state machine have no use for it. + /// internal MainWindowViewModel( ClientPaths paths, ClientCacheFactory caches, @@ -140,6 +153,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp IDeviceKeyStore deviceKeys, SignInHandler signIn, TimeProvider clock, + ISftpSessionFactory sftpSessions, Argon2Profile? passphraseProfile = null) { this.paths = paths; @@ -151,6 +165,8 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp this.clock = clock; this.passphraseProfile = passphraseProfile; + transfers = new TransfersViewModel(sftpSessions, clock); + // Subscribed for the life of the process, because the workspace lives that long and so does the tab // list. Detached in DisposeAsync, which is the only point either of them ends. this.workspace.SessionEnded += OnWorkspaceSessionEnded; @@ -222,6 +238,14 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp [ObservableProperty] private VaultViewModel? vault; + /// The transfers screen, which the window binds to whether or not a vault is open. + /// + /// Not nullable and never replaced, unlike . The screen is unreachable while locked — + /// the whole shell is — but the object behind it is what holds a transfer that is still running, so a + /// property that went null on lock would be a transfer nothing could report on afterwards. + /// + internal TransfersViewModel Transfers => transfers; + /// /// Shells that were left running when the vault was locked. /// @@ -900,6 +924,10 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp await Vault.LoadAsync(cancellationToken).ConfigureAwait(true); + // After the load, because what the transfers screen takes from the vault is the host list and an + // empty one would leave its picker blank until the next unlock. + transfers.Attach(Vault, knownHosts); + // After the list exists, and it matters after a lock rather than after the first unlock: shells kept // running while the vault was closed, so some of these hosts are connected before their rows are a // second old. @@ -947,6 +975,11 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp // reappearing behind a lock screen. knownHosts.Close(); + // Before the vault goes, because its host rows carry decrypted secrets and the transfers screen is + // holding references to them. What it does not give up is its connection or its queue — a transfer + // in flight is exactly the work this method exists not to destroy. + transfers.Detach(); + if (Vault is { } open) { Vault = null; @@ -973,6 +1006,11 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp knownHosts.Close(); + // Before the vault, and it waits: a transfer still writing has an open remote file and an open local + // one, and a process that exits while those are in flight leaves a part file longer than the bytes + // that reached it. + await transfers.DisposeAsync().ConfigureAwait(false); + if (Vault is { } open) { await open.DisposeAsync().ConfigureAwait(false); diff --git a/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs b/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs new file mode 100644 index 0000000..d3a2bbf --- /dev/null +++ b/src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs @@ -0,0 +1,935 @@ +using System.Collections.ObjectModel; +using System.Globalization; +using Avalonia.Threading; +using CommunityToolkit.Mvvm.ComponentModel; +using CommunityToolkit.Mvvm.Input; +using DodoSSH.Client.Session; +using DodoSSH.Client.Ssh; +using DodoSSH.Client.Transfer; + +namespace DodoSSH.Client.App.ViewModels; + +/// One segment of a path, as a button in a breadcrumb trail. +/// What the segment is called. +/// The absolute path that reaches it. +internal sealed record CrumbViewModel(string Name, string Path); + +/// One remote file or directory, as a row. +internal sealed class RemoteEntryRowViewModel(SftpEntry entry) +{ + internal SftpEntry Entry => entry; + + internal string Name => entry.Name; + + internal string FullPath => entry.FullPath; + + internal bool IsNavigable => entry.IsNavigable; + + internal bool IsFile => entry.Kind is SftpEntryKind.File; + + /// + /// A directory shows nothing rather than a zero. Its inode's size is a number no user has ever wanted, + /// and a column of zeroes beside real sizes reads as a listing that failed to measure them. + /// + internal string Size => entry.Kind is SftpEntryKind.File ? ByteSize.Format(entry.Length) : string.Empty; + + internal string Modified => Timestamps.Format(entry.LastWriteTimeUtc); + + /// The mode as drwxr-xr-x, which is the design's PERMS column. + internal string Permissions => entry.Permissions; +} + +/// One local file or directory, as a row. +/// +/// The same shape as the remote row minus the permissions, which have no honest value here: this client is +/// developed on Windows, where a POSIX mode is not a fact about a file. The column is empty on this side +/// rather than filled with a plausible-looking -rw-r--r--. +/// +internal sealed class LocalEntryRowViewModel(LocalEntry entry) +{ + internal LocalEntry Entry => entry; + + internal string Name => entry.Name; + + internal string FullPath => entry.FullPath; + + internal bool IsNavigable => entry.IsDirectory; + + internal bool IsFile => !entry.IsDirectory; + + internal string Size => entry.IsDirectory ? string.Empty : ByteSize.Format(entry.Length); + + internal string Modified => Timestamps.Format(entry.LastWriteTimeUtc); +} + +/// How this screen writes a modification time. +/// +/// +/// UTC and ISO-ordered, in one place, because both panes show this column side by side: a local pane in this +/// machine's conventions beside a remote pane in the server's would invite comparing two timestamps that are +/// not written the same way, which is the only thing anybody does with this column. +/// +/// +/// The one place in this application that deliberately ignores the user's locale — see the App project's +/// InvariantGlobalization, which is false precisely so dates elsewhere follow it. Sortable order and +/// an unambiguous zone beat familiarity when the two columns have to be read against each other. +/// +/// +internal static class Timestamps +{ + internal static string Format(DateTimeOffset moment) => + moment.UtcDateTime.ToString("yyyy-MM-dd HH:mm", CultureInfo.InvariantCulture); +} + +/// One transfer, as a row in the queue. +/// +/// Observable and long-lived, unlike the two listing rows, because a transfer's progress changes several +/// times a second while its identity does not. It is refreshed from rather +/// than holding the queue's own object: the queue mutates its entries from a thread-pool thread, and this is +/// only ever read from the UI thread. +/// +internal sealed partial class TransferRowViewModel(TransferSnapshot snapshot) : ObservableObject +{ + [ObservableProperty] + private TransferSnapshot transfer = snapshot; + + internal Guid Id => Transfer.Id; + + internal string Name => Transfer.Name; + + /// Which way, as an arrow the eye can scan a column of. + internal string Arrow => Transfer.Direction is TransferDirection.Download ? "↓" : "↑"; + + /// The end that is not this machine, which is the one worth showing. + internal string Path => Transfer.Direction is TransferDirection.Download + ? Transfer.RemotePath + : Transfer.LocalPath; + + internal double Percent => Transfer.Fraction * 100; + + /// + /// What the row says about where it has got to. + /// + /// + /// The bytes and the rate together while it runs, because either alone leaves the obvious question + /// unanswered — a rate with no total cannot say how long is left, and a total with no rate cannot say + /// whether it is still moving. + /// + internal string Progress => Transfer.State switch + { + TransferState.Queued => "queued", + TransferState.Running => + $"{ByteSize.Format(Transfer.Transferred)} of {ByteSize.Format(Transfer.Length)}" + + $" · {ByteSize.Format((long)Transfer.BytesPerSecond)}/s", + TransferState.Completed => ByteSize.Format(Transfer.Length), + TransferState.Cancelled when Transfer.Transferred > 0 => + $"stopped at {ByteSize.Format(Transfer.Transferred)} of {ByteSize.Format(Transfer.Length)}", + TransferState.Cancelled => "stopped", + _ => Transfer.Failure ?? "failed", + }; + + internal string StateLabel => Transfer.State switch + { + TransferState.Queued => "QUEUED", + TransferState.Running => "RUNNING", + TransferState.Completed => "DONE", + TransferState.Cancelled => "STOPPED", + _ => "FAILED", + }; + + internal bool IsRunning => Transfer.State is TransferState.Running or TransferState.Queued; + + internal bool IsFinished => Transfer.IsFinished; + + internal bool CanResume => Transfer.CanResume; + + internal bool HasFailed => Transfer.State is TransferState.Failed; + + /// Whether a stopped transfer is worth offering to run again at all. + /// + /// Wider than : a transfer that failed before it moved a byte — the host was + /// unreachable, the destination was occupied — is worth retrying from the start, and only the button's + /// wording differs. See . + /// + internal bool CanRetry => Transfer.IsFinished && Transfer.State is not TransferState.Completed; + + internal string RetryLabel => CanResume ? "RESUME" : "RETRY"; + + /// + /// Every derived member at once. They are one fact — the snapshot — read from eight directions, and + /// raising only the ones that happened to change is how a progress bar moves under a label that still + /// says "queued". + /// + partial void OnTransferChanged(TransferSnapshot value) + { + OnPropertyChanged(nameof(Progress)); + OnPropertyChanged(nameof(Percent)); + OnPropertyChanged(nameof(StateLabel)); + OnPropertyChanged(nameof(IsRunning)); + OnPropertyChanged(nameof(IsFinished)); + OnPropertyChanged(nameof(CanResume)); + OnPropertyChanged(nameof(CanRetry)); + OnPropertyChanged(nameof(HasFailed)); + OnPropertyChanged(nameof(RetryLabel)); + } +} + +/// +/// The transfers screen: a host, two directory panes, and the queue between them. +/// +/// +/// +/// Its connection is its own. SSH.NET cannot open an SFTP subsystem on a transport that is already +/// carrying a shell — see ISftpSession — so this screen authenticates separately, and connecting here +/// is a deliberate act with its own button rather than something that happens because a terminal is open. +/// The consequence a user sees is that the host records a second login, and that a host whose password is +/// typed each time asks for it again here. +/// +/// +/// It outlives a lock, as terminals do. This object is created once and the vault is attached to it +/// on unlock and detached on lock, the same arrangement VaultKnownHostStore has and for the same +/// reason: MainWindowViewModel.LockAsync argues that locking must not destroy work in flight, and a +/// half-finished transfer is the clearest case of work in flight there is. What locking takes away is the +/// host list — those are decrypted vault items — and not the connection or the queue. +/// +/// +internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDisposable +{ + private readonly ISftpSessionFactory sftp; + private readonly FileTransferQueue queue; + + private VaultViewModel? vault; + private VaultKnownHostStore? knownHosts; + private ISftpSession? session; + private bool disposed; + + internal TransfersViewModel(ISftpSessionFactory sftp, TimeProvider clock) + { + this.sftp = sftp; + + // The supplier answers with whatever session is current at the moment a transfer starts, which is + // what lets a queue survive a disconnect and reconnect without every queued row failing. + queue = new FileTransferQueue(_ => Task.FromResult(RequireSession()), clock); + queue.Changed += OnTransferChanged; + + // The three "is there anything in it" flags follow their collections rather than being raised by + // hand at each of the eight places that add or clear a row. Subscribed for the life of this object, + // which is the life of the process. + RemoteEntries.CollectionChanged += (_, _) => OnPropertyChanged(nameof(HasRemoteEntries)); + LocalEntries.CollectionChanged += (_, _) => OnPropertyChanged(nameof(HasLocalEntries)); + Transfers.CollectionChanged += (_, _) => OnPropertyChanged(nameof(HasTransfers)); + } + + /// The hosts that can be connected to, which is the vault's list. + /// + /// A collection of its own rather than the vault's, because it is empty while locked and the vault's is + /// not this object's to clear. The rows are shared: they carry the decrypted host, and copying that would + /// be a second decrypted copy of a secret for no gain. + /// + internal ObservableCollection Hosts { get; } = []; + + [ObservableProperty] + private HostRowViewModel? selectedHost; + + /// + /// The transfers screen's own box, and deliberately not the one on the hosts screen. This connection is a + /// separate authentication, so a password typed to open a terminal has not been offered here — and a + /// screen that quietly reused it would make a one-time password appear to work twice. + /// + [ObservableProperty] + private string typedPassword = string.Empty; + + [ObservableProperty] + private string status = "Choose a host and connect to browse its files."; + + [ObservableProperty] + private bool isBusy; + + [ObservableProperty] + private bool isConnected; + + /// The account and endpoint actually dialled, once connected. + [ObservableProperty] + private string? connectedTo; + + [ObservableProperty] + private HostKeyPresentation? pendingHostKey; + + [ObservableProperty] + private string? hostKeyMismatch; + + internal bool HasPendingHostKey => PendingHostKey is not null; + + internal bool HasHostKeyMismatch => HostKeyMismatch is not null; + + /// Whether the chosen host will want something typed into the password box. + internal bool SelectedHostAsksForAPassword => + SelectedHost is null or { Host: { SshKeyId: null, CredentialId: null } }; + + // ---- The remote pane ---- + + [ObservableProperty] + private string remotePath = string.Empty; + + internal ObservableCollection RemoteEntries { get; } = []; + + internal ObservableCollection RemoteTrail { get; } = []; + + [ObservableProperty] + private RemoteEntryRowViewModel? selectedRemoteEntry; + + /// What a new directory would be called, when the user is making one. + [ObservableProperty] + private string newRemoteFolder = string.Empty; + + internal bool HasRemoteEntries => RemoteEntries.Count > 0; + + // ---- The local pane ---- + + [ObservableProperty] + private string localPath = LocalDirectory.Home; + + internal ObservableCollection LocalEntries { get; } = []; + + internal ObservableCollection LocalTrail { get; } = []; + + /// + /// The drives this machine has, as somewhere the local pane can jump to. + /// + /// + /// The remote pane's breadcrumb reaches everywhere, because a POSIX filesystem has one root. This one + /// does not: above C:\ is a list of drives rather than a directory, so without this the pane + /// could be walked to the top of the drive it opened on and no further — and a file on D: would + /// be unreachable from an application whose whole purpose on this screen is to move one. + /// + internal ObservableCollection LocalRoots { get; } = []; + + [ObservableProperty] + private LocalEntryRowViewModel? selectedLocalEntry; + + internal bool HasLocalEntries => LocalEntries.Count > 0; + + // ---- The queue ---- + + internal ObservableCollection Transfers { get; } = []; + + internal bool HasTransfers => Transfers.Count > 0; + + /// Whether a download of the chosen remote file would have somewhere to go. + internal bool CanDownload => IsConnected && SelectedRemoteEntry is { IsFile: true }; + + /// Whether an upload of the chosen local file would have somewhere to go. + internal bool CanUpload => IsConnected && SelectedLocalEntry is { IsFile: true }; + + /// Takes an unlocked vault, so the host list has something in it. + internal void Attach(VaultViewModel openVault, VaultKnownHostStore hostKeys) + { + vault = openVault; + knownHosts = hostKeys; + + RefreshHosts(); + + // Read once per unlock rather than per navigation: a drive appearing while the application is open + // is possible and rare, and probing every removable drive on every click into a folder is not. + LocalRoots.Clear(); + + foreach (var root in LocalDirectory.Roots()) + { + LocalRoots.Add(new CrumbViewModel(root.TrimEnd(Path.DirectorySeparatorChar), root)); + } + + RefreshLocalCommand.Execute(null); + } + + /// + /// Gives up the vault, keeping the connection and anything in flight. + /// + /// + /// The host list goes because those rows carry decrypted secrets and the vault they came from is being + /// disposed. The session and the queue stay, which is the whole point: see the remark on this type. + /// + internal void Detach() + { + vault = null; + knownHosts = null; + + Hosts.Clear(); + SelectedHost = null; + TypedPassword = string.Empty; + } + + /// Opens a file-transfer session on the chosen host. + [RelayCommand] + private async Task ConnectAsync(CancellationToken cancellationToken) + { + if (vault is not { } open || SelectedHost is not { } row) + { + Status = "Choose a host first."; + return; + } + + if (!open.TryBuildConnectionRequest(row.Host, TypedPassword, out var request, out var refusal)) + { + Status = refusal; + return; + } + + PendingHostKey = null; + HostKeyMismatch = null; + + await RunAsync( + $"Connecting to {row.Label}…", + async () => + { + await CloseSessionAsync().ConfigureAwait(true); + + try + { + session = await sftp.OpenSftpAsync(request, cancellationToken).ConfigureAwait(true); + } + catch (SshHostKeyUnknownException exception) + { + // First contact, decided here rather than inherited from a terminal. File transfer is + // its own connection, so it makes its own trust decision — and the pin it writes is the + // same pin a shell would then find. + PendingHostKey = exception.Presentation; + Status = "This host has not been seen before."; + return; + } + catch (SshHostKeyMismatchException exception) + { + HostKeyMismatch = exception.Message; + Status = "The host key has changed. Nothing was connected."; + return; + } + + TypedPassword = string.Empty; + IsConnected = true; + ConnectedTo = string.Create( + CultureInfo.InvariantCulture, + $"{request.Username}@{request.Host}:{request.Port}"); + + await ListRemoteAsync(session.HomeDirectory, cancellationToken).ConfigureAwait(true); + + Status = $"Connected to {row.Label}."; + }).ConfigureAwait(true); + } + + /// + /// Closes the file-transfer session. + /// + /// + /// Refuses while the queue has work, rather than cancelling it. Disconnecting is a tidy-up and stopping + /// a transfer is a decision about somebody's file; a button that did both would make the second one by + /// accident. + /// + [RelayCommand] + private async Task DisconnectAsync() + { + if (queue.IsBusy) + { + Status = "There are transfers still running. Stop them first, or let them finish."; + return; + } + + await CloseSessionAsync().ConfigureAwait(true); + + Status = "Disconnected. Anything already transferred is where it landed."; + } + + /// Pins the offered host key and connects. + [RelayCommand] + private async Task TrustHostKeyAsync(CancellationToken cancellationToken) + { + if (PendingHostKey is not { } presentation || knownHosts is null) + { + return; + } + + try + { + await knownHosts.TrustAsync(presentation, cancellationToken).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + Status = $"The host key could not be stored, so nothing was connected: {exception.Message}"; + return; + } + + PendingHostKey = null; + + await ConnectAsync(cancellationToken).ConfigureAwait(true); + } + + /// + /// Dismisses whichever host key card is showing, without pinning or forgetting anything. + /// + /// + /// One command for both cards, because both are dismissals and the two states are mutually exclusive. + /// The mismatch card has nothing else it may offer: withdrawing a pin is a deliberate act performed in + /// the host's editor, away from the moment of connecting, and a button here would be "continue anyway" + /// with two clicks instead of one. + /// + [RelayCommand] + private void RejectHostKey() + { + var wasOffered = PendingHostKey is not null; + + PendingHostKey = null; + HostKeyMismatch = null; + + Status = wasOffered + ? "The host key was not trusted, so nothing was connected." + : "Nothing was connected."; + } + + // ---- Navigation ---- + + /// Goes to a remote directory. + [RelayCommand] + private async Task GoRemoteAsync(string path, CancellationToken cancellationToken) + { + await NavigateRemoteAsync(path, cancellationToken).ConfigureAwait(true); + } + + /// Goes up one remote directory. + [RelayCommand] + private async Task RemoteUpAsync(CancellationToken cancellationToken) + { + if (RemotePath.Length > 0) + { + await NavigateRemoteAsync(SftpPath.Parent(RemotePath), cancellationToken).ConfigureAwait(true); + } + } + + /// Re-reads the remote directory. + [RelayCommand] + private async Task RefreshRemoteAsync(CancellationToken cancellationToken) + { + if (RemotePath.Length > 0) + { + await NavigateRemoteAsync(RemotePath, cancellationToken).ConfigureAwait(true); + } + } + + /// + /// Opens whatever is selected in the remote pane, if it is somewhere to go. + /// + /// + /// A symbolic link is tried as a directory: a listing carries lstat attributes, so a link to a + /// directory reports as a link and resolving every one of them would be a round trip per row. The + /// failure, when it is a link to a file, is the server's own and says so. + /// + [RelayCommand] + private async Task OpenRemoteAsync(CancellationToken cancellationToken) + { + if (SelectedRemoteEntry is { IsNavigable: true } row) + { + await NavigateRemoteAsync(row.FullPath, cancellationToken).ConfigureAwait(true); + } + } + + /// Goes to a local directory. + [RelayCommand] + private void GoLocal(string path) => NavigateLocal(path); + + /// Goes up one local directory, as far as the top of the drive. + /// + /// Above a drive root there is no directory to list, so this stops there and says so. Getting to another + /// drive is , which is a list of places rather than a step upwards. + /// + [RelayCommand] + private void LocalUp() + { + if (LocalDirectory.Parent(LocalPath) is { } parent) + { + NavigateLocal(parent); + return; + } + + Status = "That is the top of this drive. Use the drive list to go to another one."; + } + + /// Re-reads the local directory. + [RelayCommand] + private void RefreshLocal() => NavigateLocal(LocalPath); + + /// Opens whatever is selected in the local pane, if it is a directory. + [RelayCommand] + private void OpenLocal() + { + if (SelectedLocalEntry is { IsNavigable: true } row) + { + NavigateLocal(row.FullPath); + } + } + + // ---- Moving files ---- + + /// Queues the chosen remote file for download into the local directory showing. + [RelayCommand] + private void Download() + { + if (SelectedRemoteEntry is not { IsFile: true } row) + { + Status = "Choose a file on the host to download."; + return; + } + + var destination = Path.Combine(LocalPath, row.Name); + + queue.Enqueue(TransferDirection.Download, destination, row.FullPath, row.Entry.Length); + + Status = $"Queued {row.Name} for download into {LocalPath}."; + } + + /// Queues the chosen local file for upload into the remote directory showing. + [RelayCommand] + private void Upload() + { + if (SelectedLocalEntry is not { IsFile: true } row) + { + Status = "Choose a file on this machine to upload."; + return; + } + + var destination = SftpPath.Combine(RemotePath, row.Name); + + queue.Enqueue(TransferDirection.Upload, row.FullPath, destination, row.Entry.Length); + + Status = $"Queued {row.Name} for upload into {RemotePath}."; + } + + /// Stops one transfer. + [RelayCommand] + private void CancelTransfer(TransferRowViewModel row) => queue.Cancel(row.Id); + + /// Runs a stopped transfer again, resuming where there is something to resume from. + [RelayCommand] + private void RetryTransfer(TransferRowViewModel row) => queue.Retry(row.Id); + + /// Removes one stopped transfer and whatever it left behind. + [RelayCommand] + private async Task DiscardTransferAsync(TransferRowViewModel row, CancellationToken cancellationToken) + { + // Only when the queue agreed. It refuses to discard a transfer that has not finished, and a row + // removed anyway would take the only view of a transfer that was still running. + if (await queue.DiscardAsync(row.Id, cancellationToken).ConfigureAwait(true)) + { + Transfers.Remove(row); + } + } + + /// Clears the finished transfers, which have nothing left on disk. + [RelayCommand] + private void ClearCompleted() + { + queue.ClearCompleted(); + + foreach (var row in Transfers.Where(row => row.Transfer.State is TransferState.Completed).ToArray()) + { + Transfers.Remove(row); + } + + } + + // ---- Changing the remote directory ---- + + /// Creates a directory on the host. + [RelayCommand] + private async Task CreateRemoteFolderAsync(CancellationToken cancellationToken) + { + var name = NewRemoteFolder.Trim(); + + if (name.Length == 0) + { + Status = "Type a name for the new directory."; + return; + } + + if (name.Contains('/', StringComparison.Ordinal)) + { + // One directory, whose parent must exist. A name with a separator in it would be a request to + // create a path, and this button creates a directory in the one on screen. + Status = "A directory name cannot contain '/'. Make one level at a time."; + return; + } + + await RunAsync( + $"Creating {name}…", + async () => + { + await RequireSession() + .CreateDirectoryAsync(SftpPath.Combine(RemotePath, name), cancellationToken) + .ConfigureAwait(true); + + NewRemoteFolder = string.Empty; + + await ListRemoteAsync(RemotePath, cancellationToken).ConfigureAwait(true); + + Status = $"Created {name}."; + }).ConfigureAwait(true); + } + + /// + /// Deletes the chosen remote file, or an empty directory. + /// + /// + /// Not recursive, and the refusal comes from the server rather than from a check here — see + /// ISftpSession.DeleteAsync. It is offered because the queue refuses to overwrite: without a way + /// to remove what is in the way, "that file is already there" would be a dead end. + /// + [RelayCommand] + private async Task DeleteRemoteAsync(CancellationToken cancellationToken) + { + if (SelectedRemoteEntry is not { } row) + { + Status = "Choose something on the host to delete."; + return; + } + + await RunAsync( + $"Deleting {row.Name}…", + async () => + { + await RequireSession().DeleteAsync(row.FullPath, cancellationToken).ConfigureAwait(true); + + await ListRemoteAsync(RemotePath, cancellationToken).ConfigureAwait(true); + + Status = $"Deleted {row.Name}."; + }).ConfigureAwait(true); + } + + /// + public async ValueTask DisposeAsync() + { + if (disposed) + { + return; + } + + disposed = true; + + queue.Changed -= OnTransferChanged; + + // The queue first: it holds the session, and a transfer still writing into a part file has to finish + // unwinding before the transport under it goes. + await queue.DisposeAsync().ConfigureAwait(false); + + if (session is not null) + { + await session.DisposeAsync().ConfigureAwait(false); + session = null; + } + } + + /// + /// Goes to a remote directory, on its own. + /// + /// + /// Split from so that the commands which navigate as part of + /// something else — connecting, making a directory, deleting one — can list without going back through + /// the busy guard. returns immediately when a command is already running, so a + /// nested call did nothing at all: the pane simply stayed empty after connecting, with no failure + /// anywhere to explain it. + /// + private Task NavigateRemoteAsync(string path, CancellationToken cancellationToken) + { + if (session is null) + { + Status = "Connect to a host first."; + return Task.CompletedTask; + } + + return RunAsync($"Reading {path}…", () => ListRemoteAsync(path, cancellationToken)); + } + + private async Task ListRemoteAsync(string path, CancellationToken cancellationToken) + { + var entries = await RequireSession().ListAsync(path, cancellationToken).ConfigureAwait(true); + + RemotePath = path; + SelectedRemoteEntry = null; + + RemoteEntries.Clear(); + + foreach (var entry in entries) + { + RemoteEntries.Add(new RemoteEntryRowViewModel(entry)); + } + + RemoteTrail.Clear(); + + foreach (var (name, crumb) in SftpPath.Trail(path)) + { + RemoteTrail.Add(new CrumbViewModel(name, crumb)); + } + + + Status = string.Empty; + } + + /// + /// Synchronous, unlike its remote counterpart. A local directory listing is a filesystem call rather than + /// a network round trip, and wrapping it in a task would put a state machine and a thread hop behind + /// something that returns before the click has finished being handled. + /// + private void NavigateLocal(string path) + { + try + { + var entries = LocalDirectory.List(path); + + LocalPath = Path.GetFullPath(path); + SelectedLocalEntry = null; + + LocalEntries.Clear(); + + foreach (var entry in entries) + { + LocalEntries.Add(new LocalEntryRowViewModel(entry)); + } + + RebuildLocalTrail(); + + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // The pane stays where it was. A listing that failed leaves nothing to show, and emptying the + // pane would look like a directory that had become empty. + Status = $"{path} could not be read: {exception.Message}"; + } + } + + /// + /// Built by walking up rather than by splitting on the separator, because a Windows path's first segment + /// is C:\ — a root with a separator inside it, which splitting turns into a crumb called + /// C: that navigates to the process's current directory on that drive rather than to its root. + /// + private void RebuildLocalTrail() + { + var crumbs = new List(); + + for (var walk = LocalPath; walk is not null; walk = LocalDirectory.Parent(walk)) + { + crumbs.Insert(0, new CrumbViewModel(Path.GetFileName(walk) is { Length: > 0 } name ? name : walk, walk)); + } + + LocalTrail.Clear(); + + foreach (var crumb in crumbs) + { + LocalTrail.Add(crumb); + } + } + + private void RefreshHosts() + { + Hosts.Clear(); + + if (vault is not { } open) + { + return; + } + + foreach (var host in open.Hosts) + { + Hosts.Add(host); + } + + SelectedHost ??= Hosts.FirstOrDefault(); + } + + /// The session, or a failure a queue row can carry. + private ISftpSession RequireSession() => + session ?? throw new InvalidOperationException( + "This screen is not connected to a host, so there is nowhere to move the file."); + + private async Task CloseSessionAsync() + { + if (session is { } open) + { + session = null; + await open.DisposeAsync().ConfigureAwait(true); + } + + IsConnected = false; + ConnectedTo = null; + RemotePath = string.Empty; + RemoteEntries.Clear(); + RemoteTrail.Clear(); + SelectedRemoteEntry = null; + + } + + /// + /// The queue raises this from whichever thread its pump is on, so everything here is marshalled. The row + /// is created on first sight rather than at enqueue time, which keeps one path for "a transfer changed" + /// instead of one for the first change and one for the rest. + /// + private void OnTransferChanged(object? sender, TransferChangedEventArgs e) => + Dispatcher.UIThread.Post(() => + { + if (Transfers.FirstOrDefault(row => row.Id == e.Transfer.Id) is { } existing) + { + existing.Transfer = e.Transfer; + return; + } + + Transfers.Add(new TransferRowViewModel(e.Transfer)); + }); + + /// + /// The same funnel VaultViewModel uses, and here for the same reason: every command on this screen + /// can fail with a path the server refused, and one that forgot to clear the busy flag would leave the + /// pane permanently disabled. + /// + private async Task RunAsync(string busyMessage, Func work) + { + if (IsBusy) + { + return; + } + + IsBusy = true; + Status = busyMessage; + + try + { + await work().ConfigureAwait(true); + } + catch (OperationCanceledException) + { + Status = "Cancelled."; + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + Status = exception.Message; + } + finally + { + IsBusy = false; + } + } + + partial void OnSelectedHostChanged(HostRowViewModel? value) => + OnPropertyChanged(nameof(SelectedHostAsksForAPassword)); + + partial void OnIsConnectedChanged(bool value) + { + OnPropertyChanged(nameof(CanDownload)); + OnPropertyChanged(nameof(CanUpload)); + } + + partial void OnSelectedRemoteEntryChanged(RemoteEntryRowViewModel? value) => + OnPropertyChanged(nameof(CanDownload)); + + partial void OnSelectedLocalEntryChanged(LocalEntryRowViewModel? value) => + OnPropertyChanged(nameof(CanUpload)); + + partial void OnPendingHostKeyChanged(HostKeyPresentation? value) => + OnPropertyChanged(nameof(HasPendingHostKey)); + + partial void OnHostKeyMismatchChanged(string? value) => + OnPropertyChanged(nameof(HasHostKeyMismatch)); +} diff --git a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs index 978ae24..7d52025 100644 --- a/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs @@ -1817,7 +1817,7 @@ internal sealed partial class VaultViewModel( // Refused rather than quietly falling back to the password box. A host set up for key-only access // that silently starts offering a password is the failure worth ruling out — the user asked for one // thing and got another, and the host is the last place that would say so. - if (!TryBuildAuthentication(row.Host, out var authentication, out var refusal)) + if (!TryBuildAuthentication(row.Host, ConnectPassword, out var authentication, out var refusal)) { Status = refusal; return; @@ -2087,8 +2087,38 @@ internal sealed partial class VaultViewModel( /// answered by the more specific of the two rather than by whichever the code happened to check. /// /// + /// + /// Works out how to reach a host, or says why it cannot. + /// + /// + /// The same resolution the Connect button performs, exposed because file transfer opens its own + /// connection — see ISftpSession — and a second copy of "which key, which password, whose + /// username" would be a second place for a dangling binding to be silently turned back into a typed + /// password. The typed password is a parameter rather than because the + /// transfers screen has its own box: they are different screens, and a password typed on one is not a + /// password offered on the other. + /// + internal bool TryBuildConnectionRequest( + HostSecret host, + string typedPassword, + [NotNullWhen(true)] out SshConnectionRequest? request, + [NotNullWhen(false)] out string? reason) + { + if (!TryBuildAuthentication(host, typedPassword, out var authentication, out reason)) + { + request = null; + return false; + } + + request = new SshConnectionRequest( + host.Hostname, host.Port, authentication.Username, authentication.Credential); + + return true; + } + private bool TryBuildAuthentication( HostSecret host, + string typedPassword, [NotNullWhen(true)] out HostAuthentication? authentication, [NotNullWhen(false)] out string? reason) { @@ -2133,7 +2163,7 @@ internal sealed partial class VaultViewModel( } return Complete( - host.Username, new SshPasswordCredential(ConnectPassword), out authentication, out reason); + host.Username, new SshPasswordCredential(typedPassword), out authentication, out reason); } /// diff --git a/src/DodoSSH.Client.App/Views/MainWindow.axaml b/src/DodoSSH.Client.App/Views/MainWindow.axaml index 413c1ce..241a3b7 100644 --- a/src/DodoSSH.Client.App/Views/MainWindow.axaml +++ b/src/DodoSSH.Client.App/Views/MainWindow.axaml @@ -184,20 +184,15 @@ - - - - An SFTP subsystem channel on ISshConnection, which today offers OpenShellAsync and nothing more (DodoSSH.Client.Ssh). - Remote directory listing — names, sizes, modification times and permission bits (DodoSSH.Client.Ssh). - A transfer queue with progress, throughput and resume, and somewhere for it to live across a lock (DodoSSH.Client.Ssh, DodoSSH.Client.Session). - Routing a transfer through a bastion, which needs jump-host support the connection layer does not have — the host model already records the chain. - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +