Fix 113 hardening issues across entire Go backend

Security:
- Replace all binding: tags with validate: + c.Validate() in admin handlers
- Add rate limiting to auth endpoints (login, register, password reset)
- Add security headers (HSTS, XSS protection, nosniff, frame options)
- Wire Google Pub/Sub token verification into webhook handler
- Replace ParseUnverified with proper OIDC/JWKS key verification
- Verify inner Apple JWS signatures in webhook handler
- Add io.LimitReader (1MB) to all webhook body reads
- Add ownership verification to file deletion
- Move hardcoded admin credentials to env vars
- Add uniqueIndex to User.Email
- Hide ConfirmationCode from JSON serialization
- Mask confirmation codes in admin responses
- Use http.DetectContentType for upload validation
- Fix path traversal in storage service
- Replace os.Getenv with Viper in stripe service
- Sanitize Redis URLs before logging
- Separate DEBUG_FIXED_CODES from DEBUG flag
- Reject weak SECRET_KEY in production
- Add host check on /_next/* proxy routes
- Use explicit localhost CORS origins in debug mode
- Replace err.Error() with generic messages in all admin error responses

Critical fixes:
- Rewrite FCM to HTTP v1 API with OAuth 2.0 service account auth
- Fix user_customuser -> auth_user table names in raw SQL
- Fix dashboard verified query to use UserProfile model
- Add escapeLikeWildcards() to prevent SQL wildcard injection

Bug fixes:
- Add bounds checks for days/expiring_soon query params (1-3650)
- Add receipt_data/transaction_id empty-check to RestoreSubscription
- Change Active bool -> *bool in device handler
- Check all unchecked GORM/FindByIDWithProfile errors
- Add validation for notification hour fields (0-23)
- Add max=10000 validation on task description updates

Transactions & data integrity:
- Wrap registration flow in transaction
- Wrap QuickComplete in transaction
- Move image creation inside completion transaction
- Wrap SetSpecialties in transaction
- Wrap GetOrCreateToken in transaction
- Wrap completion+image deletion in transaction

Performance:
- Batch completion summaries (2 queries vs 2N)
- Reuse single http.Client in IAP validation
- Cache dashboard counts (30s TTL)
- Batch COUNT queries in admin user list
- Add Limit(500) to document queries
- Add reminder_stage+due_date filters to reminder queries
- Parse AllowedTypes once at init
- In-memory user cache in auth middleware (30s TTL)
- Timezone change detection cache
- Optimize P95 with per-endpoint sorted buffers
- Replace crypto/md5 with hash/fnv for ETags

Code quality:
- Add sync.Once to all monitoring Stop()/Close() methods
- Replace 8 fmt.Printf with zerolog in auth service
- Log previously discarded errors
- Standardize delete response shapes
- Route hardcoded English through i18n
- Remove FileURL from DocumentResponse (keep MediaURL only)
- Thread user timezone through kanban board responses
- Initialize empty slices to prevent null JSON
- Extract shared field map for task Update/UpdateTx
- Delete unused SoftDeleteModel, min(), formatCron, legacy handlers

Worker & jobs:
- Wire Asynq email infrastructure into worker
- Register HandleReminderLogCleanup with daily 3AM cron
- Use per-user timezone in HandleSmartReminder
- Replace direct DB queries with repository calls
- Delete legacy reminder handlers (~200 lines)
- Delete unused task type constants

Dependencies:
- Replace archived jung-kurt/gofpdf with go-pdf/fpdf
- Replace unmaintained gomail.v2 with wneessen/go-mail
- Add TODO for Echo jwt v3 transitive dep removal

Test infrastructure:
- Fix MakeRequest/SeedLookupData error handling
- Replace os.Exit(0) with t.Skip() in scope/consistency tests
- Add 11 new FCM v1 tests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Trey t
2026-03-18 23:14:13 -05:00
parent 3b86d0aae1
commit 42a5533a56
95 changed files with 2892 additions and 1783 deletions

View File

@@ -4,6 +4,7 @@ import (
"fmt"
"io"
"mime/multipart"
"net/http"
"os"
"path/filepath"
"strings"
@@ -17,7 +18,8 @@ import (
// StorageService handles file uploads to local filesystem
type StorageService struct {
cfg *config.StorageConfig
cfg *config.StorageConfig
allowedTypes map[string]struct{} // P-12: Parsed once at init for O(1) lookups
}
// UploadResult contains information about an uploaded file
@@ -44,9 +46,18 @@ func NewStorageService(cfg *config.StorageConfig) (*StorageService, error) {
}
}
log.Info().Str("upload_dir", cfg.UploadDir).Msg("Storage service initialized")
// P-12: Parse AllowedTypes once at initialization for O(1) lookups
allowedTypes := make(map[string]struct{})
for _, t := range strings.Split(cfg.AllowedTypes, ",") {
trimmed := strings.TrimSpace(t)
if trimmed != "" {
allowedTypes[trimmed] = struct{}{}
}
}
return &StorageService{cfg: cfg}, nil
log.Info().Str("upload_dir", cfg.UploadDir).Int("allowed_types", len(allowedTypes)).Msg("Storage service initialized")
return &StorageService{cfg: cfg, allowedTypes: allowedTypes}, nil
}
// Upload saves a file to the local filesystem
@@ -56,17 +67,47 @@ func (s *StorageService) Upload(file *multipart.FileHeader, category string) (*U
return nil, fmt.Errorf("file size %d exceeds maximum allowed %d bytes", file.Size, s.cfg.MaxFileSize)
}
// Get MIME type
mimeType := file.Header.Get("Content-Type")
if mimeType == "" {
mimeType = "application/octet-stream"
// Get claimed MIME type from header
claimedMimeType := file.Header.Get("Content-Type")
if claimedMimeType == "" {
claimedMimeType = "application/octet-stream"
}
// Validate MIME type
// S-09: Detect actual content type from file bytes to prevent disguised uploads
src, err := file.Open()
if err != nil {
return nil, fmt.Errorf("failed to open uploaded file: %w", err)
}
defer src.Close()
// Read the first 512 bytes for content type detection
sniffBuf := make([]byte, 512)
n, err := src.Read(sniffBuf)
if err != nil && n == 0 {
return nil, fmt.Errorf("failed to read file for content type detection: %w", err)
}
detectedMimeType := http.DetectContentType(sniffBuf[:n])
// Validate that the detected type matches the claimed type (at the category level)
// Allow application/octet-stream from detection since DetectContentType may not
// recognize all valid types, but the claimed type must still be in our allowed list
if detectedMimeType != "application/octet-stream" && !s.mimeTypesCompatible(claimedMimeType, detectedMimeType) {
return nil, fmt.Errorf("file content type mismatch: claimed %s but detected %s", claimedMimeType, detectedMimeType)
}
// Use the claimed MIME type (which is more specific) if it's allowed
mimeType := claimedMimeType
// Validate MIME type against allowed list
if !s.isAllowedType(mimeType) {
return nil, fmt.Errorf("file type %s is not allowed", mimeType)
}
// Seek back to beginning after sniffing
if _, err := src.Seek(0, io.SeekStart); err != nil {
return nil, fmt.Errorf("failed to seek file: %w", err)
}
// Generate unique filename
ext := filepath.Ext(file.Filename)
if ext == "" {
@@ -83,15 +124,11 @@ func (s *StorageService) Upload(file *multipart.FileHeader, category string) (*U
subdir = "completions"
}
// Full path
destPath := filepath.Join(s.cfg.UploadDir, subdir, newFilename)
// Open source file
src, err := file.Open()
// S-18: Sanitize path to prevent traversal attacks
destPath, err := SafeResolvePath(s.cfg.UploadDir, filepath.Join(subdir, newFilename))
if err != nil {
return nil, fmt.Errorf("failed to open uploaded file: %w", err)
return nil, fmt.Errorf("invalid upload path: %w", err)
}
defer src.Close()
// Create destination file
dst, err := os.Create(destPath)
@@ -131,19 +168,11 @@ func (s *StorageService) Delete(fileURL string) error {
// Convert URL to file path
relativePath := strings.TrimPrefix(fileURL, s.cfg.BaseURL)
relativePath = strings.TrimPrefix(relativePath, "/")
fullPath := filepath.Join(s.cfg.UploadDir, relativePath)
// Security check: ensure path is within upload directory
absUploadDir, err := filepath.Abs(s.cfg.UploadDir)
// S-18: Use SafeResolvePath to prevent path traversal
fullPath, err := SafeResolvePath(s.cfg.UploadDir, relativePath)
if err != nil {
return fmt.Errorf("failed to resolve upload directory: %w", err)
}
absFilePath, err := filepath.Abs(fullPath)
if err != nil {
return fmt.Errorf("failed to resolve file path: %w", err)
}
if !strings.HasPrefix(absFilePath, absUploadDir+string(filepath.Separator)) && absFilePath != absUploadDir {
return fmt.Errorf("invalid file path")
return fmt.Errorf("invalid file path: %w", err)
}
if err := os.Remove(fullPath); err != nil {
@@ -157,15 +186,23 @@ func (s *StorageService) Delete(fileURL string) error {
return nil
}
// isAllowedType checks if the MIME type is in the allowed list
// isAllowedType checks if the MIME type is in the allowed list.
// P-12: Uses the pre-parsed allowedTypes map for O(1) lookups instead of
// splitting the config string on every call.
func (s *StorageService) isAllowedType(mimeType string) bool {
allowed := strings.Split(s.cfg.AllowedTypes, ",")
for _, t := range allowed {
if strings.TrimSpace(t) == mimeType {
return true
}
_, ok := s.allowedTypes[mimeType]
return ok
}
// mimeTypesCompatible checks if the claimed and detected MIME types are compatible.
// Two MIME types are compatible if they share the same primary type (e.g., both "image/*").
func (s *StorageService) mimeTypesCompatible(claimed, detected string) bool {
claimedParts := strings.SplitN(claimed, "/", 2)
detectedParts := strings.SplitN(detected, "/", 2)
if len(claimedParts) < 1 || len(detectedParts) < 1 {
return false
}
return false
return claimedParts[0] == detectedParts[0]
}
// getExtensionFromMimeType returns a file extension for common MIME types
@@ -191,5 +228,12 @@ func (s *StorageService) GetUploadDir() string {
// NewStorageServiceForTest creates a StorageService without creating directories.
// This is intended only for unit tests that need a StorageService with a known config.
func NewStorageServiceForTest(cfg *config.StorageConfig) *StorageService {
return &StorageService{cfg: cfg}
allowedTypes := make(map[string]struct{})
for _, t := range strings.Split(cfg.AllowedTypes, ",") {
trimmed := strings.TrimSpace(t)
if trimmed != "" {
allowedTypes[trimmed] = struct{}{}
}
}
return &StorageService{cfg: cfg, allowedTypes: allowedTypes}
}