From 096d4369e59b3db7ace2db3ca42588c41b9b6019 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 10 Mar 2026 22:15:05 +0100 Subject: [PATCH] fix(sentinel): add token validation to prevent command injection Add validation to ensure sentinel tokens contain only safe characters (alphanumeric, dots, hyphens, underscores, plus, forward slash, equals), preventing OS command injection vulnerabilities when tokens are interpolated into shell commands. - Add ServerSetting::isValidSentinelToken() validation method - Validate tokens in StartSentinel action and metrics queries - Improve shell argument escaping with escapeshellarg() - Add comprehensive test coverage for token validation --- app/Actions/Server/StartSentinel.php | 6 +- app/Livewire/Server/Sentinel.php | 2 +- app/Models/ServerSetting.php | 9 ++ app/Traits/HasMetrics.php | 9 +- tests/Feature/SentinelTokenValidationTest.php | 95 +++++++++++++++++++ 5 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/SentinelTokenValidationTest.php diff --git a/app/Actions/Server/StartSentinel.php b/app/Actions/Server/StartSentinel.php index 1f248aec1..071f3ec46 100644 --- a/app/Actions/Server/StartSentinel.php +++ b/app/Actions/Server/StartSentinel.php @@ -4,6 +4,7 @@ use App\Events\SentinelRestarted; use App\Models\Server; +use App\Models\ServerSetting; use Lorisleiva\Actions\Concerns\AsAction; class StartSentinel @@ -23,6 +24,9 @@ public function handle(Server $server, bool $restart = false, ?string $latestVer $refreshRate = data_get($server, 'settings.sentinel_metrics_refresh_rate_seconds'); $pushInterval = data_get($server, 'settings.sentinel_push_interval_seconds'); $token = data_get($server, 'settings.sentinel_token'); + if (! ServerSetting::isValidSentinelToken($token)) { + throw new \RuntimeException('Invalid sentinel token format. Token must contain only alphanumeric characters, dots, hyphens, and underscores.'); + } $endpoint = data_get($server, 'settings.sentinel_custom_url'); $debug = data_get($server, 'settings.is_sentinel_debug_enabled'); $mountDir = '/data/coolify/sentinel'; @@ -49,7 +53,7 @@ public function handle(Server $server, bool $restart = false, ?string $latestVer } $mountDir = '/var/lib/docker/volumes/coolify_dev_coolify_data/_data/sentinel'; } - $dockerEnvironments = '-e "'.implode('" -e "', array_map(fn ($key, $value) => "$key=$value", array_keys($environments), $environments)).'"'; + $dockerEnvironments = implode(' ', array_map(fn ($key, $value) => '-e '.escapeshellarg("$key=$value"), array_keys($environments), $environments)); $dockerLabels = implode(' ', array_map(fn ($key, $value) => "$key=$value", array_keys($labels), $labels)); $dockerCommand = "docker run -d $dockerEnvironments --name coolify-sentinel -v /var/run/docker.sock:/var/run/docker.sock -v $mountDir:/app/db --pid host --health-cmd \"curl --fail http://127.0.0.1:8888/api/health || exit 1\" --health-interval 10s --health-retries 3 --add-host=host.docker.internal:host-gateway --label $dockerLabels $image"; diff --git a/app/Livewire/Server/Sentinel.php b/app/Livewire/Server/Sentinel.php index cdcdc71fc..dff379ae1 100644 --- a/app/Livewire/Server/Sentinel.php +++ b/app/Livewire/Server/Sentinel.php @@ -19,7 +19,7 @@ class Sentinel extends Component public bool $isMetricsEnabled; - #[Validate(['required'])] + #[Validate(['required', 'string', 'max:500', 'regex:/\A[a-zA-Z0-9._\-+=\/]+\z/'])] public string $sentinelToken; public ?string $sentinelUpdatedAt = null; diff --git a/app/Models/ServerSetting.php b/app/Models/ServerSetting.php index 0ad0fcf84..504cfa60a 100644 --- a/app/Models/ServerSetting.php +++ b/app/Models/ServerSetting.php @@ -92,6 +92,15 @@ protected static function booted() }); } + /** + * Validate that a sentinel token contains only safe characters. + * Prevents OS command injection when the token is interpolated into shell commands. + */ + public static function isValidSentinelToken(string $token): bool + { + return (bool) preg_match('/\A[a-zA-Z0-9._\-+=\/]+\z/', $token); + } + public function generateSentinelToken(bool $save = true, bool $ignoreEvent = false) { $data = [ diff --git a/app/Traits/HasMetrics.php b/app/Traits/HasMetrics.php index 667d58441..7ed82cc91 100644 --- a/app/Traits/HasMetrics.php +++ b/app/Traits/HasMetrics.php @@ -2,6 +2,8 @@ namespace App\Traits; +use App\Models\ServerSetting; + trait HasMetrics { public function getCpuMetrics(int $mins = 5): ?array @@ -26,8 +28,13 @@ private function getMetrics(string $type, int $mins, string $valueField): ?array $from = now()->subMinutes($mins)->toIso8601ZuluString(); $endpoint = $this->getMetricsEndpoint($type, $from); + $token = $server->settings->sentinel_token; + if (! ServerSetting::isValidSentinelToken($token)) { + throw new \Exception('Invalid sentinel token format. Please regenerate the token.'); + } + $response = instant_remote_process( - ["docker exec coolify-sentinel sh -c 'curl -H \"Authorization: Bearer {$server->settings->sentinel_token}\" {$endpoint}'"], + ["docker exec coolify-sentinel sh -c 'curl -H \"Authorization: Bearer {$token}\" {$endpoint}'"], $server, false ); diff --git a/tests/Feature/SentinelTokenValidationTest.php b/tests/Feature/SentinelTokenValidationTest.php new file mode 100644 index 000000000..43048fcaa --- /dev/null +++ b/tests/Feature/SentinelTokenValidationTest.php @@ -0,0 +1,95 @@ +create(); + $this->team = $user->teams()->first(); + + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + ]); +}); + +describe('ServerSetting::isValidSentinelToken', function () { + it('accepts alphanumeric tokens', function () { + expect(ServerSetting::isValidSentinelToken('abc123'))->toBeTrue(); + }); + + it('accepts tokens with dots, hyphens, and underscores', function () { + expect(ServerSetting::isValidSentinelToken('my-token_v2.0'))->toBeTrue(); + }); + + it('accepts long base64-like encrypted tokens', function () { + $token = 'eyJpdiI6IjRGN0V4YnRkZ1p0UXdBPT0iLCJ2YWx1ZSI6IjZqQT0iLCJtYWMiOiIxMjM0NTY3ODkwIn0'; + expect(ServerSetting::isValidSentinelToken($token))->toBeTrue(); + }); + + it('accepts tokens with base64 characters (+, /, =)', function () { + expect(ServerSetting::isValidSentinelToken('abc+def/ghi='))->toBeTrue(); + }); + + it('rejects tokens with double quotes', function () { + expect(ServerSetting::isValidSentinelToken('abc" ; id ; echo "'))->toBeFalse(); + }); + + it('rejects tokens with single quotes', function () { + expect(ServerSetting::isValidSentinelToken("abc' ; id ; echo '"))->toBeFalse(); + }); + + it('rejects tokens with semicolons', function () { + expect(ServerSetting::isValidSentinelToken('abc;id'))->toBeFalse(); + }); + + it('rejects tokens with backticks', function () { + expect(ServerSetting::isValidSentinelToken('abc`id`'))->toBeFalse(); + }); + + it('rejects tokens with dollar sign command substitution', function () { + expect(ServerSetting::isValidSentinelToken('abc$(whoami)'))->toBeFalse(); + }); + + it('rejects tokens with spaces', function () { + expect(ServerSetting::isValidSentinelToken('abc def'))->toBeFalse(); + }); + + it('rejects tokens with newlines', function () { + expect(ServerSetting::isValidSentinelToken("abc\nid"))->toBeFalse(); + }); + + it('rejects tokens with pipe operator', function () { + expect(ServerSetting::isValidSentinelToken('abc|id'))->toBeFalse(); + }); + + it('rejects tokens with ampersand', function () { + expect(ServerSetting::isValidSentinelToken('abc&&id'))->toBeFalse(); + }); + + it('rejects tokens with redirection operators', function () { + expect(ServerSetting::isValidSentinelToken('abc>/tmp/pwn'))->toBeFalse(); + }); + + it('rejects empty strings', function () { + expect(ServerSetting::isValidSentinelToken(''))->toBeFalse(); + }); + + it('rejects the reported PoC payload', function () { + expect(ServerSetting::isValidSentinelToken('abc" ; id >/tmp/coolify_poc_sentinel ; echo "'))->toBeFalse(); + }); +}); + +describe('generated sentinel tokens are valid', function () { + it('generates tokens that pass format validation', function () { + $settings = $this->server->settings; + $settings->generateSentinelToken(save: false, ignoreEvent: true); + $token = $settings->sentinel_token; + + expect($token)->not->toBeEmpty(); + expect(ServerSetting::isValidSentinelToken($token))->toBeTrue(); + }); +});