diff --git a/nanobot/security/workspace_policy.py b/nanobot/security/workspace_policy.py index c2d6db2b..6445c6a8 100644 --- a/nanobot/security/workspace_policy.py +++ b/nanobot/security/workspace_policy.py @@ -6,6 +6,7 @@ consistent across tools, but they are not a replacement for an OS sandbox. from __future__ import annotations +import os from pathlib import Path from typing import Iterable @@ -28,6 +29,18 @@ def resolve_path(path: str | Path, workspace: str | Path | None = None, *, stric return candidate.resolve(strict=strict) +def _resolve_logical_path(path: str | Path, workspace: str | Path | None = None) -> Path: + """Return an absolute normalized path without following symlinks.""" + candidate = Path(path).expanduser() + if not candidate.is_absolute() and workspace is not None: + candidate = Path(workspace).expanduser() / candidate + return Path(os.path.abspath(candidate)) + + +def _same_logical_path(left: str | Path, right: str | Path) -> bool: + return os.path.normcase(os.fspath(left)) == os.path.normcase(os.fspath(right)) + + def is_path_within(path: str | Path, root: str | Path) -> bool: """Return True when *path* resolves to *root* or a descendant of *root*.""" try: @@ -44,18 +57,20 @@ def is_path_allowed(path: str | Path, roots: Iterable[str | Path]) -> bool: return any(is_path_within(path, root) for root in roots) -def _is_path_exactly_allowed(path: str | Path, files: Iterable[str | Path]) -> bool: +def _is_path_exactly_allowed( + logical_path: Path, + resolved_path: Path, + files: Iterable[str | Path], +) -> bool: """Return True when *path* resolves exactly to one of the allowed files.""" - try: - resolved_path = Path(path).expanduser().resolve(strict=False) - except (OSError, RuntimeError, TypeError, ValueError): - return False for file in files: try: - resolved_file = Path(file).expanduser().resolve(strict=False) + allowed_file = _resolve_logical_path(file) except (OSError, RuntimeError, TypeError, ValueError): continue - if resolved_path == resolved_file: + if not _same_logical_path(logical_path, allowed_file): + continue + if _same_logical_path(resolved_path, allowed_file): return True return False @@ -87,6 +102,7 @@ def resolve_allowed_path( strict: bool = False, ) -> Path: """Resolve a path and enforce containment in allowed roots when configured.""" + logical = _resolve_logical_path(path, workspace) resolved = resolve_path(path, workspace, strict=False) files = list(extra_allowed_files or []) if allowed_root is None and not files: @@ -96,7 +112,11 @@ def resolve_allowed_path( if allowed_root is not None: roots.append(allowed_root) roots.extend(extra_allowed_roots or []) - if not is_path_allowed(resolved, roots) and not _is_path_exactly_allowed(resolved, files): + if not is_path_allowed(resolved, roots) and not _is_path_exactly_allowed( + logical, + resolved, + files, + ): boundary = Path(allowed_root).expanduser() if allowed_root is not None else "allowed files" raise WorkspaceBoundaryError( f"Path {path} is outside allowed directory {boundary}" diff --git a/tests/security/test_workspace_policy.py b/tests/security/test_workspace_policy.py index ddbfe0ea..9b2853af 100644 --- a/tests/security/test_workspace_policy.py +++ b/tests/security/test_workspace_policy.py @@ -1,5 +1,7 @@ from __future__ import annotations +import os +import subprocess from pathlib import Path import pytest @@ -11,6 +13,24 @@ from nanobot.security.workspace_policy import ( ) +def _make_directory_link(link: Path, target: Path) -> None: + if os.name == "nt": + completed = subprocess.run( + ["cmd", "/c", "mklink", "/J", str(link), str(target)], + capture_output=True, + text=True, + check=False, + ) + if completed.returncode != 0: + pytest.skip(completed.stderr.strip() or completed.stdout.strip()) + return + + try: + link.symlink_to(target, target_is_directory=True) + except OSError as exc: + pytest.skip(f"symlink creation is unavailable: {exc}") + + def test_resolve_allowed_path_accepts_workspace_relative_path(tmp_path: Path) -> None: workspace = tmp_path / "workspace" workspace.mkdir() @@ -91,3 +111,24 @@ def test_resolve_allowed_path_allows_extra_file_only_exactly(tmp_path: Path) -> allowed_root=workspace, extra_allowed_files=[allowed], ) + + +def test_resolve_allowed_path_extra_file_blocks_link_escape(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + workspace.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + outside_target = outside / "MEMORY.md" + outside_target.write_text("secret", encoding="utf-8") + + memory_link = workspace / "memory" + _make_directory_link(memory_link, outside) + logical_allowed = memory_link / "MEMORY.md" + + with pytest.raises(WorkspaceBoundaryError, match="outside allowed directory"): + resolve_allowed_path( + "memory/MEMORY.md", + workspace=workspace, + allowed_root=workspace / "skills", + extra_allowed_files=[logical_allowed], + )