Skip to content

Portable Mods: executable mods stay disabled behind a documented, tested gate (#341) - #379

Closed
raiseCatError wants to merge 1 commit into
release/v0.18.0from
feature/portable-mod-gate
Closed

raiseCatError wants to merge 1 commit into
release/v0.18.0from
feature/portable-mod-gate

Conversation

@raiseCatError

Copy link
Copy Markdown
Owner

#341. Decision: executable Portable Mods stay disabled. No platform NMSh supports has an enforced isolation design that has been built, reviewed and attacked. Based on release/v0.18.0; docs and a test only, no product behavior change.

  • docs/design/portable-mod-isolation.md: the three classes and their trust boundaries; an audit of what exists (inert PortableDescriptor, the frozen read-only deny-by-default ModBroker, the provider-native claude plugin list --json inventory, no loader); the threat model (fs/network/process/secrets, acting as the person, forged presentation, resource exhaustion, persistence, cross-target access, integrity/provenance/revocation); why a worker or child process, Node's permission model (its docs: "does not protect against malicious code"), and node:vm are not the boundary; per-platform candidates (Linux namespaces + seccomp + Landlock; macOS Seatbelt with its deprecation caveat; host-import-only WebAssembly in a limited process) and the adversarial evidence each must produce; the exact block.
  • tests/portableModGate.test.ts: no dynamic import(), require, new Function, eval, vm, worker threads or WebAssembly instantiation under src/agents/mods (verified by mutation: adding an import() fails it); descriptors are 'inert' and inventoried as not executable; the broker answers only requested ∩ granted ∩ read permissions, never approvals or conversation content, and its API has nothing but identity and request and is frozen.

Lifting the block means changing these tests in the same change that delivers isolation, its threat-model review and an adversarial suite in CI per platform.

…ted security gate

#341. The decision and its reasons are recorded in
docs/design/portable-mod-isolation.md: no supported platform has an
isolation design that has been built, reviewed and attacked, so a Portable
Mod remains an inert descriptor bound to the read-only, deny-by-default
broker.

The document audits what exists (inert descriptors, a frozen broker API, the
provider-native plugin listing), states the threat model, explains why a
worker, a child process, Node's permission model (by its own documentation
not a protection against malicious code) and node:vm are not the boundary,
and lists what an acceptable design must demonstrate per platform.

tests/portableModGate.test.ts keeps the block: no dynamic import, require,
new Function, eval, vm, worker threads or WebAssembly instantiation under
src/agents/mods (the one process use is `claude plugin list --json`);
descriptors stay inert and listed as not executable; the broker answers only
requested, granted, read permissions, never approvals or conversation
content, and its API cannot be extended. Adding a loader fails the test
(checked by mutation).
raiseCatError added a commit that referenced this pull request Oct 14, 2026
Found by dispatch CI 38608819740 on the merge-only head 207de26: #379's
gate allowed one process use under src/agents/mods (claudeDiscovery.ts
listing plugins), and #347 moved that listing into claudePlugins.ts and
added details, validate and enable/disable at the installed scope.
Neither PR's own CI could see the other.

The gate stays strict: every loader check is unchanged, the only
child_process import allowed is execFile, a shell is refused, and each
argv given to Claude's CLI must be one of the four reviewed forms
verbatim. None of them loads or runs mod code in NMSh.

view.ts used inline type imports (import('./controller.js').X), which
the loader check rightly cannot tell from a runtime import(); they are
ordinary type imports now. No runtime change.
@raiseCatError

Copy link
Copy Markdown
Owner Author

Integrated, not merged here: this head (fccf68ea) is part of the release line that shipped in v0.20.0 through the authoritative release PR #392 (merge commit 65e4de55). Closing because its changes reached master by that route; the branch is kept for history.

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.

1 participant