Skip to content

fix(openclaw): escape untrusted import fields before writing overlay YAML #775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bussyjd merged 1 commit into from
Jul 16, 2026
Merged

fix(openclaw): escape untrusted import fields before writing overlay YAML #775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 27 additions & 6 deletions internal/openclaw/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

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

// API key environment variable names for known providers.
Expand Down Expand Up @@ -217,14 +218,14 @@ func TranslateToOverlayYAML(result *ImportResult) string {
var b strings.Builder

if result.AgentModel != "" {
fmt.Fprintf(&b, "openclaw:\n agentModel: %s\n\n", result.AgentModel)
fmt.Fprintf(&b, "openclaw:\n agentModel: %s\n\n", yamlScalar(result.AgentModel))
}

if len(result.Providers) > 0 {
b.WriteString("models:\n")

for _, p := range result.Providers {
fmt.Fprintf(&b, " %s:\n", p.Name)
fmt.Fprintf(&b, " %s:\n", yamlScalar(p.Name))

if p.Disabled {
b.WriteString(" enabled: false\n")
Expand All @@ -234,7 +235,7 @@ func TranslateToOverlayYAML(result *ImportResult) string {
b.WriteString(" enabled: true\n")

if p.BaseURL != "" {
fmt.Fprintf(&b, " baseUrl: %s\n", p.BaseURL)
fmt.Fprintf(&b, " baseUrl: %s\n", yamlScalar(p.BaseURL))
}
// Always emit api to override any stale base chart value.
// Empty string makes the Helm template omit it from JSON,
Expand All @@ -246,17 +247,17 @@ func TranslateToOverlayYAML(result *ImportResult) string {
}

if p.APIKeyEnvVar != "" {
fmt.Fprintf(&b, " apiKeyEnvVar: %s\n", p.APIKeyEnvVar)
fmt.Fprintf(&b, " apiKeyEnvVar: %s\n", yamlScalar(p.APIKeyEnvVar))
}

if len(p.Models) > 0 {
b.WriteString(" models:\n")

for _, m := range p.Models {
fmt.Fprintf(&b, " - id: %s\n", m.ID)
fmt.Fprintf(&b, " - id: %s\n", yamlScalar(m.ID))

if m.Name != "" {
fmt.Fprintf(&b, " name: %s\n", m.Name)
fmt.Fprintf(&b, " name: %s\n", yamlScalar(m.Name))
}
}
}
Expand Down Expand Up @@ -457,3 +458,23 @@ func extractEnvVarName(s string) (string, bool) {
func isEnvVarRef(s string) bool {
return strings.Contains(s, "${")
}

// yamlScalar renders s as a double-quoted YAML scalar so it can be safely
// interpolated into the hand-built overlay YAML. Values sourced from an
// imported ~/.openclaw/openclaw.json are untrusted: without this, an
// embedded newline (e.g. "x\nimage:\n repository: evil") would let the
// string break out of its field and inject new keys into values-obol.yaml,
// which is later applied to the cluster via `helmfile sync`.
func yamlScalar(s string) string {
node := yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: s, Style: yaml.DoubleQuotedStyle}

out, err := yaml.Marshal(&node)
if err != nil {
// Unreachable for a plain string scalar; fall back to a manual
// double-quote/escape rather than emit the untrusted value raw.
r := strings.NewReplacer(`\`, `\\`, `"`, `\"`, "\n", `\n`, "\r", `\r`, "\t", `\t`)
return `"` + r.Replace(s) + `"`
}

return strings.TrimSuffix(string(out), "\n")
}
96 changes: 87 additions & 9 deletions internal/openclaw/import_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"path/filepath"
"strings"
"testing"

"gopkg.in/yaml.v3"
)

func TestIsEnvVarRef(t *testing.T) {
Expand Down Expand Up @@ -226,7 +228,7 @@ func TestTranslateToOverlayYAML_AgentModelOnly(t *testing.T) {
}

got := TranslateToOverlayYAML(result)
if !strings.Contains(got, "agentModel: claude-sonnet-4-6") {
if !strings.Contains(got, `agentModel: "claude-sonnet-4-6"`) {
t.Errorf("YAML missing agentModel, got:\n%s", got)
}

Expand All @@ -252,11 +254,11 @@ func TestTranslateToOverlayYAML_ProviderWithModels(t *testing.T) {
got := TranslateToOverlayYAML(result)

checks := []string{
"anthropic:\n enabled: true",
"baseUrl: https://api.anthropic.com",
"\"anthropic\":\n enabled: true",
`baseUrl: "https://api.anthropic.com"`,
"api: anthropic-messages",
"- id: claude-sonnet-4-6",
"name: Claude Sonnet 4.6",
`- id: "claude-sonnet-4-6"`,
`name: "Claude Sonnet 4.6"`,
}
for _, check := range checks {
if !strings.Contains(got, check) {
Expand All @@ -273,7 +275,7 @@ func TestTranslateToOverlayYAML_DisabledProvider(t *testing.T) {
}
got := TranslateToOverlayYAML(result)

if !strings.Contains(got, "openai:\n enabled: false") {
if !strings.Contains(got, "\"openai\":\n enabled: false") {
t.Errorf("YAML missing disabled openai, got:\n%s", got)
}

Expand Down Expand Up @@ -346,15 +348,15 @@ func TestTranslateToOverlayYAML_FullConfig(t *testing.T) {
}
got := TranslateToOverlayYAML(result)

if !strings.Contains(got, "agentModel: claude-sonnet-4-6") {
if !strings.Contains(got, `agentModel: "claude-sonnet-4-6"`) {
t.Errorf("YAML missing agentModel, got:\n%s", got)
}

if !strings.Contains(got, "anthropic:\n enabled: true") {
if !strings.Contains(got, "\"anthropic\":\n enabled: true") {
t.Errorf("YAML missing enabled anthropic, got:\n%s", got)
}

if !strings.Contains(got, "openai:\n enabled: false") {
if !strings.Contains(got, "\"openai\":\n enabled: false") {
t.Errorf("YAML missing disabled openai, got:\n%s", got)
}

Expand Down Expand Up @@ -617,3 +619,79 @@ func TestDetectExistingConfigAt_EmptyConfig(t *testing.T) {
t.Errorf("AgentModel = %q, want empty", result.AgentModel)
}
}

// TestYAMLScalar_EscapesInjection is a Canary402 full-surface audit
// regression test: values imported from ~/.openclaw/openclaw.json must never
// be able to inject additional YAML keys (e.g. overriding `image:` to
// achieve RCE via `helmfile sync`) when hand-formatted into the overlay
// values file.
func TestYAMLScalar_EscapesInjection(t *testing.T) {
tests := []struct {
name string
in string
}{
{"plain", "openai/claude-sonnet-4-6"},
{"newline_key_injection", "x\nimage:\n repository: evil"},
{"colon", "has: colon"},
{"quotes_and_backslash", `back\slash"quote`},
{"empty", ""},
{"trailing_newline_injection", "gpt-5.2\nrbac:\n create: false"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
line := "key: " + yamlScalar(tt.in) + "\n"

var doc map[string]any
if err := yaml.Unmarshal([]byte(line), &doc); err != nil {
t.Fatalf("yamlScalar(%q) produced invalid YAML %q: %v", tt.in, line, err)
}

if got, ok := doc["key"].(string); !ok || got != tt.in {
t.Errorf("round-trip = %#v, want single scalar %q (no injected keys); rendered: %q", doc["key"], tt.in, line)
}

if len(doc) != 1 {
t.Errorf("doc has %d top-level keys, want 1 (injection leaked extra keys): %#v", len(doc), doc)
}
})
}
}

// TestGenerateOverlayValues_RejectsYAMLInjection feeds generateOverlayValues
// a malicious imported agentModel and provider name (newline + "image:")
// and asserts the rendered values-obol.yaml parses to the SAME single
// scalar values, with no injected top-level "image" key — i.e. the fix for
// the confirmed Canary402 full-surface audit finding holds end-to-end.
func TestGenerateOverlayValues_RejectsYAMLInjection(t *testing.T) {
maliciousModel := "x\nimage:\n repository: pwned-by-canary402\nrbac:\n create: false"
maliciousProvider := "ollama\nimage:\n repository: pwned-by-canary402"

imported := &ImportResult{
AgentModel: maliciousModel,
Providers: []ImportedProvider{
{Name: maliciousProvider, BaseURL: "http://localhost:11434"},
},
}

rendered := generateOverlayValues(testConfig(t), "openclaw-default.obol.stack", imported, false, nil, "")

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)
}

// The chart-default image override is legitimate and expected to stay a
// map with only "tag" — injection would add a "repository" key to it.
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)
}
}

openclawSection, _ := doc["openclaw"].(map[string]any)
if openclawSection == nil || openclawSection["agentModel"] != "openai/"+maliciousModel {
t.Errorf("agentModel = %#v, want single scalar %q (rewritten with openai/ prefix); rendered:\n%s",
openclawSection["agentModel"], "openai/"+maliciousModel, rendered)
}
}
2 changes: 1 addition & 1 deletion internal/openclaw/openclaw.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ rbac:
b.WriteString("openclaw:\n")

if agentModel != "" {
fmt.Fprintf(&b, " agentModel: %s\n", agentModel)
fmt.Fprintf(&b, " agentModel: %s\n", yamlScalar(agentModel))
}

b.WriteString(` gateway:
Expand Down
16 changes: 8 additions & 8 deletions internal/openclaw/overlay_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,21 +111,21 @@ func TestOverlayYAML_LiteLLMRouted(t *testing.T) {
yaml := TranslateToOverlayYAML(result)

// Agent model should have ollama/ prefix
if !strings.Contains(yaml, "agentModel: openai/claude-sonnet-4-5-20250929") {
if !strings.Contains(yaml, `agentModel: "openai/claude-sonnet-4-5-20250929"`) {
t.Errorf("YAML missing agentModel, got:\n%s", yaml)
}

// openai should be enabled with LiteLLM baseUrl
if !strings.Contains(yaml, "openai:\n enabled: true") {
if !strings.Contains(yaml, "\"openai\":\n enabled: true") {
t.Errorf("YAML missing enabled openai provider, got:\n%s", yaml)
}

if !strings.Contains(yaml, "baseUrl: http://litellm.llm.svc.cluster.local:4000/v1") {
if !strings.Contains(yaml, `baseUrl: "http://litellm.llm.svc.cluster.local:4000/v1"`) {
t.Errorf("YAML missing LiteLLM baseUrl, got:\n%s", yaml)
}

// apiKeyEnvVar should be OPENAI_API_KEY (LiteLLM master key injected via this env var)
if !strings.Contains(yaml, "apiKeyEnvVar: OPENAI_API_KEY") {
if !strings.Contains(yaml, `apiKeyEnvVar: "OPENAI_API_KEY"`) {
t.Errorf("YAML missing apiKeyEnvVar, got:\n%s", yaml)
}

Expand All @@ -140,16 +140,16 @@ func TestOverlayYAML_LiteLLMRouted(t *testing.T) {
}

// Cloud model should appear in ollama's model list
if !strings.Contains(yaml, "- id: claude-sonnet-4-5-20250929") {
if !strings.Contains(yaml, `- id: "claude-sonnet-4-5-20250929"`) {
t.Errorf("YAML missing cloud model ID, got:\n%s", yaml)
}

// anthropic and ollama should be disabled
if !strings.Contains(yaml, "anthropic:\n enabled: false") {
if !strings.Contains(yaml, "\"anthropic\":\n enabled: false") {
t.Errorf("YAML missing disabled anthropic, got:\n%s", yaml)
}

if !strings.Contains(yaml, "ollama:\n enabled: false") {
if !strings.Contains(yaml, "\"ollama\":\n enabled: false") {
t.Errorf("YAML missing disabled ollama, got:\n%s", yaml)
}
}
Expand All @@ -159,7 +159,7 @@ func TestGenerateOverlayValues_OllamaDefaultWithModels(t *testing.T) {
models := []string{"llama3.2:3b", "mistral:7b"}
yaml := generateOverlayValues(testConfig(t), "openclaw-default.obol.stack", nil, false, models, "")

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

Expand Down