Skip to content

Commit 60129c2

Browse files
andrasbacsaiandrasbacsai
committed
fix: measure log size before ANSI strip; harden tasks and volumes
Truncate GetLogs against raw byte length, then strip ANSI colors. Run scheduled-task docker exec with explicit sudo docker and no_sudo to avoid double sudo rewriting on non-root servers. Mark git-based file volumes as files before refreshing content from the server.
1 parent 2749f45 commit 60129c2

6 files changed

Lines changed: 83 additions & 14 deletions

File tree

‎app/Jobs/ScheduledTaskJob.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,12 @@ public function handle(): void
150150
foreach ($this->containers as $containerName) {
151151
if (count($this->containers) == 1 || str_starts_with($containerName, $this->task->container.'-'.$this->resource->uuid)) {
152152
$cmd = "sh -c '".str_replace("'", "'\''", $this->task->command)."'";
153-
$execCommand = "docker exec {$containerName} {$cmd}";
153+
$dockerCommand = $this->server->isNonRoot() ? 'sudo docker' : 'docker';
154+
$execCommand = "{$dockerCommand} exec {$containerName} {$cmd}";
154155
$exec = $this->boundedTaskCommand($execCommand);
155156
// Disable SSH multiplexing to prevent race conditions when multiple tasks run concurrently
156157
// See: https://github.com/coollabsio/coolify/issues/6736
157-
$this->task_output = instant_remote_process([$exec], $this->server, true, false, $this->timeout, disableMultiplexing: true);
158+
$this->task_output = instant_remote_process([$exec], $this->server, throwError: true, no_sudo: true, timeout: $this->timeout, disableMultiplexing: true);
158159
$this->task_log->update([
159160
'status' => 'success',
160161
'message' => $this->task_output,

‎app/Livewire/Project/Shared/GetLogs.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -192,17 +192,17 @@ public function getLogs($refresh = false)
192192
return;
193193
}
194194

195-
$output = removeAnsiColors($output);
196195
$remainingBytes = self::MAX_DISPLAY_SIZE_BYTES - $accumulatedBytes;
197-
if (strlen($output) > $remainingBytes) {
198-
$logChunks[] = substr($output, 0, max(0, $remainingBytes));
196+
$outputBytes = strlen($output);
197+
if ($outputBytes > $remainingBytes) {
198+
$logChunks[] = removeAnsiColors(substr($output, 0, max(0, $remainingBytes)));
199199
$truncated = true;
200200

201201
return;
202202
}
203203

204-
$logChunks[] = $output;
205-
$accumulatedBytes += strlen($output);
204+
$logChunks[] = removeAnsiColors($output);
205+
$accumulatedBytes += $outputBytes;
206206
});
207207
$newOutputs = implode('', $logChunks);
208208

@@ -274,20 +274,19 @@ public function downloadAllLogs(): string
274274
return;
275275
}
276276

277-
$output = removeAnsiColors($output);
278277
$outputBytes = strlen($output);
279278

280279
if ($accumulatedBytes + $outputBytes > self::MAX_DOWNLOAD_SIZE_BYTES) {
281280
$remaining = self::MAX_DOWNLOAD_SIZE_BYTES - $accumulatedBytes;
282281
if ($remaining > 0) {
283-
$logChunks[] = substr($output, 0, $remaining);
282+
$logChunks[] = removeAnsiColors(substr($output, 0, $remaining));
284283
}
285284
$truncated = true;
286285

287286
return;
288287
}
289288

290-
$logChunks[] = $output;
289+
$logChunks[] = removeAnsiColors($output);
291290
$accumulatedBytes += $outputBytes;
292291
});
293292

‎bootstrap/helpers/services.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,10 @@ function getFilesystemVolumesFromServer(ServiceApplication|ServiceDatabase|Appli
167167
$isDir = instant_remote_process(["test -d $fileLocation && echo OK || echo NOK"], $server);
168168

169169
if ($isFile === 'OK') {
170+
$fileVolume->is_directory = false;
171+
$fileVolume->save();
170172
if ($fileVolume->is_based_on_git) {
171173
$fileVolume->loadStorageOnServer();
172-
} else {
173-
$fileVolume->is_directory = false;
174-
$fileVolume->save();
175174
}
176175
} elseif ($isDir === 'OK') {
177176
// If its a directory & exists

‎tests/Feature/GetLogsCommandInjectionTest.php‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@
33
use App\Livewire\Project\Shared\GetLogs;
44
use App\Models\Application;
55
use App\Models\Environment;
6+
use App\Models\PrivateKey;
67
use App\Models\Project;
78
use App\Models\Server;
89
use App\Models\StandaloneDocker;
910
use App\Models\Team;
1011
use App\Models\User;
1112
use App\Support\ValidationPatterns;
1213
use Illuminate\Foundation\Testing\RefreshDatabase;
14+
use Illuminate\Process\FakeProcessResult;
15+
use Illuminate\Support\Facades\Process;
1316
use Livewire\Attributes\Locked;
1417
use Livewire\Livewire;
1518

@@ -20,7 +23,11 @@
2023
$this->team = Team::factory()->create();
2124
$this->user->teams()->attach($this->team, ['role' => 'owner']);
2225

23-
$this->server = Server::factory()->create(['team_id' => $this->team->id]);
26+
$privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]);
27+
$this->server = Server::factory()->create([
28+
'team_id' => $this->team->id,
29+
'private_key_id' => $privateKey->id,
30+
]);
2431
// Server::created auto-creates a StandaloneDocker, reuse it
2532
$this->destination = StandaloneDocker::where('server_id', $this->server->id)->first();
2633
$this->project = Project::factory()->create(['team_id' => $this->team->id]);
@@ -67,6 +74,38 @@
6774
});
6875

