Skip to content

Commit 03e0b90

Browse files
committed
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.
1 parent f0c91fa commit 03e0b90

3 files changed

Lines changed: 51 additions & 90 deletions

File tree

‎apps/sim/lib/oauth/token-resolution.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,28 @@ describe('ordinary tool credential binding', () => {
457457
})
458458
})
459459

460+
it.each([
461+
['a session', { success: true, userId: 'user-1', authType: 'session' }],
462+
['an API key', { success: true, userId: 'user-1', authType: 'api_key' }],
463+
] as const)(
464+
'releases a saved secret to server-side callers only, never %s',
465+
async (_label, auth) => {
466+
const result = await resolveCredentialAccessToken({
467+
requestId: 'binding-test',
468+
credentialId: 'credential-1',
469+
toolId: 'oracle_fusion_financials_list_payables_invoices',
470+
authenticate: async () => auth,
471+
})
472+
473+
expect(result).toMatchObject({
474+
ok: false,
475+
status: 403,
476+
code: 'SERVICE_ACCOUNT_SESSION_FORBIDDEN',
477+
})
478+
expect(mockResolveServiceAccountToken).not.toHaveBeenCalled()
479+
}
480+
)
481+
460482
it('refuses to release a service-account secret without a tool to bind it to', async () => {
461483
const result = await resolveCredentialAccessToken({
462484
requestId: 'binding-test',

‎apps/sim/lib/oauth/token-resolution.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,8 @@ export async function resolveCredentialToken(
356356
if (!authz.ok) {
357357
return { ok: false, status: 403, error: authz.error || 'Unauthorized' }
358358
}
359-
// A saved secret never leaves the server; only in-process execution may resolve it.
360-
if (auth.authType === AuthType.SESSION) {
359+
// A saved secret never leaves the server: only server-side (internal JWT) callers may resolve it.
360+
if (auth.authType !== AuthType.INTERNAL_JWT) {
361361
return {
362362
ok: false,
363363
status: 403,

‎apps/sim/tools/index.ts‎

Lines changed: 27 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ import { describeError, getErrorMessage, toError } from '@sim/utils/errors'
55
import { sleep } from '@sim/utils/helpers'
66
import { isPlainRecord, isRecordLike, toRecord } from '@sim/utils/object'
77
import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry'
8-
import { ApiClientError } from '@/lib/api/client/errors'
9-
import { requestJson } from '@/lib/api/client/request'
108
import type { FunctionExecuteBody } from '@/lib/api/contracts'
11-
import { oauthTokenPostContract } from '@/lib/api/contracts/oauth-connections'
129
import { getBYOKKey } from '@/lib/api-key/byok'
1310
import type { InternalSandboxProfile } from '@/lib/auth/internal'
1411
import {
@@ -1698,47 +1695,6 @@ function getPrivateToolMetadataPolicy(toolId: string): PrivateToolMetadataPolicy
16981695
return undefined
16991696
}
17001697

1701-
/**
1702-
* Resolves a credential token from the browser through `POST /api/auth/oauth/token`,
1703-
* authenticated by the session cookie. Server-side execution resolves in-process
1704-
* through `resolveExecutorCredentialToken` instead; this HTTP path exists only
1705-
* because the browser holds no server credentials.
1706-
*/
1707-
async function fetchCredentialTokenFromRoute(params: {
1708-
requestId: string
1709-
toolId: string
1710-
toolLabel: string
1711-
credentialId: string
1712-
workflowId?: string
1713-
impersonateEmail?: string
1714-
scopes?: string[]
1715-
callerUserId?: string
1716-
}): Promise<CredentialTokenPayload> {
1717-
const { requestId, toolId, toolLabel, credentialId, workflowId } = params
1718-
1719-
try {
1720-
return await requestJson(oauthTokenPostContract, {
1721-
query: { userId: params.callerUserId },
1722-
headers: {},
1723-
body: {
1724-
credentialId,
1725-
toolId,
1726-
...(workflowId ? { workflowId } : {}),
1727-
...(params.impersonateEmail ? { impersonateEmail: params.impersonateEmail } : {}),
1728-
...(params.scopes ? { scopes: params.scopes } : {}),
1729-
},
1730-
})
1731-
} catch (error: unknown) {
1732-
const status = error instanceof ApiClientError ? error.status : undefined
1733-
logger.error(`Token fetch failed for ${toolId}:`, {
1734-
requestId,
1735-
status,
1736-
error: getErrorMessage(error),
1737-
})
1738-
throw new Error(`Failed to obtain credential for ${toolLabel}: ${getErrorMessage(error)}`)
1739-
}
1740-
}
1741-
17421698
/**
17431699
* Runs private-provenance tools against an isolated registry. Unavailable authenticated lineage
17441700
* marks the parent unknown without replacing the tool's functional result; malformed metadata is
@@ -2065,50 +2021,33 @@ async function executeToolImplementation(
20652021
const credentialTool = tool
20662022
const resolveCredential = async (signal?: AbortSignal): Promise<void> => {
20672023
signal?.throwIfAborted()
2068-
let data: CredentialTokenPayload
2069-
if (typeof window === 'undefined') {
2070-
/**
2071-
* Dynamic import for the same client-bundle reason as the workflow_executor
2072-
* runner below: the resolver pulls the db/audit dependency graph, which must
2073-
* never enter the client-bundled tool registry.
2074-
*/
2075-
const { resolveExecutorCredentialToken } = await import(
2076-
'@/executor/utils/credential-token'
2077-
)
2078-
data = await waitWithAbort(
2079-
resolveExecutorCredentialToken({
2080-
requestId,
2081-
credentialId,
2082-
userId,
2083-
workflowId,
2084-
executionId: scope.executionId,
2085-
toolId,
2086-
toolLabel,
2087-
scopes: providerScopes,
2088-
impersonateEmail,
2089-
enforceCredentialAccess,
2090-
executorDelegationOrigin: executionContext?.executorDelegationOrigin,
2091-
...(operationContext?.copilotToolExecution
2092-
? { copilotExecutionContext: operationContext }
2093-
: {}),
2094-
}),
2095-
signal
2096-
)
2097-
} else {
2098-
data = await waitWithAbort(
2099-
fetchCredentialTokenFromRoute({
2100-
requestId,
2101-
toolId,
2102-
toolLabel,
2103-
credentialId,
2104-
workflowId,
2105-
impersonateEmail,
2106-
scopes: providerScopes,
2107-
callerUserId: userId && enforceCredentialAccess ? userId : undefined,
2108-
}),
2109-
signal
2110-
)
2111-
}
2024+
/**
2025+
* Dynamic import for the same client-bundle reason as the workflow_executor
2026+
* runner below: the resolver pulls the db/audit dependency graph, which must
2027+
* never enter the client-bundled tool registry.
2028+
*/
2029+
const { resolveExecutorCredentialToken } = await import(
2030+
'@/executor/utils/credential-token'
2031+
)
2032+
const data: CredentialTokenPayload = await waitWithAbort(
2033+
resolveExecutorCredentialToken({
2034+
requestId,
2035+
credentialId,
2036+
userId,
2037+
workflowId,
2038+
executionId: scope.executionId,
2039+
toolId,
2040+
toolLabel,
2041+
scopes: providerScopes,
2042+
impersonateEmail,
2043+
enforceCredentialAccess,
2044+
executorDelegationOrigin: executionContext?.executorDelegationOrigin,
2045+
...(operationContext?.copilotToolExecution
2046+
? { copilotExecutionContext: operationContext }
2047+
: {}),
2048+
}),
2049+
signal
2050+
)
21122051

21132052
signal?.throwIfAborted()
21142053

0 commit comments

Comments
 (0)