fix(security): path containment checks for Windows environments

src/path_scope.py

@@ -60,7 +60,12 @@ def validate_payload(self, payload: str, cwd: str | Path | None = None) -> PathS
     def validate_path(self, candidate: str | Path, cwd: str | Path | None = None) -> PathScopeDecision:
         raw = os.path.expandvars(os.path.expanduser(str(candidate)))
         if _is_windows_absolute(raw):
-            return self._validate_windows_path(raw)
+            if os.name != 'nt':
+                return self._validate_windows_path(raw)
+            elif not any(_is_windows_absolute(str(root)) for root in self.roots):
+                # Even on Windows, deny if no roots are Windows absolute paths (edge case)
+                return PathScopeDecision(False, 'windows absolute path is outside workspace scope', str(candidate), raw)
+
         base = Path(cwd).expanduser().resolve(strict=False) if cwd else self.roots[0]
         path = Path(raw)
         if not path.is_absolute():
@@ -116,7 +121,7 @@ def extract_path_candidates(payload: str) -> tuple[str, ...]:
         tokens = payload.split()
     raw_tokens = payload.split()
     candidates: list[str] = []
-    for token in (*tokens, *raw_tokens):
+    for token in (*raw_tokens, *tokens):
         if not token or token.startswith('-') or _ENV_ASSIGNMENT_RE.match(token):
             continue
         token = _strip_redirection_operator(token)

tests/test_security_scope.py

@@ -12,6 +12,33 @@
 from src.tools import execute_tool
 
 
+def _create_directory_link(target: Path, link: Path) -> None:
+    """Create a directory symlink or fallback to an NTFS junction on Windows.
+
+    Standard Windows user accounts cannot create symbolic links without
+    SeCreateSymbolicLinkPrivilege (Developer Mode / Elevation), but NTFS
+    directory junctions can be created unprivileged and exercise the exact same
+    path resolution logic in Path.resolve().
+
+    Note: NTFS junctions can only target local directory paths and cannot point
+    at UNC/remote targets. Links resolving to remote/UNC paths are covered
+    deterministically via `test_symlink_resolving_to_unc_escape_mocked`.
+    """
+    try:
+        link.symlink_to(target, target_is_directory=True)
+    except OSError as e:
+        if getattr(e, 'winerror', None) == 1314 and os.name == 'nt':
+            try:
+                import _winapi
+                _winapi.CreateJunction(str(target), str(link))
+                return
+            except Exception:
+                pass
+            self_skip_msg = 'Requires filesystem symlink or junction support on Windows runner'
+            raise unittest.SkipTest(self_skip_msg) from e
+        raise
+
+
 class WorkspacePathScopeTests(unittest.TestCase):
     def test_direct_parent_escape_is_denied(self) -> None:
         with tempfile.TemporaryDirectory() as tmp:
@@ -30,13 +57,58 @@ def test_issue_3007_symlink_escape_is_denied(self) -> None:
             outside.mkdir()
             (outside / 'secret.txt').write_text('secret')
             link = workspace / 'linked-outside'
-            link.symlink_to(outside, target_is_directory=True)
+            _create_directory_link(outside, link)
 
             decision = WorkspacePathScope.from_root(workspace).validate_payload('cat linked-outside/secret.txt')
 
             self.assertFalse(decision.allowed)
             self.assertIn(str(outside.resolve()), decision.resolved or '')
 
+    def test_windows_absolute_symlink_escape_is_denied(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            workspace = root / 'workspace'
+            outside = root / 'outside'
+            workspace.mkdir()
+            outside.mkdir()
+            (outside / 'secret.txt').write_text('secret')
+            link = workspace / 'linked-outside'
+            _create_directory_link(outside, link)
+
+            payload = f'cat {link}/secret.txt'
+            decision = WorkspacePathScope.from_root(workspace).validate_payload(payload)
+
+            self.assertFalse(decision.allowed)
+            self.assertIn('outside workspace scope', decision.reason)
+
+    def test_symlink_resolution_escape_mocked(self) -> None:
+        """Verify containment check catches escapes via resolve() even if unprivileged."""
+        with tempfile.TemporaryDirectory() as tmp:
+            workspace = Path(tmp) / 'workspace'
+            workspace.mkdir()
+            scope = WorkspacePathScope.from_root(workspace)
+
+            from unittest.mock import patch
+            fake_target = (Path(tmp) / 'outside' / 'secret.txt').resolve()
+            with patch.object(Path, 'resolve', return_value=fake_target):
+                decision = scope.validate_path(str(workspace / 'fake-link' / 'secret.txt'))
+                self.assertFalse(decision.allowed)
+                self.assertIn('outside workspace scope', decision.reason)
+
+    def test_symlink_resolving_to_unc_escape_mocked(self) -> None:
+        """Verify containment check denies links resolving to remote/UNC targets."""
+        with tempfile.TemporaryDirectory() as tmp:
+            workspace = Path(tmp) / 'workspace'
+            workspace.mkdir()
+            scope = WorkspacePathScope.from_root(workspace)
+
+            from unittest.mock import patch
+            unc_target = Path(r'\\remote-server\share\secret.txt')
+            with patch.object(Path, 'resolve', return_value=unc_target):
+                decision = scope.validate_path(str(workspace / 'net-link' / 'secret.txt'))
+                self.assertFalse(decision.allowed)
+                self.assertIn('outside workspace scope', decision.reason)
+
     def test_glob_expansion_must_stay_inside_workspace(self) -> None:
         with tempfile.TemporaryDirectory() as tmp:
             root = Path(tmp)
@@ -107,6 +179,11 @@ def test_explicit_worktree_roots_are_allowed(self) -> None:
 
             self.assertTrue(decision.allowed, decision.reason)
 
+    def test_extract_path_candidates_preserves_unc_paths(self) -> None:
+        payload = r'type \\server\share\secret.txt'
+        candidates = extract_path_candidates(payload)
+        self.assertIn(r'\\server\share\secret.txt', candidates)
+
     def test_windows_absolute_paths_are_denied_for_posix_workspace(self) -> None:
         with tempfile.TemporaryDirectory() as tmp:
             workspace = Path(tmp) / 'workspace'
@@ -116,9 +193,13 @@ def test_windows_absolute_paths_are_denied_for_posix_workspace(self) -> None:
             unc_decision = WorkspacePathScope.from_root(workspace).validate_payload(r'type \\server\share\secret.txt')
 
             self.assertFalse(drive_decision.allowed)
-            self.assertIn('windows absolute path', drive_decision.reason)
             self.assertFalse(unc_decision.allowed)
-            self.assertIn('windows absolute path', unc_decision.reason)
+            if os.name == 'nt':
+                self.assertIn('outside workspace scope', drive_decision.reason)
+                self.assertIn('outside workspace scope', unc_decision.reason)
+            else:
+                self.assertIn('windows absolute path', drive_decision.reason)
+                self.assertIn('windows absolute path', unc_decision.reason)
 
     def test_file_and_shell_tools_use_workspace_scope_context(self) -> None:
         with tempfile.TemporaryDirectory() as tmp: