From 9702543e20db46e2c1d34b41fbc5fe0f5e1f0d26 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 10 Mar 2026 18:32:19 +0100 Subject: [PATCH 1/5] chore: prepare for PR --- app/Actions/Server/CleanupDocker.php | 25 +++- .../Unit/Actions/Server/CleanupDockerTest.php | 134 +++++++++++++++++- 2 files changed, 154 insertions(+), 5 deletions(-) diff --git a/app/Actions/Server/CleanupDocker.php b/app/Actions/Server/CleanupDocker.php index 65a41db18..0d9ca0153 100644 --- a/app/Actions/Server/CleanupDocker.php +++ b/app/Actions/Server/CleanupDocker.php @@ -177,9 +177,10 @@ private function cleanupApplicationImages(Server $server, $applications = null): ->filter(fn ($image) => ! empty($image['tag'])); // Separate images into categories - // PR images (pr-*) and build images (*-build) are excluded from retention - // Build images will be cleaned up by docker image prune -af + // PR images (pr-*) are always deleted + // Build images (*-build) are cleaned up to match retained regular images $prImages = $images->filter(fn ($image) => str_starts_with($image['tag'], 'pr-')); + $buildImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && str_ends_with($image['tag'], '-build')); $regularImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && ! str_ends_with($image['tag'], '-build')); // Always delete all PR images @@ -209,6 +210,26 @@ private function cleanupApplicationImages(Server $server, $applications = null): 'output' => $deleteOutput ?? 'Image removed or was in use', ]; } + + // Clean up build images (-build suffix) that don't correspond to retained regular images + // Build images are intermediate artifacts (e.g. Nixpacks) not used by running containers. + // If a build is in progress, docker rmi will fail silently since the image is in use. + $keptTags = $sortedRegularImages->take($imagesToKeep)->pluck('tag'); + if (! empty($currentTag)) { + $keptTags = $keptTags->push($currentTag); + } + + foreach ($buildImages as $image) { + $baseTag = preg_replace('/-build$/', '', $image['tag']); + if (! $keptTags->contains($baseTag)) { + $deleteCommand = "docker rmi {$image['image_ref']} 2>/dev/null || true"; + $deleteOutput = instant_remote_process([$deleteCommand], $server, false); + $cleanupLog[] = [ + 'command' => $deleteCommand, + 'output' => $deleteOutput ?? 'Build image removed or was in use', + ]; + } + } } return $cleanupLog; diff --git a/tests/Unit/Actions/Server/CleanupDockerTest.php b/tests/Unit/Actions/Server/CleanupDockerTest.php index 630b1bf53..fc8b8ab9b 100644 --- a/tests/Unit/Actions/Server/CleanupDockerTest.php +++ b/tests/Unit/Actions/Server/CleanupDockerTest.php @@ -8,9 +8,7 @@ Mockery::close(); }); -it('categorizes images correctly into PR and regular images', function () { - // Test the image categorization logic - // Build images (*-build) are excluded from retention and handled by docker image prune +it('categorizes images correctly into PR, build, and regular images', function () { $images = collect([ ['repository' => 'app-uuid', 'tag' => 'abc123', 'created_at' => '2024-01-01', 'image_ref' => 'app-uuid:abc123'], ['repository' => 'app-uuid', 'tag' => 'def456', 'created_at' => '2024-01-02', 'image_ref' => 'app-uuid:def456'], @@ -25,6 +23,11 @@ expect($prImages)->toHaveCount(2); expect($prImages->pluck('tag')->toArray())->toContain('pr-123', 'pr-456'); + // Build images (tags ending with '-build', excluding PR builds) + $buildImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && str_ends_with($image['tag'], '-build')); + expect($buildImages)->toHaveCount(2); + expect($buildImages->pluck('tag')->toArray())->toContain('abc123-build', 'def456-build'); + // Regular images (neither PR nor build) - these are subject to retention policy $regularImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && ! str_ends_with($image['tag'], '-build')); expect($regularImages)->toHaveCount(2); @@ -340,3 +343,128 @@ // Other images should not be protected expect(preg_match($pattern, 'nginx:alpine'))->toBe(0); }); + +it('deletes build images not matching retained regular images', function () { + // Simulates the Nixpacks scenario from issue #8765: + // Many -build images accumulate because they were excluded from both cleanup paths + $images = collect([ + ['repository' => 'app-uuid', 'tag' => 'commit1', 'created_at' => '2024-01-01 10:00:00', 'image_ref' => 'app-uuid:commit1'], + ['repository' => 'app-uuid', 'tag' => 'commit2', 'created_at' => '2024-01-02 10:00:00', 'image_ref' => 'app-uuid:commit2'], + ['repository' => 'app-uuid', 'tag' => 'commit3', 'created_at' => '2024-01-03 10:00:00', 'image_ref' => 'app-uuid:commit3'], + ['repository' => 'app-uuid', 'tag' => 'commit4', 'created_at' => '2024-01-04 10:00:00', 'image_ref' => 'app-uuid:commit4'], + ['repository' => 'app-uuid', 'tag' => 'commit5', 'created_at' => '2024-01-05 10:00:00', 'image_ref' => 'app-uuid:commit5'], + ['repository' => 'app-uuid', 'tag' => 'commit1-build', 'created_at' => '2024-01-01 09:00:00', 'image_ref' => 'app-uuid:commit1-build'], + ['repository' => 'app-uuid', 'tag' => 'commit2-build', 'created_at' => '2024-01-02 09:00:00', 'image_ref' => 'app-uuid:commit2-build'], + ['repository' => 'app-uuid', 'tag' => 'commit3-build', 'created_at' => '2024-01-03 09:00:00', 'image_ref' => 'app-uuid:commit3-build'], + ['repository' => 'app-uuid', 'tag' => 'commit4-build', 'created_at' => '2024-01-04 09:00:00', 'image_ref' => 'app-uuid:commit4-build'], + ['repository' => 'app-uuid', 'tag' => 'commit5-build', 'created_at' => '2024-01-05 09:00:00', 'image_ref' => 'app-uuid:commit5-build'], + ]); + + $currentTag = 'commit5'; + $imagesToKeep = 2; + + $buildImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && str_ends_with($image['tag'], '-build')); + $regularImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && ! str_ends_with($image['tag'], '-build')); + + $sortedRegularImages = $regularImages + ->filter(fn ($image) => $image['tag'] !== $currentTag) + ->sortByDesc('created_at') + ->values(); + + // Determine kept tags: current + N newest rollback + $keptTags = $sortedRegularImages->take($imagesToKeep)->pluck('tag'); + if (! empty($currentTag)) { + $keptTags = $keptTags->push($currentTag); + } + + // Kept tags should be: commit5 (running), commit4, commit3 (2 newest rollback) + expect($keptTags->toArray())->toContain('commit5', 'commit4', 'commit3'); + + // Build images to delete: those whose base tag is NOT in keptTags + $buildImagesToDelete = $buildImages->filter(function ($image) use ($keptTags) { + $baseTag = preg_replace('/-build$/', '', $image['tag']); + + return ! $keptTags->contains($baseTag); + }); + + // Should delete commit1-build and commit2-build (their base tags are not kept) + expect($buildImagesToDelete)->toHaveCount(2); + expect($buildImagesToDelete->pluck('tag')->toArray())->toContain('commit1-build', 'commit2-build'); + + // Should keep commit3-build, commit4-build, commit5-build (matching retained images) + $buildImagesToKeep = $buildImages->filter(function ($image) use ($keptTags) { + $baseTag = preg_replace('/-build$/', '', $image['tag']); + + return $keptTags->contains($baseTag); + }); + expect($buildImagesToKeep)->toHaveCount(3); + expect($buildImagesToKeep->pluck('tag')->toArray())->toContain('commit5-build', 'commit4-build', 'commit3-build'); +}); + +it('deletes all build images when retention is disabled', function () { + $images = collect([ + ['repository' => 'app-uuid', 'tag' => 'commit1', 'created_at' => '2024-01-01 10:00:00', 'image_ref' => 'app-uuid:commit1'], + ['repository' => 'app-uuid', 'tag' => 'commit2', 'created_at' => '2024-01-02 10:00:00', 'image_ref' => 'app-uuid:commit2'], + ['repository' => 'app-uuid', 'tag' => 'commit1-build', 'created_at' => '2024-01-01 09:00:00', 'image_ref' => 'app-uuid:commit1-build'], + ['repository' => 'app-uuid', 'tag' => 'commit2-build', 'created_at' => '2024-01-02 09:00:00', 'image_ref' => 'app-uuid:commit2-build'], + ]); + + $currentTag = 'commit2'; + $imagesToKeep = 0; // Retention disabled + + $buildImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && str_ends_with($image['tag'], '-build')); + $regularImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && ! str_ends_with($image['tag'], '-build')); + + $sortedRegularImages = $regularImages + ->filter(fn ($image) => $image['tag'] !== $currentTag) + ->sortByDesc('created_at') + ->values(); + + // With imagesToKeep=0, only current tag is kept + $keptTags = $sortedRegularImages->take($imagesToKeep)->pluck('tag'); + if (! empty($currentTag)) { + $keptTags = $keptTags->push($currentTag); + } + + $buildImagesToDelete = $buildImages->filter(function ($image) use ($keptTags) { + $baseTag = preg_replace('/-build$/', '', $image['tag']); + + return ! $keptTags->contains($baseTag); + }); + + // commit1-build should be deleted (not retained), commit2-build kept (matches running) + expect($buildImagesToDelete)->toHaveCount(1); + expect($buildImagesToDelete->pluck('tag')->toArray())->toContain('commit1-build'); +}); + +it('preserves build image for currently running tag', function () { + $images = collect([ + ['repository' => 'app-uuid', 'tag' => 'commit1', 'created_at' => '2024-01-01 10:00:00', 'image_ref' => 'app-uuid:commit1'], + ['repository' => 'app-uuid', 'tag' => 'commit1-build', 'created_at' => '2024-01-01 09:00:00', 'image_ref' => 'app-uuid:commit1-build'], + ]); + + $currentTag = 'commit1'; + $imagesToKeep = 2; + + $buildImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && str_ends_with($image['tag'], '-build')); + $regularImages = $images->filter(fn ($image) => ! str_starts_with($image['tag'], 'pr-') && ! str_ends_with($image['tag'], '-build')); + + $sortedRegularImages = $regularImages + ->filter(fn ($image) => $image['tag'] !== $currentTag) + ->sortByDesc('created_at') + ->values(); + + $keptTags = $sortedRegularImages->take($imagesToKeep)->pluck('tag'); + if (! empty($currentTag)) { + $keptTags = $keptTags->push($currentTag); + } + + $buildImagesToDelete = $buildImages->filter(function ($image) use ($keptTags) { + $baseTag = preg_replace('/-build$/', '', $image['tag']); + + return ! $keptTags->contains($baseTag); + }); + + // Build image for running tag should NOT be deleted + expect($buildImagesToDelete)->toHaveCount(0); +}); From 5c5f67f48b893c9ee0b7142f94b734da585e97e0 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 10 Mar 2026 20:37:22 +0100 Subject: [PATCH 2/5] chore: prepare for PR --- docker-compose-maxio.dev.yml | 1 + docker-compose.dev.yml | 1 + docker/coolify-realtime/Dockerfile | 1 + docker/coolify-realtime/terminal-server.js | 277 +++++++++++------- docker/coolify-realtime/terminal-utils.js | 127 ++++++++ .../coolify-realtime/terminal-utils.test.js | 47 +++ resources/js/terminal.js | 87 ++++-- .../execute-container-command.blade.php | 3 +- routes/web.php | 18 +- .../Feature/RealtimeTerminalPackagingTest.php | 34 +++ tests/Feature/TerminalAuthIpsRouteTest.php | 51 ++++ 11 files changed, 513 insertions(+), 134 deletions(-) create mode 100644 docker/coolify-realtime/terminal-utils.js create mode 100644 docker/coolify-realtime/terminal-utils.test.js create mode 100644 tests/Feature/RealtimeTerminalPackagingTest.php create mode 100644 tests/Feature/TerminalAuthIpsRouteTest.php diff --git a/docker-compose-maxio.dev.yml b/docker-compose-maxio.dev.yml index 2c8c94466..bbb483d7a 100644 --- a/docker-compose-maxio.dev.yml +++ b/docker-compose-maxio.dev.yml @@ -73,6 +73,7 @@ services: volumes: - ./storage:/var/www/html/storage - ./docker/coolify-realtime/terminal-server.js:/terminal/terminal-server.js + - ./docker/coolify-realtime/terminal-utils.js:/terminal/terminal-utils.js environment: SOKETI_DEBUG: "false" SOKETI_DEFAULT_APP_ID: "${PUSHER_APP_ID:-coolify}" diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 808b50ff8..3af443c83 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -73,6 +73,7 @@ services: volumes: - ./storage:/var/www/html/storage - ./docker/coolify-realtime/terminal-server.js:/terminal/terminal-server.js + - ./docker/coolify-realtime/terminal-utils.js:/terminal/terminal-utils.js environment: SOKETI_DEBUG: "false" SOKETI_DEFAULT_APP_ID: "${PUSHER_APP_ID:-coolify}" diff --git a/docker/coolify-realtime/Dockerfile b/docker/coolify-realtime/Dockerfile index 18c2f9301..99157268b 100644 --- a/docker/coolify-realtime/Dockerfile +++ b/docker/coolify-realtime/Dockerfile @@ -16,6 +16,7 @@ RUN npm i RUN npm rebuild node-pty --update-binary COPY docker/coolify-realtime/soketi-entrypoint.sh /soketi-entrypoint.sh COPY docker/coolify-realtime/terminal-server.js /terminal/terminal-server.js +COPY docker/coolify-realtime/terminal-utils.js /terminal/terminal-utils.js # Install Cloudflared based on architecture RUN if [ "${TARGETPLATFORM}" = "linux/amd64" ]; then \ diff --git a/docker/coolify-realtime/terminal-server.js b/docker/coolify-realtime/terminal-server.js index 2607d2aec..3ae77857f 100755 --- a/docker/coolify-realtime/terminal-server.js +++ b/docker/coolify-realtime/terminal-server.js @@ -4,8 +4,33 @@ import pty from 'node-pty'; import axios from 'axios'; import cookie from 'cookie'; import 'dotenv/config'; +import { + extractHereDocContent, + extractSshArgs, + extractTargetHost, + extractTimeout, + isAuthorizedTargetHost, +} from './terminal-utils.js'; const userSessions = new Map(); +const terminalDebugEnabled = ['local', 'development'].includes( + String(process.env.APP_ENV || process.env.NODE_ENV || '').toLowerCase() +); + +function logTerminal(level, message, context = {}) { + if (!terminalDebugEnabled) { + return; + } + + const formattedMessage = `[TerminalServer] ${message}`; + + if (Object.keys(context).length > 0) { + console[level](formattedMessage, context); + return; + } + + console[level](formattedMessage); +} const server = http.createServer((req, res) => { if (req.url === '/ready') { @@ -31,9 +56,19 @@ const getSessionCookie = (req) => { const verifyClient = async (info, callback) => { const { xsrfToken, laravelSession, sessionCookieName } = getSessionCookie(info.req); + const requestContext = { + remoteAddress: info.req.socket?.remoteAddress, + origin: info.origin, + sessionCookieName, + hasXsrfToken: Boolean(xsrfToken), + hasLaravelSession: Boolean(laravelSession), + }; + + logTerminal('log', 'Verifying websocket client.', requestContext); // Verify presence of required tokens if (!laravelSession || !xsrfToken) { + logTerminal('warn', 'Rejecting websocket client because required auth tokens are missing.', requestContext); return callback(false, 401, 'Unauthorized: Missing required tokens'); } @@ -47,13 +82,22 @@ const verifyClient = async (info, callback) => { }); if (response.status === 200) { - // Authentication successful + logTerminal('log', 'Websocket client authentication succeeded.', requestContext); callback(true); } else { + logTerminal('warn', 'Websocket client authentication returned a non-success status.', { + ...requestContext, + status: response.status, + }); callback(false, 401, 'Unauthorized: Invalid credentials'); } } catch (error) { - console.error('Authentication error:', error.message); + logTerminal('error', 'Websocket client authentication failed.', { + ...requestContext, + error: error.message, + responseStatus: error.response?.status, + responseData: error.response?.data, + }); callback(false, 500, 'Internal Server Error'); } }; @@ -65,28 +109,62 @@ wss.on('connection', async (ws, req) => { const userId = generateUserId(); const userSession = { ws, userId, ptyProcess: null, isActive: false, authorizedIPs: [] }; const { xsrfToken, laravelSession, sessionCookieName } = getSessionCookie(req); + const connectionContext = { + userId, + remoteAddress: req.socket?.remoteAddress, + sessionCookieName, + hasXsrfToken: Boolean(xsrfToken), + hasLaravelSession: Boolean(laravelSession), + }; // Verify presence of required tokens if (!laravelSession || !xsrfToken) { + logTerminal('warn', 'Closing websocket connection because required auth tokens are missing.', connectionContext); ws.close(401, 'Unauthorized: Missing required tokens'); return; } - const response = await axios.post(`http://coolify:8080/terminal/auth/ips`, null, { - headers: { - 'Cookie': `${sessionCookieName}=${laravelSession}`, - 'X-XSRF-TOKEN': xsrfToken - }, - }); - userSession.authorizedIPs = response.data.ipAddresses || []; + + try { + const response = await axios.post(`http://coolify:8080/terminal/auth/ips`, null, { + headers: { + 'Cookie': `${sessionCookieName}=${laravelSession}`, + 'X-XSRF-TOKEN': xsrfToken + }, + }); + userSession.authorizedIPs = response.data.ipAddresses || []; + logTerminal('log', 'Fetched authorized terminal hosts for websocket session.', { + ...connectionContext, + authorizedIPs: userSession.authorizedIPs, + }); + } catch (error) { + logTerminal('error', 'Failed to fetch authorized terminal hosts.', { + ...connectionContext, + error: error.message, + responseStatus: error.response?.status, + responseData: error.response?.data, + }); + ws.close(1011, 'Failed to fetch terminal authorization data'); + return; + } + userSessions.set(userId, userSession); + logTerminal('log', 'Terminal websocket connection established.', { + ...connectionContext, + authorizedHostCount: userSession.authorizedIPs.length, + }); ws.on('message', (message) => { handleMessage(userSession, message); - }); ws.on('error', (err) => handleError(err, userId)); - ws.on('close', () => handleClose(userId)); - + ws.on('close', (code, reason) => { + logTerminal('log', 'Terminal websocket connection closed.', { + userId, + code, + reason: reason?.toString(), + }); + handleClose(userId); + }); }); const messageHandlers = { @@ -98,6 +176,7 @@ const messageHandlers = { }, pause: (session) => session.ptyProcess.pause(), resume: (session) => session.ptyProcess.resume(), + ping: (session) => session.ws.send('pong'), checkActive: (session, data) => { if (data === 'force' && session.isActive) { killPtyProcess(session.userId); @@ -110,12 +189,34 @@ const messageHandlers = { function handleMessage(userSession, message) { const parsed = parseMessage(message); - if (!parsed) return; + if (!parsed) { + logTerminal('warn', 'Ignoring websocket message because JSON parsing failed.', { + userId: userSession.userId, + rawMessage: String(message).slice(0, 500), + }); + return; + } + + logTerminal('log', 'Received websocket message.', { + userId: userSession.userId, + keys: Object.keys(parsed), + isActive: userSession.isActive, + }); Object.entries(parsed).forEach(([key, value]) => { const handler = messageHandlers[key]; - if (handler && (userSession.isActive || key === 'checkActive' || key === 'command')) { + if (handler && (userSession.isActive || key === 'checkActive' || key === 'command' || key === 'ping')) { handler(userSession, value); + } else if (!handler) { + logTerminal('warn', 'Ignoring websocket message with unknown handler key.', { + userId: userSession.userId, + key, + }); + } else { + logTerminal('warn', 'Ignoring websocket message because no PTY session is active yet.', { + userId: userSession.userId, + key, + }); } }); } @@ -124,7 +225,9 @@ function parseMessage(message) { try { return JSON.parse(message); } catch (e) { - console.error('Failed to parse message:', e); + logTerminal('error', 'Failed to parse websocket message.', { + error: e?.message ?? e, + }); return null; } } @@ -134,6 +237,9 @@ async function handleCommand(ws, command, userId) { if (userSession && userSession.isActive) { const result = await killPtyProcess(userId); if (!result) { + logTerminal('warn', 'Rejecting new terminal command because the previous PTY could not be terminated.', { + userId, + }); // if terminal is still active, even after we tried to kill it, dont continue and show error ws.send('unprocessable'); return; @@ -147,13 +253,30 @@ async function handleCommand(ws, command, userId) { // Extract target host from SSH command const targetHost = extractTargetHost(sshArgs); + logTerminal('log', 'Parsed terminal command metadata.', { + userId, + targetHost, + timeout, + sshArgs, + authorizedIPs: userSession?.authorizedIPs ?? [], + }); + if (!targetHost) { + logTerminal('warn', 'Rejecting terminal command because no target host could be extracted.', { + userId, + sshArgs, + }); ws.send('Invalid SSH command: No target host found'); return; } // Validate target host against authorized IPs - if (!userSession.authorizedIPs.includes(targetHost)) { + if (!isAuthorizedTargetHost(targetHost, userSession.authorizedIPs)) { + logTerminal('warn', 'Rejecting terminal command because target host is not authorized.', { + userId, + targetHost, + authorizedIPs: userSession.authorizedIPs, + }); ws.send(`Unauthorized: Target host ${targetHost} not in authorized list`); return; } @@ -169,6 +292,11 @@ async function handleCommand(ws, command, userId) { // NOTE: - Initiates a process within the Terminal container // Establishes an SSH connection to root@coolify with RequestTTY enabled // Executes the 'docker exec' command to connect to a specific container + logTerminal('log', 'Spawning PTY process for terminal session.', { + userId, + targetHost, + timeout, + }); const ptyProcess = pty.spawn('ssh', sshArgs.concat([hereDocContent]), options); userSession.ptyProcess = ptyProcess; @@ -182,7 +310,11 @@ async function handleCommand(ws, command, userId) { // when parent closes ptyProcess.onExit(({ exitCode, signal }) => { - console.error(`Process exited with code ${exitCode} and signal ${signal}`); + logTerminal(exitCode === 0 ? 'log' : 'error', 'PTY process exited.', { + userId, + exitCode, + signal, + }); ws.send('pty-exited'); userSession.isActive = false; }); @@ -194,28 +326,18 @@ async function handleCommand(ws, command, userId) { } } -function extractTargetHost(sshArgs) { - // Find the argument that matches the pattern user@host - const userAtHost = sshArgs.find(arg => { - // Skip paths that contain 'storage/app/ssh/keys/' - if (arg.includes('storage/app/ssh/keys/')) { - return false; - } - return /^[^@]+@[^@]+$/.test(arg); - }); - if (!userAtHost) return null; - - // Extract host from user@host - const host = userAtHost.split('@')[1]; - return host; -} - async function handleError(err, userId) { - console.error('WebSocket error:', err); + logTerminal('error', 'WebSocket error.', { + userId, + error: err?.message ?? err, + }); await killPtyProcess(userId); } async function handleClose(userId) { + logTerminal('log', 'Cleaning up terminal websocket session.', { + userId, + }); await killPtyProcess(userId); userSessions.delete(userId); } @@ -231,6 +353,11 @@ async function killPtyProcess(userId) { const attemptKill = () => { killAttempts++; + logTerminal('log', 'Attempting to terminate PTY process.', { + userId, + killAttempts, + maxAttempts, + }); // session.ptyProcess.kill() wont work here because of https://github.com/moby/moby/issues/9098 // patch with https://github.com/moby/moby/issues/9098#issuecomment-189743947 @@ -238,6 +365,10 @@ async function killPtyProcess(userId) { setTimeout(() => { if (!session.isActive || !session.ptyProcess) { + logTerminal('log', 'PTY process terminated successfully.', { + userId, + killAttempts, + }); resolve(true); return; } @@ -245,6 +376,10 @@ async function killPtyProcess(userId) { if (killAttempts < maxAttempts) { attemptKill(); } else { + logTerminal('warn', 'PTY process still active after maximum termination attempts.', { + userId, + killAttempts, + }); resolve(false); } }, 500); @@ -258,76 +393,8 @@ function generateUserId() { return Math.random().toString(36).substring(2, 11); } -function extractTimeout(commandString) { - const timeoutMatch = commandString.match(/timeout (\d+)/); - return timeoutMatch ? parseInt(timeoutMatch[1], 10) : null; -} - -function extractSshArgs(commandString) { - const sshCommandMatch = commandString.match(/ssh (.+?) 'bash -se'/); - if (!sshCommandMatch) return []; - - const argsString = sshCommandMatch[1]; - let sshArgs = []; - - // Parse shell arguments respecting quotes - let current = ''; - let inQuotes = false; - let quoteChar = ''; - let i = 0; - - while (i < argsString.length) { - const char = argsString[i]; - const nextChar = argsString[i + 1]; - - if (!inQuotes && (char === '"' || char === "'")) { - // Starting a quoted section - inQuotes = true; - quoteChar = char; - current += char; - } else if (inQuotes && char === quoteChar) { - // Ending a quoted section - inQuotes = false; - current += char; - quoteChar = ''; - } else if (!inQuotes && char === ' ') { - // Space outside quotes - end of argument - if (current.trim()) { - sshArgs.push(current.trim()); - current = ''; - } - } else { - // Regular character - current += char; - } - i++; - } - - // Add final argument if exists - if (current.trim()) { - sshArgs.push(current.trim()); - } - - // Replace RequestTTY=no with RequestTTY=yes - sshArgs = sshArgs.map(arg => arg === 'RequestTTY=no' ? 'RequestTTY=yes' : arg); - - // Add RequestTTY=yes if not present - if (!sshArgs.includes('RequestTTY=yes') && !sshArgs.some(arg => arg.includes('RequestTTY='))) { - sshArgs.push('-o', 'RequestTTY=yes'); - } - - return sshArgs; -} - -function extractHereDocContent(commandString) { - const delimiterMatch = commandString.match(/<< (\S+)/); - const delimiter = delimiterMatch ? delimiterMatch[1] : null; - const escapedDelimiter = delimiter.slice(1).trim().replace(/[/\-\\^$*+?.()|[\]{}]/g, '\\$&'); - const hereDocRegex = new RegExp(`<< \\\\${escapedDelimiter}([\\s\\S\\.]*?)${escapedDelimiter}`); - const hereDocMatch = commandString.match(hereDocRegex); - return hereDocMatch ? hereDocMatch[1] : ''; -} - server.listen(6002, () => { - console.log('Coolify realtime terminal server listening on port 6002. Let the hacking begin!'); + logTerminal('log', 'Terminal debug logging is enabled.', { + terminalDebugEnabled, + }); }); diff --git a/docker/coolify-realtime/terminal-utils.js b/docker/coolify-realtime/terminal-utils.js new file mode 100644 index 000000000..7456b282c --- /dev/null +++ b/docker/coolify-realtime/terminal-utils.js @@ -0,0 +1,127 @@ +export function extractTimeout(commandString) { + const timeoutMatch = commandString.match(/timeout (\d+)/); + return timeoutMatch ? parseInt(timeoutMatch[1], 10) : null; +} + +function normalizeShellArgument(argument) { + if (!argument) { + return argument; + } + + return argument + .replace(/'([^']*)'/g, '$1') + .replace(/"([^"]*)"/g, '$1'); +} + +export function extractSshArgs(commandString) { + const sshCommandMatch = commandString.match(/ssh (.+?) 'bash -se'/); + if (!sshCommandMatch) return []; + + const argsString = sshCommandMatch[1]; + let sshArgs = []; + + let current = ''; + let inQuotes = false; + let quoteChar = ''; + let i = 0; + + while (i < argsString.length) { + const char = argsString[i]; + + if (!inQuotes && (char === '"' || char === "'")) { + inQuotes = true; + quoteChar = char; + current += char; + } else if (inQuotes && char === quoteChar) { + inQuotes = false; + current += char; + quoteChar = ''; + } else if (!inQuotes && char === ' ') { + if (current.trim()) { + sshArgs.push(current.trim()); + current = ''; + } + } else { + current += char; + } + i++; + } + + if (current.trim()) { + sshArgs.push(current.trim()); + } + + sshArgs = sshArgs.map((arg) => normalizeShellArgument(arg)); + sshArgs = sshArgs.map(arg => arg === 'RequestTTY=no' ? 'RequestTTY=yes' : arg); + + if (!sshArgs.includes('RequestTTY=yes') && !sshArgs.some(arg => arg.includes('RequestTTY='))) { + sshArgs.push('-o', 'RequestTTY=yes'); + } + + return sshArgs; +} + +export function extractHereDocContent(commandString) { + const delimiterMatch = commandString.match(/<< (\S+)/); + const delimiter = delimiterMatch ? delimiterMatch[1] : null; + const escapedDelimiter = delimiter?.slice(1).trim().replace(/[/\-\\^$*+?.()|[\]{}]/g, '\\$&'); + + if (!escapedDelimiter) { + return ''; + } + + const hereDocRegex = new RegExp(`<< \\\\${escapedDelimiter}([\\s\\S\\.]*?)${escapedDelimiter}`); + const hereDocMatch = commandString.match(hereDocRegex); + return hereDocMatch ? hereDocMatch[1] : ''; +} + +export function normalizeHostForAuthorization(host) { + if (!host) { + return null; + } + + let normalizedHost = host.trim(); + + while ( + normalizedHost.length >= 2 && + ((normalizedHost.startsWith("'") && normalizedHost.endsWith("'")) || + (normalizedHost.startsWith('"') && normalizedHost.endsWith('"'))) + ) { + normalizedHost = normalizedHost.slice(1, -1).trim(); + } + + if (normalizedHost.startsWith('[') && normalizedHost.endsWith(']')) { + normalizedHost = normalizedHost.slice(1, -1); + } + + return normalizedHost.toLowerCase(); +} + +export function extractTargetHost(sshArgs) { + const userAtHost = sshArgs.find(arg => { + if (arg.includes('storage/app/ssh/keys/')) { + return false; + } + + return /^[^@]+@[^@]+$/.test(arg); + }); + + if (!userAtHost) { + return null; + } + + const atIndex = userAtHost.indexOf('@'); + return normalizeHostForAuthorization(userAtHost.slice(atIndex + 1)); +} + +export function isAuthorizedTargetHost(targetHost, authorizedHosts = []) { + const normalizedTargetHost = normalizeHostForAuthorization(targetHost); + + if (!normalizedTargetHost) { + return false; + } + + return authorizedHosts + .map(host => normalizeHostForAuthorization(host)) + .includes(normalizedTargetHost); +} diff --git a/docker/coolify-realtime/terminal-utils.test.js b/docker/coolify-realtime/terminal-utils.test.js new file mode 100644 index 000000000..3da444155 --- /dev/null +++ b/docker/coolify-realtime/terminal-utils.test.js @@ -0,0 +1,47 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + extractSshArgs, + extractTargetHost, + isAuthorizedTargetHost, + normalizeHostForAuthorization, +} from './terminal-utils.js'; + +test('extractTargetHost normalizes quoted IPv4 hosts from generated ssh commands', () => { + const sshArgs = extractSshArgs( + "timeout 3600 ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR -o ServerAliveInterval=20 -o ConnectTimeout=10 'root'@'10.0.0.5' 'bash -se' << \\\\$abc\necho hi\nabc" + ); + + assert.equal(extractTargetHost(sshArgs), '10.0.0.5'); +}); + +test('extractSshArgs strips shell quotes from port and user host arguments before spawning ssh', () => { + const sshArgs = extractSshArgs( + "timeout 3600 ssh -p '22' -o StrictHostKeyChecking=no 'root'@'10.0.0.5' 'bash -se' << \\\\$abc\necho hi\nabc" + ); + + assert.deepEqual(sshArgs.slice(0, 5), ['-p', '22', '-o', 'StrictHostKeyChecking=no', 'root@10.0.0.5']); +}); + +test('extractSshArgs preserves proxy command as a single normalized ssh option value', () => { + const sshArgs = extractSshArgs( + "timeout 3600 ssh -o ProxyCommand='cloudflared access ssh --hostname %h' -o StrictHostKeyChecking=no 'root'@'example.com' 'bash -se' << \\\\$abc\necho hi\nabc" + ); + + assert.equal(sshArgs[1], 'ProxyCommand=cloudflared access ssh --hostname %h'); + assert.equal(sshArgs[4], 'root@example.com'); +}); + +test('isAuthorizedTargetHost matches normalized hosts against plain allowlist values', () => { + assert.equal(isAuthorizedTargetHost("'10.0.0.5'", ['10.0.0.5']), true); + assert.equal(isAuthorizedTargetHost('"host.docker.internal"', ['host.docker.internal']), true); +}); + +test('normalizeHostForAuthorization unwraps bracketed IPv6 hosts', () => { + assert.equal(normalizeHostForAuthorization("'[2001:db8::10]'"), '2001:db8::10'); + assert.equal(isAuthorizedTargetHost("'[2001:db8::10]'", ['2001:db8::10']), true); +}); + +test('isAuthorizedTargetHost rejects hosts that are not in the allowlist', () => { + assert.equal(isAuthorizedTargetHost("'10.0.0.9'", ['10.0.0.5']), false); +}); diff --git a/resources/js/terminal.js b/resources/js/terminal.js index 6707bec98..3c52edfa0 100644 --- a/resources/js/terminal.js +++ b/resources/js/terminal.js @@ -2,6 +2,16 @@ import { Terminal } from '@xterm/xterm'; import '@xterm/xterm/css/xterm.css'; import { FitAddon } from '@xterm/addon-fit'; +const terminalDebugEnabled = import.meta.env.DEV; + +function logTerminal(level, message, ...context) { + if (!terminalDebugEnabled) { + return; + } + + console[level](message, ...context); +} + export function initializeTerminalComponent() { function terminalData() { return { @@ -30,6 +40,8 @@ export function initializeTerminalComponent() { pingTimeoutId: null, heartbeatMissed: 0, maxHeartbeatMisses: 3, + // Command buffering for race condition prevention + pendingCommand: null, // Resize handling resizeObserver: null, resizeTimeout: null, @@ -120,6 +132,7 @@ export function initializeTerminalComponent() { this.checkIfProcessIsRunningAndKillIt(); this.clearAllTimers(); this.connectionState = 'disconnected'; + this.pendingCommand = null; if (this.socket) { this.socket.close(1000, 'Client cleanup'); } @@ -154,6 +167,7 @@ export function initializeTerminalComponent() { this.pendingWrites = 0; this.paused = false; this.commandBuffer = ''; + this.pendingCommand = null; // Notify parent component that terminal disconnected this.$wire.dispatch('terminalDisconnected'); @@ -188,7 +202,7 @@ export function initializeTerminalComponent() { initializeWebSocket() { if (this.socket && this.socket.readyState !== WebSocket.CLOSED) { - console.log('[Terminal] WebSocket already connecting/connected, skipping'); + logTerminal('log', '[Terminal] WebSocket already connecting/connected, skipping'); return; // Already connecting or connected } @@ -197,7 +211,7 @@ export function initializeTerminalComponent() { // Ensure terminal config is available if (!window.terminalConfig) { - console.warn('[Terminal] Terminal config not available, using defaults'); + logTerminal('warn', '[Terminal] Terminal config not available, using defaults'); window.terminalConfig = {}; } @@ -223,7 +237,7 @@ export function initializeTerminalComponent() { } const url = `${connectionString.protocol}://${connectionString.host}${connectionString.port}${connectionString.path}` - console.log(`[Terminal] Attempting connection to: ${url}`); + logTerminal('log', `[Terminal] Attempting connection to: ${url}`); try { this.socket = new WebSocket(url); @@ -232,7 +246,7 @@ export function initializeTerminalComponent() { const timeoutMs = this.reconnectAttempts === 0 ? 15000 : this.connectionTimeout; this.connectionTimeoutId = setTimeout(() => { if (this.connectionState === 'connecting') { - console.error(`[Terminal] Connection timeout after ${timeoutMs}ms`); + logTerminal('error', `[Terminal] Connection timeout after ${timeoutMs}ms`); this.socket.close(); this.handleConnectionError('Connection timeout'); } @@ -244,13 +258,13 @@ export function initializeTerminalComponent() { this.socket.onclose = this.handleSocketClose.bind(this); } catch (error) { - console.error('[Terminal] Failed to create WebSocket:', error); + logTerminal('error', '[Terminal] Failed to create WebSocket:', error); this.handleConnectionError(`Failed to create WebSocket connection: ${error.message}`); } }, handleSocketOpen() { - console.log('[Terminal] WebSocket connection established. Cool cool cool cool cool cool.'); + logTerminal('log', '[Terminal] WebSocket connection established.'); this.connectionState = 'connected'; this.reconnectAttempts = 0; this.heartbeatMissed = 0; @@ -262,6 +276,12 @@ export function initializeTerminalComponent() { this.connectionTimeoutId = null; } + // Flush any buffered command from before WebSocket was ready + if (this.pendingCommand) { + this.sendMessage(this.pendingCommand); + this.pendingCommand = null; + } + // Start ping timeout monitoring this.resetPingTimeout(); @@ -270,16 +290,16 @@ export function initializeTerminalComponent() { }, handleSocketError(error) { - console.error('[Terminal] WebSocket error:', error); - console.error('[Terminal] WebSocket state:', this.socket ? this.socket.readyState : 'No socket'); - console.error('[Terminal] Connection attempt:', this.reconnectAttempts + 1); + logTerminal('error', '[Terminal] WebSocket error:', error); + logTerminal('error', '[Terminal] WebSocket state:', this.socket ? this.socket.readyState : 'No socket'); + logTerminal('error', '[Terminal] Connection attempt:', this.reconnectAttempts + 1); this.handleConnectionError('WebSocket error occurred'); }, handleSocketClose(event) { - console.warn(`[Terminal] WebSocket connection closed. Code: ${event.code}, Reason: ${event.reason || 'No reason provided'}`); - console.log('[Terminal] Was clean close:', event.code === 1000); - console.log('[Terminal] Connection attempt:', this.reconnectAttempts + 1); + logTerminal('warn', `[Terminal] WebSocket connection closed. Code: ${event.code}, Reason: ${event.reason || 'No reason provided'}`); + logTerminal('log', '[Terminal] Was clean close:', event.code === 1000); + logTerminal('log', '[Terminal] Connection attempt:', this.reconnectAttempts + 1); this.connectionState = 'disconnected'; this.clearAllTimers(); @@ -297,7 +317,7 @@ export function initializeTerminalComponent() { }, handleConnectionError(reason) { - console.error(`[Terminal] Connection error: ${reason} (attempt ${this.reconnectAttempts + 1})`); + logTerminal('error', `[Terminal] Connection error: ${reason} (attempt ${this.reconnectAttempts + 1})`); this.connectionState = 'disconnected'; // Only dispatch error to UI after a few failed attempts to avoid immediate error on page load @@ -310,7 +330,7 @@ export function initializeTerminalComponent() { scheduleReconnect() { if (this.reconnectAttempts >= this.maxReconnectAttempts) { - console.error('[Terminal] Max reconnection attempts reached'); + logTerminal('error', '[Terminal] Max reconnection attempts reached'); this.message = '(connection failed - max retries exceeded)'; return; } @@ -323,7 +343,7 @@ export function initializeTerminalComponent() { this.maxReconnectDelay ); - console.warn(`[Terminal] Scheduling reconnect attempt ${this.reconnectAttempts + 1} in ${delay}ms`); + logTerminal('warn', `[Terminal] Scheduling reconnect attempt ${this.reconnectAttempts + 1} in ${delay}ms`); this.reconnectInterval = setTimeout(() => { this.reconnectAttempts++; @@ -335,17 +355,21 @@ export function initializeTerminalComponent() { if (this.socket && this.socket.readyState === WebSocket.OPEN) { this.socket.send(JSON.stringify(message)); } else { - console.warn('[Terminal] WebSocket not ready, message not sent:', message); + logTerminal('warn', '[Terminal] WebSocket not ready, message not sent:', message); } }, sendCommandWhenReady(message) { if (this.isWebSocketReady()) { this.sendMessage(message); + } else { + this.pendingCommand = message; } }, handleSocketMessage(event) { + logTerminal('log', '[Terminal] Received WebSocket message:', event.data); + // Handle pong responses if (event.data === 'pong') { this.heartbeatMissed = 0; @@ -354,6 +378,10 @@ export function initializeTerminalComponent() { return; } + if (!this.term?._initialized && event.data !== 'pty-ready') { + logTerminal('warn', '[Terminal] Received message before PTY initialization:', event.data); + } + if (event.data === 'pty-ready') { if (!this.term._initialized) { this.term.open(document.getElementById('terminal')); @@ -398,17 +426,24 @@ export function initializeTerminalComponent() { // Notify parent component that terminal disconnected this.$wire.dispatch('terminalDisconnected'); + } else if ( + typeof event.data === 'string' && + (event.data.startsWith('Unauthorized:') || event.data.startsWith('Invalid SSH command:')) + ) { + logTerminal('error', '[Terminal] Backend rejected terminal startup:', event.data); + this.$wire.dispatch('error', event.data); + this.terminalActive = false; } else { try { this.pendingWrites++; this.term.write(event.data, (err) => { if (err) { - console.error('[Terminal] Write error:', err); + logTerminal('error', '[Terminal] Write error:', err); } this.flowControlCallback(); }); } catch (error) { - console.error('[Terminal] Write operation failed:', error); + logTerminal('error', '[Terminal] Write operation failed:', error); this.pendingWrites = Math.max(0, this.pendingWrites - 1); } } @@ -483,10 +518,10 @@ export function initializeTerminalComponent() { clearTimeout(this.pingTimeoutId); this.pingTimeoutId = null; } - console.log('[Terminal] Tab hidden, pausing heartbeat monitoring'); + logTerminal('log', '[Terminal] Tab hidden, pausing heartbeat monitoring'); } else if (wasVisible === false) { // Tab is now visible again - console.log('[Terminal] Tab visible, resuming connection management'); + logTerminal('log', '[Terminal] Tab visible, resuming connection management'); if (this.wasConnectedBeforeHidden && this.socket && this.socket.readyState === WebSocket.OPEN) { // Send immediate ping to verify connection is still alive @@ -508,10 +543,10 @@ export function initializeTerminalComponent() { this.pingTimeoutId = setTimeout(() => { this.heartbeatMissed++; - console.warn(`[Terminal] Ping timeout - missed ${this.heartbeatMissed}/${this.maxHeartbeatMisses}`); + logTerminal('warn', `[Terminal] Ping timeout - missed ${this.heartbeatMissed}/${this.maxHeartbeatMisses}`); if (this.heartbeatMissed >= this.maxHeartbeatMisses) { - console.error('[Terminal] Too many missed heartbeats, closing connection'); + logTerminal('error', '[Terminal] Too many missed heartbeats, closing connection'); this.socket.close(1001, 'Heartbeat timeout'); } }, this.pingTimeout); @@ -553,7 +588,7 @@ export function initializeTerminalComponent() { // Check if dimensions are valid if (height <= 0 || width <= 0) { - console.warn('[Terminal] Invalid wrapper dimensions, retrying...', { height, width }); + logTerminal('warn', '[Terminal] Invalid wrapper dimensions, retrying...', { height, width }); setTimeout(() => this.resizeTerminal(), 100); return; } @@ -562,7 +597,7 @@ export function initializeTerminalComponent() { if (!charSize.height || !charSize.width) { // Fallback values if char size not available yet - console.warn('[Terminal] Character size not available, retrying...'); + logTerminal('warn', '[Terminal] Character size not available, retrying...'); setTimeout(() => this.resizeTerminal(), 100); return; } @@ -583,10 +618,10 @@ export function initializeTerminalComponent() { }); } } else { - console.warn('[Terminal] Invalid calculated dimensions:', { rows, cols, height, width, charSize }); + logTerminal('warn', '[Terminal] Invalid calculated dimensions:', { rows, cols, height, width, charSize }); } } catch (error) { - console.error('[Terminal] Resize error:', error); + logTerminal('error', '[Terminal] Resize error:', error); } }, diff --git a/resources/views/livewire/project/shared/execute-container-command.blade.php b/resources/views/livewire/project/shared/execute-container-command.blade.php index f980d6f3c..e7d3546fd 100644 --- a/resources/views/livewire/project/shared/execute-container-command.blade.php +++ b/resources/views/livewire/project/shared/execute-container-command.blade.php @@ -21,7 +21,8 @@