From e17af4b70b3f4d0e06d48cf0799fbeef8e1a9fd8 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Mon, 24 Aug 2026 19:14:36 +0530 Subject: [PATCH 1/7] 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 2477cf246bb434f1dfa7edd7d57ab587986232e6 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Wed, 26 Aug 2026 14:34:33 +0530 Subject: [PATCH 2/7] 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 3ceb78ae526f515254393e426fdce8c13abd9025 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Fri, 28 Aug 2026 16:23:44 +0530 Subject: [PATCH 3/7] 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 38f8cfafb038e2c8facbcfac0f34f8d031a81746 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Sat, 29 Aug 2026 07:23:11 +0530 Subject: [PATCH 4/7] 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 d5efc4b3e45d7ba058fb232ebbec2ade1dd7da31 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Sun, 30 Aug 2026 09:18:20 +0530 Subject: [PATCH 5/7] 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 2b271e1dbcde01fefb386b6c1fa49f71e74b78d2 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Sun, 30 Aug 2026 09:32:57 +0530 Subject: [PATCH 6/7] 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 c3ed6071c36ef47c9f0429573fc9edaf04b8d500 Mon Sep 17 00:00:00 2001 From: Samy6767f Date: Sun, 30 Aug 2026 14:08:19 +0530 Subject: [PATCH 7/7] 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)