fix(security): close procfs credential read + wire MCP/ToolSearch into the injection scan - #1654
Open
elhoim wants to merge 2 commits into
Open
Conversation
CREDENTIAL_PATHS guards ~/.ssh keys, ~/.aws/credentials, GPG keyrings and
.env files, but procfs reaches the same secrets by another route: a
process's environ holds its API keys and tokens, and mem/maps expose its
address space.
Because `cat` is in SEARCH_TOOLS, classifyCommand() took the
read-only-command allow branch for `cat /proc/self/environ` and returned
`allow` — printing every secret in the environment with no prompt.
Add a procfs entry to CREDENTIAL_PATHS so those reads return `neutral`
and defer to the native permission prompt. Covers literal pids, `self`
and the shell forms ($$, $PID, ${PID}) that reach the same file.
Ordinary procfs reads (cpuinfo, meminfo, loadavg, /proc/sys/*) are
unaffected and still auto-allow.
(cherry picked from commit d1f7df7)
… scan Safety.hook.ts documents the intended wiring at isAttackerWritableSource: "the settings.json PostToolUse matcher only routes WebFetch/WebSearch + the qualifying mcp__ names here", with the in-code gate as the defensive backstop. The matcher only ever carried the WebFetch/WebSearch half. The ToolSearch and mcp mail/drive/calendar branches of that predicate were unreachable, so third-party-authored text arriving over those tools was never given the data-not-instructions framing or the injection-shape scan. Merge the two matcher entries into one alternation covering all four classes. The in-code gate still does the precise filtering, so unrelated MCP servers cost one neutral passthrough and nothing more. Also add dropbox to the gate: file content pulled from a sync service is attacker-writable for the same reason drive already is (shared folders, file requests). (cherry picked from commit da74472)
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
1 participant
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.
Two fixes to the same subsystem: both close a gap between what the security layer says it does and what it actually covers.
cat /proc/self/environauto-allowedCREDENTIAL_PATHSguards~/.sshkeys,~/.aws/credentials, GPG keyrings and.envfiles. procfs reaches the same secrets by another route —environis the process environment (API keys, tokens),mem/mapsare its address space.Because
catis inSEARCH_TOOLS,classifyCommand()reached the read-only-command allow branch and returnedallow, printing the whole environment with no prompt. The credential check runs before that branch, so a procfs entry inCREDENTIAL_PATHScloses it cleanly — these reads now returnneutraland defer to the native permission prompt.Covers literal pids,
self, and the shell forms ($$,$PID,${PID}) that reach the same file.Verified — 16 cases through the real
classifyCommand():neutral:/proc/self/environ,/proc/1234/environ,/proc/$$/environ,/proc/${PID}/environ,/proc/self/mem,/proc/self/cmdline,/proc/self/maps,grep -a TOKEN /proc/self/environallow:/proc/cpuinfo,/proc/meminfo,/proc/loadavg,/proc/sys/vm/swappiness,ls /proc,cat README.md,rg TODO src/No regression on ordinary procfs reads.
MCP and ToolSearch results were never injection-scanned
Safety.hook.tsdocuments the intended wiring inisAttackerWritableSource:The matcher only ever carried the WebFetch/WebSearch half. The
ToolSearchand mcp mail/drive/calendar branches of that predicate were unreachable, so third-party-authored text arriving over those tools never got the data-not-instructions framing or the injection-shape scan.Merges the two matcher entries into one alternation covering all four classes. The in-code gate still does the precise filtering, so unrelated MCP servers cost one neutral passthrough.
Also adds
dropboxto the gate: file content pulled from a sync service is attacker-writable for the same reasondrivealready is — shared folders and file requests let a stranger put text there.Verified — matcher compiled and asserted against real tool names. Routes
WebFetch,WebSearch,ToolSearch,mcp__claude_ai_Gmail__*,mcp__claude_ai_Google_Drive__*,mcp__claude_ai_Google_Calendar__*,mcp__claude_ai_Dropbox__*. SkipsBash,Read,Edit,Agent,Skill,mcp__claude_ai_Indeed__*,mcp__claude_ai_Kiwi_com__*. Matcher and in-code gate agree on every case tested.