Add delete account endpoint and file encryption at rest

Delete Account (Plan #2): - DELETE /api/auth/account/ with password or "DELETE" confirmation - Cascade delete across 15+ tables in correct FK order - Auth provider detection (email/apple/google) for /auth/me/ - File cleanup after account deletion - Handler + repository tests (12 tests) Encryption at Rest (Plan #3): - AES-256-GCM envelope encryption (per-file DEK wrapped by KEK) - Encrypt on upload, auto-decrypt on serve via StorageService.ReadFile() - MediaHandler serves decrypted files via c.Blob() - TaskService email image loading uses ReadFile() - cmd/migrate-encrypt CLI tool with --dry-run for existing files - Encryption service + storage service tests (18 tests)
2026-03-26 10:41:01 -05:00
parent 72866e935e
commit 4abc57535e
22 changed files with 1675 additions and 82 deletions
--- a/internal/services/auth_service.go
+++ b/internal/services/auth_service.go
@@ -200,10 +200,69 @@ func (s *AuthService) GetCurrentUser(userID uint) (*responses.CurrentUserRespons
 		return nil, err
 	}

-	response := responses.NewCurrentUserResponse(user)
+	authProvider, err := s.userRepo.FindAuthProvider(userID)
+	if err != nil {
+		// Log but don't fail - default to "email"
+		log.Warn().Err(err).Uint("user_id", userID).Msg("Failed to determine auth provider")
+		authProvider = "email"
+	}
+
+	response := responses.NewCurrentUserResponse(user, authProvider)
 	return &response, nil
 }

+// DeleteAccount deletes a user's account and all associated data.
+// For email auth users, password verification is required.
+// For social auth users, confirmation string "DELETE" is required.
+// Returns a list of file URLs that need to be deleted from disk.
+func (s *AuthService) DeleteAccount(userID uint, password, confirmation *string) ([]string, error) {
+	// Fetch user
+	user, err := s.userRepo.FindByID(userID)
+	if err != nil {
+		if errors.Is(err, repositories.ErrUserNotFound) {
+			return nil, apperrors.NotFound("error.user_not_found")
+		}
+		return nil, apperrors.Internal(err)
+	}
+
+	// Determine auth provider
+	authProvider, err := s.userRepo.FindAuthProvider(userID)
+	if err != nil {
+		return nil, apperrors.Internal(err)
+	}
+
+	// Validate credentials based on auth provider
+	if authProvider == "email" {
+		if password == nil || *password == "" {
+			return nil, apperrors.BadRequest("error.password_required")
+		}
+		if !user.CheckPassword(*password) {
+			return nil, apperrors.Unauthorized("error.invalid_credentials")
+		}
+	} else {
+		// Social auth (apple or google) - require confirmation
+		if confirmation == nil || *confirmation != "DELETE" {
+			return nil, apperrors.BadRequest("error.confirmation_required")
+		}
+	}
+
+	// Start transaction and cascade delete
+	var fileURLs []string
+	txErr := s.userRepo.Transaction(func(txRepo *repositories.UserRepository) error {
+		urls, err := txRepo.DeleteUserCascade(userID)
+		if err != nil {
+			return err
+		}
+		fileURLs = urls
+		return nil
+	})
+	if txErr != nil {
+		return nil, apperrors.Internal(txErr)
+	}
+
+	return fileURLs, nil
+}
+
 // UpdateProfile updates a user's profile
 func (s *AuthService) UpdateProfile(userID uint, req *requests.UpdateProfileRequest) (*responses.CurrentUserResponse, error) {
 	user, err := s.userRepo.FindByID(userID)
@@ -240,7 +299,13 @@ func (s *AuthService) UpdateProfile(userID uint, req *requests.UpdateProfileRequ
 		return nil, err
 	}

-	response := responses.NewCurrentUserResponse(user)
+	authProvider, err := s.userRepo.FindAuthProvider(userID)
+	if err != nil {
+		log.Warn().Err(err).Uint("user_id", userID).Msg("Failed to determine auth provider")
+		authProvider = "email"
+	}
+
+	response := responses.NewCurrentUserResponse(user, authProvider)
 	return &response, nil
 }

