From d9f6151f5312467fbf3af029626e39bdf87125b4 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 20 Aug 2026 00:50:27 +0200 Subject: [PATCH] feat(server): deriveSecurity.encryptionKeySet and storage secret plumbing (gate, encrypt, unmask, mask, redact) --- server/src/app-config/derive.ts | 14 ++ server/src/app-config/index.ts | 1 + server/src/nest/common/crypto/apiKeyCrypto.ts | 5 + server/src/nest/storage/storage-secrets.ts | 137 ++++++++++++++++++ server/tests/unit/app-config/derive.test.ts | 11 +- .../unit/nest/storage/storage-secrets.test.ts | 125 ++++++++++++++++ .../tests/unit/services/apiKeyCrypto.test.ts | 18 ++- 7 files changed, 309 insertions(+), 2 deletions(-) create mode 100644 server/src/nest/storage/storage-secrets.ts create mode 100644 server/tests/unit/nest/storage/storage-secrets.test.ts diff --git a/server/src/app-config/derive.ts b/server/src/app-config/derive.ts index 58f15fb42..aab8015c8 100644 --- a/server/src/app-config/derive.ts +++ b/server/src/app-config/derive.ts @@ -260,6 +260,19 @@ export function derivePaths(raw: RawEnv) { }; } +/** + * Security posture (spec: docs/superpowers/specs/2026-08-19-storage-admin-config-design.md). + * encryptionKeySet is true only when ENCRYPTION_KEY is explicitly set — the + * silent JWT-derived fallback (config.ts) is deliberately NOT reflected here: + * saving a credentialed storage backend fails closed on the fallback, naming + * the variable. (deriveBackup.encryptionKeyFromEnv answers a different + * question — whether the key file must ride inside backup archives — and + * stays untouched.) + */ +export function deriveSecurity(raw: RawEnv) { + return { encryptionKeySet: Boolean(raw.ENCRYPTION_KEY?.trim()) }; +} + export function deriveNet(raw: RawEnv) { return { allowInternalNetwork: parseBool(raw.ALLOW_INTERNAL_NETWORK) === true, @@ -284,6 +297,7 @@ export function deriveAll(raw: RawEnv) { backup: deriveBackup(raw), db: deriveDb(raw), paths: derivePaths(raw), + security: deriveSecurity(raw), net: deriveNet(raw), }; } diff --git a/server/src/app-config/index.ts b/server/src/app-config/index.ts index 211f078a4..a9450dc10 100644 --- a/server/src/app-config/index.ts +++ b/server/src/app-config/index.ts @@ -19,6 +19,7 @@ export { deriveBackup, deriveDb, derivePaths, + deriveSecurity, deriveNet, } from './derive'; export * from './parsers'; diff --git a/server/src/nest/common/crypto/apiKeyCrypto.ts b/server/src/nest/common/crypto/apiKeyCrypto.ts index 96eccea5c..ca6e0b3df 100644 --- a/server/src/nest/common/crypto/apiKeyCrypto.ts +++ b/server/src/nest/common/crypto/apiKeyCrypto.ts @@ -41,3 +41,8 @@ export function maybe_encrypt_api_key(value: unknown) { return encrypt_api_key(trimmed); } +/** True exactly for values carrying the enc:v1: envelope produced by encrypt_api_key. */ +export function is_encrypted_api_key(value: unknown): value is string { + return typeof value === 'string' && value.startsWith(ENCRYPTED_PREFIX); +} + diff --git a/server/src/nest/storage/storage-secrets.ts b/server/src/nest/storage/storage-secrets.ts new file mode 100644 index 000000000..a677821c2 --- /dev/null +++ b/server/src/nest/storage/storage-secrets.ts @@ -0,0 +1,137 @@ +import { + MASKED_SETTING_VALUE, + storageSecretFields, + type StorageBackend, + type StorageBackendTypeId, + type StorageConfig, +} from '@trek/shared'; +import { + decrypt_api_key, + is_encrypted_api_key, + maybe_encrypt_api_key, +} from '../common/crypto/apiKeyCrypto'; +import { StorageBackendError } from './storage.types'; + +/** + * Secret-field plumbing for admin-managed storage config (spec: + * docs/superpowers/specs/2026-08-19-storage-admin-config-design.md). Secrets + * live encrypted INSIDE the storage.backends JSON (the LLM addon-config + * precedent), on exactly the fields the shared type registry marks `secret`. + * Pure functions — the registry's seed import and the admin service share + * them. + */ + +export interface PlaintextSecret { + backend: string; + field: string; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** Map one backend's secret-field values; returns a new backend object. */ +function mapSecrets(backend: StorageBackend, fn: (value: unknown, field: string) => unknown): StorageBackend { + const fields = storageSecretFields(backend.type); + if (fields.length === 0) return backend; + const options: Record = { ...backend.options }; + for (const field of fields) options[field] = fn(options[field], field); + return { ...backend, options } as StorageBackend; +} + +/** Secret fields whose value is neither empty nor an enc:v1: envelope. */ +export function listPlaintextSecrets(config: StorageConfig): PlaintextSecret[] { + const found: PlaintextSecret[] = []; + for (const backend of config.backends) { + for (const field of storageSecretFields(backend.type)) { + const value = (backend.options as Record)[field]; + if (typeof value === 'string' && value !== '' && !is_encrypted_api_key(value)) { + found.push({ backend: backend.name, field }); + } + } + } + return found; +} + +/** The seed file has no mask round-trip — a literal mask sentinel is always a mistake. */ +export function assertNoMaskSentinels(config: StorageConfig): void { + for (const backend of config.backends) { + for (const field of storageSecretFields(backend.type)) { + if ((backend.options as Record)[field] === MASKED_SETTING_VALUE) { + throw new StorageBackendError( + `backend '${backend.name}' field '${field}' is the mask sentinel — a mask can never become a stored value`, + ); + } + } + } +} + +/** Fail-closed refusal text: the silent JWT-derived fallback key is not accepted for credentials. */ +export function encryptionGateError(secret: PlaintextSecret): string { + return ( + `backend '${secret.backend}' has a plaintext secret '${secret.field}' but ENCRYPTION_KEY is not set — ` + + 'set ENCRYPTION_KEY explicitly to save credentialed storage backends (the JWT-derived fallback key is not accepted)' + ); +} + +/** Encrypt every secret field (idempotent — enc:v1: values pass through unchanged). */ +export function encryptStorageSecrets(config: StorageConfig): StorageConfig { + return { + ...config, + backends: config.backends.map((backend) => + mapSecrets(backend, (value) => + typeof value === 'string' && value !== '' ? maybe_encrypt_api_key(value) : value, + ), + ), + }; +} + +/** + * Resolve mask echoes: a secret submitted as the mask is replaced by the + * stored (still-encrypted) value for the same backend name. A mask with no + * stored counterpart (rename, or a new backend) can never become a stored + * value — callers turn this throw into the 400. + */ +export function unmaskStorageConfig(config: StorageConfig, storedBackends: unknown): StorageConfig { + const stored = new Map>(); + if (Array.isArray(storedBackends)) { + for (const entry of storedBackends) { + if (isRecord(entry) && typeof entry.name === 'string' && isRecord(entry.options)) { + stored.set(entry.name, entry.options); + } + } + } + return { + ...config, + backends: config.backends.map((backend) => + mapSecrets(backend, (value, field) => { + if (value !== MASKED_SETTING_VALUE) return value; + const storedValue = stored.get(backend.name)?.[field]; + if (typeof storedValue !== 'string' || storedValue === '') { + throw new StorageBackendError( + `re-enter the secret '${field}' for '${backend.name}' — a mask can never become a stored value (new or renamed backend)`, + ); + } + return storedValue; + }), + ), + }; +} + +/** GET masking: secret fields always render as the sentinel, never a stored value. */ +export function maskBackendOptions( + type: StorageBackendTypeId, + options: Record, +): Record { + const masked = { ...options }; + for (const field of storageSecretFields(type)) masked[field] = MASKED_SETTING_VALUE; + return masked; +} + +/** Audit redaction: secret values become '***' (names and shapes survive). */ +export function redactStorageSecrets(config: StorageConfig): StorageConfig { + return { + ...config, + backends: config.backends.map((backend) => mapSecrets(backend, () => '***')), + }; +} diff --git a/server/tests/unit/app-config/derive.test.ts b/server/tests/unit/app-config/derive.test.ts index 7886637ed..38bcc248e 100644 --- a/server/tests/unit/app-config/derive.test.ts +++ b/server/tests/unit/app-config/derive.test.ts @@ -13,6 +13,7 @@ import { deriveBackup, deriveNet, derivePaths, + deriveSecurity, deriveAll, } from '../../../src/app-config/derive'; @@ -231,6 +232,14 @@ describe('derivePaths', () => { }); }); +describe('deriveSecurity', () => { + it('encryptionKeySet only for a non-blank explicit ENCRYPTION_KEY', () => { + expect(deriveSecurity({}).encryptionKeySet).toBe(false); + expect(deriveSecurity({ ENCRYPTION_KEY: ' ' }).encryptionKeySet).toBe(false); + expect(deriveSecurity({ ENCRYPTION_KEY: 'k' }).encryptionKeySet).toBe(true); + }); +}); + describe('deriveAll', () => { it('assembles every namespace', () => { const env = deriveAll({ PORT: '4000', DEMO_MODE: 'true' }); @@ -238,7 +247,7 @@ describe('deriveAll', () => { expect(env.demo.enabled).toBe(true); for (const ns of [ 'app', 'http', 'session', 'demo', 'adminBootstrap', 'oidc', 'smtp', 'mcp', - 'plugins', 'webauthn', 'integrations', 'backup', 'db', 'paths', 'net', + 'plugins', 'webauthn', 'integrations', 'backup', 'db', 'paths', 'security', 'net', ] as const) { expect(env[ns]).toBeDefined(); } diff --git a/server/tests/unit/nest/storage/storage-secrets.test.ts b/server/tests/unit/nest/storage/storage-secrets.test.ts new file mode 100644 index 000000000..d978e1661 --- /dev/null +++ b/server/tests/unit/nest/storage/storage-secrets.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('../../../../src/config', () => ({ ENCRYPTION_KEY: 'storage-secrets-test-key' })); + +import { MASKED_SETTING_VALUE, type StorageConfig } from '@trek/shared'; +import { + decrypt_api_key, + encrypt_api_key, +} from '../../../../src/nest/common/crypto/apiKeyCrypto'; +import { + assertNoMaskSentinels, + encryptionGateError, + encryptStorageSecrets, + listPlaintextSecrets, + maskBackendOptions, + redactStorageSecrets, + unmaskStorageConfig, +} from '../../../../src/nest/storage/storage-secrets'; + +const S3_OPTIONS = { + endpoint: 'http://127.0.0.1:9000', + bucket: 'trek', + accessKeyId: 'ak', + secretAccessKey: 'sk-plain', + region: 'us-east-1', + keyPrefix: '', + retries: 1, + timeoutMs: 30000, +}; + +function s3Config(secretAccessKey: string): StorageConfig { + return { + backends: [{ name: 'off-box', type: 's3', options: { ...S3_OPTIONS, secretAccessKey } }], + categories: { backups: 'off-box' }, + }; +} +const LOCAL_ONLY: StorageConfig = { + backends: [{ name: 'nas', type: 'local', options: { root: '/mnt/nas' } }], + categories: {}, +}; + +describe('listPlaintextSecrets', () => { + it('finds plaintext secrets, skips encrypted ones and secretless types', () => { + expect(listPlaintextSecrets(s3Config('sk-plain'))).toEqual([{ backend: 'off-box', field: 'secretAccessKey' }]); + expect(listPlaintextSecrets(s3Config(encrypt_api_key('sk')))).toEqual([]); + expect(listPlaintextSecrets(LOCAL_ONLY)).toEqual([]); + }); +}); + +describe('encryptionGateError', () => { + it('names ENCRYPTION_KEY, the backend, and the field', () => { + const message = encryptionGateError({ backend: 'off-box', field: 'secretAccessKey' }); + expect(message).toContain('ENCRYPTION_KEY'); + expect(message).toContain("'off-box'"); + expect(message).toContain("'secretAccessKey'"); + expect(message).toContain('fallback'); + }); +}); + +describe('encryptStorageSecrets', () => { + it('encrypts plaintext secrets and round-trips through decrypt', () => { + const out = encryptStorageSecrets(s3Config('sk-plain')); + const stored = (out.backends[0]!.options as Record).secretAccessKey as string; + expect(stored.startsWith('enc:v1:')).toBe(true); + expect(decrypt_api_key(stored)).toBe('sk-plain'); + }); + + it('is idempotent — an enc:v1: value passes through byte-for-byte', () => { + const cipher = encrypt_api_key('sk'); + const out = encryptStorageSecrets(s3Config(cipher)); + expect((out.backends[0]!.options as Record).secretAccessKey).toBe(cipher); + }); + + it('does not mutate its input', () => { + const input = s3Config('sk-plain'); + void encryptStorageSecrets(input); + expect((input.backends[0]!.options as Record).secretAccessKey).toBe('sk-plain'); + }); +}); + +describe('unmaskStorageConfig', () => { + it('replaces a mask echo with the stored ciphertext byte-for-byte', () => { + const cipher = encrypt_api_key('sk'); + const stored = [{ name: 'off-box', type: 's3', options: { ...S3_OPTIONS, secretAccessKey: cipher } }]; + const out = unmaskStorageConfig(s3Config(MASKED_SETTING_VALUE), stored); + expect((out.backends[0]!.options as Record).secretAccessKey).toBe(cipher); + }); + + it('throws for a mask with no stored counterpart (rename or new backend)', () => { + expect(() => unmaskStorageConfig(s3Config(MASKED_SETTING_VALUE), [])).toThrow( + "re-enter the secret 'secretAccessKey' for 'off-box'", + ); + const renamed = [{ name: 'old-name', type: 's3', options: { ...S3_OPTIONS, secretAccessKey: encrypt_api_key('sk') } }]; + expect(() => unmaskStorageConfig(s3Config(MASKED_SETTING_VALUE), renamed)).toThrow( + "re-enter the secret 'secretAccessKey' for 'off-box'", + ); + }); + + it('leaves non-mask values and secretless backends untouched, and tolerates a garbage stored row', () => { + expect(unmaskStorageConfig(s3Config('sk-new'), 'not-an-array')).toEqual(s3Config('sk-new')); + expect(unmaskStorageConfig(LOCAL_ONLY, undefined)).toEqual(LOCAL_ONLY); + }); +}); + +describe('assertNoMaskSentinels', () => { + it('throws when a seed-style config carries the mask sentinel', () => { + expect(() => assertNoMaskSentinels(s3Config(MASKED_SETTING_VALUE))).toThrow('mask'); + expect(() => assertNoMaskSentinels(s3Config('sk'))).not.toThrow(); + }); +}); + +describe('maskBackendOptions / redactStorageSecrets', () => { + it('masks exactly the secret fields on GET-shaped options', () => { + const masked = maskBackendOptions('s3', { ...S3_OPTIONS }); + expect(masked.secretAccessKey).toBe(MASKED_SETTING_VALUE); + expect(masked.accessKeyId).toBe('ak'); // never over-masked + expect(maskBackendOptions('local', { root: '/x' })).toEqual({ root: '/x' }); + }); + + it('redacts secrets to *** for audit details', () => { + const out = redactStorageSecrets(s3Config('sk-plain')); + expect((out.backends[0]!.options as Record).secretAccessKey).toBe('***'); + expect((out.backends[0]!.options as Record).accessKeyId).toBe('ak'); + }); +}); diff --git a/server/tests/unit/services/apiKeyCrypto.test.ts b/server/tests/unit/services/apiKeyCrypto.test.ts index 8434ffb57..0435f772f 100644 --- a/server/tests/unit/services/apiKeyCrypto.test.ts +++ b/server/tests/unit/services/apiKeyCrypto.test.ts @@ -7,7 +7,12 @@ vi.mock('../../../src/config', () => ({ updateJwtSecret: () => {}, })); -import { encrypt_api_key, decrypt_api_key, maybe_encrypt_api_key } from '../../../src/nest/common/crypto/apiKeyCrypto'; +import { + encrypt_api_key, + decrypt_api_key, + maybe_encrypt_api_key, + is_encrypted_api_key, +} from '../../../src/nest/common/crypto/apiKeyCrypto'; describe('apiKeyCrypto', () => { const PLAINTEXT_KEY = 'my-secret-api-key-12345'; @@ -76,4 +81,15 @@ describe('apiKeyCrypto', () => { expect(result).toBe(encrypted); }); }); + + describe('is_encrypted_api_key', () => { + it('is true exactly for enc:v1: envelopes', () => { + expect(is_encrypted_api_key(encrypt_api_key('s'))).toBe(true); + expect(is_encrypted_api_key('enc:v1:AAAA')).toBe(true); + expect(is_encrypted_api_key('plaintext')).toBe(false); + expect(is_encrypted_api_key('')).toBe(false); + expect(is_encrypted_api_key(null)).toBe(false); + expect(is_encrypted_api_key(42)).toBe(false); + }); + }); });