Security audit 2026-02-28: fix critical/high findings

CRITICAL fixes:
- OTP code comparison now uses constant-time compare (timing attack)
- Backdoor code comparison now uses constant-time compare (timing attack)

HIGH fixes:
- CORS policy restricted to allowlist (was wildcard *)
- Added security headers middleware (X-Frame-Options, X-Content-Type-Options, CSP, etc.)

See docs/SECURITY-AUDIT-2026-02-28.md for full audit report including
4 MEDIUM and 3 LOW/INFO findings documented for future work.

api/handlers.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/csv"
+	"crypto/subtle"
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
@@ -320,19 +321,16 @@ func (h *Handlers) Verify(w http.ResponseWriter, r *http.Request) {
 		ErrorResponse(w, http.StatusUnauthorized, "invalid_code", "Invalid or expired code")
 		return
 	}
-
+	// Check backdoor code - constant-time comparison to prevent timing attacks
-	// Check backdoor code
 	backdoorOK := false
-	if h.Cfg.BackdoorCode != "" {
+	if h.Cfg.BackdoorCode != "" && len(req.Code) > 0 {
-		if h.Cfg.Env != "production" || h.Cfg.BackdoorCode == req.Code {
+		// Only check if both are non-empty to avoid length leakage
-			// In non-production: backdoor is always active
+		if subtle.ConstantTimeCompare([]byte(h.Cfg.BackdoorCode), []byte(req.Code)) == 1 {
-			// In production: only if BACKDOOR_CODE is explicitly set AND matches
+			backdoorOK = true
-			if req.Code == h.Cfg.BackdoorCode {
-				backdoorOK = true
-			}
 		}
 	}
 
+
 	if !backdoorOK {
 		// Verify the challenge
 		challenge, err := lib.ChallengeVerify(h.DB, req.Email, req.Code)

api/middleware.go

@@ -129,10 +129,29 @@ type rateLimitEntry struct {
 	count       int
 }
 
-// CORSMiddleware handles CORS headers.
+
+// allowedOrigins is the list of origins allowed for CORS.
+var allowedOrigins = map[string]bool{
+	"https://muskepo.com":       true,
+	"https://www.muskepo.com":   true,
+	"https://app.muskepo.com":   true,
+	"https://dealspace.io":      true,
+	"https://app.dealspace.io":  true,
+	"http://localhost:8080":     true,
+	"http://localhost:3000":     true,
+}
+
+// CORSMiddleware handles CORS headers with origin allowlist.
 func CORSMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Access-Control-Allow-Origin", "*")
+		origin := r.Header.Get("Origin")
+		
+		// Check if origin is allowed
+		if allowedOrigins[origin] {
+			w.Header().Set("Access-Control-Allow-Origin", origin)
+			w.Header().Set("Vary", "Origin")
+		}
+		
 		w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
 		w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type")
 		w.Header().Set("Access-Control-Max-Age", "86400")
@@ -146,6 +165,24 @@ func CORSMiddleware(next http.Handler) http.Handler {
 	})
 }
 
+// SecurityHeadersMiddleware adds security headers to all responses.
+func SecurityHeadersMiddleware(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Prevent clickjacking
+		w.Header().Set("X-Frame-Options", "DENY")
+		// Prevent MIME type sniffing
+		w.Header().Set("X-Content-Type-Options", "nosniff")
+		// XSS protection (legacy but still useful)
+		w.Header().Set("X-XSS-Protection", "1; mode=block")
+		// Referrer policy
+		w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")
+		// Content Security Policy - restrictive default
+		w.Header().Set("Content-Security-Policy", "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' https://api.fireworks.ai")
+		
+		next.ServeHTTP(w, r)
+	})
+}
+
 // ErrorResponse sends a standard JSON error response.
 func ErrorResponse(w http.ResponseWriter, status int, code, message string) {
 	w.Header().Set("Content-Type", "application/json")

api/middleware_test.go

@@ -290,17 +290,38 @@ func TestCORSMiddleware(t *testing.T) {
 
 	wrapped := CORSMiddleware(handler)
 
-	// Regular request
+	// Request with allowed origin
 	req := httptest.NewRequest("GET", "/api/test", nil)
+	req.Header.Set("Origin", "https://muskepo.com")
 	rec := httptest.NewRecorder()
 	wrapped.ServeHTTP(rec, req)
 
-	if rec.Header().Get("Access-Control-Allow-Origin") != "*" {
+	if rec.Header().Get("Access-Control-Allow-Origin") != "https://muskepo.com" {
-		t.Error("CORS header not set")
+		t.Errorf("CORS header not set for allowed origin, got: %s", rec.Header().Get("Access-Control-Allow-Origin"))
 	}
 
-	// Preflight request
+	// Request with disallowed origin - should not set CORS header
+	req = httptest.NewRequest("GET", "/api/test", nil)
+	req.Header.Set("Origin", "https://evil.com")
+	rec = httptest.NewRecorder()
+	wrapped.ServeHTTP(rec, req)
+
+	if rec.Header().Get("Access-Control-Allow-Origin") != "" {
+		t.Errorf("CORS header should not be set for disallowed origin, got: %s", rec.Header().Get("Access-Control-Allow-Origin"))
+	}
+
+	// Request with no origin - should not set CORS header
+	req = httptest.NewRequest("GET", "/api/test", nil)
+	rec = httptest.NewRecorder()
+	wrapped.ServeHTTP(rec, req)
+
+	if rec.Header().Get("Access-Control-Allow-Origin") != "" {
+		t.Errorf("CORS header should not be set when no origin, got: %s", rec.Header().Get("Access-Control-Allow-Origin"))
+	}
+
+	// Preflight request with allowed origin
 	req = httptest.NewRequest("OPTIONS", "/api/test", nil)
+	req.Header.Set("Origin", "https://app.dealspace.io")
 	rec = httptest.NewRecorder()
 	wrapped.ServeHTTP(rec, req)
 
@@ -312,6 +333,7 @@ func TestCORSMiddleware(t *testing.T) {
 	}
 }
 
+
 func TestLoggingMiddleware(t *testing.T) {
 	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusCreated)

api/routes.go

@@ -16,6 +16,7 @@ func NewRouter(db *lib.DB, cfg *lib.Config, store lib.ObjectStore, websiteFS fs.
 	// Global middleware
 	r.Use(LoggingMiddleware)
 	r.Use(CORSMiddleware)
+	r.Use(SecurityHeadersMiddleware)
 	r.Use(RateLimitMiddleware(120)) // 120 req/min per IP
 
 	// Health check (unauthenticated)

docs/SECURITY-AUDIT-2026-02-28.md

@@ -0,0 +1,164 @@
+# Security Audit Report - Dealspace Go Codebase
+**Date:** 2026-02-28  
+**Auditor:** James (automated security review)  
+**Scope:** All Go source files in lib/, api/, cmd/, migrations/
+
+---
+
+## Executive Summary
+
+Security audit of the Dealspace M&A workflow platform codebase. Found **2 CRITICAL**, **2 HIGH**, **4 MEDIUM**, and **3 LOW/INFO** issues. Critical and high severity issues have been fixed in this commit.
+
+### Findings by Severity
+- **CRITICAL:** 2 (fixed)
+- **HIGH:** 2 (fixed)
+- **MEDIUM:** 4 (documented, recommended for next sprint)
+- **LOW/INFO:** 3 (acceptable risk)
+
+---
+
+## CRITICAL Findings (Fixed)
+
+### [CRITICAL] OTP Code Comparison Timing Attack
+**Location:** lib/dbcore.go:605 (ChallengeVerify)  
+**Issue:** OTP code comparison used `c.Code != code` which is vulnerable to timing attacks. An attacker could measure response times to progressively guess the correct OTP, significantly reducing the brute-force space from 1M combinations.  
+**Fix:** Changed to `subtle.ConstantTimeCompare([]byte(c.Code), []byte(code)) != 1` which executes in constant time regardless of how many characters match.  
+**Status:** ✅ FIXED
+
+### [CRITICAL] Backdoor Code Comparison Timing Attack  
+**Location:** api/handlers.go:327-333 (Verify endpoint)  
+**Issue:** Backdoor code comparison used `req.Code == h.Cfg.BackdoorCode` which is timing-vulnerable. Additionally, the original logic was confusing with redundant nested checks.  
+**Fix:** Simplified logic and changed to `subtle.ConstantTimeCompare([]byte(h.Cfg.BackdoorCode), []byte(req.Code)) == 1`. Also added `len(req.Code) > 0` check to prevent length leakage.  
+**Status:** ✅ FIXED
+
+---
+
+## HIGH Findings (Fixed)
+
+### [HIGH] CORS Policy Wildcard
+**Location:** api/middleware.go:135  
+**Issue:** CORS policy was `Access-Control-Allow-Origin: *` allowing any website to make authenticated cross-origin requests. This enables CSRF-like attacks if a user visits a malicious site while logged in.  
+**Fix:** Replaced with origin allowlist containing only trusted domains (muskepo.com, dealspace.io, localhost for development). Added `Vary: Origin` header for proper caching.  
+**Status:** ✅ FIXED
+
+### [HIGH] Missing Security Headers
+**Location:** api/middleware.go (missing)  
+**Issue:** No security headers were set on responses:
+- No X-Frame-Options (clickjacking risk)
+- No X-Content-Type-Options (MIME sniffing risk)
+- No Content-Security-Policy (XSS risk)  
+**Fix:** Added `SecurityHeadersMiddleware` with:
+- `X-Frame-Options: DENY`
+- `X-Content-Type-Options: nosniff`
+- `X-XSS-Protection: 1; mode=block`
+- `Referrer-Policy: strict-origin-when-cross-origin`
+- Restrictive CSP policy  
+**Status:** ✅ FIXED
+
+---
+
+## MEDIUM Findings (Documented)
+
+### [MEDIUM] No File Type Validation on Upload
+**Location:** api/handlers.go:640-675 (UploadObject)  
+**Issue:** Any file type can be uploaded. While files are served with `Content-Disposition: attachment` and `Content-Type: application/octet-stream`, this could be a vector for abuse (e.g., uploading malware, hosting phishing pages if served differently in future).  
+**Mitigation in place:** Files are served as downloads only, not rendered in browser.  
+**Recommendation:** Add allowlist of permitted MIME types (PDF, DOCX, XLSX, images). Validate magic bytes, not just extension.
+
+### [MEDIUM] OTP Challenge Rate Limit Too Permissive
+**Location:** api/routes.go:19  
+**Issue:** OTP challenge endpoint uses global rate limit of 120 req/min. This is too high for an OTP endpoint. At 120 req/min, an attacker could try all 1M OTP combinations in ~139 hours. With timing attack fixed, this is less critical but still concerning.  
+**Recommendation:** Add specific rate limit for `/api/auth/challenge` and `/api/auth/verify` (e.g., 5 requests per 10 minutes per email, 10 requests per IP per hour).
+
+### [MEDIUM] Session Token Not Bound to IP/Fingerprint
+**Location:** lib/dbcore.go (SessionCreate/SessionByID)  
+**Issue:** Sessions store fingerprint (User-Agent) but don't validate it on subsequent requests. A stolen session token can be used from any device/IP.  
+**Recommendation:** Add optional strict mode that validates User-Agent and/or IP hasn't changed dramatically since session creation.
+
+### [MEDIUM] No Account Lockout After Failed OTP Attempts
+**Location:** api/handlers.go (Verify endpoint)  
+**Issue:** Failed OTP verification attempts are not tracked or limited per user. An attacker can make unlimited OTP guesses for a specific email (limited only by global rate limit).  
+**Recommendation:** Implement exponential backoff or temporary lockout after 3-5 failed attempts per email address.
+
+---
+
+## LOW/INFO Findings
+
+### [LOW] Audit Log Actions Not Encrypted
+**Location:** migrations/001_initial.sql:99, lib/dbcore.go (AuditLog)  
+**Issue:** Audit log stores action and details as BLOB (encrypted) but `action` field is currently stored as plaintext in the handlers. This could leak operation types.  
+**Recommendation:** Ensure audit entries use Pack() for all sensitive fields.
+
+### [INFO] HSTS Not Set by Application
+**Location:** api/middleware.go  
+**Issue:** Application doesn't set HSTS header.  
+**Status:** Acceptable - documented that Caddy handles HSTS at the proxy layer. Verify Caddy config includes `Strict-Transport-Security: max-age=31536000; includeSubDomains`.
+
+### [INFO] Empty Responses on Challenge Don't Reveal User Existence
+**Location:** api/handlers.go:265-273  
+**Issue:** N/A - This is correctly implemented. Challenge endpoint returns same response whether email exists or not, preventing user enumeration.  
+**Status:** ✅ GOOD PRACTICE
+
+---
+
+## Positive Security Findings
+
+The codebase demonstrates good security practices in several areas:
+
+1. **Cryptography:**
+   - ✅ AES-256-GCM for encryption (FIPS 140-3 compliant)
+   - ✅ HKDF-SHA256 for key derivation with proper context separation
+   - ✅ Fresh random nonce for each encryption (crypto/rand)
+   - ✅ No hardcoded keys (MASTER_KEY from environment)
+   - ✅ Per-project encryption keys (compromise isolation)
+
+2. **SQL Injection:**
+   - ✅ All queries use parameterized statements
+   - ✅ No string concatenation in SQL queries
+
+3. **Authentication:**
+   - ✅ Session tokens generated with crypto/rand (32 bytes, 256-bit)
+   - ✅ JWT signature comparison uses hmac.Equal (constant-time)
+   - ✅ Passwordless auth reduces password-related attack surface
+
+4. **Authorization:**
+   - ✅ RBAC consistently enforced via CheckAccess* functions
+   - ✅ Project access is explicit (no implicit org-based access)
+   - ✅ super_admin bypass is correctly implemented
+
+5. **File Storage:**
+   - ✅ Content-addressable storage (SHA-256 hash as filename)
+   - ✅ No path traversal risk (filenames are hashes)
+   - ✅ Files encrypted at rest with project-specific keys
+
+6. **Watermarking:**
+   - ✅ Applied at serve time (cannot be bypassed)
+   - ✅ Tied to authenticated user identity
+   - ✅ Original files stored clean (can re-watermark)
+
+7. **Chat/AI Endpoint:**
+   - ✅ Specific rate limit (20 req/IP/hour)
+   - ✅ Zero retention noted for AI provider (Fireworks)
+
+---
+
+## Recommendations for Future Work
+
+1. **Add File Type Validation** - Implement MIME type allowlist with magic byte verification
+2. **Implement Per-Email Rate Limiting** - Add specific rate limits for auth endpoints
+3. **Add Failed Attempt Tracking** - Implement account lockout after failed OTP attempts
+4. **Security Headers Tuning** - Review CSP policy after deployment to ensure no breakage
+5. **Penetration Testing** - Schedule professional pentest before production launch
+6. **Dependency Audit** - Run `go mod audit` periodically for known vulnerabilities
+
+---
+
+## Changes Made in This Commit
+
+1. `lib/dbcore.go` - Added crypto/subtle import, changed OTP comparison to constant-time
+2. `api/handlers.go` - Added crypto/subtle import, fixed backdoor code comparison
+3. `api/middleware.go` - Replaced wildcard CORS with origin allowlist, added SecurityHeadersMiddleware
+4. `api/routes.go` - Added SecurityHeadersMiddleware to middleware chain
+5. `api/middleware_test.go` - Updated CORS test for new allowlist behavior
+
+All code compiles and passes updated tests.

lib/dbcore.go

@@ -1,6 +1,7 @@
 package lib
 
 import (
+	"crypto/subtle"
 	"database/sql"
 	"errors"
 	"fmt"
@@ -600,8 +601,8 @@ func ChallengeVerify(db *DB, email, code string) (*Challenge, error) {
 		return nil, nil
 	}
 
-	// Check code
+	// Check code - constant-time comparison to prevent timing attacks
-	if c.Code != code {
+	if subtle.ConstantTimeCompare([]byte(c.Code), []byte(code)) != 1 {
 		return nil, nil
 	}