From c04227cba4f2dfe9370b3bbc0279710221db9337 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 20:21:47 -0700 Subject: [PATCH 1/7] fix(attio): stop swallowing JSON parse failures and guard path segments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two integrity defects in the Attio tools. Silent data substitution: 13 parse sites across 9 tools caught a `JSON.parse` failure and substituted an empty value (`{}` / `[]`) or the raw unparsed string, then reported success. The worst was `attio_query_list_entries` — a filter the caller set was silently dropped, the unfiltered query ran against the whole list, and the tool reported success, so the caller received rows they never asked for with no signal their filter was discarded. All now throw the folder's established `Invalid JSON provided for …` error, matching `create_record` / `assert_record` / `list_records`. Path traversal: 43 sites across 31 tools interpolated an LLM-writable id straight into the request path. `encodeURIComponent` is not sufficient — `.` and `..` are unreserved, so they survive encoding and the URL parser then removes them as dot segments, popping a path segment on a fixed host with the caller's Attio bearer token still attached, including on DELETE. Every path segment now goes through `safeUrlPathSegment`, which rejects rather than encodes. `assert_record` additionally built its `matching_attribute` query value by raw interpolation; it now uses `URLSearchParams`. Adds `path_safety.test.ts` and `json_integrity.test.ts`. Both enumerate the tools from the barrel and discover their own cases, so a new unguarded path param or a new swallowed parse site fails CI without anyone remembering to extend a list. Every URL assertion resolves the built URL with `new URL(...)` rather than string-matching the template. --- apps/sim/tools/attio/assert_record.ts | 9 +- apps/sim/tools/attio/create_attribute.ts | 3 +- apps/sim/tools/attio/create_list.ts | 2 +- apps/sim/tools/attio/create_list_entry.ts | 6 +- apps/sim/tools/attio/create_record.ts | 4 +- apps/sim/tools/attio/create_task.ts | 4 +- apps/sim/tools/attio/create_webhook.ts | 2 +- apps/sim/tools/attio/delete_comment.ts | 4 +- apps/sim/tools/attio/delete_list_entry.ts | 3 +- apps/sim/tools/attio/delete_note.ts | 4 +- apps/sim/tools/attio/delete_record.ts | 3 +- apps/sim/tools/attio/delete_task.ts | 4 +- apps/sim/tools/attio/delete_webhook.ts | 4 +- apps/sim/tools/attio/get_attribute.ts | 3 +- apps/sim/tools/attio/get_comment.ts | 4 +- apps/sim/tools/attio/get_list.ts | 3 +- apps/sim/tools/attio/get_list_entry.ts | 3 +- apps/sim/tools/attio/get_member.ts | 4 +- apps/sim/tools/attio/get_note.ts | 4 +- apps/sim/tools/attio/get_object.ts | 4 +- apps/sim/tools/attio/get_record.ts | 3 +- apps/sim/tools/attio/get_task.ts | 4 +- apps/sim/tools/attio/get_thread.ts | 4 +- apps/sim/tools/attio/get_webhook.ts | 4 +- apps/sim/tools/attio/json_integrity.test.ts | 170 ++++++++++++++++++ apps/sim/tools/attio/list_attributes.ts | 3 +- apps/sim/tools/attio/list_records.ts | 4 +- apps/sim/tools/attio/path_safety.test.ts | 187 ++++++++++++++++++++ apps/sim/tools/attio/query_list_entries.ts | 8 +- apps/sim/tools/attio/update_attribute.ts | 3 +- apps/sim/tools/attio/update_list.ts | 5 +- apps/sim/tools/attio/update_list_entry.ts | 5 +- apps/sim/tools/attio/update_object.ts | 4 +- apps/sim/tools/attio/update_record.ts | 3 +- apps/sim/tools/attio/update_task.ts | 8 +- apps/sim/tools/attio/update_webhook.ts | 6 +- 36 files changed, 454 insertions(+), 44 deletions(-) create mode 100644 apps/sim/tools/attio/json_integrity.test.ts create mode 100644 apps/sim/tools/attio/path_safety.test.ts diff --git a/apps/sim/tools/attio/assert_record.ts b/apps/sim/tools/attio/assert_record.ts index 71707cdae99..3fd70553dbe 100644 --- a/apps/sim/tools/attio/assert_record.ts +++ b/apps/sim/tools/attio/assert_record.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioAssertRecordParams, AttioAssertRecordResponse } from './types' import { RECORD_OUTPUT_PROPERTIES } from './types' @@ -48,8 +49,12 @@ export const attioAssertRecordTool: ToolConfig - `https://api.attio.com/v2/objects/${params.objectType.trim()}/records?matching_attribute=${params.matchingAttribute.trim()}`, + url: (params) => { + const searchParams = new URLSearchParams({ + matching_attribute: String(params.matchingAttribute ?? '').trim(), + }) + return `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records?${searchParams.toString()}` + }, method: 'PUT', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/create_attribute.ts b/apps/sim/tools/attio/create_attribute.ts index fe362358658..f2e167a859b 100644 --- a/apps/sim/tools/attio/create_attribute.ts +++ b/apps/sim/tools/attio/create_attribute.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioCreateAttributeParams, AttioCreateAttributeResponse } from './types' import { ATTRIBUTE_OUTPUT_PROPERTIES } from './types' @@ -92,7 +93,7 @@ export const attioCreateAttributeTool: ToolConfig< request: { url: (params) => - `https://api.attio.com/v2/${params.target.trim()}/${params.identifier.trim()}/attributes`, + `https://api.attio.com/v2/${safeUrlPathSegment(params.target, 'target')}/${safeUrlPathSegment(params.identifier, 'identifier')}/attributes`, method: 'POST', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/create_list.ts b/apps/sim/tools/attio/create_list.ts index eb2d3447222..1f19f167c36 100644 --- a/apps/sim/tools/attio/create_list.ts +++ b/apps/sim/tools/attio/create_list.ts @@ -85,7 +85,7 @@ export const attioCreateListTool: ToolConfig `https://api.attio.com/v2/lists/${params.list.trim()}/entries`, + url: (params) => + `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}/entries`, method: 'POST', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -68,7 +70,7 @@ export const attioCreateListEntryTool: ToolConfig< ? JSON.parse(params.entryValues) : params.entryValues } catch { - entryValues = {} + throw new Error('Invalid JSON provided for entry values') } } const data: Record = { diff --git a/apps/sim/tools/attio/create_record.ts b/apps/sim/tools/attio/create_record.ts index 0594371ec0b..97a59b7ae21 100644 --- a/apps/sim/tools/attio/create_record.ts +++ b/apps/sim/tools/attio/create_record.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioCreateRecordParams, AttioCreateRecordResponse } from './types' import { RECORD_OBJECT_OUTPUT } from './types' @@ -39,7 +40,8 @@ export const attioCreateRecordTool: ToolConfig `https://api.attio.com/v2/objects/${params.objectType.trim()}/records`, + url: (params) => + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records`, method: 'POST', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/create_task.ts b/apps/sim/tools/attio/create_task.ts index fb2172b78d1..aa2ac5b1ce1 100644 --- a/apps/sim/tools/attio/create_task.ts +++ b/apps/sim/tools/attio/create_task.ts @@ -75,7 +75,7 @@ export const attioCreateTaskTool: ToolConfig `https://api.attio.com/v2/comments/${params.commentId.trim()}`, + url: (params) => + `https://api.attio.com/v2/comments/${safeUrlPathSegment(params.commentId, 'commentId')}`, method: 'DELETE', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/delete_list_entry.ts b/apps/sim/tools/attio/delete_list_entry.ts index e46da4d42ee..ab5cfb47fb4 100644 --- a/apps/sim/tools/attio/delete_list_entry.ts +++ b/apps/sim/tools/attio/delete_list_entry.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioDeleteListEntryParams, AttioDeleteListEntryResponse } from './types' const logger = createLogger('AttioDeleteListEntry') @@ -41,7 +42,7 @@ export const attioDeleteListEntryTool: ToolConfig< request: { url: (params) => - `https://api.attio.com/v2/lists/${params.list.trim()}/entries/${params.entryId.trim()}`, + `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}/entries/${safeUrlPathSegment(params.entryId, 'entryId')}`, method: 'DELETE', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/delete_note.ts b/apps/sim/tools/attio/delete_note.ts index bd5cfe1a05d..2dd75e53a1a 100644 --- a/apps/sim/tools/attio/delete_note.ts +++ b/apps/sim/tools/attio/delete_note.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioDeleteNoteParams, AttioDeleteNoteResponse } from './types' const logger = createLogger('AttioDeleteNote') @@ -31,7 +32,8 @@ export const attioDeleteNoteTool: ToolConfig `https://api.attio.com/v2/notes/${params.noteId.trim()}`, + url: (params) => + `https://api.attio.com/v2/notes/${safeUrlPathSegment(params.noteId, 'noteId')}`, method: 'DELETE', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/delete_record.ts b/apps/sim/tools/attio/delete_record.ts index cc28440030e..8628dbbe2f3 100644 --- a/apps/sim/tools/attio/delete_record.ts +++ b/apps/sim/tools/attio/delete_record.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioDeleteRecordParams, AttioDeleteRecordResponse } from './types' const logger = createLogger('AttioDeleteRecord') @@ -39,7 +40,7 @@ export const attioDeleteRecordTool: ToolConfig - `https://api.attio.com/v2/objects/${params.objectType.trim()}/records/${params.recordId.trim()}`, + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records/${safeUrlPathSegment(params.recordId, 'recordId')}`, method: 'DELETE', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/delete_task.ts b/apps/sim/tools/attio/delete_task.ts index 088f74707c3..3e0a24c9312 100644 --- a/apps/sim/tools/attio/delete_task.ts +++ b/apps/sim/tools/attio/delete_task.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioDeleteTaskParams, AttioDeleteTaskResponse } from './types' const logger = createLogger('AttioDeleteTask') @@ -31,7 +32,8 @@ export const attioDeleteTaskTool: ToolConfig `https://api.attio.com/v2/tasks/${params.taskId.trim()}`, + url: (params) => + `https://api.attio.com/v2/tasks/${safeUrlPathSegment(params.taskId, 'taskId')}`, method: 'DELETE', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/delete_webhook.ts b/apps/sim/tools/attio/delete_webhook.ts index eaacde34eb6..d75610ec9ef 100644 --- a/apps/sim/tools/attio/delete_webhook.ts +++ b/apps/sim/tools/attio/delete_webhook.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioDeleteWebhookParams, AttioDeleteWebhookResponse } from './types' const logger = createLogger('AttioDeleteWebhook') @@ -34,7 +35,8 @@ export const attioDeleteWebhookTool: ToolConfig< }, request: { - url: (params) => `https://api.attio.com/v2/webhooks/${params.webhookId.trim()}`, + url: (params) => + `https://api.attio.com/v2/webhooks/${safeUrlPathSegment(params.webhookId, 'webhookId')}`, method: 'DELETE', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_attribute.ts b/apps/sim/tools/attio/get_attribute.ts index 79e6af05e99..07161e8d2a5 100644 --- a/apps/sim/tools/attio/get_attribute.ts +++ b/apps/sim/tools/attio/get_attribute.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetAttributeParams, AttioGetAttributeResponse } from './types' import { ATTRIBUTE_OUTPUT_PROPERTIES } from './types' @@ -46,7 +47,7 @@ export const attioGetAttributeTool: ToolConfig - `https://api.attio.com/v2/${params.target.trim()}/${params.identifier.trim()}/attributes/${params.attribute.trim()}`, + `https://api.attio.com/v2/${safeUrlPathSegment(params.target, 'target')}/${safeUrlPathSegment(params.identifier, 'identifier')}/attributes/${safeUrlPathSegment(params.attribute, 'attribute')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_comment.ts b/apps/sim/tools/attio/get_comment.ts index 3bfa823697c..2fe675b5e43 100644 --- a/apps/sim/tools/attio/get_comment.ts +++ b/apps/sim/tools/attio/get_comment.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetCommentParams, AttioGetCommentResponse } from './types' import { COMMENT_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetCommentTool: ToolConfig `https://api.attio.com/v2/comments/${params.commentId.trim()}`, + url: (params) => + `https://api.attio.com/v2/comments/${safeUrlPathSegment(params.commentId, 'commentId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_list.ts b/apps/sim/tools/attio/get_list.ts index 440b28e81c2..20f2cde62de 100644 --- a/apps/sim/tools/attio/get_list.ts +++ b/apps/sim/tools/attio/get_list.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetListParams, AttioGetListResponse } from './types' import { LIST_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,7 @@ export const attioGetListTool: ToolConfig `https://api.attio.com/v2/lists/${params.list.trim()}`, + url: (params) => `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_list_entry.ts b/apps/sim/tools/attio/get_list_entry.ts index 22aa1c41481..0856a8e7884 100644 --- a/apps/sim/tools/attio/get_list_entry.ts +++ b/apps/sim/tools/attio/get_list_entry.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetListEntryParams, AttioGetListEntryResponse } from './types' import { LIST_ENTRY_OUTPUT_PROPERTIES } from './types' @@ -40,7 +41,7 @@ export const attioGetListEntryTool: ToolConfig - `https://api.attio.com/v2/lists/${params.list.trim()}/entries/${params.entryId.trim()}`, + `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}/entries/${safeUrlPathSegment(params.entryId, 'entryId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_member.ts b/apps/sim/tools/attio/get_member.ts index 11f2c08c07d..a06d632d3ff 100644 --- a/apps/sim/tools/attio/get_member.ts +++ b/apps/sim/tools/attio/get_member.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetMemberParams, AttioGetMemberResponse } from './types' import { MEMBER_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetMemberTool: ToolConfig `https://api.attio.com/v2/workspace_members/${params.memberId.trim()}`, + url: (params) => + `https://api.attio.com/v2/workspace_members/${safeUrlPathSegment(params.memberId, 'memberId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_note.ts b/apps/sim/tools/attio/get_note.ts index b253e975bc0..68c3ab21fd9 100644 --- a/apps/sim/tools/attio/get_note.ts +++ b/apps/sim/tools/attio/get_note.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetNoteParams, AttioGetNoteResponse } from './types' import { mapNoteTags, NOTE_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetNoteTool: ToolConfig `https://api.attio.com/v2/notes/${params.noteId.trim()}`, + url: (params) => + `https://api.attio.com/v2/notes/${safeUrlPathSegment(params.noteId, 'noteId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_object.ts b/apps/sim/tools/attio/get_object.ts index 09ecc9c267f..dd3a752c602 100644 --- a/apps/sim/tools/attio/get_object.ts +++ b/apps/sim/tools/attio/get_object.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetObjectParams, AttioGetObjectResponse } from './types' import { OBJECT_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetObjectTool: ToolConfig `https://api.attio.com/v2/objects/${params.object.trim()}`, + url: (params) => + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.object, 'object')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_record.ts b/apps/sim/tools/attio/get_record.ts index 2ae04ddbad7..093b50042f6 100644 --- a/apps/sim/tools/attio/get_record.ts +++ b/apps/sim/tools/attio/get_record.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetRecordParams, AttioGetRecordResponse } from './types' import { RECORD_OBJECT_OUTPUT } from './types' @@ -39,7 +40,7 @@ export const attioGetRecordTool: ToolConfig - `https://api.attio.com/v2/objects/${params.objectType.trim()}/records/${params.recordId.trim()}`, + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records/${safeUrlPathSegment(params.recordId, 'recordId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_task.ts b/apps/sim/tools/attio/get_task.ts index 4660c48f4b6..12e1a150b00 100644 --- a/apps/sim/tools/attio/get_task.ts +++ b/apps/sim/tools/attio/get_task.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetTaskParams, AttioGetTaskResponse } from './types' import { TASK_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetTaskTool: ToolConfig `https://api.attio.com/v2/tasks/${params.taskId.trim()}`, + url: (params) => + `https://api.attio.com/v2/tasks/${safeUrlPathSegment(params.taskId, 'taskId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_thread.ts b/apps/sim/tools/attio/get_thread.ts index 62f0a47f1d7..0619d396086 100644 --- a/apps/sim/tools/attio/get_thread.ts +++ b/apps/sim/tools/attio/get_thread.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetThreadParams, AttioGetThreadResponse } from './types' import { THREAD_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetThreadTool: ToolConfig `https://api.attio.com/v2/threads/${params.threadId.trim()}`, + url: (params) => + `https://api.attio.com/v2/threads/${safeUrlPathSegment(params.threadId, 'threadId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/get_webhook.ts b/apps/sim/tools/attio/get_webhook.ts index 7cc76f9e1dc..c6fdf0eb24a 100644 --- a/apps/sim/tools/attio/get_webhook.ts +++ b/apps/sim/tools/attio/get_webhook.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioGetWebhookParams, AttioGetWebhookResponse } from './types' import { WEBHOOK_OUTPUT_PROPERTIES } from './types' @@ -32,7 +33,8 @@ export const attioGetWebhookTool: ToolConfig `https://api.attio.com/v2/webhooks/${params.webhookId.trim()}`, + url: (params) => + `https://api.attio.com/v2/webhooks/${safeUrlPathSegment(params.webhookId, 'webhookId')}`, method: 'GET', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/json_integrity.test.ts b/apps/sim/tools/attio/json_integrity.test.ts new file mode 100644 index 00000000000..37b94d905d1 --- /dev/null +++ b/apps/sim/tools/attio/json_integrity.test.ts @@ -0,0 +1,170 @@ +/** + * @vitest-environment node + * + * Guards every Attio tool against silent data substitution when a JSON-encoded + * parameter fails to parse. + * + * Several tools caught the `JSON.parse` failure and substituted an empty value + * (`{}` / `[]`) before reporting success. The worst case was + * `attio_query_list_entries`: a filter the caller set was silently dropped, the + * **unfiltered** query ran against the whole list, and the tool reported + * success — so the caller received rows they never asked for and had no signal + * that their filter was discarded. + * + * The established pattern in this folder (`create_record`, `assert_record`, + * `update_record`, `create_attribute`, `update_attribute`, `list_records`) is + * to throw a named `Invalid JSON provided for …` error. This file pins that + * pattern for every tool, and the sweep below is written so that a NEW tool + * that swallows a parse failure fails CI without anyone remembering to add it. + */ +import { describe, expect, it } from 'vitest' +import * as attioTools from '@/tools/attio/index' +import type { ToolConfig } from '@/tools/types' + +type AnyTool = ToolConfig + +const SENTINEL = '__ATTIO_SENTINEL__' +const VALID_JSON_WITH_SENTINEL = `["${SENTINEL}"]` +const MALFORMED_JSON_WITH_SENTINEL = `["${SENTINEL}"` + +/** A valid JSON object that is also a harmless plain-string value. */ +const NEUTRAL_FILLER = '{}' + +function isAttioTool(value: unknown): value is AnyTool { + return ( + typeof value === 'object' && + value !== null && + typeof (value as AnyTool).id === 'string' && + (value as AnyTool).id.startsWith('attio_') + ) +} + +const BODY_TOOLS = Object.values(attioTools) + .filter(isAttioTool) + .filter((tool) => typeof tool.request?.body === 'function') + +function stringParamNames(tool: AnyTool): string[] { + return Object.entries(tool.params ?? {}) + .filter(([name]) => name !== 'accessToken') + .filter(([, def]) => { + const type = (def as { type?: string }).type + return type === undefined || type === 'string' || type === 'json' + }) + .map(([name]) => name) +} + +function buildParams(tool: AnyTool, overrideName: string, overrideValue: string) { + const params: Record = { accessToken: 'token' } + for (const [name, def] of Object.entries(tool.params ?? {})) { + if (name === 'accessToken') continue + const type = (def as { type?: string }).type + if (type === 'number') { + params[name] = 1 + } else if (type === 'boolean') { + params[name] = false + } else { + params[name] = NEUTRAL_FILLER + } + } + params[overrideName] = overrideValue + return params +} + +function serializeBody(tool: AnyTool, overrideName: string, overrideValue: string): string { + const body = tool.request?.body + if (typeof body !== 'function') throw new Error(`${tool.id} has no body builder`) + return JSON.stringify(body(buildParams(tool, overrideName, overrideValue) as any)) +} + +/** + * Params that actually reach the request body, discovered by feeding a valid + * JSON value carrying a sentinel and checking whether the sentinel survives. + * Params that only feed the URL are skipped rather than hard-coded. + */ +const BODY_PARAM_CASES = BODY_TOOLS.flatMap((tool) => + stringParamNames(tool) + .filter((name) => { + try { + return serializeBody(tool, name, VALID_JSON_WITH_SENTINEL).includes(SENTINEL) + } catch { + return false + } + }) + .map((param) => ({ name: `${tool.id} / ${param}`, tool, param })) +) + +describe('attio JSON parse integrity', () => { + it('discovers the body-bound params it is meant to cover', () => { + expect(BODY_PARAM_CASES.length).toBeGreaterThanOrEqual(20) + }) + + it.each(BODY_PARAM_CASES)( + '$name never silently drops a value that fails to parse', + ({ tool, param }) => { + let serialized: string + try { + serialized = serializeBody(tool, param, MALFORMED_JSON_WITH_SENTINEL) + } catch { + return + } + + expect(serialized).toContain(SENTINEL) + } + ) +}) + +/** + * The parse sites confirmed on staging as swallowing the failure. Each must now + * throw the folder's named error rather than substituting an empty value. + */ +const SWALLOWED_SITES: ReadonlyArray<{ id: string; param: string }> = [ + { id: 'attio_create_list_entry', param: 'entryValues' }, + { id: 'attio_update_list_entry', param: 'entryValues' }, + { id: 'attio_create_task', param: 'linkedRecords' }, + { id: 'attio_create_task', param: 'assignees' }, + { id: 'attio_update_task', param: 'linkedRecords' }, + { id: 'attio_update_task', param: 'assignees' }, + { id: 'attio_create_webhook', param: 'subscriptions' }, + { id: 'attio_update_webhook', param: 'subscriptions' }, + { id: 'attio_create_list', param: 'workspaceMemberAccess' }, + { id: 'attio_update_list', param: 'workspaceMemberAccess' }, + { id: 'attio_query_list_entries', param: 'filter' }, + { id: 'attio_query_list_entries', param: 'sorts' }, +] + +function toolById(id: string): AnyTool { + const tool = BODY_TOOLS.find((candidate) => candidate.id === id) + if (!tool) throw new Error(`${id} is not an Attio tool with a body builder`) + return tool +} + +describe.each(SWALLOWED_SITES)('$id / $param', ({ id, param }) => { + it('throws a named Invalid JSON error instead of substituting an empty value', () => { + expect(() => serializeBody(toolById(id), param, '{"broken":')).toThrow(/Invalid JSON provided/) + }) + + it('still accepts a well-formed value', () => { + expect(() => serializeBody(toolById(id), param, VALID_JSON_WITH_SENTINEL)).not.toThrow() + }) +}) + +/** + * `attio_query_list_entries` is called out on its own because its failure mode + * is the most damaging: dropping the filter widens the query rather than + * narrowing it, so the caller gets MORE data than they asked for. + */ +describe('attio_query_list_entries filter integrity', () => { + const tool = toolById('attio_query_list_entries') + + it('never runs an unfiltered query when the filter fails to parse', () => { + expect(() => serializeBody(tool, 'filter', '{"name":{"$eq":"acme"')).toThrow( + /Invalid JSON provided for filter/ + ) + }) + + it('forwards a well-formed filter verbatim', () => { + const serialized = serializeBody(tool, 'filter', '{"name":{"$eq":"acme"}}') + + expect(JSON.parse(serialized).filter).toEqual({ name: { $eq: 'acme' } }) + }) +}) diff --git a/apps/sim/tools/attio/list_attributes.ts b/apps/sim/tools/attio/list_attributes.ts index 3c950e92839..413e41b2e67 100644 --- a/apps/sim/tools/attio/list_attributes.ts +++ b/apps/sim/tools/attio/list_attributes.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioListAttributesParams, AttioListAttributesResponse } from './types' import { ATTRIBUTE_OUTPUT_PROPERTIES } from './types' @@ -66,7 +67,7 @@ export const attioListAttributesTool: ToolConfig< if (params.showArchived != null) searchParams.set('show_archived', String(params.showArchived)) const qs = searchParams.toString() - return `https://api.attio.com/v2/${params.target.trim()}/${params.identifier.trim()}/attributes${qs ? `?${qs}` : ''}` + return `https://api.attio.com/v2/${safeUrlPathSegment(params.target, 'target')}/${safeUrlPathSegment(params.identifier, 'identifier')}/attributes${qs ? `?${qs}` : ''}` }, method: 'GET', headers: (params) => ({ diff --git a/apps/sim/tools/attio/list_records.ts b/apps/sim/tools/attio/list_records.ts index 49b917a0f18..f1a185bf38e 100644 --- a/apps/sim/tools/attio/list_records.ts +++ b/apps/sim/tools/attio/list_records.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioListRecordsParams, AttioListRecordsResponse } from './types' import { RECORDS_ARRAY_OUTPUT } from './types' @@ -56,7 +57,8 @@ export const attioListRecordsTool: ToolConfig `https://api.attio.com/v2/objects/${params.objectType.trim()}/records/query`, + url: (params) => + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records/query`, method: 'POST', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/path_safety.test.ts b/apps/sim/tools/attio/path_safety.test.ts new file mode 100644 index 00000000000..bdabdeca11f --- /dev/null +++ b/apps/sim/tools/attio/path_safety.test.ts @@ -0,0 +1,187 @@ +/** + * @vitest-environment node + * + * Guards every Attio tool against path traversal through an LLM-writable ID + * that gets interpolated into the request path. + * + * These IDs are `visibility: 'user-or-llm'`, so prompt injection controls them. + * Interpolating one raw let a value like `../../objects/people` escape its + * intended resource once `fetch` normalized the URL, re-aiming the request (and + * the user's Attio bearer token) at an arbitrary Attio resource — including on + * DELETE. `assertRequestUrlMatchesTrust` in `tools/request-transport.ts` only + * applies its canonicalization guard to internal `/api/` routes, so nothing + * downstream catches this. + * + * Wrapping the ID in `encodeURIComponent` is NOT enough, which is why the + * vector list below includes the bare `.` and `..` segments: both are made of + * unreserved characters, so they survive encoding untouched and the URL parser + * then removes them as dot segments, popping one path segment off a fixed host. + * Every assertion here resolves the built URL with `new URL(...)` — the same + * normalization `fetch` performs — rather than string-matching the template + * output, because string matching is exactly what let this through. + */ +import { describe, expect, it } from 'vitest' +import * as attioTools from '@/tools/attio/index' +import type { ToolConfig } from '@/tools/types' + +/** + * The bare `.` and `..` entries are the whole point: their omission is why an + * `encodeURIComponent`-only fix looks correct while the hole stays live. + */ +const TRAVERSAL_IDS = [ + '..', + '.', + ' .. ', + '../../objects/people', + '..%2f..%2fobjects/people', + 'list_abc/../../../objects/people', + 'list_abc?limit=500', + 'list_abc#fragment', + 'list_abc/entries/../../../webhooks', + '\\..\\..', +] as const + +/** Values a real user legitimately supplies; none may be rejected or altered. */ +const LEGITIMATE_IDS = [ + 'people', + 'companies', + 'deals', + 'objects', + 'lists', + 'sales-pipeline', + '2e6d8c1a-6a1a-4b2e-9a6f-1c2d3e4f5a6b', + 'user_email_address', + 'example.com', + 'sub.example.co.uk', + '..foo', + 'foo..', + 'v1.2.3', +] as const + +const SAFE_ID = 'SAFEID' + +type AnyTool = ToolConfig + +function isAttioTool(value: unknown): value is AnyTool { + return ( + typeof value === 'object' && + value !== null && + typeof (value as AnyTool).id === 'string' && + (value as AnyTool).id.startsWith('attio_') + ) +} + +/** + * Builds a param object for a tool, filling every declared string param with + * `value` so whichever one reaches the path is exercised. + */ +function buildParams(tool: AnyTool, value: string): Record { + const params: Record = { accessToken: 'token' } + for (const [name, def] of Object.entries(tool.params ?? {})) { + if (name === 'accessToken') continue + const type = (def as { type?: string }).type + if (type === 'json' || type === 'array' || type === 'object') { + params[name] = [] + } else if (type === 'number') { + params[name] = 1 + } else if (type === 'boolean') { + params[name] = false + } else { + params[name] = value + } + } + return params +} + +function buildUrl(tool: AnyTool, value: string): URL { + const url = tool.request?.url + if (typeof url !== 'function') { + throw new Error(`${tool.id} does not build its URL from params`) + } + return new URL(url(buildParams(tool, value) as any)) +} + +function buildPath(tool: AnyTool, value: string): string { + return buildUrl(tool, value).pathname +} + +function segmentsOf(pathname: string): string[] { + return pathname.split('/') +} + +const DYNAMIC_PATH_TOOLS = Object.values(attioTools) + .filter(isAttioTool) + .filter((tool) => typeof tool.request?.url === 'function') + .filter((tool) => { + try { + return buildPath(tool, SAFE_ID).includes(SAFE_ID) + } catch { + return false + } + }) + .map((tool) => ({ name: tool.id, tool })) + +describe('attio path-ID traversal safety', () => { + it('covers every Attio tool that interpolates an ID into its path', () => { + expect(DYNAMIC_PATH_TOOLS.length).toBeGreaterThanOrEqual(30) + }) + + describe.each(DYNAMIC_PATH_TOOLS)('$name', ({ tool }) => { + const baseline = segmentsOf(buildPath(tool, SAFE_ID)) + + it.each(TRAVERSAL_IDS)('cannot reshape the path with %j', (value) => { + let path: string + try { + path = buildPath(tool, value) + } catch { + return + } + + const actual = segmentsOf(path) + expect(actual).toHaveLength(baseline.length) + baseline.forEach((segment, index) => { + if (segment === SAFE_ID) return + expect(actual[index]).toBe(segment) + }) + }) + + it.each(TRAVERSAL_IDS)('stays on the Attio v2 API with %j', (value) => { + let url: URL + try { + url = buildUrl(tool, value) + } catch { + return + } + + expect(url.origin).toBe('https://api.attio.com') + expect(url.pathname.startsWith('/v2/')).toBe(true) + }) + + it.each(LEGITIMATE_IDS)('passes %j through unchanged', (value) => { + const actual = segmentsOf(buildPath(tool, value)) + + expect(actual).toHaveLength(baseline.length) + baseline.forEach((segment, index) => { + expect(actual[index]).toBe(segment === SAFE_ID ? value : segment) + }) + }) + + it('rejects a bare dot-dot segment by name instead of silently popping a segment', () => { + expect(() => buildUrl(tool, '..')).toThrow(/path traversal/) + }) + + it('rejects a bare dot segment', () => { + expect(() => buildUrl(tool, '.')).toThrow(/path traversal/) + }) + + it('does not let an id inject query parameters', () => { + const url = buildUrl(tool, 'list_abc?limit=500') + + expect(url.searchParams.get('limit')).not.toBe('500') + }) + + it('trims surrounding whitespace rather than encoding it', () => { + expect(buildPath(tool, ' people ')).toBe(buildPath(tool, 'people')) + }) + }) +}) diff --git a/apps/sim/tools/attio/query_list_entries.ts b/apps/sim/tools/attio/query_list_entries.ts index 6b0574d88b9..dabb1e0354d 100644 --- a/apps/sim/tools/attio/query_list_entries.ts +++ b/apps/sim/tools/attio/query_list_entries.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioQueryListEntriesParams, AttioQueryListEntriesResponse } from './types' import { LIST_ENTRY_OUTPUT_PROPERTIES } from './types' @@ -60,7 +61,8 @@ export const attioQueryListEntriesTool: ToolConfig< }, request: { - url: (params) => `https://api.attio.com/v2/lists/${params.list.trim()}/entries/query`, + url: (params) => + `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}/entries/query`, method: 'POST', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -73,14 +75,14 @@ export const attioQueryListEntriesTool: ToolConfig< body.filter = typeof params.filter === 'string' ? JSON.parse(params.filter) : params.filter } catch { - body.filter = {} + throw new Error('Invalid JSON provided for filter') } } if (params.sorts) { try { body.sorts = typeof params.sorts === 'string' ? JSON.parse(params.sorts) : params.sorts } catch { - body.sorts = [] + throw new Error('Invalid JSON provided for sorts') } } if (params.limit != null) body.limit = params.limit diff --git a/apps/sim/tools/attio/update_attribute.ts b/apps/sim/tools/attio/update_attribute.ts index 9cfd9c5d9d5..2e8a93993d5 100644 --- a/apps/sim/tools/attio/update_attribute.ts +++ b/apps/sim/tools/attio/update_attribute.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioUpdateAttributeParams, AttioUpdateAttributeResponse } from './types' import { ATTRIBUTE_OUTPUT_PROPERTIES } from './types' @@ -90,7 +91,7 @@ export const attioUpdateAttributeTool: ToolConfig< request: { url: (params) => - `https://api.attio.com/v2/${params.target.trim()}/${params.identifier.trim()}/attributes/${params.attribute.trim()}`, + `https://api.attio.com/v2/${safeUrlPathSegment(params.target, 'target')}/${safeUrlPathSegment(params.identifier, 'identifier')}/attributes/${safeUrlPathSegment(params.attribute, 'attribute')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/update_list.ts b/apps/sim/tools/attio/update_list.ts index e36f74b2ece..39570755e85 100644 --- a/apps/sim/tools/attio/update_list.ts +++ b/apps/sim/tools/attio/update_list.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioUpdateListParams, AttioUpdateListResponse } from './types' import { LIST_OUTPUT_PROPERTIES } from './types' @@ -58,7 +59,7 @@ export const attioUpdateListTool: ToolConfig `https://api.attio.com/v2/lists/${params.list.trim()}`, + url: (params) => `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -76,7 +77,7 @@ export const attioUpdateListTool: ToolConfig - `https://api.attio.com/v2/lists/${params.list.trim()}/entries/${params.entryId.trim()}`, + `https://api.attio.com/v2/lists/${safeUrlPathSegment(params.list, 'list')}/entries/${safeUrlPathSegment(params.entryId, 'entryId')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -62,7 +63,7 @@ export const attioUpdateListEntryTool: ToolConfig< ? JSON.parse(params.entryValues) : params.entryValues } catch { - entryValues = {} + throw new Error('Invalid JSON provided for entry values') } return { data: { entry_values: entryValues } } }, diff --git a/apps/sim/tools/attio/update_object.ts b/apps/sim/tools/attio/update_object.ts index 0136bae05dd..b6c6e3d6e6e 100644 --- a/apps/sim/tools/attio/update_object.ts +++ b/apps/sim/tools/attio/update_object.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioUpdateObjectParams, AttioUpdateObjectResponse } from './types' import { OBJECT_OUTPUT_PROPERTIES } from './types' @@ -51,7 +52,8 @@ export const attioUpdateObjectTool: ToolConfig `https://api.attio.com/v2/objects/${params.object.trim()}`, + url: (params) => + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.object, 'object')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/update_record.ts b/apps/sim/tools/attio/update_record.ts index 25548f46287..3e247925f35 100644 --- a/apps/sim/tools/attio/update_record.ts +++ b/apps/sim/tools/attio/update_record.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioUpdateRecordParams, AttioUpdateRecordResponse } from './types' import { RECORD_OBJECT_OUTPUT } from './types' @@ -46,7 +47,7 @@ export const attioUpdateRecordTool: ToolConfig - `https://api.attio.com/v2/objects/${params.objectType.trim()}/records/${params.recordId.trim()}`, + `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records/${safeUrlPathSegment(params.recordId, 'recordId')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, diff --git a/apps/sim/tools/attio/update_task.ts b/apps/sim/tools/attio/update_task.ts index 66620fa6eb3..73edde9e873 100644 --- a/apps/sim/tools/attio/update_task.ts +++ b/apps/sim/tools/attio/update_task.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import type { ToolConfig } from '@/tools/types' +import { safeUrlPathSegment } from '@/tools/url-path' import type { AttioUpdateTaskParams, AttioUpdateTaskResponse } from './types' import { TASK_OUTPUT_PROPERTIES } from './types' @@ -56,7 +57,8 @@ export const attioUpdateTaskTool: ToolConfig `https://api.attio.com/v2/tasks/${params.taskId.trim()}`, + url: (params) => + `https://api.attio.com/v2/tasks/${safeUrlPathSegment(params.taskId, 'taskId')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -73,7 +75,7 @@ export const attioUpdateTaskTool: ToolConfig `https://api.attio.com/v2/webhooks/${params.webhookId.trim()}`, + url: (params) => + `https://api.attio.com/v2/webhooks/${safeUrlPathSegment(params.webhookId, 'webhookId')}`, method: 'PATCH', headers: (params) => ({ Authorization: `Bearer ${params.accessToken}`, @@ -63,7 +65,7 @@ export const attioUpdateWebhookTool: ToolConfig< ? JSON.parse(params.subscriptions) : params.subscriptions } catch { - data.subscriptions = [] + throw new Error('Invalid JSON provided for subscriptions') } } return { data } From 5d8ee7185cba5ae72b00e053eed8183632f315c0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 21:11:39 -0700 Subject: [PATCH 2/7] test(attio): fuzz one path param at a time instead of all at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The path-safety test filled every string param with the same fuzz value and skipped the vector when the URL builder threw. That looks equivalent to per-param fuzzing but is not: as soon as one param is guarded, it throws first and its unguarded siblings stop being exercised at all. `attio_get_attribute` carries three path params, so the coarse form reported full coverage while testing exactly one of them — the "a new unguarded param fails CI" property did not hold for any tool that already had one guard. Case discovery is now per (tool, param) pair: each param is probed on its own with every sibling held at a safe value, and each pair gets its own baseline. Discovery finds 43 pairs across 31 tools, matching the 43 guarded call sites one for one. Adds a per-param separator assertion and asserts each rejection names the offending param, so a throw can no longer be credited to the wrong param. Reverting the `identifier` guard in `get_attribute` now fails 15 tests, all scoped to `attio_get_attribute / identifier`; its `target` and `attribute` siblings stay green. --- apps/sim/tools/attio/path_safety.test.ts | 114 ++++++++++++++++------- 1 file changed, 81 insertions(+), 33 deletions(-) diff --git a/apps/sim/tools/attio/path_safety.test.ts b/apps/sim/tools/attio/path_safety.test.ts index bdabdeca11f..4ba0a8ead95 100644 --- a/apps/sim/tools/attio/path_safety.test.ts +++ b/apps/sim/tools/attio/path_safety.test.ts @@ -19,6 +19,15 @@ * Every assertion here resolves the built URL with `new URL(...)` — the same * normalization `fetch` performs — rather than string-matching the template * output, because string matching is exactly what let this through. + * + * **One param is fuzzed at a time**, with every sibling held at a safe value. + * Fuzzing all params at once and skipping the vector when the builder throws + * looks equivalent but is not: as soon as a tool has one guarded param, that + * param throws first and its unguarded siblings stop being exercised at all. + * A tool like `attio_get_attribute` carries three path params, so the coarse + * form would report full coverage while testing exactly one of them. Case + * discovery below is therefore per **(tool, param)** pair, and each pair gets + * its own baseline so an unguarded sibling cannot hide behind a guarded one. */ import { describe, expect, it } from 'vitest' import * as attioTools from '@/tools/attio/index' @@ -58,7 +67,10 @@ const LEGITIMATE_IDS = [ 'v1.2.3', ] as const -const SAFE_ID = 'SAFEID' +/** Held by every param except the one under test. */ +const SIBLING_ID = 'SIBLING' +/** Distinct from `SIBLING_ID` so the param under test is locatable in the path. */ +const PROBE_ID = 'PROBEVALUE' type AnyTool = ToolConfig @@ -72,10 +84,11 @@ function isAttioTool(value: unknown): value is AnyTool { } /** - * Builds a param object for a tool, filling every declared string param with - * `value` so whichever one reaches the path is exercised. + * Builds a param object with every declared param at a safe value, then puts + * `value` on `target` alone. Holding the siblings safe is what keeps a throw + * from `target` attributable to `target`. */ -function buildParams(tool: AnyTool, value: string): Record { +function buildParams(tool: AnyTool, target: string, value: string): Record { const params: Record = { accessToken: 'token' } for (const [name, def] of Object.entries(tool.params ?? {})) { if (name === 'accessToken') continue @@ -87,52 +100,77 @@ function buildParams(tool: AnyTool, value: string): Record { } else if (type === 'boolean') { params[name] = false } else { - params[name] = value + params[name] = SIBLING_ID } } + params[target] = value return params } -function buildUrl(tool: AnyTool, value: string): URL { +function buildUrl(tool: AnyTool, target: string, value: string): URL { const url = tool.request?.url if (typeof url !== 'function') { throw new Error(`${tool.id} does not build its URL from params`) } - return new URL(url(buildParams(tool, value) as any)) + return new URL(url(buildParams(tool, target, value) as any)) } -function buildPath(tool: AnyTool, value: string): string { - return buildUrl(tool, value).pathname +function buildPath(tool: AnyTool, target: string, value: string): string { + return buildUrl(tool, target, value).pathname } function segmentsOf(pathname: string): string[] { return pathname.split('/') } -const DYNAMIC_PATH_TOOLS = Object.values(attioTools) +function stringParamNames(tool: AnyTool): string[] { + return Object.entries(tool.params ?? {}) + .filter(([name]) => name !== 'accessToken') + .filter(([, def]) => { + const type = (def as { type?: string }).type + return type === undefined || type === 'string' || type === 'json' + }) + .map(([name]) => name) +} + +/** + * Every (tool, param) pair whose value reaches the request **path**, discovered + * by probing one param at a time rather than declared in a hand-kept list — a + * new path param is picked up here without anyone editing this file. + */ +const PATH_PARAM_CASES = Object.values(attioTools) .filter(isAttioTool) .filter((tool) => typeof tool.request?.url === 'function') - .filter((tool) => { - try { - return buildPath(tool, SAFE_ID).includes(SAFE_ID) - } catch { - return false - } - }) - .map((tool) => ({ name: tool.id, tool })) + .flatMap((tool) => + stringParamNames(tool) + .filter((param) => { + try { + return buildPath(tool, param, PROBE_ID).includes(PROBE_ID) + } catch { + return false + } + }) + .map((param) => ({ name: `${tool.id} / ${param}`, tool, param })) + ) describe('attio path-ID traversal safety', () => { - it('covers every Attio tool that interpolates an ID into its path', () => { - expect(DYNAMIC_PATH_TOOLS.length).toBeGreaterThanOrEqual(30) + it('covers every (tool, param) pair that reaches a request path', () => { + expect(PATH_PARAM_CASES.length).toBeGreaterThanOrEqual(43) }) - describe.each(DYNAMIC_PATH_TOOLS)('$name', ({ tool }) => { - const baseline = segmentsOf(buildPath(tool, SAFE_ID)) + it('exercises every tool that builds a dynamic path', () => { + const tools = new Set(PATH_PARAM_CASES.map(({ tool }) => tool.id)) + + expect(tools.size).toBeGreaterThanOrEqual(31) + }) + + describe.each(PATH_PARAM_CASES)('$name', ({ tool, param }) => { + const baseline = segmentsOf(buildPath(tool, param, PROBE_ID)) it.each(TRAVERSAL_IDS)('cannot reshape the path with %j', (value) => { let path: string try { - path = buildPath(tool, value) + path = buildPath(tool, param, value) } catch { return } @@ -140,7 +178,7 @@ describe('attio path-ID traversal safety', () => { const actual = segmentsOf(path) expect(actual).toHaveLength(baseline.length) baseline.forEach((segment, index) => { - if (segment === SAFE_ID) return + if (segment === PROBE_ID) return expect(actual[index]).toBe(segment) }) }) @@ -148,7 +186,7 @@ describe('attio path-ID traversal safety', () => { it.each(TRAVERSAL_IDS)('stays on the Attio v2 API with %j', (value) => { let url: URL try { - url = buildUrl(tool, value) + url = buildUrl(tool, param, value) } catch { return } @@ -158,30 +196,40 @@ describe('attio path-ID traversal safety', () => { }) it.each(LEGITIMATE_IDS)('passes %j through unchanged', (value) => { - const actual = segmentsOf(buildPath(tool, value)) + const actual = segmentsOf(buildPath(tool, param, value)) expect(actual).toHaveLength(baseline.length) baseline.forEach((segment, index) => { - expect(actual[index]).toBe(segment === SAFE_ID ? value : segment) + expect(actual[index]).toBe(segment === PROBE_ID ? value : segment) }) }) it('rejects a bare dot-dot segment by name instead of silently popping a segment', () => { - expect(() => buildUrl(tool, '..')).toThrow(/path traversal/) + expect(() => buildUrl(tool, param, '..')).toThrow( + new RegExp(`${param}\\b.*path traversal is not allowed`) + ) + }) + + it('rejects a bare dot segment by name', () => { + expect(() => buildUrl(tool, param, '.')).toThrow( + new RegExp(`${param}\\b.*path traversal is not allowed`) + ) }) - it('rejects a bare dot segment', () => { - expect(() => buildUrl(tool, '.')).toThrow(/path traversal/) + it('rejects a path separator by name', () => { + expect(() => buildUrl(tool, param, 'list_abc/entries')).toThrow( + new RegExp(`${param}\\b.*cannot contain a path separator`) + ) }) - it('does not let an id inject query parameters', () => { - const url = buildUrl(tool, 'list_abc?limit=500') + it('does not let the id inject query parameters', () => { + const url = buildUrl(tool, param, 'list_abc?limit=500') expect(url.searchParams.get('limit')).not.toBe('500') }) it('trims surrounding whitespace rather than encoding it', () => { - expect(buildPath(tool, ' people ')).toBe(buildPath(tool, 'people')) + expect(buildPath(tool, param, ' people ')).toBe(buildPath(tool, param, 'people')) }) }) }) From 601cf68d13a8084239a4e3045b07aef426b113ef Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 21:25:44 -0700 Subject: [PATCH 3/7] test(attio): drop `any` from the tool-harness types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both harnesses carried `ToolConfig` plus an `as any` on the builder call, inherited from the Vercel template they were modelled on. CLAUDE.md forbids `any`, and Greptile has flagged the same shape on sibling PRs. Uses the shape the rest of the batch is standardizing on: type PathTool = ToolConfig, ToolResponse> Params are built as `Record`, so `url(...)` and `body(...)` need no cast at all. The barrel cannot be narrowed by a plain `.filter(guard)`: its element type is a union of `ToolConfig`, and `ToolConfig` places its param type in the contravariant position of `request.url` / `request.body`, so no specific member is assignable to the widened alias. Seeding the source as `readonly unknown[]` makes the existing `isAttioTool` guard the single narrowing point — the `unknown` + type guard that CLAUDE.md prescribes — instead of pushing a cast to every call site. Pure typing change: no behavioural effect, 1724 tests pass unchanged. --- apps/sim/tools/attio/json_integrity.test.ts | 34 +++++++++++++-------- apps/sim/tools/attio/path_safety.test.ts | 31 +++++++++++-------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/apps/sim/tools/attio/json_integrity.test.ts b/apps/sim/tools/attio/json_integrity.test.ts index 37b94d905d1..95e2aeb97e2 100644 --- a/apps/sim/tools/attio/json_integrity.test.ts +++ b/apps/sim/tools/attio/json_integrity.test.ts @@ -19,9 +19,9 @@ */ import { describe, expect, it } from 'vitest' import * as attioTools from '@/tools/attio/index' -import type { ToolConfig } from '@/tools/types' +import type { ToolConfig, ToolResponse } from '@/tools/types' -type AnyTool = ToolConfig +type BodyTool = ToolConfig, ToolResponse> const SENTINEL = '__ATTIO_SENTINEL__' const VALID_JSON_WITH_SENTINEL = `["${SENTINEL}"]` @@ -30,20 +30,28 @@ const MALFORMED_JSON_WITH_SENTINEL = `["${SENTINEL}"` /** A valid JSON object that is also a harmless plain-string value. */ const NEUTRAL_FILLER = '{}' -function isAttioTool(value: unknown): value is AnyTool { +function isAttioTool(value: unknown): value is BodyTool { return ( typeof value === 'object' && value !== null && - typeof (value as AnyTool).id === 'string' && - (value as AnyTool).id.startsWith('attio_') + typeof (value as BodyTool).id === 'string' && + (value as BodyTool).id.startsWith('attio_') ) } -const BODY_TOOLS = Object.values(attioTools) - .filter(isAttioTool) - .filter((tool) => typeof tool.request?.body === 'function') +/** + * Seeded as `unknown[]` so `isAttioTool` is the single narrowing point. The + * barrel's element type is a union of `ToolConfig`, and + * `ToolConfig` places its param type in the contravariant position of + * `request.body`, so no specific member is assignable to `BodyTool` directly. + */ +const ALL_ATTIO_TOOLS: readonly unknown[] = Object.values(attioTools) + +const BODY_TOOLS = ALL_ATTIO_TOOLS.filter(isAttioTool).filter( + (tool) => typeof tool.request?.body === 'function' +) -function stringParamNames(tool: AnyTool): string[] { +function stringParamNames(tool: BodyTool): string[] { return Object.entries(tool.params ?? {}) .filter(([name]) => name !== 'accessToken') .filter(([, def]) => { @@ -53,7 +61,7 @@ function stringParamNames(tool: AnyTool): string[] { .map(([name]) => name) } -function buildParams(tool: AnyTool, overrideName: string, overrideValue: string) { +function buildParams(tool: BodyTool, overrideName: string, overrideValue: string) { const params: Record = { accessToken: 'token' } for (const [name, def] of Object.entries(tool.params ?? {})) { if (name === 'accessToken') continue @@ -70,10 +78,10 @@ function buildParams(tool: AnyTool, overrideName: string, overrideValue: string) return params } -function serializeBody(tool: AnyTool, overrideName: string, overrideValue: string): string { +function serializeBody(tool: BodyTool, overrideName: string, overrideValue: string): string { const body = tool.request?.body if (typeof body !== 'function') throw new Error(`${tool.id} has no body builder`) - return JSON.stringify(body(buildParams(tool, overrideName, overrideValue) as any)) + return JSON.stringify(body(buildParams(tool, overrideName, overrideValue))) } /** @@ -132,7 +140,7 @@ const SWALLOWED_SITES: ReadonlyArray<{ id: string; param: string }> = [ { id: 'attio_query_list_entries', param: 'sorts' }, ] -function toolById(id: string): AnyTool { +function toolById(id: string): BodyTool { const tool = BODY_TOOLS.find((candidate) => candidate.id === id) if (!tool) throw new Error(`${id} is not an Attio tool with a body builder`) return tool diff --git a/apps/sim/tools/attio/path_safety.test.ts b/apps/sim/tools/attio/path_safety.test.ts index 4ba0a8ead95..5502c6885bf 100644 --- a/apps/sim/tools/attio/path_safety.test.ts +++ b/apps/sim/tools/attio/path_safety.test.ts @@ -31,7 +31,7 @@ */ import { describe, expect, it } from 'vitest' import * as attioTools from '@/tools/attio/index' -import type { ToolConfig } from '@/tools/types' +import type { ToolConfig, ToolResponse } from '@/tools/types' /** * The bare `.` and `..` entries are the whole point: their omission is why an @@ -72,14 +72,14 @@ const SIBLING_ID = 'SIBLING' /** Distinct from `SIBLING_ID` so the param under test is locatable in the path. */ const PROBE_ID = 'PROBEVALUE' -type AnyTool = ToolConfig +type PathTool = ToolConfig, ToolResponse> -function isAttioTool(value: unknown): value is AnyTool { +function isAttioTool(value: unknown): value is PathTool { return ( typeof value === 'object' && value !== null && - typeof (value as AnyTool).id === 'string' && - (value as AnyTool).id.startsWith('attio_') + typeof (value as PathTool).id === 'string' && + (value as PathTool).id.startsWith('attio_') ) } @@ -88,7 +88,7 @@ function isAttioTool(value: unknown): value is AnyTool { * `value` on `target` alone. Holding the siblings safe is what keeps a throw * from `target` attributable to `target`. */ -function buildParams(tool: AnyTool, target: string, value: string): Record { +function buildParams(tool: PathTool, target: string, value: string): Record { const params: Record = { accessToken: 'token' } for (const [name, def] of Object.entries(tool.params ?? {})) { if (name === 'accessToken') continue @@ -107,15 +107,15 @@ function buildParams(tool: AnyTool, target: string, value: string): Record name !== 'accessToken') .filter(([, def]) => { @@ -138,8 +138,15 @@ function stringParamNames(tool: AnyTool): string[] { * by probing one param at a time rather than declared in a hand-kept list — a * new path param is picked up here without anyone editing this file. */ -const PATH_PARAM_CASES = Object.values(attioTools) - .filter(isAttioTool) +/** + * Seeded as `unknown[]` so `isAttioTool` is the single narrowing point. The + * barrel's element type is a union of `ToolConfig`, and + * `ToolConfig` places its param type in the contravariant position of + * `request.url`, so no specific member is assignable to `PathTool` directly. + */ +const ALL_ATTIO_TOOLS: readonly unknown[] = Object.values(attioTools) + +const PATH_PARAM_CASES = ALL_ATTIO_TOOLS.filter(isAttioTool) .filter((tool) => typeof tool.request?.url === 'function') .flatMap((tool) => stringParamNames(tool) From 72f18294fbf6e9d6a46d63f957571c68e88caa04 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 21:53:28 -0700 Subject: [PATCH 4/7] test(attio): pin the named-error contract for every JSON-parsed param Addresses a cubic finding on the sweep. It only detected silent empty-value substitution: a tool that forwarded the raw malformed string still contained the sentinel and passed `toContain`, and a tool that threw any error at all was let through by `catch { return }`. Both are Defect-1 violations, so the sweep pinned the contract for the 12 hard-coded sites and nothing else. The sweep now classifies each body-bound param by where a sentinel carried in a valid JSON value lands in the serialized body: as its own string inside an array or object, the tool parsed it; inside a longer string, the tool forwarded it verbatim. - A parsed param must throw `/Invalid JSON provided/` on malformed input. Dropping the value and forwarding it raw now both fail. - A plain-text param must forward its value untouched. Classification is discovered, not declared, so a new JSON param is held to the contract without editing this file. Verified by reverting `create_list` to each violation shape in turn: raw-string forwarding fails 2 tests, and throwing a non-named `Error('bad input')` fails 2 tests. Both passed under the old sweep. --- apps/sim/tools/attio/json_integrity.test.ts | 102 ++++++++++++++++---- 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/apps/sim/tools/attio/json_integrity.test.ts b/apps/sim/tools/attio/json_integrity.test.ts index 95e2aeb97e2..4ef95515798 100644 --- a/apps/sim/tools/attio/json_integrity.test.ts +++ b/apps/sim/tools/attio/json_integrity.test.ts @@ -85,38 +85,100 @@ function serializeBody(tool: BodyTool, overrideName: string, overrideValue: stri } /** - * Params that actually reach the request body, discovered by feeding a valid - * JSON value carrying a sentinel and checking whether the sentinel survives. - * Params that only feed the URL are skipped rather than hard-coded. + * Where a param's value ended up in the serialized body. + * + * - `parsed` — the sentinel appears as its own string inside an array or + * object, so the tool ran `JSON.parse` on the value. + * - `raw` — the sentinel appears only inside a longer string, so the tool + * forwarded the value verbatim. Correct for a plain-text param. + * - `absent` — the value never reaches the body (it feeds the URL instead). + * + * `parsed` wins if both are seen, so a tool that both parses and echoes a value + * is still held to the parsing contract. */ +type SentinelPlacement = 'parsed' | 'raw' | 'absent' + +function classify(body: unknown): SentinelPlacement { + let found: SentinelPlacement = 'absent' + + const walk = (value: unknown): void => { + if (typeof value === 'string') { + if (value === SENTINEL) { + found = 'parsed' + } else if (value.includes(SENTINEL) && found === 'absent') { + found = 'raw' + } + return + } + if (Array.isArray(value)) { + value.forEach(walk) + return + } + if (value !== null && typeof value === 'object') { + Object.values(value).forEach(walk) + } + } + + walk(body) + return found +} + +function placementOf(tool: BodyTool, param: string): SentinelPlacement { + try { + return classify(JSON.parse(serializeBody(tool, param, VALID_JSON_WITH_SENTINEL))) + } catch { + return 'absent' + } +} + const BODY_PARAM_CASES = BODY_TOOLS.flatMap((tool) => stringParamNames(tool) - .filter((name) => { - try { - return serializeBody(tool, name, VALID_JSON_WITH_SENTINEL).includes(SENTINEL) - } catch { - return false - } - }) - .map((param) => ({ name: `${tool.id} / ${param}`, tool, param })) + .map((param) => ({ + name: `${tool.id} / ${param}`, + tool, + param, + placement: placementOf(tool, param), + })) + .filter(({ placement }) => placement !== 'absent') ) +/** + * Params the tool actually runs `JSON.parse` on. A malformed value here must + * raise the folder's named error — substituting an empty value (the original + * defect) and forwarding the raw string (which reaches Attio as a 400 with no + * hint of the real cause) are both failures. + */ +const PARSED_PARAM_CASES = BODY_PARAM_CASES.filter(({ placement }) => placement === 'parsed') + +/** Plain-text params, which must forward whatever they are given untouched. */ +const PASSTHROUGH_PARAM_CASES = BODY_PARAM_CASES.filter(({ placement }) => placement === 'raw') + describe('attio JSON parse integrity', () => { it('discovers the body-bound params it is meant to cover', () => { expect(BODY_PARAM_CASES.length).toBeGreaterThanOrEqual(20) }) - it.each(BODY_PARAM_CASES)( - '$name never silently drops a value that fails to parse', + it('discovers every JSON-parsed param', () => { + expect(PARSED_PARAM_CASES.length).toBeGreaterThanOrEqual(18) + }) + + it.each(PARSED_PARAM_CASES)( + '$name raises the folder-standard error rather than dropping or forwarding a malformed value', ({ tool, param }) => { - let serialized: string - try { - serialized = serializeBody(tool, param, MALFORMED_JSON_WITH_SENTINEL) - } catch { - return - } + expect(() => serializeBody(tool, param, MALFORMED_JSON_WITH_SENTINEL)).toThrow( + /Invalid JSON provided/ + ) + } + ) - expect(serialized).toContain(SENTINEL) + it.each(PARSED_PARAM_CASES)('$name still accepts a well-formed value', ({ tool, param }) => { + expect(() => serializeBody(tool, param, VALID_JSON_WITH_SENTINEL)).not.toThrow() + }) + + it.each(PASSTHROUGH_PARAM_CASES)( + '$name forwards a plain-text value untouched', + ({ tool, param }) => { + expect(serializeBody(tool, param, MALFORMED_JSON_WITH_SENTINEL)).toContain(SENTINEL) } ) }) From 46df148c0e549c2c81c09c7f6e7d9bdede460a69 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 22:09:37 -0700 Subject: [PATCH 5/7] test(attio): assert the probe slot instead of skipping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The traversal loop did `if (segment === PROBE_ID) return`, so it proved the segment count and the surrounding segments but never checked what actually landed in the slot under test. A balanced traversal defeats that: the removed dot segments are matched by added ones, so length and neighbours are unchanged and only the skipped slot differs. `a/../b` is the minimal witness. Against `get_attribute` with the `identifier` guard removed it resolves the probe slot to `b` while leaving every other segment identical, and the old assertion passed it. The slot is now asserted to equal `encodeURIComponent` of the trimmed input, which is exactly what a guarded param emits for any vector it does not reject outright — so `list_abc?limit=500` and `list_abc#fragment` are now pinned to their encoded forms rather than merely to their neighbours. Adds three balanced vectors. Scoped revert of `get_attribute / identifier`: - 10 vectors, slot skipped (before) -> 15 failures - 13 vectors, slot skipped -> 17 failures, `a/../b` still passing - 13 vectors, slot asserted (this commit) -> 18 failures Suite is 2002 green. --- apps/sim/tools/attio/path_safety.test.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/sim/tools/attio/path_safety.test.ts b/apps/sim/tools/attio/path_safety.test.ts index 5502c6885bf..33d288772b7 100644 --- a/apps/sim/tools/attio/path_safety.test.ts +++ b/apps/sim/tools/attio/path_safety.test.ts @@ -20,6 +20,14 @@ * normalization `fetch` performs — rather than string-matching the template * output, because string matching is exactly what let this through. * + * The probe slot is asserted, never skipped. Checking only the segment count + * and the surrounding segments misses a **balanced** traversal, where the + * removed dot segments are matched by added ones: `list_abc/../../lists/victim` + * resolves `/v2/lists/PROBEVALUE` to `/v2/lists/victim` — same length, same + * neighbours, and only the slot the loop used to skip is different. The slot + * must equal `encodeURIComponent` of the trimmed input, which is what a guarded + * param emits for any vector it does not reject outright. + * * **One param is fuzzed at a time**, with every sibling held at a safe value. * Fuzzing all params at once and skipping the vector when the builder throws * looks equivalent but is not: as soon as a tool has one guarded param, that @@ -48,6 +56,9 @@ const TRAVERSAL_IDS = [ 'list_abc#fragment', 'list_abc/entries/../../../webhooks', '\\..\\..', + 'list_abc/../../lists/victim', + 'people/../../objects/victim', + 'a/../b', ] as const /** Values a real user legitimately supplies; none may be rejected or altered. */ @@ -185,8 +196,9 @@ describe('attio path-ID traversal safety', () => { const actual = segmentsOf(path) expect(actual).toHaveLength(baseline.length) baseline.forEach((segment, index) => { - if (segment === PROBE_ID) return - expect(actual[index]).toBe(segment) + expect(actual[index]).toBe( + segment === PROBE_ID ? encodeURIComponent(value.trim()) : segment + ) }) }) From 544658cb536ae2fe4acd57459ef56460ba9e04b8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 22:20:30 -0700 Subject: [PATCH 6/7] fix(attio): reject an empty matching_attribute instead of substituting Addresses a cubic finding on `assert_record`. Moving the query value to `URLSearchParams` left `String(params.matchingAttribute ?? '').trim()`, which turns a missing selector into `matching_attribute=` and sends the upsert anyway. That is the exact substitution this PR argues against everywhere else, and it is a local weakening: the previous `.trim()` at least raised a `TypeError` on `null`/`undefined`. The platform's required-param check makes it unreachable through the normal executor path, but the tool is callable from other surfaces and the guard should not depend on a caller's validation. Now throws `matchingAttribute is required`, matching the message style `safeUrlPathSegment` uses for the path params. Test-first: the four rejection cases (`undefined`, `null`, `''`, `' '`) were watched failing before the fix. The new block also pins that a legitimate selector is encoded unchanged, that whitespace is trimmed rather than encoded, and that the value cannot inject a second query parameter. --- apps/sim/tools/attio/assert_record.ts | 8 +++-- apps/sim/tools/attio/path_safety.test.ts | 43 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/apps/sim/tools/attio/assert_record.ts b/apps/sim/tools/attio/assert_record.ts index 3fd70553dbe..b2cd97645e0 100644 --- a/apps/sim/tools/attio/assert_record.ts +++ b/apps/sim/tools/attio/assert_record.ts @@ -50,9 +50,11 @@ export const attioAssertRecordTool: ToolConfig { - const searchParams = new URLSearchParams({ - matching_attribute: String(params.matchingAttribute ?? '').trim(), - }) + const matchingAttribute = String(params.matchingAttribute ?? '').trim() + if (!matchingAttribute) { + throw new Error('matchingAttribute is required') + } + const searchParams = new URLSearchParams({ matching_attribute: matchingAttribute }) return `https://api.attio.com/v2/objects/${safeUrlPathSegment(params.objectType, 'objectType')}/records?${searchParams.toString()}` }, method: 'PUT', diff --git a/apps/sim/tools/attio/path_safety.test.ts b/apps/sim/tools/attio/path_safety.test.ts index 33d288772b7..a9260468816 100644 --- a/apps/sim/tools/attio/path_safety.test.ts +++ b/apps/sim/tools/attio/path_safety.test.ts @@ -252,3 +252,46 @@ describe('attio path-ID traversal safety', () => { }) }) }) + +/** + * `assert_record` is the only Attio tool that puts a caller-supplied value in + * the query string rather than the path. `matching_attribute` selects which + * attribute the upsert matches on, so an empty selector is not a narrower + * request — it is an incomplete one, and the same reject-don't-substitute rule + * that governs the path segments applies to it. + */ +describe('attio_assert_record matching_attribute', () => { + const tool = ALL_ATTIO_TOOLS.filter(isAttioTool).find( + (candidate) => candidate.id === 'attio_assert_record' + ) + + function build(matchingAttribute: unknown): URL { + const url = tool?.request?.url + if (typeof url !== 'function') throw new Error('assert_record builds no URL') + return new URL( + url({ accessToken: 'token', objectType: 'people', matchingAttribute, values: {} }) + ) + } + + it('encodes a legitimate selector unchanged', () => { + expect(build('email_addresses').searchParams.get('matching_attribute')).toBe('email_addresses') + }) + + it('trims surrounding whitespace', () => { + expect(build(' domains ').searchParams.get('matching_attribute')).toBe('domains') + }) + + it('cannot inject an extra query parameter', () => { + const url = build('email_addresses&limit=500') + + expect(url.searchParams.get('limit')).toBeNull() + expect(url.searchParams.get('matching_attribute')).toBe('email_addresses&limit=500') + }) + + it.each([undefined, null, '', ' '])( + 'rejects %j rather than sending an empty selector', + (value) => { + expect(() => build(value)).toThrow(/matchingAttribute is required/) + } + ) +}) From d2699db758af25644b277c69142c4e783fc2ec2c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 22:29:30 -0700 Subject: [PATCH 7/7] fix(attio): reject a non-string matching_attribute instead of coercing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses a second cubic finding on `assert_record`. The presence guard used `String(params.matchingAttribute ?? '')`, and `String()` turns a single-element array into its element — so `['email_addresses']` coerced into a selector that looks entirely valid and passed straight through. `{}` coerced to `'[object Object]'` and `true` to `'true'`, both sent to Attio as real selectors. This is the trap `tools/url-path.ts` already documents for path segments: a bare `String()` produces a plausible but wrong value instead of a named error, turning a caller's mistake into a provider 404 they cannot debug. The same rule has to hold for the one query-string selector in the folder. The type is now checked before any stringification, so a non-string raises `matchingAttribute must be a string (received object)`. Test-first: all five coercion cases were watched failing. --- apps/sim/tools/attio/assert_record.ts | 10 +++++++++- apps/sim/tools/attio/path_safety.test.ts | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/apps/sim/tools/attio/assert_record.ts b/apps/sim/tools/attio/assert_record.ts index b2cd97645e0..83d0878c205 100644 --- a/apps/sim/tools/attio/assert_record.ts +++ b/apps/sim/tools/attio/assert_record.ts @@ -50,7 +50,15 @@ export const attioAssertRecordTool: ToolConfig { - const matchingAttribute = String(params.matchingAttribute ?? '').trim() + if (params.matchingAttribute === null || params.matchingAttribute === undefined) { + throw new Error('matchingAttribute is required') + } + if (typeof params.matchingAttribute !== 'string') { + throw new Error( + `matchingAttribute must be a string (received ${typeof params.matchingAttribute})` + ) + } + const matchingAttribute = params.matchingAttribute.trim() if (!matchingAttribute) { throw new Error('matchingAttribute is required') } diff --git a/apps/sim/tools/attio/path_safety.test.ts b/apps/sim/tools/attio/path_safety.test.ts index a9260468816..d71c1be267b 100644 --- a/apps/sim/tools/attio/path_safety.test.ts +++ b/apps/sim/tools/attio/path_safety.test.ts @@ -294,4 +294,18 @@ describe('attio_assert_record matching_attribute', () => { expect(() => build(value)).toThrow(/matchingAttribute is required/) } ) + + /** + * `String()` turns a single-element array into its element, so + * `['email_addresses']` would coerce into a selector that looks entirely + * valid. `tools/url-path.ts` rejects non-string kinds for this exact reason: + * a bare `String()` produces a plausible but wrong value instead of a named + * error. The same rule has to hold for the one query-string selector. + */ + it.each([[['email_addresses']], [{}], [true], [123], [['email_addresses', 'domains']]])( + 'rejects %j rather than coercing it into a selector', + (value) => { + expect(() => build(value)).toThrow(/matchingAttribute must be a string/) + } + ) })