Skip to content

Simpaisa API Best Practices Audit Report

Date: 2026-04-03 Scope: Disbursement SA v1.0, Remittance SA v1.0, PayIn PK Technical Specs Mode: HOLD SCOPE (make what exists bulletproof) Reviewer: CDO Review Status: Updated with source code findings (supersedes spec-only review of 2026-03-31)

Executive Summary

Simpaisa operates three API families: Disbursement (domestic PK payouts), Remittance (cross-border transfers), and PayIn (wallet-based collection), plus a fourth — Card (card payment processing) — identified during source code review.

The original spec-only audit (2026-03-31) identified 25 findings and described the underlying architecture as "solid." The source code review tells a fundamentally different story. The architecture is functional — it processes live payments — but it has critical security vulnerabilities that require immediate remediation:

  • No Spring Security in any service. Authentication is ad-hoc or absent entirely.
  • Hardcoded production credentials across multiple repos, committed to Git (Twilio, partner banks, MySQL, SMTP, JazzCash, Easypaisa, 1Link).
  • SSL certificate validation disabled on connections to partner banks (remittance), enabling man-in-the-middle attacks.
  • AES-ECB encryption (insecure) in three services handling payment data.
  • No database transactions on financial operations in remittance — balance deductions and state updates can partially fail.
  • Redis everywhere — the stated migration to SurrealDB (in-memory) has not been adopted in any production service.

37 findings total. 10 Critical, 11 High, 10 Medium, 6 Low. This is up from the original 25 findings (6 Critical, 8 High, 7 Medium, 4 Low) — the code review both upgraded existing severities and uncovered 12 entirely new findings that were invisible from specs alone.

The critical items are no longer just documentation gaps. They are active security vulnerabilities and financial integrity risks in production code.

Severity Definitions

Severity Meaning
CRITICAL Will cause production incidents, duplicate payments, security issues, or major integration failures. Includes active vulnerabilities found in source code.
HIGH Causes significant merchant friction, increases support load, or violates REST best practices. Includes code-level risks to financial integrity.
MEDIUM Inconsistency or missing detail that slows integration but has workarounds
LOW Style/convention issues worth fixing but not blocking

Finding 1: Idempotency — Optional, Fails Open on Redis Failure

Severity: CRITICAL (unchanged, but confirmed worse than spec suggested) Affects: Disbursement, Remittance, PayIn

None of the three APIs document idempotency keys for POST endpoints that create transactions. This is the #1 rule for payment APIs.

What goes wrong: A merchant calls POST /disbursements/initiate, gets a network timeout, retries ... and creates a duplicate payout. Real money leaves twice. This happens in production, regularly.

Code Reality

The source code reveals idempotency does exist in the wallet service via an @IdempotentAnnonation aspect — but it is critically flawed:

  1. Optional by default: If the Request-Id header is absent, the request proceeds without any deduplication. There is no enforcement.
  2. Fails open on Redis failure: The catch block (line 83-86) falls through to pjp.proceed() when Redis is unavailable. If Redis goes down, idempotency is silently disabled and duplicate payments can be created.
  3. No idempotency in remittance at all: The Kafka consumer has no deduplication check. With Kafka's at-least-once delivery, the same remittance can be processed twice. The inConsumer flag is set after processing, creating a race window.

This is worse than "not documented" — it is implemented in a way that gives false confidence while failing under the exact conditions (infrastructure instability) when idempotency matters most.

Best Practice: Every payment-initiating POST endpoint must accept an Idempotency-Key header (or equivalent field like clientReferenceId). The server stores the key and returns the same response for duplicate requests within a time window (typically 24-48 hours).

Recommendation: - Make Request-Id / Idempotency-Key mandatory for all payment-initiating endpoints — reject requests without it (HTTP 400) - Fix the fail-open behaviour: if the idempotency store is unavailable, reject the request (fail closed) rather than proceeding without deduplication - Add idempotency to the Kafka consumer with a pre-processing deduplication check against a persistent store (not just an in-memory flag) - Add Idempotency-Key header to: /disbursements/initiate, /disbursements/register-customer, /{merchantId}/initiate, /{merchantId}/remit-initiate, /v2/wallets/transaction/initiate - Document the idempotency window (e.g., 48 hours) - Document behaviour on duplicate with same params: return original response with HTTP 200 - Document behaviour on duplicate with different params: return HTTP 409 Conflict

Finding 2: Error Handling — Exceptions Silently Swallowed, 200 OK on Failure

Severity: CRITICAL (upgraded from CRITICAL — now confirmed as active production risk) Affects: Disbursement, Remittance, PayIn, Card

None of the three specs define what an error response looks like. The Disbursement doc mentions an external_response table that maps third-party codes to internal codes, but the actual JSON structure merchants receive is undocumented.

Code Reality

The code is significantly worse than the spec suggested:

  1. No @ControllerAdvice or @ExceptionHandler in any service. Each controller has its own try-catch with inconsistent behaviour.
  2. Silent exception swallowing: Controller catch blocks catch Exception, call e.printStackTrace(), and return whatever is in the response variable (potentially empty) with HTTP 200 OK. Callers cannot distinguish success from failure.
  3. e.printStackTrace() everywhere instead of proper logging. Stack traces go to stdout/stderr, bypass Log4j2 appenders, and are lost in containerised environments. Over 40 instances in remittance alone.
  4. finally { return response; } anti-pattern in financial-critical methods (7 instances across remittance) — exceptions during remittance processing are silently swallowed and the method returns whatever partial state exists.
  5. No correlation IDs or distributed tracing — no MDC-based request tracing, no X-Request-Id propagation. Debugging a failed payment across services is manual guesswork.

Best Practice: Define a single error response schema used across all APIs:

Success response (HTTP 200/201): 

{
  "data": { ... },
  "traceId": "abc-123-def-456"
}

Error response (HTTP 4xx/5xx): 

{
  "error": {
    "code": "INSUFFICIENT_BALANCE",
    "message": "Merchant balance is insufficient for this disbursement",
    "details": [
      {
        "field": "amount",
        "issue": "Requested 50000 PKR but available balance is 12340 PKR"
      }
    ]
  },
  "traceId": "abc-123-def-456",
  "timestamp": "2026-04-03T12:00:00Z"
}

