From 08b6a8fe0f6808c576e97a44f4c924b41544d357 Mon Sep 17 00:00:00 2001 From: NOWN-DEV <257771410+NOWN-DEV@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:05:43 +0200 Subject: [PATCH] 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. --- crates/engine/src/cmd/motion_tests.rs | 2 +- crates/engine/src/cmd/urdf_tests.rs | 2 +- crates/engine/src/recovery.rs | 9 ++- crates/engine/src/tests/path_scope.rs | 2 +- crates/io/src/vfs.rs | 82 ++++++++++++++++++++++++++- crates/mcp/src/lib.rs | 7 ++- crates/mcp/src/tests.rs | 68 +++++++++++++++++++++- crates/mcp/src/tools.rs | 2 +- crates/ui-egui/src/control.rs | 4 +- docs/mcp.md | 3 + 10 files changed, 165 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/cmd/motion_tests.rs b/crates/engine/src/cmd/motion_tests.rs index 934ceb7c..6ee3215a 100644 --- a/crates/engine/src/cmd/motion_tests.rs +++ b/crates/engine/src/cmd/motion_tests.rs @@ -130,7 +130,7 @@ fn motion_study_plays_and_exports() { // The B occurrence's origin moves by the slider value (it starts at 0 offset with value 0 at 10 mm… so 10 → −5). assert!((bx(&rows[1]) - bx(&rows[0]) + 3.75).abs() < 1e-6, "{e}"); assert!(rows[4]["stopped_by_contact"].is_object()); - let dir = std::env::temp_dir().join(format!("solidcraft-motion-{}", std::process::id())); + let dir = solidcraft_io::vfs::temp_dir().join(format!("solidcraft-motion-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let csv = dir.join("push.csv"); run(&mut s, "motion.export", json!({"study": "Push", "samples": 3, "path": csv.to_string_lossy()})); diff --git a/crates/engine/src/cmd/urdf_tests.rs b/crates/engine/src/cmd/urdf_tests.rs index ff136c53..47e19815 100644 --- a/crates/engine/src/cmd/urdf_tests.rs +++ b/crates/engine/src/cmd/urdf_tests.rs @@ -96,7 +96,7 @@ fn assert_fk(m: &UrdfModel, s: &Session, what: &str) { } fn temp(tag: &str) -> PathBuf { - let d = std::env::temp_dir().join(format!("solidcraft-urdf-{tag}-{}", std::process::id())); + let d = solidcraft_io::vfs::temp_dir().join(format!("solidcraft-urdf-{tag}-{}", std::process::id())); let _ = std::fs::remove_dir_all(&d); let _ = std::fs::create_dir_all(&d); d diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index 1e5c10dd..dcda910e 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -18,13 +18,16 @@ use solidcraft_doc::Document; use crate::{EngineError, Result, Session}; /// The recovery folder: `SOLIDCRAFT_RECOVERY_DIR`, else the platform's per-user data folder. +/// Spelled with long names (`vfs::long_names`): an agent's commands reach it by this path, and +/// short (8.3) names are refused there. pub fn default_dir() -> Option { + use solidcraft_io::vfs::long_names; // Tests never touch the user's real recovery folder. if cfg!(test) { - return Some(std::env::temp_dir().join(format!("solidcraft-test-recovery-{}", std::process::id()))); + return Some(solidcraft_io::vfs::temp_dir().join(format!("solidcraft-test-recovery-{}", std::process::id()))); } if let Some(d) = solidcraft_geom::env::var_os("RECOVERY_DIR").filter(|d| !d.is_empty()) { - return Some(PathBuf::from(d)); + return Some(long_names(Path::new(&d))); } let env = |k: &str| std::env::var_os(k).filter(|d| !d.is_empty()).map(PathBuf::from); let root = if cfg!(windows) { @@ -34,7 +37,7 @@ pub fn default_dir() -> Option { } else { env("XDG_DATA_HOME").or_else(|| env("HOME").map(|h| h.join(".local/share")))? }; - let (dir, legacy) = dirs_under(&root, cfg!(windows) || cfg!(target_os = "macos")); + let (dir, legacy) = dirs_under(&long_names(&root), cfg!(windows) || cfg!(target_os = "macos")); // Compatibility shim (docs/compatibility.md): autosaves made before the rename (or, in the // Flatpak sandbox, under the old app id) are copied to the new folder on first launch; the // old folder is left as it was. diff --git a/crates/engine/src/tests/path_scope.rs b/crates/engine/src/tests/path_scope.rs index 6b99c534..d0f1ee66 100644 --- a/crates/engine/src/tests/path_scope.rs +++ b/crates/engine/src/tests/path_scope.rs @@ -8,7 +8,7 @@ use crate::*; #[test] fn file_commands_stay_inside_the_agents_folders() { - let base = std::env::temp_dir().join(format!("solidcraft-path-scope-{}", std::process::id())); + let base = solidcraft_io::vfs::temp_dir().join(format!("solidcraft-path-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(); diff --git a/crates/io/src/vfs.rs b/crates/io/src/vfs.rs index 958fc70f..1140edfe 100644 --- a/crates/io/src/vfs.rs +++ b/crates/io/src/vfs.rs @@ -293,7 +293,7 @@ pub fn windows_refusal(path: &str) -> Option<&'static str> { // A short (8.3) name such as `PROGRA~1` or `SSH~1` is another spelling of a folder whose // real name could be anything; canonicalising expands existing ones, and a new file's // name must not be one. - if name.as_bytes().windows(2).any(|w| w[0] == b'~' && w[1].is_ascii_digit()) { + if short_name(name) { return Some("short (8.3) names such as `PROGRA~1`"); } let stem = name.split('.').next().unwrap_or("").trim_end_matches([' ', '.']).to_uppercase(); @@ -306,6 +306,45 @@ pub fn windows_refusal(path: &str) -> Option<&'static str> { None } +/// Whether a file or folder name looks like a short (8.3) name: a `~` and a digit (`PROGRA~1`). +fn short_name(name: &str) -> bool { + name.as_bytes().windows(2).any(|w| w[0] == b'~' && w[1].is_ascii_digit()) +} + +/// `path` spelled with long names, for the folders SolidCraft derives from the environment +/// itself (the temporary folder, the recovery folder). On Windows those may hold a short (8.3) +/// name: the temporary folder is often `…\NAME~1.XYZ\AppData\Local\Temp`, with the user folder's +/// short name, when that folder's name is long, and [`windows_refusal`] refuses short names in +/// every path an agent's commands reach, the server's own folders included. A path with a short +/// name is resolved as far as it exists (the rest is appended as given) and loses the `\\?\` +/// prefix resolving adds. It stays as it is when it has no short name, can't be resolved or +/// doesn't resolve to a local drive, and off Windows. +pub fn long_names(path: &std::path::Path) -> std::path::PathBuf { + if !cfg!(windows) || !path.iter().any(|n| short_name(&n.to_string_lossy())) { + return path.to_path_buf(); + } + let mut rest = Vec::new(); + let mut at = path; + loop { + if let Ok(full) = std::fs::canonicalize(at) { + let local = full + .to_str() + .and_then(|f| f.strip_prefix(r"\\?\")) + .filter(|f| matches!(f.as_bytes(), [d, b':', b'\\', ..] if d.is_ascii_alphabetic())); + return local.map_or_else(|| path.to_path_buf(), |f| rest.iter().rev().fold(std::path::PathBuf::from(f), |p, n| p.join(n))); + } + let (Some(parent), Some(name)) = (at.parent(), at.file_name()) else { return path.to_path_buf() }; + rest.push(name); + at = parent; + } +} + +/// The temporary folder spelled with long names ([`long_names`]), for the folders SolidCraft +/// makes in it. +pub fn temp_dir() -> std::path::PathBuf { + long_names(&std::env::temp_dir()) +} + /// `path` with symlinks and `..` resolved. A file that doesn't exist yet resolves through its /// folder, which must exist; a symlink whose target is missing is refused (writing through it /// would land wherever it points). @@ -536,7 +575,7 @@ mod tests { #[cfg(windows)] #[test] fn windows_unc_verbatim_drive_relative_and_device_paths_are_refused() { - let root = std::env::temp_dir().join(format!("solidcraft-vfs-win-{}", std::process::id())); + let root = super::temp_dir().join(format!("solidcraft-vfs-win-{}", std::process::id())); std::fs::create_dir_all(&root).unwrap(); let scope = super::PathScope::new([root.clone()]); let inside = |f: &str| root.join(f).to_string_lossy().to_string(); @@ -558,6 +597,45 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } + /// 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 { + 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| s.split('\\').any(super::short_name)).map(Into::into) + } + + /// The folders SolidCraft derives from the environment are spelled with long names: 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), and the server's own folders in it were refused like an agent's + /// short names. An agent's own short spelling of a folder it may use stays refused. + #[cfg(windows)] + #[test] + fn derived_folders_are_spelled_with_long_names_and_short_names_from_agents_stay_refused() { + use super::{PathScope, long_names, short_name, temp_dir}; + assert!(!temp_dir().iter().any(|n| short_name(&n.to_string_lossy())), "{}", temp_dir().display()); + let long = temp_dir().join(format!("solidcraft-vfs-long-names-{}", std::process::id())); + std::fs::create_dir_all(&long).unwrap(); + let Some(short) = short_spelling(&long) else { + eprintln!("skipped: {} has no short (8.3) name on this volume", long.display()); + let _ = std::fs::remove_dir_all(&long); + return; + }; + let expanded = long_names(&short); + assert_eq!(std::fs::canonicalize(&expanded).unwrap(), std::fs::canonicalize(&long).unwrap()); + assert!(super::windows_refusal(&expanded.to_string_lossy()).is_none(), "{}", expanded.display()); + // What doesn't exist yet is kept as given; a path without a short name is left alone. + assert_eq!(long_names(&short.join("new folder").join("part.step")), expanded.join("new folder").join("part.step")); + assert_eq!(long_names(&long), long); + // The folder through a scope: its long spelling is in reach, an agent's short one is not. + let scope = PathScope::new([long.clone()]); + assert!(scope.check(&expanded.join("part.step").to_string_lossy()).is_ok()); + let e = scope.check(&short.join("part.step").to_string_lossy()).unwrap_err(); + assert!(e.to_string().contains("short (8.3) names"), "{e}"); + let _ = std::fs::remove_dir_all(&long); + } + /// The open design's folder counts only when it is safe (#205): strictly inside home (or the /// temporary folder), not home itself, a folder above it, a root or a hidden folder; and /// hidden files inside it stay out of reach. Folders named on purpose reach everything. diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 9957a649..b796feae 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -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, allowed: impl IntoIterator/solidcraft-mcp--` 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. `` is spelled with +/// long names (`vfs::temp_dir`), since the scope refuses short (8.3) ones. pub fn scratch_dir() -> Option { static DIR: std::sync::OnceLock> = std::sync::OnceLock::new(); DIR.get_or_init(|| { @@ -57,7 +58,7 @@ pub fn scratch_dir() -> Option { 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) }) }) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 71179266..3245a27e 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -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 { + 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] diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 99cce369..dc6d6693 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -589,7 +589,7 @@ fn capture(b: &mut dyn Backend, a: &Map) -> Result<(Vec, 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") { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 47f89d6e..b15f149f 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -814,7 +814,7 @@ mod tests { #[test] fn engine_batch_stays_inside_allow_paths() { let mut h = crate::scenario::Harness::new(); - let base = std::env::temp_dir().join(format!("solidcraft-batch-scope-{}", std::process::id())); + let base = vfs::temp_dir().join(format!("solidcraft-batch-scope-{}", std::process::id())); let (root, out) = (base.join("root"), base.join("out")); std::fs::create_dir_all(&root).unwrap(); std::fs::create_dir_all(&out).unwrap(); @@ -859,7 +859,7 @@ mod tests { let mut s = Session::default(); s.execute("solid.box", &json!({"length": 10, "width": 10, "height": 10})).unwrap(); let mut app = SolveApp::new(s, Services::default()); - let base = std::env::temp_dir().join(format!("solidcraft-control-scope-{}", std::process::id())); + let base = vfs::temp_dir().join(format!("solidcraft-control-scope-{}", std::process::id())); let (root, out) = (base.join("root"), base.join("out")); std::fs::create_dir_all(&root).unwrap(); std::fs::create_dir_all(&out).unwrap(); diff --git a/docs/mcp.md b/docs/mcp.md index b06a27c4..b423a159 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -56,6 +56,9 @@ file name (alternate data streams), reserved device names with any extension (`N `COM1.txt`, …) and short 8.3 names (`PROGRA~1`) are refused before any file system call, and the resolved path is checked again, so a link or junction that leads to a share is refused too. This includes a `sketch.text` font given as a file path; installed fonts named by family still work. +The server spells its own folders (scratch, recovery) with long names, so they work when the +temporary folder's path has a short name (`…\NAME~1.XYZ\AppData\Local\Temp`, as Windows often +gives when the user folder's name is long). A refused path is a tool error saying why. A hard link inside an allowed folder can't be told apart from an ordinary file, so a hard link