feat: Use SafePathJoin, Raw SQL queries for insertions (#20244)

This commit is contained in:
Stephen Wright 2025-10-01 11:14:48 +01:00 committed by GitHub
parent 02df05e3aa
commit af1391853b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 60 additions and 32 deletions

View File

@ -1,16 +1,16 @@
import { Command } from '@n8n/decorators';
import { z } from 'zod';
import path from 'path';
import { Container } from '@n8n/di';
import { BaseCommand } from '../base-command';
import { ExportService } from '@/services/export.service';
import { safeJoinPath } from '@n8n/backend-common';
const flagsSchema = z.object({
outputDir: z
.string()
.describe('Output directory path')
.default(path.join(__dirname, './outputs')),
.default(safeJoinPath(__dirname, './outputs')),
});
@Command({

View File

@ -1,4 +1,4 @@
import { type Logger } from '@n8n/backend-common';
import { safeJoinPath, type Logger } from '@n8n/backend-common';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { type DataSource, type EntityManager } from '@n8n/typeorm';
import { mock } from 'jest-mock-extended';
@ -13,6 +13,10 @@ jest.mock('fs/promises');
jest.mock('@/utils/compression.util');
jest.mock('@n8n/backend-common', () => ({
safeJoinPath: jest.fn(),
}));
// Mock @n8n/db
jest.mock('@n8n/db', () => ({
CredentialsRepository: mock<CredentialsRepository>(),
@ -229,6 +233,10 @@ describe('ImportService', () => {
const mockFiles = ['user.jsonl', 'workflowentity.jsonl', 'migrations.jsonl'];
jest.mocked(readdir).mockResolvedValue(mockFiles as any);
jest
.mocked(safeJoinPath)
.mockReturnValueOnce('/test/input/user.jsonl')
.mockReturnValueOnce('/test/input/workflowentity.jsonl');
const result = await importService.getImportMetadata('/test/input');
@ -245,6 +253,11 @@ describe('ImportService', () => {
const mockFiles = ['user.jsonl', 'user.2.jsonl', 'user.3.jsonl'];
jest.mocked(readdir).mockResolvedValue(mockFiles as any);
jest
.mocked(safeJoinPath)
.mockReturnValueOnce('/test/input/user.jsonl')
.mockReturnValueOnce('/test/input/user.2.jsonl')
.mockReturnValueOnce('/test/input/user.3.jsonl');
const result = await importService.getImportMetadata('/test/input');
@ -298,6 +311,8 @@ describe('ImportService', () => {
jest.mocked(readdir).mockResolvedValue(mockFiles as any);
jest.mocked(safeJoinPath).mockReturnValue('/test/input/user.jsonl');
const result = await importService.getImportMetadata('/test/input');
expect(result).toEqual({
@ -384,6 +399,10 @@ describe('ImportService', () => {
tableNames: ['user'],
};
mockDataSource.driver.escapeQueryWithParameters = jest
.fn()
.mockReturnValue(['INSERT COMMAND', { data: 'data' }]);
const mockEntities = [{ id: 1, name: 'Test User' }];
const mockContent = JSON.stringify(mockEntities[0]);
jest.mocked(readFile).mockResolvedValue(mockContent);
@ -396,7 +415,7 @@ describe('ImportService', () => {
);
expect(readFile).toHaveBeenCalledWith('/test/input/user.jsonl', 'utf8');
expect(mockEntityManager.insert).toHaveBeenCalledWith('user', mockEntities);
expect(mockEntityManager.query).toHaveBeenCalledWith('INSERT COMMAND', { data: 'data' });
});
it('should handle empty input directory', async () => {
@ -749,11 +768,7 @@ describe('ImportService', () => {
decompressFolder: mockDecompressFolder,
}));
// Mock path.join
const mockPathJoin = jest.fn().mockReturnValue(entitiesZipPath);
jest.doMock('path', () => ({
join: mockPathJoin,
}));
jest.mocked(safeJoinPath).mockReturnValue(entitiesZipPath);
// @ts-expect-error For testing purposes
await importService.decompressEntitiesZip(inputDir);

View File

@ -1,6 +1,5 @@
import { Logger } from '@n8n/backend-common';
import { Logger, safeJoinPath } from '@n8n/backend-common';
import { mkdir, rm, readdir, appendFile } from 'fs/promises';
import path from 'path';
import { Service } from '@n8n/di';
@ -29,7 +28,7 @@ export class ExportService {
` 🗑️ Found ${entityFiles.length} existing file(s) for ${entityName}, deleting...`,
);
for (const file of entityFiles) {
await rm(path.join(outputDir, file));
await rm(safeJoinPath(outputDir, file));
this.logger.info(` Deleted: ${file}`);
}
}
@ -62,7 +61,7 @@ export class ExportService {
const allMigrations = await this.dataSource.query(`SELECT * FROM ${formattedTableName}`);
const fileName = 'migrations.jsonl';
const filePath = path.join(outputDir, fileName);
const filePath = safeJoinPath(outputDir, fileName);
const migrationsJsonl: string = allMigrations
.map((migration: unknown) => JSON.stringify(migration))
@ -150,7 +149,7 @@ export class ExportService {
const targetFileIndex = Math.floor(totalEntityCount / entitiesPerFile) + 1;
const fileName =
targetFileIndex === 1 ? `${entityName}.jsonl` : `${entityName}.${targetFileIndex}.jsonl`;
const filePath = path.join(outputDir, fileName);
const filePath = safeJoinPath(outputDir, fileName);
// If we've moved to a new file, log the completion of the previous file
if (targetFileIndex > fileIndex) {
@ -195,7 +194,7 @@ export class ExportService {
}
// Compress the output directory to entities.zip
const zipPath = path.join(outputDir, 'entities.zip');
const zipPath = safeJoinPath(outputDir, 'entities.zip');
this.logger.info(`\n🗜 Compressing export to ${zipPath}...`);
await compressFolder(outputDir, zipPath, {
@ -209,7 +208,7 @@ export class ExportService {
const files = await readdir(outputDir);
for (const file of files) {
if (file.endsWith('.jsonl') && file !== 'entities.zip') {
await rm(path.join(outputDir, file));
await rm(safeJoinPath(outputDir, file));
}
}

View File

@ -1,4 +1,4 @@
import { Logger } from '@n8n/backend-common';
import { Logger, safeJoinPath } from '@n8n/backend-common';
import type { TagEntity, ICredentialsDb, IWorkflowDb } from '@n8n/db';
import {
Project,
@ -14,12 +14,12 @@ import { Service } from '@n8n/di';
import { type INode, type INodeCredentialsDetails, type IWorkflowBase } from 'n8n-workflow';
import { v4 as uuid } from 'uuid';
import { readdir, readFile } from 'fs/promises';
import path from 'path';
import { replaceInvalidCredentials } from '@/workflow-helpers';
import { validateDbTypeForImportEntities } from '@/utils/validate-database-type';
import { Cipher } from 'n8n-core';
import { decompressFolder } from '@/utils/compression.util';
import { z } from 'zod';
@Service()
export class ImportService {
@ -228,7 +228,7 @@ export class ImportService {
if (!entityFiles[entityName]) {
entityFiles[entityName] = [];
}
entityFiles[entityName].push(path.join(inputDir, file));
entityFiles[entityName].push(safeJoinPath(inputDir, file));
}
}
@ -243,9 +243,10 @@ export class ImportService {
* @param filePath - Path to the JSONL file
* @returns Array of parsed entity objects
*/
async readEntityFile(filePath: string): Promise<unknown[]> {
async readEntityFile(filePath: string): Promise<Array<Record<string, unknown>>> {
const content = await readFile(filePath, 'utf8');
const entities: unknown[] = [];
const entities: Record<string, unknown>[] = [];
const entitySchema = z.record(z.string(), z.unknown());
for (const block of content.split('\n')) {
const lines = this.cipher.decrypt(block).split(/\r?\n/);
@ -256,7 +257,7 @@ export class ImportService {
if (!line) continue;
try {
entities.push(JSON.parse(line));
entities.push(entitySchema.parse(JSON.parse(line)));
} catch (error: unknown) {
// If parsing fails, it might be because the JSON spans multiple lines
// This shouldn't happen in proper JSONL, but let's handle it gracefully
@ -273,7 +274,7 @@ export class ImportService {
}
private async decompressEntitiesZip(inputDir: string): Promise<void> {
const entitiesZipPath = path.join(inputDir, 'entities.zip');
const entitiesZipPath = safeJoinPath(inputDir, 'entities.zip');
const { existsSync } = await import('fs');
if (!existsSync(entitiesZipPath)) {
@ -331,7 +332,7 @@ export class ImportService {
const files = await readdir(inputDir);
for (const file of files) {
if (file.endsWith('.jsonl') && file !== 'entities.zip') {
await rm(path.join(inputDir, file));
await rm(safeJoinPath(inputDir, file));
this.logger.info(` Removed: ${file}`);
}
}
@ -378,21 +379,34 @@ export class ImportService {
return;
}
const tableName = entityMetadata.tableName;
const tableName = this.dataSource.driver.escape(entityMetadata.tableName);
this.logger.info(` 📋 Target table: ${tableName}`);
let entityCount = 0;
await Promise.all(
files.map(async (filePath) => {
this.logger.info(` 📁 Reading file: ${path.basename(filePath)}`);
this.logger.info(` 📁 Reading file: ${filePath}`);
const entities = await this.readEntityFile(filePath);
const entities: Array<Record<string, unknown>> = await this.readEntityFile(filePath);
this.logger.info(` Found ${entities.length} entities`);
if (entities.length > 0) {
await transactionManager.insert(tableName, entities);
entityCount += entities.length;
}
await Promise.all(
entities.map(async (entity) => {
const columns = Object.keys(entity);
const columnNames = columns.map(this.dataSource.driver.escape).join(', ');
const columnValues = columns.map((key) => `:${key}`).join(', ');
const [query, parameters] = this.dataSource.driver.escapeQueryWithParameters(
`INSERT INTO ${tableName} (${columnNames}) VALUES (${columnValues})`,
entity,
{},
);
await transactionManager.query(query, parameters);
}),
);
entityCount += entities.length;
}),
);
@ -461,7 +475,7 @@ export class ImportService {
* @returns Promise that resolves if migrations match, throws error if they don't
*/
async validateMigrations(inputDir: string): Promise<void> {
const migrationsFilePath = path.join(inputDir, 'migrations.jsonl');
const migrationsFilePath = safeJoinPath(inputDir, 'migrations.jsonl');
try {
// Check if migrations file exists