From b1fa3cb7e085cb403c5a25c5dd4c0e79b3dab967 Mon Sep 17 00:00:00 2001 From: Virgile Pourchet Date: Sun, 13 Sep 2026 23:29:41 +0200 Subject: [PATCH] fix(model-hf): reject rooted POSIX paths in delete on Windows too `parse_delete_model_ref` rejects filesystem paths by testing `is_absolute()` plus a few literal shapes. Windows does not consider a leading `/` absolute, since that needs a drive letter or a UNC prefix, so a rooted POSIX path slipped through the guard there and fell into the Hugging Face branch instead. Measured on Windows 11 by calling `resolve_model_identifier` directly: /tmp/model.gguf is_absolute=false -> "Expected a model stem or Hugging Face ref..." /home/user/model.gguf is_absolute=false -> same C:\models\model.gguf is_absolute=true -> rejected as a path C:/models/model.gguf is_absolute=true -> rejected as a path \\server\share\model.gguf is_absolute=true -> rejected as a path ./model.gguf is_absolute=false -> rejected as a path ~/model.gguf is_absolute=false -> rejected as a path So `mesh-llm models delete /tmp/model.gguf` answered with a message about model stems and Hugging Face refs rather than saying that paths are not supported. Wrong answer, not a dangerous one, and only on Windows. A leading `/` is now rejected outright. No model stem or Hugging Face ref starts with one on any platform, and on Unix `is_absolute()` already covered it, so behaviour there is unchanged. The test grew from one input to seven, covering the rooted POSIX, Windows drive, UNC, relative and tilde shapes. `C:/models/model.gguf` is deliberately absent: Linux has no reason to treat it as a path, and asserting it would pass here and fail in CI. --- .../src/models/delete_tests.rs | 28 ++++++++++++++----- crates/model-hf/src/store/delete.rs | 4 +++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/crates/mesh-llm-host-runtime/src/models/delete_tests.rs b/crates/mesh-llm-host-runtime/src/models/delete_tests.rs index 1ff1736455..568564e194 100644 --- a/crates/mesh-llm-host-runtime/src/models/delete_tests.rs +++ b/crates/mesh-llm-host-runtime/src/models/delete_tests.rs @@ -52,13 +52,27 @@ fn create_cache_repo_file( #[tokio::test] async fn resolve_model_identifier_rejects_filesystem_paths() { - let err = resolve_model_identifier("/tmp/model.gguf") - .await - .unwrap_err(); - assert!( - err.to_string() - .contains("does not support filesystem paths") - ); + // Every shape here is a filesystem path on some platform and a valid + // model stem on none, so each is rejected the same way everywhere. + // `/tmp/model.gguf` is the one that mattered: Windows does not consider + // it absolute, so it used to fall through to the Hugging Face branch and + // fail with a message about model stems instead. + for input in [ + "/tmp/model.gguf", + "/home/user/model.gguf", + "C:\\models\\model.gguf", + "\\\\server\\share\\model.gguf", + "./model.gguf", + "../model.gguf", + "~/model.gguf", + ] { + let err = resolve_model_identifier(input).await.unwrap_err(); + assert!( + err.to_string() + .contains("does not support filesystem paths"), + "{input} should be rejected as a path, got: {err}" + ); + } } #[tokio::test] diff --git a/crates/model-hf/src/store/delete.rs b/crates/model-hf/src/store/delete.rs index f7db2cba9a..48ca6d9277 100644 --- a/crates/model-hf/src/store/delete.rs +++ b/crates/model-hf/src/store/delete.rs @@ -41,7 +41,11 @@ async fn parse_delete_model_ref( if input.starts_with("http://") || input.starts_with("https://") { bail!("Delete does not support direct URLs. Use a model stem or Hugging Face ref."); } + // A leading `/` is rooted on Windows without being absolute there, so + // `is_absolute()` alone lets POSIX-style paths through on that platform. + // No model stem or Hugging Face ref starts with one on any platform. if Path::new(input).is_absolute() + || input.starts_with('/') || input.contains('\\') || input.starts_with("./") || input.starts_with("../")