fix(tool-service): harden _coerce_to_tool_result last-resort fallback against broken str/repr

Codex stop-review found that the prior fix still had escape hatches:
``type(payload).__name__``, ``str(payload)`` and ``repr(payload)`` all
live inside the last-resort fallback but none were individually
guarded, so a payload whose ``__str__`` and ``__repr__`` both raise
(or whose ``type().__name__`` lookup fails) could still escape the
helper and break the "never raises" invariant.

Extract two module-level helpers with staged defensive fallbacks:

* ``_safe_type_name(obj)`` — returns ``type(obj).__name__`` or
  ``"<untypeable>"`` sentinel, never raises.
* ``_safe_text_repr(obj, fallback_type)`` — tries ``str`` → ``repr`` →
  ``f"<{fallback_type} object (unrepresentable)>"``. Guarantees a
  ``str`` return on every branch; each coercion attempt is
  try/except-guarded, and the final tier is a pre-computed literal
  so even a metaclass whose ``__name__`` property raises cannot
  propagate.

``_coerce_to_tool_result``'s opaque-JSON fallback now computes
``payload_type`` via ``_safe_type_name`` once up front and passes it
both to the DEBUG/WARNING log sites and to ``_safe_text_repr``.

Regression guard: ``test_payload_with_broken_str_and_repr_still_produces_toolresult``
exercises an ``AdversarialPayload`` whose ``__str__`` and ``__repr__``
both raise ``RuntimeError``. Against the pre-fix code either the
``str(payload)`` call or the fall-through ``repr(payload)`` would
propagate; against the new shape the helper emits the third-tier
sentinel text and a WARNING log. Empirically validated via an
in-line stress test of ``_safe_type_name`` and ``_safe_text_repr``
against a metaclass with a ``__name__`` property that raises.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

Author: jonpspri

.secrets.baseline

@@ -3,7 +3,7 @@
     "files": "^.secrets.baseline|package-lock.json|Cargo.lock|scripts/sign_image.sh|scripts/zap|sonar-project.properties|uv.lock|go.sum|mcpgateway/sri_hashes.json|^.secrets.baseline$",
     "lines": null
   },
