ritwik-g · Oct 28, 2026 · Oct 28, 2026 · Oct 28, 2026 · Oct 28, 2026 · chandrasekharan-zipstack
diff --git a/backend/dashboard_metrics/tests/test_health_check.py b/backend/dashboard_metrics/tests/test_health_check.py
@@ -0,0 +1,77 @@
+"""``metrics/health/`` must not hand infrastructure details to the caller.
+
+The endpoint answers any org member. A failing DB or cache raises an exception
+whose text names the host, port and user; that belongs in the server log, not
+the response body (CodeQL py/stack-trace-exposure).
+"""
+
+from unittest import mock
+
+from rest_framework import status
+from rest_framework.test import APIRequestFactory
+
+from dashboard_metrics import views
+from dashboard_metrics.views import DashboardMetricsViewSet
+
+DB_ERROR = (
+    'connection to server at "db.internal" (10.0.0.5), port 5432 failed: '
+    'FATAL: password authentication failed for user "unstract_dev"'
+)
+CACHE_ERROR = "Error 111 connecting to redis.internal:6379. Connection refused."
+
+
+def _health():
+    viewset = DashboardMetricsViewSet()
+    request = APIRequestFactory().get("/metrics/health/")
+    return viewset.health(request)
+
+
+def test_database_failure_hides_exception_text_and_logs_it():
+    with (
+        mock.patch.object(
+            views.EventMetricsHourly.objects, "exists", side_effect=Exception(DB_ERROR)
+        ),
+        mock.patch.object(views.cache, "set"),
+        mock.patch.object(views.cache, "get", return_value="ok"),
+        mock.patch.object(views.logger, "exception") as log_exception,
+    ):
+        response = _health()
+
+    assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE
+    assert response.data["healthy"] is False
+    assert response.data["checks"]["database"] == {
+        "status": "error",
+        "message": "Database unavailable",
+    }
+    assert response.data["checks"]["cache"] == {"status": "ok"}
+    assert "db.internal" not in str(response.data)
+    log_exception.assert_called_once_with("Database health check failed")
+
+
+def test_cache_failure_hides_exception_text_and_logs_it():
+    with (
+        mock.patch.object(views.EventMetricsHourly.objects, "exists", return_value=True),
+        mock.patch.object(views.cache, "set", side_effect=Exception(CACHE_ERROR)),
+        mock.patch.object(views.logger, "exception") as log_exception,
+    ):
+        response = _health()
+
+    assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE
+    assert response.data["checks"]["cache"] == {
+        "status": "error",
+        "message": "Cache unavailable",
+    }
+    assert "redis.internal" not in str(response.data)
+    log_exception.assert_called_once_with("Cache health check failed")
+
+
+def test_all_healthy_is_200():
+    with (
+        mock.patch.object(views.EventMetricsHourly.objects, "exists", return_value=True),
+        mock.patch.object(views.cache, "set"),
+        mock.patch.object(views.cache, "get", return_value="ok"),
+    ):
+        response = _health()
+
+    assert response.status_code == status.HTTP_200_OK
+    assert response.data["healthy"] is True
diff --git a/backend/dashboard_metrics/views.py b/backend/dashboard_metrics/views.py
@@ -1161,9 +1161,11 @@ def _check_database(self) -> dict:
         try:
             EventMetricsHourly.objects.exists()
             return {"status": "ok"}
-        except Exception as e:
+        except Exception:
+            # The exception text names the DB host, port and user, and this
+            # endpoint answers any org member, so it stays in the server log.
-            # The exception text names the DB host, port and user, and this
-            # endpoint answers any org member, so it stays in the server log.
-            # The exception text names the DB host, port and user, and this
-            # endpoint answers any org member, so it stays in the server log.
             logger.exception("Database health check failed")
-            return {"status": "error", "message": str(e)}
+            return {"status": "error", "message": "Database unavailable"}
 
     def _check_cache(self) -> dict:
         """Check cache connectivity."""
