diff --git a/CHANGELOG.md b/CHANGELOG.md index c8fa4732eb..f61c5cd609 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,18 @@ This section accumulates work toward the **2.3.0** milestone threads. A group bound with its own `/connect` keeps its thread; a legacy group binding, which covers every group of the bot, starts a new one in each group. ([#7794]) + +- **memory:** An agent-scoped memory write whose `agent_name` is a Windows + reserved device name (`NUL`, `CON`, `COM1`, ...) is now rejected with a `422` + on the two routes that create that scope, `POST /memory/facts` and + `POST /memory/import`. DeerMem resolves the scope to + `{storage}/users/{uid}/agents/{name}/facts` and creates it on the first write; + the character-class check admitted such names, so the write raised + `FileNotFoundError` and the endpoint answered an opaque `500`. The check reuses + `windows_incompatible_segment()` and applies on every platform. Read and delete + routes stay permissive, so a scope with a reserved-name identifier that an + older release stored on a Linux host can still be exported and cleared. + ([#7941]) - **skills:** A custom skill name that is a Windows reserved device name (`nul`, `con`, `com1`, ...) is now rejected when the skill is created — @@ -16138,6 +16150,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#7831]: https://github.com/bytedance/deer-flow/pull/7831 [#7892]: https://github.com/bytedance/deer-flow/pull/7892 [#7907]: https://github.com/bytedance/deer-flow/pull/7907 +[#7941]: https://github.com/bytedance/deer-flow/pull/7941 [#7945]: https://github.com/bytedance/deer-flow/issues/7945 [#7953]: https://github.com/bytedance/deer-flow/issues/7953 [#7958]: https://github.com/bytedance/deer-flow/pull/7958 diff --git a/backend/app/gateway/routers/AGENTS.md b/backend/app/gateway/routers/AGENTS.md index 8dddba0be4..a57e7590f3 100644 --- a/backend/app/gateway/routers/AGENTS.md +++ b/backend/app/gateway/routers/AGENTS.md @@ -121,3 +121,13 @@ Import probes each new profile, saves through the drained writer, and atomically deduplicates retries without changing existing disabled profiles. Saved-key reuse requires matching connection and revision, checked again under the write lock after probing (409 on change); provider errors remain redacted. + +Agent-scoped memory routes take `agent_name` and validate it with +`AGENT_NAME_PATTERN`; the backend must also declare +`supports_agent_scoped_management` or the route answers 501. The two routes that +create the scope — `POST /memory/facts` and `POST /memory/import` — additionally +reject Windows reserved device names through `windows_incompatible_segment()`. +The scope is a host directory (`agents/{name}/facts`), so the character class +alone let the first write fail with `FileNotFoundError` and answer 500. Keep +reads and deletes permissive: a scope an older release stored under a reserved +name must stay exportable and clearable. Tests: `test_memory_router.py`. diff --git a/backend/app/gateway/routers/memory.py b/backend/app/gateway/routers/memory.py index 80f3ead0b5..921718305a 100644 --- a/backend/app/gateway/routers/memory.py +++ b/backend/app/gateway/routers/memory.py @@ -16,6 +16,7 @@ from deerflow.config.agents_config import AGENT_NAME_PATTERN from deerflow.config.memory_config import get_memory_config from deerflow.config.paths import make_safe_user_id from deerflow.runtime.user_context import get_effective_user_id +from deerflow.utils.host_paths import windows_incompatible_segment router = APIRouter(prefix="/api", tags=["memory"]) @@ -161,6 +162,8 @@ def _unsupported_501(manager: object, label: str) -> HTTPException: def _management_agent_name_or_501( manager: MemoryManager, agent_name: str | None, + *, + for_creation: bool = False, ) -> str | None: """Validate an optional agent-scoped management target. @@ -170,6 +173,16 @@ def _management_agent_name_or_501( into a mutation of the wrong bucket. Preserve the caller's spelling here: remote backends such as Mem0 use the agent name as a case-sensitive storage identity, while DeerMem performs its own lowercase canonicalization. + + ``for_creation`` also refuses names that cannot become a host path + component. DeerMem resolves an agent scope to + ``{storage}/users/{uid}/agents/{name}/facts`` and creates it on the first + write; the character class above admits Windows reserved device names + (``NUL``, ``CON``, ``COM1``, ...), so that write raised + ``FileNotFoundError`` and the endpoint's ``OSError`` branch answered + ``500``. Only the two endpoints that create the scope set this: reads and + deletes stay permissive so a scope an older release stored can still be + exported and cleared. """ if agent_name is None: return None @@ -178,6 +191,20 @@ def _management_agent_name_or_501( status_code=422, detail=(f"Invalid agent name '{agent_name}'. Must match {AGENT_NAME_PATTERN.pattern} (letters, digits, and hyphens only)."), ) + if for_creation: + # Reuse the canonical Windows-name check that agent directories, + # thread IDs, uploads, sandbox writes and skill support files use. + # Sibling PR #7939 applies the same rule inside the shared + # ``validate_agent_name()`` (``deerflow.config.agents_config``); this + # router inlines ``AGENT_NAME_PATTERN`` and preserves caller spelling for + # case-sensitive backends rather than delegating to it, so keep the two + # copies in sync if the reserved set ever changes. + reason = windows_incompatible_segment(agent_name) + if reason is not None: + raise HTTPException( + status_code=422, + detail=f"Invalid agent name '{agent_name}'. Contains a {reason}.", + ) if getattr(manager, "supports_agent_scoped_management", False) is not True: raise HTTPException( status_code=501, @@ -412,7 +439,7 @@ async def clear_memory(request: Request, agent_name: str | None = None) -> Memor async def create_memory_fact_endpoint(body: FactCreateRequest, request: Request, agent_name: str | None = None) -> MemoryResponse: """Create a single fact manually.""" manager = await asyncio.to_thread(get_memory_manager) - selected_agent = _management_agent_name_or_501(manager, agent_name) + selected_agent = _management_agent_name_or_501(manager, agent_name, for_creation=True) scope_kwargs = _agent_scope_kwargs(selected_agent) try: memory_data, fact_id = await run_drained_write( @@ -545,7 +572,7 @@ async def export_memory(request: Request, agent_name: str | None = None) -> Memo async def import_memory(body: MemoryResponse, request: Request, agent_name: str | None = None) -> MemoryResponse: """Import memory data, preserving shared summaries for a selected agent.""" manager = await asyncio.to_thread(get_memory_manager) - selected_agent = _management_agent_name_or_501(manager, agent_name) + selected_agent = _management_agent_name_or_501(manager, agent_name, for_creation=True) scope_kwargs = _agent_scope_kwargs(selected_agent) try: memory_data = await run_drained_write( diff --git a/backend/tests/test_memory_router.py b/backend/tests/test_memory_router.py index 65709b224a..1fbdc6d174 100644 --- a/backend/tests/test_memory_router.py +++ b/backend/tests/test_memory_router.py @@ -1049,3 +1049,96 @@ def test_import_blank_fact_returns_400_without_replacing_saved_memory(tmp_path): response = client.post("/api/memory/import", json=payload) assert response.status_code == 400 assert manager.get_memory(user_id="alice") == before + + +# ── Windows reserved agent names ──────────────────────────────────────────── + + +@pytest.mark.parametrize("agent_name", ["NUL", "nul", "Nul", "CON", "COM1", "LPT9"]) +def test_create_fact_rejects_windows_reserved_agent_name(tmp_path, agent_name): + """An agent scope is a directory under ``agents/``, so a device name cannot be written. + + ``AGENT_NAME_PATTERN`` admits ``NUL``/``CON``/``COM1``. DeerMem resolves the + scope to ``{storage}/users/{uid}/agents/{name}/facts`` and creates it on the + first write; on Windows that path never appears and the write raises + ``FileNotFoundError``, which the endpoint's ``OSError`` branch turned into + ``500 Failed to create memory fact``. Reject the scope up front instead. + """ + app = make_authed_test_app() + app.include_router(memory.router) + manager = DeerMem(backend_config={"storage_path": str(tmp_path)}) + with ( + patch("app.gateway.routers.memory.get_memory_manager", return_value=manager), + patch("app.gateway.routers.memory.get_effective_user_id", return_value="alice"), + TestClient(app) as client, + ): + response = client.post("/api/memory/facts", params={"agent_name": agent_name}, json={"content": "hello"}) + + assert response.status_code == 422 + assert not (tmp_path / "users" / "alice" / "agents").exists() + + +@pytest.mark.parametrize("agent_name", ["NUL", "CON"]) +def test_import_rejects_windows_reserved_agent_name(tmp_path, agent_name): + """The import path creates the same scope directory and needs the same guard.""" + app = make_authed_test_app() + app.include_router(memory.router) + manager = DeerMem(backend_config={"storage_path": str(tmp_path)}) + fact = { + "id": "fact-reserved", + "content": "Agent-scoped import", + "category": "context", + "confidence": 0.5, + "createdAt": "2026-07-21T00:00:00Z", + "source": "import", + } + with ( + patch("app.gateway.routers.memory.get_memory_manager", return_value=manager), + patch("app.gateway.routers.memory.get_effective_user_id", return_value="alice"), + TestClient(app) as client, + ): + response = client.post("/api/memory/import", params={"agent_name": agent_name}, json=_sample_memory(facts=[fact])) + + assert response.status_code == 422 + assert not (tmp_path / "users" / "alice" / "agents").exists() + + +def test_read_routes_keep_working_for_windows_reserved_agent_name(tmp_path): + """Reads must stay permissive so an older release's scope can still be drained. + + ``windows_incompatible_segment()`` applies when a name is created. Refusing a + stored scope on read would leave the owner unable to export or clear it. + """ + app = make_authed_test_app() + app.include_router(memory.router) + manager = DeerMem(backend_config={"storage_path": str(tmp_path)}) + with ( + patch("app.gateway.routers.memory.get_memory_manager", return_value=manager), + patch("app.gateway.routers.memory.get_effective_user_id", return_value="alice"), + TestClient(app) as client, + ): + response = client.get("/api/memory/export", params={"agent_name": "NUL"}) + + assert response.status_code == 200 + + +def test_clear_route_keeps_working_for_windows_reserved_agent_name(tmp_path): + """Deletes must stay permissive so an older release's scope can still be cleared. + + ``windows_incompatible_segment()`` applies when a name is created, not when an + existing scope is removed. A delete-only change set on a fresh scope writes + nothing under ``agents/``, so a legacy reserved-name scope stored by an older + Linux release remains clearable (200) rather than rejected with 422. + """ + app = make_authed_test_app() + app.include_router(memory.router) + manager = DeerMem(backend_config={"storage_path": str(tmp_path)}) + with ( + patch("app.gateway.routers.memory.get_memory_manager", return_value=manager), + patch("app.gateway.routers.memory.get_effective_user_id", return_value="alice"), + TestClient(app) as client, + ): + response = client.delete("/api/memory", params={"agent_name": "NUL"}) + + assert response.status_code == 200 + assert not (tmp_path / "users" / "alice" / "agents").exists()