Skip to content

fix(refunds): verify refund PIN asynchronously outside write transaction (#1227) - #1228

Merged
khaira777 merged 5 commits into
mainfrom
fix/refund-pin-async
Oct 20, 2026
Merged

khaira777 merged 5 commits into
mainfrom
fix/refund-pin-async

Conversation

@khaira777

Copy link
Copy Markdown
Contributor

Fixes #1227 (Item 14 from the remediation backlog).

Problem

In main/routes/refunds.ts and main/services/refund.ts, manager/owner PIN verification for late refunds (>1 hour within the same business day) was executed synchronously via bcrypt.compareSync inside withTxn.

  1. Event Loop Starvation: bcrypt.compareSync with 10 salt rounds blocks the Node.js event loop for ~70ms per attempt. During this time, the entire POS main process was unresponsive to incoming network requests, WebSocket heartbeats, and UI IPC events.
  2. Database Write Lock Contention: Because the synchronous hash check occurred inside an active SQLite immediate transaction (withTxn), SQLite's single-writer lock was held across the entire 70ms cryptographic computation, stalling concurrent writes (order creation, payments, kitchen status updates).

Solution

  1. Decoupled Verification Architecture:
    • Extracted checkIsLateRefund and validateRefundPreconditions in main/services/refund.ts so business precondition checks (bill existence, completed order, business day eligibility, split allocation, refundable balance) run before PIN evaluation.
    • In main/routes/refunds.ts, idempotent replays are checked outside withTxn first.
    • For late refunds requiring PIN override, asynchronous PIN verification runs via verifyPinAsync outside the database write transaction, yielding the event loop between candidate checks.
  2. Atomic Write-Boundary Revalidation:
    • The verified approver is passed to createRefund as preVerifiedApprover.
    • Inside withTxn, createRefund authoritatively verifies the candidate approver is still active (is_active === 1), holds an eligible role (manager or owner), and has matching pin_hash === preVerified.verifiedPinHash. If state changed concurrently, it fails closed with HTTP 403.
  3. Concurrency and Rate-Limit Protection:
    • Serialized PIN attempts per IP/caller using withRefundPinLock mutex to prevent parallel bcrypt bursts from exhausting rate limits.
    • Failed attempts increment the failure counter; successful auth clears it.
  4. Comprehensive Test Suite:
    • Added tests/async-refund-pin-event-loop.test.ts with 19 tests verifying event loop ticks during verification, concurrent WAL database writes while bcrypt executes, transaction rollback on stale approver state, rate-limiting, and idempotency replay.
    • Wired suite into test:refunds and package.json; script coverage passes with 0 unreachables.

Verification

  • npm run test:async-refund-pin-event-loop: 19/19 passed
  • npm run test:refunds: 81/81 passed
  • npm run test:refund-completed-orders: 18/18 passed
  • npm run test:script-coverage: passed
  • npm run lint: passed
  • npm run build: passed
  • node scripts/ci/check-changed-format.cjs --base origin/main: passed

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: FreeOpenSourcePOS/FloCafe/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1d33ed69-9e13-46da-8ed2-a7acf1fb5f21

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@khaira777
khaira777 merged commit c1e428d into main Oct 20, 2026
23 checks passed
@khaira777
khaira777 deleted the fix/refund-pin-async branch October 20, 2026 06:09
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.

fix(auth): execute refund manager PIN verification asynchronously outside database write transaction

1 participant