fix(mcp): Apply parameters override in _apply_tool_overrides

[prince-shakyaa]

fix (mcp): Apply parameters override in _apply_tool_overrides

Fixes #547

What changed

finbot/mcp/factory.py - _apply_tool_overrides

Previously the function only read description from each tool override entry and silently discarded any parameters / inputSchema block the caller had stored.
This PR widens the implementation to also apply the parameter schema when one is present in the override dict.

Why

The PUT /darklab/api/v1/supply-chain/servers/{server_type}/tools endpoint accepts and persists arbitrary JSON per tool (including parameters), the Dark Lab UI displays the full stored object, and the supply-chain stats endpoint counted parameter-bearing overrides as "poisoned tools" -- but the agent runtime never saw the changed schema. This created a silent mismatch between what was stored and what was actually active.

Changes

finbot/mcp/factory.py

-async def _apply_tool_overrides(server: FastMCP, overrides: dict) -> None:
-    """Apply user-supplied tool description overrides to a FastMCP server.
-
-    Modifies tool descriptions (the text the LLM sees) via the provider's
-    get_tool() API. This is the primary CTF attack surface for tool poisoning.
-    """
+async def _apply_tool_overrides(server: FastMCP, overrides: dict) -> None:
+    """Apply user-supplied tool overrides to a FastMCP server.
+
+    Supports two override keys per tool:
+    - description: replaces the natural-language description the LLM sees.
+    - parameters / inputSchema: replaces the JSON Schema used for arguments.
+
+    Fixes #547: previously only description was applied; parameters was
+    silently discarded even though the API accepted and stored it.
+    """
     ...
     for tool_name, override in overrides.items():
         new_description = override.get("description")
-        if new_description:
-            try:
-                tool = await provider.get_tool(tool_name)
-                if tool:
-                    tool.description = new_description
-            except Exception:
-                logger.debug("Tool '%s' not found for override", tool_name)
+        new_parameters = override.get("parameters") or override.get("inputSchema")
+
+        if not (new_description or new_parameters):
+            continue
+
+        try:
+            tool = await provider.get_tool(tool_name)
+            if tool:
+                if new_description:
+                    tool.description = new_description
+                if new_parameters:
+                    tool.inputSchema = new_parameters
+        except Exception:
+            logger.debug("Tool '%s' not found for override", tool_name)

Behaviour preserved

• Description-only overrides work exactly as before.
• The if not (new_description or new_parameters): continue guard means entries with neither key are skipped, same as before.
• Accepts parameters (Dark Lab UI key) and inputSchema (FastMCP native key) interchangeably.

Testing

Run the existing dark lab route security tests to confirm nothing regresses:

pytest tests/unit/apps/darklab/ -v

Manual verification:

1. PUT a tool override with both description and parameters for finmail/send_email
2. Trigger the agent to call send_email
3. Confirm the agent receives the modified parameter schema (e.g. bcc field is now known to the LLM)

[prince-shakyaa]

Hii @saikishu , @e2hln

This PR resolves #547 where parameter schema overrides stored via the Dark Lab supply-chain API were being silently discarded by the agent runtime. I updated _apply_tool_overrides in finbot/mcp/factory.py to properly apply parameters and inputSchema overrides alongside description overrides, and added unit test coverage in tests/unit/mcp/test_tool_override_parameters.py to prevent regressions.

Let me know if you need any changes.

Thank You.

[prince-shakyaa]

Update: Added defensive guards and edge-case fixes

(Note for Maintainers: As discussed over in my comment on PR #577 (here), @Deez-Automations and I both independently caught this vulnerability! I took a defensive approach here in the factory, while they added API-level validation. Happy to combine approaches or defer to whatever fits best!)

I just pushed a new commit to make the tool override application much more robust against edge cases and malformed payloads.

Here is a breakdown of what this commit changes:

1. Fixing the Falsy Logic Bug

Before: If a user deliberately tried to clear a tool's parameters by passing an empty dictionary ({}), it was treated as a falsy value by if not (new_description or new_parameters): and silently ignored.
Now: The logic uses explicit is None checks. Passing an empty dictionary now successfully strips the parameters as intended.
2. Defending Against Malformed Payloads

Before: If an attacker passed a string or list instead of a dictionary for the override (e.g. {"send_email": "poison string"}), it would throw an uncaught AttributeError on .get() and completely crash the _apply_tool_overrides loop, breaking server initialization.
Now: I added an explicit isinstance(override, dict) guard. Malformed payloads are now safely skipped without crashing the agent runtime.
3. Enhanced Test Coverage

Before: The unit tests only verified that the inputSchema alias was updated, leaving the actual internal parameters attribute unverified.
Now: Tests assert that both inputSchema and parameters are correctly updated, and I added a new test (test_malformed_override_type_is_skipped) to guarantee our new defensive type-check works.
Ready for review! Let me know what you think.

@e2hln @saikishu