mirror of
https://github.com/mudler/LocalAI.git
synced 2026-09-29 01:25:03 -04:00
fix: make the remaining VerifyPath checks effective (#12326)
utils.VerifyPath joins its argument onto the base path, so a path that the caller already joined always passes. Several callers gave it joined paths, and their checks could not fail: - modeladmin (config view, patch, edit, pin and state): the config file path from the loader. A config loaded from outside the models directory (--models-config-file) could be pinned, and the pin wrote the outside file. The patch and state paths stopped later, in the mutation snapshot, with a different error. - core/backend/tts.go: the model path joined onto the models path. - The trellis2cpp and stablediffusion-ggml backends: option paths (*_path) joined onto the model path. A "../" value outside the model directory was accepted. Add utils.VerifyResolvedPath for a full path. modeladmin and tts use it. The backends now check the relative option value before they join it. A rename in modeladmin checks the new relative name. For models from a config file outside the models directory, the admin API and web UI now return ErrPathNotTrusted for view, edit, pin, and enable or disable. The docs describe this. 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>
This commit is contained in:
1 parent
bc01ef2350
commit
50c284fdcc
11 files changed
+103
-12
No files matched your search
@@ -137,8 +137,8 @@ func (sd *SDGGML) Load(opts *pb.ModelOptions) error {
|
||||
// If it's an option path, we resolve absolute path from the model path
|
||||
if strings.Contains(op, ":") && strings.Contains(op, "path") {
|
||||
data := strings.Split(op, ":")
|
||||
data[1] = filepath.Join(opts.ModelPath, data[1])
|
||||
if err := utils.VerifyPath(data[1], opts.ModelPath); err == nil {
|
||||
data[1] = filepath.Join(opts.ModelPath, data[1])
|
||||
oo = append(oo, strings.Join(data, ":"))
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -161,10 +161,10 @@ func resolveModels(modelFile, modelPath string, options []string) (modelSet, err
|
||||
continue
|
||||
}
|
||||
if !filepath.IsAbs(value) {
|
||||
value = filepath.Join(modelPath, value)
|
||||
if err := utils.VerifyPath(value, modelPath); err != nil {
|
||||
return modelSet{}, fmt.Errorf("option %s: %w", key, err)
|
||||
}
|
||||
value = filepath.Join(modelPath, value)
|
||||
}
|
||||
overrides[key] = value
|
||||
}
|
||||
|
||||
@@ -134,9 +134,12 @@ var _ = Describe("resolveModels", func() {
|
||||
|
||||
It("rejects option paths escaping the model directory", func() {
|
||||
touch(dir, fullSet...)
|
||||
// The escaping file exists, so only the containment check can
|
||||
// reject it; a missing file would fail for an unrelated reason.
|
||||
touch(filepath.Dir(dir), "outside.gguf")
|
||||
|
||||
_, err := resolveModels("ss_flow_f16.gguf", dir, []string{"dino_path:../outside.gguf"})
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err).To(MatchError(ContainSubstring("outside of trusted root")))
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
+2
-2
@@ -88,7 +88,7 @@ func ModelTTS(
|
||||
// a FS path
|
||||
mp := filepath.Join(loader.ModelPath, modelConfig.Model)
|
||||
if _, err := os.Stat(mp); err == nil {
|
||||
if err := utils.VerifyPath(mp, appConfig.SystemState.Model.ModelsPath); err != nil {
|
||||
if err := utils.VerifyResolvedPath(mp, appConfig.SystemState.Model.ModelsPath); err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
modelPath = mp
|
||||
@@ -189,7 +189,7 @@ func ModelTTSStream(
|
||||
// a FS path
|
||||
mp := filepath.Join(loader.ModelPath, modelConfig.Model)
|
||||
if _, err := os.Stat(mp); err == nil {
|
||||
if err := utils.VerifyPath(mp, appConfig.SystemState.Model.ModelsPath); err != nil {
|
||||
if err := utils.VerifyResolvedPath(mp, appConfig.SystemState.Model.ModelsPath); err != nil {
|
||||
return err
|
||||
}
|
||||
modelPath = mp
|
||||
|
||||
@@ -93,7 +93,7 @@ func (s *ConfigService) GetConfig(_ context.Context, name string) (*ConfigView,
|
||||
if configPath == "" {
|
||||
return nil, ErrConfigFileMissing
|
||||
}
|
||||
if err := utils.VerifyPath(configPath, s.modelsPath()); err != nil {
|
||||
if err := utils.VerifyResolvedPath(configPath, s.modelsPath()); err != nil {
|
||||
return nil, fmt.Errorf("%w: %v", ErrPathNotTrusted, err)
|
||||
}
|
||||
data, err := os.ReadFile(configPath)
|
||||
@@ -137,7 +137,7 @@ func (s *ConfigService) patchConfig(ctx context.Context, name string, patch map[
|
||||
return nil, fmt.Errorf("%w: PATCH cannot rename model %q to %q; use the model edit endpoint", ErrInvalidConfig, name, patchedName)
|
||||
}
|
||||
configPath := cfg.GetModelConfigFile()
|
||||
if err := utils.VerifyPath(configPath, s.modelsPath()); err != nil {
|
||||
if err := utils.VerifyResolvedPath(configPath, s.modelsPath()); err != nil {
|
||||
return nil, fmt.Errorf("%w: %v", ErrPathNotTrusted, err)
|
||||
}
|
||||
diskYAML, err := os.ReadFile(configPath)
|
||||
@@ -289,7 +289,7 @@ func (s *ConfigService) editYAML(ctx context.Context, name string, body []byte)
|
||||
|
||||
configPath := existing.GetModelConfigFile()
|
||||
modelsPath := s.modelsPath()
|
||||
if err := utils.VerifyPath(configPath, modelsPath); err != nil {
|
||||
if err := utils.VerifyResolvedPath(configPath, modelsPath); err != nil {
|
||||
return nil, fmt.Errorf("%w: %v", ErrPathNotTrusted, err)
|
||||
}
|
||||
|
||||
@@ -304,7 +304,7 @@ func (s *ConfigService) editYAML(ctx context.Context, name string, body []byte)
|
||||
}
|
||||
newConfigPath := filepath.Join(modelsPath, req.Name+".yaml")
|
||||
paths = append(paths, newConfigPath, filepath.Join(modelsPath, gallery.GalleryFileName(name)), filepath.Join(modelsPath, gallery.GalleryFileName(req.Name)))
|
||||
if err := utils.VerifyPath(newConfigPath, modelsPath); err != nil {
|
||||
if err := utils.VerifyPath(req.Name+".yaml", modelsPath); err != nil {
|
||||
return nil, fmt.Errorf("%w: %v", ErrPathNotTrusted, err)
|
||||
}
|
||||
if _, err := os.Stat(newConfigPath); err == nil {
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
package modeladmin
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
// A model config can be loaded from outside the models directory (for
|
||||
// example with --config-file). The admin mutations write the config file
|
||||
// back, so they must refuse a file outside the models directory rather than
|
||||
// write wherever the loader found it.
|
||||
var _ = Describe("ConfigService config file containment", func() {
|
||||
var (
|
||||
svc *ConfigService
|
||||
ctx context.Context
|
||||
outside string
|
||||
orig []byte
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
svc, _ = newTestService()
|
||||
ctx = context.Background()
|
||||
outside = filepath.Join(GinkgoT().TempDir(), "external.yaml")
|
||||
orig = []byte("name: external\nbackend: llama-cpp\n")
|
||||
Expect(os.WriteFile(outside, orig, 0o644)).To(Succeed())
|
||||
Expect(svc.Loader.ReadModelConfig(outside, svc.AppConfig.ToConfigLoaderOptions()...)).To(Succeed())
|
||||
cfg, ok := svc.Loader.GetModelConfig("external")
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(cfg.GetModelConfigFile()).To(Equal(outside))
|
||||
})
|
||||
|
||||
It("refuses to pin a model whose config file is outside the models directory", func() {
|
||||
_, err := svc.TogglePinned(ctx, "external", ActionPin, nil)
|
||||
Expect(err).To(MatchError(ErrPathNotTrusted))
|
||||
Expect(os.ReadFile(outside)).To(Equal(orig))
|
||||
})
|
||||
|
||||
It("refuses to toggle the state of such a model", func() {
|
||||
_, err := svc.ToggleState(ctx, "external", ActionDisable)
|
||||
Expect(err).To(MatchError(ErrPathNotTrusted))
|
||||
Expect(os.ReadFile(outside)).To(Equal(orig))
|
||||
})
|
||||
|
||||
It("refuses to patch such a model", func() {
|
||||
_, err := svc.PatchConfig(ctx, "external", map[string]any{"context_size": 4096})
|
||||
Expect(err).To(MatchError(ErrPathNotTrusted))
|
||||
Expect(os.ReadFile(outside)).To(Equal(orig))
|
||||
})
|
||||
|
||||
It("refuses to read such a model's config", func() {
|
||||
_, err := svc.GetConfig(ctx, "external")
|
||||
Expect(err).To(MatchError(ErrPathNotTrusted))
|
||||
})
|
||||
})
|
||||
@@ -29,7 +29,7 @@ func (s *ConfigService) TogglePinned(_ context.Context, name string, action Acti
|
||||
if configPath == "" {
|
||||
return nil, ErrConfigFileMissing
|
||||
}
|
||||
if err := utils.VerifyPath(configPath, s.modelsPath()); err != nil {
|
||||
if err := utils.VerifyResolvedPath(configPath, s.modelsPath()); err != nil {
|
||||
return nil, fmt.Errorf("%w: %v", ErrPathNotTrusted, err)
|
||||
}
|
||||
if err := mutateYAMLBoolFlag(configPath, "pinned", action == ActionPin); err != nil {
|
||||
|
||||
@@ -49,7 +49,7 @@ func (s *ConfigService) toggleState(ctx context.Context, name string, action Act
|
||||
if configPath == "" {
|
||||
return nil, ErrConfigFileMissing
|
||||
}
|
||||
if err := utils.VerifyPath(configPath, s.modelsPath()); err != nil {
|
||||
if err := utils.VerifyResolvedPath(configPath, s.modelsPath()); err != nil {
|
||||
return nil, fmt.Errorf("%w: %v", ErrPathNotTrusted, err)
|
||||
}
|
||||
var result *ToggleResult
|
||||
|
||||
@@ -74,6 +74,8 @@ When using `--models-config-file`, you can define multiple models as a list:
|
||||
backend: llama-cpp
|
||||
```
|
||||
|
||||
LocalAI changes only config files that are inside the models directory. If the file from `--models-config-file` is outside the models directory, you cannot view, edit, pin, enable or disable its models from the web UI or the model admin API. Edit the file directly, then restart LocalAI.
|
||||
|
||||
## Core Configuration Fields
|
||||
|
||||
### Basic Model Settings
|
||||
|
||||
+10
-1
@@ -27,12 +27,21 @@ func InTrustedRoot(path string, trustedRoot string) error {
|
||||
}
|
||||
}
|
||||
|
||||
// VerifyPath verifies that path is based in basePath.
|
||||
// VerifyPath verifies that path, taken relative to basePath, is based in
|
||||
// basePath. It joins path onto basePath first, so an absolute path is read as
|
||||
// relative to the base as well: give it the untrusted relative name, never a
|
||||
// path that has already been joined. For a full path use VerifyResolvedPath.
|
||||
func VerifyPath(path, basePath string) error {
|
||||
c := filepath.Clean(filepath.Join(basePath, path))
|
||||
return InTrustedRoot(c, filepath.Clean(basePath))
|
||||
}
|
||||
|
||||
// VerifyResolvedPath verifies that path, a full path rather than one relative
|
||||
// to basePath, is based in basePath.
|
||||
func VerifyResolvedPath(path, basePath string) error {
|
||||
return InTrustedRoot(filepath.Clean(path), filepath.Clean(basePath))
|
||||
}
|
||||
|
||||
// SanitizeFileName sanitizes the given filename
|
||||
func SanitizeFileName(fileName string) string {
|
||||
// filepath.Clean to clean the path
|
||||
|
||||
@@ -72,6 +72,25 @@ var _ = Describe("utils/path tests", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("VerifyResolvedPath", func() {
|
||||
It("accepts a full path inside the base", func() {
|
||||
Expect(VerifyResolvedPath("/srv/models/a/model.yaml", "/srv/models")).To(Succeed())
|
||||
})
|
||||
|
||||
It("rejects a full path outside the base", func() {
|
||||
// VerifyPath would join this onto the base and accept it.
|
||||
Expect(VerifyResolvedPath("/etc/passwd", "/srv/models")).ToNot(Succeed())
|
||||
})
|
||||
|
||||
It("rejects a joined path that climbed out of the base", func() {
|
||||
Expect(VerifyResolvedPath(filepath.Join("/srv/models", "../other/x"), "/srv/models")).ToNot(Succeed())
|
||||
})
|
||||
|
||||
It("cleans both paths before comparing", func() {
|
||||
Expect(VerifyResolvedPath("/srv/models/./a/../b.yaml", "/srv/models/")).To(Succeed())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("InTrustedRoot", func() {
|
||||
It("accepts a strict descendant of the trusted root", func() {
|
||||
Expect(InTrustedRoot("/srv/models/file", "/srv/models")).To(Succeed())
|
||||
|
||||
Reference in new issue
Block a user