Skip to content

Commit fac4d07

Browse files
committed
fix(security): deny unresolvable paths and enable Git bash detection on Windows
1 parent c3ed607 commit fac4d07

4 files changed

Lines changed: 87 additions & 21 deletions

File tree

src/path_scope.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,32 @@ def validate_path(self, candidate: str | Path, cwd: str | Path | None = None) ->
7272
path = base / path
7373
expanded = self._expand_glob(path)
7474
for expanded_path in expanded:
75-
resolved = expanded_path.resolve(strict=False)
75+
try:
76+
resolved = expanded_path.resolve(strict=False)
77+
except (OSError, ValueError, RuntimeError):
78+
return PathScopeDecision(
79+
False,
80+
'path cannot be resolved or is invalid',
81+
str(candidate),
82+
str(expanded_path),
83+
)
7684
if not any(_is_relative_to(resolved, root) for root in self.roots):
7785
return PathScopeDecision(
7886
False,
7987
'path resolves outside workspace scope',
8088
str(candidate),
8189
str(resolved),
8290
)
83-
return PathScopeDecision(True, 'path is inside workspace scope', str(candidate), str(expanded[0].resolve(strict=False)))
91+
try:
92+
final_resolved = str(expanded[0].resolve(strict=False))
93+
except (OSError, ValueError, RuntimeError):
94+
return PathScopeDecision(
95+
False,
96+
'path cannot be resolved or is invalid',
97+
str(candidate),
98+
str(expanded[0]),
99+
)
100+
return PathScopeDecision(True, 'path is inside workspace scope', str(candidate), final_resolved)
84101

85102
def _expand_glob(self, path: Path) -> tuple[Path, ...]:
86103
path_text = str(path)

tests/test_pre_push_hook_contract.py

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,35 @@
1-
from __future__ import annotations
2-
3-
import unittest
41
import os
2+
import shutil
3+
import subprocess
4+
import unittest
5+
from pathlib import Path
56

6-
def require_bash() -> bool:
7-
import shutil
8-
bash = shutil.which('bash')
7+
8+
def get_bash_executable() -> str | None:
99
if os.name == 'nt':
10-
return False
11-
return bash is not None
10+
for candidate in (
11+
r'C:\Program Files\Git\bin\bash.exe',
12+
r'C:\Program Files\Git\usr\bin\bash.exe',
13+
r'C:\Program Files (x86)\Git\bin\bash.exe',
14+
os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'),
15+
):
16+
if os.path.exists(candidate):
17+
return candidate
18+
bash = shutil.which('bash')
19+
if bash and 'WindowsApps' not in bash:
20+
return bash
21+
return None
1222

1323

14-
import os
15-
import subprocess
16-
import unittest
17-
from pathlib import Path
24+
def require_bash() -> bool:
25+
bash = get_bash_executable()
26+
if not bash:
27+
return False
28+
try:
29+
res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2)
30+
return res.returncode == 0
31+
except Exception:
32+
return False
1833

1934

2035
REPO_ROOT = Path(__file__).resolve().parents[1]
@@ -24,11 +39,12 @@ def require_bash() -> bool:
2439
class PrePushHookContractTests(unittest.TestCase):
2540
@unittest.skipUnless(require_bash(), 'Requires bash')
2641
def test_skip_escape_hatch_exits_successfully_with_stderr_notice(self) -> None:
42+
bash_cmd = get_bash_executable() or 'bash'
2743
env = os.environ.copy()
2844
env['SKIP_CLAW_PRE_PUSH_BUILD'] = '1'
2945

3046
result = subprocess.run(
31-
['bash', str(PRE_PUSH_HOOK)],
47+
[bash_cmd, str(PRE_PUSH_HOOK)],
3248
cwd=REPO_ROOT,
3349
env=env,
3450
check=True,

tests/test_roadmap_helpers.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,38 @@
1616

1717
import sys
1818

19-
def require_bash() -> bool:
19+
def get_bash_executable() -> str | None:
2020
import os
21-
import shutil
22-
bash = shutil.which('bash')
2321
if os.name == 'nt':
24-
# On Windows, 'bash' often resolves to WSL which fails if not configured
22+
for candidate in (
23+
r'C:\Program Files\Git\bin\bash.exe',
24+
r'C:\Program Files\Git\usr\bin\bash.exe',
25+
r'C:\Program Files (x86)\Git\bin\bash.exe',
26+
os.path.expandvars(r'%LOCALAPPDATA%\Programs\Git\bin\bash.exe'),
27+
):
28+
if os.path.exists(candidate):
29+
return candidate
30+
bash = shutil.which('bash')
31+
if bash and 'WindowsApps' not in bash:
32+
return bash
33+
return None
34+
35+
36+
def require_bash() -> bool:
37+
bash = get_bash_executable()
38+
if not bash:
2539
return False
26-
return bash is not None
40+
try:
41+
res = subprocess.run([bash, '-c', 'echo 1'], capture_output=True, text=True, timeout=2)
42+
return res.returncode == 0
43+
except Exception:
44+
return False
45+
2746

2847
def run_next_id(roadmap: Path, script: Path = NEXT_ID) -> subprocess.CompletedProcess[str]:
48+
bash_cmd = get_bash_executable() or 'bash'
2949
return subprocess.run(
30-
['bash', str(script), str(roadmap)],
50+
[bash_cmd, str(script), str(roadmap)],
3151
cwd=REPO_ROOT,
3252
capture_output=True,
3353
text=True,

tests/test_security_scope.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,19 @@ def test_symlink_resolving_to_unc_escape_mocked(self) -> None:
109109
self.assertFalse(decision.allowed)
110110
self.assertIn('outside workspace scope', decision.reason)
111111

112+
def test_unresolvable_path_raises_oserror_is_denied(self) -> None:
113+
"""Verify that paths raising OSError during resolve() are explicitly denied."""
114+
with tempfile.TemporaryDirectory() as tmp:
115+
workspace = Path(tmp) / 'workspace'
116+
workspace.mkdir()
117+
scope = WorkspacePathScope.from_root(workspace)
118+
119+
from unittest.mock import patch
120+
with patch.object(Path, 'resolve', side_effect=OSError('dangling symlink or filesystem error')):
121+
decision = scope.validate_path(str(workspace / 'broken_link.txt'))
122+
self.assertFalse(decision.allowed)
123+
self.assertIn('cannot be resolved', decision.reason)
124+
112125
def test_glob_expansion_must_stay_inside_workspace(self) -> None:
113126
with tempfile.TemporaryDirectory() as tmp:
114127
root = Path(tmp)

0 commit comments

Comments
 (0)