From a008d74e8744fbd9616cc6776bf974bea922e596 Mon Sep 17 00:00:00 2001 From: qorexdevs Date: Fri, 24 Apr 2026 12:43:24 +0500 Subject: [PATCH] fix(policy): resolve project_root before path-traversal check (#886) --- src/apm_cli/policy/discovery.py | 4 ++++ src/apm_cli/utils/path_security.py | 17 +++++++++++++++-- tests/unit/policy/test_discovery.py | 4 +++- tests/unit/policy/test_pr_832_findings.py | 16 ++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/apm_cli/policy/discovery.py b/src/apm_cli/policy/discovery.py index 4ca6455a3..37c02fa3f 100644 --- a/src/apm_cli/policy/discovery.py +++ b/src/apm_cli/policy/discovery.py @@ -1025,6 +1025,10 @@ def _get_cache_dir(project_root: Path) -> Path: project root -- a configuration that, while unusual, would let cache reads/writes escape the project tree. """ + # Resolve early so candidate inherits long-name form on Windows; + # without this, resolve() on a not-yet-existing candidate keeps + # 8.3 short names while the base resolves to long names (#886). + project_root = project_root.resolve() base = project_root / "apm_modules" candidate = base / POLICY_CACHE_DIR # Resolve both ends and assert containment under ``project_root``, diff --git a/src/apm_cli/utils/path_security.py b/src/apm_cli/utils/path_security.py index 98b51b085..eb60d99b4 100644 --- a/src/apm_cli/utils/path_security.py +++ b/src/apm_cli/utils/path_security.py @@ -74,6 +74,19 @@ def validate_path_segments( ) +def _strip_extended_prefix(p: Path) -> Path: + """Strip the ``\\\\?\\`` extended-length prefix that Windows' resolve() may add. + + On Windows, ``Path.resolve()`` can inconsistently add the prefix to + one path but not another, making ``is_relative_to`` fail even when + both paths share the same physical root (#886). + """ + s = str(p) + if s.startswith("\\\\?\\"): + return Path(s[4:]) + return p + + def ensure_path_within(path: Path, base_dir: Path) -> Path: """Resolve *path* and assert it lives inside *base_dir*. @@ -83,8 +96,8 @@ def ensure_path_within(path: Path, base_dir: Path) -> Path: This is intentionally strict: symlinks are resolved so that a link pointing outside the base is caught as well. """ - resolved = path.resolve() - resolved_base = base_dir.resolve() + resolved = _strip_extended_prefix(path.resolve()) + resolved_base = _strip_extended_prefix(base_dir.resolve()) try: if not resolved.is_relative_to(resolved_base): raise PathTraversalError( diff --git a/tests/unit/policy/test_discovery.py b/tests/unit/policy/test_discovery.py index a23136967..0b852c306 100644 --- a/tests/unit/policy/test_discovery.py +++ b/tests/unit/policy/test_discovery.py @@ -234,7 +234,9 @@ def test_cache_key_different_refs(self): def test_get_cache_dir(self): root = Path("/fake/project") - expected = root / "apm_modules" / ".policy-cache" + # _get_cache_dir resolves project_root (#886), compare + # against the resolved form + expected = root.resolve() / "apm_modules" / ".policy-cache" self.assertEqual(_get_cache_dir(root), expected) diff --git a/tests/unit/policy/test_pr_832_findings.py b/tests/unit/policy/test_pr_832_findings.py index 38d1b5c10..0c7076a6c 100644 --- a/tests/unit/policy/test_pr_832_findings.py +++ b/tests/unit/policy/test_pr_832_findings.py @@ -284,6 +284,22 @@ def test_symlinked_apm_modules_outside_project_is_rejected(self, tmp_path): with pytest.raises(PathTraversalError): _get_cache_dir(project) + def test_unresolved_project_root_does_not_raise(self, tmp_path): + # Regression for #886: on Windows, tempfile.mkdtemp() may return + # an 8.3 short-name path (e.g. RUNNER~1). _get_cache_dir must + # resolve project_root before building the candidate path so + # both sides of ensure_path_within use consistent long names. + real = tmp_path / "real-project" + real.mkdir() + try: + link = tmp_path / "indirect" + os.symlink(real, link) + except (OSError, NotImplementedError): + pytest.skip("symlink creation not supported on this platform") + + cache_dir = _get_cache_dir(link) + assert cache_dir.parent.name == "apm_modules" + # ────────────────────────────────────────────────────────────────────── # #8: discover_policy_with_chain has no ``no_policy`` parameter