fix: harden ssh target handling

This commit is contained in:
Peter Steinberger
2026-01-26 21:10:36 +00:00
parent d34ae86114
commit fb14146033
8 changed files with 196 additions and 77 deletions

View File

@@ -282,22 +282,6 @@ enum CommandResolver {
guard !settings.target.isEmpty else { return nil }
guard let parsed = self.parseSSHTarget(settings.target) else { return nil }
var args: [String] = [
"-o", "BatchMode=yes",
"-o", "StrictHostKeyChecking=accept-new",
"-o", "UpdateHostKeys=yes",
]
if parsed.port > 0 { args.append(contentsOf: ["-p", String(parsed.port)]) }
let identity = settings.identity.trimmingCharacters(in: .whitespacesAndNewlines)
if !identity.isEmpty {
// Only use IdentitiesOnly when an explicit identity file is provided.
// This allows 1Password SSH agent and other SSH agents to provide keys.
args.append(contentsOf: ["-o", "IdentitiesOnly=yes"])
args.append(contentsOf: ["-i", identity])
}
let userHost = parsed.user.map { "\($0)@\(parsed.host)" } ?? parsed.host
args.append(userHost)
// Run the real clawdbot CLI on the remote host.
let exportedPath = [
"/opt/homebrew/bin",
@@ -324,7 +308,7 @@ enum CommandResolver {
} else {
"""
PRJ=\(self.shellQuote(userPRJ))
cd \(self.shellQuote(userPRJ)) || { echo "Project root not found: \(userPRJ)"; exit 127; }
cd "$PRJ" || { echo "Project root not found: $PRJ"; exit 127; }
"""
}
@@ -378,7 +362,16 @@ enum CommandResolver {
echo "clawdbot CLI missing on remote host"; exit 127;
fi
"""
args.append(contentsOf: ["/bin/sh", "-c", scriptBody])
let options: [String] = [
"-o", "BatchMode=yes",
"-o", "StrictHostKeyChecking=accept-new",
"-o", "UpdateHostKeys=yes",
]
let args = self.sshArguments(
target: parsed,
identity: settings.identity,
options: options,
remoteCommand: ["/bin/sh", "-c", scriptBody])
return ["/usr/bin/ssh"] + args
}
@@ -427,8 +420,11 @@ enum CommandResolver {
}
static func parseSSHTarget(_ target: String) -> SSHParsedTarget? {
let trimmed = target.trimmingCharacters(in: .whitespacesAndNewlines)
let trimmed = self.normalizeSSHTargetInput(target)
guard !trimmed.isEmpty else { return nil }
if trimmed.rangeOfCharacter(from: CharacterSet.whitespacesAndNewlines.union(.controlCharacters)) != nil {
return nil
}
let userHostPort: String
let user: String?
if let atRange = trimmed.range(of: "@") {
@@ -444,13 +440,31 @@ enum CommandResolver {
if let colon = userHostPort.lastIndex(of: ":"), colon != userHostPort.startIndex {
host = String(userHostPort[..<colon])
let portStr = String(userHostPort[userHostPort.index(after: colon)...])
port = Int(portStr) ?? 22
guard let parsedPort = Int(portStr), parsedPort > 0, parsedPort <= 65535 else {
return nil
}
port = parsedPort
} else {
host = userHostPort
port = 22
}
return SSHParsedTarget(user: user, host: host, port: port)
return self.makeSSHTarget(user: user, host: host, port: port)
}
static func sshTargetValidationMessage(_ target: String) -> String? {
let trimmed = self.normalizeSSHTargetInput(target)
guard !trimmed.isEmpty else { return nil }
if trimmed.hasPrefix("-") {
return "SSH target cannot start with '-'"
}
if trimmed.rangeOfCharacter(from: CharacterSet.whitespacesAndNewlines.union(.controlCharacters)) != nil {
return "SSH target cannot contain spaces"
}
if self.parseSSHTarget(trimmed) == nil {
return "SSH target must look like user@host[:port]"
}
return nil
}
private static func shellQuote(_ text: String) -> String {
@@ -468,6 +482,64 @@ enum CommandResolver {
return URL(fileURLWithPath: expanded)
}
private static func normalizeSSHTargetInput(_ target: String) -> String {
var trimmed = target.trimmingCharacters(in: .whitespacesAndNewlines)
if trimmed.hasPrefix("ssh ") {
trimmed = trimmed.replacingOccurrences(of: "ssh ", with: "")
.trimmingCharacters(in: .whitespacesAndNewlines)
}
return trimmed
}
private static func isValidSSHComponent(_ value: String, allowLeadingDash: Bool = false) -> Bool {
if value.isEmpty { return false }
if !allowLeadingDash, value.hasPrefix("-") { return false }
let invalid = CharacterSet.whitespacesAndNewlines.union(.controlCharacters)
return value.rangeOfCharacter(from: invalid) == nil
}
static func makeSSHTarget(user: String?, host: String, port: Int) -> SSHParsedTarget? {
let trimmedHost = host.trimmingCharacters(in: .whitespacesAndNewlines)
guard self.isValidSSHComponent(trimmedHost) else { return nil }
let trimmedUser = user?.trimmingCharacters(in: .whitespacesAndNewlines)
let normalizedUser: String?
if let trimmedUser {
guard self.isValidSSHComponent(trimmedUser) else { return nil }
normalizedUser = trimmedUser.isEmpty ? nil : trimmedUser
} else {
normalizedUser = nil
}
guard port > 0, port <= 65535 else { return nil }
return SSHParsedTarget(user: normalizedUser, host: trimmedHost, port: port)
}
private static func sshTargetString(_ target: SSHParsedTarget) -> String {
target.user.map { "\($0)@\(target.host)" } ?? target.host
}
static func sshArguments(
target: SSHParsedTarget,
identity: String,
options: [String],
remoteCommand: [String] = []) -> [String]
{
var args = options
if target.port > 0 {
args.append(contentsOf: ["-p", String(target.port)])
}
let trimmedIdentity = identity.trimmingCharacters(in: .whitespacesAndNewlines)
if !trimmedIdentity.isEmpty {
// Only use IdentitiesOnly when an explicit identity file is provided.
// This allows 1Password SSH agent and other SSH agents to provide keys.
args.append(contentsOf: ["-o", "IdentitiesOnly=yes"])
args.append(contentsOf: ["-i", trimmedIdentity])
}
args.append("--")
args.append(self.sshTargetString(target))
args.append(contentsOf: remoteCommand)
return args
}
#if SWIFT_PACKAGE
static func _testNodeManagerBinPaths(home: URL) -> [String] {
self.nodeManagerBinPaths(home: home)

View File

@@ -243,25 +243,36 @@ struct GeneralSettings: View {
}
private var remoteSshRow: some View {
HStack(alignment: .center, spacing: 10) {
Text("SSH target")
.font(.callout.weight(.semibold))
.frame(width: self.remoteLabelWidth, alignment: .leading)
TextField("user@host[:22]", text: self.$state.remoteTarget)
.textFieldStyle(.roundedBorder)
.frame(maxWidth: .infinity)
Button {
Task { await self.testRemote() }
} label: {
if self.remoteStatus == .checking {
ProgressView().controlSize(.small)
} else {
Text("Test remote")
let trimmedTarget = self.state.remoteTarget.trimmingCharacters(in: .whitespacesAndNewlines)
let validationMessage = CommandResolver.sshTargetValidationMessage(trimmedTarget)
let canTest = !trimmedTarget.isEmpty && validationMessage == nil
return VStack(alignment: .leading, spacing: 4) {
HStack(alignment: .center, spacing: 10) {
Text("SSH target")
.font(.callout.weight(.semibold))
.frame(width: self.remoteLabelWidth, alignment: .leading)
TextField("user@host[:22]", text: self.$state.remoteTarget)
.textFieldStyle(.roundedBorder)
.frame(maxWidth: .infinity)
Button {
Task { await self.testRemote() }
} label: {
if self.remoteStatus == .checking {
ProgressView().controlSize(.small)
} else {
Text("Test remote")
}
}
.buttonStyle(.borderedProminent)
.disabled(self.remoteStatus == .checking || !canTest)
}
if let validationMessage {
Text(validationMessage)
.font(.caption)
.foregroundStyle(.red)
.padding(.leading, self.remoteLabelWidth + 10)
}
.buttonStyle(.borderedProminent)
.disabled(self.remoteStatus == .checking || self.state.remoteTarget
.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty)
}
}
@@ -540,8 +551,15 @@ extension GeneralSettings {
}
// Step 1: basic SSH reachability check
guard let sshCommand = Self.sshCheckCommand(
target: settings.target,
identity: settings.identity)
else {
self.remoteStatus = .failed("SSH target is invalid")
return
}
let sshResult = await ShellExecutor.run(
command: Self.sshCheckCommand(target: settings.target, identity: settings.identity),
command: sshCommand,
cwd: nil,
env: nil,
timeout: 8)
@@ -587,20 +605,20 @@ extension GeneralSettings {
return !host.isEmpty
}
private static func sshCheckCommand(target: String, identity: String) -> [String] {
var args: [String] = [
"/usr/bin/ssh",
private static func sshCheckCommand(target: String, identity: String) -> [String]? {
guard let parsed = CommandResolver.parseSSHTarget(target) else { return nil }
let options = [
"-o", "BatchMode=yes",
"-o", "ConnectTimeout=5",
"-o", "StrictHostKeyChecking=accept-new",
"-o", "UpdateHostKeys=yes",
]
if !identity.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {
args.append(contentsOf: ["-i", identity])
}
args.append(target)
args.append("echo ok")
return args
let args = CommandResolver.sshArguments(
target: parsed,
identity: identity,
options: options,
remoteCommand: ["echo", "ok"])
return ["/usr/bin/ssh"] + args
}
private func formatSSHFailure(_ response: Response, target: String) -> String {

View File

@@ -559,22 +559,21 @@ final class NodePairingApprovalPrompter {
let process = Process()
process.executableURL = URL(fileURLWithPath: "/usr/bin/ssh")
var args = [
"-o",
"BatchMode=yes",
"-o",
"ConnectTimeout=5",
"-o",
"NumberOfPasswordPrompts=0",
"-o",
"PreferredAuthentications=publickey",
"-o",
"StrictHostKeyChecking=accept-new",
let options = [
"-o", "BatchMode=yes",
"-o", "ConnectTimeout=5",
"-o", "NumberOfPasswordPrompts=0",
"-o", "PreferredAuthentications=publickey",
"-o", "StrictHostKeyChecking=accept-new",
]
if port > 0, port != 22 {
args.append(contentsOf: ["-p", String(port)])
guard let target = CommandResolver.makeSSHTarget(user: user, host: host, port: port) else {
return false
}
args.append(contentsOf: ["-l", user, host, "/usr/bin/true"])
let args = CommandResolver.sshArguments(
target: target,
identity: "",
options: options,
remoteCommand: ["/usr/bin/true"])
process.arguments = args
let pipe = Pipe()
process.standardOutput = pipe

View File

@@ -206,6 +206,16 @@ extension OnboardingView {
.textFieldStyle(.roundedBorder)
.frame(width: fieldWidth)
}
if let message = CommandResolver.sshTargetValidationMessage(self.state.remoteTarget) {
GridRow {
Text("")
.frame(width: labelWidth, alignment: .leading)
Text(message)
.font(.caption)
.foregroundStyle(.red)
.frame(width: fieldWidth, alignment: .leading)
}
}
GridRow {
Text("Identity file")
.font(.callout.weight(.semibold))

View File

@@ -70,7 +70,7 @@ final class RemotePortTunnel {
"ssh tunnel using default remote port " +
"host=\(sshHost, privacy: .public) port=\(remotePort, privacy: .public)")
}
var args: [String] = [
let options: [String] = [
"-o", "BatchMode=yes",
"-o", "ExitOnForwardFailure=yes",
"-o", "StrictHostKeyChecking=accept-new",
@@ -81,16 +81,11 @@ final class RemotePortTunnel {
"-N",
"-L", "\(localPort):127.0.0.1:\(resolvedRemotePort)",
]
if parsed.port > 0 { args.append(contentsOf: ["-p", String(parsed.port)]) }
let identity = settings.identity.trimmingCharacters(in: .whitespacesAndNewlines)
if !identity.isEmpty {
// Only use IdentitiesOnly when an explicit identity file is provided.
// This allows 1Password SSH agent and other SSH agents to provide keys.
args.append(contentsOf: ["-o", "IdentitiesOnly=yes"])
args.append(contentsOf: ["-i", identity])
}
let userHost = parsed.user.map { "\($0)@\(parsed.host)" } ?? parsed.host
args.append(userHost)
let args = CommandResolver.sshArguments(
target: parsed,
identity: identity,
options: options)
let process = Process()
process.executableURL = URL(fileURLWithPath: "/usr/bin/ssh")

View File

@@ -123,11 +123,16 @@ import Testing
configRoot: [:])
#expect(cmd.first == "/usr/bin/ssh")
#expect(cmd.contains("clawd@example.com"))
if let marker = cmd.firstIndex(of: "--") {
#expect(cmd[marker + 1] == "clawd@example.com")
} else {
#expect(Bool(false))
}
#expect(cmd.contains("-i"))
#expect(cmd.contains("/tmp/id_ed25519"))
if let script = cmd.last {
#expect(script.contains("cd '/srv/clawdbot'"))
#expect(script.contains("PRJ='/srv/clawdbot'"))
#expect(script.contains("cd \"$PRJ\""))
#expect(script.contains("clawdbot"))
#expect(script.contains("status"))
#expect(script.contains("--json"))
@@ -135,6 +140,12 @@ import Testing
}
}
@Test func rejectsUnsafeSSHTargets() async throws {
#expect(CommandResolver.parseSSHTarget("-oProxyCommand=calc") == nil)
#expect(CommandResolver.parseSSHTarget("host:-oProxyCommand=calc") == nil)
#expect(CommandResolver.parseSSHTarget("user@host:2222")?.port == 2222)
}
@Test func configRootLocalOverridesRemoteDefaults() async throws {
let defaults = self.makeDefaults()
defaults.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey)

View File

@@ -11,7 +11,12 @@ struct MasterDiscoveryMenuSmokeTests {
discovery.statusText = "Searching…"
discovery.gateways = []
let view = GatewayDiscoveryInlineList(discovery: discovery, currentTarget: nil, onSelect: { _ in })
let view = GatewayDiscoveryInlineList(
discovery: discovery,
currentTarget: nil,
currentUrl: nil,
transport: .ssh,
onSelect: { _ in })
_ = view.body
}
@@ -32,7 +37,12 @@ struct MasterDiscoveryMenuSmokeTests {
]
let currentTarget = "\(NSUserName())@office.tailnet-123.ts.net:2222"
let view = GatewayDiscoveryInlineList(discovery: discovery, currentTarget: currentTarget, onSelect: { _ in })
let view = GatewayDiscoveryInlineList(
discovery: discovery,
currentTarget: currentTarget,
currentUrl: nil,
transport: .ssh,
onSelect: { _ in })
_ = view.body
}