Fix flaky 2FA encryption test for AES-256-CBC wrong-key behavior (#18126)

## 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 <cursoragent@cursor.com>
This commit is contained in:
Félix Malfait
2026-02-20 14:41:31 +01:00
committed by GitHub
parent 00209f7e2c
commit ec57d3fe0f
@@ -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 () => {