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

[devin-ai-integration]

Summary

AccountService.transferAmount only checked fromAccount.balance >= amount, which a negative amount trivially satisfies. Both balance updates then invert — subtract(-X) credits the sender and add(-X) debits the recipient — so any authenticated user could drain an arbitrary account just by knowing its username (POST /transfer?toUsername=victim&amount=-1000000). deposit was equally unguarded (negative deposit = self-withdrawal bypassing the funds check).

Fix centralizes the guard in the service so every entry point is covered:

private void validateAmount(BigDecimal amount) {   // called first in deposit/withdraw/transferAmount
    amount != null && amount > 0                   // else "Amount must be greater than zero"
    amount.scale() <= 2                            // no sub-cent amounts
    amount <= MAX_TRANSACTION_AMOUNT (1_000_000.00)
}

transferAmount also rejects transfers to self (after recipient lookup), and BankController.deposit now wraps the service call in the same try/catch the other endpoints use so a rejected amount renders the dashboard error instead of a 500.

Verification: no Maven/plugin repository is reachable from this session (only github.com is on the network allowlist), so ./mvnw test cannot resolve plugins. The new AccountServiceAmountValidationTest compiles, and its scenarios were executed against the compiled classes with Mockito via a throwaway launcher (negative/zero/null/over-max/sub-cent/self transfer rejected with balances untouched; positive transfer, deposit and withdraw still move money) — all passed. CI is authoritative.

Written by Devin

Devin-Org: engineering

[devin-ai-integration]

Runtime verification — negative-amount transfer theft is fixed

Tested end-to-end through the UI with two live builds running side by side against separate MySQL schemas: BEFORE = upstream 305826d on :8081, AFTER = this branch 3aa6bee on :8080. Same users, same starting balances ($100 attacker / $5000 victim), same inputs.

The attack: POST /transfer with toUsername=victim, amount=-1000

	BEFORE (upstream)	AFTER (this PR)
Result	accepted silently	rejected: "Amount must be greater than zero"
Attacker balance	$100 → $1100	$100 → $100
Victim balance	$5000 → $4000	$5000 → $5000

Before/after screen recording:

BEFORE — attacker credited $1000, victim robbed of $1000:

AFTER — identical attack rejected, balances unchanged:

The victim ending at exactly $5040.00 ( = 5000 + the legitimate 40 transfer) independently confirms the -1000 transfer never moved any of their money.

Also verified on the fixed build

Negative deposit (-500) → error rendered on the dashboard, not a 500 error page (the BankController.deposit try/catch), balance unchanged at $100:

Negative withdraw (-500) → same error, balance unchanged at $100:

Regression: legitimate transfer of 40 → attacker $100 → $60, victim $5000 → $5040:

Not exercised at runtime: the scale > 2, > 1,000,000.00, and self-transfer branches of validateAmount (covered by the new unit tests only).

Written by Devin