Skip to content

Commit e1e8689

Browse files
authored
fix(search): recheck Zoom approval and bound MCP serialization (#8496)
* fix(search): recheck Zoom approval and bound MCP serialization * fix(search): preserve byte-only MCP response limits * fix(search): reject inherited JSON serializers
1 parent 96fd30d commit e1e8689

8 files changed

Lines changed: 219 additions & 4 deletions

File tree

‎apps/sim/.env.example‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,5 +269,6 @@ CRON_SECRET=your_cron_secret # Use `openssl rand -hex 32` to generate. Authentic
269269

270270
# Zoom member search: separate General OAuth app to preserve workflow grants.
271271
# Register ${NEXT_PUBLIC_APP_URL}/api/mcp/oauth/callback with the two meeting read scopes.
272+
# ZOOM_SEARCH=false # Off-AppConfig fallback; set true to enable for eligible organization-owned scopes
272273
# ZOOM_MCP_CLIENT_ID=
273274
# ZOOM_MCP_CLIENT_SECRET=

‎apps/sim/lib/core/utils/bounded-json.test.ts‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it, vi } from 'vitest'
2-
import { stringifyBoundedJson } from '@/lib/core/utils/bounded-json'
2+
import { isJsonWithinByteLimit, stringifyBoundedJson } from '@/lib/core/utils/bounded-json'
33

