This PR introduces payment processing but has critical security issues that must be addressed before merging. The SQL query construction is vulnerable to injection, and there's a null reference risk in the customer lookup flow.
| Location | Issue | |
|---|---|---|
PaymentService.cs:47 |
SQL injection vulnerability in query construction | |
PaymentService.cs:82 |
Missing null check before accessing customer.Email | |
RefundHandler.cs:31 |
No test coverage for partial refund edge case |
The customerId parameter is concatenated directly into the SQL query string without parameterization, allowing attackers to inject arbitrary SQL.
Impact: Security concern
cmd.Parameters.AddWithValue("@id", customerId);
The GetCustomer() call can return null if the customer doesn't exist, but the code immediately accesses .Email without checking.
Impact: Potential runtime failure
if (customer is null) return Result.NotFound();
await SendReceipt(customer.Email);
The partial refund logic handles amounts greater than the original payment, but RefundHandlerTests.cs has no test for this scenario.
> Related: RefundHandlerTests.cs