Skip to content

Commit c6d56df

Browse files
committed
fix(zoho-desk): strip ticket description HTML, classify body-reported refresh failures
Final validation pass findings. descriptionText never stripped anything. It was gated on a `descriptionContentType` discriminator that Zoho does not send: the Ticket_Add webhook sample ships `"description": "<div>Description</div>"` with no such key, and the ticket GET/PATCH response field lists have no content-type sibling either. So get_ticket, update_ticket, and every webhook ticket payload emitted descriptionText as a byte-identical copy of the raw HTML, while the declared output promised stripped text. The tests did not catch it because they fabricated the shape - both fixtures constructed `descriptionContentType: 'html'`, a key Zoho never emits, proving the branch works without proving it is ever taken. Ticket descriptions are HTML by convention, so the strip is now unconditional (html-to-text is a near-identity on genuinely plain text), an explicit descriptionContentType is still honored if Zoho ever adds one, and the fixtures now use Zoho's real shape with no content-type key anywhere. A body-reported refresh failure was unclassified. Zoho answers a revoked refresh token with HTTP 200 and `{"error":"invalid_client"}`; refreshOAuthToken only checked `data.ok === false` (a Slack-ism), so the request fell through to the "no access token" guard and returned no errorCode. isTerminalRefreshError could therefore never recognize invalid_client as terminal, the credential was never marked dead, and every later execution retried a refresh that cannot succeed - with the user shown "No access token in refresh response" instead of a reconnect prompt. The body is now classified before the status is trusted, matching what the token exchange and the service-account mint already did. That guard also stopped logging the whole response body, which carries live tokens on a partial success. Also: an unrecognized dataCenter now fails with a named error instead of quietly resolving to US and surfacing as an opaque invalid_client (blank still means US); the webhook JWKS cache is bounded, since its key derives from a providerConfig field that SYSTEM_MANAGED_FIELDS protects from diffing but not from being written; and the attachment `size` output no longer asserts bytes, a unit Zoho documents as KB.
1 parent c126294 commit c6d56df

9 files changed

Lines changed: 138 additions & 43 deletions

File tree

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

Lines changed: 7 additions & 7 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; HTML stripped when Zoho marks it HTML, otherwise mirrors description |
73+
|`descriptionText` | string | Plain-text rendering of the description with HTML stripped \(Zoho sends ticket descriptions as HTML\) |
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; HTML stripped when Zoho marks it HTML, otherwise mirrors description |
124+
|`descriptionText` | string | Plain-text rendering of the description with HTML stripped \(Zoho sends ticket descriptions as HTML\) |
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; HTML stripped when Zoho marks it HTML, otherwise mirrors description |
185+
|`descriptionText` | string | Plain-text rendering of the description with HTML stripped \(Zoho sends ticket descriptions as HTML\) |
186186
|`status` | string | Ticket status |
187187
|`statusType` | string | Status category \(Open/Closed/On Hold\) |
188188
|`priority` | string | Ticket priority |
@@ -248,7 +248,7 @@ List comments on a Zoho Desk ticket.
248248
|`attachments` | array | Comment attachments |
249249
|`id` | string | Attachment ID |
250250
|`name` | string | File name |
251-
|`size` | string | File size in bytes |
251+
|`size` | string | File size as reported by Zoho |
252252
|`href` | string | Download href |
253253
| `count` | number | Number of comments returned |
254254

@@ -291,7 +291,7 @@ Add a comment to a Zoho Desk ticket.
291291
|`attachments` | array | Comment attachments |
292292
|`id` | string | Attachment ID |
293293
|`name` | string | File name |
294-
|`size` | string | File size in bytes |
294+
|`size` | string | File size as reported by Zoho |
295295
|`href` | string | Download href |
296296

