From 587d3a3344f70326e7c1e22db07a39c5615eb0c6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:39:31 -0800 Subject: [PATCH 01/21] fix(api): tell a missing key owner from an owner whose ban state is null user.banned is nullable, so a real owner with banned = NULL had every personal key refused as orphaned. Select the joined user id and test that instead; the ban and suspension checks are unchanged. --- .../api/server/routes/v2-api-key-auth.test.ts | 21 +++++++++++++++++++ .../lib/api/server/routes/v2-api-key-auth.ts | 5 ++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts b/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts index 8308eee3ceb..36a0d751577 100644 --- a/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts +++ b/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts @@ -199,6 +199,26 @@ describe('v2 API key authentication', () => { ) }) + it('accepts a personal key whose owner has never had a ban state recorded', async () => { + queueTableRows(schemaMock.apiKey, [ + { + id: 'key-1', + userId: 'user-1', + workspaceId: null, + type: 'personal', + access: 'write', + expiresAt: null, + ownerUserId: 'user-1', + userBanned: null, + userSuspendedAt: null, + }, + ]) + + const result = await authenticateV2ApiKey({ apiKey: 'secret', bearer: null }) + + expect(result.principal).toEqual(createPersonalApiKeyPrincipal()) + }) + it('refuses a personal key whose owner row is gone as invalid, expired or not', async () => { for (const expiresAt of [null, new Date(Date.now() - 1)]) { queueTableRows(schemaMock.apiKey, [ @@ -209,6 +229,7 @@ describe('v2 API key authentication', () => { type: 'personal', access: 'write', expiresAt, + ownerUserId: null, userBanned: null, userSuspendedAt: null, }, diff --git a/apps/sim/lib/api/server/routes/v2-api-key-auth.ts b/apps/sim/lib/api/server/routes/v2-api-key-auth.ts index f4ac64492d0..fb0a28f6320 100644 --- a/apps/sim/lib/api/server/routes/v2-api-key-auth.ts +++ b/apps/sim/lib/api/server/routes/v2-api-key-auth.ts @@ -77,6 +77,8 @@ interface ApiKeyRow { type: string access: ApiKeyAccess expiresAt: Date | null + /** The joined owner's id: `null` only when the owner row is gone (`user.banned` is itself nullable). */ + ownerUserId: string | null userBanned: boolean | null userSuspendedAt: Date | null } @@ -84,7 +86,7 @@ interface ApiKeyRow { function requireValidRow(row: ApiKeyRow | undefined): ApiKeyRow { if (!row) throw new V2ApiKeyUnauthenticatedError() if (row.type === 'personal' && row.workspaceId === null) { - if (row.userBanned === null) { + if (row.ownerUserId === null) { logger.error('Personal API key is missing its credential owner', { keyId: row.id }) throw new V2ApiKeyUnauthenticatedError() } @@ -118,6 +120,7 @@ async function authenticateApiKey(apiKeyHeader: string): Promise Date: Tue, 10 Nov 2026 16:42:17 -0800 Subject: [PATCH 02/21] fix(credentials): report a refused connection host as host_not_allowed - egress validation names its refusal (dns or policy) instead of callers inferring it from a missing cause; an empty DNS answer is ENOTFOUND - the token check judges every DNS record when an environment proxy will carry the request, so a mixed public/private host is refused up front rather than failing in the transport as a retryable 502; the transport's own refusal is a typed EgressRefusedError and maps the same way - new host_not_allowed code (400) with its own connect-modal message - Flow and GitLab personal-token verification classify a refused or missing host the same way - the typed-URL scheme guard is shared and ignores the tabs and newlines the URL parser drops --- apps/sim/lib/core/network/transport.server.ts | 13 ++++- apps/sim/lib/core/security/egress/validate.ts | 25 +++++++-- .../security/input-validation.server.test.ts | 8 +++ .../core/security/input-validation.server.ts | 9 +++- .../credentials/gitlab-personal-token.test.ts | 10 ++++ .../lib/credentials/gitlab-personal-token.ts | 9 ++++ .../token-service-accounts/descriptors.ts | 2 + .../token-service-accounts/errors.ts | 2 + .../validators/flow.test.ts | 38 +++++++++++++ .../token-service-accounts/validators/flow.ts | 9 ++++ .../validators/http.test.ts | 54 +++++++++++++++++-- .../token-service-accounts/validators/http.ts | 48 +++++++++++------ packages/security/src/dns.test.ts | 5 +- packages/security/src/dns.ts | 3 +- 14 files changed, 207 insertions(+), 28 deletions(-) create mode 100644 apps/sim/lib/credentials/token-service-accounts/validators/flow.test.ts diff --git a/apps/sim/lib/core/network/transport.server.ts b/apps/sim/lib/core/network/transport.server.ts index b188fb7bb70..136643cfcd9 100644 --- a/apps/sim/lib/core/network/transport.server.ts +++ b/apps/sim/lib/core/network/transport.server.ts @@ -6,6 +6,14 @@ import { OutboundRoutingError } from '@/lib/core/network/routing' import type { EgressProfile } from '@/lib/core/security/egress/profiles' import { validateEgressUrl } from '@/lib/core/security/egress/validate' +/** The egress policy refused a destination the transport was asked to reach through a proxy. */ +export class EgressRefusedError extends Error { + constructor(message: string) { + super(message) + this.name = 'EgressRefusedError' + } +} + interface OutboundTransportOptions { profile: EgressProfile resolvedIP?: string @@ -58,7 +66,10 @@ export function createOutboundTransport(options: OutboundTransportOptions): Outb signal, requireEveryAddress: true, }) - if (!validation.isValid) throw new Error(validation.error, { cause: validation.cause }) + if (!validation.isValid) { + if (validation.refusal === 'policy') throw new EgressRefusedError(validation.error) + throw new Error(validation.error, { cause: validation.cause }) + } if (closed) throw new OutboundRoutingError('GATEWAY_UNAVAILABLE') let agent = environmentProxies.get(proxyUrl) if (!agent) { diff --git a/apps/sim/lib/core/security/egress/validate.ts b/apps/sim/lib/core/security/egress/validate.ts index afc17ddc680..84639492733 100644 --- a/apps/sim/lib/core/security/egress/validate.ts +++ b/apps/sim/lib/core/security/egress/validate.ts @@ -35,9 +35,13 @@ export interface EgressValidationSuccess { readonly originalHostname: string } +/** Why a destination failed: its name could not be resolved, or the policy refused it. */ +export type EgressRefusal = 'dns' | 'policy' + export interface EgressValidationFailure { readonly isValid: false readonly error: string + readonly refusal: EgressRefusal /** Retains resolver failure codes so callers can distinguish an outage from a policy denial. */ readonly cause?: unknown } @@ -67,7 +71,11 @@ function fail( ? { profile, reason: decision.reason, paramName } : { profile, reason: decision.reason, detail: decision.detail, paramName } ) - return { isValid: false, error: describeEgressDenial(decision, paramName, profile) } + return { + isValid: false, + error: describeEgressDenial(decision, paramName, profile), + refusal: 'policy', + } } /** @@ -85,14 +93,18 @@ export async function validateEgressUrl( ): Promise { options.signal?.throwIfAborted() if (!url || typeof url !== 'string') { - return { isValid: false, error: `${paramName} is required and must be a string` } + return { + isValid: false, + error: `${paramName} is required and must be a string`, + refusal: 'policy', + } } let parsed: URL try { parsed = new URL(url) } catch { - return { isValid: false, error: `${paramName} must be a valid URL` } + return { isValid: false, error: `${paramName} must be a valid URL`, refusal: 'policy' } } const policy = resolveEgressPolicy(profile) @@ -129,7 +141,12 @@ export async function validateEgressUrl( ? { profile, paramName } : { profile, paramName, host, error: toError(error).message } ) - return { isValid: false, error: `${paramName} hostname could not be resolved`, cause: error } + return { + isValid: false, + error: `${paramName} hostname could not be resolved`, + cause: error, + refusal: 'dns', + } } // Refused records are filtered rather than failing the whole host: pinning to a diff --git a/apps/sim/lib/core/security/input-validation.server.test.ts b/apps/sim/lib/core/security/input-validation.server.test.ts index c3570720075..38e8e88ea0e 100644 --- a/apps/sim/lib/core/security/input-validation.server.test.ts +++ b/apps/sim/lib/core/security/input-validation.server.test.ts @@ -11,6 +11,7 @@ vi.mock('@sim/security/dns', () => ({ addresses.find((address) => address.includes('.')) ?? addresses[0], })) +import { EgressRefusedError } from '@/lib/core/network/transport.server' import { secureFetchWithValidation, validateUrlWithDNS, @@ -122,6 +123,13 @@ describe('validateUrlWithDNS address classification', () => { ).rejects.toMatchObject({ message: 'url hostname could not be resolved', cause }) }) + it('throws a policy refusal as an egress refusal, distinct from a lookup failure', async () => { + mockResolve.mockResolvedValue(resolved(['10.0.0.5'])) + await expect( + secureFetchWithValidation('https://internal.example/api', { profile: 'configuredEndpoint' }) + ).rejects.toBeInstanceOf(EgressRefusedError) + }) + it('can conceal credential-derived host details in validation logs', async () => { mockResolve.mockRejectedValue(new Error('DNS failure with credential-host-canary')) diff --git a/apps/sim/lib/core/security/input-validation.server.ts b/apps/sim/lib/core/security/input-validation.server.ts index faa4cfd6ec8..28274c14bc7 100644 --- a/apps/sim/lib/core/security/input-validation.server.ts +++ b/apps/sim/lib/core/security/input-validation.server.ts @@ -20,12 +20,14 @@ import { import { OutboundRoutingError } from '@/lib/core/network/routing' import { createOutboundTransport, + EgressRefusedError, requestWithOutboundDispatcher, } from '@/lib/core/network/transport.server' import { describeEgressDenial, type EgressProfile } from '@/lib/core/security/egress/profiles' import { checkEgressUrl, checkResolvedEgress, + type EgressRefusal, type EgressValidationOptions, validateEgressUrl, } from '@/lib/core/security/egress/validate' @@ -48,6 +50,8 @@ export type AsyncValidationResult = isValid: false error: string cause?: unknown + /** Set by {@link validateUrlWithDNS}: a failed lookup, or a destination the policy refused. */ + refusal?: EgressRefusal resolvedIP?: undefined originalHostname?: undefined } @@ -74,7 +78,7 @@ export async function validateUrlWithDNS( const result = await validateEgressUrl(url, paramName, profile, options) return result.isValid ? { isValid: true, resolvedIP: result.resolvedIP, originalHostname: result.originalHostname } - : { isValid: false, error: result.error, cause: result.cause } + : { isValid: false, error: result.error, cause: result.cause, refusal: result.refusal } } /** @@ -1582,7 +1586,7 @@ export async function secureFetchWithPinnedIP( * @param options - Fetch options, including the required egress `profile` * @param paramName - Name of the parameter for error messages (default: 'url') * @returns SecureFetchResponse - * @throws Error if URL validation fails + * @throws EgressRefusedError if the egress policy refuses the URL, Error if its host cannot be resolved */ export async function secureFetchWithValidation( url: string, @@ -1594,6 +1598,7 @@ export async function secureFetchWithValidation( signal: options.signal, }) if (!validation.isValid) { + if (validation.refusal === 'policy') throw new EgressRefusedError(validation.error) throw new Error(validation.error, { cause: validation.cause }) } return secureFetchWithPinnedIP(url, validation.resolvedIP, options) diff --git a/apps/sim/lib/credentials/gitlab-personal-token.test.ts b/apps/sim/lib/credentials/gitlab-personal-token.test.ts index 689c31e76ea..4230b6e4d6c 100644 --- a/apps/sim/lib/credentials/gitlab-personal-token.test.ts +++ b/apps/sim/lib/credentials/gitlab-personal-token.test.ts @@ -8,6 +8,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) vi.mock('@/lib/core/security/encryption', () => encryptionMock) +import { EgressRefusedError } from '@/lib/core/network/transport.server' import { decryptPersonalToken, encryptPersonalToken, @@ -96,6 +97,15 @@ describe('GitLab personal token verification', () => { await expect(verifyGitLabPersonalToken('secret')).rejects.toThrow('GitLab rejected this token') expect(mocks.fetch).toHaveBeenCalledTimes(1) }) + it('reports a host the egress policy refuses as the user input', async () => { + mocks.fetch + .mockReset() + .mockRejectedValue(new EgressRefusedError('url resolves to a private address')) + await expect(verifyGitLabPersonalToken('secret', 'gitlab.internal')).rejects.toMatchObject({ + code: 'validation', + message: expect.stringContaining("can't connect to that address"), + }) + }) it('binds organization tokens independently of the execution workspace', async () => { mocks.encrypt.mockResolvedValue({ encrypted: 'ciphertext' }) const { workspaceId: _workspaceId, ...identity } = envelope diff --git a/apps/sim/lib/credentials/gitlab-personal-token.ts b/apps/sim/lib/credentials/gitlab-personal-token.ts index e1253723839..e655c0fca82 100644 --- a/apps/sim/lib/credentials/gitlab-personal-token.ts +++ b/apps/sim/lib/credentials/gitlab-personal-token.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { EgressRefusedError } from '@/lib/core/network/transport.server' import { OrchestrationError } from '@/lib/core/orchestration/types' import { resourceScopeFromOwner, sameResourceScope } from '@/lib/core/resource-scope' import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' @@ -55,6 +56,14 @@ export async function verifyGitLabPersonalToken(accessToken: string, rawHost?: s timeout: 10_000, maxResponseBytes: 64 * 1024, maxRedirects: 0, + }).catch((error: unknown) => { + if (error instanceof EgressRefusedError) { + throw new OrchestrationError( + 'validation', + "Sim can't connect to that address. Use your GitLab instance's public https URL." + ) + } + throw error }) if (!response.ok) { throw new OrchestrationError( diff --git a/apps/sim/lib/credentials/token-service-accounts/descriptors.ts b/apps/sim/lib/credentials/token-service-accounts/descriptors.ts index 808684be2ba..e5e12b3cf0c 100644 --- a/apps/sim/lib/credentials/token-service-accounts/descriptors.ts +++ b/apps/sim/lib/credentials/token-service-accounts/descriptors.ts @@ -2338,6 +2338,8 @@ export function getTokenServiceAccountErrorMessage( ) case 'site_not_found': return "We couldn't find an account at that domain. Check the spelling and try again." + case 'host_not_allowed': + return "Sim can't connect to that address. Use the service's public https URL." case 'provider_unavailable': return `We couldn't reach ${descriptor.serviceLabel} to verify these credentials. Try again in a moment.` case 'duplicate_display_name': diff --git a/apps/sim/lib/credentials/token-service-accounts/errors.ts b/apps/sim/lib/credentials/token-service-accounts/errors.ts index e334edb966c..1d394f57fe2 100644 --- a/apps/sim/lib/credentials/token-service-accounts/errors.ts +++ b/apps/sim/lib/credentials/token-service-accounts/errors.ts @@ -14,6 +14,8 @@ export type TokenServiceAccountValidationCode = | 'invalid_credentials' | 'permission_conflict' | 'site_not_found' + /** The egress policy refused the typed host (a private address, or plain http). */ + | 'host_not_allowed' | 'provider_unavailable' export class TokenServiceAccountValidationError extends Error { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/flow.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/flow.test.ts new file mode 100644 index 00000000000..c9464c73fe3 --- /dev/null +++ b/apps/sim/lib/credentials/token-service-accounts/validators/flow.test.ts @@ -0,0 +1,38 @@ +import { + inputValidationMock, + inputValidationMockFns, +} from '@sim/testing/mocks/input-validation.mock' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) + +import { EgressRefusedError } from '@/lib/core/network/transport.server' +import { validateFlowServiceAccount } from '@/lib/credentials/token-service-accounts/validators/flow' + +const fields = { apiToken: 'tok', domain: 'flow.internal', providerWorkspaceId: 'ws-1' } + +describe('validateFlowServiceAccount', () => { + it('reports a host the egress policy refuses as the user input, not a retryable outage', async () => { + inputValidationMockFns.mockSecureFetchWithValidation.mockRejectedValue( + new EgressRefusedError('url resolves to a private address') + ) + + await expect(validateFlowServiceAccount(fields)).rejects.toMatchObject({ + code: 'host_not_allowed', + status: 400, + }) + }) + + it('reports a host that does not exist as not found', async () => { + inputValidationMockFns.mockSecureFetchWithValidation.mockRejectedValue( + new Error('url hostname could not be resolved', { + cause: Object.assign(new Error('getaddrinfo ENOTFOUND'), { code: 'ENOTFOUND' }), + }) + ) + + await expect(validateFlowServiceAccount(fields)).rejects.toMatchObject({ + code: 'site_not_found', + status: 400, + }) + }) +}) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/flow.ts b/apps/sim/lib/credentials/token-service-accounts/validators/flow.ts index 9e819f73bb0..5096fc6860e 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/flow.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/flow.ts @@ -1,7 +1,9 @@ import { toRecord } from '@sim/utils/object' +import { EgressRefusedError } from '@/lib/core/network/transport.server' import { secureFetchWithValidation } from '@/lib/core/security/input-validation.server' import { userPrincipal } from '@/lib/credentials/principal' import { + isHostNotFound, isTransientProviderStatus, TokenServiceAccountValidationError, } from '@/lib/credentials/token-service-accounts/errors' @@ -60,6 +62,13 @@ export async function validateFlowServiceAccount( } } catch (error) { if (error instanceof TokenServiceAccountValidationError) throw error + // A host the egress policy refuses, or one that doesn't exist, is the user's input. + if (error instanceof EgressRefusedError) { + throw new TokenServiceAccountValidationError('host_not_allowed', 400) + } + if (error instanceof Error && isHostNotFound(error.cause)) { + throw new TokenServiceAccountValidationError('site_not_found', 400) + } throw new TokenServiceAccountValidationError('provider_unavailable', 502) } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts index a4856417897..317bd715fd7 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts @@ -3,10 +3,18 @@ import { inputValidationMock, inputValidationMockFns, } from '@sim/testing/mocks/input-validation.mock' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockResolveHost } = vi.hoisted(() => ({ mockResolveHost: vi.fn() })) vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) +vi.mock('@sim/security/dns', async (importOriginal) => ({ + ...(await importOriginal()), + resolveHostAddresses: mockResolveHost, +})) +import { EgressRefusedError } from '@/lib/core/network/transport.server' +import type * as InputValidation from '@/lib/core/security/input-validation.server' import { normalizeHttpBaseUrl, refuseUnsafeTypedUrl, @@ -178,6 +186,9 @@ describe('tokenCheckValidator with a typed host', () => { expect(() => normalizeHttpBaseUrl('https:/grafana.example.com')).toThrow(/scheme/) expect(() => refuseUnsafeTypedUrl('https:/evil.com')).toThrow(/scheme/) expect(() => refuseUnsafeTypedUrl('HTTP:evil.com')).toThrow(/scheme/) + // The URL parser drops tabs and newlines, so they must not hide a scheme from the guard. + expect(() => refuseUnsafeTypedUrl('ht\ntps:/evil.com')).toThrow(/scheme/) + expect(() => normalizeHttpBaseUrl('ht\ttps:/evil.com')).toThrow(/scheme/) expect(refuseUnsafeTypedUrl('https://grafana.example.com')).toBe('https://grafana.example.com') expect(normalizeHttpBaseUrl('grafana.example.com:3000/')).toBe( 'https://grafana.example.com:3000' @@ -193,14 +204,50 @@ describe('tokenCheckValidator with a typed host', () => { }) it('reports a host the egress policy refuses as the user input, not a retryable outage', async () => { - validateUrl.mockResolvedValue({ isValid: false, error: 'url resolves to a private address' }) + validateUrl.mockResolvedValue({ + isValid: false, + error: 'url resolves to a private address', + refusal: 'policy', + }) await expect( validateHost({ apiToken: 'good', domain: 'grafana.internal' }) - ).rejects.toMatchObject({ code: 'invalid_credentials', status: 400 }) + ).rejects.toMatchObject({ code: 'host_not_allowed', status: 400 }) expect(pinnedFetch).not.toHaveBeenCalled() }) + describe('behind an environment proxy', () => { + afterEach(() => { + vi.unstubAllEnvs() + }) + + it('refuses a host with a private record alongside its public one before sending the token', async () => { + vi.stubEnv('HTTPS_PROXY', 'http://proxy.internal:3128') + const actual = await vi.importActual( + '@/lib/core/security/input-validation.server' + ) + validateUrl.mockImplementation(actual.validateUrlWithDNS) + mockResolveHost.mockResolvedValue({ + addresses: ['93.184.216.34', '10.0.0.5'], + preferred: '93.184.216.34', + }) + pinnedFetch.mockResolvedValue(jsonResponse({ ok: true })) + + await expect( + validateHost({ apiToken: 'good', domain: 'split.example.com' }) + ).rejects.toMatchObject({ code: 'host_not_allowed', status: 400 }) + expect(pinnedFetch).not.toHaveBeenCalled() + }) + + it("reports the proxy route's own refusal as the host, not an outage", async () => { + pinnedFetch.mockRejectedValue(new EgressRefusedError('url resolves to a private address')) + + await expect( + validateHost({ apiToken: 'good', domain: 'tools.example.com' }) + ).rejects.toMatchObject({ code: 'host_not_allowed', status: 400 }) + }) + }) + it('reports a host that cannot be reached as the provider being unavailable', async () => { pinnedFetch.mockRejectedValue(new Error('connect ECONNREFUSED')) @@ -222,6 +269,7 @@ describe('tokenCheckValidator with a typed host', () => { isValid: false, error: 'url hostname could not be resolved', cause, + refusal: 'dns', }) await expect( diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/http.ts b/apps/sim/lib/credentials/token-service-accounts/validators/http.ts index 9eefe6eb482..c7b652e6554 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/http.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/http.ts @@ -1,4 +1,6 @@ +import { resolveEnvironmentProxy } from '@sim/security/environment-proxy' import { truncate } from '@sim/utils/string' +import { EgressRefusedError } from '@/lib/core/network/transport.server' import type { EgressProfile } from '@/lib/core/security/egress/profiles' import { secureFetchWithPinnedIP, @@ -98,6 +100,16 @@ export function readJsonBody(response: ProviderResponse, serviceLabel: string): }) } +/** + * Drops the tabs and newlines the URL parser ignores, then refuses `https:/host`, which + * would otherwise get a second scheme and parse as a host named `https`. + */ +function refuseMalformedScheme(typed: string): string { + const stripped = typed.replace(/[\t\n\r]/g, '') + if (/^https?:(?!\/\/)/i.test(stripped)) throw new Error('malformed scheme') + return stripped +} + /** * A typed http(s) base URL in the form tools build request paths on: a bare host * is taken as https, trailing slashes are dropped, and any other scheme, @@ -105,10 +117,9 @@ export function readJsonBody(response: ProviderResponse, serviceLabel: string): * request. */ export function normalizeHttpBaseUrl(raw: string | undefined): string { - const typed = (raw ?? '').trim() - if (!typed) throw new Error('missing URL') - // `https:/host` would otherwise get a second scheme and parse as a host named `https`. - if (/^https?:(?!\/\/)/i.test(typed)) throw new Error('malformed scheme') + const trimmed = (raw ?? '').trim() + if (!trimmed) throw new Error('missing URL') + const typed = refuseMalformedScheme(trimmed) const url = new URL(/^[a-z][a-z\d+.-]*:\/\//i.test(typed) ? typed : `https://${typed}`) if (url.protocol !== 'https:' && url.protocol !== 'http:') throw new Error('http(s) only') if (url.username || url.password || url.search || url.hash) { @@ -123,8 +134,7 @@ export function normalizeHttpBaseUrl(raw: string | undefined): string { * a host, and so verify a different identity or host than the user meant. */ export function refuseUnsafeTypedUrl(raw: string | undefined): string { - const typed = (raw ?? '').trim() - if (/^https?:(?!\/\/)/i.test(typed)) throw new Error('malformed scheme') + const typed = refuseMalformedScheme((raw ?? '').trim()) if (/^(?!https?:\/\/)[a-z][a-z\d+.-]*:\/\//i.test(typed)) throw new Error('http(s) only') const authority = typed.replace(/^[a-z][a-z\d+.-]*:\/\//i, '').split(/[/?#]/)[0] if (authority.includes('@')) throw new Error('credentials in URL') @@ -157,25 +167,29 @@ function normalizedField( } } +const hostNotAllowed = () => + new TokenServiceAccountValidationError('host_not_allowed', 400, { + step: 'verify', + reason: 'host not allowed', + }) + /** * Sends a check through the guarded egress profile. A host the policy refuses * (a private address, or plain http where https is required) is the user's - * input, so it fails as `invalid_credentials` rather than `provider_unavailable`, + * input, so it fails as `host_not_allowed` rather than `provider_unavailable`, * which renders as a retryable 503. A host that doesn't exist is `site_not_found`. */ async function sendThroughEgress( profile: NonNullable, { url, method = 'GET', headers, body }: TokenCheckRequest ): Promise { - const validation = await validateUrlWithDNS(url, 'url', profile, { logDetails: false }) + // A forward proxy re-resolves the host, so the transport refuses one with any refused record. + const validation = await validateUrlWithDNS(url, 'url', profile, { + logDetails: false, + requireEveryAddress: resolveEnvironmentProxy(new URL(url)) !== null, + }) if (!validation.isValid) { - // Only a failed DNS lookup carries a cause; without one the policy refused the host. - if (validation.cause === undefined) { - throw new TokenServiceAccountValidationError('invalid_credentials', 400, { - step: 'verify', - reason: 'host not allowed', - }) - } + if (validation.refusal !== 'dns') throw hostNotAllowed() const notFound = isHostNotFound(validation.cause) throw new TokenServiceAccountValidationError( notFound ? 'site_not_found' : 'provider_unavailable', @@ -194,7 +208,9 @@ async function sendThroughEgress( maxResponseBytes: MAX_EGRESS_RESPONSE_BYTES, logUrlValidationDetails: false, }) - } catch { + } catch (error) { + // The proxy route judges the host again; its answer may have changed since the check above. + if (error instanceof EgressRefusedError) throw hostNotAllowed() throw new TokenServiceAccountValidationError('provider_unavailable', 502, { step: 'verify', reason: 'host unreachable', diff --git a/packages/security/src/dns.test.ts b/packages/security/src/dns.test.ts index e56b4e7fea0..787f1c4c1d0 100644 --- a/packages/security/src/dns.test.ts +++ b/packages/security/src/dns.test.ts @@ -35,7 +35,10 @@ describe('resolveHostAddresses', () => { it('rejects rather than returning nothing when the resolver answers empty', async () => { mockLookup.mockResolvedValue([]) - await expect(resolveHostAddresses('empty.example')).rejects.toThrow('No addresses') + await expect(resolveHostAddresses('empty.example')).rejects.toMatchObject({ + message: expect.stringContaining('No addresses'), + code: 'ENOTFOUND', + }) }) it('cancels a pending lookup at the caller deadline and cleans up its listener and timer', async () => { diff --git a/packages/security/src/dns.ts b/packages/security/src/dns.ts index dc3611d07b2..8c0cc896284 100644 --- a/packages/security/src/dns.ts +++ b/packages/security/src/dns.ts @@ -93,7 +93,8 @@ export async function resolveHostAddresses( }), ]) if (resolved.length === 0) { - throw new Error(`No addresses for ${host}`) + // An empty answer is a name with no records, which the resolver itself reports as ENOTFOUND. + throw Object.assign(new Error(`No addresses for ${host}`), { code: 'ENOTFOUND' }) } const addresses = resolved.map((entry) => entry.address) as [string, ...string[]] return { From ba17d53118e676987dfa8e0a39413e115501d770 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:42:18 -0800 Subject: [PATCH 03/21] fix(credentials): report an oversized Databricks token success as a 502 An oversized 2xx token reply carried status 200 into provider_unavailable and its logs. A refusal keeps its own status. --- .../lib/credentials/databricks-service-account.server.test.ts | 1 + apps/sim/lib/credentials/databricks-service-account.server.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/credentials/databricks-service-account.server.test.ts b/apps/sim/lib/credentials/databricks-service-account.server.test.ts index e088e979c12..87681df75a8 100644 --- a/apps/sim/lib/credentials/databricks-service-account.server.test.ts +++ b/apps/sim/lib/credentials/databricks-service-account.server.test.ts @@ -194,6 +194,7 @@ describe('getting a Databricks token for a run', () => { await expect(resolveDatabricksCredentialToken('cred-1')).rejects.toMatchObject({ code: 'provider_unavailable', + status: 502, logDetail: { reason: 'Databricks token response exceeded the size limit' }, }) }) diff --git a/apps/sim/lib/credentials/databricks-service-account.server.ts b/apps/sim/lib/credentials/databricks-service-account.server.ts index 40a54c9be0d..ca1d56df7fb 100644 --- a/apps/sim/lib/credentials/databricks-service-account.server.ts +++ b/apps/sim/lib/credentials/databricks-service-account.server.ts @@ -153,9 +153,10 @@ async function requestWorkspaceToken( if (error instanceof SyntaxError) return {} signal?.throwIfAborted() if (isPayloadSizeLimitError(error)) { + // A refusal's status still holds; an oversized success is the provider misbehaving. throw new DatabricksCredentialError( 'Databricks token response exceeded the size limit', - response.status + response.ok ? 502 : response.status ) } // An interrupted body says nothing about the credential, whatever the status line was. From b5206f68f964e6cd27b536aef8f97cc64f714cc3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:43:11 -0800 Subject: [PATCH 04/21] docs(api): say v2 execute options need a full-access key or api:write run.version, async and waitTimeoutSeconds refuse read-only keys; the meta example lists search:read with the full-access scopes. --- apps/docs/openapi-v2-resources.json | 4 ++-- apps/docs/openapi-v2-workflows.json | 6 +++--- apps/sim/lib/api/contracts/v2/openapi/resources.ts | 2 +- apps/sim/lib/api/contracts/v2/workflows.ts | 6 +++--- packages/sim-cli/src/generated/v2-api.ts | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index c2e01560bf5..ed3d70bfc25 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -4986,7 +4986,7 @@ "v2Enabled": true, "keyType": "personal", "expiresAt": null, - "scopes": ["api:read", "api:write"] + "scopes": ["api:read", "api:write", "search:read"] } } } @@ -18677,7 +18677,7 @@ "v2Enabled": true, "keyType": "personal", "expiresAt": null, - "scopes": ["api:read", "api:write"] + "scopes": ["api:read", "api:write", "search:read"] } } ] diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index f0b30c41413..d18c6fee64d 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -14928,7 +14928,7 @@ "description": "Execute a deployed workflow version." }, "version": { - "description": "Deployment version to execute instead of the active one. Requires an API key or OAuth access token with workspace write access, and the workflow must be deployed. An unknown version returns `404`.", + "description": "Deployment version to execute instead of the active one. Requires a full-access API key or an OAuth access token with `api:write`, plus workspace write access, and the workflow must be deployed. An unknown version returns `404`.", "type": "integer", "minimum": 1, "maximum": 2147483647 @@ -15006,7 +15006,7 @@ }, "async": { "default": false, - "description": "Queue the run and return a 202 receipt. Requires an API key or OAuth access token and a deployed run source. Cannot stream, select outputs, enable thinking/tool events, or include file-inlining options (`includeFileBase64` or `base64MaxBytes`).", + "description": "Queue the run and return a 202 receipt. Requires a full-access API key or an OAuth access token with `api:write`, and a deployed run source. Cannot stream, select outputs, enable thinking/tool events, or include file-inlining options (`includeFileBase64` or `base64MaxBytes`).", "type": "boolean" }, "executionTimeoutSeconds": { @@ -15016,7 +15016,7 @@ "maximum": 604800 }, "waitTimeoutSeconds": { - "description": "Wait at most this many seconds for a synchronous run; `0` waits until it settles. A run still going then continues in the background and the response carries its run receipt instead of its result: `202` on a JSON response, or a closing `detached` frame on an `application/x-ndjson` result stream, which keeps its `200`. The receipt is sent only once the run can be read on the runs resource, where it can also be cancelled. The run is admitted under asynchronous limits, and a dropped connection no longer cancels it. Requires an API key or OAuth access token. Cannot be combined with `async` or `stream`.", + "description": "Wait at most this many seconds for a synchronous run; `0` waits until it settles. A run still going then continues in the background and the response carries its run receipt instead of its result: `202` on a JSON response, or a closing `detached` frame on an `application/x-ndjson` result stream, which keeps its `200`. The receipt is sent only once the run can be read on the runs resource, where it can also be cancelled. The run is admitted under asynchronous limits, and a dropped connection no longer cancels it. Requires a full-access API key or an OAuth access token with `api:write`. Cannot be combined with `async` or `stream`.", "type": "integer", "minimum": 0, "maximum": 604800 diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index 48c288c984b..4adeaba2f02 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -1939,7 +1939,7 @@ const declaredRoutes = [ v2Enabled: true, keyType: 'personal', expiresAt: null, - scopes: ['api:read', 'api:write'], + scopes: ['api:read', 'api:write', 'search:read'], }, }, ] diff --git a/apps/sim/lib/api/contracts/v2/workflows.ts b/apps/sim/lib/api/contracts/v2/workflows.ts index 8f51805f36f..127aae80a39 100644 --- a/apps/sim/lib/api/contracts/v2/workflows.ts +++ b/apps/sim/lib/api/contracts/v2/workflows.ts @@ -1508,7 +1508,7 @@ export const v2WorkflowRunSelectionSchema = z.discriminatedUnion('source', [ version: versionNumberSchema .optional() .describe( - 'Deployment version to execute instead of the active one. Requires an API key or OAuth access token with workspace write access, and the workflow must be deployed. An unknown version returns `404`.' + 'Deployment version to execute instead of the active one. Requires a full-access API key or an OAuth access token with `api:write`, plus workspace write access, and the workflow must be deployed. An unknown version returns `404`.' ), }) .strict(), @@ -1599,7 +1599,7 @@ export const v2ExecuteWorkflowBodySchema = z .optional() .default(false) .describe( - 'Queue the run and return a 202 receipt. Requires an API key or OAuth access token and a deployed run source. Cannot stream, select outputs, enable thinking/tool events, or include file-inlining options (`includeFileBase64` or `base64MaxBytes`).' + 'Queue the run and return a 202 receipt. Requires a full-access API key or an OAuth access token with `api:write`, and a deployed run source. Cannot stream, select outputs, enable thinking/tool events, or include file-inlining options (`includeFileBase64` or `base64MaxBytes`).' ), /** * An upper bound on the request, not the effective timeout: the server @@ -1628,7 +1628,7 @@ export const v2ExecuteWorkflowBodySchema = z .max(MAX_WORKFLOW_EXECUTION_TIMEOUT_SECONDS) .optional() .describe( - 'Wait at most this many seconds for a synchronous run; `0` waits until it settles. A run still going then continues in the background and the response carries its run receipt instead of its result: `202` on a JSON response, or a closing `detached` frame on an `application/x-ndjson` result stream, which keeps its `200`. The receipt is sent only once the run can be read on the runs resource, where it can also be cancelled. The run is admitted under asynchronous limits, and a dropped connection no longer cancels it. Requires an API key or OAuth access token. Cannot be combined with `async` or `stream`.' + 'Wait at most this many seconds for a synchronous run; `0` waits until it settles. A run still going then continues in the background and the response carries its run receipt instead of its result: `202` on a JSON response, or a closing `detached` frame on an `application/x-ndjson` result stream, which keeps its `200`. The receipt is sent only once the run can be read on the runs resource, where it can also be cancelled. The run is admitted under asynchronous limits, and a dropped connection no longer cancels it. Requires a full-access API key or an OAuth access token with `api:write`. Cannot be combined with `async` or `stream`.' ), stream: z .boolean() diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 90fc3b7111f..e2d7d149ccc 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -18702,7 +18702,7 @@ export const V2_OPERATIONS = { kind: 'boolean', default: false, describe: - 'Queue the run and return a 202 receipt. Requires an API key or OAuth access token and a deployed run source. Cannot stream, select outputs, enable thinking/tool events, or include file-inlining options (`includeFileBase64` or `base64MaxBytes`).', + 'Queue the run and return a 202 receipt. Requires a full-access API key or an OAuth access token with `api:write`, and a deployed run source. Cannot stream, select outputs, enable thinking/tool events, or include file-inlining options (`includeFileBase64` or `base64MaxBytes`).', }, executionTimeoutSeconds: { kind: 'integer', @@ -18712,7 +18712,7 @@ export const V2_OPERATIONS = { waitTimeoutSeconds: { kind: 'integer', describe: - 'Wait at most this many seconds for a synchronous run; `0` waits until it settles. A run still going then continues in the background and the response carries its run receipt instead of its result: `202` on a JSON response, or a closing `detached` frame on an `application/x-ndjson` result stream, which keeps its `200`. The receipt is sent only once the run can be read on the runs resource, where it can also be cancelled. The run is admitted under asynchronous limits, and a dropped connection no longer cancels it. Requires an API key or OAuth access token. Cannot be combined with `async` or `stream`.', + 'Wait at most this many seconds for a synchronous run; `0` waits until it settles. A run still going then continues in the background and the response carries its run receipt instead of its result: `202` on a JSON response, or a closing `detached` frame on an `application/x-ndjson` result stream, which keeps its `200`. The receipt is sent only once the run can be read on the runs resource, where it can also be cancelled. The run is admitted under asynchronous limits, and a dropped connection no longer cancels it. Requires a full-access API key or an OAuth access token with `api:write`. Cannot be combined with `async` or `stream`.', }, stream: { kind: 'boolean', From 894ab5687c20c57bd87e66d2286982dcccdb3b6b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:43:12 -0800 Subject: [PATCH 05/21] chore(scripts): scope the payload-param allowlist to internal operations Every listed tool is an internal operation, so the probed-request fallback no longer builds or consults it. --- apps/sim/scripts/saved-credential-contract.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/sim/scripts/saved-credential-contract.ts b/apps/sim/scripts/saved-credential-contract.ts index 4d8b9d6398c..8fb5d3696ac 100644 --- a/apps/sim/scripts/saved-credential-contract.ts +++ b/apps/sim/scripts/saved-credential-contract.ts @@ -31,8 +31,9 @@ const DESTINATION_PARAM = /(host|hostname|domain|endpoint|url|uri|origin|server|region|instance|shop|account|site|org|tenant|workspace|team|subdomain|space|cluster|environment|location|project)(name|slug|id)?$/i /** - * Destination-named params of operation-backed tools that are request data, not the host + * Destination-named params of internal-operation tools that are request data, not the host * the credential goes to: each operation sends its token only to the vendor's fixed API. + * Only the operation branch reads it; a request tool's URL is probed instead. */ const OPERATION_PAYLOAD_PARAMS: Readonly> = { // The page the remote browser opens; the token goes to api.browser-use.com. @@ -166,9 +167,8 @@ export function hostSteeringParams(tool: ExecutableToolConfig): HostSteeringResu } } if (!probed) { - const payload = new Set(OPERATION_PAYLOAD_PARAMS[tool.id] ?? []) return { - params: stringParams.filter((name) => DESTINATION_PARAM.test(name) && !payload.has(name)), + params: stringParams.filter((name) => DESTINATION_PARAM.test(name)), probed: false, } } From cc7816bb5940e040f67a9186d225c13374b195be Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 15:58:40 -0800 Subject: [PATCH 06/21] fix(redaction): keep Infisical secret names visible; redact only deployKey --- apps/sim/lib/core/security/redaction.test.ts | 5 ++--- apps/sim/lib/core/security/redaction.ts | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/sim/lib/core/security/redaction.test.ts b/apps/sim/lib/core/security/redaction.test.ts index cca3c164b04..07a562e9a5b 100644 --- a/apps/sim/lib/core/security/redaction.test.ts +++ b/apps/sim/lib/core/security/redaction.test.ts @@ -31,9 +31,8 @@ describe('isSensitiveKey', () => { 'exa_api_key', 'secretAccessKey', 'awsSecretAccessKey', - 'secretKey', - 'bedrockSecretKey', 'deployKey', + 'convexDeployKey', `${'a'.repeat(10000)}password`, ])('treats %s as sensitive', (key) => { expect(isSensitiveKey(key)).toBe(true) @@ -42,7 +41,7 @@ describe('isSensitiveKey', () => { it.concurrent.each([ 'tokenCount', 'tokenizer', - 'secretKeyId', + 'secretKey', 'deployKeyName', 'passwordStrength', 'authMethod', diff --git a/apps/sim/lib/core/security/redaction.ts b/apps/sim/lib/core/security/redaction.ts index a1c1a52aa11..635774fc85c 100644 --- a/apps/sim/lib/core/security/redaction.ts +++ b/apps/sim/lib/core/security/redaction.ts @@ -29,9 +29,8 @@ const SENSITIVE_KEY_PATTERNS: RegExp[] = [ /^.*api[_-]?key$/i, // AWS's `secretAccessKey` / `awsSecretAccessKey`, which no suffix above covers. /^.*secret[_-]?access[_-]?key$/i, - // Credential fields named for the key itself: Clerk's `secretKey`, Bedrock's - // `bedrockSecretKey`, and Convex's `deployKey`. - /^.*secret[_-]?key$/i, + // Convex's `deployKey`. A bare `secretKey` is left out: Infisical returns each + // secret's name in that field, and redacting it would hide every listed name. /^.*deploy[_-]?key$/i, /^passphrase$/i, /^authorization$/i, From d3d4d9bf5e2533f29ada3b1eb6723f8bc54fa80a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:35:33 -0800 Subject: [PATCH 07/21] docs(networking): the SSRF guard matches NO_PROXY like curl, not like Bun --- apps/docs/content/docs/platform/self-hosting/networking.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/docs/content/docs/platform/self-hosting/networking.mdx b/apps/docs/content/docs/platform/self-hosting/networking.mdx index 23ce8ebcdbd..62b8aecd14d 100644 --- a/apps/docs/content/docs/platform/self-hosting/networking.mdx +++ b/apps/docs/content/docs/platform/self-hosting/networking.mdx @@ -265,7 +265,7 @@ Most `/api` requests also pass through Next.js Proxy. The current app configurat The app makes outbound calls to model providers, integration APIs, your email provider, object storage, and your telemetry backend. Whether `HTTP_PROXY` / `HTTPS_PROXY` apply depends on which of those paths a call takes — there is no single answer, and no global setting that covers all of them. -The server runs on Bun, and Bun's native `fetch` honors `$HTTP_PROXY`, `$HTTPS_PROXY`, and `$NO_PROXY`. Sim installs no global dispatcher. Default `fetch` uses the configured proxy unless `NO_PROXY` excludes the destination. The SSRF guard reads the same three variables, with the same rules, for every request it sends. The rest either build their own HTTP agent or speak a non-HTTP protocol. +The server runs on Bun, and Bun's native `fetch` honors `$HTTP_PROXY`, `$HTTPS_PROXY`, and `$NO_PROXY`. Sim installs no global dispatcher. Default `fetch` uses the configured proxy unless `NO_PROXY` excludes the destination. The SSRF guard reads the same three variables for every request it sends, matching `NO_PROXY` the way curl does (see [Internal destinations and `NO_PROXY`](#internal-destinations-and-no_proxy)). The rest either build their own HTTP agent or speak a non-HTTP protocol. This depends on the runtime. The published images run Bun. If you build the standalone output and run it under Node instead, Node ignores these variables unless started with `NODE_USE_ENV_PROXY=1` (Node 22.21+ / 24.5+), and the `fetch`-based "Yes" rows stop being proxied — model providers, Resend, Gmail sending, the desktop update feed, and the telemetry relay. The Azure Blob, GCS and Azure Communication Services rows still hold: those SDKs read the proxy variables through their own agents rather than through `fetch`. See [Bun fetch](https://bun.sh/docs/runtime/networking/fetch) and [Node proxy configuration](https://nodejs.org/learn/http/enterprise-network-configuration). From 49ef21a28cb772594cf3e72639a033c5a75589a1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:45:17 -0800 Subject: [PATCH 08/21] fix(api-keys): never echo an internal key-creation failure to the caller The unauthenticated CLI poll route and the settings routes returned the raw error message for an internal failure. Return a fixed message and keep the cause in the server log; a conflict still names the existing key. The CLI's comment now says a 400 can also be a refused key creation. --- .../lib/api-key/orchestration/index.test.ts | 54 +++++++++++++++++++ apps/sim/lib/api-key/orchestration/index.ts | 11 ++-- packages/sim-cli/src/auth/device-flow.ts | 5 +- 3 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 apps/sim/lib/api-key/orchestration/index.test.ts diff --git a/apps/sim/lib/api-key/orchestration/index.test.ts b/apps/sim/lib/api-key/orchestration/index.test.ts new file mode 100644 index 00000000000..58218515af6 --- /dev/null +++ b/apps/sim/lib/api-key/orchestration/index.test.ts @@ -0,0 +1,54 @@ +import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { createWorkspaceKey } = vi.hoisted(() => ({ createWorkspaceKey: vi.fn() })) +vi.mock('@/lib/api-key/auth', () => ({ + createApiKey: vi.fn(), + createWorkspaceApiKey: createWorkspaceKey, +})) + +import { + performCreatePersonalApiKey, + performCreateWorkspaceApiKey, +} from '@/lib/api-key/orchestration' + +const INTERNAL_DETAIL = 'connect ECONNREFUSED 10.0.0.5:5432 for role april' + +describe('API key creation failures', () => { + beforeEach(() => { + resetDbChainMock() + createWorkspaceKey.mockReset() + }) + + it('never echoes an internal personal-key failure to the caller', async () => { + dbChainMockFns.limit.mockRejectedValueOnce(new Error(INTERNAL_DETAIL)) + + const result = await performCreatePersonalApiKey({ userId: 'user-1', name: 'CLI' }) + + expect(result).toMatchObject({ success: false, errorCode: 'internal' }) + expect(result.error).not.toContain('ECONNREFUSED') + }) + + it('never echoes an internal workspace-key failure, but still names a conflict', async () => { + createWorkspaceKey.mockRejectedValueOnce(new Error(INTERNAL_DETAIL)) + const internal = await performCreateWorkspaceApiKey({ + workspaceId: 'ws-1', + userId: 'user-1', + name: 'CLI', + }) + expect(internal).toMatchObject({ success: false, errorCode: 'internal' }) + expect(internal.error).not.toContain('ECONNREFUSED') + + createWorkspaceKey.mockRejectedValueOnce(new Error('A key named "CLI" already exists')) + const conflict = await performCreateWorkspaceApiKey({ + workspaceId: 'ws-1', + userId: 'user-1', + name: 'CLI', + }) + expect(conflict).toEqual({ + success: false, + errorCode: 'conflict', + error: 'A key named "CLI" already exists', + }) + }) +}) diff --git a/apps/sim/lib/api-key/orchestration/index.ts b/apps/sim/lib/api-key/orchestration/index.ts index c59b94fc10e..ac2b794fc07 100644 --- a/apps/sim/lib/api-key/orchestration/index.ts +++ b/apps/sim/lib/api-key/orchestration/index.ts @@ -208,7 +208,8 @@ export async function performCreatePersonalApiKey( return { success: true, key: { ...created, key: plainKey } } } catch (error) { logger.error('Failed to create personal API key', { error }) - return { success: false, errorCode: 'internal', error: toError(error).message } + // The CLI poll route is unauthenticated, so an internal failure never echoes its cause. + return { success: false, errorCode: 'internal', error: 'Failed to create API key' } } } @@ -266,10 +267,10 @@ export async function performCreateWorkspaceApiKey( } catch (error) { const message = toError(error).message logger.error('Failed to create workspace API key', { error }) - return { - success: false, - error: message, - errorCode: message.includes('already exists') ? 'conflict' : 'internal', + if (message.includes('already exists')) { + return { success: false, error: message, errorCode: 'conflict' } } + // The CLI poll route is unauthenticated, so an internal failure never echoes its cause. + return { success: false, error: 'Failed to create API key', errorCode: 'internal' } } } diff --git a/packages/sim-cli/src/auth/device-flow.ts b/packages/sim-cli/src/auth/device-flow.ts index 4a25861dd2b..9a4465ad756 100644 --- a/packages/sim-cli/src/auth/device-flow.ts +++ b/packages/sim-cli/src/auth/device-flow.ts @@ -39,8 +39,9 @@ const POLL_PATH = '/api/cli/auth/poll' * 429 is the poll cadence hitting the per-IP bucket, not a refusal. * * Everything else stays terminal: 400 means a malformed request id or verifier, - * and 401/403/404 mean the server is refusing on purpose. Retrying those just - * spins until the 15-minute timeout. + * or a key the server refused to create as requested, which the same poll would + * be refused again for; 401/403/404 mean the server is refusing on purpose. + * Retrying those just spins until the 15-minute timeout. */ const RETRYABLE_POLL_STATUSES = new Set([409, 429, 500, 502, 503, 504]) From f0c91faaabf83a1096df62ce0ae90b7918df3475 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:45:18 -0800 Subject: [PATCH 09/21] chore: log a failed route-cache clear and spell out the read-only key rollback The route-cache clear stays non-fatal but warns when it fails for any reason other than a missing build id or cache. The api-key-read-only flag says to revoke or replace read-only keys before rolling back. --- apps/sim/lib/core/config/feature-flags.ts | 2 +- apps/sim/lib/core/utils/route-cache.ts | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/core/config/feature-flags.ts b/apps/sim/lib/core/config/feature-flags.ts index 91d55e1482d..cc58baf4ac1 100644 --- a/apps/sim/lib/core/config/feature-flags.ts +++ b/apps/sim/lib/core/config/feature-flags.ts @@ -79,7 +79,7 @@ const FEATURE_FLAGS = { }, 'api-key-read-only': { description: - 'Allow creating read-only API keys. Turn on only after every pod runs a release that enforces key access, since older pods treat a read-only key as full access. Before rolling back below the release that added read-only keys, turn this off first: that older code also serves every existing read-only key as full access. Enforcement of existing read-only keys never depends on this flag. Global; disabled by default.', + 'Allow creating read-only API keys. Turn on only after every pod runs a release that enforces key access, since older pods treat a read-only key as full access. Before rolling back below the release that added read-only keys, turn this off and revoke or replace every existing read-only key: turning the flag off only stops new ones, and that older code serves existing read-only keys as full access. Enforcement of existing read-only keys never depends on this flag. Global; disabled by default.', fallback: 'API_KEY_READ_ONLY', }, 'mothership-model-selector': { diff --git a/apps/sim/lib/core/utils/route-cache.ts b/apps/sim/lib/core/utils/route-cache.ts index a65adc6cdae..caf5ae636fb 100644 --- a/apps/sim/lib/core/utils/route-cache.ts +++ b/apps/sim/lib/core/utils/route-cache.ts @@ -1,5 +1,9 @@ import { readdir, readFile, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' + +const logger = createLogger('RouteCache') const BUILD_MARKER = '.build-id' @@ -7,7 +11,8 @@ const BUILD_MARKER = '.build-id' * Clears Next's route cache when it was written by a different build. Its keys carry no build * id, and the image declares the directory a volume, so a container recreated from a newer * image would otherwise serve pages the old build rendered, pointing at chunks it no longer - * ships. Never throws: a cache that cannot be read or cleared is left to Next. + * ships. Never throws: a cache that cannot be read or cleared is left to Next, with a warning + * unless there was simply no build id or cache to read. */ export async function clearRouteCacheFromOtherBuild(nextDirectory: string): Promise { const routeCache = join(nextDirectory, 'server', 'route-cache') @@ -21,5 +26,10 @@ export async function clearRouteCacheFromOtherBuild(nextDirectory: string): Prom entries.map((entry) => rm(join(routeCache, entry), { recursive: true, force: true })) ) await writeFile(join(routeCache, BUILD_MARKER), buildId) - } catch {} + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return + logger.warn('Could not clear the route cache from an earlier build', { + error: getErrorMessage(error), + }) + } } From 03e0b90649d29244e7b785f9b1c983bcf71d2d60 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:47:21 -0800 Subject: [PATCH 10/21] fix(credentials): release saved secrets only to internal-JWT callers The token route refused only session callers; any other auth type would have been handed a saved secret. Allow internal JWT and refuse the rest. Tools never execute in the browser (the registry boundary keeps the tool registry out of every workspace client graph, and nothing else imports the executor), so the browser-only token fetch through the route is deleted. --- apps/sim/lib/oauth/token-resolution.test.ts | 22 ++++ apps/sim/lib/oauth/token-resolution.ts | 4 +- apps/sim/tools/index.ts | 115 +++++--------------- 3 files changed, 51 insertions(+), 90 deletions(-) diff --git a/apps/sim/lib/oauth/token-resolution.test.ts b/apps/sim/lib/oauth/token-resolution.test.ts index 3607961ef1f..e55add6a175 100644 --- a/apps/sim/lib/oauth/token-resolution.test.ts +++ b/apps/sim/lib/oauth/token-resolution.test.ts @@ -457,6 +457,28 @@ describe('ordinary tool credential binding', () => { }) }) + it.each([ + ['a session', { success: true, userId: 'user-1', authType: 'session' }], + ['an API key', { success: true, userId: 'user-1', authType: 'api_key' }], + ] as const)( + 'releases a saved secret to server-side callers only, never %s', + async (_label, auth) => { + const result = await resolveCredentialAccessToken({ + requestId: 'binding-test', + credentialId: 'credential-1', + toolId: 'oracle_fusion_financials_list_payables_invoices', + authenticate: async () => auth, + }) + + expect(result).toMatchObject({ + ok: false, + status: 403, + code: 'SERVICE_ACCOUNT_SESSION_FORBIDDEN', + }) + expect(mockResolveServiceAccountToken).not.toHaveBeenCalled() + } + ) + it('refuses to release a service-account secret without a tool to bind it to', async () => { const result = await resolveCredentialAccessToken({ requestId: 'binding-test', diff --git a/apps/sim/lib/oauth/token-resolution.ts b/apps/sim/lib/oauth/token-resolution.ts index f044f954876..a48a073282f 100644 --- a/apps/sim/lib/oauth/token-resolution.ts +++ b/apps/sim/lib/oauth/token-resolution.ts @@ -356,8 +356,8 @@ export async function resolveCredentialToken( if (!authz.ok) { return { ok: false, status: 403, error: authz.error || 'Unauthorized' } } - // A saved secret never leaves the server; only in-process execution may resolve it. - if (auth.authType === AuthType.SESSION) { + // A saved secret never leaves the server: only server-side (internal JWT) callers may resolve it. + if (auth.authType !== AuthType.INTERNAL_JWT) { return { ok: false, status: 403, diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index bec4023e6cb..0c2de61564f 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -5,10 +5,7 @@ import { describeError, getErrorMessage, toError } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' import { isPlainRecord, isRecordLike, toRecord } from '@sim/utils/object' import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry' -import { ApiClientError } from '@/lib/api/client/errors' -import { requestJson } from '@/lib/api/client/request' import type { FunctionExecuteBody } from '@/lib/api/contracts' -import { oauthTokenPostContract } from '@/lib/api/contracts/oauth-connections' import { getBYOKKey } from '@/lib/api-key/byok' import type { InternalSandboxProfile } from '@/lib/auth/internal' import { @@ -1698,47 +1695,6 @@ function getPrivateToolMetadataPolicy(toolId: string): PrivateToolMetadataPolicy return undefined } -/** - * Resolves a credential token from the browser through `POST /api/auth/oauth/token`, - * authenticated by the session cookie. Server-side execution resolves in-process - * through `resolveExecutorCredentialToken` instead; this HTTP path exists only - * because the browser holds no server credentials. - */ -async function fetchCredentialTokenFromRoute(params: { - requestId: string - toolId: string - toolLabel: string - credentialId: string - workflowId?: string - impersonateEmail?: string - scopes?: string[] - callerUserId?: string -}): Promise { - const { requestId, toolId, toolLabel, credentialId, workflowId } = params - - try { - return await requestJson(oauthTokenPostContract, { - query: { userId: params.callerUserId }, - headers: {}, - body: { - credentialId, - toolId, - ...(workflowId ? { workflowId } : {}), - ...(params.impersonateEmail ? { impersonateEmail: params.impersonateEmail } : {}), - ...(params.scopes ? { scopes: params.scopes } : {}), - }, - }) - } catch (error: unknown) { - const status = error instanceof ApiClientError ? error.status : undefined - logger.error(`Token fetch failed for ${toolId}:`, { - requestId, - status, - error: getErrorMessage(error), - }) - throw new Error(`Failed to obtain credential for ${toolLabel}: ${getErrorMessage(error)}`) - } -} - /** * Runs private-provenance tools against an isolated registry. Unavailable authenticated lineage * marks the parent unknown without replacing the tool's functional result; malformed metadata is @@ -2065,50 +2021,33 @@ async function executeToolImplementation( const credentialTool = tool const resolveCredential = async (signal?: AbortSignal): Promise => { signal?.throwIfAborted() - let data: CredentialTokenPayload - if (typeof window === 'undefined') { - /** - * Dynamic import for the same client-bundle reason as the workflow_executor - * runner below: the resolver pulls the db/audit dependency graph, which must - * never enter the client-bundled tool registry. - */ - const { resolveExecutorCredentialToken } = await import( - '@/executor/utils/credential-token' - ) - data = await waitWithAbort( - resolveExecutorCredentialToken({ - requestId, - credentialId, - userId, - workflowId, - executionId: scope.executionId, - toolId, - toolLabel, - scopes: providerScopes, - impersonateEmail, - enforceCredentialAccess, - executorDelegationOrigin: executionContext?.executorDelegationOrigin, - ...(operationContext?.copilotToolExecution - ? { copilotExecutionContext: operationContext } - : {}), - }), - signal - ) - } else { - data = await waitWithAbort( - fetchCredentialTokenFromRoute({ - requestId, - toolId, - toolLabel, - credentialId, - workflowId, - impersonateEmail, - scopes: providerScopes, - callerUserId: userId && enforceCredentialAccess ? userId : undefined, - }), - signal - ) - } + /** + * Dynamic import for the same client-bundle reason as the workflow_executor + * runner below: the resolver pulls the db/audit dependency graph, which must + * never enter the client-bundled tool registry. + */ + const { resolveExecutorCredentialToken } = await import( + '@/executor/utils/credential-token' + ) + const data: CredentialTokenPayload = await waitWithAbort( + resolveExecutorCredentialToken({ + requestId, + credentialId, + userId, + workflowId, + executionId: scope.executionId, + toolId, + toolLabel, + scopes: providerScopes, + impersonateEmail, + enforceCredentialAccess, + executorDelegationOrigin: executionContext?.executorDelegationOrigin, + ...(operationContext?.copilotToolExecution + ? { copilotExecutionContext: operationContext } + : {}), + }), + signal + ) signal?.throwIfAborted() From 65dec861b697180d634c77c1c06dcfee9640c1a7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:49:27 -0800 Subject: [PATCH 11/21] fix(firecrawl): treat a blank maxDiscoveryDepth as unset A cleared field arrived as '' and was sent as 0, a crawl of the start page only. It now follows the blank maxDepth handling. --- apps/sim/tools/firecrawl/crawl.test.ts | 2 ++ apps/sim/tools/firecrawl/crawl.ts | 2 +- apps/sim/tools/firecrawl/types.ts | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/sim/tools/firecrawl/crawl.test.ts b/apps/sim/tools/firecrawl/crawl.test.ts index 2b342396d14..803783796db 100644 --- a/apps/sim/tools/firecrawl/crawl.test.ts +++ b/apps/sim/tools/firecrawl/crawl.test.ts @@ -106,10 +106,12 @@ describe('Firecrawl legacy crawl depth', () => { apiKey: 'test', url: 'https://example.com/docs', maxDepth: '', + maxDiscoveryDepth: ' ', } const body = crawlTool.request.body(blankDepth) if (!isPlainRecord(body)) throw new Error('Expected a JSON request body') expect(body.includePaths).toBeUndefined() expect(body.regexOnFullURL).toBeUndefined() + expect(body).not.toHaveProperty('maxDiscoveryDepth') }) }) diff --git a/apps/sim/tools/firecrawl/crawl.ts b/apps/sim/tools/firecrawl/crawl.ts index 110b4529b76..34b79f6985e 100644 --- a/apps/sim/tools/firecrawl/crawl.ts +++ b/apps/sim/tools/firecrawl/crawl.ts @@ -154,7 +154,7 @@ export const crawlTool: ToolConfig } if (params.prompt) body.prompt = params.prompt - if (params.maxDiscoveryDepth != null) + if (params.maxDiscoveryDepth != null && String(params.maxDiscoveryDepth).trim() !== '') body.maxDiscoveryDepth = Number(params.maxDiscoveryDepth) if (params.sitemap) body.sitemap = params.sitemap if (typeof params.crawlEntireDomain === 'boolean') diff --git a/apps/sim/tools/firecrawl/types.ts b/apps/sim/tools/firecrawl/types.ts index 571909aadf9..bfe0431cdc3 100644 --- a/apps/sim/tools/firecrawl/types.ts +++ b/apps/sim/tools/firecrawl/types.ts @@ -290,7 +290,8 @@ export interface FirecrawlCrawlParams { formats?: FirecrawlFormat[] onlyMainContent?: boolean prompt?: string - maxDiscoveryDepth?: number + /** Raw field text like {@link maxDepth}: a cleared field arrives as `''`. */ + maxDiscoveryDepth?: number | string sitemap?: 'skip' | 'include' crawlEntireDomain?: boolean allowExternalLinks?: boolean From 83695951b7e67ff816e29ad43d7adcc1044d5b2e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:49:27 -0800 Subject: [PATCH 12/21] chore(credentials): log a broken JWKS and keep the successor map out of credential display The JWKS route still answers 404 when the signing key is broken, but warns with the error. The GitLab personal-token display reads its block type from the catalog entry instead of importing the generated successor map into a client-imported module. --- apps/sim/lib/credentials/workload-identity/responses.ts | 9 ++++++++- apps/sim/lib/integrations/credential-display.ts | 3 +-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/credentials/workload-identity/responses.ts b/apps/sim/lib/credentials/workload-identity/responses.ts index e55e1f2611f..6dbc7138154 100644 --- a/apps/sim/lib/credentials/workload-identity/responses.ts +++ b/apps/sim/lib/credentials/workload-identity/responses.ts @@ -1,3 +1,4 @@ +import { createLogger } from '@sim/logger' import { NextResponse } from 'next/server' import { getWorkloadIdentityDiscoveryDocument, @@ -5,6 +6,8 @@ import { isWorkloadIdentityConfigured, } from '@/lib/credentials/workload-identity/issuer' +const logger = createLogger('WorkloadIdentityDiscovery') + /** * Cloud providers fetch these anonymously (AWS STS, Databricks) and * cache them; five minutes keeps a key rotation visible quickly. "No issuer @@ -28,7 +31,11 @@ export async function workloadIdentityDiscoveryResponse() { /** The issuer's public signing keys (JWKS), or 404 when this deployment has no usable signing key. */ export async function workloadIdentityJwksResponse() { - const jwks = await getWorkloadIdentityJwks().catch(() => null) + const jwks = await getWorkloadIdentityJwks().catch((error: unknown) => { + // Still a 404 to the provider, but a broken signing key must not look like an unconfigured one. + logger.warn('Could not build the workload identity JWKS', { error }) + return null + }) if (!jwks) return notConfigured() return NextResponse.json(jwks, { headers: DISCOVERY_HEADERS }) } diff --git a/apps/sim/lib/integrations/credential-display.ts b/apps/sim/lib/integrations/credential-display.ts index d7b4eb3d5a1..83d47e54bbc 100644 --- a/apps/sim/lib/integrations/credential-display.ts +++ b/apps/sim/lib/integrations/credential-display.ts @@ -22,7 +22,6 @@ import { CANONICAL_SERVICE_ACCOUNT_SLUGS } from '@/lib/integrations/oauth-servic import { OAUTH_PROVIDERS } from '@/lib/oauth/oauth' import type { OAuthProvider, OAuthServiceConfig } from '@/lib/oauth/types' import { getServiceConfigByProviderId, parseProvider } from '@/lib/oauth/utils' -import { resolveAccessControlBlockType } from '@/lib/permission-groups/integration-allowlist' /** * Above this many covered integrations the subtitle states a count instead of @@ -227,7 +226,7 @@ export function resolveCredentialDisplay(credential: DisplayableCredential): Cre return { service: null, integration, - blockType: resolveAccessControlBlockType('gitlab'), + blockType: integration?.type ?? 'gitlab', icon: GitlabIcon, familyName: null, coveredIntegrations: integration ? [integration] : [], From 9fcf4a2eb4bff1e4536ec7eb22e14648c238f8d9 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:49:28 -0800 Subject: [PATCH 13/21] test: drop or tighten assertions that could not fail - delegation: the read-only replacement case decided its filter by scanning bound params in a fake DB; no real-boundary check exists, so it is dropped with the fake's filter - Twilio: assert how many recordings requests were sent, so an empty loop cannot pass - Pi: the removed-model case restated the absent-from-catalog case --- .../lib/mothership/chat/delegation.test.ts | 35 ++++--------------- apps/sim/providers/pi-providers.test.ts | 11 ------ .../sim/tools/twilio_voice/list_calls.test.ts | 18 +++++----- 3 files changed, 17 insertions(+), 47 deletions(-) diff --git a/apps/sim/lib/mothership/chat/delegation.test.ts b/apps/sim/lib/mothership/chat/delegation.test.ts index b28278d50be..e66cad0909f 100644 --- a/apps/sim/lib/mothership/chat/delegation.test.ts +++ b/apps/sim/lib/mothership/chat/delegation.test.ts @@ -65,18 +65,13 @@ function database() { if (!locked) throw new Error('Read before acquiring the user lock') return { from: () => ({ - where: (condition: SQL) => { - const query = dialect.sqlToQuery(condition) - // Only the access filter binds 'write'; the other predicates bind ids, names and dates. - const writeOnly = query.params.includes('write') - return { - limit: async () => { - const row = rows.get(name) - if (!row || row.expiresAt.getTime() <= Date.now() + 30 * 60_000) return [] - return writeOnly && row.access === 'read' ? [] : [row] - }, - } - }, + where: () => ({ + limit: async () => { + const row = rows.get(name) + if (!row || row.expiresAt.getTime() <= Date.now() + 30 * 60_000) return [] + return [row] + }, + }), }), } }, @@ -161,22 +156,6 @@ describe('delegation key concurrency', () => { expect(db.rows.get('mothership-delegation:alice')).toBe(old) }) - it('replaces a read-only key instead of handing it to the CLI', async () => { - const db = database() - db.rows.set('mothership-delegation:actor', { - id: 'read-key', - name: 'mothership-delegation:actor', - key: 'encrypted-read-only', - expiresAt: new Date(Date.now() + 6 * 60 * 60_000), - access: 'read', - }) - expect(await mintDelegationToken({ workspaceId: 'shared', userId: 'actor' })).toBe('plain-1') - expect(db.rows.get('mothership-delegation:actor')).toMatchObject({ - key: 'encrypted-1', - access: 'write', - }) - }) - it('does not log bound credential values from database errors', async () => { mocks.transaction.mockRejectedValue( new Error('Failed query params: plaintext-key, encrypted-key') diff --git a/apps/sim/providers/pi-providers.test.ts b/apps/sim/providers/pi-providers.test.ts index 70dbeebcb1c..f18a1c4a9fa 100644 --- a/apps/sim/providers/pi-providers.test.ts +++ b/apps/sim/providers/pi-providers.test.ts @@ -32,17 +32,6 @@ describe('Pi provider catalog', () => { ) }) - it('rejects saved model IDs removed by Pi without substituting another model', () => { - expect(resolvePiModelId('anthropic', 'claude-opus-4-1')).toBeUndefined() - expect(resolvePiModelId('cerebras', 'cerebras/zai-glm-4.7')).toBeUndefined() - expect(resolvePiModelId('zai', 'glm-5.1')).toBeUndefined() - expect(resolvePiModelId('zai', 'glm-4.5-air')).toBeUndefined() - expect(resolvePiModelId('mistral', 'mistral-large-2411')).toBeUndefined() - expect(resolvePiModelId('mistral', 'devstral-small-2507')).toBeUndefined() - expect(resolvePiModelId('mistral', 'devstral-medium-2507')).toBeUndefined() - expect(resolvePiModelId('nvidia', 'nvidia/nemotron-3-super-120b-a12b')).toBeUndefined() - }) - it('rejects provider/model pairs absent from the installed Pi catalog', () => { expect(resolvePiModelId('anthropic', 'claude-sonnet-999')).toBeUndefined() expect(resolvePiModelId('zai', 'glm-4.5-air-unknown')).toBeUndefined() diff --git a/apps/sim/tools/twilio_voice/list_calls.test.ts b/apps/sim/tools/twilio_voice/list_calls.test.ts index 08547920cd7..30d3a54d76c 100644 --- a/apps/sim/tools/twilio_voice/list_calls.test.ts +++ b/apps/sim/tools/twilio_voice/list_calls.test.ts @@ -41,15 +41,17 @@ describe('Twilio list calls recordings lookup', () => { fetchMock.mockImplementation(async () => jsonResponse({ recordings: [{ sid: 'RE1' }] })) }) - it.each(['@evil.test/x', '//evil.test/x', 'https://evil.test/x'])( - 'never sends the saved credentials off the Twilio API origin for %s', - async (path) => { - await listCalls([path]) - for (const [url] of fetchMock.mock.calls) { - expect(new URL(String(url)).origin).toBe('https://api.twilio.com') - } + it.each([ + ['@evil.test/x', 1], + ['//evil.test/x', 0], + ['https://evil.test/x', 0], + ])('never sends the saved credentials off the Twilio API origin for %s', async (path, calls) => { + await listCalls([path]) + expect(fetchMock).toHaveBeenCalledTimes(calls) + for (const [url] of fetchMock.mock.calls) { + expect(new URL(String(url)).origin).toBe('https://api.twilio.com') } - ) + }) it('fetches recordings for Twilio-relative paths', async () => { const result = await listCalls(['/2010-04-01/Accounts/AC123/Calls/CA0/Recordings.json']) From d45b05d43b2a35f0d06f24d6388ab586ce5a93e1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:50:45 -0800 Subject: [PATCH 14/21] fix(security): ignore a malformed CIDR NO_PROXY entry 10.0.0.0/8/junk parsed as /8. An entry with extra segments is dropped like any other unparseable entry, and the TSDoc says IP entries match within their address family. --- apps/sim/lib/core/network/environment-proxy.server.test.ts | 6 ++++++ packages/security/src/environment-proxy.ts | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/core/network/environment-proxy.server.test.ts b/apps/sim/lib/core/network/environment-proxy.server.test.ts index 5f9d618b094..e554ff64563 100644 --- a/apps/sim/lib/core/network/environment-proxy.server.test.ts +++ b/apps/sim/lib/core/network/environment-proxy.server.test.ts @@ -389,6 +389,12 @@ describe('proxy selection from the environment', () => { target: 'https://11.0.0.1/', proxy: 'http://p:3128/', }, + { + case: 'a malformed CIDR NO_PROXY entry is ignored rather than read as its first prefix', + env: { HTTPS_PROXY: 'http://p:3128', NO_PROXY: '10.0.0.0/8/junk' }, + target: 'https://10.20.30.40/', + proxy: 'http://p:3128/', + }, { case: 'an IPv6 CIDR NO_PROXY entry excludes addresses inside the range', env: { HTTP_PROXY: 'http://p:3128', NO_PROXY: 'fd00::/8' }, diff --git a/packages/security/src/environment-proxy.ts b/packages/security/src/environment-proxy.ts index bfd9c6bbfa9..c02a1b75b74 100644 --- a/packages/security/src/environment-proxy.ts +++ b/packages/security/src/environment-proxy.ts @@ -41,8 +41,9 @@ function parseNoProxyEntry(entry: string): NoProxyRule | null { host = withPort[1] port = Number(withPort[2]) } - const [address, prefix] = host.split('/') + const [address, prefix, ...rest] = host.split('/') const family = ipFamily(address) + if (rest.length > 0) return null if (!family) return { kind: 'domain', domain: host.replace(/^\*?\./, '').toLowerCase(), port } const addresses = new BlockList() try { @@ -74,7 +75,7 @@ function noProxyRules(raw: string): NoProxyRule[] { * Whether `NO_PROXY` sends this destination direct, with curl's matching: `*` matches * everything; `example.com`, `.example.com` and `*.example.com` all match the domain and * every subdomain; `host:port` and `[v6]:port` match only that port; IP entries match any - * spelling of the address; and `10.0.0.0/8` or `fd00::/8` match IP-literal destinations in + * spelling of the address in the same address family; and `10.0.0.0/8` or `fd00::/8` match IP-literal destinations in * the range (never a hostname, which is not resolved here). */ function isExcludedByNoProxy(hostname: string, port: number): boolean { From e1b532ea14234ee0ccfd75a894b47fc812a917fd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:50:45 -0800 Subject: [PATCH 15/21] chore(workflows): scope the paused-row read to its workflow and reuse the mutability guard - the execution-status paused-row read filters by workflowId as well - delete-workflow uses requireMutableWorkflow instead of an inline copy - polling webhook logs carry the webhook id in metadata --- apps/sim/lib/webhooks/polling/utils.ts | 17 ++++++++++------- .../workflows/application/delete-workflow.ts | 12 ++---------- .../lib/workflows/executor/execution-status.ts | 7 ++++++- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/apps/sim/lib/webhooks/polling/utils.ts b/apps/sim/lib/webhooks/polling/utils.ts index 519af916bea..46ceec0ff9f 100644 --- a/apps/sim/lib/webhooks/polling/utils.ts +++ b/apps/sim/lib/webhooks/polling/utils.ts @@ -64,7 +64,9 @@ export function throwIfAdmissionRefused(result: PolledWebhookEventResult): void /** Logs a poll stopped by {@link PollAdmissionRefusedError} and reports it as skipped. */ export function skipAdmissionRefusedPoll(logger: Logger, webhookId: string): 'skipped' { - logger.info(`Execution admission refused webhook ${webhookId}; left its items for a later poll`) + logger.info('Execution admission refused the webhook; left its items for a later poll', { + webhookId, + }) return 'skipped' } @@ -175,12 +177,13 @@ export async function markWebhookFailed(webhookId: string, logger: Logger): Prom .returning({ isActive: webhook.isActive, wasActive: prior.wasActive }) if (result?.wasActive && !result.isActive) { - logger.warn( - `Webhook ${webhookId} auto-disabled after ${MAX_CONSECUTIVE_FAILURES} consecutive failures` - ) + logger.warn('Webhook auto-disabled after consecutive failures', { + webhookId, + consecutiveFailures: MAX_CONSECUTIVE_FAILURES, + }) } } catch (err) { - logger.error(`Failed to mark webhook ${webhookId} as failed:`, err) + logger.error('Failed to mark webhook as failed', { webhookId, error: err }) } } @@ -197,7 +200,7 @@ export async function markWebhookSuccess(webhookId: string, logger: Logger): Pro and(eq(webhook.id, webhookId), or(isNull(webhook.failedCount), ne(webhook.failedCount, 0))) ) } catch (err) { - logger.error(`Failed to mark webhook ${webhookId} as successful:`, err) + logger.error('Failed to mark webhook as successful', { webhookId, error: err }) } } @@ -322,7 +325,7 @@ export async function updateWebhookProviderConfig( }) .where(eq(webhook.id, webhookId)) } catch (err) { - logger.error(`Failed to update webhook ${webhookId} config:`, err) + logger.error('Failed to update webhook config', { webhookId, error: err }) } } diff --git a/apps/sim/lib/workflows/application/delete-workflow.ts b/apps/sim/lib/workflows/application/delete-workflow.ts index 08c24205b7a..e2ab6f3bb60 100644 --- a/apps/sim/lib/workflows/application/delete-workflow.ts +++ b/apps/sim/lib/workflows/application/delete-workflow.ts @@ -1,10 +1,9 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { resolvePrincipalAttribution } from '@sim/auth/principal' import { createLogger } from '@sim/logger' -import { assertWorkflowMutable, WorkflowLockedError } from '@sim/platform-authz/workflow' -import { OrchestrationError } from '@/lib/core/orchestration/types' import { notifyWorkflowDeleted, notifyWorkspaceWorkflowsChanged } from '@/lib/realtime/notify' import { defineAuthorizedWorkflowUseCase } from '@/lib/workflows/application/authorized-workflow-use-case' +import { requireMutableWorkflow } from '@/lib/workflows/application/mutability' import { workflowOperations } from '@/lib/workflows/application/operations' import { resolvePrincipalWorkflowContext } from '@/lib/workflows/application/principal-scope' import { requireWorkflowTransition } from '@/lib/workflows/application/transition-result' @@ -21,14 +20,7 @@ export const deleteWorkflow = defineAuthorizedWorkflowUseCase({ operation: workflowOperations.delete, resolveContext: resolvePrincipalWorkflowContext, async execute({ principal, context }) { - try { - await assertWorkflowMutable(context.workflowId) - } catch (error) { - if (error instanceof WorkflowLockedError) { - throw new OrchestrationError('locked', error.message) - } - throw error - } + await requireMutableWorkflow(context.workflowId) const transition = await deleteWorkflowRecord({ workflowId: context.workflowId, diff --git a/apps/sim/lib/workflows/executor/execution-status.ts b/apps/sim/lib/workflows/executor/execution-status.ts index 40a1bda292b..11cee921d2b 100644 --- a/apps/sim/lib/workflows/executor/execution-status.ts +++ b/apps/sim/lib/workflows/executor/execution-status.ts @@ -493,7 +493,12 @@ async function readWorkflowExecutionStatus( nextResumeAt: pausedExecutions.nextResumeAt, }) .from(pausedExecutions) - .where(eq(pausedExecutions.executionId, executionId)) + .where( + and( + eq(pausedExecutions.executionId, executionId), + eq(pausedExecutions.workflowId, workflowId) + ) + ) .limit(1) const isCurrentlyPaused = From eb276fbf2236d0f2597062316220c85371fada65 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:52:26 -0800 Subject: [PATCH 16/21] fix(credentials): never name a sibling credential in a trust-setup refusal The caller may not be able to see the credential that holds or blocks the subject, so both refusals name only the workspace or organization. The uniqueness check documents its limits. --- ...orkload-identity-subject-reuse.integration.ts | 4 +++- .../lib/credentials/workload-identity/subject.ts | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/credentials/__integration__/workload-identity-subject-reuse.integration.ts b/apps/sim/lib/credentials/__integration__/workload-identity-subject-reuse.integration.ts index e06a15cc191..5108985d368 100644 --- a/apps/sim/lib/credentials/__integration__/workload-identity-subject-reuse.integration.ts +++ b/apps/sim/lib/credentials/__integration__/workload-identity-subject-reuse.integration.ts @@ -234,7 +234,9 @@ describe('reusing a keyless trust setup', () => { const result = await create(`Blocked by ${unreadableId}`, keyless(grant())) expect(result).toMatchObject({ success: false, errorCode: 'conflict' }) - expect(result.error).toContain('Unreadable sibling') + // The caller may not be able to see the sibling, so the refusal never names it. + expect(result.error).not.toContain('Unreadable sibling') + expect(result.error).toContain('this workspace') } finally { await db.delete(credential).where(eq(credential.id, unreadableId)) } diff --git a/apps/sim/lib/credentials/workload-identity/subject.ts b/apps/sim/lib/credentials/workload-identity/subject.ts index 63294be40f5..1bb5120f384 100644 --- a/apps/sim/lib/credentials/workload-identity/subject.ts +++ b/apps/sim/lib/credentials/workload-identity/subject.ts @@ -205,6 +205,11 @@ function lockSubject(tx: DbTransaction, subject: string): Promise { ) } +/** Names the owner without naming a credential, which the caller may not be able to see. */ +function scopeNoun(scope: ResourceScope): string { + return scope.kind === 'workspace' ? 'this workspace' : 'this organization' +} + /** Throws unless no live keyless credential of the owner other than `credentialId` holds `subject`. */ async function assertNoOtherCredentialHolds( tx: DbTransaction, @@ -213,7 +218,6 @@ async function assertNoOtherCredentialHolds( const rows = await tx .select({ id: credential.id, - displayName: credential.displayName, encryptedServiceAccountKey: credential.encryptedServiceAccountKey, }) .from(credential) @@ -233,12 +237,12 @@ async function assertNoOtherCredentialHolds( credentialId: row.id, }) throw new WorkloadIdentitySubjectError( - `Sim could not read the saved credential "${row.displayName}", so it cannot confirm this trust setup is unused. Reconnect or delete that credential, then try again.` + `Sim could not read another keyless credential in ${scopeNoun(input.scope)}, so it cannot confirm this trust setup is unused. Ask an admin to reconnect or delete the broken keyless credential, then try again.` ) } if (storedSubject === input.subject) { throw new WorkloadIdentitySubjectError( - 'Another credential already uses this trust setup. Start the keyless connection again.' + `Another credential in ${scopeNoun(input.scope)} already uses this trust setup. Start the keyless connection again.` ) } } @@ -256,6 +260,12 @@ async function assertNoOtherCredentialHolds( * credential: the advisory lock holds concurrent saves of one subject until * commit, so the later save sees the earlier row. A sibling whose secret cannot * be read might hold the subject, so it refuses the save rather than skip it. + * + * Caveats: uniqueness holds only within one owner (a subject embeds its owner, + * so another owner's credential cannot hold it), only among live AWS and + * Databricks keyless credentials, and only for writers that call this inside + * their write transaction. The refusal never names the sibling, which the + * caller may not be able to see. */ export async function assertWorkloadIdentitySubjectUnclaimed( tx: DbTransaction, From 3d3a9c90529f4b0f1ac44fd04b2a46c5bef36c82 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:52:27 -0800 Subject: [PATCH 17/21] docs: correct the Helm readOnlyRootFilesystem note and the deployed-run variables TSDoc --- apps/sim/lib/workflows/persistence/utils.ts | 5 +++-- helm/sim/README.md | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts index 7aa642764d9..d9843851548 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -106,9 +106,10 @@ export interface DeployedWorkflowData extends NormalizedWorkflowData { } /** - * The variables a queued deployed run (async API, schedule, webhook) executes with: the - * deployed version's own, or the workflow's saved variables for a version deployed before + * The variables an unpinned queued deployed run (async API, schedule, webhook) executes with: + * the deployed version's own, or the workflow's saved variables for a version deployed before * deployments snapshotted them, which is what those runs have always used for such a version. + * A run pinned to a version reads that version's variables directly instead. */ export function resolveDeployedRunVariables( deployedState: DeployedWorkflowData, diff --git a/helm/sim/README.md b/helm/sim/README.md index a534b3a84e3..556731ae42f 100644 --- a/helm/sim/README.md +++ b/helm/sim/README.md @@ -340,7 +340,7 @@ Before installing in production, confirm each of the following: matchLabels: kubernetes.io/metadata.name: ingress-nginx ``` -* **Namespace hardening** — label the install namespace with Pod Security Standards `restricted` enforcement (`pod-security.kubernetes.io/enforce=restricted`). All workloads set `runAsNonRoot`, drop all Linux capabilities, disable privilege escalation, and set `seccompProfile: RuntimeDefault` — the four controls the Restricted profile requires. `readOnlyRootFilesystem` is intentionally **not** defaulted anywhere (Postgres/Ollama genuinely need a writable root; the stateless services — `realtime`, `pii`, `copilot` — could tolerate it but aren't pre-wired with a `/tmp` `emptyDir`). If your policy requires it, set `.securityContext.readOnlyRootFilesystem: true` and mount writable `emptyDir` volumes yourself via `extraVolumes`/`extraVolumeMounts`: `/tmp` for every component, plus `/app/apps/sim/.next/cache` and `/app/apps/sim/.next/server/route-cache` for `app`. +* **Namespace hardening** — label the install namespace with Pod Security Standards `restricted` enforcement (`pod-security.kubernetes.io/enforce=restricted`). All workloads set `runAsNonRoot`, drop all Linux capabilities, disable privilege escalation, and set `seccompProfile: RuntimeDefault` — the four controls the Restricted profile requires. `readOnlyRootFilesystem` is defaulted on only for the bundled Redis (Postgres/Ollama genuinely need a writable root; the stateless services — `realtime`, `pii`, `copilot` — could tolerate it but aren't pre-wired with a `/tmp` `emptyDir`). If your policy requires it, set `.securityContext.readOnlyRootFilesystem: true` and mount writable `emptyDir` volumes yourself via `extraVolumes`/`extraVolumeMounts`: `/tmp` for every component, plus `/app/apps/sim/.next/cache` and `/app/apps/sim/.next/server/route-cache` for `app`. * **Env validation** — keys under `app.env`, `realtime.env`, and `copilot.server.env` are passed through to the application and validated at startup. The JSON Schema intentionally does not enforce `additionalProperties: false` (would break custom user envs), so typos like `OPENA_API_KEY` (instead of `OPENAI_API_KEY`) surface as missing-key errors at runtime, not at `helm install` time. Review your env block carefully. * **Set public URLs** — `app.env.NEXT_PUBLIC_APP_URL` and `app.env.BETTER_AUTH_URL` must match your public origin (e.g. `https://sim.example.com`). Leaving them as `localhost` breaks sign-in. From 36450c5dd4111c4a29c0a1686bf1654e07a4d424 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:53:15 -0800 Subject: [PATCH 18/21] test(credentials): rely on the shared env reset in the proxy validator tests --- .../token-service-accounts/validators/http.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts index 317bd715fd7..0e9ba7a0fd5 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts @@ -3,7 +3,7 @@ import { inputValidationMock, inputValidationMockFns, } from '@sim/testing/mocks/input-validation.mock' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockResolveHost } = vi.hoisted(() => ({ mockResolveHost: vi.fn() })) @@ -217,10 +217,6 @@ describe('tokenCheckValidator with a typed host', () => { }) describe('behind an environment proxy', () => { - afterEach(() => { - vi.unstubAllEnvs() - }) - it('refuses a host with a private record alongside its public one before sending the token', async () => { vi.stubEnv('HTTPS_PROXY', 'http://proxy.internal:3128') const actual = await vi.importActual( From 5a845484de1ba06374a8a987458f4d49e2b4c72f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 16:58:50 -0800 Subject: [PATCH 19/21] chore(helm): bump the chart version for the README correction --- helm/sim/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/sim/Chart.yaml b/helm/sim/Chart.yaml index 5862d9a3e93..60d359fe440 100644 --- a/helm/sim/Chart.yaml +++ b/helm/sim/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: sim description: A Helm chart for Sim - the open-source AI workspace where teams build, deploy, and manage AI agents type: application -version: 1.11.13 +version: 1.11.14 appVersion: "v0.8.26" kubeVersion: ">=1.25.0-0" home: https://sim.ai From 397444ba6f5d5927ebc5baa50d49913016a83475 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 17:02:37 -0800 Subject: [PATCH 20/21] fix(redaction): keep prefixed secret keys masked while leaving Infisical's bare secretKey visible --- apps/sim/lib/core/security/redaction.test.ts | 4 ++++ apps/sim/lib/core/security/redaction.ts | 7 +++++-- .../credentials/token-service-accounts/validators/http.ts | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/core/security/redaction.test.ts b/apps/sim/lib/core/security/redaction.test.ts index 07a562e9a5b..2de2123fc9c 100644 --- a/apps/sim/lib/core/security/redaction.test.ts +++ b/apps/sim/lib/core/security/redaction.test.ts @@ -31,6 +31,10 @@ describe('isSensitiveKey', () => { 'exa_api_key', 'secretAccessKey', 'awsSecretAccessKey', + 'bedrockSecretKey', + 'apiSecretKey', + 'clerkSecretKey', + 'stripe_secret_key', 'deployKey', 'convexDeployKey', `${'a'.repeat(10000)}password`, diff --git a/apps/sim/lib/core/security/redaction.ts b/apps/sim/lib/core/security/redaction.ts index 635774fc85c..c86d47b506d 100644 --- a/apps/sim/lib/core/security/redaction.ts +++ b/apps/sim/lib/core/security/redaction.ts @@ -29,8 +29,11 @@ const SENSITIVE_KEY_PATTERNS: RegExp[] = [ /^.*api[_-]?key$/i, // AWS's `secretAccessKey` / `awsSecretAccessKey`, which no suffix above covers. /^.*secret[_-]?access[_-]?key$/i, - // Convex's `deployKey`. A bare `secretKey` is left out: Infisical returns each - // secret's name in that field, and redacting it would hide every listed name. + // Prefixed secret keys such as Bedrock's `bedrockSecretKey` and Clerk's `clerkSecretKey`. + // The prefix is required: Infisical returns each secret's name in a bare `secretKey` + // field, and redacting it would hide every listed name. + /^.+secret[_-]?key$/i, + // Convex's `deployKey`. /^.*deploy[_-]?key$/i, /^passphrase$/i, /^authorization$/i, diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/http.ts b/apps/sim/lib/credentials/token-service-accounts/validators/http.ts index c7b652e6554..dd96bb75d21 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/http.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/http.ts @@ -184,9 +184,10 @@ async function sendThroughEgress( { url, method = 'GET', headers, body }: TokenCheckRequest ): Promise { // A forward proxy re-resolves the host, so the transport refuses one with any refused record. + // An unparseable URL skips the proxy lookup; validation then refuses it as the user's input. const validation = await validateUrlWithDNS(url, 'url', profile, { logDetails: false, - requireEveryAddress: resolveEnvironmentProxy(new URL(url)) !== null, + requireEveryAddress: URL.canParse(url) && resolveEnvironmentProxy(new URL(url)) !== null, }) if (!validation.isValid) { if (validation.refusal !== 'dns') throw hostNotAllowed() From 17cf2df4d5eb711e3457347adfaaa4b3340f2b49 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 10 Nov 2026 17:11:13 -0800 Subject: [PATCH 21/21] fix(release): keep bare secretKey redacted, leave the proxy every-address rule to the transport, and report unknown GitLab hosts as input errors --- apps/sim/lib/core/security/redaction.test.ts | 2 +- apps/sim/lib/core/security/redaction.ts | 8 +++--- apps/sim/lib/core/utils/route-cache.ts | 2 +- .../credentials/gitlab-personal-token.test.ts | 13 ++++++++++ .../lib/credentials/gitlab-personal-token.ts | 7 ++++++ .../validators/gitlab.test.ts | 1 + .../validators/http.test.ts | 25 ------------------- .../token-service-accounts/validators/http.ts | 10 +++----- 8 files changed, 30 insertions(+), 38 deletions(-) diff --git a/apps/sim/lib/core/security/redaction.test.ts b/apps/sim/lib/core/security/redaction.test.ts index 2de2123fc9c..04058e34b91 100644 --- a/apps/sim/lib/core/security/redaction.test.ts +++ b/apps/sim/lib/core/security/redaction.test.ts @@ -31,6 +31,7 @@ describe('isSensitiveKey', () => { 'exa_api_key', 'secretAccessKey', 'awsSecretAccessKey', + 'secretKey', 'bedrockSecretKey', 'apiSecretKey', 'clerkSecretKey', @@ -45,7 +46,6 @@ describe('isSensitiveKey', () => { it.concurrent.each([ 'tokenCount', 'tokenizer', - 'secretKey', 'deployKeyName', 'passwordStrength', 'authMethod', diff --git a/apps/sim/lib/core/security/redaction.ts b/apps/sim/lib/core/security/redaction.ts index c86d47b506d..baa661c4234 100644 --- a/apps/sim/lib/core/security/redaction.ts +++ b/apps/sim/lib/core/security/redaction.ts @@ -29,10 +29,10 @@ const SENSITIVE_KEY_PATTERNS: RegExp[] = [ /^.*api[_-]?key$/i, // AWS's `secretAccessKey` / `awsSecretAccessKey`, which no suffix above covers. /^.*secret[_-]?access[_-]?key$/i, - // Prefixed secret keys such as Bedrock's `bedrockSecretKey` and Clerk's `clerkSecretKey`. - // The prefix is required: Infisical returns each secret's name in a bare `secretKey` - // field, and redacting it would hide every listed name. - /^.+secret[_-]?key$/i, + // Secret keys, bare or prefixed: Clerk and Amplitude take their credential in a bare + // `secretKey`, Bedrock in `bedrockSecretKey`. This also masks the secret names Infisical + // returns under `secretKey` in display copies; a hidden name is safer than a leaked key. + /^.*secret[_-]?key$/i, // Convex's `deployKey`. /^.*deploy[_-]?key$/i, /^passphrase$/i, diff --git a/apps/sim/lib/core/utils/route-cache.ts b/apps/sim/lib/core/utils/route-cache.ts index caf5ae636fb..0affa2c5eb4 100644 --- a/apps/sim/lib/core/utils/route-cache.ts +++ b/apps/sim/lib/core/utils/route-cache.ts @@ -28,7 +28,7 @@ export async function clearRouteCacheFromOtherBuild(nextDirectory: string): Prom await writeFile(join(routeCache, BUILD_MARKER), buildId) } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return - logger.warn('Could not clear the route cache from an earlier build', { + logger.error('Could not clear the route cache from an earlier build', { error: getErrorMessage(error), }) } diff --git a/apps/sim/lib/credentials/gitlab-personal-token.test.ts b/apps/sim/lib/credentials/gitlab-personal-token.test.ts index 4230b6e4d6c..825d9851022 100644 --- a/apps/sim/lib/credentials/gitlab-personal-token.test.ts +++ b/apps/sim/lib/credentials/gitlab-personal-token.test.ts @@ -106,6 +106,19 @@ describe('GitLab personal token verification', () => { message: expect.stringContaining("can't connect to that address"), }) }) + it('reports a GitLab host that does not resolve as the user input', async () => { + mocks.fetch.mockReset().mockRejectedValue( + Object.assign(new Error('getaddrinfo ENOTFOUND gitlab.example.invalid'), { + code: 'ENOTFOUND', + }) + ) + await expect( + verifyGitLabPersonalToken('secret', 'gitlab.example.invalid') + ).rejects.toMatchObject({ + code: 'validation', + message: expect.stringContaining("couldn't find that GitLab host"), + }) + }) it('binds organization tokens independently of the execution workspace', async () => { mocks.encrypt.mockResolvedValue({ encrypted: 'ciphertext' }) const { workspaceId: _workspaceId, ...identity } = envelope diff --git a/apps/sim/lib/credentials/gitlab-personal-token.ts b/apps/sim/lib/credentials/gitlab-personal-token.ts index e655c0fca82..a0ee529c7b0 100644 --- a/apps/sim/lib/credentials/gitlab-personal-token.ts +++ b/apps/sim/lib/credentials/gitlab-personal-token.ts @@ -4,6 +4,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types' import { resourceScopeFromOwner, sameResourceScope } from '@/lib/core/resource-scope' import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' import { secureFetchWithValidation } from '@/lib/core/security/input-validation.server' +import { isHostNotFound } from '@/lib/credentials/token-service-accounts/errors' import { normalizeGitLabHost } from '@/tools/gitlab/utils' const gitLabUserSchema = z.object({ @@ -63,6 +64,12 @@ export async function verifyGitLabPersonalToken(accessToken: string, rawHost?: s "Sim can't connect to that address. Use your GitLab instance's public https URL." ) } + if (isHostNotFound(error)) { + throw new OrchestrationError( + 'validation', + "Sim couldn't find that GitLab host. Check the address and try again." + ) + } throw error }) if (!response.ok) { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/gitlab.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/gitlab.test.ts index 7889c7b562b..409bbc85b50 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/gitlab.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/gitlab.test.ts @@ -77,6 +77,7 @@ describe('GitLab saved token verification', () => { validateUrl.mockResolvedValue({ isValid: false, error: 'url hostname could not be resolved', + refusal: 'dns', cause: Object.assign(new Error('getaddrinfo ENOTFOUND'), { code: 'ENOTFOUND' }), }) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts index 0e9ba7a0fd5..6fad7b593e9 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts @@ -5,16 +5,9 @@ import { } from '@sim/testing/mocks/input-validation.mock' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockResolveHost } = vi.hoisted(() => ({ mockResolveHost: vi.fn() })) - vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) -vi.mock('@sim/security/dns', async (importOriginal) => ({ - ...(await importOriginal()), - resolveHostAddresses: mockResolveHost, -})) import { EgressRefusedError } from '@/lib/core/network/transport.server' -import type * as InputValidation from '@/lib/core/security/input-validation.server' import { normalizeHttpBaseUrl, refuseUnsafeTypedUrl, @@ -217,24 +210,6 @@ describe('tokenCheckValidator with a typed host', () => { }) describe('behind an environment proxy', () => { - it('refuses a host with a private record alongside its public one before sending the token', async () => { - vi.stubEnv('HTTPS_PROXY', 'http://proxy.internal:3128') - const actual = await vi.importActual( - '@/lib/core/security/input-validation.server' - ) - validateUrl.mockImplementation(actual.validateUrlWithDNS) - mockResolveHost.mockResolvedValue({ - addresses: ['93.184.216.34', '10.0.0.5'], - preferred: '93.184.216.34', - }) - pinnedFetch.mockResolvedValue(jsonResponse({ ok: true })) - - await expect( - validateHost({ apiToken: 'good', domain: 'split.example.com' }) - ).rejects.toMatchObject({ code: 'host_not_allowed', status: 400 }) - expect(pinnedFetch).not.toHaveBeenCalled() - }) - it("reports the proxy route's own refusal as the host, not an outage", async () => { pinnedFetch.mockRejectedValue(new EgressRefusedError('url resolves to a private address')) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/http.ts b/apps/sim/lib/credentials/token-service-accounts/validators/http.ts index dd96bb75d21..e31c2be069e 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/http.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/http.ts @@ -1,4 +1,3 @@ -import { resolveEnvironmentProxy } from '@sim/security/environment-proxy' import { truncate } from '@sim/utils/string' import { EgressRefusedError } from '@/lib/core/network/transport.server' import type { EgressProfile } from '@/lib/core/security/egress/profiles' @@ -183,12 +182,9 @@ async function sendThroughEgress( profile: NonNullable, { url, method = 'GET', headers, body }: TokenCheckRequest ): Promise { - // A forward proxy re-resolves the host, so the transport refuses one with any refused record. - // An unparseable URL skips the proxy lookup; validation then refuses it as the user's input. - const validation = await validateUrlWithDNS(url, 'url', profile, { - logDetails: false, - requireEveryAddress: URL.canParse(url) && resolveEnvironmentProxy(new URL(url)) !== null, - }) + // The transport applies the stricter every-address rule itself when it routes through an + // environment proxy; its refusal is classified below, so a gateway route is not pre-judged. + const validation = await validateUrlWithDNS(url, 'url', profile, { logDetails: false }) if (!validation.isValid) { if (validation.refusal !== 'dns') throw hostNotAllowed() const notFound = isHostNotFound(validation.cause)