Skip to content

Commit 701d013

Browse files
committed
fix(bigquery): derive body identifiers from the path guard, not a bare trim
safeUrlPathSegment deliberately accepts a finite number or a bigint, because an LLM tool call can serialize a numeric-looking id as a JSON number. The .trim() this replaces did not, so a numeric projectId built the path fine and then threw a raw TypeError while building the body — the request died after passing its own guard. Adds canonicalBigQueryId, which round-trips through safeUrlPathSegment and undoes only the percent-encoding, so the body reuses the path guard's accepted input kinds, trimming and dot-segment rejection instead of restating them. Applied to all six body identifiers, including the five .trim() sites that predate this branch and shared the same fragility. Pinned with a numeric-projectId test verified red against the bare trim.
1 parent cfaf77a commit 701d013

5 files changed

Lines changed: 97 additions & 6 deletions

File tree

apps/sim/tools/google_bigquery/create_dataset.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
GoogleBigQueryCreateDatasetParams,
33
GoogleBigQueryCreateDatasetResponse,
44
} from '@/tools/google_bigquery/types'
5+
import { canonicalBigQueryId } from '@/tools/google_bigquery/utils'
56
import type { ToolConfig } from '@/tools/types'
67
import { safeUrlPathSegment } from '@/tools/url-path'
78

@@ -69,8 +70,8 @@ export const googleBigQueryCreateDatasetTool: ToolConfig<
6970
body: (params) => {
7071
const body: Record<string, unknown> = {
7172
datasetReference: {
72-
projectId: params.projectId.trim(),
73-
datasetId: params.datasetId.trim(),
73+
projectId: canonicalBigQueryId(params.projectId, 'projectId'),
74+
datasetId: canonicalBigQueryId(params.datasetId, 'datasetId'),
7475
},
7576
}
7677
if (params.location) body.location = params.location

apps/sim/tools/google_bigquery/create_table.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
GoogleBigQueryCreateTableParams,
33
GoogleBigQueryCreateTableResponse,
44
} from '@/tools/google_bigquery/types'
5+
import { canonicalBigQueryId } from '@/tools/google_bigquery/utils'
56
import type { ToolConfig } from '@/tools/types'
67
import { safeUrlPathSegment } from '@/tools/url-path'
78

@@ -94,9 +95,9 @@ export const googleBigQueryCreateTableTool: ToolConfig<
9495

9596
const body: Record<string, unknown> = {
9697
tableReference: {
97-
projectId: params.projectId.trim(),
98-
datasetId: params.datasetId.trim(),
99-
tableId: params.tableId.trim(),
98+
projectId: canonicalBigQueryId(params.projectId, 'projectId'),
99+
datasetId: canonicalBigQueryId(params.datasetId, 'datasetId'),
100+
tableId: canonicalBigQueryId(params.tableId, 'tableId'),
100101
},
101102
schema: { fields },
102103
}

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
toolsWithoutPathParams,
1818
} from '@/tools/__tests__/path-safety'
1919
import * as bigQueryTools from '@/tools/google_bigquery/index'
20+
import { canonicalBigQueryId } from '@/tools/google_bigquery/utils'
2021

2122
const ORIGIN = 'https://bigquery.googleapis.com'
2223

