Repository navigation
Harden repository: accuracy, security, currency, CI (0.2.0) - #2
Merged
Merged
Conversation
Apply in-range updates and npm audit fix. Remove the unused, deprecated @modelcontextprotocol/server-github devDependency, which carried high-severity advisories with no upstream fix.
- .nvmrc pins Node 24 (Active LTS); engines >=22.18.0 (Node 20 is EOL) - commander 15, cosmiconfig 10, chalk 6, vitest 5, TypeScript 6.0 - raise runtime dependency floors to patched releases - hold TypeScript 7: tsup's dts build needs the compiler API it lacks
The npm name mcp-tool-validator belongs to an unrelated package, so the README install command pulled third-party code. Use the repository name, which is free on npm, and read name/version from package.json for the MCP clientInfo and SARIF driver instead of hard-coded strings. Also point SARIF $schema at the current spec path; the old master path returns 404.
The CLI decided whether to run by matching process.argv[1] against path suffixes, so it silently did nothing when launched via node_modules/.bin (npm, npx) or Windows paths. Export run() and call it from the bin script; detect direct execution by comparing real paths. Set process.exitCode instead of calling process.exit() so large piped JSON/SARIF output is flushed, and report serve bind failures.
POST /validate reused the file config schema, so a caller could supply an llm section (provider, baseUrl) and make the server send its own ANTHROPIC_API_KEY/OPENAI_API_KEY to an arbitrary host. Allow-all CORS let any web page do this to a local instance. - reject llm in request config (validateRequestConfig) - remove CORS; require application/json (415 otherwise) - body size and tool count limits; 400 for malformed JSON and non-object tools; generic 500 without internal messages - resolve the server config once at startup (serve -c), never search the working directory per request, and omit its path from results - JSON-lines request logging without bodies or headers; log listening only after the socket is bound - add validate() discoverConfig option used by the service
- follow nextCursor pagination for legacy (2025-11-25) servers, which were silently truncated to the first page, with a per-request timeout - cap tools/list pagination at 100 pages and HTTP response bodies at 10 MiB; refuse redirects instead of replaying the request elsewhere - spawn 2026-07-28 stdio servers with the SDK's minimal default environment, as the legacy path already did, so API keys and tokens in the caller's environment are not passed to the server under test - truncate server stderr/error bodies quoted in error messages - add --timeout and ValidateOptions.timeout for live discovery
Tool names, messages, suggestions and LLM output come from untrusted definitions or servers; replace C0/C1 control characters so they cannot emit terminal escape sequences. Implement --quiet in the human reporter instead of filtering its rendered output by matching literal strings in the CLI.
--llm-provider openai (or a config with only a provider) kept the default Anthropic model, so requests named a model the provider does not have, and a partial llm config section had no provider or timeout. Resolve omitted LLM fields from per-provider defaults in one module. Also centralize supported MCP spec revisions (previously repeated in the CLI, config schema, loader, validator and client) and derive the default rule set from the rule registry instead of a hand-copied list.
BP-007 recursed into oneOf/anyOf/allOf without counting the step, so a long composition chain overflowed the stack and surfaced as a rule failure error. Count every recursive step against MAX_SCHEMA_DEPTH.
getAllRuleIds, isRuleRegistered, getRuleById, loadRuleModule, getEffectiveSeverity, isRuleEnabled, getRuleSeverity, hasToken and the SCH_xxx named exports were not used by the library, CLI or service (none are part of the public index exports). Remove them and the tests that only exercised them.
Config files and HTTP requests accepted any rule ID, and --rule silently dropped unrecognized settings, so typos (or the nonexistent NAM-001 in the sample config) looked configured but did nothing. Reject both with a descriptive error. Correct the sample config's rule comments (SCH-001 is the name check, NAM-001 does not exist) and stale NAM-001/kebab-case references.
auto, breakdown, chunks-beads, impl, plan-to-beads and retro depend on the beads issue tracker that was removed in 9d65bdb.
- reports/filesystem.json and the code-reviewer agent embedded a local /Users/... path; use repository-relative paths - analyze:servers passed an absolute path to the filesystem server and used default 2026-07-28 discovery, which the reference servers do not support; use a relative path and 2025-11-25 discovery - llm:analyze overwrote a report misdated 2025-01-08 and described the model as Claude 3 Haiku; date the output per run and name the model from one constant
Flat ESLint config with typescript-eslint recommended rules and eslint-config-prettier. Fix the findings: unused imports and constants, error causes on rethrown parse errors, prefer-const, and a control character regex in a test. Add lint, format, format:check, test:coverage and an aggregate check script. Drop the unused test:run alias (identical to test without the build).
Formatting only; no behavior changes.
Coverage floors (statements 90, branches 85, functions 90, lines 90) sit just below the measured values (92.9 / 87.3 / 94.7 / 93.5) so regressions fail CI.
…nfig The spec page anchor is #$ref-resolution, not #ref-resolution. Add a lychee configuration that checks Markdown, HTML and source URLs, including fragments.
- read-only token by default; CodeQL job alone gets security-events: write - third-party actions pinned to full commit SHAs with version comments - concurrency groups cancel superseded runs; every job has a timeout - test on Node 22 and 24 (Node 20 is EOL): lint, format check, typecheck, build + tests with coverage floors, example validation and a production dependency audit - lychee link check over Markdown, HTML and source URLs - CodeQL for JavaScript/TypeScript and Actions; weekly Dependabot updates for npm and GitHub Actions
package.json declared MIT but no license text was committed, so GitHub reported no license. Add the MIT text, a security policy with private reporting and the tool's security model, a short contributing guide, .editorconfig, and issue and pull request templates.
- ignore all .env variants except .env.example, Claude Code local settings, SARIF output and lychee reports - gitleaks and GitHub secret scanning: exclude the clearly fake key in the code-review anti-pattern example
Absolute paths in JSON/SARIF reports exposed local directory layout in shared CI artifacts and committed evidence.
Add CHANGELOG.md. Tests now assert the version from package.json instead of a hard-coded string.
- quickstart from a clean clone (the package is not on npm; the old npm name is an unrelated package) - why it matters, architecture overview, configuration table, testing, status and limitations, license, CI badge - document the HTTP service hardening, --timeout, exit codes, and that --server executes the given command with a minimal environment - 'finalized' 2026-07-28 wording corrected: the spec labels it Current - Drive evidence: 87/100 is labelled as recorded without retained output; the committed 2026-10-02 re-run scored 86/100
Re-ran the follow-up command on 2026-10-02 (validator 0.2.0) and commit its JSON output. Label the 2026-08-05 figures as hand-recorded without retained output, note that the findings-by-rule rows sum to 66 of 74, and record that default 2026-07-28 discovery now succeeds.
- quick commands install from source and npm link instead of assuming mcp-validate is on PATH - Drive demo and claims use the committed 2026-10-02 re-run (86/100) - presentation: real sample output, real memory-server score (79, server 2026.8.31 with legacy discovery), correct server package name, renamed package, test counts, and accurate issue and LLM descriptions
Every changed claim was checked against the code or a CLI run. - RULES.md: LLM-001 severity; LLM-009, LLM-011, LLM-012 and BP-012 examples now behave as labelled; SCH-006 documented-example claim; SEC-007 term list; BP-009 escalation; BP-010 icon sources and MIME requirements cited to the spec; BP-014 figure cited to its source - specs: remove error codes that do not exist; real human/JSON samples; metadata, options, HTTP service, discovery limits, dependencies - architecture: no npx install, current stack versions, rule export pattern, issue documentation links, Cosmiconfig search places - test plan and breakdown: current counts, CI gate, dates - examples README: needs-work example has no naming findings - reports: fix 2025 dates (renamed files to their real 2026 dates), v0.1.1, recompute the Issues by Category table from the fixtures, reference-server attribution; errata list each correction - BP-010 and SEC-007 messages no longer overstate what is accepted or flagged; 'finalized' wording replaced (the spec labels 2026-07-28 Current)
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
github.com answers automated requests for blob pages with 503, which failed the link check. Skip those URLs in lychee and verify the linked files exist in the checkout instead.
GitHub's default setup covers JavaScript/TypeScript and Actions with the standard query suite, keeps the analysis current without pinned actions to maintain, and cannot run alongside an advanced workflow. Also correct the test plan: the link check is a job in ci.yml.
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
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.
Hardening pass based on the Phase 1 audit in AUDIT.md, released as 0.2.0 (CHANGELOG.md). Each commit is small and focused (conventional commits). No history on
masterwas rewritten, and no repository settings were changed.1. Summary of changes
Security
POST /validateaccepted anllmblock (includingbaseUrl) in the request config. Combined with allow-all CORS, any web page could make a localserveinstance sendANTHROPIC_API_KEY/OPENAI_API_KEYto an arbitrary host. Now:llmin request config is rejected.serve -c).npm auditis now 0 high/critical (was 12 high, 3 of them in production deps). One low-severity advisory remains, see open items. The deprecatedserver-githubdev dependency is removed.Correctness
mcp-tool-description-validator. The README'snpm install mcp-tool-validatorinstalled an unrelated third-party package.node_modules/.bin. It printed nothing and exited 0, so npm/npx installs were broken. Fixed, with a regression test that fails on the old code.process.exit()could truncate piped JSON/SARIF output; the CLI now setsprocess.exitCode.servebind errors were ignored and "listening" was logged before binding.serve -cwas swallowed by the top-level-c.--llm-provider openaisent the Anthropic model name to OpenAI.llmconfig had no provider or timeout.NAM-001) and invalid--rulevalues were silently ignored; they are now rejected.anyOfchains.$schemaURL returned 404, and the SCH-009 doc anchor was wrong.Design (no redesign)
validate()gains adiscoverConfigoption (dependency injection for the HTTP service).createApp()takes config, limits and a logger.package.json.metadata.configUsedis relative to the working directory.--quietis implemented in the reporter instead of by string matching.--timeout.Currency
engines >=22.18,.nvmrc24, CI on Node 22 and 24.CI status: all checks green on the latest commit (Test on Node 22 and 24, Links).
Tests
startServerCI/CD
ci.yml: install, lint (ESLint), format check (Prettier), typecheck, build + tests with coverage, example validation, productionnpm audit, and a lychee link check.permissions: contents: read, SHA-pinned actions with version comments, concurrency groups and per-job timeouts.dependabot.yml(npm and Actions). Code scanning uses GitHub's CodeQL default setup (a repository setting; see section 4) rather than a workflow file. An advancedcodeql.ymlwas tried in this PR and passed, then removed in favour of default setup.Docs and hygiene
.editorconfig, issue and PR templates, gitleaks and secret-scanning allow-lists for one clearly fake example key,.gitignorecoverage of.env*, and.git-blame-ignore-revsfor the formatting-only commit./Users/...paths removed from committed files..claude/commands/were deleted, as you approved.Phase 3 verification (fresh clone of this branch)
I also verified the following outside the test suite:
npm install <path>) imports and its.bin/mcp-validate --versionprints0.2.0.servereturns 400 for a requestllmblock.--server "env NAME=value …"passes a variable through while other variables stay out.2. Claims table
IDs refer to AUDIT.md. Fixed = the text or code was changed to be true. Verified = true as written, evidence in AUDIT.md. Reworded = could not be verified, so the wording was qualified or evidence added.
npm install mcp-tool-validatormcp-validateafter clonenode bin/mcp-validate.jsornpm link; bin works via.bin(test)analyze:serversconnects to the official serversnpm ci, build,npm linknpx -y @mcp/server-memory@modelcontextprotocol/server-memory --discovery-spec-version 2025-11-25#iconsnpx mcp-validate…-2026-01-08.md/…-by-llm-2026-01-09.mdtests/fixtures/*.json: 671$schemaURL and SCH-009 anchorsrc/too3. Claims whose wording or numbers changed (for your profile README and resume)
mcp-tool-validator→mcp-tool-description-validator. Not published; the CLI is stillmcp-validate.4. Recommended repository settings
I did not apply any of these. Suggested order: 0 → 4 → 2, 3, 5, 6 → 1 (create the ruleset last, once CodeQL has produced results). The default branch is
master. I recommend renaming it tomainfirst; the ruleset below uses~DEFAULT_BRANCH, so it works either way.Also consider disabling the wiki (
gh repo edit $REPO --enable-wiki=false); it is enabled but unused.5. Open items not addressed
esbuild(viatsup), GHSA-g7r4-m6w7-qqqr. It affects only esbuild's dev server on Windows, which this project never runs. Fixing it needs tsup to move to esbuild 0.28.ignoreDeprecations: "6.0"scoped to the tsup dts build.src/llm/analyzer.tsand the peer-dependency ranges. The patch updates within v6 were applied.@anthropic-ai/sdk0.131 (dev, scripts only) was upgraded, butnpm run llm:analyzewas not run because it needs an API key and costs money.npm run analyze:serverswas also not run, so the historical reports are not overwritten. Its discovery fix was verified with the CLI againstserver-memory.scripts/llm-analyze.mjsduplicates the analyzer prompt and parser; it could callanalyzeTools().rule-engine.ts; it could live on each rule.core/validator.tscould take injected parsers and transports for fuller dependency injection.llmvector closed, DNS rebinding can only make it run validations./docs, unchanged and working. A workflow-based Pages deploy would let the same hardening apply; that is optional.blob/...pages with 503, which failed the first CI run. lychee now skips this repository'sblob/treeURLs, andscripts/check-repo-links.mjsverifies that each linked file exists in the checkout (it fails on a missing file). All other external links are still checked.tools/list(no auth, no tool calls). Following the old README literally ran an unrelated npm package,mcp-validate@1.0.0, in a throwaway temp directory, which was then deleted.