From ffed94cdced01704e895820e27518905b419a03e Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Tue, 18 Aug 2026 09:18:37 +0530 Subject: [PATCH 01/12] feat: add header decorator to /guardrails --- backend/app/services/llm/guardrails.py | 36 ++++++++++--------- .../app/tests/services/llm/test_guardrails.py | 12 +++++-- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index 5b85e08b8..f3a70d2ec 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -13,6 +13,23 @@ logger = logging.getLogger(__name__) +def _guardrails_headers( + organization_id: int | None, project_id: int | None +) -> dict[str, str]: + """Guardrails is internal-only: tenant travels in headers set from the + auth context, never from caller-supplied body/query fields.""" + headers = { + "accept": "application/json", + "Authorization": f"Bearer {settings.KAAPI_GUARDRAILS_AUTH}", + "Content-Type": "application/json", + } + if organization_id is not None: + headers["X-ORGANIZATION-ID"] = str(organization_id) + if project_id is not None: + headers["X-PROJECT-ID"] = str(project_id) + return headers + + @dataclass class GuardrailsOutcome: """Result of a single guardrails service call, in domain-agnostic form. @@ -166,8 +183,6 @@ def run_guardrails_validation( payload = { "request_id": str(job_id), - "project_id": project_id, - "organization_id": organization_id, "input": input_text, "validators": validators, } @@ -175,11 +190,7 @@ def run_guardrails_validation( if output_text is not None: payload["output"] = output_text - headers = { - "accept": "application/json", - "Authorization": f"Bearer {settings.KAAPI_GUARDRAILS_AUTH}", - "Content-Type": "application/json", - } + headers = _guardrails_headers(organization_id, project_id) url = f"{settings.KAAPI_GUARDRAILS_URL}/" payload_bytes = json.dumps(payload).encode() @@ -247,21 +258,14 @@ def list_validators_config( if not input_validator_config_ids and not output_validator_config_ids: return [], [] - headers = { - "accept": "application/json", - "Authorization": f"Bearer {settings.KAAPI_GUARDRAILS_AUTH}", - "Content-Type": "application/json", - } + headers = _guardrails_headers(organization_id, project_id) endpoint = f"{settings.KAAPI_GUARDRAILS_URL}/validators/configs/" def _build_params(validator_ids: list[UUID]) -> dict[str, Any]: - params = { - "organization_id": organization_id, - "project_id": project_id, + return { "ids": [str(validator_config_id) for validator_config_id in validator_ids], } - return {key: value for key, value in params.items() if value is not None} try: with httpx.Client(timeout=10.0) as client: diff --git a/backend/app/tests/services/llm/test_guardrails.py b/backend/app/tests/services/llm/test_guardrails.py index 22004d179..ef96280f3 100644 --- a/backend/app/tests/services/llm/test_guardrails.py +++ b/backend/app/tests/services/llm/test_guardrails.py @@ -58,8 +58,10 @@ def test_run_guardrails_validation_success(mock_client_cls) -> None: assert kwargs["json"]["input"] == TEST_TEXT assert kwargs["json"]["validators"] == TEST_CONFIG assert kwargs["json"]["request_id"] == str(TEST_JOB_ID) - assert kwargs["json"]["project_id"] == TEST_PROJECT_ID - assert kwargs["json"]["organization_id"] == TEST_ORGANIZATION_ID + assert "project_id" not in kwargs["json"] + assert "organization_id" not in kwargs["json"] + assert kwargs["headers"]["X-PROJECT-ID"] == str(TEST_PROJECT_ID) + assert kwargs["headers"]["X-ORGANIZATION-ID"] == str(TEST_ORGANIZATION_ID) assert kwargs["params"]["suppress_pass_logs"] == "true" assert kwargs["headers"]["Authorization"].startswith("Bearer ") assert kwargs["headers"]["Content-Type"] == "application/json" @@ -254,6 +256,8 @@ def test_list_validators_config_fetches_input_and_output_by_refs( assert second_call_kwargs["params"]["ids"] == [ str(v.validator_config_id) for v in output_validator_configs ] + assert first_call_kwargs["headers"]["X-ORGANIZATION-ID"] == "1" + assert first_call_kwargs["headers"]["X-PROJECT-ID"] == "1" @patch("app.services.llm.guardrails.httpx.Client") @@ -273,7 +277,7 @@ def test_list_validators_config_empty_short_circuits_without_http( @patch("app.services.llm.guardrails.httpx.Client") -def test_list_validators_config_omits_none_query_params(mock_client_cls) -> None: +def test_list_validators_config_omits_none_tenant(mock_client_cls) -> None: input_validator_configs = [Validator(validator_config_id=uuid.uuid4())] mock_response = MagicMock() @@ -297,6 +301,8 @@ def test_list_validators_config_omits_none_query_params(mock_client_cls) -> None ] assert "organization_id" not in kwargs["params"] assert "project_id" not in kwargs["params"] + assert "X-ORGANIZATION-ID" not in kwargs["headers"] + assert "X-PROJECT-ID" not in kwargs["headers"] @patch("app.services.llm.guardrails.httpx.Client") From 707cfa576fe134ed9055656c3abf0b86ea4bebb6 Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Tue, 18 Aug 2026 10:58:39 +0530 Subject: [PATCH 02/12] feat(guardrails): proxy management API and fail closed on auth errors Expose the internal kaapi-guardrails management API (validator catalogue, ban lists, LLM prompt configs, validator configs) through 16 passthrough routes behind project auth. Tenant travels only in X-ORGANIZATION-ID / X-PROJECT-ID headers set from the auth context; 401/403/422 from the service fail the job instead of bypassing, with client-visible errors sanitized to status codes only. Co-Authored-By: Claude Fable 5 --- backend/app/api/routes/guardrails.py | 301 +++++++++++++++++- backend/app/services/llm/guardrails.py | 112 ++++++- .../app/tests/api/routes/test_guardrails.py | 194 ++++++++++- .../app/tests/services/llm/test_guardrails.py | 52 ++- docs/wiki/modules/llm-call.md | 7 +- 5 files changed, 659 insertions(+), 7 deletions(-) diff --git a/backend/app/api/routes/guardrails.py b/backend/app/api/routes/guardrails.py index 3f0db3ad1..874b6349a 100644 --- a/backend/app/api/routes/guardrails.py +++ b/backend/app/api/routes/guardrails.py @@ -1,7 +1,9 @@ import logging +from typing import Annotated, Any from uuid import UUID -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Query, Response +from fastapi.responses import JSONResponse from opentelemetry import trace from app.api.deps import AuthContextDep, SessionDep @@ -17,6 +19,7 @@ GuardrailsRequest, ) from app.services.guardrails.jobs import start_job +from app.services.llm.guardrails import proxy_guardrails_request from app.utils import APIResponse, load_description, validate_callback_url logger = logging.getLogger(__name__) @@ -91,6 +94,300 @@ def apply_guardrails_endpoint( ) +def _upstream_response(status_code: int, payload: Any) -> Response: + """An empty upstream body must stay empty (204s cannot carry one).""" + if payload is None: + return Response(status_code=status_code) + return JSONResponse(status_code=status_code, content=payload) + + +BAN_LISTS_PATH = "/ban_lists" +LLM_PROMPT_CONFIGS_PATH = "/llm_prompt_configs" +VALIDATOR_CONFIGS_PATH = "/validators/configs" + + +# ROUTE ORDERING: every fixed single-segment path below collides with the +# GET /guardrails/{job_id} route declared after this section. FastAPI matches in +# declaration order and does not fall through when {job_id} fails UUID parsing, +# so these must stay above it. + + +@router.get( + "/guardrails", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def list_guardrails_validator_types(_current_user: AuthContextDep) -> Response: + """List the validator types supported upstream and their JSON schemas.""" + status_code, payload = proxy_guardrails_request( + "GET", + "/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + +@router.post( + "/guardrails/ban_lists", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def create_guardrails_ban_list( + _current_user: AuthContextDep, body: dict[str, Any] +) -> Response: + status_code, payload = proxy_guardrails_request( + "POST", + f"{BAN_LISTS_PATH}/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + json_body=body, + ) + return _upstream_response(status_code, payload) + + +@router.get( + "/guardrails/ban_lists", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def list_guardrails_ban_lists( + _current_user: AuthContextDep, + offset: Annotated[int, Query(ge=0)] = 0, + limit: Annotated[int | None, Query(ge=1, le=100)] = None, +) -> Response: + status_code, payload = proxy_guardrails_request( + "GET", + f"{BAN_LISTS_PATH}/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + params={"offset": offset, "limit": limit}, + ) + return _upstream_response(status_code, payload) + + +@router.post( + "/guardrails/llm_prompt_configs", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def create_guardrails_llm_prompt_config( + _current_user: AuthContextDep, body: dict[str, Any] +) -> Response: + status_code, payload = proxy_guardrails_request( + "POST", + f"{LLM_PROMPT_CONFIGS_PATH}/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + json_body=body, + ) + return _upstream_response(status_code, payload) + + +@router.get( + "/guardrails/llm_prompt_configs", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def list_guardrails_llm_prompt_configs( + _current_user: AuthContextDep, + validator_name: str | None = None, + offset: Annotated[int, Query(ge=0)] = 0, + limit: Annotated[int | None, Query(ge=1, le=100)] = None, +) -> Response: + status_code, payload = proxy_guardrails_request( + "GET", + f"{LLM_PROMPT_CONFIGS_PATH}/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + params={"validator_name": validator_name, "offset": offset, "limit": limit}, + ) + return _upstream_response(status_code, payload) + + +@router.post( + "/guardrails/validators/configs", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def create_guardrails_validator_config( + _current_user: AuthContextDep, body: dict[str, Any] +) -> Response: + status_code, payload = proxy_guardrails_request( + "POST", + f"{VALIDATOR_CONFIGS_PATH}/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + json_body=body, + ) + return _upstream_response(status_code, payload) + + +@router.get( + "/guardrails/validators/configs", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def list_guardrails_validator_configs( + _current_user: AuthContextDep, + ids: Annotated[list[UUID] | None, Query()] = None, + stage: str | None = None, + type: str | None = None, +) -> Response: + status_code, payload = proxy_guardrails_request( + "GET", + f"{VALIDATOR_CONFIGS_PATH}/", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + params={ + "ids": [str(config_id) for config_id in ids] if ids else None, + "stage": stage, + "type": type, + }, + ) + return _upstream_response(status_code, payload) + + +@router.get( + "/guardrails/validators/configs/{config_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def get_guardrails_validator_config( + _current_user: AuthContextDep, config_id: UUID +) -> Response: + status_code, payload = proxy_guardrails_request( + "GET", + f"{VALIDATOR_CONFIGS_PATH}/{config_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + +@router.patch( + "/guardrails/validators/configs/{config_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def update_guardrails_validator_config( + _current_user: AuthContextDep, config_id: UUID, body: dict[str, Any] +) -> Response: + status_code, payload = proxy_guardrails_request( + "PATCH", + f"{VALIDATOR_CONFIGS_PATH}/{config_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + json_body=body, + ) + return _upstream_response(status_code, payload) + + +@router.delete( + "/guardrails/validators/configs/{config_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def delete_guardrails_validator_config( + _current_user: AuthContextDep, config_id: UUID +) -> Response: + status_code, payload = proxy_guardrails_request( + "DELETE", + f"{VALIDATOR_CONFIGS_PATH}/{config_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + +@router.get( + "/guardrails/ban_lists/{ban_list_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def get_guardrails_ban_list( + _current_user: AuthContextDep, ban_list_id: UUID +) -> Response: + status_code, payload = proxy_guardrails_request( + "GET", + f"{BAN_LISTS_PATH}/{ban_list_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + +@router.patch( + "/guardrails/ban_lists/{ban_list_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def update_guardrails_ban_list( + _current_user: AuthContextDep, ban_list_id: UUID, body: dict[str, Any] +) -> Response: + status_code, payload = proxy_guardrails_request( + "PATCH", + f"{BAN_LISTS_PATH}/{ban_list_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + json_body=body, + ) + return _upstream_response(status_code, payload) + + +@router.delete( + "/guardrails/ban_lists/{ban_list_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def delete_guardrails_ban_list( + _current_user: AuthContextDep, ban_list_id: UUID +) -> Response: + status_code, payload = proxy_guardrails_request( + "DELETE", + f"{BAN_LISTS_PATH}/{ban_list_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + +@router.get( + "/guardrails/llm_prompt_configs/{prompt_config_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def get_guardrails_llm_prompt_config( + _current_user: AuthContextDep, prompt_config_id: UUID +) -> Response: + status_code, payload = proxy_guardrails_request( + "GET", + f"{LLM_PROMPT_CONFIGS_PATH}/{prompt_config_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + +@router.patch( + "/guardrails/llm_prompt_configs/{prompt_config_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def update_guardrails_llm_prompt_config( + _current_user: AuthContextDep, prompt_config_id: UUID, body: dict[str, Any] +) -> Response: + status_code, payload = proxy_guardrails_request( + "PATCH", + f"{LLM_PROMPT_CONFIGS_PATH}/{prompt_config_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + json_body=body, + ) + return _upstream_response(status_code, payload) + + +@router.delete( + "/guardrails/llm_prompt_configs/{prompt_config_id}", + dependencies=[Depends(require_permission(Permission.REQUIRE_PROJECT))], +) +def delete_guardrails_llm_prompt_config( + _current_user: AuthContextDep, prompt_config_id: UUID +) -> Response: + status_code, payload = proxy_guardrails_request( + "DELETE", + f"{LLM_PROMPT_CONFIGS_PATH}/{prompt_config_id}", + organization_id=_current_user.organization_.id, + project_id=_current_user.project_.id, + ) + return _upstream_response(status_code, payload) + + @router.get( "/guardrails/{job_id}", response_model=APIResponse[GuardrailsJobPublic], @@ -112,7 +409,7 @@ def get_guardrails_job_status( tag="guardrails", system="guardrails", lifecycle="api.guardrails.status", - job_id=job_id, + job_id=str(job_id), project_id=project_id, organization_id=_current_user.organization_.id, ): diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index f3a70d2ec..09e31e2c9 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -1,11 +1,13 @@ import json import logging import time +from collections.abc import Sequence from dataclasses import dataclass, field -from typing import Any +from typing import Any, TypeGuard from uuid import UUID import httpx +from fastapi import HTTPException from app.core.config import settings from app.models.llm.request import Validator @@ -13,6 +15,9 @@ logger = logging.getLogger(__name__) +GUARDRAILS_PROXY_TIMEOUT_SECONDS = 30.0 + + def _guardrails_headers( organization_id: int | None, project_id: int | None ) -> dict[str, str]: @@ -30,6 +35,81 @@ def _guardrails_headers( return headers +def proxy_guardrails_request( + method: str, + path: str, + *, + organization_id: int, + project_id: int, + params: dict[str, Any] | None = None, + json_body: dict[str, Any] | None = None, +) -> tuple[int, Any]: + """Forward a management-API call to the guardrails service verbatim. + + No fail-open: these are synchronous CRUD calls, so upstream status codes and + bodies (including its 422s) are handed back to the caller unchanged. + """ + url = f"{settings.KAAPI_GUARDRAILS_URL}{path}" + headers = _guardrails_headers(organization_id, project_id) + # Unset query params must be omitted, not sent as empty values. + query = {k: v for k, v in (params or {}).items() if v is not None} + + logger.info( + f"[proxy_guardrails_request] Forwarding to guardrails | method: {method}, " + f"url: {url}, organization_id: {organization_id}, project_id: {project_id}" + ) + + try: + with httpx.Client(timeout=GUARDRAILS_PROXY_TIMEOUT_SECONDS) as client: + response = client.request( + method, url, params=query, json=json_body, headers=headers + ) + except httpx.RequestError as e: + logger.error( + f"[proxy_guardrails_request] [KAAPI] Could not reach the guardrails service — " + f"retry shortly, and contact Kaapi if it persists (code: {type(e).__name__}) | " + f"method: {method}, url: {url}", + exc_info=True, + ) + raise HTTPException( + status_code=502, detail="Guardrails service unavailable" + ) from e + + if response.status_code >= 500: + logger.error( + f"[proxy_guardrails_request] [GUARDRAILS] Upstream error " + f"(code: {response.status_code}) | method: {method}, url: {url}" + ) + elif response.status_code >= 400: + logger.warning( + f"[proxy_guardrails_request] [GUARDRAILS] Request rejected " + f"(code: {response.status_code}) | method: {method}, url: {url}" + ) + + if not response.content: + return response.status_code, None + try: + return response.status_code, response.json() + except ValueError: + logger.error( + f"[proxy_guardrails_request] [GUARDRAILS] Non-JSON response body " + f"(code: {response.status_code}) | method: {method}, url: {url}" + ) + raise HTTPException( + status_code=502, detail="Guardrails service returned an invalid response" + ) from None + + +def _is_auth_error(e: Exception) -> TypeGuard[httpx.HTTPStatusError]: + # 422 included: a missing/invalid tenant header is a backend bug, not a + # transient outage, so it must not fall open like one. + return isinstance(e, httpx.HTTPStatusError) and e.response.status_code in ( + 401, + 403, + 422, + ) + + @dataclass class GuardrailsOutcome: """Result of a single guardrails service call, in domain-agnostic form. @@ -151,7 +231,7 @@ def apply_guardrails( def run_guardrails_validation( input_text: str, - guardrail_config: list[Validator | dict[str, Any]], + guardrail_config: Sequence[Validator | dict[str, Any]], job_id: UUID, project_id: int | None, organization_id: int | None, @@ -219,6 +299,21 @@ def run_guardrails_validation( return response.json() except Exception as e: elapsed_ms = int((time.monotonic() - started) * 1000) + if _is_auth_error(e): + # Auth failure means a broken deploy (token/IP mismatch), not a + # transient outage — fail the job instead of silently bypassing. + logger.error( + f"[run_guardrails_validation] Guardrails auth failed. " + f"job_id={job_id}, elapsed_ms={elapsed_ms}, error={e}" + ) + status_code = e.response.status_code + return { + "success": False, + "bypassed": False, + # Status only — str(e) embeds the internal service URL and this + # string is client-visible via job.error_message. + "error": f"Guardrails service rejected the request (HTTP {status_code})", + } logger.warning( f"[run_guardrails_validation] Service unavailable. Bypassing guardrails. " f"job_id={job_id}, elapsed_ms={elapsed_ms}, error={e}" @@ -304,6 +399,19 @@ def _fetch_by_ids(validator_ids: list[UUID]) -> list[dict[str, Any]]: return input_guardrails, output_guardrails except Exception as e: + if _is_auth_error(e): + # Propagate so job executors fail the job instead of running + # without guardrails on a misconfigured token/IP. Sanitized: + # str(e) embeds the internal service URL and the executors put + # this message into the client-visible job error. + logger.error( + f"[list_validators_config] Guardrails auth failed | " + f"organization_id={organization_id}, project_id={project_id}, " + f"endpoint={endpoint}, error={e}" + ) + raise ValueError( + f"Guardrails config fetch rejected (HTTP {e.response.status_code})" + ) from e logger.warning( "[list_validators_config] Guardrails service unavailable or invalid response. " "Proceeding without input/output guardrails. " diff --git a/backend/app/tests/api/routes/test_guardrails.py b/backend/app/tests/api/routes/test_guardrails.py index 8b34a67ce..66f97c70c 100644 --- a/backend/app/tests/api/routes/test_guardrails.py +++ b/backend/app/tests/api/routes/test_guardrails.py @@ -1,6 +1,10 @@ -from unittest.mock import patch +from contextlib import contextmanager +from typing import Any +from unittest.mock import MagicMock, patch from uuid import uuid4 +import httpx +import pytest from fastapi.testclient import TestClient from sqlmodel import Session @@ -218,9 +222,197 @@ def test_get_guardrails_failed_returns_error_message( assert data["guardrails_response"] is None +# ---------- management-API proxy routes ---------- + + +class TestProxyPassthrough: + @pytest.mark.parametrize( + "status_code, body", + [ + (200, {"success": True, "data": [{"name": "pii"}]}), + (422, {"detail": [{"loc": ["body", "name"], "msg": "field required"}]}), + (404, {"detail": "Ban list not found"}), + ], + ) + def test_upstream_status_and_body_echoed( + self, + client: TestClient, + user_api_key_header: dict[str, str], + status_code: int, + body: dict[str, Any], + ) -> None: + with _mock_upstream(status_code=status_code, json_body=body): + resp = client.get("api/v1/guardrails", headers=user_api_key_header) + + assert resp.status_code == status_code + assert resp.json() == body + + def test_empty_upstream_body_returns_status_with_no_body( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(status_code=204, content=b""): + resp = client.delete( + f"api/v1/guardrails/ban_lists/{uuid4()}", headers=user_api_key_header + ) + + assert resp.status_code == 204 + assert resp.content == b"" + + def test_create_echoes_upstream_status_and_body( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + created = {"id": str(uuid4()), "validator_name": "toxicity"} + # Upstream create routes return FastAPI's default 200. + with _mock_upstream(status_code=200, json_body=created) as calls: + resp = client.post( + "api/v1/guardrails/llm_prompt_configs", + json={"validator_name": "toxicity", "prompt": "be nice"}, + headers=user_api_key_header, + ) + + assert resp.status_code == 200 + assert resp.json() == created + assert calls[0]["kwargs"]["json"] == { + "validator_name": "toxicity", + "prompt": "be nice", + } + + def test_connect_error_returns_502( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(raises=httpx.ConnectError("connection refused")): + resp = client.get("api/v1/guardrails", headers=user_api_key_header) + + assert resp.status_code == 502 + assert resp.json()["error"] == "Guardrails service unavailable" + + def test_non_json_upstream_body_returns_502( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(status_code=200, content=b"gateway"): + resp = client.get("api/v1/guardrails", headers=user_api_key_header) + + assert resp.status_code == 502 + assert resp.json()["error"] == "Guardrails service returned an invalid response" + + +class TestProxyForwardedRequest: + def test_unset_limit_is_dropped_from_forwarded_params( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + client.get("api/v1/guardrails/ban_lists", headers=user_api_key_header) + + params = calls[0]["kwargs"]["params"] + assert params == {"offset": 0} + + def test_ids_forwarded_as_list( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + first, second = str(uuid4()), str(uuid4()) + with _mock_upstream(json_body={"data": []}) as calls: + client.get( + f"api/v1/guardrails/validators/configs?ids={first}&ids={second}" + "&stage=input", + headers=user_api_key_header, + ) + + assert calls[0]["kwargs"]["params"] == { + "ids": [first, second], + "stage": "input", + } + + def test_tenant_headers_come_from_auth_context_not_request( + self, + client: TestClient, + user_api_key: TestAuthContext, + user_api_key_header: dict[str, str], + ) -> None: + with _mock_upstream(status_code=200, json_body={"id": str(uuid4())}) as calls: + client.post( + "api/v1/guardrails/ban_lists?organization_id=999", + json={"name": "slurs", "organization_id": 999, "project_id": 888}, + headers=user_api_key_header, + ) + + headers = calls[0]["kwargs"]["headers"] + assert headers["X-ORGANIZATION-ID"] == str(user_api_key.organization_id) + assert headers["X-PROJECT-ID"] == str(user_api_key.project_id) + + def test_ban_list_detail_path_forwarded( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + ban_list_id = uuid4() + with _mock_upstream(json_body={"id": str(ban_list_id)}) as calls: + resp = client.patch( + f"api/v1/guardrails/ban_lists/{ban_list_id}", + json={"name": "renamed"}, + headers=user_api_key_header, + ) + + assert resp.status_code == 200 + method, url = calls[0]["args"] + assert method == "PATCH" + assert url.endswith(f"/ban_lists/{ban_list_id}") + + +class TestProxyRouteOrdering: + def test_ban_lists_list_route_wins_over_job_status_route( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + resp = client.get( + "api/v1/guardrails/ban_lists", headers=user_api_key_header + ) + + assert resp.status_code == 200 + assert calls[0]["args"][1].endswith("/ban_lists/") + + +def test_list_ban_lists_requires_auth(client: TestClient) -> None: + resp = client.get("api/v1/guardrails/ban_lists") + assert resp.status_code in (401, 403) + + # ---------- helpers ---------- +@contextmanager +def _mock_upstream( + *, + status_code: int = 200, + json_body: Any = None, + content: bytes | None = None, + raises: Exception | None = None, +): + """Stub the guardrails HTTP boundary; yields the recorded client.request calls.""" + calls: list[dict[str, Any]] = [] + + response = MagicMock() + response.status_code = status_code + if content is None: + import json as _json + + response.content = _json.dumps(json_body).encode() + response.json.return_value = json_body + else: + response.content = content + response.json.side_effect = ValueError("not json") + + def _request(*args: Any, **kwargs: Any): + calls.append({"args": args, "kwargs": kwargs}) + if raises is not None: + raise raises + return response + + client = MagicMock() + client.request.side_effect = _request + + with patch("app.services.llm.guardrails.httpx.Client") as mock_client_cls: + mock_client_cls.return_value.__enter__.return_value = client + yield calls + + def _stub_job() -> Job: """Minimal in-memory Job for routes that only read id/status/timestamps.""" from datetime import datetime, timezone diff --git a/backend/app/tests/services/llm/test_guardrails.py b/backend/app/tests/services/llm/test_guardrails.py index ef96280f3..87a4c261d 100644 --- a/backend/app/tests/services/llm/test_guardrails.py +++ b/backend/app/tests/services/llm/test_guardrails.py @@ -70,8 +70,9 @@ def test_run_guardrails_validation_success(mock_client_cls) -> None: @patch("app.services.llm.guardrails.httpx.Client") def test_run_guardrails_validation_http_error_bypasses(mock_client_cls) -> None: mock_response = MagicMock() + mock_response.status_code = 500 mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( - "bad", request=None, response=None + "bad", request=MagicMock(), response=mock_response ) mock_client = MagicMock() @@ -91,6 +92,55 @@ def test_run_guardrails_validation_http_error_bypasses(mock_client_cls) -> None: assert result["data"]["safe_text"] == TEST_TEXT +@pytest.mark.parametrize("status_code", [401, 403, 422]) +@patch("app.services.llm.guardrails.httpx.Client") +def test_run_guardrails_validation_auth_error_fails_closed( + mock_client_cls, status_code +) -> None: + mock_response = MagicMock() + mock_response.status_code = status_code + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "unauthorized", request=MagicMock(), response=mock_response + ) + + mock_client = MagicMock() + mock_client.post.return_value = mock_response + mock_client_cls.return_value.__enter__.return_value = mock_client + + result = run_guardrails_validation( + TEST_TEXT, + TEST_CONFIG, + TEST_JOB_ID, + TEST_PROJECT_ID, + TEST_ORGANIZATION_ID, + ) + + assert result["success"] is False + assert result.get("bypassed") is False + assert "rejected the request" in result["error"] + + +@patch("app.services.llm.guardrails.httpx.Client") +def test_list_validators_config_auth_error_raises(mock_client_cls) -> None: + mock_response = MagicMock() + mock_response.status_code = 403 + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "forbidden", request=MagicMock(), response=mock_response + ) + + mock_client = MagicMock() + mock_client.get.return_value = mock_response + mock_client_cls.return_value.__enter__.return_value = mock_client + + with pytest.raises(ValueError, match=r"rejected \(HTTP 403\)"): + list_validators_config( + input_validator_configs=[Validator(validator_config_id=uuid.uuid4())], + output_validator_configs=[], + organization_id=1, + project_id=1, + ) + + @patch("app.services.llm.guardrails.httpx.Client") def test_run_guardrails_validation_uses_settings(mock_client_cls) -> None: mock_response = MagicMock() diff --git a/docs/wiki/modules/llm-call.md b/docs/wiki/modules/llm-call.md index 462fbc048..4b8c728ba 100644 --- a/docs/wiki/modules/llm-call.md +++ b/docs/wiki/modules/llm-call.md @@ -10,7 +10,12 @@ All paths relative to `backend/app/`. - `api/routes/llm_chain.py` — chains - `api/routes/llm_sts.py` — speech-to-speech - `api/routes/config/config.py`, `api/routes/config/version.py` — saved config CRUD + versions -- `api/routes/guardrails.py` — guardrail validators +- `api/routes/guardrails.py` — `POST /guardrails` (async job) + `GET /guardrails/{job_id}` (poll), plus thin proxies over the internal kaapi-guardrails management API: + - `GET /guardrails` — validator catalogue + - `/guardrails/ban_lists` — POST, GET (`offset`, `limit`); `/{id}` GET/PATCH/DELETE + - `/guardrails/llm_prompt_configs` — POST, GET (`validator_name`, `offset`, `limit`); `/{id}` GET/PATCH/DELETE + - `/guardrails/validators/configs` — POST, GET (`ids`, `stage`, `type`); `/{id}` GET/PATCH/DELETE + - Gotcha: the fixed `/guardrails/*` paths must stay declared above `GET /guardrails/{job_id}` — FastAPI matches in declaration order and won't fall through on a UUID parse failure. ## Tables (SQLModel) | Table | Model | From 319c0ea8d0ad059f1422aa077debda2b7ed0e4fe Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Thu, 3 Sep 2026 22:48:35 +0530 Subject: [PATCH 03/12] feat: carry over sentry logging between backend and kaapi-guardrails service --- backend/app/celery/tasks/job_execution.py | 22 ++++++++------ backend/app/core/telemetry.py | 8 ++++- backend/app/services/llm/guardrails.py | 37 +++++++++++++++++++++-- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/backend/app/celery/tasks/job_execution.py b/backend/app/celery/tasks/job_execution.py index 3ddd898f9..299bd28b2 100644 --- a/backend/app/celery/tasks/job_execution.py +++ b/backend/app/celery/tasks/job_execution.py @@ -63,19 +63,21 @@ def _extract_parent_context(task_instance) -> otel_context.Context: def _run_with_otel_parent(task_instance, fn): - """Attach extracted parent context and execute function. - - When Celery auto-instrumentation is active, there is already a current - `run/...` span. Re-attaching extracted parent context here would make - service spans become siblings of `run/...` instead of children. - - We only attach extracted context as a fallback when no active span exists. + """Attach the extracted parent context and execute `fn` under it. + + opentelemetry-instrumentation-celery's own extraction (CeleryGetter) + reads headers via getattr(task.request, key), but propagation headers + live in task.request.headers — so it never finds them and its `run/...` + span is always an unparented root. We extract from `.headers` ourselves + (see _extract_parent_context) and attach that as current before running + the task body, so spans created inside `fn` correctly nest under the + enqueueing request's trace instead of starting a disconnected one. """ - current_ctx = trace.get_current_span().get_span_context() - if current_ctx and current_ctx.is_valid: + parent_ctx = _extract_parent_context(task_instance) + parent_span_ctx = trace.get_current_span(parent_ctx).get_span_context() + if not (parent_span_ctx and parent_span_ctx.is_valid): return fn() - parent_ctx = _extract_parent_context(task_instance) token = otel_context.attach(parent_ctx) try: return fn() diff --git a/backend/app/core/telemetry.py b/backend/app/core/telemetry.py index 99d2fc959..c8effbfbc 100644 --- a/backend/app/core/telemetry.py +++ b/backend/app/core/telemetry.py @@ -14,6 +14,7 @@ from opentelemetry.instrumentation.logging import LoggingInstrumentor from opentelemetry.instrumentation.requests import RequestsInstrumentor from opentelemetry.instrumentation.utils import _SUPPRESS_HTTP_INSTRUMENTATION_KEY +from opentelemetry.propagate import set_global_textmap from opentelemetry.sdk.resources import SERVICE_NAME, Resource from opentelemetry.sdk.trace import TracerProvider @@ -192,9 +193,14 @@ def setup_telemetry(service_name: str | None = None) -> None: # Bridge OTel spans into Sentry as Sentry transactions and spans, with full attribute and error capture. if settings.SENTRY_DSN: - from sentry_sdk.integrations.opentelemetry import SentrySpanProcessor + from sentry_sdk.integrations.opentelemetry import ( + SentryPropagator, + SentrySpanProcessor, + ) tracer_provider.add_span_processor(SentrySpanProcessor()) + # Downstream services extract sentry-trace, not W3C traceparent. + set_global_textmap(SentryPropagator()) trace.set_tracer_provider(tracer_provider) diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index 09e31e2c9..7754c06d7 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -8,11 +8,13 @@ import httpx from fastapi import HTTPException +from opentelemetry import trace from app.core.config import settings from app.models.llm.request import Validator logger = logging.getLogger(__name__) +tracer = trace.get_tracer(__name__) GUARDRAILS_PROXY_TIMEOUT_SECONDS = 30.0 @@ -60,7 +62,16 @@ def proxy_guardrails_request( ) try: - with httpx.Client(timeout=GUARDRAILS_PROXY_TIMEOUT_SECONDS) as client: + with ( + tracer.start_as_current_span( + f"guardrails.proxy {method} {path}", + attributes={ + "kaapi.organization_id": str(organization_id), + "kaapi.project_id": str(project_id), + }, + ), + httpx.Client(timeout=GUARDRAILS_PROXY_TIMEOUT_SECONDS) as client, + ): response = client.request( method, url, params=query, json=json_body, headers=headers ) @@ -283,7 +294,18 @@ def run_guardrails_validation( started = time.monotonic() try: - with httpx.Client(timeout=45.0) as client: + with ( + tracer.start_as_current_span( + "guardrails.proxy.validate", + attributes={ + "kaapi.job_id": str(job_id), + "kaapi.organization_id": str(organization_id), + "kaapi.project_id": str(project_id), + "kaapi.validator_count": len(validators), + }, + ), + httpx.Client(timeout=45.0) as client, + ): response = client.post( url, json=payload, @@ -363,7 +385,16 @@ def _build_params(validator_ids: list[UUID]) -> dict[str, Any]: } try: - with httpx.Client(timeout=10.0) as client: + with ( + tracer.start_as_current_span( + "guardrails.proxy.list_validator_configs", + attributes={ + "kaapi.organization_id": str(organization_id), + "kaapi.project_id": str(project_id), + }, + ), + httpx.Client(timeout=10.0) as client, + ): def _fetch_by_ids(validator_ids: list[UUID]) -> list[dict[str, Any]]: if not validator_ids: From 0cc41d82d0d96beafd84cba15c8a93e0516cc3e5 Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Fri, 4 Sep 2026 09:52:55 +0530 Subject: [PATCH 04/12] set supress_pass_logs= False --- backend/app/services/llm/guardrails.py | 4 ++-- backend/app/tests/services/llm/test_guardrails.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index 7754c06d7..b2364c118 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -203,7 +203,7 @@ def apply_guardrails( job_id, project_id, organization_id, - suppress_pass_logs=True, + suppress_pass_logs=False, output_text=output_text, ) @@ -246,7 +246,7 @@ def run_guardrails_validation( job_id: UUID, project_id: int | None, organization_id: int | None, - suppress_pass_logs: bool = True, + suppress_pass_logs: bool = False, output_text: str | None = None, ) -> dict[str, Any]: """ diff --git a/backend/app/tests/services/llm/test_guardrails.py b/backend/app/tests/services/llm/test_guardrails.py index 87a4c261d..8e357aaf3 100644 --- a/backend/app/tests/services/llm/test_guardrails.py +++ b/backend/app/tests/services/llm/test_guardrails.py @@ -62,7 +62,7 @@ def test_run_guardrails_validation_success(mock_client_cls) -> None: assert "organization_id" not in kwargs["json"] assert kwargs["headers"]["X-PROJECT-ID"] == str(TEST_PROJECT_ID) assert kwargs["headers"]["X-ORGANIZATION-ID"] == str(TEST_ORGANIZATION_ID) - assert kwargs["params"]["suppress_pass_logs"] == "true" + assert kwargs["params"]["suppress_pass_logs"] == "false" assert kwargs["headers"]["Authorization"].startswith("Bearer ") assert kwargs["headers"]["Content-Type"] == "application/json" From 9dcea439d9decd2e725760b460c8fe11c0938085 Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Sat, 5 Sep 2026 20:48:27 +0530 Subject: [PATCH 05/12] fix: guardrails route cleanups --- backend/app/api/routes/guardrails.py | 38 +++++++++++++--------------- backend/app/celery/celery_app.py | 3 +++ backend/app/main.py | 3 +++ backend/app/services/llm/jobs.py | 23 +++++++++++++++++ 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/backend/app/api/routes/guardrails.py b/backend/app/api/routes/guardrails.py index 874b6349a..0d4002f5c 100644 --- a/backend/app/api/routes/guardrails.py +++ b/backend/app/api/routes/guardrails.py @@ -101,11 +101,6 @@ def _upstream_response(status_code: int, payload: Any) -> Response: return JSONResponse(status_code=status_code, content=payload) -BAN_LISTS_PATH = "/ban_lists" -LLM_PROMPT_CONFIGS_PATH = "/llm_prompt_configs" -VALIDATOR_CONFIGS_PATH = "/validators/configs" - - # ROUTE ORDERING: every fixed single-segment path below collides with the # GET /guardrails/{job_id} route declared after this section. FastAPI matches in # declaration order and does not fall through when {job_id} fails UUID parsing, @@ -136,7 +131,7 @@ def create_guardrails_ban_list( ) -> Response: status_code, payload = proxy_guardrails_request( "POST", - f"{BAN_LISTS_PATH}/", + "/ban_lists/", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, json_body=body, @@ -150,15 +145,16 @@ def create_guardrails_ban_list( ) def list_guardrails_ban_lists( _current_user: AuthContextDep, + domain: str | None = None, offset: Annotated[int, Query(ge=0)] = 0, limit: Annotated[int | None, Query(ge=1, le=100)] = None, ) -> Response: status_code, payload = proxy_guardrails_request( "GET", - f"{BAN_LISTS_PATH}/", + "/ban_lists/", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, - params={"offset": offset, "limit": limit}, + params={"domain": domain, "offset": offset, "limit": limit}, ) return _upstream_response(status_code, payload) @@ -172,7 +168,7 @@ def create_guardrails_llm_prompt_config( ) -> Response: status_code, payload = proxy_guardrails_request( "POST", - f"{LLM_PROMPT_CONFIGS_PATH}/", + "/llm_prompt_configs/", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, json_body=body, @@ -192,7 +188,7 @@ def list_guardrails_llm_prompt_configs( ) -> Response: status_code, payload = proxy_guardrails_request( "GET", - f"{LLM_PROMPT_CONFIGS_PATH}/", + "/llm_prompt_configs/", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, params={"validator_name": validator_name, "offset": offset, "limit": limit}, @@ -209,7 +205,7 @@ def create_guardrails_validator_config( ) -> Response: status_code, payload = proxy_guardrails_request( "POST", - f"{VALIDATOR_CONFIGS_PATH}/", + "/validators/configs/", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, json_body=body, @@ -229,7 +225,7 @@ def list_guardrails_validator_configs( ) -> Response: status_code, payload = proxy_guardrails_request( "GET", - f"{VALIDATOR_CONFIGS_PATH}/", + "/validators/configs/", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, params={ @@ -250,7 +246,7 @@ def get_guardrails_validator_config( ) -> Response: status_code, payload = proxy_guardrails_request( "GET", - f"{VALIDATOR_CONFIGS_PATH}/{config_id}", + f"/validators/configs/{config_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, ) @@ -266,7 +262,7 @@ def update_guardrails_validator_config( ) -> Response: status_code, payload = proxy_guardrails_request( "PATCH", - f"{VALIDATOR_CONFIGS_PATH}/{config_id}", + f"/validators/configs/{config_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, json_body=body, @@ -283,7 +279,7 @@ def delete_guardrails_validator_config( ) -> Response: status_code, payload = proxy_guardrails_request( "DELETE", - f"{VALIDATOR_CONFIGS_PATH}/{config_id}", + f"/validators/configs/{config_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, ) @@ -299,7 +295,7 @@ def get_guardrails_ban_list( ) -> Response: status_code, payload = proxy_guardrails_request( "GET", - f"{BAN_LISTS_PATH}/{ban_list_id}", + f"/ban_lists/{ban_list_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, ) @@ -315,7 +311,7 @@ def update_guardrails_ban_list( ) -> Response: status_code, payload = proxy_guardrails_request( "PATCH", - f"{BAN_LISTS_PATH}/{ban_list_id}", + f"/ban_lists/{ban_list_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, json_body=body, @@ -332,7 +328,7 @@ def delete_guardrails_ban_list( ) -> Response: status_code, payload = proxy_guardrails_request( "DELETE", - f"{BAN_LISTS_PATH}/{ban_list_id}", + f"/ban_lists/{ban_list_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, ) @@ -348,7 +344,7 @@ def get_guardrails_llm_prompt_config( ) -> Response: status_code, payload = proxy_guardrails_request( "GET", - f"{LLM_PROMPT_CONFIGS_PATH}/{prompt_config_id}", + f"/llm_prompt_configs/{prompt_config_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, ) @@ -364,7 +360,7 @@ def update_guardrails_llm_prompt_config( ) -> Response: status_code, payload = proxy_guardrails_request( "PATCH", - f"{LLM_PROMPT_CONFIGS_PATH}/{prompt_config_id}", + f"/llm_prompt_configs/{prompt_config_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, json_body=body, @@ -381,7 +377,7 @@ def delete_guardrails_llm_prompt_config( ) -> Response: status_code, payload = proxy_guardrails_request( "DELETE", - f"{LLM_PROMPT_CONFIGS_PATH}/{prompt_config_id}", + f"/llm_prompt_configs/{prompt_config_id}", organization_id=_current_user.organization_.id, project_id=_current_user.project_.id, ) diff --git a/backend/app/celery/celery_app.py b/backend/app/celery/celery_app.py index e0ddf8dcd..c2cba4cfb 100644 --- a/backend/app/celery/celery_app.py +++ b/backend/app/celery/celery_app.py @@ -44,6 +44,9 @@ def _initialize_worker_observability() -> None: release=settings.API_VERSION, instrumenter="otel", traces_sample_rate=1.0, + # LLM input/output is end-user text; never attach request/response + # bodies to error events or trace transactions. + max_request_body_size="never", enable_logs=True, before_send_transaction=before_send_transaction_filter, integrations=[ diff --git a/backend/app/main.py b/backend/app/main.py index 5d2a09cf0..5ad2ad6e0 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -34,6 +34,9 @@ release=settings.API_VERSION, instrumenter="otel", traces_sample_rate=1.0, + # LLM input/output is end-user text; never attach request/response + # bodies to error events or trace transactions. + max_request_body_size="never", enable_logs=True, before_send_transaction=before_send_transaction_filter, integrations=[ diff --git a/backend/app/services/llm/jobs.py b/backend/app/services/llm/jobs.py index 1c32b0b5e..00cdea402 100644 --- a/backend/app/services/llm/jobs.py +++ b/backend/app/services/llm/jobs.py @@ -412,6 +412,20 @@ def apply_input_guardrails( # No-op paths (no validators, bypassed) leave the query untouched. if outcome.applied and outcome.safe_text is not None: + if not outcome.safe_text.strip(): + # A fix-mode validator that supplies no fix_value (e.g. topic_relevance + # with no built-in fix) falls back to "" — forwarding that to the LLM + # provider fails with a confusing provider-side error instead of a + # clear guardrails-blocked one. + logger.warning( + f"[apply_input_guardrails] Guardrails reduced input to empty text; " + f"blocking request | job_id={job_id}" + ) + return ( + query, + "Input guardrails rejected the request and left no usable content.", + None, + ) query.input.content.value = outcome.safe_text return query, None, None @@ -456,6 +470,15 @@ def apply_output_guardrails( return result, outcome.error if outcome.applied and outcome.safe_text is not None: + if not outcome.safe_text.strip(): + logger.warning( + f"[apply_output_guardrails] Guardrails reduced output to empty text; " + f"blocking response | job_id={job_id}" + ) + return ( + result, + "Output guardrails rejected the response and left no usable content.", + ) result.response.response.output.content.value = outcome.safe_text return result, None From ecc5396d9e43ad9fc323bc02aef782f1de74f431 Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Mon, 7 Sep 2026 07:37:20 +0530 Subject: [PATCH 06/12] test: cover new guardrails proxy routes; fix coderabbit findings - Add test coverage for the 16 new guardrails proxy routes (forwarding, filters, UUID normalization, pagination bounds, route ordering, auth). - Set Cache-Control: no-store on guardrails proxy responses to prevent caching of tenant-scoped data (CWE-525). - Add narrow type hints to _run_with_otel_parent/_extract_parent_context per repo's strict mypy contract. Co-Authored-By: Claude Sonnet 5 --- backend/app/api/routes/guardrails.py | 11 +- backend/app/celery/tasks/job_execution.py | 11 +- .../app/tests/api/routes/test_guardrails.py | 212 ++++++++++++++++++ 3 files changed, 228 insertions(+), 6 deletions(-) diff --git a/backend/app/api/routes/guardrails.py b/backend/app/api/routes/guardrails.py index 0d4002f5c..afd8ab4b3 100644 --- a/backend/app/api/routes/guardrails.py +++ b/backend/app/api/routes/guardrails.py @@ -95,10 +95,15 @@ def apply_guardrails_endpoint( def _upstream_response(status_code: int, payload: Any) -> Response: - """An empty upstream body must stay empty (204s cannot carry one).""" + """An empty upstream body must stay empty (204s cannot carry one). + + Tenant-scoped data must never be cached by a shared/intermediary cache + (CWE-525); `no-store` is stronger than `private` for that guarantee. + """ + headers = {"Cache-Control": "no-store"} if payload is None: - return Response(status_code=status_code) - return JSONResponse(status_code=status_code, content=payload) + return Response(status_code=status_code, headers=headers) + return JSONResponse(status_code=status_code, content=payload, headers=headers) # ROUTE ORDERING: every fixed single-segment path below collides with the diff --git a/backend/app/celery/tasks/job_execution.py b/backend/app/celery/tasks/job_execution.py index 299bd28b2..a408983c7 100644 --- a/backend/app/celery/tasks/job_execution.py +++ b/backend/app/celery/tasks/job_execution.py @@ -13,7 +13,8 @@ """ import logging -from typing import TYPE_CHECKING +from collections.abc import Callable +from typing import TYPE_CHECKING, TypeVar from asgi_correlation_id import correlation_id from celery import Task, current_task @@ -32,6 +33,8 @@ logger = logging.getLogger(__name__) +T = TypeVar("T") + # Sentinel correlation id used when no trace id is propagated from the # enqueueing request. Matches the codebase-wide "N/A" default (see # app/core/logger.py and app/celery/utils.py). @@ -43,7 +46,7 @@ def _set_trace(trace_id: str) -> None: logger.info(f"[_set_trace] Set correlation ID: {trace_id}") -def _extract_parent_context(task_instance) -> otel_context.Context: +def _extract_parent_context(task_instance: Task) -> otel_context.Context: """Extract OTel parent context from Celery headers if available.""" headers = getattr(task_instance.request, "headers", None) or {} carrier: dict[str, str] = {} @@ -62,7 +65,9 @@ def _extract_parent_context(task_instance) -> otel_context.Context: return extract(carrier) -def _run_with_otel_parent(task_instance, fn): +def _run_with_otel_parent( + task_instance: Task, fn: Callable[[], T] +) -> T: # noqa: UP047 (black doesn't support PEP 695 generics yet) """Attach the extracted parent context and execute `fn` under it. opentelemetry-instrumentation-celery's own extraction (CeleryGetter) diff --git a/backend/app/tests/api/routes/test_guardrails.py b/backend/app/tests/api/routes/test_guardrails.py index 66f97c70c..7ac271b31 100644 --- a/backend/app/tests/api/routes/test_guardrails.py +++ b/backend/app/tests/api/routes/test_guardrails.py @@ -8,6 +8,7 @@ from fastapi.testclient import TestClient from sqlmodel import Session +from app.core.config import settings from app.crud import JobCrud from app.models import Job, JobStatus, JobType, JobUpdate from app.tests.utils.auth import TestAuthContext @@ -15,6 +16,69 @@ VALIDATOR_ID = str(uuid4()) +# (client method, kaapi path, request body, upstream path) — "{id}" is filled +# with a freshly generated UUID by the tests that consume this table. +PROXY_ROUTES = [ + ("GET", "/guardrails", None, "/"), + ("POST", "/guardrails/ban_lists", {"name": "slurs"}, "/ban_lists/"), + ("GET", "/guardrails/ban_lists", None, "/ban_lists/"), + ("GET", "/guardrails/ban_lists/{id}", None, "/ban_lists/{id}"), + ("PATCH", "/guardrails/ban_lists/{id}", {"name": "renamed"}, "/ban_lists/{id}"), + ("DELETE", "/guardrails/ban_lists/{id}", None, "/ban_lists/{id}"), + ( + "POST", + "/guardrails/llm_prompt_configs", + {"validator_name": "toxicity", "prompt": "be nice"}, + "/llm_prompt_configs/", + ), + ("GET", "/guardrails/llm_prompt_configs", None, "/llm_prompt_configs/"), + ( + "GET", + "/guardrails/llm_prompt_configs/{id}", + None, + "/llm_prompt_configs/{id}", + ), + ( + "PATCH", + "/guardrails/llm_prompt_configs/{id}", + {"prompt": "be nicer"}, + "/llm_prompt_configs/{id}", + ), + ( + "DELETE", + "/guardrails/llm_prompt_configs/{id}", + None, + "/llm_prompt_configs/{id}", + ), + ( + "POST", + "/guardrails/validators/configs", + {"type": "pii", "stage": "input"}, + "/validators/configs/", + ), + ("GET", "/guardrails/validators/configs", None, "/validators/configs/"), + ( + "GET", + "/guardrails/validators/configs/{id}", + None, + "/validators/configs/{id}", + ), + ( + "PATCH", + "/guardrails/validators/configs/{id}", + {"stage": "output"}, + "/validators/configs/{id}", + ), + ( + "DELETE", + "/guardrails/validators/configs/{id}", + None, + "/validators/configs/{id}", + ), +] + +PROXY_ROUTE_IDS = [f"{method} {path}" for method, path, _, _ in PROXY_ROUTES] + def _payload(**overrides): body = { @@ -297,6 +361,40 @@ def test_non_json_upstream_body_returns_502( class TestProxyForwardedRequest: + @pytest.mark.parametrize( + "method, kaapi_path, body, upstream_path", PROXY_ROUTES, ids=PROXY_ROUTE_IDS + ) + def test_route_forwards_method_path_body_and_tenant( + self, + client: TestClient, + user_api_key: TestAuthContext, + user_api_key_header: dict[str, str], + method: str, + kaapi_path: str, + body: dict[str, Any] | None, + upstream_path: str, + ) -> None: + resource_id = uuid4() + upstream_body = {"ok": True} + with _mock_upstream(json_body=upstream_body) as calls: + resp = client.request( + method, + f"api/v1{kaapi_path.format(id=resource_id)}", + json=body, + headers=user_api_key_header, + ) + + assert resp.status_code == 200 + assert resp.json() == upstream_body + assert calls[0]["args"] == ( + method, + f"{settings.KAAPI_GUARDRAILS_URL}{upstream_path.format(id=resource_id)}", + ) + assert calls[0]["kwargs"]["json"] == body + headers = calls[0]["kwargs"]["headers"] + assert headers["X-ORGANIZATION-ID"] == str(user_api_key.organization_id) + assert headers["X-PROJECT-ID"] == str(user_api_key.project_id) + def test_unset_limit_is_dropped_from_forwarded_params( self, client: TestClient, user_api_key_header: dict[str, str] ) -> None: @@ -322,6 +420,74 @@ def test_ids_forwarded_as_list( "stage": "input", } + def test_ban_list_filters_forwarded( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + client.get( + "api/v1/guardrails/ban_lists?domain=email&offset=5&limit=10", + headers=user_api_key_header, + ) + + assert calls[0]["kwargs"]["params"] == { + "domain": "email", + "offset": 5, + "limit": 10, + } + + def test_llm_prompt_config_filters_forwarded( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + client.get( + "api/v1/guardrails/llm_prompt_configs?validator_name=toxicity&limit=50", + headers=user_api_key_header, + ) + + assert calls[0]["kwargs"]["params"] == { + "validator_name": "toxicity", + "offset": 0, + "limit": 50, + } + + def test_validator_config_filters_forwarded_without_ids( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + client.get( + "api/v1/guardrails/validators/configs?stage=output&type=pii", + headers=user_api_key_header, + ) + + assert calls[0]["kwargs"]["params"] == {"stage": "output", "type": "pii"} + + def test_ids_are_normalised_to_canonical_uuid_strings( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + """Upstream expects strings; UUID parsing also canonicalises the casing.""" + config_id = uuid4() + with _mock_upstream(json_body={"data": []}) as calls: + client.get( + f"api/v1/guardrails/validators/configs?ids={str(config_id).upper()}", + headers=user_api_key_header, + ) + + assert calls[0]["kwargs"]["params"] == {"ids": [str(config_id)]} + + @pytest.mark.parametrize( + "kaapi_path", ["/guardrails/ban_lists", "/guardrails/llm_prompt_configs"] + ) + @pytest.mark.parametrize("query", ["limit=0", "limit=101", "offset=-1"]) + def test_out_of_range_pagination_rejected( + self, + client: TestClient, + user_api_key_header: dict[str, str], + kaapi_path: str, + query: str, + ) -> None: + resp = client.get(f"api/v1{kaapi_path}?{query}", headers=user_api_key_header) + assert resp.status_code == 422 + def test_tenant_headers_come_from_auth_context_not_request( self, client: TestClient, @@ -368,12 +534,58 @@ def test_ban_lists_list_route_wins_over_job_status_route( assert resp.status_code == 200 assert calls[0]["args"][1].endswith("/ban_lists/") + def test_validator_configs_list_and_detail_routes_do_not_collide( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + config_id = uuid4() + with _mock_upstream(json_body={"data": []}) as calls: + client.get( + "api/v1/guardrails/validators/configs", headers=user_api_key_header + ) + client.get( + f"api/v1/guardrails/validators/configs/{config_id}", + headers=user_api_key_header, + ) + + assert calls[0]["args"][1].endswith("/validators/configs/") + assert calls[1]["args"][1].endswith(f"/validators/configs/{config_id}") + + def test_llm_prompt_configs_list_route_wins_over_job_status_route( + self, client: TestClient, user_api_key_header: dict[str, str] + ) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + resp = client.get( + "api/v1/guardrails/llm_prompt_configs", headers=user_api_key_header + ) + + assert resp.status_code == 200 + assert calls[0]["args"][1].endswith("/llm_prompt_configs/") + def test_list_ban_lists_requires_auth(client: TestClient) -> None: resp = client.get("api/v1/guardrails/ban_lists") assert resp.status_code in (401, 403) +@pytest.mark.parametrize( + "method, kaapi_path, body, _upstream_path", PROXY_ROUTES, ids=PROXY_ROUTE_IDS +) +def test_proxy_routes_require_auth( + client: TestClient, + method: str, + kaapi_path: str, + body: dict[str, Any] | None, + _upstream_path: str, +) -> None: + with _mock_upstream(json_body={"data": []}) as calls: + resp = client.request( + method, f"api/v1{kaapi_path.format(id=uuid4())}", json=body + ) + + assert resp.status_code in (401, 403) + assert calls == [] + + # ---------- helpers ---------- From 4554c1297acfe082b3f7c18183c104f28a67133a Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Wed, 9 Sep 2026 11:33:19 +0530 Subject: [PATCH 07/12] feat:add metadata field to store and send guardrails intermediate results --- .../versions/083_add_llm_call_metadata.py | 40 +++++ backend/app/api/routes/llm.py | 4 +- backend/app/celery/celery_app.py | 3 +- backend/app/core/sentry_filters.py | 56 +++++++ backend/app/crud/llm.py | 10 ++ backend/app/main.py | 3 +- backend/app/models/llm/request.py | 10 ++ backend/app/services/llm/guardrails.py | 20 +++ backend/app/services/llm/jobs.py | 109 +++++++++--- backend/app/tests/core/test_sentry_filters.py | 79 +++++++++ .../app/tests/services/llm/test_guardrails.py | 53 ++++++ backend/app/tests/services/llm/test_jobs.py | 158 ++++++++++++++++++ 12 files changed, 522 insertions(+), 23 deletions(-) create mode 100644 backend/app/alembic/versions/083_add_llm_call_metadata.py create mode 100644 backend/app/tests/core/test_sentry_filters.py diff --git a/backend/app/alembic/versions/083_add_llm_call_metadata.py b/backend/app/alembic/versions/083_add_llm_call_metadata.py new file mode 100644 index 000000000..53e4e9f0f --- /dev/null +++ b/backend/app/alembic/versions/083_add_llm_call_metadata.py @@ -0,0 +1,40 @@ +"""Add llm_call.metadata (generic extensibility catch-all) + +Revision ID: 083 +Revises: 082 +Create Date: 2026-09-08 00:00:00.000000 + +Re-adds a `metadata` JSONB column on `llm_call`, previously dropped by 079 +(as collateral of an unrelated feature revert, not because the column was a +bad idea). This time it's a generic catch-all, mirroring `llm_chain.metadata` +(added via `metadata_` in the model to dodge SQLAlchemy's reserved +`Base.metadata` attribute) — first use case: persisting input/output +guardrail results so /llm/call polling (GET /llm/call/{job_id}) can surface +them, matching what's already sent on the callback payload's `metadata` +field. +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "083" +down_revision = "082" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "llm_call", + sa.Column( + "metadata", + postgresql.JSONB(astext_type=sa.Text()), + nullable=True, + comment="Future-proof extensibility catch-all (e.g. guardrail results)", + ), + ) + + +def downgrade(): + op.drop_column("llm_call", "metadata") diff --git a/backend/app/api/routes/llm.py b/backend/app/api/routes/llm.py index 94d3b341e..91ee0d67e 100644 --- a/backend/app/api/routes/llm.py +++ b/backend/app/api/routes/llm.py @@ -189,6 +189,7 @@ def get_llm_call_status( raise HTTPException(status_code=404, detail="Job not found") llm_call_response = None + call_metadata: dict | None = None if job.status.value == JobStatus.SUCCESS: llm_calls = get_llm_calls_by_job_id( session=session, job_id=job_id, project_id=project_id @@ -229,6 +230,7 @@ def get_llm_call_status( usage=Usage(**usage_payload), provider_raw_response=None, ) + call_metadata = llm_call.metadata_ job_response = LLMJobPublic( job_id=job.id, @@ -237,4 +239,4 @@ def get_llm_call_status( error_message=job.error_message, ) - return APIResponse.success_response(data=job_response) + return APIResponse.success_response(data=job_response, metadata=call_metadata) diff --git a/backend/app/celery/celery_app.py b/backend/app/celery/celery_app.py index c2cba4cfb..0859f0c1c 100644 --- a/backend/app/celery/celery_app.py +++ b/backend/app/celery/celery_app.py @@ -13,7 +13,7 @@ from app.core.config import settings from app.core.logger import configure_logging -from app.core.sentry_filters import before_send_transaction_filter +from app.core.sentry_filters import before_send_filter, before_send_transaction_filter logger = logging.getLogger(__name__) _telemetry_initialized = False @@ -48,6 +48,7 @@ def _initialize_worker_observability() -> None: # bodies to error events or trace transactions. max_request_body_size="never", enable_logs=True, + before_send=before_send_filter, before_send_transaction=before_send_transaction_filter, integrations=[ LoggingIntegration( diff --git a/backend/app/core/sentry_filters.py b/backend/app/core/sentry_filters.py index 6712803bd..13dc0a393 100644 --- a/backend/app/core/sentry_filters.py +++ b/backend/app/core/sentry_filters.py @@ -2,6 +2,20 @@ from typing import Any +_REDACTED = "[REDACTED]" + +# sentry_sdk's CeleryIntegration attaches the task's raw args/kwargs to every +# event captured during that task as extra["celery-job"]. For these tasks, +# kwargs["request_data"] carries the caller's raw query text and (for chain +# jobs) prior block responses -- none of that should reach Sentry. +_LLM_JOB_TASK_NAMES = { + "app.celery.tasks.job_execution.run_llm_job", + "app.celery.tasks.job_execution.run_llm_chain_job", + "app.celery.tasks.job_execution.run_response_job", +} +_SENSITIVE_REQUEST_DATA_KEYS = ("query", "request_metadata", "response", "output") + + _SQL_OR_CONNECT = re.compile(r"^(select|insert|update|delete|connect)\b", re.IGNORECASE) _HTTP_SEND_RECEIVE = re.compile(r"http (send|receive)$", re.IGNORECASE) _DB_QUERY_SPAN = re.compile(r"^db\.query$", re.IGNORECASE) @@ -85,3 +99,45 @@ def before_send_transaction_filter( event["spans"] = filtered return event + + +def _redact_llm_job_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]: + """Replaces end-user text fields in an LLM job's request_data with a + placeholder, leaving non-text fields (config, ids, callback_url) intact. + """ + request_data = kwargs.get("request_data") + if not isinstance(request_data, dict): + return kwargs + + redacted_request_data = dict(request_data) + for key in _SENSITIVE_REQUEST_DATA_KEYS: + if key in redacted_request_data: + redacted_request_data[key] = _REDACTED + + redacted_kwargs = dict(kwargs) + redacted_kwargs["request_data"] = redacted_request_data + return redacted_kwargs + + +def before_send_filter( + event: dict[str, Any], hint: dict[str, Any] +) -> dict[str, Any] | None: + """Strips end-user query/response text from LLM job celery-job context + before an event reaches Sentry. + """ + extra = event.get("extra") + if not isinstance(extra, dict): + return event + + celery_job = extra.get("celery-job") + if not isinstance(celery_job, dict): + return event + + if celery_job.get("task_name") not in _LLM_JOB_TASK_NAMES: + return event + + kwargs = celery_job.get("kwargs") + if isinstance(kwargs, dict): + celery_job["kwargs"] = _redact_llm_job_kwargs(kwargs) + + return event diff --git a/backend/app/crud/llm.py b/backend/app/crud/llm.py index 44e605fb7..a0c6448e1 100644 --- a/backend/app/crud/llm.py +++ b/backend/app/crud/llm.py @@ -65,6 +65,7 @@ def create_llm_call( organization_id: int, resolved_config: ConfigBlob, original_provider: str, + metadata: dict[str, Any] | None = None, ) -> LlmCall: """ Create a new LLM call record in the database. @@ -76,6 +77,7 @@ def create_llm_call( project_id: Project this LLM call belongs to organization_id: Organization this LLM call belongs to resolved_config: The resolved configuration blob (either from stored config or ad-hoc) + metadata: Extensibility catch-all dict (e.g. input guardrail results) Returns: LlmCall: The created LLM call record @@ -150,6 +152,7 @@ def create_llm_call( conversation_id=conversation_id, auto_create=auto_create, config=config_dict, + metadata_=metadata, ) session.add(db_llm_call) @@ -172,6 +175,7 @@ def update_llm_call_response( content: dict[str, Any] | None = None, usage: dict[str, Any] | None = None, conversation_id: str | None = None, + metadata: dict[str, Any] | None = None, ) -> LlmCall: """ Update an LLM call record with response data. @@ -183,6 +187,7 @@ def update_llm_call_response( content: Response content dict usage: Token usage dict conversation_id: Conversation ID if created/updated + metadata: Extensibility catch-all dict, merged into any existing value Returns: LlmCall: The updated LLM call record @@ -218,6 +223,10 @@ def update_llm_call_response( db_llm_call.usage = usage if conversation_id is not None: db_llm_call.conversation_id = conversation_id + if metadata is not None: + existing_metadata = db_llm_call.metadata_ or {} + existing_metadata.update(metadata) + db_llm_call.metadata_ = existing_metadata db_llm_call.updated_at = now() @@ -283,6 +292,7 @@ def save_rephrase_guardrail_call( resolved_config=config_blob, original_provider=str(config_blob.completion.provider), chain_id=chain_id, + metadata=request_metadata, ) try: update_llm_call_response( diff --git a/backend/app/main.py b/backend/app/main.py index 5ad2ad6e0..fd97c03da 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -18,7 +18,7 @@ from app.core.exception_handlers import register_exception_handlers from app.core.logger import configure_logging from app.core.middleware import StripTrailingSlashMiddleware, http_request_logger -from app.core.sentry_filters import before_send_transaction_filter +from app.core.sentry_filters import before_send_filter, before_send_transaction_filter from app.core.telemetry import instrument_app, setup_telemetry from app.load_env import load_environment @@ -38,6 +38,7 @@ # bodies to error events or trace transactions. max_request_body_size="never", enable_logs=True, + before_send=before_send_filter, before_send_transaction=before_send_transaction_filter, integrations=[ LoggingIntegration( diff --git a/backend/app/models/llm/request.py b/backend/app/models/llm/request.py index c028a978a..63b2e7fbb 100644 --- a/backend/app/models/llm/request.py +++ b/backend/app/models/llm/request.py @@ -764,6 +764,16 @@ class LlmCall(SQLModel, table=True): ), ) + metadata_: dict[str, Any] | None = Field( + default=None, + sa_column=sa.Column( + "metadata", + JSONB, + nullable=True, + comment="Future-proof extensibility catch-all (e.g. guardrail results)", + ), + ) + # Timestamps inserted_at: datetime = Field( default_factory=now, diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index b2364c118..bf12512e7 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -240,6 +240,26 @@ def apply_guardrails( ) +def summarize_validator_results(outcome: GuardrailsOutcome) -> list[dict[str, Any]]: + """Per-validator name/outcome/text summary, extracted from the raw + guardrails service response, for use in llm_call metadata.""" + data = outcome.raw.get("data") or {} + validator_results = data.get("validator_results") or [] + + summaries = [] + for validator_result in validator_results: + summaries.append( + { + "name": validator_result.get("name"), + "outcome": validator_result.get("outcome"), + "error": validator_result.get("error"), + "input_text": validator_result.get("input_text"), + "output_text": validator_result.get("output_text"), + } + ) + return summaries + + def run_guardrails_validation( input_text: str, guardrail_config: Sequence[Validator | dict[str, Any]], diff --git a/backend/app/services/llm/jobs.py b/backend/app/services/llm/jobs.py index 00cdea402..62f5baf34 100644 --- a/backend/app/services/llm/jobs.py +++ b/backend/app/services/llm/jobs.py @@ -67,7 +67,7 @@ Usage, ) from app.services.llm.chain.types import BlockResult -from app.services.llm.guardrails import apply_guardrails +from app.services.llm.guardrails import apply_guardrails, summarize_validator_results from app.services.llm.mappers import ( resolve_default_audio_provider, transform_kaapi_config_to_native, @@ -371,19 +371,13 @@ def apply_input_guardrails( job_id: UUID, project_id: int, organization_id: int, -) -> tuple[QueryParams, str | None, str | None]: +) -> tuple[QueryParams, str | None, str | None, dict[str, Any] | None]: """Apply input guardrails from a config_blob. Shared with llm-call and llm-chain. - Thin adapter over ``apply_guardrails`` that maps the outcome onto a - ``QueryParams`` payload. - - Returns (query, error, guardrail_direct_response) where: - - error is set when guardrails hard-block the request - - guardrail_direct_response is set when rephrase_needed=True and the safe_text - should be returned directly to the user without hitting the LLM + Returns (query, error, guardrail_direct_response, metadata). """ if not config_blob or not config_blob.input_guardrails: - return query, None, None + return query, None, None, None if not isinstance(query.input, TextInput): logger.info( @@ -391,24 +385,34 @@ def apply_input_guardrails( f"job_id={job_id}, " f"input_type={getattr(query.input, 'type', type(query.input).__name__)}" ) - return query, None, None + return query, None, None, None + original_input_text = query.input.content.value outcome = apply_guardrails( - text=query.input.content.value, + text=original_input_text, validators=config_blob.input_guardrails, job_id=job_id, project_id=project_id, organization_id=organization_id, ) + metadata = None + if outcome.applied: + metadata = { + "input_guardrail": { + "user_input": original_input_text, + "input_sent_to_llm": outcome.safe_text, + "validators": summarize_validator_results(outcome), + } + } if outcome.error is not None: - return query, outcome.error, None + return query, outcome.error, None, metadata if outcome.rephrase_needed: logger.info( f"[apply_input_guardrails] rephrase_needed=True, returning safe_text directly | job_id={job_id}" ) - return query, None, outcome.safe_text + return query, None, outcome.safe_text, metadata # No-op paths (no validators, bypassed) leave the query untouched. if outcome.applied and outcome.safe_text is not None: @@ -425,9 +429,10 @@ def apply_input_guardrails( query, "Input guardrails rejected the request and left no usable content.", None, + metadata, ) query.input.content.value = outcome.safe_text - return query, None, None + return query, None, None, metadata def apply_output_guardrails( @@ -441,9 +446,6 @@ def apply_output_guardrails( ) -> tuple[BlockResult, str | None]: """Apply output guardrails from a config_blob. Shared by /llm/call and /llm/chain. - Thin adapter over ``apply_guardrails`` that maps the outcome onto a - ``BlockResult``. - Returns (modified_result, None) on success, or (result, error_string) on failure. """ if not config_blob or not config_blob.output_guardrails: @@ -457,15 +459,25 @@ def apply_output_guardrails( ) return result, None + original_output_text = result.response.response.output.content.value outcome = apply_guardrails( text=input_text or "", validators=config_blob.output_guardrails, job_id=job_id, project_id=project_id, organization_id=organization_id, - output_text=result.response.response.output.content.value, + output_text=original_output_text, ) + if outcome.applied: + existing_metadata = result.metadata or {} + existing_metadata["output_guardrail"] = { + "llm_output_pre_guardrail": original_output_text, + "response_to_user": outcome.safe_text, + "validators": summarize_validator_results(outcome), + } + result.metadata = existing_metadata + if outcome.error is not None: return result, outcome.error @@ -483,6 +495,32 @@ def apply_output_guardrails( return result, None +def persist_output_guardrail_result( + *, + llm_call_id: UUID | None, + content: dict[str, Any] | None, + metadata: dict[str, Any] | None, +) -> None: + """Re-persists content/metadata onto the already-created LlmCall row + after output guardrails run. Uses its own session since the caller's + session may already be closed by this point.""" + if not llm_call_id: + return + if content is None and metadata is None: + return + try: + with Session(engine) as session: + update_llm_call_response( + session, llm_call_id=llm_call_id, content=content, metadata=metadata + ) + except Exception as e: + logger.error( + f"[persist_output_guardrail_result] Failed to persist guardrail " + f"result: {e} | llm_call_id={llm_call_id}", + exc_info=True, + ) + + DETECTED_LANGUAGE_FALLBACK = "en-IN" _TTS_LANGUAGE_KEYS = ("target_language_code", "language_code") @@ -605,13 +643,22 @@ def execute_llm_call( project_id=project_id, organization_id=organization_id, ) - query, input_error, guardrail_direct_response = apply_input_guardrails( + ( + query, + input_error, + guardrail_direct_response, + input_guardrail_metadata, + ) = apply_input_guardrails( config_blob=config_blob, query=query, job_id=job_id, project_id=project_id, organization_id=organization_id, ) + if input_guardrail_metadata: + if request_metadata is None: + request_metadata = {} + request_metadata.update(input_guardrail_metadata) if guardrail_direct_response is not None: # Runs before the Kaapi->native transform, so params may be # a typed model (Kaapi/proxy variants) rather than a dict. @@ -715,6 +762,7 @@ def execute_llm_call( resolved_config=config_blob, original_provider=Provider.PROXY.value, chain_id=chain_id, + metadata=request_metadata, ) llm_call_id = llm_call.id except Exception as e: @@ -876,6 +924,17 @@ def execute_llm_call( trace.Status(trace.StatusCode.ERROR, output_error) ) return BlockResult(error=output_error, llm_call_id=llm_call_id) + if config_blob.output_guardrails: + updated_content = None + if isinstance(result.response.response.output, TextOutput): + updated_content = ( + result.response.response.output.model_dump() + ) + persist_output_guardrail_result( + llm_call_id=llm_call_id, + content=updated_content, + metadata=result.metadata, + ) return result @@ -945,6 +1004,7 @@ def execute_llm_call( resolved_config=resolved_config_blob, original_provider=original_provider, chain_id=chain_id, + metadata=request_metadata, ) llm_call_id = llm_call.id _set_traceability_attributes(create_span, llm_call_id=llm_call_id) @@ -1257,6 +1317,15 @@ def execute_llm_call( trace.Status(trace.StatusCode.ERROR, output_error) ) return BlockResult(error=output_error, llm_call_id=llm_call_id) + if config_blob.output_guardrails: + updated_content = None + if isinstance(result.response.response.output, TextOutput): + updated_content = result.response.response.output.model_dump() + persist_output_guardrail_result( + llm_call_id=llm_call_id, + content=updated_content, + metadata=result.metadata, + ) return result diff --git a/backend/app/tests/core/test_sentry_filters.py b/backend/app/tests/core/test_sentry_filters.py new file mode 100644 index 000000000..8f750a9d7 --- /dev/null +++ b/backend/app/tests/core/test_sentry_filters.py @@ -0,0 +1,79 @@ +from app.core.sentry_filters import before_send_filter + + +def _event_with_celery_job(task_name: str, kwargs: dict) -> dict: + return { + "extra": { + "celery-job": { + "task_name": task_name, + "args": [], + "kwargs": kwargs, + } + } + } + + +def test_before_send_filter_redacts_query_for_llm_job(): + event = _event_with_celery_job( + "app.celery.tasks.job_execution.run_llm_job", + { + "job_id": "b9ce6621-9b5a-45c4-969f-df7613ff7dc4", + "organization_id": 1, + "project_id": 1, + "request_data": { + "callback_url": "https://webhooksite.net/some-id", + "config": {"blob": {"completion": {"provider": "openai"}}}, + "query": { + "input": { + "type": "text", + "content": {"value": "Amit Gupta phone number is 919611188278"}, + } + }, + "request_metadata": None, + }, + }, + ) + + result = before_send_filter(event, {}) + + request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] + assert request_data["query"] == "[REDACTED]" + # Non-sensitive fields are left untouched so the trace stays useful. + assert request_data["callback_url"] == "https://webhooksite.net/some-id" + assert request_data["config"] == {"blob": {"completion": {"provider": "openai"}}} + + +def test_before_send_filter_redacts_query_for_chain_and_response_jobs(): + for task_name in ( + "app.celery.tasks.job_execution.run_llm_chain_job", + "app.celery.tasks.job_execution.run_response_job", + ): + event = _event_with_celery_job( + task_name, + {"request_data": {"query": {"input": "sensitive text"}}}, + ) + + result = before_send_filter(event, {}) + + request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] + assert request_data["query"] == "[REDACTED]" + + +def test_before_send_filter_ignores_unrelated_tasks(): + event = _event_with_celery_job( + "app.celery.tasks.job_execution.run_doctransform_job", + {"request_data": {"query": {"input": "not an llm job"}}}, + ) + + result = before_send_filter(event, {}) + + request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] + assert request_data["query"] == {"input": "not an llm job"} + + +def test_before_send_filter_passes_through_events_without_celery_job(): + event = {"message": "some unrelated error"} + + result = before_send_filter(event, {}) + + assert result == event diff --git a/backend/app/tests/services/llm/test_guardrails.py b/backend/app/tests/services/llm/test_guardrails.py index 8e357aaf3..b1ccabcdb 100644 --- a/backend/app/tests/services/llm/test_guardrails.py +++ b/backend/app/tests/services/llm/test_guardrails.py @@ -20,8 +20,10 @@ Validator, ) from app.services.llm.guardrails import ( + GuardrailsOutcome, list_validators_config, run_guardrails_validation, + summarize_validator_results, ) from app.tests.utils.utils import get_project @@ -374,6 +376,57 @@ def test_list_validators_config_network_error_fails_open(mock_client_cls) -> Non assert output_guardrails == [] +def test_summarize_validator_results_extracts_per_validator_fields() -> None: + outcome = GuardrailsOutcome( + safe_text="My credit card is [REDACTED]", + error=None, + bypassed=False, + rephrase_needed=False, + raw={ + "success": True, + "data": { + "safe_text": "My credit card is [REDACTED]", + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "input", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": "My credit card is 4111 1111 1111 1111", + "output_text": "My credit card is [REDACTED]", + } + ], + }, + }, + ) + + summaries = summarize_validator_results(outcome) + + assert summaries == [ + { + "name": "PIIRemover", + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": "My credit card is 4111 1111 1111 1111", + "output_text": "My credit card is [REDACTED]", + } + ] + + +def test_summarize_validator_results_empty_when_raw_has_no_validator_results() -> None: + outcome = GuardrailsOutcome( + safe_text="hello", + error=None, + bypassed=True, + rephrase_needed=False, + raw={}, + ) + + assert summarize_validator_results(outcome) == [] + + _SAFE_TEXT = "Please rephrase: content not allowed." _CONFIG_BLOB = ConfigBlob( completion=NativeCompletionConfig( diff --git a/backend/app/tests/services/llm/test_jobs.py b/backend/app/tests/services/llm/test_jobs.py index 9ce03f2ca..fd8bdf898 100644 --- a/backend/app/tests/services/llm/test_jobs.py +++ b/backend/app/tests/services/llm/test_jobs.py @@ -1344,6 +1344,85 @@ def test_guardrails_sanitize_input_before_provider( assert result["success"] + def test_guardrails_metadata_reports_per_validator_text_for_input_guardrail( + self, db, job_env, job_for_execution + ): + """metadata.input_guardrail should surface the original text, what + was sent to the LLM, and each validator's own before/after text -- + not just the raw kaapi-guardrails response wrapper. + """ + env = job_env + env["provider"].execute.return_value = (env["mock_llm_response"], None) + + unsafe_input = "My credit card is 4111 1111 1111 1111" + sanitized_input = "My credit card is [REDACTED]" + + with ( + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + ): + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": sanitized_input, + "rephrase_needed": False, + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "input", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": unsafe_input, + "output_text": sanitized_input, + } + ], + }, + } + mock_fetch_configs.return_value = ( + [{"type": "pii_remover", "stage": "input"}], + [], + ) + + request_data = { + "query": {"input": unsafe_input}, + "config": { + "blob": { + "completion": { + "provider": "openai-native", + "type": "text", + "params": {"model": "gpt-4o"}, + }, + "input_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_1} + ], + "output_guardrails": [], + } + }, + "include_provider_raw_response": False, + "callback_url": None, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + + input_guardrail_metadata = result["metadata"]["input_guardrail"] + assert input_guardrail_metadata["user_input"] == unsafe_input + assert input_guardrail_metadata["input_sent_to_llm"] == sanitized_input + + validators = input_guardrail_metadata["validators"] + assert len(validators) == 1 + assert validators[0]["name"] == "PIIRemover" + assert validators[0]["outcome"] == "FAIL" + assert validators[0]["input_text"] == unsafe_input + assert validators[0]["output_text"] == sanitized_input + def test_guardrails_skip_input_validation_for_audio_input( self, db, job_env, job_for_execution ): @@ -1443,6 +1522,85 @@ def test_guardrails_sanitize_output_after_provider( assert "REDACTED" in result["data"]["response"]["output"]["content"]["value"] + def test_guardrails_metadata_reports_per_validator_text_for_output_guardrail( + self, db, job_env, job_for_execution + ): + """metadata.output_guardrail should surface the raw LLM output + (pre-guardrail), the final response text, and each validator's own + before/after text -- not just the raw kaapi-guardrails response + wrapper. + """ + env = job_env + + raw_llm_output = "Aadhar no 123-45-6789" + sanitized_output = "Aadhar [REDACTED]" + env["mock_llm_response"].response.output.content.value = raw_llm_output + env["provider"].execute.return_value = (env["mock_llm_response"], None) + + with ( + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + ): + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": sanitized_output, + "rephrase_needed": False, + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "output", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": raw_llm_output, + "output_text": sanitized_output, + } + ], + }, + } + mock_fetch_configs.return_value = ( + [], + [{"type": "pii_remover", "stage": "output"}], + ) + + request_data = { + "query": {"input": "hello"}, + "config": { + "blob": { + "completion": { + "provider": "openai-native", + "type": "text", + "params": {"model": "gpt-4o"}, + }, + "input_guardrails": [], + "output_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_2} + ], + } + }, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + + output_guardrail_metadata = result["metadata"]["output_guardrail"] + assert output_guardrail_metadata["llm_output_pre_guardrail"] == raw_llm_output + assert output_guardrail_metadata["response_to_user"] == sanitized_output + + validators = output_guardrail_metadata["validators"] + assert len(validators) == 1 + assert validators[0]["name"] == "PIIRemover" + assert validators[0]["outcome"] == "FAIL" + assert validators[0]["input_text"] == raw_llm_output + assert validators[0]["output_text"] == sanitized_output + def test_guardrails_output_validation_sends_input_output_pair( self, db, job_env, job_for_execution ): From 6858906c6fd1a163c832a037ac943bd2db0f2517 Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Wed, 9 Sep 2026 11:53:45 +0530 Subject: [PATCH 08/12] fix: bugs --- backend/app/core/sentry_filters.py | 13 ++++++++++--- backend/app/crud/llm.py | 2 +- backend/app/tests/core/test_sentry_filters.py | 3 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/backend/app/core/sentry_filters.py b/backend/app/core/sentry_filters.py index 13dc0a393..6ac3720c7 100644 --- a/backend/app/core/sentry_filters.py +++ b/backend/app/core/sentry_filters.py @@ -13,7 +13,13 @@ "app.celery.tasks.job_execution.run_llm_chain_job", "app.celery.tasks.job_execution.run_response_job", } -_SENSITIVE_REQUEST_DATA_KEYS = ("query", "request_metadata", "response", "output") +_SENSITIVE_REQUEST_DATA_KEYS = ( + "query", + "request_metadata", + "response", + "output", + "callback_url", +) _SQL_OR_CONNECT = re.compile(r"^(select|insert|update|delete|connect)\b", re.IGNORECASE) @@ -102,8 +108,9 @@ def before_send_transaction_filter( def _redact_llm_job_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]: - """Replaces end-user text fields in an LLM job's request_data with a - placeholder, leaving non-text fields (config, ids, callback_url) intact. + """Replaces end-user text fields and identifying URLs in an LLM job's + request_data with a placeholder, leaving non-sensitive fields (config, ids) + intact. """ request_data = kwargs.get("request_data") if not isinstance(request_data, dict): diff --git a/backend/app/crud/llm.py b/backend/app/crud/llm.py index a0c6448e1..f96b598a1 100644 --- a/backend/app/crud/llm.py +++ b/backend/app/crud/llm.py @@ -224,7 +224,7 @@ def update_llm_call_response( if conversation_id is not None: db_llm_call.conversation_id = conversation_id if metadata is not None: - existing_metadata = db_llm_call.metadata_ or {} + existing_metadata = dict(db_llm_call.metadata_ or {}) existing_metadata.update(metadata) db_llm_call.metadata_ = existing_metadata diff --git a/backend/app/tests/core/test_sentry_filters.py b/backend/app/tests/core/test_sentry_filters.py index 8f750a9d7..1873b7368 100644 --- a/backend/app/tests/core/test_sentry_filters.py +++ b/backend/app/tests/core/test_sentry_filters.py @@ -38,8 +38,9 @@ def test_before_send_filter_redacts_query_for_llm_job(): request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] assert request_data["query"] == "[REDACTED]" + # callback_url can carry identifying/credential data and is redacted too. + assert request_data["callback_url"] == "[REDACTED]" # Non-sensitive fields are left untouched so the trace stays useful. - assert request_data["callback_url"] == "https://webhooksite.net/some-id" assert request_data["config"] == {"blob": {"completion": {"provider": "openai"}}} From 834d8a3e387d03af7e8209f6fcfaa894e68e2dab Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Thu, 10 Sep 2026 16:25:51 +0530 Subject: [PATCH 09/12] fix(guardrails): Address PR review comments on comments/logging Trims overlong code comments, drops redundant log lines in the guardrails proxy, and extracts auth error status codes into a constant, per PR #1195 review feedback. Co-Authored-By: Claude Sonnet 5 --- backend/app/api/routes/guardrails.py | 5 +---- backend/app/celery/celery_app.py | 2 -- backend/app/celery/tasks/job_execution.py | 9 ++------- backend/app/core/sentry_filters.py | 6 ++---- backend/app/services/llm/guardrails.py | 23 ++++++++--------------- 5 files changed, 13 insertions(+), 32 deletions(-) diff --git a/backend/app/api/routes/guardrails.py b/backend/app/api/routes/guardrails.py index afd8ab4b3..d3052ee88 100644 --- a/backend/app/api/routes/guardrails.py +++ b/backend/app/api/routes/guardrails.py @@ -106,10 +106,7 @@ def _upstream_response(status_code: int, payload: Any) -> Response: return JSONResponse(status_code=status_code, content=payload, headers=headers) -# ROUTE ORDERING: every fixed single-segment path below collides with the -# GET /guardrails/{job_id} route declared after this section. FastAPI matches in -# declaration order and does not fall through when {job_id} fails UUID parsing, -# so these must stay above it. +# ROUTE ORDERING: these fixed paths must stay above GET /guardrails/{job_id}. @router.get( diff --git a/backend/app/celery/celery_app.py b/backend/app/celery/celery_app.py index 0859f0c1c..c563277e1 100644 --- a/backend/app/celery/celery_app.py +++ b/backend/app/celery/celery_app.py @@ -44,8 +44,6 @@ def _initialize_worker_observability() -> None: release=settings.API_VERSION, instrumenter="otel", traces_sample_rate=1.0, - # LLM input/output is end-user text; never attach request/response - # bodies to error events or trace transactions. max_request_body_size="never", enable_logs=True, before_send=before_send_filter, diff --git a/backend/app/celery/tasks/job_execution.py b/backend/app/celery/tasks/job_execution.py index a408983c7..e49be35d9 100644 --- a/backend/app/celery/tasks/job_execution.py +++ b/backend/app/celery/tasks/job_execution.py @@ -70,13 +70,8 @@ def _run_with_otel_parent( ) -> T: # noqa: UP047 (black doesn't support PEP 695 generics yet) """Attach the extracted parent context and execute `fn` under it. - opentelemetry-instrumentation-celery's own extraction (CeleryGetter) - reads headers via getattr(task.request, key), but propagation headers - live in task.request.headers — so it never finds them and its `run/...` - span is always an unparented root. We extract from `.headers` ourselves - (see _extract_parent_context) and attach that as current before running - the task body, so spans created inside `fn` correctly nest under the - enqueueing request's trace instead of starting a disconnected one. + Needed because otel's Celery instrumentation misses propagation headers + under `task.request.headers`, leaving `run/...` spans unparented. """ parent_ctx = _extract_parent_context(task_instance) parent_span_ctx = trace.get_current_span(parent_ctx).get_span_context() diff --git a/backend/app/core/sentry_filters.py b/backend/app/core/sentry_filters.py index 6ac3720c7..ae82b436f 100644 --- a/backend/app/core/sentry_filters.py +++ b/backend/app/core/sentry_filters.py @@ -4,10 +4,8 @@ _REDACTED = "[REDACTED]" -# sentry_sdk's CeleryIntegration attaches the task's raw args/kwargs to every -# event captured during that task as extra["celery-job"]. For these tasks, -# kwargs["request_data"] carries the caller's raw query text and (for chain -# jobs) prior block responses -- none of that should reach Sentry. +# LLM job kwargs land in Sentry via sentry_sdk's CeleryIntegration; these keys +# carry end-user text and must be redacted before that happens. _LLM_JOB_TASK_NAMES = { "app.celery.tasks.job_execution.run_llm_job", "app.celery.tasks.job_execution.run_llm_chain_job", diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index bf12512e7..a57fcb9d6 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -56,11 +56,6 @@ def proxy_guardrails_request( # Unset query params must be omitted, not sent as empty values. query = {k: v for k, v in (params or {}).items() if v is not None} - logger.info( - f"[proxy_guardrails_request] Forwarding to guardrails | method: {method}, " - f"url: {url}, organization_id: {organization_id}, project_id: {project_id}" - ) - try: with ( tracer.start_as_current_span( @@ -111,13 +106,15 @@ def proxy_guardrails_request( ) from None +# 422 included: a missing/invalid tenant header is a backend bug, not a +# transient outage, so it must not fall open like one. +_AUTH_ERROR_STATUS_CODES = (401, 403, 422) + + def _is_auth_error(e: Exception) -> TypeGuard[httpx.HTTPStatusError]: - # 422 included: a missing/invalid tenant header is a backend bug, not a - # transient outage, so it must not fall open like one. - return isinstance(e, httpx.HTTPStatusError) and e.response.status_code in ( - 401, - 403, - 422, + return ( + isinstance(e, httpx.HTTPStatusError) + and e.response.status_code in _AUTH_ERROR_STATUS_CODES ) @@ -344,10 +341,6 @@ def run_guardrails_validation( if _is_auth_error(e): # Auth failure means a broken deploy (token/IP mismatch), not a # transient outage — fail the job instead of silently bypassing. - logger.error( - f"[run_guardrails_validation] Guardrails auth failed. " - f"job_id={job_id}, elapsed_ms={elapsed_ms}, error={e}" - ) status_code = e.response.status_code return { "success": False, From 1fa27561a69b135444c1482fdfbcc7ad66e1a0bb Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Thu, 10 Sep 2026 16:54:54 +0530 Subject: [PATCH 10/12] feat: change metadata fields keys to make it more readable --- backend/app/services/llm/jobs.py | 8 ++++---- backend/app/tests/services/llm/test_jobs.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/app/services/llm/jobs.py b/backend/app/services/llm/jobs.py index 62f5baf34..166249e00 100644 --- a/backend/app/services/llm/jobs.py +++ b/backend/app/services/llm/jobs.py @@ -399,8 +399,8 @@ def apply_input_guardrails( if outcome.applied: metadata = { "input_guardrail": { - "user_input": original_input_text, - "input_sent_to_llm": outcome.safe_text, + "input_from_user": original_input_text, + "input_to_llm": outcome.safe_text, "validators": summarize_validator_results(outcome), } } @@ -472,8 +472,8 @@ def apply_output_guardrails( if outcome.applied: existing_metadata = result.metadata or {} existing_metadata["output_guardrail"] = { - "llm_output_pre_guardrail": original_output_text, - "response_to_user": outcome.safe_text, + "output_from_llm": original_output_text, + "output_to_user": outcome.safe_text, "validators": summarize_validator_results(outcome), } result.metadata = existing_metadata diff --git a/backend/app/tests/services/llm/test_jobs.py b/backend/app/tests/services/llm/test_jobs.py index fd8bdf898..1ca5e362a 100644 --- a/backend/app/tests/services/llm/test_jobs.py +++ b/backend/app/tests/services/llm/test_jobs.py @@ -1413,8 +1413,8 @@ def test_guardrails_metadata_reports_per_validator_text_for_input_guardrail( assert result["success"] input_guardrail_metadata = result["metadata"]["input_guardrail"] - assert input_guardrail_metadata["user_input"] == unsafe_input - assert input_guardrail_metadata["input_sent_to_llm"] == sanitized_input + assert input_guardrail_metadata["input_from_user"] == unsafe_input + assert input_guardrail_metadata["input_to_llm"] == sanitized_input validators = input_guardrail_metadata["validators"] assert len(validators) == 1 @@ -1591,8 +1591,8 @@ def test_guardrails_metadata_reports_per_validator_text_for_output_guardrail( assert result["success"] output_guardrail_metadata = result["metadata"]["output_guardrail"] - assert output_guardrail_metadata["llm_output_pre_guardrail"] == raw_llm_output - assert output_guardrail_metadata["response_to_user"] == sanitized_output + assert output_guardrail_metadata["output_from_llm"] == raw_llm_output + assert output_guardrail_metadata["output_to_user"] == sanitized_output validators = output_guardrail_metadata["validators"] assert len(validators) == 1 From c6fe07d81e3b80670a536589b60898fe678fe2ba Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Thu, 10 Sep 2026 17:12:35 +0530 Subject: [PATCH 11/12] feat(guardrails): Gate intermediate guardrail metadata behind opt-in toggle Guardrail input/output text and per-validator results were always attached to llm_call metadata whenever guardrails ran, leaking pre-sanitisation text by default. Add include_guardrail_metadata on LLMCallRequest (default false) to make this opt-in. Co-Authored-By: Claude Sonnet 5 --- backend/app/models/llm/request.py | 4 ++ backend/app/services/llm/jobs.py | 16 +++-- backend/app/tests/services/llm/test_jobs.py | 69 +++++++++++++++++++++ 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/backend/app/models/llm/request.py b/backend/app/models/llm/request.py index 63b2e7fbb..f18e6af4e 100644 --- a/backend/app/models/llm/request.py +++ b/backend/app/models/llm/request.py @@ -589,6 +589,10 @@ class LLMCallRequest(SQLModel): default=False, description="Whether to include the raw LLM provider response in the output", ) + include_guardrail_metadata: bool = Field( + default=False, + description="Include per-validator guardrail metadata (input/output text, pass/fail) in the response", + ) request_metadata: dict[str, Any] | None = Field( default=None, description=( diff --git a/backend/app/services/llm/jobs.py b/backend/app/services/llm/jobs.py index 166249e00..c07084d34 100644 --- a/backend/app/services/llm/jobs.py +++ b/backend/app/services/llm/jobs.py @@ -371,6 +371,7 @@ def apply_input_guardrails( job_id: UUID, project_id: int, organization_id: int, + include_guardrail_metadata: bool = False, ) -> tuple[QueryParams, str | None, str | None, dict[str, Any] | None]: """Apply input guardrails from a config_blob. Shared with llm-call and llm-chain. @@ -396,7 +397,8 @@ def apply_input_guardrails( organization_id=organization_id, ) metadata = None - if outcome.applied: + # outocome.applied if true i.e guarrails not bypassed. + if outcome.applied and include_guardrail_metadata: metadata = { "input_guardrail": { "input_from_user": original_input_text, @@ -417,10 +419,6 @@ def apply_input_guardrails( # No-op paths (no validators, bypassed) leave the query untouched. if outcome.applied and outcome.safe_text is not None: if not outcome.safe_text.strip(): - # A fix-mode validator that supplies no fix_value (e.g. topic_relevance - # with no built-in fix) falls back to "" — forwarding that to the LLM - # provider fails with a confusing provider-side error instead of a - # clear guardrails-blocked one. logger.warning( f"[apply_input_guardrails] Guardrails reduced input to empty text; " f"blocking request | job_id={job_id}" @@ -443,6 +441,7 @@ def apply_output_guardrails( project_id: int, organization_id: int, input_text: str | None = None, + include_guardrail_metadata: bool = False, ) -> tuple[BlockResult, str | None]: """Apply output guardrails from a config_blob. Shared by /llm/call and /llm/chain. @@ -469,7 +468,7 @@ def apply_output_guardrails( output_text=original_output_text, ) - if outcome.applied: + if outcome.applied and include_guardrail_metadata: existing_metadata = result.metadata or {} existing_metadata["output_guardrail"] = { "output_from_llm": original_output_text, @@ -562,6 +561,7 @@ def execute_llm_call( request_metadata: dict | None, langfuse_credentials: dict | None, include_provider_raw_response: bool = False, + include_guardrail_metadata: bool = False, chain_id: UUID | None = None, detected_language: str | None = None, ) -> BlockResult: @@ -654,6 +654,7 @@ def execute_llm_call( job_id=job_id, project_id=project_id, organization_id=organization_id, + include_guardrail_metadata=include_guardrail_metadata, ) if input_guardrail_metadata: if request_metadata is None: @@ -918,6 +919,7 @@ def execute_llm_call( project_id=project_id, organization_id=organization_id, input_text=original_input_value, + include_guardrail_metadata=include_guardrail_metadata, ) if output_error: out_guard_span.set_status( @@ -1311,6 +1313,7 @@ def execute_llm_call( project_id=project_id, organization_id=organization_id, input_text=original_input_value, + include_guardrail_metadata=include_guardrail_metadata, ) if output_error: out_guard_span.set_status( @@ -1423,6 +1426,7 @@ def execute_job( request_metadata=request.request_metadata, langfuse_credentials=langfuse_credentials, include_provider_raw_response=request.include_provider_raw_response, + include_guardrail_metadata=request.include_guardrail_metadata, ) logger.info( diff --git a/backend/app/tests/services/llm/test_jobs.py b/backend/app/tests/services/llm/test_jobs.py index 1ca5e362a..6ed5bd648 100644 --- a/backend/app/tests/services/llm/test_jobs.py +++ b/backend/app/tests/services/llm/test_jobs.py @@ -1406,6 +1406,7 @@ def test_guardrails_metadata_reports_per_validator_text_for_input_guardrail( } }, "include_provider_raw_response": False, + "include_guardrail_metadata": True, "callback_url": None, } result = self._execute_job(job_for_execution, db, request_data) @@ -1423,6 +1424,73 @@ def test_guardrails_metadata_reports_per_validator_text_for_input_guardrail( assert validators[0]["input_text"] == unsafe_input assert validators[0]["output_text"] == sanitized_input + def test_guardrails_metadata_omitted_by_default( + self, db, job_env, job_for_execution + ): + """Guardrails still sanitize the input, but input_guardrail metadata is + left out unless include_guardrail_metadata is explicitly set.""" + env = job_env + env["provider"].execute.return_value = (env["mock_llm_response"], None) + + unsafe_input = "My credit card is 4111 1111 1111 1111" + sanitized_input = "My credit card is [REDACTED]" + + with ( + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + ): + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": sanitized_input, + "rephrase_needed": False, + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "input", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": unsafe_input, + "output_text": sanitized_input, + } + ], + }, + } + mock_fetch_configs.return_value = ( + [{"type": "pii_remover", "stage": "input"}], + [], + ) + + request_data = { + "query": {"input": unsafe_input}, + "config": { + "blob": { + "completion": { + "provider": "openai-native", + "type": "text", + "params": {"model": "gpt-4o"}, + }, + "input_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_1} + ], + "output_guardrails": [], + } + }, + "include_provider_raw_response": False, + "callback_url": None, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + assert not result["metadata"] or "input_guardrail" not in result["metadata"] + def test_guardrails_skip_input_validation_for_audio_input( self, db, job_env, job_for_execution ): @@ -1585,6 +1653,7 @@ def test_guardrails_metadata_reports_per_validator_text_for_output_guardrail( ], } }, + "include_guardrail_metadata": True, } result = self._execute_job(job_for_execution, db, request_data) From fce8e4dbd8b46d149e8e2f988dd7067e5946f56f Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Fri, 11 Sep 2026 17:10:19 +0530 Subject: [PATCH 12/12] fix: codecov tests cases --- backend/app/tests/services/llm/test_jobs.py | 156 +++++++++++++++++++- 1 file changed, 154 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/services/llm/test_jobs.py b/backend/app/tests/services/llm/test_jobs.py index a8f339f87..81bad7eb7 100644 --- a/backend/app/tests/services/llm/test_jobs.py +++ b/backend/app/tests/services/llm/test_jobs.py @@ -323,10 +323,15 @@ class TestExecuteJob: def mock_llm_call_crud(self): with ( patch("app.services.llm.jobs.create_llm_call") as mock_create_llm_call, - patch("app.services.llm.jobs.update_llm_call_response"), + patch( + "app.services.llm.jobs.update_llm_call_response" + ) as mock_update_llm_call_response, ): mock_create_llm_call.return_value = MagicMock(id=uuid4()) - yield + yield { + "create_llm_call": mock_create_llm_call, + "update_llm_call_response": mock_update_llm_call_response, + } @pytest.fixture def job_for_execution(self, db: Session): @@ -914,6 +919,94 @@ def test_proxy_output_guardrails_failure_returns_error( assert not result["success"] assert "Output blocked by guardrails" in result["error"] + def test_proxy_output_guardrails_success_persists_sanitized_content( + self, db, job_env, job_for_execution, mock_llm_call_crud + ): + """Successful output-guardrail sanitisation in the proxy branch is + re-persisted onto the LlmCall row via update_llm_call_response + (jobs.py:929-939).""" + request_data = { + "query": {"input": "hi"}, + "config": { + "blob": { + "completion": { + "type": "proxy", + "provider": None, + "params": { + "client_llm_url": "https://api.tap.example/v1/predictions" + }, + }, + "input_guardrails": [], + "output_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_2} + ], + } + }, + "include_provider_raw_response": False, + "callback_url": None, + } + + fake_resp = MagicMock() + fake_resp.raise_for_status = MagicMock() + fake_resp.json.return_value = { + "id": "resp_abc", + "model": "gpt-5", + "output": [ + { + "type": "message", + "content": [ + {"type": "output_text", "text": "Aadhar no 123-45-6789"} + ], + } + ], + "usage": {"input_tokens": 1, "output_tokens": 1, "total_tokens": 2}, + } + fake_client = MagicMock() + fake_client.__enter__.return_value = fake_client + fake_client.__exit__.return_value = None + fake_client.post.return_value = fake_resp + + with ( + patch( + "app.services.llm.jobs.get_provider_credential", + return_value={"api_key": "tap-token"}, + ), + patch("app.services.llm.jobs.httpx.Client", return_value=fake_client), + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + ): + mock_fetch_configs.return_value = ( + [], + [{"type": "pii_remover", "stage": "output"}], + ) + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": "Aadhar [REDACTED]", + "rephrase_needed": False, + }, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + assert ( + result["data"]["response"]["output"]["content"]["value"] + == "Aadhar [REDACTED]" + ) + + mock_update = mock_llm_call_crud["update_llm_call_response"] + assert mock_update.call_count == 2 + _, guardrail_persist_kwargs = mock_update.call_args_list[-1] + assert ( + guardrail_persist_kwargs["content"]["content"]["value"] + == "Aadhar [REDACTED]" + ) + def test_metadata_in_callback_response( self, db, job_env, job_for_execution, request_data ): @@ -1724,6 +1817,65 @@ def test_guardrails_output_validation_sends_input_output_pair( assert kwargs.get("output_text") == llm_output assert mock_guardrails.call_args[0][0] == user_query + def test_guardrails_output_persists_sanitized_content_via_update_llm_call_response( + self, db, job_env, job_for_execution, mock_llm_call_crud + ): + """Successful output-guardrail sanitisation on the non-proxy branch + is re-persisted onto the LlmCall row via update_llm_call_response + (jobs.py:1323-1331).""" + env = job_env + env["mock_llm_response"].response.output.content.value = "Aadhar no 123-45-6789" + env["provider"].execute.return_value = (env["mock_llm_response"], None) + + with ( + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + ): + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": "Aadhar [REDACTED]", + "rephrase_needed": False, + }, + } + mock_fetch_configs.return_value = ( + [], + [{"type": "pii_remover", "stage": "output"}], + ) + + request_data = { + "query": {"input": "hello"}, + "config": { + "blob": { + "completion": { + "provider": "openai-native", + "type": "text", + "params": {"model": "gpt-4o"}, + }, + "input_guardrails": [], + "output_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_2} + ], + } + }, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + + mock_update = mock_llm_call_crud["update_llm_call_response"] + assert mock_update.call_count == 2 + _, guardrail_persist_kwargs = mock_update.call_args_list[-1] + assert ( + guardrail_persist_kwargs["content"]["content"]["value"] + == "Aadhar [REDACTED]" + ) + def test_guardrails_bypass_does_not_modify_output( self, db, job_env, job_for_execution ):