fix(zoho-desk): delta-audit findings - prevState scope, status leak, 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.

Author: waleedlatif1

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

@@ -483,10 +483,21 @@ export const ZohoDeskBlock: BlockConfig<ZohoDeskResponse> = {
         // stale (or half-typed) JSON left behind after switching away from
         // Update Ticket would otherwise fail every unrelated operation with
         // "Invalid JSON provided for custom fields" - on runs that never send it.
-        // Both tools take `status` / `priority`; pick the field belonging to the
-        // selected operation so a stale value from the other one can never leak.
-        const activeStatus = params.operation === 'list_tickets' ? rawStatusFilter : rawStatus
-        const activePriority = params.operation === 'list_tickets' ? rawPriorityFilter : rawPriority
+        // Only list_tickets and update_ticket declare status/priority. The other
+        // eight operations must receive neither - a ternary with a bare `else`
+        // would forward a stale Update Ticket value into e.g. get_ticket.
+        const activeStatus =
+          params.operation === 'list_tickets'
+            ? rawStatusFilter
+            : params.operation === 'update_ticket'
+              ? rawStatus
+              : undefined
+        const activePriority =
+          params.operation === 'list_tickets'
+            ? rawPriorityFilter
+            : params.operation === 'update_ticket'
+              ? rawPriority
+              : undefined
         if (activeStatus !== undefined && activeStatus !== null && activeStatus !== '') {
           result.status = activeStatus
         }

apps/sim/lib/credentials/orchestration/index.ts

@@ -8,6 +8,7 @@ import type { NextRequest } from 'next/server'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
 import { getCredentialActorContext } from '@/lib/credentials/access'
 import { AtlassianValidationError } from '@/lib/credentials/atlassian-service-account'
+import { isClientCredentialAccountProviderId } from '@/lib/credentials/client-credential-accounts/descriptors'
 import { type CredentialDeleteReason, deleteCredential } from '@/lib/credentials/deletion'
 import {
   deleteWorkspaceEnvCredentials,
@@ -168,8 +169,12 @@ export async function performUpdateCredential(
       // like the Zoho data center would be silently dropped, moving an EU/IN/AU
       // credential back to the US accounts server. Carry the stored value forward
       // when the caller did not supply one.
+      // Scoped to the providers that actually have a dataCenter field, so no
+      // other service-account reconnect (Slack, Atlassian, every token-paste
+      // provider) pays for a DB read plus a decrypt it can never use.
       const carriedDataCenter =
-        params.dataCenter === undefined
+        params.dataCenter === undefined &&
+        isClientCredentialAccountProviderId(access.credential.providerId ?? '')
           ? await readStoredDataCenter(access.credential.id)
           : params.dataCenter
 

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

@@ -34,16 +34,6 @@ const ZOHO_DESK_BASE_URL_REGEX = /__zoho_domain__:([^\s,]+)/
  */
 const jwksCache = new Map<string, ReturnType<typeof jose.createRemoteJWKSet>>()
 
-/**
- * Bound on distinct Desk hosts held in {@link jwksCache}. The host is derived
- * from `providerConfig.apiDomain`, which `SYSTEM_MANAGED_FIELDS` protects from
- * *diffing* but not from being written by a workspace member. `safeZohoDeskBase`
- * already clamps it to a Zoho apex so no key material is ever fetched off-Zoho,
- * but any `*.zoho.com` label still passes - so without a cap, repeated writes
- * plus webhook hits could grow one JWKS instance (and its key cache) per label.
- * Zoho has a handful of data centers; anything beyond this is not legitimate
- * traffic, so evicting oldest-first is safe.
- */
 /**
  * Events whose subscription filter accepts `departmentIds`. Zoho documents the
  * rest as taking no filter at all.
@@ -58,6 +48,31 @@ const DEPARTMENT_FILTERABLE_EVENTS = new Set([
   'Task_Update',
 ])
 
+/**
+ * Events whose filter accepts `includePrevState`. Enumerated rather than derived
+ * from an `_Update` suffix: Zoho documents the attribute on Ticket/Contact/Agent/
+ * Task/Article update events but NOT on `Ticket_Comment_Update`, which lists only
+ * `departmentIds`. Sending an undocumented filter key on a live create is the
+ * same class of risk as an undocumented query param.
+ */
+const PREV_STATE_EVENTS = new Set([
+  'Ticket_Update',
+  'Contact_Update',
+  'Agent_Update',
+  'Task_Update',
+  'Article_Update',
+])
+
+/**
+ * Bound on distinct Desk hosts held in {@link jwksCache}. The host is derived
+ * from `providerConfig.apiDomain`, which `SYSTEM_MANAGED_FIELDS` protects from
+ * *diffing* but not from being written by a workspace member. `safeZohoDeskBase`
+ * already clamps it to a Zoho apex so no key material is ever fetched off-Zoho,
+ * but any `*.zoho.com` label still passes - so without a cap, repeated writes
+ * plus webhook hits could grow one JWKS instance (and its key cache) per label.
+ * Zoho has a handful of data centers; anything beyond this is not legitimate
+ * traffic, so evicting oldest-first is safe.
+ */
 const JWKS_CACHE_MAX_ENTRIES = 16
 
 function getJwks(deskHost: string): ReturnType<typeof jose.createRemoteJWKSet> {
@@ -73,7 +88,6 @@ function getJwks(deskHost: string): ReturnType<typeof jose.createRemoteJWKSet> {
   // jose's default timeoutDuration is 5000ms, exactly the deadline.
   const created = jose.createRemoteJWKSet(new URL(`https://${deskHost}/.well-known/jwks.json`), {
     timeoutDuration: 1500,
-    cooldownDuration: 30_000,
   })
   jwksCache.set(deskHost, created)
   return created
@@ -281,11 +295,10 @@ export const zohoDeskHandler: WebhookProviderHandler = {
       const departmentIds = splitCsv(config.triggerDepartmentIds)
       if (departmentIds.length > 0) filter.departmentIds = departmentIds
     }
-    // `includePrevState` defaults to false and is supported on EVERY *_Update
-    // event, not just tickets - without it Zoho never sends `prevState`, so the
-    // trigger's declared prevState output would be permanently null for contact,
-    // agent, task and article updates.
-    if (eventType.endsWith('_Update')) {
+    // `includePrevState` defaults to false, so without it Zoho never sends
+    // `prevState` and the trigger's declared output is permanently null for the
+    // update events that do support it.
+    if (PREV_STATE_EVENTS.has(eventType)) {
       filter.includePrevState = true
     }
     if (eventType === 'Ticket_Update') {

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

@@ -289,6 +289,28 @@ describe('zoho desk tool utils', () => {
       }
     })
 
+    // A bare angle-bracket pair is not markup. These are realistic support-ticket
+    // bodies, and running them through html-to-text deletes everything between
+    // the brackets ("if x<y then z>0" became "if x0").
+    it('preserves plain text containing angle-bracket pairs', () => {
+      const cases = [
+        'if x<y then z>0',
+        'replace <username> with the real name',
+        'SELECT * FROM t WHERE a<b AND c>d',
+      ]
+      for (const description of cases) {
+        const result = withDerivedContentText({ description }) as Record<string, unknown>
+        expect(result.descriptionText).toBe(description)
+      }
+    })
+
+    it('strips hex-entity encoded bodies too', () => {
+      const result = withDerivedContentText({
+        description: 'Sam&#x27;s order failed',
+      }) as Record<string, unknown>
+      expect(result.descriptionText).toBe("Sam's order failed")
+    })
+
     it('still strips a genuinely HTML description', () => {
       const result = withDerivedContentText({
         description: '<div>order is <b>late</b></div>',

apps/sim/tools/zoho_desk/utils.ts

@@ -51,7 +51,17 @@ export function deriveZohoContentText(content: unknown, contentType: unknown): s
  * entities and delete tag-shaped text that was never markup).
  */
 function looksLikeHtml(value: string): boolean {
-  return /<[a-z!/][^>]*>/i.test(value) || /&(?:[a-z]+|#\d+);/i.test(value)
+  // Requires a real element - a paired <tag>...</tag>, a self-closing <tag/>, or
+  // an HTML comment/doctype. A bare `<`...`>` pair is NOT enough: plain support
+  // text like "if x<y then z>0" or "replace <username> with the real name" would
+  // otherwise be run through html-to-text and silently lose everything between
+  // the brackets. Entity form covers named, decimal and hex references.
+  return (
+    /<([a-z][a-z0-9]*)\b[^>]*>[\s\S]*<\/\1\s*>/i.test(value) ||
+    /<[a-z][a-z0-9]*\b[^>]*\/>/i.test(value) ||
+    /<!(?:--|doctype)/i.test(value) ||
+    /&(?:[a-z]+|#\d+|#x[0-9a-f]+);/i.test(value)
+  )
 }
 
 /**