fix(agents): block directory traversal in command write paths (#2229)

src/specify_cli/agents.py

@@ -6,6 +6,7 @@
 command files into agent-specific directories in the correct format.
 """
 
+import os
 from pathlib import Path
 from typing import Dict, List, Any
 
@@ -399,6 +400,28 @@ def _compute_output_name(
 
         return f"speckit-{short_name}"
 
+    @staticmethod
+    def _ensure_inside(candidate: Path, base: Path) -> None:
+        """Validate that a write target stays within the expected base directory.
+
+        Uses lexical normalization so traversal via ``..`` or absolute paths is
+        rejected while intentionally symlinked sub-directories remain
+        supported.
+
+        Args:
+            candidate: Path that will be written.
+            base: Directory the write must remain within.
+
+        Raises:
+            ValueError: If the normalized candidate path escapes ``base``.
+        """
+        normalized = Path(os.path.normpath(candidate))
+        base_normalized = Path(os.path.normpath(base))
+        if not normalized.is_relative_to(base_normalized):
+            raise ValueError(
+                f"Output path {candidate!r} escapes directory {base!r}"
+            )
+
     def register_commands(
         self,
         agent_name: str,
@@ -485,6 +508,7 @@ def register_commands(
                 raise ValueError(f"Unsupported format: {agent_config['format']}")
 
             dest_file = commands_dir / f"{output_name}{agent_config['extension']}"
+            self._ensure_inside(dest_file, commands_dir)
             dest_file.parent.mkdir(parents=True, exist_ok=True)
             dest_file.write_text(output, encoding="utf-8")
 
@@ -550,12 +574,7 @@ def register_commands(
                 alias_file = (
                     commands_dir / f"{alias_output_name}{agent_config['extension']}"
                 )
-                try:
-                    alias_file.resolve().relative_to(commands_dir.resolve())
-                except ValueError:
-                    raise ValueError(
-                        f"Alias output path escapes commands directory: {alias_file!r}"
-                    )
+                self._ensure_inside(alias_file, commands_dir)
                 alias_file.parent.mkdir(parents=True, exist_ok=True)
                 alias_file.write_text(alias_output, encoding="utf-8")
                 if agent_name == "copilot":
@@ -575,6 +594,7 @@ def write_copilot_prompt(project_root: Path, cmd_name: str) -> None:
         prompts_dir = project_root / ".github" / "prompts"
         prompts_dir.mkdir(parents=True, exist_ok=True)
         prompt_file = prompts_dir / f"{cmd_name}.prompt.md"
+        CommandRegistrar._ensure_inside(prompt_file, prompts_dir)
         prompt_file.write_text(f"---\nagent: {cmd_name}\n---\n", encoding="utf-8")
 
     def register_commands_for_all_agents(

tests/test_registrar_path_traversal.py

@@ -0,0 +1,204 @@
+"""Tests for CommandRegistrar directory traversal guards around issue #2229."""
+
+import errno
+from pathlib import Path
+
+import pytest
+
+from specify_cli.agents import CommandRegistrar
+
+
+TRAVERSAL_PAYLOADS = [
+    "../pwned",
+    "../../etc/passwd",
+    "subdir/../../escape",
+    "/absolute/evil",
+]
+
+
+def _write_source(ext_dir: Path) -> Path:
+    ext_dir.mkdir(parents=True, exist_ok=True)
+    (ext_dir / "commands").mkdir(exist_ok=True)
+    (ext_dir / "commands" / "cmd.md").write_text(
+        "---\ndescription: test\n---\n\nbody\n", encoding="utf-8"
+    )
+    return ext_dir
+
+
+def _cmd(name: str, aliases: list[str] | None = None) -> dict[str, object]:
+    return {
+        "name": name,
+        "file": "commands/cmd.md",
+        "aliases": list(aliases or []),
+    }
+
+
+def _project_and_source(tmp_path):
+    project = tmp_path / "project"
+    project.mkdir()
+    ext_dir = _write_source(tmp_path / "ext-src")
+    return project, ext_dir
+
+
+def _assert_no_stray_files(tmp_root: Path, marker: str) -> None:
+    """Fail if a file matching ``marker`` exists outside the project tree."""
+    stray = [
+        p for p in tmp_root.rglob("*")
+        if p.is_file() and marker in p.name and "project" not in p.parts
+    ]
+    assert stray == [], (
+        f"Traversal payload leaked files outside the project tree: {stray}"
+    )
+
+
+class TestPrimaryCommandTraversal:
+    """Primary command names must not escape the agent's commands directory."""
+
+    @pytest.mark.parametrize("bad_name", TRAVERSAL_PAYLOADS)
+    def test_gemini_rejects_traversal_in_primary_name(self, tmp_path, bad_name):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".gemini" / "commands").mkdir(parents=True)
+
+        registrar = CommandRegistrar()
+        with pytest.raises(ValueError, match="escapes|outside|Invalid"):
+            registrar.register_commands(
+                "gemini", [_cmd(bad_name)], "myext", ext_dir, project
+            )
+
+        _assert_no_stray_files(tmp_path, Path(bad_name).name.replace("/", ""))
+
+    @pytest.mark.parametrize("bad_name", TRAVERSAL_PAYLOADS)
+    def test_copilot_rejects_traversal_in_primary_name(self, tmp_path, bad_name):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".github" / "agents").mkdir(parents=True)
+        (project / ".github" / "prompts").mkdir(parents=True)
+
+        registrar = CommandRegistrar()
+        with pytest.raises(ValueError, match="escapes|outside|Invalid"):
+            registrar.register_commands(
+                "copilot", [_cmd(bad_name)], "myext", ext_dir, project
+            )
+
+        _assert_no_stray_files(tmp_path, Path(bad_name).name.replace("/", ""))
+
+
+class TestAliasTraversal:
+    """Free-form aliases must not escape commands_dir (regression for b67b285)."""
+
+    @pytest.mark.parametrize("bad_alias", TRAVERSAL_PAYLOADS)
+    def test_gemini_rejects_traversal_in_alias(self, tmp_path, bad_alias):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".gemini" / "commands").mkdir(parents=True)
+
+        registrar = CommandRegistrar()
+        with pytest.raises(ValueError, match="escapes|outside|Invalid"):
+            registrar.register_commands(
+                "gemini",
+                [_cmd("speckit.myext.ok", [bad_alias])],
+                "myext",
+                ext_dir,
+                project,
+            )
+
+        _assert_no_stray_files(tmp_path, Path(bad_alias).name.replace("/", ""))
+
+    @pytest.mark.parametrize("bad_alias", TRAVERSAL_PAYLOADS)
+    def test_copilot_rejects_traversal_in_alias(self, tmp_path, bad_alias):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".github" / "agents").mkdir(parents=True)
+        (project / ".github" / "prompts").mkdir(parents=True)
+
+        registrar = CommandRegistrar()
+        with pytest.raises(ValueError, match="escapes|outside|Invalid"):
+            registrar.register_commands(
+                "copilot",
+                [_cmd("speckit.myext.ok", [bad_alias])],
+                "myext",
+                ext_dir,
+                project,
+            )
+
+        _assert_no_stray_files(tmp_path, Path(bad_alias).name.replace("/", ""))
+
+
+class TestCopilotPromptTraversal:
+    """`write_copilot_prompt` is a public static method — guard it directly."""
+
+    @pytest.mark.parametrize("bad_name", TRAVERSAL_PAYLOADS)
+    def test_rejects_traversal_names(self, tmp_path, bad_name):
+        project = tmp_path / "project"
+        (project / ".github" / "prompts").mkdir(parents=True)
+
+        with pytest.raises(ValueError, match="escapes|outside|Invalid"):
+            CommandRegistrar.write_copilot_prompt(project, bad_name)
+
+        _assert_no_stray_files(tmp_path, Path(bad_name).name.replace("/", ""))
+
+
+class TestSafeRegistration:
+    """Positive regression — well-formed names continue to register."""
+
+    def test_symlinked_subdir_under_commands_dir_is_preserved(self, tmp_path):
+        """Lexical check must not block legitimately symlinked sub-directories.
+
+        Teams sometimes symlink shared skills into their agent commands dir
+        (e.g. ``.gemini/commands/shared -> /team/shared-commands``). The
+        guard is purely lexical, so such a setup continues to work even though
+        the resolved target lives outside commands_dir on disk.
+        """
+        project, ext_dir = _project_and_source(tmp_path)
+        commands_dir = project / ".gemini" / "commands"
+        commands_dir.mkdir(parents=True)
+
+        external_shared = tmp_path / "external-shared"
+        external_shared.mkdir()
+        try:
+            (commands_dir / "shared").symlink_to(
+                external_shared, target_is_directory=True
+            )
+        except OSError as exc:
+            if exc.errno in {errno.EPERM, errno.EACCES}:
+                pytest.skip("symlink creation is not permitted in this environment")
+            raise
+
+        registrar = CommandRegistrar()
+        registered = registrar.register_commands(
+            "gemini",
+            [_cmd("shared/hello")],
+            "myext",
+            ext_dir,
+            project,
+        )
+
+        assert registered == ["shared/hello"]
+        assert (external_shared / "hello.toml").exists()
+
+    def test_safe_command_and_alias_still_register(self, tmp_path):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".claude" / "skills").mkdir(parents=True)
+
+        registrar = CommandRegistrar()
+        registered = registrar.register_commands(
+            "claude",
+            [_cmd("speckit.myext.hello", ["speckit.myext.hi"])],
+            "myext",
+            ext_dir,
+            project,
+        )
+
+        assert "speckit.myext.hello" in registered
+        assert "speckit.myext.hi" in registered
+        assert (
+            project
+            / ".claude"
+            / "skills"
+            / "speckit-myext-hello"
+            / "SKILL.md"
+        ).exists()
+        assert (
+            project
+            / ".claude"
+            / "skills"
+            / "speckit-myext-hi"
+            / "SKILL.md"
+        ).exists()