fix: address 8 security vulnerabilities from codebase review

- S0 [CRITICAL]: Gate X-User-DID header behind TRUST_PROXY_HEADERS config
  (default: false) to prevent admin auth bypass via header spoofing
  Fixes #6
- S1: Sanitize JSONExtract/JSONExtractPath with regex validation to
  prevent SQL injection in both SQLite and PostgreSQL executors
  Fixes #2
- S2: Make WebSocket CheckOrigin configurable via ALLOWED_ORIGINS;
  defaults to same-origin policy instead of allowing all origins
  Fixes #3
- S3: Replace backfillActive bool with atomic.Bool and use CompareAndSwap
  to prevent race conditions in concurrent backfill triggers
  Fixes #4
- S4: Treat JTI Insert failures as replay attempts (ErrDPoPReplay)
  instead of generic server errors for proper TOCTOU handling
  Fixes #5
- S5: Add requireAdmin() to all admin queries (statistics, settings,
  lexicons, activity, etc.) — only currentSession remains public
  (also part of #6)
- S6: Add upload size limits (10MB ZIP, 500 files, 1MB/file) with
  io.LimitReader to prevent memory exhaustion via ZIP bombs
  Fixes #14
- S7: Disable global WriteTimeout (set to 0) to support long-lived
  WebSocket subscriptions; per-handler deadlines remain in place
  Fixes #16

Author: daviddao

.gitignore

@@ -56,3 +56,6 @@ logs/
 tmp/
 temp/
 .beads/
+
+# AI tools
+.opencode/

cmd/hypergoat/main.go

@@ -323,7 +323,7 @@ func run() error {
 		domainDID = "did:web:" + cfg.Host // Derive from host
 	}
 
-	adminHandler, err := admin.NewHandler(adminRepos, authMiddleware, configRepo, domainDID)
+	adminHandler, err := admin.NewHandler(adminRepos, authMiddleware, configRepo, domainDID, cfg.TrustProxyHeaders)
 	if err != nil {
 		slog.Error("Failed to create admin GraphQL handler", "error", err)
 	} else {
@@ -500,7 +500,14 @@ func run() error {
 		slog.Info("GraphQL endpoint enabled", "path", "/graphql")
 
 		// Add WebSocket subscription endpoint
-		subscriptionHandler := subscription.NewHandler(graphqlHandler.Schema())
+		var allowedOrigins []string
+		if cfg.AllowedOrigins != "" {
+			allowedOrigins = strings.Split(cfg.AllowedOrigins, ",")
+			for i := range allowedOrigins {
+				allowedOrigins[i] = strings.TrimSpace(allowedOrigins[i])
+			}
+		}
+		subscriptionHandler := subscription.NewHandler(graphqlHandler.Schema(), allowedOrigins)
 		r.Handle("/graphql/ws", subscriptionHandler)
 		slog.Info("GraphQL subscriptions enabled", "path", "/graphql/ws")
 	}
@@ -652,10 +659,14 @@ func run() error {
 
 	// Create HTTP server
 	srv := &http.Server{
-		Addr:         cfg.Address(),
-		Handler:      r,
-		ReadTimeout:  15 * time.Second,
-		WriteTimeout: 15 * time.Second,
+		Addr:        cfg.Address(),
+		Handler:     r,
+		ReadTimeout: 15 * time.Second,
+		// WriteTimeout disabled (set to 0) to support long-lived WebSocket connections.
+		// Individual handlers enforce their own write deadlines:
+		// - WebSocket: per-message deadline in subscription/handler.go
+		// - HTTP: standard response lifecycle
+		WriteTimeout: 0,
 		IdleTimeout:  60 * time.Second,
 	}
 

internal/config/config.go

@@ -38,6 +38,10 @@ type Config struct {
 	BackfillMaxPDSWorkers     int
 	BackfillMaxHTTPConcurrent int
 	BackfillRepoTimeoutMS     int
+
+	// Security
+	TrustProxyHeaders bool   // Trust X-User-DID header from reverse proxy (default: false, DANGEROUS if true without proxy)
+	AllowedOrigins    string // Comma-separated allowed WebSocket/CORS origins (empty = same-origin only, "*" = allow all)
 }
 
 // Load reads configuration from environment variables.
@@ -59,6 +63,8 @@ func Load() (*Config, error) {
 		BackfillMaxPDSWorkers:     getEnvInt("BACKFILL_MAX_PDS_WORKERS", 10),
 		BackfillMaxHTTPConcurrent: getEnvInt("BACKFILL_MAX_HTTP_CONCURRENT", 50),
 		BackfillRepoTimeoutMS:     getEnvInt("BACKFILL_REPO_TIMEOUT", 60000),
+		TrustProxyHeaders:         getEnvBool("TRUST_PROXY_HEADERS", false),
+		AllowedOrigins:            getEnv("ALLOWED_ORIGINS", ""),
 	}
 
 	// Generate SecretKeyBase if not provided
@@ -103,7 +109,14 @@ func (c *Config) LogConfig() {
 		"oauth_loopback_mode", c.OAuthLoopbackMode,
 		"oauth_signing_key_set", c.OAuthSigningKey != "",
 		"jetstream_disable_cursor", c.JetstreamDisableCursor,
+		"trust_proxy_headers", c.TrustProxyHeaders,
+		"allowed_origins", c.AllowedOrigins,
 	)
+
+	if c.TrustProxyHeaders {
+		slog.Warn("TRUST_PROXY_HEADERS is enabled: X-User-DID header will be trusted for authentication. " +
+			"Only enable this when running behind a trusted reverse proxy that sets this header.")
+	}
 }
 
 // Address returns the server address in host:port format.

internal/database/postgres/executor.go

@@ -6,13 +6,17 @@ import (
 	"database/sql"
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 
 	_ "github.com/jackc/pgx/v5/stdlib" // PostgreSQL driver
 
 	"github.com/GainForest/hypergoat/internal/database"
 )
 
+// validJSONFieldName matches safe JSON field names to prevent SQL injection.
+var validJSONFieldName = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
+
 // Executor implements database.Executor for PostgreSQL.
 type Executor struct {
 	db *sql.DB
@@ -103,15 +107,25 @@ func (e *Executor) Placeholders(count, startIndex int) string {
 }
 
 // JSONExtract generates PostgreSQL JSON extraction SQL.
+// The field parameter is validated to prevent SQL injection.
 func (e *Executor) JSONExtract(column, field string) string {
+	if !validJSONFieldName.MatchString(field) {
+		panic(fmt.Sprintf("postgres: invalid JSON field name: %q (must match ^[a-zA-Z_][a-zA-Z0-9_]*$)", field))
+	}
 	return fmt.Sprintf("%s->>'%s'", column, field)
 }
 
 // JSONExtractPath generates PostgreSQL JSON path extraction SQL.
+// All path segments are validated to prevent SQL injection.
 func (e *Executor) JSONExtractPath(column string, path []string) string {
 	if len(path) == 0 {
 		return column
 	}
+	for _, p := range path {
+		if !validJSONFieldName.MatchString(p) {
+			panic(fmt.Sprintf("postgres: invalid JSON path segment: %q (must match ^[a-zA-Z_][a-zA-Z0-9_]*$)", p))
+		}
+	}
 	if len(path) == 1 {
 		return fmt.Sprintf("%s->>'%s'", column, path[0])
 	}

internal/database/sqlite/executor.go

@@ -5,13 +5,17 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
+	"regexp"
 	"strings"
 
 	_ "modernc.org/sqlite" // Pure Go SQLite driver
 
 	"github.com/GainForest/hypergoat/internal/database"
 )
 
+// validJSONFieldName matches safe JSON field names to prevent SQL injection.
+var validJSONFieldName = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
+
 // Executor implements database.Executor for SQLite.
 type Executor struct {
 	db *sql.DB
@@ -122,12 +126,22 @@ func (e *Executor) Placeholders(count, startIndex int) string {
 }
 
 // JSONExtract generates SQLite JSON extraction SQL.
+// The field parameter is validated to prevent SQL injection.
 func (e *Executor) JSONExtract(column, field string) string {
+	if !validJSONFieldName.MatchString(field) {
+		panic(fmt.Sprintf("sqlite: invalid JSON field name: %q (must match ^[a-zA-Z_][a-zA-Z0-9_]*$)", field))
+	}
 	return fmt.Sprintf("json_extract(%s, '$.%s')", column, field)
 }
 
 // JSONExtractPath generates SQLite JSON path extraction SQL.
+// All path segments are validated to prevent SQL injection.
 func (e *Executor) JSONExtractPath(column string, path []string) string {
+	for _, p := range path {
+		if !validJSONFieldName.MatchString(p) {
+			panic(fmt.Sprintf("sqlite: invalid JSON path segment: %q (must match ^[a-zA-Z_][a-zA-Z0-9_]*$)", p))
+		}
+	}
 	jsonPath := "$." + strings.Join(path, ".")
 	return fmt.Sprintf("json_extract(%s, '%s')", column, jsonPath)
 }

internal/graphql/admin/handler.go

@@ -14,14 +14,17 @@ import (
 
 // Handler handles admin GraphQL requests with authentication.
 type Handler struct {
-	schema     *graphql.Schema
-	resolver   *Resolver
-	middleware *oauth.AuthMiddleware
-	configRepo *repositories.ConfigRepository
+	schema            *graphql.Schema
+	resolver          *Resolver
+	middleware        *oauth.AuthMiddleware
+	configRepo        *repositories.ConfigRepository
+	trustProxyHeaders bool
 }
 
 // NewHandler creates a new admin GraphQL handler.
-func NewHandler(repos *Repositories, middleware *oauth.AuthMiddleware, configRepo *repositories.ConfigRepository, domainDID string) (*Handler, error) {
+// trustProxyHeaders controls whether the X-User-DID header is trusted for authentication.
+// This should only be true when running behind a trusted reverse proxy.
+func NewHandler(repos *Repositories, middleware *oauth.AuthMiddleware, configRepo *repositories.ConfigRepository, domainDID string, trustProxyHeaders bool) (*Handler, error) {
 	resolver := NewResolver(repos, domainDID)
 
 	builder := NewSchemaBuilder(resolver)
@@ -31,10 +34,11 @@ func NewHandler(repos *Repositories, middleware *oauth.AuthMiddleware, configRep
 	}
 
 	return &Handler{
-		schema:     schema,
-		resolver:   resolver,
-		middleware: middleware,
-		configRepo: configRepo,
+		schema:            schema,
+		resolver:          resolver,
+		middleware:        middleware,
+		configRepo:        configRepo,
+		trustProxyHeaders: trustProxyHeaders,
 	}, nil
 }
 
@@ -43,7 +47,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// Handle CORS
 	w.Header().Set("Access-Control-Allow-Origin", "*")
 	w.Header().Set("Access-Control-Allow-Methods", "POST, GET, OPTIONS")
-	w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization, DPoP, X-User-DID")
+	allowedHeaders := "Content-Type, Authorization, DPoP"
+	if h.trustProxyHeaders {
+		allowedHeaders += ", X-User-DID"
+	}
+	w.Header().Set("Access-Control-Allow-Headers", allowedHeaders)
 
 	if r.Method == "OPTIONS" {
 		w.WriteHeader(http.StatusOK)
@@ -76,9 +84,16 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 	userDID := oauth.UserIDFromContext(ctx)
 
-	// Fall back to X-User-DID header if no OAuth token (for Next.js frontend proxy)
-	if userDID == "" {
+	// Only trust X-User-DID header when explicitly configured (TRUST_PROXY_HEADERS=true).
+	// This is intended for deployments behind a trusted reverse proxy (e.g., Next.js frontend).
+	// WARNING: Without a trusted proxy, this header can be spoofed by any client.
+	if userDID == "" && h.trustProxyHeaders {
 		userDID = r.Header.Get("X-User-DID")
+		if userDID != "" {
+			slog.Warn("[admin] Auth via X-User-DID proxy header",
+				"did", userDID,
+				"remote_addr", r.RemoteAddr)
+		}
 	}
 	handle := "" // Would need to resolve from DID
 

internal/graphql/admin/resolvers.go

@@ -12,8 +12,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"github.com/GainForest/hypergoat/internal/database/repositories"
@@ -47,7 +49,7 @@ type LexiconChangeCallback func(collections []string) error
 // Resolver provides methods for resolving admin GraphQL queries and mutations.
 type Resolver struct {
 	repos                 *Repositories
-	backfillActive        bool
+	backfillActive        atomic.Bool
 	domainDID             string // The DID of this labeler instance
 	backfillCallback      BackfillCallback
 	fullBackfillCallback  FullBackfillCallback
@@ -95,7 +97,7 @@ func (r *Resolver) notifyLexiconChange(ctx context.Context) {
 
 	if err := r.lexiconChangeCallback(collections); err != nil {
 		// Log but don't fail the operation
-		fmt.Printf("Failed to notify lexicon change: %v\n", err)
+		slog.Warn("Failed to notify lexicon change", "error", err)
 	}
 }
 
@@ -175,12 +177,12 @@ func (r *Resolver) Settings(ctx context.Context) (map[string]interface{}, error)
 
 // IsBackfilling returns whether a backfill is currently active.
 func (r *Resolver) IsBackfilling() bool {
-	return r.backfillActive
+	return r.backfillActive.Load()
 }
 
 // SetBackfillActive sets the backfill status.
 func (r *Resolver) SetBackfillActive(active bool) {
-	r.backfillActive = active
+	r.backfillActive.Store(active)
 }
 
 // Lexicons returns all lexicon definitions.
@@ -225,8 +227,22 @@ func (r *Resolver) OAuthClients(ctx context.Context) ([]map[string]interface{},
 	return result, nil
 }
 
+// Upload size limits for lexicon ZIP files.
+const (
+	maxLexiconUploadBytes = 10 * 1024 * 1024 // 10MB max ZIP size
+	maxLexiconFileCount   = 500              // Max files in ZIP
+	maxLexiconFileSize    = 1 * 1024 * 1024  // 1MB max per file
+)
+
 // UploadLexicons extracts lexicons from a base64-encoded ZIP file.
 func (r *Resolver) UploadLexicons(ctx context.Context, zipBase64 string) (int, error) {
+	// Validate base64 input size before decoding (base64 encodes 3 bytes as 4 chars)
+	maxBase64Len := maxLexiconUploadBytes * 4 / 3
+	if len(zipBase64) > maxBase64Len {
+		return 0, fmt.Errorf("upload too large: estimated %d bytes exceeds %d byte limit",
+			len(zipBase64)*3/4, maxLexiconUploadBytes)
+	}
+
 	// Decode base64
 	zipData, err := base64.StdEncoding.DecodeString(zipBase64)
 	if err != nil {
@@ -239,6 +255,12 @@ func (r *Resolver) UploadLexicons(ctx context.Context, zipBase64 string) (int, e
 		return 0, fmt.Errorf("invalid ZIP file: %w", err)
 	}
 
+	// Check file count to prevent zip bombs
+	if len(zipReader.File) > maxLexiconFileCount {
+		return 0, fmt.Errorf("too many files in ZIP: %d exceeds limit of %d",
+			len(zipReader.File), maxLexiconFileCount)
+	}
+
 	// Process each file
 	count := 0
 	for _, file := range zipReader.File {
@@ -247,17 +269,27 @@ func (r *Resolver) UploadLexicons(ctx context.Context, zipBase64 string) (int, e
 			continue
 		}
 
-		// Open and read file
+		// Check individual uncompressed file size
+		if file.UncompressedSize64 > maxLexiconFileSize {
+			return count, fmt.Errorf("file %s too large: %d bytes exceeds %d byte limit",
+				file.Name, file.UncompressedSize64, maxLexiconFileSize)
+		}
+
+		// Open and read file with size limit
 		rc, err := file.Open()
 		if err != nil {
 			continue
 		}
 
-		data, err := io.ReadAll(rc)
+		data, err := io.ReadAll(io.LimitReader(rc, maxLexiconFileSize+1))
 		rc.Close()
 		if err != nil {
 			continue
 		}
+		if len(data) > maxLexiconFileSize {
+			return count, fmt.Errorf("file %s exceeds %d byte limit after decompression",
+				file.Name, maxLexiconFileSize)
+		}
 
 		// Parse JSON to extract lexicon ID
 		var lexicon struct {
@@ -287,26 +319,24 @@ func (r *Resolver) UploadLexicons(ctx context.Context, zipBase64 string) (int, e
 }
 
 // TriggerBackfill starts a full backfill process.
+// Uses atomic CompareAndSwap to prevent concurrent backfill launches (race-safe).
 func (r *Resolver) TriggerBackfill(ctx context.Context) (bool, error) {
-	if r.backfillActive {
-		return false, fmt.Errorf("backfill already in progress")
-	}
-
 	if r.fullBackfillCallback == nil {
 		return false, fmt.Errorf("full backfill not configured")
 	}
 
-	r.backfillActive = true
+	// Atomically check-and-set to prevent concurrent backfill launches
+	if !r.backfillActive.CompareAndSwap(false, true) {
+		return false, fmt.Errorf("backfill already in progress")
+	}
 
 	// Run backfill in background goroutine
 	go func() {
-		defer func() {
-			r.backfillActive = false
-		}()
+		defer r.backfillActive.Store(false)
 
 		// Use background context since HTTP request context will be cancelled
 		if err := r.fullBackfillCallback(context.Background()); err != nil {
-			// Error is logged by the callback
+			slog.Error("[backfill] Full backfill failed in background", "error", err)
 			return
 		}
 	}()