fix(core): Resolve SSH path issues on Windows (#18889)

packages/cli/src/environments.ee/source-control/__tests__/source-control-git.service.test.ts

@@ -5,6 +5,7 @@ import type { SimpleGit } from 'simple-git';
 
 import { SourceControlGitService } from '../source-control-git.service.ee';
 import type { SourceControlPreferences } from '../types/source-control-preferences';
+import type { SourceControlPreferencesService } from '../source-control-preferences.service.ee';
 
 const MOCK_BRANCHES = {
 	all: ['origin/master', 'origin/feature/branch'],
@@ -15,18 +16,21 @@ const MOCK_BRANCHES = {
 	current: 'master',
 };
 
+const mockGitInstance = {
+	branch: jest.fn().mockResolvedValue(MOCK_BRANCHES),
+	env: jest.fn().mockReturnThis(),
+};
+
 jest.mock('simple-git', () => {
 	return {
-		simpleGit: jest.fn().mockImplementation(() => ({
-			branch: jest.fn().mockResolvedValue(MOCK_BRANCHES),
-		})),
+		simpleGit: jest.fn().mockImplementation(() => mockGitInstance),
 	};
 });
 
 describe('SourceControlGitService', () => {
 	const sourceControlGitService = new SourceControlGitService(mock(), mock(), mock());
 
-	beforeAll(() => {
+	beforeEach(() => {
 		sourceControlGitService.git = simpleGit();
 	});
 
@@ -105,4 +109,100 @@ describe('SourceControlGitService', () => {
 			expect(content).toBe(expectedContent);
 		});
 	});
+
+	describe('path normalization', () => {
+		describe('cross-platform path handling', () => {
+			beforeEach(() => {
+				jest.clearAllMocks();
+			});
+
+			it('should normalize Windows paths to POSIX format for SSH command', async () => {
+				// Arrange
+				const mockPreferencesService = mock<SourceControlPreferencesService>();
+				const windowsPath = 'C:\\Users\\Test\\.n8n\\ssh_private_key_temp';
+				const sshFolder = 'C:\\Users\\Test\\.n8n\\.ssh';
+
+				// Mock the getPrivateKeyPath to return a Windows path
+				mockPreferencesService.getPrivateKeyPath.mockResolvedValue(windowsPath);
+
+				const gitService = new SourceControlGitService(mock(), mock(), mockPreferencesService);
+
+				// Act
+				await gitService.setGitSshCommand('/git/folder', sshFolder);
+
+				// Assert - verify Windows paths are normalized to POSIX format
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('C:/Users/Test/.n8n/ssh_private_key_temp'), // Forward slashes
+				);
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('C:/Users/Test/.n8n/.ssh/known_hosts'), // Forward slashes
+				);
+				// Ensure no backslashes remain in the SSH command
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.not.stringContaining('\\'),
+				);
+			});
+
+			it('should create properly quoted SSH command', async () => {
+				// Arrange
+				const mockPreferencesService = mock<SourceControlPreferencesService>();
+				const privateKeyPath = 'C:/Users/Test User/.n8n/ssh_private_key_temp';
+				const sshFolder = 'C:/Users/Test User/.n8n/.ssh';
+
+				// Mock the getPrivateKeyPath to return a path with spaces
+				mockPreferencesService.getPrivateKeyPath.mockResolvedValue(privateKeyPath);
+
+				const gitService = new SourceControlGitService(mock(), mock(), mockPreferencesService);
+
+				// Act
+				await gitService.setGitSshCommand('/git/folder', sshFolder);
+
+				// Assert - verify paths with spaces are properly quoted
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('"C:/Users/Test User/.n8n/ssh_private_key_temp"'), // Quoted path with spaces
+				);
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('"C:/Users/Test User/.n8n/.ssh/known_hosts"'), // Quoted known_hosts path
+				);
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('UserKnownHostsFile='),
+				);
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('StrictHostKeyChecking=no'),
+				);
+			});
+
+			it('should escape double quotes in paths to prevent command injection', async () => {
+				// Arrange
+				const mockPreferencesService = mock<SourceControlPreferencesService>();
+				const pathWithQuotes = 'C:/Users/Test"User/.n8n/ssh_private_key_temp';
+				const sshFolder = 'C:/Users/Test"User/.n8n/.ssh';
+
+				// Mock the getPrivateKeyPath to return a path with quotes
+				mockPreferencesService.getPrivateKeyPath.mockResolvedValue(pathWithQuotes);
+
+				const gitService = new SourceControlGitService(mock(), mock(), mockPreferencesService);
+
+				// Act
+				await gitService.setGitSshCommand('/git/folder', sshFolder);
+
+				// Assert - verify the SSH command was properly escaped
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.stringContaining('Test\\"User'), // Escaped quote
+				);
+				expect(mockGitInstance.env).toHaveBeenCalledWith(
+					'GIT_SSH_COMMAND',
+					expect.not.stringContaining('Test"User'), // No unescaped quote in final command
+				);
+			});
+		});
+	});
 });