Recommendation: - Immediate: Add @ControllerAdvice with @ExceptionHandler in every service to enforce consistent error responses - Immediate: Remove all finally { return } blocks from financial methods — let exceptions propagate to the global handler - Replace all e.printStackTrace() with logger.error("Context message", e) (global find-and-replace, add Checkstyle/PMD rule to prevent regression) - Add correlation IDs (MDC-based X-Request-Id propagation) across all services - Define the error schema once in a shared API standards document - Include traceId in every response (maps to OpenSearch for debugging) - Use machine-readable error codes: INSUFFICIENT_BALANCE, INVALID_ACCOUNT, RATE_LIMIT_EXCEEDED, CHANNEL_UNAVAILABLE, etc.

Finding 3: Internal Implementation Details in Merchant-Facing Docs

Severity: CRITICAL Affects: PayIn (most severe), Disbursement, Remittance

The PayIn spec names internal database tables: merchant_details, product_configuration, transaction, api_logs, easypaisa_credentials, operator_token, recursion, payment_logs, wallet_transaction_threshold, product_webhook, external_response. The Disbursement doc describes a 5-minute scheduler interval and internal architecture components.

Code Reality

The code review confirms and extends this concern:

  1. Raw JDBC throughout — all three services use raw JDBC queries with string-concatenated SQL, exposing internal table structure in the code itself. No ORM abstraction exists.
  2. HashMap<String, Object> used as request/response bodies across all services — no typed DTOs, meaning internal field naming conventions leak directly through the API contract.
  3. No OpenAPI/Swagger — no springdoc-openapi or springfox in any service. No machine-readable API contract exists to enforce separation of internal and external schemas.

Recommendation: - Create two separate doc tiers: - Merchant Integration Guide (external): endpoints, auth, request/response schemas, error codes, webhooks, SDKs - Solution Architecture (internal): infrastructure, database schema, scheduler design, retry logic - Introduce typed DTOs (request/response classes) to all services — this provides compile-time safety and enables automatic OpenAPI generation - Add springdoc-openapi for machine-readable API contracts - Remove all table names, cache references, and scheduler details from merchant-facing docs

Finding 4: Webhooks — Implemented but No Retry, No Signing

Severity: CRITICAL (confirmed with code evidence) Affects: Disbursement, Remittance, PayIn

All three systems mention webhooks/postbacks but none document: payload format, delivery guarantees, retry policy, signature verification, or expected merchant response.

Code Reality

The wallet service has a MerchantPostbackService that sends merchant notifications, but:

  1. Single attempt only: One POST request with no retry. If the merchant's server is down, the postback is permanently lost.
  2. No webhook signing: No HMAC signature, no shared secret, no authenticity verification. Merchants cannot verify postbacks are genuinely from Simpaisa.
  3. Abandoned queue infrastructure: RabbitMQ queue constants exist (POSTBACK_QUEUE, POSTBACK_EXCHANGE) suggesting a queue-based retry approach was planned but never implemented.
  4. No dead letter handling: In remittance, the Kafka error handler's exhaustion callback is empty — when all retries fail, the message is silently dropped. No Dead Letter Topic is configured.

Best Practice: A webhook spec must include:

  1. Payload schema (exact JSON structure with all possible fields)
  2. Event types (e.g., disbursement.completed, disbursement.failed, remittance.status_changed)
  3. Signature verification (HMAC-SHA256 over raw body, with shared secret, timestamp to prevent replay)
  4. Delivery guarantees (at-least-once, with deduplication guidance)
  5. Retry policy (e.g., 3 retries at 1min, 5min, 30min intervals)
  6. Expected response (merchant must return HTTP 200 within 5 seconds)
  7. Timeout handling (what happens if merchant endpoint is down for 24 hours)

Recommendation: - Immediate: Implement webhook retry with exponential backoff (complete the RabbitMQ-based approach already partially built) - Add HMAC-SHA256 webhook signing with per-merchant shared secrets - Configure Kafka Dead Letter Topics for failed remittance messages - Write a shared Webhook Integration Guide - Include a webhook testing endpoint (merchant can trigger test events)

Finding 5: Authentication — No Spring Security, CORS Wide Open, Endpoints Unauthenticated

Severity: CRITICAL (upgraded — code reveals far worse than spec suggested) Affects: All APIs

The Disbursement doc mentions "API keys, secrets, or OAuth" and "HMAC/SHA signatures." The Remittance doc mentions "HMAC/SHA signature decryption and validation." The PayIn doc shows a JSESSIONID cookie in a curl example.

Code Reality

None of the API families include spring-boot-starter-security. There is:

  • No authentication filter chain
  • No CSRF protection
  • No security headers (Content-Security-Policy, X-Frame-Options, etc.)
  • No authorisation framework

Authentication is handled ad-hoc: - Wallet: Merchant ID lookup from request body (no actual authentication) - Card: client-id header lookup (trivially spoofable) - Remittance consumer: Nothing at all — Kafka messages are processed without any authentication - Card (Safepay endpoints): /fetch-details and /redirection are exposed with zero authentication — no client-id check, no signature, no IP whitelist. Anyone on the internet can call these endpoints. - Both wallet and card controllers have @CrossOrigin(origins = "*") — unrestricted CORS on server-to-server payment APIs.

The IP security filter in Card is disabled — IpSecurityFilter.doFilter() passes all requests through unconditionally. The validation logic and FilterConfig are commented out. The filter exists but does nothing.

The IP filter in Disbursement has a logic bug — it reads X-Forwarded-For into a variable but then checks getRemoteAddr() instead. Behind a load balancer, getRemoteAddr() returns the load balancer's IP, not the client's.

Best Practice: Authentication documentation and implementation must include:

  1. Credential provisioning (how merchants get API keys)
  2. Authentication mechanism (exact header names, format)
  3. Signature generation (step-by-step with code examples in 3+ languages)
  4. Timestamp requirements (clock skew tolerance)
  5. Key rotation (how to rotate without downtime)
  6. IP whitelisting (if applicable, how to manage)

