fix: address code audit findings (24 items) and review fixes

db/migrations/000004_trigram_search.up.sql

@@ -1,17 +1,17 @@
 -- Enable pg_trgm extension for trigram-based text search (CJK support).
 CREATE EXTENSION IF NOT EXISTS pg_trgm;
 
--- CONCURRENTLY avoids locking the table during index creation.
--- NOTE: CONCURRENTLY cannot run inside a transaction block.
--- golang-migrate runs each file in a transaction by default;
--- the operator must run this migration manually with:
---   psql -f 000004_trigram_search.up.sql
--- or disable transactions in the migration tool.
-
 -- GIN trigram index on messages.text_content for ILIKE fallback search.
-CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_messages_text_content_trgm
+-- NOTE: For large production tables with existing data, create these indexes
+-- manually with CONCURRENTLY before running this migration (they will be
+-- no-ops due to IF NOT EXISTS):
+--   CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_messages_text_content_trgm
+--       ON messages USING gin (text_content gin_trgm_ops);
+--   CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memories_content_trgm
+--       ON memories USING gin (content gin_trgm_ops);
+CREATE INDEX IF NOT EXISTS idx_messages_text_content_trgm
     ON messages USING gin (text_content gin_trgm_ops);
 
 -- GIN trigram index on memories.content for similarity() scoring.
-CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memories_content_trgm
+CREATE INDEX IF NOT EXISTS idx_memories_content_trgm
     ON memories USING gin (content gin_trgm_ops);

db/migrations/000005_owner_id_check.down.sql

@@ -0,0 +1,2 @@
+ALTER TABLE sessions ALTER COLUMN owner_id SET DEFAULT '';
+ALTER TABLE sessions DROP CONSTRAINT IF EXISTS sessions_owner_id_not_empty;

db/migrations/000005_owner_id_check.up.sql

@@ -0,0 +1,7 @@
+-- Backfill empty owner_id before adding constraint.
+-- Sessions created before ownership was enforced may have owner_id = ''.
+UPDATE sessions SET owner_id = 'anonymous' WHERE owner_id = '';
+
+-- Prevent empty owner_id in sessions (enforces NOT NULL + non-empty).
+ALTER TABLE sessions ADD CONSTRAINT sessions_owner_id_not_empty CHECK (owner_id != '');
+ALTER TABLE sessions ALTER COLUMN owner_id DROP DEFAULT;

internal/api/chat.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+
 	"github.com/koopa0/koopa/internal/chat"
 	"github.com/koopa0/koopa/internal/session"
 	"github.com/koopa0/koopa/internal/tools"

internal/api/doc.go

@@ -4,7 +4,7 @@
 //
 // The API server uses Go 1.22+ routing with a layered middleware stack:
 //
-//	Recovery → Logging → RateLimit → CORS → Session → CSRF → Routes
+//	Recovery → RequestID → Logging → CORS → RateLimit → User → Session → CSRF → Routes
 //
 // Health probes (/health, /ready) bypass the middleware stack via a
 // top-level mux, ensuring they remain fast and unauthenticated.
@@ -24,11 +24,24 @@
 //   - GET    /api/v1/sessions/{id}         — get session by ID
 //   - GET    /api/v1/sessions/{id}/messages — get session messages
 //   - DELETE /api/v1/sessions/{id}         — delete session
+//   - GET    /api/v1/sessions/{id}/export  — export session
 //
 // Chat (ownership-enforced):
 //   - POST /api/v1/chat        — initiate chat, returns stream URL
 //   - GET  /api/v1/chat/stream — SSE endpoint for streaming responses
 //
+// Search:
+//   - GET /api/v1/search — full-text search across messages
+//
+// Stats:
+//   - GET /api/v1/stats — usage statistics (sessions, messages, memories)
+//
+// Memory (ownership-enforced):
+//   - GET    /api/v1/memories        — list memories
+//   - POST   /api/v1/memories        — create memory
+//   - DELETE /api/v1/memories/{id}   — delete memory
+//   - GET    /api/v1/memories/search — search memories
+//
 // # CSRF Token Model
 //
 // Two token types prevent cross-site request forgery:
