From 7160659a269f67ba7ef411efb6d817892ed59c3b Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Mon, 6 Apr 2026 23:34:09 +0200 Subject: [PATCH] fix: reject symlinks in file discovery and resolution Reject symlinked primitive files outright via is_symlink() checks at all file discovery and resolution choke points. This matches the documented security policy ('symlinks are never followed') and prevents symlink- based traversal attacks from malicious packages. Protected paths: - PromptCompiler._resolve_prompt_file (prompt compilation/preview) - ScriptRunner._discover_prompt_file (prompt discovery) - find_primitive_files (instruction/agent/context discovery) - BaseIntegrator.find_files_by_glob (integrator file discovery) - HookIntegrator.find_hook_files (hook JSON discovery) - 5 new security tests with Windows symlink portability guards - Updated security docs to reflect full symlink rejection coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/enterprise/security.md | 9 +- src/apm_cli/core/script_runner.py | 30 +-- src/apm_cli/integration/base_integrator.py | 4 + src/apm_cli/integration/hook_integrator.py | 4 + src/apm_cli/primitives/discovery.py | 12 +- tests/unit/test_symlink_containment.py | 188 +++++++++++++++++++ 6 files changed, 229 insertions(+), 18 deletions(-) create mode 100644 tests/unit/test_symlink_containment.py diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 73406942..e6793bf7 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -175,12 +175,15 @@ A path must pass all three checks. Failure on any check prevents the file from b ### Symlink handling -Symlinks are never followed during artifact operations: +Symlinks are never followed during file discovery or artifact operations: -- **Tree copy operations** skip symlinks entirely — they are excluded from the copy via an ignore filter. +- **Primitive discovery** (instructions, agents, prompts, contexts, skills) rejects symlinked files during glob-based file enumeration. Symlinks are silently skipped. +- **Prompt resolution** (`apm preview`, `apm run`) rejects symlinked `.prompt.md` files with an explicit error message. +- **Integrator file discovery** (agents, instructions, prompts, skills, hooks) rejects symlinked files via `is_symlink()` checks in `find_files_by_glob` and `find_hook_files`. +- **Tree copy operations** skip symlinks entirely -- they are excluded from the copy via an ignore filter. - **MCP configuration files** that are symlinks are rejected with a warning and not parsed. - **Manifest parsing** requires files to pass both `.is_file()` and `not .is_symlink()` checks. -- **Archive creation** — `apm pack` excludes symlinks from bundled archives. Packaged artifacts contain no symbolic links, preventing symlink-based escape attacks in distributed bundles. +- **Archive creation** -- `apm pack` excludes symlinks from bundled archives. Packaged artifacts contain no symbolic links, preventing symlink-based escape attacks in distributed bundles. This prevents symlink-based attacks that could escape allowed directories or cause APM to read or write outside the project root. diff --git a/src/apm_cli/core/script_runner.py b/src/apm_cli/core/script_runner.py index 6c79ae2b..f873ca66 100644 --- a/src/apm_cli/core/script_runner.py +++ b/src/apm_cli/core/script_runner.py @@ -546,22 +546,24 @@ def _discover_prompt_file(self, name: str) -> Optional[Path]: ] for path in local_search_paths: - if path.exists(): + if path.exists() and not path.is_symlink(): return path # 2. Search in dependencies and detect collisions apm_modules = Path("apm_modules") if apm_modules.exists(): # Collect ALL .prompt.md matches to detect collisions - matches = list(apm_modules.rglob(search_name)) + raw_matches = list(apm_modules.rglob(search_name)) # Also search for SKILL.md in directories matching the name - # e.g., name="architecture-blueprint-generator" -> find */architecture-blueprint-generator/SKILL.md for skill_dir in apm_modules.rglob(name): if skill_dir.is_dir(): skill_file = skill_dir / "SKILL.md" if skill_file.exists(): - matches.append(skill_file) + raw_matches.append(skill_file) + + # Filter out symlinks + matches = [m for m in raw_matches if not m.is_symlink()] if len(matches) == 0: return None @@ -945,6 +947,8 @@ def compile(self, prompt_file: str, params: Dict[str, str]) -> str: def _resolve_prompt_file(self, prompt_file: str) -> Path: """Resolve prompt file path, checking local directory first, then common directories, then dependencies. + Symlinks are rejected outright to prevent traversal attacks. + Args: prompt_file: Relative path to the .prompt.md file @@ -952,40 +956,40 @@ def _resolve_prompt_file(self, prompt_file: str) -> Path: Path: Resolved path to the prompt file Raises: - FileNotFoundError: If prompt file is not found in local or dependency modules + FileNotFoundError: If prompt file is not found or is a symlink """ prompt_path = Path(prompt_file) # First check if it exists in current directory (local) if prompt_path.exists(): + if prompt_path.is_symlink(): + raise FileNotFoundError( + f"Prompt file '{prompt_file}' is a symlink. " + f"Symlinks are not allowed for security reasons." + ) return prompt_path # Check in common project directories common_dirs = [".github/prompts", ".apm/prompts"] for common_dir in common_dirs: common_path = Path(common_dir) / prompt_file - if common_path.exists(): + if common_path.exists() and not common_path.is_symlink(): return common_path # If not found locally, search in dependency modules apm_modules_dir = Path("apm_modules") if apm_modules_dir.exists(): - # Search all dependency directories for the prompt file - # Handle org/repo directory structure (e.g., apm_modules/microsoft/apm-sample-package/) for org_dir in apm_modules_dir.iterdir(): if org_dir.is_dir() and not org_dir.name.startswith("."): - # Iterate through repos within the org for repo_dir in org_dir.iterdir(): if repo_dir.is_dir() and not repo_dir.name.startswith("."): - # Check in the root of the repository dep_prompt_path = repo_dir / prompt_file - if dep_prompt_path.exists(): + if dep_prompt_path.exists() and not dep_prompt_path.is_symlink(): return dep_prompt_path - # Also check in common subdirectories for subdir in ["prompts", ".", "workflows"]: sub_prompt_path = repo_dir / subdir / prompt_file - if sub_prompt_path.exists(): + if sub_prompt_path.exists() and not sub_prompt_path.is_symlink(): return sub_prompt_path # If still not found, raise an error with helpful message diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 066c6abe..fa7c92ee 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -409,6 +409,8 @@ def find_files_by_glob( ) -> List[Path]: """Search *package_path* (and optional subdirectories) for *pattern*. + Symlinks are rejected outright to prevent traversal attacks. + Args: package_path: Root of the installed package. pattern: Glob pattern (e.g. ``"*.prompt.md"``). @@ -429,6 +431,8 @@ def find_files_by_glob( if not d.exists(): continue for f in sorted(d.glob(pattern)): + if f.is_symlink(): + continue resolved = f.resolve() if resolved not in seen: seen.add(resolved) diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index 658e6388..e21acdb6 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -151,6 +151,8 @@ def find_hook_files(self, package_path: Path) -> List[Path]: apm_hooks = package_path / ".apm" / "hooks" if apm_hooks.exists(): for f in sorted(apm_hooks.glob("*.json")): + if f.is_symlink(): + continue resolved = f.resolve() if resolved not in seen: seen.add(resolved) @@ -160,6 +162,8 @@ def find_hook_files(self, package_path: Path) -> List[Path]: hooks_dir = package_path / "hooks" if hooks_dir.exists(): for f in sorted(hooks_dir.glob("*.json")): + if f.is_symlink(): + continue resolved = f.resolve() if resolved not in seen: seen.add(resolved) diff --git a/src/apm_cli/primitives/discovery.py b/src/apm_cli/primitives/discovery.py index 7d10baa8..41c09d24 100644 --- a/src/apm_cli/primitives/discovery.py +++ b/src/apm_cli/primitives/discovery.py @@ -375,6 +375,9 @@ def _discover_skill_in_directory(directory: Path, collection: PrimitiveCollectio def find_primitive_files(base_dir: str, patterns: List[str]) -> List[Path]: """Find primitive files matching the given patterns. + Symlinks are rejected outright to prevent symlink-based traversal + attacks from malicious packages. + Args: base_dir (str): Base directory to search in. patterns (List[str]): List of glob patterns to match. @@ -402,10 +405,15 @@ def find_primitive_files(base_dir: str, patterns: List[str]) -> List[Path]: seen.add(abs_path) unique_files.append(Path(abs_path)) - # Filter out directories and ensure files are readable + # Filter out directories, symlinks, and unreadable files valid_files = [] for file_path in unique_files: - if file_path.is_file() and _is_readable(file_path): + if not file_path.is_file(): + continue + if file_path.is_symlink(): + logger.debug("Rejected symlink: %s", file_path) + continue + if _is_readable(file_path): valid_files.append(file_path) return valid_files diff --git a/tests/unit/test_symlink_containment.py b/tests/unit/test_symlink_containment.py new file mode 100644 index 00000000..12392ac1 --- /dev/null +++ b/tests/unit/test_symlink_containment.py @@ -0,0 +1,188 @@ +"""Tests for symlink containment enforcement across APM subsystems. + +Validates that symlinked primitive files are rejected at discovery and +resolution time, preventing arbitrary local file reads. +""" + +import json +import os +import tempfile +import shutil +import unittest +from pathlib import Path + + +def _try_symlink(link: Path, target: Path): + """Create a symlink or skip the test on platforms that don't support it.""" + try: + link.symlink_to(target) + except OSError: + raise unittest.SkipTest("Symlinks not supported on this platform") + + +class TestPromptCompilerSymlinkContainment(unittest.TestCase): + """PromptCompiler._resolve_prompt_file rejects external symlinks.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.project = Path(self.tmpdir) / "project" + self.project.mkdir() + self.outside = Path(self.tmpdir) / "outside" + self.outside.mkdir() + # Create a file outside the project + self.secret = self.outside / "secret.txt" + self.secret.write_text("sensitive-data", encoding="utf-8") + # Create apm.yml so the project is valid + (self.project / "apm.yml").write_text( + "name: test\nversion: 1.0.0\n", encoding="utf-8" + ) + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def test_symlinked_prompt_outside_project_rejected(self): + """Symlinked .prompt.md is rejected with clear error message.""" + from apm_cli.core.script_runner import PromptCompiler + + prompts_dir = self.project / ".apm" / "prompts" + prompts_dir.mkdir(parents=True) + symlink = prompts_dir / "evil.prompt.md" + _try_symlink(symlink, self.secret) + + compiler = PromptCompiler() + old_cwd = os.getcwd() + try: + os.chdir(self.project) + with self.assertRaises(FileNotFoundError) as ctx: + compiler._resolve_prompt_file(".apm/prompts/evil.prompt.md") + self.assertIn("symlink", str(ctx.exception).lower()) + finally: + os.chdir(old_cwd) + + def test_normal_prompt_within_project_allowed(self): + """Non-symlinked prompt files within the project are allowed.""" + from apm_cli.core.script_runner import PromptCompiler + + prompts_dir = self.project / ".apm" / "prompts" + prompts_dir.mkdir(parents=True) + prompt = prompts_dir / "safe.prompt.md" + prompt.write_text("# Safe prompt", encoding="utf-8") + + compiler = PromptCompiler() + old_cwd = os.getcwd() + try: + os.chdir(self.project) + result = compiler._resolve_prompt_file(".apm/prompts/safe.prompt.md") + self.assertTrue(result.exists()) + finally: + os.chdir(old_cwd) + + +class TestPrimitiveDiscoverySymlinkContainment(unittest.TestCase): + """find_primitive_files rejects symlinks outside base directory.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.project = Path(self.tmpdir) / "project" + self.project.mkdir() + self.outside = Path(self.tmpdir) / "outside" + self.outside.mkdir() + self.secret = self.outside / "leak.instructions.md" + self.secret.write_text("---\napplyTo: '**'\n---\nLeaked!", encoding="utf-8") + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def test_symlinked_instruction_outside_base_rejected(self): + """Symlinked .instructions.md outside base_dir is filtered out.""" + from apm_cli.primitives.discovery import find_primitive_files + + instructions_dir = self.project / ".github" / "instructions" + instructions_dir.mkdir(parents=True) + symlink = instructions_dir / "evil.instructions.md" + _try_symlink(symlink, self.secret) + + # Also add a normal file + normal = instructions_dir / "safe.instructions.md" + normal.write_text("---\napplyTo: '**'\n---\nSafe", encoding="utf-8") + + results = find_primitive_files( + str(self.project), + [".github/instructions/*.instructions.md"], + ) + names = [f.name for f in results] + self.assertIn("safe.instructions.md", names) + self.assertNotIn("evil.instructions.md", names) + + +class TestBaseIntegratorSymlinkContainment(unittest.TestCase): + """BaseIntegrator.find_files_by_glob rejects external symlinks.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.pkg = Path(self.tmpdir) / "pkg" + self.pkg.mkdir() + self.outside = Path(self.tmpdir) / "outside" + self.outside.mkdir() + self.secret = self.outside / "leak.agent.md" + self.secret.write_text("# Leaked agent", encoding="utf-8") + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def test_symlinked_agent_outside_package_rejected(self): + """Symlinked .agent.md outside package dir is filtered out.""" + from apm_cli.integration.base_integrator import BaseIntegrator + + agents_dir = self.pkg / ".apm" / "agents" + agents_dir.mkdir(parents=True) + symlink = agents_dir / "evil.agent.md" + _try_symlink(symlink, self.secret) + + normal = agents_dir / "safe.agent.md" + normal.write_text("# Safe agent", encoding="utf-8") + + results = BaseIntegrator.find_files_by_glob( + self.pkg, "*.agent.md", subdirs=[".apm/agents"], + ) + names = [f.name for f in results] + self.assertIn("safe.agent.md", names) + self.assertNotIn("evil.agent.md", names) + + +class TestHookIntegratorSymlinkContainment(unittest.TestCase): + """HookIntegrator.find_hook_files rejects external symlinks.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.pkg = Path(self.tmpdir) / "pkg" + self.pkg.mkdir() + self.outside = Path(self.tmpdir) / "outside" + self.outside.mkdir() + self.secret = self.outside / "evil.json" + self.secret.write_text(json.dumps({"hooks": {}}), encoding="utf-8") + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def test_symlinked_hook_json_outside_package_rejected(self): + """Symlinked hook JSON outside package dir is filtered out.""" + from apm_cli.integration.hook_integrator import HookIntegrator + + hooks_dir = self.pkg / ".apm" / "hooks" + hooks_dir.mkdir(parents=True) + symlink = hooks_dir / "evil.json" + _try_symlink(symlink, self.secret) + + normal = hooks_dir / "safe.json" + normal.write_text(json.dumps({"hooks": {}}), encoding="utf-8") + + integrator = HookIntegrator() + results = integrator.find_hook_files(self.pkg) + names = [f.name for f in results] + self.assertIn("safe.json", names) + self.assertNotIn("evil.json", names) + + +if __name__ == "__main__": + unittest.main()