Skip to content

feat(templating): QuickJS template-tag sandbox behind a flag, with e2e canary#10207

Merged
jackkav merged 6 commits into
developfrom
claude/modest-poitras-0e50fb
Jul 6, 2026
Merged

feat(templating): QuickJS template-tag sandbox behind a flag, with e2e canary#10207
jackkav merged 6 commits into
developfrom
claude/modest-poitras-0e50fb

Conversation

@jackkav

@jackkav jackkav commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

PR 0 of the sandbox plan (https://gist.github.com/jackkav/4bbf717954a18a6753b64f33ab678ad4): lands the QuickJS template-tag sandbox PoC from #10095, rebased onto develop, plus the Playwright smoke-test harness every subsequent sandbox PR extends.

What

  • Everything from PoC: run plugin template tags in a QuickJS-WASM sandbox #10095, rebased across the feature-folder reorg (refactor(structure): reorganize feature folders by runtime context #10092): src/templating/sandbox/ (quickjs-runtime, marshal, host-bridge, in-sandbox-bootstrap, plugin-tag-sandbox + 21 unit tests), main-side routing behind the new templateTagSandboxEnabled setting (default off), Scripting-settings toggle, quickjs kept external in esbuild, and the examples/insomnia-plugin-sandbox-demo manual fixture.
  • New e2e canary tests/smoke/sandbox-template-tags.test.ts: installs an inline probe plugin into the data-path plugins dir, then asserts via the tag editor Live Preview that
    • flag off → probe tag renders ran in: main-process;
    • flag on (toggled through Preferences → Scripting) → same tag renders ran in: sandbox, and the async host bridge (context.util.nodeOS()) round-trips;
    • a require('crypto') sha256 workload renders byte-identical output on both paths (parity).

Every later PR in the plan (module registry, manifest gating, capability gating, profiles) starts from this canary so parity tests can never silently pass against the legacy path.

Test notes

  • The persistent "Plugin system updated" toast fires from a Root mount effect whenever user plugins exist and intercepts pointer events over the page; the test seeds its once-only localStorage guard and dismisses any toast already in flight. (The effect can also double-fire and race its own guard — flagged as a separate follow-up.)
  • Added data-testid="toggle-template-tag-sandbox" to the Scripting settings switch for the toggle step.

Verification

  • npx vitest run src/templating/sandbox — 21/21
  • npm run test:smoke:dev -- sandbox-template-tags — 1 passed
  • npm run test:smoke:dev -- template-tags-interactions — 1 passed (flag-off regression)
  • Repo-wide npm run lint && npm run type-check && npm test — clean

Comment thread packages/insomnia/src/templating/sandbox/plugin-tag-sandbox.test.ts Fixed
@jackkav jackkav force-pushed the claude/modest-poitras-0e50fb branch from 505d8e1 to 945b857 Compare July 6, 2026 08:02
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

✅ Circular References Report

Generated at: 2026-07-06T18:56:54.239Z
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.

@jackkav jackkav marked this pull request as ready for review July 6, 2026 08:38
Comment thread packages/insomnia/src/main/templating-worker-database.ts Outdated
Comment thread packages/insomnia/src/main/templating-worker-database.ts Outdated
Comment thread packages/insomnia/src/main/templating-worker-database.ts Outdated
Comment thread packages/insomnia/src/main/templating-worker-database.ts Outdated
Comment thread packages/insomnia/src/templating/sandbox/plugin-tag-sandbox.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an experimental, default-off QuickJS-WASM sandbox execution path for plugin template tags, plus a Playwright smoke-test canary to ensure legacy vs sandbox parity and prevent regressions as the broader sandbox plan evolves.

Changes:

  • Introduces a new templateTagSandboxEnabled setting and UI toggle in Scripting settings (default off).
  • Adds the QuickJS sandbox runtime/bootstrap, host-bridge marshaling, and unit tests for parity (base64/os/crypto).
  • Adds a smoke-test canary + fixture collection to validate flag-off vs flag-on behavior, and updates build/deps to support QuickJS WASM at runtime.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/insomnia/src/ui/components/settings/scripting-settings.tsx Adds the experimental toggle in Preferences → Scripting.
packages/insomnia/src/templating/sandbox/quickjs-runtime.ts Caches/loading wrapper for the QuickJS WASM module.
packages/insomnia/src/templating/sandbox/plugin-tag-sandbox.ts Core sandbox executor: host bridge wiring, crypto host functions, and VM job driving.
packages/insomnia/src/templating/sandbox/plugin-tag-sandbox.test.ts Unit tests validating parity and error propagation for the sandbox runner.
packages/insomnia/src/templating/sandbox/marshal.ts Defines the bulk-copied envelope and bridge result encoding.
packages/insomnia/src/templating/sandbox/in-sandbox-bootstrap.ts In-sandbox JS bootstrap: polyfills, require shim, context rebuild, invocation runner.
packages/insomnia/src/templating/sandbox/host-bridge.ts Host bridge interface + helper to build a bridge from a handler map.
packages/insomnia/src/main/templating-worker-database.ts Routes plugin tag execution through the sandbox when the flag is enabled.
packages/insomnia/package.json Adds quickjs-emscripten dependency.
packages/insomnia/esbuild.entrypoints.ts Marks QuickJS packages as external so .wasm resolves at runtime.
packages/insomnia-smoke-test/tests/smoke/sandbox-template-tags.test.ts Adds e2e canary test for legacy vs sandbox execution + crypto parity.
packages/insomnia-smoke-test/fixtures/sandbox-probe-collection.yaml Fixture collection containing probe tags used by the canary.
packages/insomnia-data/src/models/settings.ts Adds templateTagSandboxEnabled default in settings init.
packages/insomnia-data/common-src/settings.ts Adds templateTagSandboxEnabled to the Settings interface.
package-lock.json Locks quickjs-emscripten and transitive dependencies.
examples/insomnia-plugin-sandbox-demo/README.md Documents manual verification steps for the sandbox toggle.
examples/insomnia-plugin-sandbox-demo/package.json Adds a demo plugin manifest for manual testing.
examples/insomnia-plugin-sandbox-demo/index.js Adds a demo probe template tag for manual testing.
Comments suppressed due to low confidence (1)

packages/insomnia/src/main/templating-worker-database.ts:372

  • Both plugin.execute*Tag handlers now call services.settings.get() on every tag execution just to check templateTagSandboxEnabled. services.settings.get() hits the DB (findOne) each time, so this adds noticeable overhead even when the flag is off (default path), potentially impacting live preview performance.
  },
  // execute the plugin tag with the given parameters
  'plugin.executeBundlePluginTag': async (body: {
    args: any[];

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/insomnia/src/main/templating-worker-database.ts Outdated

@kwburns-kong kwburns-kong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR for one main fix #10209

Comment thread packages/insomnia/src/main/templating-worker-database.ts
@jackkav

jackkav commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged #10209 — thanks for the hardening pass, the util.render re-entry especially.

Confirming the util.render question: I checked every in-repo caller. The only code passing potentially tag-bearing strings to context.util.render is the local request tag (re-rendering request URLs/headers/params, which can legitimately contain nested tags) — but local tags keep their in-worker run() and never route through the sandbox path (worker.ts only rewires bundle/user plugin tags to plugin.execute*PluginTag). So for the sandboxed bridge, plain {{ variable }} interpolation is indeed the only legitimate existing use, and rejecting {% tag %} there closes the re-entry without breaking the local tags' behavior. If a real plugin surfaces that needs nested tag rendering under the flag, the right shape is a sandboxed re-render, not a bypass.

Also synced the stacked M1 branch (#10208) with the merge — all 139 unit tests green on top of it.

🤖 Addressed by Claude Code

@jackkav jackkav requested a review from kwburns-kong July 6, 2026 18:48

@kwburns-kong kwburns-kong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🫡

jackkav and others added 6 commits July 6, 2026 20:52
Behind a new `templateTagSandboxEnabled` setting (default off), route plugin
template-tag execution through a QuickJS-WASM sandbox instead of invoking the
plugin's `run()` directly in the main process.

Approach (per PR #10072): bulk-copy render state into the sandbox as JSON,
rebuild the plugin `context` API in pure JS inside the sandbox, and bridge only
async work back to the host via the existing `pluginToMainAPI` handlers.
`node:crypto` is exposed as synchronous host functions so `require('crypto')`
works without a sync/async mismatch.

- templating/sandbox/: quickjs-runtime, marshal, host-bridge, in-sandbox-bootstrap,
  plugin-tag-sandbox (+ parity tests vs in-process tags and node:crypto)
- main/templating-worker-database.ts: route execute handlers through the sandbox
  when the flag is on; legacy path unchanged otherwise
- esbuild: keep quickjs-emscripten external so its .wasm resolves at runtime
- settings + scripting-settings UI toggle
- examples/insomnia-plugin-sandbox-demo: manual E2E fixture

Scope: template tags only; sandbox runs in main. require shim covers path + crypto
(other modules throw a clear error — follow-up work).
Installs an inline probe plugin, renders its tags via the tag editor Live
Preview, and asserts the execution path flips main-process -> sandbox when
templateTagSandboxEnabled is toggled in Preferences > Scripting, with a
require('crypto') sha256 workload staying byte-identical across both paths.
…fixture

The HMAC key is a test vector for sandbox-vs-node:crypto parity, not a
credential; rename it to make that self-evident and add the repo-standard
nosemgrep suppression.
…n directory

Reject a package.json "main" that resolves outside the plugin's own folder
and bundled-plugin names that look like paths, so the sandbox source loader
cannot be steered into reading arbitrary files.
…ross-arch-safe canary

- Move the hardcoded-hmac-key suppression onto the flagged line (line-above
  placement was not honored by the scanner).
- Wrap getPluginEntrySource failures with the plugin name for diagnosability.
- Derive the canary's expected arch from the Electron main process instead of
  the Playwright runner so cross-arch setups can't flake the assertion.
* fix(sandbox): enforce timeout on synchronous plugin loops

QuickJS's executePendingJobs() blocks the host thread until a synchronous
call returns, so the wall-clock deadline in drivePromiseToString was never
checked during a tight sync loop in plugin code, hanging the Electron main
process indefinitely. Add a QuickJS interrupt handler, which is polled
during synchronous execution, to enforce the deadline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sandbox): clamp crypto.randomBytes size to prevent OOM

hostCrypto.randomBytes(size) passed the sandboxed number straight to
Node's crypto.randomBytes with no upper bound, letting a plugin request
a multi-GB allocation (e.g. crypto.randomBytes(2 ** 31)) and crash the
host process. Clamp to 64KB before the call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sandbox): cap QuickJS heap to prevent unbounded allocation

QuickJS.newContext() had no memory limit, so a plugin allocating without
bound could exhaust the WASM heap and crash the host process. Set a 32MB
ceiling via ctx.runtime.setMemoryLimit().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sandbox): resolve symlinks before validating plugin entry path

getPluginEntrySource's containment check compared raw path strings, so a
plugin directory with a symlinked entry (e.g. index.js -> ../../../etc/secret)
passed the check while fs.readFileSync followed the symlink and read the
out-of-directory target. Re-run the check against fs.realpathSync'd paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sandbox): close util.render sandbox escape

context.util.render() bridged to the shared render() pipeline, whose Liquid
engine dispatches any registered tag's real run() directly, in-process,
regardless of templateTagSandboxEnabled. A sandboxed plugin could hand it a
string containing "{% anyTag %}" (including its own tag) and have that tag
execute completely unsandboxed. Verified with a PoC that reached
child_process execution from inside a plugin tag with no require() or Node
access.

util.render is now restricted to plain {{ variable }} interpolation (the
only real existing use, confirmed against all built-in tag call sites) via
a second Liquid engine with no tags registered; {% tag %} syntax now fails
to parse instead of dispatching. Default render() behavior is unchanged for
every other caller.

Also drops the dead renderDepth field's misleading doc comment: within one
sandboxed execution the envelope's renderDepth is always 0, so depth could
never exceed 1 regardless of enforcement — it couldn't have caught this
recursion anyway.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sandbox): fixed linting issue

* fix(sandbox): fixed linting issue

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@jackkav jackkav force-pushed the claude/modest-poitras-0e50fb branch from a185145 to 540bfcb Compare July 6, 2026 18:52
@jackkav jackkav enabled auto-merge (squash) July 6, 2026 18:52
@jackkav jackkav merged commit cc9288c into develop Jul 6, 2026
21 checks passed
@jackkav jackkav deleted the claude/modest-poitras-0e50fb branch July 6, 2026 19:01
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.

4 participants