Skip to content

Commit 8c16c9d

Browse files
committed
fix(zoho-desk): five-audit round - serializer trigger-advanced leak, 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.
1 parent 66220e4 commit 8c16c9d

24 files changed

Lines changed: 293 additions & 71 deletions

File tree

apps/docs/content/docs/en/integrations/zoho_desk.mdx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ List tickets from a Zoho Desk organization with optional filters. Returns a list
7070
|`ticketNumber` | string | Human-readable ticket number |
7171
|`subject` | string | Ticket subject |
7272
|`description` | string | Ticket description \(raw; may be HTML\) |
73-
|`descriptionText` | string | Plain-text rendering of the description with HTML stripped \(Zoho sends ticket descriptions as HTML\) |
73+
|`descriptionText` | string | Plain-text rendering of the description: HTML stripped when the body contains markup, otherwise the description verbatim |
7474
|`status` | string | Ticket status |
7575
|`statusType` | string | Status category \(Open/Closed/On Hold\) |
7676
|`priority` | string | Ticket priority |
@@ -121,7 +121,7 @@ Retrieve a single Zoho Desk ticket by ID.
121121
|`ticketNumber` | string | Human-readable ticket number |
122122
|`subject` | string | Ticket subject |
123123
|`description` | string | Ticket description \(raw; may be HTML\) |
124-
|`descriptionText` | string | Plain-text rendering of the description with HTML stripped \(Zoho sends ticket descriptions as HTML\) |
124+
|`descriptionText` | string | Plain-text rendering of the description: HTML stripped when the body contains markup, otherwise the description verbatim |
125125
|`status` | string | Ticket status |
126126
|`statusType` | string | Status category \(Open/Closed/On Hold\) |
127127
|`priority` | string | Ticket priority |
@@ -182,7 +182,7 @@ Update fields on an existing Zoho Desk ticket.
182182
|`ticketNumber` | string | Human-readable ticket number |
183183
|`subject` | string | Ticket subject |
184184
|`description` | string | Ticket description \(raw; may be HTML\) |
185-
|`descriptionText` | string | Plain-text rendering of the description with HTML stripped \(Zoho sends ticket descriptions as HTML\) |
185+
|`descriptionText` | string | Plain-text rendering of the description: HTML stripped when the body contains markup, otherwise the description verbatim |
186186
|`status` | string | Ticket status |
187187
|`statusType` | string | Status category \(Open/Closed/On Hold\) |
188188
|`priority` | string | Ticket priority |
@@ -296,7 +296,7 @@ Add a comment to a Zoho Desk ticket.
296296

297297
### `zoho_desk_list_threads`
298298

299-
List conversation threads on a Zoho Desk ticket, newest first (Zoho sorts by sendDateTime descending by default).
299+
List conversation threads on a Zoho Desk ticket, newest first (Zoho sorts by sendDateTime descending by default). Returns a list projection: message bodies (content, summary, to/cc/bcc) come back only from Get Thread.
300300

301301
#### Input
302302

