fix(agents): anchor agent-name validation against a trailing newline (#5960)

* fix(agents): anchor agent-name validation against a trailing newline

Two copies of the shared ^[A-Za-z0-9-]+$ grammar still used re.match,
whose $ also matches before a final newline, while every other copy uses
fullmatch. POST /api/agents {"name": "reviewer\n"} therefore passed the
router guard and died inside the strict file store as an HTTP 500, and
DeerMem accepted the same name as an agents/{name}/facts directory.

* test(agents): state which trailing-newline params the regression actually covered

.match only accepted "reviewer\n" on main; "reviewer\n\n" and "reviewer \n"
were already rejected, so the docstrings should not read as if every param in
these parametrizations was broken before the fix.
This commit is contained in:
Grapette.L 2026-09-30 19:38:09 +08:00 committed by GitHub
parent a83aebe702
commit f61372764e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 3 deletions

View File

@ -108,7 +108,7 @@ def _validate_agent_name(name: str) -> None:
Raises:
HTTPException: 422 if the name is invalid.
"""
if not AGENT_NAME_PATTERN.match(name):
if not AGENT_NAME_PATTERN.fullmatch(name):
raise HTTPException(
status_code=422,
detail=f"Invalid agent name '{name}'. Must match ^[A-Za-z0-9-]+$ (letters, digits, and hyphens only).",

View File

@ -57,7 +57,7 @@ def validate_agent_name(name: str) -> None:
"""Validate that the agent name is safe to use in filesystem paths."""
if not name:
raise ValueError("Agent name must be a non-empty string.")
if name != DEFAULT_AGENT_BUCKET and not AGENT_NAME_PATTERN.match(name):
if name != DEFAULT_AGENT_BUCKET and not AGENT_NAME_PATTERN.fullmatch(name):
raise ValueError(f"Invalid agent name {name!r}: names must match {AGENT_NAME_PATTERN.pattern}")

View File

@ -12,7 +12,7 @@ from fastapi.testclient import TestClient
from app.gateway.routers.agents import AGENT_NAME_PATTERN as GATEWAY_AGENT_NAME_PATTERN
from deerflow.agents.memory.backends.deermem.deermem.core.paths import AGENT_NAME_PATTERN as DEERMEM_AGENT_NAME_PATTERN
from deerflow.agents.memory.backends.deermem.deermem.core.paths import DEFAULT_AGENT_BUCKET, validate_agent_name
from deerflow.agents.memory.backends.deermem.deermem.core.paths import DEFAULT_AGENT_BUCKET, agent_facts_directory, validate_agent_name
from deerflow.config.agents_api_config import AgentsApiConfig, get_agents_api_config, set_agents_api_config
# ---------------------------------------------------------------------------
@ -26,6 +26,24 @@ def test_reserved_memory_bucket_stays_outside_both_public_agent_patterns() -> No
validate_agent_name(DEFAULT_AGENT_BUCKET) # Internal storage sentinel remains usable.
@pytest.mark.parametrize("name", ["reviewer\n", "reviewer \n"])
def test_agent_name_validation_rejects_trailing_newline(name: str) -> None:
"""``$`` in ``^[A-Za-z0-9-]+$`` also matches before a final newline.
Only ``fullmatch`` anchors it, so DeerMem's inlined copy of the host's
agent-name grammar accepted ``"reviewer\\n"`` and used it as a directory
name — which the host's own strict validator then refuses forever.
``"reviewer \\n"`` was already rejected by ``.match`` (the space falls
outside the class, so the match never reaches ``$``); it is parametrized
here to pin the grammar, not because it regressed.
"""
with pytest.raises(ValueError, match="Invalid agent name"):
validate_agent_name(name)
with pytest.raises(ValueError, match="Invalid agent name"):
agent_facts_directory(Path("memory.json"), name)
def _make_paths(base_dir: Path):
"""Return a Paths instance pointing to base_dir."""
from deerflow.config.paths import Paths
@ -609,6 +627,18 @@ class TestAgentsAPI:
assert agent_client.put("/api/agents/reviewer", json={"display_name": display_name}).status_code == 422
assert agent_client.get("/api/agents/reviewer").json()["display_name"] == "🦌" * 100
@pytest.mark.parametrize("name", ["reviewer\n", "reviewer\n\n"])
def test_trailing_newline_in_agent_name_is_rejected(self, agent_client, name):
"""The router's ``AGENT_NAME_PATTERN.match`` accepted ``"reviewer\\n"`` and the store 500'd.
``$`` matches before a single trailing newline, so that one param reached
the file store, which validates the same grammar with ``fullmatch``.
``"reviewer\\n\\n"`` was already rejected by ``.match`` on main; it is
parametrized to pin the grammar, not because it regressed.
"""
assert agent_client.post("/api/agents", json={"name": name}).status_code == 422
assert agent_client.get("/api/agents").json()["agents"] == []
def test_display_name_round_trip_keeps_stable_identity(self, agent_client):
response = agent_client.post("/api/agents", json={"name": "code-reviewer", "display_name": " 代码审查助手 "})
assert response.status_code == 201