From 9f6b5a73a5ad7e065614c9afed0d226bb5ece387 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Wed, 18 Feb 2026 15:31:32 +0400 Subject: [PATCH] fix: support keyless Ollama and harden exec working_dir checks --- pkg/providers/factory.go | 30 ++++++++++++++++++++---------- pkg/providers/factory_test.go | 18 ++++++++++++++++++ pkg/tools/shell.go | 30 +++++++++++++++++++++++++++--- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/pkg/providers/factory.go b/pkg/providers/factory.go index b6f1b5e21..efae2941d 100644 --- a/pkg/providers/factory.go +++ b/pkg/providers/factory.go @@ -25,14 +25,15 @@ const ( ) type providerSelection struct { - providerType providerType - apiKey string - apiBase string - proxy string - model string - workspace string - connectMode string - enableWebSearch bool + providerType providerType + apiKey string + apiBase string + proxy string + model string + workspace string + connectMode string + enableWebSearch bool + allowEmptyAPIKey bool } func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { @@ -172,6 +173,14 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { sel.model = "deepseek-chat" } } + case "ollama": + sel.apiKey = cfg.Providers.Ollama.APIKey + sel.apiBase = cfg.Providers.Ollama.APIBase + sel.proxy = cfg.Providers.Ollama.Proxy + if sel.apiBase == "" { + sel.apiBase = "http://localhost:11434/v1" + } + sel.allowEmptyAPIKey = true case "github_copilot", "copilot": sel.providerType = providerTypeGitHubCopilot if cfg.Providers.GitHubCopilot.APIBase != "" { @@ -268,13 +277,14 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { if sel.apiBase == "" { sel.apiBase = "https://integrate.api.nvidia.com/v1" } - case (strings.Contains(lowerModel, "ollama") || strings.HasPrefix(model, "ollama/")) && cfg.Providers.Ollama.APIKey != "": + case (strings.Contains(lowerModel, "ollama") || strings.HasPrefix(model, "ollama/")) && (cfg.Providers.Ollama.APIBase != "" || cfg.Providers.Ollama.APIKey != ""): sel.apiKey = cfg.Providers.Ollama.APIKey sel.apiBase = cfg.Providers.Ollama.APIBase sel.proxy = cfg.Providers.Ollama.Proxy if sel.apiBase == "" { sel.apiBase = "http://localhost:11434/v1" } + sel.allowEmptyAPIKey = true case cfg.Providers.VLLM.APIBase != "": sel.apiKey = cfg.Providers.VLLM.APIKey sel.apiBase = cfg.Providers.VLLM.APIBase @@ -295,7 +305,7 @@ func resolveProviderSelection(cfg *config.Config) (providerSelection, error) { } if sel.providerType == providerTypeHTTPCompat { - if sel.apiKey == "" && !strings.HasPrefix(model, "bedrock/") { + if sel.apiKey == "" && !strings.HasPrefix(model, "bedrock/") && !sel.allowEmptyAPIKey { return providerSelection{}, fmt.Errorf("no API key configured for provider (model: %s)", model) } if sel.apiBase == "" { diff --git a/pkg/providers/factory_test.go b/pkg/providers/factory_test.go index 5680f23b3..942b72c90 100644 --- a/pkg/providers/factory_test.go +++ b/pkg/providers/factory_test.go @@ -135,6 +135,24 @@ func TestResolveProviderSelection(t *testing.T) { wantType: providerTypeHTTPCompat, wantAPIBase: "http://localhost:11434/v1", }, + { + name: "explicit ollama provider allows empty key and uses default base", + setup: func(cfg *config.Config) { + cfg.Agents.Defaults.Provider = "ollama" + cfg.Agents.Defaults.Model = "qwen2.5:14b" + }, + wantType: providerTypeHTTPCompat, + wantAPIBase: "http://localhost:11434/v1", + }, + { + name: "ollama model allows api base without key", + setup: func(cfg *config.Config) { + cfg.Agents.Defaults.Model = "ollama/qwen2.5:14b" + cfg.Providers.Ollama.APIBase = "http://localhost:11434/v1" + }, + wantType: providerTypeHTTPCompat, + wantAPIBase: "http://localhost:11434/v1", + }, { name: "moonshot model keeps proxy and default base", setup: func(cfg *config.Config) { diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index a1ee0b6e1..a35147791 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -142,7 +142,8 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult return ErrorResult("command is required") } - cwd := t.workingDir + workspaceRoot := t.workingDir + cwd := workspaceRoot if wd, ok := args["working_dir"].(string); ok && wd != "" { if t.restrictToWorkspace && t.workingDir != "" { resolvedWD, err := validatePath(wd, t.workingDir, true) @@ -150,6 +151,10 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult return ErrorResult("Command blocked by safety guard (" + err.Error() + ")") } cwd = resolvedWD + } else if filepath.IsAbs(wd) { + cwd = wd + } else if workspaceRoot != "" { + cwd = filepath.Join(workspaceRoot, wd) } else { cwd = wd } @@ -162,6 +167,20 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult } } + if absCwd, err := filepath.Abs(cwd); err == nil { + cwd = absCwd + } + + if t.restrictToWorkspace && workspaceRoot != "" { + absWorkspace, err := filepath.Abs(workspaceRoot) + if err == nil { + rel, err := filepath.Rel(absWorkspace, cwd) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return ErrorResult("working_dir must be within workspace") + } + } + } + if guardError := t.guardCommand(command, cwd); guardError != "" { return ErrorResult(guardError) } @@ -285,7 +304,12 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "Command blocked by safety guard (path traversal detected)" } - cwdPath, err := filepath.Abs(cwd) + basePath := cwd + if t.workingDir != "" { + basePath = t.workingDir + } + + basePath, err := filepath.Abs(basePath) if err != nil { return "" } @@ -299,7 +323,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string { continue } - rel, err := filepath.Rel(cwdPath, p) + rel, err := filepath.Rel(basePath, p) if err != nil { continue }