Compare commits

...
Author SHA1 Message Date
laurensandClaude Opus 5.5 e0509c6679 Updates: offer an RPM only to a copy rpm owns, and never read an unknown version as older
On a distribution without rpm, a locally built package that mirrors the spec puts the nightly tree
at /opt/DodoSSH.Nightly, and that path was the whole test for "the RPM installed this". The channel
then asked rpm for the installed version, got nothing because there is no rpm, read the silence as
"0", and offered every nightly as newer, fetching a 56 MB RPM into the profile directory that the
machine could never install, every six hours.

UpdateChannels.LinuxChannel still checks the path first, then asks rpm through the new
LinuxUpdateChannel.ForInstall. A copy rpm has no record of gets an UnavailableUpdateChannel that
says it was installed outside the dodossh-nightly (or dodossh) RPM and should be updated with the
package manager that installed it. UnavailableUpdateChannel gains an optional Reason for that,
UpdateViewModel exposes it as UnsupportedReason, and the desktop preferences page shows it in place
of the sentence about checkouts and unzipped copies. That costs one rpm launch at start, and only
where the path already matched.

CheckAsync no longer falls back to "0". If rpm stops answering after composition, the check throws
instead, so CHECK NOW reports it and the timer stays quiet, and nothing is offered or fetched. The
comment that called the fallback a deliberate choice is replaced with the reason it was wrong.

LinuxUpdateChannelTests: an unknown installed version throws and downloads nothing (replacing the
test that asserted the old behaviour), a copy rpm does not own is unavailable and says why, and a
copy rpm owns gets the channel. The newer, equal and older comparisons were already covered.
UpdateFlowTests covers the reason reaching the view model. Manual check 19.13.

App tests 744/752 with 8 skips, layout tests 202/203 with 1 skip; the full solution passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-20 16:39:17 +02:00
job f7354aa7e8 Merge pull request 'SFTP: stop waiting for ever on a host whose login prints text, and draw the file panes' icons whole' (#25) from claude/sftp-connection-icons-5ad752 into main
ci / build and test (push) Successful in 3m27s
ci / api image (push) Successful in 7s
ci / deploy ssh.dodotech.cloud (push) Successful in 13s
ci / android head (push) Successful in 3m54s
ci / desktop nightly (push) Successful in 2m18s
Reviewed-on: #25
2026-10-19 09:15:24 +00:00
jaap-jan d71f057bde SFTP: stop waiting for ever on a host whose login prints text, and draw the file panes' icons whole
ci / build and test (pull_request) Successful in 2m40s
ci / desktop nightly (pull_request) Skipped
ci / api image (pull_request) Successful in 7s
ci / deploy ssh.dodotech.cloud (pull_request) Skipped
ci / android head (pull_request) Successful in 3m43s
SSH.NET 2026.0.0 runs the SFTP handshake synchronously inside ConnectAsync, with an infinite
operation timeout and without reading the cancellation token. A host whose login script prints
anything (an echo in .bashrc) puts text where the first SFTP packet belongs; SSH.NET reads its first
four bytes as a length of about a gigabyte and waits for the rest, so the file browser said
"Connecting…" for ever. The SFTP client now uses the connect timeout as its operation timeout for
the handshake and resets it to infinite once connected, so transfers on slow links are unchanged. A
timeout after authentication becomes a TimeoutException naming the likely cause.

The SSH test server gains a second port whose sessions print a banner before sftp-server, and
SftpSessionTests proves the open fails within a 3 s connect timeout with that message. Without the
fix the test runs into its 30 s guard.

Button.paneicon is a fixed 26x26 but kept Fluent's 8,5,8,6 padding, which left a 10px-wide content
box, so every 15-16px Material icon on the SFTP panes was drawn with its sides cut off. Its padding
is now 0. Rename on the host pane used U+E9AD, an italic T (text formatting); it now uses U+E9A2,
the rename glyph the phone's files screen already uses. The host pane's "Up one directory" drew the
design's arrow_downward; both panes now use arrow_upward.

