Skip to content

Commit 79d4a88

Browse files
committed
fix(algolia,box,attio): guard body ids, constrain target, attribute a probe claim
- list_indices interpolated user-or-llm page/hitsPerPage raw into the query string; the block declared them string while the tool declared number, with no coercion between - .trim() on ids that arrive as JSON numbers threw a TypeError. Box ids are numeric strings and '0' is the root folder, which a truthiness guard was dropping entirely; a whitespace-only id went out as an empty parent - get_records fell back to the tool-level indexName whenever a per-request value was not a string, silently querying a different index - attio target is a two-value enum the tool layer left unconstrained, so a direct LLM call could address /v2/<anything> - the Algolia guard's rationale cited an unreproduced live probe alongside spec-backed claims; the probe is now quarantined and nothing depends on it
1 parent 3455d44 commit 79d4a88

26 files changed

Lines changed: 782 additions & 48 deletions

apps/sim/blocks/blocks/algolia.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,25 @@ Return ONLY the JSON array.`,
605605
return defaultValue
606606
}
607607

608+
/**
609+
* Pagination fields are short-inputs, so they leave the canvas as
610+
* strings even though every Algolia tool declares them `type: 'number'`.
611+
* Coercing here — in `params`, which runs during execution after
612+
* variables resolve — keeps a `<Block.output>` reference intact; the
613+
* same call in `tools.config.tool` would run during serialization and
614+
* destroy it. A value that is not numeric is passed through untouched
615+
* so the tool reports the offending parameter by name.
616+
*/
617+
const toNumber = (value: unknown) => {
618+
if (typeof value === 'number') return value
619+
if (typeof value !== 'string') return value
620+
const parsed = Number(value.trim())
621+
return Number.isFinite(parsed) ? parsed : value
622+
}
623+
624+
if (result.page !== undefined) result.page = toNumber(result.page)
625+
if (result.hitsPerPage !== undefined) result.hitsPerPage = toNumber(result.hitsPerPage)
626+
608627
if (operation === 'partial_update_record') {
609628
result.createIfNotExists = toBool(result.createIfNotExists, true)
610629
}
@@ -636,8 +655,8 @@ Return ONLY the JSON array.`,
636655
operation: { type: 'string', description: 'Operation to perform' },
637656
indexName: { type: 'string', description: 'Algolia index name' },
638657
query: { type: 'string', description: 'Search query' },
639-
hitsPerPage: { type: 'string', description: 'Number of hits per page' },
640-
page: { type: 'string', description: 'Page number' },
658+
hitsPerPage: { type: 'number', description: 'Number of hits per page' },
659+
page: { type: 'number', description: 'Page number' },
641660
filters: { type: 'string', description: 'Algolia filter string' },
642661
attributesToRetrieve: { type: 'string', description: 'Attributes to retrieve' },
643662
facets: { type: 'string', description: 'Comma-separated facet attribute names to count' },

