Files
DodoSSH/tests/DodoSSH.Client.App.Tests/FakeSsh.cs
T
laurensandClaude Opus 5 fd92b80d4f
ci / build and test (push) Successful in 3m3s
ci / android head (push) Successful in 4m15s
ci / api image (push) Successful in 26s
ci / desktop nightly (push) Successful in 2m15s
Find out why a connection did not reach, instead of only saying it timed out
Measured against SSH.NET 2026.0.0 before anything was written, because the
premise needed checking. Four ways of failing to reach a host, and what this
client puts in front of the user for each:

  server accepts TCP, never sends a banner  ->  "Connection has timed out."
  packets black-holed, nothing comes back   ->  "Connection has timed out."
  nothing listening on the port             ->  "...actively refused it."
  name does not resolve                     ->  "No such host is known."

The first two are the same five words and are not the same problem. One is a
network, a firewall or a wrong address; the other is a machine that is up and
well and an sshd that is wedged, or a port that is not SSH at all. Nothing in
the exception tells them apart, and that sentence was the whole of what this
product could say about the failure people hit most.

◆ A SOCKET ANSWERS IT, IN ABOUT AS LONG AS IT TAKES TO OPEN ONE.

SshReachabilityProbe resolves the name and opens a bare TCP connection, and the
four answers it can come back with are the four somebody would do something
different about — unresolved, refused, silent, open. Every one of them is what
a socket actually did, measured at the moment it was asked, rather than
anything inferred from the exception that started it.

Deliberately nothing else. No ping: ICMP is filtered on most of the networks
this is pointed at, so an unanswered one would be evidence of nothing. No scan
of neighbouring ports — the question is what happened to the connection the
user asked for, and probing ports nobody asked about is how a client gets
flagged by an intrusion detector on the very network somebody is trying to
work on. No second SSH handshake against a server that has already failed one.

The budget is six seconds and the floor under it is not a guess: a refusal is
not immediate on Windows, and a closed loopback port took ~2s to answer when
this was measured, because the stack retries the SYN first. At two seconds the
probe calls a machine that is up and refusing a firewall. The test for that
case runs at the real budget on purpose, so lowering the number fails it.

◆ ONLY WHILE REACHING, AND THE WINDOW IS CLOSED AT BOTH ENDS.

Past SshConnectionPhase.Reaching the machine has offered a host key, so it has
demonstrably been reached and a refused password is not a reachability
question. Before it, nothing has been dialled at all — a private key that will
not parse throws out of BuildConnectionInfo before the factory reports
anything — and probing there finds a healthy port and appends "the SSH
handshake is what did not finish" to a message about an unreadable key. That
is a confident wrong answer replacing an accurate one, so both ends are shut.
PhaseReporter keeps the furthest phase to decide it.

The finding is added to what SSH.NET said rather than replacing it: the
exception is the fact, and this is a reading of a second measurement.

Every shell in ShellFlowTests now gets a scripted probe. Without one a failed
connection in any test in that file would do a live DNS lookup for db.internal
and open a real socket — slow, network-dependent, and worse than either, since
db.internal resolves on any machine whose resolver has a search domain that
makes it, which would make the asserted sentence depend on whose network the
suite ran from.

◆ WHAT THIS DOES NOT FIX, FOUND WHILE READING THE SAME PATH.

  - SshConnectionRequest.ConnectTimeout is documented, plumbed, and passed by
    nothing. Every connection this client makes is hard-wired to the 15-second
    default, and there is no per-host field and no setting that can change it.
    Fifteen seconds bounds the handshake waits as well as the connect — a
    server that accepts TCP and never sends a banner fails at exactly the
    timeout, measured — where OpenSSH's own ConnectTimeout covers only the
    connect. If timeouts are frequent on slow links, this is the first
    suspect, and it needs a product decision rather than a patch.

  - The known-host lookup in HostKeyGate.OnHostKeyReceived blocks SSH.NET's
    message thread inside that same 15-second budget. The blocking is
    acknowledged in the code; that it competes with the key exchange's own
    timeout is not. On a cold or locked vault a slow read would surface as a
    connection timeout, which is exactly the symptom above.

  - ConnectThroughHostKeyGateAsync never unsubscribes HostKeyReceived on
    success, so the gate outlives the handshake holding the connect-time
    CancellationToken. If SSH.NET raises that event again on rekey it would
    re-enter the blocking store read with a token whose source is long gone.
    Not verified either way — it needs a real sshd and a forced rekey.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-26 16:05:12 +02:00

