Fix #1748: make cloud-metadata/private-range denylist non-overridable#2055
Fix #1748: make cloud-metadata/private-range denylist non-overridable#2055S0412-2007 wants to merge 1 commit into
Conversation
_init_default_policies() built the entire network denylist from the single Pydantic field settings.network_denylist. Pydantic replaces (rather than merges) a list field's default when SECUSCAN_NETWORK_DENYLIST is set via env var, so any operator adding even one custom denylist entry silently dropped the built-in protection for cloud metadata (169.254.169.254), loopback, RFC1918/CGNAT ranges, and IPv6 link-local/ULA space -- reopening SSRF to the metadata endpoint despite the code comment claiming the denylist was 'always enforced'. Fix: move those ranges into a new MANDATORY_DENYLIST module constant that is not read from settings and is applied unconditionally in _init_default_policies before any operator-configured entries. The operator-facing network_denylist setting is now purely additive. Also updates the existing default-denylist test and adds a regression test reproducing the exact scenario from #1748.
|
@utksh1 ready for review. One thing worth calling out: the original issue's exact repro (default denylist missing 169.254.169.254 entirely) doesn't reproduce on current This PR splits those ranges into a Happy to adjust the approach if you'd rather keep denylist config fully in Settings and handle the merge differently (e.g. a validator that appends instead of replaces) — went with the module-constant approach since it guarantees the protection can't be disabled even by a misconfigured env var, not just a careless one. |
Description
_init_default_policies()built the entire network denylist from a single Pydantic field,settings.network_denylist. Pydantic replaces (rather than merges) a list field's default whenSECUSCAN_NETWORK_DENYLISTis set via env var, so any operator adding even one custom denylist entry silently dropped the built-in protection for cloud metadata (169.254.169.254), loopback, RFC1918/CGNAT ranges, and IPv6 link-local/ULA space — reopening SSRF to the metadata endpoint despite the code comment claiming the denylist was "always enforced."Note: the issue's literal repro (default denylist missing the metadata IP entirely) doesn't reproduce as-is on current
main— that part looks to have been partially addressed already. This PR fixes the related, still-live variant: the protection exists by default but isn't durable against operator customization.Fix: moved those ranges into a new
MANDATORY_DENYLISTmodule constant that is not read from settings/env and is applied unconditionally in_init_default_policies()before any operator-configured entries. The operator-facingnetwork_denylistsetting is now purely additive.Related Issues
Closes #1748
Type of Change
How Has This Been Tested?
SECUSCAN_NETWORK_DENYLISTto a custom, unrelated value must not lose metadata/private-range protection.testing/backend/unit/test_network_policy.py,test_admin_network_policy.py,test_validate_command_egress_policy.py, andtest_network_scanner_plugin.py— all 63 tests pass.Checklist