apps/sim/tools/algolia/copy_move_index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { algoliaIndexName } from '@/tools/algolia/index-name'
12
import type {
23
AlgoliaCopyMoveIndexParams,
34
AlgoliaCopyMoveIndexResponse,
@@ -66,7 +67,7 @@ export const copyMoveIndexTool: ToolConfig<
6667
body: (params) => {
6768
const body: Record<string, unknown> = {
6869
operation: params.operation,
69-
destination: params.destination.trim(),
70+
destination: algoliaIndexName(params.destination, 'destination'),
7071
}
7172
if (params.scope) {
7273
const scope = typeof params.scope === 'string' ? JSON.parse(params.scope) : params.scope

apps/sim/tools/algolia/get_records.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { algoliaIndexName } from '@/tools/algolia/index-name'
12
import type { AlgoliaGetRecordsParams, AlgoliaGetRecordsResponse } from '@/tools/algolia/types'
23
import type { ToolConfig } from '@/tools/types'
34

@@ -49,7 +50,9 @@ export const getRecordsTool: ToolConfig<AlgoliaGetRecordsParams, AlgoliaGetRecor
4950
const requests = (parsed as Record<string, unknown>[]).map((req) => ({
5051
...req,
5152
indexName:
52-
typeof req.indexName === 'string' ? req.indexName.trim() : params.indexName.trim(),
53+
req.indexName === undefined || req.indexName === null
54+
? algoliaIndexName(params.indexName, 'indexName')
55+
: algoliaIndexName(req.indexName, 'indexName'),
5356
}))
5457
return { requests }
5558
},
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Normalization for an Algolia index name that a tool puts into a request
3+
* **body** rather than into the URL path.
4+
*
5+
* The path-zone sites run through `@/tools/url-path`, whose guards already
6+
* stringify a numeric value before encoding it. That module's contract is
7+
* path-specific — rejecting dot segments and separators — and a body field
8+
* needs neither, so the coercion lives here instead of widening the path
9+
* module's public surface.
10+
*
11+
* An index named `2024` is ordinary, and a `visibility: 'user-or-llm'` slot
12+
* filled with `2024` arrives as a JSON number, where a bare `.trim()` is an
13+
* unhandled `TypeError: x.trim is not a function`. `null` and `undefined` are
14+
* rejected before coercion so a missing parameter is reported rather than sent
15+
* as an index literally named `"undefined"`.
16+
*/
17+
18+
/**
19+
* Coerces a required Algolia index name to a trimmed string.
20+
*
21+
* @param value - The raw index name, typically LLM- or user-supplied.
22+
* @param paramName - The parameter name, used to name the offender in errors.
23+
* @returns The index name as a trimmed string.
24+
* @throws If the value is nullish or trims to empty.
25+
*/
26+
export function algoliaIndexName(value: unknown, paramName: string): string {
27+
if (value === null || value === undefined) {
28+
throw new Error(`${paramName} is required`)
29+
}
30+
31+
const trimmed = String(value).trim()
32+
33+
if (!trimmed) {
34+
throw new Error(`${paramName} is required`)
35+
}
36+
37+
return trimmed
38+
}

apps/sim/tools/algolia/list_indices.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,28 @@
11
import type { AlgoliaListIndicesParams, AlgoliaListIndicesResponse } from '@/tools/algolia/types'
22
import type { ToolConfig } from '@/tools/types'
33

4+
/**
5+
* Coerces a pagination parameter to the non-negative integer Algolia declares.
6+
*
7+
* The parameter is `visibility: 'user-or-llm'` and the Algolia block still
8+
* forwards it as a short-input string, so `'0&hitsPerPage=1000'` is a value the
9+
* builder genuinely receives. `URLSearchParams` alone would percent-encode the
10+
* `&` and stop the smuggling, but it would also send that whole string as the
11+
* value of `page`; rejecting a value that is not a number reports the bad input
12+
* instead of asking Algolia to reject it. A numeric string (`'0'`, `'100'`) is
13+
* the normal UI path and passes through unchanged.
14+
*/
15+
function paginationValue(value: string | number, paramName: string): string {
16+
const raw = typeof value === 'number' ? value : String(value).trim()
17+
const parsed = raw === '' ? Number.NaN : Number(raw)
18+
19+
if (!Number.isInteger(parsed) || parsed < 0) {
20+
throw new Error(`${paramName} must be a non-negative integer`)
21+
}
22+
23+
return String(parsed)
24+
}
25+
426
export const listIndicesTool: ToolConfig<AlgoliaListIndicesParams, AlgoliaListIndicesResponse> = {
527
id: 'algolia_list_indices',
628
name: 'Algolia List Indices',
@@ -38,10 +60,15 @@ export const listIndicesTool: ToolConfig<AlgoliaListIndicesParams, AlgoliaListIn
3860
method: 'GET',
3961
url: (params) => {
4062
const base = `https://${params.applicationId}-dsn.algolia.net/1/indexes`
41-
const queryParams: string[] = []
42-
if (params.page !== undefined) queryParams.push(`page=${params.page}`)
43-
if (params.hitsPerPage !== undefined) queryParams.push(`hitsPerPage=${params.hitsPerPage}`)
44-
return queryParams.length > 0 ? `${base}?${queryParams.join('&')}` : base
63+
const queryParams = new URLSearchParams()
64+
if (params.page !== undefined) {
65+
queryParams.set('page', paginationValue(params.page, 'page'))
66+
}
67+
if (params.hitsPerPage !== undefined) {
68+
queryParams.set('hitsPerPage', paginationValue(params.hitsPerPage, 'hitsPerPage'))
69+
}
70+
const query = queryParams.toString()
71+
return query ? `${base}?${query}` : base
4572
},
4673
headers: (params) => ({
4774
'x-algolia-application-id': params.applicationId,
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Guards `algolia_list_indices`' query string against parameter smuggling.
5+
*
6+
* `page` and `hitsPerPage` are `visibility: 'user-or-llm'`, so prompt injection
7+
* controls them. The tool declares them `type: 'number'`, but the Algolia block
8+
* declared them `type: 'string'` in `inputs` and its `tools.config.params`
9+
* copies values through verbatim, so a string genuinely reaches the URL
10+
* builder. Interpolating that string into `page=${value}` lets one parameter
11+
* carry a `&`, appending arbitrary further query parameters to a request that
12+
* still carries the caller's admin API key.
13+
*
14+
* Every assertion resolves the built URL through `new URL(...)` and reads
15+
* `searchParams`, never `includes()` on the template text: a smuggled
16+
* `hitsPerPage` is invisible to a substring check on `page=...` but shows up
17+
* as a second, separate key once the URL is actually parsed.
18+
*/
19+
import { describe, expect, it } from 'vitest'
20+
import { listIndicesTool } from '@/tools/algolia/list_indices'
21+
22+
const BASE = { applicationId: 'APPID', apiKey: 'KEY' }
23+
24+
function buildUrl(extra: Record<string, unknown>): URL {
25+
const build = listIndicesTool.request.url as (params: Record<string, unknown>) => string
26+
return new URL(build({ ...BASE, ...extra }))
27+
}
28+
29+
describe('algolia_list_indices query construction', () => {
30+
it('rejects a page value carrying a smuggled second parameter', () => {
31+
expect(() => buildUrl({ page: '0&hitsPerPage=1000' })).toThrow(/page/)
32+
})
33+
34+
it('rejects a hitsPerPage value carrying a smuggled second parameter', () => {
35+
expect(() => buildUrl({ hitsPerPage: '1&page=99' })).toThrow(/hitsPerPage/)
36+
})
37+
38+
it('rejects a non-numeric page outright rather than sending it', () => {
39+
expect(() => buildUrl({ page: 'abc' })).toThrow(/page/)
40+
})
41+
42+
it('rejects a fractional page rather than silently truncating it', () => {
43+
expect(() => buildUrl({ page: '1.5' })).toThrow(/page/)
44+
})
45+
46+
it('rejects an empty string rather than coercing it to page 0', () => {
47+
expect(() => buildUrl({ page: '' })).toThrow(/page/)
48+
expect(() => buildUrl({ hitsPerPage: ' ' })).toThrow(/hitsPerPage/)
49+
})
50+
51+
it('rejects a negative page', () => {
52+
expect(() => buildUrl({ page: -1 })).toThrow(/page/)
53+
})
54+
55+
it('encodes rather than interpolates when a value survives coercion', () => {
56+
const url = buildUrl({ page: 2, hitsPerPage: 50 })
57+
expect(url.searchParams.get('page')).toBe('2')
58+
expect(url.searchParams.get('hitsPerPage')).toBe('50')
59+
expect([...url.searchParams.keys()]).toEqual(['page', 'hitsPerPage'])
60+
})
61+
62+
it('accepts the numeric strings the block still forwards from a short-input', () => {
63+
const url = buildUrl({ page: '0', hitsPerPage: '100' })
64+
expect(url.searchParams.get('page')).toBe('0')
65+
expect(url.searchParams.get('hitsPerPage')).toBe('100')
66+
expect([...url.searchParams.keys()]).toEqual(['page', 'hitsPerPage'])
67+
})
68+
69+
it('is byte-identical to the pre-fix output for legitimate values', () => {
70+
const build = listIndicesTool.request.url as (params: Record<string, unknown>) => string
71+
expect(build({ ...BASE })).toBe('https://APPID-dsn.algolia.net/1/indexes')
72+
expect(build({ ...BASE, page: 0 })).toBe('https://APPID-dsn.algolia.net/1/indexes?page=0')
73+
expect(build({ ...BASE, hitsPerPage: 100 })).toBe(
74+
'https://APPID-dsn.algolia.net/1/indexes?hitsPerPage=100'
75+
)
76+
expect(build({ ...BASE, page: 3, hitsPerPage: 25 })).toBe(
77+
'https://APPID-dsn.algolia.net/1/indexes?page=3&hitsPerPage=25'
78+
)
79+
})
80+
})
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Guards the Algolia tools that put an index name into a request **body**
5+
* against it arriving as a JSON number.
6+
*
7+
* The path-zone sites already run through `safeUrlPathSegment`, whose
8+
* `toGuardedString` stringifies a number; the body-zone sites kept a bare
9+
* `.trim()`. An index literally named `2024` is ordinary, and a
10+
* `visibility: 'user-or-llm'` slot filled with `2024` reaches the builder as a
11+
* JSON number — `TypeError: x.trim is not a function`, surfaced as a tool crash
12+
* rather than a validation error. Sending the string `"undefined"` as an index
13+
* name instead of reporting the missing parameter would be no better, so the
14+
* nullish case is rejected before coercion.
15+
*/
16+
import { describe, expect, it } from 'vitest'
17+
import { copyMoveIndexTool } from '@/tools/algolia/copy_move_index'
18+
import { getRecordsTool } from '@/tools/algolia/get_records'
19+
import { searchTool } from '@/tools/algolia/search'
20+
import type { ToolConfig } from '@/tools/types'
21+
22+
type AnyTool = ToolConfig<any, any>
23+
24+
const BASE = { applicationId: 'APPID', apiKey: 'KEY' }
25+
26+
function bodyOf(tool: AnyTool, params: Record<string, unknown>): Record<string, any> {
27+
const build = tool.request.body as (p: Record<string, unknown>) => Record<string, any>
28+
return build({ ...BASE, ...params })
29+
}
30+
31+
describe('algolia_search indexName coercion', () => {
32+
it('accepts a numeric index name emitted as a JSON number', () => {
33+
expect(
34+
bodyOf(searchTool as AnyTool, { indexName: 2024, query: 'q' }).requests[0].indexName
35+
).toBe('2024')
36+
})
37+
38+
it('still trims a string index name byte-identically', () => {
39+
expect(bodyOf(searchTool as AnyTool, { indexName: ' products ', query: 'q' })).toEqual({
40+
requests: [{ indexName: 'products', query: 'q' }],
41+
})
42+
})
43+
44+
it('reports a missing required index name by name rather than crashing', () => {
45+
expect(() => bodyOf(searchTool as AnyTool, { query: 'q' })).toThrow(/indexName/)
46+
})
47+
})
48+
49+
describe('algolia_copy_move_index destination coercion', () => {
50+
it('accepts a numeric destination emitted as a JSON number', () => {
51+
expect(
52+
bodyOf(copyMoveIndexTool as AnyTool, {
53+
indexName: 'src',
54+
destination: 2025,
55+
operation: 'copy',
56+
}).destination
57+
).toBe('2025')
58+
})
59+
60+
it('still trims a string destination byte-identically', () => {
61+
expect(
62+
bodyOf(copyMoveIndexTool as AnyTool, {
63+
indexName: 'src',
64+
destination: ' dest ',
65+
operation: 'move',
66+
})
67+
).toEqual({ operation: 'move', destination: 'dest' })
68+
})
69+
70+
it('reports a missing required destination by name rather than crashing', () => {
71+
expect(() =>
72+
bodyOf(copyMoveIndexTool as AnyTool, { indexName: 'src', operation: 'copy' })
73+
).toThrow(/destination/)
74+
})
75+
})
76+
77+
describe('algolia_get_records indexName coercion', () => {
78+
it('accepts a numeric fallback index name emitted as a JSON number', () => {
79+
const body = bodyOf(getRecordsTool as AnyTool, {
80+
indexName: 2024,
81+
requests: [{ objectID: 'a' }],
82+
})
83+
expect(body.requests).toEqual([{ objectID: 'a', indexName: '2024' }])
84+
})
85+
86+
it('accepts a numeric per-request index name', () => {
87+
const body = bodyOf(getRecordsTool as AnyTool, {
88+
indexName: 'fallback',
89+
requests: [{ objectID: 'a', indexName: 2024 }],
90+
})
91+
expect(body.requests).toEqual([{ objectID: 'a', indexName: '2024' }])
92+
})
93+
94+
it('still trims string index names byte-identically', () => {
95+
const body = bodyOf(getRecordsTool as AnyTool, {
96+
indexName: ' fallback ',
97+
requests: [{ objectID: 'a' }, { objectID: 'b', indexName: ' own ' }],
98+
})
99+
expect(body.requests).toEqual([
100+
{ objectID: 'a', indexName: 'fallback' },
101+
{ objectID: 'b', indexName: 'own' },
102+
])
103+
})
104+
})

apps/sim/tools/algolia/path_safety.test.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,21 @@
2525
* `fetch` performs — and compares *segment shape* rather than template text,
2626
* because `pathname.startsWith(prefix)` stays green after a segment is popped.
2727
*
28-
* The vector lists are **per parameter**, because the two path parameters do
29-
* not share a charset. Algolia validates `indexName` (`/1/indexes/instant%2Fsearch/settings`
30-
* → `400 indexName is not valid`) but not `objectID`: `foo%2Fbar` and a
31-
* URL-keyed id both return `404 ObjectID does not exist`, i.e. well-formed but
32-
* absent, and Algolia's own conformance suite exercises `Batman and Robin`.
28+
* The vector lists are **per parameter**, because the two path parameters are
29+
* not treated alike. `objectID` is opaque in Algolia's published sources: the
30+
* OpenAPI gives it a bare `type: string` with no `pattern` (where `userID` in
31+
* the same bundled spec carries `^[a-zA-Z0-9 \-*.]+$`, which excludes `/`), the
32+
* official JS client `encodeURIComponent`s it into the segment, and the client
33+
* conformance suite round-trips a space on a record id (`Batman and Robin` →
34+
* `/1/indexes/cts_e2e_browse/Batman%20and%20Robin`) and a literal slash on the
35+
* rules id (`test/with/slash` → `/1/indexes/indexName/rules/test%2Fwith%2Fslash`).
36+
* `indexName` is treated as a named resource here — a separator in it means the
37+
* caller passed the wrong thing — which is a repository choice, not a
38+
* documented Algolia constraint: the spec gives `indexName` a bare
39+
* `type: string` too. (A live probe suggested Algolia rejects a slashed
40+
* `indexName` with `400 indexName is not valid`; that is unverified and
41+
* unreproduced here, and nothing below rests on it.)
42+
*
3343
* A single shared list that contained no `/`-bearing legitimate value is what
3444
* let a guard rejecting `/` in `objectID` ship green.
3545
*/
@@ -81,9 +91,11 @@ const LEGITIMATE = [
8191
* Legitimate ids that only an opaque-id parameter accepts. The URL-keyed form
8292
* is the common site-search pattern; `Batman and Robin` is lifted from
8393
* Algolia's own client conformance suite (`/1/indexes/cts_e2e_browse/Batman%20and%20Robin`
84-
* → 200); `a/../../b` and `docs/` are legal ids whose dot and trailing
85-
* separators must survive as inert `%2F`-joined text rather than be rejected
86-
* or emitted as real separators.
94+
* → 200), which exercises a space rather than a slash — the suite's slash case
95+
* is the rules id (`test/with/slash` → `.../rules/test%2Fwith%2Fslash`);
96+
* `a/../../b` and `docs/` are legal ids whose dot and trailing separators must
97+
* survive as inert `%2F`-joined text rather than be rejected or emitted as real
98+
* separators.
8799
*/
88100
const OPAQUE_LEGITIMATE = [
89101
'https://example.com/docs/getting-started',

apps/sim/tools/algolia/search.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { algoliaIndexName } from '@/tools/algolia/index-name'
12
import type { AlgoliaSearchParams, AlgoliaSearchResponse } from '@/tools/algolia/types'
23
import type { ToolConfig } from '@/tools/types'
34

@@ -106,7 +107,7 @@ export const searchTool: ToolConfig<AlgoliaSearchParams, AlgoliaSearchResponse>
106107
}),
107108
body: (params) => {
108109
const request: Record<string, unknown> = {
109-
indexName: params.indexName.trim(),
110+
indexName: algoliaIndexName(params.indexName, 'indexName'),
110111
query: params.query,
111112
}
112113
if (params.hitsPerPage !== undefined) request.hitsPerPage = Number(params.hitsPerPage)

0 commit comments

Comments
 (0)