Harden command registration path handling

src/specify_cli/agents.py

@@ -10,7 +10,7 @@
 import platform
 import re
 from copy import deepcopy
-from pathlib import Path
+from pathlib import Path, PurePosixPath, PureWindowsPath
 from typing import Any, Dict, List, Optional
 
 import yaml
@@ -540,17 +540,53 @@ def register_commands(
 
         registered = []
         is_cline_ext = agent_name == "cline" and source_id != "core"
+        source_root = source_dir.resolve()
 
         for cmd_info in commands:
             cmd_name = cmd_info["name"]
             aliases = cmd_info.get("aliases", [])
             cmd_file = cmd_info["file"]
 
-            source_file = source_dir / cmd_file
-            if not source_file.exists():
+            # Skip malformed entries: a non-string/empty ``file`` cannot be a
+            # valid command body and would otherwise raise when used as a path.
+            if not isinstance(cmd_file, str) or not cmd_file:
                 continue
 
-            content = source_file.read_text(encoding="utf-8")
+            # Guard against path traversal, keeping runtime policy aligned with
+            # ExtensionManifest._validate() and the skill/preset/restore readers.
+            # Evaluate the value under both POSIX and Windows path flavors
+            # because a native ``Path`` is OS-dependent (a ``PurePosixPath`` on
+            # POSIX does not interpret Windows drive/UNC forms). Reject any
+            # non-empty anchor — which covers POSIX-absolute (``/abs``), Windows
+            # drive-relative (``C:foo``, anchored but not ``is_absolute()`` yet
+            # resolved against the CWD on its drive), Windows absolute
+            # (``C:\foo``), and UNC roots — and any ``..`` segment in either
+            # separator style, so a malicious manifest ``file`` field
+            # (e.g. ``../../../outside.txt``) cannot read arbitrary host files
+            # into a generated command. The resolve()/relative_to() check below
+            # is the final containment backstop.
+            posix_path = PurePosixPath(cmd_file)
+            win_path = PureWindowsPath(cmd_file)
+            if (
+                posix_path.anchor
+                or win_path.anchor
+                or ".." in posix_path.parts
+                or ".." in win_path.parts
+            ):
+                continue
+            try:
+                source_file = (source_root / cmd_file).resolve()
+                source_file.relative_to(source_root)  # raises ValueError if outside
+            except (OSError, ValueError):
+                continue
+
+            if not source_file.is_file():
+                continue
+
+            try:
+                content = source_file.read_text(encoding="utf-8")
+            except (OSError, UnicodeDecodeError):
+                continue
             frontmatter, body = self.parse_frontmatter(content)
 
             if frontmatter.get("strategy") == "wrap":

src/specify_cli/extensions.py

@@ -18,7 +18,7 @@
 import zipfile
 from dataclasses import dataclass
 from datetime import datetime, timezone
-from pathlib import Path
+from pathlib import Path, PurePosixPath, PureWindowsPath
 from typing import Any, Callable, Dict, List, Optional, Set
 
 import pathspec
@@ -290,6 +290,42 @@ def _validate(self):
             if "name" not in cmd or "file" not in cmd:
                 raise ValidationError("Command missing 'name' or 'file'")
 
+            # Validate the 'file' field for path traversal at manifest-load
+            # time. This is defense-in-depth: the command/skill/preset readers
+            # also contain the resolved path, but rejecting a traversal here
+            # surfaces a clear error instead of silently skipping the command.
+            cmd_file = cmd["file"]
+            if not isinstance(cmd_file, str) or not cmd_file:
+                raise ValidationError(
+                    f"Command 'file' for '{cmd.get('name')}' must be a non-empty string"
+                )
+            if cmd_file.strip() != cmd_file:
+                raise ValidationError(
+                    f"Invalid command 'file' '{cmd_file}': must not have leading or "
+                    "trailing whitespace"
+                )
+            # Evaluate the value under both POSIX and Windows path semantics so
+            # the check is platform-independent. A native ``Path`` is OS-
+            # dependent — a ``PurePosixPath`` on POSIX won't interpret Windows
+            # drive/UNC forms, and ``C:foo`` is anchored but not
+            # ``is_absolute()``. Reject any non-empty anchor — which covers
+            # POSIX-absolute (``/abs``), Windows drive-relative (``C:foo``),
+            # Windows absolute (``C:\foo``), and UNC/rooted forms — plus ``..``
+            # segments written with either separator.
+            posix_path = PurePosixPath(cmd_file)
+            win_path = PureWindowsPath(cmd_file)
+            if (
+                posix_path.anchor
+                or win_path.anchor
+                or ".." in posix_path.parts
+                or ".." in win_path.parts
+            ):
+                raise ValidationError(
+                    f"Invalid command 'file' '{cmd_file}': must be a relative path "
+                    "within the extension directory (no absolute paths, drive "
+                    "letters, or '..' segments)"
+                )
+
             # Validate command name format
             if not EXTENSION_COMMAND_NAME_PATTERN.match(cmd["name"]):
                 corrected = self._try_correct_command_name(cmd["name"], ext["id"])

tests/test_extensions.py

@@ -377,6 +377,40 @@ def test_invalid_command_name(self, temp_dir, valid_manifest_data):
         with pytest.raises(ValidationError, match="Invalid command name"):
             ExtensionManifest(manifest_path)
 
+    @pytest.mark.parametrize(
+        "bad_file",
+        ["../../../outside.md", "../escape.md", "a/../../escape.md", "/abs/outside.md", "C:escape.md", "C:\\Windows\\x.md", "..\\..\\escape.md"],
+    )
+    def test_command_file_traversal_rejected(self, temp_dir, valid_manifest_data, bad_file):
+        """Manifest 'file' field with traversal/absolute path raises ValidationError.
+
+        Defense-in-depth for GHSA-w5fv-7w9x-7fc5.
+        """
+        import yaml
+
+        valid_manifest_data["provides"]["commands"][0]["file"] = bad_file
+
+        manifest_path = temp_dir / "extension.yml"
+        with open(manifest_path, 'w', encoding='utf-8') as f:
+            yaml.safe_dump(valid_manifest_data, f)
+
+        with pytest.raises(ValidationError, match="Invalid command 'file'"):
+            ExtensionManifest(manifest_path)
+
+    @pytest.mark.parametrize("bad_file", [" commands/hello.md", "commands/hello.md ", "\tcommands/hello.md"])
+    def test_command_file_whitespace_rejected(self, temp_dir, valid_manifest_data, bad_file):
+        """Manifest 'file' with leading/trailing whitespace raises ValidationError."""
+        import yaml
+
+        valid_manifest_data["provides"]["commands"][0]["file"] = bad_file
+
+        manifest_path = temp_dir / "extension.yml"
+        with open(manifest_path, 'w', encoding='utf-8') as f:
+            yaml.safe_dump(valid_manifest_data, f)
+
+        with pytest.raises(ValidationError, match="leading or trailing whitespace"):
+            ExtensionManifest(manifest_path)
+
     def test_command_name_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data):
         """Test that 'speckit.command' is auto-corrected to 'speckit.{ext_id}.command'."""
         import yaml

