fix(attio): stop swallowing JSON parse failures and guard path segments by waleedlatif1 · Pull Request #7256 · simstudioai/sim · GitHub

Two integrity defects in the Attio tools, plus the test harnesses that pin them.

Defect 1 — silent data substitution on JSON parse failure

13 parse sites across 9 tools caught a JSON.parse failure and substituted an empty value ({} / []), or forwarded the raw unparsed string, then reported success.

All now throw the folder's established named error, matching what create_record, assert_record, update_record, create_attribute, update_attribute and list_records already did.

Tool	Param	Was	Consequence before
query_list_entries	filter	{}	Unfiltered query over the whole list, reported as success — the caller gets more rows than they asked for
query_list_entries	sorts	[]	Unsorted results, reported as success
create_list_entry / update_list_entry	entryValues	{}	Entry written with no values
create_task / update_task	linkedRecords	[]	Task created/updated with no links
create_task / update_task	assignees	[]	Task created/updated unassigned
create_webhook	subscriptions	[]	Webhook created subscribed to nothing — it silently never fires
update_webhook	subscriptions	[]	Destructive: replaces a live webhook's subscriptions with [], silently disabling it
create_list / update_list	workspaceMemberAccess	raw string	Attio 400 with an opaque provider error

Verified against Attio's official docs: filter is an object and sorts an array in the POST /v2/lists/{list}/entries/query body; data.subscriptions is a required array on POST /v2/webhooks. So [] was not a harmless default — it was a valid-looking body with the caller's intent removed.

Defect 2 — path traversal

43 path segments across 31 tools interpolated an LLM-writable id straight into the request path. These params are visibility: 'user-or-llm', so prompt injection controls them.

encodeURIComponent is not sufficient: . and .. are unreserved, so they survive encoding untouched 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 segment now goes through the shared safeUrlPathSegment, which rejects rather than encodes.

assert_record additionally built its matching_attribute query value by raw interpolation; it now uses URLSearchParams. Docs confirm matching_attribute is a required query parameter on PUT /v2/objects/{object}/records.

Two follow-up hardenings on that same selector, both found in review and both the same class of bug this PR exists to fix:

An absent selector was being substituted with matching_attribute= and the upsert sent anyway — the exact silent substitution the rest of the PR removes, and a local weakening, since the previous .trim() at least threw on null/undefined. It now throws matchingAttribute is required.
A bare String() coerced non-strings into plausible-but-wrong selectors: String(['email_addresses']) is 'email_addresses', so a single-element array passed as a completely valid-looking selector, and {} became '[object Object]'. This is the trap tools/url-path.ts documents for path segments; the runtime type is now checked before any stringification.

Behaviour changes and backwards compatibility

Every behaviour change, and why it cannot break a working workflow.

1. Malformed JSON now throws instead of substituting. This is the one change an existing user can notice. A workflow that was unknowingly passing malformed JSON — and therefore unknowingly running an unfiltered query_list_entries, or writing an unassigned task — previously reported success. It now fails loudly. The data was already wrong; only the reporting changes. Stated plainly because it is a change for an existing user.

Unaffected, verified by test: valid JSON; an omitted param; a blank optional param (''); and an already-parsed object/array piped from another block. The truthy guard is deliberately preserved — apps/sim/tools/params.ts:858-863 treats '' as the platform's canonical "not provided" value, so a blank optional field must not throw.

2. Values that previously reached the wire and now throw — the complete list:

exactly . or .. after trimming
any value containing / or \

Neither is a legitimate Attio identifier. Attio ids are UUIDs and slugs; a dot inside a longer segment (example.com, v1.2.3, ..foo, foo..) is preserved untouched and is covered by LEGITIMATE_IDS.

3. Values that previously threw, and now throw a better error (no compatibility surface):

null / undefined — was TypeError: params.X.trim is not a function, now X is required
object / boolean / array — was the same TypeError, now X must be a string or a number (received object)
unpaired UTF-16 surrogate — was a bare URIError naming nothing, now named

4. Values that previously threw and now succeed (strictly more permissive): a numeric id arriving as a JSON number, which .trim() used to crash on.

