mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(sentinel): pass the traffic API token to curl on stdin
The token is no longer a process argument that other users on the server can read in /proc. Also correct the docker login comment about where the registry password is visible. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
f3af1c9197
commit
1501a44cfe
@@ -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
|
||||
{
|
||||
|
||||
@@ -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<int, string> $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
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
<?php
|
||||
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use App\Services\SentinelTrafficClient;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\File;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Symfony\Component\Process\Process as SymfonyProcess;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
/**
|
||||
* Process arguments are readable by every user on the server (/proc/<pid>/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<int, string> $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] <container> <command...>` 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<int, string>, stdin: array<int, string>}
|
||||
*/
|
||||
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']);
|
||||
Reference in New Issue
Block a user