MCP tool risk scoring — hook relay (4/4)#224
Conversation
On an MCP PreToolUse call, compute the server's cache key from its config, read
~/.unbound/mcp-tools-cache.json, and attach the called tool's content hash to
mcp_server_config as tool_content_hash so the gateway can resolve a pre-computed
risk score. Fully fail-open: any miss or error attaches nothing.
- cache_key = sha256 of the non-empty {name, url, command, args} subset — same
one rule as the discovery tool, no fingerprint / extraction logic (device-side
public code). Embedded byte-identical across all 5 hooks (drift-guard test).
- Single-server scan dispatch passes UNBOUND_CODING_TOOL so the on-demand scan
caches under the right agent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BmB1ccF76u6GziqtTwQqSa
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if len(data) > _MCP_TOOLS_CACHE_MAX_BYTES: | ||
| return {} | ||
| parsed = json.loads(data.decode('utf-8')) | ||
| return parsed if isinstance(parsed, dict) else {} | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
When the home cache is oversized, malformed, or unreadable, this function returns {} instead of checking the valid /var/tmp candidate. MCP requests then omit tool_content_hash, so the gateway cannot resolve their precomputed risk score. Handle each candidate's failure inside the loop and continue to the next path; the same fix is required in all five hook copies.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Every read, decode, and parse error is silently discarded, making a failed risk-cache lookup indistinguishable from a normal miss. Operators cannot determine why tool_content_hash was omitted from a request; log the cache path and exception before trying the next candidate. This silent failure is mirrored in all five hook copies.
Context Used: P0 — Critical (must block merge)
Django / Backend ... (source)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec894d1. Configure here.
| return parsed if isinstance(parsed, dict) else {} | ||
| except Exception: | ||
| pass | ||
| return {} |
There was a problem hiding this comment.
Bad cache file blocks fallbacks
Medium Severity
The _read_mcp_tools_cache function's error handling prevents it from checking subsequent cache file candidates if the first one is oversized, corrupt, or contains invalid JSON. This means a valid cache in a fallback location is ignored, and tool_content_hash won't be attached.
Reviewed by Cursor Bugbot for commit ec894d1. Configure here.
| name=server_name, | ||
| command=server_cfg.get('command'), | ||
| url=server_cfg.get('url'), | ||
| args=server_cfg.get('args'), |
There was a problem hiding this comment.
Sanitized config misses cache keys
High Severity
On Augment and Copilot, tool_content_hash lookup builds the cache key from mcp_server_config after _redact_args / _redact_url (and Augment command splitting). The shared key contract keeps full args such as -y, so these hooks miss discovery cache entries for typical npx MCP servers and never attach the hash. Claude, Codex, and Cursor key from raw config fields and do not have this gap.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ec894d1. Configure here.
| metadata['mcp_tool'] = mcp_tool_name | ||
| if mcp_cfg: | ||
| metadata['mcp_server_config'] = mcp_cfg | ||
| _attach_tool_content_hash(metadata) |
There was a problem hiding this comment.
Empty config skips hash attach
Medium Severity
Augment and Copilot only call _attach_tool_content_hash when mcp_server_config is truthy. Both store missing normalized fields as {}, which is falsy, so attach never runs. Name-only cache keys are supported and work when a dict is present, so empty-config servers that discovery keyed by name alone never get tool_content_hash.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ec894d1. Configure here.
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
5 findings — 3 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 World-writable fallback cache enables local risk-score spoofing
claude-code/hooks/unbound.py:1073 (identical in all 5 hooks)
Impact: /var/tmp/unbound-{uid} (and the non-POSIX shared temp dir) can be pre-created by another local user/process; a planted mcp-tools-cache.json can map a malicious MCP server's cache key to a benign tool's content hash, bypassing precomputed risk scores when ~/.unbound is absent.
Fix: Before reading outside $HOME, stat/lstat the directory and require st_uid == os.getuid(), no group/other write bits, and no symlink; or restrict reads to ~/.unbound only.
Flagged by: Claude, Lead
🔴 Sanitized config breaks cache-key parity (Augment / Copilot)
augment/hooks/unbound.py:760, copilot/hooks/unbound.py:744
Impact: Hash lookup keys are derived from redacted mcp_server_config (_redact_args / _redact_url), but discovery keys on full args (e.g. npx + ['-y', …]); typical servers never get tool_content_hash and the gateway cannot resolve risk scores on these agents.
Fix: Compute compute_mcp_cache_key from the same raw config fields the discovery tool uses (pre-redaction), then attach the hash to the redacted config sent upstream.
Flagged by: Cursor
🔴 Invalid preferred cache blocks valid fallback cache
claude-code/hooks/unbound.py:1095 (identical in all 5 hooks)
Impact: An oversized, corrupt, or unreadable ~/.unbound/mcp-tools-cache.json causes _read_mcp_tools_cache to return {} without trying /var/tmp/unbound-{uid}, silently dropping tool_content_hash even when a valid fallback cache exists.
Fix: Handle each candidate's failure inside the loop (continue to next path) instead of returning early or aborting the whole function on first-path error.
Flagged by: Greptile, Cursor, Lead
🟡 Empty normalized config skips hash attach (Augment / Copilot)
augment/hooks/unbound.py:1333, copilot/hooks/unbound.py:1217
Impact: _attach_tool_content_hash runs only when mcp_cfg is truthy; name-only servers normalized to {} never attach a hash despite supported name-only cache keys.
Fix: Call _attach_tool_content_hash whenever mcp_server and mcp_tool are known; pass at least {'name': server} (or equivalent) into the lookup path.
Flagged by: Cursor
🟡 Cache read failures are fully silent
claude-code/hooks/unbound.py:1095 (identical in all 5 hooks)
Impact: All read/decode/parse errors are swallowed, so omitted tool_content_hash is indistinguishable from a normal miss and operators cannot diagnose cache/keying failures.
Fix: Emit a lightweight diagnostic (path + exception) before advancing to the next cache candidate.
Flagged by: Greptile
Previously acknowledged (not re-flagged)
- Fail-open cache lookup (miss, corrupt file, or any error → no
tool_content_hashattached) — accepted by design per PR description.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head ec894d16 · 2026-07-16T21:44Z
2 participants
MCP tool risk scoring — hook relay (4 of 4)
On an MCP PreToolUse call, each hook computes the server's cache key from its config, reads
~/.unbound/mcp-tools-cache.json, and attaches the called tool's content hash tomcp_server_configastool_content_hashso the gateway can resolve a pre-computed risk score.What's here
cache_key = sha256of the non-empty{name, url, command, args}subset — the same one rule as the discovery tool. No fingerprint / extraction logic (device-side, public code).UNBOUND_CODING_TOOLso the on-demand scan caches under the right agent.Testing
26 tests (keying vectors, hit/miss/corrupt/oversized, dispatch env, cross-hook byte-identity drift guard). Other hook suites green (the two pre-existing machine-state failures are unrelated, verified on the base branch).
Part of a 4-PR set
Reads the cache written by coding-discovery-tool; the hash it relays is scored by ai-gateway-data and resolved by ai-gateway. Merge last.
🤖 Generated with Claude Code
Note
Medium Risk
Touches the PreToolUse path that forwards MCP metadata to the gateway; behavior is fail-open on cache errors, but incorrect keying or drift across hooks could omit or mis-attach risk hashes.
Overview
Adds MCP tool risk-scoring relay across all five agent hooks (Claude Code, Codex, Copilot, Augment, Cursor). On MCP PreToolUse, each hook hashes the server config with the same
compute_mcp_cache_keyrule as the discovery tool, readsmcp-tools-cache.json, and when there is a hit attachestool_content_hashonmcp_server_configfor the gateway.The shared block is duplicated per hook (with per-agent coding-tool constants) and guarded by a cross-hook drift test. Lookup is fail-open (miss, corrupt, oversized file, or bad hash → no field). Background
mcp-scandispatch now setsUNBOUND_CODING_TOOLso cache writes align with lookup keys.New
claude-code/hooks/test_tool_content_hash.pycovers cache keying, attach behavior, dispatch env, and section consistency.Reviewed by Cursor Bugbot for commit ec894d1. Bugbot is set up for automated code reviews on this repo. Configure here.
Greptile Summary
This PR relays cached MCP tool hashes from coding-tool hooks to the gateway. The main changes are:
tool_content_hashto MCP server metadata on cache hits.Confidence Score: 4/5
The cache fallback path needs a fix before merging.
The mirrored
_read_mcp_tools_cachefunction in all five hook files.Security Review
A malformed, oversized, or unreadable preferred cache can shadow a valid fallback cache. Affected MCP requests are sent without the content hash used to resolve their precomputed risk score.
Important Files Changed
Reviews (1): Last reviewed commit: "feat(mcp): relay per-tool content hash f..." | Re-trigger Greptile
Context used:
Learned From
websentry-ai/ai-gateway-data#448
Django / Backend ... (source)