fix(security): path containment checks for Windows environments

src/path_scope.py

@@ -60,22 +60,44 @@ 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():
             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,
                     'path resolves outside workspace scope',
                     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)
@@ -116,7 +138,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/_bash.py

@@ -0,0 +1,49 @@
+import functools
+import os
+import shutil
+import subprocess
+
+# 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, 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 _check_bash_state() -> tuple[str | None, str]:
+    reasons = []
+    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):
+                if _probe_bash(candidate):
+                    return candidate, ""
+                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, ""
+        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:
+    return _check_bash_state()[0]
+
+def require_bash() -> bool:
+    return get_bash_executable() is not None
+
+def bash_skip_reason() -> str:
+    return _check_bash_state()[1]

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()

tests/test_pre_push_hook_contract.py

@@ -1,22 +1,34 @@
-from __future__ import annotations
-
+import functools
 import os
+import shutil
 import subprocess
 import unittest
 from pathlib import Path
 
+from tests._bash import get_bash_executable, require_bash, bash_skip_reason, _check_bash_state
+
 
 REPO_ROOT = Path(__file__).resolve().parents[1]
 PRE_PUSH_HOOK = REPO_ROOT / '.github' / 'hooks' / 'pre-push'
 
 
 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'
         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,
@@ -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(), bash_skip_reason())
     def test_default_build_gate_uses_workspace_locked_cargo_build(self) -> None:
         hook = PRE_PUSH_HOOK.read_text()
 

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
@@ -11,12 +14,13 @@
 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, _check_bash_state
 
 
 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,
@@ -25,8 +29,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 +40,15 @@ 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:
             roadmap = Path(temp_dir) / 'ROADMAP.md'
@@ -46,6 +60,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(), 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'
@@ -59,6 +74,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(), 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'
@@ -70,6 +86,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(), 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'
@@ -100,7 +117,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,23 +129,24 @@ 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',
             'doctor',
             '--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 +169,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)