fix(api): rate-limit /ready and /metrics, expose db pool telemetry

[parthrohit22]

What does this PR do?

Closes the remaining scope of #296 (readiness-endpoint pool exhaustion): rate-limits the unauthenticated /ready and /metrics probe/scrape endpoints per source IP, and exposes database connection pool utilization/exhaustion telemetry on /metrics.

Type of change

• Bug fix
• API endpoint
• Documentation

Scope note

#306 already fixed the core connection-checkout leak (g.db + the existing close_db teardown handler) and added /health as a DB-free liveness probe, with a real PostgreSQL-backed integration test (test_ready_returns_every_real_connection_to_the_pool) proving checkout/return counts stay balanced across repeated readiness probes, including on the failure path. That work is not touched or duplicated here. This PR picks up the two acceptance criteria from #296 that were still open after #306:

1. Restrict /ready//metrics at the edge, while keeping an appropriate liveness endpoint. /health already stays public and DB-free (unchanged). Render's Blueprint format has no path-based access control, so this repository can't express "restrict this path" in render.yaml itself — documented that in docs/deployment/render.md under a new "Restricting probe/scrape endpoints at the edge" section, with the operational recommendation for whoever runs a reverse proxy/CDN/WAF in front of a real deployment. As the in-app backstop, added api.observability.probe_rate_limit: an in-memory, per-process, per-source-IP rate limiter, wired onto /ready (5 requests/10s — half the default DB_POOL_MAX_CONN) and /metrics (20/10s, generous for normal Prometheus scrape intervals). Deliberately not the existing Postgres-backed api.rate_limit.rate_limit: that does its own DB round trip per check, which would add database load to the exact endpoint meant to protect the database from overload, and risks checking out a second, separately-tracked pooled connection under its own g.db alongside the one /ready's own handler manages — only one of which the teardown handler would return. The rejection happens before the view body runs, so a request over budget never reaches the database work it would otherwise trigger.
2. Expose pool utilization/exhaustion telemetry without publishing sensitive operational data. Added DatabaseManager's sibling get_pool_stats() in api/models/finding.py, returning only in-use/idle/max-connection counts and a utilization percentage — never the DSN, host, or credentials. Wired into three new Prometheus gauges (openshield_db_pool_connections_in_use/idle/max) refreshed on every /metrics scrape via a provider callback registered from api/app.py, keeping api/observability.py free of project imports (it's reused by the worker, per its own module docstring).

Testing

• All CI checks pass
• No hardcoded credentials or secrets

New file tests/test_readiness_hardening.py covers:

• probe_rate_limit's budget/window-reset/per-source-IP-isolation/testing-mode-bypass/stale-key-pruning behavior, in isolation against a minimal Flask app.
• /ready actually rejecting a source once its budget is spent, and confirming a rejected request never calls ping() at all.
• get_pool_stats()'s zero-before-any-connection case, a populated-pool case, the zero-division guard, and that its output never contains a DSN, host, or credential substring.
• /metrics rendering the three new gauges with correct values, never leaking the DSN into the scrape body, and surviving a broken stats provider without failing the rest of the scrape.

Verified: full backend suite (796 passed, 3 skipped — pre-existing, unrelated), plus tests/test_observability.py's real PostgreSQL-backed readiness-leak test run against a local Postgres instance to confirm the new decorator doesn't disturb #306's fix. ruff check . and ruff format --check . clean.

Related issue

Closes #296

Checklist

• Every commit includes a DCO Signed-off-by trailer
• I have not committed any real Azure credentials
• My branch name follows the convention: fix/description
• Matching CLI playbook / compliance framework mappings — not applicable, no scanner rule added

[m-khan-97]

Parth, the overall direction is sound: keeping probe limiting out of PostgreSQL is the right boundary, rejected /ready calls do avoid the database, and the pool metrics expose counts rather than connection details. I also checked the current CI head; all 20 checks are green.

There is one blocking denial-of-service gap in the limiter itself. _probe_hits only removes an expired entry when that exact (remote_addr, path) key is requested again. A caller that continuously rotates source addresses—or spoofed forwarded addresses anywhere the trusted-proxy boundary is misconfigured—creates a new dictionary entry per address, and those one-shot keys are never revisited or removed. The new unauthenticated protection can therefore become an unbounded process-memory sink.

