v0.7.40: library, mcp hardening#5761
Conversation
* feat(library): add Best Relay.app Alternatives in 2026 post * improvement(library): clarify Sim free tier has usage limits in comparison table
…ghs DoS) (#5756) * fix(security): bound YAML alias expansion in file-parser to prevent DoS The YAML parser called yaml.load() then JSON.stringify() on the result. YAML aliases (*anchor) resolve to shared references, so yaml.load cheaply builds a compact DAG, but JSON.stringify expands that DAG into a full tree, duplicating every shared node. A crafted sub-1KB .yaml/.yml document could therefore expand to hundreds of MB / GB during serialization, pinning CPU and OOM-killing the shared parse/ingestion worker (CWE-776). Add a bounded, iterative traversal that runs before JSON.stringify and aborts once expanded node count, estimated serialized size, or nesting depth exceeds safe caps. This detects the amplification against the compact DAG before any allocation, and also terminates cyclic anchor structures. Replaces the unbounded recursive getYamlDepth (which spread large arrays into Math.max(...array), risking a stack overflow) with the same bounded walk. * harden YAML guard: charge keys + escaping, bound stack, fail closed Addresses review findings on the alias-expansion guard: - Charge object keys, not just values. JSON.stringify re-emits every key on each alias expansion, so an aliased object with a long key amplified without being counted. Keys are now charged against the size cap. - Compute the exact JSON-escaped string length (quotes, backslashes, control chars) instead of a flat multiplier. Plain text is charged its true 1:1 size (no false rejection of large legitimate documents) while escape-heavy strings are charged their real, larger cost (no cap bypass). - Count each node as it is enqueued and only push container nodes onto the traversal stack. A pathologically wide fan-out now trips a cap during the enqueue loop instead of first materializing millions of stack entries and exhausting memory inside the guard itself. - Fail closed in POST /api/files/parse: a YamlComplexityError rejection is now re-thrown (mirroring isPayloadSizeLimitError) rather than silently falling back to storing the crafted document as raw text. * yaml guard: charge lone surrogates at their escaped length serializedStringLength treated surrogate code units as cost 1, but well-formed JSON.stringify escapes a lone surrogate to \uXXXX (six units). Charge lone surrogates as 6 and valid high+low pairs as-is (two units), matching JSON.stringify exactly. Verified against JSON.stringify across plain text, escapes, control chars, lone high/low surrogates, and valid pairs.
* feat(library): add BYOK multi-model AI agent builder post * fix(library): scope BYOK FAQ to providers in the actual BYOK contract
…TP/2 (#5757) * improvement(mcp): negotiate HTTP/2 for MCP transport MCP servers are commonly behind HTTP/2 fronts (CDNs, cloud LBs), but the SSRF-pinned undici Agent is HTTP/1.1-only unless it opts into h2 via ALPN. Add an allowH2 option to createPinnedFetch (default false, so LLM-provider callers are unchanged) and centralize the MCP h2 decision in one createPinnedMcpFetch routed through the transport, OAuth probe, and SSRF-guarded fetch. Pinning is unaffected: the pinned lookup forces every connection to the resolved IP. * fix(mcp): correct auth-type handling, OAuth-pending UX, and safe error logging - Classify a non-OAuth UnauthorizedError as an auth failure instead of an OAuth redirect, so static-bearer servers that merely advertise OAuth stop being diverted into the OAuth flow (fixes the class of server that couldn't connect). - Reset a server to disconnected when it is switched to OAuth, so it can't falsely read as connected before completing its auth flow. - Surface OAuth-pending state as 'OAuth authorization required' in the server list and refresh action instead of a generic 'Not Connected'. - Return an actionable 422 for OAuth servers that don't support dynamic client registration. - Redact error messages/cause/session-ids from MCP transport/connect logs via a shared getMcpSafeErrorDiagnostics helper. * fix(mcp): reset connection on any auth-type flip, scope h2 to live transport * fix(mcp): close pinned h2 Agent on disconnect and revoke OAuth tokens on auth-type change * fix(mcp): release pinned Agent on failed connect and point DCR error at client-id/secret setup * fix(mcp): make pinned Agent teardown idempotent to avoid double-destroy on cancel/failed connect
…5760) * feat(mcp): reuse warm connections for tool execution and discovery * fix(mcp): make connection pool concurrency-safe and auth-scoped * fix(mcp): decouple pool validity from updatedAt, widen dead-connection detection, guard ping race * fix(mcp): retire pooled connections on auth failure and server delete * improvement(mcp): defer bulk-discovery env resolution to the pool miss path * fix(mcp): retire in-flight creates evicted mid-connect via server generation * fix(mcp): use evicted-mid-create connections one-shot and decouple pool eviction from cache clear * improvement(mcp): dedupe create thunks into buildClient, harden dispose against liveness race * fix(mcp): close acquire-gap races (re-resolve after stale ping, skip closing entries) * fix(mcp): retry auth failures in executeTool; simplify acquire re-check and clear generations on dispose * fix(mcp): let bulk discovery reconnect a lost notification connection with current config * fix(mcp): recover rotated credentials on discovery via shared fetchServerTools auth-retry * chore(mcp): add debug logging for pool hit/miss/eviction observability
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview MCP: Tool execution and discovery reuse warm connections per Workflows: Loading latest execution state only passes non-null Content: Two new library articles (Relay.app alternatives, BYOK multi-model builder). Reviewed by Cursor Bugbot for commit bd636d4. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bd636d4. Configure here.
| */ | ||
| export function isYamlComplexityError(error: unknown): error is YamlComplexityError { | ||
| return error instanceof YamlComplexityError | ||
| } |
There was a problem hiding this comment.
YAML fail-closed check can miss
High Severity
isYamlComplexityError relies only on instanceof, but the complexity error is thrown from the YAML parser loaded via require() in file-parsers/index.ts while the parse route checks it through a static import. On Next.js 16 (Turbopack by default), that can yield two class identities, so the guard fails, the handler falls back to raw-text parsing, and a rejected alias bomb is returned as success: true instead of fail-closed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit bd636d4. Configure here.
Greptile SummaryThis release bundles a YAML alias-expansion DoS fix, a warm MCP connection pool, OAuth UX improvements, and a chat execution-state loader bug fix. The changes are well-scoped and thoroughly tested.
Confidence Score: 4/5Safe to merge; no regressions in core data paths and the security fix is correctly implemented end-to-end. The YAML fix is well-hardened and properly propagated through the fallback path. The connection pool handles the key concurrency races correctly. Two things keep the score from being higher: the DCR error detection in the OAuth route relies on a brittle substring match that would silently degrade if the SDK message changes, and the serverGenerations map in the pool accumulates entries for every server ever evicted without pruning, which could matter for long-running processes with high server churn. apps/sim/lib/mcp/connection-pool.ts (serverGenerations growth) and apps/sim/app/api/mcp/oauth/start/route.ts (string-match DCR detection) deserve a second look. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as Tool Executor / Discovery
participant Service as McpService
participant Pool as McpConnectionPool
participant Client as McpClient
participant Server as MCP Server
Caller->>Service: executeTool() / discoverServerTools()
Service->>Pool: acquire(key, serverId, create)
alt Pool hit
Pool->>Pool: tryReuse(entry): ping if stale
Pool-->>Service: ConnectionLease (existing client)
else Pool miss
Pool->>Service: invoke create()
Service->>Service: resolveConfigEnvVars + SSRF pin
Service->>Client: new McpClient(pinnedFetch + H2)
Client->>Server: connect()
Server-->>Client: initialized
Client-->>Service: connected McpClient
Service-->>Pool: entry registered
Pool-->>Service: ConnectionLease
end
Service->>Client: listTools() / callTool()
Client->>Server: tools/list or tools/call
Server-->>Client: result
Client-->>Service: result
alt Success
Service->>Pool: "lease.release(poison=false)"
Pool->>Pool: keep connection warm
else Dead connection (400/401/404/reset)
Service->>Pool: "lease.release(poison=true)"
Pool->>Pool: "retire entry, closeIfIdle when borrowers=0"
end
Service-->>Caller: result / error
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller as Tool Executor / Discovery
participant Service as McpService
participant Pool as McpConnectionPool
participant Client as McpClient
participant Server as MCP Server
Caller->>Service: executeTool() / discoverServerTools()
Service->>Pool: acquire(key, serverId, create)
alt Pool hit
Pool->>Pool: tryReuse(entry): ping if stale
Pool-->>Service: ConnectionLease (existing client)
else Pool miss
Pool->>Service: invoke create()
Service->>Service: resolveConfigEnvVars + SSRF pin
Service->>Client: new McpClient(pinnedFetch + H2)
Client->>Server: connect()
Server-->>Client: initialized
Client-->>Service: connected McpClient
Service-->>Pool: entry registered
Pool-->>Service: ConnectionLease
end
Service->>Client: listTools() / callTool()
Client->>Server: tools/list or tools/call
Server-->>Client: result
Client-->>Service: result
alt Success
Service->>Pool: "lease.release(poison=false)"
Pool->>Pool: keep connection warm
else Dead connection (400/401/404/reset)
Service->>Pool: "lease.release(poison=true)"
Pool->>Pool: "retire entry, closeIfIdle when borrowers=0"
end
Service-->>Caller: result / error
Reviews (1): Last reviewed commit: "feat(mcp): reuse warm connections for to..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const OAUTH_START_TTL_MS = 10 * 60 * 1000 | ||
| const MAX_SURFACED_ERROR_LENGTH = 250 | ||
| const DCR_UNSUPPORTED_MESSAGE = | ||
| "This server doesn't support automatic OAuth client registration. Add a pre-registered OAuth client ID and secret, or configure a token instead." | ||
|
|
||
| function isDynamicClientRegistrationUnsupported(error: unknown): boolean { | ||
| return getErrorMessage(error, '') |
There was a problem hiding this comment.
isDynamicClientRegistrationUnsupported checks for a hardcoded substring of the MCP SDK's error message. If the SDK ever rephrases that message, the function silently returns false, the 422 branch is skipped, and the error propagates to the outer catch as a generic 500 — the user sees no actionable guidance instead of the specific instructions to add a pre-registered client ID/secret. Consider asserting against a typed SDK error class or error code if one is available, or at least capturing the specific message string from the SDK source so it's obvious when a version bump needs updating here.
| private entries = new Map<string, PoolEntry>() | ||
| private pending = new Map<string, Promise<PoolEntry>>() | ||
| /** Per-server counter bumped by `evictServer`; an in-flight create built against an older value is retired on completion. */ | ||
| private serverGenerations = new Map<string, number>() | ||
| private idleCheckTimer: ReturnType<typeof setInterval> | null = null | ||
| private disposed = false |
There was a problem hiding this comment.
serverGenerations map grows without bound
evictServer increments a server's entry in serverGenerations but nothing removes stale entries for servers that have since been deleted. dispose() calls serverGenerations.clear() but the singleton pool is never disposed in normal operation. In a long-running process with many server create/delete cycles, each server ID accumulates a permanent map entry. The fix is to delete the entry after retirement: this.serverGenerations.delete(serverId) at the end of evictServer, once all entries for that server have been confirmed retired.
#5762) Response-side Zod .catch() tolerance on the three strict-enum-over-free-text MCP columns (transport/authType/connectionStatus) so one legacy row can't fail the whole server-list validation; fork copy normalizes transport; create/upsert path resets connection status on any auth-type flip (mirrors update path); bulk discovery drops the positive tool cache on OAuth-pending. Request bodies stay strict.
… can't strand the connect (#5767) * fix(mcp): deliver OAuth callback result over BroadcastChannel so COOP can't strand the connect An MCP provider whose authorization page sets `Cross-Origin-Opener-Policy: same-origin` (e.g. Gauge) severs `window.opener` when the popup navigates through it, so the callback's `window.opener.postMessage` was silently lost and the row hung on "Connecting…" forever — even when the callback succeeded. - Signal completion over a same-origin `BroadcastChannel` instead, which is origin-scoped and immune to opener severance (the MDN/Chrome-recommended COOP workaround). Scope the message by workspaceId so other open workspaces ignore it. - Log every OAuth callback failure with its reason + serverId. The early-return gates previously returned a silent `ok:false` popup close, so a failed authorization was undiagnosable from the server logs. * fix(mcp): react to the OAuth broadcast only in the tab that opened the popup A BroadcastChannel reaches every same-origin tab, so the previous workspace-id scoping (which the success path carried but failure paths omitted) still let unrelated tabs clear state, refetch, and show spurious toasts. Gate every reaction on whether this tab actually has an in-flight popup for the result's server (`popupIntervalsRef`) — strictly more precise than workspace scoping and correct for both cross-workspace and same-workspace-second-tab cases. Removes the now-redundant workspaceId from the callback message. * fix(mcp): decouple OAuth result correlation from popup.closed Cursor flagged two defects in the previous gate, both rooted in keying the BroadcastChannel filter on `popupIntervalsRef` (the popup.closed poll map): - A genuine completion was dropped whenever the poll had already removed the server's entry — and COOP can make `popup.closed` misreport, which is exactly the case this PR targets, so the fix could silently fail to apply. - A result without a serverId fell back to "any in-flight popup", waking unrelated same-origin tabs with spurious toasts/refetches. Introduce `pendingFlowsRef` (serverId -> safety timeout) as the correlation source of truth: cleared only on completion or a 10-min timeout (matching the server OAuth start TTL), never by popup.closed. The popup.closed poll now only clears the spinner (best-effort abandon UX) and never touches correlation, so a real completion is always processed. Results without a serverId are ignored; the initiating tab's safety timeout clears its own "Connecting…". * fix(mcp): correlate the OAuth result on the state nonce, not serverId Cursor flagged that a failure which can't resolve a serverId (notably invalid_state) broadcasts ok:false with no serverId, so the serverId-keyed gate ignored it and the initiating tab sat on "Connecting…" until the safety timeout with no error feedback. Correlate on the OAuth `state` instead — the per-flow nonce the callback echoes on every result, success or failure. The client parses it from the authorization URL and keys the in-flight map by it; the callback includes it on every response via a `respond` helper. This is the canonical popup-OAuth correlation: it reaches the initiating tab even when no serverId exists, and — because each flow has a unique state — it also fixes the same-user-same-server-in-two-tabs edge a serverId key left open. Results with no parseable state (a malformed callback) are still ignored; that flow clears via its timeout. * fix(mcp): drop a server's prior in-flight OAuth flow when it is retried Each start mints a new `state`, so an abandoned attempt's safety timeout was keyed under a different state than its retry and never cleared. In the contrived case where both stayed pending ~10 min, the stale timer would clear the newer flow's spinner. Sweep any prior in-flight flow for the same serverId on a new start (replacing the can't-happen same-state check). Result delivery was already correct; this makes the state machine airtight.
2 participants
Uh oh!
There was an error while loading. Please reload this page.