From f8170891f5f4fff6f21ca200cbc707a0a87b0580 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 17:26:09 -0700 Subject: [PATCH 1/2] fix(integrations): validate runtime values the type system only claims to constrain Three defects surfaced by review of the v0.8.16 release PR (#7224), each one a declared type standing in for a check that never runs. datadog: `DatadogSite` is a compile-time union, erased at runtime, and `site` is interpolated straight into the request host while every Datadog request carries DD-API-KEY and DD-APPLICATION-KEY. An unvalidated value therefore chose where the workspace's Datadog credentials were sent: `evil.com` addresses api.evil.com, and `datadoghq.com@evil.com` addresses evil.com with the expected host as userinfo. The site list is now a runtime array with the type derived from it, and both host builders -- `datadogApiUrl` (31 call sites) and the logs intake in send_logs -- resolve through one validator. Not reachable from the editor today, since the block renders a dropdown and the param is user-only; the value still survives in stored workflow state, which imports and programmatic edits write directly. cbinsights: `params.x?.trim()` guards undefined, not the type, so a block-to-block reference resolving to a number threw a bare TypeError naming no parameter. The sibling history operation already used `parseOptionalStringParam`; the remaining 19 sites now do too. Behaviour is otherwise unchanged -- `compactBody` already dropped '' and undefined alike, and every non-compactBody use tests falsiness. cbinsights rag: the guard admitted a 10,000-character message while its own error and the tool's param description both say "under 10,000". managed-agent: `denyMessage` was trimmed above the try block, so a non-string threw past every `success: false` path the operation otherwise returns. It is now coerced the same way `decision` is a few lines above. Two further findings on that PR were checked and left alone: the Drive addParents/removeParents overlap matches Google's own documented move sample in all four languages, and Bitbucket's Range behaviour on a zero-byte file is undocumented, so neither is a verified defect. --- .../internal/cbinsights/operations/chat.ts | 5 +- .../get-exit-probability-history.ts | 5 +- .../operations/get-mosaic-history.ts | 10 ++- .../cbinsights/operations/get-org-fundings.ts | 3 +- .../operations/get-org-investments.ts | 3 +- .../operations/list-business-relationships.ts | 3 +- .../cbinsights/operations/list-fundings.ts | 3 +- .../cbinsights/operations/list-investments.ts | 3 +- .../operations/list-portfolio-exits.ts | 3 +- .../operations/lookup-organizations.ts | 5 +- .../lib/internal/cbinsights/operations/rag.ts | 11 +++- .../operations/search-firmographics.ts | 11 ++-- .../operations/respond-tool-confirmation.ts | 5 +- apps/sim/tools/cbinsights/cbinsights.test.ts | 64 +++++++++++++++++++ apps/sim/tools/datadog/datadog.test.ts | 48 ++++++++++++++ apps/sim/tools/datadog/send_logs.ts | 11 +--- apps/sim/tools/datadog/types.ts | 32 ++++++---- apps/sim/tools/datadog/utils.ts | 33 +++++++++- .../respond_tool_confirmation.test.ts | 21 ++++++ 19 files changed, 236 insertions(+), 43 deletions(-) diff --git a/apps/sim/lib/internal/cbinsights/operations/chat.ts b/apps/sim/lib/internal/cbinsights/operations/chat.ts index f05b575b8fd..dcb050d7ae8 100644 --- a/apps/sim/lib/internal/cbinsights/operations/chat.ts +++ b/apps/sim/lib/internal/cbinsights/operations/chat.ts @@ -6,12 +6,13 @@ import { asStringArray, cbInsightsRequest, compactBody, + parseOptionalStringParam, } from '@/tools/cbinsights/utils' export const executeCbinsightsChatOperation: InternalToolOperationImplementation< CbInsightsChatParams > = async (params, signal) => { - const message = params.message?.trim() + const message = parseOptionalStringParam(params.message, 'message') if (!message) throw new Error('CB Insights "message" is required') return cbInsightsRequest<{ @@ -25,7 +26,7 @@ export const executeCbinsightsChatOperation: InternalToolOperationImplementation params, { path: '/v2/chatcbi', - body: compactBody({ message, chatID: params.chatId?.trim() }), + body: compactBody({ message, chatID: parseOptionalStringParam(params.chatId, 'chatId') }), }, (data) => ({ chatId: asString(data.chatID), diff --git a/apps/sim/lib/internal/cbinsights/operations/get-exit-probability-history.ts b/apps/sim/lib/internal/cbinsights/operations/get-exit-probability-history.ts index b98fd85fd09..f439356d450 100644 --- a/apps/sim/lib/internal/cbinsights/operations/get-exit-probability-history.ts +++ b/apps/sim/lib/internal/cbinsights/operations/get-exit-probability-history.ts @@ -5,6 +5,7 @@ import { asString, cbInsightsRequest, compactBody, + parseOptionalStringParam, requireOrgId, } from '@/tools/cbinsights/utils' @@ -17,8 +18,8 @@ export const executeCbinsightsGetExitProbabilityHistoryOperation: InternalToolOp { path: `/v2/organizations/${orgId}/exitprobabilityhistory`, body: compactBody({ - startDate: params.startDate?.trim(), - endDate: params.endDate?.trim(), + startDate: parseOptionalStringParam(params.startDate, 'startDate'), + endDate: parseOptionalStringParam(params.endDate, 'endDate'), }), }, (data) => ({ diff --git a/apps/sim/lib/internal/cbinsights/operations/get-mosaic-history.ts b/apps/sim/lib/internal/cbinsights/operations/get-mosaic-history.ts index 7ae034284cf..31a9191fdbc 100644 --- a/apps/sim/lib/internal/cbinsights/operations/get-mosaic-history.ts +++ b/apps/sim/lib/internal/cbinsights/operations/get-mosaic-history.ts @@ -1,6 +1,12 @@ import type { InternalToolOperationImplementation } from '@/lib/internal/tool-operations/types' import type { CbInsightsMosaicHistoryParams } from '@/tools/cbinsights/get_mosaic_history' -import { asArray, cbInsightsRequest, compactBody, requireOrgId } from '@/tools/cbinsights/utils' +import { + asArray, + cbInsightsRequest, + compactBody, + parseOptionalStringParam, + requireOrgId, +} from '@/tools/cbinsights/utils' export const executeCbinsightsGetMosaicHistoryOperation: InternalToolOperationImplementation< CbInsightsMosaicHistoryParams @@ -16,7 +22,7 @@ export const executeCbinsightsGetMosaicHistoryOperation: InternalToolOperationIm params, { path: `/v2/organizations/${orgId}/mosaichistory`, - body: compactBody({ startDate: params.startDate?.trim() }), + body: compactBody({ startDate: parseOptionalStringParam(params.startDate, 'startDate') }), }, (data) => ({ overall: asArray(data.overall), diff --git a/apps/sim/lib/internal/cbinsights/operations/get-org-fundings.ts b/apps/sim/lib/internal/cbinsights/operations/get-org-fundings.ts index f0a97d42c3c..b525afef962 100644 --- a/apps/sim/lib/internal/cbinsights/operations/get-org-fundings.ts +++ b/apps/sim/lib/internal/cbinsights/operations/get-org-fundings.ts @@ -6,6 +6,7 @@ import { clampLimit, compactBody, pageInfo, + parseOptionalStringParam, requireOrgId, } from '@/tools/cbinsights/utils' @@ -25,7 +26,7 @@ export const executeCbinsightsGetOrgFundingsOperation: InternalToolOperationImpl path: `/v2/organizations/${orgId}/financialtransactions/fundings`, body: compactBody({ limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ diff --git a/apps/sim/lib/internal/cbinsights/operations/get-org-investments.ts b/apps/sim/lib/internal/cbinsights/operations/get-org-investments.ts index a2aea867ff2..e9f296b311f 100644 --- a/apps/sim/lib/internal/cbinsights/operations/get-org-investments.ts +++ b/apps/sim/lib/internal/cbinsights/operations/get-org-investments.ts @@ -6,6 +6,7 @@ import { clampLimit, compactBody, pageInfo, + parseOptionalStringParam, requireOrgId, } from '@/tools/cbinsights/utils' @@ -24,7 +25,7 @@ export const executeCbinsightsGetOrgInvestmentsOperation: InternalToolOperationI path: `/v2/organizations/${orgId}/financialtransactions/investments`, body: compactBody({ limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ investments: asArray(data.investments), ...pageInfo(data) }), diff --git a/apps/sim/lib/internal/cbinsights/operations/list-business-relationships.ts b/apps/sim/lib/internal/cbinsights/operations/list-business-relationships.ts index c779ac6d697..ece9747b27c 100644 --- a/apps/sim/lib/internal/cbinsights/operations/list-business-relationships.ts +++ b/apps/sim/lib/internal/cbinsights/operations/list-business-relationships.ts @@ -5,6 +5,7 @@ import { asString, cbInsightsRequest, compactBody, + parseOptionalStringParam, requireOrgIds, } from '@/tools/cbinsights/utils' @@ -17,7 +18,7 @@ export const executeCbinsightsListBusinessRelationshipsOperation: InternalToolOp path: '/v2/businessrelationships', body: compactBody({ orgIds: requireOrgIds(params.orgIds), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ orgs: asArray(data.orgs), nextPageToken: asString(data.nextPageToken) }), diff --git a/apps/sim/lib/internal/cbinsights/operations/list-fundings.ts b/apps/sim/lib/internal/cbinsights/operations/list-fundings.ts index 4e4bde42bb9..9bf41c7777a 100644 --- a/apps/sim/lib/internal/cbinsights/operations/list-fundings.ts +++ b/apps/sim/lib/internal/cbinsights/operations/list-fundings.ts @@ -6,6 +6,7 @@ import { clampLimit, compactBody, pageInfo, + parseOptionalStringParam, requireOrgIds, } from '@/tools/cbinsights/utils' @@ -24,7 +25,7 @@ export const executeCbinsightsListFundingsOperation: InternalToolOperationImplem body: compactBody({ orgIds: requireOrgIds(params.orgIds), limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ orgs: asArray(data.orgs), ...pageInfo(data) }), diff --git a/apps/sim/lib/internal/cbinsights/operations/list-investments.ts b/apps/sim/lib/internal/cbinsights/operations/list-investments.ts index 254a8971c40..02dcf1ff9e7 100644 --- a/apps/sim/lib/internal/cbinsights/operations/list-investments.ts +++ b/apps/sim/lib/internal/cbinsights/operations/list-investments.ts @@ -6,6 +6,7 @@ import { clampLimit, compactBody, pageInfo, + parseOptionalStringParam, requireOrgIds, } from '@/tools/cbinsights/utils' @@ -24,7 +25,7 @@ export const executeCbinsightsListInvestmentsOperation: InternalToolOperationImp body: compactBody({ orgIds: requireOrgIds(params.orgIds), limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ orgs: asArray(data.orgs), ...pageInfo(data) }), diff --git a/apps/sim/lib/internal/cbinsights/operations/list-portfolio-exits.ts b/apps/sim/lib/internal/cbinsights/operations/list-portfolio-exits.ts index 07416ca6054..e603d4e2df0 100644 --- a/apps/sim/lib/internal/cbinsights/operations/list-portfolio-exits.ts +++ b/apps/sim/lib/internal/cbinsights/operations/list-portfolio-exits.ts @@ -6,6 +6,7 @@ import { clampLimit, compactBody, pageInfo, + parseOptionalStringParam, requireOrgIds, } from '@/tools/cbinsights/utils' @@ -24,7 +25,7 @@ export const executeCbinsightsListPortfolioExitsOperation: InternalToolOperation body: compactBody({ orgIds: requireOrgIds(params.orgIds), limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ orgs: asArray(data.orgs), ...pageInfo(data) }), diff --git a/apps/sim/lib/internal/cbinsights/operations/lookup-organizations.ts b/apps/sim/lib/internal/cbinsights/operations/lookup-organizations.ts index 1eef6801bc3..18a1cf6aefe 100644 --- a/apps/sim/lib/internal/cbinsights/operations/lookup-organizations.ts +++ b/apps/sim/lib/internal/cbinsights/operations/lookup-organizations.ts @@ -6,6 +6,7 @@ import { clampLimit, compactBody, pageInfo, + parseOptionalStringParam, parseStringListParam, } from '@/tools/cbinsights/utils' @@ -14,7 +15,7 @@ export const executeCbinsightsLookupOrganizationsOperation: InternalToolOperatio > = async (params, signal) => { const names = parseStringListParam(params.names, 'names') const urls = parseStringListParam(params.urls, 'urls') - const profileUrl = params.profileUrl?.trim() + const profileUrl = parseOptionalStringParam(params.profileUrl, 'profileUrl') if (!names && !urls && !profileUrl) { throw new Error('CB Insights lookup requires at least one of "names", "urls", or "profileUrl"') @@ -39,7 +40,7 @@ export const executeCbinsightsLookupOrganizationsOperation: InternalToolOperatio urls, profileUrl, limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), }, (data) => ({ orgs: asArray(data.orgs), ...pageInfo(data) }), diff --git a/apps/sim/lib/internal/cbinsights/operations/rag.ts b/apps/sim/lib/internal/cbinsights/operations/rag.ts index 4ee1bceb9fd..e4a9f3ab64e 100644 --- a/apps/sim/lib/internal/cbinsights/operations/rag.ts +++ b/apps/sim/lib/internal/cbinsights/operations/rag.ts @@ -1,13 +1,18 @@ import type { InternalToolOperationImplementation } from '@/lib/internal/tool-operations/types' import type { CbInsightsRagParams } from '@/tools/cbinsights/rag' -import { asString, asStringArray, cbInsightsRequest } from '@/tools/cbinsights/utils' +import { + asString, + asStringArray, + cbInsightsRequest, + parseOptionalStringParam, +} from '@/tools/cbinsights/utils' export const executeCbinsightsRagOperation: InternalToolOperationImplementation< CbInsightsRagParams > = async (params, signal) => { - const message = params.message?.trim() + const message = parseOptionalStringParam(params.message, 'message') if (!message) throw new Error('CB Insights "message" is required') - if (message.length > 10_000) { + if (message.length >= 10_000) { throw new Error('CB Insights "message" must be under 10,000 characters') } diff --git a/apps/sim/lib/internal/cbinsights/operations/search-firmographics.ts b/apps/sim/lib/internal/cbinsights/operations/search-firmographics.ts index 3010d349517..10b690a6563 100644 --- a/apps/sim/lib/internal/cbinsights/operations/search-firmographics.ts +++ b/apps/sim/lib/internal/cbinsights/operations/search-firmographics.ts @@ -12,6 +12,7 @@ import { parseIntegerParam, parseNumberParam, parseOptionalOrgIds, + parseOptionalStringParam, parseStringListParam, } from '@/tools/cbinsights/utils' @@ -19,7 +20,7 @@ export const executeCbinsightsSearchFirmographicsOperation: InternalToolOperatio CbInsightsFirmographicsParams > = async (params, signal) => { const filters = compactBody({ - keyword: params.keyword?.trim(), + keyword: parseOptionalStringParam(params.keyword, 'keyword'), orgIds: parseOptionalOrgIds(params.orgIds), orgNames: parseStringListParam(params.orgNames, 'orgNames'), urls: parseStringListParam(params.urls, 'urls'), @@ -67,8 +68,8 @@ export const executeCbinsightsSearchFirmographicsOperation: InternalToolOperatio params.maxValuationInMillions, 'maxValuationInMillions' ), - minLastFundingDate: params.minLastFundingDate?.trim(), - maxLastFundingDate: params.maxLastFundingDate?.trim(), + minLastFundingDate: parseOptionalStringParam(params.minLastFundingDate, 'minLastFundingDate'), + maxLastFundingDate: parseOptionalStringParam(params.maxLastFundingDate, 'maxLastFundingDate'), vcBacked: parseBooleanParam(params.vcBacked, 'vcBacked'), }) @@ -86,13 +87,13 @@ export const executeCbinsightsSearchFirmographicsOperation: InternalToolOperatio ...filters, ...compactBody({ limit: clampLimit(params.limit), - nextPageToken: params.nextPageToken?.trim(), + nextPageToken: parseOptionalStringParam(params.nextPageToken, 'nextPageToken'), }), } /* The API takes one sort object; the block exposes it as two plain fields so neither has to be typed as JSON. */ - const sortField = params.sortField?.trim() + const sortField = parseOptionalStringParam(params.sortField, 'sortField') if (sortField) { body.sort = { field: sortField, direction: sortDirection(params.sortDirection) } } diff --git a/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts b/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts index 1ba0cfa81b4..a9f95fecf1f 100644 --- a/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts +++ b/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts @@ -36,7 +36,10 @@ export const executeManagedAgentRespondToolConfirmationOperation: InternalToolOp } } - const denyMessage = params.denyMessage?.trim() + /* Coerced the same way `decision` is above: this runs outside the try below, so a + non-string arriving from a stored workflow would throw past every `success: false` + path this operation otherwise returns. */ + const denyMessage = (params.denyMessage ?? '').toString().trim() try { await sendToolConfirmations({ apiKey: target.apiKey, diff --git a/apps/sim/tools/cbinsights/cbinsights.test.ts b/apps/sim/tools/cbinsights/cbinsights.test.ts index c95afe745f9..5f3fb627461 100644 --- a/apps/sim/tools/cbinsights/cbinsights.test.ts +++ b/apps/sim/tools/cbinsights/cbinsights.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { executeCbinsightsChatOperation } from '@/lib/internal/cbinsights/operations/chat' import { executeCbinsightsGetCommercialMaturityHistoryOperation } from '@/lib/internal/cbinsights/operations/get-commercial-maturity-history' +import { executeCbinsightsGetExitProbabilityHistoryOperation } from '@/lib/internal/cbinsights/operations/get-exit-probability-history' import { executeCbinsightsGetOrgFundingsOperation } from '@/lib/internal/cbinsights/operations/get-org-fundings' import { executeCbinsightsGetOrgOutlookOperation } from '@/lib/internal/cbinsights/operations/get-org-outlook' import { executeCbinsightsListBusinessRelationshipsOperation } from '@/lib/internal/cbinsights/operations/list-business-relationships' @@ -689,3 +690,66 @@ describe('cbinsights model-input projection', () => { expect(cbinsightsSearchFirmographicsTool.operation.modelInput).toBeUndefined() }) }) + +describe('cbinsights non-text runtime values', () => { + /* + * `params.x?.trim()` guards `undefined`, not the type: a block-to-block reference + * resolving to a number reached `.trim()` and threw a bare TypeError naming no + * parameter. The sibling history operation already used parseOptionalStringParam. + */ + it('names the offending date parameter instead of throwing a TypeError', async () => { + mockFetch([AUTH_OK]) + await expect( + executeCbinsightsGetExitProbabilityHistoryOperation({ + ...CREDS, + orgId: 123, + startDate: 20240101, + } as never) + ).rejects.toThrow('CB Insights "startDate" must be a string') + }) + + it('names a non-text keyword on search rather than crashing', async () => { + mockFetch([AUTH_OK]) + await expect( + executeCbinsightsSearchFirmographicsOperation({ ...CREDS, keyword: { a: 1 } } as never) + ).rejects.toThrow('CB Insights "keyword" must be a string') + }) + + it('names a non-text page token rather than crashing', async () => { + mockFetch([AUTH_OK]) + await expect( + executeCbinsightsListFundingsOperation({ + ...CREDS, + orgIds: '123', + nextPageToken: 42, + } as never) + ).rejects.toThrow('CB Insights "nextPageToken" must be a string') + }) + + it('still treats a blank optional value as omitted, exactly as before', async () => { + mockFetch([AUTH_OK, { body: {} }]) + await executeCbinsightsGetExitProbabilityHistoryOperation({ + ...CREDS, + orgId: 123, + startDate: ' ', + } as never) + expect(JSON.parse(String(calls[1].init.body))).not.toHaveProperty('startDate') + }) +}) + +describe('cbinsights rag message bound', () => { + /* The tool description and this error both say "under 10,000", so the guard must + reject the boundary value rather than forward it. */ + it('rejects a message of exactly 10,000 characters', async () => { + mockFetch([AUTH_OK]) + await expect( + executeCbinsightsRagOperation({ ...CREDS, message: 'a'.repeat(10_000) } as never) + ).rejects.toThrow('CB Insights "message" must be under 10,000 characters') + }) + + it('accepts the largest message the contract allows', async () => { + mockFetch([AUTH_OK, { body: { data: 'ok' } }]) + await executeCbinsightsRagOperation({ ...CREDS, message: 'a'.repeat(9_999) } as never) + expect(JSON.parse(String(calls[1].init.body)).message).toHaveLength(9_999) + }) +}) diff --git a/apps/sim/tools/datadog/datadog.test.ts b/apps/sim/tools/datadog/datadog.test.ts index 9fb3166d3cf..80ca5c89b51 100644 --- a/apps/sim/tools/datadog/datadog.test.ts +++ b/apps/sim/tools/datadog/datadog.test.ts @@ -18,12 +18,15 @@ import { queryLogsTool } from '@/tools/datadog/query_logs' import { queryTimeseriesTool } from '@/tools/datadog/query_timeseries' import { sendLogsTool } from '@/tools/datadog/send_logs' import { submitMetricsTool } from '@/tools/datadog/submit_metrics' +import { DATADOG_SITES } from '@/tools/datadog/types' import { unmuteMonitorTool } from '@/tools/datadog/unmute_monitor' import { updateIncidentTool } from '@/tools/datadog/update_incident' import { buildSloPayload, + datadogApiUrl, datadogErrorMessage, mergeSloUpdatePayload, + resolveDatadogSite, splitCommaList, } from '@/tools/datadog/utils' @@ -604,3 +607,48 @@ describe('undisclosed vendor limits and Sim defaults', () => { expect(body[0].ddsource).toBe('custom') }) }) + +describe('datadog site is validated before it reaches the request host', () => { + /* + * `DatadogSite` is a compile-time union and is erased at runtime, so it kept + * nothing out of the URL. Every Datadog request carries DD-API-KEY and + * DD-APPLICATION-KEY, so an unchecked `site` chose where those were sent. + */ + it('rejects an arbitrary host', () => { + expect(() => datadogApiUrl('evil.com' as never, '/api/v1/slo')).toThrow( + /Datadog "site" must be one of/ + ) + }) + + it('rejects a host smuggled in as userinfo', () => { + expect(() => datadogApiUrl('datadoghq.com@evil.com' as never, '/api/v1/slo')).toThrow( + /Datadog "site" must be one of/ + ) + }) + + it('rejects a value that would escape the host into a path', () => { + expect(() => datadogApiUrl('datadoghq.com/../..' as never, '/api/v1/slo')).toThrow( + /Datadog "site" must be one of/ + ) + }) + + it('defaults an absent site to US1 and keeps every published region', () => { + expect(datadogApiUrl(undefined, '/api/v1/slo')).toBe('https://api.datadoghq.com/api/v1/slo') + for (const site of DATADOG_SITES) { + expect(datadogApiUrl(site, '/x')).toBe(`https://api.${site}/x`) + expect(resolveDatadogSite(site)).toBe(site) + } + }) + + it('builds the logs intake host from the validated site', () => { + const url = sendLogsTool.request.url({ + apiKey: 'k', + site: 'datadoghq.eu', + logs: '[]', + } as never) + expect(url).toBe('https://http-intake.logs.datadoghq.eu/api/v2/logs') + expect(() => + sendLogsTool.request.url({ apiKey: 'k', site: 'evil.com', logs: '[]' } as never) + ).toThrow(/Datadog "site" must be one of/) + }) +}) diff --git a/apps/sim/tools/datadog/send_logs.ts b/apps/sim/tools/datadog/send_logs.ts index 9b4c7b31700..a31e56749c6 100644 --- a/apps/sim/tools/datadog/send_logs.ts +++ b/apps/sim/tools/datadog/send_logs.ts @@ -1,6 +1,6 @@ import { filterUndefined } from '@sim/utils/object' import type { LogEntry, SendLogsParams, SendLogsResponse } from '@/tools/datadog/types' -import { datadogErrorMessage, parseJsonParam } from '@/tools/datadog/utils' +import { datadogErrorMessage, parseJsonParam, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const sendLogsTool: ToolConfig = { @@ -33,14 +33,9 @@ export const sendLogsTool: ToolConfig = { request: { url: (params) => { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) // Logs API uses a different subdomain - const logsHost = - site === 'datadoghq.com' - ? 'http-intake.logs.datadoghq.com' - : site === 'datadoghq.eu' - ? 'http-intake.logs.datadoghq.eu' - : `http-intake.logs.${site}` + const logsHost = `http-intake.logs.${site}` return `https://${logsHost}/api/v2/logs` }, method: 'POST', diff --git a/apps/sim/tools/datadog/types.ts b/apps/sim/tools/datadog/types.ts index d642521ce74..ee9699be1fa 100644 --- a/apps/sim/tools/datadog/types.ts +++ b/apps/sim/tools/datadog/types.ts @@ -2,17 +2,27 @@ import type { ToolResponse } from '@/tools/types' // Datadog Site/Region options -/** Regional sites Datadog serves the API from, per the `site` server variable enum. */ -export type DatadogSite = - | 'datadoghq.com' - | 'us3.datadoghq.com' - | 'us5.datadoghq.com' - | 'datadoghq.eu' - | 'ap1.datadoghq.com' - | 'ap2.datadoghq.com' - | 'uk1.datadoghq.com' - | 'ddog-gov.com' - | 'us2.ddog-gov.com' +/** + * Regional sites Datadog serves the API from, per the `site` server variable enum. + * + * A runtime array rather than a bare type union: `site` is interpolated into the + * request host, and a type union is erased at runtime, so it cannot keep a value + * that arrived from a stored workflow out of the URL. {@link DatadogSite} is + * derived from this list so the two can never drift. + */ +export const DATADOG_SITES = [ + 'datadoghq.com', + 'us3.datadoghq.com', + 'us5.datadoghq.com', + 'datadoghq.eu', + 'ap1.datadoghq.com', + 'ap2.datadoghq.com', + 'uk1.datadoghq.com', + 'ddog-gov.com', + 'us2.ddog-gov.com', +] as const + +export type DatadogSite = (typeof DATADOG_SITES)[number] // Base parameters for write-only operations (only need API key) interface DatadogWriteOnlyParams { diff --git a/apps/sim/tools/datadog/utils.ts b/apps/sim/tools/datadog/utils.ts index f293c03cb68..b05578469bc 100644 --- a/apps/sim/tools/datadog/utils.ts +++ b/apps/sim/tools/datadog/utils.ts @@ -4,6 +4,7 @@ import type { SecuritySignalTriageData, UpdateSloParams, } from '@/tools/datadog/types' +import { DATADOG_SITES } from '@/tools/datadog/types' /** * Builds a fully-qualified Datadog API URL for the caller's site/region. @@ -11,7 +12,37 @@ import type { * `ddog-gov.com`, ...), so every request must be built from the configured site. */ export function datadogApiUrl(site: DatadogSite | undefined, path: string): string { - return `https://api.${site || 'datadoghq.com'}${path}` + return `https://api.${resolveDatadogSite(site)}${path}` +} + +/** + * Resolves the caller's `site` to one of Datadog's published regional hosts. + * + * `DatadogSite` is a compile-time union, so it constrains nothing at runtime: the + * value is interpolated into the request host, and every Datadog request carries + * `DD-API-KEY` and `DD-APPLICATION-KEY`. An unchecked value therefore decides where + * the workspace's Datadog credentials are sent — `evil.com` addresses `api.evil.com`, + * and `datadoghq.com@evil.com` addresses `evil.com` with the expected host as + * userinfo. The block renders `site` as a dropdown and the param is `user-only`, so + * neither the editor nor a model can reach this today; the check exists because the + * value survives in stored workflow state, which imports and programmatic edits write + * without passing through that dropdown. + * + * Typed `unknown` rather than `DatadogSite`: the declared type is exactly what this + * cannot trust, and a stored workflow can hand over a blank string or a non-string. + * + * @param site - The caller-supplied site, or `undefined`/blank for the default region. + * @returns The validated site host. + * @throws If the value is present but is not a published Datadog site. + */ +export function resolveDatadogSite(site: unknown): DatadogSite { + if (site === undefined || site === null || site === '') return 'datadoghq.com' + if (typeof site !== 'string' || !(DATADOG_SITES as readonly string[]).includes(site)) { + throw new Error( + `Datadog "site" must be one of ${DATADOG_SITES.join(', ')}, but was ${String(site)}` + ) + } + return site as DatadogSite } /** diff --git a/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts b/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts index ff9fe26b7f4..c3ad4a383f6 100644 --- a/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts +++ b/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts @@ -19,6 +19,27 @@ describe('Managed Agent tool confirmations', () => { mockSendToolConfirmations.mockResolvedValue(undefined) }) + /* + * denyMessage was read with `params.denyMessage?.trim()` above the try block, so a + * non-string arriving from a stored workflow threw past every `success: false` path + * this operation otherwise returns. + */ + it('returns a result rather than throwing when denyMessage is not text', async () => { + const result = await executeManagedAgentRespondToolConfirmationOperation({ + accessToken: 'token', + sessionId: 'session-1', + toolUseIds: ['tool-use-1'], + decision: 'deny', + denyMessage: 42, + } as never) + expect(result.success).toBe(true) + expect(mockSendToolConfirmations).toHaveBeenLastCalledWith({ + apiKey: 'token', + sessionId: 'session-1', + confirmations: [{ toolUseId: 'tool-use-1', result: 'deny', denyMessage: '42' }], + }) + }) + it('sends a denial message only for deny decisions', async () => { await executeManagedAgentRespondToolConfirmationOperation({ accessToken: 'token', From 7bce07a6af9871968720ca8d16d6f3b7212d9b2d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 17:35:05 -0700 Subject: [PATCH 2/2] fix(datadog): route every host builder through the allowlist, coerce scalars safely Review round 1 on #7244 found the first pass incomplete, and both findings reproduce. The site allowlist only covered the shared `datadogApiUrl` and the logs intake. Ten tools build the host inline -- `const site = params.site || 'datadoghq.com'` in cancel_downtime, create_downtime, create_event, create_monitor, get_monitor, list_downtimes, list_monitors, query_logs, query_timeseries and submit_metrics -- so they never reached the validator while still attaching DD-API-KEY and, where the endpoint needs it, DD-APPLICATION-KEY. All ten now resolve through it. The new test sweeps the tool registry rather than naming tools, so a future tool that reintroduces an inline builder fails instead of shipping an unguarded request. `(value ?? '').toString()` was itself unsafe: an object whose `toString` is not a function, and one with a null prototype, both throw TypeError, and `String(value)` throws on the same two. That read sits above the try block, so it escaped the structured `success: false` result this operation promises. `normalizeScalarText` converts only the scalar kinds `String()` cannot fail on and returns '' otherwise, matching how `normalizeStringList` already treats a value of the wrong type. The identical hazard on `decision` two lines above is fixed with it as well. --- .../operations/respond-tool-confirmation.ts | 9 +-- apps/sim/tools/datadog/cancel_downtime.ts | 4 +- apps/sim/tools/datadog/create_downtime.ts | 9 ++- apps/sim/tools/datadog/create_event.ts | 4 +- apps/sim/tools/datadog/create_monitor.ts | 4 +- apps/sim/tools/datadog/datadog.test.ts | 58 +++++++++++++++++++ apps/sim/tools/datadog/get_monitor.ts | 4 +- apps/sim/tools/datadog/list_downtimes.ts | 4 +- apps/sim/tools/datadog/list_monitors.ts | 4 +- apps/sim/tools/datadog/query_logs.ts | 4 +- apps/sim/tools/datadog/query_timeseries.ts | 4 +- apps/sim/tools/datadog/submit_metrics.ts | 4 +- apps/sim/tools/managed_agent/normalizers.ts | 22 +++++++ .../respond_tool_confirmation.test.ts | 28 +++++++++ 14 files changed, 136 insertions(+), 26 deletions(-) diff --git a/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts b/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts index a9f95fecf1f..34c3bd9d726 100644 --- a/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts +++ b/apps/sim/lib/internal/managed-agent/operations/respond-tool-confirmation.ts @@ -1,7 +1,7 @@ import { getErrorMessage } from '@sim/utils/errors' import type { InternalToolOperationImplementation } from '@/lib/internal/tool-operations/types' import { sendToolConfirmations } from '@/lib/managed-agents/session-client' -import { normalizeStringList } from '@/tools/managed_agent/normalizers' +import { normalizeScalarText, normalizeStringList } from '@/tools/managed_agent/normalizers' import { resolveSessionTarget } from '@/tools/managed_agent/shared' import type { ManagedAgentToolConfirmationParams, @@ -17,7 +17,7 @@ export const executeManagedAgentRespondToolConfirmationOperation: InternalToolOp return { success: false, output: emptyOutput, error: target.error } } - const decision = (params.decision ?? '').toString().trim().toLowerCase() + const decision = normalizeScalarText(params.decision).toLowerCase() if (decision !== 'allow' && decision !== 'deny') { return { success: false, @@ -36,10 +36,7 @@ export const executeManagedAgentRespondToolConfirmationOperation: InternalToolOp } } - /* Coerced the same way `decision` is above: this runs outside the try below, so a - non-string arriving from a stored workflow would throw past every `success: false` - path this operation otherwise returns. */ - const denyMessage = (params.denyMessage ?? '').toString().trim() + const denyMessage = normalizeScalarText(params.denyMessage) try { await sendToolConfirmations({ apiKey: target.apiKey, diff --git a/apps/sim/tools/datadog/cancel_downtime.ts b/apps/sim/tools/datadog/cancel_downtime.ts index e46b1a3bb1c..8fca5089a56 100644 --- a/apps/sim/tools/datadog/cancel_downtime.ts +++ b/apps/sim/tools/datadog/cancel_downtime.ts @@ -1,5 +1,5 @@ import type { CancelDowntimeParams, CancelDowntimeResponse } from '@/tools/datadog/types' -import { datadogErrorMessage, datadogPathSegment } from '@/tools/datadog/utils' +import { datadogErrorMessage, datadogPathSegment, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const cancelDowntimeTool: ToolConfig = { @@ -37,7 +37,7 @@ export const cancelDowntimeTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) const downtimeId = datadogPathSegment(params.downtimeId) return `https://api.${site}/api/v2/downtime/${downtimeId}` }, diff --git a/apps/sim/tools/datadog/create_downtime.ts b/apps/sim/tools/datadog/create_downtime.ts index 4d9f235d79d..a315cf99b86 100644 --- a/apps/sim/tools/datadog/create_downtime.ts +++ b/apps/sim/tools/datadog/create_downtime.ts @@ -3,7 +3,12 @@ import type { CreateDowntimeResponse, DowntimeAttributes, } from '@/tools/datadog/types' -import { datadogErrorMessage, parseMonitorIds, splitCommaList } from '@/tools/datadog/utils' +import { + datadogErrorMessage, + parseMonitorIds, + resolveDatadogSite, + splitCommaList, +} from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const createDowntimeTool: ToolConfig = { @@ -85,7 +90,7 @@ export const createDowntimeTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) return `https://api.${site}/api/v2/downtime` }, method: 'POST', diff --git a/apps/sim/tools/datadog/create_event.ts b/apps/sim/tools/datadog/create_event.ts index f993e96977c..b7f4465af0b 100644 --- a/apps/sim/tools/datadog/create_event.ts +++ b/apps/sim/tools/datadog/create_event.ts @@ -4,7 +4,7 @@ import type { EventAlertType, EventPriority, } from '@/tools/datadog/types' -import { datadogErrorMessage } from '@/tools/datadog/utils' +import { datadogErrorMessage, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const createEventTool: ToolConfig = { @@ -88,7 +88,7 @@ export const createEventTool: ToolConfig request: { url: (params) => { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) return `https://api.${site}/api/v1/events` }, method: 'POST', diff --git a/apps/sim/tools/datadog/create_monitor.ts b/apps/sim/tools/datadog/create_monitor.ts index d2d99e3882c..e78e59b7181 100644 --- a/apps/sim/tools/datadog/create_monitor.ts +++ b/apps/sim/tools/datadog/create_monitor.ts @@ -1,5 +1,5 @@ import type { CreateMonitorParams, CreateMonitorResponse, MonitorType } from '@/tools/datadog/types' -import { datadogErrorMessage, parseJsonParam } from '@/tools/datadog/utils' +import { datadogErrorMessage, parseJsonParam, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const createMonitorTool: ToolConfig = { @@ -77,7 +77,7 @@ export const createMonitorTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) return `https://api.${site}/api/v1/monitor` }, method: 'POST', diff --git a/apps/sim/tools/datadog/datadog.test.ts b/apps/sim/tools/datadog/datadog.test.ts index 80ca5c89b51..302cb2645db 100644 --- a/apps/sim/tools/datadog/datadog.test.ts +++ b/apps/sim/tools/datadog/datadog.test.ts @@ -3,6 +3,7 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' import { executeUpdateSloOperation } from '@/lib/internal/datadog/operations/update-slo' +import * as datadogTools from '@/tools/datadog' import { cancelDowntimeTool } from '@/tools/datadog/cancel_downtime' import { createDowntimeTool } from '@/tools/datadog/create_downtime' import { createEventTool } from '@/tools/datadog/create_event' @@ -652,3 +653,60 @@ describe('datadog site is validated before it reaches the request host', () => { ).toThrow(/Datadog "site" must be one of/) }) }) + +describe('every Datadog tool routes its host through the allowlist', () => { + /* + * The first pass guarded only the shared `datadogApiUrl`; ten tools built the + * host inline from `params.site` and bypassed it entirely. Sweeping the registry + * rather than listing tools means a new tool that reintroduces an inline builder + * fails here instead of shipping an unguarded credentialed request. + */ + const REQUIRED = { + monitorId: '1', + downtimeId: '1', + dashboardId: 'abc-def-ghi', + incidentId: '1', + sloId: 'abc', + signalId: 'abc', + testId: 'abc', + publicId: 'abc', + resultId: 'abc', + query: 'x', + from: '1', + to: '2', + logs: '[]', + metrics: '[]', + series: '[]', + title: 't', + text: 't', + name: 'n', + type: 'metric alert', + scope: '*', + start: '1', + end: '2', + testIds: 'a', + } + + it('rejects an attacker-chosen site in every tool that builds a URL', () => { + const builders = Object.values(datadogTools).filter( + (tool) => typeof tool?.request?.url === 'function' + ) + expect(builders.length).toBeGreaterThan(20) + + const unguarded: string[] = [] + for (const tool of builders) { + const params = { ...REQUIRED, apiKey: 'k', applicationKey: 'a', site: 'evil.com' } + try { + const url = String((tool.request as { url: (p: unknown) => string }).url(params)) + if (!/^https:\/\/(api|http-intake\.logs)\.(datadoghq\.com|datadoghq\.eu)/.test(url)) { + unguarded.push(`${tool.id} -> ${url}`) + } + } catch (error) { + if (!/Datadog "site" must be one of/.test(String(error))) { + unguarded.push(`${tool.id} -> ${String(error)}`) + } + } + } + expect(unguarded).toEqual([]) + }) +}) diff --git a/apps/sim/tools/datadog/get_monitor.ts b/apps/sim/tools/datadog/get_monitor.ts index 2cd3fdab2a4..7d2ace45106 100644 --- a/apps/sim/tools/datadog/get_monitor.ts +++ b/apps/sim/tools/datadog/get_monitor.ts @@ -1,5 +1,5 @@ import type { GetMonitorParams, GetMonitorResponse } from '@/tools/datadog/types' -import { datadogErrorMessage, datadogPathSegment } from '@/tools/datadog/utils' +import { datadogErrorMessage, datadogPathSegment, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const getMonitorTool: ToolConfig = { @@ -50,7 +50,7 @@ export const getMonitorTool: ToolConfig = request: { url: (params) => { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) const queryParams = new URLSearchParams() if (params.groupStates) queryParams.set('group_states', params.groupStates) diff --git a/apps/sim/tools/datadog/list_downtimes.ts b/apps/sim/tools/datadog/list_downtimes.ts index a8c269fbc1b..610911e76d8 100644 --- a/apps/sim/tools/datadog/list_downtimes.ts +++ b/apps/sim/tools/datadog/list_downtimes.ts @@ -4,7 +4,7 @@ import type { ListDowntimesParams, ListDowntimesResponse, } from '@/tools/datadog/types' -import { datadogErrorMessage } from '@/tools/datadog/utils' +import { datadogErrorMessage, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const listDowntimesTool: ToolConfig = { @@ -55,7 +55,7 @@ export const listDowntimesTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) const queryParams = new URLSearchParams() if (params.currentOnly) queryParams.set('current_only', 'true') diff --git a/apps/sim/tools/datadog/list_monitors.ts b/apps/sim/tools/datadog/list_monitors.ts index ca4ec09a95e..20a192b1e63 100644 --- a/apps/sim/tools/datadog/list_monitors.ts +++ b/apps/sim/tools/datadog/list_monitors.ts @@ -1,5 +1,5 @@ import type { ListMonitorsParams, ListMonitorsResponse, MonitorData } from '@/tools/datadog/types' -import { datadogErrorMessage } from '@/tools/datadog/utils' +import { datadogErrorMessage, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const listMonitorsTool: ToolConfig = { @@ -77,7 +77,7 @@ export const listMonitorsTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) const queryParams = new URLSearchParams() if (params.groupStates) queryParams.set('group_states', params.groupStates) diff --git a/apps/sim/tools/datadog/query_logs.ts b/apps/sim/tools/datadog/query_logs.ts index c668820efb1..60a02dc6898 100644 --- a/apps/sim/tools/datadog/query_logs.ts +++ b/apps/sim/tools/datadog/query_logs.ts @@ -4,7 +4,7 @@ import type { QueryLogsParams, QueryLogsResponse, } from '@/tools/datadog/types' -import { datadogErrorMessage } from '@/tools/datadog/utils' +import { datadogErrorMessage, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const queryLogsTool: ToolConfig = { @@ -83,7 +83,7 @@ export const queryLogsTool: ToolConfig = { request: { url: (params) => { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) return `https://api.${site}/api/v2/logs/events/search` }, method: 'POST', diff --git a/apps/sim/tools/datadog/query_timeseries.ts b/apps/sim/tools/datadog/query_timeseries.ts index ee4be02dfc2..7ec5017ca2b 100644 --- a/apps/sim/tools/datadog/query_timeseries.ts +++ b/apps/sim/tools/datadog/query_timeseries.ts @@ -3,7 +3,7 @@ import type { QueryTimeseriesParams, QueryTimeseriesResponse, } from '@/tools/datadog/types' -import { datadogErrorMessage } from '@/tools/datadog/utils' +import { datadogErrorMessage, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' export const queryTimeseriesTool: ToolConfig = { @@ -55,7 +55,7 @@ export const queryTimeseriesTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) const queryParams = new URLSearchParams({ query: params.query, from: String(params.from), diff --git a/apps/sim/tools/datadog/submit_metrics.ts b/apps/sim/tools/datadog/submit_metrics.ts index 7b0155f2e01..39723fba90c 100644 --- a/apps/sim/tools/datadog/submit_metrics.ts +++ b/apps/sim/tools/datadog/submit_metrics.ts @@ -4,7 +4,7 @@ import type { SubmitMetricsParams, SubmitMetricsResponse, } from '@/tools/datadog/types' -import { datadogErrorMessage, parseJsonParam } from '@/tools/datadog/utils' +import { datadogErrorMessage, parseJsonParam, resolveDatadogSite } from '@/tools/datadog/utils' import type { ToolConfig } from '@/tools/types' /** @@ -49,7 +49,7 @@ export const submitMetricsTool: ToolConfig { - const site = params.site || 'datadoghq.com' + const site = resolveDatadogSite(params.site) return `https://api.${site}/api/v2/series` }, method: 'POST', diff --git a/apps/sim/tools/managed_agent/normalizers.ts b/apps/sim/tools/managed_agent/normalizers.ts index 0b45be961b6..c31ce6f481b 100644 --- a/apps/sim/tools/managed_agent/normalizers.ts +++ b/apps/sim/tools/managed_agent/normalizers.ts @@ -78,6 +78,28 @@ export function normalizeFiles(value: unknown): Array<{ fileId: string; mountPat * string, comma-separated string, or single string — into a trimmed * `string[]`. */ +/** + * Trims a scalar the block may hand over as something other than a string. + * + * `String(value)` and `value.toString()` are both unsafe here: an object whose + * `toString` is not a function, and one with a null prototype, each throw + * `TypeError` rather than producing text. Stored workflow state can hold either, + * and these values are read before the operation's try block, so a throw escapes + * the structured `success: false` result the caller is promised. Only the scalar + * kinds `String()` can never fail on are converted; anything else becomes empty, + * which every caller already treats as "not supplied". + * + * @param value - The raw parameter value. + * @returns The trimmed text, or `''` for a value with no safe text form. + */ +export function normalizeScalarText(value: unknown): string { + if (typeof value === 'string') return value.trim() + if (typeof value === 'number' || typeof value === 'bigint' || typeof value === 'boolean') { + return String(value).trim() + } + return '' +} + export function normalizeStringList(value: unknown): string[] { if (Array.isArray(value)) { return value diff --git a/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts b/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts index c3ad4a383f6..e868cdbaa46 100644 --- a/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts +++ b/apps/sim/tools/managed_agent/respond_tool_confirmation.test.ts @@ -40,6 +40,34 @@ describe('Managed Agent tool confirmations', () => { }) }) + it('returns a structured failure for a value with no safe text form', async () => { + const hostile = { toString: 'not a function' } + const result = await executeManagedAgentRespondToolConfirmationOperation({ + accessToken: 'token', + sessionId: 'session-1', + toolUseIds: ['tool-use-1'], + decision: 'deny', + denyMessage: hostile, + } as never) + expect(result.success).toBe(true) + expect(mockSendToolConfirmations).toHaveBeenLastCalledWith({ + apiKey: 'token', + sessionId: 'session-1', + confirmations: [{ toolUseId: 'tool-use-1', result: 'deny' }], + }) + }) + + it('rejects a null-prototype decision instead of throwing past the result contract', async () => { + const result = await executeManagedAgentRespondToolConfirmationOperation({ + accessToken: 'token', + sessionId: 'session-1', + toolUseIds: ['tool-use-1'], + decision: Object.create(null), + } as never) + expect(result.success).toBe(false) + expect(result.error).toMatch(/Decision must be/) + }) + it('sends a denial message only for deny decisions', async () => { await executeManagedAgentRespondToolConfirmationOperation({ accessToken: 'token',