From fb1f49afc842fad133e6f21e4d4704433527b517 Mon Sep 17 00:00:00 2001 From: Alpamys Date: Thu, 26 Mar 2026 13:57:03 +0500 Subject: [PATCH] test: add missing coverage for _parse_json_array, _validate_example, SSRF guards Addresses TDD review findings: - Tests for _parse_json_array (markdown fences, NDJSON fallback, empty, invalid) - Tests for _validate_example (alpaca, sharegpt, chatml, unknown format) - Tests for _generate_server SSRF validation (scheme whitelist, remote HTTP block) - Tests for SGLang SSRF validation (URL model path rejection) - Fix vacuous test_server_provider_accepted assertion 1369 tests, 58.84% coverage. --- tests/test_server_generate.py | 187 +++++++++++++++++++++++++++++++++- tests/test_sglang_serve.py | 48 +++++++++ 2 files changed, 230 insertions(+), 5 deletions(-) diff --git a/tests/test_server_generate.py b/tests/test_server_generate.py index 63b096a..4d3276f 100644 --- a/tests/test_server_generate.py +++ b/tests/test_server_generate.py @@ -11,12 +11,20 @@ import pytest class TestServerProviderValidation: """Test that 'server' is a valid provider for soup data generate.""" - def test_server_provider_accepted(self): - """'server' should be a valid provider choice.""" + def test_server_provider_routes_without_error(self): + """'server' should route to _generate_server without error.""" + from soup_cli.commands.generate import _generate_batch - # _generate_batch dispatches to the correct provider - # Verify it doesn't reject 'server' as invalid - assert "server" in ("openai", "local", "server") + with mock_patch( + "soup_cli.commands.generate._generate_server", + return_value=[], + ): + result = _generate_batch( + prompt="test", count=1, fmt="alpaca", + provider="server", model_name="m", api_key=None, + api_base=None, temperature=0.8, seed_examples=[], + ) + assert result == [] def test_invalid_provider_rejected(self): """Invalid providers should cause an exit.""" @@ -305,3 +313,172 @@ class TestGenerateBatchRouting: ) mock_local.assert_called_once() + + +# ─── _parse_json_array Tests ───────────────────────────────────────────── + + +class TestParseJsonArray: + """Test the JSON array parsing function used by all providers.""" + + def test_parse_valid_json_array(self): + """Should parse a clean JSON array.""" + from soup_cli.commands.generate import _parse_json_array + + result = _parse_json_array('[{"a": 1}, {"b": 2}]') + assert len(result) == 2 + assert result[0]["a"] == 1 + + def test_parse_markdown_code_fence(self): + """Should strip markdown code fences.""" + from soup_cli.commands.generate import _parse_json_array + + content = '```json\n[{"instruction": "test", "output": "ok"}]\n```' + result = _parse_json_array(content) + assert len(result) == 1 + assert result[0]["instruction"] == "test" + + def test_parse_json_with_surrounding_text(self): + """Should extract JSON array from surrounding text.""" + from soup_cli.commands.generate import _parse_json_array + + content = 'Here are the examples:\n[{"a": 1}]\nDone!' + result = _parse_json_array(content) + assert len(result) == 1 + + def test_parse_ndjson_fallback(self): + """Should fall back to line-by-line JSON parsing.""" + from soup_cli.commands.generate import _parse_json_array + + content = '{"a": 1}\n{"b": 2}\n{"c": 3}' + result = _parse_json_array(content) + assert len(result) == 3 + + def test_parse_empty_array(self): + """Should return empty list for empty array.""" + from soup_cli.commands.generate import _parse_json_array + + assert _parse_json_array("[]") == [] + + def test_parse_invalid_json_returns_empty(self): + """Should return empty list for completely invalid JSON.""" + from soup_cli.commands.generate import _parse_json_array + + assert _parse_json_array("not json at all") == [] + + def test_parse_filters_non_dict_items(self): + """Should filter out non-dict items from the array.""" + from soup_cli.commands.generate import _parse_json_array + + result = _parse_json_array('[{"a": 1}, "string", 42, {"b": 2}]') + assert len(result) == 2 + + def test_parse_code_fence_without_language(self): + """Should strip code fences without language specifier.""" + from soup_cli.commands.generate import _parse_json_array + + content = '```\n[{"x": 1}]\n```' + result = _parse_json_array(content) + assert len(result) == 1 + + +# ─── _validate_example Tests ───────────────────────────────────────────── + + +class TestValidateExample: + """Test format validation for generated examples.""" + + def test_validate_alpaca_valid(self): + from soup_cli.commands.generate import _validate_example + + assert _validate_example({"instruction": "Q", "output": "A"}, "alpaca") + + def test_validate_alpaca_missing_output(self): + from soup_cli.commands.generate import _validate_example + + assert not _validate_example({"instruction": "Q"}, "alpaca") + + def test_validate_sharegpt_valid(self): + from soup_cli.commands.generate import _validate_example + + row = { + "conversations": [ + {"from": "human", "value": "Hi"}, + {"from": "gpt", "value": "Hello"}, + ] + } + assert _validate_example(row, "sharegpt") + + def test_validate_sharegpt_too_few_turns(self): + from soup_cli.commands.generate import _validate_example + + row = {"conversations": [{"from": "human", "value": "Hi"}]} + assert not _validate_example(row, "sharegpt") + + def test_validate_chatml_valid(self): + from soup_cli.commands.generate import _validate_example + + row = { + "messages": [ + {"role": "user", "content": "Hi"}, + {"role": "assistant", "content": "Hello"}, + ] + } + assert _validate_example(row, "chatml") + + def test_validate_chatml_empty_messages(self): + from soup_cli.commands.generate import _validate_example + + assert not _validate_example({"messages": []}, "chatml") + + def test_validate_unknown_format(self): + from soup_cli.commands.generate import _validate_example + + assert not _validate_example({"a": 1}, "unknown") + + +# ─── SSRF Validation Tests ─────────────────────────────────────────────── + + +class TestServerSSRFValidation: + """Test SSRF protection in _generate_server.""" + + def test_server_blocks_non_http_scheme(self): + """file:// scheme should be rejected.""" + from soup_cli.commands.generate import _generate_server + + with pytest.raises(ValueError, match="HTTP or HTTPS"): + _generate_server( + prompt="test", count=1, fmt="alpaca", + model_name="m", api_base="file:///etc/passwd", + temperature=0.8, seed_examples=[], + ) + + def test_server_blocks_remote_http(self): + """Remote HTTP (non-localhost) should be rejected.""" + from soup_cli.commands.generate import _generate_server + + with pytest.raises(ValueError, match="HTTPS for remote"): + _generate_server( + prompt="test", count=1, fmt="alpaca", + model_name="m", api_base="http://169.254.169.254/latest", + temperature=0.8, seed_examples=[], + ) + + def test_server_allows_localhost_http(self): + """HTTP to localhost should be allowed.""" + from soup_cli.commands.generate import _generate_server + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = { + "choices": [{"message": {"content": "[]"}}] + } + + with mock_patch("httpx.post", return_value=mock_response): + result = _generate_server( + prompt="test", count=1, fmt="alpaca", + model_name="m", api_base="http://127.0.0.1:8000", + temperature=0.8, seed_examples=[], + ) + assert result == [] diff --git a/tests/test_sglang_serve.py b/tests/test_sglang_serve.py index 2d599c6..2b8295e 100644 --- a/tests/test_sglang_serve.py +++ b/tests/test_sglang_serve.py @@ -3,6 +3,8 @@ from unittest.mock import MagicMock from unittest.mock import patch as mock_patch +import pytest + # ─── SGLang Detection Tests ────────────────────────────────────────────── @@ -254,3 +256,49 @@ class TestDoctorSGLang: pkg_names = [dep[1] for dep in DEPS] assert "librosa" in pkg_names + + +# ─── SGLang SSRF Validation Tests ──────────────────────────────────────── + + +class TestSGLangSSRF: + """Test SSRF protection in SGLang runtime creation.""" + + def test_create_runtime_blocks_http_model_path(self): + """HTTP URLs for model_path should be rejected.""" + mock_sgl = MagicMock() + + with mock_patch.dict("sys.modules", {"sglang": mock_sgl}): + from soup_cli.utils.sglang import create_sglang_runtime + + with pytest.raises(ValueError, match="not a URL"): + create_sglang_runtime( + model_path="http://evil.com/model", + ) + + def test_create_runtime_blocks_http_base_model(self): + """HTTP URLs for base_model should be rejected.""" + mock_sgl = MagicMock() + + with mock_patch.dict("sys.modules", {"sglang": mock_sgl}): + from soup_cli.utils.sglang import create_sglang_runtime + + with pytest.raises(ValueError, match="not a URL"): + create_sglang_runtime( + model_path="/path/to/adapter", + base_model="https://evil.com/model", + is_adapter=True, + ) + + def test_create_runtime_allows_hf_model_id(self): + """HuggingFace model IDs should be allowed.""" + mock_sgl = MagicMock() + mock_sgl.Runtime.return_value = MagicMock() + + with mock_patch.dict("sys.modules", {"sglang": mock_sgl}): + from soup_cli.utils.sglang import create_sglang_runtime + + runtime, name = create_sglang_runtime( + model_path="meta-llama/Llama-3.1-8B", + ) + assert name == "meta-llama/Llama-3.1-8B"