mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-12 14:00:35 +00:00
fix(memory): reject Windows reserved device names in an agent memory scope (#7941)
* fix(memory): reject Windows reserved device names in an agent memory scope
`_management_agent_name_or_501()` validated the `agent_name` query parameter
with `AGENT_NAME_PATTERN` (`^[A-Za-z0-9-]+$`) only, which admits `NUL`, `CON`,
`COM1` and the rest of the Windows reserved device names.
An agent scope is not only a label: DeerMem resolves it to
`{storage}/users/{uid}/agents/{name}/facts` and creates that directory on the
first write. On Windows the path never materialises, so `create_fact` raised
`FileNotFoundError`, which the endpoint's `OSError` branch turned into
`500 Failed to create memory fact`.
Run `windows_incompatible_segment()` at the same boundary -- the check agent
directories, thread IDs, uploads, sandbox writes and skill support files
already go through -- and answer `422`. The `for_creation` flag scopes the rule
to the two routes that create the scope, `POST /memory/facts` and
`POST /memory/import`; read and delete routes stay permissive so a scope that an
older release stored under a reserved name can still be exported and cleared.
Closes #7940
* fix(memory): address review feedback for Windows reserved agent names
Cite PR #7941 in CHANGELOG instead of issue #7940; pin the clear route's permissive behavior for a legacy reserved-name scope with a regression test; cross-reference the shared validate_agent_name() reserved-name check so the two copies cannot drift.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
24884e8120
commit
4c00e8d4a9
13
CHANGELOG.md
13
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
|
||||
|
||||
@ -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`.
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user