fix(security): harden remote-URL & runtime-arg validation; honest keychain notice (M4/M5/I1)

src/__tests__/commands/install.test.ts

@@ -1176,7 +1176,15 @@ describe("validateRuntimeArgs", () => {
   });
 
   it("rejects arguments containing path traversal (..)", () => {
-    expect(() => validateRuntimeArgs(["../../etc/passwd"])).toThrow(/unrecognized/i);
+    // M4b: a ".." path segment is rejected up front (more specific than the
+    // generic "unrecognized" allowlist failure), in bare and flag-value forms.
+    expect(() => validateRuntimeArgs(["../../etc/passwd"])).toThrow(/traversal/i);
+    expect(() => validateRuntimeArgs([".."])).toThrow(/traversal/i);
+    expect(() => validateRuntimeArgs(["a/../../b"])).toThrow(/traversal/i);
+    expect(() => validateRuntimeArgs(["--config=../secret"])).toThrow(/traversal/i);
+    expect(() => validateRuntimeArgs(["--config=../../secret"])).toThrow(/traversal/i);
+    // A non-traversal double-dot inside a token is left alone.
+    expect(() => validateRuntimeArgs(["--range=1..10"])).not.toThrow();
   });
 
   it("rejects shell metacharacters in flag values", () => {
@@ -1203,8 +1211,19 @@ describe("validateRemoteUrl", () => {
     expect(() => validateRemoteUrl("https://api.example.com/mcp")).not.toThrow();
   });
 
-  it("accepts http URLs", () => {
+  it("accepts http URLs only for loopback hosts", () => {
     expect(() => validateRemoteUrl("http://localhost:3000/mcp")).not.toThrow();
+    expect(() => validateRemoteUrl("http://127.0.0.1:8080/mcp")).not.toThrow();
+    expect(() => validateRemoteUrl("http://app.localhost/mcp")).not.toThrow();
+    // IPv6 loopback — the bracket-strip in isLoopbackHost is load-bearing.
+    expect(() => validateRemoteUrl("http://[::1]:8080/mcp")).not.toThrow();
+  });
+
+  it("rejects plaintext http to non-loopback hosts (M4a)", () => {
+    expect(() => validateRemoteUrl("http://api.example.com/mcp")).toThrow(/https for non-loopback|interception/i);
+    expect(() => validateRemoteUrl("http://10.0.0.5/mcp")).toThrow(/https for non-loopback|interception/i);
+    // https to the same host is always fine.
+    expect(() => validateRemoteUrl("https://api.example.com/mcp")).not.toThrow();
   });
 
   it("rejects file:// URLs", () => {

src/__tests__/commands/up.test.ts

@@ -515,4 +515,74 @@ servers:
     expect(confirm).not.toHaveBeenCalled();
     expect(adapter.removeServer).toHaveBeenCalledWith("/mock/config.json", "extra-server");
   });
+
+  // M4a (PR #66): the up path now validates stack-file `url:` servers before writing.
+  // These exercise processUrlServer end-to-end (previously untested at this level).
+  const urlStack = (url: string) => `
+version: "1"
+servers:
+  io.github.test/remote:
+    url: "${url}"
+`;
+
+  it("installs a valid https url: server to Cursor", async () => {
+    const stackPath = await writeStackAndLock(urlStack("https://api.example.com/mcp"), basicLock);
+    const adapter = makeAdapter();
+    const deps = makeDeps({
+      detectClients: vi.fn<() => Promise<ClientId[]>>().mockResolvedValue(["cursor"]),
+      getAdapter: vi.fn().mockReturnValue(adapter),
+    });
+
+    await handleUp({ stackFile: stackPath }, deps);
+
+    expect(adapter.addServer).toHaveBeenCalledWith(
+      "/mock/config.json",
+      "io.github.test/remote",
+      { url: "https://api.example.com/mcp" },
+      { force: true }
+    );
+  });
+
+  it("blocks a url: server with a non-loopback plaintext-http URL (M4a)", async () => {
+    const stackPath = await writeStackAndLock(urlStack("http://10.0.0.5/mcp"), basicLock);
+    const adapter = makeAdapter();
+    const deps = makeDeps({
+      detectClients: vi.fn<() => Promise<ClientId[]>>().mockResolvedValue(["cursor"]),
+      getAdapter: vi.fn().mockReturnValue(adapter),
+    });
+
+    await expect(handleUp({ stackFile: stackPath }, deps)).rejects.toThrow(/could not be installed/);
+    expect(adapter.addServer).not.toHaveBeenCalled();
+  });
+
+  it("blocks a url: server with a non-http(s) scheme", async () => {
+    const stackPath = await writeStackAndLock(urlStack("file:///etc/passwd"), basicLock);
+    const adapter = makeAdapter();
+    const deps = makeDeps({
+      detectClients: vi.fn<() => Promise<ClientId[]>>().mockResolvedValue(["cursor"]),
+      getAdapter: vi.fn().mockReturnValue(adapter),
+    });
+
+    await expect(handleUp({ stackFile: stackPath }, deps)).rejects.toThrow(/could not be installed/);
+    expect(adapter.addServer).not.toHaveBeenCalled();
+  });
+
+  // Regression: --dry-run is read-only — an invalid URL must NOT throw or exit
+  // non-zero; it is previewed as "would reject".
+  it("dry-run does not fail on an invalid url: server (regression)", async () => {
+    const stackPath = await writeStackAndLock(urlStack("http://10.0.0.5/mcp"), basicLock);
+    const adapter = makeAdapter();
+    const deps = makeDeps({
+      detectClients: vi.fn<() => Promise<ClientId[]>>().mockResolvedValue(["cursor"]),
+      getAdapter: vi.fn().mockReturnValue(adapter),
+    });
+
+    await expect(
+      handleUp({ stackFile: stackPath, dryRun: true }, deps)
+    ).resolves.toBeUndefined();
+    expect(adapter.addServer).not.toHaveBeenCalled();
+
+    const out = (deps.output as ReturnType<typeof vi.fn>).mock.calls.map((c) => c[0]).join("\n");
+    expect(out).toMatch(/would reject URL/);
+  });
 });

src/commands/install.ts

@@ -43,6 +43,32 @@ export function validateRemoteUrl(url: string): void {
       `Remote URL must use http or https protocol, got: "${parsed.protocol}"`
     );
   }
+  // M4a: plaintext http to a non-loopback host is interceptable once written to an
+  // IDE config. Allow http only for loopback (local dev servers); require https for
+  // every other host. https is always allowed.
+  if (parsed.protocol === "http:" && !isLoopbackHost(parsed.hostname)) {
+    throw new Error(
+      `Remote URL must use https for non-loopback hosts (plaintext http is ` +
+        `vulnerable to interception), got: "${url}"`
+    );
+  }
+}
+
+/**
+ * True for localhost / loopback literals, where plaintext http is acceptable.
+ * Recognizes localhost / *.localhost / 127.0.0.1 / ::1. Exotic loopback spellings
+ * (IPv4-mapped `::ffff:127.0.0.1`, `127.x.x.x`, decimal/octal/hex IPs) are NOT
+ * recognized and fall through to the https requirement — over-rejection only, never
+ * a bypass (a non-loopback host can never be mistaken for loopback).
+ */
+function isLoopbackHost(hostname: string): boolean {
+  const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); // strip IPv6 brackets
+  return (
+    h === "localhost" ||
+    h.endsWith(".localhost") ||
+    h === "127.0.0.1" ||
+    h === "::1"
+  );
 }
 
 const NPM_IDENTIFIER_RE = /^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/;
