mirror of
https://gitee.com/wanwujie/deer-flow
synced 2026-04-26 07:14:47 +08:00
fix(harness): allow agent read access to /mnt/skills in local sandbox (#1178)
* fix(harness): allow agent read access to /mnt/skills in local sandbox Skill files under /mnt/skills/ were blocked by the path validator, preventing agents from reading skill definitions. This change: - Refactors `resolve_local_tool_path` into `validate_local_tool_path`, a pure security gate that no longer resolves paths (left to the sandbox) - Permits read-only access to the skills container path (/mnt/skills by default, configurable via config.skills.container_path) - Blocks write access to skills paths (PermissionError) - Allows /mnt/skills in bash command path validation - Adds `LocalSandbox.update_path_mappings` and injects per-thread user-data mappings into the sandbox so all virtual-path resolution is handled uniformly by the sandbox layer - Covers all new behaviour with tests Fixes #1177 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(sandbox): unify all virtual path resolution in tools.py Move skills path resolution from LocalSandbox into tools.py so that all virtual-to-host path translation (user-data and skills) lives in one layer. LocalSandbox becomes a pure execution layer that receives only real host paths — no more path_mappings, _resolve_path, or reverse resolve logic. This addresses architecture feedback that path resolution was split across two layers (tools.py for user-data, LocalSandbox for skills), making the flow hard to follow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(sandbox): address Copilot review — cache-on-success and error path masking - Replace @lru_cache with manual cache-on-success for _get_skills_container_path and _get_skills_host_path so transient failures at startup don't permanently disable skills access. - Add _sanitize_error() helper that masks host filesystem paths in error messages via mask_local_paths_in_output before returning them to the agent. - Apply _sanitize_error() to all catch-all (Exception/OSError) handlers in sandbox tool functions to prevent host path leakage in error output. - Remove unused lru_cache import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
DanielWalnut
@@ -1,137 +1,20 @@
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
from deerflow.sandbox.local.list_dir import list_dir
|
||||
from deerflow.sandbox.sandbox import Sandbox
|
||||
|
||||
|
||||
class LocalSandbox(Sandbox):
|
||||
def __init__(self, id: str, path_mappings: dict[str, str] | None = None):
|
||||
def __init__(self, id: str):
|
||||
"""
|
||||
Initialize local sandbox with optional path mappings.
|
||||
Initialize local sandbox.
|
||||
|
||||
Args:
|
||||
id: Sandbox identifier
|
||||
path_mappings: Dictionary mapping container paths to local paths
|
||||
Example: {"/mnt/skills": "/absolute/path/to/skills"}
|
||||
"""
|
||||
super().__init__(id)
|
||||
self.path_mappings = path_mappings or {}
|
||||
|
||||
def _resolve_path(self, path: str) -> str:
|
||||
"""
|
||||
Resolve container path to actual local path using mappings.
|
||||
|
||||
Args:
|
||||
path: Path that might be a container path
|
||||
|
||||
Returns:
|
||||
Resolved local path
|
||||
"""
|
||||
path_str = str(path)
|
||||
|
||||
# Try each mapping (longest prefix first for more specific matches)
|
||||
for container_path, local_path in sorted(self.path_mappings.items(), key=lambda x: len(x[0]), reverse=True):
|
||||
if path_str.startswith(container_path):
|
||||
# Replace the container path prefix with local path
|
||||
relative = path_str[len(container_path) :].lstrip("/")
|
||||
resolved = str(Path(local_path) / relative) if relative else local_path
|
||||
return resolved
|
||||
|
||||
# No mapping found, return original path
|
||||
return path_str
|
||||
|
||||
def _reverse_resolve_path(self, path: str) -> str:
|
||||
"""
|
||||
Reverse resolve local path back to container path using mappings.
|
||||
|
||||
Args:
|
||||
path: Local path that might need to be mapped to container path
|
||||
|
||||
Returns:
|
||||
Container path if mapping exists, otherwise original path
|
||||
"""
|
||||
path_str = str(Path(path).resolve())
|
||||
|
||||
# Try each mapping (longest local path first for more specific matches)
|
||||
for container_path, local_path in sorted(self.path_mappings.items(), key=lambda x: len(x[1]), reverse=True):
|
||||
local_path_resolved = str(Path(local_path).resolve())
|
||||
if path_str.startswith(local_path_resolved):
|
||||
# Replace the local path prefix with container path
|
||||
relative = path_str[len(local_path_resolved) :].lstrip("/")
|
||||
resolved = f"{container_path}/{relative}" if relative else container_path
|
||||
return resolved
|
||||
|
||||
# No mapping found, return original path
|
||||
return path_str
|
||||
|
||||
def _reverse_resolve_paths_in_output(self, output: str) -> str:
|
||||
"""
|
||||
Reverse resolve local paths back to container paths in output string.
|
||||
|
||||
Args:
|
||||
output: Output string that may contain local paths
|
||||
|
||||
Returns:
|
||||
Output with local paths resolved to container paths
|
||||
"""
|
||||
import re
|
||||
|
||||
# Sort mappings by local path length (longest first) for correct prefix matching
|
||||
sorted_mappings = sorted(self.path_mappings.items(), key=lambda x: len(x[1]), reverse=True)
|
||||
|
||||
if not sorted_mappings:
|
||||
return output
|
||||
|
||||
# Create pattern that matches absolute paths
|
||||
# Match paths like /Users/... or other absolute paths
|
||||
result = output
|
||||
for container_path, local_path in sorted_mappings:
|
||||
local_path_resolved = str(Path(local_path).resolve())
|
||||
# Escape the local path for use in regex
|
||||
escaped_local = re.escape(local_path_resolved)
|
||||
# Match the local path followed by optional path components
|
||||
pattern = re.compile(escaped_local + r"(?:/[^\s\"';&|<>()]*)?")
|
||||
|
||||
def replace_match(match: re.Match) -> str:
|
||||
matched_path = match.group(0)
|
||||
return self._reverse_resolve_path(matched_path)
|
||||
|
||||
result = pattern.sub(replace_match, result)
|
||||
|
||||
return result
|
||||
|
||||
def _resolve_paths_in_command(self, command: str) -> str:
|
||||
"""
|
||||
Resolve container paths to local paths in a command string.
|
||||
|
||||
Args:
|
||||
command: Command string that may contain container paths
|
||||
|
||||
Returns:
|
||||
Command with container paths resolved to local paths
|
||||
"""
|
||||
import re
|
||||
|
||||
# Sort mappings by length (longest first) for correct prefix matching
|
||||
sorted_mappings = sorted(self.path_mappings.items(), key=lambda x: len(x[0]), reverse=True)
|
||||
|
||||
# Build regex pattern to match all container paths
|
||||
# Match container path followed by optional path components
|
||||
if not sorted_mappings:
|
||||
return command
|
||||
|
||||
# Create pattern that matches any of the container paths
|
||||
patterns = [re.escape(container_path) + r"(?:/[^\s\"';&|<>()]*)??" for container_path, _ in sorted_mappings]
|
||||
pattern = re.compile("|".join(f"({p})" for p in patterns))
|
||||
|
||||
def replace_match(match: re.Match) -> str:
|
||||
matched_path = match.group(0)
|
||||
return self._resolve_path(matched_path)
|
||||
|
||||
return pattern.sub(replace_match, command)
|
||||
|
||||
@staticmethod
|
||||
def _get_shell() -> str:
|
||||
@@ -150,11 +33,8 @@ class LocalSandbox(Sandbox):
|
||||
raise RuntimeError("No suitable shell executable found. Tried /bin/zsh, /bin/bash, /bin/sh, and `sh` on PATH.")
|
||||
|
||||
def execute_command(self, command: str) -> str:
|
||||
# Resolve container paths in command before execution
|
||||
resolved_command = self._resolve_paths_in_command(command)
|
||||
|
||||
result = subprocess.run(
|
||||
resolved_command,
|
||||
command,
|
||||
executable=self._get_shell(),
|
||||
shell=True,
|
||||
capture_output=True,
|
||||
@@ -167,46 +47,26 @@ class LocalSandbox(Sandbox):
|
||||
if result.returncode != 0:
|
||||
output += f"\nExit Code: {result.returncode}"
|
||||
|
||||
final_output = output if output else "(no output)"
|
||||
# Reverse resolve local paths back to container paths in output
|
||||
return self._reverse_resolve_paths_in_output(final_output)
|
||||
return output if output else "(no output)"
|
||||
|
||||
def list_dir(self, path: str, max_depth=2) -> list[str]:
|
||||
resolved_path = self._resolve_path(path)
|
||||
entries = list_dir(resolved_path, max_depth)
|
||||
# Reverse resolve local paths back to container paths in output
|
||||
return [self._reverse_resolve_paths_in_output(entry) for entry in entries]
|
||||
return list_dir(path, max_depth)
|
||||
|
||||
def read_file(self, path: str) -> str:
|
||||
resolved_path = self._resolve_path(path)
|
||||
try:
|
||||
with open(resolved_path, encoding="utf-8") as f:
|
||||
return f.read()
|
||||
except OSError as e:
|
||||
# Re-raise with the original path for clearer error messages, hiding internal resolved paths
|
||||
raise type(e)(e.errno, e.strerror, path) from None
|
||||
with open(path, encoding="utf-8") as f:
|
||||
return f.read()
|
||||
|
||||
def write_file(self, path: str, content: str, append: bool = False) -> None:
|
||||
resolved_path = self._resolve_path(path)
|
||||
try:
|
||||
dir_path = os.path.dirname(resolved_path)
|
||||
if dir_path:
|
||||
os.makedirs(dir_path, exist_ok=True)
|
||||
mode = "a" if append else "w"
|
||||
with open(resolved_path, mode, encoding="utf-8") as f:
|
||||
f.write(content)
|
||||
except OSError as e:
|
||||
# Re-raise with the original path for clearer error messages, hiding internal resolved paths
|
||||
raise type(e)(e.errno, e.strerror, path) from None
|
||||
dir_path = os.path.dirname(path)
|
||||
if dir_path:
|
||||
os.makedirs(dir_path, exist_ok=True)
|
||||
mode = "a" if append else "w"
|
||||
with open(path, mode, encoding="utf-8") as f:
|
||||
f.write(content)
|
||||
|
||||
def update_file(self, path: str, content: bytes) -> None:
|
||||
resolved_path = self._resolve_path(path)
|
||||
try:
|
||||
dir_path = os.path.dirname(resolved_path)
|
||||
if dir_path:
|
||||
os.makedirs(dir_path, exist_ok=True)
|
||||
with open(resolved_path, "wb") as f:
|
||||
f.write(content)
|
||||
except OSError as e:
|
||||
# Re-raise with the original path for clearer error messages, hiding internal resolved paths
|
||||
raise type(e)(e.errno, e.strerror, path) from None
|
||||
dir_path = os.path.dirname(path)
|
||||
if dir_path:
|
||||
os.makedirs(dir_path, exist_ok=True)
|
||||
with open(path, "wb") as f:
|
||||
f.write(content)
|
||||
|
||||
Reference in New Issue
Block a user