Skip to content

Commit 2c55d27

Browse files
authored
fix(server): reject empty string list evaluator values (#121)
## Summary - reject empty-string list entries at list evaluator config validation time - defensively filter exact empty strings in the evaluator so legacy invalid configs do not compile into a match-all regex - add evaluator and server regression tests for both validation paths ## Testing - make evaluators-test - make server-test - make lint - make typecheck Fixes #120
1 parent 14f7e48 commit 2c55d27

5 files changed

Lines changed: 170 additions & 5 deletions

File tree

‎evaluators/builtin/src/agent_control_evaluators/list/config.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from typing import Literal
44

5-
from pydantic import Field
5+
from pydantic import Field, field_validator
66

77
from agent_control_evaluators._base import EvaluatorConfig
88

@@ -24,3 +24,11 @@ class ListEvaluatorConfig(EvaluatorConfig):
2424
description="'exact' for full string match, 'contains' for keyword/substring match",
2525
)
2626
case_sensitive: bool = Field(False, description="Whether matching is case sensitive")
27+
28+
@field_validator("values")
29+
@classmethod
30+
def validate_values(cls, values: list[str | int | float]) -> list[str | int | float]:
31+
"""Reject blank string entries that would compile into pathological regexes."""
32+
if any(isinstance(value, str) and value.strip() == "" for value in values):
33+
raise ValueError("values must not contain empty or whitespace-only strings")
34+
return values

‎evaluators/builtin/src/agent_control_evaluators/list/evaluator.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ class ListEvaluator(Evaluator[ListEvaluatorConfig]):
3535

3636
def __init__(self, config: ListEvaluatorConfig) -> None:
3737
super().__init__(config)
38-
self._values = [str(v) for v in config.values]
38+
# Defensive filtering keeps legacy invalid configs from compiling into pathological regexes.
39+
normalized_values = [str(v) for v in config.values]
40+
self._values = [value for value in normalized_values if value.strip() != ""]
3941
self._regex: Any = self._build_regex()
4042

4143
def _build_regex(self) -> Any:
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
"""Tests for list evaluator."""
2+
3+
import pytest
4+
from pydantic import ValidationError
5+
6+
from agent_control_evaluators.list import ListEvaluator, ListEvaluatorConfig
7+
8+
9+
class TestListEvaluatorConfig:
10+
"""Tests for list evaluator config validation."""
11+
12+
def test_empty_string_value_rejected(self) -> None:
13+
"""Test that empty-string list entries are rejected at config validation time."""
14+
# Given: a list evaluator config with an empty-string value
15+
# When: constructing the config model
16+
with pytest.raises(
17+
ValidationError, match="values must not contain empty or whitespace-only strings"
18+
):
19+
ListEvaluatorConfig(values=[""])
20+
# Then: validation rejects the config (asserted by pytest)
21+
22+
def test_whitespace_only_value_rejected(self) -> None:
23+
"""Test that whitespace-only list entries are rejected at config validation time."""
24+
# Given: a list evaluator config with a whitespace-only value
25+
# When: constructing the config model
26+
with pytest.raises(
27+
ValidationError, match="values must not contain empty or whitespace-only strings"
28+
):
29+
ListEvaluatorConfig(values=[" "])
30+
# Then: validation rejects the config (asserted by pytest)
31+
32+
33+
class TestListEvaluator:
34+
"""Tests for list evaluator runtime behavior."""
35+
36+
@pytest.mark.asyncio
37+
async def test_legacy_empty_string_value_is_ignored_defensively(self) -> None:
38+
"""Test that legacy invalid configs do not compile into a match-all regex."""
39+
# Given: a legacy invalid config constructed without validation
40+
config = ListEvaluatorConfig.model_construct(
41+
values=[""],
42+
logic="any",
43+
match_on="match",
44+
match_mode="contains",
45+
case_sensitive=False,
46+
)
47+
evaluator = ListEvaluator(config)
48+
49+
# When: evaluating normal text against the legacy config
50+
result = await evaluator.evaluate("Tell me a joke")
51+
52+
# Then: the evaluator ignores the empty control values
53+
assert result.matched is False
54+
assert result.message == "Empty control values - control ignored"
55+
56+
@pytest.mark.asyncio
57+
async def test_legacy_whitespace_only_value_is_ignored_defensively(self) -> None:
58+
"""Test that legacy whitespace-only configs do not compile into pathological regexes."""
59+
# Given: a legacy invalid config with a whitespace-only value
60+
config = ListEvaluatorConfig.model_construct(
61+
values=[" "],
62+
logic="any",
63+
match_on="match",
64+
match_mode="contains",
65+
case_sensitive=False,
66+
)
67+
evaluator = ListEvaluator(config)
68+
69+
# When: evaluating normal text against the legacy config
70+
result = await evaluator.evaluate("Tell me a joke")
71+
72+
# Then: the evaluator ignores the empty control values
73+
assert result.matched is False
74+
assert result.message == "Empty control values - control ignored"
75+
76+
@pytest.mark.asyncio
77+
async def test_legacy_empty_string_allowlist_does_not_block_all(self) -> None:
78+
"""Test that legacy invalid allowlist configs do not block all inputs."""
79+
# Given: a legacy invalid allowlist config constructed without validation
80+
config = ListEvaluatorConfig.model_construct(
81+
values=[""],
82+
logic="any",
83+
match_on="no_match",
84+
match_mode="contains",
85+
case_sensitive=False,
86+
)
87+
evaluator = ListEvaluator(config)
88+
89+
# When: evaluating normal text against the legacy config
90+
result = await evaluator.evaluate("legitimate_value")
91+
92+
# Then: the evaluator ignores the empty control values instead of blocking all input
93+
assert result.matched is False
94+
assert result.message == "Empty control values - control ignored"