@@ -126,6 +152,16 @@ const SAFE_ARG_PATTERNS: readonly RegExp[] = [
  */
 export function validateRuntimeArgs(args: string[]): void {
   for (const arg of args) {
+    // Layer 0 (M4b): reject a ".." path-traversal segment anywhere in the argument
+    // — "../x", "a/../../etc/passwd", "--config=../secret". A ".." segment is one
+    // bounded by start-of-arg, "=" (flag value), or a path separator on the left,
+    // and a separator or end-of-arg on the right. The Layer-2 allowlist permits "."
+    // and "/" inside values, so without this a traversal would slip through; a
+    // non-traversal double dot like "--range=1..10" is left untouched.
+    if (/(?:^|[=\\/])\.\.(?:[\\/]|$)/.test(arg)) {
+      throw new Error(`Rejected path traversal in runtime argument: "${arg}"`);
+    }
+
     // Layer 1: reject dangerous Node.js flags
     const isDangerous = DANGEROUS_FLAG_PREFIXES.some(
       (prefix) => arg === prefix || arg.startsWith(`${prefix}=`)
@@ -512,8 +548,11 @@ export async function handleInstall(
   if (!options.json) {
     if (secretsMode === "keychain" && storedSecretCount > 0) {
       output(
-        `\x1b[32mStored ${storedSecretCount} secret(s) encrypted at rest in ~/.mcpm ` +
-        "(protects against other-user/offline access, not same-user processes). " +
+        `\x1b[32mStored ${storedSecretCount} secret(s) encrypted at rest in ~/.mcpm. ` +
+        "With an OS keychain this protects against other-user/offline access (not " +
+        "same-user processes); without one a machine-derived key is used that guards " +
+        "casual local inspection only, NOT file exfiltration — run `mcpm secrets migrate` " +
+        "once a keychain is available. " +
         "Run `mcpm guard enable` (then restart your IDE) so they resolve at launch — " +
         "until guard wraps this server it receives the literal placeholder.\x1b[0m"
       );

src/commands/up.ts

@@ -39,7 +39,7 @@ import {
 } from "../stack/schema.js";
 import { checkTrustPolicy } from "../stack/policy.js";
 import { parseEnvFile } from "../stack/env.js";
-import { resolveInstallEntry, parseSecretsMode } from "./install.js";
+import { resolveInstallEntry, parseSecretsMode, validateRemoteUrl } from "./install.js";
 import { extractRegistryMeta } from "../utils/format-trust.js";
 import { applyKeychainSecrets, type SecretsMode, setSecrets as _setSecrets } from "../store/keychain.js";
 
@@ -240,7 +240,10 @@ export async function handleUp(
   );
   if (options.secrets === "keychain" && totalSecretsStored > 0 && !options.dryRun) {
     deps.output(
-      "Secrets stored encrypted at rest in ~/.mcpm (protects against other-user/offline access, not same-user processes). " +
+      "Secrets stored encrypted at rest in ~/.mcpm. With an OS keychain this protects " +
+        "against other-user/offline access (not same-user processes); without one a " +
+        "machine-derived key is used that guards casual local inspection only, NOT file " +
+        "exfiltration — run `mcpm secrets migrate` once a keychain is available. " +
         "Run `mcpm guard enable` (then restart your IDE) so they resolve at launch."
     );
   }
@@ -423,6 +426,19 @@ async function processUrlServer(
     };
   }
 
+  // M4a: validate the URL before it is written to any client config. The up path
+  // previously wrote stack-file `url:` servers unvalidated; this rejects non-http(s)
+  // schemes and non-loopback plaintext http (interceptable once in an IDE config).
+  // Capture (don't throw): `--dry-run` must stay a read-only, exit-zero preview, and
+  // an invalid URL is categorized as `blocked` (mirroring trust-policy blocks) rather
+  // than crashing the per-server loop into a generic `failed`.
+  let urlError: string | undefined;
+  try {
+    validateRemoteUrl(url);
+  } catch (err) {
+    urlError = err instanceof Error ? err.message : String(err);
+  }
+
   const cursorClients = clients.filter((c) => c === "cursor");
 
   if (cursorClients.length === 0) {
@@ -434,7 +450,13 @@ async function processUrlServer(
   }
 
   if (options.dryRun) {
-    return { name, status: "skipped", message: `would install URL ${url} to Cursor` };
+    return urlError
+      ? { name, status: "skipped", message: `would reject URL ${url}: ${urlError}` }
+      : { name, status: "skipped", message: `would install URL ${url} to Cursor` };
+  }
+
+  if (urlError) {
+    return { name, status: "blocked", message: urlError };
   }
 
   for (const clientId of cursorClients) {