Skip to content

Commit 21d20ca

Browse files
committed
fix(security): strip a trailing FQDN dot from vendor-hosted URLs
1 parent 0351567 commit 21d20ca

3 files changed

Lines changed: 24 additions & 4 deletions

File tree

‎apps/sim/lib/core/security/input-validation.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,12 @@ describe('validateServiceNowInstanceUrl (vendor-hosted allowlist)', () => {
615615
expect(result.sanitized).toBe('https://acme.servicenowservices.com/api/now')
616616
})
617617

618+
it.concurrent('drops a trailing FQDN dot, which TLS hostname verification rejects', () => {
619+
const result = validateServiceNowInstanceUrl('https://acme.service-now.com./api/now')
620+
expect(result.isValid).toBe(true)
621+
expect(result.sanitized).toBe('https://acme.service-now.com/api/now')
622+
})
623+
618624
it.concurrent.each([
619625
['https://support.acme.com', 'vanity CNAME'],
620626
['https://acme.service-now.com.evil.com', 'lookalike suffix'],

‎apps/sim/lib/core/security/input-validation.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,8 +1167,10 @@ function validateVendorHostedUrl(
11671167
if (!urlResult.isValid) return urlResult
11681168

11691169
const parsed = new URL(candidate)
1170-
// A fully qualified name's trailing dot names the same host.
1171-
const hostname = parsed.hostname.toLowerCase().replace(/\.$/, '')
1170+
// A trailing FQDN dot names the same host, but TLS hostname verification rejects it.
1171+
const fullyQualified = parsed.hostname.endsWith('.')
1172+
if (fullyQualified) parsed.hostname = parsed.hostname.slice(0, -1)
1173+
const hostname = parsed.hostname.toLowerCase()
11721174
const allowed = suffixes.some(
11731175
(suffix) => (allowBareSuffix && hostname === suffix.slice(1)) || hostname.endsWith(suffix)
11741176
)
@@ -1186,7 +1188,8 @@ function validateVendorHostedUrl(
11861188
}
11871189
}
11881190

1189-
return { isValid: true, sanitized: sanitize === 'origin' ? parsed.origin : candidate }
1191+
if (sanitize === 'origin') return { isValid: true, sanitized: parsed.origin }
1192+
return { isValid: true, sanitized: fullyQualified ? parsed.href : candidate }
11901193
}
11911194

11921195
/**

‎apps/sim/tools/databricks/databricks.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ describe('databricks workspace host allowlist', () => {
8484
['adb-123.4.databricks.azure.cn', 'https://adb-123.4.databricks.azure.cn'],
8585
['dbc-a1b2.cloud.databricks.mil', 'https://dbc-a1b2.cloud.databricks.mil'],
8686
['https://acme.databricks.com/', 'https://acme.databricks.com'],
87-
['dbc-a1b2.cloud.databricks.com.', 'https://dbc-a1b2.cloud.databricks.com.'],
8887
])('builds the same request URLs for workspace host %s', (host, origin) => {
8988
const params = { ...REQUEST_PARAMS, host }
9089
expect(urlBuilder('databricks_list_clusters')(params)).toBe(`${origin}/api/2.0/clusters/list`)
@@ -97,4 +96,16 @@ describe('databricks workspace host allowlist', () => {
9796
`${origin}/api/2.0/genie/spaces/space1/conversations/conv1/messages/msg1`
9897
)
9998
})
99+
100+
it.each([
101+
['dbc-a1b2.cloud.databricks.com.', 'https://dbc-a1b2.cloud.databricks.com'],
102+
['https://dbc-a1b2.cloud.databricks.com.:8443/', 'https://dbc-a1b2.cloud.databricks.com:8443'],
103+
])(
104+
'drops the trailing FQDN dot of %s, which the old tools kept and Bun TLS rejects',
105+
(host, origin) => {
106+
expect(urlBuilder('databricks_list_clusters')({ ...REQUEST_PARAMS, host })).toBe(
107+
`${origin}/api/2.0/clusters/list`
108+
)
109+
}
110+
)
100111
})

0 commit comments

Comments
 (0)