From 3f2b1d01727a1eca632052ab4674c98d3d6eca7d Mon Sep 17 00:00:00 2001 From: 6tizer <6tizer@gmail.com> Date: Sat, 8 Aug 2026 10:31:57 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(mcp-server):=20=E6=96=B0=E5=A2=9E=20zc?= =?UTF-8?q?ode=5Fpr=5Freview=20=E2=80=94=20PR=20=E5=AE=A1=E6=9F=A5?= =?UTF-8?q?=E6=A8=A1=E5=BC=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - git diff base...HEAD (三点 merge-base 语义) 算改动清单与完整 diff; base 不传自动探测 (origin/HEAD → main/master → origin/main|master) - mimosa 全仓扫描 focus_files=改动文件 (deep 档业务逻辑投研聚焦, 默认 deep) - diff+findings 作附件喂 zcode 出 PR 复核报告 (P0/P1/P2 + findings 核实 + 能否合并结论); diff 超 ZCODE_BRIDGE_PR_DIFF_MAX (默认 500KB) 截断保清单 - 相对 base 无改动直接返回提示, 不消耗 LLM 调用 - 新增 6 个测试用例 (43 全绿); README/SKILL/agent-help 同步 --- README.md | 3 +- packages/agent-help/zcode-agent-help | 1 + packages/mcp-server/zcode-mcp-server | 222 ++++++++++++++++++++++++++- skills/zcode-bridge-guide/SKILL.md | 11 ++ tests/test_security_review.py | 135 +++++++++++++++- 5 files changed, 369 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 17f2c2f..b7ed73b 100644 --- a/README.md +++ b/README.md @@ -105,13 +105,14 @@ zcode --prompt "继续" --resume sess_xxxx ### MCP server（`zcode-mcp-server`） -暴露三个 MCP tool： +暴露四个 MCP tool： | Tool | 作用 | |------|------| | `get_zcode_capabilities` | 返回 ZCode 能力清单（调 agent-help） | | `zcode_review` | 调 ZCode 审查代码（yolo + 写/执行工具物理禁用，全程免授权但改不了文件，安全） | | `zcode_security_review` | 安全专项审查：mimosa 确定性规则引擎预扫 → ZCode 拿 findings 逐条核实（确认/误报/存疑 + 攻击路径 + 修复建议）。`depth=normal` 秒级快扫（默认），`depth=deep` 含业务逻辑投研（异步任务管线） | +| `zcode_pr_review` | PR 审查模式：自动算 `git diff base...HEAD`（merge-base 语义，base 可自动探测）→ mimosa 全仓扫描且业务逻辑复核聚焦改动文件（focus_files）→ ZCode 出 PR 复核报告（P0/P1/P2 分级 + findings 核实 + 能否合并结论）。默认 `depth=deep`；diff 超 `ZCODE_BRIDGE_PR_DIFF_MAX`（默认 500KB）截断保清单 | > **只读原理（2026-08-08 重构，告别 `--mode plan`）**：review 体系不再用 plan 模式——plan 只禁「改文件」，读探索/子代理照样放行（限流超时主因），且 plan→build 的规划惯性容易让 review 变成「边审边修」。新方案用 `--mode yolo`（全程免授权）+ `--disallowed-tools` 把 `Write/Edit/MultiEdit/ApplyPatch/Bash` 连同 Node REPL 一族（`js` / `mcp__node_repl__js*`）一起禁掉：`--disallowed-tools` 是工具集级物理移除、先于权限层，yolo 也绕不过；Node REPL 一族必须同禁，否则可被 `execSync` 打穿 Bash 黑名单（0.16.1 实测复现）。读工具（Read/Grep/Glob）全开，不影响审查能力。prompt 层另有「只审不修」职责约束（不修改文件、不提议帮忙修复）作双保险。 diff --git a/packages/agent-help/zcode-agent-help b/packages/agent-help/zcode-agent-help index 23f0df3..8e8a22d 100755 --- a/packages/agent-help/zcode-agent-help +++ b/packages/agent-help/zcode-agent-help @@ -522,6 +522,7 @@ ECOSYSTEM = { {"name": "get_zcode_capabilities", "description": "返回 zcode 能力清单 (调 agent-help)"}, {"name": "zcode_review", "description": "调 zcode 审查代码 (yolo+写工具物理禁用: 免授权只读)"}, {"name": "zcode_security_review", "description": "mimosa 预扫 (normal 快扫/deep 业务逻辑深扫) + zcode 只读复核 (安全专项)"}, + {"name": "zcode_pr_review", "description": "PR 审查: git diff 改动清单 + mimosa 聚焦深扫 (focus_files) + zcode P0/P1/P2 复核报告"}, ], "config": "~/.zcode/cli/config.json 的 mcp.servers.zcode-mcp", "use_case": "让 MCP client (zcode 自身/Claude Code/Cursor) 标准化调用 zcode", diff --git a/packages/mcp-server/zcode-mcp-server b/packages/mcp-server/zcode-mcp-server index 1316fc6..b8808c1 100755 --- a/packages/mcp-server/zcode-mcp-server +++ b/packages/mcp-server/zcode-mcp-server @@ -9,6 +9,7 @@ Claude Code、Cursor 等) 能标准化地发现并调用 headless zcode。 1. get_zcode_capabilities — 返回 headless zcode 完整能力清单 (JSON) 2. zcode_review — 调 zcode 审查代码 (yolo + 写工具物理禁用, 免授权且只读) 3. zcode_security_review — mimosa 规则引擎预扫 + zcode 只读逐条复核 (安全专项) + 4. zcode_pr_review — PR 审查: git diff + mimosa 聚焦深扫 + zcode 复核报告 协议: MCP over stdio (JSON-RPC 2.0, 每行一条消息) 日志: 全部走 stderr (绝不污染 stdout 协议流) @@ -38,7 +39,7 @@ ZCODE_BIN = os.environ.get("ZCODE_BIN", "zcode") # MCP 协议版本 PROTOCOL_VERSION = "2024-11-05" -SERVER_INFO = {"name": "zcode-mcp-server", "version": "1.1.0"} +SERVER_INFO = {"name": "zcode-mcp-server", "version": "1.2.0"} def log(msg): @@ -558,6 +559,49 @@ def _compact_findings(findings): return out +# ============================================================ +# PR 审查: git diff 计算 + base 自动探测 +# ============================================================ +def _git(repo, *git_args): + """跑 git 子命令, 返回 (returncode, stdout, stderr)。不抛异常, 调用方判。""" + try: + r = subprocess.run(["git", "-C", repo, *git_args], + capture_output=True, text=True, timeout=30) + return r.returncode, r.stdout, r.stderr + except (OSError, subprocess.TimeoutExpired) as e: + return 128, "", str(e) + + +def _resolve_pr_base(repo): + """自动探测 PR base 分支: origin/HEAD → main/master (本地) → origin/main|master。""" + rc, out, _ = _git(repo, "symbolic-ref", "--quiet", "--short", + "refs/remotes/origin/HEAD") + candidates = [] + if rc == 0 and out.strip(): + candidates.append(out.strip()) # 形如 origin/main + candidates += ["main", "master", "origin/main", "origin/master"] + for cand in candidates: + rc, _, _ = _git(repo, "rev-parse", "--verify", "--quiet", cand) + if rc == 0: + return cand + return None + + +def _pr_diff(repo, base, head): + """计算 base...head 的改动文件清单 + diff 全文 (三点 = merge-base 语义)。 + + 返回 (changed_files, diff_text); 失败抛 RuntimeError。 + """ + rc, files_out, err = _git(repo, "diff", "--name-only", f"{base}...{head}") + if rc != 0: + raise RuntimeError(f"git diff {base}...{head} 失败: {err.strip()[:200]}") + changed = [ln.strip() for ln in files_out.splitlines() if ln.strip()] + rc, diff_text, err = _git(repo, "diff", f"{base}...{head}") + if rc != 0: + raise RuntimeError(f"git diff {base}...{head} 失败: {err.strip()[:200]}") + return changed, diff_text + + # ============================================================ # Tool 定义 # ============================================================ @@ -659,6 +703,49 @@ TOOLS = [ "additionalProperties": False, }, }, + { + "name": "zcode_pr_review", + "description": ( + "PR 审查模式: 自动算 git diff (base...HEAD, merge-base 语义) 得到改动清单," + "mimosa 全仓扫描 + 业务逻辑复核优先聚焦改动文件 (focus_files)," + "ZCode 拿着 diff+findings 出 PR 复核报告 (P0/P1/P2 分级 + 能否合并结论)。" + "全程只读: yolo+写工具物理禁用, 免人工授权。需要本机装有 mimosa 插件。" + ), + "inputSchema": { + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "git 仓库目录,默认当前目录", + }, + "base": { + "type": "string", + "description": "PR base 分支/commit,不传则自动探测 " + "(origin/HEAD → main/master → origin/main|master)", + }, + "head": { + "type": "string", + "description": "PR head (默认 HEAD)", + "default": "HEAD", + }, + "depth": { + "type": "string", + "enum": ["normal", "deep"], + "description": "mimosa 扫描深度,PR 审查默认 deep (业务逻辑投研聚焦改动文件)", + "default": "deep", + }, + "focus": { + "type": "string", + "description": "可选,额外审查重点", + }, + "cwd": { + "type": "string", + "description": "zcode 工作目录,默认与 path 相同", + }, + }, + "additionalProperties": False, + }, + }, ] @@ -922,6 +1009,138 @@ def tool_zcode_security_review(args): pass +def tool_zcode_pr_review(args): + """PR 审查模式: git diff + mimosa 聚焦深扫 + zcode 复核报告。 + + ① git diff base...head (merge-base 语义) 算改动清单与完整 diff; + ② mimosa 全仓扫描 (focus_files=改动文件, deep 档业务逻辑投研聚焦改动); + ③ diff+findings 作附件喂 zcode 出 PR 复核报告 (P0/P1/P2 + 能否合并)。 + 全程只读: 写工具物理禁用; git/mimosa 都是只读调用。 + """ + scan_path = args.get("path") or args.get("cwd") or os.getcwd() + focus = args.get("focus", "") + depth = args.get("depth", "deep") + head = args.get("head", "HEAD") + + if depth not in ("normal", "deep"): + return {"content": [{"type": "text", + "text": f"非法 depth: {depth!r} (只支持 normal|deep)"}], + "isError": True} + if not os.path.isdir(scan_path): + return {"content": [{"type": "text", + "text": f"仓库目录不存在或不是目录: {scan_path}"}], + "isError": True} + + # ① git diff (只读) + rc, _, err = _git(scan_path, "rev-parse", "--git-dir") + if rc != 0: + return {"content": [{"type": "text", + "text": f"{scan_path} 不是 git 仓库: {err.strip()[:200]}"}], + "isError": True} + base = args.get("base") + if not base: + base = _resolve_pr_base(scan_path) + if not base: + return {"content": [{"type": "text", "text": ( + "无法自动探测 PR base 分支 (试了 origin/HEAD、main/master、" + "origin/main|master)。请显式传 base 参数, 如 base='main'。" + )}], "isError": True} + rc, _, err = _git(scan_path, "rev-parse", "--verify", "--quiet", base) + if rc != 0: + return {"content": [{"type": "text", + "text": f"base 不存在: {base!r} (git: {err.strip()[:150]})"}], + "isError": True} + try: + changed, diff_text = _pr_diff(scan_path, base, head) + except RuntimeError as e: + return {"content": [{"type": "text", "text": str(e)}], "isError": True} + if not changed: + return {"content": [{"type": "text", "text": ( + f"相对 {base}...{head} 没有任何改动, 无需审查。" + "如果改动还在工作区未 commit, 请先提交或调整 base/head。" + )}]} + + # diff 体积上限 (附件体积保护, ZCODE_BRIDGE_PR_DIFF_MAX 可配) + max_bytes = max(10_000, _env_int("ZCODE_BRIDGE_PR_DIFF_MAX", 500_000, + maximum=5_000_000)) + if len(diff_text.encode("utf-8")) > max_bytes: + diff_text = diff_text[:max_bytes] + ( + f"\n\n[... diff 超 {max_bytes} 字节已截断; 改动文件清单完整, " + "可用只读工具阅读完整文件 ...]") + + root = _find_mimosa_root() + if not root: + return {"content": [{"type": "text", "text": ( + "未找到 mimosa 安全扫描插件。请安装 mimosa,或设 ZCODE_BRIDGE_MIMOSA_ROOT " + "指向插件根目录 (含 payload/ 的那层);也可改用 zcode_review 做纯 AI 审查。" + )}], "isError": True} + + # ② mimosa 扫描 (focus_files=改动文件; deep 档业务逻辑投研聚焦, 全仓仍全量扫) + log(f"mimosa {depth} 扫描 (PR 模式): {scan_path} (root={root}, " + f"{len(changed)} 个改动文件)") + try: + if depth == "deep": + summary_text, findings = _mimosa_deep_scan(root, scan_path, changed) + else: + summary_text, findings = _mimosa_quick_scan(root, scan_path) + except Exception as e: + return {"content": [{"type": "text", "text": f"mimosa 扫描失败: {e}"}], + "isError": True} + + # ③ 附件 = PR 元信息 + diff + mimosa 摘要 + findings + attachment = ( + f"# PR 信息\n- base: {base} → head: {head}\n" + f"- 改动文件 ({len(changed)} 个):\n" + + "".join(f" - {f}\n" for f in changed) + + f"\n# PR diff\n```diff\n{diff_text}\n```\n" + + f"\n# mimosa 扫描摘要 (depth={depth})\n{summary_text}\n" + ) + if findings: + compact = _compact_findings(findings) + attachment += ( + f"\n# findings 清单 ({len(compact)} 条)\n" + + json.dumps(compact, ensure_ascii=False, indent=1) + ) + else: + attachment += ("\n# findings 清单\n(未取到结构化 findings, 以摘要为准;" + "如摘要显示 0 发现, 也请按你的判断抽查关键代码)\n") + + tmp_path = None + try: + tmp_path = _write_temp(attachment, "zcode-pr-review-") + prompt = ( + "你是资深安全审查专家。这是一次 PR 审查: 附件含 ① 本 PR 的完整 diff " + f"(base={base} → {head}, 改动 {len(changed)} 个文件) ② mimosa 安全扫描结果" + f"(depth={depth}, 业务逻辑复核优先聚焦本次改动文件)。\n" + "任务:\n" + "1. 审查 diff 中新引入的问题, 按 P0(阻断合并)/P1(应修)/P2(建议) 分级," + "每个问题含文件位置/判定依据/修复建议。重点是'新引入';" + "存量老问题不阻断本次合并的, 标为 P2 提示即可。\n" + "2. 核实 mimosa findings: 逐条判定【确认漏洞】/【误报】/【存疑】," + "优先核实落在改动文件里的; 每条给出依据 (攻击路径或为何不可利用)。\n" + "3. 可以用只读工具 (Read/Grep/Glob) 阅读仓库代码获取上下文。\n" + "规则: 绝对不要修改、创建或删除任何文件 (写/执行工具已被物理禁用);" + "不要提出\"帮你修复\"的提议; 只输出审查报告。\n" + "输出格式: 先给汇总 (P0/P1/P2 各几条 + findings 确认/误报/存疑各几条" + " + 能否合并的结论), 再逐条详述。用中文。" + ) + if focus: + prompt += f"\n额外审查重点: {focus}。" + + cmd = _build_review_cmd(prompt, args.get("cwd") or scan_path) + cmd += ["--attach", tmp_path] + + log(f"调用 zcode PR 复核 (yolo+只读黑名单, {len(changed)} 个改动文件)") + env = _merge_env_with_creds(load_zcode_credentials()) + return _run_zcode_headless(cmd, env, _review_timeout()) + finally: + if tmp_path: + try: + os.unlink(tmp_path) + except OSError: + pass + + def _env_int(name, default, maximum=None): """从环境变量读整数, 失败用默认值; maximum 给上界 (复审 R2 P2-5: 之前只靠调用点 max() 兜下限, env 误设天文数字没有防线)。""" @@ -938,6 +1157,7 @@ TOOL_HANDLERS = { "get_zcode_capabilities": tool_get_zcode_capabilities, "zcode_review": tool_zcode_review, "zcode_security_review": tool_zcode_security_review, + "zcode_pr_review": tool_zcode_pr_review, } diff --git a/skills/zcode-bridge-guide/SKILL.md b/skills/zcode-bridge-guide/SKILL.md index 83fe69a..7d3796c 100644 --- a/skills/zcode-bridge-guide/SKILL.md +++ b/skills/zcode-bridge-guide/SKILL.md @@ -329,6 +329,17 @@ MCP server 暴露三个标准 MCP tool，供 Claude Code / Cursor 等 MCP client | `get_zcode_capabilities` | 返回 ZCode 完整能力清单 | 只读 | | `zcode_review` | 调用 ZCode 审查代码 | 只读（`--mode yolo` + `--disallowed-tools` 物理禁用写/执行工具，全程免授权但改不了文件） | | `zcode_security_review` | 安全专项审查：mimosa 规则引擎全仓预扫 → ZCode 逐条核实 findings | 只读（同上；需本机装有 mimosa 或设 `ZCODE_BRIDGE_MIMOSA_ROOT`） | +| `zcode_pr_review` | PR 审查：git diff 改动清单 + mimosa 聚焦深扫（focus_files）→ ZCode 出 P0/P1/P2 复核报告 + 能否合并结论 | 只读（同上；base 不传自动探测，默认 depth=deep） | + +### `zcode_pr_review` 参数 +- `path`：git 仓库目录（默认当前目录） +- `base`：PR base 分支/commit，不传自动探测（origin/HEAD → main/master → origin/main|master） +- `head`：默认 HEAD +- `depth`：mimosa 扫描深度，PR 审查默认 `deep` +- `focus`：可选，额外审查重点 +- `cwd`：zcode 工作目录（默认与 path 相同） + +> 流程：`git diff base...head`（三点 = merge-base 语义）算改动清单与完整 diff → mimosa 全仓扫描（`focus_files`=改动文件，deep 档业务逻辑投研聚焦）→ diff+findings 作附件喂 zcode，出「P0 阻断/P1 应修/P2 建议 + findings 确认/误报/存疑 + 能否合并」报告。diff 超 `ZCODE_BRIDGE_PR_DIFF_MAX`（默认 500KB）截断但改动文件清单完整。相对 base 无改动时直接返回提示、不消耗 LLM 调用。 ### `zcode_review` 参数 - `files`：要审查的文件路径列表 diff --git a/tests/test_security_review.py b/tests/test_security_review.py index 64fca88..e989026 100644 --- a/tests/test_security_review.py +++ b/tests/test_security_review.py @@ -71,7 +71,8 @@ class _EnvGuard(unittest.TestCase): ENV_KEYS = ("ZCODE_BRIDGE_REVIEW_LOCK", "ZCODE_BRIDGE_MIMOSA_ROOT", "ZCODE_BRIDGE_REVIEW_TIMEOUT", "ZCODE_BRIDGE_MIMOSA_SCAN_ROOT", "ZCODE_BRIDGE_MIMOSA_DEEP_TIMEOUT", - "ZCODE_BRIDGE_MIMOSA_POLL_INTERVAL") + "ZCODE_BRIDGE_MIMOSA_POLL_INTERVAL", + "ZCODE_BRIDGE_PR_DIFF_MAX") def setUp(self): self._saved = {k: os.environ.get(k) for k in self.ENV_KEYS} @@ -828,5 +829,137 @@ def fake_run(cmd, *a, **kw): self.assertIn("depth=deep", prompt) +class TestPrReview(_EnvGuard): + """tool_zcode_pr_review: git diff + mimosa 聚焦 + zcode 复核""" + + @classmethod + def setUpClass(cls): + cls.mod = _load_mcp_module() + + def _patch(self, changed=None, diff_text="diff --git a/app.py b/app.py\n+new line\n"): + """patch git/mimosa/zcode 三路。changed=None 表示非 git 仓库。""" + import tempfile + mod = self.mod + proj = tempfile.mkdtemp(prefix="zcode-pr-proj-") + self.addCleanup(lambda: __import__("shutil").rmtree(proj, ignore_errors=True)) + saved = { + "find_root": mod._find_mimosa_root, + "deep": mod._mimosa_deep_scan, + "quick": mod._mimosa_quick_scan, + "run": mod.subprocess.run, + } + captured = {} + + def fake_run(cmd, *a, **kw): + if cmd[0] == "git": + if changed is None: # 非 git 仓库 + return _FakeCompletedProcess(128, "", "not a git repository") + sub = cmd[3] # ["git", "-C", repo, , ...] + if sub == "rev-parse" or sub == "symbolic-ref": + return _FakeCompletedProcess(0, "ok\n", "") + if sub == "diff" and "--name-only" in cmd: + return _FakeCompletedProcess( + 0, "".join(f + "\n" for f in changed), "") + if sub == "diff": + return _FakeCompletedProcess(0, diff_text, "") + captured["cmd"] = cmd # zcode 调用 + return _FakeCompletedProcess( + 0, json.dumps({"response": "PR 报告"}, ensure_ascii=False), "") + + def fake_find_root(): + return "/fake/mimosa" + + def fake_deep(root, path, focus_files=None): + captured["focus_files"] = focus_files + return ("deep 摘要", [{"identity": {"publicClass": "sql-injection"}, + "severity": "high", "cwe": ["CWE-89"], + "location": {"path": "app.py", "line": 11}, + "title": "SQL 注入", "message": "拼接查询"}]) + + mod.subprocess.run = fake_run + mod._find_mimosa_root = fake_find_root + mod._mimosa_deep_scan = fake_deep + mod._mimosa_quick_scan = lambda r, p: ("normal 摘要", []) + os.environ["ZCODE_BRIDGE_REVIEW_LOCK"] = "0" + return mod, saved, proj, captured + + def _restore(self, mod, saved): + mod._find_mimosa_root = saved["find_root"] + mod._mimosa_deep_scan = saved["deep"] + mod._mimosa_quick_scan = saved["quick"] + mod.subprocess.run = saved["run"] + + def test_pr0_not_a_git_repo(self): + """PR0: 非 git 仓库 → 明确报错""" + mod, saved, proj, _ = self._patch(changed=None) + try: + result = mod.tool_zcode_pr_review({"path": proj}) + finally: + self._restore(mod, saved) + self.assertTrue(result.get("isError")) + self.assertIn("不是 git 仓库", result["content"][0]["text"]) + + def test_pr1_no_changes_friendly_exit(self): + """PR1: 无改动 → 友好提示, 不调 mimosa/zcode""" + mod, saved, proj, captured = self._patch(changed=[]) + try: + result = mod.tool_zcode_pr_review({"path": proj, "base": "main"}) + finally: + self._restore(mod, saved) + self.assertNotIn("isError", result) + self.assertIn("没有任何改动", result["content"][0]["text"]) + self.assertNotIn("cmd", captured, "无改动不应调 zcode") + + def test_pr2_happy_path(self): + """PR2: 正常管线 — diff 进附件, focus_files=改动文件, 默认 deep""" + mod, saved, proj, captured = self._patch(changed=["app.py", "util.py"]) + try: + result = mod.tool_zcode_pr_review({"path": proj, "base": "main"}) + finally: + self._restore(mod, saved) + self.assertNotIn("isError", result) + self.assertEqual(result["content"][0]["text"], "PR 报告") + # focus_files 透传改动清单 + self.assertEqual(captured["focus_files"], ["app.py", "util.py"]) + cmd = captured["cmd"] + self.assertEqual(cmd[cmd.index("--mode") + 1], "yolo") + self.assertIn("--disallowed-tools", cmd) + prompt = cmd[cmd.index("--prompt") + 1] + self.assertIn("PR 审查", prompt) + self.assertIn("P0", prompt) + self.assertIn("能否合并", prompt) + + def test_pr3_base_autodetect_used(self): + """PR3: 不传 base → 走自动探测 (fake git 的 rev-parse 全通过)""" + mod, saved, proj, captured = self._patch(changed=["a.py"]) + try: + result = mod.tool_zcode_pr_review({"path": proj}) + finally: + self._restore(mod, saved) + self.assertNotIn("isError", result) + + def test_pr4_bad_depth_rejected(self): + """PR4: 非法 depth 在校验 git 之前就被拒""" + mod, saved, proj, captured = self._patch(changed=["a.py"]) + try: + result = mod.tool_zcode_pr_review({"path": proj, "depth": "x"}) + finally: + self._restore(mod, saved) + self.assertTrue(result.get("isError")) + self.assertNotIn("cmd", captured) + + def test_pr5_diff_truncation(self): + """PR5: diff 超 ZCODE_BRIDGE_PR_DIFF_MAX → 截断并标注""" + os.environ["ZCODE_BRIDGE_PR_DIFF_MAX"] = "10000" + big_diff = "x" * 20000 + mod, saved, proj, captured = self._patch(changed=["a.py"], diff_text=big_diff) + try: + result = mod.tool_zcode_pr_review({"path": proj, "base": "main"}) + finally: + self._restore(mod, saved) + os.environ.pop("ZCODE_BRIDGE_PR_DIFF_MAX", None) + self.assertNotIn("isError", result) + + if __name__ == "__main__": unittest.main(verbosity=2) From f867053aa9b880adce060b51133bba8afd294902 Mon Sep 17 00:00:00 2001 From: 6tizer <6tizer@gmail.com> Date: Sat, 8 Aug 2026 10:37:06 -0400 Subject: [PATCH 2/3] =?UTF-8?q?fix(mcp-server):=20=E8=87=AA=E5=AE=A1=20P1?= =?UTF-8?q?=20=E9=97=AD=E7=8E=AF=20=E2=80=94=20base/head=20git=20=E9=80=89?= =?UTF-8?q?=E9=A1=B9=E6=B3=A8=E5=85=A5=E9=98=B2=E6=8A=A4=20+=20UTF-8=20?= =?UTF-8?q?=E5=AD=97=E8=8A=82=E6=88=AA=E6=96=AD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P1-1: base/head 统一 _validate_rev 校验 (拒绝 - 开头防 --output= 等选项注入; head 此前完全无校验); diff 命令补 -- 分隔符 - P1-2: diff 截断改按字节切 + errors=ignore 容错解码, 防多字节字符切半 - 新增 PR6/PR7 注入拒绝用例 (45 全绿) --- packages/mcp-server/zcode-mcp-server | 30 +++++++++++++++++++--------- tests/test_security_review.py | 24 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/packages/mcp-server/zcode-mcp-server b/packages/mcp-server/zcode-mcp-server index b8808c1..0f12939 100755 --- a/packages/mcp-server/zcode-mcp-server +++ b/packages/mcp-server/zcode-mcp-server @@ -592,16 +592,27 @@ def _pr_diff(repo, base, head): 返回 (changed_files, diff_text); 失败抛 RuntimeError。 """ - rc, files_out, err = _git(repo, "diff", "--name-only", f"{base}...{head}") + rc, files_out, err = _git(repo, "diff", "--name-only", f"{base}...{head}", "--") if rc != 0: raise RuntimeError(f"git diff {base}...{head} 失败: {err.strip()[:200]}") changed = [ln.strip() for ln in files_out.splitlines() if ln.strip()] - rc, diff_text, err = _git(repo, "diff", f"{base}...{head}") + rc, diff_text, err = _git(repo, "diff", f"{base}...{head}", "--") if rc != 0: raise RuntimeError(f"git diff {base}...{head} 失败: {err.strip()[:200]}") return changed, diff_text +def _validate_rev(repo, rev, what): + """校验 git rev 合法: 非 - 开头 (防 git 选项注入, 自审 P1-1: --output= 等 + 会让只读 diff 写出文件) 且 rev-parse 可解析。返回错误文案或 None。""" + if not rev or rev.startswith("-"): + return f"非法 {what}: {rev!r} (空值或 - 开头会被 git 当选项解析, 已拒绝)" + rc, _, err = _git(repo, "rev-parse", "--verify", "--quiet", rev) + if rc != 0: + return f"{what} 不存在: {rev!r} (git: {err.strip()[:150]})" + return None + + # ============================================================ # Tool 定义 # ============================================================ @@ -1045,11 +1056,10 @@ def tool_zcode_pr_review(args): "无法自动探测 PR base 分支 (试了 origin/HEAD、main/master、" "origin/main|master)。请显式传 base 参数, 如 base='main'。" )}], "isError": True} - rc, _, err = _git(scan_path, "rev-parse", "--verify", "--quiet", base) - if rc != 0: - return {"content": [{"type": "text", - "text": f"base 不存在: {base!r} (git: {err.strip()[:150]})"}], - "isError": True} + for rev, what in ((base, "base"), (head, "head")): + bad = _validate_rev(scan_path, rev, what) + if bad: + return {"content": [{"type": "text", "text": bad}], "isError": True} try: changed, diff_text = _pr_diff(scan_path, base, head) except RuntimeError as e: @@ -1063,8 +1073,10 @@ def tool_zcode_pr_review(args): # diff 体积上限 (附件体积保护, ZCODE_BRIDGE_PR_DIFF_MAX 可配) max_bytes = max(10_000, _env_int("ZCODE_BRIDGE_PR_DIFF_MAX", 500_000, maximum=5_000_000)) - if len(diff_text.encode("utf-8")) > max_bytes: - diff_text = diff_text[:max_bytes] + ( + diff_bytes = diff_text.encode("utf-8") + if len(diff_bytes) > max_bytes: + # 按字节截断再容错解码, 防多字节字符被切成半个 (自审 P1-2) + diff_text = diff_bytes[:max_bytes].decode("utf-8", errors="ignore") + ( f"\n\n[... diff 超 {max_bytes} 字节已截断; 改动文件清单完整, " "可用只读工具阅读完整文件 ...]") diff --git a/tests/test_security_review.py b/tests/test_security_review.py index e989026..8cc44a7 100644 --- a/tests/test_security_review.py +++ b/tests/test_security_review.py @@ -960,6 +960,30 @@ def test_pr5_diff_truncation(self): os.environ.pop("ZCODE_BRIDGE_PR_DIFF_MAX", None) self.assertNotIn("isError", result) + def test_pr6_dash_base_rejected(self): + """PR6: base 以 - 开头 → 拒绝 (自审 P1-1: git 选项注入防护)""" + mod, saved, proj, captured = self._patch(changed=["a.py"]) + try: + result = mod.tool_zcode_pr_review( + {"path": proj, "base": "--output=/tmp/pwn"}) + finally: + self._restore(mod, saved) + self.assertTrue(result.get("isError")) + self.assertIn("非法 base", result["content"][0]["text"]) + self.assertNotIn("cmd", captured, "注入企图不应到达 zcode") + + def test_pr7_dash_head_rejected(self): + """PR7: head 以 - 开头 → 拒绝 (head 同样校验, 不再漏检)""" + mod, saved, proj, captured = self._patch(changed=["a.py"]) + try: + result = mod.tool_zcode_pr_review( + {"path": proj, "base": "main", "head": "--stdout"}) + finally: + self._restore(mod, saved) + self.assertTrue(result.get("isError")) + self.assertIn("非法 head", result["content"][0]["text"]) + self.assertNotIn("cmd", captured) + if __name__ == "__main__": unittest.main(verbosity=2) From 435c3bf12daadc2f9576b296b4ff6c0e46d70f11 Mon Sep 17 00:00:00 2001 From: 6tizer <6tizer@gmail.com> Date: Sat, 8 Aug 2026 10:43:35 -0400 Subject: [PATCH 3/3] =?UTF-8?q?fix(mcp-server):=20=E8=87=AA=E5=AE=A1?= =?UTF-8?q?=E7=AC=AC=E4=BA=8C=E8=BD=AE=20P1/P2=20=E9=97=AD=E7=8E=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P1-1 补强: rev-parse/diff 全面加 --end-of-options (git≥2.24), 关掉 rev 被当选项解析的残余路径 - P1-2: diff 截断先退到最后完整行再容错解码, 防半行误导 diff 结构 - P2-1: focus_files (改动清单) 与 security_review 对齐 200 上限 - P2-3: focus 参数三个 review tool 统一 _clamp_focus 钳 2000 字符 - P2-4: 补 base 探测失败/mimosa 失败/附件清理三个用例 (48 全绿) - P2-2 (ref 名含 ...): git refname 规则本就不允许 '..', 记录为无需修 --- packages/mcp-server/zcode-mcp-server | 57 +++++++++++++++++++++------- tests/test_security_review.py | 48 ++++++++++++++++++++++- 2 files changed, 89 insertions(+), 16 deletions(-) diff --git a/packages/mcp-server/zcode-mcp-server b/packages/mcp-server/zcode-mcp-server index 0f12939..3228a58 100755 --- a/packages/mcp-server/zcode-mcp-server +++ b/packages/mcp-server/zcode-mcp-server @@ -581,7 +581,8 @@ def _resolve_pr_base(repo): candidates.append(out.strip()) # 形如 origin/main candidates += ["main", "master", "origin/main", "origin/master"] for cand in candidates: - rc, _, _ = _git(repo, "rev-parse", "--verify", "--quiet", cand) + rc, _, _ = _git(repo, "rev-parse", "--verify", "--quiet", + "--end-of-options", cand) if rc == 0: return cand return None @@ -591,23 +592,32 @@ def _pr_diff(repo, base, head): """计算 base...head 的改动文件清单 + diff 全文 (三点 = merge-base 语义)。 返回 (changed_files, diff_text); 失败抛 RuntimeError。 + base/head 需先过 _validate_rev; 这里再加 --end-of-options 双保险, + 末尾 -- 隔离 pathspec。 """ - rc, files_out, err = _git(repo, "diff", "--name-only", f"{base}...{head}", "--") + range_spec = f"{base}...{head}" + rc, files_out, err = _git(repo, "diff", "--name-only", "--end-of-options", + range_spec, "--") if rc != 0: - raise RuntimeError(f"git diff {base}...{head} 失败: {err.strip()[:200]}") + raise RuntimeError(f"git diff {range_spec} 失败: {err.strip()[:200]}") changed = [ln.strip() for ln in files_out.splitlines() if ln.strip()] - rc, diff_text, err = _git(repo, "diff", f"{base}...{head}", "--") + rc, diff_text, err = _git(repo, "diff", "--end-of-options", range_spec, "--") if rc != 0: - raise RuntimeError(f"git diff {base}...{head} 失败: {err.strip()[:200]}") + raise RuntimeError(f"git diff {range_spec} 失败: {err.strip()[:200]}") return changed, diff_text def _validate_rev(repo, rev, what): """校验 git rev 合法: 非 - 开头 (防 git 选项注入, 自审 P1-1: --output= 等 - 会让只读 diff 写出文件) 且 rev-parse 可解析。返回错误文案或 None。""" + 会让只读 diff 写出文件) 且 rev-parse 可解析。返回错误文案或 None。 + + rev-parse 带 --end-of-options (git ≥ 2.24, 复审 P1-1 补强): 彻底关掉 + rev 被当选项解析的残余路径 (如 rev-parse 自身选项别名)。 + """ if not rev or rev.startswith("-"): return f"非法 {what}: {rev!r} (空值或 - 开头会被 git 当选项解析, 已拒绝)" - rc, _, err = _git(repo, "rev-parse", "--verify", "--quiet", rev) + rc, _, err = _git(repo, "rev-parse", "--verify", "--quiet", + "--end-of-options", rev) if rc != 0: return f"{what} 不存在: {rev!r} (git: {err.strip()[:150]})" return None @@ -873,10 +883,20 @@ def _write_temp(text, prefix): return tmp.name +def _clamp_focus(focus): + """focus 来自 MCP client (信任边界外): 长度钳 2000 字符 (复审 P2-3: + 超长 focus 会撑大 prompt; 三个 review tool 统一在此收口)。""" + focus = focus or "" + if len(focus) > 2000: + log(f"⚠ focus 超 2000 字符 ({len(focus)}), 截断") + focus = focus[:2000] + return focus + + def tool_zcode_review(args): files = args.get("files", []) code = args.get("code") - focus = args.get("focus", "全面审查: 安全、正确性、可维护性") + focus = _clamp_focus(args.get("focus")) or "全面审查: 安全、正确性、可维护性" cwd = args.get("cwd") # 构造 prompt — "只审不修"职责钉死在文本层, 写工具在工具集层物理禁用, @@ -930,7 +950,7 @@ def tool_zcode_security_review(args): 读工具全开可查证上下文, 写工具物理禁用保证只审不修。 """ scan_path = args.get("path") or args.get("cwd") or os.getcwd() - focus = args.get("focus", "") + focus = _clamp_focus(args.get("focus")) depth = args.get("depth", "normal") focus_files = args.get("focus_files") or [] @@ -1029,7 +1049,7 @@ def tool_zcode_pr_review(args): 全程只读: 写工具物理禁用; git/mimosa 都是只读调用。 """ scan_path = args.get("path") or args.get("cwd") or os.getcwd() - focus = args.get("focus", "") + focus = _clamp_focus(args.get("focus")) depth = args.get("depth", "deep") head = args.get("head", "HEAD") @@ -1075,8 +1095,13 @@ def tool_zcode_pr_review(args): maximum=5_000_000)) diff_bytes = diff_text.encode("utf-8") if len(diff_bytes) > max_bytes: - # 按字节截断再容错解码, 防多字节字符被切成半个 (自审 P1-2) - diff_text = diff_bytes[:max_bytes].decode("utf-8", errors="ignore") + ( + # 按字节截断: 先退到最后一个完整行 (复审 P1-2: 防半行误导 diff 结构), + # 再容错解码防多字节字符切半 + cut = diff_bytes[:max_bytes] + nl = cut.rfind(b"\n") + if nl > 0: + cut = cut[:nl] + diff_text = cut.decode("utf-8", errors="ignore") + ( f"\n\n[... diff 超 {max_bytes} 字节已截断; 改动文件清单完整, " "可用只读工具阅读完整文件 ...]") @@ -1087,12 +1112,16 @@ def tool_zcode_pr_review(args): "指向插件根目录 (含 payload/ 的那层);也可改用 zcode_review 做纯 AI 审查。" )}], "isError": True} - # ② mimosa 扫描 (focus_files=改动文件; deep 档业务逻辑投研聚焦, 全仓仍全量扫) + # ② mimosa 扫描 (focus_files=改动文件, 上限 200 与 security_review 一致; + # deep 档业务逻辑投研聚焦, 全仓仍全量扫) + focus_files = changed[:200] + if len(changed) > 200: + log(f"⚠ 改动文件 {len(changed)} 超 200, focus_files 截断 (清单在附件里完整)") log(f"mimosa {depth} 扫描 (PR 模式): {scan_path} (root={root}, " f"{len(changed)} 个改动文件)") try: if depth == "deep": - summary_text, findings = _mimosa_deep_scan(root, scan_path, changed) + summary_text, findings = _mimosa_deep_scan(root, scan_path, focus_files) else: summary_text, findings = _mimosa_quick_scan(root, scan_path) except Exception as e: diff --git a/tests/test_security_review.py b/tests/test_security_review.py index 8cc44a7..125f161 100644 --- a/tests/test_security_review.py +++ b/tests/test_security_review.py @@ -836,8 +836,10 @@ class TestPrReview(_EnvGuard): def setUpClass(cls): cls.mod = _load_mcp_module() - def _patch(self, changed=None, diff_text="diff --git a/app.py b/app.py\n+new line\n"): - """patch git/mimosa/zcode 三路。changed=None 表示非 git 仓库。""" + def _patch(self, changed=None, diff_text="diff --git a/app.py b/app.py\n+new line\n", + rev_ok=True): + """patch git/mimosa/zcode 三路。changed=None 表示非 git 仓库; + rev_ok=False 表示所有 rev 解析失败 (测 base 自动探测失败)。""" import tempfile mod = self.mod proj = tempfile.mkdtemp(prefix="zcode-pr-proj-") @@ -855,7 +857,11 @@ def fake_run(cmd, *a, **kw): if changed is None: # 非 git 仓库 return _FakeCompletedProcess(128, "", "not a git repository") sub = cmd[3] # ["git", "-C", repo, , ...] + if sub == "rev-parse" and "--git-dir" in cmd: + return _FakeCompletedProcess(0, ".git\n", "") # 仓库探测恒过 if sub == "rev-parse" or sub == "symbolic-ref": + if not rev_ok: # rev 校验/分支探测失败 + return _FakeCompletedProcess(1, "", "unknown revision") return _FakeCompletedProcess(0, "ok\n", "") if sub == "diff" and "--name-only" in cmd: return _FakeCompletedProcess( @@ -984,6 +990,44 @@ def test_pr7_dash_head_rejected(self): self.assertIn("非法 head", result["content"][0]["text"]) self.assertNotIn("cmd", captured) + def test_pr8_base_autodetect_failure(self): + """PR8: base 自动探测全部失败 → 明确报错建议显式传 base (复审 P2-4)""" + mod, saved, proj, captured = self._patch(changed=["a.py"], rev_ok=False) + try: + result = mod.tool_zcode_pr_review({"path": proj}) + finally: + self._restore(mod, saved) + self.assertTrue(result.get("isError")) + self.assertIn("无法自动探测", result["content"][0]["text"]) + self.assertNotIn("cmd", captured) + + def test_pr9_mimosa_failure_clean_error(self): + """PR9: mimosa 扫描失败 → 明确报错, 不调 zcode (复审 P2-4)""" + mod, saved, proj, captured = self._patch(changed=["a.py"]) + + def boom(root, path, focus_files=None): + raise RuntimeError("engine boom") + + mod._mimosa_deep_scan = boom + try: + result = mod.tool_zcode_pr_review({"path": proj, "base": "main"}) + finally: + self._restore(mod, saved) + self.assertTrue(result.get("isError")) + self.assertIn("mimosa 扫描失败", result["content"][0]["text"]) + self.assertNotIn("cmd", captured) + + def test_pr10_attachment_tmpfile_cleaned(self): + """PR10: PR 附件临时文件用后被清理 (复审 P2-4)""" + mod, saved, proj, captured = self._patch(changed=["a.py"]) + try: + mod.tool_zcode_pr_review({"path": proj, "base": "main"}) + finally: + self._restore(mod, saved) + attach_path = captured["cmd"][captured["cmd"].index("--attach") + 1] + self.assertIn("zcode-pr-review-", attach_path) + self.assertFalse(os.path.exists(attach_path), "PR 附件临时文件应被清理") + if __name__ == "__main__": unittest.main(verbosity=2)