github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-05-31 16:57:08 +02:00
Code Issues Actions 9 Packages Projects Releases Wiki Activity Graph

fix(Git Node): Clone repositories into target path (#30822)

Browse Source 
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Dawid Myslak 2026-05-21 08:48:04 +02:00 committed by GitHub
parent 374e7ed0b2
commit bd3aafce75
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 80 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
packages/nodes-base/nodes/Git/Git.node.ts
View File

@ -1,11 +1,12 @@
import { DeploymentConfig, SecurityConfig } from '@n8n/config';
import { Container } from '@n8n/di';
import { access, mkdir } from 'fs/promises';
import { mkdir } from 'fs/promises';
import type {
IExecuteFunctions,
INodeExecutionData,
INodeType,
INodeTypeDescription,
ResolvedFilePath,
} from 'n8n-workflow';
import {
NodeConnectionTypes,
@ -13,6 +14,7 @@ import {
assertParamIsBoolean,
assertParamIsString,
} from 'n8n-workflow';
import { basename, dirname, join } from 'path';
import type { LogOptions, SimpleGit, SimpleGitOptions } from 'simple-git';
import simpleGit from 'simple-git';
import { URL } from 'url';
@ -295,12 +297,37 @@ export class Git implements INodeType {
}
};
const isFileNotFoundError = (error: unknown) =>
error instanceof Error && 'code' in error && error.code === 'ENOENT';
const resolvePathAllowingMissingParents = async (path: string): Promise<ResolvedFilePath> => {
try {
return await this.helpers.resolvePath(path);
} catch (error) {
if (!isFileNotFoundError(error)) {
throw error;
}
const parentPath = dirname(path);
if (parentPath === path) {
throw error;
}
const resolvedParentPath = await resolvePathAllowingMissingParents(parentPath);
return join(resolvedParentPath, basename(path)) as ResolvedFilePath;
}
};
const operation = this.getNodeParameter('operation', 0);
const returnItems: INodeExecutionData[] = [];
for (let itemIndex = 0; itemIndex < items.length; itemIndex++) {
try {
const repositoryPath = this.getNodeParameter('repositoryPath', itemIndex, '') as string;
const resolvedRepositoryPath = await this.helpers.resolvePath(repositoryPath);
const resolvedRepositoryPath =
operation === 'clone'
? await resolvePathAllowingMissingParents(repositoryPath)
: await this.helpers.resolvePath(repositoryPath);
const isFilePathBlocked = this.helpers.isFilePathBlocked(resolvedRepositoryPath);
if (isFilePathBlocked) {
throw new NodeOperationError(
@ -312,12 +339,7 @@ export class Git implements INodeType {
const options = this.getNodeParameter('options', itemIndex, {});
if (operation === 'clone') {
// Create repository folder if it does not exist
try {
await access(resolvedRepositoryPath);
} catch (error) {
await mkdir(resolvedRepositoryPath);
}
await mkdir(dirname(resolvedRepositoryPath), { recursive: true });
}
const gitConfig: string[] = [];
@ -335,7 +357,7 @@ export class Git implements INodeType {
}
const gitOptions: Partial<SimpleGitOptions> = {
baseDir: resolvedRepositoryPath,
baseDir: operation === 'clone' ? dirname(resolvedRepositoryPath) : resolvedRepositoryPath,
config: gitConfig,
// simple-git blocks callers from setting `core.hooksPath` via `config`
// unless this flag is set. We set it deliberately as a mitigation, so
@ -410,7 +432,7 @@ export class Git implements INodeType {
let sourceRepository = this.getNodeParameter('sourceRepository', itemIndex, '') as string;
sourceRepository = await prepareRepository(sourceRepository);
await git.clone(sourceRepository, '.');
await git.clone(sourceRepository, resolvedRepositoryPath);
returnItems.push({
json: {

65
packages/nodes-base/nodes/Git/test/Git.node.test.ts
View File

@ -1,6 +1,6 @@
import * as fsPromises from 'fs/promises';
import { mock } from 'jest-mock-extended';
import type { IExecuteFunctions } from 'n8n-workflow';
import type { IExecuteFunctions, ResolvedFilePath } from 'n8n-workflow';
import type { SimpleGit } from 'simple-git';
import { Container } from '@n8n/di';
import { SecurityConfig } from '@n8n/config';
@ -31,9 +31,11 @@ const mockGit = {
jest.mock('simple-git', () => ({
__esModule: true,
default: () => mockGit,
default: jest.fn(() => mockGit),
}));
const mockSimpleGit = jest.requireMock<{ default: jest.Mock }>('simple-git').default;
// Mock filesystem operations
jest.mock('fs/promises', () => ({
access: jest.fn(),
@ -545,40 +547,69 @@ describe('Git Node', () => {
expect(mockGit.addConfig).toHaveBeenCalledWith('user.name', 'test user', false);
});
it('should handle clone operation and create directory when it does not exist', async () => {
it('should handle clone operation and create the parent directory', async () => {
const missingParentError = Object.assign(new Error('Directory does not exist'), {
code: 'ENOENT',
});
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('clone')
.mockReturnValueOnce('/new-repo')
.mockReturnValueOnce('/git/new-repo')
.mockReturnValueOnce({})
.mockReturnValueOnce('https://github.com/test/repo.git');
// Simulate directory not existing - access() throws
mockFsPromises.access.mockRejectedValueOnce(new Error('Directory does not exist'));
mockExecuteFunctions.helpers.resolvePath = jest
.fn()
.mockRejectedValueOnce(missingParentError)
.mockResolvedValueOnce('/git' as ResolvedFilePath);
mockFsPromises.mkdir.mockResolvedValueOnce(undefined);
const result = await gitNode.execute.call(mockExecuteFunctions);
expect(mockFsPromises.access).toHaveBeenCalledWith('/new-repo');
expect(mockFsPromises.mkdir).toHaveBeenCalledWith('/new-repo');
expect(mockGit.clone).toHaveBeenCalledWith('https://github.com/test/repo.git', '.');
expect(mockFsPromises.access).not.toHaveBeenCalled();
expect(mockExecuteFunctions.helpers.resolvePath).toHaveBeenCalledWith('/git/new-repo');
expect(mockExecuteFunctions.helpers.resolvePath).toHaveBeenCalledWith('/git');
expect(mockFsPromises.mkdir).toHaveBeenCalledWith('/git', { recursive: true });
expect(mockSimpleGit).toHaveBeenCalledWith(expect.objectContaining({ baseDir: '/git' }));
expect(mockGit.clone).toHaveBeenCalledWith(
'https://github.com/test/repo.git',
'/git/new-repo',
);
expect(result[0]).toEqual([{ json: { success: true }, pairedItem: { item: 0 } }]);
});
it('should handle clone operation when directory already exists', async () => {
it('should not create the parent directory when clone path is blocked', async () => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('clone')
.mockReturnValueOnce('/existing-repo')
.mockReturnValueOnce('/blocked/repo')
.mockReturnValueOnce({});
mockExecuteFunctions.helpers.isFilePathBlocked = jest.fn(() => true);
await expect(gitNode.execute.call(mockExecuteFunctions)).rejects.toThrow(
'Access to the repository path is not allowed',
);
expect(mockFsPromises.mkdir).not.toHaveBeenCalled();
expect(mockSimpleGit).not.toHaveBeenCalled();
expect(mockGit.clone).not.toHaveBeenCalled();
});
it('should pass the resolved repository path as the clone destination', async () => {
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('clone')
.mockReturnValueOnce('/git/existing-repo')
.mockReturnValueOnce({})
.mockReturnValueOnce('https://github.com/test/repo.git');
// Simulate directory already exists - access() succeeds
mockFsPromises.access.mockResolvedValueOnce(undefined);
await gitNode.execute.call(mockExecuteFunctions);
expect(mockFsPromises.access).toHaveBeenCalledWith('/existing-repo');
expect(mockFsPromises.mkdir).not.toHaveBeenCalled();
expect(mockGit.clone).toHaveBeenCalledWith('https://github.com/test/repo.git', '.');
expect(mockFsPromises.access).not.toHaveBeenCalled();
expect(mockFsPromises.mkdir).toHaveBeenCalledWith('/git', { recursive: true });
expect(mockSimpleGit).toHaveBeenCalledWith(expect.objectContaining({ baseDir: '/git' }));
expect(mockGit.clone).toHaveBeenCalledWith(
'https://github.com/test/repo.git',
'/git/existing-repo',
);
});
it('should handle fetch operation', async () => {