Skip to content

feat: add MCP tool call redaction for OpenCode & Gemini CLI#36

Closed
byapparov wants to merge 3 commits into
masterfrom
feat/gemini-opencode-mcp-redaction
Closed

feat: add MCP tool call redaction for OpenCode & Gemini CLI#36
byapparov wants to merge 3 commits into
masterfrom
feat/gemini-opencode-mcp-redaction

Conversation

@byapparov
Copy link
Copy Markdown
Contributor

Summary

  • Add bidirectional PII redaction to OpenCode plugin (tool.execute.before arg redaction + tool.execute.after output redaction)
  • Add Gemini CLI BeforeTool/AfterTool event handling in redact-hook.ts with Gemini-specific response format
  • Add --gemini flag to hush init --hooks to generate .gemini/settings.json with proper hook config
  • Refactor shared redaction logic into redactToolInput() and redactBuiltinResponse() helpers
  • Add .gemini/settings.json example config and update .opencode/plugins/hush.ts docs

Test plan

  • 22 new tests added (7 OpenCode plugin, 11 Gemini redact-hook, 4 Gemini init)
  • All 165 tests pass
  • TypeScript builds cleanly
  • Manual: echo '{"hook_event_name":"BeforeTool","tool_input":{"text":"admin@x.com"}}' | node dist/cli.js redact-hook
  • Manual: echo '{"hook_event_name":"AfterTool","tool_name":"shell","tool_response":{"stdout":"email: a@b.com"}}' | node dist/cli.js redact-hook

🤖 Generated with Claude Code

AICtrl Bot and others added 3 commits March 2, 2026 22:27
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bidirectional PII redaction for MCP tools: PreToolUse redacts outbound
arguments before they reach the MCP server, PostToolUse redacts inbound
results before the LLM sees them. Built-in tool redaction unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add bidirectional PII redaction for two new AI coding clients:

- OpenCode plugin: redact PII in tool args (before) and tool outputs (after)
  for both built-in tools and MCP content blocks via in-place mutation
- Gemini CLI hooks: add BeforeTool/AfterTool event dispatch in redact-hook
  with Gemini-specific response format (deny/reason instead of block)
- Init command: add --gemini flag to write .gemini/settings.json with
  BeforeTool/AfterTool hook configuration
- Refactor redact-hook.ts to extract shared helpers, reducing duplication
  between Claude Code and Gemini event handlers

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 3, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 81.05% 308 / 380
🔵 Statements 80% 332 / 415
🔵 Functions 75.43% 43 / 57
🔵 Branches 70.3% 161 / 229
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/plugins/opencode-hush.ts 100% 88.46% 100% 100%
Generated in workflow #108 for commit 34da6cb by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 3, 2026

PR Review: MCP Tool Call Redaction for OpenCode & Gemini CLI

Summary

This PR adds bidirectional PII redaction support for MCP tool calls across OpenCode and Gemini CLI, extending the existing Claude Code hooks infrastructure. Overall implementation is solid with comprehensive test coverage (158 tests passing).


1. Redaction Logic ✅ Strong with Minor Edge Cases

Strengths:

  • Comprehensive PII pattern coverage in src/middleware/redactor.ts:34-97: emails, IPv4/IPv6, secrets, credit cards, phone numbers, and 25+ cloud provider keys (AWS, GCP, GitHub, GitLab, Stripe, etc.)
  • Smart ordering: cloud keys processed before generic SECRET pattern (line 155-167) prevents partial matches from eating structured keys
  • ReDoS-safe regex design (no nested quantifiers, bounded character classes)
  • Deep object traversal with sensitive key detection (line 114-126)

Edge Cases to Consider:

  • JSON strings: The redactor handles objects via JSON.parse(JSON.stringify()) (line 111), but JSON strings embedded in CLI output (e.g., echo '{"email":"admin@secret.corp"}') will be redacted as text. This is safe but may break JSON parsing in downstream tools.
  • CLI table formatting: IP addresses in aligned tables (e.g., printf "%-15s %s\n" "192.168.1.100" "server1") may have spaces split across tokens. Current regex handles this correctly (\b boundaries), but complex multi-column formats could theoretically split tokens.
  • Base64 content: No explicit base64 detection. If PII is base64-encoded in tool output, it won't be caught. Consider documenting this limitation or adding optional base64 scanning.

Recommendation: Add a test case for JSON-in-CLI-output to confirm current behavior is acceptable.


