From f61372764e6b2e0c07412746beed1eabfe213006 Mon Sep 17 00:00:00 2001 From: "Grapette.L" <90967079+Lesereingrape@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:38:09 +0800 Subject: [PATCH] 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. --- backend/app/gateway/routers/agents.py | 2 +- .../backends/deermem/deermem/core/paths.py | 2 +- backend/tests/test_custom_agent.py | 32 ++++++++++++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/backend/app/gateway/routers/agents.py b/backend/app/gateway/routers/agents.py index b52a502a76..6f18369f78 100644 --- a/backend/app/gateway/routers/agents.py +++ b/backend/app/gateway/routers/agents.py @@ -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).", diff --git a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/paths.py b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/paths.py index 37fd6a7ef5..06a77dcc4a 100644 --- a/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/paths.py +++ b/backend/packages/harness/deerflow/agents/memory/backends/deermem/deermem/core/paths.py @@ -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}") diff --git a/backend/tests/test_custom_agent.py b/backend/tests/test_custom_agent.py index f26353e976..4f3e2781f7 100644 --- a/backend/tests/test_custom_agent.py +++ b/backend/tests/test_custom_agent.py @@ -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