5. Percent-encoding is now applied. For Attio slugs and UUIDs encoding is the identity, so no legitimate value changes; path_safety.test.ts proves this with LEGITIMATE_IDS. A literal % would now become %25, but no Attio identifier contains one, and a raw % previously formed an invalid escape anyway.

6. matching_attribute encoding and validation. For documented values (email_addresses, domains) encoding is the identity. A value containing & or = previously injected query parameters. Newly rejected: an absent selector (was silently sent as empty) and a non-string selector (was coerced). Both were previously reachable only from surfaces that do not run the platform's required-param check, since matchingAttribute is required: true and params.ts:858-863 rejects '' on the normal executor path — the guard should not depend on a caller's validation.

Not changed: no subBlock id, no visibility, no required, no param/output/description line. blocks/blocks/attio.ts is untouched. Every changed line in the 34 tool files is one of exactly three kinds — the import, a URL builder, or a catch body. No new non-test file, and no new helper: safeUrlPathSegment is imported from the pre-existing shared @/tools/url-path.

Tests

Both harnesses 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 extending a list. Every URL assertion resolves with new URL(...) — the normalization fetch performs — rather than string-matching the template.

Three holes were found and closed during review, each verified by a scoped revert:

Fuzzing all params at once. The original harness filled every string param with the same value and did catch { return }, so the first guarded param threw and its siblings were never exercised. Discovery is now per (tool, param) pair — 43 pairs across 31 tools, matching the 43 guarded call sites one for one.
Skipping the probe slot. if (segment === PROBE_ID) return proved the segment count and the neighbours but never what landed in the slot. A balanced traversal defeats that. The slot is now asserted to equal encodeURIComponent of the trimmed input.
A sweep that only caught empty-value substitution. Raw-string forwarding and non-named errors both slipped through. The sweep now classifies each body param by where a sentinel lands and holds parsed params to /Invalid JSON provided/.

Scoped revert of get_attribute / identifier, showing each hole in turn:

Harness	Failures
10 vectors, slot skipped (original)	15
13 vectors, slot skipped	17 — a/../b still passing
13 vectors, slot asserted (current)	18

a/../b is the minimal witness: it pops one segment and adds one, so length and neighbours are identical and only the slot differs.

Red-first for the sweep, by reverting create_list to each violation shape: raw-string forwarding fails 2 tests, throw new Error('bad input') fails 2 tests. Both passed before.

2014 tests pass.

Validation against Attio's official API docs

Ran the validate-integration checklist. Verified from the official docs (Context7 /websites/attio_mintlify_app):

POST /v2/objects/{object}/records/query and POST /v2/lists/{list}/entries/query — filter object, sorts array, limit/offset in the body. Matches.
PUT /v2/objects/{object}/records with matching_attribute as a required query param. Matches.
POST /v2/webhooks — data.target_url + required data.subscriptions array of {event_type, filter?}. Matches.
Task write bodies — linked_records and assignees arrays. Matches.
Response envelope — all 45 tools read data.data, consistent with the documented { data: ... } shape.

Conventions: 45/45 tools registered in tools/registry.ts and exported from the barrel with no orphans and no phantom entries; all 45 accessToken params are visibility: 'hidden'; every param has explicit required, visibility and description; no duplicate subBlock ids.

Unverifiable, reported rather than guessed: per-endpoint response field lists for the individual get_*/list_* tools are not enumerated in the public docs beyond the { data: ... } envelope. Those transformResponse field extractions are pre-existing and untouched by this PR, so they are flagged for a follow-up with sample payloads rather than adjusted on inference.

Gates

