← Docs Hub Audit · 1 / 4 · Tổng quan PR Annotated diff →

🧹🔐 Cleanup + Security/Performance Audit

Threads API v2 · branch dev · backend (Express 5 + Prisma + Redis + BullMQ)

This PR ships a small, safe code change (dead-import removal) and bundles a full-repo security & performance audit. The code change is low-risk and typecheck-verified; the audit findings are documented for follow-up, not auto-applied.

4files changed
5lines removed
2critical findings
3warning findings
0SQLi / RCE
PASStsc exit 0

What changed

1 · Dead-import removal applied

Ran tsc --noUnusedLocals --noUnusedParameters across the whole repo (79 hits). Critically, I did not blindly strip all 79 — most are not removable:

Only 4 genuine unused import statements were removed (auth middleware, two access-control files, role service). Typecheck passes after removal → provably dead.

2 · Audit documented no code change

OWASP Top-10 + performance pass over auth, webhooks, uploads, logging, raw SQL, rate limiting, indexing, and counter integrity. Findings are in code-review-pr.html (annotated diffs) and implementation-plan.html (risk table).

🎯 Where to focus the review

If you only read three things, read these — ordered by blast radius.

critical  1. Credentials & secrets in logs — src/middlewares/logger.ts, leonardo-webhook.ts

The HTTP logger serializes the entire request body on every request, so POST /auth/login and /auth/register write plaintext passwords to logs. Separately, the Leonardo webhook logs the full Authorization header and bearer token. This is the highest-risk area in the codebase: any log sink (file, CloudWatch, 3rd-party) becomes a credential store. One-line fixes, do these first.

high  2. Auth abuse surface — ratelimit.ts + hook-hls.ts

Rate limiting is a single in-memory global rule — no dedicated limit on /auth/* (brute-force/credential-stuffing) and it breaks the moment you run more than one process (which the compose file does). The HLS webhook secret falls back to a public hard-coded default and compares with non-constant-time !==. These are the things an attacker probes first.

warning  3. Data integrity & feed performance — follower.service.ts + schema.prisma

Denormalized counters (followersCount) are updated outside a transaction → permanent drift on partial failure. And the feed's hot predicate (isDeleted/isHidden/visibility + createdAt sort) has no covering composite index → filesort at scale. Both are cheap fixes with outsized payoff (the index change pairs with an EXPLAIN screenshot for your portfolio).

What is already solid (don't re-litigate)

AreaVerdict
SQL injectionsafe all raw queries parameterized via Prisma.sql / tagged templates
Error handlingsafe no stack leak in prod; Prisma codes mapped to clean statuses
Secret filessafe .env/.env.prod gitignored & untracked
Transport hardeningsafe helmet + compression + 1mb body cap
Paginationstrong keyset/cursor everywhere (scales)
N+1 in feedmitigated author lookups batched, not per-row
Like counterstrong atomic Redis Lua (set + counter + event) before async DB sync

Testing & verification

Merge recommendation

Part 1 (imports) safe to merge as-is. Part 2 critical findings should be a fast-follow PR before any public deployment — the log-redaction fix in particular is a 5-minute change blocking a real credential-exposure path.

See implementation-plan.html for the prioritized vulnerability → fix table.