fix(gallery): keep model deletion inside the models directory (#12324)

listModelFiles gave utils.VerifyPath paths that it had already joined
onto the models directory. VerifyPath joins its argument onto the base
again, so an absolute path always passes and none of the four checks
could fail. Model deletion then removed files outside the models
directory:

- A model name such as "../outside/victim" removed
  outside/victim.yaml. The in-process MCP delete_model tool passes the
  name from the tool call without a check.
- A gallery file that lists a files: entry with "../" removed that
  file.

listModelFiles now gives VerifyPath the relative names.

InTrustedRoot also looped forever when a relative path was outside a
relative root. filepath.Dir stops at "." for a relative path, and the
loop waited for "/". The loop now stops when Dir returns its input.

Assisted-by: Claude:claude-opus-5-5 [Claude Code]

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>

core/gallery/delete_model_paths_test.go

@@ -0,0 +1,72 @@
+package gallery_test
+
+import (
+	"os"
+	"path/filepath"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/mudler/LocalAI/core/gallery"
+	"github.com/mudler/LocalAI/pkg/system"
+)
+
+// DeleteModelFromSystem removes files named by a model name and by the
+// model's gallery file. Neither may reach outside the models directory: the
+// name can come from an API caller or from an assistant tool call, and the
+// gallery file is a YAML file on disk.
+var _ = Describe("DeleteModelFromSystem path containment", func() {
+	var (
+		root       string
+		modelsPath string
+		outside    string
+		state      *system.SystemState
+	)
+
+	BeforeEach(func() {
+		root = GinkgoT().TempDir()
+		modelsPath = filepath.Join(root, "models")
+		outside = filepath.Join(root, "outside")
+		Expect(os.MkdirAll(modelsPath, 0o755)).To(Succeed())
+		Expect(os.MkdirAll(outside, 0o755)).To(Succeed())
+		var err error
+		state, err = system.GetSystemState(system.WithModelPath(modelsPath))
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("refuses a model name that escapes the models directory", func() {
+		victim := filepath.Join(outside, "victim.yaml")
+		Expect(os.WriteFile(victim, []byte("name: victim\n"), 0o644)).To(Succeed())
+
+		Expect(gallery.DeleteModelFromSystem(state, "../outside/victim")).ToNot(Succeed())
+		Expect(victim).To(BeARegularFile())
+	})
+
+	It("does not remove gallery-declared files outside the models directory", func() {
+		secret := filepath.Join(outside, "secret.bin")
+		Expect(os.WriteFile(secret, []byte("x"), 0o644)).To(Succeed())
+		Expect(os.WriteFile(filepath.Join(modelsPath, "m.yaml"), []byte("name: m\n"), 0o644)).To(Succeed())
+		Expect(os.WriteFile(filepath.Join(modelsPath, gallery.GalleryFileName("m")), []byte(`
+files:
+  - filename: ../outside/secret.bin
+`), 0o644)).To(Succeed())
+
+		_ = gallery.DeleteModelFromSystem(state, "m")
+		Expect(secret).To(BeARegularFile())
+	})
+
+	It("still deletes a normal model and its declared files", func() {
+		weights := filepath.Join(modelsPath, "m", "w.gguf")
+		Expect(os.MkdirAll(filepath.Dir(weights), 0o755)).To(Succeed())
+		Expect(os.WriteFile(weights, []byte("w"), 0o644)).To(Succeed())
+		Expect(os.WriteFile(filepath.Join(modelsPath, "m.yaml"), []byte("name: m\nparameters:\n  model: m/w.gguf\n"), 0o644)).To(Succeed())
+		Expect(os.WriteFile(filepath.Join(modelsPath, gallery.GalleryFileName("m")), []byte(`
+files:
+  - filename: m/w.gguf
+`), 0o644)).To(Succeed())
+
+		Expect(gallery.DeleteModelFromSystem(state, "m")).To(Succeed())
+		Expect(weights).ToNot(BeAnExistingFile())
+		Expect(filepath.Join(modelsPath, "m.yaml")).ToNot(BeAnExistingFile())
+	})
+})

core/gallery/models.go

@@ -808,8 +808,11 @@ func GetLocalModelConfiguration(basePath string, name string) (*ModelConfig, err
 
 func listModelFiles(systemState *system.SystemState, name string) ([]string, error) {
 
+	// VerifyPath joins its argument onto the models path itself, so every
+	// check below passes the relative name: an already-joined absolute path
+	// always lands inside the base and the check would pass anything.
 	configFile := filepath.Join(systemState.Model.ModelsPath, fmt.Sprintf("%s.yaml", name))
-	if err := utils.VerifyPath(configFile, systemState.Model.ModelsPath); err != nil {
+	if err := utils.VerifyPath(fmt.Sprintf("%s.yaml", name), systemState.Model.ModelsPath); err != nil {
 		return nil, fmt.Errorf("failed to verify path %s: %w", configFile, err)
 	}
 
@@ -817,7 +820,7 @@ func listModelFiles(systemState *system.SystemState, name string) ([]string, err
 	name = strings.ReplaceAll(name, string(os.PathSeparator), "__")
 
 	galleryFile := filepath.Join(systemState.Model.ModelsPath, galleryFileName(name))
-	if err := utils.VerifyPath(galleryFile, systemState.Model.ModelsPath); err != nil {
+	if err := utils.VerifyPath(galleryFileName(name), systemState.Model.ModelsPath); err != nil {
 		return nil, fmt.Errorf("failed to verify path %s: %w", galleryFile, err)
 	}
 
@@ -847,7 +850,7 @@ func listModelFiles(systemState *system.SystemState, name string) ([]string, err
 	if err == nil && galleryconfig != nil {
 		for _, f := range galleryconfig.Files {
 			fullPath := filepath.Join(systemState.Model.ModelsPath, f.Filename)
-			if err := utils.VerifyPath(fullPath, systemState.Model.ModelsPath); err != nil {
+			if err := utils.VerifyPath(f.Filename, systemState.Model.ModelsPath); err != nil {
 				return allFiles, fmt.Errorf("failed to verify path %s: %w", fullPath, err)
 			}
 			allFiles = append(allFiles, fullPath)
@@ -858,7 +861,7 @@ func listModelFiles(systemState *system.SystemState, name string) ([]string, err
 
 	for _, f := range additionalFiles {
 		fullPath := filepath.Join(filepath.Join(systemState.Model.ModelsPath, f))
-		if err := utils.VerifyPath(fullPath, systemState.Model.ModelsPath); err != nil {
+		if err := utils.VerifyPath(f, systemState.Model.ModelsPath); err != nil {
 			return allFiles, fmt.Errorf("failed to verify path %s: %w", fullPath, err)
 		}
 		allFiles = append(allFiles, fullPath)

pkg/utils/path.go

@@ -13,13 +13,18 @@ func ExistsInPath(path string, s string) bool {
 }
 
 func InTrustedRoot(path string, trustedRoot string) error {
-	for path != "/" {
-		path = filepath.Dir(path)
+	for {
+		parent := filepath.Dir(path)
+		// Dir stops changing at "/" for an absolute path and at "." for a
+		// relative one; waiting for "/" alone spins forever on the latter.
+		if parent == path {
+			return fmt.Errorf("path is outside of trusted root")
+		}
+		path = parent
 		if path == trustedRoot {
 			return nil
 		}
 	}
-	return fmt.Errorf("path is outside of trusted root")
 }
 
 // VerifyPath verifies that path is based in basePath.

pkg/utils/path_test.go

@@ -3,6 +3,7 @@ package utils_test
 import (
 	"os"
 	"path/filepath"
+	"time"
 
 	. "github.com/mudler/LocalAI/pkg/utils"
 	. "github.com/onsi/ginkgo/v2"
@@ -93,6 +94,21 @@ var _ = Describe("utils/path tests", func() {
 		It("rejects an unrelated absolute path", func() {
 			Expect(InTrustedRoot("/etc/passwd", "/srv/models")).ToNot(Succeed())
 		})
+
+		It("rejects a relative path outside a relative root instead of looping", func() {
+			// Walking up a relative path ends at ".", never at "/", so the
+			// walk must stop when it stops making progress.
+			done := make(chan error, 1)
+			go func() { done <- InTrustedRoot("x", "models") }()
+			Eventually(done).WithTimeout(2 * time.Second).Should(Receive(HaveOccurred()))
+
+			go func() { done <- VerifyPath("../x", "models") }()
+			Eventually(done).WithTimeout(2 * time.Second).Should(Receive(HaveOccurred()))
+		})
+
+		It("accepts a relative descendant of a relative root", func() {
+			Expect(InTrustedRoot("models/a/file", "models")).To(Succeed())
+		})
 	})
 
 	Describe("SanitizeFileName", func() {