diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 97a8566bac..ea0ea51eec 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -29,7 +29,7 @@ const COMMAND_OUTPUT_LIMIT: u64 = 16 * 1024; /// /// The output results will be shown to the user already, only list it again if necessary, avoid being redundant. /// -/// Make sure you use the `cd` parameter to navigate to one of the root directories of the project. NEVER do it as part of the `command` itself, otherwise it will error. +/// Always set the working directory with the `cd` parameter, never with `cd` inside `command`; otherwise it will error. /// /// Do not generate terminal commands that use shell substitutions or interpolations such as `$VAR`, `${VAR}`, `$(...)`, backticks, `$((...))`, `<(...)`, or `>(...)`. Resolve those values yourself before calling this tool, or ask the user for the literal value to use. /// @@ -53,7 +53,7 @@ pub struct TerminalToolInput { /// /// REMINDER: read-only git commands (`git log`, `git diff`, `git show`, `git blame`) MUST include `--no-pager` (e.g. `git --no-pager log`). Prefer `git --no-optional-locks status` over `git status` to avoid optional metadata writes. Git commands that may open an editor (`git rebase`, `git commit`, `git merge`, `git tag`) MUST be prefixed with `GIT_EDITOR=true ` (e.g. `GIT_EDITOR=true git rebase origin/main`). Otherwise the terminal will hang. pub command: String, - /// Working directory for the command. This must be one of the root directories of the project. + /// Working directory: a project root directory or any subdirectory of one, given by name or absolute path. E.g. `my-project/src`, `/home/user/my-project`, or on Windows `my-project\src` or `C:\Users\me\my-project`. pub cd: String, /// Optional maximum runtime (in milliseconds). If exceeded, the running terminal task is killed. pub timeout_ms: Option, @@ -71,7 +71,7 @@ pub struct TerminalToolInput { /// /// The output results will be shown to the user already, only list it again if necessary, avoid being redundant. /// -/// Make sure you use the `cd` parameter to navigate to one of the root directories of the project. NEVER do it as part of the `command` itself, otherwise it will error. +/// Always set the working directory with the `cd` parameter, never with `cd` inside `command`; otherwise it will error. /// /// Do not generate terminal commands that use shell substitutions or interpolations such as `$VAR`, `${VAR}`, `$(...)`, backticks, `$((...))`, `<(...)`, or `>(...)`. Resolve those values first or ask the user for the literal value to use. /// @@ -95,7 +95,7 @@ pub struct SandboxedTerminalToolInput { /// /// REMINDER: read-only git commands (`git log`, `git diff`, `git show`, `git blame`) MUST include `--no-pager` (e.g. `git --no-pager log`). Prefer `git --no-optional-locks status` over `git status` to avoid optional metadata writes. Git commands that may open an editor (`git rebase`, `git commit`, `git merge`, `git tag`) MUST be prefixed with `GIT_EDITOR=true ` (e.g. `GIT_EDITOR=true git rebase origin/main`). Otherwise the terminal will hang. pub command: String, - /// Working directory for the command. This must be one of the root directories of the project. + /// Working directory: a project root directory or any subdirectory of one, given by name or absolute path. E.g. `my-project/src`, `/home/user/my-project`, or on Windows `my-project\src` or `C:\Users\me\my-project`. pub cd: String, /// Optional maximum runtime (in milliseconds). If exceeded, the running terminal task is killed. pub timeout_ms: Option, @@ -1355,27 +1355,189 @@ fn working_dir(cd: &str, project: &Entity, cx: &mut App) -> Result Ok(None), } } else { - let input_path = Path::new(cd); + let path_style = project.path_style(cx); + let worktree_roots = project + .worktrees(cx) + .filter_map(|worktree| { + let worktree = worktree.read(cx); + // Skip single-file worktrees: a file can't be a working directory. + let root_dir = worktree.root_dir()?; + Some((worktree.root_name_str(), root_dir.to_path_buf())) + }) + .collect::>(); - if input_path.is_absolute() { - if project - .worktrees(cx) - .any(|worktree| input_path.starts_with(&worktree.read(cx).abs_path())) - { - return Ok(Some(input_path.into())); - } - } else if let Some(worktree) = project.worktree_for_root_name(cd, cx) { - return Ok(Some(worktree.read(cx).abs_path().to_path_buf())); + if let Some(dir) = resolve_cd_in_worktrees(cd, path_style, &worktree_roots) { + return Ok(Some(dir)); } - anyhow::bail!("`cd` directory {cd:?} was not in any of the project's worktrees."); + anyhow::bail!("`cd` directory {cd:?} was not in any root directory in the project."); } } +/// Resolves a `cd` argument to an absolute worktree directory. `cd` may be a +/// worktree's root name or an absolute path to a worktree or a subdirectory +/// therein. +/// +/// Absolute paths are classified with the project's [`PathStyle`] rather than +/// the host's, so an absolute POSIX path resolves correctly on a Windows host +/// driving a WSL/SSH project (#60040). +/// +/// Both `cd` and the worktree roots are lexically normalized before prefix +/// matching. This resolves `.` and `..` components up front, so a path that +/// escapes a worktree does not have that worktree's root as a prefix and is +/// rejected (#60014). On Windows-style projects it also unifies `/` and `\` +/// separators, since models frequently write `C:/foo/bar` for a root stored +/// as `C:\foo\bar`. +fn resolve_cd_in_worktrees( + cd: &str, + path_style: util::paths::PathStyle, + worktree_roots: &[(&str, PathBuf)], +) -> Option { + let cd = path_style.normalize(cd); + let cd_path = Path::new(&cd); + let is_absolute = path_style.is_absolute(&cd); + + worktree_roots.iter().find_map(|(root_name, abs_path)| { + let prefix = if is_absolute { + path_style.normalize(abs_path.to_str()?) + } else { + (*root_name).to_string() + }; + let subpath = path_style.strip_prefix(cd_path, Path::new(&prefix))?; + if subpath.is_empty() { + Some(abs_path.clone()) + } else { + path_style + .join_path(abs_path, &*subpath.display(path_style)) + .ok() + } + }) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn test_resolve_cd_uses_project_path_style() { + use util::paths::PathStyle::{Unix, Windows}; + + // Deliberately ambiguous root names to stress test path resolution. + let unix_roots: Vec<(&str, PathBuf)> = vec![ + ("worktree", PathBuf::from("/a/worktree")), + ("worktree", PathBuf::from("/b/worktree")), + ]; + // Worktree roots are stored with backslash separators on Windows, but + // models frequently write paths with forward slashes; both must match. + let windows_roots = vec![("worktree", PathBuf::from("C:\\work\\worktree"))]; + + // absolute paths + assert_eq!( + resolve_cd_in_worktrees("/b/worktree", Unix, &unix_roots), + Some(PathBuf::from("/b/worktree")), + "a POSIX-absolute path resolves under a POSIX project path style even on a Windows host" + ); + assert_eq!( + resolve_cd_in_worktrees("/a/worktree/src", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree/src")), + "an absolute path inside a worktree resolves to the same path" + ); + assert_eq!( + resolve_cd_in_worktrees("/elsewhere", Unix, &unix_roots), + None, + "an absolute path outside every worktree is rejected" + ); + assert_eq!( + resolve_cd_in_worktrees("/a/worktree/src/../docs", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree/docs")), + "an absolute path that stays within its worktree via `..` resolves to the same path" + ); + assert_eq!( + resolve_cd_in_worktrees("/a/worktree/../escape", Unix, &unix_roots), + None, + "an absolute path that escapes its worktree via `..` is rejected" + ); + assert_eq!( + resolve_cd_in_worktrees("/a/worktree/../../b/worktree", Unix, &unix_roots), + Some(PathBuf::from("/b/worktree")), + "a path whose `..` components lexically resolve into a valid worktree is accepted" + ); + assert_eq!( + resolve_cd_in_worktrees("/a/worktree//src/", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree/src")), + "doubled and trailing separators are normalized away" + ); + + // relative root names + assert_eq!( + resolve_cd_in_worktrees("worktree", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree")), + "a root-relative path to a worktree root resolves to the first matching worktree" + ); + assert_eq!( + resolve_cd_in_worktrees("worktree/src", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree/src")), + "a root-relative path to a subdirectory resolves to the absolute path" + ); + assert_eq!( + resolve_cd_in_worktrees("worktree/src/../doc", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree/doc")), + "a root-relative path to a subdirectory with `..` resolves to a clean absolute path" + ); + assert_eq!( + resolve_cd_in_worktrees("worktree/../escape", Unix, &unix_roots), + None, + "a root-relative path that escapes the worktree via `..` is rejected" + ); + assert_eq!( + resolve_cd_in_worktrees("worktreeextra", Unix, &unix_roots), + None, + "a root-relative path that is not any of the worktree roots is rejected" + ); + assert_eq!( + resolve_cd_in_worktrees("./worktree", Unix, &unix_roots), + Some(PathBuf::from("/a/worktree")), + "a leading `./` is normalized away" + ); + + // Windows paths + assert_eq!( + resolve_cd_in_worktrees("C:\\work\\worktree", Windows, &windows_roots), + Some(PathBuf::from("C:\\work\\worktree")), + "Windows-absolute paths to root directories resolve" + ); + assert_eq!( + resolve_cd_in_worktrees("C:/work/worktree", Windows, &windows_roots), + Some(PathBuf::from("C:\\work\\worktree")), + "forward-slash separators match a backslash-stored root" + ); + assert_eq!( + resolve_cd_in_worktrees("c:\\work\\worktree", Windows, &windows_roots), + Some(PathBuf::from("C:\\work\\worktree")), + "drive letters match case-insensitively" + ); + assert_eq!( + resolve_cd_in_worktrees("C:/work/worktree/src", Windows, &windows_roots), + Some(PathBuf::from("C:\\work\\worktree\\src")), + "Windows-absolute paths to subdirectories resolve regardless of separator style" + ); + assert_eq!( + resolve_cd_in_worktrees("worktree\\src", Windows, &windows_roots), + Some(PathBuf::from("C:\\work\\worktree\\src")), + "Windows-relative paths to subdirectories resolve to the absolute path" + ); + assert_eq!( + resolve_cd_in_worktrees("worktree/src", Windows, &windows_roots), + Some(PathBuf::from("C:\\work\\worktree\\src")), + "forward-slash relative paths resolve under a Windows path style" + ); + assert_eq!( + resolve_cd_in_worktrees("C:\\work\\worktree\\..\\escape", Windows, &windows_roots), + None, + "a Windows-absolute path that escapes its worktree via `..` is rejected" + ); + } + #[test] fn test_initial_title_shows_full_multiline_command() { let input = TerminalToolInput {