Add ExecuteSubsystemAsync for subsystem based proc #183
Conversation
Adds the ExecuteSubsystemAsync call which can create an SSH channel from a subsystem and return the RemoteProcess interface. This is used when the SSH server defines custom subsystems in its sshd_config file that allows a client to create a channel using a pre-defined subsystem name rather than through an explicit command line argument.
| @@ -11,6 +11,7 @@ interface ISshClientImplementation | |||
| { | |||
| Task ConnectAsync(CancellationToken cancellationToken); | |||
| Task<ISshChannel> OpenRemoteProcessChannelAsync(Type channelType, string command, CancellationToken cancellationToken); | |||
| Task<ISshChannel> OpenRemoteSubsystemChannelAsync(Type channelType, string subsystem, CancellationToken cancellationToken); | |||
There was a problem hiding this comment.
I'm unsure how you want to handle this, IIRC the default access modifier for an interface is public so this may be a breaking change API wise.
There was a problem hiding this comment.
This interface itself is not public so there is no backwards compatibility concern.
In addition to the managed implementation, there once also was a libssh based backend. This interface is a reminiscent of that, and something to be removed.
fyi, C# has a feature to extend interfaces so it doesn't affect existing implementations, see default interface methods.
There was a problem hiding this comment.
Thanks for clarifying
| Assert.False(isError); | ||
| Assert.Equal(helloWorld1Bytes, buffer.AsSpan(0, bytesRead).ToArray()); | ||
|
|
||
| await process.WriteLineAsync("echo -n 'hello world 2'"); |
There was a problem hiding this comment.
I tried to do >&2 to test out isError but it seems like SSH doesn't return SSH_EXTENDED_DATA_STDERR on subsystem based channels. I was unable to even get the ssh binary to return stderr on a pre-configured subsystem that wrote to stderr so I assume it's not something that can happen. I couldn't find anything in the RFC around this but I didn't look too hard.
There was a problem hiding this comment.
The test is good as is. It covers what we need.
src/Tmds.Ssh/SshSession.cs
Outdated
| @@ -620,8 +623,11 @@ public async Task<ISshChannel> OpenUnixConnectionChannelAsync(Type channelType, | |||
| } | |||
|
|
|||
| public async Task<ISshChannel> OpenSftpClientChannelAsync(Action<SshChannel> onAbort, CancellationToken cancellationToken) | |||
| => await OpenSubsystemChannelAsync(typeof(SftpChannel), onAbort, "sft", cancellationToken).ConfigureAwait(false); | |||
There was a problem hiding this comment.
Oops sorry about that.
test/Tmds.Ssh.Tests/SshServer.cs
Outdated
| @@ -20,6 +20,7 @@ public class SshServer : IDisposable | |||
| public string TestUserHome => $"/home/{TestUser}"; | |||
| public string TestUserPassword => "secret"; | |||
| public string TestUserIdentityFile => $"{ContainerBuildContext}/user_key_rsa"; | |||
| public string TestSubsystem = "tmds_test"; | |||
There was a problem hiding this comment.
nit: rename tmds_test to test_subsystem
There was a problem hiding this comment.
Has been renamed.
|
Thanks for this contribution @jborean93! Until this becomes available in a nuget.org release, you can use it from the CI feed: https://github.com/tmds/Tmds.Ssh?tab=readme-ov-file#ci-feed. |
|
Thanks very much for the speedy review and this awesome library. It opens up a few things I’ve been wanting to do in PowerShell and .NET for a while now. |
Adds the ExecuteSubsystemAsync call which can create an SSH channel from a subsystem and return the RemoteProcess interface. This is used when the SSH server defines custom subsystems in its sshd_config file that allows a client to create a channel using a pre-defined subsystem name rather than through an explicit command line argument.
Fixes: #182