Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ public enum SSHPTYAttachExitCode: Int32 {

/// Builds the POSIX shell loop shared by persistent SSH PTY attach entry points.
///
/// The attach environment is exported on its own lines rather than as an
/// assignment prefix, because a prefix is only legal before a simple command
/// and callers legitimately pass compound commands.
///
/// - Parameters:
/// - command: The shell command that performs one attach attempt.
/// - reauthenticates: Whether transient failures should request foreground authentication.
Expand Down Expand Up @@ -99,7 +103,11 @@ public enum SSHPTYAttachExitCode: Int32 {
" fi",
" if [ \"$cmux_ssh_attach_reauth_required\" -eq 0 ]; then",
" if [ \"$cmux_ssh_attach_reconnect_unbounded\" -eq 1 ] || [ \"$cmux_ssh_attach_retry\" -lt \"$cmux_ssh_attach_reconnect_limit\" ]; then cmux_ssh_attach_can_retry=1; else cmux_ssh_attach_can_retry=0; fi",
" CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY=\"$cmux_ssh_attach_can_retry\" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY=\"$cmux_ssh_attach_no_progress_retry\" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT=\"$cmux_ssh_attach_no_progress_limit\" \(command)",
" CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY=\"$cmux_ssh_attach_can_retry\"",
" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY=\"$cmux_ssh_attach_no_progress_retry\"",
" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT=\"$cmux_ssh_attach_no_progress_limit\"",
" export CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT",
" \(command)",
" cmux_ssh_attach_status=$?",
" case \"$cmux_ssh_attach_status\" in",
" \(noProgressPolicy.status)) cmux_ssh_attach_no_progress_retry=$((cmux_ssh_attach_no_progress_retry + 1)); cmux_ssh_attach_reconnect_delay=\"$cmux_ssh_attach_reconnect_initial_delay\"; \(noProgressPolicy.limitReachedCommand) ;;",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ public struct SSHPTYAttachRetryScriptBuilder: Sendable {
/// `reauthenticates` is true and installs `cmux_ssh_attach_signal_exit`
/// before these lines execute.
///
/// The attach environment is exported on its own lines rather than as an
/// assignment prefix, because a prefix is only legal before a simple command
/// and callers legitimately pass compound commands.
///
/// - Parameters:
/// - command: Shell command that performs one PTY attachment attempt.
/// - reauthenticates: Whether status 255 requires foreground authentication before reattaching.
Expand Down Expand Up @@ -67,7 +71,11 @@ public struct SSHPTYAttachRetryScriptBuilder: Sendable {
" fi",
" if [ \"$cmux_ssh_attach_reauth_required\" -eq 0 ]; then",
" if [ \"$cmux_ssh_attach_reconnect_unbounded\" -eq 1 ] || [ \"$cmux_ssh_attach_retry\" -lt \"$cmux_ssh_attach_reconnect_limit\" ]; then cmux_ssh_attach_can_retry=1; else cmux_ssh_attach_can_retry=0; fi",
" CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY=\"$cmux_ssh_attach_can_retry\" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY=\"$cmux_ssh_attach_no_progress_retry\" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT=\"$cmux_ssh_attach_no_progress_limit\" \(command)",
" CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY=\"$cmux_ssh_attach_can_retry\"",
" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY=\"$cmux_ssh_attach_no_progress_retry\"",
" CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT=\"$cmux_ssh_attach_no_progress_limit\"",
" export CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT",
" \(command)",
" cmux_ssh_attach_status=$?",
" case \"$cmux_ssh_attach_status\" in",
" \(noProgressStatus)) cmux_ssh_attach_no_progress_retry=$((cmux_ssh_attach_no_progress_retry + 1)); cmux_ssh_attach_reconnect_delay=\"$cmux_ssh_attach_reconnect_initial_delay\"; \(noProgressPolicy.limitReachedCommand) ;;",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,38 @@ struct SSHPTYAttachExitCodeTests {
#expect(!fileManager.fileExists(atPath: authAttempts.path))
}

// Regression for #9443: env-assignment prefixes are only legal before a
// simple command, so a compound attach command used to produce
// "syntax error near unexpected token `then'".
@Test("retry loop stays valid POSIX shell for compound attach commands")
func retryLoopAcceptsCompoundAttachCommands() throws {
let compoundCommand = [
"if [ \"$cmux_ssh_attach_no_progress_retry\" -gt 0 ]; then :; fi",
"exit 0",
].joined(separator: "\n")
let script = SSHPTYAttachExitCode.retryLoopLines(
command: compoundCommand,
reauthenticates: false
).joined(separator: "\n")
let scriptURL = FileManager.default.temporaryDirectory
.appendingPathComponent("cmux-pty-retry-syntax-\(UUID().uuidString).sh")
defer { try? FileManager.default.removeItem(at: scriptURL) }
try (script + "\n").write(to: scriptURL, atomically: true, encoding: .utf8)

let process = Process()
let stderrPipe = Pipe()
process.executableURL = URL(fileURLWithPath: "/bin/sh")
process.arguments = ["-n", scriptURL.path]
process.standardError = stderrPipe
try process.run()
process.waitUntilExit()
let stderr = String(
data: stderrPipe.fileHandleForReading.readDataToEndOfFile(),
encoding: .utf8
) ?? ""
#expect(process.terminationStatus == 0, "\(stderr)\n\(script)")
}

@Test("lifecycle codes retain precedence over transient-looking messages")
func lifecycleCodesRetainPrecedence() {
#expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,47 @@ struct SSHPTYAttachRetryScriptBuilderTests {
#expect(try String(contentsOf: logURL, encoding: .utf8) == "attach\nsleep:2\nattach\n")
}

// Regression for #9443: the loop used to prefix the attach command with env
// assignments, which POSIX only allows before a simple command, so a compound
// attach command made the generated cmux-ssh-startup script fail with
// "syntax error near unexpected token `then'".
@Test func retryLoopIsValidPOSIXShellForCompoundAttachCommands() throws {
let logURL = FileManager.default.temporaryDirectory
.appendingPathComponent("cmux-ssh-attach-compound-\(UUID().uuidString)")
let scriptURL = FileManager.default.temporaryDirectory
.appendingPathComponent("cmux-ssh-attach-compound-\(UUID().uuidString).sh")
defer {
try? FileManager.default.removeItem(at: logURL)
try? FileManager.default.removeItem(at: scriptURL)
}

let compoundCommand = [
"if [ \"$cmux_ssh_attach_no_progress_retry\" -gt 0 ]; then : || exit 1; fi",
"printf '%s/%s/%s\\n' \"$CMUX_SSH_PTY_ATTACH_WRAPPER_CAN_RETRY\" \"$CMUX_SSH_PTY_ATTACH_NO_PROGRESS_RETRY\" \"$CMUX_SSH_PTY_ATTACH_NO_PROGRESS_LIMIT\" >> \"$CMUX_TEST_LOG\"",
"exit 0",
].joined(separator: "\n")
let script = SSHPTYAttachRetryScriptBuilder().lines(
command: compoundCommand,
reauthenticates: false
).joined(separator: "\n")
try (script + "\n").write(to: scriptURL, atomically: true, encoding: .utf8)

let syntaxCheck = try run("/bin/sh -n '\(scriptURL.path)'", environment: [:])
#expect(syntaxCheck.status == 0, "\(syntaxCheck.stderr)\n\(script)")

// The loop must still hand the retry budget to the command it runs.
let execution = try run(
script,
environment: [
"CMUX_TEST_LOG": logURL.path,
"CMUX_SSH_RECONNECT_LIMIT": "",
"CMUX_SSH_PTY_NO_PROGRESS_RETRY_LIMIT": "3",
]
)
#expect(execution.status == 0, "\(execution.stderr)")
#expect(try String(contentsOf: logURL, encoding: .utf8) == "1/0/3\n")
}

@Test func unclassifiedReauthenticationFailsClosedAfterEstablishedSession() throws {
let logURL = FileManager.default.temporaryDirectory
.appendingPathComponent("cmux-ssh-attach-unclassified-reauth-\(UUID().uuidString)")
Expand Down