Skip to content

Commit 177bcd4

Browse files
committed
fix(salesforce): correct the depth off-by-one and the LIMIT ceiling
Salesforce counts relationship levels, not dotted segments -- its own example is 'Contact.Account.Owner.FirstName (three levels)', four segments. The check compared segment count, so a legal five-level path was rejected, and the test asserted that rejection as correct. 2000 is the synchronous batch size, not a LIMIT maximum. Capping there removed the paging workflow these tools ship query_more for: a larger LIMIT returns the first batch with done:false and a locator. Also allows the documented FIELDS()/toLabel()/FORMAT()/convertCurrency() wrappers, recursing into the same field-path check so only a validated API name reaches the statement; seven new vectors attack the wrapper syntax.
1 parent 4298418 commit 177bcd4

10 files changed

Lines changed: 211 additions & 41 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* The block's skill `content` is prompt text an LLM reads and acts on, so it is
5+
* held to the same contract as `outputs`: it may only promise fields the tool
6+
* actually returns. Qdrant's upsert endpoint returns an `UpdateResult`
7+
* (`operation_id`, `status`) and no count of the points written.
8+
*/
9+
import { describe, expect, it } from 'vitest'
10+
import { QdrantBlockMeta } from '@/blocks/blocks/qdrant'
11+
import { UPSERT_RESULT_OUTPUT_PROPERTIES } from '@/tools/qdrant/types'
12+
13+
const upsertSkill = QdrantBlockMeta.skills.find((skill) => skill.name === 'upsert-points')
14+
15+
describe('QdrantBlockMeta upsert-points skill', () => {
16+
it('is registered', () => {
17+
expect(upsertSkill).toBeDefined()
18+
})
19+
20+
it('does not instruct the model to read an upserted count the API never returns', () => {
21+
expect(Object.keys(UPSERT_RESULT_OUTPUT_PROPERTIES)).toEqual(['operation_id', 'status'])
22+
expect(upsertSkill?.content).not.toMatch(/upserted count/i)
23+
expect(upsertSkill?.content).not.toMatch(/how many points were upserted/i)
24+
})
25+
26+
it('points the model at the fields the upsert response does carry', () => {
27+
expect(upsertSkill?.content).toMatch(/operation_id/)
28+
expect(upsertSkill?.content).toMatch(/status/)
29+
})
30+
})

apps/sim/blocks/blocks/qdrant.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export const QdrantBlockMeta = {
303303
name: 'upsert-points',
304304
description: 'Insert or update vector points with payload metadata into a Qdrant collection.',
305305
content:
306-
'# Upsert Points\n\nLoad vectors into a Qdrant collection.\n\n## Steps\n1. Use the Upsert operation with the Qdrant URL, Collection name, and API Key.\n2. Provide Points as a JSON array, each with an id, a vector matching the collection dimension, and an optional payload of metadata for later filtering.\n3. Confirm the upserted count from the response.\n\n## Output\nReport how many points were upserted into which collection and surface any payload validation issues.',
306+
'# Upsert Points\n\nLoad vectors into a Qdrant collection.\n\n## Steps\n1. Use the Upsert operation with the Qdrant URL, Collection name, and API Key.\n2. Provide Points as a JSON array, each with an id, a vector matching the collection dimension, and an optional payload of metadata for later filtering.\n3. Confirm the response status (`acknowledged` or `completed`) and keep the `operation_id` if you need to track an async write.\n\n## Output\nReport the upsert status and operation_id, which collection was targeted, and surface any payload validation issues. Qdrant does not return a count of points written \u2014 if you cite a number, cite the number of points you sent.',
307307
},
308308
{
309309
name: 'search-vectors',

apps/sim/tools/salesforce/get_accounts.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,19 @@ export const salesforceGetAccountsTool: ToolConfig<
5555
type: 'string',
5656
required: false,
5757
visibility: 'user-or-llm',
58-
description: 'Maximum number of results (default: 100, max: 2000)',
58+
description: 'Maximum number of results (default: 100)',
5959
},
6060
fields: {
6161
type: 'string',
6262
required: false,
6363
visibility: 'user-or-llm',
64-
description: 'Comma-separated field API names (e.g., "Id,Name,Industry,Phone")',
64+
description: 'Comma-separated field API names (e.g., "Id,Name,Industry,Phone") Also accepts FIELDS(STANDARD|CUSTOM|ALL) and toLabel()/FORMAT()/convertCurrency() around a single field.',
6565
},
6666
orderBy: {
6767
type: 'string',
6868
required: false,
6969
visibility: 'user-or-llm',
70-
description: 'Field and direction for sorting (e.g., "Name ASC" or "CreatedDate DESC")',
70+
description: 'Field and direction for sorting (e.g., "Name ASC" or "CreatedDate DESC") Bare field API names only, optionally with ASC/DESC and NULLS FIRST/LAST; SOQL functions such as DISTANCE() are not accepted here — use the Salesforce Query tool for those.',
7171
},
7272
},
7373

