diff --git a/src/path_scope.py b/src/path_scope.py index 31828a6a9f..ba84229a79 100644 --- a/src/path_scope.py +++ b/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) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 59275dda78..c5b075d2c0 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -30,13 +30,40 @@ 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) + try: + link.symlink_to(outside, target_is_directory=True) + except OSError as e: + if getattr(e, 'winerror', None) == 1314: + self.skipTest('Requires symlink privileges on Windows') + raise 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' + try: + link.symlink_to(outside, target_is_directory=True) + except OSError as e: + if getattr(e, 'winerror', None) == 1314: + self.skipTest('Requires symlink privileges on Windows') + raise + + payload = f'cat {link.resolve()}/secret.txt' + decision = WorkspacePathScope.from_root(workspace).validate_payload(payload) + + 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 +134,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 +148,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: