From f5866e642446c8194f9a7adae1743d684a7da624 Mon Sep 17 00:00:00 2001 From: SaulBuilds Date: Fri, 25 Sep 2026 09:02:14 -0700 Subject: [PATCH] hardening(docs): MCP pepper quality check, build guard and startup warning - mcpPepperStatus: the pepper must be at least 32 chars after trimming, have at least 8 distinct characters and no surrounding whitespace. A whitespace-only or low-variety pepper is refused. - The resolver logs once when MCP_API_KEYS is configured without a usable pepper (keys fail closed to public). - check:mcp-keys fails the build in the same case. - The weak-pepper assertions in check-mcp-keys.mjs now store the key under the weak pepper, so only the pepper rule can refuse it. - Docs: the pepper requirements, and that keys are minted only with the script. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_012cD3fDq5vhh2YWZPU2SV6H --- .env.example | 3 ++- README.md | 2 +- lib/auth/mcp-keys.ts | 43 ++++++++++++++++++++++++++++--- scripts/check-mcp-keys.mjs | 18 +++++++++++-- test/helpers/mcp-key.ts | 2 +- test/mcp-key-floor.test.ts | 53 ++++++++++++++++++++++++++++++++++---- 6 files changed, 108 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 98712318..d1b7cc60 100644 --- a/.env.example +++ b/.env.example @@ -29,7 +29,8 @@ OIDC_SCOPES=openid profile wallet kyc offline_access # It prints the key once (format cdk_<43 base64url chars>, 32 random bytes) and the JSON entry to merge: # { "": { "tier": "commercial", "sub": "org:acme", "expiresAt": 1790000000000 } } # Any other key format, an unset/short pepper, a malformed store, an unknown or expired key: "public". -# MCP_KEY_PEPPER: server-only secret, >= 32 chars (openssl rand -base64 48). Rotating it revokes every key. +# MCP_KEY_PEPPER: server-only secret, >= 32 chars, >= 8 distinct, no surrounding whitespace (openssl rand -base64 48). +# Rotating it revokes every key. A store without a usable pepper fails the build (check:mcp-keys). # MCP_KEY_PEPPER= # MCP_API_KEYS={} diff --git a/README.md b/README.md index 7187755c..b9fea970 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ Copy `.env.example` → `.env.local`. Key variables (full annotated list in `.en | `CITRATE_GATEWAY_API_KEY` | — | `cgk_` bearer key (server-only) | | `CITRATE_RPC_URL` | `https://rpc.citrate.ai` | chain 40204 RPC for the read-only sandboxes | | `MCP_API_KEYS` | `{}` (fail-closed → `public`) | HMAC-keyed key→tier map for the MCP toolbox; entries come from `node scripts/mint-mcp-key.mjs` | -| `MCP_KEY_PEPPER` | unset (no key resolves) | server-only HMAC pepper for MCP keys, ≥ 32 chars | +| `MCP_KEY_PEPPER` | unset (no key resolves) | server-only HMAC pepper for MCP keys: ≥ 32 chars, ≥ 8 distinct, no padding. A store without a usable pepper fails the build. Mint keys only with `scripts/mint-mcp-key.mjs` | | `DOCS_TRUST_VERCEL_FORWARDED` | unset | `1` on Vercel only: key the rate limiter on `x-vercel-forwarded-for` | `.env.example` documents the fail-closed defaults for auth, the MCP key hashing, and the diff --git a/lib/auth/mcp-keys.ts b/lib/auth/mcp-keys.ts index 668a6ffd..54d31a0f 100644 --- a/lib/auth/mcp-keys.ts +++ b/lib/auth/mcp-keys.ts @@ -44,8 +44,41 @@ interface RawEntry { */ export const MCP_KEY_RE = /^cdk_[A-Za-z0-9_-]{43}$/; -/** Minimum length of the server-side pepper (`MCP_KEY_PEPPER`). Shorter or unset: no key resolves. */ +/** Minimum length of the server-side pepper (`MCP_KEY_PEPPER`), after trimming. */ export const MIN_MCP_KEY_PEPPER_LENGTH = 32; +/** Minimum distinct characters in the pepper (rejects "aaaa…" / whitespace padding). */ +export const MIN_MCP_KEY_PEPPER_DISTINCT = 8; + +export type McpPepperStatus = "ok" | "missing" | "weak"; + +/** Whether `MCP_KEY_PEPPER` is usable: set, >= 32 chars after trimming, >= 8 distinct characters. */ +export function mcpPepperStatus(env: NodeJS.ProcessEnv = process.env): McpPepperStatus { + const raw = env.MCP_KEY_PEPPER ?? ""; + if (!raw.trim()) return "missing"; + const p = raw.trim(); + if (p !== raw || p.length < MIN_MCP_KEY_PEPPER_LENGTH || new Set(p).size < MIN_MCP_KEY_PEPPER_DISTINCT) return "weak"; + return "ok"; +} + +/** True when MCP_API_KEYS holds at least one entry. */ +export function mcpStoreConfigured(env: NodeJS.ProcessEnv = process.env): boolean { + return Object.keys(keyStore(env)).length > 0; +} + +let warnedPepper = false; +/** + * A key store without a usable pepper silently resolves every partner key to "public" (fail closed). + * Say so once in the server log so the misconfiguration is diagnosable; the build check + * (check:mcp-keys) fails the deploy for the same condition. + */ +function warnPepperOnce(status: McpPepperStatus): void { + if (warnedPepper) return; + warnedPepper = true; + console.error( + `[mcp-keys] MCP_API_KEYS is configured but MCP_KEY_PEPPER is ${status}: every MCP key resolves to public. ` + + "Set a random MCP_KEY_PEPPER (>= 32 chars) and re-mint keys with scripts/mint-mcp-key.mjs.", + ); +} /** * The store index for a presented key: HMAC-SHA256(key) under the server-only `MCP_KEY_PEPPER`. @@ -86,8 +119,12 @@ export function resolveMcpKeyCap( ): McpKeyCap { const key = (presentedKey ?? "").trim(); if (key.length < MIN_MCP_KEY_LENGTH || !MCP_KEY_RE.test(key)) return publicCap(); - const pepper = env.MCP_KEY_PEPPER ?? ""; - if (pepper.length < MIN_MCP_KEY_PEPPER_LENGTH) return publicCap(); // no pepper → nothing resolves (fail closed) + const status = mcpPepperStatus(env); + if (status !== "ok") { + if (mcpStoreConfigured(env)) warnPepperOnce(status); + return publicCap(); // no usable pepper → nothing resolves (fail closed) + } + const pepper = env.MCP_KEY_PEPPER as string; const entry = keyStore(env)[mcpKeyDigest(key, pepper)]; if (!entry) return publicCap(); diff --git a/scripts/check-mcp-keys.mjs b/scripts/check-mcp-keys.mjs index 1bb1998c..02e2a3a9 100644 --- a/scripts/check-mcp-keys.mjs +++ b/scripts/check-mcp-keys.mjs @@ -81,7 +81,7 @@ console.log('[check:mcp-keys] ✓ with no MCP_API_KEYS store, every key (incl. t // ── Guard 2b: an HMAC-keyed store entry grants its tier; expiry is honoured; unknown tier never escalates; // unminted formats, a missing pepper and a bare-SHA-256 (pre-R2) store never resolve. ── const { mintMcpKey } = await import(path.join(ROOT, "scripts/mint-mcp-key.mjs")); -const pepper = "p".repeat(40); // test-only +const pepper = "test-only-pepper-0123456789-abcdefghij"; // test-only, >= 8 distinct chars const good = mintMcpKey({ pepper, tier: "academic", sub: "org:academic_partner", expiresAt: now + 30 * 86_400_000 }); const expired = mintMcpKey({ pepper, tier: "academic", sub: "org:stale", expiresAt: now - 1 }); const badTier = { key: "cdk_" + "b".repeat(43) }; @@ -101,7 +101,21 @@ assert.equal(resolveMcpKeyCap(badTier.key, now, env).tier, "public", "unknown ti assert.equal(resolveMcpKeyCap("cdk_" + "u".repeat(43), now, env).tier, "public", "unknown key must resolve to public"); assert.equal(resolveMcpKeyCap(humanKey, now, env).tier, "public", "a non-minted key format must never resolve, even if stored"); assert.equal(resolveMcpKeyCap(good.key, now, { MCP_API_KEYS: env.MCP_API_KEYS }).tier, "public", "no pepper: nothing resolves"); -assert.equal(resolveMcpKeyCap(good.key, now, { ...env, MCP_KEY_PEPPER: "short" }).tier, "public", "short pepper: nothing resolves"); +// Weak peppers: the store is keyed UNDER the weak pepper, so only the pepper rule can refuse it. +for (const [weak, why] of [["short-pepper-0123", "short"], [" ".repeat(40), "whitespace-only"], ["a".repeat(40), "low-variety"], [` ${pepper}`, "padded"]]) { + const k = mintMcpKey({ pepper: "x".repeat(8) + "0123456789abcdefghijklmnop", tier: "academic", sub: "org:w" }).key; + const weakEnv = { MCP_KEY_PEPPER: weak, MCP_API_KEYS: JSON.stringify({ [createHmac("sha256", weak).update(k).digest("hex")]: { tier: "academic", sub: "org:w" } }) }; + assert.equal(resolveMcpKeyCap(k, now, weakEnv).tier, "public", `${why} pepper: nothing resolves`); +} assert.equal(resolveMcpKeyCap(good.key, now, { MCP_API_KEYS: JSON.stringify({ [sha256Hex(good.key)]: { tier: "academic" } }), MCP_KEY_PEPPER: pepper }).tier, "public", "a bare-SHA-256 (pre-R2) store entry must not resolve"); assert.equal(resolveMcpKeyCap(good.key, now, { ...env, MCP_API_KEYS: "{ not json" }).tier, "public", "malformed store must fail closed"); +// ── Guard 3: THIS build's deployment env. A configured store without a usable pepper would silently +// downgrade every partner key to public; fail the build instead. ── +{ + const { mcpPepperStatus, mcpStoreConfigured } = await loadResolver(); + if (mcpStoreConfigured(process.env) && mcpPepperStatus(process.env) !== "ok") { + console.error(`[check:mcp-keys] ✗ MCP_API_KEYS is set but MCP_KEY_PEPPER is ${mcpPepperStatus(process.env)} (need >= 32 chars, >= 8 distinct, no padding).`); + process.exit(1); + } +} console.log("[check:mcp-keys] ✓ shipped resolver: HMAC-keyed minted grant honoured, format/pepper enforced, expiry honoured, unknown tier/key/malformed-store all fail closed to public (DOC-B-003)."); diff --git a/test/helpers/mcp-key.ts b/test/helpers/mcp-key.ts index d1482f30..913066aa 100644 --- a/test/helpers/mcp-key.ts +++ b/test/helpers/mcp-key.ts @@ -1,7 +1,7 @@ /** Test-only MCP key store builder matching lib/auth/mcp-keys.ts (HMAC under MCP_KEY_PEPPER). */ import { createHmac } from "node:crypto"; -export const TEST_PEPPER = "t".repeat(40); // test-only, not a secret +export const TEST_PEPPER = "test-pepper-0123456789-abcdefghijklmn"; // test-only, not a secret /** A syntactically valid minted-format key (not random: tests only). */ export const testKey = (c: string) => `cdk_${c.repeat(43).slice(0, 43)}`; export const digest = (key: string, pepper = TEST_PEPPER) => createHmac("sha256", pepper).update(key).digest("hex"); diff --git a/test/mcp-key-floor.test.ts b/test/mcp-key-floor.test.ts index 7a3f31dc..502fda1c 100644 --- a/test/mcp-key-floor.test.ts +++ b/test/mcp-key-floor.test.ts @@ -4,7 +4,8 @@ */ import { describe, it, expect } from "vitest"; import { createHash } from "node:crypto"; -import { resolveMcpKeyCap, MIN_MCP_KEY_LENGTH, MCP_KEY_RE, MIN_MCP_KEY_PEPPER_LENGTH } from "@/lib/auth/mcp-keys"; +import { resolveMcpKeyCap, MIN_MCP_KEY_LENGTH, MCP_KEY_RE, MIN_MCP_KEY_PEPPER_LENGTH, mcpPepperStatus, mcpStoreConfigured } from "@/lib/auth/mcp-keys"; +import { vi } from "vitest"; // @ts-expect-error -- plain ESM script import { mintMcpKey } from "@/scripts/mint-mcp-key.mjs"; import { TEST_PEPPER, digest, storeEnv, testKey } from "./helpers/mcp-key"; @@ -30,16 +31,16 @@ describe("MCP key format, pepper and HMAC", () => { const env = { MCP_KEY_PEPPER: TEST_PEPPER, MCP_API_KEYS: JSON.stringify(entry) }; expect(resolveMcpKeyCap(key, NOW, env as never)).toEqual({ tier: "academic", sub: "org:m", expiresAt: null }); expect(resolveMcpKeyCap(key, NOW, { MCP_API_KEYS: env.MCP_API_KEYS } as never).tier).toBe("public"); - expect(resolveMcpKeyCap(key, NOW, { ...env, MCP_KEY_PEPPER: "x".repeat(MIN_MCP_KEY_PEPPER_LENGTH - 1) } as never).tier).toBe("public"); - expect(resolveMcpKeyCap(key, NOW, { ...env, MCP_KEY_PEPPER: "y".repeat(40) } as never).tier).toBe("public"); + expect(resolveMcpKeyCap(key, NOW, { ...env, MCP_KEY_PEPPER: TEST_PEPPER.slice(0, MIN_MCP_KEY_PEPPER_LENGTH - 1) } as never).tier).toBe("public"); + expect(resolveMcpKeyCap(key, NOW, { ...env, MCP_KEY_PEPPER: "another-pepper-9876543210-zyxwvutsrq" } as never).tier).toBe("public"); // a pepper of exactly the minimum length works - const p32 = "z".repeat(MIN_MCP_KEY_PEPPER_LENGTH); + const p32 = "0123456789abcdefghijklmnopqrstuv"; // exactly 32, >= 8 distinct expect(resolveMcpKeyCap(key, NOW, { MCP_KEY_PEPPER: p32, MCP_API_KEYS: JSON.stringify({ [digest(key, p32)]: { tier: "commercial", sub: "s" } }) } as never).tier).toBe("commercial"); }); it("an empty or short pepper disables resolution even when the store is keyed under that pepper", () => { const k = testKey("r"); - const short = "s".repeat(MIN_MCP_KEY_PEPPER_LENGTH - 1); + const short = TEST_PEPPER.slice(0, MIN_MCP_KEY_PEPPER_LENGTH - 1); const under = (p: string) => JSON.stringify({ [digest(k, p)]: { tier: "academic", sub: "r" } }); expect(resolveMcpKeyCap(k, NOW, { MCP_KEY_PEPPER: short, MCP_API_KEYS: under(short) } as never).tier).toBe("public"); expect(resolveMcpKeyCap(k, NOW, { MCP_KEY_PEPPER: "", MCP_API_KEYS: under("") } as never).tier).toBe("public"); @@ -68,3 +69,45 @@ describe("MCP key format, pepper and HMAC", () => { expect(Object.keys(a.entry)[0]).toBe(digest(a.key)); }); }); + +describe("pepper quality and diagnosability (verify2)", () => { + it("mcpPepperStatus: missing / weak / ok", () => { + expect(mcpPepperStatus({} as never)).toBe("missing"); + expect(mcpPepperStatus({ MCP_KEY_PEPPER: " ".repeat(40) } as never)).toBe("missing"); + expect(mcpPepperStatus({ MCP_KEY_PEPPER: "a".repeat(40) } as never)).toBe("weak"); + expect(mcpPepperStatus({ MCP_KEY_PEPPER: "abcdefg".repeat(6) } as never)).toBe("weak"); // 7 distinct + expect(mcpPepperStatus({ MCP_KEY_PEPPER: "abcdefgh".repeat(4) } as never)).toBe("ok"); // 32, 8 distinct + expect(mcpPepperStatus({ MCP_KEY_PEPPER: ` ${TEST_PEPPER}` } as never)).toBe("weak"); // padded + expect(mcpPepperStatus({ MCP_KEY_PEPPER: TEST_PEPPER.slice(0, 31) } as never)).toBe("weak"); + expect(mcpPepperStatus({ MCP_KEY_PEPPER: TEST_PEPPER } as never)).toBe("ok"); + }); + + it("a whitespace-only pepper refuses a key stored under it", () => { + const k = testKey("v"); + const ws = " ".repeat(40); + expect(resolveMcpKeyCap(k, NOW, { MCP_KEY_PEPPER: ws, MCP_API_KEYS: JSON.stringify({ [digest(k, ws)]: { tier: "academic" } }) } as never).tier).toBe("public"); + }); + + it("warns once in the server log when a store is configured without a usable pepper", async () => { + vi.resetModules(); + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + const mod = await import("@/lib/auth/mcp-keys"); + const env = { MCP_API_KEYS: JSON.stringify({ abc: { tier: "academic" } }) } as never; + expect(mod.mcpStoreConfigured(env)).toBe(true); + mod.resolveMcpKeyCap(testKey("a"), NOW, env); + mod.resolveMcpKeyCap(testKey("b"), NOW, env); + expect(spy).toHaveBeenCalledTimes(1); + expect(String(spy.mock.calls[0][0])).toMatch(/MCP_KEY_PEPPER is missing/); + // no store: silent + spy.mockClear(); + vi.resetModules(); + const mod2 = await import("@/lib/auth/mcp-keys"); + mod2.resolveMcpKeyCap(testKey("a"), NOW, {} as never); + expect(spy).not.toHaveBeenCalled(); + expect(mcpStoreConfigured({} as never)).toBe(false); + } finally { + spy.mockRestore(); + } + }); +});