fix: 🐛 fix error handling, type safety, security vulnerabilities, and add test coverage (#44)

Co-authored-by: lehuygiang28 <58503884+lehuygiang28@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Author: Copilot

package-lock.json

@@ -83,7 +83,7 @@ export class VerificationService {
 
         let outputResults = {
             isVerified,
-            isSuccess: cloneQuery.vnp_ResponseCode === '00',
+            isSuccess: cloneQuery.vnp_ResponseCode === '00' || cloneQuery.vnp_ResponseCode === 0,
             message: getResponseByStatusCode(
                 cloneQuery.vnp_ResponseCode?.toString() ?? '',
                 this.config.vnp_Locale,

src/services/verification.service.ts

@@ -185,12 +185,21 @@ export class VNPay {
             },
         );
 
+        if (!response.ok) {
+            throw new Error(`Failed to fetch bank list: HTTP ${response.status}`);
+        }
+
         const bankList = (await response.json()) as Bank[];
 
         for (const bank of bankList) {
+            // Handle logo_link safely - only slice if it starts with a slash
+            const logoPath =
+                bank.logo_link && bank.logo_link.startsWith('/')
+                    ? bank.logo_link.slice(1)
+                    : bank.logo_link;
             bank.logo_link = resolveUrlString(
                 this.globalConfig.vnpayHost ?? VNPAY_GATEWAY_SANDBOX_HOST,
-                bank.logo_link.slice(1),
+                logoPath,
             );
         }
 

src/vnpay.ts

@@ -1,5 +1,9 @@
-import { createTestVNPayInstance, DEFAULT_BANK_LIST, DEFAULT_VNPAY_CONFIG } from '../../__helpers__';
-import { mockFetchSuccess } from '../../__helpers__/mock-helpers';
+import {
+    createTestVNPayInstance,
+    DEFAULT_BANK_LIST,
+    DEFAULT_VNPAY_CONFIG,
+} from '../../__helpers__';
+import { mockFetchSuccess, mockFetchError } from '../../__helpers__/mock-helpers';
 
 global.fetch = jest.fn();
 
@@ -52,4 +56,52 @@ describe('VNPay.getBankList', () => {
             },
         );
     });
+
+    it('should throw error when fetch fails with non-ok response', async () => {
+        // Arrange
+        mockFetchError(500, 'Internal Server Error');
+
+        // Act & Assert
+        await expect(vnpay.getBankList()).rejects.toThrow('Failed to fetch bank list: HTTP 500');
+    });
+
+    it('should handle logo_link without leading slash', async () => {
+        // Arrange
+        const mockBankList = [
+            {
+                bank_code: 'TCB',
+                bank_name: 'Techcombank',
+                logo_link: 'images/tcb.png', // No leading slash
+                bank_type: 1,
+                display_order: 1,
+            },
+        ];
+        mockFetchSuccess(mockBankList);
+
+        // Act
+        const bankList = await vnpay.getBankList();
+
+        // Assert
+        expect(bankList[0].logo_link).toBe(`${DEFAULT_VNPAY_CONFIG.vnpayHost}/images/tcb.png`);
+    });
+
+    it('should handle empty logo_link', async () => {
+        // Arrange
+        const mockBankList = [
+            {
+                bank_code: 'ACB',
+                bank_name: 'ACB Bank',
+                logo_link: '', // Empty string
+                bank_type: 1,
+                display_order: 1,
+            },
+        ];
+        mockFetchSuccess(mockBankList);
+
+        // Act
+        const bankList = await vnpay.getBankList();
+
+        // Assert
+        expect(bankList[0].logo_link).toBe(`${DEFAULT_VNPAY_CONFIG.vnpayHost}/`);
+    });
 });

test/integration/vnpay/get-bank-list.test.ts

@@ -266,4 +266,33 @@ describe('verifyReturnUrl', () => {
         expect(result.isSuccess).toBe(true);
         expect(result.message).toBe(getResponseByStatusCode('00', VnpLocale.VN));
     });
+
+    it('should handle vnp_ResponseCode as number 0 and mark as success', () => {
+        // Arrange: Create input with vnp_ResponseCode as number 0 instead of string '00'
+        const base = createReturnQueryInput();
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        const { vnp_SecureHash: _oldHash, ...clone } = base as any;
+        clone.vnp_ResponseCode = 0; // Number 0 instead of string '00'
+
+        const search = buildPaymentUrlSearchParams(clone);
+        const newHash = calculateSecureHash({
+            secureSecret: 'test_secret',
+            data: search.toString(),
+            hashAlgorithm: HashAlgorithm.SHA512,
+            bufferEncode: 'utf-8',
+        });
+
+        const input = {
+            ...clone,
+            vnp_SecureHash: newHash,
+        } as any;
+
+        // Act
+        const result = vnpay.verifyReturnUrl(input);
+
+        // Assert
+        expect(result.isVerified).toBe(true);
+        expect(result.isSuccess).toBe(true);
+        expect(result.message).toBe(getResponseByStatusCode('0', VnpLocale.VN));
+    });
 });