diff --git a/evaluators/builtin/src/agent_control_evaluators/list/config.py b/evaluators/builtin/src/agent_control_evaluators/list/config.py index 3ba7efbf..43ae664c 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 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 7c0612ed..55bcd78b 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 pathological regexes. + normalized_values = [str(v) for v in config.values] + 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 new file mode 100644 index 00000000..81ca2818 --- /dev/null +++ b/evaluators/builtin/tests/list/test_list.py @@ -0,0 +1,94 @@ +"""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.""" + # 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 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.""" + + @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", + 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_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 cd5084ae..36ba0ac7 100644 --- a/server/tests/test_controls_validation.py +++ b/server/tests/test_controls_validation.py @@ -86,6 +86,32 @@ 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_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": [" "], + "logic": "any", + "match_on": "match", + "match_mode": "contains", + }, + } + + # 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", []) + assert any("values" in str(e.get("field", "")) 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): """Test validation of regex pattern syntax.""" # Given: a control and regex config with invalid pattern (unclosed bracket) @@ -147,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 '*' @@ -156,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 @@ -170,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 08187f4c..c1c7a0bc 100644 --- a/server/tests/test_evaluator_configs.py +++ b/server/tests/test_evaluator_configs.py @@ -142,6 +142,29 @@ 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_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"}, + ) + + # When: creating the evaluator config + resp = client.post("/api/v1/evaluator-configs", json=payload) + + # Then: the invalid config is rejected + 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 or whitespace-only 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: @@ -450,6 +473,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 @@ -578,10 +603,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