Security hardening: body size limit, Gemini key header, AI rate limiting, playbook path traversal

[TFT444]

Summary

Closes #149 (FIX 1: Security hardening).

• SEC-001 (GET /api/* bypasses JWT): already fixed on dev via feat(auth): require JWT for GET /api/* endpoints with optional public demo mode #144; added a regression test to lock it in.
• SEC-002 (no request body size limit): added MAX_CONTENT_LENGTH (2MB) with a JSON 413 error handler.
• SEC-003 (Gemini key leaks via URL/logs): moved the API key from the ?key= query param to the x-goog-api-key header.
• SEC-004 (no rate limiting on AI endpoints): added a lightweight in-memory sliding-window limiter (20 req/min/IP), applied to all 5 AI routes; auto-disabled in test mode.
• SEC-005 (playbook path traversal): rule_id is now validated against a strict pattern and the resolved script path is confirmed to stay inside playbooks/cli/ before being read.

Test plan

• tests/test_app_security.py — JWT enforcement on GET routes, 413 on oversized body
• tests/test_ai_provider.py — Gemini key sent via header, absent from URL
• tests/test_rate_limit.py — limiter enforces per-route/per-IP limits, bypassed in test mode
• tests/test_findings_playbook.py — traversal/absolute-path/slash rule_id rejected, valid rule_id still works
• Full suite: 144 passed, 2 skipped, 2 pre-existing failures unrelated to this change (confirmed identical on a clean dev checkout)

[m-khan-97]

Reviewed the full diff. The Gemini header fix and the playbook path-traversal fix (regex allowlist + resolve() + is_relative_to() double-guard) are both correct and well covered by tests — good work on SEC-003 and SEC-005.

Two things on the rate limiter (SEC-004) I want to raise before we call this closed:

1. State is per-process, but startup.sh runs gunicorn --workers 2. _hits is a plain module-level dict, so each worker process keeps its own count. That means "20 requests/min per IP" is actually enforced as up to ~40/min with our current startup script — not a hypothetical multi-instance scenario, it's true today.
2. The limiter keys on request.remote_addr with no ProxyFix/X-Forwarded-For handling, and we sit behind Render's proxy. If Render doesn't forward the real client IP straight through to the container, every external client collapses into a single remote_addr, which turns "20/min per client" into "20/min for the whole API" — a much harsher and almost certainly unintended limit. Can we confirm what Render actually hands us here before relying on this in prod?

Minor/non-blocking: _hits entries for a given IP+path are never evicted once that key goes quiet, so there's slow unbounded memory growth over a long-running process. Worth a TODO, not urgent.

None of this is a regression risk and everything else here is solid, but I'd like the rate-limiting behavior confirmed against our actual Render setup (or moved to something process-shared, e.g. Redis-backed) before we treat SEC-004 as fully closed. Requesting changes on that basis — happy to re-review quickly once addressed.

[Vishnu2707]

@TFT444 , fixes requested.

[ritiksah141]

I reviewed this against #149. The security fixes are moving in the right direction, and the focused tests pass locally, but I am requesting changes before merge.

Findings:

1. The branch fails the current dev lint and format gate. Current dev now enforces Ruff checks, and the touched files do not pass that gate:

   • api/app.py:263 has E402 Module level import not at top of file
   • api/routes/findings.py:100 has E741 Ambiguous variable name: l
   • api/routes/findings.py:109 has F541 f-string without placeholders
   • ruff format --check would reformat api/app.py, api/rate_limit.py, api/routes/ai.py, api/routes/findings.py, and api/services/ai_provider.py

   Please rebase or merge latest dev, then run the current lint and format checks before this merges.

2. api/rate_limit.py:30 keys the AI rate limit on request.remote_addr. On Render or any reverse-proxy deployment, that value can be the proxy or load balancer address instead of the real client. That means unrelated users can share one bucket, and one noisy client can rate-limit everyone behind the same proxy. Since SEC-004 is meant to protect AI endpoints per caller, this should use a trusted proxy-aware client identity helper, for example via configured proxy handling or a controlled access_route/forwarded-header strategy.

3. This branch is stale against current dev and needs revalidation with the newer app changes. The branch predates the current observability and CI updates, and the merge base shows overlap in api/app.py and api/routes/findings.py. Please update the branch and make sure the current /ready, /metrics, request-id/error handling, async scan persistence, and new CI checks still pass after the security changes are applied.

Validation I ran:

• Focused security tests: 17 passed
• Full pytest: 125 passed, 2 skipped, 1 failed
• The full pytest failure is the known local ONNX Runtime temp-directory error in tests/test_ai_hallucination_guard.py::TestHallucinationGuard::test_vector_store_purity, not related to this PR
• git diff --check dev...HEAD passed
• PR checks currently shown on GitHub are the older Run All CI Checks and CodeQL checks, not the current dev CI pipeline

[TFT444]

Gonna be fix soon.
I was busy with Something

thanks for the review