apps/sim/tools/salesforce/get_cases.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,19 @@ export const salesforceGetCasesTool: ToolConfig<
4646
type: 'string',
4747
required: false,
4848
visibility: 'user-or-llm',
49-
description: 'Maximum number of results to return (default: 100, max: 2000)',
49+
description: 'Maximum number of results to return (default: 100)',
5050
},
5151
fields: {
5252
type: 'string',
5353
required: false,
5454
visibility: 'user-or-llm',
55-
description: 'Comma-separated list of field API names to return',
55+
description: 'Comma-separated list of field API names to return Also accepts FIELDS(STANDARD|CUSTOM|ALL) and toLabel()/FORMAT()/convertCurrency() around a single field.',
5656
},
5757
orderBy: {
5858
type: 'string',
5959
required: false,
6060
visibility: 'user-or-llm',
61-
description: 'Field and direction for sorting (e.g., CreatedDate DESC)',
61+
description: 'Field and direction for sorting (e.g., CreatedDate DESC) Bare field API names only, optionally with ASC/DESC and NULLS FIRST/LAST; SOQL functions such as DISTANCE() are not accepted here — use the Salesforce Query tool for those.',
6262
},
6363
},
6464

apps/sim/tools/salesforce/get_contacts.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,19 @@ export const salesforceGetContactsTool: ToolConfig<
5252
type: 'string',
5353
required: false,
5454
visibility: 'user-or-llm',
55-
description: 'Maximum number of results (default: 100, max: 2000). Only for list query.',
55+
description: 'Maximum number of results (default: 100). Only for list query.',
5656
},
5757
fields: {
5858
type: 'string',
5959
required: false,
6060
visibility: 'user-or-llm',
61-
description: 'Comma-separated field API names (e.g., "Id,FirstName,LastName,Email,Phone")',
61+
description: 'Comma-separated field API names (e.g., "Id,FirstName,LastName,Email,Phone") Also accepts FIELDS(STANDARD|CUSTOM|ALL) and toLabel()/FORMAT()/convertCurrency() around a single field.',
6262
},
6363
orderBy: {
6464
type: 'string',
6565
required: false,
6666
visibility: 'user-or-llm',
67-
description: 'Field and direction for sorting (e.g., "LastName ASC"). Only for list query.',
67+
description: 'Field and direction for sorting (e.g., "LastName ASC"). Only for list query. Bare field API names only, optionally with ASC/DESC and NULLS FIRST/LAST; SOQL functions such as DISTANCE() are not accepted here — use the Salesforce Query tool for those.',
6868
},
6969
},
7070

apps/sim/tools/salesforce/get_leads.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,19 @@ export const salesforceGetLeadsTool: ToolConfig<
4646
type: 'string',
4747
required: false,
4848
visibility: 'user-or-llm',
49-
description: 'Maximum number of results to return (default: 100, max: 2000)',
49+
description: 'Maximum number of results to return (default: 100)',
5050
},
5151
fields: {
5252
type: 'string',
5353
required: false,
5454
visibility: 'user-or-llm',
55-
description: 'Comma-separated list of field API names to return',
55+
description: 'Comma-separated list of field API names to return Also accepts FIELDS(STANDARD|CUSTOM|ALL) and toLabel()/FORMAT()/convertCurrency() around a single field.',
5656
},
5757
orderBy: {
5858
type: 'string',
5959
required: false,
6060
visibility: 'user-or-llm',
61-
description: 'Field and direction for sorting (e.g., LastName ASC)',
61+
description: 'Field and direction for sorting (e.g., LastName ASC) Bare field API names only, optionally with ASC/DESC and NULLS FIRST/LAST; SOQL functions such as DISTANCE() are not accepted here — use the Salesforce Query tool for those.',
6262
},
6363
},
6464

