From ff5cfd4253b34f463564943673a09ee9cdcab5f1 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:58:12 +0200 Subject: [PATCH] fix(api): normalize log endpoint query handling Clamp log line counts, parse timestamp flags consistently, and filter service subcontainers by Coolify labels. Document log endpoint timestamp parameters and database/service log routes in OpenAPI. --- .../Api/ApplicationsController.php | 4 +- .../Controllers/Api/DatabasesController.php | 7 +- .../Controllers/Api/ServicesController.php | 10 +- bootstrap/helpers/docker.php | 42 ++++- openapi.json | 168 ++++++++++++++++++ openapi.yaml | 118 ++++++++++++ tests/Unit/Api/LogEndpointHelpersTest.php | 87 +++++++++ 7 files changed, 417 insertions(+), 19 deletions(-) create mode 100644 tests/Unit/Api/LogEndpointHelpersTest.php diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index ddfd15ef7..127130530 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -2087,8 +2087,8 @@ public function logs_by_uuid(Request $request) ], 400); } - $lines = $request->query->get('lines', 100); - $showTimestamps = $request->query->get('show_timestamps', false); + $lines = normalizeLogLines($request->query('lines')); + $showTimestamps = parseLogTimestampFlag($request->query('show_timestamps')); $logs = getContainerLogs($application->destination->server, $container['ID'], $lines, $showTimestamps); return response()->json([ diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index e68b18fbe..a9f12006f 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -2246,6 +2246,7 @@ public function create_database(Request $request, NewDatabaseTypes $type) return response()->json(['message' => 'Invalid database type requested.'], 400); } + #[OA\Get( summary: 'Get database logs.', description: 'Get database logs by UUID.', @@ -2331,7 +2332,7 @@ public function logs_by_uuid(Request $request) } $containers = getCurrentDatabaseContainerStatus($database->destination->server, $database->id); - + if ($containers->count() == 0) { return response()->json([ 'message' => 'Database is not running.', @@ -2347,8 +2348,8 @@ public function logs_by_uuid(Request $request) ], 400); } - $lines = $request->query->get('lines', 100); - $showTimestamps = $request->query->get('show_timestamps', false); + $lines = normalizeLogLines($request->query('lines')); + $showTimestamps = parseLogTimestampFlag($request->query('show_timestamps')); $logs = getContainerLogs($database->destination->server, $container['ID'], $lines, $showTimestamps); return response()->json([ diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 19867384d..3b37e73b9 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -733,7 +733,7 @@ public function service_by_uuid(Request $request) #[OA\Get( summary: 'Get service logs.', - description: 'Get service logs by UUID.', + description: 'Get logs for a specific service sub-resource by service UUID. The `sub_service_name` query parameter must match the `name` field of one of the service applications or databases returned by `GET /services/{uuid}`.', path: '/services/{uuid}/logs', operationId: 'get-service-logs-by-uuid', security: [ @@ -754,9 +754,9 @@ public function service_by_uuid(Request $request) new OA\Parameter( name: 'sub_service_name', in: 'query', - description: 'Sub service name.', + description: 'Sub-service name from `GET /services/{uuid}` under `applications[].name` or `databases[].name`. Do not use `human_name` or the Docker container name with the service UUID suffix.', required: true, - schema: new OA\Schema(type: 'string'), + schema: new OA\Schema(type: 'string', example: 'appwrite-console'), ), new OA\Parameter( name: 'lines', @@ -841,8 +841,8 @@ public function logs_by_uuid(Request $request) ], 400); } - $lines = $request->query->get('lines', 100); - $showTimestamps = $request->query->get('show_timestamps', false); + $lines = normalizeLogLines($request->query('lines')); + $showTimestamps = parseLogTimestampFlag($request->query('show_timestamps')); $logs = getContainerLogs($service->destination->server, $container['ID'], $lines, $showTimestamps); return response()->json([ diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 105bcbacb..e6643a337 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -87,15 +87,19 @@ function getCurrentDatabaseContainerStatus(Server $server, int $id): Collection function getCurrentServiceSubContainerStatus(Server $server, int $id, string $name): Collection { - $containers = collect([]); - if (! $server->isSwarm()) { - $containers = instant_remote_process(["docker ps -a --filter='label=coolify.serviceId={$id}' --filter='label=coolify.name={$name}' --format '{{json .}}' "], $server); - $containers = format_docker_command_output_to_json($containers); + return filterServiceSubContainersByName(getCurrentServiceContainerStatus($server, $id), $name); +} - return $containers->filter(); - } +function filterServiceSubContainersByName(Collection $containers, string $name): Collection +{ + return $containers->filter(function ($container) use ($name) { + $labels = data_get($container, 'Labels', []); + if (is_string($labels)) { + $labels = format_docker_labels_to_json($labels); + } - return $containers; + return collect($labels)->get('coolify.name') === $name; + })->values(); } function format_docker_command_output_to_json($rawOutput): Collection @@ -1273,7 +1277,22 @@ function validateComposeFile(string $compose, int $server_id): string|Throwable } } -function getContainerLogs(Server $server, string $container_id, int $lines = 100, bool $showTimestamps = false): string +function normalizeLogLines(mixed $lines, int $default = 100, int $max = 10000): int +{ + $lines = filter_var($lines, FILTER_VALIDATE_INT); + if ($lines === false || $lines <= 0) { + return $default; + } + + return min($lines, $max); +} + +function parseLogTimestampFlag(mixed $showTimestamps): bool +{ + return filter_var($showTimestamps, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; +} + +function buildContainerLogsCommand(Server $server, string $container_id, int $lines = 100, bool $showTimestamps = false): string { $command = "docker logs -n {$lines}"; if ($server->isSwarm()) { @@ -1284,7 +1303,12 @@ function getContainerLogs(Server $server, string $container_id, int $lines = 100 $command .= ' --timestamps'; } - $output = instant_remote_process(["{$command} {$container_id} 2>&1"], $server); + return "{$command} ".escapeshellarg($container_id).' 2>&1'; +} + +function getContainerLogs(Server $server, string $container_id, int $lines = 100, bool $showTimestamps = false): string +{ + $output = instant_remote_process([buildContainerLogsCommand($server, $container_id, $lines, $showTimestamps)], $server); $output = removeAnsiColors($output); return $output; diff --git a/openapi.json b/openapi.json index ca445ade0..816cdb535 100644 --- a/openapi.json +++ b/openapi.json @@ -2675,6 +2675,16 @@ "format": "int32", "default": 100 } + }, + { + "name": "show_timestamps", + "in": "query", + "description": "Show timestamps in the logs.", + "required": false, + "schema": { + "type": "boolean", + "default": false + } } ], "responses": { @@ -5952,6 +5962,80 @@ ] } }, + "\/databases\/{uuid}\/logs": { + "get": { + "tags": [ + "Databases" + ], + "summary": "Get database logs.", + "description": "Get database logs by UUID.", + "operationId": "get-database-logs-by-uuid", + "parameters": [ + { + "name": "uuid", + "in": "path", + "description": "UUID of the database.", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + }, + { + "name": "lines", + "in": "query", + "description": "Number of lines to show from the end of the logs.", + "required": false, + "schema": { + "type": "integer", + "format": "int32", + "default": 100 + } + }, + { + "name": "show_timestamps", + "in": "query", + "description": "Show timestamps in the logs.", + "required": false, + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "200": { + "description": "Get database logs by UUID.", + "content": { + "application\/json": { + "schema": { + "properties": { + "logs": { + "type": "string" + } + }, + "type": "object" + } + } + } + }, + "401": { + "$ref": "#\/components\/responses\/401" + }, + "400": { + "$ref": "#\/components\/responses\/400" + }, + "404": { + "$ref": "#\/components\/responses\/404" + } + }, + "security": [ + { + "bearerAuth": [] + } + ] + } + }, "\/databases\/{uuid}\/backups\/{scheduled_backup_uuid}\/executions\/{execution_uuid}": { "delete": { "tags": [ @@ -11293,6 +11377,90 @@ ] } }, + "\/services\/{uuid}\/logs": { + "get": { + "tags": [ + "Services" + ], + "summary": "Get service logs.", + "description": "Get logs for a specific service sub-resource by service UUID. The `sub_service_name` query parameter must match the `name` field of one of the service applications or databases returned by `GET \/services\/{uuid}`.", + "operationId": "get-service-logs-by-uuid", + "parameters": [ + { + "name": "uuid", + "in": "path", + "description": "UUID of the service.", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + }, + { + "name": "sub_service_name", + "in": "query", + "description": "Sub-service name from `GET \/services\/{uuid}` under `applications[].name` or `databases[].name`. Do not use `human_name` or the Docker container name with the service UUID suffix.", + "required": true, + "schema": { + "type": "string", + "example": "appwrite-console" + } + }, + { + "name": "lines", + "in": "query", + "description": "Number of lines to show from the end of the logs.", + "required": false, + "schema": { + "type": "integer", + "format": "int32", + "default": 100 + } + }, + { + "name": "show_timestamps", + "in": "query", + "description": "Show timestamps in the logs.", + "required": false, + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "200": { + "description": "Get service logs by UUID.", + "content": { + "application\/json": { + "schema": { + "properties": { + "logs": { + "type": "string" + } + }, + "type": "object" + } + } + } + }, + "401": { + "$ref": "#\/components\/responses\/401" + }, + "400": { + "$ref": "#\/components\/responses\/400" + }, + "404": { + "$ref": "#\/components\/responses\/404" + } + }, + "security": [ + { + "bearerAuth": [] + } + ] + } + }, "\/services\/{uuid}\/envs": { "get": { "tags": [ diff --git a/openapi.yaml b/openapi.yaml index 6182cacd3..dacacece1 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1716,6 +1716,14 @@ paths: type: integer format: int32 default: 100 + - + name: show_timestamps + in: query + description: 'Show timestamps in the logs.' + required: false + schema: + type: boolean + default: false responses: '200': description: 'Get application logs by UUID.' @@ -3908,6 +3916,57 @@ paths: security: - bearerAuth: [] + '/databases/{uuid}/logs': + get: + tags: + - Databases + summary: 'Get database logs.' + description: 'Get database logs by UUID.' + operationId: get-database-logs-by-uuid + parameters: + - + name: uuid + in: path + description: 'UUID of the database.' + required: true + schema: + type: string + format: uuid + - + name: lines + in: query + description: 'Number of lines to show from the end of the logs.' + required: false + schema: + type: integer + format: int32 + default: 100 + - + name: show_timestamps + in: query + description: 'Show timestamps in the logs.' + required: false + schema: + type: boolean + default: false + responses: + '200': + description: 'Get database logs by UUID.' + content: + application/json: + schema: + properties: + logs: { type: string } + type: object + '401': + $ref: '#/components/responses/401' + '400': + $ref: '#/components/responses/400' + '404': + $ref: '#/components/responses/404' + security: + - + bearerAuth: [] '/databases/{uuid}/backups/{scheduled_backup_uuid}/executions/{execution_uuid}': delete: tags: @@ -7150,6 +7209,65 @@ paths: security: - bearerAuth: [] + '/services/{uuid}/logs': + get: + tags: + - Services + summary: 'Get service logs.' + description: 'Get logs for a specific service sub-resource by service UUID. The `sub_service_name` query parameter must match the `name` field of one of the service applications or databases returned by `GET /services/{uuid}`.' + operationId: get-service-logs-by-uuid + parameters: + - + name: uuid + in: path + description: 'UUID of the service.' + required: true + schema: + type: string + format: uuid + - + name: sub_service_name + in: query + description: 'Sub-service name from `GET /services/{uuid}` under `applications[].name` or `databases[].name`. Do not use `human_name` or the Docker container name with the service UUID suffix.' + required: true + schema: + type: string + example: appwrite-console + - + name: lines + in: query + description: 'Number of lines to show from the end of the logs.' + required: false + schema: + type: integer + format: int32 + default: 100 + - + name: show_timestamps + in: query + description: 'Show timestamps in the logs.' + required: false + schema: + type: boolean + default: false + responses: + '200': + description: 'Get service logs by UUID.' + content: + application/json: + schema: + properties: + logs: { type: string } + type: object + '401': + $ref: '#/components/responses/401' + '400': + $ref: '#/components/responses/400' + '404': + $ref: '#/components/responses/404' + security: + - + bearerAuth: [] '/services/{uuid}/envs': get: tags: diff --git a/tests/Unit/Api/LogEndpointHelpersTest.php b/tests/Unit/Api/LogEndpointHelpersTest.php new file mode 100644 index 000000000..0b0107e73 --- /dev/null +++ b/tests/Unit/Api/LogEndpointHelpersTest.php @@ -0,0 +1,87 @@ +toBeTrue(); + + return; + } + + expect(normalizeLogLines(null))->toBe(100) + ->and(normalizeLogLines(''))->toBe(100) + ->and(normalizeLogLines('abc'))->toBe(100) + ->and(normalizeLogLines('0'))->toBe(100) + ->and(normalizeLogLines('-5'))->toBe(100) + ->and(normalizeLogLines('50'))->toBe(50) + ->and(normalizeLogLines('50000'))->toBe(10000); +}); + +it('parses show_timestamps query values as booleans', function () { + if (! function_exists('parseLogTimestampFlag')) { + expect(function_exists('parseLogTimestampFlag'))->toBeTrue(); + + return; + } + + expect(parseLogTimestampFlag('true'))->toBeTrue() + ->and(parseLogTimestampFlag('1'))->toBeTrue() + ->and(parseLogTimestampFlag(true))->toBeTrue() + ->and(parseLogTimestampFlag('false'))->toBeFalse() + ->and(parseLogTimestampFlag('0'))->toBeFalse() + ->and(parseLogTimestampFlag(false))->toBeFalse() + ->and(parseLogTimestampFlag('not-a-bool'))->toBeFalse() + ->and(parseLogTimestampFlag(null))->toBeFalse(); +}); + +it('builds docker log commands with options before an escaped container id', function () { + if (! function_exists('buildContainerLogsCommand')) { + expect(function_exists('buildContainerLogsCommand'))->toBeTrue(); + + return; + } + + $server = new Server; + $server->settings = ['is_swarm_manager' => false]; + + expect(buildContainerLogsCommand($server, 'container-1', 25, true)) + ->toBe("docker logs -n 25 --timestamps 'container-1' 2>&1"); +}); + +it('builds swarm service log commands with options before an escaped service id', function () { + if (! function_exists('buildContainerLogsCommand')) { + expect(function_exists('buildContainerLogsCommand'))->toBeTrue(); + + return; + } + + $server = new Server; + $server->settings = ['is_swarm_manager' => true]; + + expect(buildContainerLogsCommand($server, "service'name", 25, true)) + ->toBe("docker service logs -n 25 --timestamps 'service'\\''name' 2>&1"); +}); + +it('filters service sub containers in PHP instead of using user input in shell filters', function () { + if (! function_exists('filterServiceSubContainersByName')) { + expect(function_exists('filterServiceSubContainersByName'))->toBeTrue(); + + return; + } + + $containers = collect([ + ['ID' => 'first', 'Labels' => 'coolify.serviceId=10,coolify.name=app-service-uuid,coolify.type=service'], + ['ID' => 'second', 'Labels' => 'coolify.serviceId=10,coolify.name=db-service-uuid,coolify.type=service'], + ['ID' => 'third', 'Labels' => ['coolify.name' => 'app-service-uuid']], + ]); + + expect(filterServiceSubContainersByName($containers, 'app-service-uuid')->pluck('ID')->all()) + ->toBe(['first', 'third']); +}); + +it('does not interpolate the requested service name into the docker ps shell command', function () { + $source = file_get_contents(__DIR__.'/../../../bootstrap/helpers/docker.php'); + + expect($source)->not->toContain('coolify.name={$name}'); +});