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.
This commit is contained in:
Michel Roegl-Brunner
2025-10-14 09:32:13 +02:00
committed by GitHub
parent 08e0c82f4e
commit 0555e4c0dd
5 changed files with 94 additions and 38 deletions

View File

@@ -59,6 +59,7 @@ export function InstalledScriptsTab() {
} | null>(null); } | null>(null);
const [controllingScriptId, setControllingScriptId] = useState<number | null>(null); const [controllingScriptId, setControllingScriptId] = useState<number | null>(null);
const scriptsRef = useRef<InstalledScript[]>([]); const scriptsRef = useRef<InstalledScript[]>([]);
const statusCheckTimeoutRef = useRef<NodeJS.Timeout | null>(null);
// Error modal state // Error modal state
const [errorModal, setErrorModal] = useState<{ const [errorModal, setErrorModal] = useState<{
@@ -312,17 +313,34 @@ export function InstalledScriptsTab() {
// Function to fetch container statuses - simplified to just check all servers // Function to fetch container statuses - simplified to just check all servers
const fetchContainerStatuses = useCallback(() => { const fetchContainerStatuses = useCallback(() => {
const currentScripts = scriptsRef.current; console.log('fetchContainerStatuses called, isPending:', containerStatusMutation.isPending);
// Get unique server IDs from scripts // Prevent multiple simultaneous status checks
const serverIds = [...new Set(currentScripts if (containerStatusMutation.isPending) {
.filter(script => script.server_id) console.log('Status check already pending, skipping');
.map(script => script.server_id!))]; return;
if (serverIds.length > 0) {
containerStatusMutation.mutate({ serverIds });
} }
}, [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) // Run cleanup when component mounts and scripts are loaded (only once)
useEffect(() => { useEffect(() => {
@@ -335,9 +353,19 @@ export function InstalledScriptsTab() {
useEffect(() => { useEffect(() => {
if (scripts.length > 0) { if (scripts.length > 0) {
console.log('Status check triggered - scripts length:', scripts.length);
fetchContainerStatuses(); 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 => ({ const scriptsWithStatus = scripts.map(script => ({
...script, ...script,

View File

@@ -93,9 +93,25 @@ export function SSHKeyInput({ value, onChange, onError, disabled = false }: SSHK
); );
if (keyLine) { if (keyLine) {
const keyType = keyLine.includes('RSA') ? 'RSA' : let keyType = 'Unknown';
keyLine.includes('ED25519') ? 'ED25519' :
keyLine.includes('ECDSA') ? 'ECDSA' : '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)`; return `${keyType} key (${keyContent.length} characters)`;
} }
@@ -142,7 +158,7 @@ export function SSHKeyInput({ value, onChange, onError, disabled = false }: SSHK
<input <input
ref={fileInputRef} ref={fileInputRef}
type="file" type="file"
accept=".pem,.key,.id_rsa,.id_ed25519,.id_ecdsa" accept=".pem,.key,.id_rsa,.id_ed25519,.id_ecdsa,ed25519,id_rsa,id_ed25519,id_ecdsa,*"
onChange={handleFileSelect} onChange={handleFileSelect}
className="hidden" className="hidden"
disabled={disabled} disabled={disabled}
@@ -153,7 +169,7 @@ export function SSHKeyInput({ value, onChange, onError, disabled = false }: SSHK
Drag and drop your SSH private key here, or click to browse Drag and drop your SSH private key here, or click to browse
</p> </p>
<p className="text-xs text-muted-foreground"> <p className="text-xs text-muted-foreground">
Supported formats: RSA, ED25519, ECDSA (.pem, .key, .id_rsa, etc.) Supported formats: RSA, ED25519, ECDSA (.pem, .key, .id_rsa, ed25519, etc.)
</p> </p>
</div> </div>
</div> </div>

View File

@@ -551,23 +551,31 @@ export const installedScriptsRouter = createTRPCRouter({
const listCommand = 'pct list'; const listCommand = 'pct list';
let listOutput = ''; let listOutput = '';
await new Promise<void>((resolve, reject) => { // Add timeout to prevent hanging connections
void sshExecutionService.executeCommand( const timeoutPromise = new Promise<never>((_, reject) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument setTimeout(() => reject(new Error('SSH command timeout after 30 seconds')), 30000);
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();
}
);
}); });
await Promise.race([
new Promise<void>((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 // Parse pct list output
const lines = listOutput.split('\n').filter(line => line.trim()); const lines = listOutput.split('\n').filter(line => line.trim());

View File

@@ -32,7 +32,9 @@ class SSHExecutionService {
const tempDir = mkdtempSync(join(tmpdir(), 'ssh-key-')); const tempDir = mkdtempSync(join(tmpdir(), 'ssh-key-'));
const tempKeyPath = join(tempDir, 'private_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 chmodSync(tempKeyPath, 0o600); // Set proper permissions
return tempKeyPath; return tempKeyPath;
@@ -295,7 +297,7 @@ class SSHExecutionService {
try { try {
unlinkSync(tempKeyPath); unlinkSync(tempKeyPath);
const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/'));
unlinkSync(tempDir); rmdirSync(tempDir);
} catch (cleanupError) { } catch (cleanupError) {
console.warn('Failed to clean up temporary SSH key file:', cleanupError); console.warn('Failed to clean up temporary SSH key file:', cleanupError);
} }
@@ -314,7 +316,7 @@ class SSHExecutionService {
try { try {
unlinkSync(tempKeyPath); unlinkSync(tempKeyPath);
const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/'));
unlinkSync(tempDir); rmdirSync(tempDir);
} catch (cleanupError) { } catch (cleanupError) {
console.warn('Failed to clean up temporary SSH key file:', cleanupError); console.warn('Failed to clean up temporary SSH key file:', cleanupError);
} }
@@ -328,7 +330,7 @@ class SSHExecutionService {
try { try {
unlinkSync(tempKeyPath); unlinkSync(tempKeyPath);
const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/'));
unlinkSync(tempDir); rmdirSync(tempDir);
} catch (cleanupError) { } catch (cleanupError) {
console.warn('Failed to clean up temporary SSH key file:', cleanupError); console.warn('Failed to clean up temporary SSH key file:', cleanupError);
} }
@@ -383,7 +385,7 @@ class SSHExecutionService {
try { try {
unlinkSync(tempKeyPath); unlinkSync(tempKeyPath);
const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/'));
unlinkSync(tempDir); rmdirSync(tempDir);
} catch (cleanupError) { } catch (cleanupError) {
console.warn('Failed to clean up temporary SSH key file:', cleanupError); console.warn('Failed to clean up temporary SSH key file:', cleanupError);
} }
@@ -414,7 +416,7 @@ class SSHExecutionService {
try { try {
unlinkSync(tempKeyPath); unlinkSync(tempKeyPath);
const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/')); const tempDir = tempKeyPath.substring(0, tempKeyPath.lastIndexOf('/'));
unlinkSync(tempDir); rmdirSync(tempDir);
} catch (cleanupError) { } catch (cleanupError) {
console.warn('Failed to clean up temporary SSH key file:', cleanupError); console.warn('Failed to clean up temporary SSH key file:', cleanupError);
} }

View File

@@ -557,7 +557,9 @@ expect {
tempKeyPath = join(tempDir, 'private_key'); tempKeyPath = join(tempDir, 'private_key');
// Write the private key to temporary file // 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 chmodSync(tempKeyPath, 0o600); // Set proper permissions
// Build SSH command // Build SSH command