297297
### `zoho_desk_list_threads`
@@ -344,7 +344,7 @@ List conversation threads on a Zoho Desk ticket, newest first (Zoho sorts by sen
344344
|`attachments` | array | Thread attachments |
345345
|`id` | string | Attachment ID |
346346
|`name` | string | File name |
347-
|`size` | string | File size in bytes |
347+
|`size` | string | File size as reported by Zoho |
348348
|`href` | string | Download href |
349349
| `count` | number | Number of threads returned |
350350

@@ -397,7 +397,7 @@ Retrieve the full content of a single Zoho Desk ticket thread.
397397
|`attachments` | array | Thread attachments |
398398
|`id` | string | Attachment ID |
399399
|`name` | string | File name |
400-
|`size` | string | File size in bytes |
400+
|`size` | string | File size as reported by Zoho |
401401
|`href` | string | Download href |
402402

403403
### `zoho_desk_get_contact`

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -235,16 +235,18 @@ describe('mintZohoDeskServiceAccountToken', () => {
235235
}
236236
)
237237

238-
it('falls back to the US data center for an unrecognized value instead of erroring', async () => {
239-
mockFetch.mockResolvedValueOnce(jsonResponse(200, { access_token: 'zoho-access' }))
240-
241-
const result = await mintZohoDeskServiceAccountToken(
242-
{ ...FIELDS, dataCenter: 'jp' },
243-
{ skipIdentity: true }
244-
)
245-
246-
mintBody(TOKEN_URL)
247-
expect(result.apiDomain).toBe('https://desk.zoho.com')
238+
// A typed-but-unrecognized region must NOT quietly resolve to US and then fail
239+
// against Zoho as an opaque invalid_client — the operator would have no way to
240+
// tell a wrong region from wrong credentials. Blank still means US (see above).
241+
it('rejects an unrecognized data center instead of silently using US', async () => {
242+
await expect(
243+
mintZohoDeskServiceAccountToken({ ...FIELDS, dataCenter: 'jp' })
244+
).rejects.toMatchObject({
245+
code: 'invalid_credentials',
246+
status: 400,
247+
logDetail: expect.objectContaining({ step: 'data_center_validation', dataCenter: 'jp' }),
248+
})
249+
expect(mockFetch).not.toHaveBeenCalled()
248250
})
249251

250252
it('records the resolved data center in the stored metadata', async () => {

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { createLogger } from '@sim/logger'
22
import {
3+
normalizeZohoDeskDataCenter,
34
normalizeZohoDeskSoid,
45
resolveZohoDeskDataCenter,
6+
ZOHO_DESK_DATA_CENTER_IDS,
7+
ZOHO_DESK_DATA_CENTER_REGEX,
58
ZOHO_DESK_SOID_REGEX,
69
} from '@/lib/credentials/client-credential-accounts/descriptors'
710
import type {
@@ -144,7 +147,21 @@ export async function mintZohoDeskServiceAccountToken(
144147
})
145148
}
146149

147-
const dataCenter = resolveZohoDeskDataCenter(fields.dataCenter)
150+
// A blank data center legitimately means "US" (the pre-field default), but a
151+
// value that was typed and not recognized must not silently resolve to US and
152+
// then fail against Zoho as an opaque `invalid_client`. Reject it here, next to
153+
// the soid check, so the connect modal names the real problem. This runs on
154+
// every execution-time re-mint too, where there is no UI to show a format hint.
155+
const rawDataCenter = normalizeZohoDeskDataCenter(fields.dataCenter ?? '')
156+
if (rawDataCenter && !ZOHO_DESK_DATA_CENTER_REGEX.test(rawDataCenter)) {
157+
throw new TokenServiceAccountValidationError('invalid_credentials', 400, {
158+
step: 'data_center_validation',
159+
dataCenter: rawDataCenter,
160+
reason: `unrecognized data center; expected one of ${ZOHO_DESK_DATA_CENTER_IDS.join(', ')} (or blank for us)`,
161+
})
162+
}
163+
164+
const dataCenter = resolveZohoDeskDataCenter(rawDataCenter)
148165

149166
// Zoho requires a comma-separated scope list on this endpoint; a
150167
// space-separated list is rejected as an invalid scope.

apps/sim/lib/oauth/oauth.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1874,6 +1874,31 @@ export async function refreshOAuthToken(
18741874

18751875
const data = await response.json()
18761876

1877+
// Some providers - Zoho notably - report OAuth failures in the JSON body with
1878+
// HTTP 200 rather than a 4xx (e.g. `{"error":"invalid_client"}` for a revoked
1879+
// refresh token). Without this the `!accessToken` guard below returns a
1880+
// failure with no `errorCode`, so `isTerminalRefreshError` cannot recognize a
1881+
// terminal condition, the credential is never marked dead, and every later
1882+
// execution retries a refresh that can never succeed. Classify on the body
1883+
// before trusting the status, matching the token-exchange and service-account
1884+
// mint paths, which already do this.
1885+
if (data && typeof data === 'object' && typeof data.error === 'string' && data.error) {
1886+
logger.error('Token refresh failed with an error body:', {
1887+
status: response.status,
1888+
error: data.error,
1889+
errorDescription:
1890+
typeof data.error_description === 'string' ? data.error_description : null,
1891+
providerId,
1892+
})
1893+
return {
1894+
ok: false,
1895+
errorCode: data.error,
1896+
message: `Failed to refresh token: ${data.error}${
1897+
typeof data.error_description === 'string' ? ` - ${data.error_description}` : ''
1898+
}`,
1899+
}
1900+
}
1901+
18771902
if (data && typeof data === 'object' && data.ok === false) {
18781903
logger.error('Token refresh failed:', {
18791904
status: response.status,
@@ -1904,7 +1929,12 @@ export async function refreshOAuthToken(
19041929
const expiresIn = data.expires_in || data.expiresIn || 3600
19051930

19061931
if (!accessToken) {
1907-
logger.warn('No access token found in refresh response', { providerId, response: data })
1932+
// Never log `data` itself here - on a partial success it carries live
1933+
// tokens. The error-body branch above already surfaces the diagnosable case.
1934+
logger.warn('No access token found in refresh response', {
1935+
providerId,
1936+
responseKeys: Object.keys(data ?? {}),
1937+
})
19081938
return { ok: false, message: 'No access token in refresh response' }
19091939
}
19101940

apps/sim/lib/webhooks/providers/zoho-desk.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,30 @@ const ZOHO_DESK_BASE_URL_REGEX = /__zoho_domain__:([^\s,]+)/
3333
* verification within Zoho's 5-second delivery deadline.
3434
*/
3535
const jwksCache = new Map<string, ReturnType<typeof jose.createRemoteJWKSet>>()
36+
37+
/**
38+
* Bound on distinct Desk hosts held in {@link jwksCache}. The host is derived
39+
* from `providerConfig.apiDomain`, which `SYSTEM_MANAGED_FIELDS` protects from
40+
* *diffing* but not from being written by a workspace member. `safeZohoDeskBase`
41+
* already clamps it to a Zoho apex so no key material is ever fetched off-Zoho,
42+
* but any `*.zoho.com` label still passes - so without a cap, repeated writes
43+
* plus webhook hits could grow one JWKS instance (and its key cache) per label.
44+
* Zoho has a handful of data centers; anything beyond this is not legitimate
45+
* traffic, so evicting oldest-first is safe.
46+
*/
47+
const JWKS_CACHE_MAX_ENTRIES = 16
48+
3649
function getJwks(deskHost: string): ReturnType<typeof jose.createRemoteJWKSet> {
37-
let set = jwksCache.get(deskHost)
38-
if (!set) {
39-
set = jose.createRemoteJWKSet(new URL(`https://${deskHost}/.well-known/jwks.json`))
40-
jwksCache.set(deskHost, set)
50+
const set = jwksCache.get(deskHost)
51+
if (set) return set
52+
53+
if (jwksCache.size >= JWKS_CACHE_MAX_ENTRIES) {
54+
const oldest = jwksCache.keys().next()
55+
if (!oldest.done) jwksCache.delete(oldest.value)
4156
}
42-
return set
57+
const created = jose.createRemoteJWKSet(new URL(`https://${deskHost}/.well-known/jwks.json`))
58+
jwksCache.set(deskHost, created)
59+
return created
4360
}
4461

4562
/**

apps/sim/tools/generated/tool-outputs.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/zoho_desk/types.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,10 @@ export interface ZohoDeskResponse extends ToolResponse {
8686
export const ZOHO_DESK_ATTACHMENT_PROPERTIES: Record<string, ToolOutputProperty> = {
8787
id: { type: 'string', description: 'Attachment ID' },
8888
name: { type: 'string', description: 'File name', optional: true },
89-
size: { type: 'string', description: 'File size in bytes', optional: true },
89+
// Zoho documents this as KB, but serializes it as a string and its samples are
90+
// not self-consistent about the unit — report it as Zoho gives it rather than
91+
// asserting a unit we would be guessing at.
92+
size: { type: 'string', description: 'File size as reported by Zoho', optional: true },
9093
href: { type: 'string', description: 'Download href', optional: true },
9194
}
9295

@@ -103,7 +106,7 @@ export const ZOHO_DESK_TICKET_PROPERTIES: Record<string, ToolOutputProperty> = {
103106
descriptionText: {
104107
type: 'string',
105108
description:
106-
'Plain-text rendering of the description; HTML stripped when Zoho marks it HTML, otherwise mirrors description',
109+
'Plain-text rendering of the description with HTML stripped (Zoho sends ticket descriptions as HTML)',
107110
optional: true,
108111
nullable: true,
109112
},

apps/sim/tools/zoho_desk/utils.test.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -251,28 +251,40 @@ describe('zoho desk tool utils', () => {
251251
expect(withDerivedContentText('str')).toBe('str')
252252
})
253253

254-
// Ticket resources carry their body on `description`, not `content`, so a
255-
// webhook ticket payload would otherwise reach the workflow as raw HTML with
256-
// no plain-text sibling — unlike get_ticket, which derives one.
257-
it('derives descriptionText for ticket-shaped resources', () => {
258-
const ticket = {
259-
id: '5',
260-
subject: 'Delay',
261-
description: '<div>order is late</div>',
262-
descriptionContentType: 'html',
263-
}
254+
// Zoho ships ticket descriptions as HTML with NO content-type key — this is
255+
// the exact shape from Zoho's own Ticket_Add webhook sample. Gating the strip
256+
// on a `descriptionContentType` that never arrives made descriptionText a
257+
// byte-identical copy of the markup, so this case must stay unfabricated: no
258+
// descriptionContentType key anywhere in the fixture.
259+
it('strips HTML from a ticket description that carries no content-type key', () => {
260+
const ticket = { id: '5', subject: 'Delay', description: '<div>order is late</div>' }
264261
const result = withDerivedContentText(ticket) as Record<string, unknown>
265262
expect(result.description).toBe('<div>order is late</div>')
266263
expect(result.descriptionText).toBe('order is late')
267264
expect('contentText' in result).toBe(false)
268265
})
269266

267+
it('leaves a plain-text ticket description readable', () => {
268+
const result = withDerivedContentText({
269+
id: '7',
270+
description: 'order is late',
271+
}) as Record<string, unknown>
272+
expect(result.descriptionText).toBe('order is late')
273+
})
274+
275+
it('still honors an explicit descriptionContentType if Zoho ever sends one', () => {
276+
const result = withDerivedContentText({
277+
description: '<b>literal</b>',
278+
descriptionContentType: 'plainText',
279+
}) as Record<string, unknown>
280+
expect(result.descriptionText).toBe('<b>literal</b>')
281+
})
282+
270283
it('derives both fields when a resource carries content and description', () => {
271284
const result = withDerivedContentText({
272285
content: '<b>c</b>',
273286
contentType: 'text/html',
274287
description: '<b>d</b>',
275-
descriptionContentType: 'html',
276288
}) as Record<string, unknown>
277289
expect(result.contentText).toBe('c')
278290
expect(result.descriptionText).toBe('d')

apps/sim/tools/zoho_desk/utils.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,25 @@ export function withDerivedContentText(resource: unknown): unknown {
5656
const record = resource as Record<string, unknown>
5757

5858
const contentText = deriveZohoContentText(record.content, record.contentType)
59-
// Ticket resources carry their body on `description` rather than `content`, so
60-
// a webhook's ticket payload would otherwise reach the workflow as raw HTML
61-
// with no plain-text sibling - unlike get_ticket, which derives one. Mirror
62-
// that here so trigger and tool outputs agree.
63-
const descriptionText = deriveZohoContentText(record.description, record.descriptionContentType)
59+
// Ticket resources carry their body on `description`, not `content`, so without
60+
// this a webhook ticket payload reaches the workflow as raw HTML with no
61+
// plain-text sibling.
62+
//
63+
// Unlike comments and threads, a ticket's description has NO content-type
64+
// discriminator: Zoho's own Ticket_Add webhook sample ships
65+
// `"description": "<div>Description</div>"` with no `descriptionContentType`
66+
// key, and the ticket GET/PATCH response field lists have no content-type
67+
// sibling either. Gating on one meant the strip never ran and `descriptionText`
68+
// was a byte-identical copy of the HTML. Ticket descriptions are HTML by
69+
// convention, so convert unconditionally - html-to-text is a near-identity on
70+
// genuinely plain text. A `descriptionContentType` is still honored if Zoho
71+
// ever starts sending one.
72+
const descriptionText =
73+
typeof record.description === 'string'
74+
? typeof record.descriptionContentType === 'string'
75+
? deriveZohoContentText(record.description, record.descriptionContentType)
76+
: convertZohoHtmlToText(record.description)
77+
: undefined
6478

6579
if (contentText === undefined && descriptionText === undefined) return record
6680
return {

0 commit comments

Comments
 (0)