fix(skill_manage): refuse agent writes to human-authored skills
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
c493639119
commit
e54f42c0eb
1 file changed
+32
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user