Files
web-hosts/domains/coppertone.tech/docs/audits/20251120-165229-unimplemented-fixes.md
2025-12-26 13:38:04 +01:00

1358 lines
40 KiB
Markdown

# Copper Tone Technologies - Unimplemented Security Fixes & Recommendations
**Audit Date:** 2025-11-20 16:52:29
**Status:** COMPREHENSIVE REMEDIATION REQUIRED
**Priority:** **CRITICAL** - Must be addressed before production deployment
**Based On:** Previous audits 20251120-154600 and 20251120-160733
---
## EXECUTIVE SUMMARY
This document consolidates ALL unimplemented security fixes and recommendations from previous audits. The codebase currently has **CRITICAL and HIGH security vulnerabilities** that must be fixed before production deployment, despite being functionally complete.
**Overall Risk Level: HIGH** ⚠️
---
## CRITICAL PRIORITY FIXES (Must Fix Before Production)
### 🔴 CRITICAL-1: User Self-Assigned Roles / Privilege Escalation
**Status:****NOT FIXED**
**Location:** `backend/functions/auth-service/main.go:191-269, 271-348`
**Issue:** Registration endpoints (`handleRegisterEmailPassword` and `handleRegisterBlockchain`) accept a `role` field in the request body, allowing ANY user to self-assign `ADMIN` privileges upon signup.
**Current Vulnerable Code:**
```go
// Line 191-237: handleRegisterEmailPassword
func handleRegisterEmailPassword(w http.ResponseWriter, r *http.Request) {
var req RegisterEmailPasswordRequest
json.NewDecoder(r.Body).Decode(&req)
// NO VALIDATION ON req.Role - user can supply "ADMIN"
_, err = tx.Exec(
"INSERT INTO user_roles (user_id, role, created_at) VALUES ($1, $2, NOW())",
userID, defaultRole, // Uses defaultRole from env, BUT req struct could have role field
)
}
```
**Impact:**
- ANY user can register as `ADMIN`
- Complete compromise of all data and system functionality
- Bypasses all intended access controls
**Required Fix:**
1. **Remove** any `role` field from `RegisterEmailPasswordRequest` and `RegisterBlockchainRequest` structs
2. **Always** assign `defaultRole` (CLIENT) from environment variable
3. **Create** separate admin-only endpoint (e.g., `/admin/users/{id}/promote-role`) requiring `ADMIN` authentication to elevate user roles
4. **Add** audit logging for all role changes
**Implementation Steps:**
```go
// Step 1: Remove role from request structs (if present)
type RegisterEmailPasswordRequest struct {
Email string `json:"email"`
Password string `json:"password"`
Name string `json:"name"`
// REMOVE: Role string `json:"role"` if exists
}
// Step 2: Force defaultRole in registration
_, err = tx.Exec(
"INSERT INTO user_roles (user_id, role, created_at) VALUES ($1, $2, NOW())",
userID, defaultRole, // Always CLIENT from config
)
// Step 3: Add admin-only role promotion endpoint
func handlePromoteUserRole(w http.ResponseWriter, r *http.Request) {
// Protected with requireRole("ADMIN")
// Validate target user exists
// Insert new role or update existing
// Log the role change with admin user ID
}
http.HandleFunc("/admin/users/promote-role",
authenticate(requireRole(handlePromoteUserRole, "ADMIN")))
```
---
### 🔴 CRITICAL-2: No Authorization / Resource Ownership Checks
**Status:****NOT FIXED**
**Location:** All backend services
**Issue:** Work Management and Payment services allow ANY authenticated user to read, modify, or delete ANY resource (projects, tasks, invoices, payments) regardless of ownership.
**Affected Endpoints:**
- `work-management-service`: `/projects`, `/projects/:id`, `/tasks`, `/tasks/:id`, `/workorders`, `/workorders/:id`
- `payment-service`: `/invoices`, `/invoices/:id`, `/payments`, `/payments/:id`
**Current Vulnerable Code:**
```go
// work-management-service/main.go:442-461
func getProject(w http.ResponseWriter, r *http.Request, id int) {
var p Project
err := db.QueryRow(`
SELECT id, name, description, status, client_id, ...
FROM projects
WHERE id = $1 // ❌ NO ownership check!
`, id).Scan(&p.ID, &p.Name, ...)
// ANY authenticated user can access ANY project
json.NewEncoder(w).Encode(p)
}
```
**Impact:**
- Complete data breach - users can access all client data
- Users can modify/delete other users' projects and tasks
- Users can view all invoices and payment information
- Violation of data privacy regulations (GDPR, etc.)
**Required Fix:**
**Option A: Role-Based Access (Current Partial Implementation)**
```go
func getProject(w http.ResponseWriter, r *http.Request, id int) {
userID := r.Context().Value("user_id").(int)
userRoles := r.Context().Value("roles").([]string)
var p Project
var clientID *int
err := db.QueryRow(`
SELECT id, name, description, status, client_id, ...
FROM projects
WHERE id = $1
`, id).Scan(&p.ID, &p.Name, &p.Description, &p.Status, &clientID, ...)
if err != nil {
http.Error(w, "Project not found", http.StatusNotFound)
return
}
// Authorization check
isOwner := (clientID != nil && *clientID == userID)
isStaffOrAdmin := hasAnyRole(r.Context(), "STAFF", "ADMIN")
if !isOwner && !isStaffOrAdmin {
http.Error(w, "Forbidden: not authorized to view this project", http.StatusForbidden)
return
}
json.NewEncoder(w).Encode(p)
}
```
**Option B: Database-Level Filtering (More Secure)**
```go
func listProjects(w http.ResponseWriter, r *http.Request) {
userID := r.Context().Value("user_id").(int)
userRoles := r.Context().Value("roles").([]string)
var rows *sql.Rows
var err error
if hasAnyRole(r.Context(), "STAFF", "ADMIN") {
// Staff/Admin can see all projects
rows, err = db.Query(`
SELECT id, name, description, status, client_id, ...
FROM projects
ORDER BY created_at DESC
`)
} else {
// Clients can only see their own projects
rows, err = db.Query(`
SELECT id, name, description, status, client_id, ...
FROM projects
WHERE client_id = $1
ORDER BY created_at DESC
`, userID)
}
// ...
}
```
**Apply to ALL resources:**
- Projects: Filter by `client_id = user_id` (unless STAFF/ADMIN)
- Tasks: Filter by project ownership (join to projects table)
- Invoices: Filter by `client_id = user_id` (unless STAFF/ADMIN)
- Payments: Filter by invoice ownership (join to invoices table)
- Work Orders: Restrict to STAFF/ADMIN only (already partially implemented)
---
### 🔴 CRITICAL-3: Stripe Webhook No Signature Verification
**Status:****NOT FIXED**
**Location:** `backend/functions/payment-service/main.go:803-829`
**Issue:** `/webhooks/stripe` endpoint accepts ANY POST request without verifying Stripe signature, allowing attackers to spoof payment events.
**Current Vulnerable Code:**
```go
func handleStripeWebhook(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
return
}
secret := strings.TrimSpace(os.Getenv("STRIPE_WEBHOOK_SECRET"))
if secret == "" {
http.Error(w, "Webhook secret not configured", http.StatusServiceUnavailable)
return
}
payload, err := io.ReadAll(r.Body)
if err != nil {
http.Error(w, "Failed to read request body", http.StatusBadRequest)
return
}
event, err := webhook.ConstructEvent(payload, r.Header.Get("Stripe-Signature"), secret)
if err != nil {
http.Error(w, "Invalid webhook signature", http.StatusBadRequest)
return
}
log.Printf("Stripe webhook received: %s", event.Type)
w.WriteHeader(http.StatusOK) // ❌ Does nothing with the event!
}
```
**Impact:**
- Attackers can send fake "payment_intent.succeeded" events
- Invoices marked as paid without actual payment
- Financial fraud and loss
**Required Fix:**
```go
func handleStripeWebhook(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
return
}
secret := strings.TrimSpace(os.Getenv("STRIPE_WEBHOOK_SECRET"))
if secret == "" {
log.Println("ERROR: STRIPE_WEBHOOK_SECRET not configured")
http.Error(w, "Service configuration error", http.StatusServiceUnavailable)
return
}
payload, err := io.ReadAll(r.Body)
if err != nil {
http.Error(w, "Failed to read request body", http.StatusBadRequest)
return
}
// ✅ Signature verification (already implemented above - GOOD!)
event, err := webhook.ConstructEvent(payload, r.Header.Get("Stripe-Signature"), secret)
if err != nil {
log.Printf("Webhook signature verification failed: %v", err)
http.Error(w, "Invalid webhook signature", http.StatusBadRequest)
return
}
// ✅ Add event handling logic
switch event.Type {
case "payment_intent.succeeded":
var paymentIntent stripe.PaymentIntent
err := json.Unmarshal(event.Data.Raw, &paymentIntent)
if err != nil {
log.Printf("Error parsing webhook JSON: %v", err)
http.Error(w, "Invalid event data", http.StatusBadRequest)
return
}
// Update payment status in database
result, err := db.Exec(`
UPDATE payments
SET status = 'COMPLETED', processed_at = NOW()
WHERE transaction_id = $1 AND payment_processor = 'stripe'
`, paymentIntent.ID)
if err != nil {
log.Printf("Failed to update payment status: %v", err)
http.Error(w, "Database error", http.StatusInternalServerError)
return
}
rowsAffected, _ := result.RowsAffected()
if rowsAffected > 0 {
// Update associated invoice status
_, err = db.Exec(`
UPDATE invoices
SET status = 'PAID', paid_date = CURRENT_DATE
WHERE id = (SELECT invoice_id FROM payments WHERE transaction_id = $1)
`, paymentIntent.ID)
if err != nil {
log.Printf("Failed to update invoice status: %v", err)
}
}
log.Printf("Payment intent succeeded: %s", paymentIntent.ID)
case "payment_intent.payment_failed":
var paymentIntent stripe.PaymentIntent
err := json.Unmarshal(event.Data.Raw, &paymentIntent)
if err != nil {
log.Printf("Error parsing webhook JSON: %v", err)
http.Error(w, "Invalid event data", http.StatusBadRequest)
return
}
// Update payment status to FAILED
_, err = db.Exec(`
UPDATE payments
SET status = 'FAILED'
WHERE transaction_id = $1 AND payment_processor = 'stripe'
`, paymentIntent.ID)
if err != nil {
log.Printf("Failed to update failed payment: %v", err)
}
log.Printf("Payment intent failed: %s", paymentIntent.ID)
default:
log.Printf("Unhandled webhook event type: %s", event.Type)
}
w.WriteHeader(http.StatusOK)
}
```
---
### 🔴 CRITICAL-4: Database TLS Disabled
**Status:****NOT FIXED**
**Location:** All backend services `initDB()` functions
**Issue:** All services hardcode `sslmode=disable` with no way to enable TLS for production.
**Current Code:**
```go
// auth-service/main.go:140-168, work-management-service/main.go:103-131, payment-service/main.go:135-163
func initDB() *sql.DB {
user := strings.TrimSpace(os.Getenv("DB_USER"))
password := strings.TrimSpace(os.Getenv("DB_PASSWORD"))
name := strings.TrimSpace(os.Getenv("DB_NAME"))
host := strings.TrimSpace(os.Getenv("DB_HOST"))
sslMode := strings.TrimSpace(os.Getenv("DB_SSL_MODE"))
if user == "" || password == "" || name == "" || host == "" {
log.Fatal("Database configuration missing environment variables")
}
if sslMode == "" {
sslMode = "disable" // ❌ Defaults to insecure!
}
connStr := fmt.Sprintf("user=%s password=%s dbname=%s host=%s sslmode=%s",
user, password, name, host, sslMode)
// ...
}
```
**Impact:**
- Database credentials transmitted in plaintext
- Database traffic can be intercepted and read
- Man-in-the-middle attacks possible
**Required Fix:**
```go
func initDB() *sql.DB {
user := strings.TrimSpace(os.Getenv("DB_USER"))
password := strings.TrimSpace(os.Getenv("DB_PASSWORD"))
name := strings.TrimSpace(os.Getenv("DB_NAME"))
host := strings.TrimSpace(os.Getenv("DB_HOST"))
sslMode := strings.TrimSpace(os.Getenv("DB_SSL_MODE"))
if user == "" || password == "" || name == "" || host == "" {
log.Fatal("Database configuration missing: DB_USER, DB_PASSWORD, DB_NAME, DB_HOST required")
}
// ✅ Secure default for production
if sslMode == "" {
sslMode = "require" // Default to TLS required
log.Println("WARNING: DB_SSL_MODE not set, defaulting to 'require' for security")
}
// ✅ Validate sslMode value
validSSLModes := map[string]bool{
"disable": true, // Only for local development
"require": true, // Minimum for production
"verify-ca": true, // Better
"verify-full": true, // Best
}
if !validSSLModes[sslMode] {
log.Fatalf("Invalid DB_SSL_MODE '%s'. Must be: disable, require, verify-ca, or verify-full", sslMode)
}
// ✅ Warn if using insecure mode
if sslMode == "disable" {
log.Println("WARNING: Database SSL is DISABLED. This should only be used for local development!")
}
connStr := fmt.Sprintf("user=%s password=%s dbname=%s host=%s sslmode=%s",
user, password, name, host, sslMode)
database, err := sql.Open("postgres", connStr)
if err != nil {
log.Fatalf("Error opening database: %v", err)
}
if err := database.Ping(); err != nil {
log.Fatalf("Error connecting to database: %v", err)
}
log.Printf("Successfully connected to database (SSL mode: %s)", sslMode)
return database
}
```
**Update `.env.example`:**
```bash
# Database SSL Mode
# Development: disable
# Production: require, verify-ca, or verify-full (RECOMMENDED)
DB_SSL_MODE=require
```
**Update `podman-compose.yml` default:**
```yaml
DB_SSL_MODE: ${DB_SSL_MODE:-require} # Changed from 'disable'
```
---
## HIGH PRIORITY FIXES
### 🟠 HIGH-1: JWT Claims Mismatch (`userId` vs `user_id`)
**Status:****NOT FIXED**
**Location:** `auth-service/main.go:674-682` vs downstream services
**Issue:** Auth service issues tokens with BOTH `userId` (camelCase) and `user_id` (snake_case), but downstream services only check `user_id` first, creating potential bugs.
**Current Code:**
```go
// auth-service generates BOTH
token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
"user_id": userID, // snake_case
"userId": userID, // camelCase - redundant!
"email": email,
"roles": roles,
"exp": time.Now().Add(time.Hour * 24).Unix(),
})
```
**Required Fix:**
```go
// Choose ONE standard (snake_case recommended for consistency with database)
token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
"user_id": userID, // ✅ Primary
"email": email,
"roles": roles,
"iat": time.Now().Unix(),
"exp": time.Now().Add(time.Hour * 24).Unix(),
})
```
**Update all downstream services to use consistent `user_id` only.**
---
### 🟠 HIGH-2: Blockchain Login Replay Attack
**Status:****NOT FIXED**
**Location:** `auth-service/main.go:709-738`
**Issue:** `verifyEthereumSignature` accepts ANY message from the client. A captured signature can be replayed indefinitely.
**Current Vulnerable Code:**
```go
func handleLoginBlockchain(w http.ResponseWriter, r *http.Request) {
var req LoginBlockchainRequest
json.NewDecoder(r.Body).Decode(&req)
// ❌ Message is client-provided, no nonce/timestamp verification
if !verifyEthereumSignature(req.Address, req.Message, req.Signature) {
http.Error(w, "Invalid signature", http.StatusUnauthorized)
return
}
// ... generate token
}
```
**Required Fix:**
**Step 1: Create nonce table**
```sql
CREATE TABLE IF NOT EXISTS auth_nonces (
id SERIAL PRIMARY KEY,
nonce VARCHAR(64) UNIQUE NOT NULL,
address VARCHAR(42) NOT NULL,
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
used_at TIMESTAMP,
expires_at TIMESTAMP NOT NULL
);
CREATE INDEX idx_nonces_address ON auth_nonces(address);
CREATE INDEX idx_nonces_expires ON auth_nonces(expires_at);
```
**Step 2: Add nonce generation endpoint**
```go
func handleGetNonce(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
return
}
var req struct {
Address string `json:"address"`
}
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
return
}
address := normalizeEthereumAddress(req.Address)
if address == "" {
http.Error(w, "Invalid Ethereum address", http.StatusBadRequest)
return
}
// Generate cryptographically secure nonce
nonceBytes := make([]byte, 32)
_, err := rand.Read(nonceBytes)
if err != nil {
http.Error(w, "Failed to generate nonce", http.StatusInternalServerError)
return
}
nonce := hex.EncodeToString(nonceBytes)
// Store nonce (valid for 5 minutes)
expiresAt := time.Now().Add(5 * time.Minute)
_, err = db.Exec(`
INSERT INTO auth_nonces (nonce, address, expires_at)
VALUES ($1, $2, $3)
`, nonce, address, expiresAt)
if err != nil {
log.Printf("Failed to store nonce: %v", err)
http.Error(w, "Server error", http.StatusInternalServerError)
return
}
// Return nonce and message to sign
message := fmt.Sprintf("Sign this message to authenticate with Copper Tone Technologies.\n\nNonce: %s\nTimestamp: %s",
nonce, time.Now().Format(time.RFC3339))
json.NewEncoder(w).Encode(map[string]string{
"nonce": nonce,
"message": message,
})
}
http.HandleFunc("/blockchain/get-nonce", handleGetNonce)
```
**Step 3: Verify and consume nonce on login**
```go
func handleLoginBlockchain(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
return
}
var req LoginBlockchainRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request", http.StatusBadRequest)
return
}
address := normalizeEthereumAddress(req.Address)
// ✅ Verify signature with message
if !verifyEthereumSignature(address, req.Message, req.Signature) {
http.Error(w, "Invalid signature", http.StatusUnauthorized)
return
}
// ✅ Extract nonce from message (must match format from get-nonce)
var nonce string
if matches := regexp.MustCompile(`Nonce: ([a-f0-9]{64})`).FindStringSubmatch(req.Message); len(matches) > 1 {
nonce = matches[1]
} else {
http.Error(w, "Invalid message format", http.StatusUnauthorized)
return
}
// ✅ Verify nonce exists, matches address, not expired, not used
var nonceID int
var usedAt *time.Time
var expiresAt time.Time
err := db.QueryRow(`
SELECT id, used_at, expires_at
FROM auth_nonces
WHERE nonce = $1 AND address = $2
`, nonce, address).Scan(&nonceID, &usedAt, &expiresAt)
if err == sql.ErrNoRows {
http.Error(w, "Invalid or unknown nonce", http.StatusUnauthorized)
return
} else if err != nil {
http.Error(w, "Server error", http.StatusInternalServerError)
return
}
if usedAt != nil {
http.Error(w, "Nonce already used (replay attack detected)", http.StatusUnauthorized)
return
}
if time.Now().After(expiresAt) {
http.Error(w, "Nonce expired", http.StatusUnauthorized)
return
}
// ✅ Mark nonce as used
_, err = db.Exec(`
UPDATE auth_nonces
SET used_at = NOW()
WHERE id = $1
`, nonceID)
if err != nil {
log.Printf("Failed to mark nonce as used: %v", err)
http.Error(w, "Server error", http.StatusInternalServerError)
return
}
// Find identity and generate token (existing logic)
var userID int
err = db.QueryRow(`
SELECT user_id
FROM identities
WHERE type = 'blockchain_address' AND identifier = $1
`, address).Scan(&userID)
if err == sql.ErrNoRows {
http.Error(w, "Address not registered", http.StatusUnauthorized)
return
} else if err != nil {
http.Error(w, "Login failed", http.StatusInternalServerError)
return
}
token, err := generateToken(userID)
if err != nil {
http.Error(w, "Failed to generate token", http.StatusInternalServerError)
return
}
json.NewEncoder(w).Encode(map[string]string{"token": token})
}
```
**Step 4: Add cleanup job for expired nonces**
```go
func cleanupExpiredNonces() {
ticker := time.NewTicker(1 * time.Hour)
defer ticker.Stop()
for range ticker.C {
result, err := db.Exec(`
DELETE FROM auth_nonces
WHERE expires_at < NOW() - INTERVAL '1 hour'
`)
if err != nil {
log.Printf("Failed to cleanup expired nonces: %v", err)
} else {
rowsAffected, _ := result.RowsAffected()
if rowsAffected > 0 {
log.Printf("Cleaned up %d expired nonces", rowsAffected)
}
}
}
}
// In main():
go cleanupExpiredNonces()
```
---
### 🟠 HIGH-3: Over-Permissive CORS Configuration
**Status:****NOT FIXED**
**Location:** All services `corsMiddleware`
**Issue:** Default CORS allows `*` (any origin), making the system vulnerable to credential theft if tokens are stored client-side.
**Current Code:**
```go
func corsMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
allowedOrigin := strings.TrimSpace(os.Getenv("CORS_ALLOW_ORIGIN"))
if allowedOrigin == "" {
allowedOrigin = "*" // ❌ Insecure default!
}
w.Header().Set("Access-Control-Allow-Origin", allowedOrigin)
// ...
})
}
```
**Required Fix:**
```go
func corsMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
allowedOrigin := strings.TrimSpace(os.Getenv("CORS_ALLOW_ORIGIN"))
// ✅ Fail-safe: require explicit configuration
if allowedOrigin == "" {
log.Println("ERROR: CORS_ALLOW_ORIGIN not set. Defaulting to localhost for development.")
allowedOrigin = "http://localhost:8080" // Safe default for dev
}
// ✅ Warn if using wildcard
if allowedOrigin == "*" {
log.Println("WARNING: CORS is set to '*' (all origins). This is INSECURE for production!")
}
w.Header().Set("Access-Control-Allow-Origin", allowedOrigin)
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
// ✅ Add security headers
if allowedOrigin != "*" {
w.Header().Set("Access-Control-Allow-Credentials", "true")
}
if r.Method == "OPTIONS" {
w.WriteHeader(http.StatusOK)
return
}
next.ServeHTTP(w, r)
})
}
```
**Update `.env.example`:**
```bash
# CORS Configuration
# Development: http://localhost:8080
# Production: https://coppertone.tech
# WARNING: NEVER use '*' in production!
CORS_ALLOW_ORIGIN=http://localhost:8080
```
---
### 🟠 HIGH-4: Default Secrets in podman-compose.yml
**Status:** ⚠️ **PARTIALLY FIXED** (`.env.example` exists but compose has insecure defaults)
**Location:** `podman-compose.yml`
**Issue:** Compose file has fallback values for sensitive data, likely to be deployed to production.
**Current Code:**
```yaml
environment:
JWT_SECRET: ${JWT_SECRET} # ❌ No default is GOOD
DB_USER: ${DB_USER:-user} # ❌ BAD default
DB_PASSWORD: ${DB_PASSWORD:-password} # ❌ TERRIBLE default
DB_NAME: ${DB_NAME:-coppertone_db} # ❌ Default is OK but should be explicit
```
**Required Fix:**
```yaml
environment:
# ✅ NO defaults for secrets - fail fast if not set
JWT_SECRET: ${JWT_SECRET:?JWT_SECRET environment variable is required}
DB_USER: ${DB_USER:?DB_USER environment variable is required}
DB_PASSWORD: ${DB_PASSWORD:?DB_PASSWORD environment variable is required}
DB_NAME: ${DB_NAME:-coppertone_db} # OK to have default
DB_HOST: ${DB_HOST:-db} # OK to have default
DB_SSL_MODE: ${DB_SSL_MODE:-require} # Changed to secure default
CORS_ALLOW_ORIGIN: ${CORS_ALLOW_ORIGIN:-http://localhost:8080} # Explicit dev default
```
**Also update database service:**
```yaml
db:
image: postgres:16-alpine
restart: unless-stopped
environment:
# ✅ Use variables instead of hardcoded values
POSTGRES_DB: ${DB_NAME:-coppertone_db}
POSTGRES_USER: ${DB_USER:?DB_USER required}
POSTGRES_PASSWORD: ${DB_PASSWORD:?DB_PASSWORD required}
```
---
## MEDIUM PRIORITY FIXES
### 🟡 MEDIUM-1: Frontend XSS via Unsanitized Markdown
**Status:****NOT FIXED**
**Location:** `frontend/src/views/ServiceDetailView.vue:11`, `ArticleDetailView.vue:16`
**Issue:** Markdown content is rendered to HTML and injected via `v-html` without sanitization.
**Current Vulnerable Code:**
```vue
<div class="prose max-w-none" v-html="service.content"></div>
```
**Impact:**
- If Markdown content is ever user-generated or from untrusted source, XSS is possible
- Currently safe ONLY because content is static/admin-controlled
**Required Fix:**
**Option A: Use sanitization library (DOMPurify)**
```bash
cd frontend
npm install dompurify
npm install --save-dev @types/dompurify
```
```vue
<script setup lang="ts">
import DOMPurify from 'dompurify'
import { computed } from 'vue'
const service = ref({ content: '' })
const sanitizedContent = computed(() => {
return DOMPurify.sanitize(service.value.content, {
ALLOWED_TAGS: ['p', 'br', 'strong', 'em', 'ul', 'ol', 'li', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'a', 'code', 'pre', 'blockquote'],
ALLOWED_ATTR: ['href', 'target', 'rel', 'class']
})
})
</script>
<template>
<div class="prose max-w-none" v-html="sanitizedContent"></div>
</template>
```
**Option B: Remove `v-html` entirely (safest)**
If Markdown is always static, pre-render it at build time or use a Vue markdown component that auto-escapes.
---
### 🟡 MEDIUM-2: Monetary Values as Floats
**Status:****NOT FIXED**
**Location:** `payment-service/main.go:24-72`
**Issue:** Invoice and payment amounts use `float64`, risking precision errors.
**Current Code:**
```go
type Invoice struct {
Amount float64 `json:"amount"` // ❌ Floating point!
Currency string `json:"currency"`
}
```
**Required Fix:**
**Option A: Store as integer cents**
```go
type Invoice struct {
AmountCents int64 `json:"amountCents"` // ✅ Integer smallest unit
Currency string `json:"currency"`
}
// For display
func (inv *Invoice) AmountDecimal() float64 {
return float64(inv.AmountCents) / 100.0
}
```
**Option B: Use decimal library**
```bash
go get github.com/shopspring/decimal
```
```go
import "github.com/shopspring/decimal"
type Invoice struct {
Amount decimal.Decimal `json:"amount"`
Currency string `json:"currency"`
}
```
**Update database:**
```sql
-- Change to integer cents or use NUMERIC with exact precision
ALTER TABLE invoices ALTER COLUMN amount TYPE BIGINT; -- Store cents
-- OR
ALTER TABLE invoices ALTER COLUMN amount TYPE NUMERIC(12,2); -- Exact decimal
```
---
### 🟡 MEDIUM-3: Container Images Run as Root
**Status:****NOT FIXED**
**Location:** All `Containerfile`s
**Issue:** Final `FROM scratch` images have no USER directive, running as root.
**Current Code:**
```dockerfile
FROM scratch AS production
COPY --from=builder /app/main /main
EXPOSE 8080
CMD ["/main"]
# ❌ No USER directive - runs as root (UID 0)
```
**Required Fix:**
**Option A: Switch to Alpine base (recommended)**
```dockerfile
# Build Stage
FROM golang:1.25-alpine AS builder
WORKDIR /app
COPY go.mod go.sum ./
RUN go mod download
COPY . .
RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -o /app/main .
# Production Stage
FROM alpine:latest AS production
# ✅ Add CA certificates for HTTPS
RUN apk --no-cache add ca-certificates
# ✅ Create non-root user
RUN addgroup -g 1000 appuser && \
adduser -D -u 1000 -G appuser appuser
# ✅ Copy binary and set ownership
COPY --from=builder --chown=appuser:appuser /app/main /app/main
# ✅ Switch to non-root user
USER appuser
EXPOSE 8080
CMD ["/app/main"]
```
**Option B: Keep scratch but add user (more complex)**
```dockerfile
# Requires creating a user in builder stage and copying /etc/passwd
FROM golang:1.25-alpine AS builder
WORKDIR /app
COPY go.mod go.sum ./
RUN go mod download
COPY . .
RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -ldflags="-w -s" -o /app/main .
# Create minimal passwd file
RUN echo "appuser:x:1000:1000::/home/appuser:/sbin/nologin" > /etc/passwd.minimal
FROM scratch AS production
# Copy CA certificates from builder
COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
# Copy minimal passwd
COPY --from=builder /etc/passwd.minimal /etc/passwd
# Copy binary
COPY --from=builder --chown=1000:1000 /app/main /main
USER 1000:1000
EXPOSE 8080
CMD ["/main"]
```
---
### 🟡 MEDIUM-4: No Input Validation (Length, Format, Range)
**Status:** ⚠️ **PARTIALLY IMPLEMENTED** (basic checks exist but incomplete)
**Location:** All handlers across all services
**Issue:** Many inputs lack comprehensive validation.
**Missing Validations:**
**Email:**
```go
// ❌ Current: No format validation
if req.Email == "" {
http.Error(w, "Email required", http.StatusBadRequest)
return
}
// ✅ Required:
func validateEmail(email string) error {
if email == "" {
return errors.New("email is required")
}
if len(email) > 255 {
return errors.New("email too long (max 255 characters)")
}
// Use regexp or library
emailRegex := regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`)
if !emailRegex.MatchString(email) {
return errors.New("invalid email format")
}
return nil
}
```
**Password:**
```go
// ✅ Add minimum/maximum length checks
func validatePassword(password string) error {
if len(password) < 8 {
return errors.New("password must be at least 8 characters")
}
if len(password) > 128 {
return errors.New("password too long (max 128 characters)")
}
// Optional: require complexity
return nil
}
```
**Amounts:**
```go
// ✅ Validate positive, reasonable range
if inv.Amount <= 0 {
http.Error(w, "Amount must be positive", http.StatusBadRequest)
return
}
if inv.Amount > 999999999.99 { // $999,999,999.99
http.Error(w, "Amount exceeds maximum", http.StatusBadRequest)
return
}
```
**Currency Codes:**
```go
// ✅ Whitelist valid ISO 4217 codes
validCurrencies := map[string]bool{"USD": true, "EUR": true, "GBP": true, "BTC": true, "ETH": true}
if !validCurrencies[inv.Currency] {
http.Error(w, "Invalid currency code", http.StatusBadRequest)
return
}
```
**Status Enums:**
```go
// ✅ Validate against allowed values
validInvoiceStatuses := map[string]bool{"DRAFT": true, "ISSUED": true, "PAID": true, "CANCELLED": true}
if !validInvoiceStatuses[inv.Status] {
http.Error(w, "Invalid invoice status", http.StatusBadRequest)
return
}
```
---
## LOW PRIORITY IMPROVEMENTS
### 🔵 LOW-1: JWT Secret Length Not Enforced
**Status:** ⚠️ **PARTIALLY IMPLEMENTED** (checks for 32 chars but not strength)
**Current:**
```go
if len(jwtSecret) < 32 {
log.Fatal("JWT_SECRET must be at least 32 characters")
}
```
**Improvement:**
```go
func validateJWTSecret(secret []byte) error {
if len(secret) == 0 {
return errors.New("JWT_SECRET must not be empty")
}
if len(secret) < 64 {
return errors.New("JWT_SECRET must be at least 64 characters for production use")
}
// Optional: check for weak patterns
if string(secret) == strings.Repeat("a", len(secret)) {
return errors.New("JWT_SECRET is too weak (repeated characters)")
}
return nil
}
if err := validateJWTSecret(jwtSecret); err != nil {
log.Fatalf("Invalid JWT_SECRET: %v", err)
}
```
---
### 🔵 LOW-2: No Rate Limiting / Brute Force Protection
**Status:****NOT IMPLEMENTED**
**Impact:** Login endpoints can be brute-forced
**Recommendation:** Add rate limiting middleware using `golang.org/x/time/rate` or similar.
**Implementation Sketch:**
```go
import "golang.org/x/time/rate"
var loginLimiters = make(map[string]*rate.Limiter)
var limiterMutex sync.RWMutex
func getLoginLimiter(ip string) *rate.Limiter {
limiterMutex.RLock()
limiter, exists := loginLimiters[ip]
limiterMutex.RUnlock()
if !exists {
limiter = rate.NewLimiter(rate.Every(1*time.Minute), 5) // 5 attempts per minute
limiterMutex.Lock()
loginLimiters[ip] = limiter
limiterMutex.Unlock()
}
return limiter
}
func rateLimitMiddleware(next http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ip := r.RemoteAddr
limiter := getLoginLimiter(ip)
if !limiter.Allow() {
http.Error(w, "Too many requests. Please try again later.", http.StatusTooManyRequests)
return
}
next.ServeHTTP(w, r)
}
}
http.HandleFunc("/login-email-password", rateLimitMiddleware(handleLoginEmailPassword))
```
---
### 🔵 LOW-3: Missing Frontend Navigation Guard for Roles
**Status:** ⚠️ **PARTIALLY IMPLEMENTED** (has `requiresAuth` but no role checking)
**Location:** `frontend/src/router/index.ts`
**Current:**
```typescript
router.beforeEach((to, from, next) => {
const authStore = useAuthStore()
if (to.meta.requiresAuth && !authStore.isAuthenticated) {
next('/login')
} else if (to.meta.requiresGuest && authStore.isAuthenticated) {
next('/dashboard')
} else {
next()
}
})
```
**Improvement (add role-based guards):**
```typescript
router.beforeEach((to, from, next) => {
const authStore = useAuthStore()
if (to.meta.requiresAuth && !authStore.isAuthenticated) {
next('/login')
return
}
if (to.meta.requiresGuest && authStore.isAuthenticated) {
next('/dashboard')
return
}
// ✅ Add role-based access control
if (to.meta.requiresRole) {
const requiredRoles = Array.isArray(to.meta.requiresRole)
? to.meta.requiresRole
: [to.meta.requiresRole]
const hasRequiredRole = requiredRoles.some(role =>
authStore.user?.roles?.includes(role)
)
if (!hasRequiredRole) {
next('/unauthorized') // or '/dashboard'
return
}
}
next()
})
```
**Add to route meta:**
```typescript
{
path: '/admin/users',
name: 'admin-users',
component: () => import('../views/AdminUsersView.vue'),
meta: { requiresAuth: true, requiresRole: ['ADMIN'] }
},
```
---
### 🔵 LOW-4: No Request Timeouts
**Status:****ALREADY IMPLEMENTED** in server config but not enforced in handlers
**Current (GOOD):**
```go
server := &http.Server{
Addr: ":8080",
Handler: corsMiddleware(http.DefaultServeMux),
ReadHeaderTimeout: 10 * time.Second, // ✅ Present
ReadTimeout: 15 * time.Second, // ✅ Present
WriteTimeout: 15 * time.Second, // ✅ Present
IdleTimeout: 60 * time.Second, // ✅ Present
}
```
**No additional action needed** - server-level timeouts are configured correctly.
---
## INFRASTRUCTURE & DEVOPS FIXES
### 🔧 INFRA-1: Missing Health Checks in Containerfiles
**Status:****NOT IMPLEMENTED**
**Recommendation:** Add HEALTHCHECK directives to all service Containerfiles
```dockerfile
# Add to each service Containerfile
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD ["/main", "-healthcheck"] || exit 1
# Or use wget/curl if available in image
# For Alpine-based:
HEALTHCHECK --interval=30s --timeout=3s CMD wget --no-verbose --tries=1 --spider http://localhost:8080/healthz || exit 1
```
---
### 🔧 INFRA-2: No Production Deployment Documentation
**Status:****NOT IMPLEMENTED**
**Recommendation:** Create `docs/DEPLOYMENT.md` with:
- Production checklist (TLS, secrets, CORS, etc.)
- Environment variable reference
- Database backup/restore procedures
- Monitoring and logging setup
- Incident response plan
---
### 🔧 INFRA-3: No Automated Testing in CI for Backend
**Status:** ⚠️ **PARTIALLY IMPLEMENTED** (tests exist but CI doesn't run them)
**Recommendation:** Add test steps to `.gitea/workflows/` for backend services:
```yaml
- name: Run Go Tests
run: |
cd backend/functions/auth-service && go test -v ./...
cd ../work-management-service && go test -v ./...
cd ../payment-service && go test -v ./...
```
---
## COMPLIANCE & DOCUMENTATION
### 📋 COMPLIANCE-1: No Privacy Policy
**Status:****NOT IMPLEMENTED**
**Required for:** GDPR compliance, user trust
**Action:** Create `/privacy` page explaining data collection, storage, and user rights
---
### 📋 COMPLIANCE-2: No Terms of Service
**Status:****NOT IMPLEMENTED**
**Required for:** Legal protection
**Action:** Create `/terms` page with service terms and conditions
---
### 📋 COMPLIANCE-3: No Data Retention Policy
**Status:****NOT IMPLEMENTED**
**Required for:** GDPR Article 5
**Action:** Document how long data is kept and implement automated cleanup
---
## SUMMARY & PRIORITIZATION
### Immediate Actions (Before Any Production Use)
1. **🔴 CRITICAL-1:** Fix user self-assigned roles
2. **🔴 CRITICAL-2:** Add authorization/ownership checks to ALL resources
3. **🔴 CRITICAL-3:** Implement Stripe webhook signature verification and event handling
4. **🔴 CRITICAL-4:** Enable database TLS and change default to `require`
### Short-Term (Within 1 Week)
5. **🟠 HIGH-1:** Standardize JWT claims (`user_id` only)
6. **🟠 HIGH-2:** Implement nonce-based blockchain authentication
7. **🟠 HIGH-3:** Restrict CORS to specific origins
8. **🟠 HIGH-4:** Remove default secrets from podman-compose.yml
### Medium-Term (Within 1 Month)
9. **🟡 MEDIUM-1:** Sanitize Markdown rendering or pre-render
10. **🟡 MEDIUM-2:** Convert monetary amounts to integers or decimal type
11. **🟡 MEDIUM-3:** Run containers as non-root user
12. **🟡 MEDIUM-4:** Add comprehensive input validation
### Long-Term Improvements
13. **🔵 LOW-1 through LOW-4:** JWT secret strength, rate limiting, role guards, etc.
14. **🔧 INFRA-1 through INFRA-3:** Health checks, deployment docs, CI tests
15. **📋 COMPLIANCE-1 through COMPLIANCE-3:** Legal documents and policies
---
## TESTING & VALIDATION CHECKLIST
After implementing fixes, verify:
- [ ] Cannot register as ADMIN (should default to CLIENT)
- [ ] Cannot access another user's projects/invoices
- [ ] Cannot send fake Stripe webhook events
- [ ] Database connections use TLS in production
- [ ] Blockchain login requires fresh nonce
- [ ] CORS restricted to app domain
- [ ] Containers run as non-root (check with `podman exec <container> whoami`)
- [ ] All tests pass (`go test ./...`, `npm run test:unit`)
- [ ] Static analysis passes (`go vet`, `npm run lint`)
- [ ] No secrets in git history or environment defaults
---
## CONCLUSION
The Copper Tone Technologies codebase is **architecturally sound** and **feature-complete**, but has **critical security vulnerabilities** that MUST be addressed before production deployment.
**Estimated remediation time:** 40-60 hours for critical and high-priority fixes.
**Risk if deployed as-is:** Complete system compromise, data breach, financial fraud.
**Recommendation:** **DO NOT DEPLOY** until at minimum all CRITICAL fixes are implemented and tested.