fix(sandbox): prevent cross-tenant data access in the plugin/template-tag sandbox - #10302
fix(sandbox): prevent cross-tenant data access in the plugin/template-tag sandbox#10302kwburns-kong wants to merge 50 commits into
Conversation
Adds the sandbox core for routing user-plugin actions (request/requestGroup/ workspace/document) through the QuickJS sandbox, mirroring the H1 hook path. - marshal.ts: envelope gains actionKind/actionLabel/actionDomainData. An action is fire-and-effect — it reads the copied-in domain models and performs side effects only through the capability-gated context.* bridge; nothing marshals back. - in-sandbox-bootstrap.ts: __invokeAction() rebuilds context via __buildContext, resolves the kind-specific action list, finds the entry by label, and calls action(context, actionDomainData); ACTION_RUNNER drives it. Locked alongside the other internals. - plugin-tag-sandbox.ts: runner selection picks ACTION_RUNNER when actionKind is set. - sandbox-surface.ts + snapshot: __invokeAction added to the reviewed internal set. - sandbox-actions.test.ts: label match + domainData passthrough, capability gating (ungranted context.network absent), missing-label error, kind-specific list. Host wiring (plugin.runUserAction handler, invoke-method routing, e2e) follows.
Host + plugin-window wiring for the in-sandbox action API. - templating-worker-database.ts: runActionInSandbox() mirrors runRequestHookInSandbox (same bridge/grants/crypto), building an action envelope; nothing is marshaled back. New protocol handler plugin.runUserAction resolves the plugin via resolveTrustedPlugin (registry-trusted directory + permissions, not caller-supplied) before dispatch. - types.ts: PluginToMainAPIPaths gains 'plugin.runUserAction'. - invoke-method.ts (plugin window): executeAction routes user plugins (directory !== '') through plugin.runUserAction when the sandbox is enabled; bundle plugins and the flag-off path stay in-process. Tag actions (templateTag.actions) are unchanged — the L1 manifest does not discover them, so they remain a flag-off surface. Actions are UI-triggered only, so there is no node-runtime/inso path to gate.
…andbox A request action writes a path canary (INSOMNIA_TEMPLATE_SANDBOX) plus whether it received the domain models into plugin storage; a sibling template tag reads it back (actions are fire-and-effect, so the canary rides the side effect). Flag off → the action runs in-process; flag on → the same action runs in the sandbox (its in-process fn is a throw-stub after discovery, so a successful write proves routing) and the domain models still reach it. Asserts on unique canary values, not header names.
…ites Mirrors the realpath re-check getPluginEntrySource/readPluginModuleMap already apply, so a symlinked directory under responses/ can't redirect a write outside it despite the string-based containment check passing.
…ink escapes Bound allowed-root matching by a path separator (not a bare startsWith) so a sibling directory sharing a name prefix can't pass, and resolve symlinks on both the target path and each allowed root so a link inside an allowed directory can't redirect outside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s fixture The 'invalid' dataFolders entry only worked by string-prefix coincidence with invalid.cert/invalid.key, which the separator-boundary fix in ebece69 correctly no longer allows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nstance A disallowed or unreadable path on one client certificate threw out of the forEach loop and aborted curl setup for the whole request, so a single bad certificate silently dropped every other valid one too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lders fixture 'certificates/client' isn't a real directory — client.crt/client.key are flat files under certificates/. It only worked by the same string-prefix coincidence the separator-boundary fix in ebece69 correctly no longer allows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ock cross-collection write The bridge handler forwarded the caller-supplied originCredential document and patch straight to docUpdate with no allowlist, so a caller could patch an arbitrary credential's _id/type/parentId. Reload the real credential server-side by id and strip identity fields from the patch before writing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d response, not the caller's bodyPath assertResponseBodyPathReadOwnership only confirmed a supplied bodyPath belonged to *some* persisted response, so a caller supplying a real response id alongside a different, real response's bodyPath still read that other response's body. When an id is supplied, re-load the response server-side and read only its own bodyPath, ignoring the caller's; fall back to the existing bodyPath-ownership check when no id is available (the pre-persistence response-hook call site). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e reaching services.*
A caller-supplied id/parentId/key reached NeDB unstringified in ~11
pluginToMainAPI handlers. NeDB is a MongoDB-style query engine, so an object
like { $ne: null } in place of a plain string id is interpreted as a query
operator rather than a literal value, matching an arbitrary document instead
of failing "not found" (confirmed live: request.getById({ $ne: null }) leaked
a real, unrelated request's id through a zero-permission template tag).
Wrap every bare id/parentId/key/requestId/environmentId argument in String(...)
before it reaches its services.* call. Adds a source-scanning detector
(templating-worker-database-coercion-surface.ts, wired into npm run
sandbox:coercion(:test)) that enumerates every handler programmatically so a
future addition to this family is covered automatically, plus unit and
Playwright smoke coverage confirmed red against unpatched source first.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oke test Item 1's id-reload fix (commit 92b25cb) landed with unit coverage but no Playwright smoke test, despite the audit plan requiring one. Adds it: a zero-permission template tag looks up its own real response by id and a "victim" response by id, then calls response.getBodyBuffer({ _id: own, bodyPath: victim's }) — confirmed this returned the victim's content against the id-reload logic reverted, and the caller's own content with it restored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…path (liquid-extension.ts) Liquid-syntax template tags bypass the sandboxed Nunjucks bridge entirely, so none of the id-coercion, cloudCredential reload+strip, or response.getBodyBuffer id-reload protections applied there carried over. Extract those into shared helpers (templating/db-trust.ts) reused by both templating-worker-database.ts and liquid-extension.ts, and add a dynamic parity detector (liquid-extension-parity-surface.ts) that inspects the real, captured models object rather than re-implementing the checks.
…te tag The `response` tag resolved its author-supplied request `id` with only an existence check, letting any request body in any workspace read another workspace's response body/header/url just by referencing its real req_... id. Resolve the request's ancestor chain and reject (same not-found message) unless it matches the current render's workspaceId.
… results request.getById, request.getAncestors, workspace.getById, oAuth2Token.getByRequestId, cookieJar.getOrCreateForParentId/getCookiesForUrl, and response.getLatestForRequestId let a plugin in one workspace read another workspace's records by id, since models.read is baseline-granted with no manifest and none of these handlers checked the resolved record's ancestry. Every zero-permission template tag, request hook, and plugin action shares these handlers, so all three surfaces were affected. Adds recordBelongsToCallerWorkspace (templating/db-trust.ts), which walks a resolved record's ancestor chain and compares its Workspace ancestor against the caller's own, host-verified workspace id. The id is snapshotted into a primitive local in the sandbox bootstrap before any plugin code runs, so a plugin can't mutate its own trust anchor before a bridge call. Threads the same workspace id into request hooks (from the render context) and plugin actions (a new ExecutePluginActionArgs.workspaceId field, from the three real UI action call sites), which previously ran with no workspace anchor at all. Adds a dynamic detector (templating-worker-database-ancestor-surface.ts, wired into npm run sandbox:ancestor:test) that selects every models.read-capability handler by capability rather than a hand-picked list, so a future handler is covered automatically.
Enumerates every renderer-reachable services.<serviceName>.<methodName> call site and flags which pairs still route through the generic services.invoke gateway vs. a named per-pair IPC handler, so the retirement of that gateway can be tracked incrementally instead of relying on a hand-run grep snapshot. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s.invoke gateway (Phase 1a) Registers a named services.<serviceName>.<methodName> IPC handler for each of the 10 caCertificate/clientCertificate pairs instead of routing them through the generic reflection-based services.invoke dispatcher. The renderer proxy (preload bridge and the non-isolated fallback) now consults a shared migrated-pairs list to pick the named channel for these pairs while everything else keeps using the legacy gateway, with a test cross-checking the list against what main/ipc actually registers so the two can't drift apart. A parity test proves each named handler forwards the same args to the same services.* call the old dispatch made. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e gateway (Phase 1b)
Extends the named-handler migration to the remaining 10 Phase 1 pairs in this batch:
cloudCredential.{all,create,getById,getByName,remove,update} and
settings.{get,getOrCreate,patch,update}. Same shape as Phase 1a — thin forwarding
handlers registered under services.<serviceName>.<methodName>, added to the shared
migrated-pairs allowlist, and covered by the existing parity/cross-check tests.
Also fixes two Phase 0 fixture tests that implicitly depended on the real repo's
main/ipc state (they didn't override mainIpcDir, so a pair migrating for real could
have silently flipped their expected hasNamedHandler value); both now scope
mainIpcDir to their own empty fixture directory.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oke gateway (Phase 1c) Extends the named-handler migration to the final Phase 1c batch: 6 gitCredentials pairs (all, create, getById, remove, removeAll, update) and 3 userSession pairs (get, remove, update). Same shape as prior batches — thin forwarding handlers registered under services.<serviceName>.<methodName>, added to the shared migrated-pairs allowlist, and covered by the parity/cross-check tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ase 1d) Extends the named-handler migration to the final Phase 1 batch: all 10 environment pairs (create, update, list, listByParentId, getOrCreateForParentId, getById, getByParentId, duplicate, remove, removeAllSecrets). Same shape as prior batches. This completes Phase 1 — all 39 high-risk (sensitive-domain) pairs identified in SERVICES-INVOKE-MIGRATION-PLAN.md now have a named handler; 39/183 pairs migrated per the services-invoke-surface detector. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ay (Phase 2 batch 1) Starts Phase 2 (lighter-bar remaining tier) of the services.invoke retirement: 4 apiSpec pairs (getByParentId, getOrCreateForParentId, update, updateOrCreateForParentId) and 3 cookieJar pairs (getById, getOrCreateForParentId, update). The plan's §4/§5 batch listed apiSpec+cookieJar as 8 pairs; the real, detector-confirmed call-site count is 7 — no cookieJar.create call site exists anywhere in renderer-reachable code today. Same forwarder/registration/allowlist/parity-test shape as every Phase 1 batch. 46/183 pairs migrated after this batch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s.invoke gateway (Phase 2 batch 2) 5 gitRepository pairs (all, getAllByCredentialId, getById, remove, update), 2 grpcRequest pairs (create, findByProtoFileId), 2 grpcRequestMeta pairs (getByParentId, updateOrCreateByParentId). Plan's §5 batch listed this as 11 pairs; detector-confirmed real call-site count is 9 (no gitRepository.getByDirectory, grpcRequest.remove/update/ findByParentId/duplicate/all, or grpcRequestMeta.create/getOrCreateByParentId/all call sites exist in renderer-reachable code today). Same shape as prior batches. 55/183 pairs migrated after this batch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e 2 batch 3) All 13 helpers.* pairs: abortCommandSearch, commandSearch, duplicateRequest, findRequestByParentId, getRequestById, getResponseBodyBuffer, getResponseTimeline, queryAllWorkspaceUrls, readCurlResponse, removeRequest, removeResponse, removeResponsesForRequest, updateRequest. helpers is a synthetic aggregate service (per plan §4's caveat) so each forwarder's arg types are pulled from the real service function's own Parameters<> rather than a shared model type; three request-shaped forwarders (duplicateRequest/removeRequest/updateRequest) use `any` since the underlying services.helpers functions are generic over request subtypes determined by a runtime models.*.isXRequest check, not something a static forwarder signature can express. 68/183 pairs migrated after this batch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ces.invoke gateway (Phase 2 batch 4)
7 pairs: mcpPayload.{getByParentIdAndUrl,updateOrCreateByParentIdAndUrl},
mcpRequest.{create,getById,getByParentId}, mcpResponse.{getById,getLatestForRequestId}.
Plan's §5 batch listed this as 11 pairs; detector-confirmed real call-site count is 7
(no mcpPayload.{create,remove,update,duplicate,getOrCreateByParentIdAndUrl,all} or
mcpRequest.{remove,all,findByParentId,update,clearResourceSubscriptions} or
mcpResponse.{findByParentId,all,create,updateOrCreate} call sites exist in
renderer-reachable code today). Same shape as prior batches.
75/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…m as IpcMainInvokeEvent Replaces the unused first parameter's `unknown` type with the real IpcMainInvokeEvent (what ipcMainHandle actually passes) across every forwarder in services-invoke-migrated-handlers.ts. No behavior or security change — none of these forwarders read the event object, and the migration's security property comes from eliminating the reflection-based dispatch, not from this parameter's type. Doing it anyway: it's free, more accurate, and keeps the door open for a future handler to add an event.sender check (as templatingDb.getAuthToken already does) without a type change first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ces.invoke gateway (Phase 2 batch 5)
11 pairs: mockRoute.{create,findByParentId,getById,remove,update}, mockServer.
{findByProjectId,getById,getByParentId,getOrCreateForParentId,update}, organization.list.
Matches §5's 11-pair count for this batch exactly.
86/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…se 2 batch 6)
5 pairs: pluginData.{all,getByKey,removeAll,removeByKey,upsertByKey}. Matches §5's
5-pair count for this batch exactly.
91/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…2 batch 7)
9 pairs: project.{count,create,get,getById,list,listByGitRepositoryIds,
listByOrganizationIds,remove,update}. Plan's §5 batch listed this as 10 pairs;
detector-confirmed real call-site count is 9 (no project.getByRemoteId call site
exists in renderer-reachable code today).
100/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ces.invoke gateway (Phase 2 batch 8)
7 pairs: projectLintRuleset.{getByParentId,remove,upsert}, protoDirectory.
{all,create,findByParentId,remove}. Matches §5's 7-pair count for this batch exactly
(no protoDirectory.getById call site exists, but that's outside this pairing anyway).
107/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ay (Phase 2 batch 9)
10 pairs: protoFile.{all,create,findByParentId,remove,update}, request.
{create,findByParentId,getById,getByParentId,update}. Plan's §5 batch listed this
as 12 pairs; detector-confirmed real call-site count is 10 (no request.remove or
request.duplicate call site exists in renderer-reachable code today).
117/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…invoke gateway (Phase 2 batch 10)
10 pairs: requestGroup.{create,duplicate,findByParentId,getById,remove,update},
requestGroupMeta.{create,getByParentId,update,updateOrCreateForParentId}. Matches
§5's 10-pair count for this batch exactly.
127/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…vices.invoke gateway (Phase 2 batch 11)
10 pairs: requestMeta.{getByParentId,getOrCreateByParentId,update,
updateOrCreateByParentId}, requestVersion.{findByParentId,restore},
response.{create,getByBodyPath,getById,getLatestForRequestId}. Plan's §5 batch
listed this as 11 pairs; detector-confirmed real call-site count is 10 (no
response.findByParentId call site exists in renderer-reachable code today).
137/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oke gateway (Phase 2 batch 12)
8 pairs: runnerTestResult.{create,findByParentId,getById,remove},
socketIOPayload.{getOrCreateByParentId,updateOrCreateByParentId},
socketIORequest.create, socketIORequestMeta.updateOrCreateByParentId. Plan's §5
batch listed this as 11 pairs; detector-confirmed real call-site count is 8.
145/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…invoke gateway (Phase 2 batch 13)
11 pairs: stats.{get,incrementCreatedRequests,incrementCreatedRequestsForDescendents,
incrementDeletedRequests,incrementDeletedRequestsForDescendents,
incrementExecutedRequests,update}, unitTest.{create,remove,update},
unitTestResult.create. Matches §5's 11-pair count for this batch exactly.
156/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e gateway (Phase 2 batch 14)
10 pairs: unitTestSuite.{create,findByParentId,getById,remove,update},
webSocketPayload.{create,getByParentId,update}, webSocketRequest.create,
webSocketRequestMeta.updateOrCreateByParentId. Plan's §5 batch listed this as 12
pairs (also folding in webSocketResponse, handled separately in batch 15); real
call-site count for this batch's actual pairing is 10.
166/183 pairs migrated after this batch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e 2 batch 15)
7 pairs: workspace.{count,create,getById,list,listByParentId,remove,update}. No
webSocketResponse pairs exist in renderer-reachable code today (0 call sites), so
this batch is workspace-only despite §5 pairing it with webSocketResponse.
173/183 pairs migrated after this batch — only workspaceMeta (batch 16) remains.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ke gateway (Phase 2 batch 16, final) 6 workspaceMeta pairs (create/getByParentId/getOrCreateByParentId/list/update/ updateByParentId) matching §5's final batch, plus 4 oAuth2Token pairs (create/getByParentId/remove/update) the detector found that weren't in the plan's original §4/§5 enumeration at all (added to the codebase after that snapshot, or missed by the original hand-run grep) — folded in here so Phase 2 finishes with zero pairs left on the generic gateway rather than leaving a known gap. This completes Phase 2: 183/183 pairs now have a named services.<x>.<y> IPC handler instead of routing through the generic services.invoke dispatcher. Phase 3 (tripwire: replace services.invoke's body with a loud throw, run the full test + smoke suite, fix anything that surfaces) is next. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ipwire throw (Phase 3) All 183 legitimate (serviceName, methodName) pairs have named handlers now, so the generic gateway's body is replaced with a throw naming any pair still routed through it — surfacing a missed migration immediately instead of silently falling back to unrestricted reflection dispatch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tIOResponse pairs reached only via dynamic dispatch
The Phase 3 tripwire caught services.invoke("response", "findByParentId")
live in the Playwright smoke suite. Its real call site is getResponseOperations()
in the request detail route, which returns services.response /
services.webSocketResponse / services.socketIOResponse as a plain variable and
calls .findByParentId/.getById/.getLatestForRequestId on it — a shape the
static services-invoke-surface detector's `services.<x>.<y>(` regex can't see
since the method call never appears as that literal token. The same dynamic
dispatch also reaches webSocketResponse and socketIOResponse's getById and
getLatestForRequestId from the response-delete route, which the detector had
previously reported as having zero call sites at all.
Adds the seven missing named handlers and documents these known
dynamic-dispatch call sites directly in the detector so its cross-check test
stays accurate instead of silently going blind to this class of miss.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…te generic type args in the surface detector response-history-dropdown.tsx calls services.requestVersion.getRequest<Request | WebSocketRequest>(rv) — a real, literal call site the services-invoke-surface detector's regex missed because it required `(` immediately after the method name, and this call site has an explicit generic type argument in between. Widens the pattern to tolerate a single level of `<...>` and adds the missing named handler. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…bridge main/templating-worker-database.ts's request.getById, request.getAncestors, workspace.getById, oAuth2Token.getByRequestId, cookieJar.getOrCreateForParentId, cookieJar.getCookiesForUrl, and response.getLatestForRequestId handlers all gate on recordBelongsToCallerWorkspace(record, callerWorkspaceId), which fails closed whenever callerWorkspaceId is missing. The worker-side Liquid tag bridge (common/templating/liquid-extension-worker.ts, used by first-party tags like `response` when they execute in the templating Web Worker) never forwarded this field, so every one of these lookups silently returned not-found regardless of whether the record actually belonged to the caller's workspace — breaking, among other things, chained `response` tag resends. The render context's own workspaceId (already available as renderMeta) was sitting right there; it just wasn't being threaded through. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ale-write races one-line-editor.tsx fired two independent onChange triggers per edit: an immediate blur-flush and a separately-scheduled 100ms-debounced call from the CodeMirror 'changes' event. Blur never cancelled the pending debounced timer, so if another edit (e.g. adding a new environment KV row) landed within that 100ms window, the stale debounced call fired afterward with a closure over the pre-edit state and silently reverted/re-persisted it - the root cause of the flaky "manage environment" smoke test (row added via Add Row would vanish, environment values would fall back to the base environment). misc.debounce() now exposes cancel(), and the blur handler calls it before flushing. Also retains the persistedPairs/commitPairs local-state fix in key-value-editor.tsx (avoids building the next array from a stale `data` prop snapshot) - both fixes were needed to fully close the race. Verified with 8 consecutive full runs of environment-editor-interactions.test.ts (previously ~50% flake rate, now 0/8 failures) and a clean full npm test run (149 files, 2428 passed).
| /** Every `(serviceName, methodName)` match of `pattern` (a single-match-shaped regex, applied globally) found in `source`. */ | ||
| const extractPairs = (source: string, pattern: RegExp): { serviceName: string; methodName: string }[] => { | ||
| const matches: { serviceName: string; methodName: string }[] = []; | ||
| const re = new RegExp(pattern.source, 'g'); |
✅ Circular References ReportGenerated at: 2026-08-04T21:46:49.221Z Summary
Click to view all circular references in PR (9)Click to view all circular references in base branch (9)Analysis✅ No Change: This PR does not introduce or remove any circular references. This report was generated automatically by comparing against the |
All 183 real (serviceName, methodName) pairs have named handlers and the full Playwright smoke suite (164 tests, batched) now runs clean, so the tripwire from Phase 3 can come out for real: the 'services.invoke' ipcMainHandle registration is deleted from main.ts, its HandleChannels entry is removed, and the two renderer-side callers (entry.preload.ts's _dataServicesInvoke, ui/renderer-services-proxy.ts's servicesProxy) drop their now-dead fallback branch to that channel. resolveServicesInvokeChannel (migrated-services-invoke-pairs.ts) now throws for any (serviceName, methodName) pair without a named handler instead of routing it to the retired gateway, keeping the tripwire's loud-failure property without the reflection dispatch it used to fail into. createServicesProxy/ui/services-proxy.ts is intentionally kept: it's the mechanism that reconstructs the services.* proxy client-side from named per-pair channels (entry.client.tsx, entry.hidden-window-preload.ts, entry.plugin-window.ts all depend on it), not part of the insecure gateway. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Finding 1 (caller-supplied permissions bypassing the trusted plugin registry) was fixed in templating-worker-database.ts's resolveTrustedPlugin handlers; the doc's own regression test now asserts the fixed behavior. Nothing left to track.
…ites Two edits fired close together could still race on a slow persistence round trip: an earlier edit's echo through `data` would revert persistedPairs after a later edit had already advanced local state, silently dropping it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…gs docs Findings/plan docs produced during the H1/M2/L1-style sandbox security review chain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CROSS-TENANT-DB-ACCESS-FINDINGS.md and SANDBOX-SECURITY-FINDINGS.md are working documents for the security review process, not reviewable history — they stay on disk, untracked, so future reviews can still read them locally.
…tenant-fixes # Conflicts: # packages/insomnia/src/main/ipc/main.ts # packages/insomnia/src/main/templating-worker-database.ts # packages/insomnia/src/plugins/invoke-method.ts # packages/insomnia/src/ui/components/editors/environment-key-value-editor/key-value-editor.tsx
…-render OneLineEditor's debounced change/blur listeners depended on onChange and updateTooltipValue by reference, so any parent re-render (a fresh inline onChange closure, or handleRender's loader data getting a new reference after a fetcher revalidation) tore down the listener and cancelled a pending debounced call before it could fire, silently dropping the edit. Track both in refs so the listeners are stable across re-renders. Also close a related hint-dropdown leak: completeAfter() only checked focus before its async lookups, not after, so a slow-resolving autocomplete could pop a hint over an editor the user had already left, and since it was orphaned it never got a blur event to close it.
|
Fixed (hopefully) the |
What this PR does
PR provides QoL hardening improvements for capabilities aligned with the sandbox changes. Closes the "bare
_id/parentIdlookup, no ownership check" bug class across every plugin/template surface that reaches the local NeDB — a caller (template tag, hook, or the renderer's own bridge) could supply anyidit could guess or observe and read/write a document belonging to a different workspace, project, or org, since the underlying lookups never checked the resolved record's ancestor chain against the caller's own context.liquid-extension.ts) — models.* forwarded straight to services.* with no check at all, so any Liquid tag could pull another workspace's request/response data. Fixed by applying the same ancestor-check pattern already used elsewhere in the codebase, plus threading a realcallerWorkspaceIdthrough the worker-side bridge so the check has something trustworthy to compare against.responselocal template tag (local-template-tags.ts) — accepted an author-typed request id with only an existence check, letting{% response 'body', 'req_from_another_project' %}leak that request's latest response body/headers. * Fixed by verifying the resolved request's workspace matches the render context's own workspace.models.readhandlers —request.getById,workspace.getById,oAuth2Token.getByRequestId,cookieJar.*,response.getLatestForRequestId/getBodyBufferhad no ownership check, reachable by any plugin with zero manifest permissions (the profile floor). Same fix pattern applied here, plus re-verifying getBodyBuffer against the id-resolved response instead of trusting a caller-supplied bodyPath.cloudCredential.updatetook a caller-supplied originCredential document and patch straight to the DB write, no reload-by-id, so a forged_idcould get an arbitrary credential document patched. Fixed by reloading the credential server-side by id and stripping identity fields (_id/type/parentId) from the patch before writing.id/parentId/keyargs reached NeDB unstringified, so a crafted object like{ $ne: null }would be interpreted as a Mongo query operator instead of a literal id, matching arbitrary documents. Fixed by coercing every one of these args toString()before the service call.services.invokea fully generic reflection-based dispatcher (serviceName/methodName/...args) let any trusted-renderer call site invoke any method on any service with no allowlist. Migrated all ~183 legitimate (serviceName, methodName) pairs to purpose-built named IPC handlers (in batches, by model), replaced the reflection dispatch with a tripwire that throws on anything unmigrated, then deleted the generic gateway outright.secureReadFile's allowlist andresponse.setBody's containment check didn't re-verify viarealpathSync, so a symlinked path segment could resolve outside the intended directory at write time. Fixed by re-checking containment against the resolved real path.