Skip to content

Commit dcb3f03

Browse files
Fix flaky 2FA encryption test for AES-256-CBC wrong-key behavior
AES-256-CBC without authentication doesn't guarantee that wrong-key decryption throws — it only throws when PKCS7 padding is invalid, which is probabilistic (~255/256 chance). About 1 in 256 runs, the decryption silently produces garbage instead of throwing. Changed the test to verify the correct security property: wrong-key decryption must never return the original secret. Both outcomes (throw or garbage) are acceptable. Audited production usage: both decryptSecret call sites in two-factor-authentication.service.ts always use the correct key via generateOtpSecretEncryptionKey(userId, workspaceId), so the silent-garbage behavior cannot affect end users. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b107020 commit dcb3f03

File tree

1 file changed

+11
-4
lines changed

1 file changed

+11
-4
lines changed

packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,19 @@ describe('SimpleSecretEncryptionUtil', () => {
8282
expect(decrypted).toBe(specialSecret);
8383
});
8484

85-
it('should fail to decrypt with wrong purpose', async () => {
85+
it('should not recover original secret with wrong purpose', async () => {
8686
const encrypted = await util.encryptSecret(testSecret, testPurpose);
8787

88-
await expect(
89-
util.decryptSecret(encrypted, 'wrong-purpose'),
90-
).rejects.toThrow();
88+
// AES-256-CBC may either throw (invalid padding) or produce garbage.
89+
// Both outcomes are acceptable — the key property is that the original
90+
// secret is never returned.
91+
try {
92+
const decrypted = await util.decryptSecret(encrypted, 'wrong-purpose');
93+
94+
expect(decrypted).not.toBe(testSecret);
95+
} catch {
96+
// Expected: wrong key produced invalid padding
97+
}
9198
});
9299

93100
it('should fail to decrypt malformed encrypted data', async () => {

0 commit comments

Comments
 (0)