fix: MCP011/AI010 fetch-detection false positive + prototype-collision crash - #27
Open
akanthed wants to merge 1 commit into
Open
fix: MCP011/AI010 fetch-detection false positive + prototype-collision crash#27akanthed wants to merge 1 commit into
akanthed wants to merge 1 commit into
Conversation
…sparse-checkout the two slow regression repos
isFetchLikeCall (shared by AI010 and MCP011) did a raw substring check
against the callee's lowercased text, so `extra.sendRequest(...)` — an
MCP protocol call to the connected client (sampling/elicitation), not
an HTTP fetch — matched the "request(" pattern because "sendRequest"
contains "request" as a substring. Found on
modelcontextprotocol/servers' trigger-sampling-request*.ts fixtures,
which MCP011 flagged as proven/likely untrusted-tool-source findings.
Rewrote isFetchLikeCall to match on actual identifier/property names
via the AST (bare call name for fetch/got/ky/request, base+method pair
for axios/http/https/superagent) instead of substring search. Also
switched PROPERTY_FETCH_CALLS from a plain object to a Map: a base
identifier literally named `constructor`/`toString`/etc. (found in a
minified Next.js bundle under litellm/proxy/_experimental/out) resolved
to an Object.prototype value on a plain-object lookup, crashing AI010
with "allowedMethods?.has is not a function" and silently skipping the
rule for the whole file (scan.ts isolates per-rule failures, so this
went unnoticed rather than failing the scan).
New safe fixture: test-fixtures/safe/mcp_untrusted_tool_source.ts
(the sendRequest pattern from modelcontextprotocol/servers).
Also sparse-checkout litellm (-> litellm/proxy/) and llama_index
(-> llama-index-core/ + llama-index-integrations/vector_stores/) in
scripts/regression-scan.js instead of full-repo clones: ~1.7k files
each instead of ~5.7k/~10k, same rule coverage (proxy is where the LLC
config examples and the historical AI003/MCP001/MCP002 false positives
were found; core+vector_stores is where every VEC001 baseline finding
outside those two paths lives), full scan now finishes in minutes
instead of timing out.
Verified against all 10 regression repos: servers false positive gone,
ai's genuine MCP011 finding baselined after review, litellm's AI010
crash gone with no new findings, llama_index clean. npm test 151/151.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
isFetchLikeCalldid a raw substring match ("request(" ->.includes("request")), soextra.sendRequest(...)(an MCP protocol call to the client, not an HTTP fetch) matched. Rewrote to match actual identifier/property names via the AST. Found scanningmodelcontextprotocol/servers; permanent safe fixture added.PROPERTY_FETCH_CALLSwas a plain object; a base identifier namedconstructor(found in a minified bundle under litellm's proxy) resolved toObject.prototype.constructor, crashing the rule withallowedMethods?.has is not a function. Switched to aMap.litellm(->litellm/proxy/) andllama_index(->llama-index-core/+llama-index-integrations/vector_stores/) inscripts/regression-scan.js— same rule coverage, ~1.7k files instead of ~5.7k/~10k, full regression run now finishes in minutes instead of timing out.Test plan