PETZY POS — BUG AUDIT
Generated: 2026-08-24 — all 27 items fixed 2026-08-25
Scope: backend (controllers/middleware/services/routes) + frontend (POS/payment/hotel/context critical paths)
Status legend: [ ] open   [x] fixed
Verification: node --check on every edited backend file, `require('./src/app.js')`
loads the full route/controller/model tree with no errors, edited frontend files
pass a babel parse, and `npm run build` (webpack production) compiles clean.
No MySQL instance was available in this session, so nothing was exercised
against a live database/browser — see BUGFIX-PLAN.md for the full caveat.

=====================================================================
P0 — CROSS-TENANT IDOR / DATA LEAK (security-critical)
=====================================================================
Pattern: a record is looked up by :id (findByPk/findOne) and then read/mutated
with NO check that it belongs to the caller's own outlet/company. Route-level
middleware (middleware/authorize.js) only checks permission slugs here, not
ownership — requireOutletAccess/requireCompany are not applied to these
routes, and the controller itself skips the check too. The correct pattern
already exists in orderController.js (`orderInScope(req, order)`) and
utils/outletScope.js (`resolveOutletIds`) — this bug is those fixes not
having been propagated to the rest of the hand-written controllers.

[x] 1. backend/src/controllers/paymentController.js — refundPayment (~L182-213)
    No outlet/company scope check on `Payment.findByPk(req.params.id)`.
    ALSO: no guard against double-refunding — a second call to the same
    endpoint isn't blocked by payment.status, only by amount > payment.amount.
    Failure: any user with pos.refund can refund another company's payment,
    or refund the same payment repeatedly (each time reducing the order's
    paid_amount further, potentially into negative balance).
    Fix: check payment.outlet_id against req.user.outletIds/isSuperAdmin;
    reject if payment.status === 'refunded'.

[x] 2. backend/src/controllers/paymentController.js — chargeToRoom (~L108-179)
    `Order.findByPk(req.params.id)` with no outlet scope check before posting
    a charge to a hotel folio and closing the order.
    Failure: order from another outlet/company can be charged to a room and
    force-closed.
    Fix: reuse orderController's orderInScope-style check.

