From 5c23e5d2628e370d3a52380fd65a4b81b392d6d1 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:14:36 +0530 Subject: [PATCH 01/15] fix(security): path containment checks for Windows environments - Fixed an issue in extract_path_candidates where shlex.split(posix=True) would strip backslashes from Windows paths, mangling UNC paths (e.g. \\server\share) before they could be evaluated by _is_windows_absolute. - Fixed a bypass in validate_path where Windows absolute paths bypassed glob expansion and symlink resolution. On Windows, they now fall through to the standard Path logic, allowing glob expansion and strict resolution while still properly checking containment. --- src/path_scope.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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) From 3093bafd702d4ef66cb6f8f43604a1c3718758f9 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:34:33 +0530 Subject: [PATCH 02/15] test: add UNC path test for path extraction and fix windows tests --- tests/test_security_scope.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 59275dda78..4c16773b63 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -107,6 +107,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 +121,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: From 1339a07d4539e2cf7f1fc2cae4f44b642d48071e Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Fri, 28 Aug 2026 16:23:44 +0530 Subject: [PATCH 03/15] test: add regression test for symlink escape using absolute windows path --- tests/test_security_scope.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 4c16773b63..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) From 59f1809211f183431ffaecf1c57c2e560b7e1391 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:23:11 +0530 Subject: [PATCH 04/15] test: support unprivileged Windows runners with NTFS junction fallback and add mocked escape test --- tests/test_security_scope.py | 53 +++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index c5b075d2c0..9d0a678107 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -12,6 +12,29 @@ 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(). + """ + 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,12 +53,7 @@ def test_issue_3007_symlink_escape_is_denied(self) -> None: 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 + _create_directory_link(outside, link) decision = WorkspacePathScope.from_root(workspace).validate_payload('cat linked-outside/secret.txt') @@ -51,19 +69,28 @@ def test_windows_absolute_symlink_escape_is_denied(self) -> None: 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 + _create_directory_link(outside, link) - payload = f'cat {link.resolve()}/secret.txt' + 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_glob_expansion_must_stay_inside_workspace(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) From a36cfd01c5f99b1f16a9c18cb799e73dfd801f31 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sun, 30 Aug 2026 09:18:20 +0530 Subject: [PATCH 05/15] docs(test): document junction UNC limitation and add mocked UNC link escape test --- tests/test_security_scope.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 9d0a678107..862b5f2c6f 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -19,6 +19,10 @@ def _create_directory_link(target: Path, link: Path) -> None: 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) @@ -91,6 +95,20 @@ def test_symlink_resolution_escape_mocked(self) -> None: 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) From e26897e1510c6ce0981248bcc3022868336dd96e Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sun, 30 Aug 2026 09:32:57 +0530 Subject: [PATCH 06/15] style: remove trailing whitespace in path_scope.py --- src/path_scope.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/path_scope.py b/src/path_scope.py index ba84229a79..2a8a4af203 100644 --- a/src/path_scope.py +++ b/src/path_scope.py @@ -65,7 +65,7 @@ def validate_path(self, candidate: str | Path, cwd: str | Path | None = None) -> 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(): From 4bde1eebf12c581e222cc40e83bbe8ee5d56cad5 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sun, 30 Aug 2026 14:08:19 +0530 Subject: [PATCH 07/15] test: fix windows test compatibility --- tests/test_pre_push_hook_contract.py | 13 ++++++++++++ tests/test_roadmap_helpers.py | 31 +++++++++++++++++++++------- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/tests/test_pre_push_hook_contract.py b/tests/test_pre_push_hook_contract.py index b38a5d45ee..7a958a05c6 100644 --- a/tests/test_pre_push_hook_contract.py +++ b/tests/test_pre_push_hook_contract.py @@ -1,5 +1,16 @@ from __future__ import annotations +import unittest +import os + +def require_bash() -> bool: + import shutil + bash = shutil.which('bash') + if os.name == 'nt': + return False + return bash is not None + + import os import subprocess import unittest @@ -11,6 +22,7 @@ class PrePushHookContractTests(unittest.TestCase): + @unittest.skipUnless(require_bash(), 'Requires bash') def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None: env = os.environ.copy() env['SKIP_CLAW_PRE_PUSH_BUILD'] = '1' @@ -28,6 +40,7 @@ def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None: self.assertIn('SKIP_CLAW_PRE_PUSH_BUILD=1', result.stderr) self.assertIn('skipping cargo workspace build', result.stderr) + @unittest.skipUnless(require_bash(), 'Requires bash') def test_default_build_gate_uses_workspace_locked_cargo_build(self) -> None: hook = PRE_PUSH_HOOK.read_text() diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py index 3c8751980b..27031c5d3d 100644 --- a/tests/test_roadmap_helpers.py +++ b/tests/test_roadmap_helpers.py @@ -14,6 +14,17 @@ +import sys + +def require_bash() -> bool: + import os + import shutil + bash = shutil.which('bash') + if os.name == 'nt': + # On Windows, 'bash' often resolves to WSL which fails if not configured + return False + return bash is not None + def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]: return subprocess.run( ['bash', str(script), str(roadmap)], @@ -25,8 +36,9 @@ def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedPr def run_dogfood_probe(args: list[str]) -> subprocess.CompletedProcess[str]: + import sys return subprocess.run( - ['python3', str(DOGFOOD_PROBE), *args], + [sys.executable, str(DOGFOOD_PROBE), *args], cwd=REPO_ROOT, capture_output=True, text=True, @@ -35,6 +47,7 @@ def run_dogfood_probe(args: list[str]) -> subprocess.CompletedProcess[str]: class RoadmapHelperTests(unittest.TestCase): + @unittest.skipUnless(require_bash(), 'Requires bash') def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: roadmap = Path(temp_dir) / 'ROADMAP.md' @@ -46,6 +59,7 @@ def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None self.assertEqual('725\n', result.stdout) self.assertEqual('', result.stderr) + @unittest.skipUnless(require_bash(), 'Requires bash') def test_roadmap_next_id_fails_fast_on_helper_era_duplicate(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: roadmap = Path(temp_dir) / 'ROADMAP.md' @@ -59,6 +73,7 @@ def test_roadmap_next_id_fails_fast_on_helper_era_duplicate(self) -> None: self.assertIn('999', result.stderr) self.assertNotIn('1000', result.stdout) + @unittest.skipUnless(require_bash(), 'Requires bash') def test_roadmap_next_id_fails_when_explicit_roadmap_path_is_missing(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: roadmap = Path(temp_dir) / 'missing-ROADMAP.md' @@ -70,6 +85,7 @@ def test_roadmap_next_id_fails_when_explicit_roadmap_path_is_missing(self) -> No self.assertIn('ROADMAP not found', result.stderr) self.assertIn(str(roadmap), result.stderr) + @unittest.skipUnless(require_bash(), 'Requires bash') def test_roadmap_next_id_fails_closed_when_checker_is_unavailable(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: script_dir = Path(temp_dir) / 'scripts' @@ -100,7 +116,7 @@ def test_dogfood_probe_runs_explicit_argv_and_separates_channels(self) -> None: result = run_dogfood_probe([ '--stdout-json-byte0', '--', - 'python3', + sys.executable, str(fixture), '--output-format', 'json', @@ -112,7 +128,7 @@ def test_dogfood_probe_runs_explicit_argv_and_separates_channels(self) -> None: payload = __import__('json').loads(result.stdout) self.assertEqual('ok', payload['kind']) self.assertEqual([ - 'python3', + sys.executable, str(fixture), '--output-format', 'json', @@ -120,15 +136,16 @@ def test_dogfood_probe_runs_explicit_argv_and_separates_channels(self) -> None: '--help', ], payload['argv']) self.assertEqual(0, payload['returncode']) - self.assertEqual('{"argv": ["--output-format", "json", "doctor", "--help"]}\n', payload['stdout']) - self.assertEqual('diagnostic\n', payload['stderr']) + self.assertEqual('{"argv": ["--output-format", "json", "doctor", "--help"]}\n', payload['stdout'].replace('\r\n', '\n')) + self.assertEqual('diagnostic\n', payload['stderr'].replace('\r\n', '\n')) def test_dogfood_probe_labels_timeout_separately_from_product_error(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: fixture = Path(temp_dir) / 'sleep.py' fixture.write_text('import time\ntime.sleep(2)\n') - result = run_dogfood_probe(['--timeout', '0.1', '--', 'python3', str(fixture)]) + import sys + result = run_dogfood_probe(['--timeout', '0.1', '--', sys.executable, str(fixture)]) self.assertEqual(1, result.returncode) payload = __import__('json').loads(result.stdout) @@ -151,7 +168,7 @@ def test_dogfood_probe_labels_stdout_json_prefix_failure_as_product_error(self) fixture = Path(temp_dir) / 'prefixed.py' fixture.write_text('print("warning before json")\nprint("{}")\n') - result = run_dogfood_probe(['--stdout-json-byte0', '--', 'python3', str(fixture)]) + result = run_dogfood_probe(['--stdout-json-byte0', '--', sys.executable, str(fixture)]) self.assertEqual(1, result.returncode) payload = __import__('json').loads(result.stdout) From 31a037f5d5a2f58e0d5d55733d1f28e2dc97ff60 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:41:39 +0530 Subject: [PATCH 08/15] fix(security): deny unresolvable paths and enable Git bash detection on Windows --- src/path_scope.py | 21 ++++++++++++-- tests/test_pre_push_hook_contract.py | 42 +++++++++++++++++++--------- tests/test_roadmap_helpers.py | 32 +++++++++++++++++---- tests/test_security_scope.py | 13 +++++++++ 4 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/path_scope.py b/src/path_scope.py index 2a8a4af203..be25976049 100644 --- a/src/path_scope.py +++ b/src/path_scope.py @@ -72,7 +72,15 @@ def validate_path(self, candidate: str | Path, cwd: str | Path | None = None) -> path = base / path expanded = self._expand_glob(path) for expanded_path in expanded: - resolved = expanded_path.resolve(strict=False) + try: + resolved = expanded_path.resolve(strict=False) + except (OSError, ValueError, RuntimeError): + return PathScopeDecision( + False, + 'path cannot be resolved or is invalid', + str(candidate), + str(expanded_path), + ) if not any(_is_relative_to(resolved, root) for root in self.roots): return PathScopeDecision( False, @@ -80,7 +88,16 @@ def validate_path(self, candidate: str | Path, cwd: str | Path | None = None) -> str(candidate), str(resolved), ) - return PathScopeDecision(True, 'path is inside workspace scope', str(candidate), str(expanded[0].resolve(strict=False))) + try: + final_resolved = str(expanded[0].resolve(strict=False)) + except (OSError, ValueError, RuntimeError): + return PathScopeDecision( + False, + 'path cannot be resolved or is invalid', + str(candidate), + str(expanded[0]), + ) + return PathScopeDecision(True, 'path is inside workspace scope', str(candidate), final_resolved) def _expand_glob(self, path: Path) -> tuple[Path, ...]: path_text = str(path) diff --git a/tests/test_pre_push_hook_contract.py b/tests/test_pre_push_hook_contract.py index 7a958a05c6..0cc1f2872f 100644 --- a/tests/test_pre_push_hook_contract.py +++ b/tests/test_pre_push_hook_contract.py @@ -1,20 +1,35 @@ -from __future__ import annotations - -import unittest import os +import shutil +import subprocess +import unittest +from pathlib import Path -def require_bash() -> bool: - import shutil - bash = shutil.which('bash') + +def get_bash_executable() -> str | None: if os.name == 'nt': - return False - return bash is not None + for candidate in ( + r'C:\Program Files\Git\bin\bash.exe', + r'C:\Program Files\Git\usr\bin\bash.exe', + r'C:\Program Files (x86)\Git\bin\bash.exe', + os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'), + ): + if os.path.exists(candidate): + return candidate + bash = shutil.which('bash') + if bash and 'WindowsApps' not in bash: + return bash + return None -import os -import subprocess -import unittest -from pathlib import Path +def require_bash() -> bool: + bash = get_bash_executable() + if not bash: + return False + try: + res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) + return res.returncode == 0 + except Exception: + return False REPO_ROOT = Path(__file__).resolve().parents[1] @@ -24,11 +39,12 @@ def require_bash() -> bool: class PrePushHookContractTests(unittest.TestCase): @unittest.skipUnless(require_bash(), 'Requires bash') def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None: + bash_cmd = get_bash_executable() or 'bash' env = os.environ.copy() env['SKIP_CLAW_PRE_PUSH_BUILD'] = '1' result = subprocess.run( - ['bash', str(PRE_PUSH_HOOK)], + [bash_cmd, str(PRE_PUSH_HOOK)], cwd=REPO_ROOT, env=env, check=True, diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py index 27031c5d3d..3b376357be 100644 --- a/tests/test_roadmap_helpers.py +++ b/tests/test_roadmap_helpers.py @@ -16,18 +16,38 @@ import sys -def require_bash() -> bool: +def get_bash_executable() -> str | None: import os - import shutil - bash = shutil.which('bash') if os.name == 'nt': - # On Windows, 'bash' often resolves to WSL which fails if not configured + for candidate in ( + r'C:\Program Files\Git\bin\bash.exe', + r'C:\Program Files\Git\usr\bin\bash.exe', + r'C:\Program Files (x86)\Git\bin\bash.exe', + os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'), + ): + if os.path.exists(candidate): + return candidate + bash = shutil.which('bash') + if bash and 'WindowsApps' not in bash: + return bash + return None + + +def require_bash() -> bool: + bash = get_bash_executable() + if not bash: return False - return bash is not None + try: + res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) + return res.returncode == 0 + except Exception: + return False + def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]: + bash_cmd = get_bash_executable() or 'bash' return subprocess.run( - ['bash', str(script), str(roadmap)], + [bash_cmd, str(script), str(roadmap)], cwd=REPO_ROOT, capture_output=True, text=True, diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 862b5f2c6f..2e4e38fa35 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -109,6 +109,19 @@ def test_symlink_resolving_to_unc_escape_mocked(self) -> None: self.assertFalse(decision.allowed) self.assertIn('outside workspace scope', decision.reason) + def test_unresolvable_path_raises_oserror_is_denied(self) -> None: + """Verify that paths raising OSError during resolve() are explicitly denied.""" + with tempfile.TemporaryDirectory() as tmp: + workspace = Path(tmp) / 'workspace' + workspace.mkdir() + scope = WorkspacePathScope.from_root(workspace) + + from unittest.mock import patch + with patch.object(Path, 'resolve', side_effect=OSError('dangling symlink or filesystem error')): + decision = scope.validate_path(str(workspace / 'broken_link.txt')) + self.assertFalse(decision.allowed) + self.assertIn('cannot be resolved', decision.reason) + def test_glob_expansion_must_stay_inside_workspace(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) From c1f2471810bae293d6f7ce2d9ea170a051a72ff5 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:54:53 +0530 Subject: [PATCH 09/15] perf(test): cache bash probe resolution per session with functools.lru_cache --- tests/test_pre_push_hook_contract.py | 3 +++ tests/test_roadmap_helpers.py | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/test_pre_push_hook_contract.py b/tests/test_pre_push_hook_contract.py index 0cc1f2872f..a9ef026a72 100644 --- a/tests/test_pre_push_hook_contract.py +++ b/tests/test_pre_push_hook_contract.py @@ -1,3 +1,4 @@ +import functools import os import shutil import subprocess @@ -5,6 +6,7 @@ from pathlib import Path +@functools.lru_cache(maxsize=1) def get_bash_executable() -> str | None: if os.name == 'nt': for candidate in ( @@ -21,6 +23,7 @@ def get_bash_executable() -> str | None: return None +@functools.lru_cache(maxsize=1) def require_bash() -> bool: bash = get_bash_executable() if not bash: diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py index 3b376357be..f7307c71a1 100644 --- a/tests/test_roadmap_helpers.py +++ b/tests/test_roadmap_helpers.py @@ -1,7 +1,10 @@ from __future__ import annotations +import functools +import os import shutil import subprocess +import sys import tempfile import unittest from pathlib import Path @@ -12,12 +15,8 @@ DOGFOOD_PROBE = REPO_ROOT / 'scripts' / 'dogfood-probe.py' - - -import sys - +@functools.lru_cache(maxsize=1) def get_bash_executable() -> str | None: - import os if os.name == 'nt': for candidate in ( r'C:\Program Files\Git\bin\bash.exe', @@ -33,6 +32,7 @@ def get_bash_executable() -> str | None: return None +@functools.lru_cache(maxsize=1) def require_bash() -> bool: bash = get_bash_executable() if not bash: From cabcdc0fb05756383da0ead7b2149a7806bb8dec Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Thu, 3 Sep 2026 08:56:45 +0530 Subject: [PATCH 10/15] test: extract bash detection to shared _bash helper Moves get_bash_executable and require_bash into a shared module so their lru_cache state is per-session rather than per-test-module. Also adds a comment regarding monkeypatching since lru_cache will silently drop changes to os.environ if it has already been populated. Closes follow-up items from PR review. --- tests/_bash.py | 36 ++++++++++++++++++++++++++++ tests/test_pre_push_hook_contract.py | 29 +--------------------- tests/test_roadmap_helpers.py | 29 +--------------------- 3 files changed, 38 insertions(+), 56 deletions(-) create mode 100644 tests/_bash.py diff --git a/tests/_bash.py b/tests/_bash.py new file mode 100644 index 0000000000..f480b3306b --- /dev/null +++ b/tests/_bash.py @@ -0,0 +1,36 @@ +import functools +import os +import shutil +import subprocess + +# Warning: get_bash_executable and require_bash use @functools.lru_cache. +# Do not monkeypatch os.environ['PATH'] or shutil.which in tests and expect +# these functions to re-evaluate. If you must patch them, call .cache_clear() +# before and after the test. + +@functools.lru_cache(maxsize=1) +def get_bash_executable() -> str | None: + if os.name == 'nt': + for candidate in ( + r'C:\Program Files\Git\bin\bash.exe', + r'C:\Program Files\Git\usr\bin\bash.exe', + r'C:\Program Files (x86)\Git\bin\bash.exe', + os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'), + ): + if os.path.exists(candidate): + return candidate + bash = shutil.which('bash') + if bash and 'WindowsApps' not in bash: + return bash + return None + +@functools.lru_cache(maxsize=1) +def require_bash() -> bool: + bash = get_bash_executable() + if not bash: + return False + try: + res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) + return res.returncode == 0 + except Exception: + return False diff --git a/tests/test_pre_push_hook_contract.py b/tests/test_pre_push_hook_contract.py index a9ef026a72..a7e96eed91 100644 --- a/tests/test_pre_push_hook_contract.py +++ b/tests/test_pre_push_hook_contract.py @@ -5,34 +5,7 @@ import unittest from pathlib import Path - -@functools.lru_cache(maxsize=1) -def get_bash_executable() -> str | None: - if os.name == 'nt': - for candidate in ( - r'C:\Program Files\Git\bin\bash.exe', - r'C:\Program Files\Git\usr\bin\bash.exe', - r'C:\Program Files (x86)\Git\bin\bash.exe', - os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'), - ): - if os.path.exists(candidate): - return candidate - bash = shutil.which('bash') - if bash and 'WindowsApps' not in bash: - return bash - return None - - -@functools.lru_cache(maxsize=1) -def require_bash() -> bool: - bash = get_bash_executable() - if not bash: - return False - try: - res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) - return res.returncode == 0 - except Exception: - return False +from tests._bash import get_bash_executable, require_bash REPO_ROOT = Path(__file__).resolve().parents[1] diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py index f7307c71a1..8fe5e0e408 100644 --- a/tests/test_roadmap_helpers.py +++ b/tests/test_roadmap_helpers.py @@ -14,34 +14,7 @@ NEXT_ID = REPO_ROOT / 'scripts' / 'roadmap-next-id.sh' DOGFOOD_PROBE = REPO_ROOT / 'scripts' / 'dogfood-probe.py' - -@functools.lru_cache(maxsize=1) -def get_bash_executable() -> str | None: - if os.name == 'nt': - for candidate in ( - r'C:\Program Files\Git\bin\bash.exe', - r'C:\Program Files\Git\usr\bin\bash.exe', - r'C:\Program Files (x86)\Git\bin\bash.exe', - os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'), - ): - if os.path.exists(candidate): - return candidate - bash = shutil.which('bash') - if bash and 'WindowsApps' not in bash: - return bash - return None - - -@functools.lru_cache(maxsize=1) -def require_bash() -> bool: - bash = get_bash_executable() - if not bash: - return False - try: - res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) - return res.returncode == 0 - except Exception: - return False +from tests._bash import get_bash_executable, require_bash def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]: From 1b6a32b09d444cff30c70755b02b3022b81ed146 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:27:37 +0530 Subject: [PATCH 11/15] test: enforce bash lru_cache clearance and expose specific skip reasons Adds a conftest.py pytest fixture to explicitly clear the lru_cache for bash detection, addressing the monkeypatching hazard in a way that doesn't rely solely on comments. (Note: CI currently runs python -m unittest, but this keeps local pytest runners safe.) Also updates the bash check to run the execution probe directly inside the cached _check_bash_state, exposing the difference between 'no bash found' and 'bash present but execution failed' directly to the unittest skip reason. --- tests/_bash.py | 39 +++++++++++++++++----------- tests/conftest.py | 8 ++++++ tests/test_pre_push_hook_contract.py | 6 ++--- tests/test_roadmap_helpers.py | 10 +++---- 4 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 tests/conftest.py diff --git a/tests/_bash.py b/tests/_bash.py index f480b3306b..868f7fa1cc 100644 --- a/tests/_bash.py +++ b/tests/_bash.py @@ -3,13 +3,20 @@ import shutil import subprocess -# Warning: get_bash_executable and require_bash use @functools.lru_cache. +# Warning: _check_bash_state uses @functools.lru_cache. # Do not monkeypatch os.environ['PATH'] or shutil.which in tests and expect # these functions to re-evaluate. If you must patch them, call .cache_clear() -# before and after the test. +# before and after the test, or use the _clear_bash_cache pytest fixture. + +def _probe_bash(bash_path: str) -> bool: + try: + res = subprocess.run([bash_path, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) + return res.returncode == 0 + except Exception: + return False @functools.lru_cache(maxsize=1) -def get_bash_executable() -> str | None: +def _check_bash_state() -> tuple[str | None, str]: if os.name == 'nt': for candidate in ( r'C:\Program Files\Git\bin\bash.exe', @@ -18,19 +25,21 @@ def get_bash_executable() -> str | None: os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'), ): if os.path.exists(candidate): - return candidate + if _probe_bash(candidate): + return candidate, "" + return None, f"bash present at {candidate} but unusable" bash = shutil.which('bash') if bash and 'WindowsApps' not in bash: - return bash - return None + if _probe_bash(bash): + return bash, "" + return None, f"bash present at {bash} but unusable" + return None, "Requires bash" + +def get_bash_executable() -> str | None: + return _check_bash_state()[0] -@functools.lru_cache(maxsize=1) def require_bash() -> bool: - bash = get_bash_executable() - if not bash: - return False - try: - res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2) - return res.returncode == 0 - except Exception: - return False + return get_bash_executable() is not None + +def bash_skip_reason() -> str: + return _check_bash_state()[1] diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000000..666f80160a --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,8 @@ +import pytest +from tests._bash import _check_bash_state + +@pytest.fixture(autouse=True) +def _clear_bash_cache(): + _check_bash_state.cache_clear() + yield + _check_bash_state.cache_clear() diff --git a/tests/test_pre_push_hook_contract.py b/tests/test_pre_push_hook_contract.py index a7e96eed91..4606142ba3 100644 --- a/tests/test_pre_push_hook_contract.py +++ b/tests/test_pre_push_hook_contract.py @@ -5,7 +5,7 @@ import unittest from pathlib import Path -from tests._bash import get_bash_executable, require_bash +from tests._bash import get_bash_executable, require_bash, bash_skip_reason REPO_ROOT = Path(__file__).resolve().parents[1] @@ -13,7 +13,7 @@ class PrePushHookContractTests(unittest.TestCase): - @unittest.skipUnless(require_bash(), 'Requires bash') + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None: bash_cmd = get_bash_executable() or 'bash' env = os.environ.copy() @@ -32,7 +32,7 @@ def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None: self.assertIn('SKIP_CLAW_PRE_PUSH_BUILD=1', result.stderr) self.assertIn('skipping cargo workspace build', result.stderr) - @unittest.skipUnless(require_bash(), 'Requires bash') + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_default_build_gate_uses_workspace_locked_cargo_build(self) -> None: hook = PRE_PUSH_HOOK.read_text() diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py index 8fe5e0e408..58d408420b 100644 --- a/tests/test_roadmap_helpers.py +++ b/tests/test_roadmap_helpers.py @@ -14,7 +14,7 @@ NEXT_ID = REPO_ROOT / 'scripts' / 'roadmap-next-id.sh' DOGFOOD_PROBE = REPO_ROOT / 'scripts' / 'dogfood-probe.py' -from tests._bash import get_bash_executable, require_bash +from tests._bash import get_bash_executable, require_bash, bash_skip_reason def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]: @@ -40,7 +40,7 @@ def run_dogfood_probe(args: list[str]) -> subprocess.CompletedProcess[str]: class RoadmapHelperTests(unittest.TestCase): - @unittest.skipUnless(require_bash(), 'Requires bash') + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: roadmap = Path(temp_dir) / 'ROADMAP.md' @@ -52,7 +52,7 @@ def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None self.assertEqual('725\n', result.stdout) self.assertEqual('', result.stderr) - @unittest.skipUnless(require_bash(), 'Requires bash') + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_roadmap_next_id_fails_fast_on_helper_era_duplicate(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: roadmap = Path(temp_dir) / 'ROADMAP.md' @@ -66,7 +66,7 @@ def test_roadmap_next_id_fails_fast_on_helper_era_duplicate(self) -> None: self.assertIn('999', result.stderr) self.assertNotIn('1000', result.stdout) - @unittest.skipUnless(require_bash(), 'Requires bash') + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_roadmap_next_id_fails_when_explicit_roadmap_path_is_missing(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: roadmap = Path(temp_dir) / 'missing-ROADMAP.md' @@ -78,7 +78,7 @@ def test_roadmap_next_id_fails_when_explicit_roadmap_path_is_missing(self) -> No self.assertIn('ROADMAP not found', result.stderr) self.assertIn(str(roadmap), result.stderr) - @unittest.skipUnless(require_bash(), 'Requires bash') + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_roadmap_next_id_fails_closed_when_checker_is_unavailable(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: script_dir = Path(temp_dir) / 'scripts' From 37e1673f19054e33883457fa9ef0db0425f1ed23 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sat, 5 Sep 2026 12:45:34 +0530 Subject: [PATCH 12/15] test: ensure full candidate probing and clear cache in setUp - Updates _check_bash_state to not short-circuit on the first unusable bash candidate on Windows, ensuring a working fallback is found if present. - Clears bash detection cache in unittest setUp/tearDown rather than just relying on a pytest fixture, covering native unittest runners. --- tests/_bash.py | 8 ++++++-- tests/test_pre_push_hook_contract.py | 10 +++++++++- tests/test_roadmap_helpers.py | 10 +++++++++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/_bash.py b/tests/_bash.py index 868f7fa1cc..a4e56adf24 100644 --- a/tests/_bash.py +++ b/tests/_bash.py @@ -17,6 +17,7 @@ def _probe_bash(bash_path: str) -> bool: @functools.lru_cache(maxsize=1) def _check_bash_state() -> tuple[str | None, str]: + reasons = [] if os.name == 'nt': for candidate in ( r'C:\Program Files\Git\bin\bash.exe', @@ -27,12 +28,15 @@ def _check_bash_state() -> tuple[str | None, str]: if os.path.exists(candidate): if _probe_bash(candidate): return candidate, "" - return None, f"bash present at {candidate} but unusable" + reasons.append(f"present at {candidate} but unusable") bash = shutil.which('bash') if bash and 'WindowsApps' not in bash: if _probe_bash(bash): return bash, "" - return None, f"bash present at {bash} but unusable" + reasons.append(f"present at {bash} but unusable") + + if reasons: + return None, "bash found but broken: " + "; ".join(reasons) return None, "Requires bash" def get_bash_executable() -> str | None: diff --git a/tests/test_pre_push_hook_contract.py b/tests/test_pre_push_hook_contract.py index 4606142ba3..d16f15f3bd 100644 --- a/tests/test_pre_push_hook_contract.py +++ b/tests/test_pre_push_hook_contract.py @@ -5,7 +5,7 @@ import unittest from pathlib import Path -from tests._bash import get_bash_executable, require_bash, bash_skip_reason +from tests._bash import get_bash_executable, require_bash, bash_skip_reason, _check_bash_state REPO_ROOT = Path(__file__).resolve().parents[1] @@ -13,6 +13,14 @@ class PrePushHookContractTests(unittest.TestCase): + def setUp(self) -> None: + _check_bash_state.cache_clear() + super().setUp() + + def tearDown(self) -> None: + _check_bash_state.cache_clear() + super().tearDown() + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None: bash_cmd = get_bash_executable() or 'bash' diff --git a/tests/test_roadmap_helpers.py b/tests/test_roadmap_helpers.py index 58d408420b..55502213b0 100644 --- a/tests/test_roadmap_helpers.py +++ b/tests/test_roadmap_helpers.py @@ -14,7 +14,7 @@ NEXT_ID = REPO_ROOT / 'scripts' / 'roadmap-next-id.sh' DOGFOOD_PROBE = REPO_ROOT / 'scripts' / 'dogfood-probe.py' -from tests._bash import get_bash_executable, require_bash, bash_skip_reason +from tests._bash import get_bash_executable, require_bash, bash_skip_reason, _check_bash_state def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]: @@ -40,6 +40,14 @@ def run_dogfood_probe(args: list[str]) -> subprocess.CompletedProcess[str]: class RoadmapHelperTests(unittest.TestCase): + def setUp(self) -> None: + _check_bash_state.cache_clear() + super().setUp() + + def tearDown(self) -> None: + _check_bash_state.cache_clear() + super().tearDown() + @unittest.skipUnless(require_bash(), bash_skip_reason()) def test_roadmap_next_id_prints_only_next_id_after_duplicate_check(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: From 25ed82a99e43c750e7e3dc1c91b02e79e4ff1c09 Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:06:16 +0530 Subject: [PATCH 13/15] test: add regression coverage for drive-relative and escaped UNC paths Adds explicit test coverage to ensure drive-relative paths (e.g. C:foo) and escaped UNC paths (e.g. \\\\server\\share\\dir) from JSON payloads are correctly validated by the workspace containment checks. Closes final review suggestions. --- tests/test_security_scope.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 2e4e38fa35..3e8a574489 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -214,6 +214,40 @@ def test_windows_absolute_paths_are_denied_for_posix_workspace(self) -> None: self.assertIn('windows absolute path', drive_decision.reason) self.assertIn('windows absolute path', unc_decision.reason) + def test_drive_relative_paths_are_resolved_and_denied_if_cross_drive(self) -> None: + """Verify that drive-relative paths like C:foo behave correctly.""" + with tempfile.TemporaryDirectory() as tmp: + workspace = Path(tmp) / 'workspace' + workspace.mkdir() + scope = WorkspacePathScope.from_root(workspace) + + # A drive-relative path on a DIFFERENT drive acts like an absolute escape + # If tmp is on C:, Z:foo resolves to Z:\foo which is outside. + other_drive = 'Z:' if workspace.drive.upper() != 'Z:' else 'Y:' + decision_cross = scope.validate_path(f'{other_drive}foo') + + # A drive-relative path on the SAME drive acts like a relative path to the CWD + # C:foo in C:\workspace resolves to C:\workspace\foo + decision_same = scope.validate_path(f'{workspace.drive}foo') + + if os.name == 'nt': + self.assertFalse(decision_cross.allowed) + self.assertIn('outside workspace scope', decision_cross.reason) + self.assertTrue(decision_same.allowed) + + def test_escaped_unc_paths_are_extracted_and_denied(self) -> None: + """Verify that UNC paths with double backslashes (e.g. from JSON payloads) are properly identified and denied.""" + with tempfile.TemporaryDirectory() as tmp: + workspace = Path(tmp) / 'workspace' + workspace.mkdir() + + payload = r'type \\\\server\\share\\secret.txt' + candidates = extract_path_candidates(payload) + self.assertIn(r'\\\\server\\share\\secret.txt', candidates) + + decision = WorkspacePathScope.from_root(workspace).validate_payload(payload) + self.assertFalse(decision.allowed) + def test_file_and_shell_tools_use_workspace_scope_context(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) From d8b71697fb706fec79fbc9f1cca1b2fa39ebd73c Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Sun, 6 Sep 2026 10:18:14 +0530 Subject: [PATCH 14/15] test: add end-to-end UNC containment test Replaces the basic extraction test with an end-to-end test that verifies UNC paths are correctly denied when outside the workspace (e.g. local root), but correctly allowed when the workspace root is itself a UNC share. Validates the fix from the PR review. --- tests/test_security_scope.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 3e8a574489..09d4a3d0d0 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -235,18 +235,35 @@ def test_drive_relative_paths_are_resolved_and_denied_if_cross_drive(self) -> No self.assertIn('outside workspace scope', decision_cross.reason) self.assertTrue(decision_same.allowed) - def test_escaped_unc_paths_are_extracted_and_denied(self) -> None: - """Verify that UNC paths with double backslashes (e.g. from JSON payloads) are properly identified and denied.""" + def test_unc_paths_are_evaluated_correctly_inside_and_outside(self) -> None: + """Verify that UNC paths are properly identified and validated for containment.""" with tempfile.TemporaryDirectory() as tmp: - workspace = Path(tmp) / 'workspace' - workspace.mkdir() + local_workspace = Path(tmp) / 'workspace' + local_workspace.mkdir() + # 1. Test extraction of escaped UNC paths (like from JSON payloads) payload = r'type \\\\server\\share\\secret.txt' candidates = extract_path_candidates(payload) self.assertIn(r'\\\\server\\share\\secret.txt', candidates) - decision = WorkspacePathScope.from_root(workspace).validate_payload(payload) - self.assertFalse(decision.allowed) + # 2. Test denial of UNC path when workspace is on a local drive + decision_outside = WorkspacePathScope.from_root(local_workspace).validate_payload(payload) + self.assertFalse(decision_outside.allowed) + + # 3. Test allowance of UNC path when workspace root is itself a UNC path + # We mock resolve() because resolving a fake UNC path might raise OSError or hang + if os.name == 'nt': + from unittest.mock import patch + unc_workspace = Path(r'\\server\share\workspace') + inside_payload = r"type '\\server\share\workspace\secret.txt'" + + def _fake_resolve(self, strict=False): + return self + + with patch.object(Path, 'resolve', autospec=True, side_effect=_fake_resolve): + scope = WorkspacePathScope.from_root(unc_workspace) + decision_inside = scope.validate_payload(inside_payload) + self.assertTrue(decision_inside.allowed) def test_file_and_shell_tools_use_workspace_scope_context(self) -> None: with tempfile.TemporaryDirectory() as tmp: From 87a64b590222a4e13fc021480d88bab84400b98f Mon Sep 17 00:00:00 2001 From: Gracy769 <169199616+Gracy769@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:22:52 +0530 Subject: [PATCH 15/15] test: add explicit coverage for unquoted UNC paths Asserts that an unquoted UNC path like \\server\share\file.txt survives extract_path_candidates unchanged, preventing the regression where shlex.split(posix=True) stripped the backslashes. --- tests/test_security_scope.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_security_scope.py b/tests/test_security_scope.py index 09d4a3d0d0..7276baf8bf 100644 --- a/tests/test_security_scope.py +++ b/tests/test_security_scope.py @@ -242,12 +242,17 @@ def test_unc_paths_are_evaluated_correctly_inside_and_outside(self) -> None: local_workspace.mkdir() # 1. Test extraction of escaped UNC paths (like from JSON payloads) - payload = r'type \\\\server\\share\\secret.txt' - candidates = extract_path_candidates(payload) - self.assertIn(r'\\\\server\\share\\secret.txt', candidates) + payload_escaped = r'type \\\\server\\share\\secret.txt' + candidates_escaped = extract_path_candidates(payload_escaped) + self.assertIn(r'\\\\server\\share\\secret.txt', candidates_escaped) + + # 1b. Test extraction of unquoted UNC paths to ensure they survive shlex.split(posix=True) + payload_unquoted = r'type \\server\share\secret.txt' + candidates_unquoted = extract_path_candidates(payload_unquoted) + self.assertIn(r'\\server\share\secret.txt', candidates_unquoted) # 2. Test denial of UNC path when workspace is on a local drive - decision_outside = WorkspacePathScope.from_root(local_workspace).validate_payload(payload) + decision_outside = WorkspacePathScope.from_root(local_workspace).validate_payload(payload_escaped) self.assertFalse(decision_outside.allowed) # 3. Test allowance of UNC path when workspace root is itself a UNC path