diff --git a/backend/api/routers/settings.py b/backend/api/routers/settings.py index fef74486..802b2a20 100644 --- a/backend/api/routers/settings.py +++ b/backend/api/routers/settings.py @@ -34,6 +34,7 @@ def sensitive_keys(settings: dict) -> set: async def probe_provider_key(provider: str, key: str, settings: dict | None = None) -> dict: import httpx from llm import _OPENAI_COMPAT_BASE_URLS + from llm.client import _validate_base_url started = time.perf_counter() try: @@ -53,53 +54,97 @@ async def probe_provider_key(provider: str, key: str, settings: dict | None = No "messages": [{"role": "user", "content": "ping"}], }, ) - status = "ok" if response.status_code in {200, 400} else "invalid_key" if response.status_code == 401 else "unreachable" + status = ( + "ok" + if response.status_code in {200, 400} + else "invalid_key" + if response.status_code == 401 + else "unreachable" + ) elif provider == "openai": response = await client.get( "https://api.openai.com/v1/models", headers={"Authorization": f"Bearer {key}"}, ) - status = "ok" if response.status_code == 200 else "invalid_key" if response.status_code == 401 else "unreachable" + status = ( + "ok" + if response.status_code == 200 + else "invalid_key" + if response.status_code == 401 + else "unreachable" + ) elif provider == "groq": response = await client.get( "https://api.groq.com/openai/v1/models", headers={"Authorization": f"Bearer {key}"}, ) - status = "ok" if response.status_code == 200 else "invalid_key" if response.status_code == 401 else "unreachable" + status = ( + "ok" + if response.status_code == 200 + else "invalid_key" + if response.status_code == 401 + else "unreachable" + ) elif provider == "gemini": response = await client.get( "https://generativelanguage.googleapis.com/v1beta/openai/models", headers={"Authorization": f"Bearer {key}"}, ) - status = "ok" if response.status_code == 200 else "invalid_key" if response.status_code in {401, 403} else "unreachable" + status = ( + "ok" + if response.status_code == 200 + else "invalid_key" + if response.status_code in {401, 403} + else "unreachable" + ) elif provider == "deepseek": response = await client.get( "https://api.deepseek.com/models", headers={"Authorization": f"Bearer {key}"}, ) - status = "ok" if response.status_code == 200 else "invalid_key" if response.status_code in {401, 403} else "unreachable" + status = ( + "ok" + if response.status_code == 200 + else "invalid_key" + if response.status_code in {401, 403} + else "unreachable" + ) elif provider in _OPENAI_COMPAT_BASE_URLS: response = await client.get( f"{_OPENAI_COMPAT_BASE_URLS[provider].rstrip('/')}/models", headers={"Authorization": f"Bearer {key}"}, ) - status = "ok" if response.status_code == 200 else "invalid_key" if response.status_code in {401, 403} else "unreachable" + status = ( + "ok" + if response.status_code == 200 + else "invalid_key" + if response.status_code in {401, 403} + else "unreachable" + ) elif provider == "azure": cfg = settings or {} - endpoint = str( - cfg.get("azure_openai_endpoint") - or os.environ.get("AZURE_OPENAI_ENDPOINT", "") - ).strip().rstrip("/") + endpoint = ( + str(cfg.get("azure_openai_endpoint") or os.environ.get("AZURE_OPENAI_ENDPOINT", "")) + .strip() + .rstrip("/") + ) if not endpoint: status = "unchecked" else: if not endpoint.endswith("/openai/v1"): endpoint = f"{endpoint}/openai/v1" + _validate_base_url(endpoint) response = await client.get( f"{endpoint}/models", headers={"api-key": key}, ) - status = "ok" if response.status_code == 200 else "invalid_key" if response.status_code in {401, 403} else "unreachable" + status = ( + "ok" + if response.status_code == 200 + else "invalid_key" + if response.status_code in {401, 403} + else "unreachable" + ) else: status = "unchecked" except Exception: @@ -110,6 +155,7 @@ async def probe_provider_key(provider: str, key: str, settings: dict | None = No async def list_provider_models(provider: str, key: str, settings: dict | None = None) -> list[str]: import httpx from llm import _OPENAI_COMPAT_BASE_URLS + from llm.client import _validate_base_url cfg = settings or {} headers = {"Authorization": f"Bearer {key}"} @@ -128,15 +174,15 @@ async def list_provider_models(provider: str, key: str, settings: dict | None = elif provider == "deepseek": url = "https://api.deepseek.com/models" elif provider == "azure": - endpoint = str( - cfg.get("azure_openai_endpoint") - or os.environ.get("AZURE_OPENAI_ENDPOINT", "") - ).strip().rstrip("/") + endpoint = ( + str(cfg.get("azure_openai_endpoint") or os.environ.get("AZURE_OPENAI_ENDPOINT", "")).strip().rstrip("/") + ) if not endpoint: return [] if not endpoint.endswith("/openai/v1"): endpoint = f"{endpoint}/openai/v1" url = f"{endpoint}/models" + _validate_base_url(url) headers = {"api-key": key} elif provider in _OPENAI_COMPAT_BASE_URLS: url = f"{_OPENAI_COMPAT_BASE_URLS[provider].rstrip('/')}/models" @@ -296,6 +342,7 @@ async def post_provider_models(provider: str, body: SettingsBody, repo: Reposito async def subscription_status(): """Install + login state for the subscription-CLI providers (no API key needed).""" from llm import SUBSCRIPTION_CLI_PROVIDERS, subscription_cli + out = {} for p in sorted(SUBSCRIPTION_CLI_PROVIDERS): s = subscription_cli.status(p) @@ -308,13 +355,18 @@ async def subscription_status(): async def subscription_login(provider: str): """Launch the CLI's own browser sign-in; the UI then polls subscription-status.""" from llm import SUBSCRIPTION_CLI_PROVIDERS, subscription_cli + if provider not in SUBSCRIPTION_CLI_PROVIDERS: raise HTTPException(status_code=400, detail="unknown subscription provider") try: return subscription_cli.login(provider) except subscription_cli.CliNotInstalled as exc: - return {"started": False, "error": "not_installed", - "hint": subscription_cli.install_hint(provider), "detail": str(exc)} + return { + "started": False, + "error": "not_installed", + "hint": subscription_cli.install_hint(provider), + "detail": str(exc), + } @router.post("/settings") async def save_cfg(body: SettingsBody, repo: Repository = Depends(get_repository)): diff --git a/backend/discovery/sources/apify.py b/backend/discovery/sources/apify.py index 936bd7ae..f6b0128b 100644 --- a/backend/discovery/sources/apify.py +++ b/backend/discovery/sources/apify.py @@ -1,11 +1,18 @@ from __future__ import annotations import asyncio +import re from dataclasses import dataclass, field import httpx from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_exponential +# Apify actor ids are "/" slugs (e.g. "apify/web-scraper"). Constrain +# the path segment so a configured actor can't reshape the acts/... URL (path +# traversal / host smuggling): each segment must start and end alphanumeric, with +# only ".", "_", "-" allowed inside, so "..", "../x", and "apify/" are rejected. +_ACTOR_RE = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?(?:/[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?)?$") + @dataclass class BoardScanResult: @@ -21,6 +28,8 @@ class BoardScanResult: reraise=True, ) async def run_actor(actor: str, inp: dict, token: str) -> list: + if not _ACTOR_RE.match(str(actor or "")): + raise ValueError(f"invalid apify actor: {actor!r}") async with httpx.AsyncClient(timeout=60) as cx: response = await cx.post( f"https://api.apify.com/v2/acts/{actor}/run-sync-get-dataset-items", diff --git a/backend/tests/test_apify_actor_validation.py b/backend/tests/test_apify_actor_validation.py new file mode 100644 index 00000000..b336d589 --- /dev/null +++ b/backend/tests/test_apify_actor_validation.py @@ -0,0 +1,53 @@ +"""The apify actor is interpolated into the request path, so it must be +allowlisted to stop a configured actor from reshaping the acts/... URL +(path traversal / host smuggling).""" + +import asyncio + +import pytest + +from discovery.sources.apify import _ACTOR_RE, run_actor + + +@pytest.mark.unit +@pytest.mark.parametrize( + "actor", + [ + "apify/web-scraper", + "apify.cheerio-scraper", + "user.actor-name_v2", + "apify", + ], +) +def test_actor_regex_accepts_well_formed(actor): + assert _ACTOR_RE.match(actor) + + +@pytest.mark.unit +@pytest.mark.parametrize( + "actor", + [ + "../apify", # path traversal + "a/b/c", # too deep for acts// + "", # empty + "evil host", # whitespace + "apify/../x", # traversal mid-string + "apify web", # space + ], +) +def test_actor_regex_rejects_malformed(actor): + assert not _ACTOR_RE.match(actor) + + +@pytest.mark.unit +def test_run_actor_raises_on_traversal_actor(): + # Validation runs before the HTTP client is used, so this never touches the + # network — it proves the guard is wired into run_actor, not a dead regex. + with pytest.raises(ValueError): + asyncio.run(run_actor("../evil", {}, "tok")) + + +@pytest.mark.unit +def test_run_actor_raises_on_empty_actor(): + with pytest.raises(ValueError): + asyncio.run(run_actor("", {}, "tok")) diff --git a/backend/tests/test_provider_endpoint_ssrf.py b/backend/tests/test_provider_endpoint_ssrf.py new file mode 100644 index 00000000..9a9fd081 --- /dev/null +++ b/backend/tests/test_provider_endpoint_ssrf.py @@ -0,0 +1,37 @@ +"""SSRF guard on the Azure OpenAI endpoint used by the provider probe/model-list. + +api/routers/settings.py rebuilds the Azure endpoint from user settings and +fetches it to validate the key / list models. That duplicated the LLM client's +URL construction but skipped the client's ``_validate_base_url`` SSRF guard, so a +loopback / metadata / private endpoint could be reached. These tests pin the fix: +a non-public Azure endpoint must never be probed (it resolves to "unreachable" +without any network call, because validation raises before the request). +""" + +import asyncio + +import pytest + +from api.routers.settings import probe_provider_key + + +@pytest.mark.unit +@pytest.mark.parametrize( + "endpoint", + [ + "http://127.0.0.1", # loopback + "http://localhost", # loopback name + "http://169.254.169.254", # cloud metadata + "http://10.0.0.5", # private + "http://192.168.1.10", # private + ], +) +def test_azure_non_public_endpoint_not_probed(endpoint): + result = asyncio.run(probe_provider_key("azure", "any-key", {"azure_openai_endpoint": endpoint})) + assert result["status"] == "unreachable" + + +@pytest.mark.unit +def test_azure_missing_endpoint_is_unchecked(): + result = asyncio.run(probe_provider_key("azure", "k", {})) + assert result["status"] == "unchecked"