From 39f95015ee6bf29dc29bdec2d2b65c023ce4ade2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 00:20:25 +0000 Subject: [PATCH] fix(encryption): fix cache key bug and add missing tests Fix critical bug in decryptBatch where salt.buffer returned the entire ArrayBuffer instead of just the 16-byte salt, defeating key caching. - Use salt.slice().buffer to create proper cache key - Add tests for encryptPayload() and decryptPayload() methods - Fix test assertion to verify reference identity with toBe() --- src/app/op-log/encryption/encryption.ts | 4 +- .../sync/operation-encryption.service.spec.ts | 45 +++++++++++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/app/op-log/encryption/encryption.ts b/src/app/op-log/encryption/encryption.ts index f7663d1955..2beb93542c 100644 --- a/src/app/op-log/encryption/encryption.ts +++ b/src/app/op-log/encryption/encryption.ts @@ -359,7 +359,9 @@ export const decryptBatch = async ( // Argon2 format: extract salt and use cached key const salt = new Uint8Array(dataBuffer, 0, SALT_LENGTH); - const saltKey = ab2base64(salt.buffer); + // IMPORTANT: Use slice() to create a copy with its own ArrayBuffer. + // Without slice(), salt.buffer returns the entire dataBuffer, not just the salt! + const saltKey = ab2base64(salt.slice().buffer); // Check cache for this salt let keyInfo = keyCache.get(saltKey); diff --git a/src/app/op-log/sync/operation-encryption.service.spec.ts b/src/app/op-log/sync/operation-encryption.service.spec.ts index 345c2ece24..5cabd83fd1 100644 --- a/src/app/op-log/sync/operation-encryption.service.spec.ts +++ b/src/app/op-log/sync/operation-encryption.service.spec.ts @@ -191,7 +191,7 @@ describe('OperationEncryptionService', () => { expect(decrypted).toEqual([]); }); - it('should return all operations unchanged when none are encrypted', async () => { + it('should return same array reference when none are encrypted', async () => { const ops = [ createMockSyncOp({ title: 'Plain 1' }), createMockSyncOp({ title: 'Plain 2' }), @@ -200,9 +200,9 @@ describe('OperationEncryptionService', () => { const result = await service.decryptOperations(ops, TEST_PASSWORD); - // Should return the same array (or equivalent) + // Should return the same array reference (optimization: no copying needed) + expect(result).toBe(ops); expect(result.length).toBe(3); - expect(result).toEqual(ops); }); it('should throw DecryptError for malformed encrypted operation (non-string payload)', async () => { @@ -229,6 +229,45 @@ describe('OperationEncryptionService', () => { }); }); + describe('encryptPayload / decryptPayload', () => { + it('should encrypt and decrypt an object payload', async () => { + const payload = { foo: 'bar', count: 42 }; + const encrypted = await service.encryptPayload(payload, TEST_PASSWORD); + + expect(typeof encrypted).toBe('string'); + expect(encrypted).not.toContain('foo'); // Should be encrypted + + const decrypted = await service.decryptPayload(encrypted, TEST_PASSWORD); + expect(decrypted).toEqual(payload); + }); + + it('should encrypt and decrypt an array payload', async () => { + const payload = [1, 2, 3, { nested: true }]; + const encrypted = await service.encryptPayload(payload, TEST_PASSWORD); + const decrypted = await service.decryptPayload( + encrypted, + TEST_PASSWORD, + ); + + expect(decrypted).toEqual(payload); + }); + + it('should throw DecryptError for wrong password', async () => { + const payload = { secret: 'data' }; + const encrypted = await service.encryptPayload(payload, TEST_PASSWORD); + + await expectAsync( + service.decryptPayload(encrypted, 'wrong-password'), + ).toBeRejectedWithError(DecryptError); + }); + + it('should throw DecryptError for corrupted ciphertext', async () => { + await expectAsync( + service.decryptPayload('invalid-ciphertext!!!', TEST_PASSWORD), + ).toBeRejectedWithError(DecryptError); + }); + }); + describe('round-trip with various payload types', () => { it('should handle null payload', async () => { const op = createMockSyncOp(null);