6976
describe('GetLogs Livewire action validation', function () {
77+
test('getLogs marks ANSI-colored output truncated based on raw bytes', function () {
78+
$this->server->settings->fill([
79+
'is_reachable' => true,
80+
'is_usable' => true,
81+
'force_disabled' => false,
82+
])->save();
83+
$server = Server::with('settings')->find($this->server->id);
84+
$output = "\e[31m".str_repeat('a', GetLogs::MAX_DISPLAY_SIZE_BYTES - 4);
85+
86+
expect(strlen($output))->toBe(GetLogs::MAX_DISPLAY_SIZE_BYTES + 1);
87+
88+
Process::shouldReceive('timeout')->once()->andReturnSelf();
89+
Process::shouldReceive('run')->andReturnUsing(function (string $command, ?callable $callback = null) use ($output): FakeProcessResult {
90+
if ($callback) {
91+
$callback('out', $output);
92+
}
93+
94+
return new FakeProcessResult(command: $command);
95+
});
96+
97+
$component = new GetLogs;
98+
$component->server = $server;
99+
$component->resource = $this->application;
100+
$component->container = 'test-container';
101+
$component->showTimeStamps = false;
102+
$component->getLogs(true);
103+
104+
expect($component->outputs)
105+
->toContain('[... Output truncated at 5MB limit ...]')
106+
->not->toContain("\e[31m");
107+
});
108+
70109
test('getLogs rejects invalid container name', function () {
71110
// Make server functional by setting settings directly
72111
$this->server->settings->fill([

‎tests/Unit/LocalFileVolumeContentSizeTest.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,14 @@
7777
->toContain('if ($fileVolume->is_based_on_git)')
7878
->toContain('$fileVolume->loadStorageOnServer();');
7979
});
80+
81+
it('marks git-based file volumes as files before refreshing their content', function () {
82+
$helpers = file_get_contents(base_path('bootstrap/helpers/services.php'));
83+
$fileBranch = str($helpers)
84+
->after("if (\$isFile === 'OK') {")
85+
->before("} elseif (\$isDir === 'OK') {");
86+
87+
expect($fileBranch->value())->toMatch(
88+
'/\$fileVolume->is_directory = false;\s+\$fileVolume->save\(\);\s+if \(\$fileVolume->is_based_on_git\) \{/'
89+
);
90+
});

‎tests/Unit/RemoteOutputSizeLimitsTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Livewire\Project\Shared\GetLogs;
66
use App\Livewire\Server\Proxy\DynamicConfigurations;
77
use App\Models\Application;
8+
use App\Models\Server;
89

910
function remoteOutputSource(string $path): string
1011
{
@@ -77,6 +78,25 @@ function remoteOutputSource(string $path): string
7778
->and(implode("\n", $failureOutput))->toBe('failure');
7879
});
7980

81+
it('does not pass the scheduled task output wrapper through the sudo rewriter', function () {
82+
$source = remoteOutputSource('app/Jobs/ScheduledTaskJob.php');
83+
$reflection = new ReflectionClass(ScheduledTaskJob::class);
84+
$method = $reflection->getMethod('boundedTaskCommand');
85+
$command = $method->invoke($reflection->newInstanceWithoutConstructor(), 'sudo docker exec example true');
86+
$server = Mockery::mock(Server::class)->makePartial();
87+
$server->shouldReceive('getAttribute')->with('user')->andReturn('ubuntu');
88+
$rewrittenCommand = parseCommandsByLineForSudo(collect([$command]), $server)[0];
89+
90+
exec('bash -n -c '.escapeshellarg($command), $output, $exitCode);
91+
exec('bash -n -c '.escapeshellarg($rewrittenCommand).' 2>/dev/null', $rewrittenOutput, $rewrittenExitCode);
92+
93+
expect($source)
94+
->toContain("\$dockerCommand = \$this->server->isNonRoot() ? 'sudo docker' : 'docker'")
95+
->toContain('instant_remote_process([$exec], $this->server, throwError: true, no_sudo: true')
96+
->and($exitCode)->toBe(0)
97+
->and($rewrittenExitCode)->not->toBe(0);
98+
});
99+
80100
it('bounds proxy configuration backfill before storing it', function () {
81101
$source = remoteOutputSource('app/Actions/Proxy/GetProxyConfiguration.php');
82102

0 commit comments

Comments
 (0)