mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-18 05:59:42 +02:00
fix(API): Match the published request contract for workflow create
The migrated body accepted four things the hand-written spec rejected with a 400, found by diffing live responses against master: - `description`, which `workflowCreate.yml` never listed - nodes carrying unknown or wrongly typed properties, which `node.yml` constrained with `additionalProperties: false` - a `staticData` string that is not JSON, which `format: jsonString` caught - and it rejected `shared`, the one relation the old spec left writable A non-object node also reached the persistence layer and surfaced as a 500 rather than a 400. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
99f968ae21
commit
d17f24448e
|
|
@ -16,7 +16,6 @@ describe('CreateWorkflowPublicDto', () => {
|
|||
test('accepts every writable optional field', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
description: 'A test workflow',
|
||||
staticData: { key: 'value' },
|
||||
pinData: { Start: [{ json: {} }] },
|
||||
nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: [] }],
|
||||
|
|
@ -27,6 +26,15 @@ describe('CreateWorkflowPublicDto', () => {
|
|||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test('rejects description, which only update accepts', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
description: 'A test workflow',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test('accepts staticData as a raw JSON string', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
|
|
@ -37,6 +45,24 @@ describe('CreateWorkflowPublicDto', () => {
|
|||
expect(result.data?.staticData).toBe('{"id":1}');
|
||||
});
|
||||
|
||||
test('rejects staticData as a string that is not JSON', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
staticData: 'not json',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test('accepts shared, which the old spec never marked read-only', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
shared: [{ role: 'workflow:owner' }],
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test('accepts a null parentFolderId', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
|
|
@ -119,6 +145,106 @@ describe('CreateWorkflowPublicDto', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('nodes', () => {
|
||||
const node = {
|
||||
id: '0f5532f9-36ba-4bef-86c7-30d607400b15',
|
||||
name: 'Start',
|
||||
type: 'n8n-nodes-base.start',
|
||||
typeVersion: 1,
|
||||
position: [100, 200],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
test('accepts a fully specified node', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
nodes: [
|
||||
{
|
||||
...node,
|
||||
webhookId: 'wh1',
|
||||
disabled: false,
|
||||
notes: 'hello',
|
||||
notesInFlow: true,
|
||||
executeOnce: true,
|
||||
alwaysOutputData: true,
|
||||
retryOnFail: true,
|
||||
maxTries: 3,
|
||||
waitBetweenTries: 1000,
|
||||
continueOnFail: false,
|
||||
onError: 'continueRegularOutput',
|
||||
credentials: { jiraSoftwareCloudApi: { id: '35', name: 'jiraApi' } },
|
||||
customTelemetryTags: { tag: [{ key: 'a', value: 'b' }] },
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test('accepts an empty node object, which the old spec required nothing of', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({ ...minimalWorkflow, nodes: [{}] });
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['an unknown property', { ...node, bogusProp: 1 }],
|
||||
['extendsCredential', { ...node, extendsCredential: 'x' }],
|
||||
['rewireOutputLogTo', { ...node, rewireOutputLogTo: 'main' }],
|
||||
[
|
||||
'forceCustomOperation',
|
||||
{ ...node, forceCustomOperation: { resource: 'a', operation: 'b' } },
|
||||
],
|
||||
['the read-only createdAt', { ...node, createdAt: '2024-01-01T00:00:00.000Z' }],
|
||||
['the read-only updatedAt', { ...node, updatedAt: '2024-01-01T00:00:00.000Z' }],
|
||||
])('rejects a node carrying %s', (_label, value) => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({ ...minimalWorkflow, nodes: [value] });
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['name', 7],
|
||||
['type', 7],
|
||||
['typeVersion', '1'],
|
||||
['disabled', 'yes'],
|
||||
['position', { x: 1 }],
|
||||
['parameters', []],
|
||||
['credentials', 'x'],
|
||||
])('rejects a wrongly typed node %s', (field, value) => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
nodes: [{ ...node, [field]: value }],
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test('rejects a non-numeric position entry', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
nodes: [{ ...node, position: ['a', 'b'] }],
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test('rejects a customTelemetryTags entry missing value', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
...minimalWorkflow,
|
||||
nodes: [{ ...node, customTelemetryTags: { tag: [{ key: 'a' }] } }],
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['a scalar', 'x'],
|
||||
['null', null],
|
||||
])('rejects %s in place of a node object', (_label, value) => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({ ...minimalWorkflow, nodes: [value] });
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('settings', () => {
|
||||
test('accepts every documented setting', () => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({
|
||||
|
|
@ -217,7 +343,6 @@ describe('CreateWorkflowPublicDto', () => {
|
|||
['triggerCount', 3],
|
||||
['meta', { templateId: 't1' }],
|
||||
['tags', [{ id: 't1', name: 'prod' }]],
|
||||
['shared', [{ role: 'workflow:owner' }]],
|
||||
['activeVersion', { versionId: 'v1' }],
|
||||
])('rejects a supplied %s with a read-only error', (field, value) => {
|
||||
const result = CreateWorkflowPublicDto.safeParse({ ...minimalWorkflow, [field]: value });
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { IConnections, INode } from 'n8n-workflow';
|
||||
import type { IConnections } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type { workflowPublicSchema } from './workflow-public.dto';
|
||||
|
|
@ -33,7 +33,6 @@ const READ_ONLY_PUBLIC_FIELDS = new Set<string>([
|
|||
'triggerCount',
|
||||
'meta',
|
||||
'tags',
|
||||
'shared',
|
||||
'activeVersion',
|
||||
] satisfies Array<keyof typeof workflowPublicSchema.shape>);
|
||||
|
||||
|
|
@ -52,15 +51,46 @@ export const readOnlyPublicFieldErrorMap: z.ZodErrorMap = (issue, ctx) => {
|
|||
return { message: ctx.defaultError };
|
||||
};
|
||||
|
||||
// Nodes, connections, settings, static data, and pin data all carry arbitrary,
|
||||
// node-type-specific shapes that only the workflow engine can fully validate --
|
||||
// re-validating their internals here would risk rejecting legitimately shaped
|
||||
// data. Mirrors the shape-only checks `WorkflowPublicDto` already applies on the
|
||||
// way out; the engine (`WorkflowHelpers.validateWorkflowStructure`, etc.) still
|
||||
// validates the real structure server-side, exactly as it did before this DTO existed.
|
||||
const nodesWritePublicSchema = z.custom<INode[]>((value) => Array.isArray(value), {
|
||||
message: 'Nodes must be an array',
|
||||
});
|
||||
/**
|
||||
* Mirrors the hand-written `node.yml`, down to `additionalProperties: false` and its lack of any
|
||||
* required property. A node the old API rejected -- an unknown key, a wrongly typed field, a
|
||||
* non-object -- has to keep being rejected here, so every property it listed is reproduced even
|
||||
* where `INode` is wider (`extendsCredential`, `rewireOutputLogTo` and `forceCustomOperation` are
|
||||
* absent on purpose: the published contract never accepted them).
|
||||
*
|
||||
* `createdAt` and `updatedAt` are omitted for the same reason the workflow-level server-managed
|
||||
* fields are -- see `READ_ONLY_PUBLIC_FIELDS` above.
|
||||
*/
|
||||
const nodeWritePublicSchema = z
|
||||
.object({
|
||||
id: z.string().optional(),
|
||||
name: z.string().optional(),
|
||||
webhookId: z.string().optional(),
|
||||
disabled: z.boolean().optional(),
|
||||
notesInFlow: z.boolean().optional(),
|
||||
notes: z.string().optional(),
|
||||
type: z.string().optional(),
|
||||
typeVersion: z.number().optional(),
|
||||
executeOnce: z.boolean().optional(),
|
||||
alwaysOutputData: z.boolean().optional(),
|
||||
retryOnFail: z.boolean().optional(),
|
||||
maxTries: z.number().optional(),
|
||||
waitBetweenTries: z.number().optional(),
|
||||
continueOnFail: z.boolean().optional(),
|
||||
onError: z.string().optional(),
|
||||
position: z.array(z.number()).optional(),
|
||||
parameters: z.record(z.string(), z.unknown()).optional(),
|
||||
credentials: z.record(z.string(), z.unknown()).optional(),
|
||||
customTelemetryTags: z
|
||||
.object({
|
||||
tag: z.array(z.object({ key: z.string(), value: z.string() }).strict()).optional(),
|
||||
})
|
||||
.strict()
|
||||
.optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
const nodesWritePublicSchema = z.array(nodeWritePublicSchema);
|
||||
|
||||
const connectionsWritePublicSchema = z.custom<IConnections>(
|
||||
(value) => typeof value === 'object' && value !== null && !Array.isArray(value),
|
||||
|
|
@ -112,21 +142,61 @@ const settingsWritePublicSchema = z
|
|||
|
||||
// Accepts the pre-parsed object shape or a raw JSON string, matching the wire contract
|
||||
// (`anyOf: [jsonString, object]`) -- parsing the string is left to the persistence layer's
|
||||
// column transformer, exactly as it was before this DTO existed.
|
||||
// column transformer, exactly as it was before this DTO existed. The string still has to *be*
|
||||
// JSON: `format: jsonString` made the old API reject anything else with a 400, and without the
|
||||
// check the persistence layer throws on a parse failure and the caller sees a 500.
|
||||
const jsonStringPublicSchema = z.string().refine(
|
||||
(value) => {
|
||||
try {
|
||||
JSON.parse(value);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
},
|
||||
{ message: 'Must be a JSON string' },
|
||||
);
|
||||
|
||||
const staticDataWritePublicSchema = z
|
||||
.union([z.string(), z.record(z.string(), z.unknown())])
|
||||
.union([jsonStringPublicSchema, z.record(z.string(), z.unknown())])
|
||||
.nullable();
|
||||
|
||||
const pinDataWritePublicSchema = z.record(z.string(), z.unknown()).nullable();
|
||||
|
||||
/**
|
||||
* Fields shared by the create and update Public API request bodies: both accept the same
|
||||
* writable workflow shape (the update body is a full-object `PUT`, not a partial `PATCH`).
|
||||
* `CreateWorkflowPublicDto` extends this with `projectId`; nothing else differs.
|
||||
* `sharedWorkflow.yml` was the one relation the old spec never marked `readOnly`, so a caller could
|
||||
* send `shared` and have it accepted. The entity mapper's write allowlist drops it, so it has never
|
||||
* had any effect -- but rejecting it now would turn a working request into a 400.
|
||||
*/
|
||||
const sharedWritePublicSchema = z.array(
|
||||
z
|
||||
.object({
|
||||
role: z.string().optional(),
|
||||
workflowId: z.string().optional(),
|
||||
projectId: z.string().optional(),
|
||||
project: z
|
||||
.object({
|
||||
id: z.string().optional(),
|
||||
name: z.string().optional(),
|
||||
type: z.string().optional(),
|
||||
})
|
||||
.passthrough()
|
||||
.optional(),
|
||||
})
|
||||
.strict(),
|
||||
);
|
||||
|
||||
/**
|
||||
* Fields the create and update Public API request bodies have in common (both are full-object
|
||||
* bodies -- the update is a `PUT`, not a partial `PATCH`).
|
||||
*
|
||||
* The two hand-written bodies were not identical, and each difference is reproduced at the call
|
||||
* site rather than merged in here: `workflowCreate.yml` had `projectId` and no `description`,
|
||||
* `workflow.yml` had `description` and no `projectId`. Sharing either one would start accepting a
|
||||
* field the old API answered with a 400.
|
||||
*/
|
||||
export const workflowWritePublicShape = {
|
||||
name: z.string(),
|
||||
description: z.string().nullable().optional(),
|
||||
nodes: nodesWritePublicSchema,
|
||||
connections: connectionsWritePublicSchema,
|
||||
nodeGroups: nodeGroupsWritePublicSchema.optional(),
|
||||
|
|
@ -134,4 +204,5 @@ export const workflowWritePublicShape = {
|
|||
staticData: staticDataWritePublicSchema.optional(),
|
||||
pinData: pinDataWritePublicSchema.optional(),
|
||||
parentFolderId: z.string().nullable().optional(),
|
||||
shared: sharedWritePublicSchema.optional(),
|
||||
} as const;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user