Skip to content

Commit 411cc9c

Browse files
authored
fix(security): harden saved-credential, proxy egress, and keyless-mint paths from the release audit (#9865)
* fix(security): harden saved-credential, proxy egress, and response-cap paths from the release audit - Refuse session callers resolving saved service-account secrets on POST /api/auth/oauth/token - Refuse a proxied hostname when any resolved record is blocked (the proxy re-resolves) - Apply the response cap to every body, not only decoded ones (undici's http proxy wrapper ignores maxResponseSize) - Bind workload-identity subjects to their owner at mint time - Bound Databricks token/identity JSON reads; refuse single-slash https:/ in typed URLs; case-insensitive Dynatrace /api/vN strip - Twilio recordings: stay on the Twilio API origin and bound fetch concurrency - Firecrawl: count crawl maxDepth below the start URL; stop sending unused search sources - sim-setup reset refuses when Docker is installed but unreachable - Remove the dead requireDeployment branch from workflow access middleware; reuse requireMutableWorkflow; move a misplaced TSDoc - Docs: CONTRIBUTING setup via sim-setup, sim-setup README, Helm read-only-root mounts, proxy and NO_PROXY behavior, GitLab CSV body limit * fix(security): stop queued Twilio recordings fetches on cancel, type token-route auth mocks, bump chart - Twilio list calls takes the run's signal: queued recordings requests stop once it aborts, and fetch receives it - Token route tests type the hybrid-auth mock through the real AuthResult and pass a route context - Chart version 1.11.13 for the README and helper comment changes
1 parent 15cf51d commit 411cc9c

25 files changed

Lines changed: 427 additions & 55 deletions

File tree

‎apps/docs/content/docs/platform/self-hosting/networking.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ This depends on the runtime. The published images run Bun. If you build the stan
278278
| Everything through the SSRF guard — SSO OIDC discovery, the HTTP block, tools, connectors, outbound webhooks, content fetches, MCP servers, and model endpoints typed into a block | Yes — see [Proxying guarded requests](#proxying-guarded-requests) |
279279
| SSO sign-in — the token, JWKS, and userinfo calls made while a user signs in | Yes — they use the default `fetch` |
280280
| SSO domain verification (TXT lookup) | No — DNS, sent to `1.1.1.1` and `8.8.8.8` unless `DOMAIN_VERIFICATION_NAMESERVERS` names your resolvers |
281-
| Amazon Bedrock, object storage on S3, and every AWS integration (DynamoDB, SQS, Lambda, Textract, and the rest) | Yes — each AWS client gets a proxy-aware agent while a proxy is set. Bedrock then uses HTTP/1.1 instead of HTTP/2 |
281+
| Amazon Bedrock, object storage on S3, and every AWS integration (DynamoDB, SQS, Lambda, Textract, and the rest) | Yes — each AWS service client gets a proxy-aware agent while a proxy is set, and STS calls made for its credentials inherit it. Bedrock then uses HTTP/1.1 instead of HTTP/2. IAM Identity Center (SSO) credential lookups do not use the proxy |
282282
| Email via SMTP, and the SMTP block | No — SMTP is not HTTP. Use a mail relay the app can reach directly, or Resend or SES |
283283
| Email via Amazon SES | Yes — the AWS SDK client above |
284284
| OTLP export from the server SDK | Yes — the exporters get the same proxy-aware agent |
@@ -289,7 +289,7 @@ The practical consequence: a mandatory-egress-proxy environment can route LLM tr
289289
290290
### Proxying guarded requests
291291
292-
A request through the SSRF guard keeps its checks when it is proxied. Before each request, and again for each redirect hop, the app resolves the destination's hostname with its own DNS and refuses it if any policy in [Security](/platform/self-hosting/security#the-ssrf-boundary) blocks the address. It then hands the hostname, not the address, to the proxy, so a proxy that allowlists by domain sees the name it expects.
292+
A request through the SSRF guard keeps its checks when it is proxied. Before each request, and again for each redirect hop, the app resolves the destination's hostname with its own DNS and refuses it if any policy in [Security](/platform/self-hosting/security#the-ssrf-boundary) blocks any of the addresses it resolves to, since the proxy may connect to any of them. It then hands the hostname, not the address, to the proxy, so a proxy that allowlists by domain sees the name it expects.
293293
294294
- **DNS must still resolve inside the container.** A destination that the app cannot resolve is refused before the proxy is asked.
295295
- **The proxy resolves the name again.** The app cannot pin the address across a proxy, so the proxy's own egress policy is the last check.

‎apps/docs/openapi-v2-resources.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23929,7 +23929,7 @@
2392923929
"type": "string",
2393023930
"minLength": 1,
2393123931
"maxLength": 4194304,
23932-
"description": "UTF-8 CSV contents, up to 4 MiB."
23932+
"description": "UTF-8 CSV contents, up to 4 MiB. Both files travel in one request body limited to 10 MiB after JSON encoding."
2393323933
}
2393423934
},
2393523935
"required": ["filename", "content"],
@@ -23949,7 +23949,7 @@
2394923949
"type": "string",
2395023950
"minLength": 1,
2395123951
"maxLength": 4194304,
23952-
"description": "UTF-8 CSV contents, up to 4 MiB."
23952+
"description": "UTF-8 CSV contents, up to 4 MiB. Both files travel in one request body limited to 10 MiB after JSON encoding."
2395323953
}
2395423954
},
2395523955
"required": ["filename", "content"],

