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`,