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 10 Packages Projects Releases Wiki Activity Graph

fix(core): Safely handle paths at Data Table CSV imports (no-changelog) (#22139)

Browse Source
This commit is contained in:
Jaakko Husso 2025-11-21 13:58:41 +02:00 committed by GitHub
parent ed2dea674e
commit 94505bfad4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 98 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
packages/cli/src/modules/data-table/__tests__/csv-parser.service.test.ts Normal file
View File

@ -0,0 +1,78 @@
import { testModules } from '@n8n/backend-test-utils';
import type { GlobalConfig } from '@n8n/config';
import { mock } from 'jest-mock-extended';
import { CsvParserService } from '../csv-parser.service';
beforeAll(async () => {
await testModules.loadModules(['data-table']);
});
describe('CsvParserService', () => {
describe('parseFile', () => {
it('should not allow path traversal when parsing CSV file metadata', async () => {
const globalConfig = mock<GlobalConfig>({
dataTable: {
uploadDir: '/safe/upload/dir',
},
});
const csvParserService = new CsvParserService(globalConfig);
const maliciousFileId = '../some/other/directory/malicious-file.csv';
await expect(csvParserService.parseFile(maliciousFileId)).rejects.toThrowError(
'Path traversal detected',
);
});
it('should try to access file if it is within upload directory', async () => {
const globalConfig = mock<GlobalConfig>({
dataTable: {
uploadDir: '/safe/upload/dir',
},
});
const csvParserService = new CsvParserService(globalConfig);
const safeFileId = 'valid-file.csv';
// Since we are not actually testing file reading here, just ensure no error is thrown
await expect(csvParserService.parseFile(safeFileId)).rejects.toThrowError(
"ENOENT: no such file or directory, open '/safe/upload/dir/valid-file.csv",
);
});
});
describe('parseFileData', () => {
it('should not allow path traversal when parsing CSV file data', async () => {
const globalConfig = mock<GlobalConfig>({
dataTable: {
uploadDir: '/safe/upload/dir',
},
});
const csvParserService = new CsvParserService(globalConfig);
const maliciousFileId = '../some/other/directory/malicious-file.csv';
await expect(csvParserService.parseFileData(maliciousFileId)).rejects.toThrowError(
'Path traversal detected',
);
});
it('should try to access file if it is within upload directory', async () => {
const globalConfig = mock<GlobalConfig>({
dataTable: {
uploadDir: '/safe/upload/dir',
},
});
const csvParserService = new CsvParserService(globalConfig);
const safeFileId = 'valid-file.csv';
// Since we are not actually testing file reading here, just ensure no error is thrown
await expect(csvParserService.parseFileData(safeFileId)).rejects.toThrowError(
"ENOENT: no such file or directory, open '/safe/upload/dir/valid-file.csv",
);
});
});
});

8
packages/cli/src/modules/data-table/__tests__/data-table-file-cleanup.service.test.ts
View File

@ -66,6 +66,14 @@ describe('DataTableFileCleanupService', () => {
await expect(service.deleteFile(fileId)).rejects.toThrow('Permission denied');
});
it('should not allow path traversal when deleting file', async () => {
const maliciousFileId = '../some/other/directory/malicious-file.csv';
await expect(service.deleteFile(maliciousFileId)).rejects.toThrowError(
'Path traversal detected',
);
});
});
describe('start and shutdown', () => {

20
packages/cli/src/modules/data-table/csv-parser.service.ts
View File

@ -1,8 +1,8 @@
import { safeJoinPath } from '@n8n/backend-common';
import { GlobalConfig } from '@n8n/config';
import { Service } from '@n8n/di';
import { parse } from 'csv-parse';
import { createReadStream } from 'fs';
import path from 'path';
export interface CsvColumnMetadata {
name: string;
@ -44,7 +44,7 @@ export class CsvParserService {
* Parses a CSV file and returns metadata including row count, column count, and inferred column types
*/
async parseFile(fileId: string, hasHeaders: boolean = true): Promise<CsvMetadata> {
const filePath = path.join(this.uploadDir, fileId);
const filePath = safeJoinPath(this.uploadDir, fileId);
let rowCount = 0;
let firstDataRow: Record<string, string> | null = null;
let columnNames: string[] = [];
@ -58,10 +58,7 @@ export class CsvParserService {
}
: false,
skip_empty_lines: true,
});
createReadStream(filePath)
.pipe(parser)
})
.on('data', (row: Record<string, string> | string[]) => {
rowCount++;
@ -90,6 +87,8 @@ export class CsvParserService {
});
})
.on('error', reject);
createReadStream(filePath).on('error', reject).pipe(parser);
});
}
@ -100,7 +99,7 @@ export class CsvParserService {
fileId: string,
hasHeaders: boolean = true,
): Promise<Array<Record<string, string>>> {
const filePath = path.join(this.uploadDir, fileId);
const filePath = safeJoinPath(this.uploadDir, fileId);
const rows: Array<Record<string, string>> = [];
let columnNames: string[] = [];
@ -109,10 +108,7 @@ export class CsvParserService {
const parser = parse({
columns: hasHeaders ? true : false,
skip_empty_lines: true,
});
createReadStream(filePath)
.pipe(parser)
})
.on('data', (row: Record<string, string> | string[]) => {
if (!hasHeaders && Array.isArray(row)) {
const processed = this.processRowWithoutHeaders(row, columnNames);
@ -126,6 +122,8 @@ export class CsvParserService {
resolve(rows);
})
.on('error', reject);
createReadStream(filePath).on('error', reject).pipe(parser);
});
}

6
packages/cli/src/modules/data-table/data-table-file-cleanup.service.ts
View File

@ -1,7 +1,7 @@
import { safeJoinPath } from '@n8n/backend-common';
import { GlobalConfig } from '@n8n/config';
import { Service } from '@n8n/di';
import { promises as fs } from 'fs';
import path from 'path';
@Service()
export class DataTableFileCleanupService {
@ -47,7 +47,7 @@ export class DataTableFileCleanupService {
const maxAge = this.globalConfig.dataTable.fileMaxAgeMs;
for (const file of files) {
const filePath = path.join(this.uploadDir, file);
const filePath = safeJoinPath(this.uploadDir, file);
try {
const stats = await fs.stat(filePath);
const fileAge = now - stats.mtimeMs;
@ -74,7 +74,7 @@ export class DataTableFileCleanupService {
* Deletes a specific CSV file by its fileId
*/
async deleteFile(fileId: string): Promise<void> {
const filePath = path.join(this.uploadDir, fileId);
const filePath = safeJoinPath(this.uploadDir, fileId);
try {
await fs.unlink(filePath);
} catch (error) {