Public Access
Clear the SSH gate: window-change reaches the remote, and licence as MIT
Licence is MIT, set solution-wide rather than only on the packable project: DodoSSH.Contracts is published so clients can build against it, and a package with no licence expression is one a corporate policy scanner rejects outright. The SSH.NET spike is the M1 client gate and it passes. SSH.NET 2025.1.0 exposes ShellStream.ChangeWindowSize, but a method existing is not the remote observing it, so the tests read `stty size` back from a real sshd after resizing rather than asserting the call did not throw. Repeated resizes each take effect too, which matters because dragging a window edge produces a stream of them. The IChannelSession fallback is not needed. Also verified against a real sshd: password and public-key auth, that the host key arrives as a raw blob we can fingerprint ourselves rather than reading SSH.NET's MD5 property, and that refusing the key via CanTrust actually aborts the connection -- without which the TOFU dialog would be decoration. Kept as a permanent suite, not deleted after the spike. An upgrade that silently stopped sending the request would present as wrapped output only after a resize, which is easy to misattribute to the terminal emulator. Two bugs in the test itself, both worth naming because either would have been read as "resize does not work": - A PTY emits CRLF, and the anchored regex rejected the CR. The output visibly contained `24 80` while the match failed. - Each read can begin with output still buffered from the previous command, including its size line. Taking the first match would have reported the pre-resize size. platform-flags.md now records window-change as resolved rather than unverified -- a stale flag is worse than none -- plus the three real SSH.NET limits found on the way: ShellStream does not override ReadAsync so every idle session parks a pool thread, one connection cannot serve both SshClient and SftpClient, and agent forwarding needs an upstream change.
This commit is contained in:
@@ -0,0 +1,249 @@
|
||||
using System.Globalization;
|
||||
using System.Text;
|
||||
using System.Text.RegularExpressions;
|
||||
using Renci.SshNet;
|
||||
using Renci.SshNet.Common;
|
||||
|
||||
namespace DodoSSH.Client.Ssh.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// The M1 client gate: does a PTY resize actually reach the remote?
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The plan flagged <c>window-change</c> on <c>ShellStream</c> as unverified, and it decides the
|
||||
/// terminal's whole design — a terminal that cannot resize is unusable, and the fallback is
|
||||
/// dropping to <c>IChannelSession</c> and driving the channel requests directly. SSH.NET 2025.1.0
|
||||
/// does expose <c>ChangeWindowSize</c>, but a method existing is not the remote observing it, so
|
||||
/// these read the size back from the server rather than asserting the call did not throw.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Kept as a permanent regression suite rather than deleted after the spike. An SSH.NET upgrade
|
||||
/// that silently stopped sending the request would otherwise ship, and the symptom — wrapped
|
||||
/// output only after the user resizes the window — is easy to blame on the terminal emulator.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Collection(SshCollection.Name)]
|
||||
public sealed class PtyAndResizeSpikeTests(SshServerFixture fixture)
|
||||
{
|
||||
private static readonly TimeSpan ReadTimeout = TimeSpan.FromSeconds(20);
|
||||
|
||||
/// <summary>
|
||||
/// Matches busybox <c>stty size</c> output: rows then columns.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Surrounding horizontal whitespace is tolerated, and the caller normalises CR to LF first.
|
||||
/// A PTY emits CRLF, so an anchored pattern that does not account for the CR fails on output
|
||||
/// that is visibly correct — which reads as "the resize did not work" and sends you looking in
|
||||
/// entirely the wrong place.
|
||||
/// </remarks>
|
||||
private static readonly Regex SizeLine = new(
|
||||
@"^[ \t]*(?<rows>[0-9]{1,5})[ \t]+(?<columns>[0-9]{1,5})[ \t]*$",
|
||||
RegexOptions.Multiline | RegexOptions.ExplicitCapture,
|
||||
TimeSpan.FromSeconds(1));
|
||||
|
||||
[Fact]
|
||||
public async Task PasswordAuthentication_Connects()
|
||||
{
|
||||
using var client = CreatePasswordClient();
|
||||
|
||||
await client.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
client.IsConnected.ShouldBeTrue();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PublicKeyAuthentication_Connects()
|
||||
{
|
||||
// PKCS#1 PEM ("BEGIN RSA PRIVATE KEY"), which SSH.NET has read for years. The vault stores
|
||||
// key material in its own format, so this is only about SSH.NET accepting a standard PEM.
|
||||
var pem = fixture.ClientKey.ExportRSAPrivateKeyPem();
|
||||
using var keyStream = new MemoryStream(Encoding.ASCII.GetBytes(pem));
|
||||
using var privateKey = new PrivateKeyFile(keyStream);
|
||||
|
||||
using var client = new SshClient(
|
||||
new ConnectionInfo(
|
||||
fixture.Host,
|
||||
fixture.Port,
|
||||
SshServerFixture.Username,
|
||||
new PrivateKeyAuthenticationMethod(SshServerFixture.Username, privateKey)));
|
||||
|
||||
await client.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
client.IsConnected.ShouldBeTrue();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task TheHostKey_ArrivesAsARawBlobWeCanFingerprint()
|
||||
{
|
||||
// Known-hosts entries are a synced vault entity, so the fingerprint we store has to be
|
||||
// computed from the wire blob rather than read off a library property. This confirms the
|
||||
// blob is exposed at all, and that our formatting matches OpenSSH's shape.
|
||||
using var client = CreatePasswordClient();
|
||||
|
||||
byte[]? blob = null;
|
||||
string? algorithm = null;
|
||||
|
||||
client.HostKeyReceived += (_, e) =>
|
||||
{
|
||||
blob = e.HostKey;
|
||||
algorithm = e.HostKeyName;
|
||||
};
|
||||
|
||||
await client.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
blob.ShouldNotBeNull();
|
||||
blob.Length.ShouldBeGreaterThan(0);
|
||||
algorithm.ShouldNotBeNullOrEmpty();
|
||||
|
||||
var fingerprint = SshHostKeyFingerprint.Format(blob);
|
||||
|
||||
fingerprint.ShouldStartWith(SshHostKeyFingerprint.Prefix);
|
||||
fingerprint.ShouldNotEndWith("=");
|
||||
|
||||
// 32 bytes of SHA-256 render as 43 unpadded base64 characters.
|
||||
fingerprint.Length.ShouldBe(SshHostKeyFingerprint.Prefix.Length + 43);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RefusingTheHostKey_AbortsTheConnection()
|
||||
{
|
||||
// The mismatch path has to be a hard block, not a warning. If CanTrust were advisory the
|
||||
// TOFU dialog would be decoration.
|
||||
using var client = CreatePasswordClient();
|
||||
|
||||
client.HostKeyReceived += (_, e) => e.CanTrust = false;
|
||||
|
||||
await Should.ThrowAsync<SshConnectionException>(
|
||||
async () => await client.ConnectAsync(TestContext.Current.CancellationToken));
|
||||
|
||||
client.IsConnected.ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task APtyShell_ReportsTheSizeItWasCreatedWith()
|
||||
{
|
||||
using var client = CreatePasswordClient();
|
||||
await client.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
using var shell = client.CreateShellStream("xterm-256color", 80, 24, 800, 600, 4096);
|
||||
|
||||
var (rows, columns) = await ReadSizeAsync(shell);
|
||||
|
||||
rows.ShouldBe(24);
|
||||
columns.ShouldBe(80);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ChangeWindowSize_IsObservedByTheRemote()
|
||||
{
|
||||
// The gate. Reads the size back from the server after resizing, so this fails if
|
||||
// ChangeWindowSize silently does nothing.
|
||||
using var client = CreatePasswordClient();
|
||||
await client.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
using var shell = client.CreateShellStream("xterm-256color", 80, 24, 800, 600, 4096);
|
||||
|
||||
var initial = await ReadSizeAsync(shell);
|
||||
initial.ShouldBe((24, 80));
|
||||
|
||||
shell.ChangeWindowSize(columns: 132, rows: 43, width: 1320, height: 1075);
|
||||
|
||||
var resized = await ReadSizeAsync(shell);
|
||||
|
||||
resized.ShouldBe(
|
||||
(43, 132),
|
||||
"The remote did not observe the window-change request. ShellStream.ChangeWindowSize is "
|
||||
+ "not usable, and the terminal must drive IChannelSession directly instead.");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ChangeWindowSize_CanBeCalledRepeatedly()
|
||||
{
|
||||
// A user dragging a window edge produces a stream of these. If only the first took effect,
|
||||
// the terminal would settle on whatever size the drag started at.
|
||||
using var client = CreatePasswordClient();
|
||||
await client.ConnectAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
using var shell = client.CreateShellStream("xterm-256color", 80, 24, 800, 600, 4096);
|
||||
await ReadSizeAsync(shell);
|
||||
|
||||
foreach (var (columns, rows) in new[] { (100u, 30u), (120u, 40u), (90u, 25u) })
|
||||
{
|
||||
shell.ChangeWindowSize(columns, rows, columns * 10, rows * 25);
|
||||
|
||||
var observed = await ReadSizeAsync(shell);
|
||||
observed.ShouldBe(((int)rows, (int)columns));
|
||||
}
|
||||
}
|
||||
|
||||
// ---- Helpers ----
|
||||
|
||||
private SshClient CreatePasswordClient() =>
|
||||
new(fixture.Host, fixture.Port, SshServerFixture.Username, SshServerFixture.Password);
|
||||
|
||||
/// <summary>Asks the remote what size it thinks the terminal is.</summary>
|
||||
/// <remarks>
|
||||
/// The marker is written split — <c>"MAR""KER"</c> — so the PTY's echo of the command line
|
||||
/// does not contain it. Without that the read completes on the echo and never sees the output,
|
||||
/// which looks exactly like a resize that did not take effect.
|
||||
/// </remarks>
|
||||
private static async Task<(int Rows, int Columns)> ReadSizeAsync(ShellStream shell)
|
||||
{
|
||||
shell.WriteLine("""stty size; echo "MAR""KER" """);
|
||||
|
||||
var output = (await ReadUntilAsync(shell, "MARKER")).Replace('\r', '\n');
|
||||
|
||||
var matches = SizeLine.Matches(output);
|
||||
matches.Count.ShouldBeGreaterThan(0, $"No 'rows cols' line in output:\n{output}");
|
||||
|
||||
// The last match, not the first. A read can begin with output still buffered from the
|
||||
// previous command, including its size line — taking the first match would report the size
|
||||
// from before the resize and turn a working resize into a failing test.
|
||||
var match = matches[^1];
|
||||
|
||||
return (
|
||||
int.Parse(match.Groups["rows"].Value, CultureInfo.InvariantCulture),
|
||||
int.Parse(match.Groups["columns"].Value, CultureInfo.InvariantCulture));
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Raced against a timeout rather than cancelled. <c>ShellStream</c> does not override
|
||||
/// <c>ReadAsync</c>, so the base implementation runs the blocking read on a pool thread and
|
||||
/// cannot observe a token once it has started — passing one would produce a test that hangs
|
||||
/// instead of failing. The abandoned read dies with the process.
|
||||
/// </remarks>
|
||||
private static async Task<string> ReadUntilAsync(ShellStream shell, string marker)
|
||||
{
|
||||
var accumulated = new StringBuilder();
|
||||
var deadline = TimeProvider.System.GetUtcNow() + ReadTimeout;
|
||||
|
||||
while (TimeProvider.System.GetUtcNow() < deadline)
|
||||
{
|
||||
var buffer = new byte[4096];
|
||||
var read = shell.ReadAsync(buffer.AsMemory()).AsTask();
|
||||
|
||||
var remaining = deadline - TimeProvider.System.GetUtcNow();
|
||||
if (remaining <= TimeSpan.Zero || await Task.WhenAny(read, Task.Delay(remaining)) != read)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
var count = await read;
|
||||
if (count == 0)
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
accumulated.Append(Encoding.UTF8.GetString(buffer, 0, count));
|
||||
|
||||
if (accumulated.ToString().Contains(marker, StringComparison.Ordinal))
|
||||
{
|
||||
return accumulated.ToString();
|
||||
}
|
||||
}
|
||||
|
||||
throw new TimeoutException(
|
||||
$"Did not see '{marker}' within {ReadTimeout}. Output so far:\n{accumulated}");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user