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.
This commit is contained in:
@@ -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}"
|
||||
|
||||
@@ -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],
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user