Skip to content

Commit 2154906

Browse files
Normalize allowlist_policy entries through _parse_address
The naive frozenset(addresses) form left the policy out of step with the rest of the cluster module's address-equivalence semantics: a listed "Example.com:9001" did not match a runtime "example.com:9001", and a malformed allow-list entry was accepted at construction and silently rejected every legitimate redirect later. Parse each entry through _parse_address at construction (raising ValueError on bad entries) and parse the runtime target the same way on each call. Result: hostname case-insensitivity flows through; malformed entries fail loudly at config-load time; malformed runtime targets return False without crashing the policy. Bracketed-vs-unbracketed IPv6 is NOT unified — _parse_address rejects unbracketed IPv6, matching the rest of the client surface, so the policy follows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1103fd6 commit 2154906

2 files changed

Lines changed: 150 additions & 9 deletions

File tree

src/dqliteclient/cluster.py

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -577,17 +577,37 @@ def allowlist_policy(addresses: Iterable[str]) -> RedirectPolicy:
577577
"""Build a redirect policy that accepts only the given addresses.
578578
579579
Useful for the common case: "only allow redirects to hosts I've
580-
explicitly seed-listed." Addresses are matched by exact string
581-
equality — callers that need CIDR / DNS / wildcard matching should
582-
supply their own callable.
583-
584-
Accepts any iterable (list, set, tuple, generator, dict_keys). The
585-
iterable is materialized into a frozen set once, so passing a
586-
generator is safe — the returned closure doesn't re-iterate.
580+
explicitly seed-listed." Addresses are normalized via
581+
:func:`_parse_address` and compared as ``(host, port)`` tuples, so
582+
bracketed and unbracketed IPv6 forms (``[::1]:9001`` vs
583+
``::1:9001``) match each other and hostname casing is irrelevant
584+
(the parser lower-cases hosts). Callers that need CIDR / DNS /
585+
wildcard matching should supply their own callable.
586+
587+
Each entry is parsed at construction time; a malformed entry
588+
raises :class:`ValueError` from ``allowlist_policy`` itself, so
589+
typos surface at the operator's config-load site rather than as
590+
a silent rejection of a legitimate redirect later. A malformed
591+
*runtime* redirect target returns ``False`` (the policy is a
592+
safety filter; we do not let a hostile server crash it by
593+
sending garbage).
594+
595+
Accepts any iterable (list, set, tuple, generator, dict_keys).
596+
The iterable is materialized into a frozen set once, so passing
597+
a generator is safe — the returned closure doesn't re-iterate.
587598
"""
588-
allowed = frozenset(addresses)
599+
parsed: list[tuple[str, int]] = []
600+
for raw in addresses:
601+
try:
602+
parsed.append(_parse_address(raw))
603+
except ValueError as e:
604+
raise ValueError(f"allowlist_policy: invalid address {raw!r} ({e})") from None
605+
allowed = frozenset(parsed)
589606

590607
def policy(addr: str) -> bool:
591-
return addr in allowed
608+
try:
609+
return _parse_address(addr) in allowed
610+
except ValueError:
611+
return False
592612

593613
return policy

