fix: security, concurrency, and correctness hardening

- tools.go: regex-based command filtering, mutex-protected globals, safeJoin fix, grep -- separator
- agent.go: ErrUnknownTool sentinel, toolsMu for thread-safe map access, panic recovery in goroutines, shared maxIterations constant
- types.go: ProviderRegistry thread safety, correct CacheHitRate formula
- sub_agent.go: mutex-protected SubAgentPool
- nvidia.go: Complete() now respects CompletionOptions
- bedrock.go: remove incorrect X-Amz-Target header, remove dead init()
- gemini.go: move API key from URL query to X-Goog-Api-Key header
- vertex.go: remove confusing dead credential file read
- context.go: ceiling division in EstimateTokens, fix slice aliasing in compaction
- openaicompat.go: proper URL hostname check instead of substring match
- openapi/adapter.go: filter path/query params from request body
- mcp/client.go: handle json.Marshal errors
- mcp/transport.go: Wait() on process to prevent zombies
- openairesponses.go: add 120s HTTP timeout
- retry.go: context check before first attempt, nil guard in RetryableError
- sse.go: return error instead of silently breaking on read errors
- mock.go: range over string instead of strings.Split
- azure.go: reuse buffer in SSEDecoder
- provider.go: list valid providers in error message

Author: Patel230

agent.go

@@ -6,6 +6,7 @@ package iteragent
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"strings"
@@ -15,6 +16,11 @@ import (
 	"github.com/GrayCodeAI/iteragent/openapi"
 )
 
