Skip to content

fix(security): rate-limit follow-ups v2 — alert rule, fail-fast env gate, dependency removal, dead code (card 5221c63f) - #282

Merged
SabaTech-dev merged 6 commits into
mainfrom
fix/rate-limit-followups-5221c63f
Oct 13, 2026
Merged

SabaTech-dev merged 6 commits into
mainfrom
fix/rate-limit-followups-5221c63f

Conversation

@SabaTech-dev

Copy link
Copy Markdown
Owner

Re-scope 01-oct (review batch): reemplaza a #264

Base nueva: origin/main @ 8d9f5cb (la base de #264, fix/rate-limit-fail-open @ 33bfeef, quedó obsoleta al cerrarse #106 como superseded — su contenido vive en la línea F-1 de #270). Este PR sustituye a #264 (cerrable como superseded).

Relación con #270 (línea F-1→F-7, OPEN): desacoplado a propósito — este PR no toca los mismos hunks salvo los deletes de dead code (both-delete idéntico, merge limpio). La métrica rate_limit_backend_failures_total llega a main vía #270; la alerta es safe pre-merge (serie ausente = nunca dispara). El fix del hook bandit en pre-commit es byte-idéntico al de #270.

Los 4 items (card 5221c63f)

1. Alerta Prometheus RateLimitBackendFailures + tests

  • rate_limit_backend_failures_total > 0, severity warning, for: 1m, en dashboard/monitoring/prometheus/alerts/qa-framework-alerts.yml (el fichero cableado en prometheus.yml → rule_files).
  • Tests: dashboard/backend/tests/unit/test_alert_rules.py — presencia, expr, severity/for, unicidad de nombres de alerta.

2. Fail-fast de RATE_LIMIT_FAIL_MODE en deploy/CI (L-A security)

  • scripts/validate_rate_limit_fail_mode.py: valida el valor en los compose trackeados (open|closed, tolerante a case/whitespace igual que el middleware strip().lower()). Typo cloed → error claro con valor, fichero y valores permitidos, exit 1.
  • Wiring blocking: step en pr-checks.yml (job raíz) + Check 15 en scripts/pre-deploy-check.sh.
  • Tests: tests/unit/test_validate_rate_limit_fail_mode.py (6 casos, incl. compose unparsable y multi-fichero).

3. check_rate_limit: eliminada con justificación (opción B de la card)

4. Dead code borrado (0 imports re-verificado sobre ESTA base)

Commits

  1. chore(ci): pre-commit bandit fix (alineado byte a byte con 🚀 [GO Joker 01-oct] Línea F-1→F-7 → main (F-123 0697a60 + F-6/F-7 47205d6) #270 — el hook antiguo bloqueaba todo commit .py)
  2. feat(monitoring): item 1
  3. feat(deploy): item 2
  4. refactor(rate-limit): item 3
  5. chore(cleanup): item 4 + CHANGELOG

Verificación local

  • Backend: tests/middleware + tests/unit/test_alert_rules.py → 47 passed.
  • Raíz: tests/unit/test_validate_rate_limit_fail_mode.py → 6 passed.
  • Smoke: validador sobre los 2 compose del repo → OK. YAMLs parsean, bash -n pre-deploy OK.
  • Preexistentes (no tocados por este PR): 6 falros locales de test_browser_use_agent_013_runtime (entorno local; en CI pasan) y 3 wiring 403-vs-401 locales (idem, 🚀 [GO Joker 01-oct] Línea F-1→F-7 → main (F-123 0697a60 + F-6/F-7 47205d6) #270 verde en CI con los mismos).

Conflictos conocidos

NO mergear desde aquí — gate de Alfred (card 5221c63f).

Joker added 5 commits October 12, 2026 23:50
Identical change to the one already open in #270: the old
`-r src -f json` args made pre-commit append staged .py files as
targets, which bandit's argparse rejects — blocking every .py commit.
Applying it here too so this branch's hooks run for real; both changes
are byte-identical and merge cleanly.
…sts (card 5221c63f)

Fires on rate_limit_backend_failures_total > 0 for 1m (severity
warning): the security condition for the default-open rate-limiter
fail mode in BETA. Safe pre-merge: the counter arrives with the F-1
line and an absent series never fires. Tests validate rule presence,
expression, severity/for and alert-name uniqueness.
…nd pre-deploy (card 5221c63f)

New scripts/validate_rate_limit_fail_mode.py rejects any value other
than open|closed (case/whitespace tolerant, mirroring the middleware's
strip().lower()) across tracked compose files — a typo like 'cloed'
previously fell silently back to open. Wired as a blocking step in
pr-checks.yml and as Check 15 in scripts/pre-deploy-check.sh.
…d 5221c63f)

Zero usages on current main (grep Depends(check_rate_limit) = 0).
RateLimitMiddleware is the single live mechanism; the dependency (and
the untested 503 branch that only ever existed on the superseded
PR #106 line, never ported to F-1) only offered an untested second
path. Advisory resolved as 'remove' per card option. Also strips
pre-existing trailing whitespace so hooks pass.
…21c63f)

granular_rate_limiting.py (188 lines) and rate_limiting.py (33 lines):
zero importers on current main, re-verified before deletion. Supersedes
the same deletions already present in PR #270 (both-delete resolves
cleanly). CHANGELOG updated for all four card items.
@github-actions

Copy link
Copy Markdown

PR Check Results

✅ Code Formatting: Failed
✅ Linting: Passed
✅ Type Checking: Failed
✅ Tests: Failed

⚠️ Some checks failed. Please review the details above.

@github-actions

Copy link
Copy Markdown

⚠️ WARN Trivy Security Scan Results

Severity Count
🔴 CRITICAL 0
🟠 HIGH 47
Total 47

Scans performed:

  • 📁 Filesystem (vulnerabilities, secrets, misconfigurations)
  • 🐳 Backend Docker image
  • 🐳 Frontend Docker image
  • 🏗️ IaC misconfiguration detection

⚠️ HIGH vulnerabilities should be addressed. Review required.

📋 View detailed SARIF reports

Download artifacts from this workflow run for full SARIF reports.
Results are also available in the GitHub Security tab.


Trivy v0.71.0 | aquasecurity/trivy

@github-actions

Copy link
Copy Markdown

PR Check Results

✅ Code Formatting: Passed
✅ Linting: Passed
✅ Type Checking: Failed
✅ Tests: Failed

⚠️ Some checks failed. Please review the details above.

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🔧 Backend Preview Deployment

App Coolify: qa-framework-backend-pr-282 (gaejlanpiq0fkopkgufgvwog)
Estado: ⚠️ running:unknown
Commit: 950c24d982c44f72e95a02e4f7688a92f97b2927
Branch: fix/rate-limit-followups-5221c63f

Preview interna (sin FQDN público hasta decisión de proxy — card 52a85645).

@github-actions

Copy link
Copy Markdown

🎨 Frontend Preview Deployment

App Coolify: qa-framework-frontend-pr-282 (bgkmnm2gokzd4ghqm9yzdnku)
Estado: ⚠️ running:unknown
Commit: 950c24d982c44f72e95a02e4f7688a92f97b2927
Branch: fix/rate-limit-followups-5221c63f

Preview interna (sin FQDN público hasta decisión de proxy — card 52a85645).

@github-actions

Copy link
Copy Markdown

🧹 Single-latest preview policy

Testing completo en verde sobre la preview de PR #282 → previews anteriores eliminadas (apps Coolify + imágenes del registry local). Solo queda viva la más nueva.

@SabaTech-dev
SabaTech-dev merged commit 1fe3c28 into main Oct 13, 2026
40 checks passed
@github-actions

Copy link
Copy Markdown

🧹 Preview Cleanup

PR #282 - Merged

✅ Preview apps eliminadas de Coolify (verificado via API): backend + frontend.


Resources freed at 2026-10-13T00:14:12.874Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant