From 4adf3c80ae5f66fd37ae5004e1a8e4e92ff379ca Mon Sep 17 00:00:00 2001 From: Cedric Stephan Date: Fri, 24 Jul 2026 17:30:58 +0200 Subject: [PATCH] fix(mcp): reroot markerless sessions through extra roots --- rust/src/tools/mod.rs | 41 +++++++++++ rust/src/tools/server_paths.rs | 126 +++++++++++++++++++-------------- 2 files changed, 112 insertions(+), 55 deletions(-) diff --git a/rust/src/tools/mod.rs b/rust/src/tools/mod.rs index 56ac20a067..ac0ddf6aee 100644 --- a/rust/src/tools/mod.rs +++ b/rust/src/tools/mod.rs @@ -326,6 +326,47 @@ mod resolve_path_tests { assert_eq!(session.project_root.as_deref(), Some(real_root.as_str())); } + #[cfg(not(feature = "no-jail"))] + #[tokio::test] + #[allow(clippy::await_holding_lock)] + async fn resolve_path_auto_reroots_even_when_extra_root_allows_path() { + let _iso = crate::core::data_dir::isolated_data_dir(); + crate::test_env::remove_var("LEAN_CTX_ALLOW_REROOT"); + let tmp = tempfile::tempdir().unwrap(); + let client_cwd = tmp.path().join("Users").join("user"); + let real = tmp.path().join("workspaces").join("lean-ctx"); + std::fs::create_dir_all(&client_cwd).unwrap(); + let real_root = create_git_root(&real); + std::fs::write(real.join("rust.rs"), "ok").unwrap(); + + let server = LeanCtxServer::new_with_startup( + None, + None, + SessionMode::Personal, + "default", + "default", + ); + { + let mut session = server.session.write().await; + session.project_root = Some(client_cwd.to_string_lossy().to_string()); + session.shell_cwd = Some(client_cwd.to_string_lossy().to_string()); + session.extra_roots.push(real_root.clone()); + } + + let out = server + .resolve_path(&real.join("rust.rs").to_string_lossy()) + .await + .unwrap(); + assert!( + out.ends_with("/rust.rs"), + "extra root must not suppress markerless-cwd reroot: {out}" + ); + + let session = server.session.read().await; + assert_eq!(session.project_root.as_deref(), Some(real_root.as_str())); + assert_eq!(session.shell_cwd.as_deref(), Some(real_root.as_str())); + } + #[cfg(not(feature = "no-jail"))] #[tokio::test] #[allow(clippy::await_holding_lock)] diff --git a/rust/src/tools/server_paths.rs b/rust/src/tools/server_paths.rs index f4e9d01054..2e0309c9be 100644 --- a/rust/src/tools/server_paths.rs +++ b/rust/src/tools/server_paths.rs @@ -88,7 +88,10 @@ impl LeanCtxServer { jail_root_path, &extra_roots, ) { - Ok(p) => p, + Ok(p) => self + .maybe_reroot_for_absolute_path(&resolved, jail_root_path, &extra_roots, false) + .await? + .unwrap_or(p), Err(e) => { // #899: the rejected path is dependency source in a language // cache (Go module cache, cargo registry, site-packages, @@ -106,60 +109,11 @@ impl LeanCtxServer { cache_root.display() )); } - if p.is_absolute() { - if let Some(new_root) = maybe_derive_project_root_from_absolute(&resolved) { - let cfg_allow = std::env::var("LEAN_CTX_ALLOW_REROOT").map_or_else( - |_| crate::core::config::Config::load().allow_auto_reroot, - |v| v == "1" || v == "true", - ); - let candidate_under_jail = resolved.starts_with(jail_root_path); - // #580/#649: when the MCP server was launched from an - // agent/IDE config dir (e.g. ~/.copilot) or a markerless - // client cwd (e.g. WSL VS Code starting in /mnt/c/Users), - // that jail is not a real project boundary. The derived - // root already carries a project marker, so correcting to - // it is a root fix, not a jail weakening. Real project - // roots and trusted startup roots still keep the - // conservative gate. - let allow_reroot = if candidate_under_jail { - false - } else if is_suspicious_root(jail_root_path) - || (self.startup_project_root.is_none() - && !has_project_marker(jail_root_path)) - { - true - } else if !cfg_allow { - false - } else if let Some(ref trusted_root) = self.startup_project_root { - std::path::Path::new(trusted_root) == new_root.as_path() - } else { - !has_project_marker(jail_root_path) - }; - - if allow_reroot { - let mut session = self.session.write().await; - let new_root_str = new_root.to_string_lossy().to_string(); - session.project_root = Some(new_root_str.clone()); - session.shell_cwd = self - .startup_shell_cwd - .as_ref() - .filter(|cwd| std::path::Path::new(cwd).starts_with(&new_root)) - .cloned() - .or_else(|| Some(new_root_str.clone())); - let _ = session.save(); - - crate::core::pathjail::jail_path_with_roots( - &resolved, - &new_root, - &extra_roots, - ) - .map_err(|e| e.to_string())? - } else { - return Err(e.to_string()); - } - } else { - return Err(e.to_string()); - } + if let Some(jailed) = self + .maybe_reroot_for_absolute_path(&resolved, jail_root_path, &extra_roots, true) + .await? + { + jailed } else { return Err(e.to_string()); } @@ -173,6 +127,68 @@ impl LeanCtxServer { )) } + async fn maybe_reroot_for_absolute_path( + &self, + resolved: &std::path::Path, + jail_root_path: &std::path::Path, + extra_roots: &[String], + require_opt_in_for_real_jail: bool, + ) -> Result, String> { + if !resolved.is_absolute() { + return Ok(None); + } + let Some(new_root) = maybe_derive_project_root_from_absolute(resolved) else { + return Ok(None); + }; + let candidate_under_jail = resolved.starts_with(jail_root_path); + // #580/#649: when the MCP server was launched from an agent/IDE config + // dir (e.g. ~/.copilot) or a markerless client cwd (e.g. WSL VS Code + // starting in /mnt/c/Users), that jail is not a real project boundary. + // The derived root already carries a project marker, so correcting to it + // is a root fix, not a jail weakening. + let allow_reroot = if candidate_under_jail { + false + } else if is_suspicious_root(jail_root_path) + || (self.startup_project_root.is_none() && !has_project_marker(jail_root_path)) + { + true + } else if require_opt_in_for_real_jail { + let cfg_allow = std::env::var("LEAN_CTX_ALLOW_REROOT").map_or_else( + |_| crate::core::config::Config::load().allow_auto_reroot, + |v| v == "1" || v == "true", + ); + cfg_allow + && self + .startup_project_root + .as_ref() + .is_some_and(|trusted_root| std::path::Path::new(trusted_root) == new_root) + } else { + false + }; + + if !allow_reroot { + return Ok(None); + } + + self.reroot_to_project(&new_root).await; + crate::core::pathjail::jail_path_with_roots(resolved, &new_root, extra_roots) + .map(Some) + .map_err(|e| e.to_string()) + } + + async fn reroot_to_project(&self, new_root: &std::path::Path) { + let mut session = self.session.write().await; + let new_root_str = new_root.to_string_lossy().to_string(); + session.project_root = Some(new_root_str.clone()); + session.shell_cwd = self + .startup_shell_cwd + .as_ref() + .filter(|cwd| std::path::Path::new(cwd).starts_with(new_root)) + .cloned() + .or_else(|| Some(new_root_str.clone())); + let _ = session.save(); + } + /// Like `resolve_path`, but returns the original path on failure instead of an error. pub async fn resolve_path_or_passthrough(&self, path: &str) -> String { self.resolve_path(path)