-  "generated_at": "2026-04-14T23:25:53Z",
+  "generated_at": "2026-04-14T23:29:31Z",
   "plugins_used": [
     {
       "name": "AWSKeyDetector"
@@ -8666,95 +8666,95 @@
         "hashed_secret": "2dd2850bcc4ae4e74154aed99ee5f68292ecfec5",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 2936,
+        "line_number": 2980,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "d92342976d720ff38cf5dcb329be41959ab1ba6c",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 3623,
+        "line_number": 3667,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "f93640458964af294a485bc6d597694cf3308e1f",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 3632,
+        "line_number": 3676,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "0857abc2d90c5d9821229fc0a880f1c38ffb0e04",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 4023,
+        "line_number": 4067,
         "type": "Hex High Entropy String",
         "verified_result": null
       },
       {
         "hashed_secret": "99834bc4eff3f1e1c1e4692d2476b593b501d045",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6951,
+        "line_number": 6995,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "f2b14f68eb995facb3a1c35287b778d5bd785511",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6969,
+        "line_number": 7013,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "b8cec023ad982a1355abb9e7c7700e42d7e6fac3",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6993,
+        "line_number": 7037,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "0614eb27c6e0d2f4ed9a1cce2a5bcbbbc17aa556",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 7057,
+        "line_number": 7101,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "a38fea79a043f0c9a62f851659d459dc3b716ea9",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 7068,
+        "line_number": 7112,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "a50da1fb101595ac5158f2c9394a65a84061b56c",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 7109,
+        "line_number": 7153,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "ed3e0017cb8e4b06a59af1a441f62cbe58d2ef59",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 7115,
+        "line_number": 7159,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "9bdc408babb4c5740aa9ee42e65b9308bd01f5f9",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 8238,
+        "line_number": 8282,
         "type": "Secret Keyword",
         "verified_result": null
       }

mcpgateway/services/tool_service.py

@@ -371,6 +371,61 @@ def _looks_like_mcp_envelope(payload: dict) -> bool:
     return False
 
 
+def _safe_type_name(obj: Any) -> str:
+    """Return ``type(obj).__name__``, or a sentinel if that itself raises.
+
+    Used inside :meth:`ToolService._coerce_to_tool_result`'s last-resort
+    fallback so logging and diagnostic text on a pathological object
+    (broken proxy, ``__class__`` descriptor that raises, etc.) cannot
+    themselves raise and escape the "always returns a valid
+    ``ToolResult``" invariant.
+    """
+    try:
+        return type(obj).__name__
+    except Exception:  # pylint: disable=broad-except
+        return "<untypeable>"
+
+
+def _safe_text_repr(obj: Any, fallback_type: str) -> str:
+    """Coerce an arbitrary object to a non-raising text representation.
+
+    Tries ``str(obj)``, then ``repr(obj)``, then ``f"<{fallback_type}
+    object>"``, and finally a fixed sentinel. Used by the opaque-JSON
+    last-resort branch of
+    :meth:`ToolService._coerce_to_tool_result` — neither ``str`` nor
+    ``repr`` is guaranteed to succeed on arbitrary Python objects (a
+    class can override either to raise), and without this staged
+    fallback a malicious or simply buggy payload could escape the
+    helper and break the "never raises" contract downstream code relies
+    on.
+
+    Args:
+        obj: The payload to stringify.
+        fallback_type: Pre-computed type name used in the third-tier
+            sentinel; keeps ``type().__name__`` out of this function's
+            hot path since it has its own :func:`_safe_type_name`
+            guard upstream.
+
+    Returns:
+        A ``str``. Never raises.
+    """
+    try:
+        text = str(obj)
+        if isinstance(text, str):
+            return text
+    except Exception:  # pylint: disable=broad-except
+        pass
+    try:
+        text = repr(obj)
+        if isinstance(text, str):
+            return text
+    except Exception:  # pylint: disable=broad-except
+        pass
+    # ``fallback_type`` came from ``_safe_type_name`` so it's
+    # guaranteed to be a ``str`` already.
+    return f"<{fallback_type} object (unrepresentable)>"
+
+
 def _handle_json_parse_error(response, error, is_error_response: bool = False) -> dict:
     """Handle JSON parsing failures with graceful fallback to raw text.
 
@@ -1673,36 +1728,27 @@ def _coerce_to_tool_result(self, payload: Any) -> ToolResult:
 
         # Last resort — opaque JSON body. Preserves the payload for the
         # caller to inspect while keeping the rest of the pipeline on a
-        # uniform ``ToolResult`` contract. Log at DEBUG so operators can
-        # spot integrations regularly hitting this branch (which usually
-        # indicates an upstream whose shape the heuristic ought to learn
-        # to recognise).
-        logger.debug("Coercing %s payload to opaque text content", type(payload).__name__)
-        # Both ``BaseModel.model_dump`` and ``orjson.dumps`` can raise on
-        # pathological inputs: a BaseModel with a custom field serialiser
-        # that throws, a ``PydanticSerializationError`` (``ValueError``
-        # subclass, not ``TypeError``), or a plain Python object that
-        # isn't JSON-representable. Wrap the whole dump + serialise
-        # sequence so the helper's "always returns a valid ``ToolResult``"
-        # invariant holds even in the presence of upstream bugs.
+        # uniform ``ToolResult`` contract. The helper's invariant is
+        # "always returns a valid ``ToolResult``, never raises", so every
+        # step below must be guarded — both ``BaseModel.model_dump`` and
+        # ``orjson.dumps`` can raise on pathological inputs (custom
+        # serialisers that throw, ``PydanticSerializationError`` —
+        # ``ValueError`` subclass, not ``TypeError`` — fields holding
+        # non-representable objects), and even ``str()`` / ``repr()`` /
+        # ``type().__name__`` can fail on sufficiently broken proxy
+        # objects.
+        payload_type = _safe_type_name(payload)
+        logger.debug("Coercing %s payload to opaque text content", payload_type)
         try:
             serializable_payload = payload.model_dump(mode="json", by_alias=True) if isinstance(payload, BaseModel) else payload
             serialized = orjson.dumps(serializable_payload, option=orjson.OPT_INDENT_2)
         except Exception:  # pylint: disable=broad-except
-            # Last-resort ``str(payload)`` so callers still see *something*.
-            # ``str()`` on a pathological object could itself raise, but
-            # ``repr()`` on an arbitrary Python object is effectively
-            # guaranteed by the runtime — keep the fallback defensive.
             logger.warning(
-                "Payload of type %s could not be JSON-serialised; using str() fallback",
-                type(payload).__name__,
+                "Payload of type %s could not be JSON-serialised; using textual fallback",
+                payload_type,
                 exc_info=True,
             )
-            try:
-                text = str(payload)
-            except Exception:  # pylint: disable=broad-except
-                text = repr(payload)
-            return ToolResult(content=[TextContent(type="text", text=text)])
+            return ToolResult(content=[TextContent(type="text", text=_safe_text_repr(payload, payload_type))])
         return ToolResult(content=[TextContent(type="text", text=serialized.decode())])
 
     async def register_tool(

tests/unit/mcpgateway/services/test_tool_service_coverage.py

@@ -2332,6 +2332,50 @@ def test_rejects_dict_with_only_meta_marker(self, tool_service):
         roundtripped = orjson.loads(result.content[0].text)
         assert roundtripped == payload
 
+    def test_payload_with_broken_str_and_repr_still_produces_toolresult(self, tool_service, caplog):
+        """Even objects whose ``__str__`` *and* ``__repr__`` raise yield a ToolResult.
+
+        The helper's hardest invariant is "never raises, always returns
+        a valid ``ToolResult``". This test exercises the extreme tail of
+        the opaque-JSON last-resort branch: a payload that (1) is not a
+        BaseModel with MCP-shaped fields, (2) isn't JSON-serialisable
+        (so ``orjson.dumps`` raises), (3) whose ``__str__`` raises when
+        the helper tries ``str(payload)``, and (4) whose ``__repr__``
+        also raises. Without ``_safe_text_repr``'s third-tier
+        ``f"<{type_name} object>"`` sentinel, either of those would
+        escape the helper.
+
+        Pairs with ``test_non_json_serialisable_payload_falls_back_to_str``
+        (normal ``str()`` path) and
+        ``test_basemodel_whose_model_dump_raises_falls_back_to_str``
+        (BaseModel.model_dump raising).
+        """
+
+        class AdversarialPayload:
+            """Payload whose textual representations both raise."""
+
+            def __str__(self) -> str:  # pragma: no cover - intentionally explodes
+                raise RuntimeError("__str__ is broken")
+
+            def __repr__(self) -> str:  # pragma: no cover - intentionally explodes
+                raise RuntimeError("__repr__ is also broken")
+
+        payload = AdversarialPayload()
+
+        with caplog.at_level("WARNING", logger="mcpgateway.services.tool_service"):
+            result = tool_service._coerce_to_tool_result(payload)
+
+        # Contract held — we got a ``ToolResult`` rather than an exception.
+        assert isinstance(result, ToolResult)
+        assert result.content[0].type == "text"
+        # Third-tier sentinel used; must be a non-empty string.
+        assert "AdversarialPayload" in result.content[0].text
+        assert "unrepresentable" in result.content[0].text
+        # WARNING-severity reshape log should still have fired.
+        relevant = [r for r in caplog.records if "could not be JSON-serialised" in r.message]
+        assert relevant, "Expected a WARNING-level log from the last-resort fallback"
+        assert relevant[-1].levelname == "WARNING"
+
     def test_basemodel_whose_model_dump_raises_falls_back_to_str(self, tool_service, caplog):
         """``BaseModel.model_dump`` raising is caught by the last-resort handler.