Skip to content

sec(templating): QuickJS template-tag sandbox additions #10209

Merged
jackkav merged 7 commits into
claude/modest-poitras-0e50fbfrom
sec/modest-poitras-0e50fb/10207
Jul 6, 2026
Merged

sec(templating): QuickJS template-tag sandbox additions #10209
jackkav merged 7 commits into
claude/modest-poitras-0e50fbfrom
sec/modest-poitras-0e50fb/10207

Conversation

@kwburns-kong

Copy link
Copy Markdown
Contributor

What this PR does

  • Enforce the sandbox timeout during synchronous plugin code ( previously hung the main process indefinitely).
  • Clamp crypto.randomBytes(size) to prevent a multi-GB allocation/crash.
  • Cap the QuickJS heap so unbounded allocation can't exhaust host memory.
  • Resolve symlinks before validating a plugin's entry-point path, closing a traversal bypass.
  • context.util.render() invoked any template tag (including the calling plugin's own tag) through the normal, unsandboxed Liquid pipeline. It's now restricted to plain {{ variable }} interpolation the only real existing use (@jackkav, can you confirm this?). It rejects {% tag %} syntax instead of executing it outside the sandbox. Verified this technique worked within the base PR.

kwburns-kong and others added 5 commits July 6, 2026 12:59
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>
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>
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>
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>
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>
@kwburns-kong kwburns-kong requested a review from jackkav July 6, 2026 18:21
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

✅ Circular References Report

Generated at: 2026-07-06T18:38:35.879Z
Status: ✅ NO CHANGE

Summary

Metric Base (claude/modest-poitras-0e50fb) 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 claude/modest-poitras-0e50fb branch.

@jackkav jackkav merged commit a185145 into claude/modest-poitras-0e50fb Jul 6, 2026
16 of 17 checks passed
@jackkav jackkav deleted the sec/modest-poitras-0e50fb/10207 branch July 6, 2026 18:46
jackkav pushed a commit that referenced this pull request Jul 6, 2026
* 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 added a commit that referenced this pull request Jul 6, 2026
…e canary (#10207)

* feat(templating): PoC run plugin template tags in a QuickJS-WASM sandbox

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).

* test(smoke): e2e canary for the template-tag sandbox flag

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.

* test(sandbox): suppress hardcoded-hmac-key semgrep finding on parity 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.

* fix(templating): contain sandbox plugin entry resolution to the plugin 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.

* fix(review): inline nosemgrep placement, plugin-load error context, cross-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.

* sec(templating): QuickJS template-tag sandbox additions  (#10209)

* 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>

---------

Co-authored-by: kwburns-kong <kyle.burns@konghq.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

2 participants