From 4cfa9c5d34c91be93fb6348e0d6280e33d152eef Mon Sep 17 00:00:00 2001 From: Lasse Lian Date: Wed, 14 Oct 2026 20:09:05 +0200 Subject: [PATCH] MCP: a path into a missing folder inside an allowed one says the folder is missing (#1082) PathScope::check reports a missing folder inside the scope as NotFound with the nearest existing folder, instead of calling it outside the allowed folders. Paths outside the scope and dangling links are still refused. Fixes #1077. --- crates/io/src/vfs.rs | 56 ++++++++++++++++++++++++++++++++++++++++---- docs/mcp.md | 2 +- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/crates/io/src/vfs.rs b/crates/io/src/vfs.rs index 9e6b305d..5749f2af 100644 --- a/crates/io/src/vfs.rs +++ b/crates/io/src/vfs.rs @@ -232,15 +232,52 @@ impl PathScope { { return Err(windows(why)); } - let resolved = resolve(std::path::Path::new(path)).map_err(|_| refused())?; + let resolved = match resolve(std::path::Path::new(path)) { + Ok(r) => r, + Err(e) if e.kind() == ErrorKind::NotFound => return Err(self.missing_folder(path).unwrap_or_else(refused)), + Err(_) => return Err(refused()), + }; // Judged again once resolved: a link or junction inside an allowed folder may lead to a // network share or a device even though the path given looked local. if cfg!(windows) && !local_drive_path(&resolved) { return Err(windows("only paths on a local drive")); } - let inside_design = - || self.design_roots.iter().any(|r| resolved.strip_prefix(r).is_ok_and(|rel| !rel.components().any(|c| hidden(c.as_os_str())))); - if self.roots.iter().any(|r| resolved.starts_with(r)) || inside_design() { Ok(()) } else { Err(refused()) } + if self.contains(&resolved) { Ok(()) } else { Err(refused()) } + } + + /// Whether the resolved path `p` lies inside one of the folders. + fn contains(&self, p: &std::path::Path) -> bool { + let inside_design = || self.design_roots.iter().any(|r| p.strip_prefix(r).is_ok_and(|rel| !rel.components().any(|c| hidden(c.as_os_str())))); + self.roots.iter().any(|r| p.starts_with(r)) || inside_design() + } + + /// The "folder does not exist" error for a path whose folder is missing, but only when the + /// nearest folder that does exist is one of the scope's own: telling the user which folder is + /// missing must not become a way to probe the disk outside it. + fn missing_folder(&self, path: &str) -> Option { + let given = std::path::Path::new(path); + // A dangling link (the path itself, or in its folders) is not a missing folder: it leads + // somewhere, and that is refused. + let mut missing = given.parent()?; + let existing = loop { + let here = if missing.as_os_str().is_empty() { std::path::Path::new(".") } else { missing }; + if let Ok(c) = std::fs::canonicalize(here) { + break c; + } + if std::fs::symlink_metadata(here).is_ok() { + return None; + } + missing = missing.parent()?; + }; + if std::fs::symlink_metadata(given).is_ok() { + return None; + } + self.contains(&existing).then(|| { + Error::new( + ErrorKind::NotFound, + format!("{path}: its folder does not exist (nothing exists below {}); create the folder first", existing.display()), + ) + }) } } @@ -476,6 +513,8 @@ mod tests { assert!(super::write(&p, b"x").is_err() && super::remove(&p).is_err() && super::len(&p).is_err() && !super::exists(&p), "{p}"); } assert!(super::write(&s(out.join("new.txt")), b"x").is_err()); + let e = super::check(&s(root.join("dangling/x.txt"))).unwrap_err(); + assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied, "{e}"); assert!(super::check(&s(root.join("sub/../sub/b.txt"))).is_ok(), "`..` that stays inside is fine"); }); assert_eq!(std::fs::read_to_string(out.join("secret.txt")).unwrap(), "secret"); @@ -503,10 +542,17 @@ mod tests { let scope = PathScope::new([a.clone()]); let s = |p: std::path::PathBuf| p.to_string_lossy().to_string(); assert!(scope.check(&s(a.join("new.txt"))).is_ok()); - for refused in [ab.join("x.txt"), a.join("missing/x.txt"), a.join("link/new.txt")] { + for refused in [ab.join("x.txt"), a.join("link/new.txt"), base.join("missing/x.txt")] { let e = scope.check(&s(refused.clone())).unwrap_err(); assert!(e.to_string().contains("outside the folders this agent may use"), "{}: {e}", refused.display()); } + // A folder that is missing inside an allowed one is reported as missing (#1077), not as + // out of scope: widening `--allow-path` would not help. Nothing is created. + for missing in [a.join("missing/x.txt"), a.join("missing/deeper/x.txt")] { + let e = scope.check(&s(missing.clone())).unwrap_err(); + assert_eq!(e.kind(), std::io::ErrorKind::NotFound, "{}: {e}", missing.display()); + assert!(e.to_string().contains("its folder does not exist") && !e.to_string().contains("--allow-path"), "{e}"); + } super::scoped(Some(scope), || assert!(super::write(&s(a.join("link/new.txt")), b"x").is_err())); assert!(!out.join("new.txt").exists() && !a.join("missing").exists()); let _ = std::fs::remove_dir_all(&base); diff --git a/docs/mcp.md b/docs/mcp.md index afd5e4fb..69c272a8 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -49,7 +49,7 @@ start-up and says when the `--in` design's folder was left out; `--allow-path` a Paths are resolved before they are compared, so `..` and symlinks that lead out are refused (saving through a link writes the file it points to, so the link's target is what counts), and -a new file must go into a folder that exists. On Windows a path must be on a local drive +a new file must go into a folder that exists (a missing folder inside an allowed one is reported as missing; it is not created and does not call for a wider `--allow-path`). On Windows a path must be on a local drive (`C:\…` or `\\?\C:\…`): UNC, device and NT paths in any spelling (`\\server\share`, `//server/share`, `\\?\UNC\…`, `\??\…`, `\\.\…`), drive-relative paths (`C:file`), a `:` in a file name (alternate data streams), reserved device names with any extension (`NUL`, `CON`,