From b6ceccd6bb1d37e9664647edaa3570415866c3ea Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Thu, 12 Mar 2026 12:29:34 -0700 Subject: [PATCH 1/4] fix: reject empty string list evaluator values --- .../agent_control_evaluators/list/config.py | 10 +++++- .../list/evaluator.py | 4 ++- evaluators/builtin/tests/list/test_list.py | 36 +++++++++++++++++++ server/tests/test_controls_validation.py | 23 ++++++++++++ server/tests/test_evaluator_configs.py | 20 +++++++++++ 5 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 evaluators/builtin/tests/list/test_list.py diff --git a/evaluators/builtin/src/agent_control_evaluators/list/config.py b/evaluators/builtin/src/agent_control_evaluators/list/config.py index 3ba7efbf..3277227c 100644 --- a/evaluators/builtin/src/agent_control_evaluators/list/config.py +++ b/evaluators/builtin/src/agent_control_evaluators/list/config.py @@ -2,7 +2,7 @@ from typing import Literal -from pydantic import Field +from pydantic import Field, field_validator from agent_control_evaluators._base import EvaluatorConfig @@ -24,3 +24,11 @@ class ListEvaluatorConfig(EvaluatorConfig): description="'exact' for full string match, 'contains' for keyword/substring match", ) case_sensitive: bool = Field(False, description="Whether matching is case sensitive") + + @field_validator("values") + @classmethod + def validate_values(cls, values: list[str | int | float]) -> list[str | int | float]: + """Reject empty-string entries that would compile into a match-all regex.""" + if any(isinstance(value, str) and value == "" for value in values): + raise ValueError("values must not contain empty strings") + return values diff --git a/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py b/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py index 7c0612ed..92e7df11 100644 --- a/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py +++ b/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py @@ -35,7 +35,9 @@ class ListEvaluator(Evaluator[ListEvaluatorConfig]): def __init__(self, config: ListEvaluatorConfig) -> None: super().__init__(config) - self._values = [str(v) for v in config.values] + # Defensive filtering keeps legacy invalid configs from compiling into match-all regexes. + normalized_values = [str(v) for v in config.values] + self._values = [value for value in normalized_values if value != ""] self._regex: Any = self._build_regex() def _build_regex(self) -> Any: diff --git a/evaluators/builtin/tests/list/test_list.py b/evaluators/builtin/tests/list/test_list.py new file mode 100644 index 00000000..b232d987 --- /dev/null +++ b/evaluators/builtin/tests/list/test_list.py @@ -0,0 +1,36 @@ +"""Tests for list evaluator.""" + +import pytest +from pydantic import ValidationError + +from agent_control_evaluators.list import ListEvaluator, ListEvaluatorConfig + + +class TestListEvaluatorConfig: + """Tests for list evaluator config validation.""" + + def test_empty_string_value_rejected(self) -> None: + """Test that empty-string list entries are rejected at config validation time.""" + with pytest.raises(ValidationError, match="values must not contain empty strings"): + ListEvaluatorConfig(values=[""]) + + +class TestListEvaluator: + """Tests for list evaluator runtime behavior.""" + + @pytest.mark.asyncio + async def test_legacy_empty_string_value_is_ignored_defensively(self) -> None: + """Test that legacy invalid configs do not compile into a match-all regex.""" + config = ListEvaluatorConfig.model_construct( + values=[""], + logic="any", + match_on="match", + match_mode="contains", + case_sensitive=False, + ) + evaluator = ListEvaluator(config) + + result = await evaluator.evaluate("Tell me a joke") + + assert result.matched is False + assert result.message == "Empty control values - control ignored" diff --git a/server/tests/test_controls_validation.py b/server/tests/test_controls_validation.py index cd5084ae..e4b90621 100644 --- a/server/tests/test_controls_validation.py +++ b/server/tests/test_controls_validation.py @@ -86,6 +86,29 @@ def test_validation_regex_flags_list(client: TestClient): assert any("flags" in str(e.get("field", "")) for e in errors) +def test_validation_list_values_reject_empty_strings(client: TestClient): + """Test that list evaluator config rejects empty-string entries.""" + control_id = create_control(client) + payload = VALID_CONTROL_PAYLOAD.copy() + payload["evaluator"] = { + "name": "list", + "config": { + "values": [""], + "logic": "any", + "match_on": "match", + "match_mode": "contains", + }, + } + + resp = client.put(f"/api/v1/controls/{control_id}/data", json={"data": payload}) + + assert resp.status_code == 422 + response_data = resp.json() + errors = response_data.get("errors", []) + assert any("values" in str(e.get("field", "")) for e in errors) + assert any("empty strings" in e.get("message", "") for e in errors) + + def test_validation_invalid_regex_pattern(client: TestClient): """Test validation of regex pattern syntax.""" # Given: a control and regex config with invalid pattern (unclosed bracket) diff --git a/server/tests/test_evaluator_configs.py b/server/tests/test_evaluator_configs.py index 08187f4c..e2938810 100644 --- a/server/tests/test_evaluator_configs.py +++ b/server/tests/test_evaluator_configs.py @@ -142,6 +142,26 @@ def test_create_evaluator_config_invalid_config_422(client: TestClient) -> None: assert any("config" in str(err.get("field", "")) for err in data.get("errors", [])) +def test_create_evaluator_config_rejects_empty_list_values(client: TestClient) -> None: + # Given: a payload with an empty-string entry for the list evaluator + name = f"config-{uuid.uuid4().hex}" + payload = _create_config_payload( + name=name, + evaluator="list", + config={"values": [""], "logic": "any", "match_on": "match", "match_mode": "contains"}, + ) + + # When: creating the evaluator config + resp = client.post("/api/v1/evaluator-configs", json=payload) + + # Then: validation error is returned + assert resp.status_code == 422 + data = resp.json() + assert data["error_code"] == "INVALID_CONFIG" + assert any("config.values" in str(err.get("field", "")) for err in data.get("errors", [])) + assert any("empty strings" in err.get("message", "") for err in data.get("errors", [])) + + def test_create_evaluator_config_invalid_parameters_type_error_422( client: TestClient, monkeypatch ) -> None: From dcd6bcd8f19bfd806cc06a628f2871c652f62f6f Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Thu, 12 Mar 2026 17:23:35 -0700 Subject: [PATCH 2/4] test: align new regressions with given-when-then style --- evaluators/builtin/tests/list/test_list.py | 6 ++++++ server/tests/test_controls_validation.py | 3 +++ server/tests/test_evaluator_configs.py | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/evaluators/builtin/tests/list/test_list.py b/evaluators/builtin/tests/list/test_list.py index b232d987..b9f65c77 100644 --- a/evaluators/builtin/tests/list/test_list.py +++ b/evaluators/builtin/tests/list/test_list.py @@ -11,8 +11,11 @@ class TestListEvaluatorConfig: def test_empty_string_value_rejected(self) -> None: """Test that empty-string list entries are rejected at config validation time.""" + # Given: a list evaluator config with an empty-string value + # When: constructing the config model with pytest.raises(ValidationError, match="values must not contain empty strings"): ListEvaluatorConfig(values=[""]) + # Then: validation rejects the config (asserted by pytest) class TestListEvaluator: @@ -21,6 +24,7 @@ class TestListEvaluator: @pytest.mark.asyncio async def test_legacy_empty_string_value_is_ignored_defensively(self) -> None: """Test that legacy invalid configs do not compile into a match-all regex.""" + # Given: a legacy invalid config constructed without validation config = ListEvaluatorConfig.model_construct( values=[""], logic="any", @@ -30,7 +34,9 @@ async def test_legacy_empty_string_value_is_ignored_defensively(self) -> None: ) evaluator = ListEvaluator(config) + # When: evaluating normal text against the legacy config result = await evaluator.evaluate("Tell me a joke") + # Then: the evaluator ignores the empty control values assert result.matched is False assert result.message == "Empty control values - control ignored" diff --git a/server/tests/test_controls_validation.py b/server/tests/test_controls_validation.py index e4b90621..9e9e6eb7 100644 --- a/server/tests/test_controls_validation.py +++ b/server/tests/test_controls_validation.py @@ -88,6 +88,7 @@ def test_validation_regex_flags_list(client: TestClient): def test_validation_list_values_reject_empty_strings(client: TestClient): """Test that list evaluator config rejects empty-string entries.""" + # Given: a control and a list evaluator payload with an empty-string value control_id = create_control(client) payload = VALID_CONTROL_PAYLOAD.copy() payload["evaluator"] = { @@ -100,8 +101,10 @@ def test_validation_list_values_reject_empty_strings(client: TestClient): }, } + # When: setting control data resp = client.put(f"/api/v1/controls/{control_id}/data", json={"data": payload}) + # Then: the invalid config is rejected assert resp.status_code == 422 response_data = resp.json() errors = response_data.get("errors", []) diff --git a/server/tests/test_evaluator_configs.py b/server/tests/test_evaluator_configs.py index e2938810..05d543f5 100644 --- a/server/tests/test_evaluator_configs.py +++ b/server/tests/test_evaluator_configs.py @@ -154,7 +154,7 @@ def test_create_evaluator_config_rejects_empty_list_values(client: TestClient) - # When: creating the evaluator config resp = client.post("/api/v1/evaluator-configs", json=payload) - # Then: validation error is returned + # Then: the invalid config is rejected assert resp.status_code == 422 data = resp.json() assert data["error_code"] == "INVALID_CONFIG" From 8e20d0e5664d06288c34282edccf07d4c6239648 Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Thu, 12 Mar 2026 17:25:44 -0700 Subject: [PATCH 3/4] test: complete given-when-then comments in server tests --- server/tests/test_controls_validation.py | 10 ++++++++-- server/tests/test_evaluator_configs.py | 8 +++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/server/tests/test_controls_validation.py b/server/tests/test_controls_validation.py index 9e9e6eb7..56717879 100644 --- a/server/tests/test_controls_validation.py +++ b/server/tests/test_controls_validation.py @@ -173,6 +173,8 @@ def test_validation_none_path_defaults_to_star(client: TestClient): # When: reading back get_resp = client.get(f"/api/v1/controls/{control_id}/data") + + # Then: reading back the control succeeds assert get_resp.status_code == 200 # Then: path should default to '*' @@ -182,11 +184,15 @@ def test_validation_none_path_defaults_to_star(client: TestClient): def test_get_control_data_returns_typed_response(client: TestClient): """Test that GET control data returns a typed ControlDefinition.""" - # Given: a control with valid control data + # Given: a control shell control_id = create_control(client) + + # When: setting valid control data resp_put = client.put( f"/api/v1/controls/{control_id}/data", json={"data": VALID_CONTROL_PAYLOAD} ) + + # Then: the control data is stored successfully assert resp_put.status_code == 200 # When: getting control data @@ -196,7 +202,7 @@ def test_get_control_data_returns_typed_response(client: TestClient): assert resp_get.status_code == 200 data = resp_get.json()["data"] - # Should have required ControlDefinition fields + # Then: the response includes required ControlDefinition fields assert "evaluator" in data assert "action" in data assert "selector" in data diff --git a/server/tests/test_evaluator_configs.py b/server/tests/test_evaluator_configs.py index 05d543f5..af13e90d 100644 --- a/server/tests/test_evaluator_configs.py +++ b/server/tests/test_evaluator_configs.py @@ -470,6 +470,8 @@ def test_list_evaluator_configs_cursor_pagination(client: TestClient) -> None: # When: requesting first page with limit=2 resp = client.get("/api/v1/evaluator-configs", params={"limit": 2}) + + # Then: the first page indicates there are more results assert resp.status_code == 200 page1 = resp.json() assert page1["pagination"]["has_more"] is True @@ -598,10 +600,14 @@ def test_delete_evaluator_config_success(client: TestClient) -> None: # When: deleting the evaluator config resp = client.delete(f"/api/v1/evaluator-configs/{created['id']}") - # Then: success is returned and the config is gone + # Then: the delete call succeeds assert resp.status_code == 200 assert resp.json()["success"] is True + + # When: fetching the deleted evaluator config get_resp = client.get(f"/api/v1/evaluator-configs/{created['id']}") + + # Then: the config is gone assert get_resp.status_code == 404 From 319eedea1cde51a84c02808c33bc37e1851bf02a Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 22:38:53 -0700 Subject: [PATCH 4/4] fix: reject blank list evaluator values --- .../agent_control_evaluators/list/config.py | 6 +-- .../list/evaluator.py | 4 +- evaluators/builtin/tests/list/test_list.py | 54 ++++++++++++++++++- server/tests/test_controls_validation.py | 10 ++-- server/tests/test_evaluator_configs.py | 11 ++-- 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/evaluators/builtin/src/agent_control_evaluators/list/config.py b/evaluators/builtin/src/agent_control_evaluators/list/config.py index 3277227c..43ae664c 100644 --- a/evaluators/builtin/src/agent_control_evaluators/list/config.py +++ b/evaluators/builtin/src/agent_control_evaluators/list/config.py @@ -28,7 +28,7 @@ class ListEvaluatorConfig(EvaluatorConfig): @field_validator("values") @classmethod def validate_values(cls, values: list[str | int | float]) -> list[str | int | float]: - """Reject empty-string entries that would compile into a match-all regex.""" - if any(isinstance(value, str) and value == "" for value in values): - raise ValueError("values must not contain empty strings") + """Reject blank string entries that would compile into pathological regexes.""" + if any(isinstance(value, str) and value.strip() == "" for value in values): + raise ValueError("values must not contain empty or whitespace-only strings") return values diff --git a/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py b/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py index 92e7df11..55bcd78b 100644 --- a/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py +++ b/evaluators/builtin/src/agent_control_evaluators/list/evaluator.py @@ -35,9 +35,9 @@ class ListEvaluator(Evaluator[ListEvaluatorConfig]): def __init__(self, config: ListEvaluatorConfig) -> None: super().__init__(config) - # Defensive filtering keeps legacy invalid configs from compiling into match-all regexes. + # Defensive filtering keeps legacy invalid configs from compiling into pathological regexes. normalized_values = [str(v) for v in config.values] - self._values = [value for value in normalized_values if value != ""] + self._values = [value for value in normalized_values if value.strip() != ""] self._regex: Any = self._build_regex() def _build_regex(self) -> Any: diff --git a/evaluators/builtin/tests/list/test_list.py b/evaluators/builtin/tests/list/test_list.py index b9f65c77..81ca2818 100644 --- a/evaluators/builtin/tests/list/test_list.py +++ b/evaluators/builtin/tests/list/test_list.py @@ -13,10 +13,22 @@ def test_empty_string_value_rejected(self) -> None: """Test that empty-string list entries are rejected at config validation time.""" # Given: a list evaluator config with an empty-string value # When: constructing the config model - with pytest.raises(ValidationError, match="values must not contain empty strings"): + with pytest.raises( + ValidationError, match="values must not contain empty or whitespace-only strings" + ): ListEvaluatorConfig(values=[""]) # Then: validation rejects the config (asserted by pytest) + def test_whitespace_only_value_rejected(self) -> None: + """Test that whitespace-only list entries are rejected at config validation time.""" + # Given: a list evaluator config with a whitespace-only value + # When: constructing the config model + with pytest.raises( + ValidationError, match="values must not contain empty or whitespace-only strings" + ): + ListEvaluatorConfig(values=[" "]) + # Then: validation rejects the config (asserted by pytest) + class TestListEvaluator: """Tests for list evaluator runtime behavior.""" @@ -40,3 +52,43 @@ async def test_legacy_empty_string_value_is_ignored_defensively(self) -> None: # Then: the evaluator ignores the empty control values assert result.matched is False assert result.message == "Empty control values - control ignored" + + @pytest.mark.asyncio + async def test_legacy_whitespace_only_value_is_ignored_defensively(self) -> None: + """Test that legacy whitespace-only configs do not compile into pathological regexes.""" + # Given: a legacy invalid config with a whitespace-only value + config = ListEvaluatorConfig.model_construct( + values=[" "], + logic="any", + match_on="match", + match_mode="contains", + case_sensitive=False, + ) + evaluator = ListEvaluator(config) + + # When: evaluating normal text against the legacy config + result = await evaluator.evaluate("Tell me a joke") + + # Then: the evaluator ignores the empty control values + assert result.matched is False + assert result.message == "Empty control values - control ignored" + + @pytest.mark.asyncio + async def test_legacy_empty_string_allowlist_does_not_block_all(self) -> None: + """Test that legacy invalid allowlist configs do not block all inputs.""" + # Given: a legacy invalid allowlist config constructed without validation + config = ListEvaluatorConfig.model_construct( + values=[""], + logic="any", + match_on="no_match", + match_mode="contains", + case_sensitive=False, + ) + evaluator = ListEvaluator(config) + + # When: evaluating normal text against the legacy config + result = await evaluator.evaluate("legitimate_value") + + # Then: the evaluator ignores the empty control values instead of blocking all input + assert result.matched is False + assert result.message == "Empty control values - control ignored" diff --git a/server/tests/test_controls_validation.py b/server/tests/test_controls_validation.py index 56717879..36ba0ac7 100644 --- a/server/tests/test_controls_validation.py +++ b/server/tests/test_controls_validation.py @@ -86,15 +86,15 @@ def test_validation_regex_flags_list(client: TestClient): assert any("flags" in str(e.get("field", "")) for e in errors) -def test_validation_list_values_reject_empty_strings(client: TestClient): - """Test that list evaluator config rejects empty-string entries.""" - # Given: a control and a list evaluator payload with an empty-string value +def test_validation_list_values_reject_blank_strings(client: TestClient): + """Test that list evaluator config rejects empty and whitespace-only entries.""" + # Given: a control and a list evaluator payload with a whitespace-only value control_id = create_control(client) payload = VALID_CONTROL_PAYLOAD.copy() payload["evaluator"] = { "name": "list", "config": { - "values": [""], + "values": [" "], "logic": "any", "match_on": "match", "match_mode": "contains", @@ -109,7 +109,7 @@ def test_validation_list_values_reject_empty_strings(client: TestClient): response_data = resp.json() errors = response_data.get("errors", []) assert any("values" in str(e.get("field", "")) for e in errors) - assert any("empty strings" in e.get("message", "") for e in errors) + assert any("empty or whitespace-only strings" in e.get("message", "") for e in errors) def test_validation_invalid_regex_pattern(client: TestClient): diff --git a/server/tests/test_evaluator_configs.py b/server/tests/test_evaluator_configs.py index af13e90d..c1c7a0bc 100644 --- a/server/tests/test_evaluator_configs.py +++ b/server/tests/test_evaluator_configs.py @@ -142,13 +142,13 @@ def test_create_evaluator_config_invalid_config_422(client: TestClient) -> None: assert any("config" in str(err.get("field", "")) for err in data.get("errors", [])) -def test_create_evaluator_config_rejects_empty_list_values(client: TestClient) -> None: - # Given: a payload with an empty-string entry for the list evaluator +def test_create_evaluator_config_rejects_blank_list_values(client: TestClient) -> None: + # Given: a payload with a whitespace-only entry for the list evaluator name = f"config-{uuid.uuid4().hex}" payload = _create_config_payload( name=name, evaluator="list", - config={"values": [""], "logic": "any", "match_on": "match", "match_mode": "contains"}, + config={"values": [" "], "logic": "any", "match_on": "match", "match_mode": "contains"}, ) # When: creating the evaluator config @@ -159,7 +159,10 @@ def test_create_evaluator_config_rejects_empty_list_values(client: TestClient) - data = resp.json() assert data["error_code"] == "INVALID_CONFIG" assert any("config.values" in str(err.get("field", "")) for err in data.get("errors", [])) - assert any("empty strings" in err.get("message", "") for err in data.get("errors", [])) + assert any( + "empty or whitespace-only strings" in err.get("message", "") + for err in data.get("errors", []) + ) def test_create_evaluator_config_invalid_parameters_type_error_422(