From 2bd2e72656bca49e68872ca02f027121903bd1f2 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Sat, 13 Jun 2026 16:03:19 -0300 Subject: [PATCH] Validate webhook payload template is JSON before it can run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A webhook node parses its payload template as JSON before resolving placeholders, so a template with unquoted {{ }} placeholders or any malformed JSON could be saved, tested, and activated — only to fail midway through a run. Reject it up front instead: AutomationConfigValidator is the single source of truth for per-node config issues (keyed to the field the editor surfaces them under), enforced on save (field errors), on activate, and before a test run. The editor mirrors the check to disable Test/Activate with a clear reason, and the test panel now surfaces the server's message instead of a generic toast. --- .../Automation/ActivateAutomation.php | 18 +-- .../Automation/Node/RunWebhookNode.php | 2 +- app/Actions/Automation/Run/TestAutomation.php | 13 ++- .../Automations/UpdateAutomationRequest.php | 22 ++-- .../Automation/AutomationConfigValidator.php | 59 ++++++++++ .../Automation/WebhookNodeValidator.php | 36 ++++++ .../components/automations/EditorSidebar.vue | 3 +- .../components/automations/TestRunPanel.vue | 19 ++- .../automations/config-validation.ts | 44 +++++++ .../automations/config/WebhookNodeConfig.vue | 14 +-- resources/js/pages/automations/Form.vue | 4 + resources/js/pages/automations/Settings.vue | 21 +++- .../Automation/WebhookNodeValidationTest.php | 110 ++++++++++++++++++ .../AutomationConfigValidatorTest.php | 56 +++++++++ 14 files changed, 370 insertions(+), 51 deletions(-) create mode 100644 app/Services/Automation/AutomationConfigValidator.php create mode 100644 app/Services/Automation/WebhookNodeValidator.php create mode 100644 resources/js/components/automations/config-validation.ts create mode 100644 tests/Feature/Automation/WebhookNodeValidationTest.php create mode 100644 tests/Unit/Automation/AutomationConfigValidatorTest.php diff --git a/app/Actions/Automation/Automation/ActivateAutomation.php b/app/Actions/Automation/Automation/ActivateAutomation.php index 8b78eac9..a06d340f 100644 --- a/app/Actions/Automation/Automation/ActivateAutomation.php +++ b/app/Actions/Automation/Automation/ActivateAutomation.php @@ -7,12 +7,12 @@ use App\Enums\Automation\Node\Type as NodeType; use App\Enums\Automation\Status; use App\Models\Automation; -use App\Services\Automation\GenerateNodeValidator; +use App\Services\Automation\AutomationConfigValidator; use DomainException; class ActivateAutomation { - public function __construct(private GenerateNodeValidator $generateValidator) {} + public function __construct(private AutomationConfigValidator $configValidator) {} public function __invoke(Automation $automation): Automation { @@ -32,7 +32,7 @@ private function validate(Automation $automation): void $nodes = $automation->nodes ?? []; $connections = $automation->connections ?? []; - $triggers = collect($nodes)->where('type', 'trigger'); + $triggers = collect($nodes)->where('type', NodeType::Trigger->value); if ($triggers->count() !== 1) { throw new DomainException(__('automations.errors.must_have_one_trigger')); } @@ -43,16 +43,10 @@ private function validate(Automation $automation): void throw new DomainException(__('automations.errors.trigger_must_be_connected')); } - foreach ($nodes as $node) { - if (data_get($node, 'type') !== NodeType::Generate->value) { - continue; - } + $issue = $this->configValidator->firstMessage($nodes); - $issue = $this->generateValidator->issueFor((array) data_get($node, 'data', [])); - - if ($issue !== null) { - throw new DomainException($issue); - } + if ($issue !== null) { + throw new DomainException($issue); } } } diff --git a/app/Actions/Automation/Node/RunWebhookNode.php b/app/Actions/Automation/Node/RunWebhookNode.php index ce7475c6..0b5498c9 100644 --- a/app/Actions/Automation/Node/RunWebhookNode.php +++ b/app/Actions/Automation/Node/RunWebhookNode.php @@ -49,7 +49,7 @@ public function __invoke(AutomationRun $run, array $config): NodeRunResult // Parse the template as JSON FIRST, then resolve placeholders in its // string leaves — so a value containing `"`/`&`/newlines can't corrupt // the JSON (the final json_encode escapes it). - $template = $config['payload_template'] ?? '{}'; + $template = (string) data_get($config, 'payload_template', '{}'); $trimmedTemplate = trim($template); if ($trimmedTemplate === '' || $trimmedTemplate === 'null') { diff --git a/app/Actions/Automation/Run/TestAutomation.php b/app/Actions/Automation/Run/TestAutomation.php index 69a58d9b..070378d1 100644 --- a/app/Actions/Automation/Run/TestAutomation.php +++ b/app/Actions/Automation/Run/TestAutomation.php @@ -9,6 +9,8 @@ use App\Models\Automation; use App\Models\AutomationRun; use App\Models\Post; +use App\Services\Automation\AutomationConfigValidator; +use DomainException; /** * Kicks off a manual run from the editor without waiting for the real trigger @@ -25,10 +27,19 @@ */ class TestAutomation { - public function __construct(private AdvanceAutomationRun $advance) {} + public function __construct( + private AdvanceAutomationRun $advance, + private AutomationConfigValidator $configValidator, + ) {} public function __invoke(Automation $automation, bool $withRealData = false): AutomationRun { + $issue = $this->configValidator->firstMessage($automation->nodes ?? []); + + if ($issue !== null) { + throw new DomainException($issue); + } + $triggerNode = collect($automation->nodes ?? [])->firstWhere('type', 'trigger'); $context = ['trigger' => $this->synthesizePayload($automation, $triggerNode ?? [])]; diff --git a/app/Http/Requests/App/Automations/UpdateAutomationRequest.php b/app/Http/Requests/App/Automations/UpdateAutomationRequest.php index 4937bd88..bae5eb20 100644 --- a/app/Http/Requests/App/Automations/UpdateAutomationRequest.php +++ b/app/Http/Requests/App/Automations/UpdateAutomationRequest.php @@ -12,6 +12,7 @@ use App\Enums\Automation\Publish\Mode as PublishMode; use App\Enums\Automation\ScheduleField; use App\Enums\Automation\Trigger\Type as TriggerType; +use App\Services\Automation\AutomationConfigValidator; use App\Services\Automation\GenerateNodeValidator; use Illuminate\Contracts\Validation\Validator; use Illuminate\Foundation\Http\FormRequest; @@ -66,9 +67,10 @@ public function rules(): array } /** - * Block saving a Generate node whose intended image count doesn't fit a - * selected account's content-type (mirrors the inline frontend validation), - * keyed so the frontend surfaces it under that node's accounts field. + * Block saving a node whose config can't run: a Generate node whose image + * count doesn't fit a selected account's content-type, or a Webhook node + * whose payload template isn't valid JSON. Each issue is keyed to the field + * the frontend surfaces it under (mirrors the inline frontend validation). */ public function withValidator(Validator $validator): void { @@ -79,18 +81,8 @@ public function withValidator(Validator $validator): void return; } - $generateValidator = app(GenerateNodeValidator::class); - - foreach ($nodes as $i => $node) { - if (data_get($node, 'type') !== NodeType::Generate->value) { - continue; - } - - $issue = $generateValidator->issueFor((array) data_get($node, 'data', [])); - - if ($issue !== null) { - $validator->errors()->add("nodes.{$i}.data.accounts", $issue); - } + foreach (app(AutomationConfigValidator::class)->issues($nodes) as $issue) { + $validator->errors()->add("nodes.{$issue['node_index']}.data.{$issue['field']}", $issue['message']); } }); } diff --git a/app/Services/Automation/AutomationConfigValidator.php b/app/Services/Automation/AutomationConfigValidator.php new file mode 100644 index 00000000..aa0236a5 --- /dev/null +++ b/app/Services/Automation/AutomationConfigValidator.php @@ -0,0 +1,59 @@ +> $nodes + * @return list + */ + public function issues(array $nodes): array + { + $issues = []; + + foreach ($nodes as $index => $node) { + $config = (array) data_get($node, 'data', []); + + [$field, $message] = match (data_get($node, 'type')) { + NodeType::Generate->value => ['accounts', $this->generateValidator->issueFor($config)], + NodeType::Webhook->value => ['payload_template', $this->webhookValidator->issueFor($config)], + default => [null, null], + }; + + if ($message !== null) { + $issues[] = ['node_index' => $index, 'field' => $field, 'message' => $message]; + } + } + + return $issues; + } + + /** + * The first config issue's message, or null when every node is runnable. + * + * @param array> $nodes + */ + public function firstMessage(array $nodes): ?string + { + return $this->issues($nodes)[0]['message'] ?? null; + } +} diff --git a/app/Services/Automation/WebhookNodeValidator.php b/app/Services/Automation/WebhookNodeValidator.php new file mode 100644 index 00000000..ab5b35bc --- /dev/null +++ b/app/Services/Automation/WebhookNodeValidator.php @@ -0,0 +1,36 @@ + $config + */ + public function issueFor(array $config): ?string + { + $template = trim((string) data_get($config, 'payload_template', '')); + + if ($template === '' || $template === 'null') { + return null; + } + + json_decode($template); + + if (json_last_error() !== JSON_ERROR_NONE) { + return __('automations.errors.webhook_invalid_payload_json'); + } + + return null; + } +} diff --git a/resources/js/components/automations/EditorSidebar.vue b/resources/js/components/automations/EditorSidebar.vue index eaa1d87d..b1e32edb 100644 --- a/resources/js/components/automations/EditorSidebar.vue +++ b/resources/js/components/automations/EditorSidebar.vue @@ -11,6 +11,7 @@ import type { AutomationVariable } from '@/types/automation/automation'; defineProps<{ automationId: string; beforeRun?: () => Promise | boolean; + configIssue?: string | null; editing?: boolean; nodeTitle?: string; deletable?: boolean; @@ -63,7 +64,7 @@ const variables = defineModel('variables', { default: () = - + diff --git a/resources/js/components/automations/TestRunPanel.vue b/resources/js/components/automations/TestRunPanel.vue index 1488cc68..90dba74c 100644 --- a/resources/js/components/automations/TestRunPanel.vue +++ b/resources/js/components/automations/TestRunPanel.vue @@ -33,7 +33,7 @@ interface Run { is_dry_run: boolean; } -const props = defineProps<{ automationId: string; beforeRun?: () => Promise | boolean }>(); +const props = defineProps<{ automationId: string; beforeRun?: () => Promise | boolean; configIssue?: string | null }>(); const isStarting = ref(false); const realData = ref(false); @@ -88,20 +88,22 @@ const start = async () => { body: JSON.stringify({ with_real_data: realData.value }), }); if (!response.ok) { - throw new Error('start failed'); + const body = await response.json().catch(() => null); + throw new Error(body?.message ?? ''); } const { run_id: runId } = await response.json(); activeRunId.value = runId; await fetchRun(runId); - } catch { - toast.error(trans('automations.test.error_starting')); + } catch (error) { + const message = error instanceof Error && error.message ? error.message : trans('automations.test.error_starting'); + toast.error(message); } finally { isStarting.value = false; } }; const runTest = async () => { - if (isStarting.value) return; + if (isStarting.value || props.configIssue) return; const proceed = await (props.beforeRun?.() ?? true); if (proceed === false) return; await start(); @@ -154,13 +156,18 @@ const isZeroFetchResult = (nodeRun: NodeRun): boolean => { {{ $t('automations.test.with_real_data') }} - +

