From 223560a7ec5be650c16a56cad73f8d7a92c1f7a0 Mon Sep 17 00:00:00 2001 From: oyi77 Date: Mon, 4 May 2026 19:39:39 +0700 Subject: [PATCH] fix: address PR review feedback --- src/lib/db/encryption.ts | 15 ++-- .../unit}/encryption.test.ts | 72 ++++++++++--------- vitest.config.ts | 2 +- 3 files changed, 45 insertions(+), 44 deletions(-) rename {src/lib/db/__tests__ => tests/unit}/encryption.test.ts (89%) diff --git a/src/lib/db/encryption.ts b/src/lib/db/encryption.ts index a392f7f3f4..bd937a4ef0 100644 --- a/src/lib/db/encryption.ts +++ b/src/lib/db/encryption.ts @@ -35,6 +35,11 @@ const STATIC_SALT = "omniroute-field-encryption-v1"; let _staticKey: Buffer | null = null; let _legacyDynamicKey: Buffer | null = null; +// Module-level migration flag. Safe in Node.js because: +// 1. Node.js is single-threaded — no concurrent access race conditions +// 2. decrypt() is synchronous — no interleaving between flag-set and flag-read +// 3. Used as a "check-after-decrypt" signal, not a persistent state dependency +// 4. Same pattern as _staticKey/_legacyDynamicKey cache variables above let _migrationNeeded = false; /** Connection object with potentially encrypted credential fields. */ @@ -55,15 +60,7 @@ function getStaticKey(): Buffer | null { if (_staticKey !== null) return _staticKey; const secret = process.env.STORAGE_ENCRYPTION_KEY; - if (!secret) return null; - - if (typeof secret !== "string" || secret.trim().length === 0) { - console.error( - "[Encryption] STORAGE_ENCRYPTION_KEY is set but empty or invalid. " + - "Generate a valid key with: openssl rand -base64 32" - ); - return null; - } + if (!secret || typeof secret !== "string" || secret.trim().length === 0) return null; try { _staticKey = scryptSync(secret, STATIC_SALT, KEY_LENGTH); diff --git a/src/lib/db/__tests__/encryption.test.ts b/tests/unit/encryption.test.ts similarity index 89% rename from src/lib/db/__tests__/encryption.test.ts rename to tests/unit/encryption.test.ts index ed76bc48a9..00da12c739 100644 --- a/src/lib/db/__tests__/encryption.test.ts +++ b/tests/unit/encryption.test.ts @@ -39,7 +39,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt } = await import("../encryption"); + const { encrypt, decrypt } = await import("@/lib/db/encryption"); const plaintext = "my-secret-api-key"; const encrypted = encrypt(plaintext); @@ -56,7 +56,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt } = await import("../encryption"); + const { encrypt, decrypt } = await import("@/lib/db/encryption"); const values = ["token1", "token2", "token3"]; const encrypted = values.map((v) => encrypt(v)); @@ -77,7 +77,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", secret); vi.resetModules(); - const { decrypt } = await import("../encryption"); + const { decrypt } = await import("@/lib/db/encryption"); const decrypted = decrypt(legacyEncrypted); expect(decrypted).toBe(plaintext); @@ -92,7 +92,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", secret); vi.resetModules(); - const { decrypt } = await import("../encryption"); + const { decrypt } = await import("@/lib/db/encryption"); const decrypted = legacyEncrypted.map((e) => decrypt(e)); expect(decrypted).toEqual(values); @@ -108,7 +108,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", secret); vi.resetModules(); - const { decrypt, isMigrationNeeded } = await import("../encryption"); + const { decrypt, isMigrationNeeded } = await import("@/lib/db/encryption"); expect(isMigrationNeeded()).toBe(false); @@ -121,7 +121,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt, isMigrationNeeded } = await import("../encryption"); + const { encrypt, decrypt, isMigrationNeeded } = await import("@/lib/db/encryption"); const plaintext = "modern-token"; const encrypted = encrypt(plaintext); @@ -143,7 +143,8 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", secret); vi.resetModules(); - const { decrypt, isMigrationNeeded, resetMigrationFlag } = await import("../encryption"); + const { decrypt, isMigrationNeeded, resetMigrationFlag } = + await import("@/lib/db/encryption"); decrypt(legacyEncrypted); expect(isMigrationNeeded()).toBe(true); @@ -158,7 +159,7 @@ describe("encryption module", () => { // No STORAGE_ENCRYPTION_KEY set vi.resetModules(); - const { encrypt, decrypt, isEncryptionEnabled } = await import("../encryption"); + const { encrypt, decrypt, isEncryptionEnabled } = await import("@/lib/db/encryption"); expect(isEncryptionEnabled()).toBe(false); @@ -174,7 +175,7 @@ describe("encryption module", () => { it("should handle null and undefined in passthrough mode", async () => { vi.resetModules(); - const { encrypt, decrypt } = await import("../encryption"); + const { encrypt, decrypt } = await import("@/lib/db/encryption"); expect(encrypt(null)).toBeNull(); expect(encrypt(undefined)).toBeUndefined(); @@ -188,7 +189,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encryptConnectionFields } = await import("../encryption"); + const { encryptConnectionFields } = await import("@/lib/db/encryption"); const conn = { id: "conn-123", @@ -211,7 +212,8 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encryptConnectionFields, decryptConnectionFields } = await import("../encryption"); + const { encryptConnectionFields, decryptConnectionFields } = + await import("@/lib/db/encryption"); const conn = { id: "conn-123", @@ -235,7 +237,8 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encryptConnectionFields, decryptConnectionFields } = await import("../encryption"); + const { encryptConnectionFields, decryptConnectionFields } = + await import("@/lib/db/encryption"); expect(encryptConnectionFields(null)).toBeNull(); expect(encryptConnectionFields(undefined)).toBeUndefined(); @@ -247,9 +250,10 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encryptConnectionFields, decryptConnectionFields } = await import("../encryption"); + const { encryptConnectionFields, decryptConnectionFields } = + await import("@/lib/db/encryption"); - const conn: import("../encryption").ConnectionFields = { + const conn: import("@/lib/db/encryption").ConnectionFields = { id: "conn-123", apiKey: "plain-api-key", // accessToken, refreshToken, idToken missing @@ -271,7 +275,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", secret); vi.resetModules(); - const { decryptConnectionFields, isMigrationNeeded } = await import("../encryption"); + const { decryptConnectionFields, isMigrationNeeded } = await import("@/lib/db/encryption"); const conn = { id: "conn-123", @@ -291,7 +295,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt } = await import("../encryption"); + const { encrypt, decrypt } = await import("@/lib/db/encryption"); expect(encrypt(null)).toBeNull(); expect(encrypt(undefined)).toBeUndefined(); @@ -303,7 +307,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt } = await import("../encryption"); + const { encrypt } = await import("@/lib/db/encryption"); const plaintext = "my-secret"; const encrypted = encrypt(plaintext); @@ -319,7 +323,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { decrypt } = await import("../encryption"); + const { decrypt } = await import("@/lib/db/encryption"); const malformed = "enc:v1:onlyonepart"; const result = decrypt(malformed); @@ -330,7 +334,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { decrypt } = await import("../encryption"); + const { decrypt } = await import("@/lib/db/encryption"); const malformed = "enc:v1:notvalidhex:notvalidhex:notvalidhex"; const result = decrypt(malformed); @@ -341,7 +345,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt } = await import("../encryption"); + const { encrypt, decrypt } = await import("@/lib/db/encryption"); const plaintext = "my-secret"; const encrypted = encrypt(plaintext); @@ -359,7 +363,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { decrypt } = await import("../encryption"); + const { decrypt } = await import("@/lib/db/encryption"); const plaintext = "not-encrypted-value"; const result = decrypt(plaintext); @@ -370,7 +374,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt } = await import("../encryption"); + const { encrypt } = await import("@/lib/db/encryption"); const plaintext = "my-secret"; const encrypted = encrypt(plaintext); @@ -379,7 +383,7 @@ describe("encryption module", () => { vi.unstubAllEnvs(); vi.resetModules(); - const { decrypt } = await import("../encryption"); + const { decrypt } = await import("@/lib/db/encryption"); const result = decrypt(encrypted!); expect(result).toBeNull(); @@ -390,7 +394,7 @@ describe("encryption module", () => { it("should return valid when no key is set (passthrough mode)", async () => { vi.resetModules(); - const { validateEncryptionConfig } = await import("../encryption"); + const { validateEncryptionConfig } = await import("@/lib/db/encryption"); const result = validateEncryptionConfig(); expect(result.valid).toBe(true); @@ -401,7 +405,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { validateEncryptionConfig } = await import("../encryption"); + const { validateEncryptionConfig } = await import("@/lib/db/encryption"); const result = validateEncryptionConfig(); expect(result.valid).toBe(true); @@ -412,7 +416,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", ""); vi.resetModules(); - const { validateEncryptionConfig } = await import("../encryption"); + const { validateEncryptionConfig } = await import("@/lib/db/encryption"); const result = validateEncryptionConfig(); expect(result.valid).toBe(true); @@ -423,7 +427,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", " "); vi.resetModules(); - const { validateEncryptionConfig } = await import("../encryption"); + const { validateEncryptionConfig } = await import("@/lib/db/encryption"); const result = validateEncryptionConfig(); expect(result.valid).toBe(false); @@ -435,7 +439,7 @@ describe("encryption module", () => { it("should return false when no key is set", async () => { vi.resetModules(); - const { isEncryptionEnabled } = await import("../encryption"); + const { isEncryptionEnabled } = await import("@/lib/db/encryption"); expect(isEncryptionEnabled()).toBe(false); }); @@ -444,7 +448,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { isEncryptionEnabled } = await import("../encryption"); + const { isEncryptionEnabled } = await import("@/lib/db/encryption"); expect(isEncryptionEnabled()).toBe(true); }); @@ -453,7 +457,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", ""); vi.resetModules(); - const { isEncryptionEnabled } = await import("../encryption"); + const { isEncryptionEnabled } = await import("@/lib/db/encryption"); // isEncryptionEnabled only checks if the env var is truthy expect(isEncryptionEnabled()).toBe(false); @@ -465,7 +469,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt, isMigrationNeeded } = await import("../encryption"); + const { encrypt, decrypt, isMigrationNeeded } = await import("@/lib/db/encryption"); const plaintext = "new-token"; const encrypted = encrypt(plaintext); @@ -481,7 +485,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", "test-secret-key-12345"); vi.resetModules(); - const { encrypt, decrypt, isMigrationNeeded } = await import("../encryption"); + const { encrypt, decrypt, isMigrationNeeded } = await import("@/lib/db/encryption"); const values = ["token1", "token2", "token3"]; const encrypted = values.map((v) => encrypt(v)); @@ -506,7 +510,7 @@ describe("encryption module", () => { vi.stubEnv("STORAGE_ENCRYPTION_KEY", secret); vi.resetModules(); - const { decrypt, isMigrationNeeded } = await import("../encryption"); + const { decrypt, isMigrationNeeded } = await import("@/lib/db/encryption"); expect(isMigrationNeeded()).toBe(false); @@ -528,7 +532,7 @@ describe("encryption module", () => { vi.resetModules(); const { decrypt, encrypt, isMigrationNeeded, resetMigrationFlag } = - await import("../encryption"); + await import("@/lib/db/encryption"); // Decrypt legacy value const decrypted = decrypt(legacyEncrypted); diff --git a/vitest.config.ts b/vitest.config.ts index dcc32a55e7..34b035d509 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -11,7 +11,7 @@ export default defineConfig({ "src/app/**/dashboard/endpoint/__tests__/**/*.test.tsx", "src/lib/memory/__tests__/**/*.test.ts", "src/lib/skills/__tests__/**/*.test.ts", - "src/lib/db/__tests__/**/*.test.ts", + "tests/unit/encryption.test.ts", "open-sse/**/__tests__/**/*.test.ts", "open-sse/services/**/__tests__/**/*.test.ts", "tests/e2e/ecosystem.test.ts",