github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-05-28 07:17:04 +02:00
Code Issues Actions 65 Packages Projects Releases Wiki Activity Graph

chore: Bundle/2.x (#30359)

Browse Source 
Co-authored-by: Matsu <matias.huhta@n8n.io>
Co-authored-by: RomanDavydchuk <roman.davydchuk@n8n.io>
Co-authored-by: Dawid Myslak <dawid.myslak@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Bernhard Wittmann <bernhard.wittmann@n8n.io>
Co-authored-by: Daria <daria.staferova@n8n.io>
Co-authored-by: Andreas Fitzek <andreas.fitzek@n8n.io>
This commit is contained in:
n8n-assistant[bot] 2026-05-13 08:23:48 +03:00 committed by GitHub
parent 3dda7b706a
commit 293d5afd64
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 936 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
packages/@n8n/api-types/src/index.ts
View File

@ -221,6 +221,7 @@ export {
type DataTableListSortBy,
dateTimeSchema,
dataTableColumnNameSchema,
dataTableIdSchema,
} from './schemas/data-table.schema';
export type {

5
packages/@n8n/api-types/src/schemas/data-table.schema.ts
View File

@ -5,7 +5,10 @@ import type { ListDataTableQueryDto } from '../dto';
export const insertRowReturnType = z.union([z.literal('all'), z.literal('count'), z.literal('id')]);
export const dataTableNameSchema = z.string().trim().min(1).max(128);
export const dataTableIdSchema = z.string().max(36);
export const dataTableIdSchema = z
.string()
.max(36)
.regex(/^[a-zA-Z0-9]+$/);
// Postgres does not allow leading numbers or -
export const DATA_TABLE_COLUMN_REGEX = /^[a-zA-Z][a-zA-Z0-9_]*$/;

4
packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts
View File

@ -31,7 +31,7 @@ describe('OAuth1CredentialController', () => {
describe('getAuthUri', () => {
it('should return a valid auth URI', async () => {
const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
oauthService.getCredential.mockResolvedValueOnce(mockResolvedCredential);
oauthService.getCredentialForUpdate.mockResolvedValueOnce(mockResolvedCredential);
oauthService.generateAOauth1AuthUri.mockResolvedValueOnce(
'https://example.domain/oauth/authorize?oauth_token=random-token',
);
@ -90,7 +90,7 @@ describe('OAuth1CredentialController', () => {
createdAt: timestamp,
data: 'encrypted-data',
};
oauthService.getCredential.mockResolvedValueOnce(mockResolvedCredential);
oauthService.getCredentialForUpdate.mockResolvedValueOnce(mockResolvedCredential);
// @ts-ignore
oauthService.getDecryptedData.mockResolvedValue({ csrfSecret: 'invalid' });
oauthService.getOAuthCredentials.mockResolvedValueOnce({

2
packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts
View File

@ -49,7 +49,7 @@ describe('OAuth2CredentialController', () => {
);
const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
oauthService.getCredential.mockResolvedValueOnce(mockResolvedCredential);
oauthService.getCredentialForUpdate.mockResolvedValueOnce(mockResolvedCredential);
oauthService.getOAuthCredentials.mockResolvedValueOnce({
clientId: 'client_id',
clientSecret: 'client_secret',

2
packages/cli/src/controllers/oauth/oauth1-credential.controller.ts
View File

@ -22,7 +22,7 @@ export class OAuth1CredentialController {
/** Get Authorization url */
@Get('/auth')
async getAuthUri(req: OAuthRequest.OAuth1Credential.Auth): Promise<string> {
const credential = await this.oauthService.getCredential(req);
const credential = await this.oauthService.getCredentialForUpdate(req);
const uri = await this.oauthService.generateAOauth1AuthUri(credential, {
cid: credential.id,

2
packages/cli/src/controllers/oauth/oauth2-credential.controller.ts
View File

@ -26,7 +26,7 @@ export class OAuth2CredentialController {
/** Get Authorization url */
@Get('/auth')
async getAuthUri(req: OAuthRequest.OAuth2Credential.Auth): Promise<string> {
const credential = await this.oauthService.getCredential(req);
const credential = await this.oauthService.getCredentialForUpdate(req);
const uri = await this.oauthService.generateAOauth2AuthUri(credential, {
cid: credential.id,

5
packages/cli/src/modules/data-table/utils/sql-utils.ts
View File

@ -1,5 +1,6 @@
import {
dataTableColumnNameSchema,
dataTableIdSchema,
DATA_TABLE_COLUMN_ERROR_MESSAGE,
type DataTableCreateColumnSchema,
} from '@n8n/api-types';
@ -79,6 +80,10 @@ export function isValidColumnName(name: string) {
return dataTableColumnNameSchema.safeParse(name).success;
}
export function isValidDataTableId(id: string) {
return dataTableIdSchema.safeParse(id).success;
}
export function addColumnQuery(
tableName: DataTableUserTableName,
column: DataTableCreateColumnSchema,

106
packages/cli/src/modules/source-control.ee/__tests__/source-control-import.service.ee.test.ts
View File

@ -2746,7 +2746,7 @@ describe('SourceControlImportService', () => {
id: 'dt1',
name: 'Test Table 1',
projectId: 'project1',
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
@ -2754,7 +2754,7 @@ describe('SourceControlImportService', () => {
id: 'dt2',
name: 'Test Table 2',
projectId: 'project2',
columns: [{ id: 'col2', name: 'Column 2', type: 'number', index: 0 }],
columns: [{ id: 'col2', name: 'Column2', type: 'number', index: 0 }],
createdAt: '2024-01-03T00:00:00.000Z',
updatedAt: '2024-01-04T00:00:00.000Z',
};
@ -2824,7 +2824,7 @@ describe('SourceControlImportService', () => {
id: 'dt1',
name: 'Test Table',
projectId: 'project1',
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: new Date('2024-01-01'),
updatedAt: new Date('2024-01-02'),
project: {
@ -2851,7 +2851,7 @@ describe('SourceControlImportService', () => {
projectId: 'project1',
projectName: 'Team Project 1',
},
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
filename: expect.stringContaining('dt1.json'),
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
@ -2960,7 +2960,7 @@ describe('SourceControlImportService', () => {
teamId: 'project1',
teamName: 'Team Project 1',
},
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
@ -3003,7 +3003,7 @@ describe('SourceControlImportService', () => {
projectName: 'User Name',
personalEmail: 'user@example.com',
},
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
@ -3041,8 +3041,8 @@ describe('SourceControlImportService', () => {
teamName: 'Team Project 1',
},
columns: [
{ id: 'col1', name: 'Column 1', type: 'string', index: 0 },
{ id: 'col2', name: 'Column 2', type: 'number', index: 1 },
{ id: 'col1', name: 'Column1', type: 'string', index: 0 },
{ id: 'col2', name: 'Column2', type: 'number', index: 1 },
],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
@ -3052,12 +3052,12 @@ describe('SourceControlImportService', () => {
id: 'dt1',
name: 'Old Name',
projectId: 'project1',
columns: [{ id: 'col1', name: 'Column 1' }],
columns: [{ id: 'col1', name: 'Column1' }],
};
fsReadFile.mockResolvedValue(JSON.stringify(mockDataTable) as any);
dataTableRepository.findOne.mockResolvedValue(existingTable as any);
dataTableColumnRepository.find.mockResolvedValue([{ id: 'col1', name: 'Column 1' }] as any);
dataTableColumnRepository.find.mockResolvedValue([{ id: 'col1', name: 'Column1' }] as any);
dataTableColumnRepository.save.mockImplementation(async (col: any) => col);
projectRepository.findOne.mockResolvedValue({ id: 'project1', type: 'team' } as any);
@ -3126,7 +3126,7 @@ describe('SourceControlImportService', () => {
teamId: 'project1',
teamName: 'Team Project 1',
},
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
@ -3136,16 +3136,16 @@ describe('SourceControlImportService', () => {
name: 'Test Table',
projectId: 'project1',
columns: [
{ id: 'col1', name: 'Column 1' },
{ id: 'col2', name: 'Column 2' },
{ id: 'col1', name: 'Column1' },
{ id: 'col2', name: 'Column2' },
],
};
fsReadFile.mockResolvedValue(JSON.stringify(mockDataTable) as any);
dataTableRepository.findOne.mockResolvedValue(existingTable as any);
dataTableColumnRepository.find.mockResolvedValue([
{ id: 'col1', name: 'Column 1' },
{ id: 'col2', name: 'Column 2' },
{ id: 'col1', name: 'Column1' },
{ id: 'col2', name: 'Column2' },
] as any);
dataTableColumnRepository.save.mockResolvedValue({ id: 'col1' } as any);
projectRepository.findOne.mockResolvedValue({ id: 'project1', type: 'team' } as any);
@ -3156,7 +3156,7 @@ describe('SourceControlImportService', () => {
// Assert
expect(dataTableDDLService.dropColumnFromTable).toHaveBeenCalledWith(
'dt1',
'Column 2',
'Column2',
'sqlite',
expect.anything(),
);
@ -3183,7 +3183,7 @@ describe('SourceControlImportService', () => {
teamId: 'project1',
teamName: 'Team Project 1',
},
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
@ -3204,13 +3204,81 @@ describe('SourceControlImportService', () => {
expect(dataTableRepository.upsert).not.toHaveBeenCalled();
});
it('should skip columns with invalid names', async () => {
// Arrange
const mockDataTable = {
id: 'dt1',
name: 'Test Table',
ownedBy: {
type: 'team',
teamId: 'project1',
teamName: 'Team Project 1',
},
columns: [
{ id: 'col1', name: 'validName', type: 'string', index: 0 },
{
id: 'col2',
name: 'invalid" text); create INVALID table; --',
type: 'string',
index: 1,
},
],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
fsReadFile.mockResolvedValue(JSON.stringify(mockDataTable) as any);
dataTableRepository.findOne.mockResolvedValue(null);
dataTableColumnRepository.find.mockResolvedValue([]);
dataTableColumnRepository.save.mockImplementation(async (col: any) => col);
projectRepository.findOne.mockResolvedValue({ id: 'project1', type: 'team' } as any);
// Act
await service.importDataTablesFromWorkFolder([mockCandidate], mockUser.id);
// Assert
expect(dataTableDDLService.createTableWithColumns).toHaveBeenCalledWith(
'dt1',
expect.arrayContaining([expect.objectContaining({ id: 'col1', name: 'validName' })]),
expect.anything(),
);
const columns = (dataTableDDLService.createTableWithColumns as jest.Mock).mock.calls[0][1];
expect(columns).not.toEqual(
expect.arrayContaining([expect.objectContaining({ id: 'col2' })]),
);
});
it('should skip data tables with invalid IDs', async () => {
// Arrange
const mockDataTable = {
id: 'invalid"; create INVALID table;--',
name: 'Test Table',
ownedBy: {
type: 'team',
teamId: 'project1',
teamName: 'Team Project 1',
},
columns: [{ id: 'col1', name: 'validName', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
fsReadFile.mockResolvedValue(JSON.stringify(mockDataTable) as any);
// Act
await service.importDataTablesFromWorkFolder([mockCandidate], mockUser.id);
// Assert
expect(dataTableRepository.upsert).not.toHaveBeenCalled();
});
it('should not partially import when a name collision exists among multiple tables', async () => {
// Arrange — two tables: dt1 is valid, dt2 has a name collision
const validTable = {
id: 'dt1',
name: 'Valid Table',
ownedBy: { type: 'team', teamId: 'project1', teamName: 'Team Project 1' },
columns: [{ id: 'col1', name: 'Column 1', type: 'string', index: 0 }],
columns: [{ id: 'col1', name: 'Column1', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};
@ -3218,7 +3286,7 @@ describe('SourceControlImportService', () => {
id: 'dt2',
name: 'Colliding Table',
ownedBy: { type: 'team', teamId: 'project1', teamName: 'Team Project 1' },
columns: [{ id: 'col2', name: 'Column 2', type: 'string', index: 0 }],
columns: [{ id: 'col2', name: 'Column2', type: 'string', index: 0 }],
createdAt: '2024-01-01T00:00:00.000Z',
updatedAt: '2024-01-02T00:00:00.000Z',
};

15
packages/cli/src/modules/source-control.ee/source-control-import.service.ee.ts
View File

@ -46,6 +46,7 @@ import { DataTableColumn } from '@/modules/data-table/data-table-column.entity';
import { DataTableColumnRepository } from '@/modules/data-table/data-table-column.repository';
import { DataTableDDLService } from '@/modules/data-table/data-table-ddl.service';
import { DataTableRepository } from '@/modules/data-table/data-table.repository';
import { isValidColumnName, isValidDataTableId } from '@/modules/data-table/utils/sql-utils';
import { RedactionEnforcementService } from '@/modules/redaction/redaction-enforcement.service';
import { isUniqueConstraintError } from '@/response-helper';
import { TagService } from '@/services/tag.service';
@ -1239,6 +1240,13 @@ export class SourceControlImportService {
continue;
}
if (!isValidDataTableId(dataTable.id)) {
this.logger.warn(
`Invalid data table ID "${dataTable.id}" in file ${candidate.file}. Skipping.`,
);
continue;
}
let targetProject: Project | null = null;
if (dataTable.ownedBy) {
@ -1354,6 +1362,13 @@ export class SourceControlImportService {
// Upsert columns
const columnEntities = [];
for (const column of dataTable.columns) {
if (!isValidColumnName(column.name)) {
this.logger.warn(
`Invalid column name "${column.name}" in data table ${dataTable.name}. Skipping column.`,
);
continue;
}
if (!isValidDataTableColumnType(column.type)) {
this.logger.warn(
`Invalid column type "${column.type}" in data table ${dataTable.name}, column ${column.name}. Skipping column.`,

51
packages/cli/src/oauth/__tests__/oauth.service.test.ts
View File

@ -136,7 +136,7 @@ describe('OauthService', () => {
enumerable: true,
});
const promise = service.getCredential(req);
const promise = service.getCredentialForUpdate(req);
await expect(promise).rejects.toThrow(BadRequestError);
await expect(promise).rejects.toThrow('Required credential ID is missing');
});
@ -149,10 +149,10 @@ describe('OauthService', () => {
credentialsFinderService.findCredentialForUser.mockResolvedValue(null);
await expect(service.getCredential(req)).rejects.toThrow(NotFoundError);
await expect(service.getCredentialForUpdate(req)).rejects.toThrow(NotFoundError);
expect(logger.error).toHaveBeenCalledWith(
'OAuth credential authorization failed because the current user does not have the correct permissions',
{ userId: '123' },
{ userId: '123', credentialId: 'credential-id' },
);
});
@ -165,13 +165,13 @@ describe('OauthService', () => {
credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential);
const result = await service.getCredential(req);
const result = await service.getCredentialForUpdate(req);
expect(result).toBe(mockCredential);
expect(credentialsFinderService.findCredentialForUser).toHaveBeenCalledWith(
'credential-id',
req.user,
['credential:read'],
['credential:update'],
);
});
});
@ -404,20 +404,28 @@ describe('OauthService', () => {
};
const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
cipher.decryptV2.mockResolvedValue(JSON.stringify(csrfData));
const mockCredential = mock<CredentialsEntity>({ id: 'credential-id' });
credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential);
const req = mock<AuthenticatedRequest>({
user: mock<User>({ id: 'user-id' }),
});
const result = await (service as any).decodeCsrfState(encodedState, req);
const [decodedState, credential] = await (service as any).decodeCsrfState(encodedState, req);
expect(cipher.decryptV2).toHaveBeenCalledWith('encrypted-data');
expect(result).toMatchObject({
expect(decodedState).toMatchObject({
token: 'token',
createdAt: timestamp,
cid: 'credential-id',
userId: 'user-id',
origin: 'static-credential',
});
expect(credential).toBe(mockCredential);
expect(credentialsFinderService.findCredentialForUser).toHaveBeenCalledWith(
'credential-id',
req.user,
['credential:update'],
);
});
it('should throw error when state format is invalid', async () => {
@ -529,20 +537,24 @@ describe('OauthService', () => {
};
const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
cipher.decryptV2.mockResolvedValue(JSON.stringify(csrfData));
const mockCredential = mock<CredentialsEntity>({ id: 'credential-id' });
credentialsRepository.findOneBy.mockResolvedValue(mockCredential as any);
const req = mock<AuthenticatedRequest>({
user: mock<User>({ id: 'user-id' }),
});
const result = await (service as any).decodeCsrfState(encodedState, req);
const [decodedState, credential] = await (service as any).decodeCsrfState(encodedState, req);
expect(result).toMatchObject({
expect(decodedState).toMatchObject({
token: 'token',
createdAt: timestamp,
cid: 'credential-id',
userId: 'different-user-id',
origin: 'dynamic-credential',
});
expect(credential).toBe(mockCredential);
expect(cipher.decryptV2).toHaveBeenCalledWith('encrypted-data');
expect(credentialsFinderService.findCredentialForUser).not.toHaveBeenCalled();
});
it('should bypass user validation for dynamic-credential origin even when req.user is undefined', async () => {
@ -558,19 +570,22 @@ describe('OauthService', () => {
};
const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
cipher.decryptV2.mockResolvedValue(JSON.stringify(csrfData));
const mockCredential = mock<CredentialsEntity>({ id: 'credential-id' });
credentialsRepository.findOneBy.mockResolvedValue(mockCredential as any);
const req = mock<AuthenticatedRequest>({
user: undefined,
});
const result = await (service as any).decodeCsrfState(encodedState, req);
const [decodedState, credential] = await (service as any).decodeCsrfState(encodedState, req);
expect(result).toMatchObject({
expect(decodedState).toMatchObject({
token: 'token',
createdAt: timestamp,
cid: 'credential-id',
userId: 'user-id',
origin: 'dynamic-credential',
});
expect(credential).toBe(mockCredential);
expect(cipher.decryptV2).toHaveBeenCalledWith('encrypted-data');
});
@ -760,7 +775,7 @@ describe('OauthService', () => {
});
cipher.decryptV2.mockResolvedValue(JSON.stringify(csrfData));
credentialsRepository.findOneBy.mockResolvedValue(mockCredential);
credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential);
jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
@ -781,7 +796,7 @@ describe('OauthService', () => {
});
});
it('should throw UnexpectedError when credential is not found', async () => {
it('should throw NotFoundError when credential is not found', async () => {
const csrfData = {
cid: 'credential-id',
userId: 'user-id',
@ -800,12 +815,10 @@ describe('OauthService', () => {
user: mock<User>({ id: 'user-id' }),
});
credentialsRepository.findOneBy.mockResolvedValue(null);
credentialsFinderService.findCredentialForUser.mockResolvedValue(null);
await expect(service.resolveCredential(req)).rejects.toThrow(UnexpectedError);
await expect(service.resolveCredential(req)).rejects.toThrow(
'OAuth callback failed because of insufficient permissions',
);
await expect(service.resolveCredential(req)).rejects.toThrow(NotFoundError);
await expect(service.resolveCredential(req)).rejects.toThrow('Credential not found');
});
it('should throw UnexpectedError when CSRF state is invalid', async () => {
@ -832,7 +845,7 @@ describe('OauthService', () => {
user: mock<User>({ id: 'user-id' }),
});
credentialsRepository.findOneBy.mockResolvedValue(mockCredential);
credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential);
jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);

40
packages/cli/src/oauth/oauth.service.ts
View File

@ -90,7 +90,7 @@ export class OauthService {
return `${restUrl}/oauth${oauthVersion}-credential`;
}
async getCredential(
async getCredentialForUpdate(
req: OAuthRequest.OAuth1Credential.Auth | OAuthRequest.OAuth2Credential.Auth,
): Promise<CredentialsEntity> {
const { id: credentialId } = req.query;
@ -102,13 +102,13 @@ export class OauthService {
const credential = await this.credentialsFinderService.findCredentialForUser(
credentialId,
req.user,
['credential:read'],
['credential:update'],
);
if (!credential) {
this.logger.error(
'OAuth credential authorization failed because the current user does not have the correct permissions',
{ userId: req.user.id },
{ userId: req.user.id, credentialId },
);
throw new NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL);
}
@ -250,7 +250,7 @@ export class OauthService {
protected async decodeCsrfState(
encodedState: string,
req: AuthenticatedRequest,
): Promise<CsrfState & CreateCsrfStateData> {
): Promise<[CsrfState & CreateCsrfStateData, CredentialsEntity | null]> {
const errorMessage = 'Invalid state format';
const decodedState = Buffer.from(encodedState, 'base64').toString();
const decoded = jsonParse<CsrfState>(decodedState, {
@ -270,28 +270,31 @@ export class OauthService {
// Dynamic credentials: skip user validation (e.g. embed/iframe flows) as they do not contain an n8n user
if (decryptedState.origin === 'dynamic-credential') {
return {
...decoded,
...decryptedState,
};
return [
{ ...decoded, ...decryptedState },
await this.getCredentialWithoutUser(decryptedState.cid),
];
}
// Static credentials: skip user validation only when N8N_SKIP_AUTH_ON_OAUTH_CALLBACK is true (e.g. embed/iframe)
if (skipAuthOnOAuthCallback) {
return {
...decoded,
...decryptedState,
};
return [
{ ...decoded, ...decryptedState },
await this.getCredentialWithoutUser(decryptedState.cid),
];
}
if (req.user?.id === undefined || decryptedState.userId !== req.user.id) {
throw new AuthError('Unauthorized');
}
return {
...decoded,
...decryptedState,
};
const credential = await this.credentialsFinderService.findCredentialForUser(
decryptedState.cid,
req.user,
['credential:update'],
);
return [{ ...decoded, ...decryptedState }, credential];
}
protected verifyCsrfState(
@ -313,10 +316,9 @@ export class OauthService {
[CredentialsEntity, ICredentialDataDecryptedObject, T, CsrfState & CreateCsrfStateData]
> {
const { state: encodedState } = req.query;
const state = await this.decodeCsrfState(encodedState, req);
const credential = await this.getCredentialWithoutUser(state.cid);
const [state, credential] = await this.decodeCsrfState(encodedState, req);
if (!credential) {
throw new UnexpectedError('OAuth callback failed because of insufficient permissions');
throw new NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL);
}
const additionalData = await this.getAdditionalData();

82
packages/cli/src/task-runners/__tests__/task-runner-process-py.test.ts Normal file
View File

@ -0,0 +1,82 @@
import { Logger } from '@n8n/backend-common';
import { mockInstance } from '@n8n/backend-test-utils';
import { TaskRunnersConfig } from '@n8n/config';
import { mock } from 'jest-mock-extended';
import type { ChildProcess, SpawnOptions } from 'node:child_process';
import type { TaskBrokerAuthService } from '@/task-runners/task-broker/auth/task-broker-auth.service';
import { PyTaskRunnerProcess } from '@/task-runners/task-runner-process-py';
const spawnMock = jest.fn(() =>
mock<ChildProcess>({
stdout: {
pipe: jest.fn(),
},
stderr: {
pipe: jest.fn(),
},
}),
);
require('child_process').spawn = spawnMock;
describe('PyTaskRunnerProcess', () => {
const logger = mockInstance(Logger);
const runnerConfig = mockInstance(TaskRunnersConfig);
runnerConfig.mode = 'internal';
const authService = mock<TaskBrokerAuthService>();
let taskRunnerProcess = new PyTaskRunnerProcess(logger, runnerConfig, authService, mock());
afterEach(() => {
spawnMock.mockClear();
});
describe('start', () => {
beforeEach(() => {
taskRunnerProcess = new PyTaskRunnerProcess(logger, runnerConfig, authService, mock());
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
});
test.each([
'PATH',
'N8N_RUNNERS_STDLIB_ALLOW',
'N8N_RUNNERS_EXTERNAL_ALLOW',
'N8N_RUNNERS_BUILTINS_DENY',
'N8N_BLOCK_RUNNER_ENV_ACCESS',
])('should propagate %s from env as is', async (envVar) => {
process.env[envVar] = 'custom value';
await taskRunnerProcess.start();
// @ts-expect-error The type is not correct
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(options.env).toEqual(
expect.objectContaining({
[envVar]: 'custom value',
}),
);
});
it('should build env with a null prototype', async () => {
await taskRunnerProcess.start();
// @ts-expect-error The type is not correct
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(Object.getPrototypeOf(options.env)).toBeNull();
});
it('should not inherit env keys from Object.prototype', async () => {
const proto = Object.prototype as Record<string, unknown>;
proto.NODE_OPTIONS = '--inherited-value';
try {
await taskRunnerProcess.start();
// @ts-expect-error The type is not correct
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(options.env?.NODE_OPTIONS).toBeUndefined();
} finally {
delete proto.NODE_OPTIONS;
}
});
});
});

27
packages/cli/src/task-runners/__tests__/task-runner-process.test.ts
View File

@ -120,6 +120,33 @@ describe('TaskRunnerProcess', () => {
expect(options.env).not.toHaveProperty('NODE_OPTIONS');
});
it('should build env with a null prototype', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
await taskRunnerProcess.start();
// @ts-expect-error The type is not correct
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(Object.getPrototypeOf(options.env)).toBeNull();
});
it('should not inherit env keys from Object.prototype', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
runnerConfig.maxOldSpaceSize = '';
const proto = Object.prototype as Record<string, unknown>;
proto.NODE_OPTIONS = '--inherited-value';
try {
await taskRunnerProcess.start();
// @ts-expect-error The type is not correct
const options = spawnMock.mock.calls[0][2] as SpawnOptions;
expect(options.env?.NODE_OPTIONS).toBeUndefined();
} finally {
delete proto.NODE_OPTIONS;
}
});
it('should pass N8N_RUNNERS_TASK_TIMEOUT if set', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
runnerConfig.taskTimeout = 123;

4
packages/cli/src/task-runners/task-runner-process-js.ts
View File

@ -53,7 +53,7 @@ export class JsTaskRunnerProcess extends TaskRunnerProcessBase {
}
private getProcessEnvVars(grantToken: string, taskBrokerUri: string) {
const envVars: Record<string, string | undefined> = {
const envVars: Record<string, string | undefined> = Object.assign(Object.create(null), {
// system environment
PATH: process.env.PATH,
HOME: process.env.HOME ?? process.env.USERPROFILE,
@ -78,7 +78,7 @@ export class JsTaskRunnerProcess extends TaskRunnerProcessBase {
N8N_RUNNERS_TASK_TIMEOUT: this.runnerConfig.taskTimeout.toString(),
N8N_RUNNERS_HEARTBEAT_INTERVAL: this.runnerConfig.heartbeatInterval.toString(),
N8N_RUNNERS_INSECURE_MODE: process.env.N8N_RUNNERS_INSECURE_MODE,
};
});
if (this.runnerConfig.maxOldSpaceSize) {
envVars.NODE_OPTIONS = `--max-old-space-size=${this.runnerConfig.maxOldSpaceSize}`;

4
packages/cli/src/task-runners/task-runner-process-py.ts
View File

@ -37,7 +37,7 @@ export class PyTaskRunnerProcess extends TaskRunnerProcessBase {
return spawn(venvPath, ['-m', 'src.main'], {
cwd: pythonDir,
env: {
env: Object.assign(Object.create(null), {
// system environment
PATH: process.env.PATH,
HOME: process.env.HOME ?? process.env.USERPROFILE,
@ -55,7 +55,7 @@ export class PyTaskRunnerProcess extends TaskRunnerProcessBase {
N8N_RUNNERS_EXTERNAL_ALLOW: process.env.N8N_RUNNERS_EXTERNAL_ALLOW,
N8N_RUNNERS_BUILTINS_DENY: process.env.N8N_RUNNERS_BUILTINS_DENY,
N8N_BLOCK_RUNNER_ENV_ACCESS: process.env.N8N_BLOCK_RUNNER_ENV_ACCESS,
},
}),
});
}

187
packages/cli/test/integration/controllers/oauth/oauth1.api.test.ts Normal file
View File

@ -0,0 +1,187 @@
import { createTeamProject, linkUserToProject, testDb } from '@n8n/backend-test-utils';
import type { CredentialsEntity, User } from '@n8n/db';
import { Container } from '@n8n/di';
import { response as Response } from 'express';
import nock from 'nock';
import { CredentialsHelper } from '@/credentials-helper';
import { OauthService } from '@/oauth/oauth.service';
import {
decryptCredentialData,
getCredentialById,
saveCredential,
shareCredentialWithUsers,
} from '@test-integration/db/credentials';
import { createMember, createOwner } from '@test-integration/db/users';
import { setupTestServer } from '@test-integration/utils';
describe('OAuth1 API', () => {
const testServer = setupTestServer({ endpointGroups: ['oauth1'] });
let owner: User;
let credential: CredentialsEntity;
const credentialData = {
consumerKey: 'consumer_key',
consumerSecret: 'consumer_secret',
authUrl: 'https://test.domain/oauth1/auth',
requestTokenUrl: 'https://test.domain/oauth1/request_token',
accessTokenUrl: 'https://test.domain/oauth1/access_token',
signatureMethod: 'HMAC-SHA1' as const,
};
const sharedCredentialPayload = {
name: 'Shared OAuth1 credential',
type: 'testOAuth1Api',
data: credentialData,
};
CredentialsHelper.prototype.applyDefaultsAndOverwrites = async (_, decryptedDataOriginal) =>
decryptedDataOriginal;
beforeAll(async () => {
owner = await createOwner();
});
beforeEach(async () => {
await testDb.truncate(['SharedCredentials', 'CredentialsEntity']);
credential = await saveCredential(sharedCredentialPayload, {
user: owner,
role: 'credential:owner',
});
});
afterEach(() => {
jest.restoreAllMocks();
nock.cleanAll();
});
describe('OAuth reconnect authorization', () => {
const expectNoCsrfStateOnCredential = async (credentialId: string) => {
const stored = await getCredentialById(credentialId);
expect(stored).not.toBeNull();
const decrypted = (await decryptCredentialData(stored!)) as Record<string, unknown>;
expect(decrypted).not.toHaveProperty('csrfSecret');
};
const mockRequestTokenEndpoint = () => {
nock('https://test.domain')
.post('/oauth1/request_token')
.reply(200, 'oauth_token=request_token&oauth_token_secret=request_secret');
};
it('should reject auth start for a sharee with credential:user role', async () => {
const sharee = await createMember();
await shareCredentialWithUsers(credential, [sharee]);
const response = await testServer
.authAgentFor(sharee)
.get('/oauth1-credential/auth')
.query({ id: credential.id });
expect(response.statusCode).toBe(404);
await expectNoCsrfStateOnCredential(credential.id);
});
it('should reject auth start for a project viewer on a project-shared credential', async () => {
const projectViewer = await createMember();
const teamProject = await createTeamProject(undefined, owner);
await linkUserToProject(projectViewer, teamProject, 'project:viewer');
const projectCredential = await saveCredential(sharedCredentialPayload, {
project: teamProject,
role: 'credential:owner',
});
const response = await testServer
.authAgentFor(projectViewer)
.get('/oauth1-credential/auth')
.query({ id: projectCredential.id });
expect(response.statusCode).toBe(404);
await expectNoCsrfStateOnCredential(projectCredential.id);
});
it('should allow auth start for a project editor on a project-shared credential', async () => {
const projectEditor = await createMember();
const teamProject = await createTeamProject(undefined, owner);
await linkUserToProject(projectEditor, teamProject, 'project:editor');
const projectCredential = await saveCredential(sharedCredentialPayload, {
project: teamProject,
role: 'credential:owner',
});
mockRequestTokenEndpoint();
const response = await testServer
.authAgentFor(projectEditor)
.get('/oauth1-credential/auth')
.query({ id: projectCredential.id });
expect(response.statusCode).toBe(200);
expect(response.body.data).toContain('https://test.domain/oauth1/auth');
expect(response.body.data).toContain('oauth_token=request_token');
});
it('should reject callback when requester lacks credential:update on the target credential', async () => {
const sharee = await createMember();
await shareCredentialWithUsers(credential, [sharee]);
const oauthService = Container.get(OauthService);
const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) {
this.end();
return this;
});
// Build a callback state whose decrypted userId equals the requesting member,
// so the userId equality check inside decodeCsrfState passes and the credential
// scope check is the only remaining gate. The owner-initiated /auth call below
// produces a valid encrypted state; we then re-encrypt its contents with the
// member's userId before driving the callback as the member.
const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();
mockRequestTokenEndpoint();
await testServer
.authAgentFor(owner)
.get('/oauth1-credential/auth')
.query({ id: credential.id })
.expect(200);
const [, ownerState] = await csrfSpy.mock.results[0].value;
const decoded = JSON.parse(Buffer.from(ownerState, 'base64').toString());
const decryptedData = JSON.parse(oauthService['cipher'].decrypt(decoded.data)) as Record<
string,
unknown
>;
decryptedData.userId = sharee.id;
decoded.data = oauthService['cipher'].encrypt(JSON.stringify(decryptedData));
const reencodedState = Buffer.from(JSON.stringify(decoded)).toString('base64');
nock('https://test.domain')
.post('/oauth1/access_token')
.reply(200, 'oauth_token=member_token&oauth_token_secret=member_secret');
await testServer
.authAgentFor(sharee)
.get('/oauth1-credential/callback')
.query({
oauth_token: 'request_token',
oauth_verifier: 'verifier',
state: reencodedState,
})
.expect(200);
expect(renderSpy).toHaveBeenCalledWith('oauth-error-callback', {
error: { message: 'Credential not found' },
});
const stored = await Container.get(CredentialsHelper).getCredentials(
credential,
credential.type,
);
const credentials = await stored.getData();
expect(credentials.oauthTokenData).toBeUndefined();
});
});
});

143
packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts
View File

@ -1,4 +1,4 @@
import { testDb } from '@n8n/backend-test-utils';
import { createTeamProject, linkUserToProject, testDb } from '@n8n/backend-test-utils';
import type { CredentialsEntity, User } from '@n8n/db';
import { Container } from '@n8n/di';
import { response as Response } from 'express';
@ -8,7 +8,12 @@ import { parse as parseQs } from 'querystring';
import { CredentialsHelper } from '@/credentials-helper';
import { ExternalHooks } from '@/external-hooks';
import { OauthService } from '@/oauth/oauth.service';
import { saveCredential } from '@test-integration/db/credentials';
import {
decryptCredentialData,
getCredentialById,
saveCredential,
shareCredentialWithUsers,
} from '@test-integration/db/credentials';
import { createMember, createOwner } from '@test-integration/db/users';
import type { SuperAgentTest } from '@test-integration/types';
import { setupTestServer } from '@test-integration/utils';
@ -52,6 +57,10 @@ describe('OAuth2 API', () => {
);
});
afterEach(() => {
jest.restoreAllMocks();
});
it('should return a valid auth URL when the auth flow is initiated', async () => {
const response = await ownerAgent
.get('/oauth2-credential/auth')
@ -131,9 +140,10 @@ describe('OAuth2 API', () => {
it('should fail on auth when callback is called as another user', async () => {
const oauthService = Container.get(OauthService);
const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();
const renderSpy = (Response.render = jest.fn(function () {
const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) {
this.end();
}));
return this;
});
await ownerAgent.get('/oauth2-credential/auth').query({ id: credential.id }).expect(200);
@ -153,9 +163,10 @@ describe('OAuth2 API', () => {
it('should handle a valid callback without auth', async () => {
const oauthService = Container.get(OauthService);
const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();
const renderSpy = (Response.render = jest.fn(function () {
const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) {
this.end();
}));
return this;
});
await ownerAgent.get('/oauth2-credential/auth').query({ id: credential.id }).expect(200);
@ -179,4 +190,124 @@ describe('OAuth2 API', () => {
oauthTokenData: { access_token: 'updated_token' },
});
});
describe('OAuth reconnect authorization', () => {
const sharedCredentialPayload = {
name: 'Shared OAuth2 credential',
type: 'testOAuth2Api',
data: credentialData,
};
const expectNoCsrfStateOnCredential = async (credentialId: string) => {
const stored = await getCredentialById(credentialId);
expect(stored).not.toBeNull();
const decrypted = (await decryptCredentialData(stored!)) as Record<string, unknown>;
expect(decrypted).not.toHaveProperty('csrfSecret');
expect(decrypted).not.toHaveProperty('codeVerifier');
};
it('should reject auth start for a sharee with credential:user role', async () => {
const sharee = await createMember();
await shareCredentialWithUsers(credential, [sharee]);
const response = await testServer
.authAgentFor(sharee)
.get('/oauth2-credential/auth')
.query({ id: credential.id });
expect(response.statusCode).toBe(404);
await expectNoCsrfStateOnCredential(credential.id);
});
it('should reject auth start for a project viewer on a project-shared credential', async () => {
const projectViewer = await createMember();
const teamProject = await createTeamProject(undefined, owner);
await linkUserToProject(projectViewer, teamProject, 'project:viewer');
const projectCredential = await saveCredential(sharedCredentialPayload, {
project: teamProject,
role: 'credential:owner',
});
const response = await testServer
.authAgentFor(projectViewer)
.get('/oauth2-credential/auth')
.query({ id: projectCredential.id });
expect(response.statusCode).toBe(404);
await expectNoCsrfStateOnCredential(projectCredential.id);
});
it('should allow auth start for a project editor on a project-shared credential', async () => {
const projectEditor = await createMember();
const teamProject = await createTeamProject(undefined, owner);
await linkUserToProject(projectEditor, teamProject, 'project:editor');
const projectCredential = await saveCredential(sharedCredentialPayload, {
project: teamProject,
role: 'credential:owner',
});
const response = await testServer
.authAgentFor(projectEditor)
.get('/oauth2-credential/auth')
.query({ id: projectCredential.id });
expect(response.statusCode).toBe(200);
expect(response.body.data).toContain('https://test.domain/oauth2/auth');
});
it('should reject callback when requester lacks credential:update on the target credential', async () => {
const sharee = await createMember();
await shareCredentialWithUsers(credential, [sharee]);
const oauthService = Container.get(OauthService);
const renderSpy = (Response.render = jest.fn(function () {
this.end();
}));
// Build a callback state whose decrypted userId equals the requesting member,
// so the userId equality check inside decodeCsrfState passes and the credential
// scope check is the only remaining gate. The owner-initiated /auth call below
// produces a valid encrypted state; we then re-encrypt its contents with the
// member's userId before driving the callback as the member.
const ownerAgentForSetup = testServer.authAgentFor(owner);
const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();
await ownerAgentForSetup
.get('/oauth2-credential/auth')
.query({ id: credential.id })
.expect(200);
const [, ownerState] = await csrfSpy.mock.results[0].value;
const decoded = JSON.parse(Buffer.from(ownerState, 'base64').toString());
const decryptedData = JSON.parse(oauthService['cipher'].decrypt(decoded.data)) as Record<
string,
unknown
>;
decryptedData.userId = sharee.id;
decoded.data = oauthService['cipher'].encrypt(JSON.stringify(decryptedData));
const reencodedState = Buffer.from(JSON.stringify(decoded)).toString('base64');
nock('https://test.domain')
.post('/oauth2/token')
.reply(200, { access_token: 'member_token' });
await testServer
.authAgentFor(sharee)
.get('/oauth2-credential/callback')
.query({ code: 'auth_code', state: reencodedState })
.expect(200);
expect(renderSpy).toHaveBeenCalledWith('oauth-error-callback', {
error: { message: 'Credential not found' },
});
const updatedCredential = await Container.get(CredentialsHelper).getCredentials(
credential,
credential.type,
);
const credentials = await updatedCredential.getData();
expect(credentials.oauthTokenData).toBeUndefined();
});
});
});

1
packages/cli/test/integration/shared/types.ts
View File

@ -12,6 +12,7 @@ type EndpointGroup =
| 'me'
| 'users'
| 'auth'
| 'oauth1'
| 'oauth2'
| 'owner'
| 'passwordReset'

4
packages/cli/test/integration/shared/utils/test-server.ts
View File

@ -223,6 +223,10 @@ export const setupTestServer = ({
await import('@/controllers/auth.controller');
break;
case 'oauth1':
await import('@/controllers/oauth/oauth1-credential.controller');
break;
case 'oauth2':
await import('@/controllers/oauth/oauth2-credential.controller');
break;

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

@ -343,11 +343,12 @@ export class Git implements INodeType {
...(!enableHooks && { unsafe: { allowUnsafeHooksPath: true } }),
};
const git: SimpleGit = simpleGit(gitOptions)
// Tell git not to ask for any information via the terminal like for
// example the username. As nobody will be able to answer it would
// n8n keep on waiting forever.
.env('GIT_TERMINAL_PROMPT', '0');
const cleanEnv = Object.create(null) as Record<string, unknown>;
// Tell git not to ask for any information via the terminal like for
// example the username. As nobody will be able to answer it would
// n8n keep on waiting forever.
cleanEnv['GIT_TERMINAL_PROMPT'] = '0';
const git: SimpleGit = simpleGit(gitOptions).env(cleanEnv);
if (operation === 'add') {
// ----------------------------------

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

@ -61,6 +61,32 @@ describe('Git Node', () => {
jest.clearAllMocks();
});
describe('Environment validation', () => {
it('should not include invalid inherited environment keys in simple-git env', async () => {
const inheritedEnvKey = 'N8N_TEST_INVALID_ENV_KEY';
(Object.prototype as Record<string, unknown>)[inheritedEnvKey] = 'ignored';
mockExecuteFunctions.getNodeParameter
.mockReturnValueOnce('log')
.mockReturnValueOnce('/repo')
.mockReturnValueOnce({});
mockGit.log.mockResolvedValueOnce({ all: [] } as any);
try {
await gitNode.execute.call(mockExecuteFunctions);
} finally {
delete (Object.prototype as Record<string, unknown>)[inheritedEnvKey];
}
expect(mockGit.env).toHaveBeenCalledTimes(1);
const envArg = mockGit.env.mock.calls[0][0] as Record<string, string>;
expect(Object.prototype.hasOwnProperty.call(envArg, inheritedEnvKey)).toBe(false);
expect(envArg[inheritedEnvKey]).toBeUndefined();
expect(envArg.GIT_TERMINAL_PROMPT).toBe('0');
});
});
describe('Branch switching', () => {
const mockNodeParameters = (params: Record<string, unknown>) => {
mockExecuteFunctions.getNodeParameter.mockImplementation(

30
packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts
View File

@ -24,6 +24,7 @@ import {
removeCircularRefs,
sleep,
ensureError,
setSafeObjectProperty,
} from 'n8n-workflow';
import type { Readable } from 'stream';
@ -60,6 +61,10 @@ function isEmptyResponseBody(body: unknown): body is string {
return typeof body === 'string' && body.trim().length === 0;
}
function isPaginationRequestType(value: string): value is 'body' | 'headers' | 'qs' {
return value === 'body' || value === 'headers' || value === 'qs';
}
export class HttpRequestV3 implements INodeType {
description: INodeTypeDescription;
@ -639,13 +644,12 @@ export class HttpRequestV3 implements INodeType {
const paginationData: PaginationOptions = {
continue: continueExpression,
request: {},
request: Object.create(null) as Record<string, unknown>,
requestInterval: pagination.requestInterval,
};
if (pagination.paginationMode === 'updateAParameterInEachRequest') {
// Iterate over all parameters and add them to the request
paginationData.request = {};
const { parameters } = pagination.parameters;
if (
parameters.length === 1 &&
@ -658,9 +662,6 @@ export class HttpRequestV3 implements INodeType {
);
}
pagination.parameters.parameters.forEach((parameter, index) => {
if (!paginationData.request[parameter.type]) {
paginationData.request[parameter.type] = {};
}
const parameterName = parameter.name;
if (parameterName === '') {
throw new NodeOperationError(
@ -668,6 +669,18 @@ export class HttpRequestV3 implements INodeType {
`Parameter name must be set for parameter [${index + 1}] in pagination settings`,
);
}
if (!isPaginationRequestType(parameter.type)) {
throw new NodeOperationError(
this.getNode(),
`Parameter type must be one of: body, headers, qs for parameter [${
index + 1
}] in pagination settings`,
);
}
paginationData.request[parameter.type] ??= Object.create(null);
const parameterValue = parameter.value;
if (parameterValue === '') {
throw new NodeOperationError(
@ -677,7 +690,12 @@ export class HttpRequestV3 implements INodeType {
}] in pagination settings, omitting it will result in an infinite loop`,
);
}
paginationData.request[parameter.type]![parameterName] = parameterValue;
setSafeObjectProperty(
paginationData.request[parameter.type]!,
parameterName,
parameterValue,
);
});
} else if (pagination.paginationMode === 'responseContainsNextURL') {
paginationData.request.url = pagination.nextURL;

152
packages/nodes-base/nodes/HttpRequest/test/node/HttpRequestV3.test.ts
View File

@ -607,4 +607,156 @@ describe('HttpRequestV3', () => {
);
});
});
describe('Pagination parameter validation', () => {
it('should keep valid pagination parameters and ignore invalid parameter names', async () => {
const paginationTestOptions = {
...options,
response: {
response: {
neverError: false,
responseFormat: 'json',
fullResponse: false,
outputPropertyName: 'data',
},
},
};
(executeFunctions.getInputData as jest.Mock).mockReturnValue([{ json: {} }]);
(executeFunctions.getNodeParameter as jest.Mock).mockImplementation(
(paramName: string, _itemIndex: number, defaultValue: unknown) => {
switch (paramName) {
case 'method':
return 'GET';
case 'url':
return baseUrl;
case 'authentication':
return 'none';
case 'options':
return paginationTestOptions;
case 'options.pagination.pagination':
return {
paginationMode: 'updateAParameterInEachRequest',
parameters: {
parameters: [
{
type: 'qs',
// eslint-disable-next-line n8n-nodes-base/node-param-display-name-miscased
name: 'page',
value: '1',
},
{
type: 'qs',
name: 'constructor',
value: 'ignored',
},
],
},
paginationCompleteWhen: 'responseIsEmpty',
statusCodesWhenComplete: '',
completeExpression: '',
limitPagesFetched: false,
maxRequests: 10,
requestInterval: 0,
};
case 'options.response.response.responseFormat':
return 'json';
default:
return defaultValue;
}
},
);
(executeFunctions.helpers.requestWithAuthenticationPaginated as jest.Mock).mockResolvedValue([
{
headers: { 'content-type': 'application/json' },
body: { success: true },
statusCode: 200,
},
]);
const result = await node.execute.call(executeFunctions);
expect(result).toEqual([[{ json: { success: true }, pairedItem: { item: 0 } }]]);
expect(executeFunctions.helpers.requestWithAuthenticationPaginated).toHaveBeenCalledTimes(1);
const paginationData = (
executeFunctions.helpers.requestWithAuthenticationPaginated as jest.Mock
).mock.calls[0][2] as {
request: {
qs: Record<string, unknown>;
};
};
expect(paginationData.request.qs).toEqual({ page: '1' });
expect(Object.prototype.hasOwnProperty.call(paginationData.request.qs, 'constructor')).toBe(
false,
);
});
it('should reject invalid pagination parameter type values', async () => {
const paginationTestOptions = {
...options,
response: {
response: {
neverError: false,
responseFormat: 'json',
fullResponse: false,
outputPropertyName: 'data',
},
},
};
(executeFunctions.getInputData as jest.Mock).mockReturnValue([{ json: {} }]);
(executeFunctions.helpers.requestWithAuthenticationPaginated as jest.Mock).mockResolvedValue([
{
headers: { 'content-type': 'application/json' },
body: { success: true },
statusCode: 200,
},
]);
(executeFunctions.helpers.request as jest.Mock).mockResolvedValue({
headers: { 'content-type': 'application/json' },
body: Buffer.from(JSON.stringify({ success: true })),
});
(executeFunctions.getNodeParameter as jest.Mock).mockImplementation(
(paramName: string, _itemIndex: number, defaultValue: unknown) => {
switch (paramName) {
case 'method':
return 'GET';
case 'url':
return baseUrl;
case 'authentication':
return 'none';
case 'options':
return paginationTestOptions;
case 'options.pagination.pagination':
return {
paginationMode: 'updateAParameterInEachRequest',
parameters: {
parameters: [
{
type: '__proto__' as unknown as 'qs',
// eslint-disable-next-line n8n-nodes-base/node-param-display-name-miscased
name: 'page',
value: '1',
},
],
},
paginationCompleteWhen: 'responseIsEmpty',
statusCodesWhenComplete: '',
completeExpression: '',
limitPagesFetched: false,
maxRequests: 10,
requestInterval: 0,
};
default:
return defaultValue;
}
},
);
await expect(node.execute.call(executeFunctions)).rejects.toThrow(
'Parameter type must be one of: body, headers, qs for parameter [1] in pagination settings',
);
expect(executeFunctions.helpers.requestWithAuthenticationPaginated).not.toHaveBeenCalled();
});
});
});

28
packages/nodes-base/nodes/Xml/Xml.node.ts
View File

@ -236,19 +236,23 @@ export class Xml implements INodeType {
const options = this.getNodeParameter('options', 0, {});
const forbiddenKeys = ['__proto__', 'constructor', 'prototype'];
const attrkey = String(options.attrkey ?? '');
const charkey = String(options.charkey ?? '');
if (forbiddenKeys.includes(attrkey)) {
throw new NodeOperationError(
this.getNode(),
`The "Attribute Key" option value "${attrkey}" is not allowed`,
);
if (options.attrkey !== undefined) {
options.attrkey = String(options.attrkey);
if (forbiddenKeys.includes(options.attrkey)) {
throw new NodeOperationError(
this.getNode(),
`The "Attribute Key" option value "${options.attrkey}" is not allowed`,
);
}
}
if (forbiddenKeys.includes(charkey)) {
throw new NodeOperationError(
this.getNode(),
`The "Character Key" option value "${charkey}" is not allowed`,
);
if (options.charkey !== undefined) {
options.charkey = String(options.charkey);
if (forbiddenKeys.includes(options.charkey)) {
throw new NodeOperationError(
this.getNode(),
`The "Character Key" option value "${options.charkey}" is not allowed`,
);
}
}
let item: INodeExecutionData;

100
packages/nodes-base/nodes/Xml/test/node/Xml.test.ts
View File

@ -130,4 +130,104 @@ describe('Xml Node - options validation', () => {
},
);
});
describe('default option behaviour', () => {
test('should use the parser default attribute key when attrkey option is not set', async () => {
setupGetNodeParameter('xmlToJson', { mergeAttrs: false, explicitArray: false });
mockExecuteFunctions.getInputData.mockReturnValue([
{ json: { data: '<root foo="bar">x</root>' } },
]);
const result = await xmlNode.execute.call(mockExecuteFunctions);
const parsed = result[0][0].json as { root: Record<string, unknown> };
expect(parsed.root).toHaveProperty('$', { foo: 'bar' });
});
test('should use the parser default character key when charkey option is not set', async () => {
setupGetNodeParameter('xmlToJson', { mergeAttrs: false, explicitArray: false });
mockExecuteFunctions.getInputData.mockReturnValue([
{ json: { data: '<root foo="bar">text</root>' } },
]);
const result = await xmlNode.execute.call(mockExecuteFunctions);
const parsed = result[0][0].json as { root: Record<string, unknown> };
expect(parsed.root).toHaveProperty('_', 'text');
});
});
describe('attrkey and charkey are coerced to a stable string', () => {
test('should forward the coerced attrkey string to the parser in xmlToJson mode', async () => {
const statefulAttrkey = (() => {
let calls = 0;
return {
toString: () => (++calls === 1 ? 'safe' : 'changed'),
};
})();
setupGetNodeParameter('xmlToJson', {
attrkey: statefulAttrkey,
mergeAttrs: false,
explicitArray: false,
});
mockExecuteFunctions.getInputData.mockReturnValue([
{ json: { data: '<root foo="bar">x</root>' } },
]);
const result = await xmlNode.execute.call(mockExecuteFunctions);
const parsed = result[0][0].json as { root: Record<string, unknown> };
expect(parsed.root).toHaveProperty('safe', { foo: 'bar' });
expect(parsed.root).not.toHaveProperty('changed');
});
test('should forward the coerced charkey string to the parser in xmlToJson mode', async () => {
const statefulCharkey = (() => {
let calls = 0;
return {
toString: () => (++calls === 1 ? 'safe' : 'changed'),
};
})();
setupGetNodeParameter('xmlToJson', {
charkey: statefulCharkey,
mergeAttrs: false,
explicitArray: false,
});
mockExecuteFunctions.getInputData.mockReturnValue([
{ json: { data: '<root foo="bar">text</root>' } },
]);
const result = await xmlNode.execute.call(mockExecuteFunctions);
const parsed = result[0][0].json as { root: Record<string, unknown> };
expect(parsed.root).toHaveProperty('safe', 'text');
expect(parsed.root).not.toHaveProperty('changed');
});
test('should forward the coerced attrkey string to the builder in jsonToxml mode', async () => {
const statefulAttrkey = (() => {
let calls = 0;
return {
toString: () => (++calls === 1 ? 'safe' : 'changed'),
};
})();
setupGetNodeParameter('jsonToxml', {
attrkey: statefulAttrkey,
headless: true,
});
mockExecuteFunctions.getInputData.mockReturnValue([
{ json: { root: { safe: { foo: 'bar' } } } },
]);
const result = await xmlNode.execute.call(mockExecuteFunctions);
const xml = (result[0][0].json as { data: string }).data;
expect(xml).toContain('foo="bar"');
expect(xml).not.toContain('<safe');
expect(xml).not.toContain('<changed');
});
});
});