From 47f37fa80d55d90a539b6ca34e46160c459c1720 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 18 Jul 2026 10:36:04 +0000 Subject: [PATCH] fix(gallery): key meta pin recall on the installed name and detach resolved entries Six review findings on the meta-entry install path. Pin recall was keyed on the gallery entry name while applyModel writes the record under the install name (req.Name when supplied), so a meta installed under a custom name with a pin lost that pin on reinstall and was silently re-resolved onto a different variant, possibly swapping its backend. Compute the install name with applyModel's own precedence before the recall. ResolveMetaModel returned a shallow struct copy, so the resolved entry's Overrides aliased the gallery entry's map and the install path's in-place mergo merge wrote the caller's request into the shared catalog. Detach Overrides, ConfigFile, AdditionalFiles, URLs and Tags. Not exploitable today only because this path re-unmarshals the gallery per call, which is a property nobody should have to rely on. Also: overlay the meta's name onto the persisted config for meta installs so the gallery file no longer records the variant's name; move the pinned-VRAM warning below the variant validation so a pin naming a nonexistent entry does not warn about VRAM before failing for an unrelated reason; and stop seeding config.URLs in the config_file branch, which duplicated every declared URL. Add seven network-free specs driving InstallModelFromGallery with a meta entry: variant payload wins over the meta's legacy url fallback, the resolution record round-trips to disk, a pin is recorded and honored on reinstall including under a custom install name, and the resolved entry does not alias the gallery's maps. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Ettore Di Giacinto --- core/gallery/meta_install_test.go | 191 ++++++++++++++++++++++++++++++ core/gallery/models.go | 59 ++++++--- 2 files changed, 236 insertions(+), 14 deletions(-) diff --git a/core/gallery/meta_install_test.go b/core/gallery/meta_install_test.go index cdada35c0..5d441bb5a 100644 --- a/core/gallery/meta_install_test.go +++ b/core/gallery/meta_install_test.go @@ -1,11 +1,17 @@ package gallery_test import ( + "context" + "os" + "path/filepath" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "gopkg.in/yaml.v3" + "github.com/mudler/LocalAI/core/config" "github.com/mudler/LocalAI/core/gallery" + "github.com/mudler/LocalAI/pkg/system" ) var _ = Describe("GalleryModel meta entries", func() { @@ -93,6 +99,28 @@ var _ = Describe("ResolveMetaModel", func() { Expect(resolved.URL).To(Equal("file://gguf.yaml")) }) + It("detaches the resolved entry from the gallery's own maps and slices", func() { + models[0].Overrides = map[string]any{"f16": true} + models[0].AdditionalFiles = []gallery.File{{Filename: "a.bin"}} + + resolved, _, err := gallery.ResolveMetaModel(models, meta, gallery.ResolveEnv{Capability: "nvidia", VRAM: gib(24)}, "") + Expect(err).ToNot(HaveOccurred()) + + // The install path merges the caller's request overrides into this map in + // place, so aliasing the gallery's map would write one caller's request + // into the shared catalog and leak it into every later install. + resolved.Overrides["f16"] = false + resolved.Overrides["threads"] = 4 + Expect(models[0].Overrides).To(HaveKeyWithValue("f16", true)) + Expect(models[0].Overrides).ToNot(HaveKey("threads")) + + resolved.AdditionalFiles[0].Filename = "mutated.bin" + Expect(models[0].AdditionalFiles[0].Filename).To(Equal("a.bin")) + + resolved.Tags = append(resolved.Tags, "extra") + Expect(meta.Tags).To(ConsistOf("llm")) + }) + It("errors when a candidate references a missing entry", func() { meta.Candidates = []gallery.Candidate{{Model: "does-not-exist"}} _, _, err := gallery.ResolveMetaModel(models, meta, gallery.ResolveEnv{Capability: "default", VRAM: gib(8)}, "") @@ -115,3 +143,166 @@ var _ = Describe("ResolveMetaModel", func() { Expect(err).To(MatchError(gallery.ErrPinNotFound)) }) }) + +var _ = Describe("InstallModelFromGallery with meta entries", func() { + var tempdir string + var galleries []config.Gallery + var systemState *system.SystemState + + // The variants are described with an inline config_file rather than a URL so + // the whole install runs off the local filesystem with no network access. + // The meta entry keeps a url as well, because a real meta entry carries one + // as a fallback for older LocalAI releases that do not understand candidates, + // and installing that fallback instead of a variant is exactly the regression + // these specs guard against. + newGallery := func(meta gallery.GalleryModel, variants ...gallery.GalleryModel) { + fallback := gallery.ModelConfig{ + Name: "legacy-fallback", + Description: "legacy fallback payload", + ConfigFile: "backend: fallback-backend\n", + } + fallbackYAML, err := yaml.Marshal(fallback) + Expect(err).ToNot(HaveOccurred()) + fallbackPath := filepath.Join(tempdir, "fallback.yaml") + Expect(os.WriteFile(fallbackPath, fallbackYAML, 0600)).To(Succeed()) + + meta.URL = "file://" + fallbackPath + entries := append([]gallery.GalleryModel{meta}, variants...) + + out, err := yaml.Marshal(entries) + Expect(err).ToNot(HaveOccurred()) + galleryPath := filepath.Join(tempdir, "gallery.yaml") + Expect(os.WriteFile(galleryPath, out, 0600)).To(Succeed()) + + galleries = []config.Gallery{{Name: "test", URL: "file://" + galleryPath}} + } + + variant := func(name, backend string) gallery.GalleryModel { + m := gallery.GalleryModel{ConfigFile: map[string]any{"backend": backend}} + m.Name = name + m.Description = "variant " + name + return m + } + + metaEntry := func(name string, candidates ...string) gallery.GalleryModel { + m := gallery.GalleryModel{} + m.Name = name + m.Description = "the meta entry" + m.Icon = "meta.png" + for _, c := range candidates { + m.Candidates = append(m.Candidates, gallery.Candidate{Model: c}) + } + return m + } + + install := func(name string, req gallery.GalleryModel, options ...gallery.InstallOption) error { + return gallery.InstallModelFromGallery( + context.TODO(), galleries, []config.Gallery{}, systemState, nil, + name, req, func(string, string, string, float64) {}, false, false, false, options...) + } + + installedBackend := func(name string) string { + dat, err := os.ReadFile(filepath.Join(tempdir, name+".yaml")) + Expect(err).ToNot(HaveOccurred()) + content := map[string]any{} + Expect(yaml.Unmarshal(dat, &content)).To(Succeed()) + return content["backend"].(string) + } + + BeforeEach(func() { + var err error + tempdir, err = os.MkdirTemp("", "meta-install") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { Expect(os.RemoveAll(tempdir)).To(Succeed()) }) + + systemState, err = system.GetSystemState(system.WithModelPath(tempdir)) + Expect(err).ToNot(HaveOccurred()) + + newGallery( + metaEntry("qwen3-8b", "qwen3-8b-variant-a", "qwen3-8b-variant-b"), + variant("qwen3-8b-variant-a", "variant-a-backend"), + variant("qwen3-8b-variant-b", "variant-b-backend"), + ) + }) + + It("installs the resolved variant's payload, not the meta's url fallback", func() { + Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed()) + + // If meta-ness stopped winning over the url, this would be + // "fallback-backend" and every meta entry would silently install the + // legacy payload instead of a hardware-appropriate variant. + Expect(installedBackend("qwen3-8b")).To(Equal("variant-a-backend")) + }) + + It("round-trips the resolution record to disk under the meta's name", func() { + Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed()) + + record, err := gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b") + Expect(err).ToNot(HaveOccurred()) + Expect(record.MetaName).To(Equal("qwen3-8b")) + Expect(record.ResolvedVariant).To(Equal("qwen3-8b-variant-a")) + Expect(record.PinnedVariant).To(BeEmpty()) + Expect(record.Name).To(Equal("qwen3-8b")) + Expect(record.Description).To(Equal("the meta entry")) + }) + + It("records a pin and honors it on a plain reinstall", func() { + Expect(install("qwen3-8b", gallery.GalleryModel{}, gallery.WithVariant("qwen3-8b-variant-b"))).To(Succeed()) + + record, err := gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b") + Expect(err).ToNot(HaveOccurred()) + Expect(record.PinnedVariant).To(Equal("qwen3-8b-variant-b")) + Expect(record.ResolvedVariant).To(Equal("qwen3-8b-variant-b")) + Expect(installedBackend("qwen3-8b")).To(Equal("variant-b-backend")) + + // No WithVariant this time: hardware resolution would pick variant-a, so + // only the recalled pin can keep this on variant-b. + Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed()) + Expect(installedBackend("qwen3-8b")).To(Equal("variant-b-backend")) + + record, err = gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b") + Expect(err).ToNot(HaveOccurred()) + Expect(record.PinnedVariant).To(Equal("qwen3-8b-variant-b")) + }) + + It("honors a pin recorded under a custom install name", func() { + req := gallery.GalleryModel{} + req.Name = "prod-llm" + + Expect(install("qwen3-8b", req, gallery.WithVariant("qwen3-8b-variant-b"))).To(Succeed()) + + // The pin is written under the installed name, so the recall must read it + // back under that name too and not under the gallery entry's name. + record, err := gallery.GetLocalModelConfiguration(tempdir, "prod-llm") + Expect(err).ToNot(HaveOccurred()) + Expect(record.PinnedVariant).To(Equal("qwen3-8b-variant-b")) + Expect(record.MetaName).To(Equal("qwen3-8b")) + Expect(installedBackend("prod-llm")).To(Equal("variant-b-backend")) + + Expect(install("qwen3-8b", req)).To(Succeed()) + Expect(installedBackend("prod-llm")).To(Equal("variant-b-backend")) + }) + + It("does not write the caller's overrides back into the gallery entry", func() { + req := gallery.GalleryModel{Overrides: map[string]any{"f16": true}} + Expect(install("qwen3-8b", req)).To(Succeed()) + + models, err := gallery.AvailableGalleryModels(galleries, systemState) + Expect(err).ToNot(HaveOccurred()) + entry := gallery.FindGalleryElement(models, "qwen3-8b-variant-a") + Expect(entry).ToNot(BeNil()) + Expect(entry.Overrides).ToNot(HaveKey("f16")) + }) + + It("writes each declared url only once into the persisted gallery file", func() { + meta := metaEntry("qwen3-8b", "qwen3-8b-variant-a") + meta.URLs = []string{"https://example.invalid/qwen3"} + newGallery(meta, variant("qwen3-8b-variant-a", "variant-a-backend")) + + Expect(install("qwen3-8b", gallery.GalleryModel{})).To(Succeed()) + + record, err := gallery.GetLocalModelConfiguration(tempdir, "qwen3-8b") + Expect(err).ToNot(HaveOccurred()) + Expect(record.URLs).To(ConsistOf("https://example.invalid/qwen3")) + }) +}) diff --git a/core/gallery/models.go b/core/gallery/models.go index 88c81e48a..0b1e65028 100644 --- a/core/gallery/models.go +++ b/core/gallery/models.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "maps" "net/url" "os" "path/filepath" @@ -94,16 +95,6 @@ func ResolveMetaModel(models []*GalleryModel, meta *GalleryModel, env ResolveEnv return nil, Candidate{}, fmt.Errorf("resolving variant for model %q: %w", meta.Name, err) } - // A pin is an operator override and deliberately bypasses the hardware - // checks, but a silent bypass makes a later out-of-memory failure - // impossible to trace back to the pin, so it is recorded loudly here. - if pin != "" { - if floor, declared, verr := candidate.EffectiveMinVRAM(); verr == nil && declared && env.VRAM < floor { - xlog.Warn("Pinned model variant declares more VRAM than this system reports; installing anyway because the pin overrides hardware resolution", - "model", meta.Name, "variant", candidate.Model, "required_vram", floor, "available_vram", env.VRAM) - } - } - concrete := FindGalleryElement(models, candidate.Model) if concrete == nil { return nil, Candidate{}, fmt.Errorf("model %q references variant %q which does not exist in any configured gallery", meta.Name, candidate.Model) @@ -112,15 +103,38 @@ func ResolveMetaModel(models []*GalleryModel, meta *GalleryModel, env ResolveEnv return nil, Candidate{}, fmt.Errorf("model %q references variant %q which is itself a meta entry; meta entries may not nest", meta.Name, candidate.Model) } + // A pin is an operator override and deliberately bypasses the hardware + // checks, but a silent bypass makes a later out-of-memory failure + // impossible to trace back to the pin, so it is recorded loudly here. + // It is warned about only once the pin is known to name a real, installable + // entry, otherwise a pin naming a listed-but-nonexistent variant would warn + // about VRAM and then fail for an entirely unrelated reason. + if pin != "" { + if floor, declared, verr := candidate.EffectiveMinVRAM(); verr == nil && declared && env.VRAM < floor { + xlog.Warn("Pinned model variant declares more VRAM than this system reports; installing anyway because the pin overrides hardware resolution", + "model", meta.Name, "variant", candidate.Model, "required_vram", floor, "available_vram", env.VRAM) + } + } + resolved := *concrete resolved.Name = meta.Name resolved.Description = meta.Description resolved.Icon = meta.Icon resolved.License = meta.License - resolved.URLs = meta.URLs - resolved.Tags = meta.Tags resolved.Candidates = nil + // The struct copy above is shallow, so every reference-typed field still + // aliases the gallery's own entries. The install path mutates Overrides in + // place (mergo merges the caller's request overrides into it) and appends to + // the URL and tag slices, which would write the caller's request into the + // gallery catalog itself and leak between installs the moment this path + // reads from a cached, long-lived gallery listing. Detach them here. + resolved.Overrides = maps.Clone(concrete.Overrides) + resolved.ConfigFile = maps.Clone(concrete.ConfigFile) + resolved.AdditionalFiles = slices.Clone(concrete.AdditionalFiles) + resolved.URLs = slices.Clone(meta.URLs) + resolved.Tags = slices.Clone(meta.Tags) + return &resolved, candidate, nil } @@ -157,9 +171,11 @@ func InstallModelFromGallery( ConfigFile: string(reYamlConfig), Description: model.Description, License: model.License, - URLs: model.URLs, Name: model.Name, Files: make([]File, 0), // Real values get added below, must be blank + // URLs are deliberately not seeded here: they are appended once + // below for both branches, and seeding them too would write every + // URL twice into the persisted gallery file. // Prompt Template Skipped for now - I expect in this mode that they will be delivered as files. } } else { @@ -170,6 +186,10 @@ func InstallModelFromGallery( config.MetaName = record.MetaName config.ResolvedVariant = record.ResolvedVariant config.PinnedVariant = record.PinnedVariant + // The variant's own name would otherwise be persisted here, which + // contradicts the whole point of a meta entry: the model is known by + // the meta's stable name regardless of which variant backs it. + config.Name = record.MetaName } installName := model.Name @@ -227,8 +247,19 @@ func InstallModelFromGallery( // A previously recorded pin survives reinstalls and upgrades, so a user // who deliberately chose a variant is not silently re-resolved onto a // different one by a hardware or gallery change. + // + // The record is keyed by the name the model was installed under, not by the + // gallery entry name: applyModel writes it to ._gallery_.yaml, + // where installName is req.Name whenever the caller supplied one. Reading it + // back under the meta's own name would miss the record for every custom-named + // install and silently re-resolve a deliberately pinned model onto a + // different variant, possibly swapping its backend. if pin == "" { - if previous, err := GetLocalModelConfiguration(systemState.Model.ModelsPath, model.Name); err == nil && previous != nil { + installName := model.Name + if req.Name != "" { + installName = req.Name + } + if previous, err := GetLocalModelConfiguration(systemState.Model.ModelsPath, installName); err == nil && previous != nil { pin = previous.PinnedVariant } }