@@ -1173,6 +1175,7 @@ def _check_cache(self) -> dict:
             if result == "ok":
                 return {"status": "ok"}
             return {"status": "error", "message": "Cache read/write mismatch"}
-        except Exception as e:
+        except Exception:
+            # Same as the database check: the Redis error names host and port.
-            # Same as the database check: the Redis error names host and port.
-            # Same as the database check: the Redis error names host and port.
             logger.exception("Cache health check failed")
-            return {"status": "error", "message": str(e)}
+            return {"status": "error", "message": "Cache unavailable"}
diff --git a/backend/workflow_manager/endpoint_v2/destination.py b/backend/workflow_manager/endpoint_v2/destination.py
@@ -635,10 +635,19 @@ def get_tool_execution_result_from_metadata(
                 result = file_content.encode("utf-8").decode("unicode-escape")
             else:
                 raise InvalidToolOutputType()
-        except (FileNotFoundError, json.JSONDecodeError) as err:
-            msg = f"Error while getting result from the tool: {err}"
-            logger.error(msg)
-            raise APIException(detail=msg)
+        except FileNotFoundError:
+            # The error carries the execution-storage path, which stays in the
+            # server log (with the traceback); the caller gets a message
+            # without it.
-            # The error carries the execution-storage path, which stays in the
-            # server log (with the traceback); the caller gets a message
-            # without it.
-            # The error carries the execution-storage path, which stays in the
-            # server log (with the traceback); the caller gets a message
-            # without it.
+            logger.exception(f"Error while getting result from the tool ({output_file})")
+            raise APIException(
+                detail="Error while getting result from the tool: output not found"
+            )
+        except json.JSONDecodeError:
+            logger.exception(f"Error while getting result from the tool ({output_file})")
+            raise APIException(
+                detail="Error while getting result from the tool: output is not valid JSON"
+            )
 
         return result
 
@@ -1119,7 +1128,7 @@ def _read_file_content_for_queue(self, input_file_path: str, file_name: str) ->
 
         Args:
             input_file_path: Path to the file to read
-            file_name: Name of the file for logging purposes
+            file_name: Name of the file, used in the log and the caller-facing error
 
         Returns:
             Base64 encoded file content
@@ -1132,12 +1141,14 @@ def _read_file_content_for_queue(self, input_file_path: str, file_name: str) ->
             file_storage = file_system.get_file_storage()
 
             if not file_storage.exists(input_file_path):
-                raise APIException(f"File not found: {input_file_path}")
+                raise FileNotFoundError(f"File not found: {input_file_path}")
 
             file_bytes = file_storage.read(input_file_path, mode="rb")
             if isinstance(file_bytes, str):
                 file_bytes = file_bytes.encode("utf-8")
             return base64.b64encode(file_bytes).decode("utf-8")
-        except Exception as e:
-            logger.error(f"Failed to read file content for {file_name}: {e}")
-            raise APIException(f"Failed to read file content for queue: {e}")
+        except Exception:
+            # The error names the execution-storage path: log it with the
+            # traceback, but keep it out of the message returned to the caller.
-            # The error names the execution-storage path: log it with the
-            # traceback, but keep it out of the message returned to the caller.
-            # The error names the execution-storage path: log it with the
-            # traceback, but keep it out of the message returned to the caller.
+            logger.exception(f"Failed to read file content for {file_name}")
+            raise APIException(f"Failed to read file content for queue: {file_name}")
diff --git a/backend/workflow_manager/endpoint_v2/tests/test_destination_error_detail.py b/backend/workflow_manager/endpoint_v2/tests/test_destination_error_detail.py
@@ -0,0 +1,125 @@
+"""Destination read failures must not put the execution-storage path in the
+``APIException`` detail returned to the caller (CodeQL py/stack-trace-exposure).
+
+The path (bucket/org/workflow/execution layout) still goes to the server log.
+"""
+
+from __future__ import annotations
+
+import os
+import sys
+from unittest import mock
+
+import django
+import pytest
+from django.apps import apps
+
+os.environ.setdefault("DJANGO_SETTINGS_MODULE", "backend.settings.test")
+if not apps.ready:
+    django.setup()
+
+from rest_framework.exceptions import APIException  # noqa: E402
+
+from unstract.workflow_execution.constants import ToolOutputType  # noqa: E402
+from workflow_manager.endpoint_v2 import destination  # noqa: E402
+from workflow_manager.endpoint_v2.destination import DestinationConnector  # noqa: E402
+
+STORAGE_PATH = "unstract-execution/org_abc/wf_123/exec_456/file_789/INFILE"
+
+
+def _connector() -> DestinationConnector:
+    connector = DestinationConnector.__new__(DestinationConnector)
+    connector.infile = STORAGE_PATH
+    connector.get_workflow_metadata = mock.Mock(return_value={})
+    connector.get_output_type = mock.Mock(return_value=ToolOutputType.JSON)
+    return connector
+
+
+def _logged_exceptions(logger):
+    """Patch ``logger.exception``, recording (message, exception being handled).
+
+    ``logger.exception`` writes the handled exception's traceback, so the
+    exception recorded here is what reaches the server log alongside the
+    message.
+    """
+    records: list[tuple[str, BaseException | None]] = []
+    patcher = mock.patch.object(
+        logger,
+        "exception",
+        # exc_info=False would log no traceback, so record nothing handled.
+        side_effect=lambda msg, *a, **k: records.append(
+            (msg, sys.exc_info()[1] if k.get("exc_info", True) else None)
+        ),
+    )
+    return patcher, records
+
+
+def _patched_storage(storage: mock.Mock):
+    file_system = mock.Mock()
+    file_system.return_value.get_file_storage.return_value = storage
+    return mock.patch.object(destination, "FileSystem", file_system)
+
+
+@pytest.mark.parametrize(
+    ("read_error", "expected_detail"),
+    [
+        (
+            FileNotFoundError(f"No such file: {STORAGE_PATH}"),
+            "Error while getting result from the tool: output not found",
+        ),
+        (
+            # ``doc`` is the file content, as json.loads would raise it.
+            destination.json.JSONDecodeError("Expecting value", "", 0),
+            "Error while getting result from the tool: output is not valid JSON",
+        ),
+    ],
+)
+def test_tool_result_error_hides_storage_path(read_error, expected_detail):
+    storage = mock.Mock()
+    storage.mime_type.return_value = "application/json"
+    storage.read.side_effect = read_error
+
+    connector = _connector()
+    log_patch, logged = _logged_exceptions(destination.logger)
+
+    with _patched_storage(storage), log_patch:
+        with pytest.raises(APIException) as exc_info:
+            connector.get_tool_execution_result_from_metadata()
+
+    assert str(exc_info.value.detail) == expected_detail
+    assert STORAGE_PATH not in str(exc_info.value.detail)
+    [(message, handled)] = logged
+    assert STORAGE_PATH in message
+    assert handled is read_error
+
+
+@pytest.mark.parametrize(
+    "storage_setup",
+    [
+        pytest.param({"exists.return_value": False}, id="missing"),
+        pytest.param(
+            {
+                "exists.return_value": True,
+                "read.side_effect": OSError(f"read failed: {STORAGE_PATH}"),
+            },
+            id="read-error",
+        ),
+    ],
+)
+def test_queue_file_read_error_hides_storage_path(storage_setup):
+    storage = mock.Mock(**storage_setup)
+
+    connector = _connector()
+    log_patch, logged = _logged_exceptions(destination.logger)
+
+    with _patched_storage(storage), log_patch:
+        with pytest.raises(APIException) as exc_info:
+            connector._read_file_content_for_queue(STORAGE_PATH, "invoice.pdf")
+
+    detail = str(exc_info.value.detail)
+    assert detail == "Failed to read file content for queue: invoice.pdf"
+    assert STORAGE_PATH not in detail
+    # The path reaches the log through the handled exception's traceback.
+    [(message, handled)] = logged
+    assert message == "Failed to read file content for invoice.pdf"
+    assert STORAGE_PATH in str(handled)