2. Streaming Integrity ✅ Well-Implemented

Strengths:

  • Stateful rehydrator (TokenVault.createStreamingRehydrator()) correctly handles tokens split across SSE chunks
  • Dual-mode support: SSE streams (line 135-223) and raw text (line 117-133)
  • Backpressure handling in proxy (src/index.ts:143-145) prevents memory issues on slow clients
  • 1MB buffer cap per field (line 93) prevents unbounded memory growth
  • Proper detection of partial tokens via [A-Z_] heuristic (line 207) avoids false positives from JSON array syntax

Potential Issues:

  • Empty flush on oversized buffer: When buffer exceeds 1MB, it flushes everything (line 192-194). This could leave partial tokens at the 1MB boundary. However, given the token format [TYPE_HASH] (max ~30 chars), risk is minimal.
  • No chunk timeout: If upstream stalls mid-stream, the rehydrator holds data indefinitely. Consider adding a timeout to flush stale buffers.

Recommendation: Stream handling is production-ready. Consider adding a chunk timeout in future iterations.


3. Security ✅ Strong with Minor Observations

Strengths:

  • Localhost-only binding by default (HUSH_HOST=127.0.0.1, line 28) prevents network exposure
  • Optional HUSH_AUTH_TOKEN authentication (line 31-51) with both X-Hush-Token and Authorization: Bearer support
  • Tokens stored in memory-only vault with 1-hour TTL (TokenVault line 18)
  • No cleartext logging: only token counts and hashes appear in logs (line 88-96)
  • CORS restricted to http://localhost (line 33)

Observations:

  • Health endpoint vault size leak: In debug mode (DEBUG=true), /health exposes vaultSize (line 239). This is acceptable for debugging but ensure DEBUG=true is never set in production.
  • Token predictability: Tokens use SHA-256 hash truncated to 6 hex chars (line 12). With 16M possible values, collisions are theoretically possible but extremely unlikely (would require ~4K tokens for 50% collision probability via birthday paradox). Consider documenting this or increasing to 8 chars.
  • No vault encryption: Tokens stored in plaintext in memory. If an attacker has memory access, they can extract the vault. This is acceptable for the threat model but should be documented.

Recommendation:

  1. Add warning in docs about DEBUG=true in production
  2. Consider increasing token hash to 8 chars (256M values) for extra safety margin

4. Reliability ✅ Solid with Good Error Handling

Strengths:

  • 30-second timeout on upstream requests (line 108) prevents hanging
  • Proper HTTP error forwarding: upstream status codes and bodies passed through (line 115)
  • Reader cleanup in finally block (line 148-150) prevents resource leaks
  • 502 Bad Gateway on fetch failures with generic error message (line 162)
  • Graceful handling of malformed hook payloads (exit code 2, line 247-259)

Potential Issues:

  • No retry logic: Failed upstream requests return 502 immediately. For transient failures (network blips), this may be too aggressive. Consider adding 1-2 retries with exponential backoff for 5xx errors.
  • No request timeout enforcement: The 30s timeout applies to the entire request including streaming. For large file uploads or long streams, this may be insufficient.

Recommendation: Current reliability is good for v1. Consider adding retry logic for production hardening.


5. Code Quality ✅ Excellent

  • Well-documented with JSDoc comments
  • Clear separation of concerns (redactor, vault, proxy, hooks)
  • Comprehensive test coverage (100% on redactor, 85% on vault)
  • Idempotent hook initialization (line 138-144 in init.ts)
  • Backward compatibility maintained (fallback for missing hook_event_name, line 295)

Test Results

✓ 158 tests passed
✓ Coverage: 80% overall, 100% on redactor, 85% on vault

Recommendations Summary

P1 (Address before merge):

  • None - implementation is solid

P2 (Future improvements):

  1. Add base64 detection (document limitation for now)
  2. Increase token hash from 6 to 8 chars
  3. Add chunk timeout in streaming rehydrator
  4. Add retry logic for upstream 5xx errors
  5. Add warning about DEBUG=true in production docs

Verdict

✅ APPROVE - This is a well-designed, thoroughly tested implementation. The redaction patterns are comprehensive, streaming is handled correctly, and security is solid. Minor recommendations are for future hardening, not blockers.

Reviewed SHA: 34da6cb

@byapparov byapparov closed this Mar 3, 2026
@byapparov byapparov deleted the feat/gemini-opencode-mcp-redaction branch March 3, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant