From 504586fdcdfc0f43cfdd915ae4b253ef9d096e11 Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Mon, 18 May 2026 13:08:17 -0700 Subject: [PATCH] fix(security): vault integration secrets instead of plaintext config (R1) PayPal client_secret (and any secret-named integration field) was stored plaintext in integrations.config, unlike Stripe/door which use the AES-256-GCM vault. A DB read or backup leaked it in cleartext. - lib/secrets/vault.ts: canonical generic storeSecret/readSecret + isSecretConfigField (the proven Stripe vault logic, generalized). - saveIntegration: secret-named fields go to the vault, only a *_ref is kept in config; blank submit preserves the ref (write-only like Stripe); legacy plaintext auto-migrates into the vault on next save. - paypal/sync reads client_secret from the vault, transitional fallback to legacy plaintext until the next save. - Docs updated (API_REFERENCE, ARCHITECTURE 10.3). 502 tests. --- __tests__/secrets-vault.test.ts | 22 +++++++++ app/api/paypal/sync/route.ts | 11 ++++- docs/API_REFERENCE.md | 1 + docs/ARCHITECTURE.md | 8 ++-- lib/actions/settings.ts | 61 +++++++++++++++++++++++-- lib/secrets/vault.ts | 81 +++++++++++++++++++++++++++++++++ 6 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 __tests__/secrets-vault.test.ts create mode 100644 lib/secrets/vault.ts diff --git a/__tests__/secrets-vault.test.ts b/__tests__/secrets-vault.test.ts new file mode 100644 index 0000000..3bf416e --- /dev/null +++ b/__tests__/secrets-vault.test.ts @@ -0,0 +1,22 @@ +import { describe, it, expect } from 'vitest' +import { isSecretConfigField } from '@/lib/secrets/vault' + +describe('isSecretConfigField', () => { + it('treats credential-named fields as secrets to vault', () => { + for (const k of ['client_secret', 'api_key', 'secret_key', 'webhook_secret', 'signing_secret']) { + expect(isSecretConfigField(k)).toBe(true) + } + }) + + it('leaves non-secret config fields in plaintext config', () => { + for (const k of ['client_id', 'sandbox', 'mode', 'publishable_key', 'username']) { + expect(isSecretConfigField(k)).toBe(false) + } + }) + + it('never vaults derived flag/ref keys (no recursion on re-save)', () => { + expect(isSecretConfigField('client_secret_set')).toBe(false) + expect(isSecretConfigField('client_secret_ref')).toBe(false) + expect(isSecretConfigField('api_key_ref')).toBe(false) + }) +}) diff --git a/app/api/paypal/sync/route.ts b/app/api/paypal/sync/route.ts index 8e1e15b..ff41814 100644 --- a/app/api/paypal/sync/route.ts +++ b/app/api/paypal/sync/route.ts @@ -1,5 +1,7 @@ import { NextRequest, NextResponse } from 'next/server' import { createClient } from '@/lib/supabase/server' +import { createAdminClient } from '@/lib/supabase/admin' +import { readSecret } from '@/lib/secrets/vault' // PayPal Transaction Search API (v1) // Docs: https://developer.paypal.com/docs/api/transaction-search/v1/ @@ -72,7 +74,14 @@ export async function POST(req: NextRequest) { return NextResponse.json({ error: 'PayPal not connected' }, { status: 400 }) } - const { client_id, client_secret, sandbox } = integration.config as Record + const cfg = integration.config as Record + const { client_id, sandbox } = cfg + // client_secret lives in the AES-256-GCM vault (referenced by client_secret_ref). + // Legacy fallback: integrations saved before vaulting still have a plaintext + // client_secret; it is auto-migrated on the next save in Settings. + const client_secret = cfg.client_secret_ref + ? await readSecret(createAdminClient(), member.space_id, cfg.client_secret_ref) + : (cfg.client_secret ?? null) if (!client_id || !client_secret) { return NextResponse.json({ error: 'PayPal credentials incomplete' }, { status: 400 }) } diff --git a/docs/API_REFERENCE.md b/docs/API_REFERENCE.md index 005a25b..a776104 100644 --- a/docs/API_REFERENCE.md +++ b/docs/API_REFERENCE.md @@ -682,6 +682,7 @@ async function saveIntegration( **Behavior**: - Upserts on `(space_id, platform)` - Stores config with `_set` indicators for each field +- Secret-named fields (`client_secret`, `*_secret`, `api_key`, `secret_key`) are NEVER stored in `integrations.config` in plaintext: they go to the AES-256-GCM secrets vault and only a `_ref` id is kept (same vault Stripe/door use). A blank submit preserves the existing ref (write-only, like Stripe); any legacy plaintext secret already on file is auto-migrated into the vault on the next save. The PayPal sync route reads `client_secret` from the vault (with a transitional fallback to legacy plaintext until the next save). --- diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4ff8622..1d972e6 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -396,10 +396,10 @@ The admin client (`SUPABASE_SERVICE_ROLE_KEY`) is used ONLY for: ### 10.3 Secrets Storage -Integration credentials stored in `integrations.config` (JSONB): -- Values stored with `_set` indicators -- UI shows masked values -- Consider encryption at rest for production +Integration credentials: +- Secret-named fields (`client_secret`, `*_secret`, `api_key`, `secret_key`) are encrypted at rest in the AES-256-GCM secrets vault (`lib/secrets/vault.ts`, same vault Stripe + door use); only a `_ref` id is kept in `integrations.config`. Server-side decrypt only, never returned to the client. +- Non-secret fields stored in `integrations.config` (JSONB) with `_set` indicators; UI shows connected state via the flags, never the value. +- Legacy plaintext secrets (pre-vault) are auto-migrated into the vault on the next `saveIntegration`. --- diff --git a/lib/actions/settings.ts b/lib/actions/settings.ts index 890254e..19bd041 100644 --- a/lib/actions/settings.ts +++ b/lib/actions/settings.ts @@ -2,6 +2,8 @@ import { revalidatePath } from 'next/cache' import { createClient } from '@/lib/supabase/server' +import { createAdminClient } from '@/lib/supabase/admin' +import { storeSecret, isSecretConfigField } from '@/lib/secrets/vault' import { requireMemberWithRole, parseInput } from '@/lib/auth-helpers' import { updateSpaceSettingsSchema, @@ -68,13 +70,66 @@ export async function saveIntegration(platform: string, config: Record | null) ?? {} + + const admin = createAdminClient() + + // Credentials NEVER land in integrations.config in plaintext: secret-named + // fields go to the AES-256-GCM vault and only a *_ref id is kept. Non-secret + // fields are stored verbatim. *_set flags let the UI show "connected" + // without the value being readable. const safeConfig: Record = {} + let connected = false + for (const [key, value] of Object.entries(v.data.config)) { + if (isSecretConfigField(key)) continue // handled below safeConfig[key] = value ?? '' safeConfig[`${key}_set`] = value && value.length > 0 ? 'true' : 'false' + if (value && value.length > 0) connected = true + } + + // Determine the secret fields this platform may carry: those provided now + // plus any already on file (so a blank submit preserves / migrates them). + const secretKeys = new Set() + for (const k of Object.keys(v.data.config)) if (isSecretConfigField(k)) secretKeys.add(k) + for (const k of Object.keys(existing)) { + const base = k.replace(/_ref$/, '') + if (isSecretConfigField(base)) secretKeys.add(base) + } + + for (const key of secretKeys) { + const provided = v.data.config[key] + let ref: string | null = existing[`${key}_ref`] ?? null + + if (provided && provided.length > 0) { + ref = await storeSecret(admin, member.space_id, `${v.data.platform} ${key}`, provided, user?.id ?? null) + if (!ref) return { error: `Could not securely store ${key}.` } + } else if (!ref && existing[key] && existing[key].length > 0) { + // Legacy plaintext on file (pre-vault). Migrate it now, then drop it. + ref = await storeSecret(admin, member.space_id, `${v.data.platform} ${key}`, existing[key], user?.id ?? null) + if (!ref) return { error: `Could not migrate ${key} into the vault.` } + } + + if (ref) { + safeConfig[`${key}_ref`] = ref + safeConfig[`${key}_set`] = 'true' + connected = true + } else { + safeConfig[`${key}_set`] = 'false' + } } const { error } = await supabase.from('integrations').upsert( @@ -82,7 +137,7 @@ export async function saveIntegration(platform: string, config: Record value && value.length > 0), + is_connected: connected, config: safeConfig, updated_at: new Date().toISOString(), }, diff --git a/lib/secrets/vault.ts b/lib/secrets/vault.ts new file mode 100644 index 0000000..db394ca --- /dev/null +++ b/lib/secrets/vault.ts @@ -0,0 +1,81 @@ +// Generic per-space secret vault helpers. This is the canonical store/read +// pair for any third-party credential (the AES-256-GCM `secrets` table, the +// same layout the door subsystem and Stripe config use). NOT a 'use server' +// module: it takes a Supabase admin client and is imported by server actions +// and route handlers. Secrets are server-only and never returned to a client. +import type { createAdminClient } from '@/lib/supabase/admin' +import { encryptSecret, decryptSecret, encryptionAvailable } from '@/lib/secrets/crypto' + +type Admin = ReturnType + +// Store a credential in the encrypted vault. Returns the secret row id to +// keep (as a *_ref) in integrations.config. null on failure. +export async function storeSecret( + admin: Admin, + spaceId: string, + title: string, + value: string, + createdBy: string | null, +): Promise { + const row: Record = { + space_id: spaceId, + title, + label: title, + category: 'integration', + created_by: createdBy, + } + if (encryptionAvailable()) { + const { ciphertext, version } = encryptSecret(value) + row.encrypted_value = ciphertext + row.encryption_version = version + row.value = '' + } else { + row.value = value + row.encryption_version = 0 + } + const { data, error } = await admin.from('secrets').insert(row).select('id').single() + if (error || !data) return null + return data.id as string +} + +// Read + decrypt a stored credential by ref, space-scoped. Server-only. +export async function readSecret( + admin: Admin, + spaceId: string, + secretRef: string | null | undefined, +): Promise { + if (!secretRef) return null + const { data } = await admin + .from('secrets') + .select('encryption_version, encrypted_value, value') + .eq('id', secretRef) + .eq('space_id', spaceId) + .maybeSingle() + if (!data) return null + if (data.encryption_version === 1 && data.encrypted_value) { + const raw = data.encrypted_value as unknown + const buf = + typeof raw === 'string' + ? Buffer.from((raw as string).replace(/^\\x/, ''), 'hex') + : Buffer.from(raw as Uint8Array) + try { + return decryptSecret(buf, 1) + } catch { + return null + } + } + return (data.value as string | null) ?? null +} + +// Config key naming convention for credentials that must be vaulted rather +// than stored plaintext in integrations.config. Derived keys (*_set, *_ref) +// are never themselves secrets. +export function isSecretConfigField(key: string): boolean { + if (key.endsWith('_set') || key.endsWith('_ref')) return false + return ( + key === 'client_secret' || + key === 'api_key' || + key === 'secret_key' || + key.endsWith('_secret') + ) +}