Stdio MCP porting: structure travels, secrets never do

README.md

@@ -47,7 +47,7 @@ The scanner reads an explicit allowlist of paths and nothing else. These are exc
 
 File contents are scanned at export too: a file whose content matches a token pattern (known key prefixes, private-key blocks, high-entropy strings) is refused by default and carried only after explicit consent — a y/N in the guided flow, `--allow-secret <path>` in flag mode. Findings name the file and line, never the matched value.
 
-MCP server entries are recorded as a name plus a portability class. Secret values are never copied; a server that needs one gets flagged so you can re-enter it on the target. Hooks are shell commands, so each one is confirmed individually before it is included, and confirmed again on the machine that applies it.
+MCP server entries are recorded as a name plus a portability class. Remote servers travel as name and sanitized URL. Command-based (stdio) servers can travel too, as structure only — the command, its args, and the NAMES of the env variables it needs — behind a repeatable `--mcp <name>` at export and again at apply; the values are typed fresh on the target (the guided apply asks with a masked prompt, the scripted path reads the target machine's own environment and fails closed naming the missing variable — deliberately no flag, so secrets never touch argv or shell history). Secret values are never copied anywhere. Hooks are shell commands, so each one is confirmed individually before it is included, and confirmed again on the machine that applies it.
 
 ## No telemetry
 

docs/THREAT-MODEL.md

@@ -28,7 +28,7 @@ Each guarantee names the code that enforces it. All of them are covered by tests
 
 **Writes cannot escape the target.** Beyond lexical path containment, every write path is walked component by component and refused if any existing component is a symlink. The target directory itself may be a symlink; nothing under it may be (`resolveForWrite` in `src/apply/apply.ts`).
 
-**Code execution needs consent on both machines.** Hooks and `statusLine` entries are shell commands, and an enabled plugin installs marketplace code. They enter a bundle only when named with `--hook` or `--plugin` at export, and they apply only when named again at apply; a marketplace source survives the gate only when a confirmed plugin references it. Behind the gates sits a settings allowlist: a bundle's `settings.json` may carry only the keys export can produce (the portable preference keys, `statusLine`, `hooks`, `enabledPlugins`, `extraKnownMarketplaces`, each shape-checked), so keys that execute code on the target (`apiKeyHelper`, `env`, auth refresh scripts) refuse the bundle whole rather than riding past a consent filter that does not know them (`assertPortableSettings` in `src/apply/apply.ts`). A consequence with teeth: adding a new portable settings key is no longer a schema-invisible change, because an older apply will refuse a newer bundle that carries it — a future key must bump the manifest schema version, or knowingly accept that older applies fail loud on such bundles. The schema version itself is an advisory compatibility signal, not a trust boundary: every receive-side control (path allowlist, per-agent key allowlists, per-root containment) is enforced regardless of the schema a bundle claims, so no check may ever gate on it. MCP registration is opt-in per server with `--mcp`, goes through the agent's own CLI with name and URL only, and never carries tokens, headers, or environment values.
+**Code execution needs consent on both machines.** Hooks and `statusLine` entries are shell commands, and an enabled plugin installs marketplace code. They enter a bundle only when named with `--hook` or `--plugin` at export, and they apply only when named again at apply; a marketplace source survives the gate only when a confirmed plugin references it. Behind the gates sits a settings allowlist: a bundle's `settings.json` may carry only the keys export can produce (the portable preference keys, `statusLine`, `hooks`, `enabledPlugins`, `extraKnownMarketplaces`, each shape-checked), so keys that execute code on the target (`apiKeyHelper`, `env`, auth refresh scripts) refuse the bundle whole rather than riding past a consent filter that does not know them (`assertPortableSettings` in `src/apply/apply.ts`). A consequence with teeth: adding a new portable settings key is no longer a schema-invisible change, because an older apply will refuse a newer bundle that carries it — a future key must bump the manifest schema version, or knowingly accept that older applies fail loud on such bundles. The schema version itself is an advisory compatibility signal, not a trust boundary: every receive-side control (path allowlist, per-agent key allowlists, per-root containment) is enforced regardless of the schema a bundle claims, so no check may ever gate on it. MCP registration is opt-in per server with `--mcp` on BOTH sides and goes through the agent's own CLI. Remote servers carry name and sanitized URL only. Command-based (stdio) servers carry structure only — command, args, env NAMES — with values dropped at classification time so no downstream layer can see one (superseding the earlier rule that stdio servers are never applied); on the target, values are typed fresh in the guided flow or read from the machine's own environment, the untrusted manifest entry is fully revalidated before anything is displayed or assembled — string hygiene across the whole C0 range, env-name shape, size caps, and a rerun of the same portability gate export uses — and env values are masked in every echoed or dry-run line, and no shell is ever involved. Servers pinned to a machine (local paths, localhost endpoints) remain blocked. Tokens, headers, and environment values never travel in any form.
 
 **Applies are reversible.** What an apply overwrites is backed up first, the record of the apply lands before the first destructive write, and `agent-sync undo` restores it or aborts untouched if the backup is incomplete.
 

src/apply/mcp.ts

@@ -1,19 +1,35 @@
 import { execFileSync } from "node:child_process";
-import { sanitizeRemoteEndpoint } from "../scan/classify.js";
+import { classifyMcpServer, sanitizeRemoteEndpoint } from "../scan/classify.js";
 import type { ManifestMcpServer } from "../export/collect.js";
 
 const SERVER_NAME = /^[A-Za-z0-9][A-Za-z0-9_.-]*$/;
+const ENV_NAME = /^[A-Za-z_][A-Za-z0-9_]*$/;
+const MAX_ARGS = 64;
+const MAX_ENV_NAMES = 32;
+const MAX_STRING_LENGTH = 2000;
 
 export interface McpRegistration {
   name: string;
   args: string[];
 }
 
+// Env values for a stdio server are resolved ON the applying machine, never
+// carried: the guided flow prompts for them, the static path reads the
+// target's own environment.
+export type McpEnvResolver = (server: string, envName: string) => string | undefined;
+
+export function processEnvResolver(env: Record<string, string | undefined> = process.env): McpEnvResolver {
+  return (_server, envName) => env[envName];
+}
+
 // The manifest is untrusted input, so everything the export side sanitized is
-// revalidated here before it can reach an argv.
+// revalidated here before it can reach an argv. Stdio entries get the full
+// treatment: name shape, control-byte-free strings, env-name shape, and size
+// caps — then argv assembly with no shell anywhere.
 export function planMcpRegistrations(
   servers: ManifestMcpServer[],
   requested: string[],
+  resolveEnv: McpEnvResolver = processEnvResolver(),
 ): McpRegistration[] {
   const byName = new Map(servers.map((server) => [server.name, server]));
   const registrations: McpRegistration[] = [];
@@ -23,6 +39,13 @@ export function planMcpRegistrations(
     if (server === undefined) {
       throw new Error(`--mcp ${name} does not match any server in this bundle's manifest.`);
     }
+    if (!SERVER_NAME.test(name)) {
+      throw new Error(`--mcp ${displayString(name, 60)}: server name is not safe to pass along. Nothing was registered.`);
+    }
+    if (server.transport === "stdio" || server.command !== undefined) {
+      registrations.push(planStdioRegistration(name, server, resolveEnv));
+      continue;
+    }
     if (server.status !== "candidate") {
       throw new Error(
         `--mcp ${name} is not portable (${server.status}): ${server.reason} Nothing was registered.`,
@@ -31,9 +54,6 @@ export function planMcpRegistrations(
     if (server.url === undefined) {
       throw new Error(`--mcp ${name}: the bundle records no re-addable URL for this server.`);
     }
-    if (!SERVER_NAME.test(name)) {
-      throw new Error(`--mcp ${name}: server name is not safe to pass along. Nothing was registered.`);
-    }
     const endpoint = sanitizeRemoteEndpoint(server.url);
     if (!endpoint.ok || endpoint.url === undefined) {
       throw new Error(`--mcp ${name}: ${endpoint.reason ?? "URL is not clean"}. Nothing was registered.`);
@@ -48,16 +68,136 @@ export function planMcpRegistrations(
   return registrations;
 }
 
+// Everything a stdio entry must prove before it may appear on ANY screen or
+// reach an argv: name shape, control-byte-free strings, env-name shape, size
+// caps, and a rerun of the exact export-side portability gate — the complete
+// re-derivation of the untrusted manifest. Exported so the guided flow can
+// validate BEFORE it builds a consent line: a string that has not passed
+// this function must never be displayed.
+export function assertPortableStdioServer(
+  name: string,
+  server: ManifestMcpServer,
+): { command: string; args: string[]; envNames: string[] } {
+  if (!SERVER_NAME.test(name)) {
+    // The one message that may carry an unvalidated name sanitizes it first;
+    // main.ts prints error messages verbatim to stderr.
+    throw new Error(`--mcp ${displayString(name, 60)}: server name is not safe to pass along. Nothing was registered.`);
+  }
+  if (typeof server.command !== "string" || !cleanString(server.command)) {
+    throw new Error(`--mcp ${name}: the bundle's command is not a clean string. Nothing was registered.`);
+  }
+  const args = server.args ?? [];
+  if (!Array.isArray(args) || args.length > MAX_ARGS || !args.every((arg) => typeof arg === "string" && cleanString(arg))) {
+    throw new Error(`--mcp ${name}: the bundle's args are not clean strings. Nothing was registered.`);
+  }
+  const envNames = server.envNames ?? [];
+  if (
+    !Array.isArray(envNames) ||
+    envNames.length > MAX_ENV_NAMES ||
+    !envNames.every((envName) => typeof envName === "string" && ENV_NAME.test(envName))
+  ) {
+    throw new Error(`--mcp ${name}: the bundle's env names are not valid variable names. Nothing was registered.`);
+  }
+  const reclassified = classifyMcpServer({
+    command: server.command,
+    args: [...args],
+    env: Object.fromEntries(envNames.map((envName) => [envName, ""])),
+  });
+  if (reclassified.stdio === undefined) {
+    throw new Error(
+      `--mcp ${name}: this definition does not pass the portability gate (${reclassified.reason}) Nothing was registered.`,
+    );
+  }
+  return { command: server.command, args: [...args], envNames: [...envNames] };
+}
+
+function planStdioRegistration(
+  name: string,
+  server: ManifestMcpServer,
+  resolveEnv: McpEnvResolver,
+): McpRegistration {
+  const { command, args, envNames } = assertPortableStdioServer(name, server);
+
+  const argv = ["mcp", "add", "--transport", "stdio", "--scope", "user", name];
+  for (const envName of envNames) {
+    const value = resolveEnv(name, envName);
+    if (value === undefined || value.length === 0) {
+      throw new Error(
+        `--mcp ${name} needs ${envName}: set it in this machine's environment (or run the guided apply, which asks for it). Values are never carried in bundles.`,
+      );
+    }
+    argv.push("--env", `${envName}=${value}`);
+  }
+  argv.push("--", command, ...args);
+  return { name, args: argv };
+}
+
+// For hint lines that mention entries nobody validated yet (the unregistered
+// list, the PATH note): strip anything that could steer a terminal and cap
+// the length, so a hostile name or command cannot forge output.
+export function displayString(text: string, cap = 120): string {
+  const stripped = text.replace(/[\p{Cc}\p{Cf}\u2028\u2029]/gu, "\ufffd");
+  return stripped.length > cap ? `${stripped.slice(0, cap)}...` : stripped;
+}
+
+function cleanString(text: string): boolean {
+  // The whole class, not a list: Cc is every control (C0, DEL, C1 — U+009B
+  // is a one-codepoint CSI on xterm-class terminals), Cf is every invisible
+  // format character (zero-widths, bidi and isolate controls, BOM, ALM, soft
+  // hyphen, word joiner), plus the Zl/Zp separators explicitly. Invisibles
+  // let a displayed command read identically to a different argv, and
+  // commands and args have no legitimate use for any of them.
+  return (
+    text.length > 0 &&
+    text.length <= MAX_STRING_LENGTH &&
+    !/[\p{Cc}\p{Cf}\u2028\u2029]/u.test(text)
+  );
+}
+
+// A missing binary is a warning, not a refusal: the registration still
+// lands, and the user may well install the runner right after applying.
+export function commandOnPath(command: string): boolean {
+  const probe = process.platform === "win32" ? "where" : "which";
+  try {
+    execFileSync(probe, [command], { stdio: ["ignore", "ignore", "ignore"] });
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+// Anywhere a registration's argv is shown or echoed back, env values are
+// replaced with the name and an ellipsis. The only place a real value may
+// exist is the argv handed to execFile.
+export function maskRegistrationDisplay(text: string, registration: McpRegistration): string {
+  let masked = text;
+  for (let index = 0; index < registration.args.length - 1; index += 1) {
+    if (registration.args[index] !== "--env") continue;
+    const pair = registration.args[index + 1];
+    if (pair === undefined) continue;
+    const equals = pair.indexOf("=");
+    if (equals <= 0) continue;
+    const value = pair.slice(equals + 1);
+    if (value.length === 0) continue;
+    masked = masked.split(pair).join(`${pair.slice(0, equals)}=...`);
+    masked = masked.split(value).join("...");
+  }
+  return masked;
+}
+
 export function runMcpRegistration(registration: McpRegistration): void {
   try {
     execFileSync("claude", registration.args, { stdio: ["ignore", "pipe", "pipe"] });
   } catch (error) {
-    const detail =
+    const rawDetail =
       error !== null && typeof error === "object" && "stderr" in error && Buffer.isBuffer(error.stderr)
         ? error.stderr.toString("utf8").trim()
         : error instanceof Error
           ? error.message
           : String(error);
+    // A CLI that echoes bad argv back would otherwise put the secret in our
+    // error message; redact before it can reach any frame or log.
+    const detail = maskRegistrationDisplay(rawDetail, registration);
     throw new Error(`registering MCP server ${registration.name} via \`claude mcp add\` failed: ${detail}`);
   }
 }

src/commands/apply.ts

@@ -11,7 +11,7 @@ import {
   planApply,
   splitBundleByRoot,
 } from "../apply/apply.js";
-import { planMcpRegistrations, runMcpRegistration } from "../apply/mcp.js";
+import { commandOnPath, displayString, maskRegistrationDisplay, planMcpRegistrations, processEnvResolver, runMcpRegistration } from "../apply/mcp.js";
 import { stringList } from "./export.js";
 import { chooseEntry, runGuidedApply } from "./guided.js";
 
@@ -108,7 +108,13 @@ export const applyCommand: CommandDef = {
     }
     // Registrations are validated before any file write so a bad --mcp refuses
     // the whole apply, not half of it.
-    const registrations = planMcpRegistrations(bundle.manifest.mcpServers, requestedMcp);
+    // A dry run must never resolve real secrets: it plans with a "..."
+    // placeholder resolver, so the value cannot exist to be echoed.
+    const registrations = planMcpRegistrations(
+      bundle.manifest.mcpServers,
+      requestedMcp,
+      dryRun ? () => "..." : processEnvResolver(),
+    );
 
     // A bundle may span several agents; each agent root gets its own plan,
     // marker and backups, and every root is planned (write-checked) before
@@ -173,7 +179,7 @@ export const applyCommand: CommandDef = {
     let failedRegistrations = 0;
     for (const registration of registrations) {
       if (dryRun) {
-        io.out(`Would register MCP server ${registration.name}: claude ${registration.args.join(" ")}`);
+        io.out(maskRegistrationDisplay(`Would register MCP server ${registration.name}: claude ${registration.args.join(" ")}`, registration));
         continue;
       }
       try {
@@ -185,12 +191,22 @@ export const applyCommand: CommandDef = {
       }
     }
 
+    for (const registration of registrations) {
+      const server = bundle.manifest.mcpServers.find((candidate) => candidate.name === registration.name);
+      if (server?.transport === "stdio" && typeof server.command === "string" && !commandOnPath(server.command)) {
+        io.out(`Note: ${displayString(server.command)} is not on this machine's PATH yet; the ${displayString(server.name)} registration still lands.`);
+      }
+    }
+
     const unregistered = bundle.manifest.mcpServers.filter(
-      (server) => server.status === "candidate" && server.url !== undefined && !requestedMcp.includes(server.name),
+      (server) =>
+        !requestedMcp.includes(server.name) &&
+        ((server.status === "candidate" && server.url !== undefined) ||
+          (server.transport === "stdio" && server.command !== undefined)),
     );
     if (unregistered.length > 0) {
       io.out(
-        `Bundle records ${unregistered.length} portable MCP server(s) not registered; pass --mcp <name> to register: ${unregistered.map((server) => server.name).join(", ")}`,
+        `Bundle records ${unregistered.length} portable MCP server(s) not registered; pass --mcp <name> to register: ${unregistered.map((server) => displayString(server.name, 60)).join(", ")}`,
       );
     }
     if (failedRegistrations > 0) {

src/commands/export.ts

@@ -16,6 +16,7 @@ const HELP = [
   "  --plugin <name>  Include one plugin reference (repeatable), e.g. --plugin ponytail@ponytail",
   "  --skip <item>    Leave one scanned item behind (repeatable), e.g. --skip skill/boxd-cli or --skip settings",
   "  --allow-secret <path>  Carry a file despite a secret-content finding (repeatable, refuse-by-default)",
+  "  --mcp <name>     Carry one command-based MCP server's structure (repeatable; env values never travel)",
   "  --dry-run        Print exactly what would be packed and write nothing",
   "  --json           Print the manifest as JSON instead of the summary",
   "  --help           Show help",
@@ -30,6 +31,7 @@ export const exportCommand: CommandDef = {
     plugin: { type: "string", description: "Include one plugin reference (repeatable)", multiple: true },
     skip: { type: "string", description: "Leave one scanned item behind (repeatable)", multiple: true },
     "allow-secret": { type: "string", description: "Carry a file despite a secret-content finding (repeatable)", multiple: true },
+    mcp: { type: "string", description: "Carry one command-based MCP server's structure (repeatable)", multiple: true },
     "dry-run": { type: "boolean", description: "Print the packing list and write nothing" },
     json: { type: "boolean", description: "Print the manifest as JSON" },
   },
@@ -49,8 +51,9 @@ export const exportCommand: CommandDef = {
     const selectedPlugins = stringList(values.plugin);
     const skips = stringList(values.skip);
     const allowSecrets = stringList(values["allow-secret"]);
+    const selectedMcp = stringList(values.mcp);
 
-    const plan = collectExport({ confirmedHooks, selectedPlugins, skips, allowSecrets });
+    const plan = collectExport({ confirmedHooks, selectedPlugins, skips, allowSecrets, selectedMcp });
 
     if (plan.diagnostics.some((diagnostic) => diagnostic.severity === "error")) {
       for (const diagnostic of plan.diagnostics) io.err(`${diagnostic.severity}: ${diagnostic.message}`);