refactor(local-mcp): share transport + IPv6 parsing across the feature

Address the review-swarm reuse findings.

- Extract `parseClaudeJsonTransport` in mcp-config.ts so the relay's SDK transport builder and the host-agnostic `LocalMcpTransport` descriptor read a raw ~/.claude.json entry through one normalizer (carrying the stdio `env` the relay needs but the descriptor drops), instead of duplicating the branching and drifting on legacy bare-url/type-less entries.
- Move the IPv6-literal private-address kernel into `@posthog/shared` (`isPrivateIpv6Literal`), shared by `isPrivateHostname` and web-fetch's `isBlockedHost`. This also fixes a latent gap: `isPrivateHostname` only decoded the dotted `::ffff:1.2.3.4` form, so an IPv4-mapped loopback in the hex-group form `URL#hostname` normalizes to (`::ffff:7f00:1`) was misclassified as importable.
- Add unit tests for the shared IPv6 kernel (incl. the hex-group mapped case), the `mcp_response` exactly-one-of-payload/error refine, and `relayMcpServerNamesSchema`.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301

Author: richardsolomou

packages/agent/src/adapters/claude/session/mcp-config.ts

@@ -95,33 +95,70 @@ export function sanitizeHeaders(
   return entries.length > 0 ? Object.fromEntries(entries) : undefined;
 }
 