‎server/tests/test_controls_validation.py‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,32 @@ def test_validation_regex_flags_list(client: TestClient):
8686
assert any("flags" in str(e.get("field", "")) for e in errors)
8787

8888

89+
def test_validation_list_values_reject_blank_strings(client: TestClient):
90+
"""Test that list evaluator config rejects empty and whitespace-only entries."""
91+
# Given: a control and a list evaluator payload with a whitespace-only value
92+
control_id = create_control(client)
93+
payload = VALID_CONTROL_PAYLOAD.copy()
94+
payload["evaluator"] = {
95+
"name": "list",
96+
"config": {
97+
"values": [" "],
98+
"logic": "any",
99+
"match_on": "match",
100+
"match_mode": "contains",
101+
},
102+
}
103+
104+
# When: setting control data
105+
resp = client.put(f"/api/v1/controls/{control_id}/data", json={"data": payload})
106+
107+
# Then: the invalid config is rejected
108+
assert resp.status_code == 422
109+
response_data = resp.json()
110+
errors = response_data.get("errors", [])
111+
assert any("values" in str(e.get("field", "")) for e in errors)
112+
assert any("empty or whitespace-only strings" in e.get("message", "") for e in errors)
113+
114+
89115
def test_validation_invalid_regex_pattern(client: TestClient):
90116
"""Test validation of regex pattern syntax."""
91117
# 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):
147173

148174
# When: reading back
149175
get_resp = client.get(f"/api/v1/controls/{control_id}/data")
176+
177+
# Then: reading back the control succeeds
150178
assert get_resp.status_code == 200
151179

152180
# Then: path should default to '*'
@@ -156,11 +184,15 @@ def test_validation_none_path_defaults_to_star(client: TestClient):
156184

157185
def test_get_control_data_returns_typed_response(client: TestClient):
158186
"""Test that GET control data returns a typed ControlDefinition."""
159-
# Given: a control with valid control data
187+
# Given: a control shell
160188
control_id = create_control(client)
189+
190+
# When: setting valid control data
161191
resp_put = client.put(
162192
f"/api/v1/controls/{control_id}/data", json={"data": VALID_CONTROL_PAYLOAD}
163193
)
194+
195+
# Then: the control data is stored successfully
164196
assert resp_put.status_code == 200
165197

166198
# When: getting control data
@@ -170,7 +202,7 @@ def test_get_control_data_returns_typed_response(client: TestClient):
170202
assert resp_get.status_code == 200
171203
data = resp_get.json()["data"]
172204

173-
# Should have required ControlDefinition fields
205+
# Then: the response includes required ControlDefinition fields
174206
assert "evaluator" in data
175207
assert "action" in data
176208
assert "selector" in data

‎server/tests/test_evaluator_configs.py‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,29 @@ def test_create_evaluator_config_invalid_config_422(client: TestClient) -> None:
142142
assert any("config" in str(err.get("field", "")) for err in data.get("errors", []))
143143

144144

145+
def test_create_evaluator_config_rejects_blank_list_values(client: TestClient) -> None:
146+
# Given: a payload with a whitespace-only entry for the list evaluator
147+
name = f"config-{uuid.uuid4().hex}"
148+
payload = _create_config_payload(
149+
name=name,
150+
evaluator="list",
151+
config={"values": [" "], "logic": "any", "match_on": "match", "match_mode": "contains"},
152+
)
153+
154+
# When: creating the evaluator config
155+
resp = client.post("/api/v1/evaluator-configs", json=payload)
156+
157+
# Then: the invalid config is rejected
158+
assert resp.status_code == 422
159+
data = resp.json()
160+
assert data["error_code"] == "INVALID_CONFIG"
161+
assert any("config.values" in str(err.get("field", "")) for err in data.get("errors", []))
162+
assert any(
163+
"empty or whitespace-only strings" in err.get("message", "")
164+
for err in data.get("errors", [])
165+
)
166+
167+
145168
def test_create_evaluator_config_invalid_parameters_type_error_422(
146169
client: TestClient, monkeypatch
147170
) -> None:
@@ -450,6 +473,8 @@ def test_list_evaluator_configs_cursor_pagination(client: TestClient) -> None:
450473

451474
# When: requesting first page with limit=2
452475
resp = client.get("/api/v1/evaluator-configs", params={"limit": 2})
476+
477+
# Then: the first page indicates there are more results
453478
assert resp.status_code == 200
454479
page1 = resp.json()
455480
assert page1["pagination"]["has_more"] is True
@@ -578,10 +603,14 @@ def test_delete_evaluator_config_success(client: TestClient) -> None:
578603
# When: deleting the evaluator config
579604
resp = client.delete(f"/api/v1/evaluator-configs/{created['id']}")
580605

581-
# Then: success is returned and the config is gone
606+
# Then: the delete call succeeds
582607
assert resp.status_code == 200
583608
assert resp.json()["success"] is True
609+
610+
# When: fetching the deleted evaluator config
584611
get_resp = client.get(f"/api/v1/evaluator-configs/{created['id']}")
612+
613+
# Then: the config is gone
585614
assert get_resp.status_code == 404
586615

587616

0 commit comments

Comments
 (0)