+ + {{ configIssue }} +

+
| null; +} + +/** + * Mirrors the backend WebhookNodeValidator: a webhook payload template must be + * valid JSON because the runtime parses it before resolving `{{ }}` placeholders. + * An empty or literal-`null` template means "no body" and is valid. + */ +export const isPayloadTemplateValid = (template: string): boolean => { + const trimmed = template.trim(); + + if (trimmed === '' || trimmed === 'null') { + return true; + } + + try { + JSON.parse(trimmed); + return true; + } catch { + return false; + } +}; + +/** + * First node-config issue that would block a run, or null when every node is + * runnable. The frontend gate intentionally covers only the Webhook node — the + * Generate node has its own inline compliance UI, and the backend + * AutomationConfigValidator remains the safety net for everything. + */ +export const firstConfigIssue = (nodes: WorkflowNode[]): string | null => { + for (const node of nodes) { + if (node.type === NodeType.Webhook && !isPayloadTemplateValid(String(node.data?.payload_template ?? ''))) { + return trans('automations.errors.webhook_invalid_payload_json'); + } + } + + return null; +}; diff --git a/resources/js/components/automations/config/WebhookNodeConfig.vue b/resources/js/components/automations/config/WebhookNodeConfig.vue index 0f71ed7b..ad07a9de 100644 --- a/resources/js/components/automations/config/WebhookNodeConfig.vue +++ b/resources/js/components/automations/config/WebhookNodeConfig.vue @@ -1,6 +1,7 @@