deer-flow/backend/tests/test_skills_loader.py
georgelichen 641a4147e7
fix(skills): parse Responses API content blocks in moderation scanner (#4936)
* Fix skill moderation parsing for Responses API content blocks

Normalize LangChain Responses API text blocks before parsing the security moderation decision, while preserving the existing fail-closed behavior for unavailable or invalid moderation results. Add regression coverage for mixed content blocks and document the compatibility boundary.

Constraint: Responses API AIMessage content is list-shaped while Chat Completions content is string-shaped
Rejected: Disable security scanning | would weaken the skill write safety boundary
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep moderation parsing provider-format tolerant without including reasoning or tool blocks in the decision payload
Tested: 27 security scanner tests; ruff check; ruff format check
Not-tested: Live moderation request against the configured external endpoint

* Reuse shared LLM response text normalization

Route skill moderation responses through the existing provider-format normalizer so only text and output_text blocks participate in JSON parsing. Strengthen regression coverage with reasoning and tool blocks that contain misleading text fields.\n\nConstraint: Responses API content is shared across multiple harness consumers\nRejected: Keep a private normalizer | duplicated provider-shape policy diverges and can reintroduce reasoning-block contamination\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Extend the shared normalizer when a new provider content shape is verified; do not add divergent local parsers\nTested: 118 related backend tests; regression test red against the previous parser; Ruff check and format check\nNot-tested: Live GitHub CLA status refresh

* Restore trusted external skill package loading

Skill discovery follows one-level package-directory symlinks, but activation path validation rejected the resolved external path. Restore that compatibility for configured custom-skill category roots while keeping file-level symlinks and deeper escapes blocked. Add regression coverage for local and user-scoped storage plus slash activation, and document the boundary.

Constraint: Existing skill discovery follows directory symlinks and operator-managed external packages must remain loadable

Rejected: Allow arbitrary resolved paths | would weaken the skill path trust boundary

Confidence: high

Scope-risk: moderate

Directive: Keep the final SKILL.md file symlink-free and preserve one-level category-root validation

Tested: 79 targeted skill storage, loader, slash activation, and user-scoped tests passed; Ruff check and format check passed; GitNexus staged change detection reported low risk

Not-tested: Real symlink activation on this Windows host lacks SeCreateSymbolicLinkPrivilege and is skipped

Related: Skill projection copies sources into sandbox-visible views

* Exercise real filesystem symlink boundaries in skill storage tests

Replace global Path.resolve/is_symlink mocks with real directory and file symlinks, preserving the Windows privilege skip. Add regression coverage for deeper custom-root escapes and symlinks under non-custom categories so the one-level allowance remains explicit.

Constraint: Symlink creation requires SeCreateSymbolicLinkPrivilege on some Windows runners
Rejected: Keep global path-method mocks | they validate the mock behavior rather than filesystem semantics
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep security-boundary tests on real filesystem primitives; skip only when the runner lacks symlink privilege
Tested: 76 targeted loader/storage/slash tests; Ruff check; Ruff format check
Not-tested: Windows symlink-enabled execution on this host
Related: #4936

* Pin the actual nested symlink escape boundary

Place the second symlink below a real custom package directory so the test reaches the one-level relative-parent guard instead of returning early on a non-symlink parent. Keep the public-category rejection coverage unchanged.

Constraint: The security boundary depends on both symlink depth and category root
Rejected: Link the outer package directory directly | the parent is not a symlink at validation time, so the depth guard is never evaluated
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep this regression tied to the exact relative_parent.parts depth check
Tested: Targeted storage, loader, and slash suites; GitNexus staged detection
Not-tested: Symlink-enabled execution on this Windows host
Related: #4936

* Make the nested symlink regression reach the depth guard

The test now validates the SKILL.md directly through the nested symlink, so the symlink is the immediate parent and the relative-parent depth check is executed.

Constraint: Windows test execution may skip when symlink privilege is unavailable
Rejected: Keep the extra nested path segment | it bypasses the symlink-depth guard through an early return
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Mutation tests must fail when the depth restriction is removed
Tested: Targeted test (skipped on this Windows host without symlink privilege); Ruff check and format check
Not-tested: Real symlink execution on Windows; Linux CI will exercise the case
Related: #4936

* Keep sandbox projections fresh for linked external skill packages

The storage layer intentionally accepts one-level custom package-directory symlinks, but projection freshness previously hashed only the link inode. Follow the permitted target tree during custom and legacy source-signature scans so edits to SKILL.md, scripts, references, or assets trigger a rebuild before sandbox use.

Constraint: Preserve the existing one-level custom/legacy symlink boundary and do not follow public, integration, nested, or file symlinks

Rejected: Invalidate projections only from /api/skills/reload | sandbox acquisition must also detect edits made directly in external targets

Confidence: high

Scope-risk: narrow

Reversibility: clean

Directive: Keep target-tree traversal limited to the storage paths that explicitly permit external package-directory links

Tested: 77 projection, user-scoped storage, and lifecycle tests passed; Ruff check and format check passed; git diff --check passed; GitNexus staged detection reported low risk

Not-tested: Real external symlink execution on this Windows host without SeCreateSymbolicLinkPrivilege; existing tests skip that platform limitation
2026-08-24 21:27:43 +08:00

139 lines
5.6 KiB
Python

"""Tests for recursive skills loading."""
from pathlib import Path
from types import SimpleNamespace
import pytest
from deerflow.config.skills_config import SkillsConfig
from deerflow.skills.storage import get_or_new_skill_storage
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
def _write_skill(skill_dir: Path, name: str, description: str) -> None:
"""Write a minimal SKILL.md for tests."""
skill_dir.mkdir(parents=True, exist_ok=True)
content = f"---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n"
(skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
def test_get_skills_root_path_points_to_current_project_skills(tmp_path: Path, monkeypatch):
"""get_skills_root_path() should point to the caller project skills directory."""
monkeypatch.delenv("DEER_FLOW_SKILLS_PATH", raising=False)
monkeypatch.delenv("DEER_FLOW_PROJECT_ROOT", raising=False)
monkeypatch.chdir(tmp_path)
(tmp_path / "skills").mkdir()
app_config = SimpleNamespace(skills=SkillsConfig())
path = get_or_new_skill_storage(app_config=app_config).get_skills_root_path()
assert path == tmp_path / "skills"
def test_get_skills_root_path_honors_env_override(tmp_path: Path, monkeypatch):
"""DEER_FLOW_SKILLS_PATH should override the caller project skills directory."""
skills_root = tmp_path / "team-skills"
monkeypatch.setenv("DEER_FLOW_SKILLS_PATH", str(skills_root))
app_config = SimpleNamespace(skills=SkillsConfig())
path = get_or_new_skill_storage(app_config=app_config).get_skills_root_path()
assert path == skills_root
def test_load_skills_discovers_nested_skills_and_sets_container_paths(tmp_path: Path):
"""Nested skills should be discovered recursively with correct container paths."""
skills_root = tmp_path / "skills"
_write_skill(skills_root / "public" / "root-skill", "root-skill", "Root skill")
_write_skill(skills_root / "public" / "parent" / "child-skill", "child-skill", "Child skill")
_write_skill(skills_root / "custom" / "team" / "helper", "team-helper", "Team helper")
skills = get_or_new_skill_storage(skills_path=skills_root).load_skills(enabled_only=False)
by_name = {skill.name: skill for skill in skills}
assert {"root-skill", "child-skill", "team-helper"} <= set(by_name)
root_skill = by_name["root-skill"]
child_skill = by_name["child-skill"]
team_skill = by_name["team-helper"]
assert root_skill.skill_path == "root-skill"
assert root_skill.get_container_file_path() == "/mnt/skills/public/root-skill/SKILL.md"
assert child_skill.skill_path == "parent/child-skill"
assert child_skill.get_container_file_path() == "/mnt/skills/public/parent/child-skill/SKILL.md"
assert team_skill.skill_path == "team/helper"
assert team_skill.get_container_file_path() == "/mnt/skills/custom/team/helper/SKILL.md"
def test_local_storage_accepts_external_custom_skill_directory_symlink(tmp_path: Path):
skills_root = tmp_path / "skills"
external_file = tmp_path / "external-skills" / "external-skill" / "SKILL.md"
external_file.parent.mkdir(parents=True)
external_file.write_text("---\nname: external-skill\ndescription: An external skill\n---\n", encoding="utf-8")
linked_dir = skills_root / "custom" / "external-skill"
linked_file = linked_dir / "SKILL.md"
linked_dir.parent.mkdir(parents=True)
try:
linked_dir.symlink_to(external_file.parent, target_is_directory=True)
except OSError as exc:
if getattr(exc, "winerror", None) == 1314:
pytest.skip("Windows symlink creation requires SeCreateSymbolicLinkPrivilege")
raise
storage = LocalSkillStorage(host_path=str(skills_root))
assert storage.validate_skill_file_path(linked_file) == external_file
def test_load_skills_stops_at_skill_package_boundary(tmp_path: Path):
"""SKILL.md files inside an existing skill package are support data, not skills."""
skills_root = tmp_path / "skills"
_write_skill(skills_root / "public" / "reviewer", "reviewer", "Reviews skills")
_write_skill(
skills_root / "public" / "reviewer" / "evals" / "fixtures" / "injection",
"injection-example",
"Calibration fixture",
)
_write_skill(
skills_root / "public" / "reviewer" / "examples" / "helper",
"nested-example",
"Nested package example",
)
skills = get_or_new_skill_storage(skills_path=skills_root).load_skills(enabled_only=False)
assert {skill.name for skill in skills} == {"reviewer"}
def test_load_skills_skips_hidden_directories(tmp_path: Path):
"""Hidden directories should be excluded from recursive discovery."""
skills_root = tmp_path / "skills"
_write_skill(skills_root / "public" / "visible" / "ok-skill", "ok-skill", "Visible skill")
_write_skill(
skills_root / "public" / "visible" / ".hidden" / "secret-skill",
"secret-skill",
"Hidden skill",
)
skills = get_or_new_skill_storage(skills_path=skills_root).load_skills(enabled_only=False)
names = {skill.name for skill in skills}
assert "ok-skill" in names
assert "secret-skill" not in names
def test_load_skills_prefers_custom_over_public_with_same_name(tmp_path: Path):
skills_root = tmp_path / "skills"
_write_skill(skills_root / "public" / "shared-skill", "shared-skill", "Public version")
_write_skill(skills_root / "custom" / "shared-skill", "shared-skill", "Custom version")
skills = get_or_new_skill_storage(skills_path=skills_root).load_skills(enabled_only=False)
shared = next(skill for skill in skills if skill.name == "shared-skill")
assert shared.category == "custom"
assert shared.description == "Custom version"