Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/sim/lib/copilot/tool-executor/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ describe('copilot tool executor fallback', () => {
expect.objectContaining({
maxResults: 10,
credentialId: 'cred-123',
credential: 'cred-123',
_context: expect.objectContaining({
userId: 'user-1',
workflowId: 'workflow-1',
Expand All @@ -206,6 +205,7 @@ describe('copilot tool executor fallback', () => {
}),
})
)
expect(executeAppTool.mock.calls[0]?.[1]).not.toHaveProperty('credential')
expect(result).toEqual({ success: true, output: { emails: [] } })
})

Expand Down
4 changes: 0 additions & 4 deletions apps/sim/lib/copilot/tool-executor/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,6 @@ function buildAppToolParams(
): Record<string, unknown> {
const result = { ...params }

if (result.credentialId && !result.credential && !result.oauthCredential) {
result.credential = result.credentialId
}

result._context = {
...(typeof result._context === 'object' && result._context !== null
? (result._context as object)
Expand Down
172 changes: 172 additions & 0 deletions apps/sim/tools/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4541,6 +4541,16 @@ describe('Copilot OAuth Credential Enforcement', () => {
let cleanupEnvVars: () => void

beforeEach(() => {
tools.test_credential_alias = {
id: 'test_credential_alias',
name: 'Credential Alias Test',
description: 'A synthetic stored-credential tool',
version: '1.0.0',
params: {
oauthCredential: { type: 'string', required: true, visibility: 'user-or-llm' },
},
operation: { input: (params) => ({ oauthCredential: params.oauthCredential }) },
} satisfies InternalToolConfig
process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
/*
* getInternalApiBaseUrl prefers INTERNAL_API_BASE_URL over the app URL, so
Expand All @@ -4556,10 +4566,172 @@ describe('Copilot OAuth Credential Enforcement', () => {
})

afterEach(() => {
Reflect.deleteProperty(tools, 'test_credential_alias')
vi.resetAllMocks()
cleanupEnvVars()
})

it.each([
{ params: { credentialId: ' alias-id ' }, expected: 'alias-id' },
{ params: { credential: 'generic-id' }, expected: 'generic-id' },
{ params: { credential: 'generic-id', credentialId: 'alias-id' }, expected: 'generic-id' },
{
params: {
oauthCredential: 'explicit-id',
credential: 'generic-id',
credentialId: 'alias-id',
},
expected: 'explicit-id',
},
])(
'normalizes credential selectors before validation: $expected',
async ({ params, expected }) => {
mockResolveExecutorCredentialToken.mockResolvedValue({
accessToken: 'resolved-token',
credentialType: 'service_account',
})
for (const copilotToolExecution of [false, true]) {
mockExecuteInternalToolOperation.mockClear()
const result = await executeTool('test_credential_alias', params, {
executionContext: createToolExecutionContext({
workspaceId: 'workspace-456',
userId: 'user-1',
copilotToolExecution,
}),
})
expect(result.success, result.error).toBe(true)
expect(mockExecuteInternalToolOperation.mock.calls[0]?.[0].input).toEqual({
oauthCredential: expected,
})
}
}
)

it.each(['', ' ', null, false, 123])(
'does not replace an invalid explicit selector: %s',
async (oauthCredential) => {
mockExecuteInternalToolOperation.mockClear()
mockResolveExecutorCredentialToken.mockClear()
const result = await executeTool('test_credential_alias', {
oauthCredential,
credentialId: 'another-credential',
})

expect(result).toMatchObject({
success: false,
error: 'Credential selection must be a nonempty string',
})
expect(mockExecuteInternalToolOperation).not.toHaveBeenCalled()
expect(mockResolveExecutorCredentialToken).not.toHaveBeenCalled()
}
)

it.each(['credential', 'credentialId'])(
'keeps declared %s authoritative over other aliases',
async (selector) => {
tools.test_credential_alias.params = {
[selector]: { type: 'string', required: true, visibility: 'user-or-llm' },
}
mockResolveExecutorCredentialToken.mockResolvedValue({ accessToken: 'resolved-token' })
const result = await executeTool(
'test_credential_alias',
{
credential: 'declared-id',
oauthCredential: 'other-id',
credentialId: 'alias-id',
[selector]: 'declared-id',
},
{
executionContext: createToolExecutionContext({
workspaceId: 'workspace-456',
userId: 'user-1',
}),
}
)

expect(result.success, result.error).toBe(true)
expect(mockResolveExecutorCredentialToken).toHaveBeenCalledWith(
expect.objectContaining({
credentialId: 'declared-id',
})
)
}
)

it('does not bypass authorization when an alias identifies an unavailable credential', async () => {
mockResolveExecutorCredentialToken.mockRejectedValueOnce(new Error('Credential unavailable'))
mockExecuteInternalToolOperation.mockClear()
const result = await executeTool(
'test_credential_alias',
{ credentialId: 'unavailable-id' },
{
executionContext: createToolExecutionContext({
workspaceId: 'workspace-456',
userId: 'user-1',
}),
}
)

expect(result).toMatchObject({ success: false, error: 'Credential unavailable' })
expect(mockExecuteInternalToolOperation).not.toHaveBeenCalled()
})

it.each(['oauthCredential', 'credential', 'credentialId'])(
'uses resolved %s references instead of stale compatibility aliases',
async (selector) => {
tools.test_credential_alias.params = {
[selector]: { type: 'string', required: true, visibility: 'user-only' },
}
mockGetEffectiveDecryptedEnv.mockResolvedValue({ TEST_CREDENTIAL: 'resolved-id' })
mockResolveExecutorCredentialToken.mockResolvedValue({ accessToken: 'resolved-token' })
const result = await executeTool(
'test_credential_alias',
{
oauthCredential: 'other-id',
credential: 'other-id',
credentialId: 'other-id',
[selector]: '{{TEST_CREDENTIAL}}',
},
{
executionContext: createToolExecutionContext({
workspaceId: 'workspace-456',
userId: 'user-1',
copilotToolExecution: true,
}),
}
)

expect(result.success, result.error).toBe(true)
expect(mockGetEffectiveDecryptedEnv).toHaveBeenCalledWith('user-1', 'workspace-456')
expect(mockResolveExecutorCredentialToken).toHaveBeenCalledWith(
expect.objectContaining({
credentialId: 'resolved-id',
})
)
}
)

it('resolves alias-only OAuth calls through the authorized credential path', async () => {
mockResolveExecutorCredentialToken.mockResolvedValueOnce({ accessToken: 'resolved-token' })
const context = createToolExecutionContext({
workspaceId: 'workspace-456',
copilotToolExecution: true,
})
const result = await executeTool(
'gmail_read',
{ credentialId: 'alias-id' },
{ executionContext: context }
)

expect(result.success).toBe(true)
expect(mockResolveExecutorCredentialToken).toHaveBeenCalledWith(
expect.objectContaining({
credentialId: 'alias-id',
toolId: 'gmail_read',
})
)
})

it('fails fast when copilot executes an oauth tool without an explicit credential selector', async () => {
const fetchMock = vi.fn()
global.fetch = Object.assign(fetchMock, { preconnect: vi.fn() }) as typeof fetch
Expand Down
32 changes: 25 additions & 7 deletions apps/sim/tools/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,26 @@ function readExplicitCredentialSelector(params: Record<string, unknown>): string
return undefined
}

function normalizeCopilotCredentialParams(params: Record<string, unknown>): void {
const credentialId = typeof params.credentialId === 'string' ? params.credentialId.trim() : ''
if (credentialId && !params.credential && !params.oauthCredential) {
params.credential = credentialId
function normalizeCopilotCredentialParams(
tool: ToolDefinition,
params: Record<string, unknown>
): string | undefined {
const aliases = ['oauthCredential', 'credential', 'credentialId']
const selector = aliases.find(
(name) => tool.params[name] !== undefined && tool.params[name].visibility !== 'hidden'
)
const selectedKey =
selector && params[selector] !== undefined
? selector
: aliases.find((name) => params[name] !== undefined)
if (!selectedKey) return

const selected = params[selectedKey]
if (typeof selected !== 'string' || selected.trim().length === 0) {
throw new Error('Credential selection must be a nonempty string')
}
params[selector ?? 'credential'] = selectedKey === 'credentialId' ? selected.trim() : selected
return selector ?? 'credential'
}

function enforceCopilotCredentialSelection(
Expand Down Expand Up @@ -1856,6 +1871,10 @@ async function executeToolImplementation(
}
}

const credentialSelector = tool
? normalizeCopilotCredentialParams(tool, contextParams)
: undefined

// Validate the tool and its parameters
validateRequiredParametersAfterMerge(toolId, tool, contextParams)

Expand All @@ -1865,7 +1884,6 @@ async function executeToolImplementation(
}

await normalizeFileParams(tool, contextParams, scope, executionContext)
normalizeCopilotCredentialParams(contextParams)
enforceCopilotCredentialSelection(toolId, tool, contextParams, scope)
await resolveToolEnvReferences(tool, contextParams, scope, resolvedSecretTraceRegistry)

Expand All @@ -1886,8 +1904,8 @@ async function executeToolImplementation(
}

// If we have a credential parameter, fetch the access token
if (contextParams.oauthCredential) {
contextParams.credential = contextParams.oauthCredential
if (credentialSelector) {
contextParams.credential = contextParams[credentialSelector]
}
if (operationContext?.requestMode === 'assistant' && tool.personalToken) {
if (typeof window !== 'undefined' || !operationContext.workspaceId) {
Expand Down
43 changes: 43 additions & 0 deletions apps/sim/tools/params.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,49 @@ describe('Tool Parameters Utils', () => {
expect(copilotSchema.required).toContain('credentialId')
})

it.each(['oauthCredential', 'credential', 'credentialId'])(
'publishes one Copilot credential field for declared %s selectors',
(selector) => {
for (const oauth of [undefined, { required: true, provider: 'test-provider' }]) {
const tool = {
...mockToolConfig,
...(oauth ? { oauth } : {}),
params: {
[selector]: { type: 'string', required: true, visibility: 'user-only' as const },
},
}
const schema = createUserToolSchema(tool, { surface: 'copilot' })
expect(Object.keys(schema.properties)).toEqual(['credentialId'])
expect(schema.required).toEqual(['credentialId'])
expect(schema.properties.credentialId.description).not.toContain('{{VAR_NAME}}')
const defaultSchema = createUserToolSchema(tool)
expect(Object.keys(defaultSchema.properties)).toEqual([selector])
expect(defaultSchema.required).toEqual([selector])
}
}
)

it('keeps optional credential selectors optional and hidden authority unpublished', () => {
const schema = createUserToolSchema(
{
...mockToolConfig,
params: { oauthCredential: { type: 'string', required: false, visibility: 'user-only' } },
},
{ surface: 'copilot' }
)
expect(Object.keys(schema.properties)).toEqual(['credentialId'])
expect(schema.required).toEqual([])
const hiddenSchema = createUserToolSchema(
{
...mockToolConfig,
params: { credentialId: { type: 'string', required: true, visibility: 'hidden' } },
},
{ surface: 'copilot' }
)
expect(hiddenSchema.properties).toEqual({})
expect(hiddenSchema.required).toEqual([])
})

it.concurrent('emits file params as reference strings by default', () => {
const toolWithFileParams = {
...mockToolConfig,
Expand Down
19 changes: 16 additions & 3 deletions apps/sim/tools/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ export function createUserToolSchema(
options: UserToolSchemaOptions = {}
): ToolSchema {
const surface = options.surface ?? 'default'
const credentialSelectors = ['oauthCredential', 'credential', 'credentialId']
const declaredCredentialSelectors = credentialSelectors.filter(
(name) =>
toolConfig.params[name] !== undefined && toolConfig.params[name].visibility !== 'hidden'
)
const useCredentialId =
surface === 'copilot' && (toolConfig.oauth?.required || declaredCredentialSelectors.length > 0)
const hostedApiKeyParam =
options.hostedKeySupport && toolConfig.hosting && !toolConfig.hosting.enabled
? toolConfig.hosting.apiKeyParam
Expand All @@ -284,6 +291,7 @@ export function createUserToolSchema(

for (const [paramId, param] of Object.entries(toolConfig.params)) {
if (!param) continue
if (useCredentialId && credentialSelectors.includes(paramId)) continue
const visibility = param.visibility ?? 'user-or-llm'
if (visibility === 'hidden') {
continue
Expand Down Expand Up @@ -315,13 +323,18 @@ export function createUserToolSchema(
}
}

if (toolConfig.oauth?.required && surface === 'copilot') {
if (useCredentialId) {
schema.properties.credentialId = {
type: 'string',
description:
'Credential ID to use for this OAuth tool call. Required for Copilot/Superagent execution. Get valid IDs from environment/credentials.json.',
'Credential ID to use for this connected tool call. Get valid IDs from environment/credentials.json.',
}
if (
toolConfig.oauth?.required ||
declaredCredentialSelectors.some((name) => toolConfig.params[name]?.required)
) {
schema.required.push('credentialId')
}
schema.required.push('credentialId')
}

return schema
Expand Down
Loading