fix: address unresolved review comments on apm pack/unpack

docs/cli-reference.md

@@ -335,6 +335,81 @@ apm prune --dry-run
 - Removes deployed integration files (prompts, agents, hooks, etc.) for pruned packages using the `deployed_files` manifest in `apm.lock`
 - Updates `apm.lock` to reflect the pruned state
 
+### `apm pack` - Create a portable bundle
+
+Create a self-contained bundle from installed APM dependencies using the `deployed_files` recorded in `apm.lock` as the source of truth.
+
+```bash
+apm pack [OPTIONS]
+```
+
+**Options:**
+- `-o, --output TEXT` - Output directory (default: `./build/`)
+- `-t, --target [vscode|agents|claude|all]` - Filter files by target. Auto-detects from `apm.yml` if not specified
+- `--archive` - Produce a `.tar.gz` archive instead of a directory
+- `--dry-run` - List files that would be packed without writing anything
+- `--format [apm|plugin]` - Bundle format (default: `apm`)
+
+**Examples:**
+```bash
+# Pack to ./build/<name>-<version>/
+apm pack
+
+# Pack as a .tar.gz archive
+apm pack --archive
+
+# Pack only VS Code / Copilot files
+apm pack --target vscode
+
+# Preview what would be packed
+apm pack --dry-run
+
+# Custom output directory
+apm pack -o dist/
+```
+
+**Behavior:**
+- Reads `apm.lock` to enumerate all `deployed_files` from installed dependencies
+- Copies files preserving directory structure
+- Writes an enriched `apm.lock` inside the bundle with a `pack:` metadata section
+- The project's own `apm.lock` is never modified
+
+### `apm unpack` - Extract a bundle
+
+Extract an APM bundle into the current project with optional completeness verification.
+
+```bash
+apm unpack BUNDLE_PATH [OPTIONS]
+```
+
+**Arguments:**
+- `BUNDLE_PATH` - Path to a `.tar.gz` archive or an unpacked bundle directory
+
+**Options:**
+- `--output TEXT` - Target project directory (default: current directory)
+- `--skip-verify` - Skip completeness verification against the bundle lockfile
+- `--dry-run` - Show what would be extracted without writing anything
+
+**Examples:**
+```bash
+# Unpack an archive into the current directory
+apm unpack ./build/my-pkg-1.0.0.tar.gz
+
+# Unpack into a specific directory
+apm unpack bundle.tar.gz --output /path/to/project
+
+# Skip verification (useful for partial bundles)
+apm unpack bundle.tar.gz --skip-verify
+
+# Preview what would be extracted
+apm unpack bundle.tar.gz --dry-run
+```
+
+**Behavior:**
+- Additive-only: only writes files listed in the bundle's `apm.lock`; never deletes existing files
+- If a local file has the same path as a bundle file, the bundle file wins (overwrite)
+- Verification checks that all `deployed_files` from the bundle lockfile are present in the bundle
+
 ### `apm update` - Update APM to the latest version
 
 Update the APM CLI to the latest version available on GitHub releases.

src/apm_cli/bundle/packer.py

