diff --git a/src/apm_cli/compilation/link_resolver.py b/src/apm_cli/compilation/link_resolver.py index 5dd10c5d1..3914c3146 100644 --- a/src/apm_cli/compilation/link_resolver.py +++ b/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) diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 7a10a26c9..afb1932f6 100644 --- a/src/apm_cli/integration/targets.py +++ b/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 / `` is + # ```` 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: diff --git a/tests/unit/compilation/test_link_resolver.py b/tests/unit/compilation/test_link_resolver.py index be97540d4..40ef8f127 100644 --- a/tests/unit/compilation/test_link_resolver.py +++ b/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."""