From 288569297f229cddbf93651ecf4fca43f46364a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 19 Jun 2026 20:55:46 +0200 Subject: [PATCH] fix(crypto): fail closed on weak dev key outside development/test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev-only SHA-256 key padding was gated on the absence of a production marker, so a staging or preview container with NODE_ENV unset silently derived a weak key the operator never recorded — rotating to a real key later orphaned those rows (decrypt is fail-closed). Gate the padding on an explicit NODE_ENV of development or test; every other case, including unset NODE_ENV, throws the same hard error as in production. --- src/lib/__tests__/crypto.test.ts | 36 ++++++++++++++++++++++++++++++++ src/lib/crypto.ts | 12 +++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/lib/__tests__/crypto.test.ts b/src/lib/__tests__/crypto.test.ts index 25efbcd5c..fc2a0a299 100644 --- a/src/lib/__tests__/crypto.test.ts +++ b/src/lib/__tests__/crypto.test.ts @@ -132,4 +132,40 @@ describe("crypto (AES-256-GCM, key versioning)", () => { _resetCryptoCacheForTests(); expect(() => encrypt("test")).toThrow(/no matching entry/); }); + + // A 40-char hex string is too short for a 256-bit key but long enough to + // reach the dev-only SHA-256 padding branch (base64-decodes to 30 bytes, so + // it falls through to the >=32-hex path). It must only pad in development / + // test and otherwise fail closed. + const SHORT_HEX_KEY = "0123456789abcdef0123456789abcdef01234567"; + + it("fails closed when NODE_ENV is unset and only a short hex key is set", () => { + vi.stubEnv("NODE_ENV", undefined); + vi.stubEnv("ENCRYPTION_KEYS", ""); + vi.stubEnv("ENCRYPTION_ACTIVE_KEY_ID", ""); + vi.stubEnv("ENCRYPTION_KEY", SHORT_HEX_KEY); + _resetCryptoCacheForTests(); + expect(() => encrypt("test")).toThrow( + /must be 64 hex characters .* outside development\/test/, + ); + }); + + it("fails closed for a short hex key in production", () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("ENCRYPTION_KEYS", ""); + vi.stubEnv("ENCRYPTION_ACTIVE_KEY_ID", ""); + vi.stubEnv("ENCRYPTION_KEY", SHORT_HEX_KEY); + _resetCryptoCacheForTests(); + expect(() => encrypt("test")).toThrow(/outside development\/test/); + }); + + it("still pads a short hex key in development/test for local use", () => { + vi.stubEnv("NODE_ENV", "development"); + vi.stubEnv("ENCRYPTION_KEYS", ""); + vi.stubEnv("ENCRYPTION_ACTIVE_KEY_ID", ""); + vi.stubEnv("ENCRYPTION_KEY", SHORT_HEX_KEY); + _resetCryptoCacheForTests(); + const ct = encrypt("dev-secret"); + expect(decrypt(ct)).toBe("dev-secret"); + }); }); diff --git a/src/lib/crypto.ts b/src/lib/crypto.ts index 913871e9a..b2bba0cb4 100644 --- a/src/lib/crypto.ts +++ b/src/lib/crypto.ts @@ -59,9 +59,17 @@ function decodeKey(raw: string, label: string): Buffer { } } if (/^[0-9a-fA-F]+$/.test(raw) && raw.length >= 32) { - if (process.env.NODE_ENV === "production") { + // Fail closed unless we are explicitly in a local dev or test run. The + // SHA-256 padding below derives a key the operator never recorded; if a + // staging/preview container runs with NODE_ENV unset it would silently + // encrypt under that derived key, and rotating to a real key later + // orphans those rows (decrypt is fail-closed → undecryptable). Only the + // two known throwaway-data environments may take the padding path; every + // other case (production AND unset NODE_ENV) throws the same hard error. + const env = process.env.NODE_ENV; + if (env !== "development" && env !== "test") { throw new Error( - `Encryption key '${label}' must be 64 hex characters (256 bits) in production. ` + + `Encryption key '${label}' must be 64 hex characters (256 bits) outside development/test. ` + `Generate one with: openssl rand -hex 32`, ); }