diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachExitCode.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachExitCode.swift index cc0465cc1fe1..f87bd443d1cd 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachExitCode.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachExitCode.swift @@ -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. @@ -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) ;;", diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachRetryScriptBuilder.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachRetryScriptBuilder.swift index c6120a9c6657..aec92183dc4b 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachRetryScriptBuilder.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachRetryScriptBuilder.swift @@ -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. @@ -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) ;;", diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachExitCodeTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachExitCodeTests.swift index 74ea3c5033b7..17133705cf1b 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachExitCodeTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachExitCodeTests.swift @@ -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( diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachRetryScriptBuilderTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachRetryScriptBuilderTests.swift index bb3ca1df9c6c..907153e6ba5b 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachRetryScriptBuilderTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachRetryScriptBuilderTests.swift @@ -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)")