chore(compile): tighten link_resolver._resolve_path input guards

[mvanhorn]

Closes #841.

Summary

Adds an early-return guard for empty / whitespace-only input in `_resolve_path` (Minor 1 + Minor 2 from the issue). Previously `_resolve_path("")` and `_resolve_path(" ")` returned the base directory via `Path(base_dir) / ""`, which is semantically wrong even though the existing `.exists()` checks in the call sites masked the bug.

```python
if not path or not path.strip():
return None
```

Tests (Minor 3)

Adds `TestResolvePathInputGuards` to `tests/unit/compilation/test_link_resolver.py` covering each case the issue called out:

• empty string -> `None` (new guard)
• whitespace-only (spaces, tab, newline) -> `None` (new guard)
• embedded NUL byte -> must not crash; the containment check is the caller's job, so the test only locks in "no exception"
• POSIX backslash traversal (`foo\\..\\..\\etc\\passwd`) -> stays a literal segment under `base_dir`
• `file://` URI on POSIX -> treated relative, joined under `base_dir`
• non-existent relative target -> resolves normally (happy path)

Verification

`pytest tests/unit/compilation/test_link_resolver.py` -> 23 / 23 pass (17 existing + 6 new).

[danielmeppiel]

Maintainer rebase + panel verdict (per #1064 FIX-NOW orchestration)

Rebased on origin/main (clean, no functional changes) and ran the apm-review-panel critique locally.

Verdict: READY

Panel findings (7 personas)

• python-architect: surgical early-return guard; idiomatic and minimal
• cli-logging-expert: no CLI/logging surface touched
• devx-ux-expert: internal helper; no user-facing surface
• supply-chain-security-expert: LOW -- removes a latent "empty path resolves to base_dir" bug; backslash/file:// behavior intentionally documented as POSIX-relative per docstring contract
• oss-growth-hacker: internal defensive change; no growth surface
• doc-writer: no docs/CLI/manifest impact; whitespace cleanup OK
• apm-ceo: closes chore(compile): tighten link_resolver._resolve_path input guards #841 cleanly; tests align with Minor 1/2/3 asks; no escalation

Test evidence

uv run --extra dev pytest tests/unit/compilation/test_link_resolver.py tests/test_console.py -x --tb=short  # green
uv run --extra dev pytest tests/unit -x --tb=short -q
=> 6775 passed, 1 warning, 27 subtests passed in 46.31s

Per fidelity matrix: pure path-resolution, tmp_path-backed unit tests are the contracted integration surface.

Merging via squash next.

[danielmeppiel]

Approved per maintainer panel verdict (see prior comment); CI green.

[Copilot]

Pull request overview

Tightens link_resolver._resolve_path input handling so empty / whitespace-only link targets resolve to None instead of incorrectly resolving to base_path, and adds unit tests to lock in the intended edge-case behavior discussed in #841.

Changes:

• Add an early-return guard in _resolve_path for empty / whitespace-only input.
• Add a new TestResolvePathInputGuards test class covering empty/whitespace, NUL bytes, traversal-like strings, file://-like inputs, and a normal relative path case.
• Update test imports to exercise _resolve_path directly.

Show a summary per file

File	Description
src/apm_cli/compilation/link_resolver.py	Adds an early-return guard to prevent empty/whitespace paths from resolving to the base directory.
tests/unit/compilation/test_link_resolver.py	Adds focused unit tests to validate _resolve_path behavior for several problematic inputs.

Copilot's findings

Comments suppressed due to low confidence (3)

tests/unit/compilation/test_link_resolver.py:498

• This is also platform-dependent: on Windows, pathlib may treat file: as a drive component, making the path "absolute" and causing _resolve_path to return it unchanged (so it would not start with base_dir). Please skip/parametrize this test for POSIX-only behavior, or assert the Windows behavior explicitly.

    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))

src/apm_cli/compilation/link_resolver.py:450

• _resolve_path can currently return a Path containing an embedded NUL (e.g., from markdown content). Call sites in this module then call .exists() on that Path without catching ValueError, which will raise "embedded null byte" and crash compilation/validation. Consider rejecting inputs containing \x00 (return None) so downstream .exists() checks stay safe.

    if not path or not path.strip():
        return None
    try:
        if Path(path).is_absolute():
            return Path(path)
        else:
            return base_path / path
    except (OSError, ValueError):

tests/unit/compilation/test_link_resolver.py:492

• These assertions are POSIX-specific (on Windows, backslash is a path separator, so this input will be split into segments and the expected literal-segment behavior will not hold). Please skip/parametrize this test by platform (e.g., only run on non-nt) or adjust expectations for Windows to avoid post-merge failures in the Windows unit-test job.

    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"

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Copilot]

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

[danielmeppiel]

Re-approving after rebase onto post-#999 main (Ruff guardrails) — trivial import-style conflict in test_link_resolver.py resolved cleanly. All 6775 unit tests + targeted link_resolver tests pass on the new HEAD (1ba83d7).