From 419106696a44864dbba29ecf3299e4b05ef1ac37 Mon Sep 17 00:00:00 2001 From: Himanshu Ahuja Date: Mon, 10 Aug 2026 18:24:16 -0400 Subject: [PATCH] fix(backups): forward per-backup timeout to the ssh timeout wrapper instant_remote_process() applied its timeout only to Process::timeout(), while the shell-level timeout wrapper around ssh always used the global 3600s default, killing long backups and scheduled tasks at 1h with ssh exit code 255. Also applies the backup timeout to the S3 upload step and makes the default configurable via SSH_COMMAND_TIMEOUT. Co-Authored-By: Claude Fable 5 --- app/Jobs/DatabaseBackupJob.php | 2 +- bootstrap/helpers/remoteProcess.php | 2 +- config/constants.php | 2 +- tests/Feature/RemoteProcessTimeoutTest.php | 103 +++++++++++++++++++++ 4 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/RemoteProcessTimeoutTest.php diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index 104a84a1b..d432a3eeb 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -785,7 +785,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc alias set{$resolveOptions} temporary {$escapedEndpoint} {$escapedKey} {$escapedSecret}"; $commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp {$escapedBackupLocation} {$escapedS3Destination}"; - instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true); + instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true); $this->s3_uploaded = true; } catch (Throwable $e) { diff --git a/bootstrap/helpers/remoteProcess.php b/bootstrap/helpers/remoteProcess.php index 8d1a30a62..982dda551 100644 --- a/bootstrap/helpers/remoteProcess.php +++ b/bootstrap/helpers/remoteProcess.php @@ -177,7 +177,7 @@ function instant_remote_process(Collection|array $command, Server $server, bool return SshRetryHandler::retry( function () use ($server, $command_string, $effectiveTimeout, $disableMultiplexing) { - $sshCommand = SshMultiplexingHelper::generateSshCommand($server, $command_string, $disableMultiplexing); + $sshCommand = SshMultiplexingHelper::generateSshCommand($server, $command_string, $disableMultiplexing, (int) $effectiveTimeout); $process = Process::timeout($effectiveTimeout)->run($sshCommand); $output = trim($process->output()); diff --git a/config/constants.php b/config/constants.php index aa1b5c36c..fc15a6899 100644 --- a/config/constants.php +++ b/config/constants.php @@ -77,7 +77,7 @@ return [ 'mux_orphan_reap_enabled' => env('SSH_MUX_ORPHAN_REAP_ENABLED', false), // false = dry-run, only log orphans 'connection_timeout' => 10, 'server_interval' => 20, - 'command_timeout' => 3600, + 'command_timeout' => env('SSH_COMMAND_TIMEOUT', 3600), 'max_retries' => env('SSH_MAX_RETRIES', 3), 'retry_base_delay' => env('SSH_RETRY_BASE_DELAY', 2), // seconds 'retry_max_delay' => env('SSH_RETRY_MAX_DELAY', 30), // seconds diff --git a/tests/Feature/RemoteProcessTimeoutTest.php b/tests/Feature/RemoteProcessTimeoutTest.php new file mode 100644 index 000000000..9bc8e12ae --- /dev/null +++ b/tests/Feature/RemoteProcessTimeoutTest.php @@ -0,0 +1,103 @@ +create(); + $team = $user->teams()->first(); + + $privateKeyContent = '-----BEGIN OPENSSH PRIVATE KEY----- +'. + 'b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +'. + 'QyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevAAAAJi/QySHv0Mk +'. + 'hwAAAAtzc2gtZWQyNTUxOQAAACBbhpqHhqv6aI67Mj9abM3DVbmcfYhZAhC7ca4d9UCevA +'. + 'AAAECBQw4jg1WRT2IGHMncCiZhURCts2s24HoDS0thHnnRKVuGmoeGq/pojrsyP1pszcNV +'. + 'uZx9iFkCELtxrh31QJ68AAAAEXNhaWxANzZmZjY2ZDJlMmRkAQIDBA== +'. + '-----END OPENSSH PRIVATE KEY-----'; + + $privateKey = PrivateKey::create([ + 'name' => 'timeout-test-key-'.uniqid(), + 'private_key' => $privateKeyContent, + 'team_id' => $team->id, + ]); + + Storage::fake('ssh-keys'); + Storage::disk('ssh-keys')->put("ssh_key@{$privateKey->uuid}", $privateKeyContent); + + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => $privateKey->id, + ]); + + Storage::disk('ssh-keys')->put("ssh_key@{$server->privateKey->uuid}", $server->privateKey->private_key); + + return $server; +} + +it('wraps ssh commands with an explicitly passed command timeout', function () { + config(['constants.ssh.mux_enabled' => false]); + $server = makeTimeoutTestServer(); + + $command = SshMultiplexingHelper::generateSshCommand($server, 'echo ok', commandTimeout: 7200); + + expect($command)->toStartWith('timeout 7200 ssh '); +}); + +it('wraps ssh commands with the configured default timeout when none is passed', function () { + config([ + 'constants.ssh.mux_enabled' => false, + 'constants.ssh.command_timeout' => 1234, + ]); + $server = makeTimeoutTestServer(); + + $command = SshMultiplexingHelper::generateSshCommand($server, 'echo ok'); + + expect($command)->toStartWith('timeout 1234 ssh '); +}); + +it('forwards the per-call timeout of instant_remote_process to the ssh timeout wrapper', function () { + config(['constants.ssh.mux_enabled' => false]); + $server = makeTimeoutTestServer(); + + Process::fake(); + + instant_remote_process(['echo ok'], $server, timeout: 7200, disableMultiplexing: true); + + Process::assertRan(fn ($process) => str_starts_with($process->command, 'timeout 7200 ssh ')); +}); + +it('uses the configured default timeout in instant_remote_process when no timeout is passed', function () { + config([ + 'constants.ssh.mux_enabled' => false, + 'constants.ssh.command_timeout' => 1234, + ]); + $server = makeTimeoutTestServer(); + + Process::fake(); + + instant_remote_process(['echo ok'], $server, disableMultiplexing: true); + + Process::assertRan(fn ($process) => str_starts_with($process->command, 'timeout 1234 ssh ')); +});