Skip to content

Commit b9e8099

Browse files
committed
improvement(credentials): simplify and clean up the token-and-host checks
One normalizeHttpBaseUrl and one allowedDomain replace five URL normalizers and four region checks (fixing Grafana and Tinybird treating a non-http scheme as a host); region tables come from the tools; acceptsBody can name the credential (GitLab); Email Bison and Jupyter refuse credentials in the URL instead of dropping them; the Supabase public-URL policy is inlined; the Databricks token lifetime uses the issuer's cap.
1 parent d884d7e commit b9e8099

10 files changed

Lines changed: 140 additions & 166 deletions

File tree

‎apps/sim/lib/credentials/databricks-service-account.server.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
99

1010
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
1111
vi.mock('@/lib/credentials/workload-identity/issuer', () => ({
12+
MAX_WORKLOAD_IDENTITY_TOKEN_TTL_SECONDS: 3600,
1213
mintWorkloadIdentityToken: async ({ subject }: { subject: string }) => `token-for:${subject}`,
1314
}))
1415

‎apps/sim/lib/credentials/databricks-service-account.server.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from '@/lib/credentials/token-service-accounts/errors'
1313
import { WORKLOAD_IDENTITY_AUDIENCES } from '@/lib/credentials/workload-identity/audiences'
1414
import {
15+
MAX_WORKLOAD_IDENTITY_TOKEN_TTL_SECONDS,
1516
mintWorkloadIdentityToken,
1617
type WorkloadIdentityOwner,
1718
} from '@/lib/credentials/workload-identity/issuer'
@@ -183,7 +184,7 @@ async function resolveDatabricksAccessToken(
183184
audience: DATABRICKS_WORKLOAD_IDENTITY_AUDIENCE,
184185
owner,
185186
// Databricks' token expires with this one, and Genie polls for up to five minutes.
186-
ttlSeconds: 3600,
187+
ttlSeconds: MAX_WORKLOAD_IDENTITY_TOKEN_TTL_SECONDS,
187188
})
188189
return requestWorkspaceToken(
189190
config.host,

‎apps/sim/lib/credentials/token-service-accounts/validators/http.ts‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ interface TokenCheck {
4545
* of a plain fetch.
4646
*/
4747
egress?: Extract<EgressProfile, 'configuredEndpoint' | 'selfHostedService'>
48-
/** The vendor echoes the token in its error body, so that body is never read into logs. */
48+
/** The vendor echoes the token in its error body, so that body never reaches logs or error details. */
4949
errorBodyEchoesToken?: boolean
5050
/**
5151
* A cheap documented read that succeeds only with a valid token — the same
@@ -62,14 +62,10 @@ interface TokenCheck {
6262
forbiddenMeansScoped?: boolean
6363
/**
6464
* For a vendor that reports a rejected key inside a successful response (a
65-
* GraphQL `errors` array): returns whether the parsed body proves the key.
65+
* GraphQL `errors` array): returns whether the parsed body proves the key, or
66+
* a display name read from it (a GitLab token's name) when it does.
6667
*/
67-
acceptsBody?: (body: unknown) => boolean
68-
/**
69-
* Names the credential from the verified reply (a GitLab token's name), used
70-
* as its default display name alongside the domain or account id.
71-
*/
72-
describe?: (body: unknown) => string | undefined
68+
acceptsBody?: (body: unknown) => boolean | { displayName: string }
7369
/**
7470
* For a vendor whose error status alone doesn't say the key is bad (Fireflies
7571
* returns 500 with `auth_failed`; Brex returns its scope-error 403 for an
@@ -98,6 +94,49 @@ export function readJsonBody(response: ProviderResponse, serviceLabel: string):
9894
})
9995
}
10096

97+
/**
98+
* A typed http(s) base URL in the form tools build request paths on: a bare host
99+
* is taken as https, trailing slashes are dropped, and any other scheme,
100+
* credentials, a query, or a fragment is refused, since each would retarget the
101+
* request. `allowPath: false` also refuses a path, for tools that append to the
102+
* origin.
103+
*/
104+
export function normalizeHttpBaseUrl(
105+
raw: string | undefined,
106+
{ allowPath = true }: { allowPath?: boolean } = {}
107+
): string {
108+
const typed = (raw ?? '').trim()
109+
if (!typed) throw new Error('missing URL')
110+
const url = new URL(/^[a-z][a-z\d+.-]*:\/\//i.test(typed) ? typed : `https://${typed}`)
111+
if (url.protocol !== 'https:' && url.protocol !== 'http:') throw new Error('http(s) only')
112+
if (url.username || url.password || url.search || url.hash) {
113+
throw new Error('credentials, query, or fragment in URL')
114+
}
115+
const path = url.pathname.replace(/\/+$/, '')
116+
if (path && !allowPath) throw new Error('path not allowed')
117+
return `${url.origin}${path}`
118+
}
119+
120+
/**
121+
* Refuses a typed URL that carries credentials, before a tool's own normalizer
122+
* can drop them silently and verify a different identity than the user meant.
123+
*/
124+
export function refuseUrlCredentials(raw: string | undefined): string {
125+
const typed = (raw ?? '').trim()
126+
const authority = typed.replace(/^[a-z][a-z\d+.-]*:\/\//i, '').split(/[/?#]/)[0]
127+
if (authority.includes('@')) throw new Error('credentials in URL')
128+
return typed
129+
}
130+
131+
/** A domain that must be one of a fixed set (a region or regional host), compared case-insensitively. */
132+
export function allowedDomain(allowed: ReadonlySet<string>) {
133+
return ({ domain }: TokenServiceAccountFields): string => {
134+
const value = (domain ?? '').trim().toLowerCase()
135+
if (!allowed.has(value)) throw new Error('unknown region')
136+
return value
137+
}
138+
}
139+
101140
/** Runs `normalize` on a typed connection field, rejecting what it refuses as the user's input. */
102141
function normalizedField(
103142
normalize: ((fields: TokenServiceAccountFields) => string) | undefined,
@@ -224,6 +263,7 @@ export function tokenCheckValidator(check: TokenCheck): TokenServiceAccountValid
224263
}
225264
if (!scopedForbidden && !response.ok) {
226265
if (!check.errorBodyEchoesToken) await throwForProviderResponse(response, 'verify')
266+
// The body echoes the token, so it is dropped unread.
227267
await response.body?.cancel().catch(() => undefined)
228268
throw new TokenServiceAccountValidationError(
229269
providerFailureCode(response.status),
@@ -240,11 +280,15 @@ export function tokenCheckValidator(check: TokenCheck): TokenServiceAccountValid
240280
step: 'verify',
241281
})
242282
}
243-
if (!check.acceptsBody(body)) {
283+
const verdict = check.acceptsBody(body)
284+
if (!verdict) {
244285
throw new TokenServiceAccountValidationError('invalid_credentials', 400, { step: 'verify' })
245286
}
246-
const name = check.describe?.(body)
247-
return name ? { ...accepted, displayName: where ? `${name} (${where})` : name } : accepted
287+
if (typeof verdict === 'object') {
288+
const name = verdict.displayName
289+
return { ...accepted, displayName: where ? `${name} (${where})` : name }
290+
}
291+
return accepted
248292
}
249293
await response.body?.cancel().catch(() => undefined)
250294
return accepted

‎apps/sim/lib/credentials/token-service-accounts/validators/token-checks.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,40 @@
11
import { jsonResponse } from '@sim/testing/helpers/http'
2+
import {
3+
inputValidationMock,
4+
inputValidationMockFns,
5+
} from '@sim/testing/mocks/input-validation.mock'
26
import { describe, expect, it, vi } from 'vitest'
7+
8+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
9+
310
import {
411
validateAlgoliaServiceAccount,
512
validateAshbyServiceAccount,
613
validateAzureDevOpsServiceAccount,
714
validateBrexServiceAccount,
15+
validateConvexServiceAccount,
816
validateDevinServiceAccount,
17+
validateDynatraceServiceAccount,
918
validateElevenLabsServiceAccount,
19+
validateEmailBisonServiceAccount,
1020
validateFlintServiceAccount,
21+
validateGrafanaServiceAccount,
22+
validateInfisicalServiceAccount,
1123
validateInstantlyServiceAccount,
1224
validateJotformServiceAccount,
25+
validateJupyterServiceAccount,
26+
validateLogfireServiceAccount,
1327
validateMailgunServiceAccount,
1428
validateNewRelicServiceAccount,
1529
validateOktaServiceAccount,
30+
validatePlaneServiceAccount,
1631
validateResendServiceAccount,
1732
validateSupabaseServiceAccount,
1833
validateThriveServiceAccount,
34+
validateTinybirdServiceAccount,
1935
validateTwilioServiceAccount,
2036
validateTypeformServiceAccount,
37+
validateUpstashServiceAccount,
2138
validateVercelServiceAccount,
2239
validateYouTubeServiceAccount,
2340
} from '@/lib/credentials/token-service-accounts/validators/token-checks'
@@ -350,3 +367,28 @@ describe('saved credentials whose connection names a host, region, or account',
350367
).rejects.toMatchObject({ code: 'invalid_credentials' })
351368
})
352369
})
370+
371+
describe('saved credentials for a host the user chose', () => {
372+
it.each([
373+
['Convex', validateConvexServiceAccount],
374+
['Dynatrace', validateDynatraceServiceAccount],
375+
['Email Bison', validateEmailBisonServiceAccount],
376+
['Grafana', validateGrafanaServiceAccount],
377+
['Infisical', validateInfisicalServiceAccount],
378+
['Jupyter', validateJupyterServiceAccount],
379+
['Logfire', validateLogfireServiceAccount],
380+
['Plane', validatePlaneServiceAccount],
381+
['Tinybird', validateTinybirdServiceAccount],
382+
['Upstash', validateUpstashServiceAccount],
383+
])(
384+
'%s refuses credentials smuggled into the URL before sending the key anywhere',
385+
async (_, validate) => {
386+
inputValidationMockFns.mockSecureFetchWithValidation.mockReset()
387+
388+
await expect(
389+
validate({ apiToken: 'secret', domain: 'https://user:pass@instance.example.com' })
390+
).rejects.toMatchObject({ code: 'invalid_credentials' })
391+
expect(inputValidationMockFns.mockSecureFetchWithValidation).not.toHaveBeenCalled()
392+
}
393+
)
394+
})

0 commit comments

Comments
 (0)