Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/openclaw/openclaw.go
Original file line number Diff line number Diff line change
Expand Up @@ -1922,8 +1922,8 @@ func patchOverlayModelList(content string, models []string) (string, bool) {
} else {
result = append(result, indent+"models:")
for _, m := range models {
result = append(result, indent+" - id: "+m)
result = append(result, indent+" name: "+ollamaModelDisplayName(m))
result = append(result, indent+" - id: "+yamlScalar(m))
result = append(result, indent+" name: "+yamlScalar(ollamaModelDisplayName(m)))
}
}

Expand Down Expand Up @@ -2070,7 +2070,7 @@ models:
b.WriteString(" models:\n")

for _, m := range ollamaModels {
fmt.Fprintf(&b, " - id: %s\n name: %s\n", m, ollamaModelDisplayName(m))
fmt.Fprintf(&b, " - id: %s\n name: %s\n", yamlScalar(m), yamlScalar(ollamaModelDisplayName(m)))
}
} else {
b.WriteString(" models: []\n")
Expand Down
85 changes: 80 additions & 5 deletions internal/openclaw/overlay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/ObolNetwork/obol-stack/internal/config"
"gopkg.in/yaml.v3"
)

// testConfig creates a temp config dir with a .stack-id file for testing.
Expand Down Expand Up @@ -167,15 +168,53 @@ func TestGenerateOverlayValues_OllamaDefaultWithModels(t *testing.T) {
t.Errorf("default overlay missing LiteLLM baseUrl, got:\n%s", yaml)
}

if !strings.Contains(yaml, "id: llama3.2:3b") {
if !strings.Contains(yaml, `id: "llama3.2:3b"`) {
t.Errorf("default overlay missing first model, got:\n%s", yaml)
}

if !strings.Contains(yaml, "id: mistral:7b") {
if !strings.Contains(yaml, `id: "mistral:7b"`) {
t.Errorf("default overlay missing second model, got:\n%s", yaml)
}
}

// TestGenerateOverlayValues_OllamaModelNameInjection is a #775-style
// regression test for a listOllamaModels() name (or its derived display
// name) breaking out of the hand-formatted "- id: %s\n name: %s\n" lines
// and injecting new YAML keys into the overlay applied via `helmfile sync`.
func TestGenerateOverlayValues_OllamaModelNameInjection(t *testing.T) {
malicious := "x\nimage:\n repository: pwned-by-ollama\nrbac:\n create: false"

rendered := generateOverlayValues(testConfig(t), "openclaw-default.obol.stack", nil, false, []string{malicious}, "")

var doc map[string]any
if err := yaml.Unmarshal([]byte(rendered), &doc); err != nil {
t.Fatalf("rendered overlay is not valid YAML: %v\n%s", err, rendered)
}

if img, ok := doc["image"].(map[string]any); ok {
if _, hasRepo := img["repository"]; hasRepo {
t.Errorf("attacker injected an 'image.repository' key: %#v\nrendered:\n%s", img, rendered)
}
}

modelsSection, _ := doc["models"].(map[string]any)
openai, _ := modelsSection["openai"].(map[string]any)

modelList, _ := openai["models"].([]any)
if len(modelList) != 1 {
t.Fatalf("expected 1 model entry, got %d\nrendered:\n%s", len(modelList), rendered)
}

entry, _ := modelList[0].(map[string]any)
if entry["id"] != malicious {
t.Errorf("id = %#v, want single scalar %q (no injected keys); rendered:\n%s", entry["id"], malicious, rendered)
}

if entry["name"] != ollamaModelDisplayName(malicious) {
t.Errorf("name = %#v, want single scalar %q; rendered:\n%s", entry["name"], ollamaModelDisplayName(malicious), rendered)
}
}

func TestGenerateOverlayValues_OllamaDefaultNoModels(t *testing.T) {
// When no Ollama models are available, overlay should have empty model list
yaml := generateOverlayValues(testConfig(t), "openclaw-default.obol.stack", nil, false, nil, "")
Expand Down Expand Up @@ -380,11 +419,11 @@ erpc:
t.Fatal("expected change")
}

if !strings.Contains(updated, "id: claude-sonnet-4-5-20250929") {
if !strings.Contains(updated, `id: "claude-sonnet-4-5-20250929"`) {
t.Errorf("missing claude model in updated overlay:\n%s", updated)
}

if !strings.Contains(updated, "id: gpt-4o") {
if !strings.Contains(updated, `id: "gpt-4o"`) {
t.Errorf("missing gpt model in updated overlay:\n%s", updated)
}
// eRPC section should still be present
Expand Down Expand Up @@ -427,10 +466,46 @@ erpc:
t.Fatal("expected change")
}

if !strings.Contains(updated, "id: llama3.2:3b") {
if !strings.Contains(updated, `id: "llama3.2:3b"`) {
t.Errorf("missing model in updated overlay:\n%s", updated)
}
})

// #775-style regression: a malicious/malformed Ollama model name must not
// break out of the " - id: "+m hand-formatted line and inject new keys
// into the overlay applied via `helmfile sync`.
t.Run("model name injection", func(t *testing.T) {
malicious := "x\nimage:\n repository: pwned-by-ollama\nrbac:\n create: false"

updated, changed := patchOverlayModelList(overlay, []string{malicious})
if !changed {
t.Fatal("expected change")
}

var doc map[string]any
if err := yaml.Unmarshal([]byte(updated), &doc); err != nil {
t.Fatalf("patched overlay is not valid YAML: %v\n%s", err, updated)
}

if img, ok := doc["image"].(map[string]any); ok {
if _, hasRepo := img["repository"]; hasRepo {
t.Errorf("attacker injected an 'image.repository' key: %#v\nupdated:\n%s", img, updated)
}
}

modelsSection, _ := doc["models"].(map[string]any)
openai, _ := modelsSection["openai"].(map[string]any)

modelList, _ := openai["models"].([]any)
if len(modelList) != 1 {
t.Fatalf("expected 1 model entry, got %d\nupdated:\n%s", len(modelList), updated)
}

entry, _ := modelList[0].(map[string]any)
if entry["id"] != malicious {
t.Errorf("id = %#v, want single scalar %q (no injected keys); updated:\n%s", entry["id"], malicious, updated)
}
})
}

func TestPatchAgentModelsJSON(t *testing.T) {
Expand Down