SSH tests 123/123, layout tests 203/203, app tests 744/749 with the 5 Linux TPM skips.
2026-10-16 15:09:46 +02:00
jaap-jan 36b6ded63e Merge pull request 'Connections: drop the manual box's view-model state now that both heads ask through the prompt' (#24) from claude/charming-cerf-ae57b6 into main
ci / build and test (push) Successful in 3m46s
ci / api image (push) Successful in 10s
ci / deploy ssh.dodotech.cloud (push) Successful in 12s
ci / android head (push) Successful in 4m28s
ci / desktop nightly (push) Successful in 3m27s
Reviewed-on: #24
2026-10-16 12:32:14 +00:00
13 changed files with 339 additions and 40 deletions

No files matched your search

+13
View File
@@ -3538,3 +3538,16 @@ Register a device, sync something, then `sudo dnf remove dodossh`.
make an uninstall delete a user's un-synced work — the Linux spelling of the mistake ADR 0013 decision 2
exists to prevent. Files left behind under `/opt/DodoSSH` are files the package did not own, which means
something wrote into the install directory at runtime and would survive a reinstall.
### 19.13 A copy at the same path that rpm does not own is offered nothing
On a distribution without rpm — Arch, say, with a locally built package that puts the nightly tree at
`/opt/DodoSSH.Nightly` — open PREFERENCES → UPDATES.
**Pass:** **CHECK NOW** is disabled and the row says this copy was installed outside the `dodossh-nightly`
RPM and should be updated with the package manager that installed it. `~/.local/share/dodossh.nightly/updates/`
stays empty however long it runs.
**Failure means:** a banner offering a nightly is the path being taken as proof the RPM is installed, or an
rpm that cannot answer being read as an old version — the fault this check was written for, which offered
every nightly as newer and fetched a 56 MB package the machine could never install, every six hours.
+4
View File
@@ -1223,10 +1223,14 @@
The two buttons in the drawer's header: the overflow menu and the arrow that puts the drawer away.
Chrome rather than controls — they are the frame around whichever panel is showing, which is why they
are flat and quiet until the pointer finds them.
No padding, because the box is fixed: Fluent's own 8,5,8,6 left a glyph ten pixels of width inside
twenty-six, and every 15px Material icon on the SFTP panes was drawn with its sides cut off.
-->
<Style Selector="Button.paneicon">
<Setter Property="Width" Value="26" />
<Setter Property="Height" Value="26" />
<Setter Property="Padding" Value="0" />
<Setter Property="CornerRadius" Value="6" />
<Setter Property="FontSize" Value="13" />
<Setter Property="Foreground" Value="{StaticResource TextFaint}" />
@@ -127,17 +127,61 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
this.installedVersion = installedVersion;
this.handToInstaller = handToInstaller;
packageName = string.Equals(channel, "nightly", StringComparison.Ordinal)
? "dodossh-nightly"
: "dodossh";
packageName = PackageFor(channel);
}
/// <summary>The channel for a copy under the RPM's install root, if rpm says the package is there.</summary>
internal static IUpdateChannel ForInstall(string channel) =>
ForInstall(channel, InstalledRpmVersion, () => new LinuxUpdateChannel(channel));
/// <summary>
/// The channel for a copy under the RPM's install root, with rpm supplied, which is how the tests
/// reach it.
/// </summary>
/// <remarks>
/// <para>
/// <b>The path says where this copy is and only rpm says who put it there.</b>
/// <c>UpdateChannels.LinuxChannel</c> has already established the first, and on an RPM distribution
/// that is the whole story. It is not on any other: a locally built pacman or dpkg package that mirrors
/// the spec puts the same tree at the same path, and this channel then offered every nightly as newer
/// than a version nobody could read, fetched a package that machine has no way to install, and did it
/// again every six hours.
/// </para>
/// <para>
/// Asked once, at composition, for the reason the rest of this decision is: who installed this copy
/// does not change while it runs. It costs one <c>rpm</c> launch at start on a machine whose path
/// already matched, and nothing at all anywhere else.
/// </para>
/// </remarks>
/// <param name="channel">The build's channel, which names the package.</param>
/// <param name="installedVersion">What rpm says is installed under a package name, or null.</param>
/// <param name="whenOwned">The channel to use when rpm does own this copy.</param>
internal static IUpdateChannel ForInstall(
string channel,
Func<string, string?> installedVersion,
Func<IUpdateChannel> whenOwned)
{
ArgumentException.ThrowIfNullOrEmpty(channel);
ArgumentNullException.ThrowIfNull(installedVersion);
ArgumentNullException.ThrowIfNull(whenOwned);
var package = PackageFor(channel);
return installedVersion(package) is null
? new UnavailableUpdateChannel(
$"This copy of DodoSSH was installed outside the {package} RPM — rpm has no record of that "
+ "package here — and an RPM is the only kind of update it can fetch. Update it with the "
+ "package manager that installed it.")
: whenOwned();
}
/// <inheritdoc />
/// <remarks>
/// True unconditionally, because whether this copy is one that can be updated was already decided by
/// <see cref="UpdateChannels.ForThisMachine"/> — a build run from a checkout never reaches this class
/// at all. The same split Velopack's side uses, where <c>IsInstalled</c> is checked once at
/// composition rather than at every call site.
/// <see cref="UpdateChannels.ForThisMachine"/> and <see cref="ForInstall(string)"/> — a build run from
/// a checkout, or installed by something other than rpm, never reaches this class at all. The same
/// split Velopack's side uses, where <c>IsInstalled</c> is checked once at composition rather than at
/// every call site.
/// </remarks>
public bool IsSupported => true;
@@ -184,9 +228,20 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
$"The {channel} release's {manifestName} could not be read as a manifest.");
}
// ◆ No answer from rpm is a failure, never a version. ForInstall only built this class because
// rpm answered, so silence now is a package removed under a running process or an rpm that
// stopped working — and any stand-in number would make every published build look newer and
// fetch a package for a machine that cannot install it, which is what this once did.
if (installedVersion(packageName) is not { } installed)
{
throw new InvalidOperationException(
$"rpm has no record of {packageName}, so there is no installed version to compare the "
+ $"{channel} release against.");
}
// The one place null is returned, and it means what null is documented to mean: this build is
// current. Compared the way rpm compares, because rpm is what accepts or refuses the install.
if (RpmVersion.Compare(manifest.RpmVersion, installedVersion(packageName) ?? "0") <= 0)
if (RpmVersion.Compare(manifest.RpmVersion, installed) <= 0)
{
return null;
}
@@ -324,6 +379,12 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
? $"{RepositoryApi}/releases/tags/nightly-desktop"
: $"{RepositoryApi}/releases/latest";
/// <summary>The package a channel's builds are published as.</summary>
private static string PackageFor(string channel) =>
string.Equals(channel, "nightly", StringComparison.Ordinal)
? "dodossh-nightly"
: "dodossh";
private static string? Asset(LinuxForgeRelease? release, string name) =>
release?.Assets?.FirstOrDefault(
asset => string.Equals(asset.Name, name, StringComparison.Ordinal))?.DownloadUrl;
@@ -365,9 +426,11 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
/// <c>scripts/ci-linux-rpm.sh</c> performs, and two copies of one rule is a pair that can disagree.
/// </para>
/// <para>
/// A machine that cannot answer gets null, which the caller reads as <c>0</c> and makes every
/// published build look newer. That is the wrong way to fail, chosen deliberately: the failure is then
/// an install PackageKit declines rather than a security fix that never arrives.
/// A machine that cannot answer gets null — no <c>rpm</c> at all, or one with no such package — and
/// null means "rpm does not own this copy", never a version. It used to be read as <c>0</c>, on the
/// argument that the worst case was an install PackageKit declined. That holds only where PackageKit
/// installs RPMs: on any other distribution it made every published build look newer, forever, and
/// fetched a package that machine could not install. See <see cref="ForInstall(string)"/>.
/// </para>
/// </remarks>
private static string? InstalledRpmVersion(string packageName)
@@ -66,21 +66,23 @@ internal static class UpdateChannels
}
}
/// <summary>The Linux channel, if this copy is one a package manager put here.</summary>
/// <summary>The Linux channel, if this copy is one the RPM put here.</summary>
/// <remarks>
/// <para>
/// <b>The question Velopack answers with <c>IsInstalled</c>, answered by looking at where this process
/// lives.</b> The RPM owns <c>/opt/DodoSSH</c> or <c>/opt/DodoSSH.Nightly</c> — the spec puts it there
/// and nothing else writes to <c>/opt</c> under those names — so a build running from anywhere else is
/// a checkout somebody started, an extracted archive, or a tree copied by hand. None of those can be
/// updated by installing a package over them, and offering it would be an offer that cannot be carried
/// out.
/// lives and then asking rpm.</b> The RPM owns <c>/opt/DodoSSH</c> or <c>/opt/DodoSSH.Nightly</c> — the
/// spec puts it there — so a build running from anywhere else is a checkout somebody started, an
/// extracted archive, or a tree copied by hand. None of those can be updated by installing a package
/// over them, and offering it would be an offer that cannot be carried out.
/// </para>
/// <para>
/// Checked by path rather than by asking rpm which package owns the executable, which would be the
/// more thorough answer and costs a process launch on every start for a distinction nothing acts on
/// differently. The install root is a constant in two places — here and the packaging script — and
/// they are named in each other's comments.
/// <b>The path is necessary and not sufficient.</b> It used to be the whole check, on the argument
/// that asking rpm cost a process launch for a distinction nothing acted on differently. Something
/// does: a locally built pacman package that mirrors the spec puts the same tree at the same path,
/// and that copy was offered every nightly as an RPM it could never install. So a matching path is
/// then put to rpm — see <see cref="LinuxUpdateChannel.ForInstall(string)"/> — and the launch is paid
/// only where the path already matched. The install root is a constant in two places — here and the
/// packaging script — and they are named in each other's comments.
/// </para>
/// </remarks>
[SupportedOSPlatform("linux")]
@@ -91,7 +93,7 @@ internal static class UpdateChannels
var here = AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar);
return string.Equals(here, root, StringComparison.Ordinal)
? new LinuxUpdateChannel(DesktopChannel.Name)
? LinuxUpdateChannel.ForInstall(DesktopChannel.Name)
: new UnavailableUpdateChannel();
}
}
@@ -84,7 +84,10 @@
IsVisible="{Binding Updates.Status, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" />
<TextBlock Classes="settingsrowcaption"
Text="This copy of DodoSSH cannot replace itself, so nothing above does anything. That is what a build run from a source checkout looks like, and also what a copy somebody unzipped by hand looks like — it is the installer that registers the update path."
IsVisible="{Binding Updates.IsUnsupported}" />
IsVisible="{Binding Updates.IsUnsupportedForTheUsualReason}" />
<TextBlock Classes="settingsrowcaption"
Text="{Binding Updates.UnsupportedReason}"
IsVisible="{Binding Updates.UnsupportedReason, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" />
</StackPanel>
</Border>
@@ -209,10 +209,9 @@
design's, unlike HOST/BUCKET across the arrow column, which keeps its own words for a reason
recorded where it is drawn.
UP is arrow_upward here and arrow_downward on the host/bucket pane below, matching the design's own
two glyphs exactly rather than picking one "up a directory" icon for both — both buttons do the same
thing (go to the parent directory), and the pair reading differently is the design's own choice
faithfully carried over rather than a functional difference invented to justify it.
UP is arrow_upward on both panes. The design drew arrow_downward on the host/bucket pane, and that
was carried over until it was reported as a wrong icon — which it is: both buttons go to the parent
directory, and a down arrow under the tooltip "Up one directory" says the opposite of what it does.
-->
<!--
◆ THE PATH SITS IN THE STAR COLUMN, ALONE, AND THAT PLACEMENT IS LOAD-BEARING.
@@ -398,8 +397,8 @@
directory tree and a flat namespace with inferred folders — and that reason still holds, so only
the style moves to the design's 10px tracked label, not the wording.
UP is arrow_downward here, matching arrow_upward on the local pane above per the design's own two
distinct glyphs; see that pane's own remark on why the pair differs without the actions differing.
UP is arrow_upward, the same as the local pane's; see that pane's own remark for why it is no longer
the design's arrow_downward.
-->
<!--
◆ THE PATH IS TYPED HERE, and that is what the star column is now holding.
@@ -446,7 +445,7 @@
<StackPanel Grid.Column="2" Orientation="Horizontal" Spacing="4" Margin="8,0,0,0">
<Button Classes="paneicon" Command="{Binding RemoteUpCommand}"
IsEnabled="{Binding IsConnected}" ToolTip.Tip="Up one directory">
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="16" Text="&#xE5DB;" />
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="16" Text="&#xE5D8;" />
</Button>
<Button Classes="paneicon" Command="{Binding RefreshRemoteCommand}"
IsEnabled="{Binding IsConnected}" ToolTip.Tip="Refresh">
@@ -546,7 +545,7 @@
</Button>
<Button Classes="paneicon" Command="{Binding RenameRemoteCommand}"
IsEnabled="{Binding CanRenameRemote}" ToolTip.Tip="Rename on the host">
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="15" Text="&#xE9AD;" />
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="15" Text="&#xE9A2;" />
</Button>
<Button Classes="paneicon" Command="{Binding MoveRemoteCommand}"
IsEnabled="{Binding CanMoveRemote}" ToolTip.Tip="Move to another directory">
@@ -141,9 +141,32 @@ public interface IUpdateChannel
/// <see cref="UnavailableDeviceKeyStore"/>: the caller then has one shape to write against, and "no
/// updater here" is a thing the interface can express rather than a case every call site has to remember.
/// </para>
/// <para>
/// <b>One situation outside those four carries a <see cref="Reason"/>.</b> A Linux copy sitting exactly
/// where the RPM puts it that no RPM owns — built locally and installed by another distribution's package
/// manager. It looks installed, so the sentence about checkouts and unzipped copies would describe the
/// wrong machine, and the person reading it needs telling which updater is theirs.
/// </para>
/// </remarks>
public sealed class UnavailableUpdateChannel : IUpdateChannel
{
/// <summary>Creates one for the situations that need no telling apart.</summary>
public UnavailableUpdateChannel()
{
}
/// <summary>Creates one whose situation is worth a sentence of its own.</summary>
/// <param name="reason">What to tell the user, as a sentence they can act on.</param>
public UnavailableUpdateChannel(string reason)
{
ArgumentException.ThrowIfNullOrWhiteSpace(reason);
Reason = reason;
}
/// <summary>Why this copy cannot replace itself, where that is not the usual why.</summary>
public string? Reason { get; }
/// <inheritdoc />
public bool IsSupported => false;
@@ -126,6 +126,7 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
this.restart = restart;
CurrentVersion = updates.CurrentVersion;
UnsupportedReason = (updates as UnavailableUpdateChannel)?.Reason;
isAutomatic = settings.Read().AutomaticUpdateChecks;
state = updates.IsSupported ? UpdateState.Idle : UpdateState.Unsupported;
}
@@ -133,6 +134,16 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
/// <summary>What this build calls itself.</summary>
internal string CurrentVersion { get; }
/// <summary>Why this copy cannot replace itself, when the channel had something particular to say.</summary>
/// <remarks>
/// Null in the ordinary cases, where the preferences screen's own sentence about checkouts and unzipped
/// copies is the right one. See <see cref="UnavailableUpdateChannel"/> for the case where it is not.
/// </remarks>
internal string? UnsupportedReason { get; }
/// <summary>Unsupported, with nothing more particular to say than the screen's own sentence.</summary>
internal bool IsUnsupportedForTheUsualReason => IsUnsupported && UnsupportedReason is null;
[ObservableProperty]
private UpdateState state;
@@ -248,6 +259,7 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
{
OnPropertyChanged(nameof(IsSupported));
OnPropertyChanged(nameof(IsUnsupported));
OnPropertyChanged(nameof(IsUnsupportedForTheUsualReason));
OnPropertyChanged(nameof(IsChecking));
OnPropertyChanged(nameof(IsDownloading));
OnPropertyChanged(nameof(IsReady));
@@ -139,20 +139,41 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
IAsyncDisposable? lifetime,
CancellationToken cancellationToken)
{
var info = BuildConnectionInfo(request);
// Keep-alive here for the reason it is on the shell client: a transfer connection left open while
// somebody browses is idle in exactly the same way, and a file listing against a machine that has
// since rebooted would otherwise wait on a socket nothing will ever answer.
var client = new SftpClient(BuildConnectionInfo(request))
//
// ◆ The operation timeout is the connect timeout for the handshake, and nothing afterwards. SSH.NET's
// default is to wait for ever, and its ConnectAsync runs the SFTP handshake — the version exchange
// and the home directory lookup — synchronously after the login, where the cancellation token is not
// read. A host whose login prints anything into the channel (an echo in .bashrc is the classic) sends
// text SSH.NET reads as the length of a packet a gigabyte long, and the file browser then said
// "Connecting…" for ever. Reset once connected, so a slow write on a slow link is not cut off.
var client = new SftpClient(info)
{
BufferSize = SftpBufferSize,
KeepAliveInterval = KeepAliveInterval,
OperationTimeout = info.Timeout,
};
// No progress for the file-transfer path. The screen that waits on one is the file browser, which
// reports itself, and a second connection opened behind an already-open shell has nothing the user
// is watching a step list for.
var gate = await ConnectThroughHostKeyGateAsync(client, request, progress: null, cancellationToken)
.ConfigureAwait(false);
HostKeyGate gate;
try
{
gate = await ConnectThroughHostKeyGateAsync(client, request, progress: null, cancellationToken)
.ConfigureAwait(false);
}
catch (SshOperationTimeoutException exception) when (info.IsAuthenticated)
{
throw SftpHandshakeTimedOut(request, info, exception);
}
client.OperationTimeout = Timeout.InfiniteTimeSpan;
// Read once, here, rather than per call. SftpClient.WorkingDirectory canonicalises against the server
// on first read, so leaving it to the property would put a round trip behind something that reads
@@ -172,6 +193,24 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
return new SshNetSftpSession(client, gate.Presented!, home, lifetime);
}
/// <summary>Says what a timeout after the login means for a file-transfer connection.</summary>
/// <remarks>
/// Only after the login: a timeout before it is the network's or the server's, and SSH.NET's own words
/// for it are left alone. One after it is the SFTP handshake's — the host let this account in and then
/// never answered as a file server — and SSH.NET's "Operation has timed out" would send somebody looking
/// at the network for what is nearly always a line of text in a login script on the host.
/// </remarks>
private static TimeoutException SftpHandshakeTimedOut(
SshConnectionRequest request,
ConnectionInfo info,
SshOperationTimeoutException exception) =>
new(
$"{request.Host} accepted the login, but its SFTP service did not answer within "
+ $"{info.Timeout.TotalSeconds:0} seconds. This is usually a login script on the host that "
+ "prints text (an echo in .bashrc, say): SFTP needs the remote shell to stay silent when "
+ "it is not interactive.",
exception);
/// <summary>
/// Runs the handshake with host key trust attached, and translates a refusal this factory caused.
/// </summary>
@@ -86,19 +86,86 @@ public sealed class LinuxUpdateChannelTests : IDisposable
}
/// <summary>
/// A machine that cannot say what it has treats everything as newer.
/// A machine that cannot say what it has is told so, and is offered nothing.
/// </summary>
/// <remarks>
/// The wrong way to fail, chosen deliberately and for the reason the android channel gives: the cost is
/// an install the package manager declines, and the cost of the other direction is a security fix that
/// never arrives.
/// This used to read the silence as <c>0</c> and offer everything, on the argument that the cost was
/// an install the package manager declined. On a machine with no rpm at all — a pacman package of the
/// same tree at the same path — that was every nightly, every six hours, as a 56 MB download nothing
/// could install. Unknown is not older.
/// </remarks>
[Fact]
public async Task AnUnknownInstalledVersionTreatsEverythingAsNewer()
public async Task AnUnknownInstalledVersionThrowsRatherThanOfferingAnything()
{
var channel = Channel(RandomNumberGenerator.GetBytes(16), published: "0.0.1~alpha.0.1-1", installed: null);
var handed = new List<string>();
var channel = Channel(
RandomNumberGenerator.GetBytes(16),
published: "0.0.1~alpha.0.200-1",
installed: null,
handToInstaller: handed.Add);
(await channel.CheckAsync(Token)).ShouldNotBeNull();
await Should.ThrowAsync<InvalidOperationException>(async () => await channel.CheckAsync(Token));
await Should.ThrowAsync<InvalidOperationException>(
async () => await channel.DownloadAsync(
new DodoSSH.Client.Session.AvailableUpdate("0.0.1-alpha.0.200"),
new Progress<int>(),
Token));
handed.ShouldBeEmpty();
Directory.Exists(Path.Combine(directory, "updates")).ShouldBeFalse();
}
/// <summary>
/// A copy rpm does not own gets no channel at all, and a sentence saying whose it is to update.
/// </summary>
/// <remarks>
/// The case that matters: the right path and no rpm — a locally built pacman package that mirrors the
/// spec. No binary and no such package both arrive here as null.
/// </remarks>
[Fact]
public void ACopyRpmDoesNotOwn_IsUnavailableAndSaysWhy()
{
var asked = new List<string>();
var updates = LinuxUpdateChannel.ForInstall(
"nightly",
package =>
{
asked.Add(package);
return null;
},
() => throw new InvalidOperationException("Built a channel for a copy rpm does not own."));
asked.ShouldBe(["dodossh-nightly"]);
updates.IsSupported.ShouldBeFalse();
var reason = updates.ShouldBeOfType<DodoSSH.Client.Session.UnavailableUpdateChannel>().Reason;
reason.ShouldNotBeNull();
reason.ShouldContain("outside the dodossh-nightly RPM");
reason.ShouldContain("package manager that installed it");
}
[Fact]
public void ACopyRpmOwns_GetsTheChannel()
{
var asked = new List<string>();
var owned = new FakeUpdateChannel();
var updates = LinuxUpdateChannel.ForInstall(
"release",
package =>
{
asked.Add(package);
return "0.0.1-1";
},
() => owned);
asked.ShouldBe(["dodossh"]);
updates.ShouldBeSameAs(owned);
}
[Fact]
@@ -80,6 +80,32 @@ public sealed class UpdateFlowTests : IDisposable
Build().CurrentVersion.ShouldBe("0.4.2");
}
[Fact]
public void ABuildThatCannotReplaceItself_ForAReasonOfItsOwn_SaysThatInstead()
{
var updates = new UpdateViewModel(
new UnavailableUpdateChannel("Update it with the package manager that installed it."),
new ClientSettingsStore(Paths),
TimeProvider.System,
() => 0,
restart: _ => Task.CompletedTask);
updates.IsUnsupported.ShouldBeTrue();
updates.UnsupportedReason.ShouldBe("Update it with the package manager that installed it.");
updates.IsUnsupportedForTheUsualReason.ShouldBeFalse();
}
[Fact]
public void ABuildThatCannotReplaceItself_ForTheUsualReasons_LeavesTheScreenItsOwnSentence()
{
channel.IsSupported = false;
var updates = Build();
updates.UnsupportedReason.ShouldBeNull();
updates.IsUnsupportedForTheUsualReason.ShouldBeTrue();
}
[Fact]
public async Task ABackgroundPassThatFindsNothing_SaysNothing()
{
@@ -211,6 +211,31 @@ public sealed class SftpSessionTests(SshServerFixture fixture)
refusal.Presentation.Fingerprint.ShouldStartWith("SHA256:");
}
[Fact]
public async Task AHostWhoseLoginPrintsText_FailsWithinTheConnectTimeout_AndSaysWhy()
{
var knownHosts = new InMemoryKnownHostStore();
var factory = new SshNetConnectionFactory(knownHosts);
var request = Request() with { Port = fixture.ChattyPort, ConnectTimeout = TimeSpan.FromSeconds(3) };
var unknown = await Should.ThrowAsync<SshHostKeyUnknownException>(async () =>
await factory.OpenSftpAsync(request, Token));
await knownHosts.TrustAsync(unknown.Presentation, Token);
// The banner's first four bytes read as the length of a packet about a gigabyte long, and SSH.NET's
// own default is to wait for the rest of it for ever — which the file browser drew as "Connecting…"
// with no end. The outer bound is only there so that regression fails this test rather than hangs it,
// and it is a cancellation rather than a timeout so that it cannot be mistaken for the answer.
using var bound = CancellationTokenSource.CreateLinkedTokenSource(Token);
bound.CancelAfter(TimeSpan.FromSeconds(30));
var failure = await Should.ThrowAsync<TimeoutException>(async () =>
await factory.OpenSftpAsync(request, Token).WaitAsync(bound.Token));
failure.Message.ShouldContain("SFTP service did not answer within 3 seconds");
}
private SshConnectionRequest Request() => new(
fixture.Host,
fixture.Port,
@@ -42,6 +42,17 @@ public sealed class SshServerFixture : IAsyncLifetime
private const int SshPort = 2222;
/// <summary>
/// A second port on the same sshd, where every session starts with a line of text before SFTP.
/// </summary>
/// <remarks>
/// What a host with an <c>echo</c> in its login script looks like to a file-transfer client: the text
/// arrives where the server's first SFTP packet should be. A port rather than an account because this
/// image's sshd runs unprivileged and cannot log in as anybody but <see cref="Username"/>, and a port
/// rather than a change to the main one so the shell and SFTP suites keep a server that behaves.
/// </remarks>
private const int ChattySshPort = 2223;
/// <summary>
/// How many connections in a row the server has to answer before this fixture calls it ready.
/// </summary>
@@ -65,6 +76,9 @@ public sealed class SshServerFixture : IAsyncLifetime
/// <summary>Host port the container's sshd is published on.</summary>
public ushort Port => container!.GetMappedPublicPort(SshPort);
/// <summary>Host port of the same sshd whose sessions print a banner first; see <see cref="ChattySshPort"/>.</summary>
public ushort ChattyPort => container!.GetMappedPublicPort(ChattySshPort);
/// <summary>Host the container is reachable at.</summary>
public string Host => container!.Hostname;
@@ -83,6 +97,7 @@ public sealed class SshServerFixture : IAsyncLifetime
.WithEnvironment("SUDO_ACCESS", "false")
.WithEnvironment("PUBLIC_KEY", ExportOpenSshPublicKey(ClientKey))
.WithPortBinding(SshPort, assignRandomHostPort: true)
.WithPortBinding(ChattySshPort, assignRandomHostPort: true)
// The entrypoint generates host keys, rewrites sshd_config and installs the authorized
// key before sshd is usable, so a published port is not readiness. Both conditions are
@@ -176,6 +191,10 @@ public sealed class SshServerFixture : IAsyncLifetime
/// been touched — the same trap as the wrong file, one layer up. Measured from the log, after a version
/// of this fixture did exactly that and failed twenty-eight tests.
/// </para>
/// <para>
/// The <see cref="ChattySshPort"/> block goes last because a <c>Match</c> runs to the end of the file:
/// anything appended after it would apply only to that port.
/// </para>
/// </remarks>
private async Task ReconfigureAsync()
{
@@ -185,6 +204,10 @@ public sealed class SshServerFixture : IAsyncLifetime
"sed -i 's/^AllowTcpForwarding no/AllowTcpForwarding yes/' /config/sshd/sshd_config"
+ " && sed -i 's/^#*MaxStartups .*/MaxStartups 200/' /config/sshd/sshd_config"
+ " && printf '\\nPerSourcePenalties no\\n' >> /config/sshd/sshd_config"
+ $" && sed -i 's/^Port {SshPort}$/Port {SshPort}\\nPort {ChattySshPort}/' /config/sshd/sshd_config"
+ $" && printf '\\nMatch LocalPort {ChattySshPort}\\n"
+ " ForceCommand /bin/sh -c \"echo Welcome to the test server; exec /usr/lib/ssh/sftp-server\"\\n'"
+ " >> /config/sshd/sshd_config"
+ " && pkill -HUP sshd",
]);