v0.54.1 — SessionResolver path traversal guard + tool panic recovery

Author: molty3000

cmd/odek/odek.json

@@ -0,0 +1 @@
+{"skills": {"verbose": true, "auto_save": {"enabled": true, "require_llm": false}, "llm_learn": false}}

internal/loop/loop.go

@@ -779,11 +779,24 @@ if e.approver != nil && len(result.ToolCalls) > 1 {
 						if ctxTool, ok := t.(interface{ SetContext(context.Context) }); ok {
 							ctxTool.SetContext(ctx)
 						}
-						res, err := t.Call(tcRef.Function.Arguments)
-						if err != nil {
-							output = fmt.Sprintf("error: %s", err.Error())
-						} else {
-							output = redact.RedactSecrets(res)
+						// Capture any panic from the tool so it does not kill the agent.
+						var toolPanicked bool
+						func() {
+							defer func() {
+								if r := recover(); r != nil {
+									output = fmt.Sprintf("error: tool %q panicked: %v", tcRef.Function.Name, r)
+									toolPanicked = true
+								}
+							}()
+							res, err := t.Call(tcRef.Function.Arguments)
+							if err != nil {
+								output = fmt.Sprintf("error: %s", err.Error())
+							} else {
+								output = redact.RedactSecrets(res)
+							}
+						}()
+						if toolPanicked {
+							return
 						}
 					}
 					results[idx] = execResult{output: output}

internal/loop/loop_test.go

@@ -1908,3 +1908,48 @@ func TestEngine_InteractionModeDefault_ProducesRenderOutput(t *testing.T) {
 	}
 }
 
+
+// ── Bug #8: Tool goroutines lack panic recovery ─────────────────────────
+// If a tool.Call() panics, the agent should not crash.
+
+type panicTool struct {
+	name string
+}
+
+func (p *panicTool) Name() string        { return p.name }
+func (p *panicTool) Description() string  { return "panics on call" }
+func (p *panicTool) Schema() any          { return map[string]any{"type": "object"} }
+func (p *panicTool) Call(args string) (string, error) {
+	panic("deliberate panic from tool")
+}
+
+// TestToolPanic_DoesNotKillAgent verifies that a panicking tool call
+// does not crash the agent. The agent should recover and continue.
+func TestToolPanic_DoesNotKillAgent(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var body struct {
+			Messages []llm.Message `json:"messages"`
+		}
+		if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+			t.Fatal(err)
+		}
+		// First call: return tool calls with panic tool
+		if len(body.Messages) == 2 {
+			fmt.Fprint(w, `{"choices":[{"index":0,"message":{"role":"assistant","content":"","tool_calls":[{"id":"call_1","type":"function","function":{"name":"panic_tool","arguments":"{}"}}]},"finish_reason":"tool_calls"}]}`)
+			return
+		}
+		// Second call (after tool panic): return content
+		fmt.Fprint(w, `{"choices":[{"index":0,"message":{"role":"assistant","content":"Agent survived the panic!"},"finish_reason":"stop"}]}`)
+	}))
+	defer server.Close()
+
+	client := llm.New(server.URL, "sk-test", "test-model", "", 0)
+	engine := New(client, tool.NewRegistry([]tool.Tool{&panicTool{name: "panic_tool"}}), 10, "", nil, 0)
+	result, err := engine.Run(context.Background(), "test task")
+	if err != nil {
+		t.Fatalf("engine.Run should not crash on tool panic: %v", err)
+	}
+	if !strings.Contains(result, "Agent survived") {
+		t.Errorf("agent result = %q, want 'Agent survived'", result)
+	}
+}

internal/resource/resource.go

@@ -17,6 +17,8 @@ import (
 	"strings"
 	"syscall"
 	"time"
+
+	"github.com/BackendStack21/odek/internal/session"
 )
 
 // Resource is a discovered resource returned by a Resolver.
@@ -384,6 +386,10 @@ func (s *SessionResolver) Search(ctx context.Context, query string, limit int) (
 
 func (s *SessionResolver) Load(ctx context.Context, id string) (string, error) {
 	// id is the session ID (without "sess:" prefix)
+	// Security: validate session ID to prevent path traversal.
+	if err := session.ValidateSessionID(id); err != nil {
+		return "", fmt.Errorf("resource: invalid session ID: %w", err)
+	}
 	data, err := os.ReadFile(filepath.Join(s.dir, id+".json"))
 	if err != nil {
 		return "", err

internal/resource/resource_test.go

@@ -610,3 +610,42 @@ func (e *emptyResolver) Search(ctx context.Context, query string, limit int) ([]
 func (e *emptyResolver) Load(ctx context.Context, id string) (string, error) {
 	return "", os.ErrNotExist
 }
+
+// ── Bug #7: SessionResolver.Load path traversal ─────────────────────────
+
+func TestSessionResolverLoad_PathTraversal(t *testing.T) {
+	// Create a temp dir for sessions
+	dir := t.TempDir()
+
+	// Create a real session file
+	sessionID := "my-valid-session"
+	if err := os.WriteFile(filepath.Join(dir, sessionID+".json"), []byte(`{"id":"valid"}`), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	resolver := &SessionResolver{dir: dir}
+
+	// Valid session ID should succeed
+	content, err := resolver.Load(context.Background(), sessionID)
+	if err != nil {
+		t.Fatalf("Load with valid session failed: %v", err)
+	}
+	if !strings.Contains(content, "valid") {
+		t.Errorf("Load with valid session returned wrong content: %s", content)
+	}
+
+	// Path traversal attempts should FAIL
+	traversalAttempts := []string{
+		"../../etc/passwd",
+		"..%2f..%2fetc/passwd",
+		"/etc/passwd",
+		"../other-session",
+		"foo/../../etc/passwd",
+	}
+	for _, attempt := range traversalAttempts {
+		_, err := resolver.Load(context.Background(), attempt)
+		if err == nil {
+			t.Errorf("Load with path traversal %q should have failed but succeeded (security bug)", attempt)
+		}
+	}
+}