fix(agents): require config.yaml in update_agent's legacy-agent guard (#4166)

update_agent (the harness tool) and PUT /api/agents/{name} (the same
operation over HTTP) share an identical guard meant to block updates to
an agent that only exists in the legacy shared layout. The guard checked
bare directory existence:

    if not agent_dir.exists() and paths.agent_dir(name).exists():

When memory is enabled, the first time a user chats with a legacy shared
agent, the memory writer creates a per-user directory containing only
memory.json (no config.yaml). agent_dir.exists() is then true, so the
guard never fires: the tool falls through to load_agent_config, which
resolves through to the legacy shared config via the already-hardened
resolve_agent_dir, and silently writes a brand-new config.yaml/SOUL.md
into the memory-only directory. That forks the agent for just this user;
every other user keeps reading the original shared config forever, with
no error or warning.

resolve_agent_dir itself was already hardened against exactly this
failure mode: it requires config.yaml to exist, not just the directory.
Mirror that condition at both call sites here.
This commit is contained in:
Daoyuan Li
2026-07-15 08:13:17 +08:00
committed by GitHub
parent 8e96a6a252
commit 0dd90ccfde
4 changed files with 76 additions and 2 deletions
+9 -1
View File
@@ -301,7 +301,15 @@ async def update_agent(name: str, request: AgentUpdateRequest) -> AgentResponse:
paths = get_paths()
agent_dir = paths.user_agent_dir(user_id, name)
if not agent_dir.exists() and paths.agent_dir(name).exists():
legacy_dir = paths.agent_dir(name)
# Require config.yaml, not bare directory existence — a per-user agent
# directory can exist containing only memory.json (written the first
# time this user chats with a legacy shared agent, before this route
# is ever called). Bare .exists() would miss that case and let this
# fall through to a silent fork of a brand-new config.yaml/SOUL.md
# into the memory-only directory instead of blocking (mirrors
# resolve_agent_dir's guard, see #3390).
if not (agent_dir / "config.yaml").exists() and (legacy_dir / "config.yaml").exists():
raise HTTPException(
status_code=409,
detail=(f"Agent '{name}' only exists in the legacy shared layout and is not scoped to a user. Run scripts/migrate_user_isolation.py to move legacy agents into the per-user layout before updating."),
@@ -170,7 +170,16 @@ def update_agent(
paths = get_paths()
agent_dir = paths.user_agent_dir(user_id, agent_name)
if not agent_dir.exists() and paths.agent_dir(agent_name).exists():
legacy_dir = paths.agent_dir(agent_name)
# Require config.yaml, not bare directory existence — a per-user agent
# directory can exist containing only memory.json (written the first
# time this user chats with a legacy shared agent, before update_agent
# is ever called). Bare .exists() would miss that case and let this
# fall through to load_agent_config, which correctly resolves through
# to the legacy shared config via resolve_agent_dir, silently forking
# a brand-new config.yaml/SOUL.md into the memory-only directory
# instead of blocking (mirrors resolve_agent_dir's guard, see #3390).
if not (agent_dir / "config.yaml").exists() and (legacy_dir / "config.yaml").exists():
return _err(f"Agent '{agent_name}' only exists in the legacy shared layout and is not scoped to a user. Run scripts/migrate_user_isolation.py to move legacy agents into the per-user layout before updating.")
try:
+26
View File
@@ -695,6 +695,32 @@ class TestAgentsAPI:
],
}
def test_update_memory_only_user_dir_with_legacy_agent_returns_409(self, agent_client, tmp_path):
"""Regression for #3390's PUT /api/agents/{name} guard.
A per-user agent directory can exist containing only memory.json
(written the first time this user chats with a legacy shared
agent). The stale guard checked bare directory existence and
missed this case, letting the route silently fork a brand-new
config.yaml/SOUL.md into the memory-only directory instead of
blocking with the migration-script guidance.
"""
legacy_dir = tmp_path / "agents" / "legacy-agent"
legacy_dir.mkdir(parents=True)
(legacy_dir / "config.yaml").write_text("name: legacy-agent\ndescription: legacy\n", encoding="utf-8")
(legacy_dir / "SOUL.md").write_text("legacy soul", encoding="utf-8")
user_agent_dir = tmp_path / "users" / "test-user-autouse" / "agents" / "legacy-agent"
user_agent_dir.mkdir(parents=True)
(user_agent_dir / "memory.json").write_text("{}", encoding="utf-8")
response = agent_client.put("/api/agents/legacy-agent", json={"soul": "should not write"})
assert response.status_code == 409
assert not (user_agent_dir / "config.yaml").exists()
assert not (user_agent_dir / "SOUL.md").exists()
assert (user_agent_dir / "memory.json").exists(), "the user's existing memory must be left untouched"
def test_update_missing_agent_404(self, agent_client):
response = agent_client.put("/api/agents/ghost-agent", json={"soul": "new"})
assert response.status_code == 404
+31
View File
@@ -124,6 +124,37 @@ def test_update_agent_rejects_unknown_agent(tmp_path, patched_paths):
assert not _user_agent_dir(tmp_path, "ghost").exists()
def test_update_agent_rejects_legacy_agent_when_user_dir_has_only_memory(tmp_path, patched_paths):
"""Regression for #3390's update_agent guard.
A per-user agent directory can exist containing only memory.json —
written automatically the first time this user chats with a legacy
shared agent, before update_agent is ever called. The stale guard
checked bare directory existence, so it missed this case, fell
through to load_agent_config (which correctly resolves through to
the legacy shared config via resolve_agent_dir), and then silently
forked a brand-new config.yaml/SOUL.md into the memory-only
directory — splitting the agent for just this user with no warning.
"""
legacy_dir = tmp_path / "agents" / "legacy-agent"
legacy_dir.mkdir(parents=True)
(legacy_dir / "config.yaml").write_text(yaml.safe_dump({"name": "legacy-agent", "description": "legacy"}), encoding="utf-8")
(legacy_dir / "SOUL.md").write_text("legacy soul", encoding="utf-8")
user_agent_dir = _user_agent_dir(tmp_path, "legacy-agent")
user_agent_dir.mkdir(parents=True)
(user_agent_dir / "memory.json").write_text("{}", encoding="utf-8")
result = update_agent.func(runtime=_runtime(agent_name="legacy-agent"), soul="should not write")
msg = result.update["messages"][0]
assert "only exists in the legacy shared layout" in msg.content
assert msg.status == "error"
assert not (user_agent_dir / "config.yaml").exists()
assert not (user_agent_dir / "SOUL.md").exists()
assert (user_agent_dir / "memory.json").exists(), "the user's existing memory must be left untouched"
def test_update_agent_requires_at_least_one_field(tmp_path, patched_paths):
_seed_agent(tmp_path)