44
describe('bounded JSON', () => {
55
it.each([
@@ -13,11 +13,14 @@ describe('bounded JSON', () => {
1313
const bytes = Buffer.byteLength(json, 'utf8')
1414
expect(stringifyBoundedJson(value, bytes)).toBe(json)
1515
expect(stringifyBoundedJson(value, bytes - 1)).toBeUndefined()
16+
expect(isJsonWithinByteLimit(value, bytes)).toBe(true)
17+
expect(isJsonWithinByteLimit(value, bytes - 1)).toBe(false)
1618
})
1719

1820
it('rejects cycles, excessive depth, and excessive nodes', () => {
1921
const cyclic: Record<string, unknown> = {}
2022
cyclic.self = cyclic
23+
expect(isJsonWithinByteLimit(cyclic, 1024)).toBe(false)
2124
let deep: unknown = 'leaf'
2225
for (let index = 0; index < 66; index++) deep = { child: deep }
2326
for (const value of [
@@ -37,6 +40,7 @@ describe('bounded JSON', () => {
3740
const custom = Object.defineProperty({}, 'toJSON', { value: toJSON })
3841
for (const value of [accessor, custom, { output: new Uint8Array([1, 2, 3]) }]) {
3942
expect(stringifyBoundedJson(value, 1024)).toBeUndefined()
43+
expect(isJsonWithinByteLimit(value, 1024)).toBe(false)
4044
}
4145
expect(getter).not.toHaveBeenCalled()
4246
expect(toJSON).not.toHaveBeenCalled()
@@ -47,6 +51,7 @@ describe('bounded JSON', () => {
4751
const serialize = vi.spyOn(JSON, 'stringify')
4852
try {
4953
expect(stringifyBoundedJson(value, 1024)).toBeUndefined()
54+
expect(isJsonWithinByteLimit(value, 1024)).toBe(false)
5055
expect(serialize).not.toHaveBeenCalled()
5156
} finally {
5257
serialize.mockRestore()
@@ -61,6 +66,7 @@ describe('bounded JSON', () => {
6166
const serialize = vi.spyOn(JSON, 'stringify')
6267
try {
6368
expect(stringifyBoundedJson(value, 1024)).toBeUndefined()
69+
expect(isJsonWithinByteLimit(value, 1024)).toBe(false)
6470
expect(serialize).not.toHaveBeenCalled()
6571
} finally {
6672
serialize.mockRestore()
@@ -71,6 +77,7 @@ describe('bounded JSON', () => {
7177
const get = vi.fn(() => 'UNADMITTED')
7278
const value = new Proxy({ text: 'admitted' }, { get })
7379
expect(stringifyBoundedJson(value, 1024)).toBe('{"text":"admitted"}')
80+
expect(isJsonWithinByteLimit(value, 19)).toBe(true)
7481
expect(get).not.toHaveBeenCalled()
7582
})
7683

@@ -79,12 +86,37 @@ describe('bounded JSON', () => {
7986
const prototype = Object.create(Array.prototype, { 0: { get } })
8087
const value = Object.setPrototypeOf(Array(1), prototype)
8188
expect(stringifyBoundedJson(value, 1024)).toBe('[null]')
89+
expect(isJsonWithinByteLimit(value, 6)).toBe(true)
8290
expect(get).not.toHaveBeenCalled()
8391
})
8492

8593
it('allows repeated references without treating them as a cycle', () => {
8694
const result = { answer: 42 }
8795
const value = { rawResponse: result, modelResponse: result }
8896
expect(stringifyBoundedJson(value, 1024)).toBe(JSON.stringify(value))
97+
expect(isJsonWithinByteLimit(value, Buffer.byteLength(JSON.stringify(value)))).toBe(true)
8998
})
99+
100+
it('counts an ordinary toJSON data field without invoking custom serialization', () => {
101+
const value = { toJSON: 'ordinary JSON field', nested: [{}, [], { flag: true }] }
102+
const bytes = Buffer.byteLength(JSON.stringify(value))
103+
expect(isJsonWithinByteLimit(value, bytes)).toBe(true)
104+
expect(isJsonWithinByteLimit(value, bytes - 1)).toBe(false)
105+
})
106+
107+
it.each(['method', 'accessor'] as const)(
108+
'rejects an inherited toJSON %s without invoking it or ignoring an own shadow',
109+
(kind) => {
110+
const serialize = vi.fn(() => 'x'.repeat(2048))
111+
const prototype = Object.create(Array.prototype, {
112+
toJSON: kind === 'method' ? { value: serialize } : { get: serialize },
113+
})
114+
const value = Object.setPrototypeOf([], Object.create(prototype))
115+
expect(isJsonWithinByteLimit(value, 1024)).toBe(false)
116+
expect(serialize).not.toHaveBeenCalled()
117+
Object.defineProperty(value, 'toJSON', { value: null })
118+
expect(isJsonWithinByteLimit(value, 2)).toBe(true)
119+
expect(serialize).not.toHaveBeenCalled()
120+
}
121+
)
90122
})

‎apps/sim/lib/core/utils/bounded-json.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,98 @@ function quotedStringBytes(value: string, remaining: number): number | undefined
2121
return bytes <= remaining ? bytes : undefined
2222
}
2323

24+
/** Measures plain JSON iteratively without serialization, copying, or additional node/depth caps. */
25+
export function isJsonWithinByteLimit(value: unknown, maxBytes: number): boolean {
26+
if (!Number.isFinite(maxBytes) || maxBytes < 0 || value === undefined) return false
27+
const invalid = Symbol('invalid JSON')
28+
const ancestors = new WeakSet<object>()
29+
const stack: {
30+
value: object
31+
entries: Generator<[string | null, unknown]>
32+
count: number
33+
}[] = []
34+
let bytes = 0
35+
const omitted = (item: unknown) =>
36+
item === undefined || typeof item === 'function' || typeof item === 'symbol'
37+
function* entries(container: object): Generator<[string | null, unknown]> {
38+
if (Array.isArray(container)) {
39+
const length = Object.getOwnPropertyDescriptor(container, 'length')?.value
40+
if (typeof length !== 'number') {
41+
yield [null, invalid]
42+
return
43+
}
44+
for (let index = 0; index < length; index++) {
45+
const field = Object.getOwnPropertyDescriptor(container, index)
46+
if (field && !('value' in field)) {
47+
yield [null, invalid]
48+
return
49+
}
50+
yield [null, omitted(field?.value) ? null : field?.value]
51+
}
52+
} else {
53+
for (const key in container) {
54+
const field = Object.getOwnPropertyDescriptor(container, key)
55+
if (!field?.enumerable) continue
56+
if (!('value' in field)) {
57+
yield [key, invalid]
58+
return
59+
}
60+
if (!omitted(field.value)) yield [key, field.value]
61+
}
62+
}
63+
}
64+
try {
65+
let item: unknown = value
66+
for (;;) {
67+
if (typeof item === 'string') {
68+
const count = quotedStringBytes(item, maxBytes - bytes)
69+
if (count === undefined) return false
70+
bytes += count
71+
} else if (item === null) bytes += 4
72+
else if (typeof item === 'number') bytes += Number.isFinite(item) ? String(item).length : 4
73+
else if (typeof item === 'boolean') bytes += item ? 4 : 5
74+
else if (typeof item === 'object') {
75+
if (ancestors.has(item)) return false
76+
const prototype = Object.getPrototypeOf(item)
77+
if (!Array.isArray(item) && prototype !== Object.prototype && prototype !== null)
78+
return false
79+
for (let owner: object | null = item; owner; owner = Object.getPrototypeOf(owner)) {
80+
const serializer = Object.getOwnPropertyDescriptor(owner, 'toJSON')
81+
if (!serializer) continue
82+
if (!('value' in serializer) || typeof serializer.value === 'function') return false
83+
break
84+
}
85+
bytes += 2
86+
ancestors.add(item)
87+
stack.push({ value: item, entries: entries(item), count: 0 })
88+
} else return false
89+
if (bytes > maxBytes) return false
90+
for (;;) {
91+
const frame = stack[stack.length - 1]
92+
if (!frame) return true
93+
const next = frame.entries.next()
94+
if (next.done) {
95+
ancestors.delete(frame.value)
96+
stack.pop()
97+
continue
98+
}
99+
if (frame.count++ > 0) bytes++
100+
const [key, child] = next.value
101+
if (key !== null) {
102+
const count = quotedStringBytes(key, maxBytes - bytes)
103+
if (count === undefined) return false
104+
bytes += count + 1
105+
}
106+
if (bytes > maxBytes) return false
107+
item = child
108+
break
109+
}
110+
}
111+
} catch {
112+
return false
113+
}
114+
}
115+
24116
/** Captures bounded plain JSON once, without executing accessors or serializing the source graph. */
25117
export function stringifyBoundedJson(value: unknown, maxBytes: number): string | undefined {
26118
let nodes = 0

‎apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from '@sim/db/schema'
1111
import * as dns from '@sim/security/dns'
1212
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
13+
import { createDeferred } from '@sim/testing/helpers/deferred'
1314
import { getPostgresErrorCode } from '@sim/utils/errors'
1415
import { generateId } from '@sim/utils/id'
1516
import { toRecord } from '@sim/utils/object'
@@ -18,7 +19,7 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi }
1819
import { listSearchIntegrationsContract } from '@/lib/api/contracts/knowledge/search-integrations'
1920
import { env } from '@/lib/core/config/env'
2021
import { createOrganizationAccountsGroup } from '@/lib/credential-groups/workspace-accounts'
21-
import { tryAcquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
22+
import { acquireAdvisoryXactLock, tryAcquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
2223
import {
2324
approveSearchIntegration,
2425
listSearchIntegrations,
@@ -195,6 +196,63 @@ describe('atomic organization live Search MCP setup', () => {
195196
}
196197
)
197198

199+
it('rejects Zoom approval when rollout is disabled while waiting for the accounts lock', async () => {
200+
const group = await db.transaction((tx) =>
201+
createOrganizationAccountsGroup(tx, ids.organization, ids.owner)
202+
)
203+
await db.insert(mcpServers).values({
204+
id: generateId(),
205+
organizationId: ids.organization,
206+
credentialGroupId: group.id,
207+
managedConnectorId: 'zoom',
208+
name: 'Zoom',
209+
transport: 'streamable-http',
210+
url: 'https://mcp.zoom.us/mcp/meeting/streamable',
211+
authType: 'oauth',
212+
enabled: true,
213+
createdBy: ids.owner,
214+
})
215+
Object.assign(env, { ZOOM_SEARCH: true })
216+
const before = await snapshot()
217+
const locked = createDeferred<number>()
218+
const release = createDeferred<void>()
219+
const blocker = db.transaction(async (tx) => {
220+
await acquireAdvisoryXactLock(
221+
tx,
222+
'search_accounts',
223+
`search-accounts:organization:${ids.organization}`
224+
)
225+
const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
226+
locked.resolve(connection.pid)
227+
await release.promise
228+
})
229+
const blockerPid = await locked.promise
230+
const attempt = approve('zoom').catch((error: unknown) => error)
231+
try {
232+
await vi.waitFor(
233+
async () => {
234+
const [state] = await db.execute<{ waiting: boolean }>(sql`
235+
SELECT EXISTS (
236+
SELECT 1 FROM pg_stat_activity
237+
WHERE ${blockerPid} = ANY(pg_blocking_pids(pid))
238+
) AS waiting
239+
`)
240+
expect(state.waiting).toBe(true)
241+
},
242+
{ timeout: 5_000 }
243+
)
244+
Object.assign(env, { ZOOM_SEARCH: false })
245+
} finally {
246+
release.resolve()
247+
await blocker
248+
await attempt
249+
}
250+
expect(await attempt).toMatchObject({ code: 'forbidden' })
251+
expect(await snapshot()).toEqual(before)
252+
Object.assign(env, { ZOOM_SEARCH: true })
253+
await expect(approve('zoom')).resolves.toMatchObject({ approved: true })
254+
})
255+
198256
it('resolves the sign-in server before the approval takes the accounts lock', async () => {
199257
const lockHeldDuringLookup: boolean[] = []
200258
vi.mocked(dns.resolveHostAddresses).mockImplementationOnce(async () => {

‎apps/sim/lib/sim-search/live/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ Zoom Search defaults off for organization-scoped rollout. Enable selected organi
159159
}
160160
```
161161

162-
Only the canonical organization ID participates in this rollout check. For local or self-hosted deployments, `ZOOM_SEARCH=true` enables Zoom Search globally; leave that boolean fallback off for an organization-targeted rollout. Setup, enrollment and retrieval enforce the flag. The dedicated Zoom MCP Search connector is gated wherever it is invoked, including generic MCP tools; the standard workflow Zoom OAuth/tools remain available. Disabling the flag preserves saved grants and conversations while denying subsequent Search use; existing approvals can still be removed and connected accounts disconnected. Other providers retain the shared Search and credential-group availability policies without a separate provider rollout gate.
162+
Only the canonical organization ID participates in this rollout check. For local or self-hosted deployments, `ZOOM_SEARCH=true` enables Zoom Search for all otherwise eligible organization-owned scopes; personal workspaces without an organization cannot use managed connected accounts. Leave that boolean fallback off for an organization-targeted rollout. Setup, enrollment and retrieval enforce the flag. The dedicated Zoom MCP Search connector is gated wherever it is invoked, including generic MCP tools; the standard workflow Zoom OAuth/tools remain available. Disabling the flag preserves saved grants and conversations while denying subsequent Search use; existing approvals can still be removed and connected accounts disconnected. Other providers retain the shared Search and credential-group availability policies without a separate provider rollout gate.
163163

164164
### Shared invariants
165165

‎apps/sim/lib/sim-search/live/managed-mcp-payload.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { isRecordLike } from '@sim/utils/object'
2+
import { isJsonWithinByteLimit } from '@/lib/core/utils/bounded-json'
23
import type { McpToolResult } from '@/lib/mcp/types'
34
import { NativeSearchError } from '@/lib/sim-search/live/http'
45

56
const MAX_SEARCH_MCP_PAYLOAD_BYTES = 4 * 1024 * 1024
67

78
/** MCP text is untrusted provider data; malformed structured search output is never an empty success. */
89
export function managedMcpPayload(result: McpToolResult, label: string): unknown {
9-
if (Buffer.byteLength(JSON.stringify(result), 'utf8') > MAX_SEARCH_MCP_PAYLOAD_BYTES)
10+
if (!isJsonWithinByteLimit(result, MAX_SEARCH_MCP_PAYLOAD_BYTES))
1011
throw new NativeSearchError(
1112
'unavailable',
1213
`${label} response exceeded the search size limit. Narrow the query.`

‎apps/sim/lib/sim-search/live/managed-mcp.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,4 +127,30 @@ describe('managed search MCP read boundary', () => {
127127
expect((failure as NativeSearchError).message).toContain('existing Granola account')
128128
expect((failure as NativeSearchError).message).not.toContain('private-provider-detail')
129129
})
130+
131+
it('rejects escaped MCP payload overflow before allocating its JSON representation', () => {
132+
const result = { structuredContent: { ['\u0000'.repeat(800_000)]: 'value' } }
133+
const serialize = vi.spyOn(JSON, 'stringify').mockImplementation(() => {
134+
throw new Error('Oversized payload reached serialization')
135+
})
136+
try {
137+
expect(() => managedMcpPayload(result, 'Fireflies')).toThrow('size limit')
138+
expect(serialize).not.toHaveBeenCalled()
139+
} finally {
140+
serialize.mockRestore()
141+
}
142+
})
143+
144+
it.each(['wide', 'deep'] as const)(
145+
'preserves byte-small %s MCP responses without imposing capture limits',
146+
(shape) => {
147+
let content: unknown = shape === 'wide' ? Array.from({ length: 100_000 }, () => 0) : 'leaf'
148+
if (shape === 'deep') {
149+
for (let index = 0; index < 128; index++) content = { child: content }
150+
}
151+
const result = { structuredContent: content }
152+
expect(Buffer.byteLength(JSON.stringify(result), 'utf8')).toBeLessThan(4 * 1024 * 1024)
153+
expect(managedMcpPayload(result, 'Fireflies')).toEqual(content)
154+
}
155+
)
130156
})

‎apps/sim/lib/sim-search/live/member-setup.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ export async function addOrganizationSearchMcpProvider(
109109
)
110110
.limit(1)
111111
if (existing) {
112+
if (!(await isSearchProviderEnabled(provider, { kind: 'organization', organizationId })))
113+
throw new OrchestrationError(
114+
'forbidden',
115+
'Zoom Search is not available for this organization'
116+
)
112117
if (!existing.enabled)
113118
throw new OrchestrationError(
114119
'validation',

0 commit comments

Comments
 (0)