@@ -39,7 +52,7 @@
 //   - Session-bound tokens ("timestamp:signature"): bound to a specific
 //     session via HMAC-SHA256, verified with constant-time comparison.
 //
-// Both expire after 24 hours with 5 minutes of clock skew tolerance.
+// Both expire after 1 hour with 5 minutes of clock skew tolerance.
 //
 // # Session Ownership
 //

internal/api/search.go

@@ -45,7 +45,7 @@ func (h *searchHandler) searchMessages(w http.ResponseWriter, r *http.Request) {
 
 	results, total, err := h.store.SearchMessages(r.Context(), userID, query, limit, offset)
 	if err != nil {
-		h.logger.Error("searching messages", "error", err, "user_id", userID, "query", query)
+		h.logger.Error("searching messages", "error", err, "user_id", userID, "query_len", len(query))
 		WriteError(w, http.StatusInternalServerError, "search_failed", "failed to search messages", h.logger)
 		return
 	}

internal/api/session.go

@@ -16,23 +16,17 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+
 	"github.com/koopa0/koopa/internal/session"
 )
 
-// Sentinel errors for session/CSRF operations.
 var (
-	// ErrSessionCookieNotFound is returned when the session cookie is absent from the request.
-	ErrSessionCookieNotFound = errors.New("session cookie not found")
-	// ErrSessionInvalid is returned when the session cookie value is not a valid UUID.
-	ErrSessionInvalid = errors.New("session ID invalid")
-	// ErrCSRFRequired is returned when a state-changing request has no CSRF token.
-	ErrCSRFRequired = errors.New("csrf token required")
-	// ErrCSRFInvalid is returned when the CSRF token signature does not match.
-	ErrCSRFInvalid = errors.New("csrf token invalid")
-	// ErrCSRFExpired is returned when the CSRF token timestamp exceeds csrfTokenTTL.
-	ErrCSRFExpired = errors.New("csrf token expired")
-	// ErrCSRFMalformed is returned when the CSRF token format cannot be parsed.
-	ErrCSRFMalformed = errors.New("csrf token malformed")
+	errSessionCookieNotFound = errors.New("session cookie not found")
+	errSessionInvalid        = errors.New("session ID invalid")
+	errCSRFRequired          = errors.New("csrf token required")
+	errCSRFInvalid           = errors.New("csrf token invalid")
+	errCSRFExpired           = errors.New("csrf token expired")
+	errCSRFMalformed         = errors.New("csrf token malformed")
 )
 
 // Pre-session CSRF token prefix to distinguish from user-bound tokens.
