From 0555e4c0dd3519183140e137378dc60e039310fb Mon Sep 17 00:00:00 2001 From: Michel Roegl-Brunner <73236783+michelroegl-brunner@users.noreply.github.com> Date: Tue, 14 Oct 2025 09:32:13 +0200 Subject: [PATCH] Fix SSH ED25519 key loading and status check loops (#140) - Fix SSH ED25519 key loading error by ensuring temporary key files end with newline - Fix cleanup logic using rmdirSync instead of unlinkSync for directories - Add timeout protection to prevent hanging SSH connections - Fix endless status checking loops by removing problematic dependencies - Add debouncing and proper cleanup for status checks - Support file upload without extensions for Windows-generated keys - Improve SSH key type detection for OpenSSH format keys Resolves libcrypto errors and prevents resource leaks in SSH operations. --- src/app/_components/InstalledScriptsTab.tsx | 48 ++++++++++++++++----- src/app/_components/SSHKeyInput.tsx | 26 ++++++++--- src/server/api/routers/installedScripts.ts | 40 ++++++++++------- src/server/ssh-execution-service.js | 14 +++--- src/server/ssh-service.js | 4 +- 5 files changed, 94 insertions(+), 38 deletions(-) diff --git a/src/app/_components/InstalledScriptsTab.tsx b/src/app/_components/InstalledScriptsTab.tsx index cf369fa..6387e3d 100644 --- a/src/app/_components/InstalledScriptsTab.tsx +++ b/src/app/_components/InstalledScriptsTab.tsx @@ -59,6 +59,7 @@ export function InstalledScriptsTab() { } | null>(null); const [controllingScriptId, setControllingScriptId] = useState(null); const scriptsRef = useRef([]); + const statusCheckTimeoutRef = useRef(null); // Error modal state const [errorModal, setErrorModal] = useState<{ @@ -312,17 +313,34 @@ export function InstalledScriptsTab() { // Function to fetch container statuses - simplified to just check all servers const fetchContainerStatuses = useCallback(() => { - const currentScripts = scriptsRef.current; + console.log('fetchContainerStatuses called, isPending:', containerStatusMutation.isPending); - // Get unique server IDs from scripts - const serverIds = [...new Set(currentScripts - .filter(script => script.server_id) - .map(script => script.server_id!))]; - - if (serverIds.length > 0) { - containerStatusMutation.mutate({ serverIds }); + // Prevent multiple simultaneous status checks + if (containerStatusMutation.isPending) { + console.log('Status check already pending, skipping'); + return; } - }, [containerStatusMutation]); + + // Clear any existing timeout + if (statusCheckTimeoutRef.current) { + clearTimeout(statusCheckTimeoutRef.current); + } + + // Debounce status checks by 500ms + statusCheckTimeoutRef.current = setTimeout(() => { + const currentScripts = scriptsRef.current; + + // Get unique server IDs from scripts + const serverIds = [...new Set(currentScripts + .filter(script => script.server_id) + .map(script => script.server_id!))]; + + console.log('Executing status check for server IDs:', serverIds); + if (serverIds.length > 0) { + containerStatusMutation.mutate({ serverIds }); + } + }, 500); + }, []); // Remove containerStatusMutation from dependencies to prevent loops // Run cleanup when component mounts and scripts are loaded (only once) useEffect(() => { @@ -335,9 +353,19 @@ export function InstalledScriptsTab() { useEffect(() => { if (scripts.length > 0) { + console.log('Status check triggered - scripts length:', scripts.length); fetchContainerStatuses(); } - }, [scripts.length, fetchContainerStatuses]); + }, [scripts.length]); // Remove fetchContainerStatuses from dependencies + + // Cleanup timeout on unmount + useEffect(() => { + return () => { + if (statusCheckTimeoutRef.current) { + clearTimeout(statusCheckTimeoutRef.current); + } + }; + }, []); const scriptsWithStatus = scripts.map(script => ({ ...script, diff --git a/src/app/_components/SSHKeyInput.tsx b/src/app/_components/SSHKeyInput.tsx index fb70f22..bd2f74a 100644 --- a/src/app/_components/SSHKeyInput.tsx +++ b/src/app/_components/SSHKeyInput.tsx @@ -93,9 +93,25 @@ export function SSHKeyInput({ value, onChange, onError, disabled = false }: SSHK ); if (keyLine) { - const keyType = keyLine.includes('RSA') ? 'RSA' : - keyLine.includes('ED25519') ? 'ED25519' : - keyLine.includes('ECDSA') ? 'ECDSA' : 'Unknown'; + let keyType = 'Unknown'; + + // Check for traditional PEM format keys + if (keyLine.includes('RSA')) { + keyType = 'RSA'; + } else if (keyLine.includes('ED25519')) { + keyType = 'ED25519'; + } else if (keyLine.includes('ECDSA')) { + keyType = 'ECDSA'; + } else if (keyLine.includes('OPENSSH PRIVATE KEY')) { + // For OpenSSH format keys, try to detect type from the key content + // Look for common patterns in the base64 content + const base64Content = keyContent.replace(/-----BEGIN.*?-----/, '').replace(/-----END.*?-----/, '').replace(/\s/g, ''); + + // This is a heuristic - OpenSSH ED25519 keys typically start with specific patterns + // We'll default to "OpenSSH" for now since we can't reliably detect the type + keyType = 'OpenSSH'; + } + return `${keyType} key (${keyContent.length} characters)`; } @@ -142,7 +158,7 @@ export function SSHKeyInput({ value, onChange, onError, disabled = false }: SSHK

- Supported formats: RSA, ED25519, ECDSA (.pem, .key, .id_rsa, etc.) + Supported formats: RSA, ED25519, ECDSA (.pem, .key, .id_rsa, ed25519, etc.)

diff --git a/src/server/api/routers/installedScripts.ts b/src/server/api/routers/installedScripts.ts index e424b8d..c6c0b1a 100644 --- a/src/server/api/routers/installedScripts.ts +++ b/src/server/api/routers/installedScripts.ts @@ -551,23 +551,31 @@ export const installedScriptsRouter = createTRPCRouter({ const listCommand = 'pct list'; let listOutput = ''; - await new Promise((resolve, reject) => { - void sshExecutionService.executeCommand( - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - server as any, - listCommand, - (data: string) => { - listOutput += data; - }, - (error: string) => { - console.error(`pct list error on server ${(server as any).name}:`, error); - reject(new Error(error)); - }, - (_exitCode: number) => { - resolve(); - } - ); + // Add timeout to prevent hanging connections + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => reject(new Error('SSH command timeout after 30 seconds')), 30000); }); + + await Promise.race([ + new Promise((resolve, reject) => { + void sshExecutionService.executeCommand( + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + server as any, + listCommand, + (data: string) => { + listOutput += data; + }, + (error: string) => { + console.error(`pct list error on server ${(server as any).name}:`, error); + reject(new Error(error)); + }, + (_exitCode: number) => { + resolve(); + } + ); + }), + timeoutPromise + ]); // Parse pct list output const lines = listOutput.split('\n').filter(line => line.trim()); diff --git a/src/server/ssh-execution-service.js b/src/server/ssh-execution-service.js index 0aa2f44..914c107 100644 --- a/src/server/ssh-execution-service.js +++ b/src/server/ssh-execution-service.js @@ -32,7 +32,9 @@ class SSHExecutionService { const tempDir = mkdtempSync(join(tmpdir(), 'ssh-key-')); const tempKeyPath = join(tempDir, 'private_key'); - writeFileSync(tempKeyPath, ssh_key); + // Normalize the key: trim any trailing whitespace and ensure exactly one newline at the end + const normalizedKey = ssh_key.trimEnd() + '\n'; + writeFileSync(tempKeyPath, normalizedKey); chmodSync(tempKeyPath, 0o600); // Set proper permissions return tempKeyPath; @@ -295,7 +297,7 @@ class SSHExecutionService { try { unlinkSync(tempKeyPath); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); - unlinkSync(tempDir); + rmdirSync(tempDir); } catch (cleanupError) { console.warn('Failed to clean up temporary SSH key file:', cleanupError); } @@ -314,7 +316,7 @@ class SSHExecutionService { try { unlinkSync(tempKeyPath); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); - unlinkSync(tempDir); + rmdirSync(tempDir); } catch (cleanupError) { console.warn('Failed to clean up temporary SSH key file:', cleanupError); } @@ -328,7 +330,7 @@ class SSHExecutionService { try { unlinkSync(tempKeyPath); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); - unlinkSync(tempDir); + rmdirSync(tempDir); } catch (cleanupError) { console.warn('Failed to clean up temporary SSH key file:', cleanupError); } @@ -383,7 +385,7 @@ class SSHExecutionService { try { unlinkSync(tempKeyPath); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); - unlinkSync(tempDir); + rmdirSync(tempDir); } catch (cleanupError) { console.warn('Failed to clean up temporary SSH key file:', cleanupError); } @@ -414,7 +416,7 @@ class SSHExecutionService { try { unlinkSync(tempKeyPath); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); - unlinkSync(tempDir); + rmdirSync(tempDir); } catch (cleanupError) { console.warn('Failed to clean up temporary SSH key file:', cleanupError); } diff --git a/src/server/ssh-service.js b/src/server/ssh-service.js index 0ad2c02..f596482 100644 --- a/src/server/ssh-service.js +++ b/src/server/ssh-service.js @@ -557,7 +557,9 @@ expect { tempKeyPath = join(tempDir, 'private_key'); // Write the private key to temporary file - writeFileSync(tempKeyPath, ssh_key); + // Normalize the key: trim any trailing whitespace and ensure exactly one newline at the end + const normalizedKey = ssh_key.trimEnd() + '\n'; + writeFileSync(tempKeyPath, normalizedKey); chmodSync(tempKeyPath, 0o600); // Set proper permissions // Build SSH command