open-legal-products/mike#252: [Security 9/9] Backend ESLint security rules, tiered to stay credible

⛔ closed · #252 · open-legal-products/mike ← amal66/mike · opened 3mo ago by amal66 · closed 3mo ago · +1,962-62 across 5 files · ↗ on GitHub

From the PR description

[Security 9/9] Backend ESLint security rules, tiered so the gate stays credible

Part of the split of #227 into single-topic PRs. Index: tracking comment on #227.

TL;DR

Add eslint-plugin-security in a flat config over backend/src, wired to a CI job. Rules that only fire on genuinely unsafe APIs (eval with an expression, new Buffer, pseudoRandomBytes, buffer noassert) are errors that block CI; heuristic detectors (object injection, non-literal fs/regex, timing attacks) are warnings that inform review. Currently 0 errors / 131 warnings on main.

Risk to user data

Severity: preventive. Static analysis doesn't fix a live breach; it stops whole classes of the bugs the other eight PRs fix by hand from being reintroduced. pseudoRandomBytes for a token, eval on user input, an unauthenticated new Buffer(n) - these are the raw material of the SSRF, crypto, and injection issues addressed elsewhere in this split. A gate catches them at PR time instead of in production.

Flows affected

CI only (.github/workflows/security-lint.yml, npm run lint).
No runtime code change.

Attack precedent - on the process

The failure mode here isn't an exploit, it's alert fatigue: a linter that cries wolf gets ignored, then disabled, then catches nothing. The security detectors are heuristic and false-positive on legitimate code (e.g. detect-object-injection fires on any obj[variable]). A gate that's 90% noise trains reviewers to rubber-stamp.

Possible fixes, and what we chose

Option	Verdict
All security rules as errors	131 mostly-false-positive failures → the gate gets disabled within a week. Rejected.
All as warnings	Never blocks anything, so a real `eval`-on-expression sails through. Rejected.
Turn on only the high-precision rules	Better, but loses the heuristic detectors' review value entirely.
Tier by precision: unsafe-API rules = error, heuristics = warning	Chosen. The blocking tier is trustworthy (near-zero false positives), so a red build means something; the warning tier still surfaces suspects for human review without gating.

flowchart TD
    R["eslint-plugin-security rule fires"] --> P{fires only on a<br/>genuinely unsafe API?}
    P -- "yes<br/>(eval-expr, new Buffer,<br/>pseudoRandomBytes, noassert)" --> E["ERROR → fails CI"]
    P -- "no<br/>(heuristic: object injection,<br/>non-literal fs/regex, timing)" --> W["warning → informs review,<br/>never blocks"]

The principle generalizes past linters: a security gate's credibility is a resource you can spend, and a noisy blocking gate spends it to zero. Precision-tier before you make anything blocking.

What's in this PR

backend/eslint.config.mjs - flat config, tiered rules (new).
backend/package.json + lockfile - eslint toolchain (lockfile regenerated on current main).
.github/workflows/security-lint.yml - CI job running npm run lint.

Kept as its own workflow file so it composes with the existing ci.yml rather than conflicting. The config is the amal66 fork's apps/api config minus its fork-specific no-console rule (this backend logs via console).

Reading

eslint-plugin-security · OWASP: static analysis (SAST)

[Security 9/9] Backend ESLint security rules, tiered to stay credible