feat: implement V2 encryption and decryption methods in Encryptor class

[Artuomka]

No description provided.

[Copilot]

Pull request overview

This PR implements V2 encryption in the Encryptor class, upgrading from CryptoJS AES to Node.js native crypto with AES-256-GCM authenticated encryption. The changes maintain backward compatibility with the existing V1 (CryptoJS) format through automatic format detection and lazy migration.

Changes:

• Implemented V2 encryption using AES-256-GCM with PBKDF2 key derivation (100,000 iterations)
• Added version prefix '$v2:k1$' to identify the encryption format
• Maintained backward compatibility by detecting and decrypting V1 format data

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 20 comments.

File	Description
rocketadmin-agent/src/helpers/encryption/encryptor.ts	Implements V2 encryption/decryption methods for master password operations with backward compatibility
backend/src/helpers/encryption/encryptor.ts	Implements V2 encryption/decryption for both PRIVATE_KEY and master password operations with backward compatibility
backend/test/ava-tests/encryptor.test.ts	Comprehensive test suite covering V2 encryption, backward compatibility, edge cases, and utility functions

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

After converting the base64 strings to buffers, the buffer lengths should be validated to match expected sizes (salt should be 32 bytes, IV should be 16 bytes, authTag should be 16 bytes for GCM). Without validation, corrupted or malicious data could pass through with incorrect buffer sizes and cause unexpected behavior or security issues during decryption. Add validation: if (salt.length !== SALT_LENGTH || iv.length !== IV_LENGTH || authTag.length !== 16) { throw new Error('Invalid V2 encrypted data format: incorrect buffer sizes'); }

Suggested change

	const encrypted = Buffer.from(encryptedB64, 'base64');
	const encrypted = Buffer.from(encryptedB64, 'base64');
	if (salt.length !== SALT_LENGTH || iv.length !== IV_LENGTH || authTag.length !== 16) {
	throw new Error('Invalid V2 encrypted data format: incorrect buffer sizes');
	}

[Copilot]

The test should verify that data with a different version prefix (e.g., '$v2:k2$someciphertext') is rejected or handled correctly. Currently, due to a bug in the isV2Format method (which only checks for '$v2:' instead of the full '$v2:k1$'), such data would incorrectly be treated as valid V2 format and fail during decryption. Add a test case to verify proper handling of incompatible version prefixes.

[Copilot]

Returning null or undefined as-is when data is null/undefined can lead to type safety issues. The function signature returns 'string', but this code returns null or undefined. This could cause runtime errors in calling code that expects a string. Consider either: 1) changing the return type to 'string | null | undefined', 2) throwing an error for invalid inputs, or 3) returning an empty string. The same issue exists in encryptData and decryptData methods in this file.

[Copilot]

Returning null or undefined as-is when data is null/undefined can lead to type safety issues. The function signature returns 'string', but this code returns null or undefined. This could cause runtime errors in calling code that expects a string. Consider either: 1) changing the return type to 'string | null | undefined', 2) throwing an error for invalid inputs, or 3) returning an empty string.

[Copilot]

The version prefix check is inconsistent with the defined constant. The constant ENCRYPTION_VERSION_PREFIX is set to '$v2:k1$' but this method only checks for '$v2:'. This means data encrypted with the full prefix '$v2:k1$' will be correctly identified as V2 format, but if data were prefixed with just '$v2:' followed by something else (like '$v2:k2$'), it would incorrectly be treated as V2 format and fail during decryption when the substring operation removes too few characters. The check should use the full constant: return encryptedData.startsWith(ENCRYPTION_VERSION_PREFIX);

Suggested change

	return encryptedData.startsWith('$v2:');
	return encryptedData.startsWith(ENCRYPTION_VERSION_PREFIX);

[Copilot]

After converting the base64 strings to buffers, the buffer lengths should be validated to match expected sizes (salt should be 32 bytes, IV should be 16 bytes, authTag should be 16 bytes for GCM). Without validation, corrupted or malicious data could pass through with incorrect buffer sizes and cause unexpected behavior or security issues during decryption. Add validation: if (salt.length !== SALT_LENGTH || iv.length !== IV_LENGTH || authTag.length !== 16) { throw new Error('Invalid V2 encrypted data format: incorrect buffer sizes'); }

Suggested change

	const encrypted = Buffer.from(encryptedB64, 'base64');
	const encrypted = Buffer.from(encryptedB64, 'base64');
	if (salt.length !== SALT_LENGTH || iv.length !== IV_LENGTH || authTag.length !== 16) {
	throw new Error('Invalid V2 encrypted data format: incorrect buffer sizes');
	}

[Copilot]

The biome-ignore comment has an empty explanation placeholder. Either provide a meaningful explanation for why the 'noStaticOnlyClass' lint rule is being ignored (e.g., 'This class serves as a utility namespace for encryption methods and doesn't require instantiation'), or consider if the class design could be improved to avoid needing this suppression.

Suggested change

	/** biome-ignore-all lint/complexity/noStaticOnlyClass: <explanation> */
	/** biome-ignore-all lint/complexity/noStaticOnlyClass: This class intentionally serves as a static utility namespace for encryption and hashing helpers and is never instantiated. */

[Copilot]

The decryptDataMasterPwd method does not have error handling with try-catch. If decryption fails (e.g., wrong password, corrupted data), the method will throw a raw error from the crypto module which may not provide user-friendly error messages. Consider wrapping the decryption calls in a try-catch block and throwing a more descriptive error, similar to how it's done in the backend version of this file (backend/src/helpers/encryption/encryptor.ts lines 111-119).

Suggested change

	if (Encryptor.isV2Format(encryptedData)) {
	return Encryptor.decryptDataV2(encryptedData, masterPwd);
	}
	return Encryptor.decryptDataV1Legacy(encryptedData, masterPwd);
	try {
	if (Encryptor.isV2Format(encryptedData)) {
	return Encryptor.decryptDataV2(encryptedData, masterPwd);
	}
	return Encryptor.decryptDataV1Legacy(encryptedData, masterPwd);
	} catch (_err) {
	throw new Error(Messages.CORRUPTED_DATA);
	}

[Copilot]

The base64 decoding from user-provided input doesn't have error handling. If any of the base64 strings are malformed, Buffer.from() will silently decode what it can and may produce incorrect buffer lengths. This should be caught by subsequent operations (createDecipheriv, setAuthTag), but it would be better to validate the base64 format explicitly or wrap the buffer creation in a try-catch to provide clearer error messages. Consider adding validation or explicit error handling around the Buffer.from() calls.

[Copilot]

Returning null or undefined as-is when data is null/undefined can lead to type safety issues. The function signature returns 'string', but this code returns null or undefined. This could cause runtime errors in calling code that expects a string. Consider either: 1) changing the return type to 'string | null | undefined', 2) throwing an error for invalid inputs, or 3) returning an empty string. The same issue exists in decryptDataMasterPwd below.

Suggested change

	public static encryptDataMasterPwd(data: string, masterPwd: string): string {
	if (data === null || data === undefined) {
	return data;
	}
	return Encryptor.encryptDataV2(data, masterPwd);
	}
	public static decryptDataMasterPwd(encryptedData: string, masterPwd: string): string {
	public static encryptDataMasterPwd(data: string, masterPwd: string): string | null | undefined {
	if (data === null || data === undefined) {
	return data;
	}
	return Encryptor.encryptDataV2(data, masterPwd);
	}
	public static decryptDataMasterPwd(encryptedData: string, masterPwd: string,): string | null | undefined {