feat(backend): read and unban auto-ban stick-table entries via HAProxy Runtime API#282
Conversation
…y Runtime API Adds an admin-level stats socket to the generated haproxy.cfg (the existing socket is operator-level, which show table but not clear table) and a BanListService that lists tracked/banned source IPs across every auto-ban-enabled vhost and clears an IP from all of its ban tables on unban, exposed via GET/DELETE /security/banned-ips.
…ror handling - Fix admin stats socket to mode 666: HAProxy runs as root while the backend drops to an unprivileged user via gosu, so a root-owned mode 660 socket was unreadable/unwritable by the backend, breaking every list/unban call. Applied to the template, static bootstrap config, and release seed config for parity. - Classify HAProxy Runtime API error replies (unknown table, permission denied, ...) inside _send_runtime_command, since HAProxy reports those as plain text rather than a socket-level failure; previously an unban could report a table as cleared even when HAProxy rejected the command. - Make list_banned raise RuntimeApiError when every ban table read fails, mirroring unban, so a fully unreachable Runtime API surfaces as a 502 instead of an empty list indistinguishable from "nothing tracked". - Document the new HAPROXY_STATS_SOCKET_PATH / HAPROXY_STATS_TIMEOUT_SECONDS settings in .env.example.
| table_name, | ||
| vhost.id, | ||
| ) | ||
| continue |
There was a problem hiding this comment.
[bug] list_banned catches every RuntimeApiError and continues. If the Runtime API is fully down (or every table read fails), the method returns [] and the router responds 200 with total: 0. That is indistinguishable from “no IPs tracked”, while unban carefully raises when failures == len(tables) so a total outage surfaces as 502. Operators can wrongly conclude nothing is banned when HAProxy is unreachable.
Suggestion: Mirror unban: track failures; if there was at least one active ban table and every show table failed, raise RuntimeApiError. In security.py list_banned_ips, catch RuntimeApiError and map it to 502 the same way unban_ip does. Keep per-table skip for partial failures only.
There was a problem hiding this comment.
Fixed in 01e1849: list_banned now tracks per-table failures and raises RuntimeApiError when every table read fails (mirroring unban), and the router maps that to 502.
| haproxy_validation_timeout_seconds: int = 10 | ||
| haproxy_master_socket_path: str = "/var/run/haproxy/master.sock" | ||
| haproxy_reload_timeout_seconds: int = 10 | ||
| haproxy_stats_socket_path: str = "/var/run/haproxy/admin.sock" |
There was a problem hiding this comment.
[nit] New settings haproxy_stats_socket_path and haproxy_stats_timeout_seconds are not documented in src/backend/.env.example next to the existing HAPROXY_MASTER_SOCKET_PATH / reload timeout comments. Operators overriding the path would not discover the knobs from the env template.
Suggestion: Add commented examples for HAPROXY_STATS_SOCKET_PATH and HAPROXY_STATS_TIMEOUT_SECONDS in .env.example.
There was a problem hiding this comment.
Documented in 01e1849: added HAPROXY_STATS_SOCKET_PATH and HAPROXY_STATS_TIMEOUT_SECONDS to .env.example.
| failures = 0 | ||
| for _vhost, table_name, _threshold in tables: | ||
| try: | ||
| _send_runtime_command(f"clear table {table_name} key {ip}") |
There was a problem hiding this comment.
[suggestion] unban increments cleared whenever the socket I/O succeeds, without inspecting HAProxy’s reply. HAProxy can return a textual error (e.g. unknown table when DB policy still has auto-ban but config was not applied / table was never created) without raising OSError. Those attempts still count as cleared, so cleared can overstate success. config_apply._reload_haproxy already classifies command output for errors; this path does not.
Suggestion: After _send_runtime_command, treat non-empty error-looking replies (or known “Unknown table” / “Permission denied” strings) as failures for that table, consistent with reload output handling. Document that cleared means “tables successfully cleared” (including no-op when the key was absent) vs “socket accepted the write”.
There was a problem hiding this comment.
Fixed in 01e1849: _send_runtime_command now classifies HAProxy's textual error replies (unknown table, permission denied, etc.) and raises RuntimeApiError, so those attempts are no longer counted in cleared. Added regex-level tests mirroring _RELOAD_ERROR_RE's test style.
| # Admin level, reachable only from the backend over the shared runtime | ||
| # volume. Needed for `clear table` (unban) — see #276 — which the | ||
| # operator-level socket above deliberately cannot perform. | ||
| stats socket /var/run/haproxy/admin.sock mode 660 level admin |
There was a problem hiding this comment.
[bug] The admin stats socket is created as mode 660 level admin. In compose, HAProxy runs as user: "0:0" and the backend drops to the unprivileged app user via gosu. A root-owned 660 UNIX socket is not accessible to app, so _send_runtime_command will raise RuntimeApiError (permission denied) for every list/unban call. The existing master socket deliberately uses mode,666 for the same shared volume reason; the new admin socket does not.
Suggestion: Align with the master-socket pattern, e.g. stats socket /var/run/haproxy/admin.sock mode 666 level admin, and document that isolation comes from the private Docker volume (not host exposure). Alternatively set explicit uid/gid/user/group on the socket to the backend process identity, but numeric IDs must stay stable across images. Apply the same change in configs/haproxy/haproxy.cfg (and release seed if kept in sync).
There was a problem hiding this comment.
Fixed in 01e1849: switched to mode 666 (matching the master socket precedent) since the backend drops to an unprivileged user via gosu and can't access a root-owned mode 660 socket. Applied to the template, static bootstrap config, and release seed config.
| # Admin level, reachable only from the backend over the shared runtime | ||
| # volume. Needed for `clear table` (unban) — see #276 — which the | ||
| # operator-level socket above deliberately cannot perform. | ||
| stats socket /var/run/haproxy/admin.sock mode 660 level admin |
There was a problem hiding this comment.
[suggestion] Dev seed configs/haproxy/haproxy.cfg and the Jinja template gained the admin socket, but release/haproxy/haproxy.cfg was not updated. Release compose seeds from ./haproxy/haproxy.cfg when no generated release exists. Until a successful seed/apply from the backend template, the release stack has no admin socket for ban-list operations.
Suggestion: Add the same admin stats socket stanza to release/haproxy/haproxy.cfg for seed parity (and keep it consistent with whatever mode fix is chosen for the socket-permissions issue).
There was a problem hiding this comment.
Fixed in 01e1849: added the same admin stats socket stanza to release/haproxy/haproxy.cfg for seed parity.
Summary
/var/run/haproxy/admin.sock) alongside the existing operator-level one, sinceclear table(unban) requiresadminwhileshow tableonly needsoperator.BanListServiceto read tracked/banned source IPs across every auto-ban-enabled vhost'sst_ban_vhost_<id>stick-table, and to clear an IP from all of its ban tables on unban.GET /security/banned-ipsandDELETE /security/banned-ips/{ip}(admin-only).Scope
Closes #276
Related to #176
Test plan
uv run pytest --cov=app(588 passed)uv run mypy app/uv run ruff check app/pnpm run type-check(src/frontend)pnpm run lint(src/frontend)haproxy -cvalidates rendered configs with the new admin socket (asserted in renderer/generator tests)