# GopherGate Remediation Action Plan

Based on the findings from the [Code Review Report](file:///home/newkirk/Projects/gophergate/code_review.md), this document outlines a phased execution plan to fix all security, concurrency, reliability, and code quality issues in **GopherGate**.

---

## Phase 1: Critical Security & Crash Fixes (Immediate)

### Task 1.1: Redact API Keys in Gemini Provider Logs
- **Target File:** [`internal/providers/gemini.go`](file:///home/newkirk/Projects/gophergate/internal/providers/gemini.go#L378)
- **Problem:** `fmt.Printf("[Gemini] POST %s\n", url)` logs the raw request URL containing `?key=AIzaSy...`, leaking API secrets to logs.
- **Action Items:**
  1. Create a helper function `sanitizeURL(rawURL string) string` in `internal/providers/helpers.go` to strip or replace sensitive query parameters (`key=...`).
  2. Replace all instances of raw URL printing in `gemini.go` (L378, L636) with `log.Printf("[Gemini] POST %s", sanitizeURL(url))`.

### Task 1.2: Add Thread Safety to Server Providers Map
- **Target File:** [`internal/server/server.go`](file:///home/newkirk/Projects/gophergate/internal/server/server.go#L30)
- **Problem:** `s.providers` map is mutated during `RefreshProviders()` without mutex locking while HTTP handlers read from it concurrently (`selectProvider()`).
- **Action Items:**
  1. Add `providersMu sync.RWMutex` field to `Server` struct in `server.go`.
  2. Acquire `s.providersMu.Lock()` before modifying `s.providers` in `RefreshProviders()`.
  3. Acquire `s.providersMu.RLock()` in `selectProvider()` and deferred `s.providersMu.RUnlock()`.

---

## Phase 2: Thread-Safety & Race Condition Hardening

### Task 2.1: Synchronize `ModelRegistry` Lookups and Reloads
- **Target Files:** [`internal/models/registry.go`](file:///home/newkirk/Projects/gophergate/internal/models/registry.go#L24), [`internal/server/server.go`](file:///home/newkirk/Projects/gophergate/internal/server/server.go#L56)
- **Problem:** Background goroutines update `s.registry` while concurrent requests access `r.FindModel()`.
- **Action Items:**
  1. Add `sync.RWMutex` to `ModelRegistry` struct in `registry.go`.
  2. Wrap `FindModel()` methods with `r.mu.RLock()` and `r.mu.RUnlock()`.
  3. Alternatively, implement `atomic.Pointer[models.ModelRegistry]` in `Server` to enable lock-free atomic pointer swaps on registry refresh.

### Task 2.2: Synchronize `Router.Reload()`
- **Target File:** [`internal/router/router.go`](file:///home/newkirk/Projects/gophergate/internal/router/router.go#L34)
- **Problem:** `Router.Reload()` replaces `r.groups` map without holding a mutex lock during active routing requests.
- **Action Items:**
  1. Add `mu sync.RWMutex` to `Router` struct in `router.go`.
  2. Acquire `r.mu.RLock()` during `Route()`, `RouteToConcrete()`, and `IsGroup()`.
  3. Acquire `r.mu.Lock()` during `Reload()`.

---

## Phase 3: Reliability & Stream Fault Tolerance

### Task 3.1: Enable Circuit Breaker Protection for Streaming Endpoints
- **Target File:** [`internal/providers/circuit_breaker.go`](file:///home/newkirk/Projects/gophergate/internal/providers/circuit_breaker.go#L52)
- **Problem:** `ChatCompletionStream` and `ResponsesStream` bypass `gobreaker` entirely.
- **Action Items:**
  1. Wrap the initial `ChatCompletionStream` call inside `cb.Execute()`.
  2. Implement stream response wrapper that monitors streaming errors and reports failure back to the circuit breaker state tracker.

### Task 3.2: Prevent Goroutine Leaks on Client Disconnect
- **Target Files:** [`internal/providers/helpers.go`](file:///home/newkirk/Projects/gophergate/internal/providers/helpers.go#L379), [`internal/providers/deepseek.go`](file:///home/newkirk/Projects/gophergate/internal/providers/deepseek.go#L226)
- **Problem:** Scanner loops push chunks via `ch <- chunk` without selecting on `ctx.Done()`.
- **Action Items:**
  1. Update channel writes in `StreamOpenAI` and `StreamGemini`:
     ```go
     select {
     case ch <- chunk:
     case <-ctx.Done():
         return
     }
     ```

---

## Phase 4: Error Handling & Security Hygiene

### Task 4.1: Fix Resty Response Body Draining in Error Handlers
- **Target Files:** `internal/providers/gemini.go`, `deepseek.go`, `xiaomi.go`
- **Problem:** Code attempts `io.ReadAll(resp.RawBody())` on consumed bodies.
- **Action Items:**
  1. Replace `io.ReadAll(resp.RawBody())` with `resp.Body()` or `resp.String()`.

### Task 4.2: Handle Database Error Returns
- **Target Files:** [`internal/server/users.go`](file:///home/newkirk/Projects/gophergate/internal/server/users.go#L73), [`internal/server/clients.go`](file:///home/newkirk/Projects/gophergate/internal/server/clients.go#L165)
- **Problem:** Errors returned by `database.Exec()` are ignored in user and client token updates.
- **Action Items:**
  1. Check and log/return HTTP error responses for all `database.Exec()` calls.

### Task 4.3: Mask Auth Tokens in System Settings API
- **Target File:** [`internal/server/system.go`](file:///home/newkirk/Projects/gophergate/internal/server/system.go#L81)
- **Problem:** `/api/system/settings` returns unmasked `auth_tokens`.
- **Action Items:**
  1. Mask static API tokens before returning in JSON output (e.g. `sk-***1234`).

---

## Verification & Validation Plan

1. **Unit Testing:** Run `go test -v -race ./...` to verify zero data races.
2. **Integration Verification:** Run test completions across all providers (`openai`, `gemini`, `deepseek`, `grok`, `ollama`) to confirm streaming and non-streaming responses work correctly.
3. **Security Audit:** Verify log output contains no plain-text API keys or tokens.