Scope: the FaceTrust AI API running locally (localhost:3000), backed by real Postgres/Redis, real face-engine, and real nin_bvn_simulator instances — not a static code review. Every finding below was demonstrated with an actual request/response against the live service, not inferred from reading the source. Conducted as authorized testing against infrastructure the same team owns and operates.

​
Summary

The most significant finding — a complete bypass of the brute-force protection on identity-linking face comparisons — was found, fixed, and the fix re-verified live in this same session. Two findings (CORS policy, operator-login lockout strategy) are configuration/design trade-offs that need a real answer (allowed frontend origins; whether username-only rate-limiting is acceptable) rather than a code fix I should make unilaterally, so they’re documented with concrete remediation options instead.

​
High: verify-face brute-force protection was fully bypassable

Where: POST /identity/{nin,bvn}/verify-face The endpoint was rate-limited (verifyFaceLimiter in middleware/rateLimit.ts), but keyed by validationToken — the short-lived token minted by POST /identity/{nin,bvn}/validate. Since validate itself had no rate limit at all, an attacker could reset their attempt budget on demand:
This was demonstrated live: 3 rounds of “re-validate → 1 attempt” all returned face_mismatch (never rate_limited), proving unlimited face-comparison attempts against a single NIN/BVN were possible. This directly defeated the control the spec calls for (story 48: “repeated verify-face attempts against one NIN/BVN are throttled as a brute-force-identity-theft control”). Fix: added a second, durable counter (src/lib/verifyFaceAttempts.ts) keyed by a hash of the NIN/BVN number itself, not the token — it survives token rotation because it’s bound to the thing actually being attacked, not the credential used to attack it. Same INCR/EXPIRE pattern already used for OTP attempt-limiting in modules/auth/service.ts. The original token-keyed limiter stays in place as a cheap first-pass throttle; the number-keyed counter is now the actual authoritative control. Re-verified after the fix: 7 rounds of “re-validate → 1 attempt,” each with a brand-new validationToken — attempts 1-5 returned face_mismatch, attempts 6-7 correctly returned 429 rate_limited, even though every single attempt used a token that had never been used before.

​
Low: oversized file upload crashed to a generic 500

Where: any multipart/form-data endpoint (verify-face, /identify, /verify/self) middleware/upload.ts caps uploads at 10MB via multer, but multer’s own MulterError (thrown when the limit is exceeded) wasn’t recognized by the global error handler — it fell through to the generic catch-all, returning 500 internal_error instead of a proper 413. Not itself exploitable (the error handler already never leaks stack traces to the client either way — see the Informational section), but it’s an incorrect status code, and it would have shown up as a false “unhandled server error” in the error-rate alert (HighHttpErrorRate in observability/prometheus/alert_rules.yml) for what’s actually a well-formed client mistake. Fix: middleware/errorHandler.ts now recognizes multer.MulterError explicitly and maps its code to the right status (LIMIT_FILE_SIZE/LIMIT_FILE_COUNT → 413, LIMIT_UNEXPECTED_FILE → 400). Re-verified: an 11MB upload now returns 413 {"error":"upload_limit_file_size","message":"File too large"} instead of a 500.

​
Medium: CORS allows any origin, on every method

Where: every endpoint (app.ts’s cors() middleware, default config)
Any website can issue cross-origin requests to this API from a visitor’s browser. The practical exposure is lower than it would be with cookie-based sessions — this API uses Authorization: Bearer <token> headers, which a malicious page on another origin cannot silently attach (same-origin policy keeps localStorage/JS-held tokens out of reach unless that other frontend has its own XSS hole) — but it’s still a real hardening gap and a bad default to carry into production: it means there is currently no origin allowlist to fall back on as a second layer if a frontend ever does something wrong with where it stores a token. Not fixed — this needs an actual list of legitimate frontend origins (mobile app doesn’t use CORS at all; the B2C web client and merchant desk web client both do) to configure cors({ origin: [...] }) correctly. Flagging with a recommendation rather than guessing at origins and getting it wrong.

​
Medium: operator-login rate limiting enables a no-credentials lockout DoS

Where: POST /auth/operator/login, operatorLoginLimiter in middleware/rateLimit.ts The limiter is keyed purely by operatorCode (10 attempts / 5 min), with no distinction between IP addresses or between failed and successful attempts. Demonstrated live: 11 wrong-password attempts against admin001 triggered the rate limit — and the next legitimate login attempt with the correct password was also blocked until the window expired. Since operator codes are often predictable (admin001, teller003, sequential per-org schemes), anyone who knows or guesses a real operator’s code can lock them out of the system for 5 minutes at a time, indefinitely, without ever needing a valid password. Not fixed — the direct fix (key by operatorCode + IP instead of operatorCode alone) reduces this but doesn’t eliminate it against a distributed attacker, and changes operational behavior (a shared-NAT branch office hammering one wrong operator code would no longer protect its other operators) — a decision worth making deliberately rather than as a pentest-driven quick patch. Documenting with the concrete remediation option rather than applying it unilaterally.

