5.5 KiB
5.5 KiB
GopherGate Remediation Action Plan
Based on the findings from the Code Review Report, 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 - Problem:
fmt.Printf("[Gemini] POST %s\n", url)logs the raw request URL containing?key=AIzaSy..., leaking API secrets to logs. - Action Items:
- Create a helper function
sanitizeURL(rawURL string) stringininternal/providers/helpers.goto strip or replace sensitive query parameters (key=...). - Replace all instances of raw URL printing in
gemini.go(L378, L636) withlog.Printf("[Gemini] POST %s", sanitizeURL(url)).
- Create a helper function
Task 1.2: Add Thread Safety to Server Providers Map
- Target File:
internal/server/server.go - Problem:
s.providersmap is mutated duringRefreshProviders()without mutex locking while HTTP handlers read from it concurrently (selectProvider()). - Action Items:
- Add
providersMu sync.RWMutexfield toServerstruct inserver.go. - Acquire
s.providersMu.Lock()before modifyings.providersinRefreshProviders(). - Acquire
s.providersMu.RLock()inselectProvider()and deferreds.providersMu.RUnlock().
- Add
Phase 2: Thread-Safety & Race Condition Hardening
Task 2.1: Synchronize ModelRegistry Lookups and Reloads
- Target Files:
internal/models/registry.go,internal/server/server.go - Problem: Background goroutines update
s.registrywhile concurrent requests accessr.FindModel(). - Action Items:
- Add
sync.RWMutextoModelRegistrystruct inregistry.go. - Wrap
FindModel()methods withr.mu.RLock()andr.mu.RUnlock(). - Alternatively, implement
atomic.Pointer[models.ModelRegistry]inServerto enable lock-free atomic pointer swaps on registry refresh.
- Add
Task 2.2: Synchronize Router.Reload()
- Target File:
internal/router/router.go - Problem:
Router.Reload()replacesr.groupsmap without holding a mutex lock during active routing requests. - Action Items:
- Add
mu sync.RWMutextoRouterstruct inrouter.go. - Acquire
r.mu.RLock()duringRoute(),RouteToConcrete(), andIsGroup(). - Acquire
r.mu.Lock()duringReload().
- Add
Phase 3: Reliability & Stream Fault Tolerance
Task 3.1: Enable Circuit Breaker Protection for Streaming Endpoints
- Target File:
internal/providers/circuit_breaker.go - Problem:
ChatCompletionStreamandResponsesStreambypassgobreakerentirely. - Action Items:
- Wrap the initial
ChatCompletionStreamcall insidecb.Execute(). - Implement stream response wrapper that monitors streaming errors and reports failure back to the circuit breaker state tracker.
- Wrap the initial
Task 3.2: Prevent Goroutine Leaks on Client Disconnect
- Target Files:
internal/providers/helpers.go,internal/providers/deepseek.go - Problem: Scanner loops push chunks via
ch <- chunkwithout selecting onctx.Done(). - Action Items:
- Update channel writes in
StreamOpenAIandStreamGemini:select { case ch <- chunk: case <-ctx.Done(): return }
- Update channel writes in
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:
- Replace
io.ReadAll(resp.RawBody())withresp.Body()orresp.String().
- Replace
Task 4.2: Handle Database Error Returns
- Target Files:
internal/server/users.go,internal/server/clients.go - Problem: Errors returned by
database.Exec()are ignored in user and client token updates. - Action Items:
- Check and log/return HTTP error responses for all
database.Exec()calls.
- Check and log/return HTTP error responses for all
Task 4.3: Mask Auth Tokens in System Settings API
- Target File:
internal/server/system.go - Problem:
/api/system/settingsreturns unmaskedauth_tokens. - Action Items:
- Mask static API tokens before returning in JSON output (e.g.
sk-***1234).
- Mask static API tokens before returning in JSON output (e.g.
Verification & Validation Plan
- Unit Testing: Run
go test -v -race ./...to verify zero data races. - Integration Verification: Run test completions across all providers (
openai,gemini,deepseek,grok,ollama) to confirm streaming and non-streaming responses work correctly. - Security Audit: Verify log output contains no plain-text API keys or tokens.