From 1ba83d7629c419c79cf44a5142e8b913ede8ee29 Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Mon, 27 Apr 2026 04:46:38 -0700 Subject: [PATCH 1/2] chore(compile): tighten link_resolver._resolve_path input guards Adds an early-return guard for empty / whitespace-only input. Previously `_resolve_path("")` and `_resolve_path(" ")` returned the base directory via `Path(base_dir) / ""`, which is semantically wrong (an empty link should resolve to nothing) even though existing callers' `.exists()` check masked the bug. Adds a TestResolvePathInputGuards class that locks in the behaviour for: - empty string - whitespace-only (spaces, tab, newline) - embedded NUL byte (must not crash; containment is the caller's job) - POSIX backslash traversal (literal segment, stays under base_dir) - file:// URI on POSIX (treated relative, joined under base_dir) - non-existent relative target (happy path) Closes #841 --- src/apm_cli/compilation/link_resolver.py | 2 + tests/unit/compilation/test_link_resolver.py | 45 +++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/apm_cli/compilation/link_resolver.py b/src/apm_cli/compilation/link_resolver.py index 8b84687c3..5dd10c5d1 100644 --- a/src/apm_cli/compilation/link_resolver.py +++ b/src/apm_cli/compilation/link_resolver.py @@ -431,6 +431,8 @@ def _resolve_path(path: str, base_path: Path) -> Path | None: Returns: Optional[Path]: Resolved path or None if invalid. """ + if not path or not path.strip(): + return None try: if Path(path).is_absolute(): return Path(path) diff --git a/tests/unit/compilation/test_link_resolver.py b/tests/unit/compilation/test_link_resolver.py index 447ac14f7..805d24de1 100644 --- a/tests/unit/compilation/test_link_resolver.py +++ b/tests/unit/compilation/test_link_resolver.py @@ -10,7 +10,7 @@ import pytest -from apm_cli.compilation.link_resolver import LinkResolutionContext, UnifiedLinkResolver +from apm_cli.compilation.link_resolver import LinkResolutionContext, UnifiedLinkResolver, _resolve_path from apm_cli.primitives.models import Context, PrimitiveCollection @@ -455,3 +455,46 @@ def test_memory_context_files(self, resolver, base_dir): # Should be rewritten to actual source location assert ".apm/context/project.memory.md" in result + + +class TestResolvePathInputGuards: + """Containment tests for _resolve_path: empty / whitespace / NUL / traversal.""" + + def test_empty_string_returns_none(self, base_dir): + """Empty link should resolve to None, not the base directory.""" + assert _resolve_path("", base_dir) is None + + def test_whitespace_only_returns_none(self, base_dir): + """Whitespace-only link should resolve to None.""" + assert _resolve_path(" ", base_dir) is None + assert _resolve_path("\t", base_dir) is None + assert _resolve_path("\n", base_dir) is None + + def test_embedded_nul_byte_does_not_crash(self, base_dir): + """An embedded NUL byte must not crash _resolve_path itself. + + The current containment relies on the caller's `.exists()` check to + reject the resulting path -- this lock-in is documented in the issue + ("Current containment code handles these correctly"). + """ + # Either return value is acceptable; what matters is no exception. + result = _resolve_path("foo\x00bar", base_dir) + assert result is None or isinstance(result, Path) + + def test_posix_backslash_traversal_stays_relative(self, base_dir): + """Backslashes are literal characters on POSIX, so the path stays under base_dir.""" + result = _resolve_path("foo\\..\\..\\etc\\passwd", base_dir) + assert result is not None + # The literal backslash filename is interpreted as a single segment under base_dir. + assert result == base_dir / "foo\\..\\..\\etc\\passwd" + + def test_file_uri_on_posix_is_treated_as_relative(self, base_dir): + """`file://...` is not absolute on POSIX, so it joins under base_dir rather than escaping it.""" + result = _resolve_path("file:///etc/passwd", base_dir) + assert result is not None + assert str(result).startswith(str(base_dir)) + + def test_nonexistent_relative_target_resolves_normally(self, base_dir): + """The happy path: a syntactically-valid relative target resolves even if the target file is missing.""" + result = _resolve_path("does/not/exist.md", base_dir) + assert result == base_dir / "does/not/exist.md" From 20ac1eaaeae6dd981044ccaa93ec6db94eeb764f Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 11:56:32 +0200 Subject: [PATCH 2/2] tests: ruff isort import-block fix on link_resolver test file Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/unit/compilation/test_link_resolver.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/compilation/test_link_resolver.py b/tests/unit/compilation/test_link_resolver.py index 805d24de1..be97540d4 100644 --- a/tests/unit/compilation/test_link_resolver.py +++ b/tests/unit/compilation/test_link_resolver.py @@ -10,7 +10,11 @@ import pytest -from apm_cli.compilation.link_resolver import LinkResolutionContext, UnifiedLinkResolver, _resolve_path +from apm_cli.compilation.link_resolver import ( + LinkResolutionContext, + UnifiedLinkResolver, + _resolve_path, +) from apm_cli.primitives.models import Context, PrimitiveCollection