Recommendation: - Emergency: Add Spring Security with a custom SecurityFilterChain to all services. Implement API key authentication at minimum, with RSA signature verification for payment-initiating endpoints. - Emergency: Authenticate Safepay endpoints (C-03) — these are completely open - Emergency: Fix the disbursement IP filter to use X-Forwarded-For behind load balancers - Emergency: Enable the card service IP security filter (currently commented out) - Remove @CrossOrigin(origins = "*") from all controllers — restrict to known merchant domains or disable entirely for server-to-server APIs - Standardise on one auth mechanism across all APIs (strongly recommend HMAC-SHA256) - Eliminate the JSESSIONID cookie approach in PayIn

Finding 6: No HTTP Status Codes — Code Returns 200 OK for Everything

Severity: CRITICAL (confirmed with code evidence) Affects: All APIs

Not a single endpoint across all three specs documents which HTTP status codes it returns.

Code Reality

The code confirms the worst case: there is effectively one status code — 200 OK. Controller catch blocks return 200 with empty or partial response bodies on failure. Some controllers inconsistently return 401 or 201, but there is no systematic use of HTTP status codes. No @ControllerAdvice exists to enforce consistent status code usage.

Best Practice: Every endpoint must document and return: - 200/201 for success (200 for queries, 201 for resource creation) - 400 for malformed requests - 401 for missing/invalid authentication - 403 for insufficient permissions - 404 for resource not found - 409 for conflict (duplicate idempotency key with different params) - 422 for business validation failures (e.g., insufficient balance) - 429 for rate limiting (with Retry-After header) - 500 for server errors - 502/503 for upstream channel failures

Recommendation: - Implement @ControllerAdvice in all services to enforce proper HTTP status codes - Document status codes per endpoint - Use 422 for business rule violations (not 400, which is for malformed syntax) - Always include Retry-After header with 429 responses

Finding 7: Inconsistent URL Patterns Across APIs

Severity: HIGH Affects: All APIs

Three different URL conventions:

API Pattern Example
Disbursement ${BASE_URL}/merchants/${merchantId}/disbursements/... /merchants/123/disbursements/initiate
Remittance ${BASE_URL}/${merchantId}/... /123/initiate
PayIn ${BASE_URL}/v2/wallets/transaction/... (merchantId in body) /v2/wallets/transaction/initiate

Code Reality

Confirmed. Additionally, the card service uses yet another pattern (/payments, /capture with client-id header). No API versioning exists in any service URL.

Recommendation: - Standardise on a single URL pattern for all APIs - merchantId should be in the path (not body) for proper REST resource identification and access control - Version in the URL path (not header) for simplicity in payment APIs - Migration: Run old and new URL patterns in parallel for 6-12 months

Finding 8: HTTP Method Misuse (POST for Queries)

Severity: HIGH Affects: Disbursement, Remittance

Several endpoints use POST for operations that should be GET:

Endpoint Current Should Be Why
/disbursements/fetch-account POST GET Fetches/queries account info, no state change
/{merchantId}/fetch POST GET with query params Searches remittances by filters
/{merchantId}/fetch-account POST GET Retrieves saved beneficiary details
/{merchantId}/getFxRate POST GET Reads current FX rates

Code Reality

Confirmed. All request bodies across all services are HashMap<String, Object> — there are no typed DTOs, meaning even POST endpoints have no schema validation. The lack of typed request objects makes it impossible to auto-generate OpenAPI specs or perform Bean Validation (@Valid, @NotNull, @Size).

Security note: If any of these POST endpoints currently accept sensitive data (account numbers, IBANs) in the request body, moving to GET puts that data in URL query parameters. For endpoints like fetch-account that handle sensitive identifiers, POST may actually be correct, or use path-based resource lookup instead.

Recommendation: - Change read-only operations to GET with query parameters (only when params are non-sensitive) - For queries involving sensitive data: keep as POST or use path-based resource lookup - Migration: Changing HTTP methods is a breaking change. Introduce new endpoints under a new version, deprecate old ones with a 6-month sunset period

Finding 9: No Pagination for List Endpoints

Severity: HIGH Affects: Disbursement, Remittance

GET /disbursements returns transaction history with no documented pagination. GET /{merchantId}/banks/list returns all banks. POST /{merchantId}/fetch searches remittances with no page/limit params.

Code Reality

Confirmed. The disbursement scheduler fetches up to 1,000 records with a simple SELECT ... WHERE state = 'published' LIMIT 1000 — no SELECT ... FOR UPDATE SKIP LOCKED. If multiple scheduler instances run, they process the same disbursements simultaneously.

Recommendation: - Add page and limit parameters to all list/history endpoints - Default limit: 20, max limit: 100 - Include pagination metadata in response - Add row-level locking (SELECT ... FOR UPDATE SKIP LOCKED) to the disbursement scheduler

Finding 10: No Request/Response Schemas — HashMap Everywhere

Severity: HIGH (upgraded — code confirms no schemas exist at any level) Affects: All APIs

The Disbursement doc lists 7 endpoints with URL patterns but zero request/response JSON examples. The Remittance doc lists 11 endpoints the same way.

Code Reality

All services accept HashMap<String, Object> as request bodies. There are no typed request/response DTOs anywhere. This means: - No compile-time type safety - No automatic OpenAPI schema generation - No Bean Validation (@Valid, @NotNull, @Size) - Field names scattered as string constants throughout the codebase

The payment body validation in the card service is literally a no-op: 

public Set<String> validatePaymentBody(Map<String, Object> body) {
    Set<String> errors = new HashSet<>();
    return errors;  // validates nothing
}

Recommendation: - Adopt OpenAPI 3.1 as the single source of truth for all API specs - Introduce typed DTOs (request/response classes) as the first step — this enables both Bean Validation and automatic OpenAPI generation via springdoc-openapi - Fix the no-op validation in the card service immediately

Finding 11: Naming Inconsistencies Across APIs

Severity: HIGH Affects: All APIs

Concept Disbursement Remittance PayIn
Start a payment /initiate /initiate and /remit-initiate /initiate
Check status Not documented /inquire /verify
Get account info /fetch-account /fetch-account N/A
Get balance /balance-data /balance-data N/A
Get banks /banks /banks/list N/A
Failure reasons /reasons /reasons/list N/A

Recommendation: - Establish a naming convention document - Eliminate the /list suffix (the HTTP method GET on a collection already implies listing) - Consolidate /initiate and /remit-initiate into one endpoint - Use consistent verbs: initiate for creation, inquire or status for checking (pick one)