apps/sim/tools/salesforce/get_opportunities.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,19 @@ export const salesforceGetOpportunitiesTool: ToolConfig<
5252
type: 'string',
5353
required: false,
5454
visibility: 'user-or-llm',
55-
description: 'Maximum number of results to return (default: 100, max: 2000)',
55+
description: 'Maximum number of results to return (default: 100)',
5656
},
5757
fields: {
5858
type: 'string',
5959
required: false,
6060
visibility: 'user-or-llm',
61-
description: 'Comma-separated list of field API names to return',
61+
description: 'Comma-separated list of field API names to return Also accepts FIELDS(STANDARD|CUSTOM|ALL) and toLabel()/FORMAT()/convertCurrency() around a single field.',
6262
},
6363
orderBy: {
6464
type: 'string',
6565
required: false,
6666
visibility: 'user-or-llm',
67-
description: 'Field and direction for sorting (e.g., CloseDate DESC)',
67+
description: 'Field and direction for sorting (e.g., CloseDate DESC) Bare field API names only, optionally with ASC/DESC and NULLS FIRST/LAST; SOQL functions such as DISTANCE() are not accepted here — use the Salesforce Query tool for those.',
6868
},
6969
},
7070

apps/sim/tools/salesforce/get_tasks.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,19 @@ export const salesforceGetTasksTool: ToolConfig<
5858
type: 'string',
5959
required: false,
6060
visibility: 'user-or-llm',
61-
description: 'Maximum number of results to return (default: 100, max: 2000)',
61+
description: 'Maximum number of results to return (default: 100)',
6262
},
6363
fields: {
6464
type: 'string',
6565
required: false,
6666
visibility: 'user-or-llm',
67-
description: 'Comma-separated list of field API names to return',
67+
description: 'Comma-separated list of field API names to return Also accepts FIELDS(STANDARD|CUSTOM|ALL) and toLabel()/FORMAT()/convertCurrency() around a single field.',
6868
},
6969
orderBy: {
7070
type: 'string',
7171
required: false,
7272
visibility: 'user-or-llm',
73-
description: 'Field and direction for sorting (e.g., ActivityDate DESC)',
73+
description: 'Field and direction for sorting (e.g., ActivityDate DESC) Bare field API names only, optionally with ASC/DESC and NULLS FIRST/LAST; SOQL functions such as DISTANCE() are not accepted here — use the Salesforce Query tool for those.',
7474
},
7575
},
7676

