# MIDHub CRM — Audit Report

Three passes: security, functional QA, and brand/copy. Findings are listed
whether or not I fixed them — some are fixed in this update, some are
flagged for you to decide on, since they involve tradeoffs only you can
make.

---

## 1. Security audit

| # | Finding | Severity | Status |
|---|---------|----------|--------|
| 1 | No brute-force protection on login | High | **Fixed** |
| 2 | Stored XSS via inline attachment rendering | High | **Fixed** |
| 3 | No security headers (X-Frame-Options, etc.) | Medium | **Fixed** |
| 4 | CSRF gap on multipart/form-data endpoints | Medium | **Fixed** |
| 5 | No attachment size cap in the IMAP poller | Medium | **Fixed** |
| 6 | Poller checkpoint only saved after full batch | Medium | **Fixed** |
| 7 | No server-side validation on contact fields | Medium | **Fixed** |
| 8 | Open Next.js 14.x middleware advisories | Medium | **Flagged, not fixed** |
| 9 | Single-process rate limiting won't survive a restart or scale-out | Low | **Flagged, not fixed** |
| 10 | No audit log of who did what | Low | **Flagged, not fixed** |

### Fixed this round

**#1 — Brute-force login.** There was no limit on password attempts against
`/api/auth/login`. Since that single password gates the entire CRM, this
was the highest-priority fix. Now: 5 failed attempts from the same IP locks
that IP out for 15 minutes. Verified with a standalone test replicating the
exact logic — correctly stays open through 4 failures, locks on the 5th,
and a successful login clears the count.

**#2 — Stored XSS via attachments.** The attachment route trusted whatever
`Content-Type` a file claimed and served everything with
`Content-Disposition: inline`. An inbound email with an attachment
mislabeled as `text/html` (or an SVG, which can carry a `<script>` tag)
would execute in your browser the moment you opened it from a contact's
thread — a real, exploitable path since the sender controls the label
attached to their own attachment. Fixed with an allowlist: only genuine
images and PDFs render inline; everything else forces a download instead.
Verified against `text/html`, `image/svg+xml`, and `application/javascript`
— all correctly forced to download.

**#3 — Missing security headers.** Added `X-Frame-Options: DENY` (blocks
clickjacking via iframe embedding), `X-Content-Type-Options: nosniff`
(stops the browser from guessing content types in ways that enable XSS),
`Referrer-Policy: same-origin`, and a restrictive `Permissions-Policy`.

**#4 — CSRF on file-upload endpoints.** JSON POST requests already get
solid CSRF protection for free (the browser requires a CORS preflight that
fails with no server-side allowance). But `multipart/form-data` requests —
used by every attachment upload — don't trigger that preflight. Added a
custom header (`X-Midhub-Crm: 1`) that the middleware requires on every
mutating request; a plain cross-site form can't set custom headers, so this
closes the gap independently of the cookie's `SameSite` setting.

**#5 & #6 — Poller robustness.** Two related issues in `poll-inbox.js`:
no cap on inbound attachment size (a malicious or oversized attachment
could fill your disk over time), and the "last processed" checkpoint was
only saved after the *entire* batch of new mail finished — so a crash
partway through a run (network blip, process killed) meant re-fetching and
re-parsing everything from that run on the next poll. Both fixed: a 25MB
default cap (configurable via `MAX_ATTACHMENT_BYTES`), and the checkpoint
now saves after every single message.

**#7 — Input validation.** `dealStage`, `status`, and `tags` were passed
straight to Prisma with no format checking — malformed input would crash
the route with an unhandled exception instead of a clean error. Added a
shared validation module (`src/lib/validation.ts`) used consistently across
every contact route. Verified against both normal and adversarial inputs
(including a SQL-injection-shaped string in a dealStage field) — all
correctly rejected before reaching the database. Prisma parameterizes
queries regardless, so this was a robustness fix more than an injection
risk, but it closes an unhandled-exception path either way.

### Flagged, not fixed

**#8 — Open Next.js 14.x advisories.** `npm audit` flags several issues in
the 14.x line — including ones touching middleware behavior — with no
patch available short of upgrading to Next 15/16. This matters more here
than in a typical internal tool because your entire login system runs
through `src/middleware.ts`. I'm not force-upgrading a major framework
version without your sign-off, since it can have breaking changes worth
testing deliberately. **My recommendation stands from before: password-
protect the subdomain in cPanel as an independent second layer, and plan
the Next 15/16 upgrade as near-term work, not indefinite deferral.**

**#9 — Rate limiting is in-memory.** The brute-force protection resets if
the app restarts, and won't work correctly if you ever run multiple server
instances behind a load balancer. Fine for a single cPanel Node process;
worth revisiting (e.g. a small database-backed counter) if that ever
changes.

**#10 — No audit trail.** There's no record of login attempts or who
changed what, beyond the database's own `updatedAt` timestamps. Low
priority for a single-user tool, but worth knowing if you ever add a second
user.

---

## 2. Functional QA — does it actually do what it's for?

The purpose was: ingest email into a database, send from `info@`, let
people reply back to `info@`, without disturbing your general inbox. Here's
where I tested that against the actual behavior, not just the intent.