Finding 12: No Rate Limiting — Anywhere

Severity: HIGH (upgraded — code confirms zero rate limiting implementation) Affects: All APIs

Rate limiting is mentioned in the architecture descriptions but no specific limits are documented for merchants.

Code Reality

No rate limiting or throttling exists in any service. No Bucket4j, no Spring Cloud Gateway rate limiter, no custom implementation. Combined with the lack of authentication (Finding 5), these APIs are fully open to abuse.

Recommendation: - Add rate limiting to every endpoint (Bucket4j or equivalent) - Return rate limit headers on every response: X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset - HTTP 429 response with Retry-After header when exceeded - Different tiers for different merchant plans (if applicable)

Finding 13: Versioning Strategy Undefined — No Versioning in Code

Severity: HIGH (upgraded — code confirms zero versioning) Affects: All APIs

PayIn uses /v2/ prefix. Payout microservices use /{version}/ in paths. Disbursement and Remittance main APIs have no version.

Code Reality

No URL versioning, no header versioning, no content negotiation in any service. Any breaking change affects all consumers simultaneously.

Recommendation: - Add /v1/ prefix to all endpoints immediately - Document the versioning strategy and deprecation timeline - Define what constitutes a breaking change vs non-breaking

Finding 14: No Sandbox/Testing Environment Documentation — And Zero Tests

Severity: HIGH (upgraded — code reveals zero test coverage) Affects: All APIs

PayIn shows qa.simpaisa.com in a curl example. Disbursement and Remittance don't mention test environments.

Code Reality

Zero unit tests, zero integration tests across all codebases. No src/test directories with any test files. No test dependencies (JUnit, Mockito) in active use. There is no automated way to verify that any code change does not break existing functionality.