packages/cli/src/environments.ee/source-control/__tests__/source-control-preferences.service.ee.test.ts

@@ -1,9 +1,16 @@
 import { mock } from 'jest-mock-extended';
-import type { InstanceSettings } from 'n8n-core';
+import type { InstanceSettings, Cipher } from 'n8n-core';
+import { readFile, access, mkdir } from 'fs/promises';
+import os from 'os';
+import path from 'path';
 
 import { SourceControlPreferencesService } from '../source-control-preferences.service.ee';
 import type { SourceControlPreferences } from '../types/source-control-preferences';
 
+// Restore real fs modules for these tests since we need actual file operations
+jest.unmock('node:fs');
+jest.unmock('node:fs/promises');
+
 describe('SourceControlPreferencesService', () => {
 	const instanceSettings = mock<InstanceSettings>({ n8nFolder: '' });
 	const service = new SourceControlPreferencesService(
@@ -24,4 +31,212 @@ describe('SourceControlPreferencesService', () => {
 		const validationResult = await service.validateSourceControlPreferences(validPreferences);
 		expect(validationResult).toBeTruthy();
 	});
+
+	describe('line ending normalization', () => {
+		let tempDir: string;
+
+		beforeEach(async () => {
+			tempDir = path.join(os.tmpdir(), 'n8n-test-' + Date.now());
+			await mkdir(tempDir, { recursive: true });
+		});
+
+		it('should normalize CRLF line endings to LF when writing private key', async () => {
+			// Arrange
+			const keyWithCRLF =
+				'-----BEGIN OPENSSH PRIVATE KEY-----\r\ntest\r\nkey\r\ndata\r\n-----END OPENSSH PRIVATE KEY-----\r\n';
+			const expectedNormalizedKey = keyWithCRLF.replace(/\r\n/g, '\n').replace(/\r/g, '\n');
+
+			const mockCipher = mock<Cipher>();
+			mockCipher.decrypt.mockReturnValue(keyWithCRLF);
+
+			const instanceSettings = mock<InstanceSettings>({ n8nFolder: tempDir });
+			const service = new SourceControlPreferencesService(
+				instanceSettings,
+				mock(),
+				mockCipher,
+				mock(),
+				mock(),
+			);
+
+			// Mock the getKeyPairFromDatabase method to return a key pair
+			jest.spyOn(service as any, 'getKeyPairFromDatabase').mockResolvedValue({
+				encryptedPrivateKey: 'encrypted',
+				publicKey: 'public',
+			});
+
+			// Act
+			const tempFilePath = await service.getPrivateKeyPath();
+
+			// Assert - check the actual file content has normalized line endings
+			const fileContent = await readFile(tempFilePath, 'utf8');
+			expect(fileContent).not.toContain('\r');
+			expect(fileContent).toBe(expectedNormalizedKey);
+		});
+
+		it('should normalize mixed CR and CRLF line endings to LF when writing private key', async () => {
+			// Arrange
+			const keyWithMixedEndings =
+				'-----BEGIN OPENSSH PRIVATE KEY-----\r\ntest\rkey\r\ndata\r-----END OPENSSH PRIVATE KEY-----\n';
+
+			const mockCipher = mock<Cipher>();
+			mockCipher.decrypt.mockReturnValue(keyWithMixedEndings);
+
+			const instanceSettings = mock<InstanceSettings>({ n8nFolder: tempDir });
+			const service = new SourceControlPreferencesService(
+				instanceSettings,
+				mock(),
+				mockCipher,
+				mock(),
+				mock(),
+			);
+
+			// Mock the getKeyPairFromDatabase method
+			jest.spyOn(service as any, 'getKeyPairFromDatabase').mockResolvedValue({
+				encryptedPrivateKey: 'encrypted',
+				publicKey: 'public',
+			});
+
+			// Act
+			const tempFilePath = await service.getPrivateKeyPath();
+
+			// Assert - check the actual file content has normalized line endings
+			const fileContent = await readFile(tempFilePath, 'utf8');
+			expect(fileContent).not.toContain('\r');
+			expect(fileContent).toBe(
+				'-----BEGIN OPENSSH PRIVATE KEY-----\ntest\nkey\ndata\n-----END OPENSSH PRIVATE KEY-----\n',
+			);
+		});
+
+		it('should leave existing LF line endings unchanged when writing private key', async () => {
+			// Arrange
+			const keyWithLF =
+				'-----BEGIN OPENSSH PRIVATE KEY-----\ntest\nkey\ndata\n-----END OPENSSH PRIVATE KEY-----\n';
+
+			const mockCipher = mock<Cipher>();
+			mockCipher.decrypt.mockReturnValue(keyWithLF);
+
+			const instanceSettings = mock<InstanceSettings>({ n8nFolder: tempDir });
+			const service = new SourceControlPreferencesService(
+				instanceSettings,
+				mock(),
+				mockCipher,
+				mock(),
+				mock(),
+			);
+
+			// Mock the getKeyPairFromDatabase method
+			jest.spyOn(service as any, 'getKeyPairFromDatabase').mockResolvedValue({
+				encryptedPrivateKey: 'encrypted',
+				publicKey: 'public',
+			});
+
+			// Act
+			const tempFilePath = await service.getPrivateKeyPath();
+
+			// Assert - check the actual file content remains unchanged
+			const fileContent = await readFile(tempFilePath, 'utf8');
+			expect(fileContent).toBe(keyWithLF);
+		});
+	});
+
+	describe('file security and permissions', () => {
+		let tempDir: string;
+
+		beforeEach(async () => {
+			tempDir = path.join(os.tmpdir(), 'n8n-test-' + Date.now());
+			await mkdir(tempDir, { recursive: true });
+		});
+
+		it('should always use restrictive permissions for SSH private keys', async () => {
+			// Arrange
+			const testKey =
+				'-----BEGIN OPENSSH PRIVATE KEY-----\ntest-key-content\n-----END OPENSSH PRIVATE KEY-----\n';
+
+			const mockCipher = mock<Cipher>();
+			mockCipher.decrypt.mockReturnValue(testKey);
+
+			const instanceSettings = mock<InstanceSettings>({ n8nFolder: tempDir });
+			const service = new SourceControlPreferencesService(
+				instanceSettings,
+				mock(),
+				mockCipher,
+				mock(),
+				mock(),
+			);
+
+			// Mock the getKeyPairFromDatabase method
+			jest.spyOn(service as any, 'getKeyPairFromDatabase').mockResolvedValue({
+				encryptedPrivateKey: 'encrypted',
+				publicKey: 'public',
+			});
+
+			// Act
+			const tempFilePath = await service.getPrivateKeyPath();
+
+			// Assert - check the actual file permissions
+			const fs = require('fs');
+			const stats = await fs.promises.stat(tempFilePath);
+			const permissions = (stats.mode & parseInt('777', 8)).toString(8);
+
+			expect(permissions).toBe('600'); // Owner read/write only
+			expect(stats.mode & 0o077).toBe(0); // Group and others have no permissions
+		});
+
+		it('should fail securely when file cannot be created', async () => {
+			// Arrange
+			const instanceSettings = mock<InstanceSettings>({ n8nFolder: '/nonexistent/path' });
+			const service = new SourceControlPreferencesService(
+				instanceSettings,
+				mock(),
+				mock(),
+				mock(),
+				mock(),
+			);
+
+			const testKey =
+				'-----BEGIN OPENSSH PRIVATE KEY-----\ntest-key-content\n-----END OPENSSH PRIVATE KEY-----\n';
+			jest.spyOn(service as any, 'getPrivateKeyFromDatabase').mockResolvedValue(testKey);
+
+			// Act & Assert - should throw UnexpectedError when file creation fails
+			await expect(service.getPrivateKeyPath()).rejects.toThrow(
+				'Failed to create SSH private key file',
+			);
+		});
+
+		it('should remove existing file before creating new one with fsRm force option', async () => {
+			// Arrange
+			const testKey =
+				'-----BEGIN OPENSSH PRIVATE KEY-----\ntest-key-content\n-----END OPENSSH PRIVATE KEY-----\n';
+
+			const mockCipher = mock<Cipher>();
+			mockCipher.decrypt.mockReturnValue(testKey);
+
+			const instanceSettings = mock<InstanceSettings>({ n8nFolder: tempDir });
+			const service = new SourceControlPreferencesService(
+				instanceSettings,
+				mock(),
+				mockCipher,
+				mock(),
+				mock(),
+			);
+
+			// Mock the getKeyPairFromDatabase method
+			jest.spyOn(service as any, 'getKeyPairFromDatabase').mockResolvedValue({
+				encryptedPrivateKey: 'encrypted',
+				publicKey: 'public',
+			});
+
+			// Create a test file first
+			const tempFilePath1 = await service.getPrivateKeyPath();
+			expect(await access(tempFilePath1)).toBeUndefined(); // File exists
+
+			// Act - call again to test file removal and recreation
+			const tempFilePath2 = await service.getPrivateKeyPath();
+
+			// Assert - should succeed without errors (fsRm with force: true handles existing file)
+			expect(tempFilePath2).toBe(tempFilePath1);
+			const fileContent = await readFile(tempFilePath2, 'utf8');
+			expect(fileContent).toBe(testKey);
+		});
+	});
 });

packages/cli/src/environments.ee/source-control/source-control-git.service.ee.ts

@@ -106,7 +106,18 @@ export class SourceControlGitService {
 		const privateKeyPath = await this.sourceControlPreferencesService.getPrivateKeyPath();
 
 		const sshKnownHosts = path.join(sshFolder, 'known_hosts');
-		const sshCommand = `ssh -o UserKnownHostsFile=${sshKnownHosts} -o StrictHostKeyChecking=no -i ${privateKeyPath}`;
+
+		// Convert paths to POSIX format for SSH command (works cross-platform)
+		// Use regex to handle both Windows (\) and POSIX (/) separators regardless of current platform
+		const normalizedPrivateKeyPath = privateKeyPath.split(/[/\\]/).join('/');
+		const normalizedKnownHostsPath = sshKnownHosts.split(/[/\\]/).join('/');
+
+		// Escape double quotes to prevent command injection
+		const escapedPrivateKeyPath = normalizedPrivateKeyPath.replace(/"/g, '\\"');
+		const escapedKnownHostsPath = normalizedKnownHostsPath.replace(/"/g, '\\"');
+
+		// Quote paths to handle spaces and special characters
+		const sshCommand = `ssh -o UserKnownHostsFile="${escapedKnownHostsPath}" -o StrictHostKeyChecking=no -i "${escapedPrivateKeyPath}"`;
 
 		this.gitOptions = {
 			baseDir: gitFolder,

packages/cli/src/environments.ee/source-control/source-control-preferences.service.ee.ts

@@ -6,7 +6,7 @@ import { validate } from 'class-validator';
 import { rm as fsRm } from 'fs/promises';
 import { Cipher, InstanceSettings } from 'n8n-core';
 import { jsonParse, UnexpectedError } from 'n8n-workflow';
-import { writeFile, chmod, readFile } from 'node:fs/promises';
+import { writeFile, readFile } from 'node:fs/promises';
 import path from 'path';
 
 import {
@@ -96,9 +96,23 @@ export class SourceControlPreferencesService {
 
 		const tempFilePath = path.join(this.instanceSettings.n8nFolder, 'ssh_private_key_temp');
 
-		await writeFile(tempFilePath, dbPrivateKey);
+		// Ensure proper line endings (LF only) for SSH keys, especially on Windows
+		const normalizedKey = dbPrivateKey.replace(/\r\n/g, '\n').replace(/\r/g, '\n');
 
-		await chmod(tempFilePath, 0o600);
+		try {
+			// Remove existing file if it exists to avoid permission conflicts
+			// Using force: true ignores ENOENT but allows other errors to surface
+			await fsRm(tempFilePath, { force: true });
+
+			// Always use restrictive permissions for SSH private keys (security requirement)
+			await writeFile(tempFilePath, normalizedKey, { mode: 0o600 });
+		} catch (error) {
+			this.logger.error('Failed to write SSH private key to temp file', {
+				tempFilePath,
+				error: error instanceof Error ? error.message : String(error),
+			});
+			throw new UnexpectedError('Failed to create SSH private key file', { cause: error });
+		}
 
 		return tempFilePath;
 	}