[x] 3. backend/src/controllers/userController.js — getUser, resetPassword,
    deactivateUser (~L22-78)
    `User.findByPk(req.params.id)` with no company_id check (updateUser at
    L44-47 DOES check — these three don't).
    Failure: resetPassword lets any user.manage holder reset the password of
    a user in a different company — full cross-tenant account takeover.
    Fix: same company_id check as updateUser, applied to all three.

[x] 4. backend/src/controllers/roleController.js — getRole, updateRole,
    removeRole
    `Role.findByPk(req.params.id)` with no company_id check.
    Failure: view/edit/deactivate another company's role & its permissions.
    Fix: add company_id ownership check.

[x] 5. backend/src/controllers/hotelReservationController.js — cancelReservation,
    checkIn, getFolio, postFolioItem, checkOut (~L61-181)
    None of these check outlet/company ownership of the reservation/folio.
    Routes that mount them (hotelRoutes.js, checkinRoutes.js,
    checkoutRoutes.js, folioRoutes.js) only requirePermission, never
    requireOutletAccess.
    Failure: any hotel.manage/hotel.view holder can check in/out, cancel, or
    post financial charges to another company's guest folio by id.
    Fix: add outlet ownership check on the resolved reservation/folio/room in
    each handler.

[x] 6. backend/src/controllers/cashController.js — closeRegister (~L19-43)
    `CashRegister.findByPk(req.params.id)` with no outlet check (route key is
    register id, not outlet_id, so requireOutletAccess never even applies).
    Failure: close another outlet's/company's open cash register.
    Fix: add outlet ownership check.

[x] 7. backend/src/controllers/categoryController.js — updateCategory,
    removeCategory ; backend/src/controllers/menuController.js — updateItem,
    removeItem
    `findByPk` with no company_id check (create* handlers DO stamp
    company_id — update/remove don't check it).
    Failure: edit/deactivate another company's menu categories/items by id.
    Fix: add company_id ownership check.

[x] 8. backend/src/controllers/hotelController.js — updateRoomType, updateRoom,
    updateHousekeeping
    `findByPk` with no outlet/company scope check.
    Fix: add ownership check.

[x] 9. backend/src/controllers/inventoryController.js — updateItem
    `InventoryItem.findByPk` with no company_id check before `.update()`.
    Fix: add ownership check.

[x] 10. backend/src/controllers/kitchenController.js — updateKotStatus
    `Kot.findByPk` with no outlet scope check; a KOT status change also emits
    socket events into that outlet's kitchen board/POS.
    Fix: add outlet ownership check.

[x] 11. backend/src/controllers/payrollController.js — getRun, finalizeRun,
    markItemPaid, leaveBalances
    All look up by PK with no company check. markItemPaid creates a real
    Expense row and marks a payslip paid for ANY PayrollItem id regardless
    of company.
    Fix: add company ownership check to all four.

[x] 12. backend/src/controllers/leaveController.js — approveLeaveRequest,
    rejectLeaveRequest, cancelLeaveRequest
    `LeaveRequest.findByPk` with no company_id check (createLeaveRequest
    does check).
    Fix: add company ownership check.

[x] 13. backend/src/controllers/hotelController.js — listRoomTypes ;
    backend/src/controllers/kitchenController.js — listStations ;
    backend/src/routes/housekeepingRoutes.js — GET /
    `const where = outlet_id ? { outlet_id } : {}` — omitting outlet_id
    returns EVERY room type / kitchen station / housekeeping row across
    EVERY company in the system.
    Fix: when outlet_id is omitted, default to the caller's own
    outlets/company (resolveOutletIds pattern) instead of no filter.

[x] 14. backend/src/routes/onlineOrderRoutes.js — PATCH /:id/status
    `OnlineOrder.findByPk` with no outlet/company check, even though sibling
    CRUD routes for the same model ARE outletScoped via crudRouter.
    Fix: add ownership check in this custom handler.

[x] 15. backend/src/routes/notificationRoutes.js — PATCH /:id/read
    `Notification.update(..., { where: { id: req.params.id } })` with no
    user_id/company_id filter — any user can mark any other user's
    notification read by id.
    Fix: scope the update to req.user.id.

[x] 16. backend/src/routes/employeeRoutes.js — POST /:id/attendance
    Upserts Attendance keyed by employee_id = req.params.id with an
    attacker-supplied outlet_id in the body and NO lookup of the employee at
    all — no check the employee belongs to the caller's company/outlet.
    Fix: fetch the employee first and check company/outlet ownership before
    upserting.

=====================================================================
P1 — BUSINESS-LOGIC CORRECTNESS BUGS
=====================================================================
[x] 17. backend/src/controllers/dayEndController.js — closeDay (~L30)
    `Refund.findAll({ where: { created_at: dateRange } })` is MISSING the
    outlet_id filter that every other query in this same function has.
    Failure: every outlet's/company's refunds for that date range get summed
    into this outlet's day-end closing report — corrupts the refund total and
    leaks cross-tenant financial data into the closing record.
    Fix: add `outlet_id` to the where clause, matching every sibling query.

[x] 18. backend/src/controllers/hotelReservationController.js —
    cancelReservation (~L61-68)
    No status-transition guard: a `checked_in` reservation can be
    "cancelled," force-setting the room back to `available` while the guest
    is still physically in it (checkIn had set room.status = 'occupied').
    Fix: reject cancellation when status is checked_in or checked_out.

[x] 19. backend/src/config/index.js — jwt.secret / jwt.refreshSecret
    Silently falls back to hard-coded strings 'change-me-dev-secret' /
    'change-me-dev-refresh-secret' if env vars are unset.
    Failure: if this path is ever hit in a real deployment (misconfigured
    prod env), tokens become forgeable by anyone who has read this file.
    Fix: fail fast (throw at startup) when NODE_ENV=production and the
    secret is unset or equals the placeholder default.

=====================================================================
P2 — FRONTEND (money / order-integrity)
=====================================================================
[x] 20. frontend/src/pages/pos/POS.jsx — sendToKitchen / billAndPay /
    holdTicket (~L192-311, ~L555-563)
    Each independently creates an order when `order` is null, gated only by
    its OWN loading flag — the other two buttons stay clickable. Double-
    tapping two of these in quick succession (easy on touchscreen) creates
    two separate orders server-side; only one is ever referenced by React
    state, the other is an orphaned duplicate order/KOT.
    Fix: one shared in-flight guard disables all three buttons while any one
    of them is submitting.

[x] 21. frontend/src/pages/pos/POS.jsx — outlet switch doesn't reset cart/order
    OutletContext's outletId can change globally (Topbar dropdown) without
    POS.jsx remounting. loadMenu reloads items/categories for the new
    outlet, but `cart`/`order` are never cleared, so a stale outlet-A item
    can ride into an order billed against outlet B.
    Fix: reset cart/order in an effect keyed on outletId.

[x] 22. frontend/src/pages/pos/POS.jsx — items added after order is sent vanish
    Once `order` is set, the footer buttons no longer offer any way to
    submit more items, but the menu grid stays fully clickable — a tap
    pushes into the now-orphaned `cart` state with no code path left to ever
    submit it. Silent data loss (guest's extra item never reaches kitchen or
    bill).
    Fix: disable the menu grid with explicit user feedback once an order is
    open (extending an open order is out of scope for this pass).

[x] 23. frontend/src/components/pos/PaymentModal.jsx — split-payment lines
    keyed by array index (~L80-81)
    Removing a non-last line reassigns DOM nodes/focus across rows during
    split-payment entry.
    Fix: stable synthetic id per line, key on that instead of idx.

[x] 24. frontend/src/pages/pos/POS.jsx — cart dedupe key depends on
    addon/modifier click order (~L132-136)
    Key built from `addons.map(a=>a.id).join('.')` in click order, not a
    canonical order — identical selections picked in a different click order
    produce two separate cart lines instead of merging quantity.
    Fix: sort ids before building the key.

[x] 25. frontend/src/context/OutletContext.jsx — outletId not reset when
    outlets becomes empty (~L14-19)
    On logout, outlets becomes [], but the early-return skips resetting
    outletId — stale id lingers in state/localStorage.
    Fix: explicitly reset outletId (and localStorage) when outlets is empty.

=====================================================================
BONUS FINDINGS (caught while fixing the items above, same bug class)
=====================================================================
[x] 26. backend/src/controllers/inventoryController.js — updateTransferStatus
    `StockTransfer.findByPk` with no company_id check before dispatching/
    receiving stock (which moves real InventoryStock balances between
    outlets). Fix: added the same company_id ownership check used elsewhere.

[x] 27. backend/src/routes/employeeRoutes.js — POST /:id/attendance/auto-mark
    Fetched the employee but never checked it belongs to the caller's own
    outlets before bulk-writing attendance rows for it. Fix: added the same
    inOutletScope() check as the sibling POST /:id/attendance fix (#16).

=====================================================================
NOTES / NON-FINDINGS (checked, no bug found)
=====================================================================
- Transaction usage ({ transaction: t } propagation, row locking) looked
  correct everywhere reviewed.
- orderPricingService.js / payrollService.js / money.js cents-based math
  looked internally consistent — no off-by-one/operator bugs found.
- No raw sequelize.query string interpolation found — no SQL injection risk
  observed.
- No missing `await` found in files reviewed.
- api.js token-refresh interceptor, useApiList/useSocket/useDebounce cleanup,
  CrudPage/PaymentModal submit-guards all reviewed and look correct.