| # | Question | Finding | Status |
|---|----------|---------|--------|
| 1 | If you email an Unassigned contact from Compose, does the queue update? | No — it stayed stuck in the queue despite an active conversation | **Fixed** |
| 2 | Same question, for replying within a contact's thread | Same bug | **Fixed** |
| 3 | Does typing the same recipient twice in Compose send it twice? | Yes | **Fixed** |
| 4 | Are emails matched case-sensitively? ("Grace@x.com" vs "grace@x.com") | Inconsistent — poller lowercased, Compose/API didn't | **Fixed** |
| 5 | Does the IMAP poll ever touch the real inbox's read state? | No — confirmed via `readOnly: true` (issues `EXAMINE`, not `SELECT`) | **Passed** |
| 6 | Can you un-ignore an archived/rejected Unassigned contact? | Only via direct API call, no UI | **Flagged** |
| 7 | Is there a UI to delete a contact? | No — the DELETE endpoint exists but nothing calls it | **Flagged** |
| 8 | What happens on first-ever poller run — does it import your whole mailbox history? | No, deliberately — baselines at "now" | **Passed (by design)** |
| 9 | Tags are fixed to lead/customer/vendor with no way to add a custom one | Confirmed limitation | **Flagged** |

### Fixed this round

**#1, #2 — Contact status didn't reflect real engagement.** This was the
most meaningful functional gap. If someone was sitting in Unassigned and
you emailed them back — either through Compose or a thread reply — they
stayed marked Unassigned even though you'd clearly decided to engage. The
Unassigned count on your Overview page would have been actively misleading
about how many people genuinely still need a decision. Fixed: sending to
anyone who isn't already Active now promotes them automatically, in both
Compose and the thread-reply endpoint.

**#3, #4 — Recipient handling.** Typing `grace@x.com` twice in Compose sent
and logged the message twice. Separately, the poller normalized addresses
to lowercase for matching but the web app didn't — meaning "Grace@x.com"
from a form submission and "grace@x.com" from an inbound email could have
silently become two different contact records for the same person. Both
fixed: recipients are deduplicated case-insensitively, and all contact
creation/lookup now normalizes to lowercase consistently.

### Passed as designed

**#5 — Non-destructive IMAP.** This was the most important behavioral
requirement given `info@` is your shared inbox, so I verified it
specifically: `poll-inbox.js` opens the mailbox with `{ readOnly: true }`,
which IMAP's `ImapFlow` library implements via the `EXAMINE` command rather
than `SELECT` — a protocol-level distinction, not just an application-level
promise. It's structurally not possible for this script to mark mail read,
move it, or delete it.

**#8 — No history backfill.** Confirmed this was the right call: importing
years of mailbox history into the CRM on first run would have flooded the
Unassigned queue with old, irrelevant threads. It baselines at "now" and
only ingests what arrives after that point.

### Flagged, not fixed (need your call)

**#6, #7 — Missing UI for two states that exist in the data model.** You
can archive/ignore a contact and delete one via direct API calls, but
there's no button for either in the interface (approve/ignore exists for
the Unassigned queue; general delete and un-archive don't have a home yet).
Not wrong, just incomplete — worth a small follow-up if you find yourself
needing either.

**#9 — Tags are a fixed set of three.** `lead`, `customer`, `vendor` cover
the cases you described, but there's no way to add a custom tag from the
UI. Fine if that's genuinely all you need; flagging in case it isn't.

---

## 3. Copy and brand review

I checked every user-facing string against two things: does it read like a
person wrote it (not inflated, promotional, or formulaic), and does it
actually deliver the brand — not just claim to.

**Copy:** scanned all page descriptions, button labels, empty states, and
error messages for the usual AI-writing tells — inflated language
("seamless," "empower," "unlock," "leverage"), the rule-of-three cadence,
promotional framing, vague hedging. Found none. The copy was already
direct and functional — sentences like "New senders to info@ that aren't in
your contacts yet" or "Sends from info@midhubsolutions.com" — which matches
how you've asked me to write elsewhere. Nothing to change here.

**Brand — two real gaps, both fixed:**

- **Carlito wasn't actually loading.** It was referenced in the Tailwind
  font stack, but nothing loaded the actual font file — so every visitor
  fell back to whatever generic sans-serif their system had, not the brand
  typeface. Carlito is available on Google Fonts, so I wired it up properly
  through `next/font/google`, the same way Poppins already was. This is a
  real rendering difference, not a cosmetic nitpick — the brand guide
  specifies Poppins + Carlito, and the app was only delivering one of them.
- **The tagline appeared nowhere.** Added "Delivering Excellence, Powering
  Innovation" to the login screen — the one place it earns its spot without
  cluttering a working dashboard someone uses daily.

**One deliberate deviation, explained rather than changed:** status pills
and error text use semantic colors (yellow for "paused," red for
"bounced"/errors) rather than pulling from Hub Orange/Hub Green. This is
intentional — if "danger" and "brand accent" were both orange, you'd lose
the ability to tell them apart at a glance. Brand colors are used for every
primary action, link, and piece of navigation; semantic colors are reserved
strictly for status meaning. I'd flag it if you disagree, but I'd recommend
keeping the distinction.

---

## What to do with this

The zip in the next message has every fix above already applied — this
isn't a to-do list for you to implement, it's a record of what changed and
why. The "flagged, not fixed" items are genuine decisions, not oversights:
the Next.js upgrade has real testing cost, the missing UI buttons are scope
calls, and the tag list might just be complete as-is. Your call on each.