Recommendation: - Document sandbox URLs for all APIs - Provide test card/account numbers (like Stripe's 4242424242424242) - Create test scenarios: successful payout, insufficient balance, invalid account, channel timeout, rate limit hit - Code: Add unit and integration test suites as a Phase 2 priority

Finding 15: Scheduler Timing Creates Implicit SLA

Severity: MEDIUM Affects: Disbursement

The Disbursement doc states the scheduler runs "every five minutes."

Recommendation: - Replace "every five minutes" with an SLA: "Disbursements are typically processed within 10 minutes of initiation" - Keep actual scheduler interval as an internal implementation detail

Finding 16: PayIn OTP Flow Lacks Security Hardening Documentation

Severity: MEDIUM Affects: PayIn

The PayIn doc describes OTP handling but doesn't document: OTP length, expiry (mentions 30 minutes — very long), max retry attempts, or lockout behaviour.

Recommendation: - Reduce OTP expiry to 5 minutes max (30 minutes is a security risk for payments) - Document the max attempt threshold clearly - Add rate limiting on OTP verification attempts

Finding 17: FX Rate Handling is Underspecified

Severity: MEDIUM Affects: Remittance

The Remittance API has /getFxRate and /confirmQuotation endpoints but doesn't document rate validity, currency format, or decimal precision.

Code Reality

Financial amounts in remittance use double throughout (merchantSettlement.getAmount(), remittance.getDeductedAmount()). Floating-point arithmetic causes rounding errors. DecimalFormat("#.##") is used to truncate amounts to 2 decimal places for postbacks, which silently loses precision. For a cross-border remittance service, this can cause settlement discrepancies.

Recommendation: - Document rate quote validity window - Define decimal precision: amounts to 2 decimal places, rates to 6 decimal places - Code: Migrate all monetary fields to BigDecimal with explicit rounding modes — double is not acceptable for money

Finding 18: No Field-Level Validation Rules — Validation Is a No-Op

Severity: MEDIUM (code confirms validation is absent at implementation level) Affects: All APIs

No endpoint documents: required vs optional fields, max string lengths, regex patterns, allowed enum values, numeric ranges.

Code Reality

Not only are validation rules undocumented — they don't exist in code either. The card service's InputValidationImpl.validatePaymentBody() returns an empty error set (validates nothing). No service uses Bean Validation annotations (@Valid, @NotNull, @Size) because all request bodies are HashMap<String, Object>.

Recommendation: - Add a validation rules table per endpoint in documentation - Code: Introduce typed DTOs with Bean Validation annotations as a prerequisite for any meaningful input validation

Finding 19: Payout Microservice Inconsistency

Severity: MEDIUM Affects: Remittance

The payout microservices show inconsistent versioning, endpoint naming, and capability sets across providers.

Code Reality

The retry scheduler's ChannelFactory only has an implementation for Bank of Asia. All other partners (Faysal Bank, Trust Bank, 1Link) default to the Bank of Asia channel, meaning retry attempts for those partners send requests to the wrong bank's API.

Recommendation: - Immediate: Fix the retry scheduler partner routing — this is an active financial risk - Normalise endpoint naming across internal services - Ensure all provider adapters implement a consistent interface

Finding 20: Missing Transaction Lifecycle Documentation

Severity: LOW Affects: All APIs

Transaction statuses are mentioned in passing but no complete state machine is documented.

Code Reality

Remittance has no @Transactional annotations anywhere. Balance deductions, state updates, and settlement insertions are not wrapped in database transactions. Partial states (balance deducted but remittance not updated, or vice versa) are possible and unrecoverable without manual intervention.

Recommendation: - Document the full transaction lifecycle as a state diagram - Code (P1): Add @Transactional to all financial operations in remittance immediately

Finding 21: No API Changelog or Migration Guide

Severity: LOW Affects: All APIs

The docs have a "Change History" table but only show v1.0.0.

Recommendation: - Maintain a public API changelog - Email merchants on breaking changes with 90+ days notice

Finding 22: No TLS/mTLS Requirements Documented — SSL Disabled in Remittance

Severity: CRITICAL (upgraded — code reveals SSL validation disabled) Affects: All APIs, Remittance most severe

None of the three specs state the minimum TLS version or whether mutual TLS is required.

Code Reality

The remittance service disables SSL certificate validation entirely. HttpClientService contains seven separate methods that create a TrustManager accepting all certificates and use NoopHostnameVerifier. This disables TLS verification on connections to partner banks (Bank of Asia, Trust Bank, Faysal Bank, 1Link).

This means a network attacker between Simpaisa and any partner bank can intercept and modify remittance instructions (amounts, beneficiary accounts) in transit.

Additionally, RSA/ECB/PKCS1Padding is used in both remittance and disbursement, which is vulnerable to Bleichenbacher padding oracle attacks.

Recommendation: - Emergency: Remove all trust-all TLS patterns from remittance. Use proper CA certificates. If partners have self-signed certs, import them into a dedicated truststore. - Emergency: Replace RSA/ECB/PKCS1Padding with RSA/ECB/OAEPWithSHA-256AndMGF1Padding - Document minimum TLS 1.2 requirement (prefer TLS 1.3) - State that TLS 1.0 and 1.1 are not supported

Finding 23: No PII Handling — PII Logged at INFO Level

Severity: HIGH (upgraded — code reveals active PII leakage in logs) Affects: All APIs

The APIs handle PII (names, phone numbers, account numbers, IBANs) and payment data. No documentation addresses data retention or PII masking.

Code Reality

Full request/response bodies including account numbers, beneficiary names, addresses, IBAN numbers, and phone numbers are logged at INFO level in remittance. SOAP XML requests contain complete customer PII. The wallet service logs cryptographic signatures to stdout (System.out.println("request signature = " + signature)).

Card data (raw PAN, CVV) passes through the application layer as plain String parameters in the card service, significantly increasing PCI-DSS scope. Card data is also stored in Redis (15-minute TTL) without confirmed TLS or ACL configuration.

Recommendation: - Immediate: Mask PII in all log output (never log full account numbers, PAN, CVV, or credentials) - Immediate: Remove System.out.println of signatures from the wallet service - Immediate: Stop passing raw PAN through the application layer — use tokenised references from the acquirer gateway - Document data retention policy - Document PII masking in API responses

Finding 24: No Health Check or Status Endpoint

Severity: LOW Affects: All APIs

No documented health check endpoint for merchants to verify API availability.

Recommendation: - Add GET /health or GET /status endpoint returning system health - Publish a status page for real-time operational visibility

Finding 25: No Bulk/Batch Operation Support Documented

Severity: LOW Affects: Disbursement

The Disbursement doc describes single-transaction initiation only.

Recommendation: - Document whether a batch disbursement endpoint exists (or is planned) - Batch payouts are standard in disbursement APIs (Stripe, Wise, Payoneer all offer them)

Finding 26: Hardcoded Credentials Across All Repositories (NEW)

Severity: CRITICAL Affects: Wallet, Remittance, Disbursement

This finding was invisible from specs and only discovered during source code review.

Production credentials are committed to Git across multiple repositories:

Service Credentials Found
Wallet Twilio Account SID + Auth Token (AC37679a..., 0fe360cd...), phone number +18587629352
Wallet PEM private keys (hbl_konnect_prv_pcks8.pem, private_key_pkcs8.pem) in src/main/resources/
Remittance Partner API username (ComPlex_API), password (7NuPOqs6jDVQFH), System ID, hardcoded RSA public key
Remittance Hardcoded IP address http://13.215.165.235:8080 in HTTP client
Disbursement MySQL password, SMTP password, JazzCash client ID/secret, Easypaisa client ID/secret, 1Link client ID/secret, SSL keystore password (changeit) — in application-prd.properties
Disbursement Easypaisa MSISDN (923332227227) and PIN (02610) in commented-out Selenium code

Action Required (Emergency): 1. Rotate all credentials listed above immediately 2. Remove PEM keys from Git history (BFG Repo-Cleaner) 3. Move all secrets to HashiCorp Vault or AWS Parameter Store 4. Add pre-commit hooks to prevent future credential commits (e.g., detect-secrets, trufflehog) 5. Purge all credentials from Git history across all repos

Finding 27: AES-ECB Encryption in Three Services (NEW)

Severity: CRITICAL Affects: Wallet, Card, Disbursement

This finding was invisible from specs and only discovered during source code review.

Three services use Cipher.getInstance("AES") which defaults to AES/ECB/PKCS5Padding. ECB mode is deterministic — identical plaintext blocks produce identical ciphertext blocks, leaking data patterns. For a card payment service handling PAN data, this is a PCI-DSS blocker.

Additionally, remittance uses MD5 for security token generation (FaysalBankService.generateMD5Hash()) — MD5 is cryptographically broken.

Recommendation: - Emergency: Migrate to AES/GCM/NoPadding with random 12-byte IV per encryption operation - Emergency: Replace MD5 token generation with HMAC-SHA256 - Note: Disbursement has a separate EncryptionUtils.java that correctly uses AES/CBC/PKCS5Padding — ensure the older insecure utility is removed and all call sites are migrated

Finding 28: Inverted Validation Booleans in Card Service (NEW)

Severity: CRITICAL Affects: Card

This finding was invisible from specs and only discovered during source code review.

Two critical validation methods return the opposite of what their names suggest:

// Returns true when email is INVALID
public static boolean isValidEmail(String email) {
    return !EMAIL_PATTERN.matcher(email.trim()).matches();
}

// Returns true when country is INVALID
public static boolean isValidCountry(String country) {
    return !COUNTRY_PATTERN.matcher(country.trim()).matches();
}

These are called in customerValidation() where isValidEmail() returning true triggers the "invalid email" error — meaning valid emails are rejected and invalid emails are accepted. This is an active bug affecting card payment processing.

Action Required (Emergency): Fix the boolean inversion. Add unit tests for all validation methods.

Finding 29: Chromedriver Binaries and Selenium Hack in Disbursement (NEW)

Severity: HIGH Affects: Disbursement

This finding was invisible from specs and only discovered during source code review.

Both disbursement scheduler repos contain chromedriver (Linux) and chromedriver.exe (Windows) — 23 MB of binaries committed to Git. These were used for a legacy Selenium hack to automate login to the Easypaisa web portal. The hack has been replaced by a proper REST login, but the binaries, Selenium dependencies (with version mismatches: selenium-chrome-driver:4.12.0 + selenium-api:3.141.59), and commented-out code remain.

Action Required: - Remove chromedriver binaries from both repos - Remove Selenium dependencies from pom.xml - Remove all commented-out browser automation code - Purge binaries from Git history (BFG Repo-Cleaner)

Finding 30: Disbursement IP Filter Logic Bug (NEW)

Severity: CRITICAL Affects: Disbursement

This finding was invisible from specs and only discovered during source code review.

IpFilter.java reads X-Forwarded-For into a variable (line 105) but then checks getRemoteAddr() instead (line 111). Behind a load balancer, getRemoteAddr() returns the load balancer's IP, not the client's. This means the IP whitelist either: - Blocks all legitimate traffic (if the LB IP isn't whitelisted), or - Allows all traffic through (if the LB IP is whitelisted)

Additionally, ~30 partner IPs are hardcoded in IpFilter.java (ByteDance, Sunshine, dLocal, Thunes, Gamify, BC Game, CODA, etc.). Adding a new partner requires a code change, rebuild, and redeployment.

Action Required (Emergency): - Fix the filter to use X-Forwarded-For for client IP detection behind load balancers - Externalise the IP whitelist to a database table or AWS Parameter Store

Finding 31: double Used for Money in Remittance (NEW)

Severity: HIGH Affects: Remittance

This finding was invisible from specs and only discovered during source code review.

Financial amounts use double throughout the remittance codebase (merchantSettlement.getAmount(), remittance.getDeductedAmount()). Floating-point arithmetic causes rounding errors. DecimalFormat("#.##") is used to truncate amounts to 2 decimal places for postbacks, which silently loses precision.

For a service processing real cross-border remittances, this can cause settlement discrepancies between Simpaisa and partner banks.

Recommendation: Migrate all monetary fields to BigDecimal with explicit rounding modes (RoundingMode.HALF_EVEN for financial calculations).

Finding 32: finally { return } Anti-Pattern in Financial Methods (NEW)

Severity: HIGH Affects: Remittance

This finding was invisible from specs and only discovered during source code review.

Seven financial-critical methods use finally { return response; } which suppresses all exceptions:

  • BankOfAsiaService.initiate() — line 117
  • BankOfAsiaService.finalize() — line 191
  • FaysalBankService.initiate() — line 457
  • FaysalBankService.finalize() — line 590
  • TrustBankService.finalize() — line 231
  • RemittanceService.markRemittanceSuccess() — line 327
  • MerchantBalanceService.lockMerchantBalancePostRequest() — line 70

If an exception occurs during a remittance (e.g., network failure mid-transfer, database write failure), it is silently swallowed and the method returns whatever partial state exists. This can lead to unrecoverable inconsistencies in financial records.

Action Required: Remove all finally { return } blocks. Let exceptions propagate to a proper error handler.

Finding 33: No @Transactional on Financial Operations (NEW)

Severity: HIGH Affects: Remittance

This finding was invisible from specs and only discovered during source code review.

No @Transactional annotation appears anywhere in the remittance codebase. Balance deductions, remittance state updates, and settlement insertions are not wrapped in database transactions. This means:

  • A balance deduction could succeed while the remittance state update fails
  • A settlement record could be inserted while the balance update fails
  • Partial states are possible and unrecoverable without manual intervention

Action Required: Add @Transactional to all service methods that perform multiple database writes as part of a financial operation.

Finding 34: No Database Migration Tooling (NEW)

Severity: MEDIUM Affects: All APIs

This finding was invisible from specs and only discovered during source code review.

No Flyway or Liquibase in any service. Schema changes are unversioned and managed externally via stored procedures. Deployments are fragile and non-reproducible.

Recommendation: Add Flyway to all services. Version all schema changes as migration scripts.

Finding 35: Disbursement Gateway on Spring Boot 2.2.6 / Java 8 (NEW)

Severity: MEDIUM Affects: Disbursement

This finding was invisible from specs and only discovered during source code review.

The disbursement-gateway runs Spring Boot 2.2.6 with Java 8 — both end-of-life. The gateway uses Netflix Zuul (also in maintenance mode). The schedulers run Spring Boot 3.x / Java 17, creating an inconsistent runtime environment.

Additionally, the gateway's HBL profile proxies requests directly to https://paymentapi.hbl.com/OpenAPIRest with no authentication, transformation, rate limiting, or logging.

Recommendation: Migrate to Spring Cloud Gateway on Spring Boot 3.x / Java 17.

Finding 36: Redis Used Everywhere — Not SurrealDB (NEW)

Severity: MEDIUM Affects: Wallet, Card, Remittance, Disbursement

This finding was invisible from specs and only discovered during source code review.

The original spec audit (and internal architecture documents) stated the caching layer was "SurrealDB (in-memory)." The source code reveals Redis is used in every service:

Service Redis Usage
Wallet 3 separate Redis instances using 3 different client libraries (Jedis, Lettuce, Redisson)
Card 2 Redis instances (merchant cache with infinite TTL, card data cache with 15-min TTL)
Remittance 1 Redis instance
Disbursement Redis via Vault integration

The stated migration to SurrealDB has not been adopted in any production service.

Recommendation: - Update all architecture documentation to accurately reflect Redis usage - If SurrealDB migration is still planned, create a formal migration plan with timeline - Consolidate the wallet service's three Redis instances into one - Add TTL to the card service merchant cache (currently infinite, meaning deactivated merchants remain cached)

Finding 37: Disbursement Scheduler Code Fork (NEW)

Severity: LOW Affects: Disbursement

This finding was invisible from specs and only discovered during source code review.

disbursement-scheduler-sunshine is a near-identical copy of disbusrment-scheduler with minor differences (no payout limit functionality). Every bug fix and feature must be applied to both repos. This doubles maintenance cost and guarantees the two copies will drift.

Additionally, DisbursementRepository.java contains a hardcoded merchant exclusion (m.merchantId not in (2000055,2000194)) — a production workaround baked into source code. And the enum Responses.MONTHLY_LIMIT_EXCEEDED has the message "Daily-limit-exceeded" — the name says monthly but the message says daily.

Recommendation: Consolidate into one repo with configuration-driven behaviour. Move merchant exclusions to a database table. Fix the enum mismatch.

Summary Table

# Finding Severity APIs Affected Source
1 Idempotency — optional, fails open CRITICAL All Spec + Code
2 Error handling — 200 OK on failure, exceptions swallowed CRITICAL All Spec + Code
3 Internal details in merchant docs CRITICAL All (PayIn most severe) Spec + Code
4 Webhooks — no retry, no signing CRITICAL All Spec + Code
5 Authentication — no Spring Security, CORS open CRITICAL All Spec + Code
6 HTTP status codes — 200 for everything CRITICAL All Spec + Code
7 Inconsistent URL patterns HIGH All Spec + Code
8 POST used for read operations HIGH Disbursement, Remittance Spec + Code
9 No pagination HIGH Disbursement, Remittance Spec + Code
10 No request/response schemas — HashMap everywhere HIGH All Spec + Code
11 Naming inconsistencies HIGH All Spec
12 No rate limiting — zero implementation HIGH All Spec + Code
13 Versioning strategy undefined — no versioning in code HIGH All Spec + Code
14 No sandbox documentation — zero tests HIGH All Spec + Code
15 Scheduler timing as implicit SLA MEDIUM Disbursement Spec
16 OTP security hardening gaps MEDIUM PayIn Spec
17 FX rate handling underspecified — double for money MEDIUM Remittance Spec + Code
18 No field-level validation — validation is a no-op MEDIUM All Spec + Code
19 Payout microservice inconsistency — wrong bank routing MEDIUM Remittance Spec + Code
20 Missing transaction lifecycle docs — no @Transactional LOW All Spec + Code
21 No API changelog/migration guide LOW All Spec
22 TLS — SSL validation disabled in remittance CRITICAL All (Remittance worst) Spec + Code
23 PII — logged at INFO level, PAN in application layer HIGH All Spec + Code
24 No health check/status endpoint LOW All Spec
25 No bulk/batch operation support LOW Disbursement Spec
26 Hardcoded credentials across all repos CRITICAL Wallet, Remittance, Disbursement Code only
27 AES-ECB encryption + MD5 tokens CRITICAL Wallet, Card, Disbursement Code only
28 Inverted validation booleans CRITICAL Card Code only
29 Chromedriver binaries / Selenium hack HIGH Disbursement Code only
30 IP filter logic bug CRITICAL Disbursement Code only
31 double for money HIGH Remittance Code only
32 finally { return } anti-pattern HIGH Remittance Code only
33 No @Transactional on financial ops HIGH Remittance Code only
34 No database migration tooling MEDIUM All Code only
35 Gateway on Spring Boot 2.2.6 / Java 8 MEDIUM Disbursement Code only
36 Redis everywhere, not SurrealDB MEDIUM All Code only
37 Scheduler code fork + hardcoded exclusions LOW Disbursement Code only

Severity Breakdown

Severity Spec-Only Audit (2026-03-31) Updated with Code (2026-04-03) Change
CRITICAL 6 10 +4
HIGH 8 11 +3
MEDIUM 7 10 +3
LOW 4 6 +2
Total 25 37 +12

Priority Action Plan

Staffing assumption: Phase 0 requires immediate security/engineering response. Phase 1A assumes one full-time technical writer and one engineer for review. Phase 1B onward requires dedicated engineering time. All timelines assume these resources are available and not split across other projects.

Phase 0: Emergency Security Remediation (This Week)

These are active security vulnerabilities that require immediate action.

  1. Rotate all hardcoded credentials — Twilio, partner bank API keys, MySQL, SMTP, JazzCash, Easypaisa, 1Link (Findings 26, D-01, W-01, R-02)
  2. Remove PEM private keys from Git history — BFG Repo-Cleaner (Finding 26)
  3. Fix SSL certificate validation — remove all trust-all patterns in remittance (Finding 22)
  4. Fix inverted validation booleans — card service rejects valid emails, accepts invalid ones (Finding 28)
  5. Authenticate Safepay endpoints — currently zero auth (Finding 5)
  6. Fix disbursement IP filter — use X-Forwarded-For, not getRemoteAddr() (Finding 30)
  7. Enable card service IP security filter — currently commented out (Finding 5)
  8. Remove chromedriver binaries and Selenium code from disbursement (Finding 29)
  9. Purge all credentials from Git history across all repos (Finding 26)

Phase 1A: Documentation-Only Fixes (Weeks 1-2)

These require no code changes, only documentation work.

  1. Separate internal architecture docs from merchant docs (Finding 3) — create the two-tier doc structure
  2. Write authentication documentation with code samples in 5 languages (Finding 5)
  3. Document HTTP status codes per endpoint (Finding 6)
  4. Define standard error response schema and document across all APIs (Finding 2)
  5. Write webhook specification with payload schemas and signature verification (Finding 4)
  6. Document TLS requirements (Finding 22)
  7. Update architecture docs to reflect Redis (not SurrealDB) as the actual caching layer (Finding 36)

Phase 1B: Critical Code Fixes (Weeks 2-4)

These require server-side implementation alongside documentation.

  1. Replace AES-ECB with AES-GCM across wallet, card, and disbursement (Finding 27)
  2. Replace MD5 token generation with HMAC-SHA256 in remittance (Finding 27)
  3. Add Spring Security with API key auth to all services (Finding 5)
  4. Remove @CrossOrigin(origins="*") from all controllers (Finding 5)
  5. Add @Transactional to all financial operations in remittance (Finding 33)
  6. Remove finally { return } anti-pattern from all financial methods (Finding 32)
  7. Make idempotency mandatory, fix fail-open on Redis failure (Finding 1)
  8. Add @ControllerAdvice global exception handler to all services (Finding 2)
  9. Replace e.printStackTrace() with proper logging across all services (Finding 2)
  10. Fix retry scheduler partner routing — currently all retries go to Bank of Asia (Finding 19)
  11. Migrate monetary fields to BigDecimal (Finding 31)
  12. Fix no-op payment validation in card service (Finding 10)
  13. Upgrade RSA padding to OAEP (Findings 22)
  14. Mask PII in logs — stop logging full account numbers, PAN, CVV (Finding 23)

Phase 2: Reliability and API Quality (Weeks 4-8)

  1. Implement webhook retry with exponential backoff (Finding 4)
  2. Configure Kafka Dead Letter Topics for failed messages (Finding 4)
  3. Add row-level locking on disbursement scheduler fetch (Finding 9)
  4. Add correlation IDs / distributed tracing (Finding 2)
  5. Add rate limiting to all endpoints (Finding 12)
  6. Introduce typed DTOs replacing HashMap across all services (Finding 10)
  7. Add API versioning /api/v1/ prefix (Finding 13)
  8. Add pagination to all list endpoints (Finding 9)
  9. Add springdoc-openapi for documentation (Finding 3)
  10. Add Flyway database migrations (Finding 34)
  11. Document sandbox environments with test credentials (Finding 14)
  12. Add field-level validation rules via Bean Validation (Finding 18)
  13. Document PII handling and data retention (Finding 23)
  14. Specify FX rate handling (validity, precision, flow) (Finding 17)
  15. Document transaction lifecycle state machine (Finding 20)

Phase 3: Consistency and Modernisation (Weeks 8-16)

  1. Standardise URL patterns across all APIs, with backward-compatible parallel run (Finding 7)
  2. Fix HTTP method misuse under new version (Finding 8)
  3. Resolve naming inconsistencies (Finding 11)
  4. Normalise payout microservice interfaces (Finding 19)
  5. Consolidate disbursement scheduler forks into single repo (Finding 37)
  6. Upgrade disbursement gateway to Spring Cloud Gateway / Boot 3.x (Finding 35)
  7. Consolidate wallet Redis instances from 3 to 1 (Finding 36)
  8. Add unit and integration test suites (Finding 14)
  9. Tighten OTP security (reduce expiry, document limits) (Finding 16)
  10. Replace scheduler timing with SLA language (Finding 15)
  11. Establish API changelog process (Finding 21)
  12. Add health check/status endpoint (Finding 24)
  13. Plan bulk/batch disbursement capability (Finding 25)

Testing Strategy (added by Eng Review)

Three layers of testing ensure the API changes work and don't drift:

Layer 1: OpenAPI Spec Linting (CI)

  • Run Spectral against all OpenAPI spec files on every PR
  • Custom Spectral ruleset enforcing the checklist below (idempotency headers, error schema, pagination params, rate limit headers, etc.)
  • Blocks merge if spec violates rules

Layer 2: Contract Tests (per API)

  • For each API family (Disbursement, Remittance, PayIn, Card), contract tests verify actual HTTP responses match the OpenAPI spec
  • Use a tool like Dredd, Schemathesis, or Prism to auto-generate test cases from the OpenAPI spec
  • Run against sandbox environments
  • Catches behavioural drift between spec and implementation

Layer 3: Integration Test Suite

Key scenarios to cover:

Scenario Expected Behaviour APIs
Duplicate idempotency key, same params Return original response, HTTP 200 All
Duplicate idempotency key, different params HTTP 409 Conflict All
Missing idempotency key on payment endpoint HTTP 400 with IDEMPOTENCY_KEY_REQUIRED All
Redis down + payment request HTTP 503 (fail closed, not fail open) All
Invalid auth signature HTTP 401 with standard error body All
Exceeded rate limit HTTP 429 with Retry-After header All
Pagination: page beyond range Empty data array, correct pagination metadata Disbursement, Remittance
Webhook delivery failure Retry 3x with backoff, then mark as failed All
FX rate quote expiry HTTP 422 with QUOTE_EXPIRED error code Remittance
OTP max attempts exceeded HTTP 429 with lockout duration PayIn
Valid email in card payment Accepted (regression test for inverted boolean fix) Card
Invalid email in card payment Rejected with validation error Card
Concurrent disbursement scheduler instances No duplicate processing (row-level locking) Disbursement
Exception during remittance Proper rollback via @Transactional Remittance

Implementation Notes (from Eng Review)

  • Idempotency storage: Redis for idempotency key lookups (this is the actual production stack). Fast key existence checks with TTL-based expiry. Fall back to MySQL for persistence beyond the 24-hour idempotency window. Must fail closed — if Redis is unavailable, reject the request rather than proceeding without deduplication.
  • Pagination: Use cursor-based pagination (not offset) for transaction history endpoints. Offset-based OFFSET 10000 scans 10K rows in MySQL and degrades at scale.
  • Rate limiting: Redis in-memory counters with TTL for per-merchant rate limits. Atomic increment operations, minor per-request overhead.
  • Note on SurrealDB: Previous recommendation to use SurrealDB (in-memory) for caching has been revised. All production services currently use Redis. The migration to SurrealDB, if still desired, should be planned as a separate initiative with proper evaluation.

Appendix: API Best Practices Checklist

Use this checklist when designing or reviewing any new Simpaisa API endpoint:

  • Endpoint uses correct HTTP method (GET for reads, POST for creates, PUT/PATCH for updates, DELETE for deletes)
  • URL follows the standardised pattern: /v{version}/{product}/{merchantId}/{resource}
  • Authentication is implemented (Spring Security filter chain) and documented (header names, signature algorithm, code samples)
  • Request schema documented and implemented as typed DTO (all fields: name, type, required/optional, constraints)
  • Response schema documented and implemented as typed DTO (success and all error cases)
  • HTTP status codes documented per endpoint and enforced via @ControllerAdvice
  • Idempotency key mandatory for state-changing operations, fails closed on store failure
  • Pagination supported for list endpoints
  • Rate limits implemented and documented with response headers
  • Error response follows standard schema with machine-readable error code
  • Webhook payload documented with signature verification and retry with backoff
  • Field validation rules specified and enforced via Bean Validation (max length, regex, enum values)
  • Curl example provided (success case)
  • Sandbox test scenario provided
  • No internal implementation details leaked (no table names, no scheduler intervals)
  • TLS 1.2+ required, documented, SSL validation enabled (no trust-all)
  • PII masked in responses and in logs where applicable
  • All monetary amounts use BigDecimal, never double or float
  • All credentials in Vault or Parameter Store, never in source code
  • @Transactional on all multi-write financial operations
  • Encryption uses AES-GCM (not ECB), RSA-OAEP (not PKCS1), no MD5
  • No finally { return } in financial methods

Tooling recommendation: OpenAPI 3.1 is the single source of truth for all API specs (decided during eng review). Use Spectral (by Stoplight) for automated linting. Use Swagger UI or Redocly for auto-generated developer portal. Use openapi-generator for client SDK generation in Java, Python, Node.js, PHP, C#.