apps/sim/tools/salesforce/soql_safety.test.ts

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,14 @@ const INJECTIONS = [
5959
"Id WHERE Name != ''",
6060
'Id LIMIT 2000',
6161
'Id) UNION (SELECT Id',
62-
'Id,toLabel(Status)',
6362
'Id,COUNT(Id)',
63+
'toLabel(Id) UNION (SELECT Id)',
64+
'FIELDS(ALL) FROM User',
65+
'FORMAT(Id) FROM User',
66+
'toLabel(Name) LIMIT 1',
67+
'toLabel(Id),(SELECT Id FROM Contacts)',
68+
'toLabel(Id;DROP)',
69+
'FIELDS(EVERYTHING)',
6470
"Id,'literal'",
6571
'Id;DROP',
6672
'Id\nFROM User',
@@ -76,6 +82,14 @@ const LEGITIMATE_FIELDS = [
7682
'Custom_Field__c',
7783
'Account__r.Region__c',
7884
'A.B.C.D.E',
85+
'A.B.C.D.E.FieldName',
86+
'FIELDS(STANDARD)',
87+
'FIELDS(ALL)',
88+
'Id,FIELDS(CUSTOM)',
89+
'toLabel(Status)',
90+
'FORMAT(Amount)',
91+
'convertCurrency(Amount)',
92+
'FORMAT(convertCurrency(Amount))',
7993
] as const
8094

8195
const LEGITIMATE_ORDER_BY = [
@@ -146,7 +160,7 @@ describe.each(LIST_TOOLS)('$name SOQL safety', ({ tool, object }) => {
146160
)
147161
})
148162

149-
it.each(['abc', '', ' ', '10; DROP', '-1', '0', '2001', '1.5', 'NaN'] as const)(
163+
it.each(['abc', '', ' ', '10; DROP', '-1', '0', '1.5', 'NaN'] as const)(
150164
'rejects %j as a limit rather than emitting LIMIT NaN',
151165
(value) => {
152166
// An unset limit legitimately falls back to the default; only junk throws.
@@ -158,8 +172,9 @@ describe.each(LIST_TOOLS)('$name SOQL safety', ({ tool, object }) => {
158172
}
159173
)
160174

161-
it('accepts the documented 2000-row ceiling', () => {
175+
it('accepts a LIMIT above the 2000-row REST batch size so query_more can page', () => {
162176
expect(buildQuery(tool, { limit: '2000' })).toContain('LIMIT 2000')
177+
expect(buildQuery(tool, { limit: '5000' })).toContain('LIMIT 5000')
163178
})
164179
})
165180

@@ -169,10 +184,51 @@ describe('sanitizeSoqlFieldList', () => {
169184
expect(sanitizeSoqlFieldList(' ', 'Id,Name')).toBe('Id, Name')
170185
})
171186

187+
it('accepts the five relationship levels SOQL allows (six dotted segments)', () => {
188+
expect(sanitizeSoqlFieldList('A.B.C.D.E.F', 'Id')).toBe('A.B.C.D.E.F')
189+
})
190+
172191
it('rejects a path deeper than the five relationship levels SOQL allows', () => {
173-
expect(() => sanitizeSoqlFieldList('A.B.C.D.E.F', 'Id')).toThrow(
174-
/at most 5|relationship levels/
192+
expect(() => sanitizeSoqlFieldList('A.B.C.D.E.F.G', 'Id')).toThrow(
193+
/6 relationship levels; SOQL allows at most 5/
194+
)
195+
})
196+
197+
it('accepts the documented field-group selectors, normalized to upper case', () => {
198+
expect(sanitizeSoqlFieldList('fields(standard)', 'Id')).toBe('FIELDS(STANDARD)')
199+
expect(sanitizeSoqlFieldList('Id,FIELDS(custom)', 'Id')).toBe('Id, FIELDS(CUSTOM)')
200+
})
201+
202+
it('accepts the documented single-field SELECT functions, normalized to documented casing', () => {
203+
expect(sanitizeSoqlFieldList('tolabel(Status)', 'Id')).toBe('toLabel(Status)')
204+
expect(sanitizeSoqlFieldList('format(Amount)', 'Id')).toBe('FORMAT(Amount)')
205+
expect(sanitizeSoqlFieldList('CONVERTCURRENCY(Amount)', 'Id')).toBe('convertCurrency(Amount)')
206+
expect(sanitizeSoqlFieldList('FORMAT(convertCurrency(Amount))', 'Id')).toBe(
207+
'FORMAT(convertCurrency(Amount))'
208+
)
209+
})
210+
211+
it('still validates the inner argument of an allowed wrapper as a field path', () => {
212+
expect(() => sanitizeSoqlFieldList("toLabel(Id WHERE Name != '')", 'Id')).toThrow(
213+
SoqlValidationError
175214
)
215+
expect(() => sanitizeSoqlFieldList('FORMAT(A.B.C.D.E.F.G)', 'Id')).toThrow(SoqlValidationError)
216+
})
217+
218+
it('rejects wrapper nesting deeper than the documented FORMAT(convertCurrency(field)) form', () => {
219+
expect(() => sanitizeSoqlFieldList('FORMAT(FORMAT(convertCurrency(Amount)))', 'Id')).toThrow(
220+
SoqlValidationError
221+
)
222+
})
223+
224+
it('still rejects COUNT(), which cannot be combined with the ORDER BY these tools always emit', () => {
225+
expect(() => sanitizeSoqlFieldList('COUNT()', 'Id')).toThrow(SoqlValidationError)
226+
expect(() => sanitizeSoqlFieldList('COUNT(Id)', 'Id')).toThrow(SoqlValidationError)
227+
})
228+
229+
it('rejects SELECT-only functions in ORDER BY, which SOQL forbids there', () => {
230+
expect(() => sanitizeSoqlOrderBy('toLabel(Status) ASC', 'Id ASC')).toThrow(SoqlValidationError)
231+
expect(() => sanitizeSoqlOrderBy('FIELDS(STANDARD)', 'Id ASC')).toThrow(SoqlValidationError)
176232
})
177233

178234
it('rejects a list that is only separators', () => {
@@ -211,4 +267,14 @@ describe('sanitizeSoqlLimit', () => {
211267
expect(() => sanitizeSoqlLimit('abc')).toThrow(SoqlValidationError)
212268
expect(() => sanitizeSoqlLimit('abc')).toThrow(/not a whole number/)
213269
})
270+
271+
it('allows a LIMIT past the 2000-row REST batch size, which query_more pages through', () => {
272+
expect(sanitizeSoqlLimit('2001')).toBe(2001)
273+
expect(sanitizeSoqlLimit(5000)).toBe(5000)
274+
expect(sanitizeSoqlLimit('50000')).toBe(50000)
275+
})
276+
277+
it('still rejects a value past the sanity ceiling', () => {
278+
expect(() => sanitizeSoqlLimit('50001')).toThrow(SoqlValidationError)
279+
})
214280
})

0 commit comments

Comments
 (0)