diff --git a/docs/DeveloperGuide.md b/docs/DeveloperGuide.md index 5eed75f..2a9ead9 100644 --- a/docs/DeveloperGuide.md +++ b/docs/DeveloperGuide.md @@ -391,7 +391,7 @@ codeweb 提供四种 MCP/外部集成方式: - 查询工具读取 `Arc` 快照后立即释放锁,长查询不会阻塞 `codeweb_analyze`。 - `codeweb_analyze` 在 `tokio::task::spawn_blocking` 中运行 `Project::analyze`(CPU 密集、同步),完成后把新 store 换入快照。 - 未初始化目录不再导致进程退出:查询返回 `status: "uninitialized"`,引导调用 `codeweb_init`。 -- 写守卫 `confine_to_root` 做词法归一化后校验路径在 `permitted_root` 内;`store.path` 逃逸时 analyze 直接返回错误。 +- 写守卫 `confine_to_root` 双层校验:词法归一化拦住 `..` 逃逸;再对最深已存在祖先做 `canonicalize` 比较,拦住 `.codeweb` 指向目录外的符号链接。`store.path` 逃逸时 analyze 直接返回错误,且不产生任何写入。 - stdout 只用于 JSON-RPC:进度条与报告一律走 stderr,且 MCP 不调用 CLI 的 `print_analyze_report`。 ### 程序化 API 示例 diff --git a/docs/plans/2026-09-20-issue-171-mcp-lifecycle-tools.md b/docs/plans/2026-09-20-issue-171-mcp-lifecycle-tools.md index d27a504..7fe929c 100644 --- a/docs/plans/2026-09-20-issue-171-mcp-lifecycle-tools.md +++ b/docs/plans/2026-09-20-issue-171-mcp-lifecycle-tools.md @@ -35,8 +35,15 @@ - 未初始化:`--project` 参数指向的目录(canonicalize 后)。 所有写操作(`codeweb.toml`、`.codeweb/`、store、manifest、`parse.log`)都必须落在该目录树内。 -`confine_to_root(root, candidate)` 做词法归一化(处理 `.` / `..`)后校验 `starts_with(root)`,防止 -`store.path = "../../escape.bincode"` 这类配置把写操作带出许可目录。读路径不校验。 +`confine_to_root(root, candidate)` 做两层校验: + +1. 词法归一化(处理 `.` / `..`)后校验 `starts_with(root)`,拦住 `store.path = "../../escape.bincode"` + 以及尚未存在的路径; +2. 对**最深已存在祖先**做 `canonicalize` 后再比较,拦住 `.codeweb` 是指向目录外符号链接的情况 + (纯词法检查看不见符号链接)。比较双方都取 canonical 形式,因此经过符号链接到达同一目录的 + 合法路径(如 macOS 上经 `/tmp`)不会被误拒。 + +读路径不校验。 ### 2. 状态改为可变,查询走快照 @@ -87,9 +94,11 @@ MCP 的 stdout 是 JSON-RPC 通道。analyze 进度条(indicatif)与报告 | 6 | `codeweb_diff` 返回变更文件分类 | 集成 | | 7 | `store.path` 逃逸许可目录时 analyze 返回错误且不写盘 | 集成 | | 8 | 已分析且无变更的项目调用 analyze 报 `is_up_to_date:true` 且给出真实 nodes/edges(而非 up-to-date 短路返回的 0) | 集成 | +| 9 | `.codeweb` 为指向目录外的符号链接时 analyze 拒绝且目标目录无写入 | 单元 + 集成 | 循环 7 的 Red 通过临时禁用 `confine_to_root` 验证:无守卫时 analyze 返回 `ready` 并在服务目录外写出文件。 循环 8 的 Red 通过临时改用 `report.nodes/edges` 验证:此时报告为 `0 nodes`,测试失败。 +循环 9 的 Red 是真实缺口:加固前 `confine_rejects_symlinked_subdir_escaping_root` 直接失败(词法检查通过)。 测试权限:`test_mcp_tools_list` 的期望工具数由 8 变 11 是本次 feature 的必然结果, 更新时保持「精确集合」断言而非放宽为子集断言,并在提交信息中说明。 @@ -123,6 +132,7 @@ cargo fmt --all -- --check - 分支:`feat/issue-171-mcp-lifecycle-tools` - PR:https://github.com/c2j/codeweb/pull/172 - 门禁结果:`cargo build --features full` 通过;`cargo test --features full -- --skip test_path_mapping_applied --skip test_serve_` 全绿(`mcp_test` 13 passed);`cargo clippy --features full -- -D warnings` 干净;`cargo fmt --all -- --check` 干净;GitHub CI(Lint / Test ubuntu full)通过。 +- 后续加固(PR #174):`confine_to_root` 增加 canonicalize 祖先校验,堵住符号链接逃逸;新增 corrupt store 自愈、请求流水线、`--project` 指向不存在目录、`init_at` 路径分支等验证用例。 - 已知遗留:默认(非 mcp)构建下 `node_sub_type_tag`、`TreeNode::has_more/more_count` 报 dead_code,为既有 mcp-gated 代码,与本次改动无关。 - `tests/mcp_test.rs::test_mcp_tools_list` 期望工具集 8 → 11 为 feature 必然结果,保持精确集合断言。 diff --git a/src/mcp/tools.rs b/src/mcp/tools.rs index 8064be2..3b2b13c 100644 --- a/src/mcp/tools.rs +++ b/src/mcp/tools.rs @@ -1068,11 +1068,24 @@ pub(super) fn normalize_lexically(path: &Path) -> std::path::PathBuf { out } +/// Canonicalize `path`, or if it does not exist yet, its deepest existing ancestor. +fn canonicalize_existing_ancestor(path: &Path) -> Option { + let mut current = path; + loop { + if let Ok(canonical) = std::fs::canonicalize(current) { + return Some(canonical); + } + current = current.parent()?; + } +} + /// Resolve `candidate` against `root` and accept it only if it stays inside. /// -/// `root` is expected to be absolute (the MCP server canonicalizes it at -/// startup). Resolution is lexical, so a symlink *inside* the root pointing -/// outside is not caught here. +/// `root` is expected to be absolute (the MCP server absolutizes and normalizes +/// it at startup). Two checks run: a lexical one that catches `..` escapes even +/// for paths that do not exist yet, and a symlink-aware one that canonicalizes +/// the deepest existing ancestor of the target (so `.codeweb` being a symlink to +/// a directory outside the root is rejected). fn confine_to_root(root: &Path, candidate: &Path) -> Result { let root_norm = normalize_lexically(root); let joined = if candidate.is_absolute() { @@ -1082,15 +1095,31 @@ fn confine_to_root(root: &Path, candidate: &Path) -> Result 0, + "analyze must rebuild a corrupted store, got: {analyzed}" + ); + + mcp.send( + r#"{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"codeweb_stats","arguments":{}}}"#, + ); + let resp = mcp.recv_response(4); + let text = resp["result"]["content"][0]["text"] + .as_str() + .expect("stats text"); + let healed: serde_json::Value = serde_json::from_str(text).expect("stats JSON"); + assert_eq!(healed["status"], "ready", "got: {healed}"); + } + + #[test] + fn test_mcp_handles_pipelined_requests() { + // Requests are written back-to-back without waiting. Two analyzes in a + // row plus a read must all answer (no deadlock between the project mutex + // and the graph lock) and agree on the final state. + let (_tmpdir, project) = create_analyzed_project(); + let mut mcp = McpChild::start(&project); + handshake(&mut mcp); + + mcp.send( + r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"codeweb_analyze","arguments":{}}}"#, + ); + mcp.send( + r#"{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"codeweb_analyze","arguments":{}}}"#, + ); + mcp.send( + r#"{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"codeweb_stats","arguments":{}}}"#, + ); + + let mut seen = std::collections::BTreeMap::new(); + // Notifications may be interleaved; keep reading until all three ids land. + for _ in 0..12 { + if seen.len() == 3 { + break; + } + let line = mcp + .read_line() + .expect("stdout closed while handling pipelined requests"); + let json: serde_json::Value = serde_json::from_str(&line).expect("valid JSON"); + if let Some(id) = json["id"].as_i64() { + seen.insert(id, json); + } + } + + for id in [2, 3, 4] { + let resp = seen + .get(&id) + .unwrap_or_else(|| panic!("no response for id {id}: {seen:?}")); + let text = resp["result"]["content"][0]["text"] + .as_str() + .expect("tool response text"); + let body: serde_json::Value = serde_json::from_str(text).expect("tool response JSON"); + assert_eq!(body["status"], "ready", "id {id} returned: {body}"); + assert!( + body["edges"].as_u64().unwrap_or(0) > 0, + "id {id} returned an empty graph: {body}" + ); + } + } + + #[test] + fn test_mcp_init_creates_missing_project_directory() { + // `--project` may point at a directory that does not exist yet. + let tmpdir = TempDir::new().expect("failed to create temp dir"); + let project = tmpdir.path().join("new").join("nested"); + assert!(!project.exists()); + + let mut mcp = McpChild::start(&project); + handshake(&mut mcp); + + mcp.send( + r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"codeweb_init","arguments":{"name":"fresh","paths":["."]}}}"#, + ); + let resp = mcp.recv_response(2); + let text = resp["result"]["content"][0]["text"] + .as_str() + .expect("init text"); + let init: serde_json::Value = serde_json::from_str(text).expect("init JSON"); + + assert_eq!(init["status"], "initialized", "got: {init}"); + assert!( + project.join("codeweb.toml").exists(), + "init must create the served directory and its config" + ); + } + + #[test] + fn test_mcp_analyze_rejects_symlinked_store_dir() { + // Lexical path checks cannot see symlinks: `.codeweb` looks like it is + // inside the project, but it points outside. The write guard must still + // refuse, and nothing may be written into the symlink target. + let tmpdir = TempDir::new().expect("failed to create temp dir"); + let project = tmpdir.path().join("proj"); + let outside = tmpdir.path().join("outside"); + std::fs::create_dir_all(&project).expect("create project dir"); + std::fs::create_dir_all(&outside).expect("create outside dir"); + copy_serve_demo_fixture(&project); + + let toml = "[project]\n\ + name = \"symlink\"\n\ + \n\ + [analysis]\n\ + paths = [\"sql\"]\n\ + \n\ + [store]\n\ + path = \".codeweb/store.bincode\"\n\ + format = \"bincode\"\n"; + std::fs::write(project.join("codeweb.toml"), toml).expect("write codeweb.toml"); + std::os::unix::fs::symlink(&outside, project.join(".codeweb")).expect("symlink .codeweb"); + + let mut mcp = McpChild::start(&project); + handshake(&mut mcp); + + mcp.send( + r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"codeweb_analyze","arguments":{}}}"#, + ); + let resp = mcp.recv_response(2); + let text = resp["result"]["content"][0]["text"] + .as_str() + .expect("analyze text"); + let result: serde_json::Value = serde_json::from_str(text).expect("analyze JSON"); + + assert_eq!( + result["status"], "error", + "a symlinked store directory must be refused, got: {result}" + ); + assert!( + result["error"] + .as_str() + .is_some_and(|e| e.contains("outside")), + "the error should say the path resolves outside the root, got: {result}" + ); + let leaked: Vec = std::fs::read_dir(&outside) + .expect("read outside dir") + .filter_map(|e| e.ok().map(|e| e.file_name().to_string_lossy().to_string())) + .collect(); + assert!( + leaked.is_empty(), + "nothing may be written through the symlink, found: {leaked:?}" + ); + } + #[test] fn test_mcp_call_stats() { let (_tmpdir, project) = create_test_project();