From ebf23f4874ce89a7ac722b7c31cf556dd76273ae Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Tue, 26 May 2026 13:48:10 +0200
Subject: [PATCH] fix(ssh): escape scp source and destination

Quote SCP operands when building commands to prevent shell injection through source or destination paths, and cover the escaping behavior in the SSH command injection tests.
---
 app/Helpers/SshMultiplexingHelper.php  |  4 ++--
 tests/Unit/SshCommandInjectionTest.php | 18 +++++++++++++++---
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/app/Helpers/SshMultiplexingHelper.php b/app/Helpers/SshMultiplexingHelper.php
index cf92deb2a..021ac3608 100644
--- a/app/Helpers/SshMultiplexingHelper.php
+++ b/app/Helpers/SshMultiplexingHelper.php
@@ -63,10 +63,10 @@ class SshMultiplexingHelper
         $scpCommand .= self::getCommonSshOptions($server, $sshKeyLocation, self::getConnectionTimeout($server), config('constants.ssh.server_interval'), isScp: true);
 
         if ($server->isIpv6()) {
-            return $scpCommand."{$source} ".escapeshellarg($server->user).'@['.escapeshellarg($server->ip)."]:{$dest}";
+            return $scpCommand.escapeshellarg($source).' '.escapeshellarg($server->user).'@['.escapeshellarg($server->ip).']:'.escapeshellarg($dest);
         }
 
-        return $scpCommand."{$source} ".self::escapedUserAtHost($server).":{$dest}";
+        return $scpCommand.escapeshellarg($source).' '.self::escapedUserAtHost($server).':'.escapeshellarg($dest);
     }
 
     public static function generateSshCommand(Server $server, string $command, bool $disableMultiplexing = false): string
diff --git a/tests/Unit/SshCommandInjectionTest.php b/tests/Unit/SshCommandInjectionTest.php
index e7eeff62d..e7806b782 100644
--- a/tests/Unit/SshCommandInjectionTest.php
+++ b/tests/Unit/SshCommandInjectionTest.php
@@ -1,6 +1,7 @@
 <?php
 
 use App\Helpers\SshMultiplexingHelper;
+use App\Models\Server;
 use App\Rules\ValidHostname;
 use App\Rules\ValidServerIp;
 
@@ -57,20 +58,20 @@ it('rejects injection payloads in server ip', function (string $payload) {
 // -------------------------------------------------------------------------
 
 it('strips dangerous characters from server ip on write', function () {
-    $server = new App\Models\Server;
+    $server = new Server;
     $server->ip = '192.168.1.1;rm -rf /';
     // Regex [^0-9a-zA-Z.:%-] removes ; space and /; hyphen is allowed
     expect($server->ip)->toBe('192.168.1.1rm-rf');
 });
 
 it('strips dangerous characters from server user on write', function () {
-    $server = new App\Models\Server;
+    $server = new Server;
     $server->user = 'root$(id)';
     expect($server->user)->toBe('rootid');
 });
 
 it('strips non-numeric characters from server port on write', function () {
-    $server = new App\Models\Server;
+    $server = new Server;
     $server->port = '22; evil';
     expect($server->port)->toBe(22);
 });
@@ -102,6 +103,17 @@ it('has no raw user@ip string interpolation in SshMultiplexingHelper', function
     expect($source)->not->toContain('{$server->user}@{$server->ip}');
 });
 
+it('escapes scp source and destination operands', function () {
+    $reflection = new ReflectionClass(SshMultiplexingHelper::class);
+    $source = file_get_contents($reflection->getFileName());
+
+    expect($source)
+        ->toContain('escapeshellarg($source)')
+        ->toContain('escapeshellarg($dest)')
+        ->not->toContain('"{$source} "')
+        ->not->toContain('":{$dest}"');
+});
+
 // -------------------------------------------------------------------------
 // ValidHostname rejects shell metacharacters
 // -------------------------------------------------------------------------