+// ErrUnknownTool is returned when a tool call references a tool that is not registered.
+var ErrUnknownTool = errors.New("unknown tool")
+
+const maxAgentIterations = 20
+
 // ToolCall represents a tool invocation.
 type ToolCall struct {
 	Tool string            `json:"tool"`
@@ -57,6 +63,7 @@ type AgentHooks struct {
 type Agent struct {
 	provider      Provider
 	tools         map[string]Tool
+	toolsMu       sync.RWMutex
 	logger        *slog.Logger
 	Events        chan Event
 	SystemPrompt  string
@@ -185,7 +192,7 @@ func (a *Agent) executeToolsSequential(ctx context.Context, calls []ToolCall, it
 	var toolResults strings.Builder
 	for _, call := range calls {
 		result, isError := a.executeSingleTool(ctx, call, iteration, emitFn)
-		if isError && result == fmt.Sprintf("unknown tool: %s", call.Tool) {
+		if isError && errors.Is(isErrorToErr(isError, result), ErrUnknownTool) {
 			toolResults.WriteString(fmt.Sprintf("Tool %s: %s\n", call.Tool, result))
 		} else {
 			toolResults.WriteString(fmt.Sprintf("Tool %s result:\n%s\n\n", call.Tool, result))
@@ -194,6 +201,17 @@ func (a *Agent) executeToolsSequential(ctx context.Context, calls []ToolCall, it
 	return toolResults.String()
 }
 
+// isErrorToErr converts the (result, isErrorBool) pair back to an error for checking.
+func isErrorToErr(isError bool, result string) error {
+	if !isError {
+		return nil
+	}
+	if strings.HasPrefix(result, "unknown tool:") {
+		return ErrUnknownTool
+	}
+	return errors.New(result)
+}
+
 func (a *Agent) executeToolsParallel(ctx context.Context, calls []ToolCall, iteration int, emitFn func(Event)) string {
 	type indexedResult struct {
 		call    ToolCall
@@ -213,11 +231,13 @@ func (a *Agent) executeToolsParallel(ctx context.Context, calls []ToolCall, iter
 				ToolName:   c.Tool,
 				ToolCallID: fmt.Sprintf("%s-%d-%d", c.Tool, iteration, i),
 			})
+			a.toolsMu.RLock()
 			tool, ok := a.tools[c.Tool]
+			a.toolsMu.RUnlock()
 			if !ok {
 				res := fmt.Sprintf("unknown tool: %s", c.Tool)
 				if a.hooks.OnToolEnd != nil {
-					a.hooks.OnToolEnd(c.Tool, res, fmt.Errorf("unknown tool: %s", c.Tool))
+					a.hooks.OnToolEnd(c.Tool, res, ErrUnknownTool)
 				}
 				results[i] = indexedResult{call: c, result: res, isError: true, unknown: true}
 				emitFn(Event{Type: string(EventToolExecutionEnd), ToolName: c.Tool, Result: results[i].result, IsError: true})
@@ -272,7 +292,9 @@ func (a *Agent) executeToolsBatched(ctx context.Context, calls []ToolCall, itera
 // executeSingleTool runs one tool call and emits start/end events.
 // Returns (result string, isError bool).
 func (a *Agent) executeSingleTool(ctx context.Context, call ToolCall, iteration int, emitFn func(Event)) (string, bool) {
+	a.toolsMu.RLock()
 	tool, ok := a.tools[call.Tool]
+	a.toolsMu.RUnlock()
 	if !ok {
 		result := fmt.Sprintf("unknown tool: %s", call.Tool)
 		emitFn(Event{Type: string(EventToolExecutionEnd), ToolName: call.Tool, Result: result, IsError: true})
@@ -330,10 +352,7 @@ func (a *Agent) Run(ctx context.Context, systemPrompt, userMessage string, emitF
 	}
 	userMessage = filtered
 
-	allTools := make([]Tool, 0, len(a.tools))
-	for _, t := range a.tools {
-		allTools = append(allTools, t)
-	}
+	allTools := a.GetTools()
 
 	messages := []Message{
 		{Role: "system", Content: systemPrompt + "\n\n" + ToolDescriptions(allTools)},
@@ -342,8 +361,7 @@ func (a *Agent) Run(ctx context.Context, systemPrompt, userMessage string, emitF
 
 	opts := a.completionOpts()
 
-	const maxIterations = 20
-	for i := 0; i < maxIterations; i++ {
+	for i := 0; i < maxAgentIterations; i++ {
 		a.logger.Info("agent iteration", "step", i+1)
 		emit(Event{Type: string(EventTurnStart), Content: fmt.Sprintf("turn %d", i+1)})
 
@@ -401,7 +419,7 @@ func (a *Agent) Run(ctx context.Context, systemPrompt, userMessage string, emitF
 		emit(Event{Type: string(EventTurnEnd), Content: ""})
 	}
 
-	return "", fmt.Errorf("agent exceeded max iterations (%d)", maxIterations)
+	return "", fmt.Errorf("agent exceeded max iterations (%d)", maxAgentIterations)
 }
 
 func (a *Agent) emit(e Event) {
@@ -517,11 +535,15 @@ func (a *Agent) WithTools(tools []Tool) *Agent {
 	for _, t := range tools {
 		toolMap[t.Name] = t
 	}
+	a.toolsMu.Lock()
 	a.tools = toolMap
+	a.toolsMu.Unlock()
 	return a
 }
 
 func (a *Agent) GetTools() []Tool {
+	a.toolsMu.RLock()
+	defer a.toolsMu.RUnlock()
 	tools := make([]Tool, 0, len(a.tools))
 	for _, t := range a.tools {
 		tools = append(tools, t)
@@ -530,7 +552,9 @@ func (a *Agent) GetTools() []Tool {
 }
 
 func (a *Agent) AddTool(tool Tool) *Agent {
+	a.toolsMu.Lock()
 	a.tools[tool.Name] = tool
+	a.toolsMu.Unlock()
 	return a
 }
 
@@ -601,6 +625,7 @@ func (a *Agent) registerMcpTools(ctx context.Context, adapter *mcp.ToolAdapter)
 	if err != nil {
 		return nil, fmt.Errorf("list mcp tools: %w", err)
 	}
+	a.toolsMu.Lock()
 	for _, t := range tools {
 		execute := t.Execute
 		a.tools[t.Name] = Tool{
@@ -609,6 +634,7 @@ func (a *Agent) registerMcpTools(ctx context.Context, adapter *mcp.ToolAdapter)
 			Execute:     execute,
 		}
 	}
+	a.toolsMu.Unlock()
 	// Track the client so Close() can shut it down.
 	if client := adapter.Client(); client != nil {
 		a.mu.Lock()
@@ -668,6 +694,7 @@ func (a *Agent) registerOpenApiTools(adapter *openapi.Adapter) (*Agent, error) {
 	if err != nil {
 		return nil, fmt.Errorf("list openapi tools: %w", err)
 	}
+	a.toolsMu.Lock()
 	for _, t := range tools {
 		execute := t.Execute
 		a.tools[t.Name] = Tool{
@@ -676,6 +703,7 @@ func (a *Agent) registerOpenApiTools(adapter *openapi.Adapter) (*Agent, error) {
 			Execute:     execute,
 		}
 	}
+	a.toolsMu.Unlock()
 	return a, nil
 }
 
@@ -731,6 +759,9 @@ func (a *Agent) Prompt(ctx context.Context, text string) chan Event {
 	a.pendingWg.Add(1)
 	go func() {
 		defer func() {
+			if r := recover(); r != nil {
+				emitFn(Event{Type: string(EventError), Content: fmt.Sprintf("panic: %v", r), IsError: true})
+			}
 			a.pendingWg.Done()
 			a.mu.Lock()
 			a.isStreaming = false
@@ -776,6 +807,9 @@ func (a *Agent) PromptMessages(ctx context.Context, messages []Message) chan Eve
 	a.pendingWg.Add(1)
 	go func() {
 		defer func() {
+			if r := recover(); r != nil {
+				emitFn(Event{Type: string(EventError), Content: fmt.Sprintf("panic: %v", r), IsError: true})
+			}
 			a.pendingWg.Done()
 			a.mu.Lock()
 			a.isStreaming = false
@@ -812,7 +846,7 @@ func (a *Agent) PromptMessages(ctx context.Context, messages []Message) chan Eve
 
 		opts := a.completionOpts()
 
-		for i := 0; i < 20; i++ {
+		for i := 0; i < maxAgentIterations; i++ {
 			// Check for cancellation before each turn.
 			select {
 			case <-loopCtx.Done():

azure.go

@@ -232,21 +232,21 @@ func (p *AzureOpenAIProvider) Stream(ctx context.Context, config StreamConfig, m
 
 type SSEDecoder struct {
 	reader io.Reader
+	buf    []byte
 }
 
 func NewSSEDecoder(reader io.Reader) *SSEDecoder {
-	return &SSEDecoder{reader: reader}
+	return &SSEDecoder{reader: reader, buf: make([]byte, 1024)}
 }
 
 func (d *SSEDecoder) Decode() (StreamEvent, error) {
 	var line string
 	for {
-		buf := make([]byte, 1024)
-		n, err := d.reader.Read(buf)
+		n, err := d.reader.Read(d.buf)
 		if n == 0 || err != nil {
 			return StreamEvent{}, err
 		}
-		line = string(buf[:n])
+		line = string(d.buf[:n])
 		if strings.HasPrefix(line, "data:") {
 			break
 		}

bedrock.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"os"
 	"strings"
 	"time"
 )
@@ -51,12 +50,8 @@ func (p *BedrockProvider) Complete(ctx context.Context, messages []Message, opts
 		if msg.Role == "system" {
 			systemPrompts = append(systemPrompts, map[string]string{"text": msg.Content})
 		} else {
-			role := msg.Role
-			if role == "assistant" {
-				role = "assistant"
-			}
 			convMessages = append(convMessages, map[string]interface{}{
-				"role": role,
+				"role": msg.Role,
 				"content": []map[string]string{
 					{"text": msg.Content},
 				},
@@ -225,20 +220,14 @@ func (p *BedrockProvider) Stream(ctx context.Context, config StreamConfig, messa
 func (p *BedrockProvider) signRequest(req *http.Request, payload string) {
 	now := time.Now().UTC()
 	date := now.Format("20060102T150405Z")
-	amzDate := os.Getenv("AWS_AMZ_DATE")
-	if amzDate != "" {
-		date = amzDate
-	}
 
 	region := p.config.Region
 	service := "bedrock"
 
 	req.Header.Set("X-Amz-Date", date)
-	req.Header.Set("X-Amz-Target", "AmazonBedrockRuntime.Converse")
 
 	host := req.Host
 	if host == "" {
-		// Strip port if present, use full hostname (not just first segment).
 		host = req.URL.Hostname()
 	}
 
@@ -248,9 +237,8 @@ func (p *BedrockProvider) signRequest(req *http.Request, payload string) {
 		"content-type:application/json",
 		fmt.Sprintf("host:%s", host),
 		fmt.Sprintf("x-amz-date:%s", date),
-		fmt.Sprintf("x-amz-target:AmazonBedrockRuntime.Converse"),
 	}
-	signedHeaders := "content-type;host;x-amz-date;x-amz-target"
+	signedHeaders := "content-type;host;x-amz-date"
 
 	canonicalRequest := strings.Join([]string{
 		"POST",
@@ -291,8 +279,3 @@ func hmacSHA256(key []byte, data string) []byte {
 	h.Write([]byte(data))
 	return h.Sum(nil)
 }
-
-func init() {
-	registry := NewProviderRegistry()
-	registry.Register(ProtocolBedrock, &BedrockProvider{})
-}

context.go

@@ -11,7 +11,10 @@ import (
 const charsPerToken = 4
 
 func EstimateTokens(text string) int {
-	return len(text) / charsPerToken
+	if len(text) == 0 {
+		return 0
+	}
+	return (len(text) + charsPerToken - 1) / charsPerToken
 }
 
 func EstimateMessageTokens(msg Message) int {
@@ -343,7 +346,7 @@ func CompactMessagesTiered(messages []Message, cfg ContextConfig) []Message {
 
 	head := compacted[:keepFirst]
 	tail := compacted[len(compacted)-keepRecent:]
-	return append(head, tail...)
+	return append(head[:len(head):len(head)], tail...)
 }
 
 type MessageSummary struct {

context_test.go

@@ -259,10 +259,10 @@ func TestContextTracker_CacheHitRate(t *testing.T) {
 	}
 	ct.UpdateWithRealUsage(usage)
 
-	// CacheHitRate = CacheRead / (InputTokens + CacheRead + CacheWrite) = 40/100 = 0.4
+	// CacheHitRate = CacheRead / InputTokens = 40/60 ≈ 0.667
 	got := ct.CacheHitRate()
-	if got < 0.39 || got > 0.41 {
-		t.Errorf("expected ~0.4 cache hit rate, got %f", got)
+	if got < 0.66 || got > 0.68 {
+		t.Errorf("expected ~0.667 cache hit rate, got %f", got)
 	}
 }
 

gemini.go

@@ -117,12 +117,13 @@ func (p *geminiProvider) CompleteStream(ctx context.Context, messages []Message,
 	}
 
 	streamURL := fmt.Sprintf(
-		"https://generativelanguage.googleapis.com/v1beta/models/%s:streamGenerateContent?key=%s&alt=sse",
-		p.cfg.Model, p.cfg.APIKey)
+		"https://generativelanguage.googleapis.com/v1beta/models/%s:streamGenerateContent?alt=sse",
+		p.cfg.Model)
 
 	var full strings.Builder
 	sseClient := NewSSEClient()
-	err = sseClient.Stream(ctx, streamURL, nil, body, func(e SSEEvent) {
+	headers := map[string]string{"X-Goog-Api-Key": p.cfg.APIKey}
+	err = sseClient.Stream(ctx, streamURL, headers, body, func(e SSEEvent) {
 		if token, ok := ParseGeminiSSE(e.Data); ok && token != "" {
 			full.WriteString(token)
 			if onToken != nil {
@@ -151,12 +152,13 @@ func (p *geminiProvider) Complete(ctx context.Context, messages []Message, opts
 		return "", fmt.Errorf("marshal request: %w", err)
 	}
 
-	url := fmt.Sprintf("https://generativelanguage.googleapis.com/v1beta/models/%s:generateContent?key=%s", p.cfg.Model, p.cfg.APIKey)
+	url := fmt.Sprintf("https://generativelanguage.googleapis.com/v1beta/models/%s:generateContent", p.cfg.Model)
 	req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(body))
 	if err != nil {
 		return "", fmt.Errorf("create request: %w", err)
 	}
 	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("X-Goog-Api-Key", p.cfg.APIKey)
 
 	resp, err := p.client.Do(req)
 	if err != nil {