tests/test_registrar_path_traversal.py

@@ -135,6 +135,105 @@ def test_rejects_traversal_names(self, tmp_path, bad_name):
         _assert_no_stray_files(tmp_path, Path(bad_name).name.replace("/", ""))
 
 
+ABS_OUTSIDE = "__ABS_OUTSIDE__"
+
+FILE_FIELD_PAYLOADS = [
+    "../outside.txt",
+    "../../outside.txt",
+    "commands/../../outside.txt",
+    "commands/../cmd.md",  # in-bounds after resolve; only the '..' check rejects it
+    "C:outside.txt",
+    ABS_OUTSIDE,
+]
+
+
+def _resolve_payload(bad_file: str, outside_file: Path) -> str:
+    """Map the absolute-path sentinel to the real, existing outside file.
+
+    Using the temp file's own absolute path (instead of ``/etc/passwd``)
+    guarantees the file exists on every platform — so the test fails if the
+    absolute-path guard regresses, rather than passing because the target
+    happens not to exist (e.g. on Windows runners).
+    """
+    return str(outside_file) if bad_file == ABS_OUTSIDE else bad_file
+
+
+def _assert_no_marker_leak(project: Path, marker: str) -> None:
+    """Fail if ``marker`` content was written into any file under ``project``."""
+    leaked = [
+        p for p in project.rglob("*")
+        if p.is_file() and marker in p.read_text(encoding="utf-8", errors="ignore")
+    ]
+    assert leaked == [], f"Outside file leaked into generated command: {leaked}"
+
+
+class TestCommandFileTraversal:
+    """The manifest ``file`` field must not read files outside source_dir.
+
+    Regression for GHSA-w5fv-7w9x-7fc5: ``register_commands`` read
+    ``source_dir / cmd_file`` with no containment check, so a manifest with
+    a traversal (``file: ../../../outside.txt``) or an absolute path read an
+    arbitrary host file verbatim into the generated agent command.
+    """
+
+    @pytest.mark.parametrize("bad_file", FILE_FIELD_PAYLOADS)
+    def test_claude_skips_traversal_in_file_field(self, tmp_path, bad_file):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".claude" / "skills").mkdir(parents=True)
+
+        outside_file = tmp_path / "outside.txt"
+        outside_file.write_text("OUTSIDE-FILE-MARKER", encoding="utf-8")
+
+        registrar = CommandRegistrar()
+        registered = registrar.register_commands(
+            "claude",
+            [{"name": "speckit.myext.hello", "file": _resolve_payload(bad_file, outside_file), "aliases": []}],
+            "myext",
+            ext_dir,
+            project,
+        )
+
+        assert registered == []
+        _assert_no_marker_leak(project, "OUTSIDE-FILE-MARKER")
+
+    @pytest.mark.parametrize("bad_file", FILE_FIELD_PAYLOADS)
+    def test_gemini_skips_traversal_in_file_field(self, tmp_path, bad_file):
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".gemini" / "commands").mkdir(parents=True)
+
+        outside_file = tmp_path / "outside.txt"
+        outside_file.write_text("OUTSIDE-FILE-MARKER", encoding="utf-8")
+
+        registrar = CommandRegistrar()
+        registered = registrar.register_commands(
+            "gemini",
+            [{"name": "speckit.myext.hello", "file": _resolve_payload(bad_file, outside_file), "aliases": []}],
+            "myext",
+            ext_dir,
+            project,
+        )
+
+        assert registered == []
+        _assert_no_marker_leak(project, "OUTSIDE-FILE-MARKER")
+
+    @pytest.mark.parametrize("bad_value", [None, 123, "", ["x"]])
+    def test_non_string_file_is_skipped(self, tmp_path, bad_value):
+        """A non-string/empty ``file`` must be skipped, not raise TypeError."""
+        project, ext_dir = _project_and_source(tmp_path)
+        (project / ".gemini" / "commands").mkdir(parents=True)
+
+        registrar = CommandRegistrar()
+        registered = registrar.register_commands(
+            "gemini",
+            [{"name": "speckit.myext.hello", "file": bad_value, "aliases": []}],
+            "myext",
+            ext_dir,
+            project,
+        )
+
+        assert registered == []
+
+
 class TestSafeRegistration:
     """Positive regression — well-formed names continue to register."""