feat(zoho-desk): add Zoho Desk integration - #6157
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Authentication introduces a Product/docs: registers The PR description notes live verification still needed for Self Client Reviewed by Cursor Bugbot for commit bfe11ab. Configure here. |
Greptile SummaryAdds a complete Zoho Desk integration covering OAuth and service-account credentials, ticket operations, selectors, attachment downloads, webhook subscription management, and verified trigger ingestion.
Confidence Score: 5/5The PR appears safe to merge, with no blocking failure remaining from the reviewed attachment-security threads. The permissive hostname check has been replaced by strict Zoho apex matching, while redirected requests remove the OAuth authorization header as explicitly accepted in the prior discussion; no blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/tools/zoho_desk/attachment/route.ts | Resolves and validates HTTPS Zoho attachment URLs, performs DNS-pinned retrieval, and strips OAuth authorization on redirects. |
| apps/sim/tools/zoho_desk/host-allowlist.ts | Defines strict apex and subdomain matching for supported Zoho data-center hosts. |
| apps/sim/lib/webhooks/providers/zoho-desk.ts | Implements Zoho webhook subscription lifecycle, JWT verification, event filtering, and payload normalization. |
| apps/sim/blocks/blocks/zoho-desk.ts | Adds the Zoho Desk block configuration, operation-specific fields, credentials, selectors, and trigger wiring. |
| apps/sim/serializer/index.ts | Excludes advanced-trigger-only fields from ordinary tool serialization consistently with standard trigger fields. |
| apps/sim/lib/oauth/oauth.ts | Extends OAuth handling to retain the regional Zoho Desk API domain required for subsequent calls. |
Sequence Diagram
sequenceDiagram
participant Zoho as Zoho Desk
participant Hook as Sim Webhook Ingress
participant Verify as Zoho JWT Verifier
participant Queue as Durable Queue
participant Exec as Workflow Executor
Hook->>Zoho: Create event subscription
Zoho->>Hook: Event + X-ZDesk-JWT
Hook->>Verify: Validate RS256 signature, issuer, audience
Verify-->>Hook: Verified event
Hook->>Queue: Enqueue normalized webhook payload
Hook-->>Zoho: Acknowledge delivery
Queue->>Exec: Execute deployed workflow
Reviews (15): Last reviewed commit: "test(zoho-desk): cover the webhook subsc..." | Re-trigger Greptile
278dc26 to
310a216
Compare
|
@cursor review |
|
@cursor review |
|
@greptile review |
|
@cursor review |
|
@cursor review |
|
@cursor review |
# Conflicts: # scripts/check-api-validation-contracts.ts
|
@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 66220e4. Configure here.
…scopes, paging
Five independent audits (OAuth/scopes, tools-vs-docs, block/selectors,
blast-radius, /validate-trigger). Findings, most severe first.
A trigger-mode field was a live tool-mode required param. `shouldSerializeSubBlock`
excluded `mode: 'trigger'` but not `'trigger-advanced'`, so the trigger's required
`manualOrgId` validated on every tool operation. Reproduced against the real
serializer: with the Organization field pinned to advanced, running
List Organizations failed with "Missing required fields: Organization ID" - a
field that operation does not even render, and which the user could not clear
without switching operations. Fixed in the serializer rather than locally,
because the Google Sheets/Drive/Calendar pollers have the identical shape.
`limit=200` on /organizations was an undocumented parameter I added by
extrapolating from /departments and /agents. Zoho documents NO parameters for
that endpoint and its sample is a bare GET; the other siblings cap at 100 and
Zoho answers out-of-range with 422. Since orgId gates every tool and both other
selectors, a 422 there would have made the whole integration unreachable. Reverted
to Zoho's documented shape.
`descriptionText` was HTML-stripping plain text. The previous round made the strip
unconditional after finding Zoho sends no `descriptionContentType`, but Zoho's REST
samples show plain descriptions while only the webhook payload is HTML - and the
webhook path runs this over contact/account/department bodies too. html-to-text is
not identity on plain text: it decodes entities and deletes tag-shaped content
("a < b > c", XML snippets). Now sniffs for markup first.
`omitUnset` made every documented field-clear impossible. Zoho's own PATCH sample
uses `"classification": ""` and `"productId": ""` to clear. Dropping `''` meant no
scalar field could be cleared. Now drops only undefined/null - the serializer-null
case it was written for - and forwards `''`.
status/priority leaked between operations. One shared subBlock served both the
list_tickets filter and the update_ticket value, and subBlock values survive an
operation switch, so a filter of "Open,On Hold" could be PATCHed onto a ticket and
an update value could silently filter a later list. Split per operation.
Auth: `invalid_code` added to TERMINAL_ERRORS - it is Zoho's code for a revoked
refresh token, so without it the previous round's refresh fix never actually
dead-flagged the credential it was written for. The shared refresh body-error
branch now also requires `!data.access_token`, so no provider can have a
successful refresh misclassified. The token route now uses the validating
`extractZohoDeskBaseFromScope` instead of a private regex with no https/allowlist
check - that value is injected into every tool call. Scope list falls back to the
requested scopes when Zoho omits `scope`, which would otherwise flag every
credential as needing reconnect. The Self Client mint no longer sends
`aaaserver.profile.READ`, a scope that grant never uses.
Trigger: `includePrevState` now set for every *_Update event, not just tickets -
it defaults to false, so prevState was permanently null for contact/agent/task/
article updates while the trigger advertised it. `departmentIds` is only sent for
events Zoho documents as accepting it, and the field is conditioned accordingly.
Empty filters serialize as `null`, matching Zoho's examples, rather than `{}`.
JWKS fetch bounded to 1.5s - jose's default is 5000ms, exactly Zoho's whole
delivery deadline, and Zoho publishes no retry. The create-time validation POST
fallback is now matched by the pending-verification probe. Ticket_Delete added.
All 17 webhook event ids, the POST /api/v1/webhooks body contract, and the JWT
claim/JWKS specifics are now confirmed verbatim against Zoho's webhook
documentation - previously 12 of 17 events and the entire subscription contract
were unverified.
Reverting two changes to shared OAuth code because their premise is inferred rather than proven, and neither meets the bar for touching a path every provider runs. `refreshOAuthToken` body-error branch. The premise was that Zoho reports refresh failures with HTTP 200 and an `error` body. That is documented and empirically confirmed for the authorization-code EXCHANGE (see the comment on getToken in auth.ts), but I never confirmed it for the REFRESH grant specifically - and if Zoho returns a proper 4xx there, the existing `!response.ok` path already classifies it via extractErrorCode, making the branch dead code that every one of the ~34 providers still executes on each refresh. A shared branch whose only justification is an unverified inference about one provider is not worth its blast radius. `invalid_code` in TERMINAL_ERRORS. Same problem, worse downside: the code is sourced from a Zoho community post rather than official docs, TERMINAL_ERRORS is consulted for every provider, and a false positive marks a credential dead for an hour. Not adding it simply preserves today's behavior (retry rather than dead-flag), so reverting costs nothing that was previously working. Both are cheap to reinstate, correctly scoped, once a live Zoho account shows what a revoked refresh token actually returns. Kept: the token-redaction on the "no access token" warn, which is an unambiguous improvement independent of Zoho. Also kept, deliberately, is the serializer `trigger-advanced` exclusion - that one rests on a reproduced bug rather than an inference, and it aligns the serializer with the convention the rest of the codebase already follows (blocks.test.ts treats `trigger` and `trigger-advanced` identically in six places, as does the copilot block-metadata tool, and blocks/types.ts documents trigger-advanced as "the advanced side of a trigger field").
|
@cursor review |
…onnect A reconnect rebuilds the service-account secret blob from the submitted fields only, and the connect modal never prefills - correctly, since for every other field in this family the stored value is a secret the admin must retype. The data center is the first non-secret member of that set, so it was being silently dropped: rotating a client secret on an EU/IN/AU credential moved it back to the US accounts server, where the next mint fails with an opaque invalid_client. performUpdateCredential now reads the stored dataCenter out of the existing blob when the caller does not supply one. The read is failure-tolerant - an undecryptable or unparseable blob yields undefined rather than throwing, so it can never block a reconnect, and the provider default applies as before. Raised independently by three reviewers; I twice argued it was acceptable because the mint fails loudly rather than corrupting silently. That was true and beside the point - the operator still had to guess why.
|
@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 50680f7. Configure here.
…HTML sniffer An audit of the commits the earlier five audits never saw. All four findings are in code written as fixes for those audits, which is where this branch has repeatedly introduced new problems. `includePrevState` was sent for Ticket_Comment_Update. The previous commit gated it on an `_Update` suffix and claimed Zoho supports it on every update event. Zoho's webhook doc lists the attribute on Ticket/Contact/Agent/Task/Article update events but NOT on Ticket_Comment_Update, which documents only `departmentIds`. That made it an undocumented filter key on a live subscription create - the same class of risk the same commit reverted `limit=200` for, so it failed that commit's own stated bar. Now an explicit set rather than a suffix rule. The status/priority split did not stop the leak it was written for. The mapping used `operation === 'list_tickets' ? filterValue : updateValue`, whose bare else covers all eight other operations - so a stale Update Ticket status was forwarded into get_ticket, list_comments and the rest. Harmless on the wire (those tools ignore it) but exactly the stale-value pattern the neighbouring gates exist to prevent. Both fields are now scoped to the two operations that declare them. The HTML sniffer destroyed plain text. `/<[a-z!\/][^>]*>/` fires on any `<` followed by a letter with a later `>`, so realistic ticket bodies lost content: "if x<y then z>0" became "if x0", and "replace <username> with the real name" lost the placeholder. It now requires a real element - a paired tag, a self-closing tag, a comment/doctype - or an entity, and the entity arm covers hex references it previously missed. Regression tests verified by reverting to the loose pattern and watching them go red. The reconnect data-center carry-forward is scoped to client-credential providers. As written it added a DB read plus a decrypt to every service-account reconnect for every provider - Slack, Atlassian, all token-paste providers - to carry a field only Zoho has. Also: the JWKS cache-bound TSDoc had been orphaned onto the wrong constant by an earlier insertion, and `cooldownDuration` was dropped since it restated jose's default while only `timeoutDuration` needed justifying.
|
@cursor review |
The subscription filter logic had no test coverage at all, and it is where the
last two rounds both found bugs - includePrevState on an event Zoho does not
document it for, and departmentIds sent to events that accept no filters.
Adds six cases against the real createSubscription: includePrevState is set for
each of the five documented update events and NOT for Ticket_Comment_Update,
departmentIds is kept for a filterable event and dropped for one that is not, and
an event with no filters serializes as null rather than an empty object.
Verified the guard bites: reverting PREV_STATE_EVENTS to the `endsWith('_Update')`
rule turns the Ticket_Comment_Update case red.
The Ticket_Comment_Update assertion checks the with-departments case as well as
the bare one - asserting only `not.toHaveProperty` on the bare filter would pass
vacuously, since that filter is legitimately null.
|
@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 bfe11ab. Configure here.
Summary
Adds a full Zoho Desk integration — tools, block, icon, and a programmatic webhook trigger.
Tools (
apps/sim/tools/zoho_desk)list_tickets,get_ticket,update_ticketlist_comments,add_commentlist_threads,get_threadget_contactlist_organizations(also backs the org selector)get_attachment— downloads a ticket/thread/comment attachment (from its documentedhref) as a SimUserFilevia an internal route, SSRF-guarded to Zoho hostsBlock (
apps/sim/blocks/blocks/zoho-desk.ts)Operation dropdown, OAuth credential (basic + advanced), an organization selector populated from
GET /organizations, per-operation conditional fields, and 8BlockMetatemplates. Wires the Zoho Desk trigger.OAuth (
zoho-deskprovider)Authorize/token at
accounts.zoho.comwithaccess_type=offline+prompt=consent. The Desk REST base is derived from the token responseapi_domainand persisted (Salesforce__sf_instance__precedent) so every call targets the correct data center instead of assumingdesk.zoho.com. All calls sendAuthorization: Zoho-oauthtoken {token}and theorgIdheader.Trigger + webhook handler
triggers/zoho_desk+lib/webhooks/providers/zoho-desk.ts: Sim creates and tears down the Zoho Desk webhook subscription (POST/DELETE /api/v1/webhooks) with per-event filters (departmentIds,includePrevState/fields forTicket_Update, direction forTicket_Thread_Add). Inbound events are verified with JWT RS256 (X-ZDesk-JWT) against the data-center JWKS (iss: orgId:{id},aud: webhookId:{id}), acknowledged via the durable queue to meet Zoho's 5-second deadline, and fail loudly on Free/Standard editions that cannot create webhooks. AnignoreSourceIdUUID is set on create and echoed as asourceIdheader on writes to suppress self-triggered events.Grounding
Response schemas and the webhook API were verified against Zoho's official OpenAPI spec (
github.com/zoho/zohodesk-oas).Verification
bun run check:api-validation:strict— passesbun run check:bare-icons— passesscripts/generate-docs.tsNotes
Reviewer notes
Non-Zoho change in this diff (please read):
apps/docs/components/icons.tsxalso addsAtlassianIcon+SimAutoIconand replaces theConfluenceIcon/JiraIconartwork. That is not hand-written — it is the docs generator syncingapps/docsup toapps/sim/components/icons.tsx, which already carried those versions onstaging; the docs app had drifted behind. It is corrective, but it does mean this PR changes the Jira and Confluence marks on the public docs site. Flagging so it is a decision rather than a surprise; happy to split it out.One shared-code change:
apps/sim/serializer/index.tsnow excludesmode: 'trigger-advanced'from tool-mode serialization alongside'trigger'. This fixes a reproduced bug (a trigger's required manual field validated on unrelated tool operations, solist_organizationsfailed withMissing required fields: Organization IDfor a field it does not render). It aligns the serializer with the convention the rest of the codebase already follows —blocks.test.tstreats the two modes identically in six places, as does the copilot block-metadata tool. Verified no change for the other seventrigger-advancedconsumers (Google Calendar/Sheets×2/Drive, Table, Slack×3): their values reachproviderConfigviabuildProviderConfig, never via tool serialization.Requires live verification before merge — these cannot be settled from Zoho's documentation:
soidservice prefix (ZohoDesk.<orgId>) and whetherzsoidequals the DeskorgIdheader value — if wrong, every Self Client mint fails/contentsub-path (Zoho publishes none)Desk.agents.READis required or already covered byDesk.basic.READ