feat(templating): (T1) flip trust model — pluginSandboxEnabled + per-plugin elevated opt-in - #10318
Open
jackkav wants to merge 5 commits into
Open
feat(templating): (T1) flip trust model — pluginSandboxEnabled + per-plugin elevated opt-in#10318jackkav wants to merge 5 commits into
jackkav wants to merge 5 commits into
Conversation
✅ Circular References ReportGenerated at: 2026-08-05T10:15:56.379Z 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 |
jackkav
marked this pull request as ready for review
August 3, 2026 12:36
…-in, centralized gate Introduces the trust-model flip's core logic and wires every untrusted-execution surface to it. - New setting `pluginSandboxEnabled` (default off) that supersedes/absorbs `templateTagSandboxEnabled`: either flag on activates the sandbox, so existing template-tag opt-ins keep working (migration bridge). - New per-plugin `pluginConfig.elevated` escape hatch: a user plugin marked elevated runs in-process with full host access even while the sandbox is on. Widened PluginConfig/PluginConfigMap/Plugin.config. - New pure resolver `common/plugins/sandbox-mode.ts` (isSandboxEnabled / resolvePluginExecutionMode / shouldSandboxPlugin) — the single source of truth replacing the 7 duplicated `templateTagSandboxEnabled && directory !== ''` conditions. Dependency-free so main, the plugin window, and the inso CLI node runtime all share it. Unit-tested (12 cases). - Wired all surfaces to shouldSandboxPlugin: load-time discovery (now per-plugin, so an elevated plugin is nodeRequire-d for live functions), request/response hooks (plugin-window + node runtime), actions, and user template tags. Bundle-tag path reads isSandboxEnabled (bundle stays trusted). No UI yet (next commit); default-off means no behavior change until a flag is toggled.
…lugin elevated + mode indicator - Scripting settings: new "Sandbox all plugin code (experimental)" toggle for pluginSandboxEnabled (data-testid toggle-plugin-sandbox), beside the existing template-tag toggle. - Plugins settings: each user plugin card now shows its resolved execution mode (Sandboxed / Elevated / In-process, data-testid plugin-mode-<name>) and a "Full host access" checkbox (data-testid plugin-elevated-<name>) that writes pluginConfig.<name>.elevated. Mode + toggle read from live settings so they update immediately, before the plugin list reloads. - Widened SerializablePlugin.config to carry the optional `elevated` flag through the bridge.
…ugin; elevating runs it in-process Composes the trust-flip's two user-visible behaviors on one action-probe plugin: - Enabling the new pluginSandboxEnabled toggle (not the legacy template-tag flag) routes the user plugin's action into the sandbox (canary reports ranin-sandboxed). - Toggling "Full host access" in Preferences → Plugins flips the mode indicator to "Elevated" and the same action then runs in-process (marker absent, ranin-mainprocess) — the per-plugin escape hatch. Reuses the sandbox-action-collection.yaml fixture and the established writePlugin/clearPluginToast helpers; adds enablePluginSandbox + setPluginElevated helpers.
jackkav
force-pushed
the
claude/sandbox-pr12-trust-flip
branch
from
August 3, 2026 12:36
ebaf931 to
12291c0
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Flips the plugin trust model toward sandbox-by-default by introducing a new pluginSandboxEnabled setting plus a per-plugin elevated escape hatch, and centralizes sandbox gating logic so all plugin execution surfaces agree.
Changes:
- Adds
pluginSandboxEnabledsetting (defaultfalse) and UI toggle in Scripting settings. - Introduces a shared resolver (
common/plugins/sandbox-mode.ts) used across plugin discovery, actions, hooks, and template tags, plus unit tests. - Adds per-plugin “Full host access” (
pluginConfig.elevated) toggle + execution-mode indicator in Preferences → Plugins, with new smoke coverage.
Reviewed changes
Copilot reviewed 13 out of 13 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 Preferences → Scripting toggle for pluginSandboxEnabled. |
| packages/insomnia/src/ui/components/settings/plugins.tsx | Adds per-plugin elevated toggle + mode badge using the shared resolver. |
| packages/insomnia/src/runtimes/network/network-adapter.node.ts | Routes request/response hooks through shouldSandboxPlugin (now per-plugin). |
| packages/insomnia/src/plugins/invoke-method.ts | Uses shouldSandboxPlugin for actions and hooks routing via templating-worker DB. |
| packages/insomnia/src/plugins/index.ts | Makes load-time discovery per-plugin (sandbox vs nodeRequire) using shared resolver. |
| packages/insomnia/src/main/templating-worker-database.ts | Uses new resolver/flag supersession for template-tag execution routing. |
| packages/insomnia/src/common/plugins/types.ts | Extends plugin config type with optional elevated. |
| packages/insomnia/src/common/plugins/sandbox-mode.ts | New single source of truth for sandbox enablement + per-plugin execution mode. |
| packages/insomnia/src/common/plugins/sandbox-mode.test.ts | Adds unit tests for flag supersession + mode resolution + gating boolean. |
| packages/insomnia/src/common/plugins/bridge-types.ts | Extends serialized plugin config to include optional elevated. |
| packages/insomnia-smoke-test/tests/smoke/sandbox-template-tags.test.ts | Adds E2E covering pluginSandboxEnabled + per-plugin elevation flip. |
| packages/insomnia-data/src/models/settings.ts | Adds pluginSandboxEnabled default to settings init. |
| packages/insomnia-data/common-src/settings.ts | Adds pluginSandboxEnabled and extends PluginConfigMap with elevated. |
Suppressed comments (1)
packages/insomnia/src/runtimes/network/network-adapter.node.ts:139
- Same as
applyRequestHooks:services.settings.get()is awaited even whencanSandboxis false (inso CLI). This adds avoidable I/O/CPU for a path that will always run hooks in-process.
// where the sandbox is unavailable and hooks run in-process — gate on process.type accordingly.
const canSandbox = !!process.type;
const settings = await services.settings.get();
const hookIndexByPlugin: Record<string, number> = {};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
Plugin sandbox trust model (T1) — referenceDocumenting the trust model this PR establishes, for future maintainers. This is the state after T1; the source of truth in code is The two questions, kept separate
Before T1 the answer to (2) was the condition Flags (question 1)
Execution modes (question 2)
The seams
The 7 gated surfaces (all now read
|
kwburns-kong
requested changes
Aug 4, 2026
kwburns-kong
left a comment
kwburns-kong
left a comment
Contributor
There was a problem hiding this comment.
Two fixes, PR #10326.
) * fix(templating): close two plugin-registry trust-cache gaps - pluginConfig.elevated is keyed by declared plugin name, not by folder; a same-named folder placed alongside an already-elevated plugin inherited its trust grant and ran in-process before any collision was even noticed. traversePluginPath now pre-scans for duplicate names (order-independent) and refuses to load any colliding folder. - applyRequestHooks/applyResponseHooks tagged a caught error with `error.plugin = plugin`, a plain assignment that a plugin-thrown Error could intercept via its own `plugin` property setter, handing the hook a live, mutable reference to its own cached registry entry and letting it flip `directory`/`config.elevated` to defeat later sandboxing. Switched to Object.defineProperty, which bypasses any such setter. * fix(plugins): use relative-path containment check instead of startsWith A bare .startsWith(base) on a resolved path accepts a sibling directory whose name happens to prefix-match the base (e.g. /plugins-evil vs /plugins). Added a shared isContainedIn helper (path.relative, rejects .. or an absolute result) and applied it to both the existing plugin-path containment check and the new duplicate-name pre-pass.
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| } | ||
| try { | ||
| // package.json is plain data — reading it runs no plugin code. | ||
| const pluginJson = getNodeRequire()(path.resolve(modulePath, 'package.json')); |
Contributor
There was a problem hiding this comment.
Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Suggested change
| const pluginJson = getNodeRequire()(path.resolve(modulePath, 'package.json')); | |
| const packageJsonPath = path.resolve(modulePath, 'package.json'); | |
| const relativeToModule = path.relative(modulePath, packageJsonPath); | |
| if (relativeToModule.startsWith('..') || path.isAbsolute(relativeToModule)) { | |
| continue; | |
| } | |
| const pluginJson = getNodeRequire()(packageJsonPath); |
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
Contributor
Author
There was a problem hiding this comment.
@AikidoSec ignore: False positive. The read target is a constant path.resolve(modulePath, 'package.json') where modulePath is a readdir-sourced entry already validated by isContainedIn(p, modulePath) two lines above; the trailing segment is the literal package.json, so it cannot escape and is not attacker-controlled. The suggested path.relative re-check can never fail for a constant child segment. Mirrors the pre-existing package.json read in the sibling traversePluginPath.
🤖 Addressed by Claude Code
Contributor
There was a problem hiding this comment.
✅ Based on your feedback, we ignored this issue because of the following reason:
False positive. The read target is a constant
path.resolve(modulePath, 'package.json')wheremodulePathis a readdir-sourced entry already validated byisContainedIn(p, modulePath)two lines above; the trailing segment is the literalpackage.json, so it cannot escape and is not attacker-controlled. The suggestedpath.relativere-check can never fail for a constant child segment. Mirrors the pre-existing package.json read in the sibling traversePluginPath.
🤖 Addressed by Claude Code
jackkav
enabled auto-merge (squash)
August 5, 2026 10:03
…, scope bundle-tag sandbox to legacy flag - network-adapter.node.ts: only read services.settings.get() when canSandbox (Electron). The pure-Node inso CLI can never reach the sandbox host, so the read was wasted work; pass undefined otherwise (shouldSandboxPlugin treats it as off). Both applyRequestHooks and applyResponseHooks. - templating-worker-database.ts: bundle (first-party/trusted) template tags now sandbox only under the legacy templateTagSandboxEnabled experiment, not the new pluginSandboxEnabled. The T1 flag isolates *untrusted* plugins; sandboxing trusted bundle tags under it was an unintended behavior change. Dropped the now-unused isSandboxEnabled import.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 12 — T1: flip the trust model (
pluginSandboxEnabled+ per-plugin elevated opt-in)Phase 2 capstone. With every untrusted surface now routable through the sandbox (L1/H1/A1), this makes the sandbox the default for user plugins and adds a deliberate, per-plugin escape hatch.
What it ships
pluginSandboxEnabled(default off) that supersedes/absorbstemplateTagSandboxEnabled: either flag on activates the sandbox, so existing template-tag opt-ins keep working (migration bridge).pluginConfig.elevatedescape hatch: a user plugin marked elevated runs in-process with full host access even while the sandbox is on.common/plugins/sandbox-mode.ts:isSandboxEnabled/resolvePluginExecutionMode/shouldSandboxPlugin) replacing thetemplateTagSandboxEnabled && directory !== ''condition that was duplicated across 7 gate sites (load discovery, request/response hooks × plugin-window + node runtime, actions, user template tags). Dependency-free so main, the plugin window, and the inso CLI share it. Unit-tested (12 cases).Trust model
directory === '') are always trusted → in-process (unchanged;unsafePluginMainActionsby design).elevatedopts an individual one back to in-process. Load-time discovery is now per-plugin (an elevated plugin isnodeRequired so its hooks/actions/tags are live functions).Tests
sandbox-mode.test.ts— flag supersession, mode resolution, the gate boolean.pluginSandboxEnabledsandboxes a user plugin's action (canaryranin-sandboxed); toggling Full host access flips the mode indicator to Elevated and the same action then runs in-process (ranin-mainprocess).Validation
tscclean (insomnia + insomnia-data), 170 sandbox+resolver unit tests green, eslint clean. Default-off → zero behavior change until a flag is toggled.Builds on the merged L1/H1/A1 work now on develop. Follow-up: PR 13 (S1) capability/bridge-handler scoping audit — see the plan gist.
🤖 Authored by Claude Code