Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/apm_cli/compilation/link_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 48 additions & 1 deletion tests/unit/compilation/test_link_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

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


Expand Down Expand Up @@ -455,3 +459,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)
Comment on lines +477 to +486
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test/docstring claims the caller's .exists() check will "reject" a path with an embedded NUL, but in practice Path.exists() raises ValueError for embedded null bytes unless explicitly caught. If the intent is "must not crash overall", it would be better to have _resolve_path return None for \x00 inputs and assert that here, rather than allowing a Path return value.

This issue also appears in the following locations of the same file:

  • line 487
  • line 494
Suggested change
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_embedded_nul_byte_returns_none(self, base_dir):
"""An embedded NUL byte should be rejected by _resolve_path."""
result = _resolve_path("foo\x00bar", base_dir)
assert result is None

Copilot uses AI. Check for mistakes.

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"
Loading