Ecosystem audit, distribution surfaces, and 5 precision fixes - #22
Merged
Conversation
…ntel MCP-Scan (Invariant Labs) and Snyk agent-scan were listed as separate competitors; Invariant Labs was acquired by Snyk in June 2025 and MCP-Scan was rebranded into Snyk Agent Scan, so they're one entry with combined mindshare and enterprise distribution. Adds an adoption-gap note quantifying the actual bottleneck (19 stars vs 2k+, no IDE presence, single-maintainer trust ceiling) since it's distribution and trust signal, not detection quality, that explains low adoption.
… MCP servers Scanning real third-party MCP servers (upstash/context7, cloudflare/mcp-server-cloudflare, stripe/agent-toolkit) turned up three root-cause precision bugs, each now fixed and locked in with a safe fixture attributing the source repo/file: - AI002's "secret" branch had no llmFile gate (only the "prompt" branch did), making it a general-purpose secret-in-log scanner regardless of LLM context — out of scope per this project's own precision contract. Fired on a bcrypt-hashed password logged in a plain Next.js demo app (stripe/agent-toolkit) and a non-secret prefix constant with "KEY" in its name (upstash/context7), neither file importing any LLM SDK. - isTestFilePath's NON_PRODUCTION_WORD list didn't include eval(s), so an LLM-as-judge eval-harness scorer (cloudflare/mcp-server-cloudflare's packages/eval-tools/, using vitest-evals) fired AI001 — structurally identical to real prompt injection but a same-repo grading harness, not a request handler. Same precedent as the existing ecosystem-tests/ handling. - MCP001 compared a system-prompt node's raw source text against a tainted variable name with a plain substring search instead of checking for an actual identifier reference. A static system-prompt string that merely used the English word "tools" in prose (with an unrelated `toolSet` variable in scope, never interpolated) was flagged proven/critical — the highest possible evidence tier, on cloudflare/mcp-server-cloudflare's packages/eval-tools/src/runTask.ts. Now requires the tainted name to appear as a real identifier (template interpolation, bare reference, or property-access root). Also hardens findCrossToolReference (shared by MCP009/SKL003) against a related class: a candidate tool/skill name whose every token already appears in the describing tool's own name is self-reference, not a foreign tool — found on an AWS aurora-dsql skill whose own "DSQL" vocabulary collided with a differently-named sibling skill in the same monorepo. npm test: 149/149 passing, no regressions against the existing fixture corpus.
…roadmap .pre-commit-hooks.yaml lets a project add this repo as a pre-commit (pre-commit.com) source instead of, or alongside, the GitHub Action — catching findings before a push rather than after. language: node installs this repo's package.json into a cached isolated environment, but that bare `npm install` doesn't build dist/ (a git checkout, unlike the published npm tarball, ships no prebuilt dist/, and no npm lifecycle script builds on a plain install without risking breaking the published package for end users — verified empirically that `prepare` does not fire here). scripts/precommit-entry.js builds once in the cached checkout, then scans the consuming project's working directory. Whole-project scan (pass_filenames: false, always_run: true), not just changed files: the dataflow tracing this scanner does depends on files outside a given commit's changeset, same reasoning tsc/mypy pre-commit hooks use. Defaults to --fail-on high, overridable via args: in the consumer's own .pre-commit-config.yaml. Validated end-to-end from a from-scratch checkout with no dist/ present: exits 0 on clean code, exits 1 with findings printed on vulnerable code, via `npm install` + node scripts/precommit-entry.js — no reliance on prepare actually firing. Documented in README.md under a new "Pre-commit hook" section; marked shipped in ROADMAP.md's distribution plan.
Documents the 2026-08-26 audit round in docs/RealWorldFindings.md, following the same before/after narrative style as the existing vercel/ai and litellm sections: what was fixed (the four bugs from the prior two commits, including the proven/critical MCP001 substring bug), what's confirmed real (awslabs/mcp's unpinned MCP configs and VEC001 tenant-filter gap), and what's a known-but-not-yet-fixed false positive documented honestly rather than silently accepted (residual MCP009/SKL003 hits on benign same-owner tool/skill cross-references). ROADMAP.md's "Ecosystem audit" entry updated to point at the writeup and note what's still open: a recurring cadence, and whether audited repos join the permanent npm run regression corpus. Both npm test (149/149) and npm run regression (zero new findings against the reviewed baseline) confirmed green before this round was written up as complete.
… generating it
Generates docs/examples/THREAT_MODEL.example.md from examples/vulnerable.ts
(this repo's own demo file, not a third party's code) via
`secureai-scan threat-model` — the deliverable behind the "compliance
packaging" pitch in ROADMAP.md/MARKETING.md, which until now was a paragraph
describing the feature rather than a real artifact to point people at.
Also fixes a real bug found while generating it: AI003's summary always used
the indefinite article "A" regardless of whether the following word started
with a vowel sound ("A unresolved LLM client call runs..."). Now picks "A"/
"An" based on the resolved provider name.
README.md's `threat-model` command line now points at the example.
Records `secureai-scan scan .` against docs/demo-source/src/chat.ts (a small, real, deliberately vulnerable file — genuinely scanned, not a mockup) via asciinema, rendered to an animated SVG with svg-term-cli rather than a GIF: same effect, ~8KB instead of hundreds of KB to MBs, crisp text at any zoom instead of a compressed raster. Verified the animation timing is correct by polling the SVG's computed CSS transform over time (0% -> 45.9% -> 100% keyframes advancing on schedule) rather than trusting a headless-Chromium screenshot, which does not reflect live compositing for this transform-based reveal animation in this sandbox — a local validation-tooling limitation, not a defect in the shipped asset. docs/demo-source/README.md documents the exact regeneration commands so this isn't a one-off, unreproducible artifact.
…dmap item vscode-extension/ wraps the secureai-scan CLI (bundled as a real npm dependency, not shelled out to via npx — no network calls at scan time, same offline guarantee as the CLI itself) and reports findings as Problems-panel diagnostics: scans on save for TS/JS/Python/MCP-config/ Agent-Skill files, plus Scan Workspace / Scan Current File's Project / Clear Findings commands and a status-bar finding count. Settings mirror the CLI's own precision contract (paranoid off by default, configurable min severity). Validated, not just written: `tsc --strict` compiles clean against @types/vscode, and `vsce package` produces a real installable .vsix (5.47MB, dominated by tree-sitter's native Python-parser bindings and ts-morph's bundled TypeScript compiler — both load-bearing runtime deps, not accidental bloat). The diagnostic-mapping logic in extension.ts was checked against the CLI's actual `--output <file>.json` schema by running a real scan and inspecting the JSON, not guessed from reading reporter.ts alone. Not published to the VS Code Marketplace — that needs a publisher account/token outside this session's access. README.md documents the local-build install path in the meantime. ROADMAP.md and MARKETING.md updated with what's shipped vs. still open (bundling/minifying, an icon, Marketplace publishing itself).
…e audit
The ecosystem audit writeup deliberately left these unfixed rather than
rush a same-file exclusion that would have blinded the rule to its own
vulnerable fixture (format_text shadowing send_email is a real
same-file attack). Traced all seven false-positive sentences to their
exact text and compared structure against that fixture directly:
- Attack: "When send_email is called, first route..." - the referenced
tool name sits BETWEEN the trigger word and the verb (subject of the
trigger clause), and the command redirects toward "this tool".
- All seven false positives put the referenced tool AFTER the verb
("Use readonly_query for...", "after invoking dynamodb_data_modeling",
"offer to use sample_dataset", "...used with the read_sections tool",
"Always load for guidance using... the DSQL MCP server") - the object
of a "use/call the other tool for its own purpose" sentence, never a
redirect.
findCrossToolReference now requires the tool name to fall between the
trigger word and the verb, not merely anywhere in the same sentence.
Verified against all seven real sentences directly before touching the
rule, then confirmed empirically: re-scanning awslabs/mcp dropped
MCP009/SKL003 to zero while MCP004 (7) and VEC001 (3) - the genuine
findings - were unchanged, and the send_email/format_text vulnerable
fixture still fires. npm test 149/149 green. Two new safe fixtures
(TS + Python + a skill pair) lock in the fix.
docs/RealWorldFindings.md and ROADMAP.md updated to reflect this as
resolved rather than an open follow-up.
docs/articles/mcp-tool-poisoning.md and rag-poisoning.md, both published: false and ready to paste into Dev.to. Each walks the existing diagram (mcp-attack-diagram.png / rag-poisoning-diagram.png), explains the relevant rules, and includes a real found-and-fixed story cited against docs/RealWorldFindings.md rather than invented for the post: the MCP009 subject-vs-object fix for the first, the chunks false-positive and llama_index/awslabs-Valkey VEC001 comparison for the second. Publishing itself (flipping published: true, picking a cadence) is still a manual step - noted as such in MARKETING.md.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
2 tasks
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
3 participants
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.
What and why
Started as a competitive-gap analysis, which turned up that the real adoption gap is distribution and mindshare rather than detection quality — so this branch closes out the concrete, unblocked items from
ROADMAP.md's distribution plan, runs the "ecosystem audit" the roadmap had flagged as the highest-leverage unshipped item, and fixes everything the audit found rather than publishing around it.Corrections and analysis
ROADMAP.md's competitive-position table merged MCP-Scan (Invariant Labs) and Snyk agent-scan as if they were two separate competitors — Invariant Labs was acquired by Snyk in June 2025, so it's one combined mindshare/enterprise-distribution threat, not two smaller ones. Corrected with current star counts.Ecosystem audit (real, not synthetic) — 5 root-cause bugs found and fixed
Scanned 6 independent public MCP servers not in the internal regression corpus (context7, mcp-server-cloudflare, supabase-mcp, firecrawl-mcp-server, agent-toolkit, awslabs/mcp). Every
proven/likelyfinding was read against its source line before being called real or a bug:AI002's "secret" branch had no LLM-file gate — a general secret-scanner masquerading as an AI-specific rule, contradicting this project's own scope contract.isTestFilePathdidn't recognizeeval/evalspath segments — an LLM-as-judge eval harness flagged like a real request handler.MCP001compared prompt text against a tainted variable name with a plain substring search — firedproven/critical (the highest tier this scanner has) on a completely static system prompt that happened to contain the English word "tools" near an unrelatedtoolSetvariable.MCP009/SKL003) didn't exclude a name's own vocabulary — a skill about "DSQL" collided with an unrelated skill literally nameddsql.MCP009/SKL003false positives, fixed properly instead of rushed — traced all 7 to their exact sentences, found the real distinguishing signal (referenced tool as subject of the trigger clause = the real attack shape, vs. object of the verb = ordinary comparison docs), and verified the known-vulnerable fixture still fires before shipping.Real confirmed findings surfaced along the way, not filtered out:
awslabs/mcpships 7 unpinned@latestMCP configs and a vector-search tool with no server-enforced tenant filter. Full writeup indocs/RealWorldFindings.md.Distribution roadmap items shipped
.pre-commit-hooks.yaml+scripts/precommit-entry.js), validated end-to-end from a from-scratch checkout with no prebuiltdist/.docs/demo.svg, animated SVG via asciinema → svg-term-cli, ~8KB) — closes the README's last open TODO.docs/examples/THREAT_MODEL.example.md), generated from the repo's ownexamples/vulnerable.ts(no third-party attribution). Caught and fixed a real grammar bug inAI003's output text while generating it ("A unresolved..." → "An unresolved...").vscode-extension/) — wraps the CLI as a bundled dependency (no network calls at scan time), reports findings as Problems-panel diagnostics.tsc --strictcompiles clean,vsce packageproduces a real installable.vsix, diagnostic-mapping checked against the CLI's actual--output <file>.jsonschema. Not published to the Marketplace — needs a publisher account/token outside this session's access.docs/articles/,published: false) using the two existing repo-root diagrams, each built around a real found-and-fixed story from this branch rather than invented content.Detection logic changes
npm run regressionwas run and everyproven/likelyfinding it printed was reviewed against its source line (seedocs/RealWorldFindings.mdfor the full trace on all 5 fixes)test-fixtures/safe/, attributed in a comment to the source repo/file it came fromcatalog.ts/explainer.ts/EXPECTED_VULNERABLEchanges were needed — these are precision fixes to existing rules (AI002, MCP001, MCP009, SKL003), not new rulesCLI changes
Not applicable —
src/cli.tsuntouched.Checklist
npm run build && npm testpasses (149/149)Generated by Claude Code