diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index f6005f6012..c5f47bf9b2 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -324,7 +324,7 @@ def _validate(self): ) # Validate extension ID format - if not re.match(r"^[a-z0-9-]+$", ext["id"]): + if not re.fullmatch(r"[a-z0-9-]+", ext["id"]): raise ValidationError( f"Invalid extension ID '{ext['id']}': " "must be lowercase alphanumeric with hyphens only" diff --git a/src/specify_cli/extensions/_commands.py b/src/specify_cli/extensions/_commands.py index 08fe6747fc..8019656fb1 100644 --- a/src/specify_cli/extensions/_commands.py +++ b/src/specify_cli/extensions/_commands.py @@ -64,9 +64,9 @@ def _command_safe_id(raw_id: object, placeholder: str = "") -> str ``foo; rm -rf ~`` could otherwise be interpolated into a command we explicitly encourage the user to copy and run. ``rich.markup.escape`` only neutralizes Rich markup, not shell metacharacters, so it is not sufficient - here. Only emit the real ID when it matches the same + here. Only emit the real ID when its entire value matches the same lowercase-alphanumeric-and-hyphen rule ``ExtensionManifest`` enforces - (``^[a-z0-9-]+$``); otherwise fall back to a literal placeholder so the + (a full match of ``[a-z0-9-]+``); otherwise fall back to a literal placeholder so the printed command never carries catalog-controlled shell text. A leading hyphen is additionally rejected: an ID like ``--force`` satisfies @@ -78,7 +78,7 @@ def _command_safe_id(raw_id: object, placeholder: str = "") -> str text = str(raw_id) if text.startswith("-"): return placeholder - if VALID_EXTENSION_ARTIFACT_NAME_PATTERN.match(text): + if VALID_EXTENSION_ARTIFACT_NAME_PATTERN.fullmatch(text): return text return placeholder diff --git a/tests/specify_cli/extensions/test_command_add.py b/tests/specify_cli/extensions/test_command_add.py index f898fa052f..4b67706704 100644 --- a/tests/specify_cli/extensions/test_command_add.py +++ b/tests/specify_cli/extensions/test_command_add.py @@ -400,7 +400,8 @@ def test_add_discovery_only_error_suggests_resolved_id(self, tmp_path): # It must not emit the space-containing display name as the command target. assert "add Jira Integration --from" not in output - def test_add_discovery_only_error_neutralizes_unsafe_id(self, tmp_path): + @pytest.mark.parametrize("malicious_id", ["foo; rm -rf ~", "foo\n"]) + def test_add_discovery_only_error_neutralizes_unsafe_id(self, tmp_path, malicious_id): """A catalog-controlled ID with shell metacharacters must never be interpolated into the suggested command; it is replaced by a literal placeholder so copying the command can't execute injected shell text.""" @@ -413,7 +414,6 @@ def test_add_discovery_only_error_neutralizes_unsafe_id(self, tmp_path): (project_dir / ".specify").mkdir() (project_dir / ".specify" / "extensions").mkdir(parents=True) - malicious_id = "foo; rm -rf ~" mock_catalog = MagicMock() mock_catalog.get_extension_info.return_value = { "id": malicious_id, diff --git a/tests/specify_cli/extensions/test_command_search.py b/tests/specify_cli/extensions/test_command_search.py index c2e8e949f9..e44dad7a54 100644 --- a/tests/specify_cli/extensions/test_command_search.py +++ b/tests/specify_cli/extensions/test_command_search.py @@ -22,6 +22,39 @@ class TestExtensionSearchCLI: """CLI tests for ``specify extension search``.""" + @pytest.mark.parametrize("install_allowed", [True, False]) + @pytest.mark.parametrize( + ("extension_id", "command_id"), + [ + ("acme-thing", "acme-thing"), + ("acme-thing\n", ""), + ], + ) + def test_search_install_suggestion_uses_safe_catalog_id( + self, project_dir, install_allowed, extension_id, command_id + ): + entry = { + "id": extension_id, + "name": "Example", + "version": "1.0.0", + "description": "Example extension", + "_install_allowed": install_allowed, + "_catalog_name": "test-catalog", + } + + with patch.object(ExtensionCatalog, "_get_merged_extensions", return_value=[entry]), \ + patch("specify_cli.extensions._commands._require_specify_project", + return_value=project_dir): + result = CliRunner().invoke(app, ["extension", "search"]) + + assert result.exit_code == 0, result.output + output = " ".join(result.output.split()) + assert f"specify extension add {command_id}" in output + if not install_allowed: + assert f"specify extension add {command_id} --from " in output + if extension_id != command_id: + assert "acme-thing" not in result.output + @pytest.mark.parametrize("downloads", ["1500", "[/red]foo"]) def test_search_survives_non_numeric_downloads(self, temp_dir, downloads): """`specify extension search` must not abort when a catalog entry's diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 13350e2acb..7f9a784a8f 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -384,11 +384,25 @@ def test_invalid_utf8_bytes_raises_validation_error(self, temp_dir): with pytest.raises(ValidationError, match="not valid UTF-8"): ExtensionManifest(manifest_path) - def test_invalid_extension_id(self, temp_dir, valid_manifest_data): + @pytest.mark.parametrize( + "extension_id", + [ + "Invalid_ID", + "", + "../test-ext", + "test-ext\n", + "test-ext\r\n", + "test\next", + "test-ext ", + " test-ext", + ], + ) + def test_invalid_extension_id(self, temp_dir, valid_manifest_data, extension_id): """Test manifest with invalid extension ID format.""" import yaml - valid_manifest_data["extension"]["id"] = "Invalid_ID" # Uppercase not allowed + valid_manifest_data["extension"]["id"] = extension_id + valid_manifest_data["provides"]["commands"] = [] manifest_path = temp_dir / "extension.yml" with open(manifest_path, 'w') as f: