Skip to content

fix(sandbox): prevent cross-tenant data access in the plugin/template-tag sandbox - #10302

Draft
kwburns-kong wants to merge 50 commits into
developfrom
sec/sandbox-cross-tenant-fixes
Draft

fix(sandbox): prevent cross-tenant data access in the plugin/template-tag sandbox#10302
kwburns-kong wants to merge 50 commits into
developfrom
sec/sandbox-cross-tenant-fixes

Conversation

@kwburns-kong

@kwburns-kong kwburns-kong commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What this PR does

PR provides QoL hardening improvements for capabilities aligned with the sandbox changes. Closes the "bare _id/parentId lookup, 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 any id it 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 engine (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 real callerWorkspaceId through the worker-side bridge so the check has something trustworthy to compare against.
  • response local 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.
  • Sandbox bridge models.read handlers — request.getById, workspace.getById, oAuth2Token.getByRequestId, cookieJar.*, response.getLatestForRequestId/getBodyBuffer had 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.update took a caller-supplied originCredential document and patch straight to the DB write, no reload-by-id, so a forged _id could 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/key args 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 to String() before the service call.
  • services.invoke a 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.
  • Symlink/path hardening (adjacent, same audit pass) — secureReadFile's allowlist and response.setBody's containment check didn't re-verify via realpathSync, so a symlinked path segment could resolve outside the intended directory at write time. Fixed by re-checking containment against the resolved real path.

jackkav and others added 30 commits July 24, 2026 20:04
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>
kwburns-kong and others added 12 commits July 26, 2026 18:33
…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');
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

✅ Circular References Report

Generated at: 2026-08-04T21:46:49.221Z
Status: ✅ NO CHANGE

Summary

Metric Base (develop) PR Change
Total Circular References 9 9 0 (0.00%)
Click to view all circular references in PR (9)
insomnia-inso/src/db/models/types.ts -> insomnia-inso/src/db/types.ts
insomnia/src/main/prompt-bridge.ts -> insomnia/src/main/window-utils.ts -> insomnia/src/main/plugin-window.ts
insomnia/src/main/window-utils.ts -> insomnia/src/main/plugin-window.ts
insomnia/src/network/network.ts -> insomnia-scripting-environment/src/objects/index.ts -> insomnia-scripting-environment/src/objects/collection.ts -> insomnia-scripting-environment/src/objects/response.ts
insomnia/src/network/network.ts -> insomnia/src/common/render.ts
insomnia/src/ui/components/settings/import-export.tsx -> insomnia/src/ui/components/modals/export-requests-modal.tsx
insomnia/src/ui/components/tabs/tab-list.tsx -> insomnia/src/ui/components/tabs/tab.tsx
insomnia/src/ui/components/templating/tag-editor-arg-sub-form.tsx -> insomnia/src/ui/components/templating/external-vault/external-vault-form.tsx
insomnia/src/ui/components/viewers/response-viewer.tsx -> insomnia/src/ui/components/viewers/response-multipart-viewer.tsx
Click to view all circular references in base branch (9)
insomnia-inso/src/db/models/types.ts -> insomnia-inso/src/db/types.ts
insomnia/src/main/prompt-bridge.ts -> insomnia/src/main/window-utils.ts -> insomnia/src/main/plugin-window.ts
insomnia/src/main/window-utils.ts -> insomnia/src/main/plugin-window.ts
insomnia/src/network/network.ts -> insomnia-scripting-environment/src/objects/index.ts -> insomnia-scripting-environment/src/objects/collection.ts -> insomnia-scripting-environment/src/objects/response.ts
insomnia/src/network/network.ts -> insomnia/src/common/render.ts
insomnia/src/ui/components/settings/import-export.tsx -> insomnia/src/ui/components/modals/export-requests-modal.tsx
insomnia/src/ui/components/tabs/tab-list.tsx -> insomnia/src/ui/components/tabs/tab.tsx
insomnia/src/ui/components/templating/tag-editor-arg-sub-form.tsx -> insomnia/src/ui/components/templating/external-vault/external-vault-form.tsx
insomnia/src/ui/components/viewers/response-viewer.tsx -> insomnia/src/ui/components/viewers/response-multipart-viewer.tsx

Analysis

No Change: This PR does not introduce or remove any circular references.


This report was generated automatically by comparing against the develop branch.

kwburns-kong and others added 8 commits July 27, 2026 13:31
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.
@kwburns-kong

Copy link
Copy Markdown
Contributor Author

Fixed (hopefully) the key-value-editor-blank-row.test.ts flake (job). Root cause wasn't the test — OneLineEditor's debounced change/blur listeners were keyed on onChange/updateTooltipValue by reference, so an unrelated parent re-render (e.g. a fetcher revalidation after a prior row's commit) could cancel an in-flight debounce timer and silently drop a keystroke's edit. Also tightened a related hint-dropdown leak where a slow-resolving autocomplete lookup could pop a suggestion box over an editor the user had already left. Details in 8ef6a59.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants