diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index 892b4cf8a..bbaf5f188 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -5,7 +5,7 @@ - **Loading**: `load_skills()` recursively scans public, per-user custom, global integration, and legacy custom locations for `SKILL.md`, parses metadata, and reads enabled state from extensions_config.json plus per-user skill state for non-public categories; that directory is a package boundary, so no nested `SKILL.md` is registered as a runtime skill. SkillScan has a deliberately narrower packaging rule: known eval fixtures are permitted as support data, while other nested `SKILL.md` files are reported as package defects. It parses runtime metadata and reads enabled state from extensions_config.json. - **External reload**: `POST /api/skills/reload` is an admin-only, process-local invalidation hook for trusted MinIO/NFS/CSI writes. `SkillStorage` instances do not cache a catalog — `load_skills()` scans on every call — so the route clears all `(app_config, user_id)` entries and the rendered prompt-section LRU, then waits up to the shared refresh timeout for the existing off-loop single-flight refresh. Each invalidation receives a generation-bound result handle; a successful scan atomically replaces the global enabled-skills cache, while a loader-level failure propagates to the HTTP waiter and preserves the last-known-good global cache. Per-user/config scans capture the refresh version and cannot repopulate shared caches if invalidation occurs while they are loading. A timed-out HTTP wait fails generically while the daemon refresh worker continues. Subsequent runs rescan after a successful reload; active runs keep their existing snapshot. Each Uvicorn worker/Kubernetes Pod must be targeted separately. Direct mount writes bypass install/edit validation, SkillScan, and history, so mounted roots are an operator-controlled trust boundary. - **Tool policy**: Agent `allowed-tools` declarations apply dynamically only to slash-activated skills and skills captured in `ThreadState.skill_context` through configured `read_file` loads; passive enabled skills and custom-agent/subagent skill allowlists remain discoverable without clamping the baseline toolset. Subagents render only skill discovery metadata at startup and reuse the same adjacent `SkillActivationMiddleware` + `SkillToolPolicyMiddleware` pair as the lead; their configured `skills` field limits discovery and activation instead of eagerly loading bodies or unioning policies. Slash policy is dominant for its run, preventing subsequently read skills from widening explicit authority; autonomous captured skills use the existing union only when no slash source exists. `tool_search` and `describe_skill` stay available as framework discovery infrastructure, while every discovered or promoted business tool still requires active-policy permission for schema visibility and execution; `task` likewise requires an explicit declaration. Each active model call intentionally reloads the full live registry so enable/disable changes, frontmatter edits, and custom/public name-shadow winners take effect without a stale TTL or unsafe direct-path cache; all tool calls produced by that model step reuse the resulting source-and-path-signed decision. Registry failures and all-invalid active sets fail closed, while stale individual paths are skipped when another valid skill remains. This is best-effort behavioral scoping, not a hard security boundary: alternate loading paths are not captured and bounded autonomous context may evict entries. -- **Sandbox projection**: `skills/projection.py` materializes enabled-only trees at `{base_dir}/skills_view/public` and `{base_dir}/users/{user_id}/skills_view/{custom,legacy,integrations}`. It hardlinks files when possible and falls back to copies across filesystems. Storage writes, archive installs, deletes, and toggles rebuild under a cross-process lock; Gateway boot ensures only the shared public view, while each user view is repaired lazily on first sandbox acquire. Managed integration packages are global, but their projected category is per-user because enabled state is isolated. Rebuilds stage a complete tree and reconcile it with per-file atomic replacement, so unrelated enabled skills remain continuously visible; disable/delete paths remove only the affected package before mutating to preserve fail-closed behavior. User projection rebuilds re-read global enable state from disk instead of the process singleton, so a toggle handled by another Gateway worker is reflected on the next acquire. Gateway public-skill toggles take the public projection lock before the shared `extensions_config_write_lock`, re-read an existing config from disk, persist the full model shape, and rebuild before responding; keep this as one worker-owned critical section so MCP writes cannot interleave and request cancellation cannot release either lock while the worker still runs. The shared public steady-state signature check runs without the global projection lock; stale/error paths take the lock and re-check before rebuilding or clearing. User-scope checks remain serialized per user. Category root inodes remain stable so live bind mounts observe content changes without sandbox recreation. Projection failures clear the affected view before raising. +- **Sandbox projection**: `skills/projection.py` materializes enabled-only trees at `{base_dir}/skills_view/public` and `{base_dir}/users/{user_id}/skills_view/{custom,legacy,integrations}`. It copies files into the view (`_copy_into_view`) so a sandbox write cannot mutate the canonical skill inode; the operational trade-off is an O(total bytes) I/O and per-user storage multiplier across rebuilds, prioritized for write isolation over zero-copy hardlinks. Steady-state freshness checks combine source and view metadata tree digests, so in-sandbox view tampering is detected and repaired on the next acquire. Storage writes, archive installs, deletes, and toggles rebuild under a cross-process lock; Gateway boot ensures only the shared public view, while each user view is repaired lazily on first sandbox acquire. Managed integration packages are global, but their projected category is per-user because enabled state is isolated. Rebuilds stage a complete tree and reconcile it with per-file atomic replacement, so unrelated enabled skills remain continuously visible; disable/delete paths remove only the affected package before mutating to preserve fail-closed behavior. User projection rebuilds re-read global enable state from disk instead of the process singleton, so a toggle handled by another Gateway worker is reflected on the next acquire. Gateway public-skill toggles take the public projection lock before the shared `extensions_config_write_lock`, re-read an existing config from disk, persist the full model shape, and rebuild before responding; keep this as one worker-owned critical section so MCP writes cannot interleave and request cancellation cannot release either lock while the worker still runs. The shared public steady-state signature check runs without the global projection lock; stale/error paths take the lock and re-check before rebuilding or clearing. User-scope checks remain serialized per user. Category root inodes remain stable so live bind mounts observe content changes without sandbox recreation. Projection failures clear the affected view before raising. - **Injection (legacy / default)**: Enabled skills are listed in the agent system prompt with full metadata and container paths (`` block). Controlled by `skills.deferred_discovery: false` (default). - **Deferred discovery** (`skills.deferred_discovery: true`): Skills are listed by name only in a compact `` block, keeping the system prompt prefix-cache friendly. The agent calls the `describe_skill` tool at runtime to fetch full metadata for skills it wants to use, then loads the SKILL.md via `read_file`. Two new modules support this path: - `skills/catalog.py` — `SkillCatalog` (immutable, searchable; query forms: `select:a,b`, `+prefix`, free-text regex); `select:` returns all requested skills without a result cap; other modes cap at `MAX_RESULTS=5`. diff --git a/backend/packages/harness/deerflow/skills/projection.py b/backend/packages/harness/deerflow/skills/projection.py index 9acd2ab02..a9e783dde 100644 --- a/backend/packages/harness/deerflow/skills/projection.py +++ b/backend/packages/harness/deerflow/skills/projection.py @@ -2,7 +2,6 @@ from __future__ import annotations -import errno import hashlib import json import logging @@ -94,15 +93,12 @@ def _projection_lock(root: Path) -> Iterator[None]: msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1) -def _link_or_copy(source: str, target: str, *, follow_symlinks: bool = True) -> str: - # Hardlinks share the source inode and provide no write isolation. Any - # read-only guarantee must come from the consuming sandbox or mount. - try: - os.link(source, target, follow_symlinks=follow_symlinks) - except OSError as exc: - if exc.errno not in {errno.EXDEV, errno.EPERM, errno.EACCES, errno.ENOTSUP}: - raise - shutil.copy2(source, target, follow_symlinks=follow_symlinks) +def _copy_into_view(source: str, target: str, *, follow_symlinks: bool = True) -> str: + # Always copy. Hardlinks share the source inode, so a LocalSandbox bash + # write through the projected view would mutate the canonical skill file. + # Isolation must live in the projection itself; PathMapping.read_only is + # only enforced by write_file / update_file, not execute_command. + shutil.copy2(source, target, follow_symlinks=follow_symlinks) return target @@ -114,7 +110,7 @@ def _stage_skill(source: Path, target: Path, nested_skill_roots: set[Path]) -> N shutil.copytree( source, target, - copy_function=_link_or_copy, + copy_function=_copy_into_view, symlinks=True, ignore=_exclude_nested_skills, dirs_exist_ok=True, @@ -298,20 +294,39 @@ def _read_manifest(scope_root: Path) -> dict | None: return value if isinstance(value, dict) else None -def _write_manifest(scope_root: Path, source_signature: str) -> None: +def _write_manifest(scope_root: Path, source_signature: str, view_signature: str | None = None) -> None: scope_root.mkdir(parents=True, exist_ok=True) target = _manifest_path(scope_root) fd, temporary_name = tempfile.mkstemp(prefix=".projection-manifest-", suffix=".tmp", dir=scope_root) temporary = Path(temporary_name) + payload = { + "version": _MANIFEST_VERSION, + "source_signature": source_signature, + } + if view_signature is not None: + payload["view_signature"] = view_signature try: with os.fdopen(fd, "w", encoding="utf-8") as stream: - json.dump({"version": _MANIFEST_VERSION, "source_signature": source_signature}, stream, sort_keys=True) + json.dump(payload, stream, sort_keys=True) temporary.replace(target) except Exception: temporary.unlink(missing_ok=True) raise +def _view_signature(paths: SkillProjectionPaths, scope: str) -> str: + digest = hashlib.sha256() + if scope == "public": + _update_tree_digest(digest, paths.public, "public_view") + elif scope == "user": + _update_tree_digest(digest, paths.custom, "custom_view") + _update_tree_digest(digest, paths.legacy, "legacy_view") + _update_tree_digest(digest, paths.integrations, "integrations_view") + else: # pragma: no cover - internal invariant + raise ValueError(f"Unknown skill projection scope: {scope}") + return digest.hexdigest() + + def _load_public_skills(storage: SkillStorage, *, enabled_only: bool) -> list[Skill]: from deerflow.config.extensions_config import ExtensionsConfig @@ -363,7 +378,7 @@ def _rebuild_public_locked(storage: SkillStorage, paths: SkillProjectionPaths) - ) after = _source_signature(storage, "public") if before == after: - _write_manifest(scope_root, after) + _write_manifest(scope_root, after, _view_signature(paths, "public")) return raise RuntimeError("Public skills changed repeatedly while rebuilding the sandbox projection") except Exception: @@ -395,7 +410,7 @@ def _rebuild_user_locked(storage: SkillStorage, paths: SkillProjectionPaths) -> ) after = _source_signature(storage, "user") if before == after: - _write_manifest(scope_root, after) + _write_manifest(scope_root, after, _view_signature(paths, "user")) return raise RuntimeError("User skills changed repeatedly while rebuilding the sandbox projection") except Exception: @@ -428,9 +443,10 @@ def _public_projection_is_fresh(storage: SkillStorage, paths: SkillProjectionPat manifest_before = _read_manifest(paths.public.parent) if manifest_before is None or manifest_before.get("version") != _MANIFEST_VERSION: return False - signature = _source_signature(storage, "public") + source_sig = _source_signature(storage, "public") + view_sig = _view_signature(paths, "public") manifest_after = _read_manifest(paths.public.parent) - return manifest_before == manifest_after and manifest_before.get("source_signature") == signature + return manifest_before == manifest_after and manifest_before.get("source_signature") == source_sig and manifest_before.get("view_signature") == view_sig def ensure_skill_projections(storage: SkillStorage) -> SkillProjectionPaths: @@ -456,8 +472,17 @@ def ensure_skill_projections(storage: SkillStorage) -> SkillProjectionPaths: with _projection_lock(paths.custom.parent): try: manifest = _read_manifest(paths.custom.parent) - signature = _source_signature(storage, "user") - if not paths.custom.is_dir() or not paths.legacy.is_dir() or not paths.integrations.is_dir() or manifest is None or manifest.get("version") != _MANIFEST_VERSION or manifest.get("source_signature") != signature: + source_sig = _source_signature(storage, "user") + view_sig = _view_signature(paths, "user") + if ( + not paths.custom.is_dir() + or not paths.legacy.is_dir() + or not paths.integrations.is_dir() + or manifest is None + or manifest.get("version") != _MANIFEST_VERSION + or manifest.get("source_signature") != source_sig + or manifest.get("view_signature") != view_sig + ): _rebuild_user_locked(storage, paths) except Exception: _clear_projection_scope(paths.custom.parent, paths.custom, paths.legacy, paths.integrations) diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py index a0093537e..c41d37bf8 100644 --- a/backend/tests/test_local_sandbox_provider_mounts.py +++ b/backend/tests/test_local_sandbox_provider_mounts.py @@ -153,6 +153,27 @@ class TestReadOnlyPath: sandbox.write_file("/mnt/skills/new_file.py", "content") assert exc_info.value.errno == errno.EROFS + def test_bash_write_to_projected_copy_does_not_mutate_source(self, tmp_path): + source = tmp_path / "canonical" / "SKILL.md" + view = tmp_path / "skills_view" / "public" / "demo" / "SKILL.md" + source.parent.mkdir(parents=True) + view.parent.mkdir(parents=True) + source.write_text("ORIGINAL\n", encoding="utf-8") + from deerflow.skills.projection import _copy_into_view + + _copy_into_view(str(source), str(view)) + assert view.stat().st_ino != source.stat().st_ino + + sandbox = LocalSandbox( + "test", + [ + PathMapping(container_path="/mnt/skills/public/demo", local_path=str(view.parent), read_only=True), + ], + ) + sandbox.execute_command("python -c \"from pathlib import Path; Path(r'/mnt/skills/public/demo/SKILL.md').write_text('MUTATED\\n', encoding='utf-8')\"") + assert source.read_text(encoding="utf-8") == "ORIGINAL\n" + assert view.read_text(encoding="utf-8") == "MUTATED\n" + def test_write_file_allowed_on_writable_mount(self, tmp_path): data_dir = tmp_path / "data" data_dir.mkdir() diff --git a/backend/tests/test_skill_projection.py b/backend/tests/test_skill_projection.py index 211e2ce36..e85f61feb 100644 --- a/backend/tests/test_skill_projection.py +++ b/backend/tests/test_skill_projection.py @@ -2,7 +2,6 @@ from __future__ import annotations -import errno import shutil import zipfile from concurrent.futures import ThreadPoolExecutor @@ -100,20 +99,37 @@ def test_nested_skill_frontmatter_is_supporting_data_inside_parent_package(proje assert (nested_view / "SKILL.md").is_file() -def test_projection_falls_back_to_copy_when_hardlink_is_unavailable(projection_env, monkeypatch) -> None: +def test_projection_copies_instead_of_hardlinking_source_files(projection_env) -> None: env = projection_env source = _write_skill(env.skills_root / "public", "demo-skill") - - def _cross_device_link(*_args, **_kwargs): - raise OSError(errno.EXDEV, "cross-device link") - - monkeypatch.setattr("deerflow.skills.projection.os.link", _cross_device_link) projected = rebuild_skill_projections(env.storage) target = projected.public / "demo-skill" / "SKILL.md" assert target.read_text(encoding="utf-8") == source.read_text(encoding="utf-8") assert target.stat().st_ino != source.stat().st_ino + target.write_text("MUTATED\n", encoding="utf-8") + assert source.read_text(encoding="utf-8") != "MUTATED\n" + + +def test_view_tampering_triggers_automatic_repair_on_ensure(projection_env) -> None: + env = projection_env + source = _write_skill(env.skills_root / "public", "demo-skill") + projected = rebuild_skill_projections(env.storage) + target = projected.public / "demo-skill" / "SKILL.md" + + target.write_text("MUTATED\n", encoding="utf-8") + ensure_skill_projections(env.storage) + assert target.read_text(encoding="utf-8") == source.read_text(encoding="utf-8") + + env.storage.write_custom_skill("demo-user", "SKILL.md", _skill_content("demo-user", "original")) + projected = rebuild_skill_projections(env.storage) + user_target = projected.custom / "demo-user" / "SKILL.md" + + user_target.write_text("MUTATED_USER\n", encoding="utf-8") + ensure_skill_projections(env.storage) + assert user_target.read_text(encoding="utf-8") == _skill_content("demo-user", "original") + def test_atomic_custom_skill_rewrite_refreshes_projection(projection_env) -> None: env = projection_env @@ -551,10 +567,10 @@ def test_boot_factory_failure_cleanup_waits_for_concurrent_public_rebuild(projec cleanup_finished = Event() real_write_manifest = projection_module._write_manifest - def _delayed_write_manifest(scope_root, signature): + def _delayed_write_manifest(*args, **kwargs): before_manifest.set() assert release_rebuild.wait(timeout=5) - real_write_manifest(scope_root, signature) + real_write_manifest(*args, **kwargs) monkeypatch.setattr(projection_module, "_write_manifest", _delayed_write_manifest) monkeypatch.setattr(