Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -420,4 +420,68 @@ describe("parseWorkflowYamlToJSON", () => {
expect(result.error).toBeUndefined();
expect(result.success).toBe(true);
});

// Regression for #6274: `with:` written as a sibling of `provider:` instead of
// nested inside it used to be accepted silently by the import path, producing a
// workflow that saved and fired with zero parameters. The schema is `.strict()`,
// so validating on import surfaces it instead.
it("should reject `with:` placed as a sibling of `provider:` instead of nested", () => {
const misplacedWithYaml = `workflow:
id: test-with-nesting
name: Test
description: Regression fixture for issue 6274
triggers:
- type: alert
filters:
- key: source
value: prometheus
actions:
- name: test-ntfy
provider:
type: ntfy
config: default
with:
message: "test"`;

const result = parseWorkflowYamlToJSON(
misplacedWithYaml,
workflowSchemaWithProviders
);

expect(result.success).toBe(false);
expect(result.error?.issues).toEqual(
expect.arrayContaining([
expect.objectContaining({ code: "unrecognized_keys" }),
])
);
});

it("should accept the same workflow once `with:` is nested under `provider:`", () => {
// Guard rail: the check above must reject only the misplacement, not the
// workflow shape itself.
const correctYaml = `workflow:
id: test-with-nesting
name: Test
description: Regression fixture for issue 6274
triggers:
- type: alert
filters:
- key: source
value: prometheus
actions:
- name: test-ntfy
provider:
type: ntfy
config: default
with:
message: "test"`;

const result = parseWorkflowYamlToJSON(
correctYaml,
workflowSchemaWithProviders
);

expect(result.success).toBe(true);
expect(result.error).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { parseWorkflowYamlToJSON } from "@/entities/workflows/lib/yaml-utils";
import { fromZodError } from "zod-validation-error";

/**
* Regression for #6274.
*
* The YAML import path in `workflow-builder-widget.tsx` used to call
* `parseWorkflowYamlStringToJSON`, a bare `yaml.parse()` with no schema
* validation. A `with:` written as a sibling of `provider:` (rather than nested
* inside it) was therefore accepted, but every consumer reads
* `provider.with` — so the parameters were silently dropped and the workflow
* saved and fired with none of them.
*
* These tests pin the validation the import handler now performs.
*/

// Mirrors the import handler in workflow-builder-widget.tsx
const validateImportedWorkflow = (contents: string): string | null => {
const result = parseWorkflowYamlToJSON(contents);
if (!result.success) {
return fromZodError(result.error).toString();
}
return null;
};

const MISPLACED_WITH = `workflow:
id: test-with-nesting
name: Test
description: Regression fixture for issue 6274
triggers:
- type: alert
filters:
- key: source
value: prometheus
actions:
- name: test-ntfy
provider:
type: ntfy
config: default
with:
message: "test"`;

const NESTED_WITH = `workflow:
id: test-with-nesting
name: Test
description: Regression fixture for issue 6274
triggers:
- type: alert
filters:
- key: source
value: prometheus
actions:
- name: test-ntfy
provider:
type: ntfy
config: default
with:
message: "test"`;

describe("workflow YAML import validation", () => {
it("rejects a workflow whose `with:` is a sibling of `provider:`", () => {
const error = validateImportedWorkflow(MISPLACED_WITH);

expect(error).not.toBeNull();
// The message must name the offending key so the user can act on it.
expect(error).toContain("with");
});

it("accepts the same workflow once `with:` is nested under `provider:`", () => {
// Guard rail: validation must reject only the misplacement, not the shape.
expect(validateImportedWorkflow(NESTED_WITH)).toBeNull();
});

it("the misplaced form is exactly what every consumer would read as empty", () => {
// Shows why silently accepting it is harmful: `provider.with` — the path the
// backend parser and the UI both read — is undefined, while the parameters
// sit unreachable at step level.
const parsed: any = parseWorkflowYamlToJSON(MISPLACED_WITH);
expect(parsed.success).toBe(false);

const raw: any = require("yaml").parse(MISPLACED_WITH);
const action = raw.workflow.actions[0];
expect(action.provider.with).toBeUndefined();
expect(action.with).toEqual({ message: "test" });
});
});
13 changes: 11 additions & 2 deletions keep-ui/widgets/workflow-builder/workflow-builder-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { useWorkflowStore } from "@/entities/workflows";
import { WorkflowMetadataModal } from "@/features/workflows/edit-metadata";
import { WorkflowEnabledSwitch } from "@/features/workflows/enable-disable";
import { WorkflowSyncStatus } from "@/app/(keep)/workflows/[workflow_id]/workflow-sync-status";
import { parseWorkflowYamlStringToJSON } from "@/entities/workflows/lib/yaml-utils";
import { parseWorkflowYamlToJSON } from "@/entities/workflows/lib/yaml-utils";
import { fromZodError } from "zod-validation-error";
import clsx from "clsx";
import { WorkflowTestRunButton } from "@/features/workflows/test-run/ui/workflow-test-run-button";
import { useUIBuilderUnsavedChanges } from "@/entities/workflows/model/workflow-store";
Expand Down Expand Up @@ -59,7 +60,15 @@ export function WorkflowBuilderWidget({
setFileName(fName);
const contents = event.target!.result as string;
try {
const _ = parseWorkflowYamlStringToJSON(contents);
// Validate against the workflow schema rather than doing a bare YAML
// parse. The schema is `.strict()`, so a misplaced key — most commonly
// `with:` written as a sibling of `provider:` instead of nested inside
// it — is reported here instead of being silently dropped and saved as
// a workflow that runs with no parameters.
const result = parseWorkflowYamlToJSON(contents);
if (!result.success) {
throw new Error(fromZodError(result.error).toString());
}
setFileContents(contents);
} catch (error) {
showErrorToast(error, "Failed to parse workflow");
Expand Down
Loading