feat/389-Webhooks-Headers-Query-params-Methods-validation#6217
Conversation
…n both canvas, agentflow is out of scope but shows temporary ui
useParams() does not update when window.history.replaceState() is used on first save (bypasses React Router). Fall back to Redux canvas.chatflow.id so NodeInputHandler re-renders reactively when SET_CHATFLOW is dispatched.
There was a problem hiding this comment.
Code Review
This pull request introduces a webhookTrigger start type for Agent Flows, enabling workflows to be triggered via external HTTP requests with configurable validation for methods, headers, and parameters. It also adds support for $webhook.* variables and provides a UI for webhook URL management. Review feedback identifies the need for case-insensitive header handling in both validation and variable resolution to align with Express, and recommends passing the webhook payload as an object to maintain property accessibility in downstream nodes.
| // Required header validation | ||
| const rawHeaderParams = startNode?.data?.inputs?.webhookHeaderParams | ||
| const webhookHeaderParams: Array<{ name: string; required: boolean }> = Array.isArray(rawHeaderParams) ? rawHeaderParams : [] | ||
| const missingHeaders = webhookHeaderParams.filter((p) => p.required && headers?.[p.name] == null).map((p) => p.name) |
There was a problem hiding this comment.
HTTP header names are case-insensitive. Express automatically lowercases all header keys in req.headers. If a user configures a required header with mixed casing (e.g., X-API-Key), this check will fail because it looks for the exact casing in the headers object. You should lowercase the configured header name before checking its presence.
| const missingHeaders = webhookHeaderParams.filter((p) => p.required && headers?.[p.name] == null).map((p) => p.name) | |
| const missingHeaders = webhookHeaderParams.filter((p) => p.required && headers?.[p.name.toLowerCase()] == null).map((p) => p.name) |
| .map(([key, value]) => `${key}: ${value}`) | ||
| .join('\n') | ||
| } else if (incomingInput.webhook) { | ||
| finalInput = JSON.stringify(incomingInput.webhook) |
There was a problem hiding this comment.
Stringifying the webhook payload here causes the Start node to receive a string instead of an object. This prevents subsequent nodes from accessing nested properties via the start node's output (e.g., {{startAgentflow_0.output.webhook.body.foo}}). Since the run method of nodes can handle objects, it's better to pass the webhook object directly.
finalInput = incomingInput.webhookThere was a problem hiding this comment.
nice catch but the fix was in a different location.
Doing as gemini told to pass webhook object directly broke downstream nodes because it was expected to be string
Instead, the fix belongs in the Start node's run(), parse the JSON string back into an object before setting outputData.webhook, so nested path access like {{startAgentflow_0.output.webhook.body.action}} works without affecting the rest of the pipeline.
…k trigger - Add webhookTrigger input type to Start node with HTTP method, content type, and expected headers/query/body param configuration - New /api/v1/webhook/:id route with method, content-type, header, body, and query param validation (400/405/415 on mismatch) - Namespace webhook payload as $webhook.body.*, $webhook.headers.*, $webhook.query.* in the flow runtime - Resolve $webhook.* variables in downstream nodes via buildAgentflow.ts - Auto-unwrap form-encoded `payload` JSON strings (e.g. GitHub webhooks) so $webhook.body.* paths work regardless of content type - Expose webhook variable suggestions in the node variable picker - Show copyable webhook URL in the Start node canvas UI
c43b746 to
58f4676
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the Agentflow Webhook trigger by adding configuration for HTTP methods, Content-Type, and validation for headers, query parameters, and body parameters. It also introduces automatic JSON parsing for form-encoded payloads and namespaces webhook data for better accessibility. Feedback highlights the need for case-insensitive Content-Type validation and more flexible type checking for form-encoded data.
|
|
||
| // Content-Type validation (startsWith handles "application/json; charset=utf-8" variants) | ||
| const webhookContentType = startNode?.data?.inputs?.webhookContentType | ||
| if (webhookContentType && !headers?.['content-type']?.startsWith(webhookContentType)) { |
There was a problem hiding this comment.
The Content-Type validation is case-sensitive, which can lead to unexpected failures if a client sends the header with different casing (e.g., Application/JSON). Since HTTP headers are case-insensitive, the incoming header value should be normalized to lowercase before comparison.
const incomingContentType = (headers?.['content-type'] ?? '').toLowerCase()
if (webhookContentType && !incomingContentType.startsWith(webhookContentType.toLowerCase())) {There was a problem hiding this comment.
webhookContentType will be lowercase since its hardcoded in the dropdown
|
|
||
| // Body type validation (only for params that have an explicit type declared) | ||
| const typeMismatch = webhookBodyParams | ||
| .filter((p) => p.type != null && body?.[p.name] != null && typeof body[p.name] !== p.type) |
There was a problem hiding this comment.
The type validation using typeof may be too restrictive for application/x-www-form-urlencoded requests where numeric or boolean values are transmitted as strings. If the webhook is intended to support standard form-encoded data (outside of the payload JSON wrapper), consider allowing string-to-number or string-to-boolean conversions or loosening this check.
There was a problem hiding this comment.
this is fine since controller already unwraps JSON payload field before validation runs, preserving proper types.
This is done in src/controllers/webhook/index.ts line 22
… and a lowercase headers issue
70ef829 to
847d803
Compare
…-Query-Params-Methods-Validation-Etc
| } | ||
| ] | ||
| }, | ||
| { |
There was a problem hiding this comment.
might need to add client type to avoid showing this in internal app

GETPOSTPUTPATCHDELETETest Videos Demo
query-params-and-auto-complete.mp4
query-params-and-auto-complete.mp4
body-headers-params-work-and-auto-complete.mp4
body-headers-params-work-and-auto-complete.mp4
method-validation-return-405-if-not-matched.mp4
method-validation-return-405-if-not-matched.mp4
headers-required-not-included-will-400.mp4
headers-required-not-included-will-400.mp4
content-type-validation.mp4
content-type-validation.mp4
Screenshots
HTTP Methods

Content Type

Only included these two to keep it simple for now. Can add further
multipart/form-dataandtext/plainin the futureExpected Headers

Expected Query Params

Test Passes