Please add bounded global cleanup rather than only same-key cleanup: for example, periodically sweep expired deques under the lock and enforce a hard maximum number of tracked keys with deterministic eviction. Add a regression test that inserts many distinct one-shot addresses, advances past the window, triggers cleanup through a different address, and proves stale keys are removed and the map remains capped. A Retry-After header on 429 would also make the endpoint friendlier to legitimate probes, but I do not consider that blocking.

Once the state is globally bounded, I will re-review promptly.

[parthrohit22]

@m-khan-97 Fixed, in two parts:

1. The gap you flagged. _probe_hits's per-call cleanup only ever pruned the one key the current request touched. Added the periodic global sweep and hard cap you asked for: a sweep every _PROBE_SWEEP_INTERVAL calls (not every call — a full-dict scan per request would defeat the point of a cheap in-memory limiter) that prunes every key whose hits have all expired, plus a hard _PROBE_MAX_TRACKED_KEYS ceiling with deterministic least-recently-touched eviction via an OrderedDict (every hit, including one that just survives a sweep, moves its key to the end). Also added the Retry-After header you suggested as non-blocking.
2. Separately: a dev merge onto this branch (picking up security: remove the public demo bearer and establish real authorization boundaries #294/fix(api): enforce token expiry, role, and subscription authorization #320's own changes to api/app.py) dropped _READY_MAX_REQUESTS_PER_WINDOW's definition while keeping its usage on /ready, which broke CI (ruff's F821, and Backend Tests for the same reason). Restored it.

New tests cover exactly the scenario you described: many distinct one-shot addresses, advance past the window, trigger cleanup through different addresses, and prove the stale keys are gone and the map stays bounded — plus direct hard-cap/LRU-eviction tests and the Retry-After header.

Verified: full backend suite (862 passed, 5 skipped — pre-existing/environment-only), all 17 tests in the readiness-hardening file including the 5 new ones, ruff clean. All 20 CI checks are green on the current head. Ready for another look whenever you have a chance.

[m-khan-97]

Parth, I verified the current head rather than relying on the update summary. The blocker is closed: stale one-shot source keys are now swept globally, tracked state has a hard 10,000-key ceiling, eviction is deterministic and activity-aware, and the rejected response includes Retry-After. The regression suite covers rotating addresses, global expiry cleanup, hard-cap behavior, LRU touch ordering, and the response header. The merge with #320 also restored the readiness budget constant correctly, and all 20 checks pass. Approving.

[m-khan-97]

@ritiksah141 @TFT444, I completed the blocker rereview and approved 5854316: the limiter now has global stale-key cleanup, a hard state cap, deterministic LRU eviction, and regression coverage. All 20 checks pass. Please provide the required independent review so it can proceed under the two-person rule.

[TFT444]

BLOCKER — LRU eviction bypass: move_to_end(key) is called unconditionally on every request including rejected ones, keeping an attacker's slot permanently warm while legitimate monitoring IPs that go quiet drift toward eviction. Once a legitimate IP's entry is evicted its hit history resets and it gets a fresh budget; the attacker's key never ages out — directly defeating the eviction-based memory cap; guard move_to_end(key) with if allowed.

BLOCKER — Shared sweep uses wrong window: _probe_hits is a module-level dict shared across all decorated endpoints, but the sweep passes the triggering endpoint's window_seconds closure to _sweep_expired_probe_hits, applying it to every key including entries owned by endpoints with different windows. Both endpoints currently default to 10 s so the bug is dormant, but the first non-default-window endpoint added will cause premature budget resets or lingering stale entries across all endpoints — store window_seconds per key so the sweep applies the correct cutoff per entry.

MAJOR — ProxyFix(x_for=1) makes remote_addr fully attacker-controlled if the origin is reachable directly (Render origin IPs surface in CT logs and Shodan); an attacker sending arbitrary X-Forwarded-For headers gets a fresh rate-limit bucket on every request, fully neutralising per-IP enforcement — add an explicit code comment stating the trusted-proxy assumption and what fails when it is violated.

Dismissing — posted before user review