-function toTransport(config: McpServerConfig): LocalMcpTransport {
+/**
+ * A raw ~/.claude.json entry parsed into a normalized transport, including the
+ * stdio `env` that the relay executor needs to spawn the process (the
+ * host-agnostic {@link LocalMcpTransport} deliberately drops it as secrets).
+ * Both the descriptor normalizer ({@link toTransport}) and the relay's SDK
+ * transport builder read entries through here, so they can't drift on how a
+ * legacy bare-`url` or type-less entry is interpreted.
+ */
+export type ParsedClaudeJsonTransport =
+  | { kind: "http" | "sse"; url: string; headers?: Record<string, string> }
+  | {
+      kind: "stdio";
+      command: string;
+      args?: string[];
+      env?: Record<string, string>;
+    }
+  | { kind: "unknown" };
+
+export function parseClaudeJsonTransport(
+  config: McpServerConfig,
+): ParsedClaudeJsonTransport {
   // ~/.claude.json is hand-editable, so treat the parsed config as untyped.
   const raw = config as Record<string, unknown>;
   const type = typeof raw.type === "string" ? raw.type : undefined;
+  const url = typeof raw.url === "string" ? raw.url : undefined;
+  const command = typeof raw.command === "string" ? raw.command : undefined;
 
-  if ((type === "http" || type === "sse") && typeof raw.url === "string") {
-    return { type, url: raw.url, headers: sanitizeHeaders(raw.headers) };
+  if ((type === "http" || type === "sse") && url) {
+    return { kind: type, url, headers: sanitizeHeaders(raw.headers) };
   }
-  if (
-    (type === undefined || type === "stdio") &&
-    typeof raw.command === "string"
-  ) {
+  if ((type === undefined || type === "stdio") && command) {
     const args = Array.isArray(raw.args)
       ? raw.args.filter((arg): arg is string => typeof arg === "string")
       : undefined;
-    return { type: "stdio", command: raw.command, args };
+    return { kind: "stdio", command, args, env: sanitizeHeaders(raw.env) };
   }
   // Legacy entries can carry a bare `url` with no `type`; streamable HTTP is
   // the current default transport, so read them as http.
-  if (type === undefined && typeof raw.url === "string") {
-    return {
-      type: "http",
-      url: raw.url,
-      headers: sanitizeHeaders(raw.headers),
-    };
+  if (type === undefined && url) {
+    return { kind: "http", url, headers: sanitizeHeaders(raw.headers) };
+  }
+  return { kind: "unknown" };
+}
+
+function toTransport(config: McpServerConfig): LocalMcpTransport {
+  const transport = parseClaudeJsonTransport(config);
+  switch (transport.kind) {
+    case "http":
+    case "sse":
+      return {
+        type: transport.kind,
+        url: transport.url,
+        headers: transport.headers,
+      };
+    case "stdio":
+      // Drop `env` — the descriptor shape excludes stdio secrets.
+      return {
+        type: "stdio",
+        command: transport.command,
+        args: transport.args,
+      };
+    default:
+      return { type: "unknown" };
   }
-  return { type: "unknown" };
 }
 
 /**

packages/agent/src/server/schemas.test.ts

@@ -1,5 +1,9 @@
 import { describe, expect, it } from "vitest";
-import { mcpServersSchema, validateCommandParams } from "./schemas";
+import {
+  mcpServersSchema,
+  relayMcpServerNamesSchema,
+  validateCommandParams,
+} from "./schemas";
 
 describe("mcpServersSchema", () => {
   it("accepts a valid HTTP server", () => {
@@ -234,4 +238,85 @@ describe("validateCommandParams", () => {
 
     expect(result.success).toBe(false);
   });
+
+  it("accepts an mcp_response carrying only a payload", () => {
+    const result = validateCommandParams("_posthog/mcp_response", {
+      requestId: "req-1",
+      server: "slack",
+      payload: { jsonrpc: "2.0", id: 1, result: {} },
+    });
+
+    expect(result.success).toBe(true);
+  });
+
+  it("accepts an mcp_response carrying only an error", () => {
+    const result = validateCommandParams("mcp_response", {
+      requestId: "req-1",
+      server: "slack",
+      error: { code: -32000, message: "boom" },
+    });
+
+    expect(result.success).toBe(true);
+  });
+
+  it("rejects an mcp_response with both payload and error", () => {
+    const result = validateCommandParams("mcp_response", {
+      requestId: "req-1",
+      server: "slack",
+      payload: { ok: true },
+      error: { code: -32000, message: "boom" },
+    });
+
+    expect(result.success).toBe(false);
+  });
+
+  it("rejects an mcp_response with neither payload nor error", () => {
+    const result = validateCommandParams("mcp_response", {
+      requestId: "req-1",
+      server: "slack",
+    });
+
+    expect(result.success).toBe(false);
+  });
+
+  it("rejects an mcp_response missing requestId or server", () => {
+    expect(
+      validateCommandParams("mcp_response", {
+        server: "slack",
+        payload: { ok: true },
+      }).success,
+    ).toBe(false);
+    expect(
+      validateCommandParams("mcp_response", {
+        requestId: "req-1",
+        payload: { ok: true },
+      }).success,
+    ).toBe(false);
+  });
+});
+
+describe("relayMcpServerNamesSchema", () => {
+  it("accepts a list of names and an empty list", () => {
+    expect(
+      relayMcpServerNamesSchema.safeParse(["slack", "linear"]).success,
+    ).toBe(true);
+    expect(relayMcpServerNamesSchema.safeParse([]).success).toBe(true);
+  });
+
+  it("rejects an empty-string name", () => {
+    expect(relayMcpServerNamesSchema.safeParse(["slack", ""]).success).toBe(
+      false,
+    );
+  });
+
+  it("rejects a name longer than 64 chars", () => {
+    expect(relayMcpServerNamesSchema.safeParse(["a".repeat(65)]).success).toBe(
+      false,
+    );
+  });
+
+  it("rejects more than 20 names", () => {
+    const names = Array.from({ length: 21 }, (_, i) => `s${i}`);
+    expect(relayMcpServerNamesSchema.safeParse(names).success).toBe(false);
+  });
 });

packages/core/src/local-mcp/localMcpImport.ts

@@ -2,7 +2,7 @@ import type {
   CloudMcpServerImport,
   LocalMcpServerDescriptor,
 } from "@posthog/shared";
-import { isPrivateIpv4Octets } from "@posthog/shared";
+import { isPrivateIpv4Octets, isPrivateIpv6Literal } from "@posthog/shared";
 import { inject, injectable } from "inversify";
 import { LOCAL_MCP_WORKSPACE_CLIENT } from "./identifiers";
 
@@ -73,17 +73,9 @@ export function isPrivateHostname(hostname: string): boolean {
   if (host.startsWith("[") && host.endsWith("]")) host = host.slice(1, -1);
   if (host === "" || host === "localhost") return true;
 
-  if (host.includes(":")) {
-    // IPv6
-    if (host === "::" || host === "::1") return true;
-    const v4Mapped = host.match(/^::ffff:(.+)$/)?.[1];
-    if (v4Mapped) {
-      const octets = parseIpv4(v4Mapped);
-      return octets ? isPrivateIpv4Octets(octets[0], octets[1]) : false;
-    }
-    // fc00::/7 unique-local, fe80::/10 link-local
-    return /^(f[cd]|fe[89ab])/.test(host);
-  }
+  // IPv6 literal: the shared kernel covers loopback/unspecified, link-local,
+  // unique-local, and IPv4-mapped (incl. the hex-group form URL normalizes to).
+  if (host.includes(":")) return isPrivateIpv6Literal(host);
 
   const octets = parseIpv4(host);
   if (octets) return isPrivateIpv4Octets(octets[0], octets[1]);

packages/harness/src/extensions/web-access/web-fetch.ts

@@ -1,6 +1,6 @@
 import { isIP } from "node:net";
 import { defineTool } from "@earendil-works/pi-coding-agent";
-import { isPrivateIpv4Octets } from "@posthog/shared";
+import { isPrivateIpv4Octets, isPrivateIpv6Literal } from "@posthog/shared";
 import { LRUCache } from "lru-cache";
 import TurndownService from "turndown";
 import { Type } from "typebox";
@@ -31,28 +31,6 @@ function ipv4Octets(hostname: string): number[] | undefined {
   return octets.length === 4 ? octets : undefined;
 }
 
-function hexGroupToBytes(group: string): [number, number] {
-  const value = Number.parseInt(group, 16);
-  return [(value >> 8) & 0xff, value & 0xff];
-}
-
-/**
- * `URL#hostname` normalizes an IPv4-mapped IPv6 literal's embedded address
- * into hex groups (`::ffff:127.0.0.1` becomes `::ffff:7f00:1`), so recovering
- * the embedded IPv4 address means decoding those two hex groups back into
- * bytes rather than pattern-matching a dotted-quad that will never appear.
- */
-function ipv4MappedAddress(normalized: string): string | undefined {
-  const hexMatch = normalized.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
-  if (hexMatch) {
-    const [b0, b1] = hexGroupToBytes(hexMatch[1]);
-    const [b2, b3] = hexGroupToBytes(hexMatch[2]);
-    return `${b0}.${b1}.${b2}.${b3}`;
-  }
-  const dottedMatch = normalized.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/);
-  return dottedMatch?.[1];
-}
-
 /**
  * Blocks loopback, private, link-local, and other non-public IPv4/IPv6
  * literal addresses — the classic SSRF-to-internal-services vector (e.g. the
@@ -61,7 +39,9 @@ function ipv4MappedAddress(normalized: string): string | undefined {
  * does not resolve hostnames, so it does not protect against DNS rebinding
  * (a public hostname whose DNS record later resolves to a private address).
  * That would require enforcing the check at connection time, not URL-parse
- * time — out of scope here, but worth remembering as a residual gap.
+ * time — out of scope here, but worth remembering as a residual gap. The
+ * IPv4-range and IPv6-literal kernels are shared with the other private-host
+ * classifiers via `@posthog/shared`.
  */
 function isBlockedHost(rawHostname: string): boolean {
   // `URL#hostname` keeps IPv6 literals bracketed ("[::1]"); `net.isIP`/our
@@ -73,15 +53,7 @@ function isBlockedHost(rawHostname: string): boolean {
   const v4 = ipv4Octets(hostname);
   if (v4) return isPrivateIpv4Octets(v4[0], v4[1]);
 
-  if (isIP(hostname) === 6) {
-    if (lower === "::1") return true; // loopback
-    if (lower === "::" || lower === "::0") return true; // unspecified
-    if (lower.startsWith("fe80:")) return true; // link-local (fe80::/10)
-    if (/^f[cd][0-9a-f]{2}:/.test(lower)) return true; // unique local (fc00::/7)
-    const mapped = ipv4MappedAddress(lower);
-    if (mapped && isBlockedHost(mapped)) return true;
-    return false;
-  }
+  if (isIP(hostname) === 6) return isPrivateIpv6Literal(lower);
 
   return false;
 }

packages/shared/src/index.ts

@@ -165,14 +165,17 @@ export {
   pathToFileUri,
   toRelativePath,
 } from "./path";
-export { isPrivateIpv4Octets } from "./private-network";
 export {
   buildPrOutput,
   mergePrUrls,
   promotePrUrl,
   readPrSummaries,
   readPrUrls,
 } from "./pr-urls";
+export {
+  isPrivateIpv4Octets,
+  isPrivateIpv6Literal,
+} from "./private-network";
 export {
   type CloudRegion,
   formatRegionBadge,

packages/shared/src/private-network.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { isPrivateIpv4Octets } from "./private-network";
+import { isPrivateIpv4Octets, isPrivateIpv6Literal } from "./private-network";
 
 describe("isPrivateIpv4Octets", () => {
   it.each([
@@ -31,3 +31,30 @@ describe("isPrivateIpv4Octets", () => {
     expect(isPrivateIpv4Octets(a, b)).toBe(false);
   });
 });
+
+describe("isPrivateIpv6Literal", () => {
+  it.each([
+    ["::1", "loopback"],
+    ["::", "unspecified"],
+    ["::0", "unspecified"],
+    ["fe80::1", "link-local"],
+    ["feb0::1", "link-local upper bound"],
+    ["fc00::1", "unique-local"],
+    ["fd12:3456::1", "unique-local"],
+    // URL#hostname normalizes ::ffff:127.0.0.1 to the hex-group form.
+    ["::ffff:7f00:1", "IPv4-mapped loopback (hex-group form)"],
+    ["::ffff:127.0.0.1", "IPv4-mapped loopback (dotted form)"],
+    ["::ffff:c0a8:1", "IPv4-mapped 192.168.0.1 (hex-group form)"],
+  ])("treats %s (%s) as private", (host) => {
+    expect(isPrivateIpv6Literal(host)).toBe(true);
+  });
+
+  it.each([
+    ["2001:db8::1", "documentation range"],
+    ["2606:4700::1", "public (Cloudflare)"],
+    ["::ffff:8.8.8.8", "IPv4-mapped public DNS (dotted form)"],
+    ["::ffff:808:808", "IPv4-mapped 8.8.8.8 (hex-group form)"],
+  ])("treats %s (%s) as public", (host) => {
+    expect(isPrivateIpv6Literal(host)).toBe(false);
+  });
+});

packages/shared/src/private-network.ts

@@ -16,3 +16,47 @@ export function isPrivateIpv4Octets(a: number, b: number): boolean {
   if (a === 192 && b === 168) return true;
   return a === 198 && (b === 18 || b === 19);
 }
+
+/**
+ * The IPv4 address embedded in an IPv4-mapped IPv6 literal (`::ffff:…`), or
+ * undefined for any other host. `URL#hostname` normalizes the embedded address
+ * into hex groups (`::ffff:127.0.0.1` becomes `::ffff:7f00:1`), so both the
+ * hex-group form and the raw dotted form are decoded.
+ */
+function ipv4MappedOctets(
+  host: string,
+): [number, number, number, number] | undefined {
+  const hex = host.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
+  if (hex) {
+    const hi = Number.parseInt(hex[1], 16);
+    const lo = Number.parseInt(hex[2], 16);
+    return [(hi >> 8) & 0xff, hi & 0xff, (lo >> 8) & 0xff, lo & 0xff];
+  }
+  const dotted = host.match(
+    /^::ffff:(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/,
+  );
+  if (dotted) {
+    const octets = dotted.slice(1).map(Number);
+    if (octets.some((octet) => octet > 255)) return undefined;
+    return octets as [number, number, number, number];
+  }
+  return undefined;
+}
+
+/**
+ * Whether a bracket-free, lowercased IPv6 literal is non-public: loopback
+ * (`::1`), unspecified (`::`/`::0`), link-local (fe80::/10), unique-local
+ * (fc00::/7), or an IPv4-mapped address whose embedded IPv4 is private. Shared
+ * by `@posthog/core`'s `isPrivateHostname` and the web-fetch tool's
+ * `isBlockedHost` so the IPv6-literal kernel can't drift between them; each
+ * still layers its own non-literal rules (bare intranet names, `.local`
+ * suffixes) on top.
+ */
+export function isPrivateIpv6Literal(host: string): boolean {
+  if (host === "::1") return true;
+  if (host === "::" || host === "::0") return true;
+  if (/^fe[89ab]/.test(host)) return true; // link-local fe80::/10
+  if (/^f[cd]/.test(host)) return true; // unique-local fc00::/7
+  const octets = ipv4MappedOctets(host);
+  return octets ? isPrivateIpv4Octets(octets[0], octets[1]) : false;
+}