security: encrypt MCP connector secrets with per-record derived keys

backend/src/lib/mcp/__tests__/crypto.test.ts

@@ -0,0 +1,65 @@
+import crypto from "crypto";
+import { beforeAll, describe, expect, it } from "vitest";
+
+// The MCP secret crypto reads its master secret from the environment lazily
+// (per call), so setting it before importing the module under test is enough.
+const SECRET = "test-mcp-master-secret-at-least-32-chars-long";
+
+let encryptString: typeof import("../client").encryptString;
+let decryptString: typeof import("../client").decryptString;
+
+beforeAll(async () => {
+    process.env.MCP_CONNECTORS_ENCRYPTION_SECRET = SECRET;
+    const mod = await import("../client");
+    encryptString = mod.encryptString;
+    decryptString = mod.decryptString;
+});
+
+// Reproduce the pre-HKDF format: one static-salt scrypt key for every secret,
+// ciphertext stored as bare base64 with no version prefix.
+function legacyEncrypt(value: string) {
+    const key = crypto.scryptSync(SECRET, "mike-user-mcp-v1", 32);
+    const iv = crypto.randomBytes(12);
+    const cipher = crypto.createCipheriv("aes-256-gcm", key, iv);
+    const encrypted = Buffer.concat([cipher.update(value, "utf8"), cipher.final()]);
+    return {
+        encrypted: encrypted.toString("base64"),
+        iv: iv.toString("base64"),
+        tag: cipher.getAuthTag().toString("base64"),
+    };
+}
+
+describe("mcp secret crypto", () => {
+    it("round-trips a value through the versioned per-row scheme", () => {
+        const secret = "sk-connector-abc123";
+        const row = encryptString(secret);
+        expect(row.encrypted.startsWith("v2.")).toBe(true);
+        expect(decryptString(row.encrypted, row.iv, row.tag)).toBe(secret);
+    });
+
+    it("derives a fresh salt per encryption (no shared key across rows)", () => {
+        const a = encryptString("same-value");
+        const b = encryptString("same-value");
+        // Different salt (packed in `encrypted`) and IV → different ciphertext,
+        // yet both decrypt back to the same plaintext.
+        expect(a.encrypted).not.toBe(b.encrypted);
+        expect(decryptString(a.encrypted, a.iv, a.tag)).toBe("same-value");
+        expect(decryptString(b.encrypted, b.iv, b.tag)).toBe("same-value");
+    });
+
+    it("still decrypts legacy static-salt ciphertext (no v2. prefix)", () => {
+        const legacy = legacyEncrypt("legacy-token");
+        expect(legacy.encrypted.startsWith("v2.")).toBe(false);
+        expect(decryptString(legacy.encrypted, legacy.iv, legacy.tag)).toBe(
+            "legacy-token",
+        );
+    });
+
+    it("fails closed when the packed salt/ciphertext is tampered", () => {
+        const row = encryptString("tamper-me");
+        const raw = Buffer.from(row.encrypted.slice("v2.".length), "base64");
+        raw[0] ^= 0xff; // flip a salt byte → wrong derived key → GCM auth fails
+        const tampered = "v2." + raw.toString("base64");
+        expect(decryptString(tampered, row.iv, row.tag)).toBeNull();
+    });
+});

backend/src/lib/mcp/client.ts

@@ -27,10 +27,50 @@ function encryptionSecret(): string {
     return secret;
 }
 
