Skip to content

Commit 0a6ee8a

Browse files
fix(tools): sanitize database execution errors
1 parent 23318a1 commit 0a6ee8a

2 files changed

Lines changed: 68 additions & 4 deletions

File tree

apps/sim/tools/index.test.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
setEnvFlags,
2626
} from '@sim/testing'
2727
import { sleep } from '@sim/utils/helpers'
28+
import { DrizzleQueryError } from 'drizzle-orm/errors'
2829
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
2930
import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
3031
import { projectToolResultForCopilot } from '@/lib/copilot/request/tools/resolved-secret-result'
@@ -54,6 +55,7 @@ const {
5455
mockGenerateInternalDelegationToken,
5556
mockGenerateInternalToken,
5657
mockResolveWorkspaceFileReference,
58+
mockAssertPermissionsAllowed,
5759
} = vi.hoisted(() => ({
5860
mockGetBYOKKey: vi.fn(),
5961
mockGetToolAsync: vi.fn(),
@@ -71,6 +73,7 @@ const {
7173
mockGenerateInternalDelegationToken: vi.fn(),
7274
mockGenerateInternalToken: vi.fn(),
7375
mockResolveWorkspaceFileReference: vi.fn(),
76+
mockAssertPermissionsAllowed: vi.fn(),
7477
}))
7578

7679
const mockSecureFetchWithPinnedIP = inputValidationMockFns.mockSecureFetchWithPinnedIP
@@ -94,7 +97,7 @@ vi.mock('@/lib/core/security/encryption', () => ({
9497
}))
9598

9699
vi.mock('@/ee/access-control/utils/permission-check', () => ({
97-
assertPermissionsAllowed: vi.fn().mockResolvedValue(undefined),
100+
assertPermissionsAllowed: mockAssertPermissionsAllowed,
98101
validateBlockType: vi.fn().mockResolvedValue(undefined),
99102
validateMcpToolsAllowed: vi.fn().mockResolvedValue(undefined),
100103
validateCustomToolsAllowed: vi.fn().mockResolvedValue(undefined),
@@ -460,6 +463,7 @@ vi.spyOn(getQueryClientModule, 'getQueryClient').mockImplementation(createMockQu
460463

461464
beforeEach(() => {
462465
vi.spyOn(getQueryClientModule, 'getQueryClient').mockImplementation(createMockQueryClient)
466+
mockAssertPermissionsAllowed.mockResolvedValue(undefined)
463467
mockGenerateInternalDelegationToken.mockResolvedValue('executor-token')
464468
mockRunWorkflowTool.mockResolvedValue({ success: true, output: {} })
465469
// Suites below call vi.resetAllMocks(), which wipes the shared env/urls mock
@@ -692,6 +696,53 @@ describe('executeTool Function', () => {
692696
tools.function_execute = originalFunctionTool
693697
})
694698

699+
it('logs database query diagnostics without exposing query details to the caller', async () => {
700+
const driverError = Object.assign(new Error('read ECONNRESET'), {
701+
code: 'ECONNRESET',
702+
errno: 'ECONNRESET',
703+
syscall: 'read',
704+
})
705+
const databaseError = new DrizzleQueryError(
706+
'select "id" from "workspace" where "workspace"."id" = $1 limit $2',
707+
['workspace-secret-id', 1],
708+
driverError
709+
)
710+
mockAssertPermissionsAllowed.mockRejectedValueOnce(databaseError)
711+
mockToolsLogger.error.mockClear()
712+
713+
const result = await executeTool(
714+
'function_execute',
715+
{ code: 'return 1' },
716+
{ executionContext: createToolExecutionContext({ userId: 'user-123' }) }
717+
)
718+
719+
expect(result.success).toBe(false)
720+
expect(result.error).toBe(
721+
'An internal error occurred while executing the tool. Please try again.'
722+
)
723+
expect(JSON.stringify(result)).not.toContain('Failed query')
724+
expect(JSON.stringify(result)).not.toContain('workspace-secret-id')
725+
expect(global.fetch).not.toHaveBeenCalled()
726+
727+
const loggedError = mockToolsLogger.error.mock.calls.at(-1)?.[1]
728+
expect(loggedError).toEqual(
729+
expect.objectContaining({
730+
cause: expect.objectContaining({
731+
name: 'Error',
732+
message: 'read ECONNRESET',
733+
code: 'ECONNRESET',
734+
errno: 'ECONNRESET',
735+
syscall: 'read',
736+
causeChain: expect.arrayContaining([
737+
expect.stringContaining('params: [redacted]'),
738+
'Error: read ECONNRESET',
739+
]),
740+
}),
741+
})
742+
)
743+
expect(JSON.stringify(loggedError)).not.toContain('workspace-secret-id')
744+
})
745+
695746
it('should call internal routes directly', async () => {
696747
const originalFunctionTool = { ...tools.function_execute }
697748
tools.function_execute = {

apps/sim/tools/index.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import { getErrorMessage, toError } from '@sim/utils/errors'
2+
import { describeError, findCause, getErrorMessage, toError } from '@sim/utils/errors'
33
import { sleep } from '@sim/utils/helpers'
44
import { isPlainRecord } from '@sim/utils/object'
55
import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry'
6+
import { DrizzleQueryError } from 'drizzle-orm/errors'
67
import { getBYOKKey } from '@/lib/api-key/byok'
78
import {
89
type GenerateInternalDelegationTokenInput,
@@ -92,6 +93,8 @@ const PRIVATE_MODEL_INPUT_DIRECT_EXECUTION_ERROR_MESSAGE =
9293
'Private model input provenance is not supported by direct execution'
9394
const PRIVATE_SECRET_PROVENANCE_DIRECT_EXECUTION_ERROR_MESSAGE =
9495
'Private secret provenance is not supported by direct execution'
96+
const INTERNAL_DATABASE_ERROR_MESSAGE =
97+
'An internal error occurred while executing the tool. Please try again.'
9598

9699
function projectToolLogMetadata(
97100
metadata: Record<string, unknown>,
@@ -2034,17 +2037,25 @@ async function executeToolImplementation(
20342037
}
20352038
} catch (error: any) {
20362039
const normalizedError = toError(error)
2040+
const databaseQueryError = findCause(
2041+
error,
2042+
(cause): cause is DrizzleQueryError => cause instanceof DrizzleQueryError
2043+
)
2044+
const databaseErrorCause = databaseQueryError ? describeError(error) : undefined
20372045
logger.error(
20382046
`[${requestId}] Error executing tool ${toolId}:`,
20392047
projectToolLogMetadata(
20402048
{
2041-
error: normalizedError.message,
2049+
...(databaseErrorCause
2050+
? { cause: databaseErrorCause }
2051+
: { error: normalizedError.message }),
20422052
stack: error instanceof Error ? error.stack : undefined,
20432053
},
20442054
resolvedSecretTraceRegistry,
20452055
{
20462056
errorName: normalizedError.name,
20472057
hasStack: Boolean(error instanceof Error && error.stack),
2058+
...(databaseErrorCause ? { cause: databaseErrorCause } : {}),
20482059
},
20492060
structuralOnlyToolLogs
20502061
)
@@ -2062,7 +2073,9 @@ async function executeToolImplementation(
20622073
let errorDetails = {}
20632074

20642075
if (error instanceof Error) {
2065-
errorMessage = error.message || `Error executing tool ${toolId}`
2076+
errorMessage = databaseQueryError
2077+
? INTERNAL_DATABASE_ERROR_MESSAGE
2078+
: error.message || `Error executing tool ${toolId}`
20662079
// HTTP errors are thrown as Error instances carrying `status`/`statusText`/
20672080
// `data` (see createTransformedErrorFromErrorInfo). Surface them on the
20682081
// output so callers can branch on the status (e.g. treat 404 as a clean

0 commit comments

Comments
 (0)