diff --git a/.gitignore b/.gitignore index 07400d2..f735a8d 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,6 @@ build/ .coverage.* htmlcov/ .env +hn_proof.py +scratch_*.py +*.local.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 95fdce4..ea898b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project are documented here. ## [Unreleased] +## [0.3.4] - 2026-07-30 + +- fix: `URLAllowList` now inspects URLs nested inside `dict`, `list`, `tuple` and `set` arguments, including dict keys. A malicious URL hidden in a payload field — `http_post(url="https://api.internal.com/v1", payload={"redirect": "http://evil.com"})` — previously passed, because 0.3.2 taught the rule to skip non-string values without teaching it to look inside them. A `redirect` or `callback` field is as much an exfiltration route as the `url` parameter itself +- fix: guard `URLAllowList` against self-referential containers, so a cyclic argument cannot hang the check + ## [0.3.3] - 2026-07-30 - docs: state plainly what the default `SensitiveDataFilter` does. It matches keywords; it is not a secret-detection engine, and the Quickstart no longer implies otherwise @@ -69,7 +74,8 @@ All notable changes to this project are documented here. - CI workflow (lint + tests), MIT license, unit/integration test suite - Package renamed from `agentshield` to `modelfuzz` -[Unreleased]: https://github.com/higagan/modelfuzz/compare/v0.3.3...HEAD +[Unreleased]: https://github.com/higagan/modelfuzz/compare/v0.3.4...HEAD +[0.3.4]: https://github.com/higagan/modelfuzz/compare/v0.3.3...v0.3.4 [0.3.3]: https://github.com/higagan/modelfuzz/compare/v0.3.2...v0.3.3 [0.3.2]: https://github.com/higagan/modelfuzz/compare/v0.3.1...v0.3.2 [0.3.1]: https://github.com/higagan/modelfuzz/compare/v0.3.0...v0.3.1 diff --git a/pyproject.toml b/pyproject.toml index 8c12d5d..b8783b4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "modelfuzz" -version = "0.3.3" +version = "0.3.4" description = "Runtime guardrails for AI agents." readme = "README.md" license = { text = "MIT" } diff --git a/src/modelfuzz/rules.py b/src/modelfuzz/rules.py index 7667ab8..e2b7aef 100644 --- a/src/modelfuzz/rules.py +++ b/src/modelfuzz/rules.py @@ -18,10 +18,17 @@ class Violation: class URLAllowList: """A policy that ensures URLs are on an allowlist and blocks parsing tricks. - The policy governs *URLs only*. Arguments that are not URLs -- an email - body, a timeout int, None -- are passed through untouched, so a single - engine can guard a tool like ``http_post(url, body)`` without flagging - ``body``. Note the tradeoff: a bare host with no scheme (``"evil.com"``) is + The policy governs *URLs only*. Values that are not URLs -- an email body, a + timeout int, None -- are passed through untouched, so a single engine can + guard a tool like ``http_post(url, body)`` without flagging ``body``. + + Containers are inspected recursively: a URL hidden inside a ``dict``, + ``list``, ``tuple`` or ``set`` argument is checked exactly as a top-level + one is, because a payload field such as ``{"redirect": "http://evil.com"}`` + is as much an exfiltration route as the ``url`` parameter itself. Dict keys + are checked as well as values. + + Note the remaining tradeoff: a bare host with no scheme (``"evil.com"``) is not identifiable as a URL and is therefore allowed through. Pair this with a rule that governs the arguments it does not. """ @@ -38,19 +45,39 @@ def __init__( else DEFAULT_URL_SCHEMES ) - def __call__(self, url: object) -> Violation | None: - """Check if a URL is allowed. + def __call__(self, data: object) -> Violation | None: + """Check every URL reachable from a value. Args: - url: The value to check. Non-URL values are not governed by this - policy and return None. + data: The value to check. Containers are walked recursively; values + that are not URLs are not governed by this policy and pass. Returns: - A Violation object if the URL is blocked, otherwise None. + A Violation object if a blocked URL is found, otherwise None. """ - if not isinstance(url, str): - return None + return self._check_recursive(data, set()) + + def _check_recursive(self, data: object, seen: set[int]) -> Violation | None: + if isinstance(data, str): + return self._check_url(data) + + if isinstance(data, (dict, list, tuple, set, frozenset)): + # Guard against self-referential containers, which a hand-built + # argument can contain even though JSON-derived ones cannot. + if id(data) in seen: + return None + seen.add(id(data)) + + # Keys can carry a URL just as values can, e.g. an endpoint map. + items = (*data.keys(), *data.values()) if isinstance(data, dict) else data + for item in items: + violation = self._check_recursive(item, seen) + if violation: + return violation + + return None + def _check_url(self, url: str) -> Violation | None: # A string carrying a scheme separator is claiming to be a URL, so a # parse failure from here on must fail closed rather than sail through. looks_like_url = "://" in url diff --git a/tests/test_rules.py b/tests/test_rules.py index b9f3686..389dcd4 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -110,6 +110,57 @@ def test_allows_non_string_values(self, url_allowlist: URLAllowList, value: obje """Non-string arguments are passed through untouched.""" assert url_allowlist(value) is None + def test_blocks_url_hidden_in_dict_value(self, url_allowlist: URLAllowList): + """The regression this class exists for: a URL inside a payload dict.""" + violation = url_allowlist({"redirect": "http://evil.com"}) + assert violation is not None + assert "not in allowlist" in violation.reason + + def test_blocks_url_hidden_in_dict_key(self, url_allowlist: URLAllowList): + """Keys carry URLs too, e.g. an endpoint map.""" + violation = url_allowlist({"http://evil.com": "data"}) + assert violation is not None + assert "not in allowlist" in violation.reason + + def test_blocks_url_hidden_in_nested_dict(self, url_allowlist: URLAllowList): + """Nesting depth doesn't matter.""" + data = {"config": {"webhooks": [{"callback": "http://evil.com/exfil"}]}} + violation = url_allowlist(data) + assert violation is not None + assert "not in allowlist" in violation.reason + + @pytest.mark.parametrize( + "container", + [ + ["http://evil.com"], + ("http://evil.com",), + {"http://evil.com"}, + frozenset({"http://evil.com"}), + [{"a": ["http://evil.com"]}], + ], + ) + def test_blocks_url_in_any_container(self, url_allowlist: URLAllowList, container): + """Lists, tuples, sets and frozensets are all walked.""" + assert url_allowlist(container) is not None + + def test_allows_allowlisted_url_inside_a_container(self, url_allowlist: URLAllowList): + """Recursion must not turn permitted URLs into violations.""" + assert url_allowlist({"callback": "https://api.internal.com/hook"}) is None + + def test_allows_container_of_non_url_values(self, url_allowlist: URLAllowList): + """A benign payload stays benign -- this is the 0.3.2 regression guard.""" + assert url_allowlist({"user": "bob", "retries": 3, "note": "hello world"}) is None + + def test_survives_a_self_referential_container(self, url_allowlist: URLAllowList): + """A cyclic argument must not hang the guard.""" + data: dict = {"name": "loop"} + data["self"] = data + assert url_allowlist(data) is None + + evil: dict = {"redirect": "http://evil.com"} + evil["self"] = evil + assert url_allowlist(evil) is not None + def test_guards_a_multi_argument_tool(self, url_allowlist: URLAllowList): """The regression that motivated this: http_post(url, body, timeout).""" from modelfuzz import ModelFuzzBlockError, PolicyEngine, shield_tool @@ -132,6 +183,19 @@ def http_post(url: str, body: str, timeout: int = 30) -> str: with pytest.raises(ModelFuzzBlockError): http_post("http://evil.com/exfil", "hello world") + # And a disallowed host hidden in a structured payload is blocked too. + with pytest.raises(ModelFuzzBlockError): + http_post( + "https://api.internal.com/v1", + {"redirect": "http://evil.com"}, + timeout=3, + ) + + # A structured payload with no URLs in it still passes. + assert http_post("https://api.internal.com/v1", {"user": "bob"}, timeout=3) == ( + "posted to https://api.internal.com/v1" + ) + class TestSensitiveDataFilter: """Tests for the SensitiveDataFilter policy.""" diff --git a/uv.lock b/uv.lock index 99b4a4f..ced93b0 100644 --- a/uv.lock +++ b/uv.lock @@ -286,7 +286,7 @@ wheels = [ [[package]] name = "modelfuzz" -version = "0.3.3" +version = "0.3.4" source = { editable = "." } dependencies = [ { name = "typer" },