mirror of
https://github.com/n8n-io/n8n.git
synced 2024-11-09 22:24:05 -08:00
fix(core): Disallow orphan executions (#7069)
Until https://github.com/n8n-io/n8n/pull/7061 we had an edge case where a manual unsaved workflow when run creates an orphan execution, i.e. a saved execution not pointing to any workflow. This execution is only ever visible to the instance owner (even if triggered by a member), and is wrongly stored as unfinished and crashed. This PR enforces that the DB disallows any such executions from making it into the DB. This is needed also for the S3 client, which will include the `workflowId` in the path-like filename. --------- Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
parent
58b3492b0d
commit
8a28e98ec8
|
@ -13,9 +13,9 @@ abstract class IndexOperation extends LazyPromise<void> {
|
|||
}
|
||||
|
||||
constructor(
|
||||
protected tablePrefix: string,
|
||||
protected tableName: string,
|
||||
protected columnNames: string[],
|
||||
protected tablePrefix: string,
|
||||
queryRunner: QueryRunner,
|
||||
protected customIndexName?: string,
|
||||
) {
|
||||
|
@ -27,14 +27,14 @@ abstract class IndexOperation extends LazyPromise<void> {
|
|||
|
||||
export class CreateIndex extends IndexOperation {
|
||||
constructor(
|
||||
tablePrefix: string,
|
||||
tableName: string,
|
||||
columnNames: string[],
|
||||
protected isUnique: boolean,
|
||||
tablePrefix: string,
|
||||
queryRunner: QueryRunner,
|
||||
customIndexName?: string,
|
||||
) {
|
||||
super(tablePrefix, tableName, columnNames, queryRunner, customIndexName);
|
||||
super(tableName, columnNames, tablePrefix, queryRunner, customIndexName);
|
||||
}
|
||||
|
||||
async execute(queryRunner: QueryRunner) {
|
||||
|
|
|
@ -11,8 +11,8 @@ abstract class TableOperation<R = void> extends LazyPromise<R> {
|
|||
protected prefix: string,
|
||||
queryRunner: QueryRunner,
|
||||
) {
|
||||
super((resolve) => {
|
||||
void this.execute(queryRunner).then(resolve);
|
||||
super((resolve, reject) => {
|
||||
void this.execute(queryRunner).then(resolve).catch(reject);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
@ -116,3 +116,47 @@ export class DropColumns extends TableOperation {
|
|||
return queryRunner.dropColumns(`${prefix}${tableName}`, columnNames);
|
||||
}
|
||||
}
|
||||
|
||||
class ModifyNotNull extends TableOperation {
|
||||
constructor(
|
||||
tableName: string,
|
||||
protected columnName: string,
|
||||
protected isNullable: boolean,
|
||||
prefix: string,
|
||||
queryRunner: QueryRunner,
|
||||
) {
|
||||
super(tableName, prefix, queryRunner);
|
||||
}
|
||||
|
||||
async execute(queryRunner: QueryRunner) {
|
||||
const { tableName, prefix, columnName, isNullable } = this;
|
||||
const table = await queryRunner.getTable(`${prefix}${tableName}`);
|
||||
if (!table) throw new Error(`No table found with the name ${tableName}`);
|
||||
const oldColumn = table.findColumnByName(columnName)!;
|
||||
const newColumn = oldColumn.clone();
|
||||
newColumn.isNullable = isNullable;
|
||||
return queryRunner.changeColumn(table, oldColumn, newColumn);
|
||||
}
|
||||
}
|
||||
|
||||
export class AddNotNull extends ModifyNotNull {
|
||||
constructor(
|
||||
tableName: string,
|
||||
protected columnName: string,
|
||||
prefix: string,
|
||||
queryRunner: QueryRunner,
|
||||
) {
|
||||
super(tableName, columnName, false, prefix, queryRunner);
|
||||
}
|
||||
}
|
||||
|
||||
export class DropNotNull extends ModifyNotNull {
|
||||
constructor(
|
||||
tableName: string,
|
||||
protected columnName: string,
|
||||
prefix: string,
|
||||
queryRunner: QueryRunner,
|
||||
) {
|
||||
super(tableName, columnName, true, prefix, queryRunner);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
import type { QueryRunner } from 'typeorm';
|
||||
import { Column } from './Column';
|
||||
import { AddColumns, CreateTable, DropColumns, DropTable } from './Table';
|
||||
import { AddColumns, AddNotNull, CreateTable, DropColumns, DropNotNull, DropTable } from './Table';
|
||||
import { CreateIndex, DropIndex } from './Indices';
|
||||
|
||||
export const createSchemaBuilder = (tablePrefix: string, queryRunner: QueryRunner) => ({
|
||||
|
@ -21,10 +21,15 @@ export const createSchemaBuilder = (tablePrefix: string, queryRunner: QueryRunne
|
|||
columnNames: string[],
|
||||
isUnique = false,
|
||||
customIndexName?: string,
|
||||
) => new CreateIndex(tablePrefix, tableName, columnNames, isUnique, queryRunner, customIndexName),
|
||||
) => new CreateIndex(tableName, columnNames, isUnique, tablePrefix, queryRunner, customIndexName),
|
||||
|
||||
dropIndex: (tableName: string, columnNames: string[], customIndexName?: string) =>
|
||||
new DropIndex(tablePrefix, tableName, columnNames, queryRunner, customIndexName),
|
||||
new DropIndex(tableName, columnNames, tablePrefix, queryRunner, customIndexName),
|
||||
|
||||
addNotNull: (tableName: string, columnName: string) =>
|
||||
new AddNotNull(tableName, columnName, tablePrefix, queryRunner),
|
||||
dropNotNull: (tableName: string, columnName: string) =>
|
||||
new DropNotNull(tableName, columnName, tablePrefix, queryRunner),
|
||||
|
||||
/* eslint-enable */
|
||||
});
|
||||
|
|
|
@ -0,0 +1,22 @@
|
|||
import type { MigrationContext, ReversibleMigration } from '@db/types';
|
||||
|
||||
export class DisallowOrphanExecutions1693554410387 implements ReversibleMigration {
|
||||
/**
|
||||
* Ensure all executions point to a workflow.
|
||||
*/
|
||||
async up({ escape, schemaBuilder: { addNotNull }, runQuery }: MigrationContext) {
|
||||
const executionEntity = escape.tableName('execution_entity');
|
||||
const workflowId = escape.columnName('workflowId');
|
||||
|
||||
await runQuery(`DELETE FROM ${executionEntity} WHERE ${workflowId} IS NULL;`);
|
||||
|
||||
await addNotNull('execution_entity', 'workflowId');
|
||||
}
|
||||
|
||||
/**
|
||||
* Reversal excludes restoring deleted rows.
|
||||
*/
|
||||
async down({ schemaBuilder: { dropNotNull } }: MigrationContext) {
|
||||
await dropNotNull('execution_entity', 'workflowId');
|
||||
}
|
||||
}
|
|
@ -46,6 +46,7 @@ import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030
|
|||
import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex';
|
||||
import { AddMfaColumns1690000000030 } from './../common/1690000000040-AddMfaColumns';
|
||||
import { CreateWorkflowHistoryTable1692967111175 } from '../common/1692967111175-CreateWorkflowHistoryTable';
|
||||
import { DisallowOrphanExecutions1693554410387 } from '../common/1693554410387-DisallowOrphanExecutions';
|
||||
import { ExecutionSoftDelete1693491613982 } from '../common/1693491613982-ExecutionSoftDelete';
|
||||
|
||||
export const mysqlMigrations: Migration[] = [
|
||||
|
@ -96,5 +97,6 @@ export const mysqlMigrations: Migration[] = [
|
|||
CreateWorkflowNameIndex1691088862123,
|
||||
AddMfaColumns1690000000030,
|
||||
CreateWorkflowHistoryTable1692967111175,
|
||||
DisallowOrphanExecutions1693554410387,
|
||||
ExecutionSoftDelete1693491613982,
|
||||
];
|
||||
|
|
|
@ -44,6 +44,7 @@ import { AddMissingPrimaryKeyOnExecutionData1690787606731 } from './169078760673
|
|||
import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex';
|
||||
import { AddMfaColumns1690000000030 } from './../common/1690000000040-AddMfaColumns';
|
||||
import { CreateWorkflowHistoryTable1692967111175 } from '../common/1692967111175-CreateWorkflowHistoryTable';
|
||||
import { DisallowOrphanExecutions1693554410387 } from '../common/1693554410387-DisallowOrphanExecutions';
|
||||
import { ExecutionSoftDelete1693491613982 } from '../common/1693491613982-ExecutionSoftDelete';
|
||||
|
||||
export const postgresMigrations: Migration[] = [
|
||||
|
@ -92,5 +93,6 @@ export const postgresMigrations: Migration[] = [
|
|||
CreateWorkflowNameIndex1691088862123,
|
||||
AddMfaColumns1690000000030,
|
||||
CreateWorkflowHistoryTable1692967111175,
|
||||
DisallowOrphanExecutions1693554410387,
|
||||
ExecutionSoftDelete1693491613982,
|
||||
];
|
||||
|
|
|
@ -43,6 +43,7 @@ import { RemoveResetPasswordColumns1690000000030 } from './1690000000030-RemoveR
|
|||
import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex';
|
||||
import { AddMfaColumns1690000000030 } from './1690000000040-AddMfaColumns';
|
||||
import { CreateWorkflowHistoryTable1692967111175 } from '../common/1692967111175-CreateWorkflowHistoryTable';
|
||||
import { DisallowOrphanExecutions1693554410387 } from '../common/1693554410387-DisallowOrphanExecutions';
|
||||
import { ExecutionSoftDelete1693491613982 } from './1693491613982-ExecutionSoftDelete';
|
||||
|
||||
const sqliteMigrations: Migration[] = [
|
||||
|
@ -90,6 +91,7 @@ const sqliteMigrations: Migration[] = [
|
|||
CreateWorkflowNameIndex1691088862123,
|
||||
AddMfaColumns1690000000030,
|
||||
CreateWorkflowHistoryTable1692967111175,
|
||||
DisallowOrphanExecutions1693554410387,
|
||||
ExecutionSoftDelete1693491613982,
|
||||
];
|
||||
|
||||
|
|
Loading…
Reference in a new issue