‎apps/sim/app/api/auth/oauth/token/route.test.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
createMockRequest,
88
hybridAuthMockFns,
99
} from '@sim/testing'
10+
import { createRouteContext } from '@sim/testing/helpers/http'
1011
import { NextRequest } from 'next/server'
1112
import { beforeEach, describe, expect, it, vi } from 'vitest'
1213

@@ -36,11 +37,13 @@ vi.mock('@/lib/credentials/application/resolve-managed-oauth-token', () => ({
3637
resolveManagedOAuthCredentialToken: { execute: mockResolveManagedOAuthCredentialToken },
3738
}))
3839

40+
import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
3941
import { createQuickBooksAccountId } from '@/lib/oauth/quickbooks'
4042
import { GET, POST } from '@/app/api/auth/oauth/token/route'
4143
import { getToolMetadata } from '@/tools/metadata'
4244

4345
const mockGetToolMetadata = vi.mocked(getToolMetadata)
46+
const mockCheckSessionOrInternalAuth = vi.mocked(checkSessionOrInternalAuth)
4447

4548
describe('OAuth Token API Routes', () => {
4649
beforeEach(() => {
@@ -290,6 +293,116 @@ describe('OAuth Token API Routes', () => {
290293
})
291294
})
292295

296+
describe('saved service-account credentials', () => {
297+
const authorized = {
298+
ok: true,
299+
requesterUserId: 'test-user-id',
300+
credentialOwnerUserId: 'test-user-id',
301+
workspaceId: 'workspace-id',
302+
}
303+
304+
beforeEach(() => {
305+
authOAuthUtilsMockFns.mockResolveOAuthAccountId.mockResolvedValueOnce({
306+
accountId: '',
307+
credentialId: 'saved-credential-id',
308+
credentialType: 'service_account',
309+
providerId: 'vanta-service-account',
310+
workspaceId: 'workspace-id',
311+
usedCredentialTable: true,
312+
})
313+
authOAuthUtilsMockFns.mockResolveServiceAccountToken.mockResolvedValue({
314+
accessToken: 'saved-secret',
315+
})
316+
mockGetToolMetadata.mockReturnValue({
317+
id: 'vanta_list_tests',
318+
params: {},
319+
oauth: { required: true, provider: 'vanta', credentialKind: 'service-account' },
320+
})
321+
})
322+
323+
it('refuses to release the secret to a session caller', async () => {
324+
mockCheckSessionOrInternalAuth.mockResolvedValueOnce({
325+
success: true,
326+
authType: AuthType.SESSION,
327+
userId: 'test-user-id',
328+
})
329+
mockAuthorizeCredentialUse.mockResolvedValueOnce(authorized)
330+
331+
const response = await POST(
332+
createMockRequest('POST', {
333+
credentialId: 'saved-credential-id',
334+
toolId: 'vanta_list_tests',
335+
}),
336+
createRouteContext({})
337+
)
338+
339+
expect(response.status).toBe(403)
340+
const body = await response.json()
341+
expect(body).toMatchObject({ code: 'SERVICE_ACCOUNT_SESSION_FORBIDDEN' })
342+
expect(JSON.stringify(body)).not.toContain('saved-secret')
343+
expect(authOAuthUtilsMockFns.mockResolveServiceAccountToken).not.toHaveBeenCalled()
344+
})
345+
346+
it('resolves the secret for an internal-JWT execution caller', async () => {
347+
mockCheckSessionOrInternalAuth.mockResolvedValueOnce({
348+
success: true,
349+
authType: AuthType.INTERNAL_JWT,
350+
userId: 'test-user-id',
351+
})
352+
mockAuthorizeCredentialUse.mockResolvedValueOnce(authorized)
353+
354+
const response = await POST(
355+
createMockRequest('POST', {
356+
credentialId: 'saved-credential-id',
357+
toolId: 'vanta_list_tests',
358+
workflowId: 'workflow-id',
359+
}),
360+
createRouteContext({})
361+
)
362+
363+
expect(response.status).toBe(200)
364+
await expect(response.json()).resolves.toMatchObject({
365+
accessToken: 'saved-secret',
366+
credentialType: 'service_account',
367+
})
368+
})
369+
})
370+
371+
it('still returns an OAuth token to a session caller', async () => {
372+
mockCheckSessionOrInternalAuth.mockResolvedValueOnce({
373+
success: true,
374+
authType: AuthType.SESSION,
375+
userId: 'test-user-id',
376+
})
377+
mockAuthorizeCredentialUse.mockResolvedValueOnce({
378+
ok: true,
379+
requesterUserId: 'test-user-id',
380+
credentialOwnerUserId: 'test-user-id',
381+
})
382+
authOAuthUtilsMockFns.mockGetCredential.mockResolvedValueOnce({
383+
id: 'credential-id',
384+
accessToken: 'test-token',
385+
refreshToken: 'refresh-token',
386+
accessTokenExpiresAt: new Date(Date.now() + 3600 * 1000),
387+
providerId: 'google',
388+
})
389+
authOAuthUtilsMockFns.mockRefreshTokenIfNeeded.mockResolvedValueOnce({
390+
accessToken: 'session-oauth-token',
391+
refreshed: false,
392+
})
393+
394+
const response = await POST(
395+
createMockRequest('POST', { credentialId: 'credential-id' }),
396+
createRouteContext({})
397+
)
398+
399+
expect(response.status).toBe(200)
400+
await expect(response.json()).resolves.toMatchObject({
401+
accessToken: 'session-oauth-token',
402+
credentialType: 'oauth',
403+
})
404+
})
405+
293406
describe('credentialAccountUserId + providerId path', () => {
294407
it('should reject internal JWT authentication', async () => {
295408
hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValueOnce({

‎apps/sim/lib/api/contracts/knowledge/gitlab-permissions.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ const gitLabCsvUploadSchema = z
1111
.string()
1212
.min(1)
1313
.max(GITLAB_CSV_MAX_BYTES)
14-
.describe('UTF-8 CSV contents, up to 4 MiB.'),
14+
.describe(
15+
'UTF-8 CSV contents, up to 4 MiB. Both files travel in one request body limited to 10 MiB after JSON encoding.'
16+
),
1517
})
1618
.strict()
1719
export const gitLabPermissionInputSchema = z

‎apps/sim/lib/core/network/environment-proxy.server.test.ts‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,17 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites
1717
vi.mock('@/lib/core/network/config.server', () => networkConfigMock)
1818
vi.mock('@sim/security/dns', async (importOriginal) => {
1919
const actual = await importOriginal<typeof import('@sim/security/dns')>()
20-
const records: Record<string, string> = {
21-
'origin.invalid': '1.1.1.1',
22-
'internal.invalid': '10.0.0.5',
20+
const records: Record<string, [string, ...string[]]> = {
21+
'origin.invalid': ['1.1.1.1', '93.184.216.34'],
22+
'internal.invalid': ['10.0.0.5'],
23+
'split.invalid': ['93.184.216.34', '169.254.169.254'],
2324
}
2425
return {
2526
...actual,
2627
resolveHostAddresses: async (host: string) => {
27-
const address = records[host]
28-
if (!address) throw new Error(`No addresses for ${host}`)
29-
return { addresses: [address], preferred: address }
28+
const addresses = records[host]
29+
if (!addresses) throw new Error(`No addresses for ${host}`)
30+
return { addresses, preferred: addresses[0] }
3031
},
3132
}
3233
})
@@ -250,6 +251,23 @@ describe('guarded requests under HTTPS_PROXY', () => {
250251
expect(tunnels).toEqual([])
251252
})
252253

254+
it('refuses a name with any blocked record, since the proxy may dial any of them', async () => {
255+
const guarded = createSsrfGuardedFetchWithDispatcher(options)
256+
try {
257+
await expect(guarded.fetch('https://split.invalid/')).rejects.toThrow(/metadata/i)
258+
await expect(secureFetchWithValidation('https://split.invalid/', options)).rejects.toThrow(
259+
/metadata/i
260+
)
261+
} finally {
262+
await guarded.dispatcher.destroy()
263+
}
264+
expect(tunnels).toEqual([])
265+
266+
const response = await secureFetchWithValidation('https://origin.invalid/', options)
267+
expect(await response.text()).toBe('tls reached')
268+
expect(tunnels.map((tunnel) => tunnel.authority)).toEqual(['origin.invalid:443'])
269+
})
270+
253271
it('judges each redirect hop again before proxying it', async () => {
254272
const pinned = createPinnedFetchWithDispatcher('1.1.1.1', options)
255273
try {

‎apps/sim/lib/core/network/transport.server.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ export function createOutboundTransport(options: OutboundTransportOptions): Outb
4242
]
4343
/**
4444
* The proxy resolves the destination itself, so pinning cannot hold across it. Every request
45-
* (each redirect hop included) is resolved and judged here first, so a name that points at
46-
* a refused address is stopped before the proxy is asked to reach it. A name that changes
47-
* its answer between this check and the proxy's own lookup is left to the proxy's egress
48-
* policy, which is why the self-hosting docs make the proxy the final egress control.
45+
* (each redirect hop included) is resolved and judged here first, and a name is refused when
46+
* any of its records is refused, since the proxy may connect to any of them. A name that
47+
* changes its answer between this check and the proxy's own lookup is left to the proxy's
48+
* egress policy, which is why the self-hosting docs make the proxy the final egress control.
4949
*/
5050
const selectEnvironmentProxy = async (
5151
target: string,
@@ -56,6 +56,7 @@ export function createOutboundTransport(options: OutboundTransportOptions): Outb
5656
const validation = await validateEgressUrl(target, 'url', options.profile, {
5757
logDetails: false,
5858
signal,
59+
requireEveryAddress: true,
5960
})
6061
if (!validation.isValid) throw new Error(validation.error, { cause: validation.cause })
6162
if (closed) throw new OutboundRoutingError('GATEWAY_UNAVAILABLE')

‎apps/sim/lib/core/security/egress/validate.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ export interface EgressValidationOptions {
4949
logDetails?: boolean
5050
/** Cancels DNS validation before a guarded connection can begin. */
5151
signal?: AbortSignal
52+
/** Refuse when any record is refused; for transports that cannot pin (a forward proxy re-resolves). */
53+
requireEveryAddress?: boolean
5254
}
5355

5456
type EgressDenial = Extract<EgressDecision, { allowed: false }>
@@ -133,7 +135,7 @@ export async function validateEgressUrl(
133135
// Refused records are filtered rather than failing the whole host: pinning to a
134136
// surviving permitted address is just as safe as refusing outright, and a
135137
// split-horizon resolver that answers with a private record alongside the
136-
// public one would otherwise be unusable.
138+
// public one would otherwise be unusable. A caller that cannot pin opts out.
137139
let refusal: EgressDenial | undefined
138140
const usable = addresses.filter((address) => {
139141
const decision = evaluateAddress(parsed, address, policy)
@@ -142,6 +144,8 @@ export async function validateEgressUrl(
142144
return false
143145
})
144146

147+
if (refusal && options.requireEveryAddress) return fail(refusal, paramName, profile, options)
148+
145149
if (usable.length === 0) {
146150
return fail(
147151
refusal ?? { allowed: false, reason: 'address-blocked', detail: 'no addresses resolved' },

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock
99

1010
import {
1111
normalizeHttpBaseUrl,
12+
refuseUnsafeTypedUrl,
1213
tokenCheckValidator,
1314
} from '@/lib/credentials/token-service-accounts/validators/http'
1415

@@ -159,8 +160,11 @@ describe('tokenCheckValidator with a typed host', () => {
159160
)
160161
})
161162

162-
it('refuses a base URL with a mistyped scheme instead of reading the scheme as a host', () => {
163+
it('refuses a mistyped scheme instead of reading the scheme as a host', () => {
163164
expect(() => normalizeHttpBaseUrl('https:/grafana.example.com')).toThrow(/scheme/)
165+
expect(() => refuseUnsafeTypedUrl('https:/evil.com')).toThrow(/scheme/)
166+
expect(() => refuseUnsafeTypedUrl('HTTP:evil.com')).toThrow(/scheme/)
167+
expect(refuseUnsafeTypedUrl('https://grafana.example.com')).toBe('https://grafana.example.com')
164168
expect(normalizeHttpBaseUrl('grafana.example.com:3000/')).toBe(
165169
'https://grafana.example.com:3000'
166170
)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ export function normalizeHttpBaseUrl(raw: string | undefined): string {
121121
*/
122122
export function refuseUnsafeTypedUrl(raw: string | undefined): string {
123123
const typed = (raw ?? '').trim()
124+
if (/^https?:(?!\/\/)/i.test(typed)) throw new Error('malformed scheme')
124125
if (/^(?!https?:\/\/)[a-z][a-z\d+.-]*:\/\//i.test(typed)) throw new Error('http(s) only')
125126
const authority = typed.replace(/^[a-z][a-z\d+.-]*:\/\//i, '').split(/[/?#]/)[0]
126127
if (authority.includes('@')) throw new Error('credentials in URL')

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ export const validateDubServiceAccount = tokenCheckValidator({
420420
*/
421421
export const validateDynatraceServiceAccount = tokenCheckValidator({
422422
serviceLabel: 'Dynatrace',
423-
domain: ({ domain }) => normalizeHttpBaseUrl(domain).replace(/\/api\/v[12]$/, ''),
423+
domain: ({ domain }) => normalizeHttpBaseUrl(domain).replace(/\/api\/v[12]$/i, ''),
424424
egress: 'configuredEndpoint',
425425
request: ({ apiToken, domain = '' }) => ({
426426
url: buildDynatraceUrl(domain, '/apiTokens/lookup'),

0 commit comments

Comments
 (0)