Skip to content

bug: reject non-positive amounts in deposit/withdraw/transfer - #299

Open
devin-ai-integration[bot] wants to merge 1 commit into
DevOpsfrom
devin/1788253657-withdraw-amount-validation
Open

bug: reject non-positive amounts in deposit/withdraw/transfer#299
devin-ai-integration[bot] wants to merge 1 commit into
DevOpsfrom
devin/1788253657-withdraw-amount-validation

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

AccountService.withdraw only guarded against insufficient funds, never against a negative amount. balance.compareTo(-1000000) < 0 is false for any non-negative balance, so the guard passed and balance.subtract(negative) increased the balance — an authenticated user could mint unlimited funds via POST /withdraw?amount=-1000000 (and then spend them through legitimate transfers). deposit and transferAmount had the same missing validation; a negative transfer would pull money out of the recipient's account.

Fix: a single validateAmount guard runs before any balance mutation in all three service methods.

private static void validateAmount(BigDecimal amount) {
    if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
        throw new RuntimeException("Amount must be greater than zero");
    }
}

RuntimeException matches the existing convention in this service/controller pair. BankController.deposit had no try/catch (unlike withdraw/transfer), so a rejected amount would have surfaced as a 500 — it now renders the dashboard with the error attribute like the other two endpoints.

New AccountServiceTest is a plain Mockito unit test (no Spring context — the existing BankappApplicationTests requires a real MySQL) covering negative/zero/null rejection with balance unchanged and no repository interaction, plus the happy paths.

Note: this environment has no cached Maven artifacts and Maven Central is unreachable from it, so ./mvnw compile/test could not be run locally; CI is authoritative.

Written by Devin

Devin-Org: engineering

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants