fix: address Copilot review findings on #1055 and #991 (consolidated)

src/apm_cli/compilation/link_resolver.py

@@ -433,6 +433,13 @@ def _resolve_path(path: str, base_path: Path) -> Path | None:
     """
     if not path or not path.strip():
         return None
+    # NUL bytes survive ``Path()`` construction on POSIX but every downstream
+    # filesystem call (``.exists()``, ``.is_file()``, ``.read_text()``) raises
+    # ``ValueError``. Callers in this module do not catch ``ValueError`` so an
+    # unguarded NUL would abort markdown link resolution / validation. Reject
+    # at the resolver boundary instead.
+    if "\x00" in path:
+        return None
     try:
         if Path(path).is_absolute():
             return Path(path)

src/apm_cli/integration/targets.py

@@ -235,6 +235,17 @@ def for_scope(self, user_scope: bool = False) -> TargetProfile | None:
                     # Keep ``root_dir`` home-relative so cleanup prefix matching holds.
                     new_root = abs_path.relative_to(home).as_posix()
                 except ValueError:
+                    # Fallback: when CLAUDE_CONFIG_DIR points outside $HOME we
+                    # store an absolute path. ``pathlib.Path / <absolute>`` is
+                    # ``<absolute>`` so deploy + cleanup write to the right
+                    # place. Caveat: the lockfile path translator
+                    # (``install/services._deployed_path_entry``) calls
+                    # ``relative_to(project_root)`` and raises ``RuntimeError``
+                    # for out-of-tree paths that are not dynamic-root targets.
+                    # Today this is unreachable because user-scope CLAUDE
+                    # installs do not flow through that translator, but any
+                    # future refactor that lockfiles user-scope deploys must
+                    # treat absolute ``root_dir`` as a dynamic-root case.
                     new_root = str(abs_path)
 
         if self.unsupported_user_primitives:

tests/unit/compilation/test_link_resolver.py

@@ -474,16 +474,20 @@ def test_whitespace_only_returns_none(self, base_dir):
         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").
+    def test_embedded_nul_byte_returns_none(self, base_dir):
+        """An embedded NUL byte must produce ``None``, not a ``Path``.
+
+        NUL bytes survive ``Path()`` construction on POSIX, but every
+        downstream filesystem call (``.exists()``, ``.is_file()``,
+        ``.read_text()``) raises ``ValueError``. Callers in
+        ``link_resolver`` (``resolve_markdown_links`` /
+        ``validate_link_targets``) do not catch ``ValueError``, so
+        returning a ``Path`` here would abort markdown link
+        resolution. The resolver rejects NUL at its boundary instead.
         """
-        # 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)
+        assert _resolve_path("foo\x00bar", base_dir) is None
+        assert _resolve_path("\x00", base_dir) is None
+        assert _resolve_path("a/b\x00c.md", base_dir) is None
 
     def test_posix_backslash_traversal_stays_relative(self, base_dir):
         """Backslashes are literal characters on POSIX, so the path stays under base_dir."""