From 3920169fb6bd24439685d9c92e6e758ed642bdbd Mon Sep 17 00:00:00 2001 From: Eric <94447110+eric7h@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:55:46 +0800 Subject: [PATCH] fix(subagents): reject Windows reserved device names in managed subagent creation (#7959) * fix(subagents): reject Windows reserved device names in managed subagent creation * Reject Windows reserved device names (CON, PRN, AUX, NUL, COM1-9, LPT1-9) in normalize_managed_subagent_name when for_creation=True via windows_incompatible_segment. * Validate subagent names on creation in FileManagedSubagentStore.create and /api/subagents POST gateway router before attempting disk writes. * Keep read, update, and delete paths permissive for backward compatibility. * Add unit tests for store validation and router endpoints. * Update CHANGELOG.md under Fixed. * fix(subagents): reject reserved names in SQL managed-subagent create Apply the same for_creation check used by the file store so both backends refuse Windows device names. Pin mixed-case Nul in tests and import the shared normalizer at module level. --------- Co-authored-by: Willem Jiang --- CHANGELOG.md | 8 +++++++ backend/app/gateway/routers/subagents.py | 5 ++-- .../persistence/managed_subagents/base.py | 8 ++++++- .../persistence/managed_subagents/file.py | 1 + .../persistence/managed_subagents/sql.py | 1 + backend/tests/test_managed_subagent_store.py | 24 +++++++++++++++++++ backend/tests/test_subagents_router.py | 22 +++++++++++++++++ 7 files changed, 66 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 678acf8ffc..b6301cfa22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ This section accumulates work toward the **2.3.0** milestone ### Fixed +- **subagents:** Managed subagent creation now rejects Windows reserved device + names (`CON`, `PRN`, `AUX`, `NUL`, `COM1-9`, `LPT1-9`) at the shared + `normalize_managed_subagent_name(..., for_creation=True)` validator and + Gateway `/api/subagents` boundary before attempting filesystem writes, + preventing `OSError` failures on Windows hosts while keeping existing + read and delete operations permissive. + - **scheduler:** One scheduled task whose saved schedule can no longer be computed no longer stops every other task. When a stored timezone or cron expression stopped working (a zone missing from the time zone database @@ -36,6 +43,7 @@ This section accumulates work toward the **2.3.0** milestone expired, every later poll failed before admitting any due task. That task is now paused with the reason as its last error, and resuming it reports the problem until the schedule is fixed. ([#7993]) + - **community:** A search or fetch API key that ends in a newline no longer leaks into the conversation. GroundRoute (`GROUNDROUTE_API_KEY`), InfoQuest, Tavily, Exa, Jina, Firecrawl, fastCRW and Crawl4AI put the key into an HTTP diff --git a/backend/app/gateway/routers/subagents.py b/backend/app/gateway/routers/subagents.py index 739bb87a22..56e04c3343 100644 --- a/backend/app/gateway/routers/subagents.py +++ b/backend/app/gateway/routers/subagents.py @@ -88,9 +88,9 @@ def _validate_model(model: str, app_config) -> None: raise HTTPException(status_code=422, detail=f"Unknown model '{model}'. Use 'inherit' or a configured model name.") -def _validate_path_name(name: str) -> str: +def _validate_path_name(name: str, *, for_creation: bool = False) -> str: try: - return normalize_managed_subagent_name(name) + return normalize_managed_subagent_name(name, for_creation=for_creation) except ValueError as exc: raise HTTPException(status_code=422, detail=str(exc)) from exc @@ -181,6 +181,7 @@ async def list_subagents(request: Request) -> SubagentsListResponse: @router.post("", response_model=SubagentResponse, status_code=201, summary="Create Managed Subagent") async def create_managed_subagent(request: Request, body: ManagedSubagentCreateRequest) -> SubagentResponse: await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL) + _validate_path_name(body.name, for_creation=True) try: definition = ManagedSubagentDefinition(**body.model_dump(exclude_none=True)) except ValidationError as exc: diff --git a/backend/packages/harness/deerflow/persistence/managed_subagents/base.py b/backend/packages/harness/deerflow/persistence/managed_subagents/base.py index 41e8fab535..1e7eda7139 100644 --- a/backend/packages/harness/deerflow/persistence/managed_subagents/base.py +++ b/backend/packages/harness/deerflow/persistence/managed_subagents/base.py @@ -8,14 +8,20 @@ from collections.abc import Hashable from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from deerflow.utils.host_paths import windows_incompatible_segment + REQUIRED_DISALLOWED_TOOLS = frozenset({"task", "ask_clarification", "present_files"}) MANAGED_SUBAGENT_NAME_PATTERN = re.compile(r"^[A-Za-z0-9-]+$") -def normalize_managed_subagent_name(value: str) -> str: +def normalize_managed_subagent_name(value: str, *, for_creation: bool = False) -> str: """Validate and normalize a managed subagent natural key.""" if not isinstance(value, str) or not MANAGED_SUBAGENT_NAME_PATTERN.fullmatch(value): raise ValueError(f"Invalid managed subagent name {value!r}. Must match {MANAGED_SUBAGENT_NAME_PATTERN.pattern}") + if for_creation: + reason = windows_incompatible_segment(value) + if reason is not None: + raise ValueError(f"Invalid managed subagent name {value!r}: {reason}") return value.lower() diff --git a/backend/packages/harness/deerflow/persistence/managed_subagents/file.py b/backend/packages/harness/deerflow/persistence/managed_subagents/file.py index ba10f4c77e..ebe6cfe550 100644 --- a/backend/packages/harness/deerflow/persistence/managed_subagents/file.py +++ b/backend/packages/harness/deerflow/persistence/managed_subagents/file.py @@ -49,6 +49,7 @@ class FileManagedSubagentStore(ManagedSubagentStore): return sorted(definitions, key=lambda item: item.name) def create(self, definition: ManagedSubagentDefinition) -> None: + normalize_managed_subagent_name(definition.name, for_creation=True) path = get_paths().managed_subagent_file(definition.name) with _write_lock: if path.exists(): diff --git a/backend/packages/harness/deerflow/persistence/managed_subagents/sql.py b/backend/packages/harness/deerflow/persistence/managed_subagents/sql.py index b47a212ea7..c299fdc7fe 100644 --- a/backend/packages/harness/deerflow/persistence/managed_subagents/sql.py +++ b/backend/packages/harness/deerflow/persistence/managed_subagents/sql.py @@ -44,6 +44,7 @@ class SqlManagedSubagentStore(ManagedSubagentStore): return [ManagedSubagentDefinition.model_validate(row.definition) for row in rows] def create(self, definition: ManagedSubagentDefinition) -> None: + normalize_managed_subagent_name(definition.name, for_creation=True) row = ManagedSubagentRow( id=uuid.uuid4().hex, name=definition.name, diff --git a/backend/tests/test_managed_subagent_store.py b/backend/tests/test_managed_subagent_store.py index 88606c9c9a..b0dabc9d17 100644 --- a/backend/tests/test_managed_subagent_store.py +++ b/backend/tests/test_managed_subagent_store.py @@ -15,6 +15,7 @@ from deerflow.persistence.managed_subagents import ( ManagedSubagentExistsError, make_managed_subagent_store, ) +from deerflow.persistence.managed_subagents.base import normalize_managed_subagent_name from deerflow.persistence.managed_subagents.file import FileManagedSubagentStore from deerflow.persistence.managed_subagents.model import ManagedSubagentRow from deerflow.persistence.managed_subagents.sql import SqlManagedSubagentStore @@ -158,3 +159,26 @@ def test_store_factory_rejects_memory_database(): ) with pytest.raises(ValueError, match="sqlite.*postgres"): make_managed_subagent_store(config) + + +@pytest.mark.parametrize("name", ["nul", "NUL", "Nul", "con", "prn", "aux", "com1", "lpt9"]) +def test_normalize_managed_subagent_name_rejects_windows_reserved_names(name: str): + with pytest.raises(ValueError, match="reserved Windows device name"): + normalize_managed_subagent_name(name, for_creation=True) + + +@pytest.mark.parametrize("name", ["nul", "con", "com1"]) +def test_normalize_managed_subagent_name_allows_reserved_names_on_read(name: str): + assert normalize_managed_subagent_name(name) == name.lower() + + +@pytest.mark.parametrize("name", ["planner", "code-reviewer-2", "researcher"]) +def test_normalize_managed_subagent_name_accepts_valid_names(name: str): + assert normalize_managed_subagent_name(name, for_creation=True) == name + + +@pytest.mark.parametrize("store_fixture", ["file_store", "sql_store"]) +def test_store_rejects_windows_reserved_device_names(request, store_fixture): + store = request.getfixturevalue(store_fixture) + with pytest.raises(ValueError, match="reserved Windows device name"): + store.create(_definition("nul")) diff --git a/backend/tests/test_subagents_router.py b/backend/tests/test_subagents_router.py index 15684c9b4e..e1fe9e678b 100644 --- a/backend/tests/test_subagents_router.py +++ b/backend/tests/test_subagents_router.py @@ -320,3 +320,25 @@ async def test_invalid_path_name_returns_422(operation: str): else: await router.delete_managed_subagent("../planner", _request("admin")) assert excinfo.value.status_code == 422 + + +@pytest.mark.parametrize("name", ["nul", "NUL", "Nul", "con", "prn", "aux", "com1", "lpt9"]) +async def test_create_managed_subagent_rejects_windows_reserved_names(name: str): + with pytest.raises(HTTPException) as excinfo: + await router.create_managed_subagent( + _request("admin"), + router.ManagedSubagentCreateRequest( + name=name, + description="Reserved name", + system_prompt="Should fail.", + ), + ) + assert excinfo.value.status_code == 422 + assert "reserved Windows device name" in str(excinfo.value.detail) + + +@pytest.mark.parametrize("name", ["nul", "con", "com1"]) +async def test_delete_managed_subagent_allows_windows_reserved_names(name: str): + with pytest.raises(HTTPException) as excinfo: + await router.delete_managed_subagent(name, _request("admin")) + assert excinfo.value.status_code == 404