Skip to content

Commit 3f2711b

Browse files
committed
If allow list has scheme, match exactly
1 parent 0dbaa66 commit 3f2711b

File tree

2 files changed

+50
-0
lines changed

2 files changed

+50
-0
lines changed

src/guardrails/checks/text/urls.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,11 @@ def _is_url_allowed(parsed_url: ParseResult, allow_list: list[str], allow_subdom
325325
if allowed_ip is not None:
326326
if url_ip is None:
327327
continue
328+
# Scheme matching for IPs: if allow list entry has explicit scheme, it must match exactly
329+
if has_explicit_scheme:
330+
allowed_scheme = parsed_allowed.scheme.lower() if parsed_allowed.scheme else ""
331+
if allowed_scheme and allowed_scheme != scheme_lower:
332+
continue
328333
if allowed_port is not None and allowed_port != url_port:
329334
continue
330335
if allowed_ip == url_ip:
@@ -355,6 +360,12 @@ def _is_url_allowed(parsed_url: ParseResult, allow_list: list[str], allow_subdom
355360
if not host_matches:
356361
continue
357362

363+
# Scheme matching: if allow list entry has explicit scheme, it must match exactly
364+
if has_explicit_scheme:
365+
allowed_scheme = parsed_allowed.scheme.lower() if parsed_allowed.scheme else ""
366+
if allowed_scheme and allowed_scheme != scheme_lower:
367+
continue
368+
358369
# Path matching with segment boundary respect
359370
if allowed_path not in ("", "/"):
360371
# Ensure path matching respects segment boundaries to prevent

tests/unit/checks/test_urls.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,42 @@ def test_validate_url_security_allows_userinfo_when_disabled() -> None:
286286

287287
assert parsed is not None # noqa: S101
288288
assert reason == "" # noqa: S101
289+
290+
291+
def test_is_url_allowed_enforces_scheme_when_explicitly_specified() -> None:
292+
"""Scheme-qualified allow list entries must match scheme exactly (security)."""
293+
config = URLConfig(
294+
url_allow_list=["https://bank.example.com"],
295+
allow_subdomains=False,
296+
allowed_schemes={"https", "http"}, # Both schemes allowed globally
297+
)
298+
# HTTPS should be allowed (matches the scheme in allow list)
299+
https_url, _ = _validate_url_security("https://bank.example.com", config)
300+
# HTTP should be BLOCKED (doesn't match the explicit https:// in allow list)
301+
http_url, _ = _validate_url_security("http://bank.example.com", config)
302+
303+
assert https_url is not None # noqa: S101
304+
assert http_url is not None # noqa: S101
305+
306+
# This is the security-critical check: scheme-qualified entries must match exactly
307+
assert _is_url_allowed(https_url, config.url_allow_list, config.allow_subdomains) is True # noqa: S101
308+
assert _is_url_allowed(http_url, config.url_allow_list, config.allow_subdomains) is False # noqa: S101
309+
310+
311+
def test_is_url_allowed_enforces_scheme_for_ips() -> None:
312+
"""Scheme-qualified IP addresses in allow list must match scheme exactly."""
313+
config = URLConfig(
314+
url_allow_list=["https://192.168.1.100"],
315+
allow_subdomains=False,
316+
allowed_schemes={"https", "http"},
317+
)
318+
# HTTPS should be allowed
319+
https_ip, _ = _validate_url_security("https://192.168.1.100", config)
320+
# HTTP should be BLOCKED
321+
http_ip, _ = _validate_url_security("http://192.168.1.100", config)
322+
323+
assert https_ip is not None # noqa: S101
324+
assert http_ip is not None # noqa: S101
325+
326+
assert _is_url_allowed(https_ip, config.url_allow_list, config.allow_subdomains) is True # noqa: S101
327+
assert _is_url_allowed(http_ip, config.url_allow_list, config.allow_subdomains) is False # noqa: S101

0 commit comments

Comments
 (0)