diff --git a/.env.example b/.env.example index 84b0573..006d6ea 100644 --- a/.env.example +++ b/.env.example @@ -43,6 +43,15 @@ POSTGRES_DB=mydb_dev # is handed this URL by compose; it is named here only so host-side tools (pg_dump, psql) can # reach it deliberately. POSTGRES_PROD_DB=mydb +# Encrypts the MCP server credentials Cameron stores (env vars, headers, OAuth tokens and client +# info) with AES-256-GCM. 32 random bytes, base64. Generate one with either: +# openssl rand -base64 32 +# node -e "console.log(require('crypto').randomBytes(32).toString('base64'))" +# Without it, saving a credential fails rather than storing it in plaintext. Keep it OUT of the +# database and its backups: losing it means re-entering those credentials; leaking it alongside a +# dump undoes the encryption. +CAMERON_SECRET_KEY= + # API KEYS ONLY. Which provider and model Cameron uses is chosen in the app (Settings) GOOGLE_API_KEY=your_google_api_key_here OPENAI_API_KEY=your_openai_api_key_here diff --git a/AGENTS.md b/AGENTS.md index 1427a73..def4807 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ This is a Next.js 15 fullstack AI agent chat application using LangGraph.js with — the one sanctioned raw-SQL site — which runs every query inside a `BEGIN TRANSACTION READ ONLY` - `statement_timeout`, **as the `cameron_analytics` role** (`SET LOCAL ROLE`), always rolls back, and hard-caps returned rows. The role can SELECT only `transaction` and `category`, so - Postgres refuses `mcp_server` (plaintext OAuth tokens/env/headers), the checkpoints, `config` + Postgres refuses `mcp_server` (OAuth tokens/env/headers, encrypted but still not the agent's), the checkpoints, `config` and `thread`; a `42501` comes back as an "isn't available to `run_sql`" payload. Why `set_config` is revoked from PUBLIC (it is `SET ROLE` as a function): [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md#what-run_sql-can-read). Both tools auto-approve. @@ -325,6 +325,9 @@ cameron_analytics` migration, and `analytics.test.ts` fails if the two differ. M - **Error translation** lives in the repositories: a duplicate name surfaces as `ConflictError` (→ HTTP 409); a missing row returns `null`/`false` (→ HTTP 404). Routes check return values, not driver error codes. +- **Encrypted columns**: `mcp_server.env`, `headers`, `auth_tokens`, `client_info` hold + `secretBox` envelopes; the row id + column are bound as AAD (`secretContext()`), so a value + copied between rows won't decrypt. Never read or write them outside the repository. - **`config` is settings, not a secret store**: `(key, value, updated_at)` only — no `description` column, because that varies per key, not per install, and no code would read it. Absent from `describe_finance_schema` and not granted to `cameron_analytics`, so `run_sql` cannot read it; @@ -359,8 +362,11 @@ cameron_analytics` migration, and `analytics.test.ts` fails if the two differ. M `type`/`transport` is absent, and judges each server separately. Switching JSON → form is refused while the JSON is invalid, so nothing is silently dropped. A paste holding several servers opens a checklist (`BatchReview`) and adds them one at a time with a per-server outcome. -- **Env values and headers are masked on screen but stored in plaintext** — say so in the UI, never - imply they are encrypted. +- **Env values, headers, OAuth tokens and client info are encrypted at rest** (AES-256-GCM, + `CAMERON_SECRET_KEY`), inside `mcpServerRepository` ONLY — see "Token Storage Security" in + [docs/OAUTH.md](docs/OAUTH.md). Legacy plaintext reads through; every write seals; a missing key + throws on the first secret write or envelope read, never writes plaintext. + Servers created before encryption must be recreated. `code_verifier` is deliberately not covered. - **Tools are listed on demand, one server at a time**: `GET /api/mcp-servers/{id}/tools` connects to that server only (enabled or not), then closes the client. Listing tools means connecting (for stdio, spawning a process), so nothing connects on page load and no tool count is stored — a stored count diff --git a/README.md b/README.md index 4b7642a..3f25360 100644 --- a/README.md +++ b/README.md @@ -121,9 +121,14 @@ git clone --branch v4 https://github.com/agentailor/cameron cd cameron cp .env.example .env # add your model API key, if your provider needs one + # and set CAMERON_SECRET_KEY (openssl rand -base64 32) docker compose --profile full up # http://localhost:3100 ``` +`CAMERON_SECRET_KEY` encrypts the credentials you give MCP servers (env vars, headers, OAuth +tokens) before they reach the database. Without it, saving one fails rather than storing it in +plaintext. Keep it out of your backups; lose it and you re-enter those credentials. + That builds the app, waits for Postgres, applies migrations, and starts everything. Nothing else is needed on your machine. @@ -144,7 +149,7 @@ Use this path to change Cameron, or to try it on sample data. From your clone: ```bash pnpm install -cp .env.example .env # add your model API key, if your provider needs one +cp .env.example .env # add your model API key and a CAMERON_SECRET_KEY docker compose up -d # dev dependencies — Postgres :5544, MinIO :9100/:9101 pnpm db:migrate pnpm db:seed # optional: sample categories and about six weeks of transactions diff --git a/compose.yaml b/compose.yaml index 5e4537c..41dd6fe 100644 --- a/compose.yaml +++ b/compose.yaml @@ -209,6 +209,8 @@ services: S3_FORCE_PATH_STYLE: "true" # Your keys, read from the environment or .env — never baked into the image. + # Encrypts stored MCP credentials; without it, saving one fails. See .env.example. + CAMERON_SECRET_KEY: ${CAMERON_SECRET_KEY:-} ANTHROPIC_API_KEY: ${ANTHROPIC_API_KEY:-} OPENAI_API_KEY: ${OPENAI_API_KEY:-} GOOGLE_API_KEY: ${GOOGLE_API_KEY:-} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a172429..3eaa048 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -359,8 +359,9 @@ model MCPServer { `run_sql` runs as the `cameron_analytics` role (`SET LOCAL ROLE` in `analyticsRepository`), which migration `0006_analytics_readonly_role.sql` grants SELECT on `transaction` and `category` only. -Postgres refuses everything else, including `mcp_server`, whose OAuth tokens, env vars and headers -are stored in plaintext. +Postgres refuses everything else, including `mcp_server`. Its OAuth tokens, env vars and headers +are encrypted at rest (see "Token Storage Security" in [OAUTH.md](OAUTH.md)), and the role keeps the +agent from reading even the ciphertext. The same migration revokes `EXECUTE` on `pg_catalog.set_config` from `PUBLIC`. **Keep that.** `SET ROLE` changes only the current role, and `set_config('role', 'none', true)` is `SET ROLE` as a diff --git a/docs/OAUTH.md b/docs/OAUTH.md index cef83dc..ccd9f67 100644 --- a/docs/OAUTH.md +++ b/docs/OAUTH.md @@ -138,8 +138,30 @@ The `NEXT_PUBLIC_APP_URL` is used to construct the OAuth redirect URI: `{NEXT_PU ### Token Storage Security -- **Encrypt tokens at rest**: Currently tokens are stored as plain JSON in the database. Use encryption (e.g., AES-256-GCM) before storing `authTokens` and `clientInfo` -- **Use a secrets manager**: Consider AWS Secrets Manager, HashiCorp Vault, or similar for sensitive OAuth credentials +Credentials are **encrypted at rest**. `auth_tokens`, `client_info`, `env` and `headers` on +`mcp_server` are sealed with AES-256-GCM (`src/lib/crypto/secretBox.ts`) under a key from +`CAMERON_SECRET_KEY` (32 random bytes, base64 — `openssl rand -base64 32`). A `pg_dump`, a backup +in `backups/` or an exposed Postgres port yields only ciphertext. + +- **Envelope**: each value is stored as `{ "enc": "aes-256-gcm", "v": 1, "iv", "tag", "data" }` + (base64), with a random 12-byte IV per write. `v` leaves room for key rotation later. +- **Bound to its place**: the row id and column name are authenticated data, so an envelope copied + to another server or column fails to open instead of being used there. +- **One seam**: encryption happens only in `mcpServerRepository`; every caller still gets plain + `MCPServer` objects. +- **No key, no secrets — never plaintext**: the key is read on first use, not at startup, so an + install with no credentials runs without one (empty values — `null`, `{}` — are stored as is). Writing a credential without a key throws + (`SecretKeyError`), and so does reading an envelope. A wrong key or an altered value throws + `SecretDecryptError`. Nothing ever falls back to writing plaintext. +- **Servers created before encryption**: recreate them. There is no migration script; until a + server is recreated its old plaintext values are still read as is (and a value is sealed the + next time it is written), so nothing breaks in the meantime. +- **Keep the key apart from the data**: it lives in env, never in the database or its dumps. + Losing it means re-entering those credentials (re-authorizing OAuth servers); there is no + recovery. +- **Not covered**: `code_verifier` (PKCE, short-lived, cleared once the server connects) and + `oauth_status`. The key protects data at rest, not a compromised running app — the process holds + it in memory. A secrets manager (Vault, AWS Secrets Manager) is still an option if you need one. ### Additional Security Measures diff --git a/src/app/api/mcp-servers/route.ts b/src/app/api/mcp-servers/route.ts index 7848282..6d1c1b9 100644 --- a/src/app/api/mcp-servers/route.ts +++ b/src/app/api/mcp-servers/route.ts @@ -6,6 +6,15 @@ import { recordOAuthRequirement } from "@/lib/mcp/oauth-detection"; import { mcpPool } from "@/lib/mcp/pool"; import { MCPServerType } from "@/types/mcp"; import { UpdateMCPServerBody } from "./schema"; +import { SecretDecryptError, SecretKeyError } from "@/lib/crypto/secretBox"; + +// A key problem is the owner's to fix, so say which one instead of a generic failure. +function secretError(error: unknown): NextResponse | null { + if (error instanceof SecretKeyError || error instanceof SecretDecryptError) { + return NextResponse.json({ error: error.message }, { status: 500 }); + } + return null; +} export async function GET() { try { @@ -13,7 +22,10 @@ export async function GET() { return NextResponse.json(servers); } catch (error) { console.error("Error fetching MCP servers:", error); - return NextResponse.json({ error: "Failed to fetch MCP servers" }, { status: 500 }); + return ( + secretError(error) ?? + NextResponse.json({ error: "Failed to fetch MCP servers" }, { status: 500 }) + ); } } @@ -59,7 +71,10 @@ export async function POST(request: Request) { if (error instanceof ConflictError) { return NextResponse.json({ error: "Server name already exists" }, { status: 409 }); } - return NextResponse.json({ error: "Failed to create MCP server" }, { status: 500 }); + return ( + secretError(error) ?? + NextResponse.json({ error: "Failed to create MCP server" }, { status: 500 }) + ); } } @@ -116,7 +131,10 @@ export async function PATCH(request: Request) { if (error instanceof ConflictError) { return NextResponse.json({ error: "Server name already exists" }, { status: 409 }); } - return NextResponse.json({ error: "Failed to update MCP server" }, { status: 500 }); + return ( + secretError(error) ?? + NextResponse.json({ error: "Failed to update MCP server" }, { status: 500 }) + ); } } diff --git a/src/components/connections/ServerForm.tsx b/src/components/connections/ServerForm.tsx index 3d63d9a..dc3a597 100644 --- a/src/components/connections/ServerForm.tsx +++ b/src/components/connections/ServerForm.tsx @@ -438,7 +438,8 @@ export function ServerForm({ {(draft.env.length > 0 || draft.headers.length > 0) && (
- Values are hidden on screen but stored unencrypted in your database. + Values are hidden on screen and encrypted in your database with your + CAMERON_SECRET_KEY.
)} diff --git a/src/lib/crypto/secretBox.test.ts b/src/lib/crypto/secretBox.test.ts new file mode 100644 index 0000000..3b23004 --- /dev/null +++ b/src/lib/crypto/secretBox.test.ts @@ -0,0 +1,122 @@ +import { randomBytes } from "node:crypto"; +import { afterEach, describe, expect, it } from "vitest"; +import { + SECRET_KEY_ENV, + SecretDecryptError, + SecretKeyError, + isEnvelope, + openSecret, + openWith, + parseKey, + sealSecret, + sealWith, +} from "./secretBox"; + +const key = randomBytes(32); +const ctx = "mcp_server.auth_tokens:abc"; +const tokens = { access_token: "at-123", refresh_token: "rt-456", expires_in: 3600 }; + +describe("sealWith / openWith", () => { + it("round-trips a JSON value and leaves no plaintext in the envelope", () => { + const env = sealWith(key, tokens, ctx); + expect(isEnvelope(env)).toBe(true); + expect(JSON.stringify(env)).not.toContain("rt-456"); + expect(openWith(key, env, ctx)).toEqual(tokens); + }); + + it("uses a fresh IV per value", () => { + const a = sealWith(key, tokens, ctx); + const b = sealWith(key, tokens, ctx); + expect(a.iv).not.toBe(b.iv); + expect(a.data).not.toBe(b.data); + }); + + it("rejects tampered ciphertext", () => { + const env = sealWith(key, tokens, ctx); + const bytes = Buffer.from(env.data, "base64"); + bytes[0] ^= 0xff; + expect(() => openWith(key, { ...env, data: bytes.toString("base64") }, ctx)).toThrow( + SecretDecryptError, + ); + }); + + it("rejects a tampered tag", () => { + const env = sealWith(key, tokens, ctx); + const tag = Buffer.from(env.tag, "base64"); + tag[0] ^= 0x01; + expect(() => openWith(key, { ...env, tag: tag.toString("base64") }, ctx)).toThrow( + SecretDecryptError, + ); + }); + + it("rejects the wrong key", () => { + const env = sealWith(key, tokens, ctx); + expect(() => openWith(randomBytes(32), env, ctx)).toThrow(SecretDecryptError); + }); + + it("rejects an envelope moved to another row or column", () => { + const env = sealWith(key, tokens, ctx); + expect(() => openWith(key, env, "mcp_server.auth_tokens:other")).toThrow(SecretDecryptError); + expect(() => openWith(key, env, "mcp_server.headers:abc")).toThrow(SecretDecryptError); + }); + + it("passes legacy plaintext through on read", () => { + expect(openWith(key, tokens, ctx)).toEqual(tokens); + expect(openWith(key, { API_TOKEN: "x" }, ctx)).toEqual({ API_TOKEN: "x" }); + expect(openWith(key, null, ctx)).toBeNull(); + }); +}); + +describe("isEnvelope", () => { + it("does not mistake a map that merely has similar keys for an envelope", () => { + const env = sealWith(key, tokens, ctx); + expect(isEnvelope({ ...env, extra: "x" })).toBe(false); + expect(isEnvelope({ ...env, v: 2 })).toBe(false); + expect(isEnvelope([env])).toBe(false); + }); +}); + +describe("parseKey", () => { + it("accepts 32 base64 bytes", () => { + expect(parseKey(key.toString("base64")).equals(key)).toBe(true); + }); + + it("refuses a missing or wrong-length key", () => { + expect(() => parseKey(undefined)).toThrow(SecretKeyError); + expect(() => parseKey(" ")).toThrow(SecretKeyError); + expect(() => parseKey(randomBytes(16).toString("base64"))).toThrow(SecretKeyError); + }); +}); + +describe("sealSecret / openSecret (env key)", () => { + const original = process.env[SECRET_KEY_ENV]; + afterEach(() => { + if (original === undefined) delete process.env[SECRET_KEY_ENV]; + else process.env[SECRET_KEY_ENV] = original; + }); + + it("refuses to write a secret without a key — never falls back to plaintext", () => { + delete process.env[SECRET_KEY_ENV]; + expect(() => sealSecret(tokens, ctx)).toThrow(SecretKeyError); + }); + + it("needs no key for empty values or legacy plaintext reads", () => { + delete process.env[SECRET_KEY_ENV]; + expect(sealSecret(null, ctx)).toBeNull(); + expect(sealSecret({}, ctx)).toEqual({}); + expect(() => sealSecret({ A: "" }, ctx)).toThrow(SecretKeyError); + expect(openSecret(tokens, ctx)).toEqual(tokens); + }); + + it("fails loudly reading an envelope without a key", () => { + process.env[SECRET_KEY_ENV] = key.toString("base64"); + const env = sealSecret(tokens, ctx); + delete process.env[SECRET_KEY_ENV]; + expect(() => openSecret(env, ctx)).toThrow(SecretKeyError); + }); + + it("round-trips with the env key", () => { + process.env[SECRET_KEY_ENV] = key.toString("base64"); + expect(openSecret(sealSecret(tokens, ctx), ctx)).toEqual(tokens); + }); +}); diff --git a/src/lib/crypto/secretBox.ts b/src/lib/crypto/secretBox.ts new file mode 100644 index 0000000..ead484b --- /dev/null +++ b/src/lib/crypto/secretBox.ts @@ -0,0 +1,131 @@ +import { createCipheriv, createDecipheriv, randomBytes } from "node:crypto"; + +/** + * Authenticated encryption for secrets stored in Postgres (AES-256-GCM, key from env). + * Values are sealed into a versioned JSON envelope; anything that is not an envelope is legacy + * plaintext and is returned as is on read. See docs/OAUTH.md ("Token Storage Security"). + */ + +export const SECRET_KEY_ENV = "CAMERON_SECRET_KEY"; + +const ALGORITHM = "aes-256-gcm"; +const KEY_BYTES = 32; +const IV_BYTES = 12; +const TAG_BYTES = 16; + +export interface SecretEnvelope { + enc: typeof ALGORITHM; + v: 1; + iv: string; + tag: string; + data: string; +} + +/** The key is missing or malformed. Thrown instead of ever writing plaintext. */ +export class SecretKeyError extends Error { + constructor(message: string) { + super(message); + this.name = "SecretKeyError"; + } +} + +/** An envelope failed to open: wrong key, tampered ciphertext, or moved to another row/column. */ +export class SecretDecryptError extends Error { + constructor(message: string) { + super(message); + this.name = "SecretDecryptError"; + } +} + +const GENERATE_HINT = `Generate one with: openssl rand -base64 32`; + +export function parseKey(raw: string | undefined): Buffer { + if (!raw || raw.trim() === "") { + throw new SecretKeyError( + `${SECRET_KEY_ENV} is not set, so MCP server credentials cannot be encrypted or read. ${GENERATE_HINT}`, + ); + } + const key = Buffer.from(raw.trim(), "base64"); + if (key.length !== KEY_BYTES) { + throw new SecretKeyError( + `${SECRET_KEY_ENV} must be ${KEY_BYTES} bytes, base64-encoded (got ${key.length}). ${GENERATE_HINT}`, + ); + } + return key; +} + +export function isEnvelope(value: unknown): value is SecretEnvelope { + if (typeof value !== "object" || value === null || Array.isArray(value)) return false; + const v = value as Record