fix(sandbox): keep skill reads on provider mappings (#4792)

README.md

@@ -740,7 +740,7 @@ An enabled skill's `allowed-tools` policy applies only after that skill is expli
 
 When you install `.skill` archives through the Gateway, DeerFlow accepts standard optional frontmatter metadata such as `version`, `author`, and `compatibility` instead of rejecting otherwise valid external skills.
 
-Disabling a skill also removes it from the sandbox filesystem view, so shell commands and structured file tools follow the same enabled state. Local, Docker/AIO, hostPath provisioner, and newly created E2B sandboxes source `/mnt/skills` from enabled-only projections that update when public, custom, legacy, or managed integration skills are toggled, edited, created, deleted, or installed. Managed integration packages remain shared, while their projected filesystem visibility follows each user's enabled state. Multi-worker Gateways re-read on-disk enable state while rebuilding user projections, so a toggle handled by one worker is honored by another worker's next sandbox acquire. Existing E2B sandboxes retain their creation-time snapshot until they are recreated. PVC-backed provisioner skills keep their configured PVC snapshot/layout for now; dynamic PVC materialization is tracked separately.
+Disabling a skill also removes it from the sandbox filesystem view, so shell commands and structured file tools follow the same enabled state. Local, Docker/AIO, hostPath provisioner, and newly created E2B sandboxes source `/mnt/skills` from enabled-only projections that update when public, custom, legacy, or managed integration skills are toggled, edited, created, deleted, or installed. Structured `read_file` calls (including line ranges and read-before-write checks) use the sandbox provider's mount mapping, so the user identity captured when the sandbox was acquired remains authoritative. Managed integration packages remain shared, while their projected filesystem visibility follows each user's enabled state. Multi-worker Gateways re-read on-disk enable state while rebuilding user projections, so a toggle handled by one worker is honored by another worker's next sandbox acquire. Existing E2B sandboxes retain their creation-time snapshot until they are recreated. PVC-backed provisioner skills keep their configured PVC snapshot/layout for now; dynamic PVC materialization is tracked separately.
 
 Managed integrations install shared read-only skill packs without mixing them
 into custom skills. The Lark/Feishu CLI integration is available under

backend/AGENTS.md

@@ -825,7 +825,7 @@ that cannot tell sibling branches apart.
 **Virtual Path System**:
 - Agent sees: `/mnt/user-data/{workspace,uploads,outputs}`, `/mnt/skills`
 - Physical: `backend/.deer-flow/users/{user_id}/threads/{thread_id}/user-data/...`; raw skills stay under `deer-flow/skills/` and managed integration storage, while sandboxes read `backend/.deer-flow/skills_view/public/` and `backend/.deer-flow/users/{user_id}/skills_view/{custom,legacy,integrations}/`
-- Translation: `LocalSandboxProvider` builds per-thread `PathMapping`s for the user-data prefixes at acquire time; `tools.py` keeps `replace_virtual_path()` / `replace_virtual_paths_in_command()` as a defense-in-depth layer (and for path validation). AIO has the directories volume-mounted at the same virtual paths inside its container, so both implementations accept `/mnt/user-data/...` natively.
+- Translation: `LocalSandboxProvider` builds per-thread `PathMapping`s at acquire time. Sandbox-backed readers resolve only `/mnt/user-data/...` in the tool layer; skills, ACP workspaces, and configured custom mounts stay virtual so the provider mount table remains the single source of acquire-time identity and visibility. Full reads, ranged reads, and read-before-write hashing share this path. `tools.py` keeps `replace_virtual_path()` / `replace_virtual_paths_in_command()` as a defense-in-depth layer (and for path validation). AIO has the directories volume-mounted at the same virtual paths inside its container, so both implementations accept `/mnt/user-data/...` natively.
 - Detection: `is_local_sandbox()` accepts both `sandbox_id == "local"` (legacy / no-thread) and `sandbox_id.startswith("local:")` (per-thread)
 
 **Sandbox Tools** (in `packages/harness/deerflow/sandbox/tools.py`):

backend/packages/harness/deerflow/sandbox/tools.py

@@ -570,14 +570,12 @@ def _resolve_max_results(name: str, requested: int, *, default: int, upper_bound
     return min(requested_max_results, configured_max_results)
 
 
-def _resolve_local_read_path(path: str, thread_data: ThreadDataState) -> str:
+def _resolve_local_read_path(path: str, thread_data: ThreadDataState | None) -> str:
     validate_local_tool_path(path, thread_data, read_only=True)
-    if _is_skills_path(path) or _is_acp_workspace_path(path):
-        # Skills and ACP workspace paths are resolved by the sandbox's
-        # PathMapping (which uses the user_id from acquire time), not
-        # by _resolve_skills_path / _resolve_acp_workspace_path (which
-        # use get_effective_user_id() from contextvar and may differ
-        # from the sandbox mapping's user_id).
+    if _is_skills_path(path) or _is_acp_workspace_path(path) or _is_custom_mount_path(path):
+        # Mounted paths are resolved by the sandbox's PathMapping (which uses
+        # acquire-time identity and provider state), not by tool-layer host
+        # path reconstruction.
         return path
     return _resolve_and_validate_user_data_path(path, thread_data)
 
@@ -859,8 +857,8 @@ def validate_local_tool_path(path: str, thread_data: ThreadDataState | None, *,
 
     This function is a security gate — it checks whether *path* may be
     accessed and raises on violation.  It does **not** resolve the virtual
-    path to a host path; callers are responsible for resolution via
-    ``resolve_and_validate_user_data_path`` or ``_resolve_skills_path``.
+    path to a host path. Sandbox-backed readers should keep mounted paths
+    virtual and let the provider's mount table resolve them.
 
     Allowed virtual-path families:
       - ``/mnt/user-data/*``  — always allowed (read + write)
@@ -2102,6 +2100,23 @@ async def _grep_tool_async(
 grep_tool.coroutine = _grep_tool_async
 
 
+def _read_file_from_sandbox(
+    runtime: Runtime | None,
+    path: str,
+    *,
+    start_line: int | None = None,
+    end_line: int | None = None,
+) -> str:
+    """Read through the sandbox while preserving provider-owned mount paths."""
+    sandbox = ensure_sandbox_initialized(runtime)
+    ensure_thread_directories_exist(runtime)
+    if is_local_sandbox(runtime):
+        path = _resolve_local_read_path(path, get_thread_data(runtime))
+    if start_line is None and end_line is None:
+        return sandbox.read_file(path)
+    return sandbox.read_file(path, start_line=start_line, end_line=end_line)
+
+
 def read_current_file_content(runtime: Runtime | None, path: str) -> str:
     """Read the full current content of ``path`` using read_file's resolution rules.
 
@@ -2110,19 +2125,7 @@ def read_current_file_content(runtime: Runtime | None, path: str) -> str:
     ``FileNotFoundError`` when the file does not exist; other sandbox errors
     propagate to the caller.
     """
-    sandbox = ensure_sandbox_initialized(runtime)
-    ensure_thread_directories_exist(runtime)
-    if is_local_sandbox(runtime):
-        thread_data = get_thread_data(runtime)
-        validate_local_tool_path(path, thread_data, read_only=True)
-        if _is_skills_path(path):
-            path = _resolve_skills_path(path)
-        elif _is_acp_workspace_path(path):
-            path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
-        elif not _is_custom_mount_path(path):
-            path = _resolve_and_validate_user_data_path(path, thread_data)
-        # Custom mount paths are resolved by LocalSandbox._resolve_path()
-    return sandbox.read_file(path)
+    return _read_file_from_sandbox(runtime, path)
 
 
 @tool("read_file", parse_docstring=True)
@@ -2155,21 +2158,9 @@ def read_file_tool(
             return "(start_line > end_line — no lines in range)"
 
         requested_path = path
-        sandbox = ensure_sandbox_initialized(runtime)
-        ensure_thread_directories_exist(runtime)
         use_line_range = start_line is not None or end_line is not None
         if use_line_range:
-            if is_local_sandbox(runtime):
-                thread_data = get_thread_data(runtime)
-                validate_local_tool_path(path, thread_data, read_only=True)
-                if _is_skills_path(path):
-                    path = _resolve_skills_path(path)
-                elif _is_acp_workspace_path(path):
-                    path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
-                elif not _is_custom_mount_path(path):
-                    path = _resolve_and_validate_user_data_path(path, thread_data)
-                # Custom mount paths are resolved by LocalSandbox._resolve_path()
-            content = sandbox.read_file(path, start_line=start_line, end_line=end_line)
+            content = _read_file_from_sandbox(runtime, path, start_line=start_line, end_line=end_line)
         else:
             content = read_current_file_content(runtime, path)
         if not content:

backend/tests/test_read_file_tool_binary.py

@@ -11,7 +11,7 @@ pin the actionable error contract and guard the normal text path.
 from pathlib import Path
 from types import SimpleNamespace
 
-from deerflow.sandbox.local.local_sandbox import LocalSandbox
+from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping
 from deerflow.sandbox.tools import read_file_tool
 
 
@@ -65,6 +65,38 @@ def test_read_file_tool_text_file_unaffected(tmp_path, monkeypatch) -> None:
     assert "binary" not in result.lower(), result
 
 
+def test_read_file_tool_keeps_custom_mount_path_provider_owned(tmp_path, monkeypatch) -> None:
+    from deerflow.config.sandbox_config import VolumeMountConfig
+
+    runtime = _local_runtime(tmp_path)
+    mounted = tmp_path / "mounted-code"
+    mounted.mkdir()
+    (mounted / "notes.txt").write_text("first\nsecond\nthird", encoding="utf-8")
+    sandbox = LocalSandbox(
+        "local:t1",
+        path_mappings=[PathMapping(container_path="/mnt/code-read", local_path=str(mounted), read_only=True)],
+    )
+    mount = VolumeMountConfig(host_path=str(mounted), container_path="/mnt/code-read", read_only=True)
+    monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda _runtime: sandbox)
+    monkeypatch.setattr("deerflow.sandbox.tools._get_custom_mounts", lambda: [mount])
+
+    full = read_file_tool.func(
+        runtime=runtime,
+        description="read mounted file",
+        path="/mnt/code-read/notes.txt",
+    )
+    ranged = read_file_tool.func(
+        runtime=runtime,
+        description="read mounted line",
+        path="/mnt/code-read/notes.txt",
+        start_line=2,
+        end_line=2,
+    )
+
+    assert full == "first\nsecond\nthird"
+    assert ranged == "second"
+
+
 def test_read_file_tool_passes_line_range_into_sandbox(monkeypatch) -> None:
     captured: dict[str, int | str | None] = {}
 

backend/tests/test_three_way_skills_mount_e2e.py

@@ -20,7 +20,9 @@ from deerflow.config.extensions_config import ExtensionsConfig, SkillStateConfig
 from deerflow.config.paths import Paths
 from deerflow.sandbox.local.local_sandbox import PathMapping
 from deerflow.sandbox.local.local_sandbox_provider import LocalSandboxProvider
+from deerflow.sandbox.tools import read_file_tool
 from deerflow.skills.projection import rebuild_skill_projections
+from deerflow.skills.storage import reset_user_skill_storage
 from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
 from deerflow.skills.types import SKILL_MD_FILE, Skill, SkillCategory
 
@@ -67,6 +69,7 @@ def skills_fs(tmp_path: Path) -> dict:
     legacy = root / "custom"
     users_dir = tmp_path / "users"
     user_custom = users_dir / "user-1" / "skills" / "custom"
+    integrations = tmp_path / "integrations" / "skills" / "demo-provider"
 
     return {
         "root": root,
@@ -77,6 +80,7 @@ def skills_fs(tmp_path: Path) -> dict:
         "pub_skill": _write_skill(pub, "pub-skill", "public skill"),
         "legacy_skill": _write_skill(legacy, "leg-skill", "legacy skill"),
         "user_skill": _write_skill(user_custom, "usr-skill", "user custom skill"),
+        "integration_skill": _write_skill(integrations, "int-skill", "integration skill"),
     }
 
 
@@ -190,6 +194,77 @@ class TestThreeWayMountEndToEnd:
         sandbox = provider.get(sid)
         assert "leg-skill" in sandbox.read_file("/mnt/skills/legacy/leg-skill/SKILL.md")
 
+    def test_read_file_tool_uses_enabled_projection_for_every_skill_category(self, skills_fs, monkeypatch):
+        """Exercise the model-visible tool through real provider mappings.
+
+        The runtime identity deliberately differs from the ambient ContextVar.
+        Acquire-time ``PathMapping`` must remain authoritative for both full and
+        ranged reads; the tool layer must never reconstruct a raw host path.
+        """
+        from deerflow.runtime.user_context import reset_current_user, set_current_user
+
+        cfg = _build_config(skills_fs["root"])
+        paths = Paths(base_dir=skills_fs["users_dir"].parent)
+        extensions = ExtensionsConfig()
+        reset_user_skill_storage()
+
+        with (
+            patch("deerflow.config.get_app_config", return_value=cfg),
+            patch("deerflow.config.paths.get_paths", return_value=paths),
+            patch("deerflow.config.extensions_config.ExtensionsConfig.from_file", return_value=extensions),
+            patch("deerflow.config.extensions_config.get_extensions_config", return_value=extensions),
+        ):
+            provider = LocalSandboxProvider()
+            sandbox_ids = {
+                "user-1": provider.acquire("thread-user", user_id="user-1"),
+                "noob": provider.acquire("thread-noob", user_id="noob"),
+            }
+
+            def _sandbox_for(runtime):
+                return provider.get(runtime.state["sandbox"]["sandbox_id"])
+
+            def _must_not_pre_resolve(_path: str) -> str:
+                raise AssertionError("skill paths must stay virtual until the sandbox provider")
+
+            monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", _sandbox_for)
+            monkeypatch.setattr("deerflow.sandbox.tools._resolve_skills_path", _must_not_pre_resolve)
+
+            token = set_current_user(SimpleNamespace(id="wrong-context-user"))
+            try:
+                cases = [
+                    ("user-1", "thread-user", "/mnt/skills/public/pub-skill/SKILL.md", "pub-skill"),
+                    ("user-1", "thread-user", "/mnt/skills/custom/usr-skill/SKILL.md", "usr-skill"),
+                    ("noob", "thread-noob", "/mnt/skills/legacy/leg-skill/SKILL.md", "leg-skill"),
+                    ("user-1", "thread-user", "/mnt/skills/integrations/demo-provider/int-skill/SKILL.md", "int-skill"),
+                ]
+                for user_id, thread_id, virtual_path, skill_name in cases:
+                    runtime = SimpleNamespace(
+                        state={
+                            "sandbox": {"sandbox_id": sandbox_ids[user_id]},
+                            "thread_data": {
+                                "workspace_path": str(paths.sandbox_work_dir(thread_id, user_id=user_id)),
+                                "uploads_path": str(paths.sandbox_uploads_dir(thread_id, user_id=user_id)),
+                                "outputs_path": str(paths.sandbox_outputs_dir(thread_id, user_id=user_id)),
+                            },
+                        },
+                        context={"thread_id": thread_id, "user_id": user_id},
+                    )
+
+                    full = read_file_tool.func(runtime=runtime, description="read skill", path=virtual_path)
+                    ranged = read_file_tool.func(
+                        runtime=runtime,
+                        description="read skill name",
+                        path=virtual_path,
+                        start_line=2,
+                        end_line=2,
+                    )
+
+                    assert f"# {skill_name}" in full
+                    assert ranged == f"name: {skill_name}"
+            finally:
+                reset_current_user(token)
+                reset_user_skill_storage()
+
     # ── Full pipeline: registry → container path → sandbox read ────────
 
     def test_registry_to_sandbox_full_pipeline(self, skills_fs):
@@ -244,7 +319,7 @@ class TestThreeWayMountEndToEnd:
         assert cp == "/mnt/skills/legacy/leg-skill/SKILL.md"
         assert "leg-skill" in sandbox_noob.read_file(cp)
 
-    def test_local_bash_observes_toggle_without_sandbox_recreation(self, tmp_path):
+    def test_local_tools_observe_toggle_without_sandbox_recreation(self, tmp_path, monkeypatch):
         skills_root = tmp_path / "skills"
         _write_skill(skills_root / "public", "secret-skill", "SECRET_PROCEDURE")
         paths = Paths(base_dir=tmp_path)
@@ -263,20 +338,42 @@ class TestThreeWayMountEndToEnd:
             sandbox_id = provider.acquire("thread-1", user_id="user-1")
             sandbox = provider.get(sandbox_id)
             assert sandbox is not None
+            runtime = SimpleNamespace(
+                state={
+                    "sandbox": {"sandbox_id": sandbox_id},
+                    "thread_data": {
+                        "workspace_path": str(paths.sandbox_work_dir("thread-1", user_id="user-1")),
+                        "uploads_path": str(paths.sandbox_uploads_dir("thread-1", user_id="user-1")),
+                        "outputs_path": str(paths.sandbox_outputs_dir("thread-1", user_id="user-1")),
+                    },
+                },
+                context={"thread_id": "thread-1", "user_id": "user-1"},
+            )
+            monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda _runtime: sandbox)
+            virtual_path = "/mnt/skills/public/secret-skill/SKILL.md"
 
-            disabled = sandbox.execute_command("cat /mnt/skills/public/secret-skill/SKILL.md")
+            disabled = sandbox.execute_command(f"cat {virtual_path}")
             assert "SECRET_PROCEDURE" not in disabled
+            structured_disabled = read_file_tool.func(runtime=runtime, description="read disabled skill", path=virtual_path)
+            assert "SECRET_PROCEDURE" not in structured_disabled
+            assert "disabled" in structured_disabled.lower()
+            assert not (paths.public_skills_view_dir / "secret-skill").exists()
+            assert (skills_root / "public" / "secret-skill" / "SKILL.md").is_file()
 
             extensions.skills["secret-skill"] = SkillStateConfig(enabled=True)
             rebuild_skill_projections(storage)
-            enabled = sandbox.execute_command("cat /mnt/skills/public/secret-skill/SKILL.md")
+            enabled = sandbox.execute_command(f"cat {virtual_path}")
             assert "SECRET_PROCEDURE" in enabled
+            assert "SECRET_PROCEDURE" in read_file_tool.func(runtime=runtime, description="read enabled skill", path=virtual_path)
             assert provider.acquire("thread-1", user_id="user-1") == sandbox_id
 
             extensions.skills["secret-skill"] = SkillStateConfig(enabled=False)
             rebuild_skill_projections(storage)
-            disabled_again = sandbox.execute_command("cat /mnt/skills/public/secret-skill/SKILL.md")
+            disabled_again = sandbox.execute_command(f"cat {virtual_path}")
             assert "SECRET_PROCEDURE" not in disabled_again
+            structured_disabled_again = read_file_tool.func(runtime=runtime, description="read disabled skill", path=virtual_path)
+            assert "SECRET_PROCEDURE" not in structured_disabled_again
+            assert "disabled" in structured_disabled_again.lower()
 
     # ── AioSandboxProvider ──────────────────────────────────────────────