Skip to content

Commit 3896437

Browse files
authored
fix(databricks): send workspace tokens only to Databricks workspace hosts (#8362)
* fix(databricks): send workspace tokens only to Databricks workspace hosts * fix(databricks): accept DoD, custom-URL, and trailing-dot workspace hosts * fix(security): strip a trailing FQDN dot from vendor-hosted URLs * test(databricks): assert every tool refuses a foreign host with the allowlist error
1 parent 635d275 commit 3896437

16 files changed

Lines changed: 170 additions & 85 deletions

‎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: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,6 +1167,9 @@ function validateVendorHostedUrl(
11671167
if (!urlResult.isValid) return urlResult
11681168

11691169
const parsed = new URL(candidate)
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)
11701173
const hostname = parsed.hostname.toLowerCase()
11711174
const allowed = suffixes.some(
11721175
(suffix) => (allowBareSuffix && hostname === suffix.slice(1)) || hostname.endsWith(suffix)
@@ -1185,7 +1188,8 @@ function validateVendorHostedUrl(
11851188
}
11861189
}
11871190

1188-
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 }
11891193
}
11901194

11911195
/**
@@ -1267,15 +1271,20 @@ export function validateWorkdayTenantUrl(
12671271
}
12681272

12691273
/**
1270-
* Every production Databricks control-plane DNS zone, mirroring `ALL_ENVS` in the
1271-
* Databricks SDK (`databricks/sdk/environments.py`). The SDK's `.dev.*`/`.staging.*`
1272-
* zones are internal and deliberately omitted; the ones that are subdomains of a
1273-
* zone listed here (e.g. `.staging.cloud.databricks.com`) match by suffix anyway.
1274+
* Every production Databricks control-plane DNS zone. All but `.cloud.databricks.mil` mirror
1275+
* `ALL_ENVS` in the Databricks SDK (`databricks/sdk/environments.py`); the DoD zone comes from the
1276+
* Databricks AWS GovCloud docs. The SDK's `.dev.*`/`.staging.*` zones are internal and
1277+
* deliberately omitted; the ones that are subdomains of a zone listed here (e.g.
1278+
* `.staging.cloud.databricks.com`) match by suffix anyway. `.databricks.com` admits workspace
1279+
* custom URLs (`acme.databricks.com`) and subsumes the AWS and GCP zones, which stay listed so
1280+
* the rejection message names them.
12741281
*/
12751282
const DATABRICKS_ALLOWED_HOST_SUFFIXES = [
12761283
'.cloud.databricks.com',
12771284
'.cloud.databricks.us',
1285+
'.cloud.databricks.mil',
12781286
'.gcp.databricks.com',
1287+
'.databricks.com',
12791288
'.azuredatabricks.net',
12801289
'.databricks.azure.us',
12811290
'.databricks.azure.cn',
@@ -1288,6 +1297,8 @@ const DATABRICKS_ALLOWED_HOST_SUFFIXES = [
12881297
* every REST call is made against it. Example valid hosts:
12891298
* - dbc-1234abcd-5678.cloud.databricks.com (AWS)
12901299
* - dbc-1234abcd-5678.cloud.databricks.us (AWS GovCloud)
1300+
* - dbc-1234abcd-5678.cloud.databricks.mil (AWS GovCloud DoD)
1301+
* - acme.databricks.com (workspace custom URL)
12911302
* - adb-1234567890123456.7.azuredatabricks.net (Azure)
12921303
* - adb-1234567890123456.7.databricks.azure.us (Azure US Government)
12931304
* - adb-1234567890123456.7.databricks.azure.cn (Azure China)

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
DatabricksCancelRunParams,
33
DatabricksCancelRunResponse,
44
} from '@/tools/databricks/types'
5+
import { databricksUrl } from '@/tools/databricks/utils'
56
import type { ToolConfig } from '@/tools/types'
67

78
export const cancelRunTool: ToolConfig<DatabricksCancelRunParams, DatabricksCancelRunResponse> = {
@@ -33,13 +34,7 @@ export const cancelRunTool: ToolConfig<DatabricksCancelRunParams, DatabricksCanc
3334
},
3435

3536
request: {
36-
url: (params) => {
37-
const host = params.host
38-
.trim()
39-
.replace(/^https?:\/\//, '')
40-
.replace(/\/$/, '')
41-
return `https://${host}/api/2.1/jobs/runs/cancel`
42-
},
37+
url: (params) => databricksUrl(params.host, '/api/2.1/jobs/runs/cancel'),
4338
method: 'POST',
4439
headers: (params) => ({
4540
'Content-Type': 'application/json',
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { inputValidationMock } from '@sim/testing'
2+
import { partialToolRegistry } from '@sim/testing/mocks/tool-registry.mock'
3+
import { getErrorMessage } from '@sim/utils/errors'
4+
import { describe, expect, it, vi } from 'vitest'
5+
6+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
7+
8+
import * as databricksTools from '@/tools/databricks'
9+
import { executeTool } from '@/tools/index'
10+
import { tools } from '@/tools/registry'
11+
12+
/** Registers only this service's configs in the global registry mock; the full one is ~6,000 modules. */
13+
Object.assign(tools, partialToolRegistry(databricksTools))
14+
15+
type ToolParams = Record<string, unknown>
16+
17+
function urlBuilder(toolId: string): (params: ToolParams) => string {
18+
const url = tools[toolId].request.url
19+
if (typeof url !== 'function') throw new Error(`${toolId} has a static url`)
20+
return url as (params: ToolParams) => string
21+
}
22+
23+
/** Every identifier any Databricks tool reads while building its URL. */
24+
const REQUEST_PARAMS = {
25+
apiKey: 'dapi-test-token',
26+
spaceId: 'space1',
27+
conversationId: 'conv1',
28+
messageId: 'msg1',
29+
attachmentId: 'att1',
30+
statementId: 'stmt1',
31+
clusterId: 'cluster1',
32+
jobId: 1,
33+
runId: 2,
34+
warehouseId: 'wh1',
35+
content: 'question',
36+
sql: 'SELECT 1',
37+
rating: 'POSITIVE',
38+
}
39+
40+
/** The validator's refusal, naming the `host` param and every allowlisted Databricks domain. */
41+
const HOST_ALLOWLIST_ERROR =
42+
'host must be a Databricks-hosted domain (e.g., *.cloud.databricks.com, *.cloud.databricks.us, *.cloud.databricks.mil, *.gcp.databricks.com, *.databricks.com, *.azuredatabricks.net, *.databricks.azure.us, *.databricks.azure.cn)'
43+
44+
const DATABRICKS_TOOL_IDS = Object.keys(tools).filter((id) => id.startsWith('databricks_'))
45+
46+
describe('databricks workspace host allowlist', () => {
47+
it.each([
48+
'attacker.example.com',
49+
'https://attacker.example.com/',
50+
'dbc-1.cloud.databricks.com.attacker.example.com',
51+
'attacker.example.com/dbc-1.cloud.databricks.com',
52+
'dbc-1.cloud.databricks.com@attacker.example.com',
53+
'databricks.com',
54+
'acme-databricks.com',
55+
])('refuses %s in every tool with the host allowlist error', (host) => {
56+
const notRefusedByAllowlist = DATABRICKS_TOOL_IDS.map((id) => {
57+
try {
58+
return `${id}: built ${urlBuilder(id)({ ...REQUEST_PARAMS, host })}`
59+
} catch (error) {
60+
return `${id}: ${getErrorMessage(error)}`
61+
}
62+
}).filter((outcome) => !outcome.endsWith(`: ${HOST_ALLOWLIST_ERROR}`))
63+
expect(notRefusedByAllowlist).toEqual([])
64+
})
65+
66+
it('fails the tool call for a foreign host with the allowlist error', async () => {
67+
const result = await executeTool('databricks_list_clusters', {
68+
host: 'attacker.example.com',
69+
apiKey: 'dapi-test-token',
70+
})
71+
72+
expect(result.success).toBe(false)
73+
expect(result.error).toContain(HOST_ALLOWLIST_ERROR)
74+
})
75+
76+
it.each([
77+
['dbc-a1b2.cloud.databricks.com', 'https://dbc-a1b2.cloud.databricks.com'],
78+
[' https://dbc-a1b2.cloud.databricks.com/ ', 'https://dbc-a1b2.cloud.databricks.com'],
79+
['http://dbc-a1b2.cloud.databricks.com', 'https://dbc-a1b2.cloud.databricks.com'],
80+
['adb-123.4.azuredatabricks.net', 'https://adb-123.4.azuredatabricks.net'],
81+
['https://123.4.gcp.databricks.com/', 'https://123.4.gcp.databricks.com'],
82+
['dbc-a1b2.cloud.databricks.us', 'https://dbc-a1b2.cloud.databricks.us'],
83+
['adb-123.4.databricks.azure.us', 'https://adb-123.4.databricks.azure.us'],
84+
['adb-123.4.databricks.azure.cn', 'https://adb-123.4.databricks.azure.cn'],
85+
['dbc-a1b2.cloud.databricks.mil', 'https://dbc-a1b2.cloud.databricks.mil'],
86+
['https://acme.databricks.com/', 'https://acme.databricks.com'],
87+
])('builds the same request URLs for workspace host %s', (host, origin) => {
88+
const params = { ...REQUEST_PARAMS, host }
89+
expect(urlBuilder('databricks_list_clusters')(params)).toBe(`${origin}/api/2.0/clusters/list`)
90+
expect(urlBuilder('databricks_execute_sql')(params)).toBe(`${origin}/api/2.0/sql/statements/`)
91+
expect(urlBuilder('databricks_get_job')(params)).toBe(`${origin}/api/2.1/jobs/get?job_id=1`)
92+
expect(urlBuilder('databricks_get_run_output')(params)).toBe(
93+
`${origin}/api/2.1/jobs/runs/get-output?run_id=2`
94+
)
95+
expect(urlBuilder('databricks_genie_get_message')(params)).toBe(
96+
`${origin}/api/2.0/genie/spaces/space1/conversations/conv1/messages/msg1`
97+
)
98+
})
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+
)
111+
})

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
DatabricksExecuteSqlParams,
33
DatabricksExecuteSqlResponse,
44
} from '@/tools/databricks/types'
5+
import { databricksUrl } from '@/tools/databricks/utils'
56
import type { ToolConfig } from '@/tools/types'
67

78
export const executeSqlTool: ToolConfig<DatabricksExecuteSqlParams, DatabricksExecuteSqlResponse> =
@@ -65,13 +66,7 @@ export const executeSqlTool: ToolConfig<DatabricksExecuteSqlParams, DatabricksEx
6566
},
6667

6768
request: {
68-
url: (params) => {
69-
const host = params.host
70-
.trim()
71-
.replace(/^https?:\/\//, '')
72-
.replace(/\/$/, '')
73-
return `https://${host}/api/2.0/sql/statements/`
74-
},
69+
url: (params) => databricksUrl(params.host, '/api/2.0/sql/statements/'),
7570
method: 'POST',
7671
headers: (params) => ({
7772
'Content-Type': 'application/json',

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
DatabricksGetClusterParams,
33
DatabricksGetClusterResponse,
44
} from '@/tools/databricks/types'
5+
import { databricksUrl } from '@/tools/databricks/utils'
56
import type { ToolConfig } from '@/tools/types'
67

78
export const getClusterTool: ToolConfig<DatabricksGetClusterParams, DatabricksGetClusterResponse> =
@@ -35,11 +36,7 @@ export const getClusterTool: ToolConfig<DatabricksGetClusterParams, DatabricksGe
3536

3637
request: {
3738
url: (params) => {
38-
const host = params.host
39-
.trim()
40-
.replace(/^https?:\/\//, '')
41-
.replace(/\/$/, '')
42-
const url = new URL(`https://${host}/api/2.0/clusters/get`)
39+
const url = new URL(databricksUrl(params.host, '/api/2.0/clusters/get'))
4340
url.searchParams.set('cluster_id', params.clusterId.trim())
4441
return url.toString()
4542
},

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { DatabricksGetJobParams, DatabricksGetJobResponse } from '@/tools/databricks/types'
2+
import { databricksUrl } from '@/tools/databricks/utils'
23
import type { ToolConfig } from '@/tools/types'
34

45
export const getJobTool: ToolConfig<DatabricksGetJobParams, DatabricksGetJobResponse> = {
@@ -30,11 +31,7 @@ export const getJobTool: ToolConfig<DatabricksGetJobParams, DatabricksGetJobResp
3031

3132
request: {
3233
url: (params) => {
33-
const host = params.host
34-
.trim()
35-
.replace(/^https?:\/\//, '')
36-
.replace(/\/$/, '')
37-
const url = new URL(`https://${host}/api/2.1/jobs/get`)
34+
const url = new URL(databricksUrl(params.host, '/api/2.1/jobs/get'))
3835
url.searchParams.set('job_id', String(params.jobId))
3936
return url.toString()
4037
},

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { DatabricksGetRunParams, DatabricksGetRunResponse } from '@/tools/databricks/types'
2+
import { databricksUrl } from '@/tools/databricks/utils'
23
import type { ToolConfig } from '@/tools/types'
34

45
export const getRunTool: ToolConfig<DatabricksGetRunParams, DatabricksGetRunResponse> = {
@@ -42,11 +43,7 @@ export const getRunTool: ToolConfig<DatabricksGetRunParams, DatabricksGetRunResp
4243

4344
request: {
4445
url: (params) => {
45-
const host = params.host
46-
.trim()
47-
.replace(/^https?:\/\//, '')
48-
.replace(/\/$/, '')
49-
const url = new URL(`https://${host}/api/2.1/jobs/runs/get`)
46+
const url = new URL(databricksUrl(params.host, '/api/2.1/jobs/runs/get'))
5047
url.searchParams.set('run_id', String(params.runId))
5148
if (params.includeHistory) url.searchParams.set('include_history', 'true')
5249
if (params.includeResolvedValues) url.searchParams.set('include_resolved_values', 'true')

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
DatabricksGetRunOutputParams,
33
DatabricksGetRunOutputResponse,
44
} from '@/tools/databricks/types'
5+
import { databricksUrl } from '@/tools/databricks/utils'
56
import type { ToolConfig } from '@/tools/types'
67

78
export const getRunOutputTool: ToolConfig<
@@ -36,13 +37,8 @@ export const getRunOutputTool: ToolConfig<
3637
},
3738

3839
request: {
39-
url: (params) => {
40-
const host = params.host
41-
.trim()
42-
.replace(/^https?:\/\//, '')
43-
.replace(/\/$/, '')
44-
return `https://${host}/api/2.1/jobs/runs/get-output?run_id=${params.runId}`
45-
},
40+
url: (params) =>
41+
databricksUrl(params.host, `/api/2.1/jobs/runs/get-output?run_id=${params.runId}`),
4642
method: 'GET',
4743
headers: (params) => ({
4844
Accept: 'application/json',

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
DatabricksExecuteSqlResponse,
33
DatabricksGetStatementParams,
44
} from '@/tools/databricks/types'
5+
import { databricksUrl } from '@/tools/databricks/utils'
56
import type { ToolConfig } from '@/tools/types'
67

78
export const getStatementTool: ToolConfig<
@@ -36,13 +37,8 @@ export const getStatementTool: ToolConfig<
3637
},
3738

3839
request: {
39-
url: (params) => {
40-
const host = params.host
41-
.trim()
42-
.replace(/^https?:\/\//, '')
43-
.replace(/\/$/, '')
44-
return `https://${host}/api/2.0/sql/statements/${params.statementId.trim()}`
45-
},
40+
url: (params) =>
41+
databricksUrl(params.host, `/api/2.0/sql/statements/${params.statementId.trim()}`),
4642
method: 'GET',
4743
headers: (params) => ({
4844
Accept: 'application/json',

0 commit comments

Comments
 (0)