From a172522d87188e160288b11c5d35b8e12a9e1adb Mon Sep 17 00:00:00 2001 From: Abhishek Kumbhar <159713562+Abhiiishek44@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:10:36 +0530 Subject: [PATCH] fix(integrations): prevent blank API integrations (#3840) * fix(integrations): validate unified API form fields * fix(integrations): validate API integration fields server-side --- src/integrations.py | 11 +++ static/js/settings.js | 6 +- tests/test_integrations_store_shape.py | 118 +++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/integrations.py b/src/integrations.py index 11fee99e7..54357511f 100644 --- a/src/integrations.py +++ b/src/integrations.py @@ -6,6 +6,7 @@ import re from typing import Dict, List, Optional, Any import httpx +from fastapi import HTTPException from core.atomic_io import atomic_write_json from core.platform_compat import safe_chmod @@ -258,6 +259,11 @@ def add_integration(data: Dict[str, Any]) -> Dict[str, Any]: integration.setdefault("name", "") integration.setdefault("base_url", "") + if not isinstance(integration.get("name"), str) or not integration["name"].strip(): + raise HTTPException(400, "Integration name is required") + if not isinstance(integration.get("base_url"), str) or not integration["base_url"].strip(): + raise HTTPException(400, "Integration base URL is required") + integrations = load_integrations() integrations.append(integration) save_integrations(integrations) @@ -266,6 +272,11 @@ def add_integration(data: Dict[str, Any]) -> Dict[str, Any]: def update_integration(integration_id: str, data: Dict[str, Any]) -> Optional[Dict[str, Any]]: """Update fields on an existing integration. Returns updated integration or None.""" + if "name" in data and (not isinstance(data["name"], str) or not data["name"].strip()): + raise HTTPException(400, "Integration name is required") + if "base_url" in data and (not isinstance(data["base_url"], str) or not data["base_url"].strip()): + raise HTTPException(400, "Integration base URL is required") + integrations = load_integrations() for item in integrations: if item.get("id") == integration_id: diff --git a/static/js/settings.js b/static/js/settings.js index 6d0906c9e..627919744 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -3644,7 +3644,11 @@ async function initUnifiedIntegrations() { el('uf-api-cancel').addEventListener('click', () => { formEl.style.display = 'none'; }); el('uf-api-save').addEventListener('click', async () => { const presetKey = preset.value || undefined; - const body = { name: name.value, base_url: url.value, auth_type: auth.value, auth_header: header.value, preset: presetKey }; + const nameValue = name.value.trim(); + const urlValue = url.value.trim(); + if (!nameValue) { el('uf-api-msg').textContent = 'Name required'; el('uf-api-msg').style.color = 'var(--red)'; return; } + if (!urlValue) { el('uf-api-msg').textContent = 'Base URL required'; el('uf-api-msg').style.color = 'var(--red)'; return; } + const body = { name: nameValue, base_url: urlValue, auth_type: auth.value, auth_header: header.value, preset: presetKey }; if (key.value) body.api_key = key.value; try { const u = _editId ? `/api/auth/integrations/${_editId}` : '/api/auth/integrations'; diff --git a/tests/test_integrations_store_shape.py b/tests/test_integrations_store_shape.py index 86bc940d4..3a4a88540 100644 --- a/tests/test_integrations_store_shape.py +++ b/tests/test_integrations_store_shape.py @@ -1,4 +1,8 @@ import json +import asyncio +from types import SimpleNamespace + +import pytest from src import integrations @@ -9,3 +13,117 @@ def test_load_integrations_skips_non_object_rows(tmp_path, monkeypatch): monkeypatch.setattr(integrations, "DATA_FILE", str(data_file)) assert integrations.load_integrations() == [{"id": "good", "name": "Good"}] + + +@pytest.fixture +def integrations_routes(tmp_path, monkeypatch): + fastapi = pytest.importorskip("fastapi") + from routes import auth_routes + + monkeypatch.setattr(integrations, "DATA_FILE", str(tmp_path / "integrations.json")) + monkeypatch.setattr(auth_routes, "migrate_from_settings", lambda: None) + + class _AuthManager: + def get_username_for_token(self, token): + return "admin" if token == "session-token" else None + + def is_admin(self, user): + return user == "admin" + + router = auth_routes.setup_auth_routes(_AuthManager()) + + def endpoint(path, method): + for route in router.routes: + if getattr(route, "path", "") == path and method in getattr(route, "methods", set()): + return route.endpoint + raise AssertionError(f"{method} {path} route not registered") + + return endpoint, auth_routes.SESSION_COOKIE, fastapi.HTTPException + + +class _JsonRequest(SimpleNamespace): + def __init__(self, body, session_cookie): + super().__init__( + cookies={session_cookie: "session-token"}, + client=SimpleNamespace(host="127.0.0.1"), + _body=body, + ) + + async def json(self): + return self._body + + +@pytest.mark.parametrize("blank_name", ["", " "]) +def test_create_integration_rejects_blank_name_without_persisting(integrations_routes, blank_name): + endpoint, session_cookie, http_exception = integrations_routes + create_integration = endpoint("/api/auth/integrations", "POST") + + with pytest.raises(http_exception) as exc: + asyncio.run(create_integration( + _JsonRequest({"name": blank_name, "base_url": "https://example.test"}, session_cookie) + )) + + assert exc.value.status_code == 400 + assert exc.value.detail == "Integration name is required" + assert integrations.load_integrations() == [] + + +@pytest.mark.parametrize("blank_base_url", ["", " "]) +def test_create_integration_rejects_blank_base_url_without_persisting(integrations_routes, blank_base_url): + endpoint, session_cookie, http_exception = integrations_routes + create_integration = endpoint("/api/auth/integrations", "POST") + + with pytest.raises(http_exception) as exc: + asyncio.run(create_integration( + _JsonRequest({"name": "Example", "base_url": blank_base_url}, session_cookie) + )) + + assert exc.value.status_code == 400 + assert exc.value.detail == "Integration base URL is required" + assert integrations.load_integrations() == [] + + +@pytest.mark.parametrize("blank_name", ["", " "]) +def test_update_integration_rejects_blank_name_without_changing_existing(integrations_routes, blank_name): + endpoint, session_cookie, http_exception = integrations_routes + update_integration = endpoint("/api/auth/integrations/{integration_id}", "PUT") + integrations.save_integrations([ + { + "id": "existing", + "name": "Original", + "base_url": "https://example.test", + } + ]) + + with pytest.raises(http_exception) as exc: + asyncio.run(update_integration( + integration_id="existing", + request=_JsonRequest({"name": blank_name}, session_cookie), + )) + + assert exc.value.status_code == 400 + assert exc.value.detail == "Integration name is required" + assert integrations.load_integrations()[0]["name"] == "Original" + + +@pytest.mark.parametrize("blank_base_url", ["", " "]) +def test_update_integration_rejects_blank_base_url_without_changing_existing(integrations_routes, blank_base_url): + endpoint, session_cookie, http_exception = integrations_routes + update_integration = endpoint("/api/auth/integrations/{integration_id}", "PUT") + integrations.save_integrations([ + { + "id": "existing", + "name": "Original", + "base_url": "https://example.test", + } + ]) + + with pytest.raises(http_exception) as exc: + asyncio.run(update_integration( + integration_id="existing", + request=_JsonRequest({"base_url": blank_base_url}, session_cookie), + )) + + assert exc.value.status_code == 400 + assert exc.value.detail == "Integration base URL is required" + assert integrations.load_integrations()[0]["base_url"] == "https://example.test"