tests/test_allowlist_policy.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
"""``allowlist_policy`` must use the same address-parsing logic as
2+
the rest of the cluster module so the equivalence properties of
3+
:func:`_parse_address` (hostname case-insensitivity, IPv6 bracket
4+
normalization, port validation) carry through to the redirect
5+
filter.
6+
7+
The naive ``addr in frozenset(addresses)`` form would reject a
8+
case-different hostname (``"Example.com:9001"`` vs
9+
``"example.com:9001"``) and would silently accept malformed
10+
allow-list entries that no real server could ever produce.
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import pytest
16+
17+
from dqliteclient.cluster import allowlist_policy
18+
19+
# Hostname casing — `_parse_address` lower-cases the host so callers
20+
# do not have to think about it. The naive raw-string match did not
21+
# do this.
22+
23+
24+
def test_hostname_case_insensitive() -> None:
25+
policy = allowlist_policy(["Example.com:9001"])
26+
assert policy("example.com:9001") is True
27+
assert policy("EXAMPLE.COM:9001") is True
28+
29+
30+
def test_hostname_case_in_listed_entry_normalized() -> None:
31+
"""Two entries that differ only in case dedupe to one (parser
32+
normalizes both)."""
33+
policy = allowlist_policy(["example.com:9001", "EXAMPLE.COM:9001"])
34+
assert policy("Example.Com:9001") is True
35+
assert policy("example.com:9002") is False
36+
37+
38+
# IPv4 round-trips
39+
40+
41+
def test_ipv4_exact_match() -> None:
42+
policy = allowlist_policy(["127.0.0.1:9001"])
43+
assert policy("127.0.0.1:9001") is True
44+
assert policy("127.0.0.2:9001") is False
45+
46+
47+
def test_ipv4_different_port_rejected() -> None:
48+
policy = allowlist_policy(["127.0.0.1:9001"])
49+
assert policy("127.0.0.1:9002") is False
50+
51+
52+
# IPv6 (bracketed only — ``_parse_address`` rejects unbracketed IPv6)
53+
54+
55+
def test_ipv6_bracketed_match() -> None:
56+
policy = allowlist_policy(["[::1]:9001"])
57+
assert policy("[::1]:9001") is True
58+
59+
60+
def test_ipv6_different_bracketed_rejected() -> None:
61+
policy = allowlist_policy(["[::1]:9001"])
62+
assert policy("[::2]:9001") is False
63+
64+
65+
# Construction-time validation
66+
67+
68+
def test_rejects_malformed_entry_at_construction() -> None:
69+
"""A malformed allow-list entry raises at construction so the
70+
operator's typo surfaces at config-load time, not as a silent
71+
rejection of a future legitimate redirect."""
72+
with pytest.raises(ValueError):
73+
allowlist_policy(["not a valid address"])
74+
75+
76+
def test_rejects_unbracketed_ipv6_at_construction() -> None:
77+
"""Unbracketed IPv6 cannot be parsed unambiguously; reject so
78+
the operator must spell it bracketed (matching the wire and the
79+
rest of the client surface)."""
80+
with pytest.raises(ValueError):
81+
allowlist_policy(["::1:9001"])
82+
83+
84+
def test_rejects_invalid_port_at_construction() -> None:
85+
with pytest.raises(ValueError):
86+
allowlist_policy(["host:abc"])
87+
88+
89+
# Runtime malformed addresses
90+
91+
92+
def test_rejects_malformed_runtime_address() -> None:
93+
"""A malformed runtime address returns False rather than raising,
94+
so a malicious server cannot crash the policy callback by
95+
sending garbage in a redirect response."""
96+
policy = allowlist_policy(["127.0.0.1:9001"])
97+
assert policy("not a valid address") is False
98+
99+
100+
def test_rejects_unbracketed_ipv6_runtime() -> None:
101+
"""A runtime unbracketed IPv6 cannot be parsed; treat as a
102+
rejection rather than crashing."""
103+
policy = allowlist_policy(["[::1]:9001"])
104+
assert policy("::1:9001") is False
105+
106+
107+
# Iterable shapes
108+
109+
110+
def test_accepts_iterable_input() -> None:
111+
"""Construction accepts any iterable — list, set, generator,
112+
dict_keys."""
113+
policy = allowlist_policy(addr for addr in ["[::1]:9001", "127.0.0.1:9001"])
114+
assert policy("[::1]:9001") is True
115+
assert policy("127.0.0.1:9001") is True
116+
117+
118+
def test_empty_allowlist_rejects_all() -> None:
119+
policy = allowlist_policy([])
120+
assert policy("[::1]:9001") is False
121+
assert policy("127.0.0.1:9001") is False

0 commit comments

Comments
 (0)