@@ -330,6 +330,9 @@ List conversation threads on a Zoho Desk ticket, newest first (Zoho sorts by sen
330330
|`bcc` | string | BCC email address |
331331
|`replyTo` | string | Reply-to email address |
332332
|`isForward` | boolean | Whether the thread is a forward |
333+
|`isContentTruncated` | boolean | Whether Zoho truncated the thread content; fetch fullContentURL for the rest |
334+
|`fullContentURL` | string | URL returning the untruncated thread content |
335+
|`plainText` | string | Zoho's own plain-text rendering of the thread, when it supplies one |
333336
|`status` | string | Delivery status of an outgoing thread \(SUCCESS/FAILED/DRAFT\) |
334337
|`isDescriptionThread` | boolean | Whether this thread is the ticket's original description |
335338
|`visibility` | string | Thread visibility \(e.g. public\) |
@@ -383,6 +386,9 @@ Retrieve the full content of a single Zoho Desk ticket thread.
383386
|`bcc` | string | BCC email address |
384387
|`replyTo` | string | Reply-to email address |
385388
|`isForward` | boolean | Whether the thread is a forward |
389+
|`isContentTruncated` | boolean | Whether Zoho truncated the thread content; fetch fullContentURL for the rest |
390+
|`fullContentURL` | string | URL returning the untruncated thread content |
391+
|`plainText` | string | Zoho's own plain-text rendering of the thread, when it supplies one |
386392
|`status` | string | Delivery status of an outgoing thread \(SUCCESS/FAILED/DRAFT\) |
387393
|`isDescriptionThread` | boolean | Whether this thread is the ticket's original description |
388394
|`visibility` | string | Thread visibility \(e.g. public\) |
@@ -504,6 +510,6 @@ Trigger a workflow when a Zoho Desk event occurs (ticket, comment, thread, conta
504510
| `eventType` | string | The Zoho Desk event type \(e.g. Ticket_Add\) |
505511
| `eventTime` | string | Event time in milliseconds since epoch |
506512
| `orgId` | string | Zoho Desk organization ID |
507-
| `payload` | json | The full resource that changed \(ticket, comment, thread, etc.\). For comment/thread events a derived plain-text `contentText` is added alongside the raw `content` + `contentType`. |
513+
| `payload` | json | The full resource that changed \(ticket, comment, thread, etc.\). Comment and thread events gain a derived plain-text `contentText` alongside the raw `content` + `contentType`; ticket events gain `descriptionText` alongside `description`. |
508514
| `prevState` | json | Previous state of the resource \(update events only\) |
509515

apps/sim/app/api/auth/oauth/token/route.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
resolveOAuthAccountId,
2121
resolveServiceAccountToken,
2222
} from '@/app/api/auth/oauth/utils'
23+
import { extractZohoDeskBaseFromScope } from '@/tools/zoho_desk/host-allowlist'
2324

2425
export const dynamic = 'force-dynamic'
2526

@@ -29,7 +30,6 @@ const SALESFORCE_INSTANCE_URL_REGEX = /__sf_instance__:([^\s]+)/
2930
// Stop at a comma or whitespace: better-auth persists Zoho's scopes comma-joined
3031
// (no spaces), so a greedy `\S+` would swallow the whole scope list into the host.
3132
// The Desk base URL itself never contains a comma or space.
32-
const ZOHO_DESK_BASE_URL_REGEX = /__zoho_domain__:([^\s,]+)/
3333

3434
/**
3535
* Get an access token for a specific credential
@@ -300,10 +300,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
300300
// assume a host. Surface it as apiDomain for tool param injection.
301301
let apiDomain: string | undefined
302302
if (credential.providerId === 'zoho-desk' && credential.scope) {
303-
const domainMatch = credential.scope.match(ZOHO_DESK_BASE_URL_REGEX)
304-
if (domainMatch) {
305-
apiDomain = domainMatch[1]
306-
}
303+
// Use the shared extractor, not a local regex: it also enforces https +
304+
// the Zoho apex allowlist. This value is injected into EVERY tool call,
305+
// so an unvalidated host here would receive the OAuth token.
306+
apiDomain = extractZohoDeskBaseFromScope(credential.scope)
307307
}
308308

309309
return NextResponse.json(
@@ -424,10 +424,10 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
424424
// assume a host. Surface it as apiDomain for tool param injection.
425425
let apiDomain: string | undefined
426426
if (credential.providerId === 'zoho-desk' && credential.scope) {
427-
const domainMatch = credential.scope.match(ZOHO_DESK_BASE_URL_REGEX)
428-
if (domainMatch) {
429-
apiDomain = domainMatch[1]
430-
}
427+
// Use the shared extractor, not a local regex: it also enforces https +
428+
// the Zoho apex allowlist. This value is injected into EVERY tool call,
429+
// so an unvalidated host here would receive the OAuth token.
430+
apiDomain = extractZohoDeskBaseFromScope(credential.scope)
431431
}
432432

433433
return NextResponse.json(

apps/sim/app/api/tools/zoho_desk/attachment/route.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8989
)
9090
}
9191

92+
// A 204 (or any non-200 success) carries no body, so arrayBuffer() would
93+
// yield zero bytes and the route would report success with an empty file.
94+
if (response.status !== 200) {
95+
return NextResponse.json(
96+
{ success: false, error: `Attachment returned no content (HTTP ${response.status})` },
97+
{ status: 502 }
98+
)
99+
}
100+
92101
const arrayBuffer = await response.arrayBuffer()
93102

94103
// ToolFileData (consumed by FileToolProcessor) keys the file name as `name`.

apps/sim/app/api/tools/zoho_desk/organizations/route.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4141
let organizationsUrl: URL
4242
try {
4343
organizationsUrl = assertZohoUrl(`${apiBase}/organizations`)
44-
// Zoho's listing APIs default to a page size of 10. Without an explicit
45-
// limit, an account with more than ten accessible portals would silently get
46-
// a truncated dropdown - and because every other selector and every tool
47-
// call is gated on orgId, a missing portal is unreachable except through the
48-
// advanced manual field. 200 is the documented per-page ceiling on the
49-
// sibling listing endpoints and is far above any real portal count, so this
50-
// needs no drain loop.
51-
organizationsUrl.searchParams.set('limit', '200')
44+
// Deliberately sends NO query parameters. Zoho's `/organizations` doc block
45+
// lists none at all - not `from`, not `limit` - and its own sample is a bare
46+
// GET. An earlier revision passed `limit=200` by extrapolating from
47+
// /departments and /agents, but the other siblings (/tickets, /contacts,
48+
// /comments) cap at 100 and Zoho answers an out-of-range value with 422
49+
// INVALID_DATA. Since `orgId` gates every tool and both other selectors, a
50+
// 422 here would make the whole integration unreachable except through the
51+
// manual field - a far worse failure than the known downside of sending
52+
// nothing, which is Zoho's default page size (10 portals).
5253
} catch {
5354
return NextResponse.json({ error: 'Credential resolved to a non-Zoho host' }, { status: 400 })
5455
}

apps/sim/blocks/blocks/zoho-desk.ts

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,19 +174,39 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
174174
placeholder: 'New subject',
175175
condition: { field: 'operation', value: 'update_ticket' },
176176
},
177+
// status/priority are deliberately split per operation rather than shared.
178+
// A subBlock keeps its value when the operation changes, and the two uses are
179+
// semantically opposite: on list_tickets they are filters (comma-separated,
180+
// matching), on update_ticket they are the new value written to the ticket.
181+
// Sharing one field meant a filter of "Open,On Hold" could be PATCHed onto a
182+
// ticket, and an update value of "Closed" could silently filter a later list.
177183
{
178184
id: 'status',
179185
title: 'Status',
180186
type: 'short-input',
181-
placeholder: 'e.g. Open, Closed',
182-
condition: { field: 'operation', value: ['update_ticket', 'list_tickets'] },
187+
placeholder: 'e.g. Closed',
188+
condition: { field: 'operation', value: 'update_ticket' },
183189
},
184190
{
185191
id: 'priority',
186192
title: 'Priority',
187193
type: 'short-input',
188194
placeholder: 'e.g. High',
189-
condition: { field: 'operation', value: ['update_ticket', 'list_tickets'] },
195+
condition: { field: 'operation', value: 'update_ticket' },
196+
},
197+
{
198+
id: 'statusFilter',
199+
title: 'Status',
200+
type: 'short-input',
201+
placeholder: 'Filter, e.g. Open,On Hold',
202+
condition: { field: 'operation', value: 'list_tickets' },
203+
},
204+
{
205+
id: 'priorityFilter',
206+
title: 'Priority',
207+
type: 'short-input',
208+
placeholder: 'Filter, e.g. High,Urgent',
209+
condition: { field: 'operation', value: 'list_tickets' },
190210
},
191211
{
192212
id: 'assigneeId',
@@ -405,6 +425,10 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
405425
limit: rawLimit,
406426
contentType,
407427
isPublic,
428+
status: rawStatus,
429+
priority: rawPriority,
430+
statusFilter: rawStatusFilter,
431+
priorityFilter: rawPriorityFilter,
408432
customFields: rawCustomFields,
409433
departmentIds: rawDepartmentIds,
410434
...rest
@@ -459,6 +483,17 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
459483
// stale (or half-typed) JSON left behind after switching away from
460484
// Update Ticket would otherwise fail every unrelated operation with
461485
// "Invalid JSON provided for custom fields" - on runs that never send it.
486+
// Both tools take `status` / `priority`; pick the field belonging to the
487+
// selected operation so a stale value from the other one can never leak.
488+
const activeStatus = params.operation === 'list_tickets' ? rawStatusFilter : rawStatus
489+
const activePriority = params.operation === 'list_tickets' ? rawPriorityFilter : rawPriority
490+
if (activeStatus !== undefined && activeStatus !== null && activeStatus !== '') {
491+
result.status = activeStatus
492+
}
493+
if (activePriority !== undefined && activePriority !== null && activePriority !== '') {
494+
result.priority = activePriority
495+
}
496+
462497
if (params.operation === 'update_ticket' && rawCustomFields !== undefined) {
463498
if (typeof rawCustomFields === 'string') {
464499
if (rawCustomFields.trim()) {
@@ -488,7 +523,9 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
488523
contentType: { type: 'string', description: 'Comment content type (plainText/html)' },
489524
isPublic: { type: 'boolean', description: 'Whether a comment is public' },
490525
subject: { type: 'string', description: 'Ticket subject' },
491-
status: { type: 'string', description: 'Ticket status' },
526+
status: { type: 'string', description: 'Ticket status to set' },
527+
statusFilter: { type: 'string', description: 'Status filter for listing tickets' },
528+
priorityFilter: { type: 'string', description: 'Priority filter for listing tickets' },
492529
priority: { type: 'string', description: 'Ticket priority' },
493530
assigneeId: { type: 'string', description: 'Assignee (agent) ID' },
494531
description: { type: 'string', description: 'Ticket description' },

apps/sim/lib/auth/auth.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2123,8 +2123,18 @@ export const auth = betterAuth({
21232123
const deskBase = deriveZohoDeskBaseFromApiDomain(
21242124
typeof data.api_domain === 'string' ? data.api_domain : undefined
21252125
)
2126-
const grantedScopes =
2126+
// Zoho's docs are inconsistent about whether the Desk token response
2127+
// carries `scope` (the Mail sample has it; the CRM/Creator samples do
2128+
// not). If it is absent, fall back to the scopes we requested and were
2129+
// granted by completing the flow - otherwise the stored scope list is
2130+
// just the domain marker, and the credential picker would show a
2131+
// permanent "needs update / reconnect" badge on every connection.
2132+
// Mirrors the existing Box fallback in this file.
2133+
const reportedScopes =
21272134
typeof data.scope === 'string' ? data.scope.split(/[\s,]+/).filter(Boolean) : []
2135+
const grantedScopes = reportedScopes.length
2136+
? reportedScopes
2137+
: getCanonicalScopesForProvider('zoho-desk')
21282138
tokens.scopes = [`__zoho_domain__:${deskBase}`, ...grantedScopes]
21292139
return tokens
21302140
},

apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,11 @@ describe('mintZohoDeskServiceAccountToken', () => {
154154
expect(body.get('grant_type')).toBe('client_credentials')
155155
expect(body.get('client_id')).toBe('zoho-cid')
156156
expect(body.get('client_secret')).toBe('zoho-secret')
157-
expect(body.get('scope')).toBe(SCOPES.join(','))
157+
// Comma-separated, and Desk-only: `aaaserver.profile.READ` belongs to the
158+
// interactive OAuth flow's getUserInfo call, which this grant never makes.
159+
expect(body.get('scope')).toBe('Desk.tickets.READ,Desk.contacts.READ')
158160
expect(body.get('scope')).not.toContain(' ')
161+
expect(body.get('scope')).not.toContain('aaaserver')
159162
expect(body.get('soid')).toBe('ZohoDesk.600123456')
160163
expect(mockGetCanonicalScopesForProvider).toHaveBeenCalledWith('zoho-desk')
161164
})

apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,14 @@ export async function mintZohoDeskServiceAccountToken(
165165

166166
// Zoho requires a comma-separated scope list on this endpoint; a
167167
// space-separated list is rejected as an invalid scope.
168-
const scope = getCanonicalScopesForProvider('zoho-desk').join(',')
168+
// Only the Desk.* scopes. `aaaserver.profile.READ` exists for the interactive
169+
// OAuth flow's getUserInfo call; this grant never hits the Accounts profile
170+
// endpoint (identity is synthesized from orgId, and skipIdentity bypasses it
171+
// entirely at execution time). Sending an Accounts-server scope on the Desk
172+
// soid grant risks an opaque invalid_scope rejection for no benefit.
173+
const scope = getCanonicalScopesForProvider('zoho-desk')
174+
.filter((s) => s.startsWith('Desk.'))
175+
.join(',')
169176

170177
const res = await fetchProvider(
171178
`${dataCenter.accountsBase}/oauth/v2/token`,

apps/sim/lib/integrations/integrations.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21844,7 +21844,7 @@
2184421844
},
2184521845
{
2184621846
"name": "List Threads",
21847-
"description": "List conversation threads on a Zoho Desk ticket, newest first (Zoho sorts by sendDateTime descending by default)."
21847+
"description": "List conversation threads on a Zoho Desk ticket, newest first (Zoho sorts by sendDateTime descending by default). Returns a list projection: message bodies (content, summary, to/cc/bcc) come back only from Get Thread."
2184821848
},
2184921849
{
2185021850
"name": "Get Thread",

apps/sim/lib/oauth/oauth.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1886,13 +1886,30 @@ export async function refreshOAuthToken(
18861886
// execution retries a refresh that can never succeed. Classify on the body
18871887
// before trusting the status, matching the token-exchange and service-account
18881888
// mint paths, which already do this.
1889-
if (data && typeof data === 'object' && typeof data.error === 'string' && data.error) {
1889+
// The `!data.access_token` guard matters because this branch runs for EVERY
1890+
// provider: without it, a provider that ever returns an informational `error`
1891+
// alongside a valid token would have all its credentials fail refresh - and
1892+
// if that string is one of TERMINAL_ERRORS, be marked dead for an hour. No
1893+
// current provider does that, but the branch should not depend on it. Zoho's
1894+
// failure bodies carry no token, so the guard costs nothing here.
1895+
if (
1896+
data &&
1897+
typeof data === 'object' &&
1898+
typeof data.error === 'string' &&
1899+
data.error &&
1900+
!data.access_token
1901+
) {
18901902
logger.error('Token refresh failed with an error body:', {
18911903
status: response.status,
1904+
statusText: response.statusText,
18921905
error: data.error,
18931906
errorDescription:
18941907
typeof data.error_description === 'string' ? data.error_description : null,
18951908
providerId,
1909+
tokenEndpoint: config.tokenEndpoint,
1910+
hasClientId: !!config.clientId,
1911+
hasClientSecret: !!config.clientSecret,
1912+
hasRefreshToken: !!refreshToken,
18961913
})
18971914
return {
18981915
ok: false,

0 commit comments

Comments
 (0)