From fc027a4fe7cbd8513ccc818095642726d6c5e255 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 20:44:40 +0200 Subject: [PATCH 1/3] fix(install): support external Claude config root Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/install/deployed_paths.py | 18 ++++-- src/apm_cli/integration/base_integrator.py | 57 +++++++++++++++++-- src/apm_cli/integration/targets.py | 10 +--- tests/integration/test_compile_global.py | 31 ++++++++++ tests/unit/install/test_services.py | 17 ++++++ .../unit/integration/test_base_integrator.py | 43 ++++++++++++++ 6 files changed, 159 insertions(+), 17 deletions(-) diff --git a/src/apm_cli/install/deployed_paths.py b/src/apm_cli/install/deployed_paths.py index 11dfbd686..f9f62222f 100644 --- a/src/apm_cli/install/deployed_paths.py +++ b/src/apm_cli/install/deployed_paths.py @@ -5,6 +5,9 @@ from pathlib import Path from typing import Any +from apm_cli.utils.path_security import ensure_path_within +from apm_cli.utils.paths import portable_relpath + def deployed_path_entry( target_path: Path, @@ -15,20 +18,27 @@ def deployed_path_entry( def _try_dynamic_root(tgts, *, strict: bool = False) -> str | None: for _t in tgts: - if _t.resolved_deploy_root is None: + deploy_root = _t.resolved_deploy_root + absolute_static_root = deploy_root is None and Path(_t.root_dir).is_absolute() + if absolute_static_root: + deploy_root = Path(_t.root_dir) + if deploy_root is None: continue - if not strict: + if not strict or absolute_static_root: try: - target_path.relative_to(_t.resolved_deploy_root) + target_path.relative_to(deploy_root) except ValueError: continue + if absolute_static_root: + resolved_target = ensure_path_within(target_path, deploy_root) + return portable_relpath(resolved_target, project_root) if _t.name == "copilot-app": from apm_cli.integration.copilot_app_db import to_lockfile_uri return to_lockfile_uri(target_path.name) from apm_cli.integration.copilot_cowork_paths import to_lockfile_path - return to_lockfile_path(target_path, _t.resolved_deploy_root) + return to_lockfile_path(target_path, deploy_root) return None if targets: diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 73bee4d44..5dc3d5919 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -5,6 +5,7 @@ import re from dataclasses import dataclass, field from pathlib import Path +from typing import Any from apm_cli.compilation.link_resolver import UnifiedLinkResolver from apm_cli.primitives.discovery import discover_primitives @@ -12,6 +13,39 @@ from apm_cli.utils.console import _rich_warning +def _managed_absolute_target_root(candidate: Path, targets: Any) -> Path | None: + """Return the configured static target root governing an absolute path.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + source = targets + if source is None: + source = [KNOWN_TARGETS[name].for_scope(user_scope=True) for name in ("claude", "hermes")] + try: + resolved = candidate.resolve() + for target_profile in source: + if target_profile is None: + continue + deploy_root = target_profile.resolved_deploy_root + if deploy_root is None and Path(target_profile.root_dir).is_absolute(): + deploy_root = Path(target_profile.root_dir) + if deploy_root is None: + continue + resolved_root = deploy_root.resolve() + for mapping in target_profile.primitives.values(): + if not mapping.subdir: + continue + primitive_root = (resolved_root / mapping.subdir).resolve() + if resolved != primitive_root and resolved.is_relative_to(primitive_root): + return resolved_root + if target_profile.hooks_config_display: + hooks_file = resolved_root / Path(target_profile.hooks_config_display).name + if resolved == hooks_file.resolve(): + return resolved_root + except (ValueError, OSError): + return None + return None + + class _SymlinkRaceError(OSError): """Raised by ``_read_bytes_no_follow`` when the path becomes a symlink between the pre-check and the open(). Caught locally; never bubbles.""" @@ -411,16 +445,21 @@ def validate_deploy_path( Checks: 1. No path-traversal components (``..``) 2. Starts with an allowed integration prefix - 3. Resolves within *project_root* (or within the cowork root - for ``cowork://`` paths) + 3. Resolves within *project_root*, a configured absolute target root, + or the cowork root for ``cowork://`` paths """ from apm_cli.integration.copilot_cowork_paths import COWORK_URI_SCHEME - if allowed_prefixes is None: - allowed_prefixes = BaseIntegrator._get_integration_prefixes(targets=targets) if ".." in rel_path: return False + candidate = Path(rel_path) + if candidate.is_absolute(): + return _managed_absolute_target_root(candidate, targets) is not None + + if allowed_prefixes is None: + allowed_prefixes = BaseIntegrator._get_integration_prefixes(targets=targets) + # --- cowork:// paths: validate against cowork root --- if rel_path.startswith(COWORK_URI_SCHEME): if not rel_path.startswith(allowed_prefixes): @@ -615,8 +654,16 @@ def cleanup_empty_parents( # Collect unique parents (skip stop_at itself) candidates: set = set() for p in deleted_paths: + cleanup_boundary = stop_resolved + try: + if not p.resolve().is_relative_to(stop_resolved): + cleanup_boundary = _managed_absolute_target_root(p, targets=None) + if cleanup_boundary is None: + cleanup_boundary = p.parent.resolve() + except (ValueError, OSError): + cleanup_boundary = p.parent.resolve() parent = p.parent - while parent != stop_at and parent.resolve() != stop_resolved: + while parent not in (parent.parent, stop_at) and parent.resolve() != cleanup_boundary: candidates.add(parent) parent = parent.parent # Sort deepest-first for safe bottom-up removal diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 5889d1469..e027f6b29 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -407,14 +407,8 @@ def for_scope(self, user_scope: bool = False) -> TargetProfile | None: # 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. + # place. The lockfile path translator treats an absolute + # ``root_dir`` as a dynamic root. new_root = str(abs_path) if self.unsupported_user_primitives: diff --git a/tests/integration/test_compile_global.py b/tests/integration/test_compile_global.py index 52519a5fe..7e6ac5a0c 100644 --- a/tests/integration/test_compile_global.py +++ b/tests/integration/test_compile_global.py @@ -42,6 +42,37 @@ def test_install_global_hints_compile_and_writes_no_root_file(tmp_path, monkeypa assert "apm compile -g" in result.output +def test_install_global_supports_claude_config_dir_outside_home(tmp_path, monkeypatch): + """Global Claude installs support an absolute config root outside HOME.""" + from apm_cli.commands.install import install as install_cmd + from apm_cli.primitives.discovery import clear_discovery_cache + + home = tmp_path / "home" + package_dir = tmp_path / "global-package" + instruction_dir = package_dir / ".apm" / "instructions" + instruction_dir.mkdir(parents=True) + (package_dir / "apm.yml").write_text( + "name: global-package\nversion: 0.1.0\n", + encoding="utf-8", + ) + (instruction_dir / "global.instructions.md").write_text( + "---\ndescription: Global install instructions\n---\nUse integration tests.\n", + encoding="utf-8", + ) + + claude_config = tmp_path / "outside-home" / "claude-config" + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(claude_config)) + clear_discovery_cache() + + result = CliRunner().invoke(install_cmd, ["-g", str(package_dir), "--target", "claude"]) + + clear_discovery_cache() + assert result.exit_code == 0, result.output + assert (claude_config / "rules" / "global.md").is_file() + assert "[x]" not in result.output + + def test_compile_global_writes_claude_md_from_real_fixtures(tmp_path, monkeypatch): """Run apm compile --global through real discovery and filesystem writes.""" from apm_cli.commands.compile.cli import compile as compile_cmd diff --git a/tests/unit/install/test_services.py b/tests/unit/install/test_services.py index 21cdd958f..7b5cb7f95 100644 --- a/tests/unit/install/test_services.py +++ b/tests/unit/install/test_services.py @@ -155,6 +155,23 @@ def test_runtime_error_when_no_matching_target(self, tmp_path: Path) -> None: with pytest.raises(RuntimeError, match="This is a bug"): _deployed_path_entry(target_path, project_root, targets=[]) + def test_absolute_static_root_rejects_symlink_escape(self, tmp_path: Path) -> None: + from apm_cli.utils.path_security import PathTraversalError + + project_root = tmp_path / "home" + project_root.mkdir() + config_root = tmp_path / "claude-config" + config_root.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + (config_root / "rules").symlink_to(outside, target_is_directory=True) + target_path = config_root / "rules" / "managed.md" + target_path.write_text("managed\n", encoding="utf-8") + claude_target = replace(KNOWN_TARGETS["claude"], root_dir=str(config_root)) + + with pytest.raises(PathTraversalError): + _deployed_path_entry(target_path, project_root, targets=[claude_target]) + def test_path_traversal_error_propagates_from_cowork_translation(self, tmp_path: Path) -> None: """PathTraversalError from to_lockfile_path must propagate, never be swallowed.""" from apm_cli.utils.path_security import PathTraversalError diff --git a/tests/unit/integration/test_base_integrator.py b/tests/unit/integration/test_base_integrator.py index 73f1f1ed3..ceb61b8b9 100644 --- a/tests/unit/integration/test_base_integrator.py +++ b/tests/unit/integration/test_base_integrator.py @@ -190,6 +190,23 @@ def test_valid_github_prompt_path(self): def test_valid_claude_rules_path(self): assert BaseIntegrator.validate_deploy_path(".claude/rules/foo.mdc", self.root) is True + def test_absolute_claude_config_path_is_valid_only_under_config_root( + self, tmp_path, monkeypatch + ): + home = tmp_path / "home" + config_root = tmp_path / "outside-home" / "claude" + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(config_root)) + + assert BaseIntegrator.validate_deploy_path(str(config_root / "rules" / "managed.md"), home) + assert not BaseIntegrator.validate_deploy_path(str(config_root), home) + assert not BaseIntegrator.validate_deploy_path( + str(config_root / "unmanaged" / "other.md"), home + ) + assert not BaseIntegrator.validate_deploy_path( + str(tmp_path / "unmanaged" / "rules" / "other.md"), home + ) + def test_traversal_rejected(self): assert BaseIntegrator.validate_deploy_path("../evil.md", self.root) is False @@ -398,6 +415,32 @@ def test_multiple_deleted_paths(self): assert not dir1.exists() assert not dir2.exists() + def test_external_claude_cleanup_stops_at_config_root(self, tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + config_root = tmp_path / "outside-home" / "claude" + rules_dir = config_root / "rules" + rules_dir.mkdir(parents=True) + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(config_root)) + + original_resolve = Path.resolve + resolve_calls = 0 + + def bounded_resolve(path, *args, **kwargs): + nonlocal resolve_calls + resolve_calls += 1 + if resolve_calls > 50: + raise RuntimeError("parent walk escaped its cleanup boundary") + return original_resolve(path, *args, **kwargs) + + monkeypatch.setattr(Path, "resolve", bounded_resolve) + BaseIntegrator.cleanup_empty_parents([rules_dir / "deleted.md"], home) + + assert not rules_dir.exists() + assert config_root.exists() + assert config_root.parent.exists() + # --------------------------------------------------------------------------- # sync_remove_files From 5c7aa9385f3b8c2b34437cbce0c2c147aa42d499 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 21:50:29 +0200 Subject: [PATCH 2/3] fix(install): fold external Claude root review Centralize managed deployment-root derivation, keep absolute path validation on the shared containment guard, add direct serialization coverage, and record the fix in the changelog. Addresses panel follow-ups from PR #2135. apm-spec-waiver: harness-install reliability fix for Claude CLAUDE_CONFIG_DIR outside HOME; no OpenAPM artifact or wire behavior change Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 ++ src/apm_cli/install/deployed_paths.py | 6 ++---- src/apm_cli/integration/base_integrator.py | 21 +++++++++++++++------ src/apm_cli/integration/targets.py | 13 +++++++++---- tests/unit/install/test_services.py | 14 ++++++++++++++ 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 674411a76..26c1ae951 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm install --target intellij` now configures JetBrains Copilot MCP support while routing package file primitives through the Copilot profile. (by @sergio-sisternes-epam; closes #1957) (#2041) +- Global Claude installs now support an absolute `CLAUDE_CONFIG_DIR` outside + `HOME` without leaving a partial deployment. (closes #2129) (#2135) ### Fixed diff --git a/src/apm_cli/install/deployed_paths.py b/src/apm_cli/install/deployed_paths.py index f9f62222f..76fbe2b06 100644 --- a/src/apm_cli/install/deployed_paths.py +++ b/src/apm_cli/install/deployed_paths.py @@ -18,10 +18,8 @@ def deployed_path_entry( def _try_dynamic_root(tgts, *, strict: bool = False) -> str | None: for _t in tgts: - deploy_root = _t.resolved_deploy_root - absolute_static_root = deploy_root is None and Path(_t.root_dir).is_absolute() - if absolute_static_root: - deploy_root = Path(_t.root_dir) + deploy_root = _t.managed_deploy_root + absolute_static_root = _t.resolved_deploy_root is None and deploy_root is not None if deploy_root is None: continue if not strict or absolute_static_root: diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 5dc3d5919..eb9de5644 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -11,23 +11,28 @@ from apm_cli.primitives.discovery import discover_primitives from apm_cli.utils.atomic_io import normalize_crlf_to_lf from apm_cli.utils.console import _rich_warning +from apm_cli.utils.path_security import PathTraversalError, ensure_path_within def _managed_absolute_target_root(candidate: Path, targets: Any) -> Path | None: - """Return the configured static target root governing an absolute path.""" + """Return the managed root for *candidate*, or ``None`` if unmanaged.""" from apm_cli.integration.targets import KNOWN_TARGETS source = targets if source is None: - source = [KNOWN_TARGETS[name].for_scope(user_scope=True) for name in ("claude", "hermes")] + source = [] + for profile in KNOWN_TARGETS.values(): + if not profile.user_supported or profile.user_root_resolver is not None: + continue + scoped = profile.for_scope(user_scope=True) + if scoped is not None: + source.append(scoped) try: resolved = candidate.resolve() for target_profile in source: if target_profile is None: continue - deploy_root = target_profile.resolved_deploy_root - if deploy_root is None and Path(target_profile.root_dir).is_absolute(): - deploy_root = Path(target_profile.root_dir) + deploy_root = target_profile.managed_deploy_root if deploy_root is None: continue resolved_root = deploy_root.resolve() @@ -35,7 +40,11 @@ def _managed_absolute_target_root(candidate: Path, targets: Any) -> Path | None: if not mapping.subdir: continue primitive_root = (resolved_root / mapping.subdir).resolve() - if resolved != primitive_root and resolved.is_relative_to(primitive_root): + try: + contained = ensure_path_within(resolved, primitive_root) + except PathTraversalError: + continue + if contained != primitive_root: return resolved_root if target_profile.hooks_config_display: hooks_file = resolved_root / Path(target_profile.hooks_config_display).name diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index e027f6b29..26a55566e 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -20,10 +20,7 @@ from collections.abc import Callable from dataclasses import dataclass -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from pathlib import Path +from pathlib import Path RULE_FORMATS: frozenset[str] = frozenset( {"cursor_rules", "claude_rules", "windsurf_rules", "kiro_steering", "antigravity_rules"} @@ -295,6 +292,14 @@ def effective_root(self, user_scope: bool = False) -> str: return self.user_root_dir return self.root_dir + @property + def managed_deploy_root(self) -> Path | None: + """Return the resolved or absolute static deployment root.""" + if self.resolved_deploy_root is not None: + return self.resolved_deploy_root + root = Path(self.root_dir) + return root if root.is_absolute() else None + def supports_at_user_scope(self, primitive: str) -> bool: """Return ``True`` if *primitive* can be deployed at user scope.""" if not self.user_supported: diff --git a/tests/unit/install/test_services.py b/tests/unit/install/test_services.py index 7b5cb7f95..883299870 100644 --- a/tests/unit/install/test_services.py +++ b/tests/unit/install/test_services.py @@ -11,6 +11,7 @@ from apm_cli.install.services import IntegratorBundle, _deployed_path_entry from apm_cli.integration.targets import KNOWN_TARGETS +from apm_cli.utils.paths import portable_relpath # --------------------------------------------------------------------------- # Helper: convert legacy integrators dict to IntegratorBundle @@ -172,6 +173,19 @@ def test_absolute_static_root_rejects_symlink_escape(self, tmp_path: Path) -> No with pytest.raises(PathTraversalError): _deployed_path_entry(target_path, project_root, targets=[claude_target]) + def test_absolute_static_root_returns_portable_relpath(self, tmp_path: Path) -> None: + project_root = tmp_path / "home" + project_root.mkdir() + config_root = tmp_path / "claude-config" + target_path = config_root / "rules" / "managed.md" + target_path.parent.mkdir(parents=True) + target_path.write_text("managed\n", encoding="utf-8") + claude_target = replace(KNOWN_TARGETS["claude"], root_dir=str(config_root)) + + result = _deployed_path_entry(target_path, project_root, targets=[claude_target]) + + assert result == portable_relpath(target_path, project_root) + def test_path_traversal_error_propagates_from_cowork_translation(self, tmp_path: Path) -> None: """PathTraversalError from to_lockfile_path must propagate, never be swallowed.""" from apm_cli.utils.path_security import PathTraversalError From a7af5d8bbad0fc2556a59ca1f67d4ab8818b8eb7 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 22:02:48 +0200 Subject: [PATCH 3/3] docs(security): clarify managed deployment roots Qualify path containment guidance for global installs using configured target roots. Addresses the final supply-chain panel finding for PR #2135. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/enterprise/security.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 7f739bcf2..e513434ba 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -234,7 +234,8 @@ APM records the license the package manifest *declares* (`license:` in `apm.yml` ## Path security -APM deploys files only to controlled subdirectories within the project root. +APM deploys files only to controlled subdirectories within the project root +at project scope or within a configured, managed target root at global scope. ### Path traversal prevention @@ -242,7 +243,9 @@ All deploy paths are validated before any file operation: 1. **No `..` segments.** Any path containing `..` is rejected outright. 2. **Allowed prefixes only.** Paths must start with an allowed target-integrator prefix (`.github/`, `.claude/`, `.cursor/`, `.opencode/`, `.codex/`, `.gemini/`, `.windsurf/`, `.kiro/`, `.agents/`). In addition, the local-bundle install path stages instructions for compile-only targets under `apm_modules//.apm/instructions/` with its own containment check (the resolved path must remain within `apm_modules/`) and `` validation rejecting traversal sequences and characters outside `[A-Za-z0-9._-]`. -3. **Resolution containment.** The fully resolved path must remain within the project root directory. +3. **Resolution containment.** The fully resolved path must remain within the + project root or the configured managed target root. Symlink escapes from + either root are rejected. A path must pass all three checks. Failure on any check prevents the file from being written.