mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-01 15:47:41 +08:00
fix(core): Avoid loading execution data during credentials security audit (#34876)
This commit is contained in:
@@ -81,7 +81,7 @@ export class ExecutionEntity {
|
||||
@DeleteDateColumn({ type: datetimeColumnType as SimpleColumnType, nullable: true })
|
||||
deletedAt: Date;
|
||||
|
||||
@Column({ nullable: true })
|
||||
@Column()
|
||||
workflowId: string;
|
||||
|
||||
@DateTimeColumn({ nullable: true })
|
||||
|
||||
@@ -1086,6 +1086,23 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
|
||||
return qb;
|
||||
}
|
||||
|
||||
/**
|
||||
* IDs of the distinct workflows that have at least one execution started at or after `date`.
|
||||
* @param date Lower bound (inclusive) for `startedAt`.
|
||||
* @returns Distinct workflow IDs, in no particular order.
|
||||
* @remarks Reads only entity columns, never the execution data blobs.
|
||||
*/
|
||||
async getWorkflowIdsWithExecutionsSince(date: Date): Promise<string[]> {
|
||||
const result = await this.createQueryBuilder('execution')
|
||||
.select('DISTINCT execution.workflowId', 'workflowId')
|
||||
.where('execution.startedAt >= :date', {
|
||||
date: DateUtils.mixedDateToUtcDatetimeString(date),
|
||||
})
|
||||
.getRawMany<{ workflowId: string }>();
|
||||
|
||||
return result.map((row) => row.workflowId);
|
||||
}
|
||||
|
||||
async getDistinctVersionIds(workflowId: string): Promise<string[]> {
|
||||
const result = await this.createQueryBuilder('execution')
|
||||
.innerJoin('execution.executionData', 'ed')
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
import { SecurityConfig } from '@n8n/config';
|
||||
import { CredentialsRepository, MoreThanOrEqual } from '@n8n/db';
|
||||
import { CredentialsRepository, ExecutionRepository } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { IWorkflowBase } from 'n8n-workflow';
|
||||
|
||||
import { ExecutionPersistence } from '@/executions/execution-persistence';
|
||||
import { CREDENTIALS_REPORT } from '@/security-audit/constants';
|
||||
import type { RiskReporter, Risk } from '@/security-audit/types';
|
||||
|
||||
@@ -11,7 +10,7 @@ import type { RiskReporter, Risk } from '@/security-audit/types';
|
||||
export class CredentialsRiskReporter implements RiskReporter {
|
||||
constructor(
|
||||
private readonly credentialsRepository: CredentialsRepository,
|
||||
private readonly executionPersistence: ExecutionPersistence,
|
||||
private readonly executionRepository: ExecutionRepository,
|
||||
private readonly securityConfig: SecurityConfig,
|
||||
) {}
|
||||
|
||||
@@ -20,7 +19,10 @@ export class CredentialsRiskReporter implements RiskReporter {
|
||||
|
||||
const allExistingCreds = await this.getAllExistingCreds();
|
||||
const { credsInAnyUse, credsInActiveUse } = this.getAllCredsInUse(workflows);
|
||||
const recentlyExecutedCreds = await this.getCredsInRecentlyExecutedWorkflows(days);
|
||||
const recentlyExecutedCreds = await this.getCredentialsInRecentlyExecutedWorkflows(
|
||||
workflows,
|
||||
days,
|
||||
);
|
||||
|
||||
const credsNotInAnyUse = allExistingCreds.filter((c) => !credsInAnyUse.has(c.id));
|
||||
const credsNotInActiveUse = allExistingCreds.filter((c) => !credsInActiveUse.has(c.id));
|
||||
@@ -113,35 +115,24 @@ export class CredentialsRiskReporter implements RiskReporter {
|
||||
return credentials.map(({ id, name }) => ({ kind: 'credential' as const, id, name }));
|
||||
}
|
||||
|
||||
private async getExecutedWorkflowsInPastDays(days: number): Promise<IWorkflowBase[]> {
|
||||
private async getCredentialsInRecentlyExecutedWorkflows(
|
||||
workflows: IWorkflowBase[],
|
||||
days: number,
|
||||
): Promise<Set<string>> {
|
||||
const date = new Date();
|
||||
|
||||
date.setDate(date.getDate() - days);
|
||||
|
||||
const executions = await this.executionPersistence.findMultipleExecutions(
|
||||
{ where: { startedAt: MoreThanOrEqual(date) } },
|
||||
{ includeData: true, unflattenData: false },
|
||||
const recentlyExecutedWorkflowIds = new Set(
|
||||
await this.executionRepository.getWorkflowIdsWithExecutionsSince(date),
|
||||
);
|
||||
|
||||
return executions.map((execution) => execution.workflowData);
|
||||
}
|
||||
const credentialIds = workflows
|
||||
.filter((workflow) => recentlyExecutedWorkflowIds.has(workflow.id))
|
||||
.flatMap((workflow) => workflow.nodes)
|
||||
.flatMap((node) => Object.values(node.credentials ?? {}))
|
||||
.map((credential) => credential.id)
|
||||
.filter((id): id is string => id !== undefined);
|
||||
|
||||
/**
|
||||
* Return IDs of credentials in workflows executed in the past n days.
|
||||
*/
|
||||
private async getCredsInRecentlyExecutedWorkflows(days: number) {
|
||||
const executedWorkflows = await this.getExecutedWorkflowsInPastDays(days);
|
||||
|
||||
return executedWorkflows.reduce<Set<string>>((acc, { nodes }) => {
|
||||
nodes.forEach((node) => {
|
||||
if (node.credentials) {
|
||||
Object.values(node.credentials).forEach((c) => {
|
||||
if (c.id) acc.add(c.id);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
return acc;
|
||||
}, new Set());
|
||||
return new Set(credentialIds);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -279,4 +279,44 @@ describe('ExecutionRepository', () => {
|
||||
expect(successExec?.status).toBe('success');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getWorkflowIdsWithExecutionsSince', () => {
|
||||
const insertExecution = async (workflowId: string, startedAt: Date) =>
|
||||
await Container.get(ExecutionRepository).insert({
|
||||
workflowId,
|
||||
mode: 'manual',
|
||||
startedAt,
|
||||
status: 'success',
|
||||
finished: true,
|
||||
createdAt: startedAt,
|
||||
});
|
||||
|
||||
it('should return distinct workflow ids for executions started at or after the date', async () => {
|
||||
const executionRepository = Container.get(ExecutionRepository);
|
||||
const [workflow1, workflow2] = await Promise.all([createWorkflow(), createWorkflow()]);
|
||||
const since = new Date('2024-01-01T00:00:00.000Z');
|
||||
|
||||
await insertExecution(workflow1.id, since); // inclusive boundary
|
||||
await insertExecution(workflow1.id, new Date('2024-06-01T00:00:00.000Z')); // same workflow again
|
||||
await insertExecution(workflow2.id, new Date('2024-03-01T00:00:00.000Z'));
|
||||
|
||||
const result = await executionRepository.getWorkflowIdsWithExecutionsSince(since);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result).toEqual(expect.arrayContaining([workflow1.id, workflow2.id]));
|
||||
});
|
||||
|
||||
it('should exclude workflows whose executions all started before the date', async () => {
|
||||
const executionRepository = Container.get(ExecutionRepository);
|
||||
const workflow = await createWorkflow();
|
||||
|
||||
await insertExecution(workflow.id, new Date('2023-12-31T23:59:59.000Z'));
|
||||
|
||||
const result = await executionRepository.getWorkflowIdsWithExecutionsSince(
|
||||
new Date('2024-01-01T00:00:00.000Z'),
|
||||
);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -256,3 +256,56 @@ test('should not report credentials in recently executed workflow', async () =>
|
||||
|
||||
expect(testAudit).toBeEmptyArray();
|
||||
});
|
||||
|
||||
test('should detect recent execution from the execution row alone, without its data', async () => {
|
||||
const credentialDetails = {
|
||||
id: generateNanoId(),
|
||||
name: 'My Slack Credential',
|
||||
data: 'U2FsdGVkX18WjITBG4IDqrGB1xE/uzVNjtwDAG3lP7E=',
|
||||
type: 'slackApi',
|
||||
};
|
||||
|
||||
const credential = await Container.get(CredentialsRepository).save(credentialDetails);
|
||||
|
||||
const workflowDetails = {
|
||||
name: 'My Test Workflow',
|
||||
connections: {},
|
||||
nodeTypes: {},
|
||||
nodes: [
|
||||
{
|
||||
id: uuid(),
|
||||
name: 'My Node',
|
||||
type: 'n8n-nodes-base.slack',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
credentials: {
|
||||
slackApi: {
|
||||
id: credential.id,
|
||||
name: credential.name,
|
||||
},
|
||||
},
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const workflow = await createActiveWorkflow(workflowDetails);
|
||||
|
||||
const date = new Date();
|
||||
date.setDate(date.getDate() - securityConfig.daysAbandonedWorkflow + 1);
|
||||
|
||||
await Container.get(ExecutionRepository).save({
|
||||
finished: true,
|
||||
mode: 'manual',
|
||||
createdAt: date,
|
||||
startedAt: date,
|
||||
stoppedAt: date,
|
||||
workflowId: workflow.id,
|
||||
waitTill: null,
|
||||
status: 'success',
|
||||
});
|
||||
|
||||
const testAudit = await securityAuditService.run(['credentials']);
|
||||
|
||||
expect(testAudit).toBeEmptyArray();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user