Skip to content

Commit 6264cba

Browse files
authored
fix(connectors): harden Zendesk connector against SSRF (#5237)
* fix(connectors): harden Zendesk connector against SSRF Route the Zendesk connector through the SSRF-safe secureFetchWithRetry (DNS-resolve + IP-pin + per-redirect revalidation) instead of the plain fetchWithRetry, and validate the user-supplied subdomain against a strict DNS-label pattern before building the base URL. Matches the GitLab/Sentry/Obsidian/S3 precedent. * fix(connectors): retry transient DNS failures in secureFetchWithRetry secureFetchWithValidation throws a validation error before the request when a hostname temporarily fails to resolve. Classify that transient DNS failure as retryable so secureFetchWithRetry mirrors the old fetchWithRetry network-retry behavior, while keeping the deterministic blocked-IP SSRF rejection non-retryable.
1 parent 3c6c6b1 commit 6264cba

4 files changed

Lines changed: 100 additions & 6 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { buildBaseUrl } from '@/connectors/zendesk/zendesk'
6+
7+
describe('buildBaseUrl', () => {
8+
it.concurrent('builds the base URL for a valid subdomain', () => {
9+
expect(buildBaseUrl('acme')).toBe('https://acme.zendesk.com')
10+
})
11+
12+
it.concurrent('allows hyphens and digits within the label', () => {
13+
expect(buildBaseUrl('acme-support-1')).toBe('https://acme-support-1.zendesk.com')
14+
})
15+
16+
it.concurrent('allows a single-character subdomain', () => {
17+
expect(buildBaseUrl('a')).toBe('https://a.zendesk.com')
18+
})
19+
20+
it.concurrent('allows the maximum 63-character label', () => {
21+
const label = `a${'b'.repeat(61)}c`
22+
expect(label).toHaveLength(63)
23+
expect(buildBaseUrl(label)).toBe(`https://${label}.zendesk.com`)
24+
})
25+
26+
it.concurrent('trims surrounding whitespace', () => {
27+
expect(buildBaseUrl(' acme ')).toBe('https://acme.zendesk.com')
28+
})
29+
30+
it.concurrent('normalizes uppercase to lowercase (DNS is case-insensitive)', () => {
31+
expect(buildBaseUrl('MyCompany')).toBe('https://mycompany.zendesk.com')
32+
})
33+
34+
describe('rejects SSRF payloads', () => {
35+
const ssrfPayloads: Array<[string, string]> = [
36+
['fragment truncation', 'webhook.site/abc#'],
37+
['fragment with path', 'evil.com/path#'],
38+
['embedded path', 'acme/api/v2'],
39+
['scheme injection', 'http://evil.com'],
40+
['userinfo', 'user@evil.com'],
41+
['port', 'acme:8080'],
42+
['open-redirect host', 'httpbin.org/redirect-to?url=http://169.254.169.254'],
43+
['loopback', '127.0.0.1'],
44+
['link-local literal', '169.254.169.254'],
45+
['whitespace injection', 'acme evil'],
46+
['leading hyphen', '-acme'],
47+
['trailing hyphen', 'acme-'],
48+
['leading dot', '.acme'],
49+
['trailing dot', 'acme.'],
50+
['empty string', ''],
51+
['whitespace only', ' '],
52+
['over-length label', 'a'.repeat(64)],
53+
['unicode', 'acmé'],
54+
]
55+
56+
it.concurrent.each(ssrfPayloads)('rejects %s', (_label, payload) => {
57+
expect(() => buildBaseUrl(payload)).toThrow('Invalid Zendesk subdomain')
58+
})
59+
})
60+
})

‎apps/sim/connectors/zendesk/zendesk.ts‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
3-
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
3+
import { secureFetchWithRetry } from '@/lib/knowledge/documents/secure-fetch.server'
4+
import { VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
45
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
56
import { htmlToPlainText, joinTagArray, parseTagDate } from '@/connectors/utils'
67
import { DEFAULT_MAX_TICKETS, zendeskConnectorMeta } from '@/connectors/zendesk/meta'
@@ -51,10 +52,31 @@ interface ZendeskComment {
5152
}
5253

5354
/**
54-
* Builds the base URL for a Zendesk subdomain.
55+
* Strict Zendesk subdomain label: a single DNS label of lowercase letters,
56+
* digits, and hyphens (not leading/trailing). Rejects anything that could
57+
* smuggle a scheme, host, path, port, or fragment into the base URL.
5558
*/
56-
function buildBaseUrl(subdomain: string): string {
57-
return `https://${subdomain}.zendesk.com`
59+
const SUBDOMAIN_PATTERN = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$/
60+
61+
/**
62+
* Validates and normalizes a Zendesk subdomain.
63+
*
64+
* @throws {Error} when the subdomain is missing or not a valid DNS label.
65+
*/
66+
function normalizeSubdomain(subdomain: string): string {
67+
const normalized = subdomain.trim().toLowerCase()
68+
if (!SUBDOMAIN_PATTERN.test(normalized)) {
69+
throw new Error('Invalid Zendesk subdomain')
70+
}
71+
return normalized
72+
}
73+
74+
/**
75+
* Builds the base URL for a Zendesk subdomain. The subdomain is validated to a
76+
* strict DNS label so it cannot be used to forge requests to arbitrary hosts.
77+
*/
78+
export function buildBaseUrl(subdomain: string): string {
79+
return `https://${normalizeSubdomain(subdomain)}.zendesk.com`
5880
}
5981

6082
/**
@@ -65,11 +87,11 @@ async function zendeskApiGet(
6587
url: string,
6688
accessToken: string,
6789
sourceConfig: Record<string, unknown>,
68-
retryOptions?: Parameters<typeof fetchWithRetry>[2]
90+
retryOptions?: Parameters<typeof secureFetchWithRetry>[2]
6991
): Promise<Record<string, unknown>> {
7092
const email = sourceConfig.email as string
7193

72-
const response = await fetchWithRetry(
94+
const response = await secureFetchWithRetry(
7395
url,
7496
{
7597
method: 'GET',

‎apps/sim/lib/knowledge/documents/utils.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@ describe('isRetryableError', () => {
132132
it.concurrent('returns true for "service unavailable" in message', () => {
133133
expect(isRetryableError(new Error('The service unavailable right now'))).toBe(true)
134134
})
135+
136+
it.concurrent('returns true for a transient DNS resolution failure', () => {
137+
expect(isRetryableError(new Error('url hostname could not be resolved'))).toBe(true)
138+
})
135139
})
136140

137141
describe('case insensitivity', () => {
@@ -178,6 +182,10 @@ describe('isRetryableError', () => {
178182
it.concurrent('returns false for plain object with non-retryable status and no message', () => {
179183
expect(isRetryableError({ status: 500 })).toBe(false)
180184
})
185+
186+
it.concurrent('returns false for the deterministic blocked-IP SSRF rejection', () => {
187+
expect(isRetryableError(new Error('url resolves to a blocked IP address'))).toBe(false)
188+
})
181189
})
182190
})
183191

‎apps/sim/lib/knowledge/documents/utils.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ export function isRetryableError(error: unknown): boolean {
6767
'enetunreach',
6868
'socket hang up',
6969
'network error',
70+
// Transient DNS resolution failure surfaced by secureFetchWithValidation
71+
// before the request is made. The deterministic "resolves to a blocked IP
72+
// address" security rejection is a distinct message and stays non-retryable.
73+
'could not be resolved',
7074
]
7175

7276
if (networkKeywords.some((keyword) => lowerMessage.includes(keyword))) {

0 commit comments

Comments
 (0)