491 lines
19 KiB
C#

using DodoSSH.Client.Ssh;
namespace DodoSSH.Client.App.Tests;
/// <summary>
/// An SSH stack that connects to nothing.
/// </summary>
/// <remarks>
/// The shell suite is about what the view models do, and the real factory would need a reachable
/// sshd — which <c>DodoSSH.Client.Ssh.Tests</c> already covers against a container. What this makes
/// testable is everything the connect path does <em>around</em> the connection.
/// </remarks>
internal sealed class FakeSshConnectionFactory : ISshConnectionFactory, ISftpSessionFactory
{
/// <summary>Thrown instead of connecting, when set. Used for the host-key paths.</summary>
internal Exception? Failure { get; set; }
/// <summary>
/// Holds a connection open until it is completed, when set.
/// </summary>
/// <remarks>
/// A handshake takes as long as a network takes, and connecting is deliberately no longer allowed to
/// hold the window still while it does — so there is now behaviour that only exists <em>during</em> a
/// connection: a tab in the strip with no session behind it. This is how a test gets to look at that
/// moment rather than at the two on either side of it.
/// </remarks>
internal TaskCompletionSource? Gate { get; set; }
/// <summary>Requests this factory was asked for, in order.</summary>
internal List<SshConnectionRequest> Requests { get; } = [];
/// <summary>Every shell this factory has opened, in order.</summary>
/// <remarks>
/// Exists so a test can reach one and drop it — see <see cref="FakeSshShellSession.Drop"/>. There was no
/// reason to hold them while the only ending a session had was the tab being closed, which the shell can
/// ask for directly.
/// </remarks>
internal List<FakeSshShellSession> Shells { get; } = [];
/// <summary>Requests for a file-transfer session, in order.</summary>
/// <remarks>
/// Kept apart from <see cref="Requests"/> deliberately: file transfer is a separate connection, and a
/// test asserting that opening a terminal did not also open one would have nothing to look at if the two
/// shared a list.
/// </remarks>
internal List<SshConnectionRequest> SftpRequests { get; } = [];
/// <summary>
/// The furthest phase a failing attempt reports before <see cref="Failure"/> is thrown, or null for one
/// that never reports any.
/// </summary>
/// <remarks>
/// <para>
/// ◆ <b>It is what decides whether the shell goes and probes the endpoint.</b> A connection that stopped
/// while reaching is worth asking a socket about; one that got a host key has plainly reached the machine,
/// and one that never reported a phase never dialled at all. See <c>VaultViewModel.ExplainAsync</c>.
/// </para>
/// <para>
/// Defaults to <see cref="SshConnectionPhase.Reaching"/>, which is what this fake reported
/// unconditionally before the field existed, so every test written against it goes on meaning what it did.
/// Null models the real factory throwing out of <c>BuildConnectionInfo</c> — a private key that will not
/// parse — which happens before it reports anything.
/// </para>
/// </remarks>
internal SshConnectionPhase? FailAfter { get; set; } = SshConnectionPhase.Reaching;
/// <summary>Every phase up to and including <paramref name="furthest"/>, in the order they happen.</summary>
private static IEnumerable<SshConnectionPhase> PhasesUpTo(SshConnectionPhase? furthest) =>
furthest is { } limit
? Enum.GetValues<SshConnectionPhase>().Where(phase => phase <= limit)
: [];
/// <inheritdoc />
public async Task<ISshConnection> ConnectAsync(
SshConnectionRequest request,
IProgress<SshConnectionPhase>? progress,
CancellationToken cancellationToken)
{
Requests.Add(request);
// Before the gate rather than after it, which is what makes this fake useful for the connecting
// card: a test that holds Gate open is a connection stuck partway through, and the step list has to
// show it stuck on a named step rather than on none.
//
// How far a failing attempt gets is FailAfter's to say; a succeeding one is always reaching by now.
foreach (var phase in Failure is null ? [SshConnectionPhase.Reaching] : PhasesUpTo(FailAfter))
{
progress?.Report(phase);
}
if (Gate is { } gate)
{
await gate.Task.WaitAsync(cancellationToken).ConfigureAwait(false);
}
if (Failure is { } failure)
{
throw failure;
}
// Only on the way to succeeding. A failure reported as having authenticated would let a test pass
// while the card showed a refused connection getting one step further than it did.
progress?.Report(SshConnectionPhase.CheckingHostKey);
progress?.Report(SshConnectionPhase.Authenticating);
return new FakeSshConnection(request, Shells);
}
/// <summary>The file-transfer session this factory last handed out, or null when it never has.</summary>
/// <remarks>
/// Kept so a test can read back what the screen actually asked the host to do — which path was renamed,
/// which was deleted, which directories were listed. The screen reports all three in its own status
/// line, and a suite that asserted only on that would be checking a sentence this code wrote about
/// itself rather than the operation it claims to have performed.
/// </remarks>
internal FakeSftpSession? OpenedSftp { get; private set; }
/// <inheritdoc />
public Task<ISftpSession> OpenSftpAsync(
SshConnectionRequest request,
CancellationToken cancellationToken)
{
SftpRequests.Add(request);
if (Failure is { } failure)
{
return Task.FromException<ISftpSession>(failure);
}
OpenedSftp = new FakeSftpSession(request);
return Task.FromResult<ISftpSession>(OpenedSftp);
}
}
/// <summary>A remote filesystem with one directory in it.</summary>
/// <remarks>
/// Enough for the shell suite, which is about what the screen does around a session rather than about
/// moving bytes. The queue's own behaviour is covered against a fuller fake in
/// <c>DodoSSH.Client.Transfer.Tests</c>, and the real subsystem against a container in
/// <c>DodoSSH.Client.Ssh.Tests</c>.
/// </remarks>
internal sealed class FakeSftpSession(SshConnectionRequest request) : ISftpSession
{
/// <inheritdoc />
public bool IsConnected { get; private set; } = true;
/// <inheritdoc />
public string? TransportFailure { get; private set; }
/// <inheritdoc />
public event EventHandler? ConnectionLost;
/// <summary>
/// Ends this session the way a rebooted host does: the connection is gone and it says so.
/// </summary>
/// <remarks>
/// Latched before the announcement, which is the ordering the real session promises — a screen reading
/// the reason out of the event would see null if a fake did it the other way round.
/// </remarks>
internal void Drop(string reason)
{
TransportFailure = reason;
IsConnected = false;
ConnectionLost?.Invoke(this, EventArgs.Empty);
}
/// <inheritdoc />
public HostKeyPresentation HostKey { get; } =
new(request.Host, request.Port, "ssh-ed25519", "SHA256:fake");
/// <inheritdoc />
public string Cipher { get; } = "aes256-gcm@openssh.com";
/// <inheritdoc />
public string HomeDirectory => $"/home/{request.Username}";
/// <summary>Every directory this session has been asked to list, in order.</summary>
/// <remarks>
/// The evidence that a refresh happened at all. A listing whose contents never change — this one always
/// answers the same single row — leaves no other trace of having been re-read, which is exactly the
/// thing the automatic refresh after a transfer has to be proved to do.
/// </remarks>
internal List<string> Listed { get; } = [];
/// <summary>Every rename this session has been asked for, as it was asked.</summary>
internal List<(string From, string To)> Renamed { get; } = [];
/// <summary>Every path this session has been asked to delete.</summary>
internal List<string> Deleted { get; } = [];
/// <summary>Every mode this session has been asked to set, as it was asked.</summary>
/// <remarks>
/// The evidence the screen's own status line cannot be: that sentence is code describing itself, and
/// what matters is which nine bits actually went to the host.
/// </remarks>
internal List<(string Path, int Mode)> PermissionsSet { get; } = [];
/// <summary>Every timestamp this session has been asked to set.</summary>
internal List<(string Path, DateTimeOffset When)> TimesSet { get; } = [];
/// <inheritdoc />
public Task SetPermissionsAsync(string path, int mode, CancellationToken cancellationToken)
{
PermissionsSet.Add((path, mode));
return Task.CompletedTask;
}
/// <inheritdoc />
public Task SetLastWriteTimeAsync(
string path,
DateTimeOffset lastWriteTimeUtc,
CancellationToken cancellationToken)
{
TimesSet.Add((path, lastWriteTimeUtc));
return Task.CompletedTask;
}
/// <inheritdoc />
public Task<IReadOnlyList<SftpEntry>> ListAsync(string path, CancellationToken cancellationToken)
{
Listed.Add(path);
return Task.FromResult<IReadOnlyList<SftpEntry>>(
[
new SftpEntry(
"notes.txt",
SftpPath.Combine(path, "notes.txt"),
SftpEntryKind.File,
12,
DateTimeOffset.UnixEpoch,
"-rw-r--r--"),
]);
}
/// <summary>
/// Paths this session admits to already having, and what it says about each.
/// </summary>
/// <remarks>
/// ◆ Empty by default, and that is load-bearing rather than lazy: the transfer queue asks
/// <see cref="StatAsync"/> whether a destination is taken and refuses to overwrite one that is, so a
/// fake answering yes for everything would make every upload in this suite fail. What needs a real
/// answer is editing, which stats the file before each write-back to see whether it moved — so a test
/// about that puts the one path it is about in here.
/// </remarks>
internal Dictionary<string, SftpEntry> Existing { get; } = new(StringComparer.Ordinal);
/// <summary>What the last write-back actually sent, so an edit's bytes are observable.</summary>
internal byte[]? Written { get; private set; }
/// <inheritdoc />
public Task<SftpEntry?> StatAsync(string path, CancellationToken cancellationToken) =>
Task.FromResult(Existing.TryGetValue(path, out var entry) ? entry : null);
/// <inheritdoc />
public Task<Stream> OpenReadAsync(string path, long offset, CancellationToken cancellationToken) =>
Task.FromResult<Stream>(new MemoryStream("hello there\n"u8.ToArray(), writable: false));
/// <inheritdoc />
/// <remarks>
/// ◆ A write updates <see cref="Existing"/> only for a path already in it, which is the difference
/// between modelling a server and breaking every other test in the suite: the transfer queue writes to
/// part files and stats them to decide whether there is anything to resume from, so a fake that
/// remembered every write would start answering yes to questions it currently answers no to. What is in
/// <see cref="Existing"/> is what a test deliberately put there.
/// </remarks>
public Task<Stream> OpenWriteAsync(string path, long offset, CancellationToken cancellationToken) =>
Task.FromResult<Stream>(new RecordingStream(bytes =>
{
Written = bytes;
if (Existing.TryGetValue(path, out var before))
{
Existing[path] = before with
{
Length = bytes.Length,
LastWriteTimeUtc = before.LastWriteTimeUtc.AddSeconds(1),
};
}
}));
/// <summary>Every directory this session was asked to make, in the order it was asked.</summary>
/// <remarks>
/// The evidence a folder upload actually built the tree before it queued the files. Order matters and is
/// asserted on: a parent has to exist before its child is asked for, and shortest-first is what
/// guarantees it.
/// </remarks>
internal List<string> Created { get; } = [];
/// <inheritdoc />
public Task CreateDirectoryAsync(string path, CancellationToken cancellationToken)
{
Created.Add(path);
return Task.CompletedTask;
}
/// <inheritdoc />
public Task DeleteAsync(string path, CancellationToken cancellationToken)
{
Deleted.Add(path);
return Task.CompletedTask;
}
/// <inheritdoc />
public Task RenameAsync(string fromPath, string toPath, CancellationToken cancellationToken)
{
Renamed.Add((fromPath, toPath));
return Task.CompletedTask;
}
/// <inheritdoc />
public ValueTask DisposeAsync()
{
IsConnected = false;
return ValueTask.CompletedTask;
}
}
internal sealed class FakeSshConnection(SshConnectionRequest request, List<FakeSshShellSession> shells)
: ISshConnection
{
/// <inheritdoc />
public bool IsConnected { get; private set; } = true;
/// <inheritdoc />
public HostKeyPresentation HostKey { get; } =
new(request.Host, request.Port, "ssh-ed25519", "SHA256:fake");
/// <inheritdoc />
public string Cipher { get; } = "aes256-gcm@openssh.com";
/// <inheritdoc />
public Task<ISshShellSession> OpenShellAsync(
TerminalSize size,
CancellationToken cancellationToken)
{
var shell = new FakeSshShellSession();
shells.Add(shell);
return Task.FromResult<ISshShellSession>(shell);
}
/// <inheritdoc />
public ValueTask DisposeAsync()
{
IsConnected = false;
return ValueTask.CompletedTask;
}
}
/// <summary>A write stream that hands its contents over when it is closed.</summary>
/// <remarks>
/// A <see cref="MemoryStream"/> cannot be read after disposal, and disposal is exactly when a write is
/// finished — so a fake that handed one back could observe a transfer starting and never what it sent.
/// </remarks>
internal sealed class RecordingStream(Action<byte[]> onClosed) : MemoryStream
{
private bool handed;
/// <inheritdoc />
protected override void Dispose(bool disposing)
{
if (disposing && !handed)
{
handed = true;
onClosed(ToArray());
}
base.Dispose(disposing);
}
}
/// <summary>A shell that is open, silent and closes only when it is told to.</summary>
/// <remarks>
/// <see cref="ReadAsync"/> blocks rather than returning 0. Returning 0 means the remote closed the
/// channel, which would end the session the moment it was opened and make the test assert against a
/// connection that had already gone. <see cref="Drop"/> and <see cref="Exit"/> are the two ways out.
/// </remarks>
internal sealed class FakeSshShellSession : ISshShellSession
{
private readonly CancellationTokenSource closed = new();
/// <inheritdoc />
public bool IsOpen { get; private set; } = true;
/// <inheritdoc />
public string? TransportFailure { get; private set; }
/// <summary>
/// Ends this shell the way a rebooted machine does: the read stops, with a transport failure latched.
/// </summary>
/// <remarks>
/// Latched before the read is released, which is the ordering the real session promises and the whole
/// reason the distinction is race-free — see <see cref="ISshShellSession.TransportFailure"/>. A fake
/// that set it afterwards would be testing a guarantee the product does not make.
/// </remarks>
internal void Drop(string reason)
{
TransportFailure = reason;
closed.Cancel();
}
/// <summary>Ends this shell the way <c>exit</c> does: the read stops, with nothing wrong underneath.</summary>
internal void Exit() => closed.Cancel();
/// <inheritdoc />
public async ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken)
{
try
{
await Task.Delay(Timeout.InfiniteTimeSpan, closed.Token).ConfigureAwait(false);
}
catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
{
// This shell ending, not the pump being asked to stop. Reported as end of stream rather than
// rethrown, because that is what a real read does for both of the endings above: the channel
// stops producing bytes, and what happened to the transport is told by the latch, not by an
// exception coming out of here.
return 0;
}
cancellationToken.ThrowIfCancellationRequested();
return 0;
}
/// <inheritdoc />
public ValueTask WriteAsync(ReadOnlyMemory<byte> data, CancellationToken cancellationToken) =>
ValueTask.CompletedTask;
/// <inheritdoc />
public void Resize(TerminalSize size)
{
}
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
IsOpen = false;
await closed.CancelAsync().ConfigureAwait(false);
closed.Dispose();
}
}
/// <summary>
/// A reachability probe that answers from a script instead of from the network.
/// </summary>
/// <remarks>
/// <para>
/// ◆ <b>Every shell in this suite is given one, and that is not only about speed.</b> The real probe resolves
/// a name and opens a socket — see <see cref="SshReachabilityProbe"/> — so a fixture without this would put a
/// live DNS lookup for <c>db.internal</c> and a live TCP connect in the middle of a unit suite, on every test
/// where a connection fails. That is slow, it needs a network, and it is worse than either: <c>db.internal</c>
/// resolves on any machine whose resolver has a search domain that makes it resolve, so the sentence the
/// assertions read would depend on whose network the suite was run from.
/// </para>
/// <para>
/// <see cref="Summary"/> defaults to empty, which is what keeps the tests that predate the probe asserting
/// the exact message they always did: an empty finding appends nothing, so those cases go on describing a
/// failure and nothing else. A test that is about the diagnosis sets it.
/// </para>
/// </remarks>
internal sealed class FakeReachabilityProbe : ISshReachabilityProbe
{
/// <summary>What every probe comes back with. Empty adds nothing to the failure it explains.</summary>
internal string Summary { get; set; } = string.Empty;
/// <summary>What it was asked about, in order, so a test can assert it was not asked at all.</summary>
internal List<string> Probed { get; } = [];
/// <inheritdoc />
public ValueTask<SshReachabilityReport> ProbeAsync(
string host,
int port,
CancellationToken cancellationToken)
{
Probed.Add($"{host}:{port}");
return ValueTask.FromResult(
new SshReachabilityReport(SshReach.Open, host, port, host, TimeSpan.Zero, Summary));
}
}