@@ -61,12 +55,12 @@ type sessionManager struct {
 func (*sessionManager) SessionID(r *http.Request) (uuid.UUID, error) {
 	cookie, err := r.Cookie(sessionCookieName)
 	if err != nil {
-		return uuid.Nil, ErrSessionCookieNotFound
+		return uuid.Nil, errSessionCookieNotFound
 	}
 
 	sessionID, err := uuid.Parse(cookie.Value)
 	if err != nil {
-		return uuid.Nil, ErrSessionInvalid
+		return uuid.Nil, errSessionInvalid
 	}
 
 	return sessionID, nil
@@ -109,17 +103,17 @@ func (sm *sessionManager) NewCSRFToken(userID string) string {
 // CheckCSRF verifies a user-bound CSRF token.
 func (sm *sessionManager) CheckCSRF(userID, token string) error {
 	if token == "" {
-		return ErrCSRFRequired
+		return errCSRFRequired
 	}
 
 	parts := strings.SplitN(token, ":", 2)
 	if len(parts) != 2 {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	timestamp, err := strconv.ParseInt(parts[0], 10, 64)
 	if err != nil {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	// SECURITY: Compute and verify HMAC BEFORE timestamp checks to prevent
@@ -133,19 +127,19 @@ func (sm *sessionManager) CheckCSRF(userID, token string) error {
 
 	actualSig, err := base64.URLEncoding.DecodeString(parts[1])
 	if err != nil {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	if subtle.ConstantTimeCompare(actualSig, expectedSig) != 1 {
-		return ErrCSRFInvalid
+		return errCSRFInvalid
 	}
 
 	age := time.Since(time.Unix(timestamp, 0))
 	if age > csrfTokenTTL {
-		return ErrCSRFExpired
+		return errCSRFExpired
 	}
 	if age < -csrfClockSkew {
-		return ErrCSRFInvalid
+		return errCSRFInvalid
 	}
 
 	return nil
@@ -168,23 +162,23 @@ func (sm *sessionManager) NewPreSessionCSRFToken() string {
 // CheckPreSessionCSRF verifies a pre-session CSRF token.
 func (sm *sessionManager) CheckPreSessionCSRF(token string) error {
 	if token == "" {
-		return ErrCSRFRequired
+		return errCSRFRequired
 	}
 
 	if !strings.HasPrefix(token, preSessionPrefix) {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	tokenBody := strings.TrimPrefix(token, preSessionPrefix)
 	parts := strings.SplitN(tokenBody, ":", 3)
 	if len(parts) != 3 {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	nonce := parts[0]
 	timestamp, err := strconv.ParseInt(parts[1], 10, 64)
 	if err != nil {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	// SECURITY: Compute and verify HMAC BEFORE timestamp checks to prevent
@@ -196,19 +190,19 @@ func (sm *sessionManager) CheckPreSessionCSRF(token string) error {
 
 	actualSig, err := base64.URLEncoding.DecodeString(parts[2])
 	if err != nil {
-		return ErrCSRFMalformed
+		return errCSRFMalformed
 	}
 
 	if subtle.ConstantTimeCompare(actualSig, expectedSig) != 1 {
-		return ErrCSRFInvalid
+		return errCSRFInvalid
 	}
 
 	age := time.Since(time.Unix(timestamp, 0))
 	if age > csrfTokenTTL {
-		return ErrCSRFExpired
+		return errCSRFExpired
 	}
 	if age < -csrfClockSkew {
-		return ErrCSRFInvalid
+		return errCSRFInvalid
 	}
 
 	return nil

internal/chat/chat.go

@@ -103,7 +103,7 @@ func (cfg Config) validate() error {
 		return errors.New("at least one tool is required")
 	}
 	if cfg.MemoryStore != nil && cfg.WG == nil {
-		return errors.New("WG is required when MemoryStore is set")
+		return errors.New("wg is required when memory store is set")
 	}
 	return nil
 }

internal/config/config.go

@@ -24,6 +24,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log"
 	"log/slog"
 	"os"
 	"path/filepath"
@@ -51,9 +52,6 @@ var (
 	// ErrInvalidEmbedderModel indicates the embedder model is invalid.
 	ErrInvalidEmbedderModel = errors.New("invalid embedder model")
 
-	// ErrInvalidEmbedderDimension indicates the embedder produces incompatible vector dimensions.
-	ErrInvalidEmbedderDimension = errors.New("incompatible embedder dimension")
-
 	// ErrInvalidPostgresHost indicates the PostgreSQL host is invalid.
 	ErrInvalidPostgresHost = errors.New("invalid PostgreSQL host")
 
@@ -158,6 +156,11 @@ type Config struct {
 	CORSOrigins []string `mapstructure:"cors_origins" json:"cors_origins"`
 	TrustProxy  bool     `mapstructure:"trust_proxy" json:"trust_proxy"` // Trust X-Real-IP/X-Forwarded-For headers (set true behind reverse proxy)
 	DevMode     bool     `mapstructure:"dev_mode" json:"dev_mode"`       // Dev mode: disables Secure flag on cookies (decoupled from DB SSL)
+
+	// API keys read from environment at Load() time.
+	// Unexported to avoid accidental serialization; validated in validateProviderAPIKey().
+	geminiAPIKey string
+	openaiAPIKey string
 }
 
 // Load loads configuration.
@@ -213,6 +216,12 @@ func Load() (*Config, error) {
 		return nil, fmt.Errorf("parsing DATABASE_URL: %w", err)
 	}
 
+	// Capture API keys from environment at load time.
+	// These are read by Genkit plugins directly, but we also need them
+	// for fail-fast validation (no os.Getenv in business logic).
+	cfg.geminiAPIKey = os.Getenv("GEMINI_API_KEY")
+	cfg.openaiAPIKey = os.Getenv("OPENAI_API_KEY")
+
 	// CRITICAL: Validate immediately (fail-fast)
 	if err := cfg.Validate(); err != nil {
 		return nil, fmt.Errorf("validating configuration: %w", err)
@@ -280,11 +289,11 @@ func setDefaults(v *viper.Viper) {
 //  2. DD_API_KEY - Datadog API key (optional, for observability)
 //  3. HMAC_SECRET - HMAC secret for CSRF protection (serve mode only)
 func bindEnvVariables(v *viper.Viper) {
-	// Helper to panic on unexpected bind errors (hardcoded strings can't fail)
-	// If this panics, it's a BUG in our code, not a runtime error
+	// Helper to fatal on unexpected bind errors (hardcoded strings can't fail)
+	// If this fails, it's a BUG in our code, not a runtime error
 	mustBind := func(key, envVar string) {
 		if err := v.BindEnv(key, envVar); err != nil {
-			panic(fmt.Sprintf("BUG: failed to bind %q to %q: %v", key, envVar, err))
+			log.Fatalf("BUG: failed to bind %q to %q: %v", key, envVar, err)
 		}
 	}
 

internal/config/validation.go

@@ -3,8 +3,9 @@
 import (
 	"fmt"
 	"log/slog"
-	"os"
 	"slices"
+
+	"github.com/koopa0/koopa/internal/rag"
 )
 
 // supportedProviders lists all valid AI provider values.
@@ -87,7 +88,7 @@
 		return fmt.Errorf("%w: postgres_password must be set in config.yaml",
 			ErrInvalidPostgresPassword)
 	}
 	if c.PostgresPassword == "koopa_dev_password" {
 		slog.Warn("Using default development password for PostgreSQL",
 			"warning", "Change postgres_password in config.yaml for production deployments")
 	}
@@ -127,7 +128,8 @@
 }
 
 // requiredVectorDimension must match the pgvector schema: embedding vector(768).
-const requiredVectorDimension = 768
+// Canonical source: rag.VectorDimension.
+var requiredVectorDimension = int(rag.VectorDimension)
 
 // validateEmbedder checks that the configured embedder model produces vectors
 // compatible with the database schema. For known models whose native dimension
@@ -200,16 +202,17 @@
 }
 
 // validateProviderAPIKey checks that the required API key is set for the configured provider.
+// API keys are captured from environment in Load() and stored as unexported fields.
 func (c *Config) validateProviderAPIKey() error {
 	switch c.resolvedProvider() {
 	case ProviderGemini:
-		if os.Getenv("GEMINI_API_KEY") == "" {
+		if c.geminiAPIKey == "" {
 			return fmt.Errorf("%w: GEMINI_API_KEY environment variable is required for provider %q\n"+
 				"Get your API key at: https://ai.google.dev/gemini-api/docs/api-key",
 				ErrMissingAPIKey, c.resolvedProvider())
 		}
 	case ProviderOpenAI:
-		if os.Getenv("OPENAI_API_KEY") == "" {
+		if c.openaiAPIKey == "" {
 			return fmt.Errorf("%w: OPENAI_API_KEY environment variable is required for provider %q\n"+
 				"Get your API key at: https://platform.openai.com/api-keys",
 				ErrMissingAPIKey, c.resolvedProvider())