From 515e418eb41c4adfbb9793dd18eb17bc8c0b9fb3 Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Wed, 17 Jun 2026 23:17:30 +0800 Subject: [PATCH] fix: block exact-file allowlist link escapes maintainer edit: compare exact-file allowlist entries using logical paths and require the resolved target to stay at that canonical path, so symlinks or junctions cannot redirect canonical memory files to an external write target. --- nanobot/security/workspace_policy.py | 36 +++++++++++++++++----- tests/security/test_workspace_policy.py | 41 +++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) 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], + )