feat(claude-plugin): redact MCP tool output and catch hook failures - #145
feat(claude-plugin): redact MCP tool output and catch hook failures#145amondnet wants to merge 3 commits into
Conversation
Anthropic has committed to shipping function hooks as "Claude Mods" and published the three built-in mod sources (mods/sec-default, diff, telemetry), so the note no longer reads as a prototype that may vanish. Record what 2.1.268 landed: `next.to`, the `classic.*` surface, and a declarable fail-closed `.catch` per registration.
Rerun `/plugin-types` on Claude Code 2.1.268; the declarations were written by 2.1.263. Mechanical output, no hand edits. Note in the README that renames land without a compatibility shim (`$.fs.readFile` became `$.fs.read`), so the file has to be regenerated after every CLI update, and that `claude-code-grep.d.ts` is still required because 2.1.268 names Grep only in a doc comment.
Match MCP tools (`mcp__<server>__<tool>`) at `tool.call` so their output is redacted like any other tool's. They are deliberately exempt from `failMode: "closed"`: MCP was matched to gain redaction, not to add a denial path, so an unreachable engine leaves an MCP call exactly as it behaved before it was matched at all. Give both registrations a `.catch()` backstop (2.1.268). The module budgets its engine calls at 8 s inside the host's 10 s, but turning a verdict into a result runs after that budget is released, so a slow engine and a large result can still overrun the host. Without a `.catch` a failed hook is simply absent and the unredacted result reaches the model, which is what `failMode: "closed"` promises it will not. The fail-closed branch never replays `next(e)`: that would hand back the very bytes the redactor never got to read. The two changes share `failsClosed()` and the same lines of both hook registrations, so they land together rather than as a broken split.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Honmoon Claude plugin's function hooks to align with Claude Code version 2.1.268 (Claude Mods), implementing fail-closed behavior via the new .catch() API and extending redaction support to MCP tools. The review feedback focuses on improving robustness by adding defensive nullish checks and safe navigation to prevent potential runtime TypeErrors in the hook and catch handlers (e.g., when accessing tool, error, or e).
| function isMcp(tool: string): boolean { | ||
| return tool.startsWith('mcp__') | ||
| } | ||
|
|
||
| /** | ||
| * Whether an unreachable engine withholds this tool's output. | ||
| * | ||
| * `failMode: "closed"` governs the tools the plugin has always covered. MCP is | ||
| * deliberately exempt: it was added to gain redaction, not to introduce a new | ||
| * denial path, so an engine failure leaves an MCP call exactly as it behaved | ||
| * before the tool was matched at all. `failMode: "open"` still opens everything. | ||
| */ | ||
| function failsClosed(tool: string): boolean { | ||
| return config.failClosed && !isMcp(tool) | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Robustness improvements for isMcp and failsClosed
Problem: The isMcp and failsClosed functions expect a non-nullable string parameter. If tool is undefined or null, calling tool.startsWith will throw a TypeError, crashing the catch handler or hook execution.
Rationale: Correctness and defensive programming. We must ensure appropriate guards exist before object property accesses to prevent runtime crashes in security-critical fallback paths.
Suggestion: Update isMcp and failsClosed to accept optional/nullable string inputs and handle them gracefully.
| function isMcp(tool: string): boolean { | |
| return tool.startsWith('mcp__') | |
| } | |
| /** | |
| * Whether an unreachable engine withholds this tool's output. | |
| * | |
| * `failMode: "closed"` governs the tools the plugin has always covered. MCP is | |
| * deliberately exempt: it was added to gain redaction, not to introduce a new | |
| * denial path, so an engine failure leaves an MCP call exactly as it behaved | |
| * before the tool was matched at all. `failMode: "open"` still opens everything. | |
| */ | |
| function failsClosed(tool: string): boolean { | |
| return config.failClosed && !isMcp(tool) | |
| } | |
| function isMcp(tool?: string): boolean { | |
| return typeof tool === 'string' && tool.startsWith('mcp__') | |
| } | |
| /** | |
| * Whether an unreachable engine withholds this tool's output. | |
| * | |
| * failMode: 'closed' governs the tools the plugin has always covered. MCP is | |
| * deliberately exempt: it was added to gain redaction, not to introduce a new | |
| * denial path, so an engine failure leaves an MCP call exactly as it behaved | |
| * before the tool was matched at all. failMode: 'open' still opens everything. | |
| */ | |
| function failsClosed(tool?: string): boolean { | |
| return config.failClosed && (!tool || !isMcp(tool)) | |
| } |
References
- Use optional chaining or nullish checks when accessing properties of potentially nullish objects to prevent runtime crashes.
| function caught(error: HookFailure): string { | ||
| return error.message ? `hook ${error.kind}: ${error.message}` : `hook ${error.kind}` | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Potential TypeError in caught if error is undefined
Problem: If next.error is undefined or null, calling caught(next.error) will throw a TypeError when trying to access error.message. This would crash the catch handler and bypass the fail-closed behavior.
Rationale: Correctness and defensive programming.
Suggestion: Guard against a nullish error parameter in the caught helper function.
function caught(error?: HookFailure): string {
if (!error) {
return 'unknown hook failure'
}
return error.message ? 'hook ' + error.kind + ': ' + error.message : 'hook ' + error.kind
}References
- Use optional chaining or nullish checks when accessing properties of potentially nullish objects to prevent runtime crashes.
| export const toolCatch: CatchHandler<typeof toolHook> = ($, e, next) => | ||
| failsClosed(e.tool) ? { deny: unavailable(caught(next.error)) } : undefined |
There was a problem hiding this comment.
[MEDIUM] Safe navigation for event object in toolCatch
Problem: In toolCatch, e.tool is accessed directly. If the hook fails during early setup or under unexpected host conditions where the event object e is nullish or does not contain tool, this will throw a TypeError, crashing the catch handler and failing open.
Rationale: Correctness and defensive programming.
Suggestion: Use safe navigation e?.tool and ensure failsClosed handles undefined values gracefully.
| export const toolCatch: CatchHandler<typeof toolHook> = ($, e, next) => | |
| failsClosed(e.tool) ? { deny: unavailable(caught(next.error)) } : undefined | |
| export const toolCatch: CatchHandler<typeof toolHook> = ($, e, next) => | |
| failsClosed(e?.tool) ? { deny: unavailable(caught(next.error)) } : undefined |
References
- Use optional chaining or nullish checks when accessing properties of potentially nullish objects to prevent runtime crashes.
|
1 participant
Summary
Extends the
honmoon-redactfunction-hooks module to cover MCP tool output, adds the host-level.catchbackstop that Claude Code 2.1.268 makes declarable, and refreshes the research note and vendored typings to that version.Changes
Research note (
.please/docs/research/md/001-claude-code-function-hooks-for-honmoon.md)Anthropic has committed to shipping function hooks, productized as "Claude Mods", and published the three built-in mod sources (
mods/sec-default,diff,telemetry). The note records the 2.1.263 → 2.1.268 delta:next.to,classic.*,next.trace, and a fail-closed.catchlanded, and$.fs.readFilewas renamed to$.fs.readwith no compatibility shim.Vendored typings (
packages/claude-plugin/.claude/types/claude-code.d.ts)Regenerated with
/plugin-typeson 2.1.268 (previously 2.1.263). Mechanical, no hand edits. This is a prerequisite for the.catchwork —Registration.catchdoes not exist in the 2.1.263 declarations..catch()backstop on both registrations (hooks/honmoon.ts)The module budgets its engine calls at 8 s inside the host's 10 s, but the work that turns a verdict into a result —
sameShapeover a large record, theJSON.stringifyin the redaction note — runs after that budget is released. Without a.catchthe declarations are blunt: a failed hook "is absent", i.e. unredacted output reaches the model, which is exactly whatfailMode: "closed"promises it will not. The handlers only decide:denythe output (or drop the prompt) underclosed, stand aside underopen. Fail-closed denies regardless ofnext.called, because replayingnext(e)would hand back the bytes the redactor never read.MCP tool coverage (
hooks/honmoon.ts)MCP tools reach
tool.callasmcp__<server>__<tool>and were never matched — by the command hooks or by this module — so their output was never redacted. The matcher now includes/^mcp__/.They are deliberately exempt from
failMode: "closed". MCP was matched to gain redaction, not to add a new denial path, so an engine outage leaves MCP calls exactly as they behaved before. Everything else still fails closed;failMode: "open"still opens everything.Known limitations
PreToolUsecredential-file check remainsRead-only, so it does not gate what an MCP server reads on its own.BashandWebFetch— rather than filtering blocks by kind.Read's image and PDF records are skipped for exactly this reason; the equivalent per-block skip for MCP is not implemented, and the runtime shape of an MCP result has not been verified against a live server.Verification
Run in
packages/claude-plugin:bun run typecheck— exit 0bun test— 42 pass / 0 fail (4 new MCP tests, 5 new.catchtests)bunx eslint hooks— exit 0All of this is unit-level against a fake
$. No live-session runtime verification was done on 2.1.268.Related issues
None — no open issue tracks this work, so the PR intentionally carries no
Closes #N.Summary by cubic
Extends
honmoon-redactso MCP tool output is redacted, and adds a.catchbackstop so a failed hook fails closed instead of letting unredacted output through.MCP tool coverage
mcp__<server>__<tool>) reachedtool.callbut were never matched, so their output went unredacted; the matcher now includes/^mcp__/.failMode: "closed"— an engine outage leaves MCP calls exactly as before; everything else still fails closed andfailMode: "open"still opens everything.Readhas is not implemented for MCP, and a live-server result shape hasn't been verified..catch()backstop and typingstool.callandprompt.submitnow register a.catchhandler (new in 2.1.268): deny the output / drop the prompt underclosed, returnundefinedunderopen.next(e)once the hook has failed — that would return the raw bytes the redactor never read./plugin-typeson 2.1.268 (mechanical, no hand edits), a prerequisite forRegistration.catch; the research note records the 2.1.263 → 2.1.268 API delta.Written for commit a4e0d32. Summary will update on new commits.