@@ -105,10 +105,21 @@ def pack_bundle(
             seen.add(f)
             unique_files.append(f)
 
-    # 5. Verify each file exists on disk
+    # 5. Verify each path is safe (no traversal) and exists on disk
+    project_root_resolved = project_root.resolve()
     missing: List[str] = []
     for rel_path in unique_files:
+        # Guard against absolute paths or path-traversal entries in deployed_files
+        p = Path(rel_path)
+        if p.is_absolute() or ".." in p.parts:
+            raise ValueError(
+                f"Refusing to pack unsafe path from lockfile: {rel_path!r}"
+            )
         abs_path = project_root / rel_path
+        if not abs_path.resolve().is_relative_to(project_root_resolved):
+            raise ValueError(
+                f"Refusing to pack path that escapes project root: {rel_path!r}"
+            )
         # deployed_files may reference directories (ending with /)
         if not abs_path.exists():
             missing.append(rel_path)

src/apm_cli/bundle/unpacker.py

@@ -1,6 +1,7 @@
 """Bundle unpacker — extracts and verifies APM bundles."""
 
 import shutil
+import sys
 import tarfile
 import tempfile
 from dataclasses import dataclass, field
@@ -58,7 +59,11 @@ def unpack_bundle(
                         raise ValueError(
                             f"Refusing to extract path-traversal entry: {member.name}"
                         )
-                tar.extractall(temp_dir, filter="data")
+                # filter="data" was added in Python 3.12; use it when available
+                if sys.version_info >= (3, 12):
+                    tar.extractall(temp_dir, filter="data")
+                else:
+                    tar.extractall(temp_dir)  # noqa: S202 — manual checks above
         except Exception:
             shutil.rmtree(temp_dir, ignore_errors=True)
             raise
@@ -80,8 +85,12 @@ def unpack_bundle(
         lockfile_path = source_dir / "apm.lock"
         lockfile = LockFile.read(lockfile_path)
         if lockfile is None:
+            if not lockfile_path.exists():
+                raise FileNotFoundError(
+                    "apm.lock not found in the bundle — the bundle may be incomplete."
+                )
             raise FileNotFoundError(
-                "apm.lock not found in the bundle — the bundle may be corrupt."
+                "apm.lock in the bundle could not be parsed — the bundle may be corrupt."
             )
 
         # Collect all deployed_files from lockfile
@@ -116,26 +125,37 @@ def unpack_bundle(
         # Dry-run: return file list without writing
         if dry_run:
             return UnpackResult(
-                extracted_dir=source_dir,
+                extracted_dir=bundle_path,
                 files=unique_files,
                 verified=verified,
             )
 
         # 4. Copy target files to output_dir (additive, no deletes)
         output_dir = Path(output_dir)
+        output_dir_resolved = output_dir.resolve()
         for rel_path in unique_files:
+            # Guard against absolute paths or path-traversal entries in deployed_files
+            p = Path(rel_path)
+            if p.is_absolute() or ".." in p.parts:
+                raise ValueError(
+                    f"Refusing to unpack unsafe path from bundle lockfile: {rel_path!r}"
+                )
+            dest = output_dir / rel_path
+            if not dest.resolve().is_relative_to(output_dir_resolved):
+                raise ValueError(
+                    f"Refusing to unpack path that escapes output directory: {rel_path!r}"
+                )
             src = source_dir / rel_path
             if not src.exists():
                 continue  # skip_verify may allow missing files
-            dest = output_dir / rel_path
             if src.is_dir():
                 shutil.copytree(src, dest, dirs_exist_ok=True)
             else:
                 dest.parent.mkdir(parents=True, exist_ok=True)
                 shutil.copy2(src, dest)
 
         return UnpackResult(
-            extracted_dir=source_dir,
+            extracted_dir=bundle_path,
             files=unique_files,
             verified=verified,
         )

src/apm_cli/cli.py

@@ -19,6 +19,7 @@
 dict = builtins.dict
 
 from apm_cli.commands.deps import deps
+from apm_cli.commands.pack import pack_cmd, unpack_cmd
 from apm_cli.compilation import AgentsCompiler, CompilationConfig
 from apm_cli.primitives.discovery import discover_primitives
 from apm_cli.utils.console import (
@@ -309,8 +310,6 @@ def cli(ctx):
 
 # Register command groups
 cli.add_command(deps)
-
-from apm_cli.commands.pack import pack_cmd, unpack_cmd
 cli.add_command(pack_cmd, name="pack")
 cli.add_command(unpack_cmd, name="unpack")
 

tests/integration/test_pack_unpack_e2e.py

@@ -10,7 +10,6 @@
 import subprocess
 
 import pytest
-import yaml
 from pathlib import Path
 
 

tests/unit/test_packer.py

@@ -211,3 +211,32 @@ def test_pack_lockfile_original_unchanged(self, tmp_path):
         pack_bundle(project, out)
 
         assert (project / "apm.lock").read_text() == original_content
+
+    def test_pack_rejects_embedded_traversal_in_deployed_path(self, tmp_path):
+        """pack_bundle must reject path-traversal entries embedded in deployed_files."""
+        project = _setup_project(tmp_path, [])
+        # A path that looks like it starts with .github/ but traverses out
+        lockfile = LockFile.read(project / "apm.lock")
+        dep = LockedDependency(
+            repo_url="owner/repo",
+            deployed_files=[".github/../../../etc/passwd"],
+        )
+        lockfile.add_dependency(dep)
+        lockfile.write(project / "apm.lock")
+
+        with pytest.raises(ValueError, match="unsafe path"):
+            pack_bundle(project, tmp_path / "out")
+
+    def test_pack_rejects_traversal_deployed_path(self, tmp_path):
+        """pack_bundle must reject path-traversal entries in deployed_files."""
+        project = _setup_project(tmp_path, [])
+        lockfile = LockFile.read(project / "apm.lock")
+        dep = LockedDependency(
+            repo_url="owner/repo",
+            deployed_files=[".github/agents/../../../../../../tmp/evil.sh"],
+        )
+        lockfile.add_dependency(dep)
+        lockfile.write(project / "apm.lock")
+
+        with pytest.raises(ValueError, match="unsafe path"):
+            pack_bundle(project, tmp_path / "out")

tests/unit/test_unpacker.py

@@ -4,9 +4,8 @@
 from pathlib import Path
 
 import pytest
-import yaml
 
-from apm_cli.bundle.unpacker import unpack_bundle, UnpackResult
+from apm_cli.bundle.unpacker import unpack_bundle
 from apm_cli.deps.lockfile import LockFile, LockedDependency
 
 
@@ -182,3 +181,31 @@ def test_unpack_lockfile_not_scattered(self, tmp_path):
 
         # apm.lock should NOT be copied to the output root
         assert not (output / "apm.lock").exists()
+
+    def test_unpack_rejects_absolute_path_in_deployed_files(self, tmp_path):
+        """unpack_bundle must reject absolute paths from bundle lockfile."""
+        bundle_dir = tmp_path / "bundle" / "test-pkg-1.0.0"
+        bundle_dir.mkdir(parents=True)
+        lockfile = LockFile()
+        dep = LockedDependency(repo_url="owner/repo", deployed_files=["/etc/passwd"])
+        lockfile.add_dependency(dep)
+        lockfile.write(bundle_dir / "apm.lock")
+        output = tmp_path / "target"
+        output.mkdir()
+
+        with pytest.raises(ValueError, match="unsafe path"):
+            unpack_bundle(bundle_dir, output, skip_verify=True)
+
+    def test_unpack_rejects_traversal_path_in_deployed_files(self, tmp_path):
+        """unpack_bundle must reject path-traversal entries from bundle lockfile."""
+        bundle_dir = tmp_path / "bundle" / "test-pkg-1.0.0"
+        bundle_dir.mkdir(parents=True)
+        lockfile = LockFile()
+        dep = LockedDependency(repo_url="owner/repo", deployed_files=["../outside.txt"])
+        lockfile.add_dependency(dep)
+        lockfile.write(bundle_dir / "apm.lock")
+        output = tmp_path / "target"
+        output.mkdir()
+
+        with pytest.raises(ValueError, match="unsafe path"):
+            unpack_bundle(bundle_dir, output, skip_verify=True)