​
Informational: dependency vulnerabilities (all in unused code paths)

npm audit reports 50 advisories (1 critical, 5 high, 44 moderate) — but every single one traces back to one of two places, neither of which is exercised by this service’s actual behavior:
  • @opentelemetry/auto-instrumentations-node pulls in instrumentation packages for dependencies this service doesn’t use at all — amqplib, aws-lambda, mongodb, mysql2, kafkajs, etc. — and the vulnerable code (@opentelemetry/core’s baggage-propagation memory issue, the Jaeger propagator DoS) lives inside those unused instrumentations’ shared dependency tree, not in the HTTP/Express/pg/ioredis paths this app actually runs.
  • vitest/vite/esbuild (critical: arbitrary file read via Vitest’s UI server) are dev/test-only tooling, never included in a production build, and the vulnerable Vitest UI server is never started anywhere in this codebase.
npm audit fix has no fix available without --force, which would pull in breaking major-version bumps (@opentelemetry/auto-instrumentations-node 0.51→0.79, @opentelemetry/sdk-node 0.55→0.221, vitest 2→4) — not something to apply without testing the OTel pipeline and test suite afterward. Recommendation: trim getNodeAutoInstrumentations() in src/instrumentation.ts to only the instrumentations actually relevant here (http, express, pg, ioredis) via its enabled: false per-package options, which shrinks both the attack surface and most of this list at once; schedule the major-version bumps as a separate, tested change.

​
What held up

Substantial testing across auth, authorization, injection, and business-logic categories found the following controls working exactly as designed, with no exploitable gap:
  • IDOR — every cross-user attempt (profile, PATCH, DELETE, history, history export, preferences) correctly returned 403, scoped by comparing the URL’s :id against the authenticated token’s subject.
  • JWT integrity — signature tampering (flipping subjectType/sub in the payload while keeping the old signature) and the classic alg: none forgery both correctly rejected with 401. jsonwebtoken’s default verify() behavior (HMAC-only inference from a string secret) already blocks alg: none without extra config.
  • Refresh token rotation — reusing an already-rotated refresh token, or guessing a random UUID, both fail cleanly; single-use + revocation-on-rotation confirmed live.
  • Cross-user token binding — validation_token, link_token, and proof_token are all checked against the issuing user’s session server-side (not just possession of the token). Demonstrated: user B could not use user A’s validationToken to verify-face, could not steal A’s linkToken to complete a link on B’s account, and could not use A’s proofToken to authorize a change to B’s profile.
  • Role-based access control — consumer tokens against every operator/admin-only route (/operators, /identify, /disputes, /analytics/*, /settings/attestation-required) correctly rejected. An active (password-reset-complete) non-admin operator correctly could not create/reset/patch operators or self-escalate their own role.
  • SQL injection — not exploitable. vectorRepository.ts uses $queryRawUnsafe/$executeRawUnsafe (Prisma’s “unsafe” only refers to the query string being programmatically built, not fixed at compile time), but every actual value — including the embedding vector literal — is passed as a bound $1/$2 parameter, never string-concatenated into the SQL text. Confirmed by code review and black-box injection attempts (', ; DROP TABLE, NoSQL-style operator objects) against every text input, all rejected at the zod schema layer before reaching a query.
  • Mass assignment — PATCH /users/{id} uses a .strict() zod schema; attempting to smuggle role/status/id/name fields through the profile-update endpoint is rejected outright, not silently dropped.
  • Error handling — the global handler never returns a stack trace, internal error message, or library-specific detail to the client; every unhandled exception becomes a flat {"error":"internal_error","message":"An unexpected error occurred"}, with the real detail only reaching the server-side logger.
  • OTP brute force — two independent layers (a 5-attempt Redis counter tied to the specific code, and a 5-per-5-min IP-independent rate limiter) both fire correctly; verified live with 7 wrong-code attempts.
  • Secrets hygiene — .env is git-ignored and confirmed not tracked in this repository.
  • Immutable settings — PATCH /settings/attestation-required has no code path that can succeed; it unconditionally throws, which was the explicit design intent (see Architecture).

​
Known, already-documented caveats (not new findings)

  • Liveness stub (LIVENESS_MODE=stub default) always passes — this is a deliberate, loudly-logged placeholder documented in Architecture → The liveness gap and face-engine’s own docs, not a discovery of this audit.
  • Dev-placeholder secrets (JWT_SECRET, PII_ENCRYPTION_KEY in .env.example) are named ...-change-me specifically so they’re never mistaken for production-ready — restated here as a pre-production checklist item alongside the liveness stub, not a new gap.
  • Multer has no fileFilter — any file type/content up to 10MB reaches face-engine before content-based rejection (undecodable_image) happens downstream. Not exploitable for code execution (face-engine only ever decodes bytes as an image, never executes anything), but adding a MIME-type allowlist at the API boundary is a reasonable defense-in-depth layer worth adding alongside the fixes above.