-function encryptionKey(): Buffer {
+// Legacy path: one scrypt-derived key for every connector secret (static salt).
+// Kept only to decrypt rows written before per-row HKDF was introduced.
+function legacyEncryptionKey(): Buffer {
     return crypto.scryptSync(encryptionSecret(), "mike-user-mcp-v1", 32);
 }
 
+// New path: HKDF (RFC 5869) derives a unique 256-bit key per secret from a random
+// 16-byte salt. One key's compromise no longer exposes every other connector
+// secret.
+function deriveKey(salt: Buffer): Buffer {
+    return Buffer.from(
+        crypto.hkdfSync(
+            "sha256",
+            Buffer.from(encryptionSecret(), "utf8"),
+            salt,
+            Buffer.from("mike-user-mcp-v2", "utf8"),
+            32,
+        ),
+    );
+}
+
+// The salt has to travel with the ciphertext, but the connector tables have no
+// salt column. Rather than a migration across four encrypted fields, pack it
+// into the stored value: `v2.` + base64(salt(16) || ciphertext). A wrong/forged
+// salt derives a wrong key, so GCM auth fails closed on decrypt. Legacy rows
+// have no `v2.` prefix and decrypt with the static key.
+const V2_PREFIX = "v2.";
+
+function packCiphertext(salt: Buffer, ciphertext: Buffer): string {
+    return V2_PREFIX + Buffer.concat([salt, ciphertext]).toString("base64");
+}
+
+// Resolve stored ciphertext to the key that decrypts it and the raw bytes.
+function unpackCiphertext(stored: string): { key: Buffer; data: Buffer } {
+    if (stored.startsWith(V2_PREFIX)) {
+        const buf = Buffer.from(stored.slice(V2_PREFIX.length), "base64");
+        return {
+            key: deriveKey(buf.subarray(0, 16)),
+            data: buf.subarray(16),
+        };
+    }
+    return { key: legacyEncryptionKey(), data: Buffer.from(stored, "base64") };
+}
+
 export function mcpOAuthCallbackUrl() {
     const base = (
         process.env.API_PUBLIC_URL ||
@@ -45,14 +85,15 @@ function encryptJson(value: Record<string, unknown>): {
     auth_config_iv: string;
     auth_config_tag: string;
 } {
+    const salt = crypto.randomBytes(16);
     const iv = crypto.randomBytes(12);
-    const cipher = crypto.createCipheriv("aes-256-gcm", encryptionKey(), iv);
+    const cipher = crypto.createCipheriv("aes-256-gcm", deriveKey(salt), iv);
     const encrypted = Buffer.concat([
         cipher.update(JSON.stringify(value), "utf8"),
         cipher.final(),
     ]);
     return {
-        encrypted_auth_config: encrypted.toString("base64"),
+        encrypted_auth_config: packCiphertext(salt, encrypted),
         auth_config_iv: iv.toString("base64"),
         auth_config_tag: cipher.getAuthTag().toString("base64"),
     };
@@ -63,14 +104,15 @@ export function encryptString(value: string): {
     iv: string;
     tag: string;
 } {
+    const salt = crypto.randomBytes(16);
     const iv = crypto.randomBytes(12);
-    const cipher = crypto.createCipheriv("aes-256-gcm", encryptionKey(), iv);
+    const cipher = crypto.createCipheriv("aes-256-gcm", deriveKey(salt), iv);
     const encrypted = Buffer.concat([
         cipher.update(value, "utf8"),
         cipher.final(),
     ]);
     return {
-        encrypted: encrypted.toString("base64"),
+        encrypted: packCiphertext(salt, encrypted),
         iv: iv.toString("base64"),
         tag: cipher.getAuthTag().toString("base64"),
     };
@@ -83,14 +125,15 @@ export function decryptString(
 ): string | null {
     if (!encrypted || !iv || !tag) return null;
     try {
+        const { key, data } = unpackCiphertext(encrypted);
         const decipher = crypto.createDecipheriv(
             "aes-256-gcm",
-            encryptionKey(),
+            key,
             Buffer.from(iv, "base64"),
         );
         decipher.setAuthTag(Buffer.from(tag, "base64"));
         const decrypted = Buffer.concat([
-            decipher.update(Buffer.from(encrypted, "base64")),
+            decipher.update(data),
             decipher.final(),
         ]);
         return decrypted.toString("utf8");
@@ -111,14 +154,15 @@ export function decryptAuthConfig(row: ConnectorRow): McpConnectorAuthConfig {
         return {};
     }
     try {
+        const { key, data } = unpackCiphertext(row.encrypted_auth_config);
         const decipher = crypto.createDecipheriv(
             "aes-256-gcm",
-            encryptionKey(),
+            key,
             Buffer.from(row.auth_config_iv, "base64"),
         );
         decipher.setAuthTag(Buffer.from(row.auth_config_tag, "base64"));
         const decrypted = Buffer.concat([
-            decipher.update(Buffer.from(row.encrypted_auth_config, "base64")),
+            decipher.update(data),
             decipher.final(),
         ]);
         const parsed = JSON.parse(decrypted.toString("utf8"));

backend/tsconfig.json

@@ -16,5 +16,5 @@
     }
   },
   "include": ["src/**/*"],
-  "exclude": ["node_modules", "dist"]
+  "exclude": ["node_modules", "dist", "src/**/*.test.ts", "src/**/__tests__/**"]
 }