diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 3c06418014..a85135f7c3 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -546,7 +546,22 @@ def register_commands( aliases = cmd_info.get("aliases", []) cmd_file = cmd_info["file"] - source_file = source_dir / cmd_file + # Guard against path traversal: reject absolute paths and ensure + # the resolved file stays within the source directory. Mirrors the + # containment checks already applied on the skill, preset, and + # restore paths (see extensions.py and presets/__init__.py) so a + # malicious manifest ``file`` field (e.g. ``../../../etc/passwd``) + # cannot read arbitrary host files into a generated command. + cmd_path = Path(cmd_file) + if cmd_path.is_absolute(): + continue + try: + source_root = source_dir.resolve() + source_file = (source_root / cmd_path).resolve() + source_file.relative_to(source_root) # raises ValueError if outside + except (OSError, ValueError): + continue + if not source_file.exists(): continue diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 42ba2fe888..8cf0bca37b 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -290,6 +290,22 @@ 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" + ) + cmd_file_path = Path(cmd_file) + if cmd_file_path.is_absolute() or ".." in cmd_file_path.parts: + raise ValidationError( + f"Invalid command 'file' '{cmd_file}': must be a relative path " + "within the extension directory (no absolute paths 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"]) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index e063571b14..5c885c3a3f 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -377,6 +377,26 @@ 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", + ["../../../etc/passwd", "../escape.md", "a/../../escape.md", "/etc/passwd"], + ) + 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') as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="Invalid command 'file'"): + 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 diff --git a/tests/test_registrar_path_traversal.py b/tests/test_registrar_path_traversal.py index fc423b4056..3de794743e 100644 --- a/tests/test_registrar_path_traversal.py +++ b/tests/test_registrar_path_traversal.py @@ -135,6 +135,72 @@ def test_rejects_traversal_names(self, tmp_path, bad_name): _assert_no_stray_files(tmp_path, Path(bad_name).name.replace("/", "")) +FILE_FIELD_PAYLOADS = [ + "../secret.txt", + "../../secret.txt", + "commands/../../secret.txt", + "/etc/passwd", +] + + +class TestCommandFileTraversal: + """The manifest ``file`` field must not read host 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 + ``file: ../../../etc/passwd`` (or an absolute path) leaked arbitrary host + files 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) + + secret = tmp_path / "secret.txt" + secret.write_text("TOP-SECRET-CREDENTIAL", encoding="utf-8") + + registrar = CommandRegistrar() + registered = registrar.register_commands( + "claude", + [{"name": "speckit.myext.hello", "file": bad_file, "aliases": []}], + "myext", + ext_dir, + project, + ) + + assert registered == [] + leaked = [ + p for p in (project).rglob("*") + if p.is_file() and "TOP-SECRET-CREDENTIAL" in p.read_text(encoding="utf-8", errors="ignore") + ] + assert leaked == [], f"Secret leaked into generated command: {leaked}" + + @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) + + secret = tmp_path / "secret.txt" + secret.write_text("TOP-SECRET-CREDENTIAL", encoding="utf-8") + + registrar = CommandRegistrar() + registered = registrar.register_commands( + "gemini", + [{"name": "speckit.myext.hello", "file": bad_file, "aliases": []}], + "myext", + ext_dir, + project, + ) + + assert registered == [] + leaked = [ + p for p in (project).rglob("*") + if p.is_file() and "TOP-SECRET-CREDENTIAL" in p.read_text(encoding="utf-8", errors="ignore") + ] + assert leaked == [], f"Secret leaked into generated command: {leaked}" + + class TestSafeRegistration: """Positive regression — well-formed names continue to register."""