diff --git a/README.md b/README.md index 5dabc71..4e2fcf6 100644 --- a/README.md +++ b/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 ` 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 ` 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 diff --git a/docs/THREAT-MODEL.md b/docs/THREAT-MODEL.md index fc02098..77fb305 100644 --- a/docs/THREAT-MODEL.md +++ b/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. diff --git a/src/apply/mcp.ts b/src/apply/mcp.ts index b3f2d6c..4c3b06e 100644 --- a/src/apply/mcp.ts +++ b/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 = 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}`); } } diff --git a/src/commands/apply.ts b/src/commands/apply.ts index fe3f9f1..21f2d93 100644 --- a/src/commands/apply.ts +++ b/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 to register: ${unregistered.map((server) => server.name).join(", ")}`, + `Bundle records ${unregistered.length} portable MCP server(s) not registered; pass --mcp to register: ${unregistered.map((server) => displayString(server.name, 60)).join(", ")}`, ); } if (failedRegistrations > 0) { diff --git a/src/commands/export.ts b/src/commands/export.ts index b835ab9..1725a09 100644 --- a/src/commands/export.ts +++ b/src/commands/export.ts @@ -16,6 +16,7 @@ const HELP = [ " --plugin Include one plugin reference (repeatable), e.g. --plugin ponytail@ponytail", " --skip Leave one scanned item behind (repeatable), e.g. --skip skill/boxd-cli or --skip settings", " --allow-secret Carry a file despite a secret-content finding (repeatable, refuse-by-default)", + " --mcp 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}`); diff --git a/src/commands/guided.ts b/src/commands/guided.ts index e9937b3..825e01b 100644 --- a/src/commands/guided.ts +++ b/src/commands/guided.ts @@ -15,14 +15,14 @@ import { type AgentRoots, } from "../apply/apply.js"; import type { LoadedBundle } from "../apply/bundle.js"; -import { planMcpRegistrations, runMcpRegistration, type McpRegistration } from "../apply/mcp.js"; +import { assertPortableStdioServer, commandOnPath, planMcpRegistrations, runMcpRegistration, type McpRegistration } from "../apply/mcp.js"; import type { JsonValue } from "../scan/classify.js"; import { commandSummary, scanClaudeCode } from "../scan/scanner.js"; import { scanCodex } from "../scan/codex.js"; import { scanOpencode } from "../scan/opencode.js"; import type { ScanItem, ScanReport } from "../scan/types.js"; import { createTheme } from "../tui/theme.js"; -import { Screen } from "../tui/terminal.js"; +import { Screen, visualWidth } from "../tui/terminal.js"; import { createFlow, type Flow, type MultiGroup } from "../tui/components.js"; import { Plain } from "../tui/plain.js"; import { wordmarkLines } from "../tui/wordmark.js"; @@ -58,6 +58,7 @@ interface Selections { skips: string[]; plugins: string[]; hooks: string[]; + mcp?: string[]; allowSecrets?: string[]; dest: string; } @@ -69,6 +70,7 @@ export function flagEcho(selections: Selections): string { for (const skip of [...selections.skips].sort()) parts.push("--skip", quote(skip)); for (const plugin of [...selections.plugins].sort()) parts.push("--plugin", quote(plugin)); for (const hook of [...selections.hooks].sort()) parts.push("--hook", quote(hook)); + for (const server of [...(selections.mcp ?? [])].sort()) parts.push("--mcp", quote(server)); for (const path of [...(selections.allowSecrets ?? [])].sort()) parts.push("--allow-secret", quote(path)); return parts.join(" "); } @@ -131,7 +133,28 @@ export function buildTravelGroups(report: ScanReport, others: ScanReport[] = []) }), }); } - // Exclusions no longer occupy the picker (the owner's call): the review's + const stdioServers = report.items.filter( + (item) => item.scope === "user" && item.kind === "mcp_server" && item.stdio !== undefined, + ); + if (stdioServers.length > 0) { + groups.push({ + title: "MCP servers \u00b7 commands, re-created with your consent", + items: stdioServers.map((item) => { + const stdio = item.stdio; + const entry: MultiGroup["items"][number] = { + value: `mcp/${item.name}`, + label: item.name, + preselected: false, + }; + if (stdio !== undefined) { + const envSuffix = stdio.envNames.length > 0 ? ` (needs ${stdio.envNames.join(", ")})` : ""; + entry.hint = `${[stdio.command, ...stdio.args].join(" ")}${envSuffix}`; + } + return entry; + }), + }); + } + // Exclusions no longer occupy the picker: the review's // closing line accounts for them where the leaves-the-machine story lives. return groups; } @@ -234,8 +257,9 @@ export async function runGuided(io: CommandIo, mode: GuidedMode, overrides: Guid const skips = groups .filter((group) => group.locked !== true) .flatMap((group) => group.items.map((item) => item.value)) - .filter((token) => !chosen.has(token) && !token.startsWith("plugin/")); + .filter((token) => !chosen.has(token) && !token.startsWith("plugin/") && !token.startsWith("mcp/")); const plugins = travel.filter((token) => token.startsWith("plugin/")).map((token) => token.slice("plugin/".length)); + const selectedMcp = travel.filter((token) => token.startsWith("mcp/")).map((token) => token.slice("mcp/".length)); let hooks: string[] = []; if (hookGroup !== null) { @@ -251,6 +275,7 @@ export async function runGuided(io: CommandIo, mode: GuidedMode, overrides: Guid const collectOptions: Parameters[0] = { confirmedHooks: hooks, selectedPlugins: plugins, + selectedMcp, skips, }; if (overrides.userDir !== undefined) collectOptions.userDir = overrides.userDir; @@ -303,7 +328,7 @@ export async function runGuided(io: CommandIo, mode: GuidedMode, overrides: Guid if (choice === "skip") { ui.outro( "Nothing was written.", - `To pack a different way, scripted: ${flagEcho({ skips, plugins, hooks, allowSecrets, dest })}`, + `To pack a different way, scripted: ${flagEcho({ skips, plugins, hooks, mcp: selectedMcp, allowSecrets, dest })}`, ); return 0; } @@ -319,7 +344,7 @@ export async function runGuided(io: CommandIo, mode: GuidedMode, overrides: Guid const destPath = join(overrides.destDir ?? process.cwd(), dest); writeDestination(destPath, dest, plan); - const selections: Selections = { skips, plugins, hooks, allowSecrets, dest }; + const selections: Selections = { skips, plugins, hooks, mcp: selectedMcp, allowSecrets, dest }; ui.outro( `Packed ${plan.entries.length} files (${formatBytes(totalBytes)}) -> ${destLabel(dest)}`, `Next time, scripted: ${flagEcho(selections)}`, @@ -331,6 +356,29 @@ export async function runGuided(io: CommandIo, mode: GuidedMode, overrides: Guid } } +// Full-content wrapping for consent blocks: every character lands on some +// line; nothing hides past an ellipsis. Widths are visual CELLS, not JS +// characters — CJK and emoji are two cells each, and a char-counted slice +// would overflow the renderer's budget and get truncated right back. +export function wrapDisplay(text: string, width: number): string[] { + if (visualWidth(text) <= width) return [text]; + const lines: string[] = []; + let current = ""; + let cells = 0; + for (const character of text) { + const characterCells = visualWidth(character); + if (cells + characterCells > width && current.length > 0) { + lines.push(lines.length === 0 ? current : ` ${current}`); + current = ""; + cells = 0; + } + current += character; + cells += characterCells; + } + lines.push(lines.length === 0 ? current : ` ${current}`); + return lines; +} + function destLabel(dest: string): string { return dest === "agent-sync-bundle" ? "agent-sync-bundle/" : dest; } @@ -547,7 +595,35 @@ export async function runGuidedApply( } const requestedMcp: string[] = []; + const stdioConsents: { name: string; command: string; envNames: string[] }[] = []; for (const server of bundle.manifest.mcpServers) { + if (server.transport === "stdio" && typeof server.command === "string") { + // Validation precedes display: a string that has not passed the + // portability-and-hygiene gate must never reach a frame, so a + // hostile manifest cannot forge or steer the consent screen. The + // block is wrapped across full lines with no elision — what you + // read is the complete command, because this display IS the + // security boundary. + const validated = assertPortableStdioServer(server.name, server); + const commandLine = [validated.command, ...validated.args].join(" "); + ui.note([ + `mcp server ${server.name} wants to register on this machine:`, + // Width follows the live terminal: the note prefix costs 3 cells, + // fit() truncates at columns-1, and continuation lines indent 4, so + // columns-8 keeps every wrapped character on screen even at 80. + ...wrapDisplay(` command: ${commandLine}`, Math.max(20, ui.width() - 8)), + validated.envNames.length > 0 + ? ` env (values asked next, never carried): ${validated.envNames.join(", ")}` + : " env: none", + ]); + const consent = await ui.confirm(`Register MCP server ${server.name}?`, false); + if (consent === null) return 2; + if (consent) { + requestedMcp.push(server.name); + stdioConsents.push({ name: server.name, command: validated.command, envNames: validated.envNames }); + } + continue; + } if (server.status !== "candidate" || server.url === undefined) continue; let registration: McpRegistration | undefined; try { @@ -564,12 +640,28 @@ export async function runGuidedApply( if (consent) requestedMcp.push(server.name); } + // Secrets are typed fresh on this machine, after consent and before any + // write; they live only in this map on their way into the argv. + const secretValues = new Map(); + for (const consent of stdioConsents) { + for (const envName of consent.envNames) { + const value = await ui.secret(`${consent.name} needs ${envName}`); + if (value === null) return 2; + secretValues.set(`${consent.name}\u0000${envName}`, value); + } + if (!commandOnPath(consent.command)) { + ui.note([`note: ${consent.command} is not on this machine's PATH yet; the registration still lands.`]); + } + } + // Assemble done; from here it is exactly the flag path. The split runs // AFTER the gates because gating rewrites settings.json in the bundle, // and every root is planned before the first root writes. gateSettingsHooks(bundle, confirmedHooks); gateSettingsPlugins(bundle, confirmedPlugins); - const registrations = planMcpRegistrations(bundle.manifest.mcpServers, requestedMcp); + const registrations = planMcpRegistrations(bundle.manifest.mcpServers, requestedMcp, (serverName, envName) => + secretValues.get(`${serverName}\u0000${envName}`), + ); const slices = splitBundleByRoot(bundle, roots); const planned = slices.map((slice) => ({ slice, plan: planApply(slice.bundle, slice.root) })); let creates = 0; @@ -641,6 +733,8 @@ interface GuidedUi { groupMultiselect(message: string, groups: MultiGroup[], coach?: string): Promise; select(message: string, items: typeof DESTINATIONS): Promise; confirm(message: string, initial?: boolean): Promise; + secret(message: string): Promise; + width(): number; review(lines: string[], dest: string): Promise<"pack" | "skip" | "dest" | null>; outro(...lines: string[]): void; close(): void; @@ -691,6 +785,14 @@ function pickerUi(overrides: GuidedOverrides & { wordmark?: boolean }): GuidedUi if (result.cancelled) open = false; return result.cancelled ? null : result.value; }, + async secret(message) { + const result = await flow.secret(message); + if (result.cancelled) open = false; + return result.cancelled ? null : result.value; + }, + width() { + return screen.columns; + }, // The review screen: the bundle tree, then "Pack it?" with the // destination as a named default. y packs, n leaves, d changes the // destination, esc cancels. The picker still owns no writes. @@ -778,6 +880,18 @@ function plainUi(io: CommandIo, overrides: GuidedOverrides): GuidedUi { if (result.cancelled) io.err("Cancelled. Nothing was written."); return result.cancelled ? null : result.value; }, + async secret(message) { + const result = await plain.secret(message); + if (result.cancelled) { + io.err("Cancelled. Nothing was written."); + return null; + } + return result.value; + }, + width() { + // Plain mode never truncates, so wrapping is cosmetic there. + return 4096; + }, // Plain mode reads the same tree and answers one y/N; changing the // destination in plain mode is the scripted flags' job, which the echo // teaches on pack and on skip alike. diff --git a/src/export/collect.ts b/src/export/collect.ts index 8042b41..71038d6 100644 --- a/src/export/collect.ts +++ b/src/export/collect.ts @@ -47,7 +47,12 @@ export interface ManifestMcpServer { reason: string; envRefs?: string[]; url?: string; - transport?: "http" | "sse"; + transport?: "http" | "sse" | "stdio"; + // Present only when the exporter consented with --mcp: the re-creatable + // command shape. Env NAMES only; values never enter a manifest. + command?: string; + args?: string[]; + envNames?: string[]; } export interface ManifestHook { @@ -106,6 +111,7 @@ export interface CollectOptions { opencodeConfigDir?: string; confirmedHooks?: string[]; selectedPlugins?: string[]; + selectedMcp?: string[]; skips?: string[]; allowSecrets?: string[]; } @@ -123,6 +129,7 @@ export function collectExport(options: CollectOptions = {}): ExportPlan { const userDir = options.userDir ?? process.env.CLAUDE_CONFIG_DIR ?? join(homedir(), ".claude"); const confirmedHooks = options.confirmedHooks ?? []; const selectedPlugins = options.selectedPlugins ?? []; + const selectedMcp = options.selectedMcp ?? []; const skips = new Set(options.skips ?? []); const allowSecrets = new Set(options.allowSecrets ?? []); @@ -231,6 +238,23 @@ export function collectExport(options: CollectOptions = {}): ExportPlan { } const includedPlugins = new Set(selectedPlugins.filter((name) => pluginNames.has(name))); + // Stdio definitions are code specs, so they travel only when named with + // --mcp — symmetric to hooks. Remote entries keep traveling as data. + const stdioCapable = new Set( + userItems + .filter((item) => item.kind === "mcp_server" && item.stdio !== undefined) + .map((item) => item.name), + ); + for (const requested of selectedMcp) { + if (!stdioCapable.has(requested)) { + diagnostics.push({ + severity: "error", + message: `--mcp ${requested} does not match any portable command-based server.`, + }); + } + } + const includedMcp = new Set(selectedMcp.filter((name) => stdioCapable.has(name))); + const settingsContent = buildPortableSettings(join(userDir, "settings.json"), { includePreferences: !skips.has("settings"), includedHooks: included, @@ -301,7 +325,7 @@ export function collectExport(options: CollectOptions = {}): ExportPlan { .sort((a, b) => (a.path < b.path ? -1 : 1)), mcpServers: userItems .filter((item) => item.kind === "mcp_server") - .map((item) => toManifestServer(item)), + .map((item) => toManifestServer(item, includedMcp)), hooks: hookItems.map((item) => ({ name: item.name, included: included.has(item.name) })), }; if (hasCodex || hasOpencode) manifest.agents = agents; @@ -326,13 +350,19 @@ export function collectExport(options: CollectOptions = {}): ExportPlan { return { manifest, entries, skipped, secretFindings, diagnostics }; } -function toManifestServer(item: ScanItem): ManifestMcpServer { +function toManifestServer(item: ScanItem, includedMcp: Set): ManifestMcpServer { const server: ManifestMcpServer = { name: item.name, status: item.status, reason: item.reason }; if (item.envRefs !== undefined && item.envRefs.length > 0) server.envRefs = item.envRefs; if (item.url !== undefined) { server.url = item.url; server.transport = item.transport ?? "http"; } + if (item.stdio !== undefined && includedMcp.has(item.name)) { + server.transport = "stdio"; + server.command = item.stdio.command; + server.args = item.stdio.args; + server.envNames = item.stdio.envNames; + } return server; } diff --git a/src/main.ts b/src/main.ts index 180660f..707ea59 100644 --- a/src/main.ts +++ b/src/main.ts @@ -6,7 +6,9 @@ import { undoCommand } from "./commands/undo.js"; import { chooseEntry, runGuided } from "./commands/guided.js"; export { classifyMcpServer, isSensitiveKey, sanitizeRemoteEndpoint, scanSecretReferences } from "./scan/classify.js"; -export { planMcpRegistrations } from "./apply/mcp.js"; +export { planMcpRegistrations, processEnvResolver, commandOnPath, assertPortableStdioServer, maskRegistrationDisplay, displayString } from "./apply/mcp.js"; +export type { McpEnvResolver, McpRegistration } from "./apply/mcp.js"; +export type { StdioDefinition } from "./scan/classify.js"; export { scanClaudeCode, PORTABLE_SETTINGS_KEYS } from "./scan/scanner.js"; export { scanCodex, CODEX_PORTABLE_SETTINGS_KEYS } from "./scan/codex.js"; export { scanOpencode, OPENCODE_PORTABLE_SETTINGS_KEYS, stripJsonc } from "./scan/opencode.js"; @@ -19,7 +21,7 @@ export { Plain, plainModeRequested } from "./tui/plain.js"; export type { ScanReport, ScanItem } from "./scan/types.js"; export { collectExport, skipToken, MANIFEST_SCHEMA_VERSION } from "./export/collect.js"; export type { Manifest, ManifestPlugin, ExportPlan } from "./export/collect.js"; -export { chooseEntry, runGuided, runGuidedApply, flagEcho, applyFlagEcho, buildTravelGroups, buildHookGroup, buildReviewLines, fitReviewLines } from "./commands/guided.js"; +export { chooseEntry, runGuided, runGuidedApply, flagEcho, applyFlagEcho, buildTravelGroups, buildHookGroup, buildReviewLines, fitReviewLines, wrapDisplay } from "./commands/guided.js"; export { wordmarkLines, wordmarkWidth } from "./tui/wordmark.js"; export { scanContentForSecrets } from "./export/secrets.js"; export { createTar } from "./export/tar.js"; diff --git a/src/scan/classify.ts b/src/scan/classify.ts index 00b4f87..ab38779 100644 --- a/src/scan/classify.ts +++ b/src/scan/classify.ts @@ -1,11 +1,20 @@ +import { scanContentForSecrets } from "../export/secrets.js"; + export type JsonValue = string | number | boolean | null | JsonValue[] | { [key: string]: JsonValue }; +export interface StdioDefinition { + command: string; + args: string[]; + envNames: string[]; +} + export interface McpClassification { status: "candidate" | "needs_secret" | "blocked" | "unsupported"; reason: string; envRefs: string[]; url?: string; transport?: "http" | "sse"; + stdio?: StdioDefinition; } export interface EndpointCheck { @@ -51,7 +60,9 @@ function isPrivateHost(hostname: string): boolean { function isPrivateIpv4(host: string): boolean { const octets = host.split(".").map(Number); - if (octets.length !== 4 || !octets.every((octet) => Number.isInteger(octet) && octet >= 0 && octet <= 255)) { + // Shorthand numeric forms ("127.1", "10.5") resolve as IPs too, so any + // all-numeric dotted host gets the range checks, not only 4-octet ones. + if (octets.length < 1 || octets.length > 4 || !octets.every((octet) => Number.isInteger(octet) && octet >= 0)) { return false; } const [a = 0, b = 0] = octets; @@ -207,11 +218,29 @@ export function classifyMcpServer(value: JsonValue): McpClassification { const executable = hasExecutableSurface(value, strings); if (executable || hasLocalPath || hasLocalUrl) { + // A command-based server whose strings are free of machine-local paths + // and local URLs is portable STRUCTURE: command, args, and env NAMES. + // Env values are dropped right here at extraction, so no downstream + // layer can ever see one. Anything pinned to this machine stays blocked. + if (executable && !hasLocalPath && !hasLocalUrl) { + const stdio = extractStdioDefinition(value, secrets.envRefs); + if (stdio !== null) { + const needsSecrets = secrets.hasSecretReference || stdio.envNames.length > 0; + return { + status: needsSecrets ? "needs_secret" : "candidate", + reason: needsSecrets + ? "Command-based server; the command travels with consent, and env values are entered fresh on the target." + : "Command-based server; the command travels with consent.", + envRefs: secrets.envRefs, + stdio, + }; + } + } const reason = hasLocalUrl ? "Localhost MCP endpoints cannot work from another machine." : hasLocalPath ? "References a machine-local path that will not exist on the target." - : "Stdio and command-based MCP servers run local programs and are never applied."; + : "Stdio and command-based MCP servers run local programs; this entry has no re-creatable command shape."; return { status: "blocked", reason, envRefs: secrets.envRefs }; } @@ -254,6 +283,60 @@ export function classifyMcpServer(value: JsonValue): McpClassification { }; } +// Pulls the re-creatable shape out of a stdio entry: command, string args, +// and the env NAMES the server expects (object keys plus $VAR references). +// Values under env are intentionally never read past their keys. +function extractStdioDefinition(value: JsonValue, envRefs: string[]): StdioDefinition | null { + if (value === null || typeof value !== "object" || Array.isArray(value)) return null; + const record = value as { [key: string]: JsonValue }; + if (typeof record.command !== "string" || record.command.trim().length === 0) return null; + const command = record.command.trim(); + // A script filename anywhere ("serve.py", "scripts/serve.py") is a + // machine-local file in disguise: it resolves against a working directory + // that only exists here. Package specifiers (@scope/name) pass; loose + // scripts do not — in the command or in any arg. + if (looksLikeLooseScript(command)) return null; + const rawArgs = record.args; + const args: string[] = []; + if (rawArgs !== undefined) { + if (!Array.isArray(rawArgs)) return null; + for (const arg of rawArgs) { + if (typeof arg !== "string") return null; + if (looksLikeLooseScript(arg)) return null; + // A URL pinned to loopback or a private range cannot work elsewhere. + const url = arg.match(/^https?:\/\//i) ? tryParseHost(arg) : null; + if (url !== null && isPrivateHost(url)) return null; + // An argument that looks like a credential IS a value; values never + // travel. The fix on the source machine is moving it into env. + if (scanContentForSecrets(Buffer.from(arg, "utf8")).length > 0) return null; + args.push(arg); + } + } + if (scanContentForSecrets(Buffer.from(command, "utf8")).length > 0) return null; + const envNames = new Set(envRefs); + const env = record.env; + if (env !== null && env !== undefined && typeof env === "object" && !Array.isArray(env)) { + for (const name of Object.keys(env)) { + if (ENV_VAR_NAME.test(name)) envNames.add(name); + } + } + return { command, args, envNames: [...envNames].sort() }; +} + +const LOOSE_SCRIPT = /\.(py|sh|bash|zsh|js|ts|mjs|cjs|rb|pl|php|lua)$/i; + +function looksLikeLooseScript(text: string): boolean { + return LOOSE_SCRIPT.test(text) && !text.startsWith("@"); +} + +function tryParseHost(raw: string): string | null { + try { + return new URL(raw).hostname; + } catch { + return null; + } +} + function hasExecutableSurface(value: JsonValue, strings: string[]): boolean { if (value !== null && typeof value === "object" && !Array.isArray(value)) { const record = value as { [key: string]: JsonValue }; diff --git a/src/scan/codex.ts b/src/scan/codex.ts index 2669e33..397268d 100644 --- a/src/scan/codex.ts +++ b/src/scan/codex.ts @@ -117,6 +117,7 @@ function scanConfigToml(path: string, scope: Scope, items: ScanItem[], diagnosti item.url = classification.url; item.transport = classification.transport ?? "http"; } + if (classification.stdio !== undefined) item.stdio = classification.stdio; items.push(item); } } diff --git a/src/scan/opencode.ts b/src/scan/opencode.ts index 046f81d..09af0f5 100644 --- a/src/scan/opencode.ts +++ b/src/scan/opencode.ts @@ -141,6 +141,7 @@ function scanOpencodeConfig(path: string, scope: Scope, items: ScanItem[], diagn item.url = classification.url; item.transport = classification.transport ?? "http"; } + if (classification.stdio !== undefined) item.stdio = classification.stdio; items.push(item); } break; diff --git a/src/scan/scanner.ts b/src/scan/scanner.ts index f25f47d..6d6b7ca 100644 --- a/src/scan/scanner.ts +++ b/src/scan/scanner.ts @@ -327,6 +327,7 @@ function scanMcpConfig( item.url = classification.url; item.transport = classification.transport ?? "http"; } + if (classification.stdio !== undefined) item.stdio = classification.stdio; items.push(item); } } diff --git a/src/scan/types.ts b/src/scan/types.ts index f74047a..e5d428f 100644 --- a/src/scan/types.ts +++ b/src/scan/types.ts @@ -22,6 +22,7 @@ export interface ScanItem { envRefs?: string[]; url?: string; transport?: "http" | "sse"; + stdio?: { command: string; args: string[]; envNames: string[] }; } export interface ExcludedPath { diff --git a/src/tui/components.ts b/src/tui/components.ts index 547496c..227095c 100644 --- a/src/tui/components.ts +++ b/src/tui/components.ts @@ -30,6 +30,7 @@ export interface Flow { note(lines: string[]): void; outro(text: string): void; confirm(message: string, initial?: boolean): Promise>; + secret(message: string): Promise>; select(message: string, items: MultiItem[]): Promise>; groupMultiselect(message: string, groups: MultiGroup[], options?: { required?: boolean; coach?: string }): Promise>; } @@ -208,7 +209,7 @@ function toggle(state: MultiState, id: number): void { else state.selected.add(id); } -// The drawing the owner signed off: chip in the header, coach line until the +// The design of record: chip in the header, coach line until the // first toggle, plain group headers with air between groups, full-row // highlight on the active item, hints demoted to one detail line, the locked // section a single counted line unless v expands it, and a three-entry footer @@ -384,6 +385,32 @@ export function createFlow(screen: Screen, theme: Theme): Flow { } }, + // Masked line entry for values that must never echo: asterisks on + // screen, the real characters only in memory, esc or ctrl+c cancels. + async secret(message) { + let value = ""; + for (;;) { + screen.renderLive([ + `${theme.paint("accent", g.stepActive)} ${theme.paint("bright", message)}`, + `${bar} ${"*".repeat(Math.min(value.length, 40))}${theme.paint("inverse", " ")}`, + `${theme.paint("accent", g.railEnd)} ${theme.paint("dim", ["type the value", "enter confirm", "esc cancel"].join(` ${g.sep} `))}`, + ]); + const key = await screen.waitKey(); + if (key.name === "cancel" || key.name === "escape") { + cancelCommit(message); + return cancelled(); + } + if (key.name === "return" || key.name === "enter") { + if (value.length === 0) continue; + screen.clearLive(); + summaryCommit(message, "********"); + return done(value); + } + if (key.name === "backspace") value = value.slice(0, -1); + else if (key.char !== null) value += key.char; + } + }, + async select(message, items) { if (items.length === 0) throw new Error("select needs at least one option."); let cursor = 0; diff --git a/src/tui/plain.ts b/src/tui/plain.ts index 415b6f8..0d61538 100644 --- a/src/tui/plain.ts +++ b/src/tui/plain.ts @@ -75,6 +75,17 @@ export class Plain { } } + // Plain mode cannot mask input (it reads cooked lines), so it says so + // before asking; the value still never appears in any later output. + async secret(message: string): Promise> { + for (;;) { + const answer = await this.ask(`${message} (input is visible in this mode)`); + if (answer === null) return cancelled(); + if (answer.trim().length > 0) return done(answer.trim()); + this.say("A value is required, or ctrl+c to cancel."); + } + } + async select(message: string, items: MultiItem[]): Promise> { for (;;) { const listing = items diff --git a/test/codex.test.mjs b/test/codex.test.mjs index 0072205..2c15bd9 100644 --- a/test/codex.test.mjs +++ b/test/codex.test.mjs @@ -82,8 +82,9 @@ test("codex mcp servers classify with the shared classifier", () => { assert.equal(linear.status, "candidate"); assert.equal(linear.url, "https://mcp.linear.app/mcp"); const local = item("local"); - assert.equal(local.status, "blocked"); + assert.equal(local.status, "needs_secret"); assert.deepEqual(local.envRefs, ["LOCAL_TOKEN"]); + assert.deepEqual(local.stdio, { command: "npx", args: ["-y", "some-mcp"], envNames: ["LOCAL_TOKEN"] }); }); test("project scope picks up config and root AGENTS.md", () => { diff --git a/test/scan.test.mjs b/test/scan.test.mjs index 5677c74..6f289fe 100644 --- a/test/scan.test.mjs +++ b/test/scan.test.mjs @@ -149,7 +149,10 @@ test("mcp servers are classified without values", () => { test("local-scope servers nested under projects are found", () => { assert.equal(item("nestedremote").status, "candidate"); + // python3 serve.py resolves against a working directory that only exists + // here, so the bare-script guard keeps it blocked. assert.equal(item("nestedlocal").status, "blocked"); + assert.equal(item("nestedlocal").stdio, undefined); }); test("a dollar sign inside a secret value never emits a fragment env ref", () => { diff --git a/test/stdio-mcp.test.mjs b/test/stdio-mcp.test.mjs new file mode 100644 index 0000000..cc9b1dc --- /dev/null +++ b/test/stdio-mcp.test.mjs @@ -0,0 +1,519 @@ +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { EventEmitter } from "node:events"; +import { execFileSync } from "node:child_process"; +import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { + buildTravelGroups, + collectExport, + createFlow, + createTheme, + flagEcho, + loadBundleFromBuffer, + planMcpRegistrations, + runGuidedApply, + scanClaudeCode, + Screen, +} from "../dist/main.js"; + +const REPO_ROOT = fileURLToPath(new URL("..", import.meta.url)); +const fakeHome = mkdtempSync(join(tmpdir(), "agent-sync-stdio-home-")); + +const PLANTED_ENV_VALUE = "planted-ctx7-env-value-928374"; + +let root; +let userDir; +let claudeJsonPath; + +before(() => { + root = mkdtempSync(join(tmpdir(), "agent-sync-stdio-")); + userDir = join(root, ".claude"); + mkdirSync(join(userDir, "skills", "reviewer"), { recursive: true }); + writeFileSync(join(userDir, "skills", "reviewer", "SKILL.md"), "# reviewer\n"); + writeFileSync(join(userDir, "settings.json"), JSON.stringify({ model: "opus" })); + claudeJsonPath = join(userDir, ".claude.json"); + writeFileSync( + claudeJsonPath, + JSON.stringify({ + mcpServers: { + linear: { type: "http", url: "https://mcp.linear.app/mcp" }, + ctx7: { command: "npx", args: ["-y", "@upstash/context7-mcp"], env: { CTX7_TOKEN: PLANTED_ENV_VALUE } }, + timeserver: { command: "uvx", args: ["mcp-time"] }, + pinned: { command: "node", args: ["/Users/someone/tool.js"] }, + }, + }), + ); +}); + +after(() => { + rmSync(root, { recursive: true, force: true }); + rmSync(fakeHome, { recursive: true, force: true }); +}); + +function collect(options = {}) { + return collectExport({ + userDir, + claudeJsonPath, + codexHome: join(fakeHome, ".codex"), + codexAgentsDir: join(fakeHome, ".agents"), + opencodeConfigDir: join(fakeHome, ".config", "opencode"), + ...options, + }); +} + +test("classification: portable stdio carries structure, env values die at extraction, pinned stays blocked", () => { + const report = scanClaudeCode({ userDir, projectDir: null, claudeJsonPath }); + const byName = (name) => report.items.find((item) => item.name === name); + + const ctx7 = byName("ctx7"); + assert.equal(ctx7.status, "needs_secret"); + assert.deepEqual(ctx7.stdio, { command: "npx", args: ["-y", "@upstash/context7-mcp"], envNames: ["CTX7_TOKEN"] }); + + const timeserver = byName("timeserver"); + assert.equal(timeserver.status, "candidate"); + assert.deepEqual(timeserver.stdio, { command: "uvx", args: ["mcp-time"], envNames: [] }); + + assert.equal(byName("pinned").status, "blocked"); + assert.equal(byName("pinned").stdio, undefined); + + assert.ok(!JSON.stringify(report).includes(PLANTED_ENV_VALUE), "an env VALUE escaped classification"); +}); + +test("export: stdio structure travels only behind --mcp, and values never reach the manifest", () => { + const withoutConsent = collect(); + const ctx7 = withoutConsent.manifest.mcpServers.find((server) => server.name === "ctx7"); + assert.equal(ctx7.command, undefined, "structure must not travel without --mcp"); + + const consented = collect({ selectedMcp: ["ctx7", "timeserver"] }); + const packed = consented.manifest.mcpServers.find((server) => server.name === "ctx7"); + assert.equal(packed.transport, "stdio"); + assert.equal(packed.command, "npx"); + assert.deepEqual(packed.args, ["-y", "@upstash/context7-mcp"]); + assert.deepEqual(packed.envNames, ["CTX7_TOKEN"]); + const everything = + JSON.stringify(consented.manifest) + consented.entries.map((entry) => entry.content.toString("latin1")).join(""); + assert.ok(!everything.includes(PLANTED_ENV_VALUE), "an env VALUE reached the export plan"); + + assert.ok(collect({ selectedMcp: ["linear"] }).diagnostics.some((d) => d.severity === "error"), + "remote servers are not --mcp consent targets at export"); + assert.ok(collect({ selectedMcp: ["nope"] }).diagnostics.some((d) => d.severity === "error")); +}); + +test("the travel picker gains an MCP group with command hints, and the flag echo spells --mcp", () => { + const report = scanClaudeCode({ userDir, projectDir: null, claudeJsonPath }); + const groups = buildTravelGroups(report, []); + const mcpGroup = groups.find((group) => group.title.startsWith("MCP servers")); + assert.deepEqual( + mcpGroup.items.map((item) => item.value).sort(), + ["mcp/ctx7", "mcp/timeserver"], + ); + const hint = mcpGroup.items.find((item) => item.value === "mcp/ctx7").hint; + assert.match(hint, /npx -y @upstash\/context7-mcp \(needs CTX7_TOKEN\)/); + assert.ok(mcpGroup.items.every((item) => item.preselected !== true), "commands are opt-in"); + + assert.equal( + flagEcho({ dest: "setup.tgz", skips: [], plugins: [], hooks: [], mcp: ["ctx7"] }), + "agent-sync export setup.tgz --mcp ctx7", + ); +}); + +test("planMcpRegistrations: stdio argv via resolver, fail-closed on missing env, refusals on dirty input", () => { + const servers = [ + { name: "ctx7", status: "needs_secret", reason: "", transport: "stdio", command: "npx", args: ["-y", "pkg"], envNames: ["CTX7_TOKEN"] }, + ]; + const [registration] = planMcpRegistrations(servers, ["ctx7"], (server, envName) => + server === "ctx7" && envName === "CTX7_TOKEN" ? "fresh-value" : undefined, + ); + assert.deepEqual(registration.args, [ + "mcp", "add", "--transport", "stdio", "--scope", "user", "ctx7", + "--env", "CTX7_TOKEN=fresh-value", "--", "npx", "-y", "pkg", + ]); + + assert.throws( + () => planMcpRegistrations(servers, ["ctx7"], () => undefined), + /needs CTX7_TOKEN/, + ); + assert.throws( + () => planMcpRegistrations([{ ...servers[0], command: "npx\u0000evil" }], ["ctx7"], () => "v"), + /not a clean string/, + ); + assert.throws( + () => planMcpRegistrations([{ ...servers[0], envNames: ["BAD NAME"] }], ["ctx7"], () => "v"), + /not valid variable names/, + ); + assert.throws( + () => planMcpRegistrations([{ ...servers[0], args: Array.from({ length: 65 }, () => "a") }], ["ctx7"], () => "v"), + /not clean strings/, + ); +}); + +function cliEnv(home, extra = {}) { + return { + ...process.env, + HOME: home, + USERPROFILE: home, + CLAUDE_CONFIG_DIR: join(home, ".claude"), + CODEX_HOME: join(home, ".codex"), + XDG_CONFIG_HOME: join(home, ".config"), + XDG_DATA_HOME: join(home, ".local", "share"), + ...extra, + }; +} + +test("static apply: env comes from the target machine's environment, missing env fails closed by name", () => { + const bundlePath = join(root, "stdio-bundle.tgz"); + execFileSync(process.execPath, ["bin/agent-sync.mjs", "export", bundlePath, "--mcp", "ctx7"], { + cwd: REPO_ROOT, + env: { ...cliEnv(fakeHome), CLAUDE_CONFIG_DIR: userDir }, + }); + + const applyHome = join(root, "static-apply-home"); + mkdirSync(join(applyHome, ".claude"), { recursive: true }); + const shimDir = join(root, "claude-shim"); + mkdirSync(shimDir, { recursive: true }); + const shimLog = join(shimDir, "log.txt"); + writeFileSync(join(shimDir, "claude"), `#!/bin/sh\necho "$@" >> "${shimLog}"\n`); + chmodSync(join(shimDir, "claude"), 0o755); + + try { + execFileSync( + process.execPath, + ["bin/agent-sync.mjs", "apply", bundlePath, "--mcp", "ctx7"], + { cwd: REPO_ROOT, encoding: "utf8", env: cliEnv(applyHome, { PATH: `${shimDir}:${process.env.PATH}` }) }, + ); + assert.fail("expected the missing env to fail closed"); + } catch (error) { + assert.equal(error.status, 2); + assert.match(String(error.stderr), /needs CTX7_TOKEN/); + assert.ok(!existsSync(join(applyHome, ".claude", "settings.json")), "fail-closed must precede file writes"); + } + + const output = execFileSync( + process.execPath, + ["bin/agent-sync.mjs", "apply", bundlePath, "--mcp", "ctx7"], + { + cwd: REPO_ROOT, + encoding: "utf8", + env: cliEnv(applyHome, { PATH: `${shimDir}:${process.env.PATH}`, CTX7_TOKEN: "target-env-value" }), + }, + ); + assert.match(output, /Registered MCP server ctx7/); + const logged = readFileSync(shimLog, "utf8"); + assert.match(logged, /--env CTX7_TOKEN=target-env-value -- npx -y @upstash\/context7-mcp/); + assert.ok(!logged.includes(PLANTED_ENV_VALUE), "the source machine's env value must never appear"); +}); + +function fakeScreenIo() { + const input = new EventEmitter(); + input.isTTY = true; + input.resume = () => {}; + input.pause = () => {}; + input.read = () => null; + input.setRawMode = () => {}; + const chunks = []; + const output = { + write: (chunk) => chunks.push(chunk), + columns: 400, + rows: 30, + isTTY: true, + on: () => {}, + off: () => {}, + }; + return { input, output, chunks }; +} + +function fakeRoots(claude) { + return { + claude, + codexHome: join(fakeHome, ".codex"), + codexAgents: join(fakeHome, ".agents"), + opencodeConfig: join(fakeHome, ".config", "opencode"), + }; +} + +test("guided apply: consent shows the command with env placeholders, the typed secret reaches argv but never the screen", async () => { + const bundlePath = join(root, "stdio-guided.tgz"); + execFileSync(process.execPath, ["bin/agent-sync.mjs", "export", bundlePath, "--mcp", "ctx7"], { + cwd: REPO_ROOT, + env: { ...cliEnv(fakeHome), CLAUDE_CONFIG_DIR: userDir }, + }); + const bundle = await loadBundleFromBuffer(readFileSync(bundlePath)); + const target = join(root, "guided-target"); + mkdirSync(target, { recursive: true }); + const registered = []; + const io = { out: () => {}, err: () => {} }; + const fake = fakeScreenIo(); + const screen = new Screen({ input: fake.input, output: fake.output }); + + const running = runGuidedApply(io, "picker", bundle, "stdio-guided.tgz", { + targetDir: target, + roots: fakeRoots(target), + screen, + env: {}, + register: (registration) => registered.push(registration), + }); + setImmediate(() => { + const press = (char, name) => fake.input.emit("keypress", char, { name, sequence: char ?? "\r" }); + press("y", "y"); + press(undefined, "return"); // consent to ctx7 (stdio) + press(undefined, "return"); // decline linear (remote), default no + for (const character of "s3cret-typed-fresh") press(character, character); + press(undefined, "return"); // submit the secret + }); + assert.equal(await running, 0); + + assert.equal(registered.length, 1); + assert.ok(registered[0].args.includes("CTX7_TOKEN=s3cret-typed-fresh")); + + const rendered = fake.chunks.join(""); + assert.match(rendered, /mcp server ctx7 wants to register on this machine/); + assert.match(rendered, /command: npx -y @upstash\/context7-mcp/); + assert.match(rendered, /env \(values asked next, never carried\): CTX7_TOKEN/); + assert.match(rendered, /Register MCP server ctx7\?/); + assert.match(rendered, /ctx7 needs CTX7_TOKEN/); + assert.match(rendered, /\*{6,}/, "the masked prompt shows asterisks"); + assert.ok(!rendered.includes("s3cret-typed-fresh"), "the typed secret leaked to the screen"); + assert.ok(!rendered.includes(PLANTED_ENV_VALUE)); +}); + +test("flow.secret: masks input, backspace works, esc cancels", async () => { + const fake = fakeScreenIo(); + const screen = new Screen({ input: fake.input, output: fake.output }); + const flow = createFlow(screen, createTheme({ env: { TERM: "linux" }, platform: "linux", isTTY: false })); + flow.intro("t"); + const pending = flow.secret("token?"); + const press = (char, name) => fake.input.emit("keypress", char, { name, sequence: char ?? "\r" }); + for (const character of "abcd") press(character, character); + press(undefined, "backspace"); + press(undefined, "return"); + assert.deepEqual(await pending, { cancelled: false, value: "abc" }); + assert.ok(!fake.chunks.join("").includes("abc"), "plaintext leaked from the masked prompt"); + screen.close(); + + const second = fakeScreenIo(); + const screen2 = new Screen({ input: second.input, output: second.output }); + const flow2 = createFlow(screen2, createTheme({ env: { TERM: "linux" }, platform: "linux", isTTY: false })); + flow2.intro("t"); + const cancelledPending = flow2.secret("token?"); + second.input.emit("keypress", undefined, { name: "escape", sequence: "\u001b" }); + assert.deepEqual(await cancelledPending, { cancelled: true }); + screen2.close(); +}); + +test("round 1: dry-run plans with placeholders and never echoes a real value", () => { + const bundlePath = join(root, "stdio-dry.tgz"); + execFileSync(process.execPath, ["bin/agent-sync.mjs", "export", bundlePath, "--mcp", "ctx7"], { + cwd: REPO_ROOT, + env: { ...cliEnv(fakeHome), CLAUDE_CONFIG_DIR: userDir }, + }); + const home = join(root, "dry-home"); + mkdirSync(join(home, ".claude"), { recursive: true }); + + const withEnv = execFileSync( + process.execPath, + ["bin/agent-sync.mjs", "apply", bundlePath, "--mcp", "ctx7", "--dry-run"], + { cwd: REPO_ROOT, encoding: "utf8", env: cliEnv(home, { CTX7_TOKEN: "real-secret-value-555" }) }, + ); + assert.match(withEnv, /--env CTX7_TOKEN=\.\.\. -- npx/); + assert.ok(!withEnv.includes("real-secret-value-555"), "dry-run echoed a resolved secret"); + + const withoutEnv = execFileSync( + process.execPath, + ["bin/agent-sync.mjs", "apply", bundlePath, "--mcp", "ctx7", "--dry-run"], + { cwd: REPO_ROOT, encoding: "utf8", env: cliEnv(home) }, + ); + assert.match(withoutEnv, /Would register MCP server ctx7/, "dry-run must not require env values"); +}); + +test("round 1: CR, LF and TAB are rejected as dirty strings", () => { + const base = { name: "x", status: "needs_secret", reason: "", transport: "stdio", command: "npx", envNames: [] }; + for (const dirty of ["a\nb", "a\rb", "a\tb"]) { + assert.throws( + () => planMcpRegistrations([{ ...base, args: [dirty] }], ["x"], () => "v"), + /not clean strings/, + JSON.stringify(dirty), + ); + } +}); + +test("round 1: the stdio branch re-runs the portability gate over the untrusted manifest", () => { + const base = { name: "x", status: "needs_secret", reason: "", transport: "stdio", envNames: [] }; + const cases = [ + { ...base, command: "/Users/attacker/payload.sh", args: [] }, + { ...base, command: "npx", args: ["scripts/serve.py"] }, + { ...base, command: "serve.py", args: [] }, + { ...base, command: "npx", args: ["http://10.0.0.1/steal"] }, + { ...base, command: "npx", args: ["http://127.1/loop"] }, + { ...base, command: "ruby", args: ["tool.rb"] }, + ]; + for (const server of cases) { + assert.throws( + () => planMcpRegistrations([server], ["x"], () => "v"), + /portability gate|not a clean string/, + JSON.stringify(server), + ); + } + assert.throws( + () => planMcpRegistrations([{ ...base, command: "npx", args: ["--token", `${"ghp"}_${"A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8"}`] }], ["x"], () => "v"), + /portability gate/, + "a token-shaped arg is a value and values never travel", + ); +}); + +test("round 1: a hostile manifest cannot reach the guided consent frame", async () => { + const { createHash } = await import("node:crypto"); + const dir = join(root, "hostile-consent"); + mkdirSync(join(dir, "files"), { recursive: true }); + const settings = Buffer.from(JSON.stringify({ model: "opus" })); + const manifest = { + schemaVersion: 1, + tool: "agent-sync", + agent: "claude-code", + files: [{ path: "settings.json", sha256: createHash("sha256").update(settings).digest("hex"), size: settings.length }], + mcpServers: [ + { + name: "evil", + status: "needs_secret", + reason: "", + transport: "stdio", + command: "npx", + args: ["ok\r\nFORGED-CONSENT-LINE: totally safe, press y"], + envNames: [], + }, + ], + hooks: [], + }; + writeFileSync(join(dir, "files", "settings.json"), settings); + writeFileSync(join(dir, "manifest.json"), JSON.stringify(manifest)); + const { loadBundleFromDirectory } = await import("../dist/main.js"); + const bundle = loadBundleFromDirectory(dir); + + const target = join(root, "hostile-consent-target"); + mkdirSync(target, { recursive: true }); + const io = { out: () => {}, err: () => {} }; + const fake = fakeScreenIo(); + const screen = new Screen({ input: fake.input, output: fake.output }); + let failure = null; + try { + await runGuidedApply(io, "picker", bundle, "evil.tgz", { + targetDir: target, + roots: fakeRoots(target), + screen, + env: {}, + register: () => {}, + }); + assert.fail("hostile stdio entry must refuse"); + } catch (error) { + failure = error; + } + assert.match(String(failure), /not clean strings/); + assert.ok(!fake.chunks.join("").includes("FORGED-CONSENT-LINE"), "the forged text reached a frame"); + assert.ok(!existsSync(join(target, "settings.json")), "nothing may be written"); +}); + +test("round 1: wrapDisplay loses no characters and maskRegistrationDisplay hides values", async () => { + const { wrapDisplay, maskRegistrationDisplay } = await import("../dist/main.js"); + const long = ` command: npx ${"x".repeat(300)}end`; + const wrapped = wrapDisplay(long, 76); + assert.ok(wrapped.length > 3); + assert.equal(wrapped.map((line, i) => (i === 0 ? line : line.slice(4))).join(""), long, "wrap must preserve every character"); + assert.ok(wrapped.join("").includes("end")); + + const registration = { name: "x", args: ["mcp", "add", "--env", "TOKEN=sup3r-s3cret", "--", "npx"] }; + const masked = maskRegistrationDisplay("claude mcp add --env TOKEN=sup3r-s3cret -- npx (sup3r-s3cret)", registration); + assert.ok(!masked.includes("sup3r-s3cret")); + assert.match(masked, /TOKEN=\.\.\./); +}); + + +test("round 2: the consent wrap survives an 80-column terminal with zero hidden characters", async () => { + const bundleDir = join(root, "narrow-consent"); + mkdirSync(join(bundleDir, "files"), { recursive: true }); + const { createHash } = await import("node:crypto"); + const settings = Buffer.from(JSON.stringify({ model: "opus" })); + const longArg = `${"a".repeat(200)}ZZENDMARKZZ`; + const manifest = { + schemaVersion: 1, + tool: "agent-sync", + agent: "claude-code", + files: [{ path: "settings.json", sha256: createHash("sha256").update(settings).digest("hex"), size: settings.length }], + mcpServers: [ + { name: "longone", status: "needs_secret", reason: "", transport: "stdio", command: "npx", args: ["-y", longArg], envNames: [] }, + ], + hooks: [], + }; + writeFileSync(join(bundleDir, "files", "settings.json"), settings); + writeFileSync(join(bundleDir, "manifest.json"), JSON.stringify(manifest)); + const { loadBundleFromDirectory } = await import("../dist/main.js"); + const bundle = loadBundleFromDirectory(bundleDir); + + const target = join(root, "narrow-consent-target"); + mkdirSync(target, { recursive: true }); + const io = { out: () => {}, err: () => {} }; + const fake = fakeScreenIo(); + fake.output.columns = 80; + const screen = new Screen({ input: fake.input, output: fake.output }); + + const running = runGuidedApply(io, "picker", bundle, "n.tgz", { + targetDir: target, + roots: fakeRoots(target), + screen, + env: {}, + register: () => {}, + }); + setImmediate(() => { + fake.input.emit("keypress", undefined, { name: "return", sequence: "\r" }); // decline the consent, default no + }); + assert.equal(await running, 0); + const rendered = fake.chunks.join(""); + assert.ok(rendered.includes("ZZENDMARKZZ"), "the tail of a wrapped consent line was hidden at 80 columns"); +}); + +test("round 2: an unvalidated hostile name never reaches stderr unsanitized, and C1/bidi controls refuse", () => { + const hostileName = "\u001b[2Jevil"; + const base = { name: hostileName, status: "needs_secret", reason: "", transport: "stdio", command: "npx", args: [], envNames: [] }; + let failure = null; + try { + planMcpRegistrations([base], [hostileName], () => "v"); + } catch (error) { + failure = String(error); + } + assert.ok(failure !== null); + assert.ok(!failure.includes("\u001b"), "raw ESC survived into the error message"); + assert.ok(failure.includes("\ufffd")); + + const clean = { name: "x", status: "needs_secret", reason: "", transport: "stdio", command: "npx", envNames: [] }; + for (const dirty of ["a\u009bb", "a\u202eb", "a\u200bb", "a\u2066b", "a\ufeffb"]) { + assert.throws( + () => planMcpRegistrations([{ ...clean, args: [dirty] }], ["x"], () => "v"), + /not clean strings/, + JSON.stringify(dirty), + ); + } +}); + + +test("round 3: wide characters cannot resurrect the consent elision, and Cf invisibles refuse", async () => { + const { wrapDisplay } = await import("../dist/main.js"); + const wide = "\u5b57".repeat(120) + "WIDETAILMARK"; + const wrapped = wrapDisplay(` command: npx ${wide}`, 72); + assert.ok(wrapped.join("").includes("WIDETAILMARK"), "the wide tail fell off the wrap"); + for (const line of wrapped) { + const content = line.startsWith(" ") ? line.slice(4) : line; + let cells = 0; + for (const ch of content) cells += /[\u1100-\u9fff]/.test(ch) ? 2 : 1; + assert.ok(cells <= 72, `a wrapped line overflows the cell budget: ${cells}`); + } + + const clean = { name: "x", status: "needs_secret", reason: "", transport: "stdio", command: "npx", envNames: [] }; + for (const dirty of ["a\u061cb", "a\u2060b", "a\u00adb"]) { + assert.throws( + () => planMcpRegistrations([{ ...clean, args: [dirty] }], ["x"], () => "v"), + /not clean strings/, + JSON.stringify(dirty), + ); + } +}); diff --git a/test/surface.test.mjs b/test/surface.test.mjs index 2b2cae9..d02256c 100644 --- a/test/surface.test.mjs +++ b/test/surface.test.mjs @@ -14,7 +14,7 @@ test("public surface is frozen", () => { assert.deepEqual(Object.keys(GLOBAL_FLAGS).sort(), ["help", "version"]); assert.deepEqual(ALL_COMMANDS.map((command) => command.word), ["scan", "export", "apply", "undo"]); assert.deepEqual(Object.keys(ALL_COMMANDS[0].flags).sort(), ["json", "no-project", "project"]); - assert.deepEqual(Object.keys(ALL_COMMANDS[1].flags).sort(), ["allow-secret", "dry-run", "hook", "json", "plugin", "skip"]); + assert.deepEqual(Object.keys(ALL_COMMANDS[1].flags).sort(), ["allow-secret", "dry-run", "hook", "json", "mcp", "plugin", "skip"]); assert.deepEqual(Object.keys(ALL_COMMANDS[2].flags).sort(), ["dry-run", "hook", "mcp", "no-input", "plain", "plugin", "target"]); assert.deepEqual(Object.keys(ALL_COMMANDS[3].flags).sort(), ["target"]); });