diff --git a/docs/adding-servers.md b/docs/adding-servers.md index bfd613a6..ab6375cf 100644 --- a/docs/adding-servers.md +++ b/docs/adding-servers.md @@ -235,17 +235,25 @@ RUN package="mcp-clickhouse@0.3.0"; \ The `mcp-security-scan` CI job runs the package directly (`uvx ` / `npx `) rather than the built image, so it does not automatically inherit anything injected -into the Dockerfile: - -- **uvx `constraints` are reapplied.** `scripts/mcp-scan` writes them to a uv overrides - file and passes `uvx --overrides`, so the scanned process resolves the same versions - the image ships. -- **npx `overrides` are not.** npm honors `overrides` only from a `package.json` it - installs into, and the scan has no such project directory. The scan logs a note when - it skips them. This is safe for the intended use case (swapping a vulnerable but - *working* dependency for a patched one) because it changes neither server startup nor - the tool surface the scanner analyzes. If you ever need an npm override that affects - whether the server *starts*, the scan will fail and this will need revisiting. +into the Dockerfile. `scripts/mcp-scan` reapplies both kinds of override so the scanned +process resolves the same dependency versions the image ships: + +- **uvx `constraints`** are written to a uv overrides requirements file and passed as + `uvx --overrides `. +- **npx `overrides`** cannot be passed on the command line, because npm honors + `overrides` only from a `package.json` it installs into. The scan stages a throwaway + project containing the server package plus the `overrides` block, runs `npm install` + in it, and runs the scanner with that directory as its working directory so `npx` + resolves the installed tree. `npx --no-install` is passed as well, so npx fails rather + than silently fetching an un-overridden copy of the package. + +Beyond matching what ships, this means the scan doubles as a check on the override +itself: an override that breaks the server's startup shows up as a scan failure before +the image is published, rather than as a broken container afterwards. + +Note that servers with `security.insecure_ignore: true` (typically those needing real +credentials to start) cannot be meaningfully scanned either way, so overrides have no +observable effect on their scan. ## Step-by-Step Process diff --git a/npx/onchain-mcp/spec.yaml b/npx/onchain-mcp/spec.yaml index 9514715c..50e55ad9 100644 --- a/npx/onchain-mcp/spec.yaml +++ b/npx/onchain-mcp/spec.yaml @@ -12,6 +12,15 @@ metadata: spec: package: "@bankless/onchain-mcp" version: "1.0.6" + overrides: + - package: "@modelcontextprotocol/sdk" + version: "1.26.0" + reason: | + @bankless/onchain-mcp exact-pins @modelcontextprotocol/sdk 1.7.0, which carries + GHSA-8r9q-7v3j-jr4g (ReDoS, fixed 1.25.2) and GHSA-w48q-cv73-mx4w (DNS rebinding, + fixed 1.24.0). 1.26.0 rather than 1.25.2 because GHSA-345p-7cg4-v4c7 affects + >=1.10.0,<=1.25.3, so a smaller bump would trade two advisories for a third. + Same-major, and the server's 10 tools still enumerate correctly. provenance: repository_uri: "https://github.com/Bankless/onchain-mcp" diff --git a/scripts/mcp-scan/generate_mcp_config.py b/scripts/mcp-scan/generate_mcp_config.py index 569f2189..666d6a3d 100644 --- a/scripts/mcp-scan/generate_mcp_config.py +++ b/scripts/mcp-scan/generate_mcp_config.py @@ -39,19 +39,14 @@ def main(): uv_overrides = [c['spec'] for c in data['spec'].get('constraints', []) if c.get('spec')] # npx: npm honors "overrides" only from a package.json it installs into, and the - # scan invokes the package via `npx ` with no such project directory. These - # overrides exist to swap a vulnerable-but-working transitive dep for a patched - # one, which does not change server startup or the tool surface being analyzed, - # so skipping them here does not affect the scan result. Warn so a future - # startup-affecting override does not fail confusingly. - npm_overrides = data['spec'].get('overrides', []) - if protocol == 'npx' and npm_overrides: - print( - f"Note: {server_name} declares spec.overrides, which are not applied to the " - "security scan (npm overrides require a package.json; npx installs ad hoc). " - "The built image still gets them.", - file=sys.stderr, - ) + # scan invokes `npx ` with no project directory of its own, so the package + # name and version travel alongside the overrides. run_scan.py stages a throwaway + # project from them and runs the scanner there. + npm_overrides = { + o['package']: o['version'] + for o in data['spec'].get('overrides', []) + if o.get('package') and o.get('version') + } if protocol in ['npx', 'uvx']: command = protocol @@ -76,6 +71,10 @@ def main(): } if protocol == 'uvx' and uv_overrides: output["uv_overrides"] = uv_overrides + if protocol == 'npx' and npm_overrides: + output["npm_overrides"] = npm_overrides + output["npm_package"] = package + output["npm_version"] = version print(json.dumps(output)) except FileNotFoundError: diff --git a/scripts/mcp-scan/run_scan.py b/scripts/mcp-scan/run_scan.py index 4b129ba3..322a2345 100644 --- a/scripts/mcp-scan/run_scan.py +++ b/scripts/mcp-scan/run_scan.py @@ -32,6 +32,9 @@ def main(): package_arg = config.get("args") mock_env = config.get("mock_env", []) uv_overrides = config.get("uv_overrides", []) + npm_overrides = config.get("npm_overrides", {}) + npm_package = config.get("npm_package") + npm_version = config.get("npm_version") except (FileNotFoundError, json.JSONDecodeError) as e: print(f"Error reading config file: {e}", file=sys.stderr) sys.exit(1) @@ -41,6 +44,7 @@ def main(): package_arg = args.package_arg mock_env = [] uv_overrides = [] + npm_overrides, npm_package, npm_version = {}, None, None else: print("Usage: run_scan.py --config ", file=sys.stderr) print(" or: run_scan.py ", file=sys.stderr) @@ -70,6 +74,57 @@ def main(): if command == "npx": scanner_args.append("--stdio-arg=--yes") + # Reapply npx dependency overrides (spec.overrides). npm honors "overrides" only from a + # package.json it installs into, and the scan otherwise invokes `npx ` ad hoc with no + # project directory. Stage a throwaway project carrying the dependency plus the overrides, + # install it, and run the scanner from there so npx resolves that tree. --no-install keeps + # npx from silently fetching an un-overridden copy instead. + npm_project = None + if npm_overrides: + if command != "npx": + print(f"Error: npm_overrides is only supported for npx, got {command}", file=sys.stderr) + sys.exit(1) + if not npm_package or not npm_version: + print("Error: npm_overrides requires npm_package and npm_version", file=sys.stderr) + sys.exit(1) + if shutil.which("npm") is None: + print("Error: npm is required to stage npx dependency overrides but was not found on PATH", + file=sys.stderr) + sys.exit(1) + + npm_project = tempfile.mkdtemp(prefix="mcp-scan-npm-") + # Staging happens before the scanner's own try/finally, so clean up here on any + # failure rather than leaving the temp project behind. + try: + with open(os.path.join(npm_project, "package.json"), "w") as f: + json.dump({ + "name": "mcp-scan-overrides", + "private": True, + "dependencies": {npm_package: npm_version}, + "overrides": npm_overrides, + }, f) + install = subprocess.run( + ["npm", "install", "--silent", "--no-audit", "--no-fund"], + cwd=npm_project, capture_output=True, text=True, check=False, timeout=300, + ) + if install.returncode != 0: + print(f"Error: npm install failed while staging overrides:\n{install.stderr}", + file=sys.stderr) + sys.exit(1) + except subprocess.TimeoutExpired: + print("Error: npm install timed out after 300 seconds while staging overrides", + file=sys.stderr) + sys.exit(1) + except OSError as e: + print(f"Error: could not stage npx dependency overrides: {e}", file=sys.stderr) + sys.exit(1) + finally: + if npm_project and not os.path.isdir(os.path.join(npm_project, "node_modules")): + shutil.rmtree(npm_project, ignore_errors=True) + npm_project = None + + scanner_args.append("--stdio-arg=--no-install") + # Reapply uvx dependency overrides (spec.constraints) so the scanned process resolves # the same dependency versions as the built image. uv takes these as a requirements # file, so write one; it must outlive this function's setup and be cleaned up after @@ -111,7 +166,7 @@ def main(): cmd = ["uv", "run", "--with", "cisco-ai-mcp-scanner", "mcp-scanner"] + scanner_args try: - result = subprocess.run(cmd, capture_output=True, text=True, check=False, timeout=300) + result = subprocess.run(cmd, cwd=npm_project, capture_output=True, text=True, check=False, timeout=300) if result.stdout: print(result.stdout) if result.stderr: @@ -129,6 +184,8 @@ def main(): os.unlink(overrides_file) except OSError: pass + if npm_project: + shutil.rmtree(npm_project, ignore_errors=True) if __name__ == "__main__": main()