Merge pull request #45 from Valentin-Kaiser/email

Improved email security

Author: valentin-kaiser

apperror/apperror_test.go

@@ -351,7 +351,7 @@ func TestCatchFunction(t *testing.T) {
 	}, "operation failed")
 }
 
-func TestCatchFunctionSuccess(t *testing.T) {
+func TestCatchFunctionSuccess(_ *testing.T) {
 	// Test successful operation (should not panic)
 	apperror.Catch(func() error {
 		return nil

cache/memory.go

@@ -95,6 +95,7 @@ func (mc *MemoryCache) Get(_ context.Context, key string, dest interface{}) (boo
 
 	memItem, ok := element.Value.(*memoryItem)
 	if !ok {
+		mc.mutex.Unlock()
 		mc.updateStats(func(s *Stats) { s.Misses++ })
 		mc.emitEvent(EventGet, key, nil, nil)
 		return false, NewCacheError("get", key, errors.New("invalid cache item type"))
@@ -162,6 +163,7 @@ func (mc *MemoryCache) Set(_ context.Context, key string, value interface{}, ttl
 		TTL:       effectiveTTL,
 		Size:      dataSize,
 		Namespace: mc.config.Namespace,
+		ExpiresAt: time.Time{}, // Default to no expiration
 	}
 
 	if effectiveTTL > 0 {
@@ -177,7 +179,8 @@ func (mc *MemoryCache) Set(_ context.Context, key string, value interface{}, ttl
 	defer mc.mutex.Unlock()
 
 	// Check if key already exists
-	if element, exists := mc.items[formattedKey]; exists {
+	element, exists := mc.items[formattedKey]
+	if exists {
 		// Update existing item
 		oldMemItem, ok := element.Value.(*memoryItem)
 		if !ok {
@@ -192,20 +195,24 @@ func (mc *MemoryCache) Set(_ context.Context, key string, value interface{}, ttl
 		mc.updateStats(func(s *Stats) {
 			s.Memory = s.Memory - oldMemItem.dataSize + dataSize
 		})
-	} else {
-		// Add new item
-		element := mc.lruList.PushFront(memItem)
-		mc.items[formattedKey] = element
 
-		mc.updateStats(func(s *Stats) {
-			s.Size++
-			s.Memory += dataSize
-		})
+		mc.updateStats(func(s *Stats) { s.Sets++ })
+		mc.emitEvent(EventSet, key, value, nil)
+		return nil
+	}
 
-		// Check if we need to evict items
-		if mc.config.MaxSize > 0 && mc.stats.Size > mc.config.MaxSize {
-			mc.evictLRU()
-		}
+	// Add new item
+	element = mc.lruList.PushFront(memItem)
+	mc.items[formattedKey] = element
+
+	mc.updateStats(func(s *Stats) {
+		s.Size++
+		s.Memory += dataSize
+	})
+
+	// Check if we need to evict items
+	if mc.config.MaxSize > 0 && mc.stats.Size > mc.config.MaxSize {
+		mc.evictLRU()
 	}
 
 	mc.updateStats(func(s *Stats) { s.Sets++ })
@@ -250,17 +257,17 @@ func (mc *MemoryCache) Exists(_ context.Context, key string) (bool, error) {
 	item := memItem.item
 
 	// Check if item has expired
-	if item.IsExpired() {
+	if !item.IsExpired() {
 		mc.mutex.RUnlock()
-		// Remove expired item
-		mc.mutex.Lock()
-		mc.removeElement(element, formattedKey)
-		mc.mutex.Unlock()
-		return false, nil
+		return true, nil
 	}
 
 	mc.mutex.RUnlock()
-	return true, nil
+	// Remove expired item
+	mc.mutex.Lock()
+	mc.removeElement(element, formattedKey)
+	mc.mutex.Unlock()
+	return false, nil
 }
 
 // Clear removes all entries from the cache
@@ -364,13 +371,11 @@ func (mc *MemoryCache) SetTTL(_ context.Context, key string, ttl time.Duration)
 		return NewCacheError("setttl", key, errors.New("invalid item type"))
 	}
 	item := memItem.item
-
+	item.ExpiresAt = time.Time{}
+	item.TTL = 0
 	if ttl > 0 {
 		item.ExpiresAt = time.Now().Add(ttl)
 		item.TTL = ttl
-	} else {
-		item.ExpiresAt = time.Time{}
-		item.TTL = 0
 	}
 
 	item.UpdatedAt = time.Now()
@@ -430,17 +435,20 @@ func (mc *MemoryCache) evictLRU() {
 	}
 
 	element := mc.lruList.Back()
-	if element != nil {
-		memItem, ok := element.Value.(*memoryItem)
-		if !ok {
-			return // Skip if invalid type
-		}
-		key := memItem.item.Key
-		mc.removeElement(element, key)
+	if element == nil {
+		return
+	}
 
-		mc.updateStats(func(s *Stats) { s.Evictions++ })
-		mc.emitEvent(EventEvict, key, nil, nil)
+	memItem, ok := element.Value.(*memoryItem)
+	if !ok {
+		return // Skip if invalid type
 	}
+
+	key := memItem.item.Key
+	mc.removeElement(element, key)
+
+	mc.updateStats(func(s *Stats) { s.Evictions++ })
+	mc.emitEvent(EventEvict, key, nil, nil)
 }
 
 // startCleanup starts the background cleanup goroutine
@@ -478,16 +486,21 @@ func (mc *MemoryCache) cleanupExpired() {
 		}
 		item := memItem.item
 
-		if !item.ExpiresAt.IsZero() && now.After(item.ExpiresAt) {
-			expiredKeys = append(expiredKeys, key)
+		if item.ExpiresAt.IsZero() || !now.After(item.ExpiresAt) {
+			continue
 		}
+
+		expiredKeys = append(expiredKeys, key)
 	}
 
 	// Remove expired items
 	for _, key := range expiredKeys {
-		if element, exists := mc.items[key]; exists {
-			mc.removeElement(element, key)
-			mc.emitEvent(EventExpire, key, nil, nil)
+		element, exists := mc.items[key]
+		if !exists {
+			continue
 		}
+
+		mc.removeElement(element, key)
+		mc.emitEvent(EventExpire, key, nil, nil)
 	}
 }

cache/redis.go

@@ -249,8 +249,9 @@ func (rc *RedisCache) GetMulti(ctx context.Context, keys []string) (map[string]i
 				}
 				result[keys[i]] = dest
 				rc.updateStats(func(s *Stats) { s.Hits++ })
+				continue
 			}
-		} else {
+
 			rc.updateStats(func(s *Stats) { s.Misses++ })
 		}
 	}
@@ -334,7 +335,8 @@ func (rc *RedisCache) SetTTL(ctx context.Context, key string, ttl time.Duration)
 	var err error
 	if ttl > 0 {
 		err = rc.client.Expire(ctx, formattedKey, ttl).Err()
-	} else {
+	}
+	if ttl <= 0 {
 		err = rc.client.Persist(ctx, formattedKey).Err()
 	}
 

cache/tiered.go

@@ -184,7 +184,8 @@ func (tc *TieredCache) GetMulti(ctx context.Context, keys []string) (map[string]
 	if err != nil {
 		tc.recordError(err)
 		// Continue to L2 even if L1 fails
-	} else {
+	}
+	if err == nil {
 		for key, value := range l1Results {
 			result[key] = value
 		}

database/config.go

@@ -47,6 +47,7 @@ func (c *Config) Validate() error {
 	return nil
 }
 
+// Changed checks if the configuration has changed
 func (c *Config) Changed(n *Config) bool {
 	if c.Driver != n.Driver {
 		return true

database/database_test.go

@@ -157,7 +157,10 @@ func TestExecuteWithoutConnection(t *testing.T) {
 	// Ensure we're in a disconnected state for this test
 	// Only disconnect if currently connected to avoid hanging
 	if database.Connected() {
-		database.Disconnect()
+		err := database.Disconnect()
+		if err != nil {
+			t.Errorf("Disconnect should not return an error: %v", err)
+		}
 		time.Sleep(100 * time.Millisecond) // Wait for disconnection
 	}
 
@@ -184,7 +187,10 @@ func TestReconnect(t *testing.T) {
 func TestAwaitConnectionTimeout(t *testing.T) {
 	// Ensure we start in a disconnected state for this test
 	if database.Connected() {
-		database.Disconnect()
+		err := database.Disconnect()
+		if err != nil {
+			t.Errorf("Disconnect should not return an error: %v", err)
+		}
 		time.Sleep(200 * time.Millisecond) // Wait for disconnection
 	}
 
@@ -230,7 +236,10 @@ func TestConnectWithInvalidConfig(t *testing.T) {
 	}
 
 	// Clean up
-	database.Disconnect()
+	err := database.Disconnect()
+	if err != nil {
+		t.Errorf("Disconnect should not return an error: %v", err)
+	}
 }
 
 func TestConnectWithSQLiteConfig(t *testing.T) {

go.mod

@@ -10,13 +10,13 @@ require (
 	github.com/fsnotify/fsnotify v1.9.0
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/websocket v1.5.3
-	github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible
 	github.com/rabbitmq/amqp091-go v1.10.0
 	github.com/redis/go-redis/v9 v9.12.1
 	github.com/rs/zerolog v1.34.0
 	github.com/spf13/pflag v1.0.7
 	github.com/spf13/viper v1.20.1
 	github.com/stoewer/go-strcase v1.3.1
+	github.com/stretchr/testify v1.10.0
 	golang.org/x/sync v0.16.0
 	golang.org/x/text v0.28.0
 	golang.org/x/time v0.12.0
@@ -31,6 +31,7 @@ require (
 	filippo.io/edwards25519 v1.1.0 // indirect
 	github.com/ProtonMail/go-crypto v1.3.0 // indirect
 	github.com/cloudflare/circl v1.6.1 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
 	github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 // indirect
 	github.com/go-sql-driver/mysql v1.8.1 // indirect
@@ -41,6 +42,7 @@ require (
 	github.com/mattn/go-isatty v0.0.19 // indirect
 	github.com/mattn/go-sqlite3 v1.14.22 // indirect
 	github.com/pelletier/go-toml/v2 v2.2.3 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/sagikazarmark/locafero v0.7.0 // indirect
 	github.com/sourcegraph/conc v0.3.0 // indirect
 	github.com/spf13/afero v1.12.0 // indirect

go.sum

@@ -43,8 +43,6 @@ github.com/jinzhu/inflection v1.0.0 h1:K317FqzuhWc8YvSVlFMCCUb36O/S9MCKRDI7QkRKD
 github.com/jinzhu/inflection v1.0.0/go.mod h1:h+uFLlag+Qp1Va5pdKtLDYj+kHp5pxUVkryuEj+Srlc=
 github.com/jinzhu/now v1.1.5 h1:/o9tlHleP7gOFmsnYNz3RGnqzefHA47wQpKrrdTIwXQ=
 github.com/jinzhu/now v1.1.5/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8=
-github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible h1:jdpOPRN1zP63Td1hDQbZW73xKmzDvZHzVdNYxhnTMDA=
-github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible/go.mod h1:1c7szIrayyPPB/987hsnvNzLushdWf4o/79s3P08L8A=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=

mail/config.go

@@ -84,6 +84,34 @@ type ServerConfig struct {
 	AllowInsecureAuth bool `yaml:"allow_insecure_auth" json:"allow_insecure_auth"`
 	// MaxConcurrentHandlers limits the number of concurrent notification handlers
 	MaxConcurrentHandlers int `yaml:"max_concurrent_handlers" json:"max_concurrent_handlers"`
+	// Security holds the security configuration for the SMTP server
+	Security SecurityConfig `yaml:"security" json:"security"`
+}
+
+// SecurityConfig holds security-related configuration for the smtp server
+type SecurityConfig struct {
+	// HeloValidation enables HELO/EHLO hostname validation
+	HeloValidation bool `yaml:"helo_validation" json:"helo_validation"`
+	// HeloRequireFQDN requires HELO hostname to be a fully qualified domain name
+	HeloRequireFQDN bool `yaml:"helo_require_fqdn" json:"helo_require_fqdn"`
+	// HeloDNSCheck enables DNS resolution check for HELO hostname
+	HeloDNSCheck bool `yaml:"helo_dns_check" json:"helo_dns_check"`
+	// IPAllowlist contains allowed IP addresses/CIDR blocks
+	IPAllowlist []string `yaml:"ip_allowlist" json:"ip_allowlist"`
+	// IPBlocklist contains blocked IP addresses/CIDR blocks
+	IPBlocklist []string `yaml:"ip_blocklist" json:"ip_blocklist"`
+	// MaxConnectionsPerIP limits connections per IP address
+	MaxConnectionsPerIP int `yaml:"max_connections_per_ip" json:"max_connections_per_ip"`
+	// RateLimitPerIP limits commands per IP per minute
+	RateLimitPerIP int `yaml:"rate_limit_per_ip" json:"rate_limit_per_ip"`
+	// AuthFailureDelay adds delay after authentication failures
+	AuthFailureDelay time.Duration `yaml:"auth_failure_delay" json:"auth_failure_delay"`
+	// MaxAuthFailures limits auth attempts before blocking IP
+	MaxAuthFailures int `yaml:"max_auth_failures" json:"max_auth_failures"`
+	// AuthFailureWindow is the time window to track auth failures
+	AuthFailureWindow time.Duration `yaml:"auth_failure_window" json:"auth_failure_window"`
+	// LogSecurityEvents enables detailed security logging
+	LogSecurityEvents bool `yaml:"log_security_events" json:"log_security_events"`
 }
 
 // QueueConfig holds the queue configuration for mail processing
@@ -143,6 +171,19 @@ func DefaultConfig() *Config {
 			MaxRecipients:         100,
 			AllowInsecureAuth:     false,
 			MaxConcurrentHandlers: 50, // Limit concurrent notification handlers
+			Security: SecurityConfig{
+				HeloValidation:      false,
+				HeloRequireFQDN:     false,
+				HeloDNSCheck:        false,
+				IPAllowlist:         []string{},
+				IPBlocklist:         []string{},
+				MaxConnectionsPerIP: 10,
+				RateLimitPerIP:      60, // 60 commands per minute
+				AuthFailureDelay:    time.Second,
+				MaxAuthFailures:     5,
+				AuthFailureWindow:   15 * time.Minute,
+				LogSecurityEvents:   true,
+			},
 		},
 		Queue: QueueConfig{
 			Enabled:     true,
@@ -168,6 +209,7 @@ func (c *ClientConfig) TLSConfig() *tls.Config {
 	}
 }
 
+// Validate checks the client configuration for errors
 func (c *ClientConfig) Validate() error {
 	if c.Host == "" {
 		return apperror.NewError("SMTP host is required")
@@ -199,6 +241,7 @@ func (c *ServerConfig) TLSConfig() *tls.Config {
 	}
 }
 
+// Validate checks the server configuration for errors
 func (c *ServerConfig) Validate() error {
 	if c.Host == "" {
 		return apperror.NewError("SMTP server host is required")
@@ -215,6 +258,7 @@ func (c *ServerConfig) Validate() error {
 	return nil
 }
 
+// Validate checks the queue configuration for errors
 func (c *QueueConfig) Validate() error {
 	if c.QueueName == "" {
 		return apperror.NewError("Queue name is required")
@@ -228,13 +272,15 @@ func (c *QueueConfig) Validate() error {
 	return nil
 }
 
+// Validate checks the template configuration for errors
 func (c *TemplateConfig) Validate() error {
 	if c.DefaultTemplate == "" {
 		return apperror.NewError("Default template is required")
 	}
 	return nil
 }
 
+// Validate checks the configuration for errors
 func (c *Config) Validate() error {
 	if err := c.Client.Validate(); err != nil {
 		return apperror.Wrap(err)