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.
This commit is contained in:
Lasse Lian authored and GitHub committed 2026-10-14 14:09:05 -04:00
1 parent e489e56f3d
commit 4cfa9c5d34
2 files changed
+52 -6

No files matched your search

+51 -5
View File
@@ -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<Error> {
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);
+1 -1
View File
@@ -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`,