fix(mcp): apply redaction to all _call_tool return paths (#166)#171
Closed
tcconnally wants to merge 1 commit into
Closed
fix(mcp): apply redaction to all _call_tool return paths (#166)#171tcconnally wants to merge 1 commit into
tcconnally wants to merge 1 commit into
Conversation
Pre-v1.0.6:
- perseus_get_context called render_source (no redaction) instead of
render_output (which does apply redaction).
- All other tool resolvers (perseus_read, perseus_query, etc.) returned
raw resolver output via _call_resolver, never passing through the
redaction pipeline.
Result: secrets configured in redaction.patterns leaked through MCP to
the connected client (Claude Desktop, Rovo Dev, etc.) even when
redaction.enabled: true was set in config. Discovered by Codex code
review (2026-06-03).
Fix:
src/perseus/mcp.py:
- New helper _mcp_redact(result, cfg) honors redaction.enabled, type-
guards non-string inputs, and swallows redactor exceptions defensively.
Uses globals() lookup for build-artifact compatibility, falls back to
explicit import in source mode.
- _call_tool wraps every successful return path:
- perseus_get_context: redact BEFORE serialization so JSON payloads
carry already-redacted text.
- perseus_get_health: redact resolver output.
- Generic directive dispatch: redact result before return.
- Exception path: redact the error string (resolver messages can echo
user content).
- Error strings constructed locally (e.g. 'Error: tool X not allowed')
bypass redaction since they never echo user content.
Tests (tests/test_bugfix_166_mcp_redaction.py — 10):
- test_perseus_get_context_redacts_secret (markdown format)
- test_perseus_get_context_json_format_redacts (JSON format)
- test_perseus_get_context_preserves_secret_when_redaction_disabled (sanity)
- test_perseus_query_result_redacts_secret (stdout redaction)
- test_perseus_read_result_redacts_secret (file content redaction)
- test_call_tool_exception_path_redacts (error path)
- test_perseus_get_health_redacts (legacy resolver shortcut)
- test_mcp_redact_returns_unchanged_when_disabled
- test_mcp_redact_returns_non_str_unchanged
- test_mcp_redact_swallows_redactor_exceptions
All 10 new tests pass.
Closes #166
Refs milestone v1.0.6
tcconnally
force-pushed
the
fix/166-mcp-redaction
branch
from
41590c8 to
9acb989
Compare
Owner
Author
|
Superseded by integration PR #179 which merges all v1.0.6 fixes into a single branch with resolved conflicts and a clean test suite (943 passed, 2 skipped). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #166. Codex-discovered Critical. See commit message for full detail. 10 regression tests added; all pass.