@@ -97,6 +98,33 @@ describe('projectId agrees between URL and body', () => {
9798
{ name: 'google_bigquery_create_dataset', tool: bigQueryTools.googleBigQueryCreateDatasetTool },
9899
]
99100

101+
/**
102+
* `safeUrlPathSegment` accepts a finite number or a bigint, because an LLM
103+
* tool call can serialize a numeric-looking id as a JSON **number**. A bare
104+
* `.trim()` in the body does not, so the path built fine while the body threw
105+
* a raw `TypeError` — the request died after passing its own guard.
106+
*/
107+
it.each(BODY_TOOLS)('$name builds from a numeric project id', ({ tool }) => {
108+
const params = {
109+
accessToken: 't',
110+
projectId: 123456,
111+
datasetId: 'my_dataset',
112+
defaultDatasetId: 'my_dataset',
113+
tableId: 'my_table',
114+
query: 'SELECT 1',
115+
schema: '[{"name":"id","type":"STRING"}]',
116+
}
117+
118+
const url = new URL((tool.request?.url as (p: typeof params) => string)(params))
119+
const body = (tool.request?.body as ((p: typeof params) => unknown) | undefined)?.(params)
120+
121+
expect(url.pathname).toContain('/projects/123456')
122+
const serialized = JSON.stringify(body)
123+
if (serialized?.includes('projectId')) {
124+
expect(serialized).toContain('"projectId":"123456"')
125+
}
126+
})
127+
100128
it.each(BODY_TOOLS)('$name sends one project id', ({ tool }) => {
101129
const params = {
102130
accessToken: 't',
@@ -119,3 +147,35 @@ describe('projectId agrees between URL and body', () => {
119147
}
120148
})
121149
})
150+
151+
/**
152+
* `canonicalBigQueryId` round-trips through the path guard and undoes only the
153+
* percent-encoding. These assertions pin the two properties that makes it safe
154+
* to use for a JSON body: the round-trip is **exact identity** even for values
155+
* containing `%` or `+`, and every rejection is inherited from the guard rather
156+
* than restated here.
157+
*/
158+
describe('canonicalBigQueryId', () => {
159+
it.each(['a%2Fb', 'a+b', 'a b', 'проект', 'a-b_c.d', 'bigquery-public-data'])(
160+
'returns %j unchanged',
161+
(value) => {
162+
expect(canonicalBigQueryId(value, 'projectId')).toBe(value)
163+
}
164+
)
165+
166+
it('trims the way the path guard does', () => {
167+
expect(canonicalBigQueryId(' my-project ', 'projectId')).toBe('my-project')
168+
})
169+
170+
it('accepts a numeric id, which a bare trim would throw on', () => {
171+
expect(canonicalBigQueryId(123456, 'projectId')).toBe('123456')
172+
})
173+
174+
it('accepts a bigint id, which a bare trim would throw on', () => {
175+
expect(canonicalBigQueryId(9007199254740991n, 'projectId')).toBe('9007199254740991')
176+
})
177+
178+
it.each(['..', '.', 'a/b', 'a\\b'])('inherits the guard rejection of %j', (value) => {
179+
expect(() => canonicalBigQueryId(value, 'projectId')).toThrow(/projectId/)
180+
})
181+
})

apps/sim/tools/google_bigquery/query.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
GoogleBigQueryQueryParams,
33
GoogleBigQueryQueryResponse,
44
} from '@/tools/google_bigquery/types'
5+
import { canonicalBigQueryId } from '@/tools/google_bigquery/utils'
56
import type { ToolConfig } from '@/tools/types'
67
import { safeUrlPathSegment } from '@/tools/url-path'
78

@@ -80,7 +81,7 @@ export const googleBigQueryQueryTool: ToolConfig<
8081
if (params.maxResults !== undefined) body.maxResults = Number(params.maxResults)
8182
if (params.defaultDatasetId) {
8283
body.defaultDataset = {
83-
projectId: params.projectId.trim(),
84+
projectId: canonicalBigQueryId(params.projectId, 'projectId'),
8485
datasetId: params.defaultDatasetId,
8586
}
8687
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { safeUrlPathSegment } from '@/tools/url-path'
2+
3+
/**
4+
* Returns the canonical, unencoded form of an identifier that appears in both
5+
* the request path and the request body.
6+
*
7+
* BigQuery names the same project, dataset and table twice per request — once
8+
* in the URL and once in `datasetReference` / `tableReference` /
9+
* `defaultDataset` — and the two must agree. Deriving the body's value from the
10+
* *path guard* rather than trimming independently is what keeps them in step:
11+
* a second normalization rule is a second thing to drift.
12+
*
13+
* Round-tripping through `safeUrlPathSegment` reuses that guard exactly — its
14+
* accepted input kinds, its trimming, and its rejection of dot segments — and
15+
* then undoes only the percent-encoding, which a JSON body must not carry.
16+
* `encodeURIComponent` and `decodeURIComponent` are exact inverses, so the
17+
* value is the guard's own output rather than an approximation of it.
18+
*
19+
* A bare `params.projectId.trim()` is what this replaces, and it was wrong in a
20+
* way the URL could not reveal: `safeUrlPathSegment` deliberately accepts a
21+
* finite number or a bigint, because an LLM tool call can serialize a
22+
* numeric-looking id as a JSON **number**. The path built fine from `123456`
23+
* while the body threw a bare `TypeError: params.projectId.trim is not a
24+
* function`, so the request died after passing its own guard.
25+
*/
26+
export function canonicalBigQueryId(value: string | number | bigint, paramName: string): string {
27+
return decodeURIComponent(safeUrlPathSegment(value, paramName))
28+
}

0 commit comments

Comments
 (0)