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

docs/design/portable-mod-isolation.md

@@ -0,0 +1,101 @@
+# Executable Portable Mods: security gate
+
+Issue: [#341](https://github.com/raiseCatError/notMyShell/issues/341). Decision recorded here: **executable Portable
+Mods stay disabled.** No platform NMSh supports has an isolation design that has been built, reviewed and attacked;
+until one has, a Portable Mod is an inert, declarative descriptor bound to a read-only, deny-by-default broker.
+
+This document states the threat model, what exists today, why the obvious shortcuts are not sandboxes, what an
+acceptable design must demonstrate per platform, and the exact block. `tests/portableModGate.test.ts` keeps the block
+in place: adding a loader fails it.
+
+## Three classes, three trust boundaries
+
+| Class | What runs | Under whose security |
+| --- | --- | --- |
+| Context Pack | nothing: declarative module data over core capabilities | NMSh core (#305, #318) |
+| NMSh Portable Mod | **nothing today**: an inert descriptor and a read-only broker | would be NMSh's, and that is what this gate is about |
+| Provider-native mod (a Claude Code plugin, …) | the provider's code | the provider's and the user's; NMSh's wrapping does not sandbox it |
+
+Provider-native permissions never imply Portable Mod authority, and a Portable Mod's approval is never the provider's
+`tool.approve`.
+
+## What exists (audited on `release/v0.18.0`)
+
+- `src/agents/mods/broker.ts`: `PortableDescriptor.execution` is the literal type `'inert'`. `ModBroker.bind` returns
+  a frozen API with only `identity` and `request(permission)`; grants are intersected with the descriptor's requested
+  permissions and with a fixed read-only set (`context.status`, `session.status`, `targets.list`,
+  `targets.aggregate`); everything else answers "Capability not granted (deny by default)". No provider handle,
+  filesystem, process or network object crosses it. `PortableInstance` holds at most 64 KiB of plain JSON state and
+  runs only host-supplied projection functions.
+- `src/agents/mods/model.ts`: the only built-in descriptor (Context Meter) is listed as `sandbox: 'Not executable'`,
+  `executesInside: 'none (inert)'`, not enabled.
+- `src/agents/mods/claudeDiscovery.ts` runs `claude plugin list --json` (the provider's own CLI, bounded by time and
+  bytes) to inventory provider-native plugins. That lists provider code; it does not load or run any of it in NMSh.
+- There is no loader, no dynamic `import()`, `vm`, worker, WebAssembly instantiation or `eval` anywhere under
+  `src/agents/mods/`.
+
+## Threat model
+
+An executable mod is untrusted code, possibly written to attack the person using it. It must not be able to:
+
+1. read or write files, spawn processes, open sockets or resolve names, except through capabilities the person
+   granted for that mod and that target;
+2. read secrets: environment variables, credentials, NMSh's configuration, other targets' transcripts;
+3. act as the person: send input to a shell or agent, approve a tool call, or answer a question, unless that exact
+   action is a granted, mediated capability with its own approval;
+4. forge presentation: write raw ANSI, overdraw NMSh chrome, spoof approval prompts or provider facts;
+5. exhaust the machine: CPU, memory, output, file descriptors, or time;
+6. persist or escalate: survive removal, modify NMSh or other mods, or load more code;
+7. cross targets: see or affect a target it was not granted, or aggregate across targets without an explicit grant.
+
+Integrity (what was installed is what runs), provenance (who published it), update and revocation are part of the
+same gate.
+
+## Why the shortcuts are not sandboxes
+
+- **A worker thread or child process alone** shares the user's identity, filesystem and network. Process separation
+  limits crashes, not authority.
+- **Node's permission model** (`--permission`): the Node.js documentation says it "does not protect against
+  malicious code" and is a "seat belt" for trusted code; it lists bypasses (existing file descriptors, symlinks,
+  `node:sqlite`). It is useful defense in depth, not the boundary.
+- **`node:vm`** contexts share the process and are not a security mechanism.
+- **Inert broker types** (what exists) prove nothing about code that is not bound by them.
+
+## What an acceptable design must demonstrate
+
+One isolation layer must be **enforced by something the mod's code cannot reach**, per supported platform:
+
+| Platform | Candidate | Open questions before it could pass |
+| --- | --- | --- |
+| Linux | a separate process in new user/mount/network/PID namespaces (as bubblewrap does) with a seccomp-bpf filter and Landlock rules; cgroup or `setrlimit` limits | unprivileged user namespaces are disabled on some distributions; Landlock ABI varies by kernel; must fail closed |
+| macOS | a separate process under a Seatbelt profile (`sandbox-exec`/`sandbox_init`) | the interface is deprecated and undocumented for third parties; profile semantics change between releases; App Sandbox needs a signed app with entitlements, which a CLI installed by npm or Homebrew is not |
+| Any | WebAssembly modules with **no WASI** and only host-defined imports (the broker), run in a separate process with OS limits; memory bounded by the module's declared maximum, CPU by an external deadline and termination | still needs the process-level limits above; side channels; the host imports become the whole attack surface and need adversarial review |
+
+Whatever is chosen must show, with adversarial tests that run in CI on each supported platform:
+
+- default deny for filesystem, network, process creation and environment, verified by mods that try each;
+- resource limits (CPU time, memory, output bytes, open files, wall time) that terminate and clean up;
+- cancellation and cleanup on target close, NMSh exit and crash;
+- per-target separation and explicit, revocable cross-target grants;
+- a mod approval owner separate from provider approvals, with keyboard-first, Safe/NO_COLOR-clear prompts;
+- contributions only as bounded typed data rendered by NMSh (status items, sidebar items, panel content, actions,
+  annotations): no ANSI, no renderer ownership, no focus stealing, no approval decoration that changes the decision;
+- integrity (pinned hashes), provenance display, update and revocation;
+- honest unsupported-platform behavior: where isolation cannot be enforced, execution is unavailable and says why.
+
+## The block
+
+- `PortableDescriptor.execution` stays the literal `'inert'`; any other value is a type error.
+- No dynamic code loading under `src/agents/mods/`: no `import()`, `require()`, `new Function`, `eval`, `node:vm`,
+  `node:worker_threads`, `WebAssembly.instantiate`/`compile`. The single `child_process` use is the provider-native
+  inventory listing (`claude plugin list --json`).
+- The broker grants only the fixed read-only permission set, and only what the descriptor requested.
+- Built-in inventory entries for Portable Mods are `Not executable`.
+
+`tests/portableModGate.test.ts` asserts each of these. Lifting the block means changing those tests in the same
+change that delivers the isolation, its threat-model review and its adversarial suite; it is not a setting.
+
+## Related
+
+- #332 and #336 consume typed contributions (panel content, actions) only after this gate.
+- #339 owns provider task/event truth; #340 Remote Control; #334 must not offer executable mods during onboarding.

tests/portableModGate.test.ts

@@ -0,0 +1,68 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import {readdirSync, readFileSync} from 'node:fs';
+import {join} from 'node:path';
+import {ModBroker, type ModPermission, type PortableDescriptor, type TargetSnapshot} from '../src/agents/mods/broker.js';
+import {builtinModInventory, CONTEXT_METER} from '../src/agents/mods/model.js';
+
+/**
+ * The executable Portable Mod gate (#341, docs/design/portable-mod-isolation.md). These tests keep execution disabled
+ * until enforced isolation exists: lifting the block means changing them together with the isolation, its review and
+ * its adversarial suite.
+ */
+
+const MODS = join(import.meta.dirname, '..', 'src', 'agents', 'mods');
+const LOADERS: ReadonlyArray<[RegExp, string]> = [
+  [/\bimport\s*\(/u, 'dynamic import()'], [/\brequire\s*\(/u, 'require()'], [/\bnew\s+Function\b/u, 'new Function'], [/\beval\s*\(/u, 'eval'],
+  [/['"]node:vm['"]|['"]vm['"]/u, 'node:vm'], [/worker_threads/u, 'worker threads'], [/WebAssembly\.(?:instantiate|compile|Module|Instance)/u, 'WebAssembly'],
+];
+
+test('no code-loading path exists under src/agents/mods', () => {
+  const files = readdirSync(MODS).filter(name => name.endsWith('.ts'));
+  assert.ok(files.length > 0);
+  for (const file of files) {
+    const source = readFileSync(join(MODS, file), 'utf8');
+    for (const [pattern, name] of LOADERS) assert.doesNotMatch(source, pattern, `${file} must not use ${name} until #341's isolation gate passes`);
+    // The one process use is the provider's own plugin listing: inventory of provider-native code, never running it in NMSh.
+    if (/child_process/u.test(source)) {
+      assert.equal(file, 'claudeDiscovery.ts', `${file} spawns processes`);
+      assert.match(source, /\['plugin', 'list', '--json'\]/u);
+    }
+  }
+});
+
+test('Portable descriptors are inert and the inventory says they are not executable', () => {
+  const execution: PortableDescriptor['execution'] = CONTEXT_METER.execution;
+  assert.equal(execution, 'inert');
+  for (const entry of builtinModInventory().filter(item => item.kind === 'Portable')) {
+    assert.equal(entry.sandbox, 'Not executable');
+    assert.match(entry.executesInside, /inert/u);
+    assert.equal(entry.enabled, 'no');
+  }
+});
+
+test('the broker is read-only and deny-by-default: only requested, granted, read permissions answer', () => {
+  const target: TargetSnapshot = {id: 't1', provider: 'claude', state: 'idle', context: {percent: 40, observedAt: 1},
+    capabilities: {'context.status': {availability: 'available'}} as unknown as TargetSnapshot['capabilities']};
+  const other: TargetSnapshot = {...target, id: 't2'};
+  const broker = new ModBroker(() => [target, other]);
+  const everything: ModPermission[] = ['context.status', 'session.status', 'conversation.read', 'tool.observe', 'approval.observe', 'approval.decorate', 'approval.explain', 'targets.list', 'targets.aggregate'];
+  const descriptor: PortableDescriptor = {id: 'probe', name: 'Probe', requested: everything, providers: ['claude'], execution: 'inert'};
+  const api = broker.bind(descriptor, 't1', 'i1', everything);
+  // Writes, approvals and conversation content are never granted, whatever is requested and granted.
+  for (const permission of ['conversation.read', 'tool.observe', 'approval.observe', 'approval.decorate', 'approval.explain'] as ModPermission[]) {
+    assert.equal(api.request(permission).ok, false, permission);
+  }
+  assert.deepEqual(api.request('context.status'), {ok: true, value: {targetId: 't1', percent: 40, observedAt: 1}});
+  // Not requested by the descriptor: denied even if granted.
+  const narrow = broker.bind({...descriptor, requested: ['context.status']}, 't1', 'i2', everything);
+  assert.equal(narrow.request('session.status').ok, false);
+  // Not granted: denied even if requested.
+  assert.deepEqual(broker.bind(descriptor, 't1', 'i3', []).request('context.status'), {ok: false, reason: 'Capability not granted (deny by default)'});
+  // A provider the descriptor does not apply to: denied.
+  assert.equal(broker.bind({...descriptor, providers: ['codex']}, 't1', 'i4', everything).request('context.status').ok, false);
+  // The API carries nothing but identity and request; it cannot be extended.
+  assert.deepEqual(Object.keys(api).sort(), ['identity', 'request']);
+  assert.ok(Object.isFrozen(api) && Object.isFrozen(api.identity));
+  assert.throws(() => { (api as unknown as Record<string, unknown>).provider = {}; });
+});