From 258591b3f2ca2557a773718ec7ca0f17051d2b3a Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Thu, 29 Oct 2026 11:06:11 +0100 Subject: [PATCH] What an engine downloads by a Hugging Face id is asked about too From review. An adapter given by its id is downloaded by vLLM itself, and a TensorRT-LLM model by its id by TensorRT-LLM, past the node's check and not held to safetensors: a model with either is refused when it is added, unless it says allow_pickle, with how to mirror its safetensors instead. vLLM's own model by id stays allowed: the agent gives it --load-format safetensors. A test holds every preset to its own pickle check. Co-Authored-By: Claude Opus 5.5 --- docs/install.md | 6 ++-- internal/control/catalog/pickle.go | 44 +++++++++++++++++++++++++ internal/control/catalog/pickle_test.go | 35 ++++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/docs/install.md b/docs/install.md index 385c698..2edaf67 100644 --- a/docs/install.md +++ b/docs/install.md @@ -892,8 +892,10 @@ files were checked, can say so: ``` A local path is judged by its name as any source is; a local directory is -not looked into, nor a model an engine downloads itself other than by its -Hugging Face id (an adapter by id, TensorRT-LLM's checkpoints). +not looked into. What an engine downloads itself by a Hugging Face id, past +the node — an adapter given by its id, a TensorRT-LLM model's — is refused +when the model is added, unless it says `allow_pickle`: mirror its +safetensors and give that instead. Put them where the node can read them. `scp`, a shared filesystem, or the engine's own downloader are all valid, and which you want depends on how many diff --git a/internal/control/catalog/pickle.go b/internal/control/catalog/pickle.go index 85e0e27..9e40cc1 100644 --- a/internal/control/catalog/pickle.go +++ b/internal/control/catalog/pickle.go @@ -2,7 +2,9 @@ package catalog import ( "fmt" + "path" "slices" + "strings" "gitea.mynodes.xyz/ilker/llmgw/internal/agent/weights" ) @@ -43,5 +45,47 @@ func (m Model) CheckPickle(except ...string) error { "use the model's safetensors or GGUF, or set allow_pickle: true if it was checked and has no other form", m.Name, s) } + // What an engine downloads itself, by a Hugging Face id, no node looks + // at: vLLM reads a model by id as safetensors only, but not an adapter, + // and TensorRT-LLM neither. + for _, s := range m.unlookedSources() { + if slices.Contains(except, s) { + continue + } + return fmt.Errorf("model %q: %s is a Hugging Face id its engine downloads itself, and may load in a pickle "+ + "format, which runs code; mirror its safetensors and give that, or set allow_pickle: true if it was checked", + m.Name, s) + } return nil } + +// unlookedSources are those an engine fetches by a Hugging Face id, past +// the node's check: adapters, and a TensorRT-LLM model's. +func (m Model) unlookedSources() []string { + var out []string + for _, a := range m.Adapters { + if hubID(a.Source) { + out = append(out, a.Source) + } + } + for _, p := range m.Profiles { + if p.Engine == "trtllm" { + if src := m.SourceOf(p); hubID(src) && !slices.Contains(out, src) { + out = append(out, src) + } + } + } + return out +} + +// hubID reports whether a source is a Hugging Face id, org/name, rather +// than a URL, a path or a file. +func hubID(s string) bool { + if s == "" || strings.Contains(s, "://") || strings.HasPrefix(s, "/") || strings.HasPrefix(s, ".") || + strings.HasPrefix(s, "hf:") || strings.Count(s, "/") != 1 || strings.Contains(s, ":") { + return false + } + // A file named so is a file: org/model.gguf is not a repository. + ext := strings.ToLower(path.Ext(s)) + return ext != ".gguf" && ext != ".safetensors" && !weights.Pickled(s) +} diff --git a/internal/control/catalog/pickle_test.go b/internal/control/catalog/pickle_test.go index 1eaaabc..61efeeb 100644 --- a/internal/control/catalog/pickle_test.go +++ b/internal/control/catalog/pickle_test.go @@ -38,3 +38,38 @@ func TestTheTripoSRPresetSaysItsCheckpointIsAllowed(t *testing.T) { t.Errorf("the preset llmgw ships is refused by its own check: %v", err) } } + +func TestWhatAnEngineDownloadsByIDIsAskedAbout(t *testing.T) { + m := Model{Name: "m", Source: "http://mirror/m/v/", Profiles: []Profile{{Name: "gpu", Engine: "vllm"}}, + Adapters: []Adapter{{Name: "a", Source: "someone/lora-x"}}} + if err := m.CheckPickle(); err == nil || !strings.Contains(err.Error(), "someone/lora-x") { + t.Errorf("an adapter by its id: %v", err) + } + m.Adapters = []Adapter{{Name: "a", Source: "http://mirror/a/v/adapter_model.safetensors"}} + if err := m.CheckPickle(); err != nil { + t.Errorf("a mirrored adapter: %v", err) + } + trt := Model{Name: "t", Source: "meta-llama/Llama-3.1-8B-Instruct", Profiles: []Profile{{Name: "gpu", Engine: "trtllm"}}} + if err := trt.CheckPickle(); err == nil { + t.Error("TensorRT-LLM downloading by id was not asked about") + } + vllm := Model{Name: "v", Source: "meta-llama/Llama-3.1-8B-Instruct", Profiles: []Profile{{Name: "gpu", Engine: "vllm"}}} + if err := vllm.CheckPickle(); err != nil { + t.Errorf("vLLM reads a model by id as safetensors only: %v", err) + } + for s, want := range map[string]bool{"Qwen/Qwen2.5-7B": true, "meta-llama/Llama-3.1-8B-Instruct": true, + "org/model.gguf": false, "o/r:Q4_K_M": false, "/models/x": false, "hf:o/r/f.gguf": false} { + if hubID(s) != want { + t.Errorf("hubID(%q) = %v", s, !want) + } + } +} + +func TestEveryPresetPassesItsOwnPickleCheck(t *testing.T) { + // What llmgw ships must be addable: TripoSR was not, once. + for _, p := range Presets() { + if err := p.Model.CheckPickle(); err != nil { + t.Errorf("%s: %v", p.Model.Name, err) + } + } +}