Skip to content

Security

_david edited this page Sep 26, 2026 · 2 revisions

Security

Protections in place

  • NoSQL injection: QuerySafe class (utils/querySafe.ts) whitelists allowed fields and rejects values containing $ or javascript: before any value reaches a Mongo query. Gotcha: QuerySafe.safeQuery() only accepts typeof value === 'string' — passing a Mongoose ObjectId instance directly (instead of .toString()-ing it first) causes the filter to be silently dropped, not rejected, which has caused two real incidents (see below) — always stringify ids explicitly before passing them through safeQuery, and always check the sanitized query actually contains the expected key before using it, rather than assuming rejection = throw.
  • Password hashing: bcrypt, 12 rounds (utils/bcrypt.ts). Never stored or compared in plaintext.
  • Password never returned by the API: candidate.service.ts's read paths explicitly exclude password.
  • Token blacklist + session revocation: Redis (blacklist:{token} key, TTL = remaining token lifetime) with in-memory fallback, plus a separate per-candidate "invalidated before" timestamp for logout-all — see Authentication.
  • CSRF: double-submit check required whenever a state-changing request authenticated purely off the httpOnly cookie (no Bearer header) — see Authentication.
  • CORS: CORS_ORIGIN-driven allow-list with credentials: true, fail-closed in production when unset — this replaced a previous origin: '*' in every environment, fixed alongside the httpOnly-cookie auth work (issue #119).
  • Rate limiting: Redis-backed, 100 req/15min general / 150 req/15min on /auth/*; falls back to in-memory on Redis failure.
  • Ownership enforcement: every authenticated write is scoped to req.user._id from the verified JWT — never a client-supplied id. Enforced centrally in verifyToken.middleware.ts (forces req.body.candidateId) plus an explicit existing-document ownership check in baseUpdateDocument/baseDeleteDocument (services/index.ts), which since issue #136 also excludes already-soft-deleted documents from that check.

Known gaps (not yet fixed)

  • No request body size limit on bodyParser.json() (server.ts) — DoS risk via oversized payloads.
  • No HTTP security headers (Helmet not installed).
  • No HTTPS/HSTS enforcement at the app layer (relies entirely on Render's edge).
  • Uploaded/generated personal files are reachable unauthenticated via express.static — anything written under public/ (PDF exports, uploaded CV PDFs, CV-section images) is served with no auth check at the static-file layer, only at the API routes that also happen to serve the same data (e.g. GET /candidate/cv-file is authenticated, but the same file sitting in public/uploads/cv/ is fetchable by anyone who can guess or obtain the filename). Confirmed live for both PDF export output and uploaded CVs.
  • PDF export's Chrome executable path is still effectively hardcoded in services/createPDF.ts — works today because Docker installs Chromium via apt at the expected path (with PUPPETEER_EXECUTABLE_PATH set) and Render's own environment happens to have it where Puppeteer expects, but nothing makes this robust to an arbitrary CI/Docker/serverless environment; a proper fix (dynamic puppeteer.executablePath() resolution or an explicit required env var) hasn't landed.
  • auth.service.ts's register path skips Mongoose model-level validation before save (a TODO comment marks this in the code) — write path isn't fully validated beyond the Joi layer.
  • No npm run lint script despite .eslintrc.cjs existing in the repo.

Incident history

2026-09-19 — NoSQL-filter-collapse: an identifier rejected by QuerySafe silently returned an arbitrary candidate

Found by /code-review. QuerySafe.safeQuery drops a rejected field (e.g. one containing "$") instead of throwing; candidate_me/index.ts's slug/email lookup and visit-recording lookup both then queried Mongo with the sanitized result unconditionally — so a rejected identifier collapsed the filter to {} and matched an arbitrary candidate, leaking that candidate's full public profile (GET /api/me/:identifier, no auth required) or misattributing a visit record. Fix: both call sites now check the sanitized query actually contains the expected key before querying — a rejected identifier now fails closed (treated as not-found) instead of matching anything. A sibling instance of the same bug class exists in candidate.service.ts's authenticated GET /candidate/:email and has not been fixed yet (own follow-up).

2026-09-19 — Soft-delete bypass: an already-deleted CV-section document could still be mutated via update/patch

Found by /code-review. The shared existence/ownership check used by baseUpdateDocument/basePatchDocument had no deletedAt filter, so a soft-deleted document — correctly hidden from GET/list — could still be found and mutated via PUT/PATCH .../update. Fix: that check gained an opt-in excludeDeleted flag, now passed by update/patch (restore intentionally still needs to find already-deleted documents, so it's exempt).

2026-08-21 — Critical: broken access control (IDOR) across all CV section CRUD

Discovered via a full manual API regression pass. req.user._id (set by verifyToken from the JWT) was not cross-checked anywhere in candidate_profile/* or candidate.service.ts — every list/create/update/delete trusted a client-supplied candidateId/_id in the request body instead. Live-confirmed: an unrelated authenticated user could read, overwrite, or delete another candidate's education/experience/etc. records and even overwrite their entire profile, just by supplying that candidate's id in the request body.

Fix: verifyToken middleware now force-overwrites req.body.candidateId with the authenticated user's own id. baseUpdateDocument additionally checks the existing target document's candidateId against the authenticated user before allowing an update. baseDeleteDocument already had an ownership check, but it was fed a spoofable value — fixed by the same middleware change.

2026-08-21 — Critical: public profile / PDF export leaked every candidate's CV data

candidate_me/index.ts's public-profile handler passed a raw Mongoose ObjectId (not a string) into QuerySafe.safeQuery(). Per the gotcha above, this silently dropped the candidateId filter, so the public profile and PDF export returned every candidate's CV section data blended together for any request. Fixed with an explicit .toString().

2026-08-21 — Password hash leaked in candidate responses

GET /api/v1/candidate/:email and PUT/PATCH /candidate/update returned the bcrypt password hash in the response body, due to a missing/double-wrapped field-selection bug. Fixed by explicitly excluding password on both paths.

2026-08-21 — POST /api/v2/auth/register completely broken

Missing await on the password-hashing call in what was then a separate, legacy v2 auth implementation meant a Promise object was passed to Mongoose as the password field, failing every registration attempt. This code path no longer exists — /api/v2/auth now calls the same real implementation /api/v1/auth uses (see Architecture).

Full diffs, root-cause writeups, and live-test transcripts for all of the above: agent-hub/evidence/ in the repo (implementer notes under evidence/implementer/<date>/, independent verification under evidence/verifier/<date>/).