bun run lint, bun run check:audits (39/39), check-block-registry — all clean. Test files type-check clean under an explicit tsconfig (note apps/sim/tsconfig.json excludes **/*.test.ts, so CI does not cover them). tool-metadata:check passes with no regeneration: no param or output changed.

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.

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 29, 2026 5:29am

Greptile Summary

The PR hardens Attio tools by rejecting malformed JSON instead of substituting values and by validating dynamic URL path segments. It also adds discovery-based regression harnesses for JSON integrity and path safety.

Routes dynamic Attio identifiers through the shared safe path-segment helper.
Encodes assert_record's matching_attribute query parameter safely.
Converts swallowed JSON parse failures into named errors.
Adds broad regression coverage for path and JSON handling.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename	Overview
apps/sim/tools/attio/assert_record.ts	Validates the object path segment and constructs matching_attribute through URLSearchParams.
apps/sim/tools/attio/query_list_entries.ts	Rejects malformed filter and sorts JSON while guarding the dynamic list path segment.
apps/sim/tools/attio/update_webhook.ts	Prevents malformed subscriptions from silently disabling a webhook and guards its identifier.
apps/sim/tools/attio/path_safety.test.ts	Adds discovery-based coverage for every Attio parameter that reaches a dynamic request path.
apps/sim/tools/attio/json_integrity.test.ts	Adds discovery-based coverage ensuring parsed parameters reject malformed JSON and accept valid values.

Reviews (6): Last reviewed commit: "fix(attio): reject a non-string matching..." | Re-trigger Greptile

3 issues found across 36 files

Confidence score: 3/5

In apps/sim/tools/attio/query_list_entries.ts, empty-string filter or sorts values bypass JSON.parse, causing requests to run without the caller’s intended filtering or sorting; check for defined values before parsing.
In apps/sim/tools/attio/update_task.ts, empty strings for either JSON-array parameter are silently omitted instead of producing the expected parse error; validate defined inputs and preserve invalid-value errors.
In apps/sim/tools/attio/json_integrity.test.ts, the sweep can pass when malformed JSON is forwarded or any error is thrown, leaving regressions undetected; add assertions for the expected parse behavior and error handling.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/tools/attio/update_task.ts">

<violation number="1" location="apps/sim/tools/attio/update_task.ts:78">
P2: When a caller supplies an empty string for either JSON-array parameter, the truthiness guard bypasses `JSON.parse`, so `update_task` silently omits the invalid value instead of reporting the parse error this change is meant to surface. Check for parameter presence rather than truthiness before parsing.</violation>
</file>

<file name="apps/sim/tools/attio/query_list_entries.ts">

<violation number="1" location="apps/sim/tools/attio/query_list_entries.ts:78">
P1: When `filter` or `sorts` is passed as an empty string, the truthy guard skips `JSON.parse`, so this new error is never thrown and the tool silently sends an unfiltered or unsorted request. Check for defined values before parsing so empty JSON input is rejected.</violation>
</file>

<file name="apps/sim/tools/attio/json_integrity.test.ts">

<violation number="1" location="apps/sim/tools/attio/json_integrity.test.ts:111">
P2: The sweep only detects silent empty-value substitution. If a new tool forwards the raw malformed JSON string, `JSON.stringify` still contains the sentinel so `toContain(SENTINEL)` passes, and if it throws any error the `catch { return }` also passes — neither fails CI, even though forwarding raw strings and throwing a non-named error are both Defect-1 violations the PR fixes. Assert in the catch that the error is the folder's named 'Invalid JSON provided' message so the guard actually pins the pattern for every body param, not just the 12 hard-coded SWALLOWED_SITES.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

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.

Both harnesses carried `ToolConfig<any, any>` 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<Record<string, unknown>, ToolResponse> Params are built as `Record<string, unknown>`, 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<AttioXParams, …>`, 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.

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.

@greptile @cubic-dev-ai review

Re-review request against the current head, 72f1829. The earlier reviews were both submitted against c04227cba4 (commit 1 of 4) and predate everything below — GitHub silently re-anchors commit_id on inline comments to the head SHA, so those threads render as if they were current when original_commit_id shows otherwise.

Three commits landed after that review:

5d8ee71 — path_safety.test.ts rewritten to fuzz one param at a time. The old harness filled every string param with the same fuzz value and did catch { return }, so the first guarded param threw and its unguarded siblings were never exercised. Discovery is now per (tool, param) pair — 43 pairs across 31 tools, matching the 43 guarded call sites one for one.
601cf68 — removed ToolConfig<any, any> and both as any from the two harnesses (greptile P2).
72f1829 — strengthened the json_integrity sweep so raw-string forwarding and non-named errors both fail (cubic P2).

All four earlier threads have individual replies and are resolved. Two cubic findings were declined with reasoning on the thread — both proposed replacing a truthy guard with a presence check on required: false params, which would make a blank optional field throw, since tools/params.ts:858-863 treats '' as the canonical "not provided" value.

Current state: lint clean, check:audits 39/39, check-block-registry clean, 1744 tests passing, no any in either harness.

@greptile review

@greptile @cubic-dev-ai review

Re-review request against the current head, 72f1829. The earlier reviews were both submitted against c04227cba4 (commit 1 of 4) and predate everything below — GitHub silently re-anchors commit_id on inline comments to the head SHA, so those threads render as if they were current when original_commit_id shows otherwise.

Three commits landed after that review:
...

@waleedlatif1 Incremental reviews are turned off for this repository. Comment @cubic review to run a full review.

@cubic-dev-ai review

@cubic-dev-ai review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@greptile review

@cubic review

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

2 issues found across 36 files

Confidence score: 3/5

apps/sim/tools/attio/query_list_entries.ts skips parsing an explicitly supplied empty filter, causing the query to run without filtering and potentially return unintended entries; check for presence with != null before parsing.
apps/sim/tools/attio/create_list_entry.ts treats an explicitly supplied empty entryValues as omitted, silently swallowing invalid JSON and reporting success with {}; distinguish omission from an empty supplied value and validate it.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/tools/attio/create_list_entry.ts">

<violation number="1" location="apps/sim/tools/attio/create_list_entry.ts:73">
P2: When `entryValues` is the supplied empty string, this branch is never reached: the truthiness check leaves `entryValues` as `{}`, so the tool reports success after swallowing invalid JSON. Distinguish an omitted value from `''` before parsing so empty input raises this named error.</violation>
</file>

<file name="apps/sim/tools/attio/query_list_entries.ts">

<violation number="1" location="apps/sim/tools/attio/query_list_entries.ts:78">
P1: When `filter` is an empty string, the truthiness guard skips `JSON.parse`, so the query silently runs without its filter and can return more entries than requested. Check for presence with `!= null` before parsing, and apply the same change to `sorts` so every supplied invalid JSON string fails explicitly.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

No issues found across 36 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

Re-trigger cubic

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.

@greptile review

@cubic review

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

1 issue found across 36 files

Confidence score: 4/5

In apps/sim/tools/attio/assert_record.ts, a missing matchingAttribute can produce an upsert request with matching_attribute= instead of failing locally, potentially causing an invalid or unintended Attio operation; validate the required value before constructing the query.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/tools/attio/assert_record.ts">

<violation number="1" location="apps/sim/tools/attio/assert_record.ts:54">
P2: When `matchingAttribute` is null or undefined, this fallback sends `matching_attribute=` instead of rejecting the incomplete upsert locally. Validate the required value before constructing the query, rather than silently substituting an empty selector.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

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.

@greptile review

@cubic review

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

1 issue found across 36 files

Confidence score: 4/5

In apps/sim/tools/attio/assert_record.ts, an array supplied to a malformed call can be coerced into the valid email_addresses selector, allowing invalid input to pass validation; check the runtime type before coercion so malformed selectors are rejected.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/tools/attio/assert_record.ts">

<violation number="1" location="apps/sim/tools/attio/assert_record.ts:53">
P2: When a malformed call supplies an array, this coercion turns `['email_addresses']` into the valid selector `email_addresses` instead of rejecting it. Validate the runtime type before coercing so malformed input cannot drive an unintended upsert.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

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.

@greptile review

@cubic review

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

No issues found across 36 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

Re-trigger cubic

Closing for now — not because of a defect. This batch grew to 17 PRs across ~700 changed call sites, and we would rather revisit it as smaller, independently testable pieces than merge this much at once.

Nothing here is lost: the branch fix/attio-integrity-and-paths is preserved and this PR can be reopened. Review state, the reasoning on every thread, and the red-first verification all stay attached.

vercel Bot temporarily deployed to Preview August 29, 2026 03:22 Inactive

greptile-apps Bot reviewed Aug 29, 2026

View reviewed changes

Comment thread apps/sim/tools/attio/path_safety.test.ts