fix: the server's own folders under a short-named temp path are expanded, not refused (#1055)
On Windows the temporary folder often has an 8.3 short name, which the path scope refuses. vfs::long_names spells folders SolidCraft derives from the environment with long names, and the MCP scratch folder and the recovery folder use it, so screenshot without a path and the recovery commands work there again.
This commit is contained in:
1 parent
cf93869c4d
commit
08b6a8fe0f
10 files changed
+165
-16
No files matched your search
@@ -19,7 +19,7 @@ pub use tools::{call_tool, tool_definitions};
|
||||
|
||||
use std::path::PathBuf;
|
||||
|
||||
use solidcraft_engine::io::vfs::PathScope;
|
||||
use solidcraft_engine::io::vfs::{self, PathScope};
|
||||
|
||||
/// The folders `solidcraft-cli mcp` lets its agent read, write and delete in by default: the
|
||||
/// app's own recovery folder, this server's private scratch folder (screenshots), and each
|
||||
@@ -48,7 +48,8 @@ pub fn agent_scope_with(recovery: Option<PathBuf>, allowed: impl IntoIterator<It
|
||||
|
||||
/// This server's own folder for screenshots it reads back and deletes (`screenshot` without a
|
||||
/// `path`): a new `<temp>/solidcraft-mcp-<pid>-<n>` made on first use (never one that was there
|
||||
/// already, which could be someone else's), private to the user on Unix.
|
||||
/// already, which could be someone else's), private to the user on Unix. `<temp>` is spelled with
|
||||
/// long names (`vfs::temp_dir`), since the scope refuses short (8.3) ones.
|
||||
pub fn scratch_dir() -> Option<PathBuf> {
|
||||
static DIR: std::sync::OnceLock<Option<PathBuf>> = std::sync::OnceLock::new();
|
||||
DIR.get_or_init(|| {
|
||||
@@ -57,7 +58,7 @@ pub fn scratch_dir() -> Option<PathBuf> {
|
||||
std::os::unix::fs::DirBuilderExt::mode(&mut b, 0o700);
|
||||
let seed = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.subsec_nanos()).unwrap_or(0);
|
||||
(0..16u32).find_map(|i| {
|
||||
let d = std::env::temp_dir().join(format!("solidcraft-mcp-{}-{:x}", std::process::id(), seed.wrapping_add(i)));
|
||||
let d = vfs::temp_dir().join(format!("solidcraft-mcp-{}-{:x}", std::process::id(), seed.wrapping_add(i)));
|
||||
b.create(&d).ok().map(|()| d)
|
||||
})
|
||||
})
|
||||
|
||||
+66
-2
@@ -462,7 +462,7 @@ fn legacy_design_files_and_resource_uris() {
|
||||
/// design is open there, without its hidden files. The working folder doesn't count at all.
|
||||
#[test]
|
||||
fn the_default_folders_are_the_designs_the_recovery_folder_and_the_scratch_folder() {
|
||||
let base = std::env::temp_dir().join(format!("solidcraft-mcp-defaults-{}", std::process::id()));
|
||||
let base = solidcraft_engine::io::vfs::temp_dir().join(format!("solidcraft-mcp-defaults-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
let (proj, other, recovery) = (base.join("proj"), base.join("other"), base.join("recovery"));
|
||||
std::fs::create_dir_all(&proj).unwrap();
|
||||
@@ -508,7 +508,7 @@ fn the_default_folders_are_the_designs_the_recovery_folder_and_the_scratch_folde
|
||||
#[test]
|
||||
fn tool_calls_stay_inside_the_agents_folders() {
|
||||
use solidcraft_engine::io::vfs::PathScope;
|
||||
let base = std::env::temp_dir().join(format!("solidcraft-mcp-scope-{}", std::process::id()));
|
||||
let base = solidcraft_engine::io::vfs::temp_dir().join(format!("solidcraft-mcp-scope-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
let (root, out) = (base.join("root"), base.join("out"));
|
||||
std::fs::create_dir_all(&root).unwrap();
|
||||
@@ -548,6 +548,70 @@ fn tool_calls_stay_inside_the_agents_folders() {
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
/// The short (8.3) spelling of an existing `path` as `cmd` gives it (`NAME~1.XYZ`), or `None`
|
||||
/// when it has no short name (the volume makes none).
|
||||
#[cfg(windows)]
|
||||
fn short_spelling(path: &std::path::Path) -> Option<std::path::PathBuf> {
|
||||
use std::os::windows::process::CommandExt;
|
||||
let out = std::process::Command::new("cmd").raw_arg(format!("/d /c for %I in (\"{}\") do @echo %~sI", path.display())).output().ok()?;
|
||||
Some(String::from_utf8_lossy(&out.stdout).trim().to_string())
|
||||
.filter(|s| solidcraft_engine::io::vfs::windows_refusal(s).is_some_and(|why| why.contains("8.3")))
|
||||
.map(Into::into)
|
||||
}
|
||||
|
||||
/// On Windows the temporary folder's path often has a short (8.3) name (the user folder's
|
||||
/// `NAME~1.XYZ` when its name is long). The server's own folders are spelled with long names
|
||||
/// there, so `screenshot` without a `path` (the scratch folder) and `doc.recovery_list` (the
|
||||
/// recovery folder) work; on such a machine both were refused as short names. Run in a child
|
||||
/// process whose `TMP`, `TEMP` and `SOLIDCRAFT_RECOVERY_DIR` are short spellings.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn the_servers_own_folders_work_when_the_temporary_folder_has_a_short_name() {
|
||||
let base = solidcraft_engine::io::vfs::temp_dir().join(format!("solidcraft-mcp-short-names-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
let (temp, recovery) = (base.join("temporary"), base.join("recovery folder"));
|
||||
std::fs::create_dir_all(&temp).unwrap();
|
||||
std::fs::create_dir_all(&recovery).unwrap();
|
||||
let (Some(short_temp), Some(short_recovery)) = (short_spelling(&temp), short_spelling(&recovery)) else {
|
||||
eprintln!("skipped: {} has no short (8.3) name on this volume", base.display());
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
return;
|
||||
};
|
||||
let out = std::process::Command::new(std::env::current_exe().unwrap())
|
||||
.args(["--exact", "tests::short_names_child", "--test-threads=1", "--nocapture"])
|
||||
.env("TMP", &short_temp)
|
||||
.env("TEMP", &short_temp)
|
||||
.env("SOLIDCRAFT_RECOVERY_DIR", &short_recovery)
|
||||
.env("SOLIDCRAFT_MCP_SHORT_NAMES_CHILD", "1")
|
||||
.output()
|
||||
.unwrap();
|
||||
let text = format!("{}{}", String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr));
|
||||
assert!(out.status.success() && text.contains("short names: ok"), "{text}");
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
/// The child process of the test above; does nothing on its own.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn short_names_child() {
|
||||
if std::env::var_os("SOLIDCRAFT_MCP_SHORT_NAMES_CHILD").is_none() {
|
||||
return;
|
||||
}
|
||||
use solidcraft_engine::io::vfs::windows_refusal;
|
||||
let temp = std::env::temp_dir();
|
||||
assert!(windows_refusal(&temp.to_string_lossy()).is_some_and(|why| why.contains("8.3")), "{}", temp.display());
|
||||
let mut s = Server::new(Box::new(Headless::default())).with_path_scope(Some(crate::agent_scope([])));
|
||||
tool(&mut s, "execute", json!({"command": "solid.box", "params": {"length": 10, "width": 10, "height": 10}}));
|
||||
tool(&mut s, "screenshot", json!({"width": 32, "height": 32}));
|
||||
tool(&mut s, "execute", json!({"command": "doc.recovery_list", "params": {}}));
|
||||
// An agent's own short spelling of the scratch folder stays refused.
|
||||
let scratch = crate::scratch_dir().unwrap();
|
||||
let short = std::env::temp_dir().join(scratch.file_name().unwrap()).join("shot.png");
|
||||
let r = raw_tool(&mut s, "screenshot", json!({"path": short.to_string_lossy(), "width": 32, "height": 32}));
|
||||
assert!(r.to_string().contains("short (8.3) names"), "{r}");
|
||||
println!("short names: ok");
|
||||
}
|
||||
|
||||
/// A bridged server sends the agent's folders with every request, so the app confines the
|
||||
/// request's files the same way (`allow_paths`, crates/ui-egui/src/control.rs).
|
||||
#[test]
|
||||
|
||||
@@ -589,7 +589,7 @@ fn capture(b: &mut dyn Backend, a: &Map<String, Value>) -> Result<(Vec<u8>, Valu
|
||||
let keep = str_arg(a, "path").filter(|p| !p.trim().is_empty()).map(str::to_string);
|
||||
let path = keep.clone().unwrap_or_else(|| {
|
||||
let n = SHOT.fetch_add(1, Ordering::Relaxed);
|
||||
let dir = crate::scratch_dir().unwrap_or_else(std::env::temp_dir);
|
||||
let dir = crate::scratch_dir().unwrap_or_else(vfs::temp_dir);
|
||||
dir.join(format!("shot-{n}.png")).to_string_lossy().to_string()
|
||||
});
|
||||
let window = match str_arg(a, "source") {
|
||||
|
||||
Reference in new issue
Block a user