--- a/internal/services/encryption_service.go
+++ b/internal/services/encryption_service.go
@@ -0,0 +1,179 @@
+package services
+
+import (
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rand"
+	"encoding/hex"
+	"fmt"
+	"io"
+)
+
+const (
+	// encryptionVersion is the current file format version byte.
+	encryptionVersion byte = 0x01
+
+	// aes256KeyLen is the required key length in bytes for AES-256.
+	aes256KeyLen = 32
+
+	// gcmNonceSize is the standard GCM nonce size (12 bytes).
+	gcmNonceSize = 12
+
+	// gcmTagSize is the standard GCM authentication tag size (16 bytes).
+	gcmTagSize = 16
+
+	// encryptedDEKSize is the encrypted DEK length: 32-byte DEK + 16-byte GCM tag.
+	encryptedDEKSize = aes256KeyLen + gcmTagSize
+
+	// headerSize is the fixed header: version(1) + KEK nonce(12) + encrypted DEK(48) + DEK nonce(12).
+	headerSize = 1 + gcmNonceSize + encryptedDEKSize + gcmNonceSize
+)
+
+// EncryptionService provides AES-256-GCM envelope encryption for files at rest.
+type EncryptionService struct {
+	kek []byte // Key Encryption Key (32 bytes)
+}
+
+// NewEncryptionService creates an EncryptionService from a 64-character hex-encoded KEK.
+func NewEncryptionService(hexKey string) (*EncryptionService, error) {
+	if len(hexKey) != 64 {
+		return nil, fmt.Errorf("encryption key must be exactly 64 hex characters (got %d)", len(hexKey))
+	}
+
+	kek, err := hex.DecodeString(hexKey)
+	if err != nil {
+		return nil, fmt.Errorf("invalid hex in encryption key: %w", err)
+	}
+
+	if len(kek) != aes256KeyLen {
+		return nil, fmt.Errorf("decoded key must be %d bytes", aes256KeyLen)
+	}
+
+	return &EncryptionService{kek: kek}, nil
+}
+
+// IsEnabled returns true if the encryption service is configured and ready.
+func (s *EncryptionService) IsEnabled() bool {
+	return s != nil && len(s.kek) == aes256KeyLen
+}
+
+// Encrypt encrypts plaintext using envelope encryption (random DEK encrypted with KEK).
+//
+// File format:
+//
+//	[1-byte version 0x01]
+//	[12-byte KEK nonce]
+//	[48-byte encrypted DEK (32-byte DEK + 16-byte GCM tag)]
+//	[12-byte DEK nonce]
+//	[ciphertext + 16-byte GCM tag]
+func (s *EncryptionService) Encrypt(plaintext []byte) ([]byte, error) {
+	// Generate a random Data Encryption Key (DEK)
+	dek := make([]byte, aes256KeyLen)
+	if _, err := io.ReadFull(rand.Reader, dek); err != nil {
+		return nil, fmt.Errorf("failed to generate DEK: %w", err)
+	}
+
+	// Encrypt the DEK with the KEK
+	kekBlock, err := aes.NewCipher(s.kek)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create KEK cipher: %w", err)
+	}
+	kekGCM, err := cipher.NewGCM(kekBlock)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create KEK GCM: %w", err)
+	}
+
+	kekNonce := make([]byte, gcmNonceSize)
+	if _, err := io.ReadFull(rand.Reader, kekNonce); err != nil {
+		return nil, fmt.Errorf("failed to generate KEK nonce: %w", err)
+	}
+	encryptedDEK := kekGCM.Seal(nil, kekNonce, dek, nil)
+
+	// Encrypt the plaintext with the DEK
+	dekBlock, err := aes.NewCipher(dek)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create DEK cipher: %w", err)
+	}
+	dekGCM, err := cipher.NewGCM(dekBlock)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create DEK GCM: %w", err)
+	}
+
+	dekNonce := make([]byte, gcmNonceSize)
+	if _, err := io.ReadFull(rand.Reader, dekNonce); err != nil {
+		return nil, fmt.Errorf("failed to generate DEK nonce: %w", err)
+	}
+	ciphertext := dekGCM.Seal(nil, dekNonce, plaintext, nil)
+
+	// Pack the output: version + kekNonce + encryptedDEK + dekNonce + ciphertext
+	out := make([]byte, 0, headerSize+len(ciphertext))
+	out = append(out, encryptionVersion)
+	out = append(out, kekNonce...)
+	out = append(out, encryptedDEK...)
+	out = append(out, dekNonce...)
+	out = append(out, ciphertext...)
+
+	return out, nil
+}
+
+// Decrypt reverses the Encrypt operation, recovering the original plaintext.
+func (s *EncryptionService) Decrypt(blob []byte) ([]byte, error) {
+	if len(blob) < headerSize {
+		return nil, fmt.Errorf("ciphertext too short (%d bytes, minimum %d)", len(blob), headerSize)
+	}
+
+	// Parse version
+	version := blob[0]
+	if version != encryptionVersion {
+		return nil, fmt.Errorf("unsupported encryption version: 0x%02x", version)
+	}
+
+	offset := 1
+
+	// Parse KEK nonce
+	kekNonce := blob[offset : offset+gcmNonceSize]
+	offset += gcmNonceSize
+
+	// Parse encrypted DEK
+	encryptedDEK := blob[offset : offset+encryptedDEKSize]
+	offset += encryptedDEKSize
+
+	// Parse DEK nonce
+	dekNonce := blob[offset : offset+gcmNonceSize]
+	offset += gcmNonceSize
+
+	// Remaining bytes are the ciphertext + GCM tag
+	ciphertext := blob[offset:]
+
+	// Decrypt the DEK with the KEK
+	kekBlock, err := aes.NewCipher(s.kek)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create KEK cipher: %w", err)
+	}
+	kekGCM, err := cipher.NewGCM(kekBlock)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create KEK GCM: %w", err)
+	}
+
+	dek, err := kekGCM.Open(nil, kekNonce, encryptedDEK, nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decrypt DEK (wrong key?): %w", err)
+	}
+
+	// Decrypt the plaintext with the DEK
+	dekBlock, err := aes.NewCipher(dek)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create DEK cipher: %w", err)
+	}
+	dekGCM, err := cipher.NewGCM(dekBlock)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create DEK GCM: %w", err)
+	}
+
+	plaintext, err := dekGCM.Open(nil, dekNonce, ciphertext, nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decrypt data (tampered?): %w", err)
+	}
+
+	return plaintext, nil
+}
--- a/internal/services/encryption_service_test.go
+++ b/internal/services/encryption_service_test.go
@@ -0,0 +1,218 @@
+package services
+
+import (
+	"bytes"
+	"crypto/rand"
+	"encoding/hex"
+	"testing"
+)
+
+// validTestKey returns a deterministic 64-char hex key for tests.
+func validTestKey() string {
+	return "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+}
+
+// randomHexKey generates a random 64-char hex key.
+func randomHexKey(t *testing.T) string {
+	t.Helper()
+	b := make([]byte, 32)
+	if _, err := rand.Read(b); err != nil {
+		t.Fatal(err)
+	}
+	return hex.EncodeToString(b)
+}
+
+func TestNewEncryptionService_Valid(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if svc == nil {
+		t.Fatal("expected non-nil service")
+	}
+}
+
+func TestNewEncryptionService_InvalidHex(t *testing.T) {
+	// 64 chars but not valid hex
+	_, err := NewEncryptionService("zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz")
+	if err == nil {
+		t.Fatal("expected error for invalid hex")
+	}
+}
+
+func TestNewEncryptionService_WrongLength(t *testing.T) {
+	tests := []struct {
+		name string
+		key  string
+	}{
+		{"too short", "0123456789abcdef"},
+		{"too long", "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef00"},
+		{"empty", ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := NewEncryptionService(tt.key)
+			if err == nil {
+				t.Fatal("expected error for wrong length key")
+			}
+		})
+	}
+}
+
+func TestEncryptDecrypt_RoundTrip(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	plaintext := []byte("Hello, encryption at rest!")
+
+	ciphertext, err := svc.Encrypt(plaintext)
+	if err != nil {
+		t.Fatalf("encrypt failed: %v", err)
+	}
+
+	decrypted, err := svc.Decrypt(ciphertext)
+	if err != nil {
+		t.Fatalf("decrypt failed: %v", err)
+	}
+
+	if !bytes.Equal(plaintext, decrypted) {
+		t.Fatalf("round-trip mismatch: got %q, want %q", decrypted, plaintext)
+	}
+}
+
+func TestEncryptDecrypt_EmptyPlaintext(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ciphertext, err := svc.Encrypt([]byte{})
+	if err != nil {
+		t.Fatalf("encrypt failed: %v", err)
+	}
+
+	decrypted, err := svc.Decrypt(ciphertext)
+	if err != nil {
+		t.Fatalf("decrypt failed: %v", err)
+	}
+
+	if len(decrypted) != 0 {
+		t.Fatalf("expected empty plaintext, got %d bytes", len(decrypted))
+	}
+}
+
+func TestEncrypt_DifferentCiphertexts(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	plaintext := []byte("same input")
+	ct1, err := svc.Encrypt(plaintext)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ct2, err := svc.Encrypt(plaintext)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if bytes.Equal(ct1, ct2) {
+		t.Fatal("encrypting the same plaintext twice should produce different ciphertexts")
+	}
+}
+
+func TestDecrypt_TamperDetection(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ciphertext, err := svc.Encrypt([]byte("sensitive data"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Flip a byte near the end (in the ciphertext portion)
+	tampered := make([]byte, len(ciphertext))
+	copy(tampered, ciphertext)
+	tampered[len(tampered)-1] ^= 0xFF
+
+	_, err = svc.Decrypt(tampered)
+	if err == nil {
+		t.Fatal("expected error when decrypting tampered ciphertext")
+	}
+}
+
+func TestDecrypt_WrongKEK(t *testing.T) {
+	svc1, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ciphertext, err := svc1.Encrypt([]byte("secret"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a second service with a different key
+	svc2, err := NewEncryptionService(randomHexKey(t))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = svc2.Decrypt(ciphertext)
+	if err == nil {
+		t.Fatal("expected error when decrypting with wrong KEK")
+	}
+}
+
+func TestDecrypt_TooShort(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = svc.Decrypt([]byte("short"))
+	if err == nil {
+		t.Fatal("expected error for too-short ciphertext")
+	}
+}
+
+func TestDecrypt_BadVersion(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ciphertext, err := svc.Encrypt([]byte("data"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Change version byte
+	ciphertext[0] = 0xFF
+
+	_, err = svc.Decrypt(ciphertext)
+	if err == nil {
+		t.Fatal("expected error for bad version")
+	}
+}
+
+func TestIsEnabled(t *testing.T) {
+	svc, err := NewEncryptionService(validTestKey())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !svc.IsEnabled() {
+		t.Fatal("expected IsEnabled() to return true")
+	}
+
+	var nilSvc *EncryptionService
+	if nilSvc.IsEnabled() {
+		t.Fatal("expected IsEnabled() to return false for nil service")
+	}
+}
--- a/internal/services/storage_service.go
+++ b/internal/services/storage_service.go
@@ -18,8 +18,9 @@ import (

 // StorageService handles file uploads to local filesystem
 type StorageService struct {
-	cfg          *config.StorageConfig
-	allowedTypes map[string]struct{} // P-12: Parsed once at init for O(1) lookups
+	cfg           *config.StorageConfig
+	allowedTypes  map[string]struct{} // P-12: Parsed once at init for O(1) lookups
+	encryptionSvc *EncryptionService
 }

 // UploadResult contains information about an uploaded file
@@ -124,28 +125,39 @@ func (s *StorageService) Upload(file *multipart.FileHeader, category string) (*U
 		subdir = "completions"
 	}

+	// If encryption is enabled, append .enc suffix to the stored filename
+	storedFilename := newFilename
+	if s.encryptionSvc.IsEnabled() {
+		storedFilename = newFilename + ".enc"
+	}
+
 	// S-18: Sanitize path to prevent traversal attacks
-	destPath, err := SafeResolvePath(s.cfg.UploadDir, filepath.Join(subdir, newFilename))
+	destPath, err := SafeResolvePath(s.cfg.UploadDir, filepath.Join(subdir, storedFilename))
 	if err != nil {
 		return nil, fmt.Errorf("invalid upload path: %w", err)
 	}

-	// Create destination file
-	dst, err := os.Create(destPath)
+	// Read all file content into memory for potential encryption
+	fileData, err := io.ReadAll(src)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create destination file: %w", err)
+		return nil, fmt.Errorf("failed to read file content: %w", err)
 	}
-	defer dst.Close()

-	// Copy file content
-	written, err := io.Copy(dst, src)
-	if err != nil {
-		// Clean up on error
-		os.Remove(destPath)
+	// Encrypt if encryption is enabled
+	if s.encryptionSvc.IsEnabled() {
+		fileData, err = s.encryptionSvc.Encrypt(fileData)
+		if err != nil {
+			return nil, fmt.Errorf("failed to encrypt file: %w", err)
+		}
+	}
+
+	// Write file content to disk
+	if err := os.WriteFile(destPath, fileData, 0644); err != nil {
 		return nil, fmt.Errorf("failed to save file: %w", err)
 	}
+	written := int64(len(fileData))

-	// Generate URL
+	// Generate URL (always uses the original filename without .enc suffix for the public URL)
 	url := fmt.Sprintf("%s/%s/%s", s.cfg.BaseURL, subdir, newFilename)

 	log.Info().
@@ -163,7 +175,61 @@ func (s *StorageService) Upload(file *multipart.FileHeader, category string) (*U
 	}, nil
 }

-// Delete removes a file from storage
+// ReadFile reads and optionally decrypts a stored file. It returns the plaintext
+// bytes and the detected MIME type. If the file is stored with an .enc suffix,
+// it is decrypted automatically.
+func (s *StorageService) ReadFile(storedURL string) ([]byte, string, error) {
+	if storedURL == "" {
+		return nil, "", fmt.Errorf("empty file URL")
+	}
+
+	// Strip base URL prefix to get relative path
+	relativePath := strings.TrimPrefix(storedURL, s.cfg.BaseURL)
+	relativePath = strings.TrimPrefix(relativePath, "/")
+
+	// Try .enc variant first, then plain file
+	var fullPath string
+	var encrypted bool
+
+	encPath, err := SafeResolvePath(s.cfg.UploadDir, relativePath+".enc")
+	if err == nil {
+		if _, statErr := os.Stat(encPath); statErr == nil {
+			fullPath = encPath
+			encrypted = true
+		}
+	}
+
+	if fullPath == "" {
+		plainPath, err := SafeResolvePath(s.cfg.UploadDir, relativePath)
+		if err != nil {
+			return nil, "", fmt.Errorf("invalid file path: %w", err)
+		}
+		fullPath = plainPath
+	}
+
+	data, err := os.ReadFile(fullPath)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to read file: %w", err)
+	}
+
+	// Decrypt if this is an encrypted file
+	if encrypted {
+		if s.encryptionSvc == nil || !s.encryptionSvc.IsEnabled() {
+			return nil, "", fmt.Errorf("encrypted file found but encryption service is not configured")
+		}
+		data, err = s.encryptionSvc.Decrypt(data)
+		if err != nil {
+			return nil, "", fmt.Errorf("failed to decrypt file: %w", err)
+		}
+	}
+
+	// Detect MIME type from decrypted content
+	mimeType := http.DetectContentType(data)
+
+	return data, mimeType, nil
+}
+
+// Delete removes a file from storage, handling both plain and .enc variants
 func (s *StorageService) Delete(fileURL string) error {
 	// Convert URL to file path
 	relativePath := strings.TrimPrefix(fileURL, s.cfg.BaseURL)
@@ -175,14 +241,35 @@ func (s *StorageService) Delete(fileURL string) error {
 		return fmt.Errorf("invalid file path: %w", err)
 	}

+	// Try to delete the plain file
+	plainDeleted := false
 	if err := os.Remove(fullPath); err != nil {
-		if os.IsNotExist(err) {
-			return nil // File already doesn't exist
+		if !os.IsNotExist(err) {
+			return fmt.Errorf("failed to delete file: %w", err)
 		}
-		return fmt.Errorf("failed to delete file: %w", err)
+	} else {
+		plainDeleted = true
+		log.Info().Str("path", fullPath).Msg("File deleted")
+	}
+
+	// Also try to delete the .enc variant
+	encPath, err := SafeResolvePath(s.cfg.UploadDir, relativePath+".enc")
+	if err == nil {
+		if err := os.Remove(encPath); err != nil {
+			if !os.IsNotExist(err) {
+				return fmt.Errorf("failed to delete encrypted file: %w", err)
+			}
+		} else {
+			log.Info().Str("path", encPath).Msg("Encrypted file deleted")
+			return nil
+		}
+	}
+
+	if !plainDeleted {
+		// Neither file existed — that's OK
+		return nil
 	}

-	log.Info().Str("path", fullPath).Msg("File deleted")
 	return nil
 }

@@ -225,6 +312,11 @@ func (s *StorageService) GetUploadDir() string {
 	return s.cfg.UploadDir
 }

+// SetEncryptionService sets the encryption service for encrypting files at rest
+func (s *StorageService) SetEncryptionService(svc *EncryptionService) {
+	s.encryptionSvc = svc
+}
+
 // 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 {
--- a/internal/services/storage_service_test.go
+++ b/internal/services/storage_service_test.go
@@ -0,0 +1,164 @@
+package services
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/treytartt/honeydue-api/internal/config"
+)
+
+func setupTestStorage(t *testing.T, encrypt bool) (*StorageService, string) {
+	t.Helper()
+
+	tmpDir := t.TempDir()
+
+	cfg := &config.StorageConfig{
+		UploadDir:    tmpDir,
+		BaseURL:      "/uploads",
+		MaxFileSize:  10 * 1024 * 1024,
+		AllowedTypes: "image/jpeg,image/png,application/pdf",
+	}
+
+	svc, err := NewStorageService(cfg)
+	if err != nil {
+		t.Fatalf("failed to create storage service: %v", err)
+	}
+
+	if encrypt {
+		encSvc, err := NewEncryptionService(validTestKey())
+		if err != nil {
+			t.Fatalf("failed to create encryption service: %v", err)
+		}
+		svc.SetEncryptionService(encSvc)
+	}
+
+	return svc, tmpDir
+}
+
+func TestReadFile_PlainFile(t *testing.T) {
+	svc, tmpDir := setupTestStorage(t, false)
+
+	// Write a plain file
+	content := []byte("hello world")
+	dir := filepath.Join(tmpDir, "images")
+	if err := os.WriteFile(filepath.Join(dir, "test.jpg"), content, 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	data, mimeType, err := svc.ReadFile("/uploads/images/test.jpg")
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+
+	if !bytes.Equal(data, content) {
+		t.Fatalf("data mismatch: got %q, want %q", data, content)
+	}
+
+	if mimeType == "" {
+		t.Fatal("expected non-empty MIME type")
+	}
+}
+
+func TestReadFile_EncryptedFile(t *testing.T) {
+	svc, tmpDir := setupTestStorage(t, true)
+
+	// Encrypt and write a file manually (simulating what Upload does)
+	originalContent := []byte("sensitive document content here - must be long enough for detection")
+	encSvc, _ := NewEncryptionService(validTestKey())
+	encrypted, err := encSvc.Encrypt(originalContent)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dir := filepath.Join(tmpDir, "documents")
+	if err := os.WriteFile(filepath.Join(dir, "test.pdf.enc"), encrypted, 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// ReadFile should find the .enc file and decrypt it
+	data, _, err := svc.ReadFile("/uploads/documents/test.pdf")
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+
+	if !bytes.Equal(data, originalContent) {
+		t.Fatalf("decrypted data mismatch: got %q, want %q", data, originalContent)
+	}
+}
+
+func TestReadFile_EncFilePreferredOverPlain(t *testing.T) {
+	svc, tmpDir := setupTestStorage(t, true)
+
+	encSvc, _ := NewEncryptionService(validTestKey())
+
+	plainContent := []byte("plain version")
+	encContent := []byte("encrypted version - the correct one")
+	encrypted, _ := encSvc.Encrypt(encContent)
+
+	dir := filepath.Join(tmpDir, "images")
+	os.WriteFile(filepath.Join(dir, "photo.jpg"), plainContent, 0644)
+	os.WriteFile(filepath.Join(dir, "photo.jpg.enc"), encrypted, 0644)
+
+	// Should prefer the .enc file
+	data, _, err := svc.ReadFile("/uploads/images/photo.jpg")
+	if err != nil {
+		t.Fatalf("ReadFile failed: %v", err)
+	}
+
+	if !bytes.Equal(data, encContent) {
+		t.Fatalf("expected encrypted version content, got %q", data)
+	}
+}
+
+func TestReadFile_EmptyURL(t *testing.T) {
+	svc, _ := setupTestStorage(t, false)
+
+	_, _, err := svc.ReadFile("")
+	if err == nil {
+		t.Fatal("expected error for empty URL")
+	}
+}
+
+func TestReadFile_MissingFile(t *testing.T) {
+	svc, _ := setupTestStorage(t, false)
+
+	_, _, err := svc.ReadFile("/uploads/images/nonexistent.jpg")
+	if err == nil {
+		t.Fatal("expected error for missing file")
+	}
+}
+
+func TestDelete_HandlesEncAndPlain(t *testing.T) {
+	svc, tmpDir := setupTestStorage(t, false)
+
+	dir := filepath.Join(tmpDir, "images")
+
+	// Create both plain and .enc files
+	os.WriteFile(filepath.Join(dir, "photo.jpg"), []byte("plain"), 0644)
+	os.WriteFile(filepath.Join(dir, "photo.jpg.enc"), []byte("encrypted"), 0644)
+
+	err := svc.Delete("/uploads/images/photo.jpg")
+	if err != nil {
+		t.Fatalf("Delete failed: %v", err)
+	}
+
+	// Both should be gone
+	if _, err := os.Stat(filepath.Join(dir, "photo.jpg")); !os.IsNotExist(err) {
+		t.Fatal("plain file should be deleted")
+	}
+	if _, err := os.Stat(filepath.Join(dir, "photo.jpg.enc")); !os.IsNotExist(err) {
+		t.Fatal("encrypted file should be deleted")
+	}
+}
+
+func TestDelete_NonexistentFile(t *testing.T) {
+	svc, _ := setupTestStorage(t, false)
+
+	// Should not error for non-existent files
+	err := svc.Delete("/uploads/images/nope.jpg")
+	if err != nil {
+		t.Fatalf("Delete should not error for non-existent file: %v", err)
+	}
+}
--- a/internal/services/task_service.go
+++ b/internal/services/task_service.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -848,39 +847,33 @@ func (s *TaskService) sendTaskCompletedNotification(task *models.Task, completio
 	}
 }

-// loadCompletionImagesForEmail reads completion images from disk and prepares them for email embedding
+// loadCompletionImagesForEmail reads completion images from disk and prepares them for email embedding.
+// Uses StorageService.ReadFile to transparently handle encrypted files.
 func (s *TaskService) loadCompletionImagesForEmail(images []models.TaskCompletionImage) []EmbeddedImage {
 	var emailImages []EmbeddedImage

-	uploadDir := s.storageService.GetUploadDir()
-
 	for i, img := range images {
-		// Resolve file path from stored URL
-		filePath := s.resolveImageFilePath(img.ImageURL, uploadDir)
-		if filePath == "" {
-			log.Warn().Str("image_url", img.ImageURL).Msg("Could not resolve image file path")
-			continue
-		}
-
-		// Read file from disk
-		data, err := os.ReadFile(filePath)
+		// Read file via storage service (handles encryption transparently)
+		data, mimeType, err := s.storageService.ReadFile(img.ImageURL)
 		if err != nil {
-			log.Warn().Err(err).Str("path", filePath).Msg("Failed to read completion image for email")
+			log.Warn().Err(err).Str("image_url", img.ImageURL).Msg("Failed to read completion image for email")
 			continue
 		}

-		// Determine content type from extension
-		contentType := s.getContentTypeFromPath(filePath)
+		// Use detected MIME type, fall back to extension-based detection
+		if mimeType == "application/octet-stream" {
+			mimeType = s.getContentTypeFromPath(img.ImageURL)
+		}

 		// Create embedded image with unique Content-ID
 		emailImages = append(emailImages, EmbeddedImage{
 			ContentID:   fmt.Sprintf("completion-image-%d", i+1),
-			Filename:    filepath.Base(filePath),
-			ContentType: contentType,
+			Filename:    filepath.Base(img.ImageURL),
+			ContentType: mimeType,
 			Data:        data,
 		})

-		log.Debug().Str("path", filePath).Int("size", len(data)).Msg("Loaded completion image for email")
+		log.Debug().Str("image_url", img.ImageURL).Int("size", len(data)).Msg("Loaded completion image for email")
 	}

 	return emailImages