From e245147cab3b73fc5d596a2acf0dbdcb4b8bc12a Mon Sep 17 00:00:00 2001 From: JoesnageL Date: Thu, 2 Jul 2026 16:13:35 -0600 Subject: [PATCH] fix: derive user settings encryption keys per user --- apps/daemon/src/user-config.test.ts | 60 +++++++++++++++++-- apps/daemon/src/user-config.ts | 54 ++++++++++------- .../src/app/api/settings/crypto-key/route.ts | 17 +++--- apps/web/src/app/api/settings/route.ts | 44 +++++++++++++- apps/web/src/lib/server-crypto.test.ts | 42 ++++++++++--- apps/web/src/lib/server-crypto.ts | 44 ++++++++++---- 6 files changed, 206 insertions(+), 55 deletions(-) diff --git a/apps/daemon/src/user-config.test.ts b/apps/daemon/src/user-config.test.ts index c0ff553..d70864e 100644 --- a/apps/daemon/src/user-config.test.ts +++ b/apps/daemon/src/user-config.test.ts @@ -3,7 +3,7 @@ * cache invalidation, and the env overlay. */ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { createCipheriv, randomBytes } from 'node:crypto'; +import { createCipheriv, createHmac, randomBytes } from 'node:crypto'; import { applyEnvOverlay, clearUserConfigCacheForTesting, @@ -52,7 +52,23 @@ function fakeClient(rows: FakeRow[]): { const TEST_KEY = randomBytes(32); -function encryptForTest(secret: Record): { ciphertext: string; iv: string; tag: string } { +function deriveUserKey(userId: string): Buffer { + return createHmac('sha256', TEST_KEY).update(userId).digest(); +} + +function encryptForTest(secret: Record, userId = 'u1'): { ciphertext: string; iv: string; tag: string } { + const iv = randomBytes(12); + const cipher = createCipheriv('aes-256-gcm', deriveUserKey(userId), iv); + const enc = Buffer.concat([cipher.update(JSON.stringify(secret), 'utf8'), cipher.final()]); + const tag = cipher.getAuthTag(); + return { + ciphertext: enc.toString('base64'), + iv: iv.toString('base64'), + tag: tag.toString('base64'), + }; +} + +function encryptLegacyForTest(secret: Record): { ciphertext: string; iv: string; tag: string } { const iv = randomBytes(12); const cipher = createCipheriv('aes-256-gcm', TEST_KEY, iv); const enc = Buffer.concat([cipher.update(JSON.stringify(secret), 'utf8'), cipher.final()]); @@ -113,6 +129,40 @@ describe('loadUserConfig — lookup precedence', () => { expect(cfg.getSecret('KRAKEN_API_KEY')).toBe('user-kraken-from-db'); }); + it('can read legacy master-key encrypted secrets', async () => { + process.env.KRAKEN_API_KEY = 'env-kraken'; + const blob = encryptLegacyForTest({ KRAKEN_API_KEY: 'legacy-user-kraken' }); + const { client } = fakeClient([{ + user_id: 'u1', + payload_plain: null, + payload_secret_ciphertext: blob.ciphertext, + payload_secret_iv: blob.iv, + payload_secret_tag: blob.tag, + updated_at: null, + }]); + setSupabaseClientForTesting(client as never); + + const cfg = await loadUserConfig('u1'); + expect(cfg.getSecret('KRAKEN_API_KEY')).toBe('legacy-user-kraken'); + }); + + it('does not decrypt another user encrypted secret blob', async () => { + delete process.env.KRAKEN_API_KEY; + const blob = encryptForTest({ KRAKEN_API_KEY: 'wrong-user-secret' }, 'other-user'); + const { client } = fakeClient([{ + user_id: 'u1', + payload_plain: null, + payload_secret_ciphertext: blob.ciphertext, + payload_secret_iv: blob.iv, + payload_secret_tag: blob.tag, + updated_at: null, + }]); + setSupabaseClientForTesting(client as never); + + const cfg = await loadUserConfig('u1'); + expect(cfg.getUserSecret('KRAKEN_API_KEY')).toBeUndefined(); + }); + it('plain overrides take precedence over process.env (for non-secret keys)', async () => { process.env.DAILY_LOSS_LIMIT_PCT = '5'; const { client } = fakeClient([{ @@ -181,7 +231,7 @@ describe('applyEnvOverlay', () => { const blob = encryptForTest({ KRAKEN_API_KEY: 'user-kraken', GEMINI_API_KEY: 'user-gemini', - }); + }, 'overlay-user'); const { client } = fakeClient([{ user_id: 'overlay-user', payload_plain: null, @@ -211,7 +261,7 @@ describe('applyEnvOverlay', () => { it('restores env even if the callback throws', async () => { process.env.KRAKEN_API_KEY = 'safe'; - const blob = encryptForTest({ KRAKEN_API_KEY: 'temp' }); + const blob = encryptForTest({ KRAKEN_API_KEY: 'temp' }, 'throws'); const { client } = fakeClient([{ user_id: 'throws', payload_plain: null, @@ -237,7 +287,7 @@ describe('loadUserConfig — degraded mode (no encryption key)', () => { process.env.KRAKEN_API_KEY = 'env-only'; // Even if the row has a secret blob, without a key we can't decrypt — env should still come through. - const blob = encryptForTest({ KRAKEN_API_KEY: 'user-from-db' }); + const blob = encryptForTest({ KRAKEN_API_KEY: 'user-from-db' }, 'no-key'); const { client } = fakeClient([{ user_id: 'no-key', payload_plain: null, diff --git a/apps/daemon/src/user-config.ts b/apps/daemon/src/user-config.ts index d564696..f5d56e7 100644 --- a/apps/daemon/src/user-config.ts +++ b/apps/daemon/src/user-config.ts @@ -30,7 +30,7 @@ * fine; for high-concurrency multi-user deployments the right fix is * to thread UserConfig through every adapter. */ -import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto'; +import { createCipheriv, createDecipheriv, createHmac, randomBytes } from 'node:crypto'; import { createClient, type SupabaseClient } from '@supabase/supabase-js'; import { ALL_OVERLAY_KEYS, SECRET_SET } from '@b1dz/core'; @@ -111,28 +111,40 @@ function loadEncryptionKey(): Buffer | null { return buf; } -function decryptBlob(row: UserSettingsRow, key: Buffer): Record | null { +function deriveUserKey(masterKey: Buffer, userId: string): Buffer { + return createHmac('sha256', masterKey).update(userId).digest(); +} + +function decryptBlobWithKey(row: UserSettingsRow, key: Buffer): Record | null { if (!row.payload_secret_ciphertext || !row.payload_secret_iv || !row.payload_secret_tag) return null; - try { - const iv = Buffer.from(row.payload_secret_iv, 'base64'); - const tag = Buffer.from(row.payload_secret_tag, 'base64'); - const ct = Buffer.from(row.payload_secret_ciphertext, 'base64'); - const dec = createDecipheriv(ALGORITHM, key, iv); - dec.setAuthTag(tag); - const out = Buffer.concat([dec.update(ct), dec.final()]).toString('utf8'); - const parsed = JSON.parse(out) as unknown; - if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { - const result: Record = {}; - for (const [k, v] of Object.entries(parsed as Record)) { - if (typeof v === 'string') result[k] = v; - else if (typeof v === 'number' || typeof v === 'boolean') result[k] = String(v); - } - return result; + const iv = Buffer.from(row.payload_secret_iv, 'base64'); + const tag = Buffer.from(row.payload_secret_tag, 'base64'); + const ct = Buffer.from(row.payload_secret_ciphertext, 'base64'); + const dec = createDecipheriv(ALGORITHM, key, iv); + dec.setAuthTag(tag); + const out = Buffer.concat([dec.update(ct), dec.final()]).toString('utf8'); + const parsed = JSON.parse(out) as unknown; + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + const result: Record = {}; + for (const [k, v] of Object.entries(parsed as Record)) { + if (typeof v === 'string') result[k] = v; + else if (typeof v === 'number' || typeof v === 'boolean') result[k] = String(v); } - return null; + return result; + } + return null; +} + +function decryptBlob(row: UserSettingsRow, key: Buffer): Record | null { + try { + return decryptBlobWithKey(row, deriveUserKey(key, row.user_id)); } catch (e) { - console.warn(`user-config: decrypt failed for ${row.user_id.slice(0, 8)}: ${(e as Error).message}`); - return null; + try { + return decryptBlobWithKey(row, key); + } catch { + console.warn(`user-config: decrypt failed for ${row.user_id.slice(0, 8)}: ${(e as Error).message}`); + return null; + } } } @@ -264,7 +276,7 @@ export async function mergeUserSecret(userId: string, patch: Record = { ...existing, ...patch }; const iv = randomBytes(12); - const cipher = createCipheriv(ALGORITHM, key, iv); + const cipher = createCipheriv(ALGORITHM, deriveUserKey(key, userId), iv); const ct = Buffer.concat([cipher.update(JSON.stringify(merged), 'utf8'), cipher.final()]); const client = defaultClient(); const { error } = await client.from('user_settings').upsert( diff --git a/apps/web/src/app/api/settings/crypto-key/route.ts b/apps/web/src/app/api/settings/crypto-key/route.ts index a2888b5..ffb7982 100644 --- a/apps/web/src/app/api/settings/crypto-key/route.ts +++ b/apps/web/src/app/api/settings/crypto-key/route.ts @@ -1,17 +1,17 @@ /** * GET /api/settings/crypto-key * - * Returns the base64-encoded AES-256-GCM key that the browser, CLI, and - * daemon all use to encrypt/decrypt the user_settings secret blob. Auth - * required — the wire is HTTPS, the key is what the client already - * implicitly trusts the server with for daemon-side decrypt anyway. + * Returns the base64-encoded per-user AES-256-GCM key that the browser, + * CLI, and daemon use to encrypt/decrypt that user's secret blob. Auth + * required. The key is derived from the server's master + * SETTINGS_ENCRYPTION_KEY plus auth.userId. * * The browser holds this key in memory only (no localStorage). The - * server NEVER receives plaintext secrets — the client-side path - * encrypts before PUT, and reveal happens locally. + * client-side settings path encrypts before PUT, and reveal happens locally. */ import type { NextRequest } from 'next/server'; import { authenticate, unauthorized } from '@/lib/api-auth'; +import { deriveUserSecretKey } from '@/lib/server-crypto'; export const dynamic = 'force-dynamic'; @@ -19,8 +19,7 @@ export async function GET(req: NextRequest) { const auth = await authenticate(req); if (!auth) return unauthorized(); - const key = process.env.SETTINGS_ENCRYPTION_KEY; - if (!key) { + if (!process.env.SETTINGS_ENCRYPTION_KEY) { return Response.json( { error: 'SETTINGS_ENCRYPTION_KEY not configured on server' }, { @@ -31,7 +30,7 @@ export async function GET(req: NextRequest) { } return Response.json( - { key }, + { key: deriveUserSecretKey(auth.userId).toString('base64') }, { headers: { 'cache-control': 'no-store', pragma: 'no-cache' } }, ); } diff --git a/apps/web/src/app/api/settings/route.ts b/apps/web/src/app/api/settings/route.ts index d3d8b17..b2f6b6b 100644 --- a/apps/web/src/app/api/settings/route.ts +++ b/apps/web/src/app/api/settings/route.ts @@ -16,6 +16,7 @@ */ import type { NextRequest } from 'next/server'; import { authenticate, unauthorized } from '@/lib/api-auth'; +import { decryptCurrentUserSecret, decryptSecret, encryptSecret } from '@/lib/server-crypto'; import { sanitizePlain, type PlainPayload } from '@/lib/settings-fields'; export const dynamic = 'force-dynamic'; @@ -61,6 +62,46 @@ function rowToCipher(row: SettingsRow | null): CipherBlob | null { }; } +async function upsertCipher(client: unknown, userId: string, blob: CipherBlob): Promise { + const { error } = await (client as { + from: (t: string) => { + upsert: (r: Record, opts: { onConflict: string }) => Promise<{ error: { message: string } | null }>; + }; + }).from('user_settings').upsert( + { + user_id: userId, + payload_secret_ciphertext: blob.ciphertext, + payload_secret_iv: blob.iv, + payload_secret_tag: blob.tag, + updated_at: new Date().toISOString(), + }, + { onConflict: 'user_id' }, + ); + if (error) throw new Error(error.message); +} + +async function migrateLegacyCipherIfNeeded(client: unknown, userId: string, row: SettingsRow | null): Promise { + const cipher = rowToCipher(row); + if (!cipher || !secretCryptoConfigured()) return cipher; + + try { + decryptCurrentUserSecret(cipher, userId); + return cipher; + } catch { + // Fall through and try the legacy master-key decrypt below. + } + + try { + const secret = decryptSecret(cipher, userId); + const migrated = encryptSecret(secret, userId); + await upsertCipher(client, userId, migrated); + return migrated; + } catch (e) { + console.warn(`[api] settings secret migration skipped user=${userId.slice(0, 8)} error=${(e as Error).message}`); + return cipher; + } +} + function isValidCipher(value: unknown): value is CipherBlob { if (!value || typeof value !== 'object') return false; const v = value as Record; @@ -89,10 +130,11 @@ export async function GET(req: NextRequest) { // RPC endpoints into every user's settings page. Operator env is for the // operator's own account only. const userPlain = sanitizePlain(row?.payload_plain ?? {}); + const cipher = await migrateLegacyCipherIfNeeded(auth.client, auth.userId, row); return Response.json({ plain: userPlain, - cipher: rowToCipher(row), + cipher, lastUpdatedAt: row?.updated_at ?? null, cryptoConfigured: secretCryptoConfigured(), }); diff --git a/apps/web/src/lib/server-crypto.test.ts b/apps/web/src/lib/server-crypto.test.ts index 42c47cb..d137669 100644 --- a/apps/web/src/lib/server-crypto.test.ts +++ b/apps/web/src/lib/server-crypto.test.ts @@ -1,39 +1,65 @@ import { describe, it, expect, beforeAll } from 'vitest'; -import { randomBytes } from 'node:crypto'; +import { createCipheriv, randomBytes } from 'node:crypto'; let encryptSecret: typeof import('./server-crypto').encryptSecret; let decryptSecret: typeof import('./server-crypto').decryptSecret; +let decryptCurrentUserSecret: typeof import('./server-crypto').decryptCurrentUserSecret; +let deriveUserSecretKey: typeof import('./server-crypto').deriveUserSecretKey; beforeAll(async () => { process.env.SETTINGS_ENCRYPTION_KEY = randomBytes(32).toString('base64'); const mod = await import('./server-crypto'); encryptSecret = mod.encryptSecret; decryptSecret = mod.decryptSecret; + decryptCurrentUserSecret = mod.decryptCurrentUserSecret; + deriveUserSecretKey = mod.deriveUserSecretKey; }); describe('server-crypto', () => { it('round-trips a secret object', () => { - const blob = encryptSecret({ ALPACA_OAUTH_TOKEN: 'tok-123', TRADIER_ACCESS_TOKEN: 'abc' }); + const blob = encryptSecret({ ALPACA_OAUTH_TOKEN: 'tok-123', TRADIER_ACCESS_TOKEN: 'abc' }, 'user-a'); expect(blob.ciphertext.length).toBeGreaterThan(0); expect(blob.iv.length).toBeGreaterThan(0); expect(blob.tag.length).toBeGreaterThan(0); - expect(decryptSecret(blob)).toEqual({ ALPACA_OAUTH_TOKEN: 'tok-123', TRADIER_ACCESS_TOKEN: 'abc' }); + expect(decryptSecret(blob, 'user-a')).toEqual({ ALPACA_OAUTH_TOKEN: 'tok-123', TRADIER_ACCESS_TOKEN: 'abc' }); }); it('produces a fresh IV each time (non-deterministic ciphertext)', () => { - const a = encryptSecret({ k: 'v' }); - const b = encryptSecret({ k: 'v' }); + const a = encryptSecret({ k: 'v' }, 'user-a'); + const b = encryptSecret({ k: 'v' }, 'user-a'); expect(a.iv).not.toEqual(b.iv); expect(a.ciphertext).not.toEqual(b.ciphertext); }); it('returns {} for an empty/absent blob', () => { - expect(decryptSecret(null)).toEqual({}); + expect(decryptSecret(null, 'user-a')).toEqual({}); }); it('fails authentication on a tampered tag', () => { - const blob = encryptSecret({ k: 'v' }); + const blob = encryptSecret({ k: 'v' }, 'user-a'); const tampered = { ...blob, tag: Buffer.from(randomBytes(16)).toString('base64') }; - expect(() => decryptSecret(tampered)).toThrow(); + expect(() => decryptSecret(tampered, 'user-a')).toThrow(); + }); + + it('derives different keys for different users', () => { + expect(deriveUserSecretKey('user-a').equals(deriveUserSecretKey('user-b'))).toBe(false); + }); + + it('does not decrypt another user secret with the current-user key', () => { + const blob = encryptSecret({ k: 'v' }, 'user-a'); + expect(() => decryptCurrentUserSecret(blob, 'user-b')).toThrow(); + }); + + it('can decrypt legacy master-key blobs for migration', () => { + const iv = randomBytes(12); + const cipher = createCipheriv('aes-256-gcm', Buffer.from(process.env.SETTINGS_ENCRYPTION_KEY!, 'base64'), iv); + const ciphertext = Buffer.concat([cipher.update(JSON.stringify({ k: 'legacy' }), 'utf8'), cipher.final()]); + const blob = { + ciphertext: ciphertext.toString('base64'), + iv: iv.toString('base64'), + tag: cipher.getAuthTag().toString('base64'), + }; + + expect(decryptSecret(blob, 'user-a')).toEqual({ k: 'legacy' }); }); }); diff --git a/apps/web/src/lib/server-crypto.ts b/apps/web/src/lib/server-crypto.ts index ba62915..4bec61f 100644 --- a/apps/web/src/lib/server-crypto.ts +++ b/apps/web/src/lib/server-crypto.ts @@ -4,13 +4,13 @@ * Normally secrets are encrypted in the browser (see lib/browser-crypto.ts) and * the server only stores ciphertext. The OAuth callback is the one server-side * writer: it receives tokens from the provider and must merge them into the - * encrypted blob. The server already holds SETTINGS_ENCRYPTION_KEY (it serves it - * to clients via /api/settings/crypto-key), so it can encrypt with the same key. + * encrypted blob. The server holds SETTINGS_ENCRYPTION_KEY and derives a + * per-user AES key from it, matching /api/settings/crypto-key. * * Format matches browser-crypto + the daemon's decryptBlob exactly: * AES-256-GCM · 12-byte IV · 16-byte tag stored separately · all base64. */ -import { createCipheriv, createDecipheriv, randomBytes } from 'node:crypto'; +import { createCipheriv, createDecipheriv, createHmac, randomBytes } from 'node:crypto'; const ALGO = 'aes-256-gcm'; const IV_BYTES = 12; @@ -21,7 +21,7 @@ export interface CipherBlob { tag: string; } -function key(): Buffer { +function masterKey(): Buffer { const raw = process.env.SETTINGS_ENCRYPTION_KEY; if (!raw) throw new Error('SETTINGS_ENCRYPTION_KEY not configured'); const buf = Buffer.from(raw, 'base64'); @@ -29,16 +29,20 @@ function key(): Buffer { return buf; } -export function encryptSecret(obj: Record): CipherBlob { +export function deriveUserSecretKey(userId: string): Buffer { + if (!userId) throw new Error('userId required'); + return createHmac('sha256', masterKey()).update(userId).digest(); +} + +export function encryptSecret(obj: Record, userId: string): CipherBlob { const iv = randomBytes(IV_BYTES); - const cipher = createCipheriv(ALGO, key(), iv); + const cipher = createCipheriv(ALGO, deriveUserSecretKey(userId), iv); const ct = Buffer.concat([cipher.update(JSON.stringify(obj), 'utf8'), cipher.final()]); return { ciphertext: ct.toString('base64'), iv: iv.toString('base64'), tag: cipher.getAuthTag().toString('base64') }; } -export function decryptSecret(blob: CipherBlob | null): Record { - if (!blob?.ciphertext || !blob.iv || !blob.tag) return {}; - const dec = createDecipheriv(ALGO, key(), Buffer.from(blob.iv, 'base64')); +function decryptWithKey(blob: CipherBlob, key: Buffer): Record { + const dec = createDecipheriv(ALGO, key, Buffer.from(blob.iv, 'base64')); dec.setAuthTag(Buffer.from(blob.tag, 'base64')); const out = Buffer.concat([dec.update(Buffer.from(blob.ciphertext, 'base64')), dec.final()]).toString('utf8'); const parsed = JSON.parse(out) as unknown; @@ -51,6 +55,24 @@ export function decryptSecret(blob: CipherBlob | null): Record { return result; } +export function decryptCurrentUserSecret(blob: CipherBlob | null, userId: string): Record { + if (!blob?.ciphertext || !blob.iv || !blob.tag) return {}; + return decryptWithKey(blob, deriveUserSecretKey(userId)); +} + +export function decryptSecret(blob: CipherBlob | null, userId: string): Record { + if (!blob?.ciphertext || !blob.iv || !blob.tag) return {}; + try { + return decryptCurrentUserSecret(blob, userId); + } catch (e) { + try { + return decryptWithKey(blob, masterKey()); + } catch { + throw e; + } + } +} + interface SettingsRowCrypto { payload_secret_ciphertext: string | null; payload_secret_iv: string | null; @@ -78,9 +100,9 @@ export async function mergeSecretIntoUserSettings( const { data, error } = await c.from('user_settings').select('*').eq('user_id', userId).maybeSingle(); if (error) throw new Error(error.message); const existing = data?.payload_secret_ciphertext && data.payload_secret_iv && data.payload_secret_tag - ? decryptSecret({ ciphertext: data.payload_secret_ciphertext, iv: data.payload_secret_iv, tag: data.payload_secret_tag }) + ? decryptSecret({ ciphertext: data.payload_secret_ciphertext, iv: data.payload_secret_iv, tag: data.payload_secret_tag }, userId) : {}; - const blob = encryptSecret({ ...existing, ...patch }); + const blob = encryptSecret({ ...existing, ...patch }, userId); const { error: upErr } = await c.from('user_settings').upsert( { user_id: userId,