diff --git a/backend/packages/harness/deerflow/skills/describe.py b/backend/packages/harness/deerflow/skills/describe.py index fe8c1c8b9..4417dbf0e 100644 --- a/backend/packages/harness/deerflow/skills/describe.py +++ b/backend/packages/harness/deerflow/skills/describe.py @@ -133,7 +133,13 @@ def _render_skill_metadata(skills: list, container_base_path: str) -> str: blocks: list[str] = [] for s in skills: mutability = "[custom, editable]" if s.category == SkillCategory.CUSTOM else "[built-in]" - tools_line = ", ".join(s.allowed_tools) if s.allowed_tools else "(all)" + # `()` is an explicit empty allowlist — the policy middleware strips every + # business tool for it — so only an omitted field (`None`) means unrestricted. + # `(all)` describes this skill's own frontmatter, not the enforced union: once + # any loaded skill declares allowed-tools, + # ``allowed_tool_names_for_skills`` (tool_policy.py) gives a `None` skill no + # tools, so a mixed set can render `(all)` while the middleware restricts it. + tools_line = "(all)" if s.allowed_tools is None else (", ".join(s.allowed_tools) or "(none)") location = s.get_container_file_path(container_base_path) # name/description/allowed-tools come from untrusted ``.skill`` frontmatter; # escape so a value cannot forge a framework tag in the describe_skill output. diff --git a/backend/tests/test_skill_describe.py b/backend/tests/test_skill_describe.py index b7bd8176b..f68edbd16 100644 --- a/backend/tests/test_skill_describe.py +++ b/backend/tests/test_skill_describe.py @@ -11,6 +11,7 @@ from deerflow.skills.describe import ( build_skill_search_setup, get_skill_index_prompt_section, ) +from deerflow.skills.tool_policy import allowed_tool_names_for_skills from deerflow.skills.types import Skill, SkillCategory # ── Helpers ──────────────────────────────────────────────────────────────────── @@ -281,3 +282,24 @@ def test_describe_tool_select_uncapped(): content = result.update["messages"][0].content for s in many_skills: assert s.name in content, f"select: truncated — {s.name} missing from result" + + +# ── Explicitly empty allowed-tools ───────────────────────────────────────────── + + +def test_render_explicit_empty_allowed_tools_does_not_claim_all(): + restricted = _make_skill("locked-down", allowed_tools=()) + rendered = _render_skill_metadata([restricted], "/mnt/skills") + assert "Allowed tools: (all)" not in rendered + assert "Allowed tools: (none)" in rendered + + +def test_rendered_allowed_tools_agree_with_skill_tool_policy(): + """`(all)` must mean unrestricted, which is the one state that renders it.""" + omitted = _make_skill("legacy") + assert allowed_tool_names_for_skills([omitted]) is None + assert "Allowed tools: (all)" in _render_skill_metadata([omitted], "/mnt/skills") + + restricted = _make_skill("locked-down", allowed_tools=()) + assert allowed_tool_names_for_skills([restricted]) == set() + assert "Allowed tools: (all)" not in _render_skill_metadata([restricted], "/mnt/skills")