fix(security): rate-limit fail-open hardening — metric, degraded header, configurable fail-closed (OBS-1) - #106
Open
SabaTech-dev wants to merge 1 commit into
Open
fix(security): rate-limit fail-open hardening — metric, degraded header, configurable fail-closed (OBS-1)#106SabaTech-dev wants to merge 1 commit into
SabaTech-dev wants to merge 1 commit into
Conversation
…er, configurable fail-closed Security audit OBS-1 (PR #105, 2026-08-23): if redis fails, RateLimitMiddleware failed OPEN silently — rate limiting disappeared without any signal. - Prometheus counter rate_limit_backend_failures_total{fail_mode} on every backing-store failure (exposed via existing /metrics mount) - Distinct structured log event rate_limit_backend_failure (server-side only) - RATE_LIMIT_FAIL_MODE=open|closed env (default open, backward compatible); closed returns 503 + Retry-After (service degradation, not 429 abuse signal) - X-RateLimit-Mode: degraded response header in open mode - Short-circuit on first backing-store failure: one event per request - No backend error strings in responses (S-3 discipline) - check_rate_limit dependency: 503 on degraded denial Tests: 8 new in TestFailModeBehavior + updated fails-open test. 27/27 test_rate_limit.py, 50/50 middleware+qa_visual_wiring.
PR Check Results✅ Code Formatting: Failed |
❌ FAIL Trivy Security Scan Results
Scans performed:
❌ CRITICAL vulnerabilities must be resolved before merge. 📋 View detailed SARIF reportsDownload artifacts from this workflow run for full SARIF reports. Trivy v0.71.0 | aquasecurity/trivy |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security audit OBS-1 (
qa-visual-fase-c-pr105-2026-08-23.md):RateLimitMiddlewarefailed OPEN silently — if redis fails (test evidence:Rate limit check failed error='Event loop is closed'), requests proceeded with no rate limit and no signal. Applies app-wide (preexisting, not from PR #105). Relevant for cost abuse of paid endpoints (qa-visual analyze) once enabled.Card:
4f9fb443[P3].Changes
Failures are now never silent:
rate_limit_backend_failures_total{fail_mode}— incremented on every backing-store failure, exposed via the existing/metricsmount (alertable:increase(rate_limit_backend_failures_total[5m]) > 0).rate_limit_backend_failure(error level, structlog) withfail_mode,error_type, key — replaces the generic message.X-RateLimit-Mode: degradedresponse header in open mode — externally visible.Configurable failure mode — env
RATE_LIMIT_FAIL_MODE=open|closed(default open, backward compatible):open: allow (as today) but flagged everywhere as above.closed: deny with 503 + Retry-After while the backing store is down. A redis outage is service degradation, not client abuse — hence 503, not 429.openwith a warning.check_rate_limitdependency updated consistently (503 on degraded denial).Hygiene:
Testing
TestFailModeBehavior): open marks degraded, closed denies, single-event short-circuit, metric increment per mode (viaREGISTRY.get_sample_value), invalid mode fallback, middleware 200+header (open), middleware 503+Retry-After+no-leak (closed).test_redis_failure_fails_open: assertsdegradedmarker + absence oferrorin info.tests/middleware/test_rate_limit.py27/27,tests/middleware/ + test_qa_visual_wiring.py50/50 — no regressions.Deployment notes
RATE_LIMIT_FAIL_MODE=closedat deploy time if strict enforcement is desired (trade-off: redis outage → dashboard 503s).