[8568d4b7] docs(http-security-guard): correct false e2e test claim + document guard-core 3.4.0 vs 3.7.0 version sensitivity

This commit is contained in:
Backend Developer 1
2026-08-02 14:38:08 +00:00
parent d829979b6a
commit 26dd53eda6
+3 -1
View File
@@ -57,4 +57,6 @@ The invariant: **whatever ranges the whitelist excludes, `trusted_proxies` must
If you ever narrow or widen the whitelist, apply the same edit to `trusted_proxies` in the same commit — the two lists are one policy, split across two guard-core knobs. If you ever narrow or widen the whitelist, apply the same edit to `trusted_proxies` in the same commit — the two lists are one policy, split across two guard-core knobs.
The invariant is anchored in two places so it can't silently drift: an `INVARIANT` comment at the `trusted_proxies` definition in `build_security_config` (`roboco/security.py`) restates the must-mirror-`_INTERNAL_NETWORKS` rule in-line, and two self-contained unit tests in `tests/unit/test_security_middleware.py` prove the boundary directly (no running server) — `test_extract_client_ip_forwarded_lan_not_peeled` shows a docker-bridge peer forwarding `X-Forwarded-For: 192.168.1.50` resolves to that LAN IP (not peeled to the peer) under the narrowed `trusted_proxies`, and `test_is_ip_allowed_rejects_lan_ranges` shows `192.168.1.50` and `10.0.0.5` are both rejected by the `[127.0.0.1, ::1, 172.16.0.0/12]` whitelist. The end-to-end `test_nginx_forwarded_lan_client_is_not_whitelisted` covers the same boundary through the full middleware stack. The invariant is anchored by two layers of tests in `tests/unit/test_security_middleware.py`. The end-to-end test `test_nginx_forwarded_lan_client_is_not_whitelisted` drives the full `SecurityMiddleware` stack (including the `@deco.custom_validation` route on `/task`) with a docker-bridge peer forwarding `X-Forwarded-For: 192.168.1.50` and asserts the request is blocked (status != 200); it passes on the pinned guard-core 3.7.0, where `IpSecurityCheck.check` (guard-core `core/checks/implementations/ip_security.py:197-208`) falls through to `_check_global_ip_restrictions` even when a `route_config` exists. Its companion `test_docker_bridge_peer_without_xff_is_whitelisted` asserts the same peer without XFF stays exempt. Two isolated unit tests — `test_extract_client_ip_forwarded_lan_not_peeled` and `test_is_ip_allowed_rejects_lan_ranges` — call `guard_core.utils` functions directly (no running server) as defense-in-depth, verifying that `extract_client_ip` resolves the LAN IP correctly and `is_ip_allowed` rejects it against the narrowed whitelist. An `INVARIANT` comment at the `trusted_proxies` definition in `build_security_config` (`roboco/security.py`) restates the must-mirror-`_INTERNAL_NETWORKS` rule in-line.
Note the guard-core version sensitivity: on 3.4.0, `IpSecurityCheck` had an early-return that skipped `_check_global_ip_restrictions` for routes with a `route_config` (any `@deco.custom_validation` route), so the e2e test failed because the resolved LAN IP was never consulted against the whitelist. The pinned 3.7.0 fixes this — the check falls through to the global IP restrictions regardless of `route_config` (the final `return await self._check_global_ip_restrictions(...)` at `ip_security.py:208` runs unconditionally after the route-level check returns `None`). The PR #817 in-path review failure was exactly this drift: the reviewer's local venv had downgraded to guard-core 3.4.0 while `uv.lock` pinned 3.7.0, so the e2e test failed in that environment and passes on the pinned version. Always run tests against the `uv.lock`-pinned version (via `make` targets, never bare `uv run`); a drifted local venv can silently downgrade guard-core and reintroduce the bypass.