feat: harden ssh tunnel command state
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
521cf14b92
commit
652f552f0d
@ -357,12 +357,13 @@ Risk: low. No protocol changes.
|
||||
|
||||
Deliverables:
|
||||
|
||||
- Mac-equivalent SSH options:
|
||||
- Mac-equivalent SSH options: **implemented for tunnel startup**
|
||||
- `BatchMode=yes`
|
||||
- `ExitOnForwardFailure=yes`
|
||||
- `ServerAliveInterval=15`
|
||||
- `ServerAliveCountMax=3`
|
||||
- `TCPKeepAlive=yes`
|
||||
- Explicit tunnel states (`NotConfigured`, `Stopped`, `Starting`, `Up`, `Restarting`, `Failed`): **implemented**
|
||||
- Fast-fail detection.
|
||||
- Optional random local port fallback.
|
||||
- WSL detection helper with timeout/cache.
|
||||
|
||||
46
src/OpenClaw.Shared/SshTunnelCommandLine.cs
Normal file
46
src/OpenClaw.Shared/SshTunnelCommandLine.cs
Normal file
@ -0,0 +1,46 @@
|
||||
using System.Text;
|
||||
using System.Text.RegularExpressions;
|
||||
|
||||
namespace OpenClaw.Shared;
|
||||
|
||||
public static class SshTunnelCommandLine
|
||||
{
|
||||
private static readonly Regex s_validSshUser = new(@"^[a-zA-Z0-9._-]+$", RegexOptions.Compiled);
|
||||
private static readonly Regex s_validSshHost = new(@"^[a-zA-Z0-9._-]+$", RegexOptions.Compiled);
|
||||
|
||||
public static string BuildArguments(string user, string host, int remotePort, int localPort)
|
||||
{
|
||||
user = user.Trim();
|
||||
host = host.Trim();
|
||||
|
||||
if (!s_validSshUser.IsMatch(user))
|
||||
throw new ArgumentException($"SSH user contains invalid characters: '{user}'", nameof(user));
|
||||
if (!s_validSshHost.IsMatch(host))
|
||||
throw new ArgumentException($"SSH host contains invalid characters: '{host}'", nameof(host));
|
||||
ValidatePort(remotePort, nameof(remotePort));
|
||||
ValidatePort(localPort, nameof(localPort));
|
||||
|
||||
var sb = new StringBuilder();
|
||||
sb.Append("-o BatchMode=yes ");
|
||||
sb.Append("-o ExitOnForwardFailure=yes ");
|
||||
sb.Append("-o ServerAliveInterval=15 ");
|
||||
sb.Append("-o ServerAliveCountMax=3 ");
|
||||
sb.Append("-o TCPKeepAlive=yes ");
|
||||
sb.Append("-N ");
|
||||
sb.Append("-L ");
|
||||
sb.Append(localPort);
|
||||
sb.Append(":127.0.0.1:");
|
||||
sb.Append(remotePort);
|
||||
sb.Append(' ');
|
||||
sb.Append(user);
|
||||
sb.Append('@');
|
||||
sb.Append(host);
|
||||
return sb.ToString();
|
||||
}
|
||||
|
||||
private static void ValidatePort(int port, string parameterName)
|
||||
{
|
||||
if (port is < 1 or > 65535)
|
||||
throw new ArgumentOutOfRangeException(parameterName, port, "SSH tunnel ports must be between 1 and 65535.");
|
||||
}
|
||||
}
|
||||
@ -2013,8 +2013,8 @@ public partial class App : Application
|
||||
var user = string.IsNullOrWhiteSpace(_sshTunnelService?.CurrentUser)
|
||||
? _settings.SshTunnelUser
|
||||
: _sshTunnelService!.CurrentUser!;
|
||||
var status = _sshTunnelService?.IsRunning == true
|
||||
? TunnelStatus.Up
|
||||
var status = _sshTunnelService?.Status is TunnelStatus.Up or TunnelStatus.Starting or TunnelStatus.Restarting or TunnelStatus.Failed
|
||||
? _sshTunnelService.Status
|
||||
: string.IsNullOrWhiteSpace(_sshTunnelService?.LastError)
|
||||
? TunnelStatus.Stopped
|
||||
: TunnelStatus.Failed;
|
||||
@ -2624,6 +2624,7 @@ public partial class App : Application
|
||||
private async void OnSshTunnelExited(object? sender, int exitCode)
|
||||
{
|
||||
Logger.Warn($"SSH tunnel exited unexpectedly (code {exitCode}); restarting in 3s...");
|
||||
_sshTunnelService?.MarkRestarting(exitCode);
|
||||
await Task.Delay(3000);
|
||||
if (_sshTunnelService != null && _settings?.UseSshTunnel == true)
|
||||
{
|
||||
|
||||
@ -1,8 +1,6 @@
|
||||
using OpenClaw.Shared;
|
||||
using System;
|
||||
using System.Diagnostics;
|
||||
using System.Text;
|
||||
using System.Text.RegularExpressions;
|
||||
|
||||
namespace OpenClawTray.Services;
|
||||
|
||||
@ -31,6 +29,13 @@ public sealed class SshTunnelService : IDisposable
|
||||
public int CurrentLocalPort { get; private set; }
|
||||
public DateTime? StartedAtUtc { get; private set; }
|
||||
public string? LastError { get; private set; }
|
||||
public TunnelStatus Status { get; private set; } = TunnelStatus.NotConfigured;
|
||||
|
||||
public void MarkRestarting(int exitCode)
|
||||
{
|
||||
Status = TunnelStatus.Restarting;
|
||||
LastError = $"SSH tunnel exited unexpectedly with code {exitCode}; restart is scheduled.";
|
||||
}
|
||||
|
||||
public void EnsureStarted(SettingsManager settings)
|
||||
{
|
||||
@ -38,6 +43,7 @@ public sealed class SshTunnelService : IDisposable
|
||||
{
|
||||
Stop();
|
||||
LastError = null;
|
||||
Status = TunnelStatus.NotConfigured;
|
||||
return;
|
||||
}
|
||||
|
||||
@ -57,10 +63,12 @@ public sealed class SshTunnelService : IDisposable
|
||||
|
||||
if (IsRunning && string.Equals(_lastSpec, spec, StringComparison.Ordinal))
|
||||
{
|
||||
Status = TunnelStatus.Up;
|
||||
return;
|
||||
}
|
||||
|
||||
Stop();
|
||||
Status = TunnelStatus.Starting;
|
||||
StartProcess(user, host, remotePort, localPort);
|
||||
_lastSpec = spec;
|
||||
}
|
||||
@ -69,6 +77,8 @@ public sealed class SshTunnelService : IDisposable
|
||||
{
|
||||
if (_process == null)
|
||||
{
|
||||
if (Status != TunnelStatus.NotConfigured)
|
||||
Status = TunnelStatus.Stopped;
|
||||
return;
|
||||
}
|
||||
|
||||
@ -93,6 +103,8 @@ public sealed class SshTunnelService : IDisposable
|
||||
_process = null;
|
||||
_lastSpec = null;
|
||||
StartedAtUtc = null;
|
||||
if (Status != TunnelStatus.NotConfigured)
|
||||
Status = TunnelStatus.Stopped;
|
||||
_stopping = false;
|
||||
}
|
||||
}
|
||||
@ -102,7 +114,7 @@ public sealed class SshTunnelService : IDisposable
|
||||
var psi = new ProcessStartInfo
|
||||
{
|
||||
FileName = "ssh",
|
||||
Arguments = BuildArguments(user, host, remotePort, localPort),
|
||||
Arguments = SshTunnelCommandLine.BuildArguments(user, host, remotePort, localPort),
|
||||
UseShellExecute = false,
|
||||
RedirectStandardOutput = true,
|
||||
RedirectStandardError = true,
|
||||
@ -143,6 +155,7 @@ public sealed class SshTunnelService : IDisposable
|
||||
_logger.Warn($"SSH tunnel exited unexpectedly (code {exitCode})");
|
||||
LastError = $"SSH tunnel exited unexpectedly with code {exitCode}.";
|
||||
StartedAtUtc = null;
|
||||
Status = TunnelStatus.Failed;
|
||||
try { process.Dispose(); } catch { }
|
||||
_process = null;
|
||||
_lastSpec = null;
|
||||
@ -160,6 +173,7 @@ public sealed class SshTunnelService : IDisposable
|
||||
catch (Exception ex)
|
||||
{
|
||||
LastError = ex.Message;
|
||||
Status = TunnelStatus.Failed;
|
||||
process.Dispose();
|
||||
throw new InvalidOperationException("Unable to start SSH tunnel process. Ensure OpenSSH client is installed and available in PATH.", ex);
|
||||
}
|
||||
@ -173,6 +187,7 @@ public sealed class SshTunnelService : IDisposable
|
||||
CurrentLocalPort = localPort;
|
||||
StartedAtUtc = DateTime.UtcNow;
|
||||
LastError = null;
|
||||
Status = TunnelStatus.Up;
|
||||
|
||||
_logger.Info($"SSH tunnel started: 127.0.0.1:{localPort} -> 127.0.0.1:{remotePort} via {user}@{host}");
|
||||
}
|
||||
@ -180,30 +195,6 @@ public sealed class SshTunnelService : IDisposable
|
||||
private static string BuildSpec(string user, string host, int remotePort, int localPort)
|
||||
=> $"{user}@{host}:{localPort}:{remotePort}";
|
||||
|
||||
// Strict validation for SSH user/host to prevent command injection
|
||||
private static readonly Regex s_validSshUser = new(@"^[a-zA-Z0-9._-]+$", RegexOptions.Compiled);
|
||||
private static readonly Regex s_validSshHost = new(@"^[a-zA-Z0-9._-]+$", RegexOptions.Compiled);
|
||||
|
||||
private static string BuildArguments(string user, string host, int remotePort, int localPort)
|
||||
{
|
||||
if (!s_validSshUser.IsMatch(user))
|
||||
throw new ArgumentException($"SSH user contains invalid characters: '{user}'");
|
||||
if (!s_validSshHost.IsMatch(host))
|
||||
throw new ArgumentException($"SSH host contains invalid characters: '{host}'");
|
||||
|
||||
var sb = new StringBuilder();
|
||||
sb.Append("-N ");
|
||||
sb.Append("-L ");
|
||||
sb.Append(localPort);
|
||||
sb.Append(":127.0.0.1:");
|
||||
sb.Append(remotePort);
|
||||
sb.Append(' ');
|
||||
sb.Append(user);
|
||||
sb.Append('@');
|
||||
sb.Append(host);
|
||||
return sb.ToString();
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
Stop();
|
||||
|
||||
@ -124,6 +124,28 @@ public class AgentActivityTests
|
||||
}
|
||||
}
|
||||
|
||||
public class SshTunnelCommandLineTests
|
||||
{
|
||||
[Fact]
|
||||
public void BuildArguments_UsesMacParitySshOptions()
|
||||
{
|
||||
var args = SshTunnelCommandLine.BuildArguments("scott", "mac-mini.local", 18789, 28789);
|
||||
|
||||
Assert.Equal("-o BatchMode=yes -o ExitOnForwardFailure=yes -o ServerAliveInterval=15 -o ServerAliveCountMax=3 -o TCPKeepAlive=yes -N -L 28789:127.0.0.1:18789 scott@mac-mini.local", args);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("bad user", "mac-mini", 18789, 28789)]
|
||||
[InlineData("scott", "mac mini", 18789, 28789)]
|
||||
[InlineData("scott", "mac-mini", 0, 28789)]
|
||||
[InlineData("scott", "mac-mini", 18789, 70000)]
|
||||
public void BuildArguments_RejectsUnsafeInputs(string user, string host, int remotePort, int localPort)
|
||||
{
|
||||
Assert.ThrowsAny<ArgumentException>(() =>
|
||||
SshTunnelCommandLine.BuildArguments(user, host, remotePort, localPort));
|
||||
}
|
||||
}
|
||||
|
||||
public class ChannelHealthTests
|
||||
{
|
||||
[Theory]
|
||||
|
||||
Loading…
Reference in New Issue
Block a user