diff --git a/app/Services/DockerRegistryLogins.php b/app/Services/DockerRegistryLogins.php index 041aaaabd9..6b95d56393 100644 --- a/app/Services/DockerRegistryLogins.php +++ b/app/Services/DockerRegistryLogins.php @@ -210,8 +210,10 @@ class DockerRegistryLogins /** * Run docker login on the server, writing to the same config file that deployments mount. * - * The secret goes to docker through stdin via the shell builtin echo, so it is never a process argument - * on the server, and it is kept out of SSH error logs and exception messages. + * The SSH wrapper sends this script to the server's shell on stdin, and the shell builtin echo passes the + * secret to docker on stdin, so it is not a process argument on the server. On the Coolify host it is still + * in the command line of the local shell that runs ssh, because SshMultiplexingHelper::generateSshCommand() + * embeds the script in a heredoc. It is kept out of SSH error logs and exception messages. */ public static function login(Server $server, string $registry, string $username, string $password): void { diff --git a/app/Services/SentinelTrafficClient.php b/app/Services/SentinelTrafficClient.php index 259a54d00d..b7f4a7733e 100644 --- a/app/Services/SentinelTrafficClient.php +++ b/app/Services/SentinelTrafficClient.php @@ -769,33 +769,41 @@ class SentinelTrafficClient /** * Build the `docker exec ... curl` command run inside the Sentinel container. * - * The URL is double-quoted inside the inner `sh -c` string so the literal `&` - * between the `from`/`to` (and `limit`) query params is not interpreted as a - * shell background operator — which would background curl after `from=...` and - * truncate every multi-param request. The app key and dimension are validated - * (assertSafeKey/assertSafeDimension) before reaching here, so the URL cannot - * contain shell metacharacters that break out of the quoting. + * The token reaches curl on stdin (`-H @-`) from a heredoc, so it is never a process argument + * that other users on the server can read in /proc. The URL is double-quoted so the literal `&` + * between the `from`/`to` (and `limit`) query params is not a shell background operator. The + * app key and dimension are validated (assertSafeKey/assertSafeDimension) before reaching here, + * so the URL cannot contain shell metacharacters that break out of the quoting. */ protected function buildFetchCommand(string $token, string $url): string { - return "docker exec coolify-sentinel sh -c 'curl -sS -H \"Authorization: Bearer {$token}\" \"{$url}\"'"; + return "docker exec -i coolify-sentinel curl -sS -H @- \"{$url}\"".$this->authorizationHeredoc($token); } /** * Build one `docker exec` that curls every URL in order and separates the responses - * with a 0x1E record separator, so warm() can split them back apart. escapeshellarg - * safely wraps the whole script; each URL stays double-quoted so its `&` is literal. + * with a 0x1E record separator, so warm() can split them back apart. The container shell + * reads the script, which holds the token, from stdin; each curl reads the header from stdin. + * No pipe or `sh -c` is used, so the non-root sudo parser only puts sudo in front of the line. * * @param array $urls */ protected function buildBatchCommand(string $token, array $urls): string { - $script = implode(' ; ', array_map( - fn ($url) => "curl -s -H \"Authorization: Bearer {$token}\" \"{$url}\" ; printf '\\036'", + $script = implode("\n", array_map( + fn ($url) => "curl -s -H @- \"{$url}\"".$this->authorizationHeredoc($token)."\nprintf '\\036'", $urls )); - return 'docker exec coolify-sentinel sh -c '.escapeshellarg($script); + return "docker exec -i coolify-sentinel sh -s <<'COOLIFY_SENTINEL_SCRIPT'\n{$script}\nCOOLIFY_SENTINEL_SCRIPT"; + } + + /** + * The token only has letters, digits and `._-+/=` (ServerSetting::isValidSentinelToken), so it cannot end the heredoc. + */ + private function authorizationHeredoc(string $token): string + { + return " <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH"; } private function guard(string $response): string diff --git a/tests/Feature/TrafficAnalytics/SentinelTrafficTokenStdinTest.php b/tests/Feature/TrafficAnalytics/SentinelTrafficTokenStdinTest.php new file mode 100644 index 0000000000..a804036682 --- /dev/null +++ b/tests/Feature/TrafficAnalytics/SentinelTrafficTokenStdinTest.php @@ -0,0 +1,122 @@ +/cmdline). The Sentinel + * token must reach curl over stdin, never as an argument of docker, the container shell, or curl. + */ +beforeEach(function () { + Server::flushIdentityMap(); + config(['constants.ssh.mux_enabled' => false, 'constants.ssh.max_retries' => 1]); + Process::fake(fn () => Process::result(output: '{"requests":1}')); + + $team = Team::factory()->create(); + $this->server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + + $this->client = new class($this->server) extends SentinelTrafficClient + { + public function fetch(string $url): string + { + return $this->remoteFetch($url); + } + + /** + * @param array $urls + */ + public function batch(array $urls): string + { + return $this->batchRemoteFetch($urls); + } + }; + + $this->bin = sys_get_temp_dir().'/sentinel-token-stdin-'.uniqid(); + File::makeDirectory($this->bin); + $log = "{$this->bin}/calls.log"; + // Each fake logs its arguments; curl also logs what it reads on stdin (the -H @- headers). + File::put("{$this->bin}/sudo", "#!/bin/sh\necho \"argv: sudo \$*\" >> {$log}\nexec \"\$@\"\n"); + // `docker exec [options] ` runs the command here. + File::put("{$this->bin}/docker", "#!/bin/sh\necho \"argv: docker \$*\" >> {$log}\nshift\nwhile [ \"\${1#-}\" != \"\$1\" ]; do shift; done\nshift\nexec \"\$@\"\n"); + File::put("{$this->bin}/curl", "#!/bin/sh\necho \"argv: curl \$*\" >> {$log}\necho \"stdin: \$(cat)\" >> {$log}\nprintf '{}'\n"); + File::put("{$this->bin}/sh", "#!/bin/sh\necho \"argv: sh \$*\" >> {$log}\nexec /bin/sh \"\$@\"\n"); + foreach (['sudo', 'docker', 'curl', 'sh'] as $fake) { + chmod("{$this->bin}/{$fake}", 0755); + } +}); + +afterEach(function () { + File::deleteDirectory($this->bin); +}); + +/** + * Runs the script that Coolify sent over SSH in a real shell with the fake binaries first on PATH. + * + * @return array{argv: array, stdin: array} + */ +function runSentSentinelScriptWithFakes(string $bin): array +{ + $script = null; + Process::assertRan(function ($process) use (&$script) { + // generateSshCommand sends the script as the body of a heredoc: first line opens it, last line closes it. + $lines = explode("\n", $process->command); + $script = implode("\n", array_slice($lines, 1, -1)); + + return true; + }); + + $run = new SymfonyProcess(['/bin/sh', '-s'], env: ['PATH' => "{$bin}:/usr/bin:/bin"]); + $run->setInput($script."\n"); + $run->mustRun(); + + $log = collect(file("{$bin}/calls.log", FILE_IGNORE_NEW_LINES)); + + return [ + 'argv' => $log->filter(fn (string $line) => str_starts_with($line, 'argv: '))->values()->all(), + 'stdin' => $log->filter(fn (string $line) => str_starts_with($line, 'stdin: '))->values()->all(), + ]; +} + +it('sends the sentinel token to curl over stdin for a single fetch', function (string $user) { + $this->server->update(['user' => $user]); + $url = 'http://localhost:8888/api/traffic/overview?from=2026-08-01T00:00:00Z&to=2026-08-02T00:00:00Z'; + + $this->client->fetch($url); + $token = $this->server->settings->fresh()->sentinel_token; + $calls = runSentSentinelScriptWithFakes($this->bin); + + expect($calls['stdin'])->toBe(["stdin: Authorization: Bearer {$token}"]) + ->and(collect($calls['argv'])->filter(fn (string $line) => str_contains($line, $token)))->toBeEmpty() + ->and(collect($calls['argv'])->filter(fn (string $line) => str_starts_with($line, 'argv: curl '))) + ->toHaveCount(1) + ->each->toContain($url); +})->with(['root', 'non-root' => 'cooluser']); + +it('sends the sentinel token to every curl over stdin for a batched fetch', function (string $user) { + $this->server->update(['user' => $user]); + $urls = [ + 'http://localhost:8888/api/traffic/overview?from=F&to=T', + 'http://localhost:8888/api/traffic/apps', + ]; + + $this->client->batch($urls); + $token = $this->server->settings->fresh()->sentinel_token; + $calls = runSentSentinelScriptWithFakes($this->bin); + + expect($calls['stdin'])->toBe([ + "stdin: Authorization: Bearer {$token}", + "stdin: Authorization: Bearer {$token}", + ]) + ->and(collect($calls['argv'])->filter(fn (string $line) => str_contains($line, $token)))->toBeEmpty(); +})->with(['root', 'non-root' => 'cooluser']);