From ec57d3fe0f4fafe5d5f8edefe719cb298bc92b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Fri, 20 Feb 2026 14:41:31 +0100 Subject: [PATCH] Fix flaky 2FA encryption test for AES-256-CBC wrong-key behavior (#18126) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes a flaky test in `simple-secret-encryption.util.spec.ts` that fails ~1 in 256 runs - AES-256-CBC doesn't guarantee wrong-key decryption throws — PKCS7 padding validation is probabilistic. When padding accidentally looks valid, decryption silently returns garbage instead of throwing. - Changed the test to verify the correct security property: wrong-key decryption must never return the original secret (both throw and garbage are acceptable) - Audited both production `decryptSecret` call sites in `two-factor-authentication.service.ts` — they always use the correct key, so end users are not affected ## Test plan - [x] Test passes 5/5 consecutive runs locally - [x] Lint passes Made with [Cursor](https://cursor.com) Co-authored-by: Cursor --- .../utils/simple-secret-encryption.util.spec.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts index e45cb60609..9a025e3c68 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts @@ -82,12 +82,19 @@ describe('SimpleSecretEncryptionUtil', () => { expect(decrypted).toBe(specialSecret); }); - it('should fail to decrypt with wrong purpose', async () => { + it('should not recover original secret with wrong purpose', async () => { const encrypted = await util.encryptSecret(testSecret, testPurpose); - await expect( - util.decryptSecret(encrypted, 'wrong-purpose'), - ).rejects.toThrow(); + // AES-256-CBC may either throw (invalid padding) or produce garbage. + // Both outcomes are acceptable — the key property is that the original + // secret is never returned. + try { + const decrypted = await util.decryptSecret(encrypted, 'wrong-purpose'); + + expect(decrypted).not.toBe(testSecret); + } catch { + // Expected: wrong key produced invalid padding + } }); it('should fail to decrypt malformed encrypted data', async () => {