# GatePass — Security & Performance Audit Consolidated audit across frontend (`web/`, React + Vite + TS) and backend (`api/`, Node/Express + Prisma). Findings are de-duplicated and prioritized. Line references were verified against source at audit time; re-check before fixing as the code evolves. > **Scope note:** each item lists severity, the file(s) involved, the concrete failure/exploit scenario, and a suggested fix. --- ## 🔴 Critical — fix first ### 1. JWT falls back to a hardcoded `'dev-secret'` - **Files:** `api/src/middleware/auth.ts:5`, `api/src/controllers/auth.controller.ts:8`, `api/src/controllers/platform.controller.ts:8`, `api/src/controllers/visitor-scanner.controller.ts:8` - **Scenario:** Every sign/verify path uses `process.env.JWT_SECRET || 'dev-secret'`, and nothing asserts the env var at boot. If it is ever unset/empty in production, anyone with this (public) repo value can forge any token — owner, sub-admin with all flags, or platform-admin. Complete authentication/authorization bypass. - **Fix:** Centralize the secret in one module; throw at startup if it is missing or equals the fallback. Never ship a usable default. Rotate the secret. --- ## 🟠 High ### 2. Socket.IO has no auth — cross-tenant realtime PII leak - **Files:** backend `api/src/server.ts:96-99` (`socket.on('join', room => socket.join(room))`), emitters `api/src/lib/events.ts:18-26`; frontend `web/src/lib/socket.ts:8`, `web/src/hooks/useApproverSocket.ts:14`, `web/src/hooks/useCheckpointSocket.ts:14` - **Scenario:** The socket accepts connections with no token and lets clients `join` any room (`owner:`, `approver:`, `checkpoint:`). Room IDs come from a **client-side** JWT decode. Anyone who learns an `ownerId` (returned in many API responses) can stream another org's live visitor data — names, phone, email, photos, decisions. It also bypasses the row-level scoping enforced on the REST side (a scope-restricted sub-admin can join `owner:` and receive every visitor's events). - **Fix:** Authenticate the handshake with `io.use()` verifying the JWT; derive room membership from the verified payload server-side; ignore client-supplied room names. ### 3. Stored XSS in the email template editor - **Files:** `web/src/components/settings/EmailTemplateEditor.tsx:367` (`innerHTML =`), `:704-708` (`toFragmentHtml` returns server HTML unsanitized), `:697-699` (`isHtmlFragment`) - **Scenario:** Template body from the server is written to a live contenteditable via `innerHTML` with no sanitization whenever it "looks like HTML". `` executes in the **Owner's** browser the moment the editor opens. Combined with #4 → one-click account takeover. (The live-preview iframe at `:667-673` is correctly `sandbox=""`; only the contenteditable path is vulnerable.) - **Fix:** Run the body through DOMPurify (allowlist tags/attrs, strip event handlers) before assigning to `innerHTML`; do the same in `handleInput`/`insertHtml`. Don't treat "looks like HTML" as trusted. ### 4. Auth JWT + role stored in `localStorage` - **Files:** `web/src/store/auth.store.ts:20-44`, `web/src/lib/axios.ts:11-15`; also `visitorScannerToken` in `web/src/pages/visitors/VisitorScannerLoginPage.tsx:27`, `web/src/hooks/useApproverSocket.ts:14`, `web/src/hooks/useCheckpointSocket.ts:14` - **Scenario:** Any XSS (see #3, #10) steals the token = session hijack. `role` is also read from localStorage for UI hydration, so a user can flip it locally to unlock UI (backend must still enforce). - **Fix:** Move the token to an `httpOnly; Secure; SameSite` cookie (the client already sets `withCredentials: true`); drop the manual bearer attach. Ship a CSP (#19) as defense-in-depth. ### 5. Checkpoint passwords stored in plaintext and returned to any authenticated user - **Files:** `api/src/controllers/visitors.controller.ts:1620` & `:1650` (write `plainPassword`), `:1489` (`listVisitorCheckpoints`, no `select`), `:1624`/`:1659` (return full objects); login fallback in `api/src/controllers/visitor-scanner.controller.ts` - **Scenario:** Each checkpoint stores a bcrypt `passwordHash` **and** a cleartext `plainPassword`. `GET /visitors/checkpoints` is gated only by `requireAuth`, so any sub-admin receives every checkpoint's plaintext + hash. There is also a legacy plaintext-equality login fallback. - **Fix:** Drop the `plainPassword` column; never return `passwordHash`/`plainPassword` (explicit `select`); remove the plaintext-equality login path. ### 6. No rate limiting on OTP / checkpoint login - **Files:** `api/src/controllers/auth.controller.ts:121-160` (`verifyOtp`), `api/src/routes/auth.routes.ts:7-8`, `api/src/routes/platform.routes.ts:13-14`, `api/src/routes/visitor-scanner.routes.ts:27` - **Scenario:** 6-digit OTP, 10-minute window, no attempt counter or throttle → the ~900k space is brute-forceable within the window, yielding a 30-day owner/platform token. The platform path and checkpoint login are equally unthrottled. - **Fix:** Per-OTP failed-attempt lockout (invalidate after ~5 wrong tries); rate-limit send/verify and checkpoint login by IP/identity; throttle `send-otp` to prevent flooding. ### 7. `Visitor` table has zero query indexes - **Files:** `api/prisma/schema.prisma:382-474` (`Visitor`); also `VisitorScanLog` `:552-560`, `VisitorRequest` `:590-619` - **Scenario:** Only `@unique` on `shortId`/`decisionToken`. Every `findMany`, `count`, `groupBy`, and the expiry `updateMany` sequentially scans the whole table, per request, per owner — this amplifies every other perf issue. - **Fix:** Add composite indexes matching access patterns: - `Visitor`: `@@index([ownerId, status])`, `([ownerId, createdAt])`, `([ownerId, visitDate])`, `([assignedAdminId, status])`, `([assignedApproverId, status])`, `([createdByAdminId])`, `([ownerId, status, expiresAt])` - `VisitorScanLog`: `@@index([visitorId])`, `([checkpointId, scannedAt])` - `VisitorRequest`: `@@index([ownerId, status])`, `([checkpointId])`, `([assignedApproverId])` ### 8. `listVisitors` unbounded + heavy include; frontend downloads it all - **Files:** `api/src/controllers/visitors.controller.ts:610-630`; frontend `web/src/pages/visitors/VisitorsPage.tsx:361-366`, `web/src/pages/dashboard/DashboardPage.tsx:135-150`, `web/src/pages/analytics/AnalyticsPage.tsx:482-491`, `web/src/pages/platform/PlatformVisitorsPage.tsx:64-68` - **Scenario:** Returns every visitor with 7 joined relations, no `take`, unindexed `orderBy`. The frontend fetches the full list on four pages and paginates/filters/aggregates in memory (`VisitorsPage` slices to 50 at `:817` *after* downloading everything; `PlatformVisitorsPage` pulls every org's visitors). Payload and client work grow O(total visitors ever created). Same unbounded pattern on `approverListVisitors` (`:313`), `listVisitorRequests` (`:1694`), `listVisitorCheckpoints` (`:1489`), `walkInQR.controller.ts:73`. - **Fix:** Server-side pagination (`take`/`skip` or cursor) + a `select` projection of only the fields the list renders; move filtering/search/sort server-side; server-aggregate analytics instead of shipping raw rows. *(This is the pagination work already discussed — see the coupling with status pills, the Approvals tab, and header stats before implementing.)* ### 9. Lazy `expireOverdueVisitors` write runs on every read/poll - **Files:** `api/src/lib/visitExpiry.ts:19-29`, called at `api/src/controllers/visitors.controller.ts:589` (listVisitors) and `:1144` (getOwnerCounts) - **Scenario:** Every list load and every badge-count poll fires a table-scanning `visitor.updateMany` (see #7) that takes row locks even when nothing has expired. `getOwnerCounts` is typically polled on an interval, so this is a scan+write several times a minute per active user. - **Fix:** Move expiry to the existing 30-min cron (`reminderCron.ts`), or throttle (skip if run within last N minutes). At minimum add the index from #7. --- ## 🟡 Medium ### 10. DOM XSS via `document.write` in the QR print poster - **File:** `web/src/pages/walk-in-qrs/WalkInQRsPage.tsx:107-127` - **Scenario:** Admin-set `qr.label` and the URL are interpolated unescaped into a new window (including inside ``), opened via `window.open('', '_blank')` **without `noopener`** (child retains `window.opener`). A label like `` executes script. - **Fix:** HTML-escape `qr.label`/`url` (or build DOM with `textContent`); open with `window.open(url, '_blank', 'noopener')`. ### 11. Modal components defined inside `VisitorsPage` → remount + refetch every render - **Files:** definitions `web/src/pages/visitors/VisitorsPage.tsx:827` (AddCheckpointModal), `:870` (EditCheckpointModal), `:923` (QRModal), `:989` (PreviewModal); rendered `:2488-2491` - **Scenario:** New function identities each render cause React to unmount/remount them. QRModal (`:943`) and PreviewModal (`:1014`) fire `/whatsapp-templates/.../render` and `/email-templates/.../render` in `useEffect` on mount, so any parent re-render re-fires those calls and resets local state. - **Fix:** Hoist to module-level components; pass data via props. ### 12. ~15 unmemoized full-array passes per render - **File:** `web/src/pages/visitors/VisitorsPage.tsx:589-743` - **Scenario:** `filtered` (`:589`), `visitorsCountScope` + 7 status counts (`:605-622`), the `approvalRows` IIFE (`:652-706`), `approvalsFiltered` (`:708`), `approvalsCountScope` + 6 counts (`:726-743`) all recompute on every keystroke, and hand fresh-identity arrays to `useTableSort`, defeating its memo. - **Fix:** Wrap each derived list/count in `useMemo`; compute the 7 status counts in a single pass. ### 13. Every socket event triggers a full `GET /visitors` refetch - **Files:** `web/src/pages/visitors/VisitorsPage.tsx:411-416`, `web/src/pages/dashboard/DashboardPage.tsx:154-160`; hook `web/src/hooks/useOwnerSocket.ts:67` - **Scenario:** `onVisitorArrived`/`Awaiting`/`Decided`/`onRequestCreated` each re-download the whole visitor table; in a busy lobby every scan broadcasts to every tab. The payload already carries the row (delta path exists at `:425` but is unused). Separately, `useOwnerSocket` re-subscribes on every render because `listeners` is an inline object in its dep array (tears down/re-adds handlers, briefly dropping events). - **Fix:** Apply the payload to state (upsert the single row); debounce unavoidable refetches. Read listeners from a `useRef` (or `useMemo` the listener bag) so the socket effect doesn't re-run each render. ### 14. `bulkCreateVisitors` processes rows fully sequentially - **File:** `api/src/controllers/visitors.controller.ts:992-1120` - **Scenario:** Per row (up to 500), awaited in series: `generateQRCodeBuffer` → `saveUpload` (I/O) → `visitor.create` → optional `visitorReason.create` ≈ up to 1,500 sequential round-trips in one request; latency = sum of all rows. - **Fix:** Generate QR buffers / run uploads concurrently in bounded batches (`Promise.all` over chunks of 10-25); bulk-insert with `createMany`/`$transaction`; `createMany` with `skipDuplicates` for reasons. ### 15. CORS reflects any origin with credentials when `FRONTEND_URL='*'` - **File:** `api/src/server.ts:43-53` - **Scenario:** `FRONTEND_URL='*'` → `cors({ origin: true, credentials: true })` reflects the caller's Origin and allows credentials, letting any site make authenticated cross-origin requests. - **Fix:** Disallow the `'*'` + credentials combination; require an explicit origin allowlist in production. ### 16. Oversized JSON body limit (50 MB) + synchronous base64 decode - **Files:** `api/src/server.ts:54-55`; decode at `api/src/controllers/visitors.controller.ts:577` (`savePhotoIfDataUrl`), called from `createVisitor:781` - **Scenario:** `express.json({ limit: '50mb' })` applies to every route incl. unauthenticated ones — repeated 50 MB posts (decoded into Buffers) can exhaust memory. Photos are decoded synchronously on the event loop with no resize/dimension check and stored full-resolution. - **Fix:** Lower the global limit; apply the large limit only to photo routes; size-check base64 before decode; offload decode + downscale (e.g. `sharp`). ### 17. Client-only permission gates + walk-in QR mismatch - **File:** `web/src/components/layout/AppShell.tsx:140-176` - **Scenario:** Route protection is a client-side redirect only; pages render regardless and a valid token can call the APIs directly. Confirmed mismatch: the walk-in QR gate uses `canAddVisitors || canManageVisitors` (`:166-172`) while the API requires `canManageSettings`, so users enter a page whose actions the backend rejects (403). Settings entry uses a loose `canManageSettings || canManageVisitors` OR (`:154-160`). - **Fix:** Treat gates as UX hints only; confirm every gated action is enforced server-side; align the walk-in-QR gate to `canManageSettings`. ### 18. Notification cleanup does substring `LIKE` scans on a growing table - **File:** `api/src/lib/events.ts:48-49` (`resolveAwaitingNotifications`), `:82-89` (`deleteVisitorNotifications`) - **Scenario:** `body: { contains: '#'+shortId }` can't use an index → full scan of `Notification` (which only grows, no retention). Runs on every approve/reject/cancel/expire; the delete builds an OR of N `contains` clauses. - **Fix:** Add an indexed `visitorId` (or `visitorShortId`) column to `Notification` and filter on it; add a retention job to prune old read notifications. --- ## 🟢 Low / Informational - **19. No Content-Security-Policy / security headers** — `web/index.html`, `api/src/server.ts` — a CSP (`script-src 'self'`, `object-src 'none'`, no `unsafe-inline`) plus `X-Content-Type-Options: nosniff`, `Referrer-Policy`, `frame-ancestors` would blunt #3/#4/#10. - **20. Approval token in URL path** — `web/src/routes.tsx:71-72` (`/decide/:token`) — leaks via Referer/history/logs. Keep single-use + short TTL; set `Referrer-Policy: no-referrer` on that route; prefer POST-with-body over a GET side effect. - **21. No route code-splitting** — `web/src/routes.tsx:1-56` eagerly imports ~25 pages incl. the 2,800-line `VisitorsPage`, 2,300-line `AddVisitorPage`, 1,600-line `VisitorScannerPage`. Add `React.lazy` + ``; consider `manualChunks` in `vite.config.ts`. - **22. Repeated per-request admin-flag lookups** — `api/src/controllers/visitors.controller.ts:686/711/738` — up to 3 `admin.findFirst` for the same row per write. Fetch once at the top and pass down. - **23. PII in logs / dev flags** — `api/src/config/getgabs.ts:92-108` logs recipient number/name/template params; ensure `EXPOSE_OTP_IN_RESPONSE` / `dev-peek-otp` (`auth.controller.ts:82-119`) stay off in production. - **24. Approvals list + rows unoptimized** — `web/src/pages/visitors/VisitorsPage.tsx:2128` renders all approval rows unpaginated/unvirtualized; no `React.memo` on rows, per-row callbacks (`canWhatsAppShare`/`canEmailShare` `:304-305`, `downloadQR` `:557`) recreated each render. --- ## ✅ Verified clean (checked, not issues) - **REST IDOR:** visitor get/update/delete/checkout/history and WhatsApp send scope via `visitorScopeForRequest` (ownerId + admin-tree / `canSeeAllVisitors`); checkpoint, walk-in QR, admin, approver, notification, template-render endpoints scope by `ownerId` and per-row identity. - **Approve/reject email links:** `decisionToken` is `nanoid(32)`, single-use (cleared on decision), length-validated — not guessable. - **Injection:** no `$queryRaw`/`$executeRaw`, no `child_process`/shell; all queries via Prisma. - **Upload path traversal:** files keyed by server-generated `shortId`; mime regex restricts extension — user input never reaches the path. - **Admin CRUD privilege escalation:** flag-capping against parent admin, owner-only guards for `canManageSubAdmins`/`canSeeAllVisitors`, deactivated-account login guard all enforced. - **No secrets committed:** `.env`/`.env.mobile` contain only public API base URLs; no `sk_live`/`AKIA`/bearer tokens in `src`. - **Safe render paths:** WhatsApp preview escapes `& < >` before `dangerouslySetInnerHTML`; email preview iframe uses `sandbox=""`; external links use `rel="noreferrer"`; share links are `encodeURIComponent`-wrapped. - **Already-good backend perf:** notifications list is cursor-paginated; counts use `Promise.all`; `listApprovers` uses `groupBy`; `getCheckpointHistory`/approver-history cap with `take`. --- ## Suggested order 1. **Security first:** #1 (JWT secret) → #2 (socket auth) → #5 (checkpoint plaintext) → #6 (rate limiting) → #3/#4 (XSS + token storage). 2. **Perf foundation:** #7 (indexes) → #8 (pagination) → #9 (expiry cron). 3. **Frontend render/refetch cleanups:** #11–#13, then the remaining Medium/Low items. **Quick, low-risk wins:** #1 (JWT secret), #7 (indexes), #17 (walk-in QR gate).