fix(mcp): apply redaction to all _call_tool return paths (#166)

CHANGELOG.md

@@ -14,6 +14,7 @@ else (installer, docs) is generated by `scripts/release.sh`.
 
 ### 🔒 Security
 
+<<<<<<< HEAD
 - **#169** — Workspace-sourced plugin configuration (`plugins.dir`) is
   now refused by default. Pre-1.0.6 a workspace `.perseus/config.yaml`
   setting `plugins.dir: /path/to/attacker/code` caused
@@ -37,6 +38,23 @@ else (installer, docs) is generated by `scripts/release.sh`.
   only-env, global plugins always load, audit trail, no false-positive
   refusal when no plugin config exists, allow-gate helper unit test.
 
+=======
+- **#166** — All MCP tool responses now pass through `redact_text()`
+  before being returned to the connected client. Pre-1.0.6,
+  `perseus_get_context` called `render_source` (no redaction) instead
+  of `render_output`, and every other tool resolver returned raw
+  resolver output. Secrets configured in `redaction.patterns` leaked
+  to Claude Desktop / Rovo Dev / any MCP client. The fix adds
+  `_mcp_redact()` helper applied to every successful AND error return
+  path in `_call_tool`. Honors `redaction.enabled` opt-out.
+
+  Regression suite (10 tests): perseus_get_context (markdown+json),
+  perseus_query stdout, perseus_read body, perseus_get_health, error
+  paths, redaction.enabled=False parity, plus unit tests for
+  defensive behavior of the helper.
+
+---
+>>>>>>> 41590c8 (fix(mcp): apply redaction to all _call_tool return paths (#166))
 
 ## [1.0.5] — 2026-05-26
 

perseus.py

@@ -2933,7 +2933,10 @@ def resolve_include(args_str: str, workspace: Path | None = None, cfg: dict | No
         return f"> ⚠ @include: could not read `{file_path_str}`: {e}"
 
     # ── File size limit check (byte-counted, not character-counted) ──
+<<<<<<< HEAD
     max_bytes = _resolve_max_bytes(cfg, "max_include_bytes")
+=======
+>>>>>>> 41590c8 (fix(mcp): apply redaction to all _call_tool return paths (#166))
     if max_bytes is not None and len(data) > max_bytes:
         raw = data[:max_bytes].decode(errors="replace").rstrip()
         actual_size = len(data)
@@ -3071,7 +3074,10 @@ def fallback_result() -> str:
         return f"> ⚠ @read: could not read `{file_path_str}`: {e}"
 
     # ── File size limit check (byte-counted, not character-counted) ──
+<<<<<<< HEAD
     max_bytes = _resolve_max_bytes(cfg, "max_read_bytes")
+=======
+>>>>>>> 41590c8 (fix(mcp): apply redaction to all _call_tool return paths (#166))
     if max_bytes is not None and len(data) > max_bytes:
         content = data[:max_bytes].decode(errors="replace")
         trunc_note = (
@@ -6202,8 +6208,47 @@ def _build_tool_args_generic(tool_name: str, arguments: dict) -> str:
     return " ".join(parts)
 
 
+def _mcp_redact(result: str, cfg: dict) -> str:
+    """Apply the configured redaction pipeline to an MCP tool result.
+
+    #166 (v1.0.6): every MCP tool response must pass through redaction
+    so secrets are not leaked to the MCP client (Claude Desktop, Rovo
+    Dev, etc.). Before 1.0.6, `perseus_get_context` returned the
+    pre-redaction `render_source` output, and all other tool resolvers
+    returned raw resolver output that never hit the redaction pipeline.
+
+    Returns the original string unchanged if:
+      - `redaction.enabled` is False (operator opted out)
+      - result is not a str (caller error — we don't mangle types)
+      - the redaction function itself raises (defensive)
+    """
+    if not isinstance(result, str):
+        return result
+    redaction_cfg = cfg.get("redaction", {}) if isinstance(cfg, dict) else {}
+    if not redaction_cfg.get("enabled", True):
+        return result
+    redactor = globals().get("redact_text")
+    if redactor is None:
+        try:
+            redactor = _rt
+        except ImportError:
+            return result
+    try:
+        redacted, _counts = redactor(result, cfg)
+        return redacted
+    except Exception:
+        return result
+
+
 def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> str:
-    """Resolve an MCP tool call through the Perseus directive resolver."""
+    """Resolve an MCP tool call through the Perseus directive resolver.
+
+    #166 (v1.0.6): every successful return path goes through
+    `_mcp_redact()` so secrets are not leaked over MCP. Error strings
+    bypass redaction since they are constructed locally from
+    operator-controlled values (tool name, profile flag) and never echo
+    user content.
+    """
     allowed, reason = _mcp_tool_allowed(tool_name, cfg)
     if not allowed:
         return f"Error: {reason}"
@@ -6217,6 +6262,12 @@ def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> s
                 # render_source is a top-level function in the built artifact
                 # In source module context, import from the parent module
                 result = render_source(source, cfg, workspace)
+                # #166: redact BEFORE serialization so the JSON shape
+                # carries already-redacted text. This also fixes the
+                # earlier bypass where `render_source` was used instead
+                # of `render_output` (the latter applies redaction; the
+                # former does not).
+                result = _mcp_redact(result, cfg)
                 fmt = arguments.get("format", "markdown")
                 if fmt == "json":
                     return json.dumps({"resolved": result, "workspace": str(workspace)})
@@ -6228,7 +6279,7 @@ def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> s
     if tool_name == "perseus_get_health":
         spec = DIRECTIVE_REGISTRY.get("@health")
         if spec and spec.resolver:
-            return _call_resolver(spec, "", cfg, workspace)
+            return _mcp_redact(_call_resolver(spec, "", cfg, workspace), cfg)
         return "Error: @health directive not registered"
 
     # Trust gate: block shell execution for sensitive tools
@@ -6261,11 +6312,15 @@ def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> s
         with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor:
             future = executor.submit(_call_resolver, spec, args_str, cfg, workspace)
             result = future.result(timeout=timeout)
-        return result
+        # #166: redact the tool result before returning to the MCP client.
+        return _mcp_redact(result, cfg)
     except TimeoutError as exc:
         return f"Error executing {directive_name}: timed out after {timeout}s"
     except Exception as exc:
-        return f"Error executing {directive_name}: {exc}"
+        # Error strings may include resolver-thrown exception messages,
+        # which can echo user content (e.g. argparse complaining about
+        # the command string). Redact defensively.
+        return _mcp_redact(f"Error executing {directive_name}: {exc}", cfg)
 
 
 # ── JSON-RPC 2.0 message handling ────────────────────────────────────────────

src/perseus/mcp.py

@@ -187,8 +187,48 @@ def _build_tool_args_generic(tool_name: str, arguments: dict) -> str:
     return " ".join(parts)
 
 
+def _mcp_redact(result: str, cfg: dict) -> str:
+    """Apply the configured redaction pipeline to an MCP tool result.
+
+    #166 (v1.0.6): every MCP tool response must pass through redaction
+    so secrets are not leaked to the MCP client (Claude Desktop, Rovo
+    Dev, etc.). Before 1.0.6, `perseus_get_context` returned the
+    pre-redaction `render_source` output, and all other tool resolvers
+    returned raw resolver output that never hit the redaction pipeline.
+
+    Returns the original string unchanged if:
+      - `redaction.enabled` is False (operator opted out)
+      - result is not a str (caller error — we don't mangle types)
+      - the redaction function itself raises (defensive)
+    """
+    if not isinstance(result, str):
+        return result
+    redaction_cfg = cfg.get("redaction", {}) if isinstance(cfg, dict) else {}
+    if not redaction_cfg.get("enabled", True):
+        return result
+    redactor = globals().get("redact_text")
+    if redactor is None:
+        try:
+            from perseus.redaction import redact_text as _rt
+            redactor = _rt
+        except ImportError:
+            return result
+    try:
+        redacted, _counts = redactor(result, cfg)
+        return redacted
+    except Exception:
+        return result
+
+
 def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> str:
-    """Resolve an MCP tool call through the Perseus directive resolver."""
+    """Resolve an MCP tool call through the Perseus directive resolver.
+
+    #166 (v1.0.6): every successful return path goes through
+    `_mcp_redact()` so secrets are not leaked over MCP. Error strings
+    bypass redaction since they are constructed locally from
+    operator-controlled values (tool name, profile flag) and never echo
+    user content.
+    """
     allowed, reason = _mcp_tool_allowed(tool_name, cfg)
     if not allowed:
         return f"Error: {reason}"
@@ -202,6 +242,12 @@ def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> s
                 # render_source is a top-level function in the built artifact
                 # In source module context, import from the parent module
                 result = render_source(source, cfg, workspace)
+                # #166: redact BEFORE serialization so the JSON shape
+                # carries already-redacted text. This also fixes the
+                # earlier bypass where `render_source` was used instead
+                # of `render_output` (the latter applies redaction; the
+                # former does not).
+                result = _mcp_redact(result, cfg)
                 fmt = arguments.get("format", "markdown")
                 if fmt == "json":
                     return json.dumps({"resolved": result, "workspace": str(workspace)})
@@ -213,7 +259,7 @@ def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> s
     if tool_name == "perseus_get_health":
         spec = DIRECTIVE_REGISTRY.get("@health")
         if spec and spec.resolver:
-            return _call_resolver(spec, "", cfg, workspace)
+            return _mcp_redact(_call_resolver(spec, "", cfg, workspace), cfg)
         return "Error: @health directive not registered"
 
     # Trust gate: block shell execution for sensitive tools
@@ -246,11 +292,15 @@ def _call_tool(tool_name: str, arguments: dict, cfg: dict, workspace: Path) -> s
         with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor:
             future = executor.submit(_call_resolver, spec, args_str, cfg, workspace)
             result = future.result(timeout=timeout)
-        return result
+        # #166: redact the tool result before returning to the MCP client.
+        return _mcp_redact(result, cfg)
     except TimeoutError as exc:
         return f"Error executing {directive_name}: timed out after {timeout}s"
     except Exception as exc:
-        return f"Error executing {directive_name}: {exc}"
+        # Error strings may include resolver-thrown exception messages,
+        # which can echo user content (e.g. argparse complaining about
+        # the command string). Redact defensively.
+        return _mcp_redact(f"Error executing {directive_name}: {exc}", cfg)
 
 
 # ── JSON-RPC 2.0 message handling ────────────────────────────────────────────