From e54f42c0eb9c377e797d410c0890c95d17ab20ba Mon Sep 17 00:00:00 2001 From: xuhd Date: Tue, 9 Jun 2026 16:00:44 +0800 Subject: [PATCH] fix(skill_manage): refuse agent writes to human-authored skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even after the previous owner check, the agent could still patch/edit a UI-uploaded skill (source=upload) it happened to own. Uploaded skills represent human-reviewed content; letting the agent rewrite them risks silent drift from what the user approved. Add _assert_skill_is_agent_authored which reads usage.json and refuses unless source==agent. Wired into all five write actions (create/edit/patch/delete/write_file/remove_file). Skills with no usage entry default to source=upload, so legacy and adopted-orphan skills are also refused — only skills the agent created via mark_agent_created in the create flow stay editable. Note: this check is independent of skill_store availability (it reads the usage file, not the DB), so it runs even when skill_store is None. Tests that exercise non-create paths against pre-existing skills will need to call mark_agent_created first or mock get_entry. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../deerflow/tools/skill_manage_tool.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/offline-backend-20260512/backend/packages/harness/deerflow/tools/skill_manage_tool.py b/offline-backend-20260512/backend/packages/harness/deerflow/tools/skill_manage_tool.py index 61cd417a..04b6bda6 100644 --- a/offline-backend-20260512/backend/packages/harness/deerflow/tools/skill_manage_tool.py +++ b/offline-backend-20260512/backend/packages/harness/deerflow/tools/skill_manage_tool.py @@ -134,6 +134,33 @@ def _default_agent_store() -> Any | None: return None +async def _assert_skill_is_agent_authored(name: str) -> None: + """Refuse to modify skills authored by a human (UI upload / adopt / legacy). + + Even when ownership lines up, we draw a hard line at content the *human* + curated through the Web UI: uploads, adopted orphan skills, and legacy + skills with no usage entry (which default to ``source='upload'``). + Those represent reviewed content the user has explicitly endorsed — + letting the agent rewrite them risks silent drift away from what was + approved. Only skills marked ``source='agent'`` (set by + ``mark_agent_created`` in the ``create`` flow) remain editable so the + evolution loop can keep refining them. + + Raises ``ValueError`` (mapped to a tool error visible to the LLM) on + refusal. + """ + from deerflow.skills.usage import get_entry + + entry = await _to_thread(get_entry, name) + if entry.get("source") != "agent": + raise ValueError( + f"Refused: skill '{name}' was authored by a human via the Web UI " + f"and cannot be modified by an agent. Ask the owner to edit it " + f"through the Web UI, or have the user duplicate it to a new " + f"agent-owned skill first." + ) + + async def _assert_agent_may_modify(store: SkillStore, name: str, user_id: str) -> dict[str, Any]: """Pre-flight ownership check before an agent-driven write touches a custom skill. @@ -241,6 +268,7 @@ async def _skill_manage_impl( await _to_thread(skill_storage.ensure_custom_skill_is_editable, name) if skill_store is not None: await _assert_agent_may_modify(skill_store, name, user_id) + await _assert_skill_is_agent_authored(name) if content is None: raise ValueError("content is required for edit.") await _to_thread(skill_storage.validate_skill_markdown_content, name, content) @@ -261,6 +289,7 @@ async def _skill_manage_impl( await _to_thread(skill_storage.ensure_custom_skill_is_editable, name) if skill_store is not None: await _assert_agent_may_modify(skill_store, name, user_id) + await _assert_skill_is_agent_authored(name) if find is None or replace is None: raise ValueError("find and replace are required for patch.") skill_file = skill_storage.get_custom_skill_file(name) @@ -291,6 +320,7 @@ async def _skill_manage_impl( record: dict[str, Any] | None = None if skill_store is not None: record = await _assert_agent_may_modify(skill_store, name, user_id) + await _assert_skill_is_agent_authored(name) await _to_thread( skill_storage.delete_custom_skill, @@ -341,6 +371,7 @@ async def _skill_manage_impl( await _to_thread(skill_storage.ensure_custom_skill_is_editable, name) if skill_store is not None: await _assert_agent_may_modify(skill_store, name, user_id) + await _assert_skill_is_agent_authored(name) if path is None or content is None: raise ValueError("path and content are required for write_file.") target = await _to_thread(skill_storage.ensure_safe_support_path, name, path) @@ -360,6 +391,7 @@ async def _skill_manage_impl( await _to_thread(skill_storage.ensure_custom_skill_is_editable, name) if skill_store is not None: await _assert_agent_may_modify(skill_store, name, user_id) + await _assert_skill_is_agent_authored(name) if path is None: raise ValueError("path is required for remove_file.") target = await _to_thread(skill_storage.ensure_safe_support_path, name, path)