Fix 8 high-severity audit findings across security, Go, API, and container domains

- Use typed context key for auth context to prevent collisions (auth.go)
- Eliminate nested locking in CheckRateLimit to prevent potential deadlock (auth.go)
- Add 1MB request body size limit middleware to prevent DoS (router.go)
- Return proper gRPC status codes instead of nil errors on failures (handler.go)
- Use key name instead of raw API key in admin URL paths to prevent secret leakage (keys.go, router.go, keystore_db.go, keystore_hybrid.go)
- Enforce RBAC authorization in service Send/SendBatch for both REST and gRPC (service.go)
- Pin runtime Docker image to alpine:3.21 for reproducible builds (Dockerfile)
- Enable readOnlyRootFilesystem with /tmp emptyDir in k8s deployment (deployment.yaml)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Dockerfile

@@ -35,7 +35,7 @@ RUN CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo \
     -o server ./cmd/server
 
 # Runtime stage
-FROM alpine:latest
+FROM alpine:3.21
 
 # Install runtime dependencies
 RUN apk --no-cache add ca-certificates tzdata

api/grpc/handler.go

@@ -8,6 +8,8 @@ import (
 	pb "github.com/igodwin/notifier/api/grpc/pb"
 	"github.com/igodwin/notifier/internal/domain"
 	"github.com/igodwin/notifier/internal/logging"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
@@ -82,12 +84,7 @@ func (h *NotifierHandler) SendNotification(ctx context.Context, req *pb.SendNoti
 	if err != nil {
 		h.logger.Errorf("gRPC: Failed to send notification - type=%s, account=%s, error=%v",
 			req.Type, req.Account, err)
-		return &pb.SendNotificationResponse{
+		return nil, status.Errorf(codes.Internal, "failed to send notification: %v", err)
-			Result: &pb.NotificationResult{
-				Success: false,
-				Error:   err.Error(),
-			},
-		}, nil
 	}
 
 	// Log success
@@ -139,7 +136,7 @@ func (h *NotifierHandler) SendBatchNotifications(ctx context.Context, req *pb.Se
 func (h *NotifierHandler) GetNotification(ctx context.Context, req *pb.GetNotificationRequest) (*pb.GetNotificationResponse, error) {
 	notification, err := h.service.GetNotification(ctx, req.Id)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.NotFound, "notification not found: %v", err)
 	}
 
 	return &pb.GetNotificationResponse{
@@ -154,7 +151,7 @@ func (h *NotifierHandler) ListNotifications(ctx context.Context, req *pb.ListNot
 
 	notifications, err := h.service.ListNotifications(ctx, filter)
 	if err != nil {
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "failed to list notifications: %v", err)
 	}
 
 	protoNotifications := make([]*pb.Notification, len(notifications))
@@ -230,7 +227,7 @@ func (h *NotifierHandler) GetNotifiers(ctx context.Context, req *pb.GetNotifiers
 	notifiers, err := h.service.GetNotifiers(ctx)
 	if err != nil {
 		h.logger.Errorf("gRPC: Failed to get notifiers - error=%v", err)
-		return nil, err
+		return nil, status.Errorf(codes.Internal, "failed to get notifiers: %v", err)
 	}
 
 	// Convert domain notifiers to proto notifiers

api/rest/keys.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/gorilla/mux"
 	"github.com/igodwin/notifier/internal/auth"
 	"github.com/igodwin/notifier/internal/logging"
 )
@@ -76,7 +77,7 @@ func (h *KeyManagementHandler) CreateKey(w http.ResponseWriter, r *http.Request)
 	ctx := r.Context()
 
 	// Check authorization - must have admin role
-	authCtx, ok := ctx.Value("auth").(*auth.AuthContext)
+	authCtx, ok := auth.GetAuthContext(ctx)
 	if !ok || !h.hasRole(authCtx, "admin") {
 		h.respondError(w, http.StatusForbidden, "Insufficient permissions", "admin role required")
 		return
@@ -143,7 +144,7 @@ func (h *KeyManagementHandler) CreateKey(w http.ResponseWriter, r *http.Request)
 func (h *KeyManagementHandler) ListKeys(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	authCtx, ok := ctx.Value("auth").(*auth.AuthContext)
+	authCtx, ok := auth.GetAuthContext(ctx)
 	if !ok {
 		h.respondError(w, http.StatusUnauthorized, "Unauthorized", "")
 		return
@@ -193,24 +194,25 @@ type RevokeKeyRequest struct {
 }
 
 // RevokeKey deactivates an API key
-// DELETE /api/v1/admin/keys/:key
+// DELETE /api/v1/admin/keys/:name
 // Requires: admin role
 func (h *KeyManagementHandler) RevokeKey(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	authCtx, ok := ctx.Value("auth").(*auth.AuthContext)
+	authCtx, ok := auth.GetAuthContext(ctx)
 	if !ok || !h.hasRole(authCtx, "admin") {
 		h.respondError(w, http.StatusForbidden, "Insufficient permissions", "admin role required")
 		return
 	}
 
-	// Extract key from path parameter
+	// Extract key name from path parameter (not the raw key, to avoid leaking secrets in URLs)
-	keyStr := strings.TrimPrefix(r.URL.Path, "/api/v1/admin/keys/")
+	vars := mux.Vars(r)
+	keyName := vars["name"]
 
 	var req RevokeKeyRequest
 	_ = json.NewDecoder(r.Body).Decode(&req) // Ignore decode errors, reason is optional
 
-	err := h.keyStore.DeactivateKey(ctx, keyStr, authCtx.ClientID)
+	err := h.keyStore.DeactivateKeyByName(ctx, keyName, authCtx.ClientID)
 	if err != nil {
 		if strings.Contains(err.Error(), "not found") {
 			h.respondError(w, http.StatusNotFound, "Key not found", "")
@@ -231,19 +233,19 @@ type RotateKeyRequest struct {
 }
 
 // RotateKey creates a new API key to replace the old one
-// POST /api/v1/admin/keys/:key/rotate
+// POST /api/v1/admin/keys/:name/rotate
 // Requires: admin role
 func (h *KeyManagementHandler) RotateKey(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	authCtx, ok := ctx.Value("auth").(*auth.AuthContext)
+	authCtx, ok := auth.GetAuthContext(ctx)
 	if !ok || !h.hasRole(authCtx, "admin") {
 		h.respondError(w, http.StatusForbidden, "Insufficient permissions", "admin role required")
 		return
 	}
 
-	oldKeyStr := strings.TrimPrefix(r.URL.Path, "/api/v1/admin/keys/")
+	vars := mux.Vars(r)
-	oldKeyStr = strings.TrimSuffix(oldKeyStr, "/rotate")
+	_ = vars["name"] // Key name from URL (rotation not yet implemented)
 
 	var req RotateKeyRequest
 	_ = json.NewDecoder(r.Body).Decode(&req)
@@ -265,19 +267,19 @@ type GetAuditLogResponse struct {
 }
 
 // GetAuditLog retrieves the audit log for a key
-// GET /api/v1/admin/keys/:key/audit
+// GET /api/v1/admin/keys/:name/audit
 // Requires: admin role
 func (h *KeyManagementHandler) GetAuditLog(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	authCtx, ok := ctx.Value("auth").(*auth.AuthContext)
+	authCtx, ok := auth.GetAuthContext(ctx)
 	if !ok || !h.hasRole(authCtx, "admin") {
 		h.respondError(w, http.StatusForbidden, "Insufficient permissions", "admin role required")
 		return
 	}
 
-	keyStr := strings.TrimPrefix(r.URL.Path, "/api/v1/admin/keys/")
+	vars := mux.Vars(r)
-	keyStr = strings.TrimSuffix(keyStr, "/audit")
+	keyName := vars["name"]
 
 	limit := 100
 	if limitStr := r.URL.Query().Get("limit"); limitStr != "" {
@@ -286,7 +288,7 @@ func (h *KeyManagementHandler) GetAuditLog(w http.ResponseWriter, r *http.Reques
 		}
 	}
 
-	logs, err := h.keyStore.GetAuditLog(ctx, keyStr, limit)
+	logs, err := h.keyStore.GetAuditLogByName(ctx, keyName, limit)
 	if err != nil {
 		h.logger.Errorf("Failed to get audit log: %v", err)
 		h.respondError(w, http.StatusInternalServerError, "Failed to get audit log", err.Error())
@@ -294,7 +296,7 @@ func (h *KeyManagementHandler) GetAuditLog(w http.ResponseWriter, r *http.Reques
 	}
 
 	resp := GetAuditLogResponse{
-		Key:      "nk_" + keyStr[len(keyStr)-4:],
+		Key:      keyName,
 		AuditLog: logs,
 	}
 

api/rest/router.go

@@ -86,20 +86,33 @@ func NewRouterWithAuthAndKeyStore(service domain.NotificationService, logger *lo
 		keyHandler := NewKeyManagementHandler(keyStore, logger)
 		v1.HandleFunc("/admin/keys", keyHandler.CreateKey).Methods(http.MethodPost)
 		v1.HandleFunc("/admin/keys", keyHandler.ListKeys).Methods(http.MethodGet)
-		v1.HandleFunc("/admin/keys/{key}", keyHandler.RevokeKey).Methods(http.MethodDelete)
+		v1.HandleFunc("/admin/keys/{name}", keyHandler.RevokeKey).Methods(http.MethodDelete)
-		v1.HandleFunc("/admin/keys/{key}/rotate", keyHandler.RotateKey).Methods(http.MethodPost)
+		v1.HandleFunc("/admin/keys/{name}/rotate", keyHandler.RotateKey).Methods(http.MethodPost)
-		v1.HandleFunc("/admin/keys/{key}/audit", keyHandler.GetAuditLog).Methods(http.MethodGet)
+		v1.HandleFunc("/admin/keys/{name}/audit", keyHandler.GetAuditLog).Methods(http.MethodGet)
 	}
 
 	// Health check route (no auth required)
 	router.HandleFunc("/health", handler.HealthCheck).Methods(http.MethodGet)
 
-	// Middleware - logging and CORS
+	// Middleware - logging, request size limit, and CORS
 	router.Use(loggingMiddleware)
+	v1.Use(maxBodySizeMiddleware(1 << 20)) // 1 MB limit on API request bodies
 
 	return router
 }
 
+// maxBodySizeMiddleware limits the size of incoming request bodies to prevent DoS.
+func maxBodySizeMiddleware(maxBytes int64) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.Body != nil {
+				r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
+			}
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
 // loggingMiddleware logs incoming requests
 func loggingMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

internal/auth/auth.go

@@ -115,24 +115,29 @@ func (s *APIKeyStore) ValidateKey(keyStr string) (*APIKey, error) {
 
 // CheckRateLimit checks if a key has exceeded its rate limit
 func (s *APIKeyStore) CheckRateLimit(keyStr string) (bool, error) {
-	s.mu.Lock()
+	// Look up key and limiter under the store lock, then release it
-	defer s.mu.Unlock()
+	// before acquiring the per-key limiter lock to avoid nested locking.
-
+	s.mu.RLock()
 	key, exists := s.keys[keyStr]
 	if !exists {
+		s.mu.RUnlock()
 		return false, fmt.Errorf("invalid API key")
 	}
 
 	// Unlimited rate limit
 	if key.RateLimit <= 0 {
+		s.mu.RUnlock()
 		return true, nil
 	}
 
 	limiter, exists := s.rateLimits[keyStr]
 	if !exists {
+		s.mu.RUnlock()
 		return false, fmt.Errorf("rate limiter not found")
 	}
+	s.mu.RUnlock()
 
+	// Now lock only the per-key rate limiter
 	limiter.mu.Lock()
 	defer limiter.mu.Unlock()
 
@@ -206,13 +211,16 @@ func (s *APIKeyStore) ListKeys(clientID string) []*APIKey {
 	return keys
 }
 
+// authContextKey is an unexported type for context keys to avoid collisions.
+type authContextKey struct{}
+
 // ContextWithAuth adds auth context to a request context
 func ContextWithAuth(ctx context.Context, auth *AuthContext) context.Context {
-	return context.WithValue(ctx, "auth", auth)
+	return context.WithValue(ctx, authContextKey{}, auth)
 }
 
 // GetAuthContext retrieves auth context from a request context
 func GetAuthContext(ctx context.Context) (*AuthContext, bool) {
-	auth, ok := ctx.Value("auth").(*AuthContext)
+	auth, ok := ctx.Value(authContextKey{}).(*AuthContext)
 	return auth, ok
 }

internal/auth/keystore_db.go

@@ -347,6 +347,89 @@ func (ks *KeyStoreDB) GetAuditLog(ctx context.Context, keyStr string, limit int)
 	return logs, rows.Err()
 }
 
+// GetKeyByName retrieves an API key by its name
+func (ks *KeyStoreDB) GetKeyByName(ctx context.Context, name string) (*APIKey, error) {
+	query := `
+	SELECT key, name, client_id, roles, created_at, last_used_at, expires_at, is_active, rate_limit
+	FROM api_keys
+	WHERE name = $1
+	`
+
+	var key APIKey
+	var roles []string
+
+	err := ks.db.QueryRowContext(ctx, query, name).Scan(
+		&key.Key,
+		&key.Name,
+		&key.ClientID,
+		pq.Array(&roles),
+		&key.CreatedAt,
+		&key.LastUsedAt,
+		&key.ExpiresAt,
+		&key.IsActive,
+		&key.RateLimit,
+	)
+
+	if err == sql.ErrNoRows {
+		return nil, ErrKeyNotFound
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to get key by name: %w", err)
+	}
+
+	key.Roles = roles
+	return &key, nil
+}
+
+// DeactivateKeyByName disables an API key by its name
+func (ks *KeyStoreDB) DeactivateKeyByName(ctx context.Context, name string, deactivatedBy string) error {
+	// First get the key to find its raw key for cache invalidation and audit
+	key, err := ks.GetKeyByName(ctx, name)
+	if err != nil {
+		return err
+	}
+
+	return ks.DeactivateKey(ctx, key.Key, deactivatedBy)
+}
+
+// GetAuditLogByName retrieves audit log entries for a key identified by name
+func (ks *KeyStoreDB) GetAuditLogByName(ctx context.Context, name string, limit int) ([]map[string]interface{}, error) {
+	query := `
+	SELECT al.action, al.performed_by, al.performed_at, al.details
+	FROM api_key_audit_log al
+	JOIN api_keys ak ON al.key_id = ak.id
+	WHERE ak.name = $1
+	ORDER BY al.performed_at DESC
+	LIMIT $2
+	`
+
+	rows, err := ks.db.QueryContext(ctx, query, name, limit)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get audit log: %w", err)
+	}
+	defer rows.Close()
+
+	var logs []map[string]interface{}
+	for rows.Next() {
+		var action, performedBy, details string
+		var performedAt time.Time
+
+		err := rows.Scan(&action, &performedBy, &performedAt, &details)
+		if err != nil {
+			return nil, err
+		}
+
+		logs = append(logs, map[string]interface{}{
+			"action":       action,
+			"performed_by": performedBy,
+			"performed_at": performedAt,
+			"details":      details,
+		})
+	}
+
+	return logs, rows.Err()
+}
+
 // Custom errors
 var (
 	ErrKeyNotFound = fmt.Errorf("API key not found")

internal/auth/keystore_hybrid.go

@@ -136,6 +136,32 @@ func (h *HybridKeyStore) GetAuditLog(ctx context.Context, keyStr string, limit i
 	return h.db.GetAuditLog(ctx, keyStr, limit)
 }
 
+// DeactivateKeyByName deactivates a key by its name (avoids exposing raw key in URLs)
+func (h *HybridKeyStore) DeactivateKeyByName(ctx context.Context, name string, deactivatedBy string) error {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+
+	// Look up the key by name in DB to get the raw key for cache invalidation
+	key, err := h.db.GetKeyByName(ctx, name)
+	if err != nil {
+		return err
+	}
+
+	// Remove from cache
+	h.cache.mu.Lock()
+	delete(h.cache.keys, key.Key)
+	delete(h.cache.rateLimits, key.Key)
+	h.cache.mu.Unlock()
+
+	// Deactivate in database
+	return h.db.DeactivateKey(ctx, key.Key, deactivatedBy)
+}
+
+// GetAuditLogByName retrieves audit log for a key identified by name
+func (h *HybridKeyStore) GetAuditLogByName(ctx context.Context, name string, limit int) ([]map[string]interface{}, error) {
+	return h.db.GetAuditLogByName(ctx, name, limit)
+}
+
 // Close closes the database connection
 func (h *HybridKeyStore) Close() error {
 	return h.db.Close()

internal/service/service.go

@@ -276,6 +276,16 @@ func (s *NotificationService) processNotification(ctx context.Context, msg *doma
 
 // Send queues a notification for delivery
 func (s *NotificationService) Send(ctx context.Context, notification *domain.Notification) (*domain.NotificationResult, error) {
+	// Enforce RBAC authorization if configured
+	if err := s.checkAuthorization(ctx, notification); err != nil {
+		return &domain.NotificationResult{
+			NotificationID: notification.ID,
+			Success:        false,
+			Error:          err.Error(),
+			SentAt:         time.Now(),
+		}, err
+	}
+
 	// Store the notification
 	s.storeNotification(notification)
 
@@ -301,6 +311,13 @@ func (s *NotificationService) Send(ctx context.Context, notification *domain.Not
 func (s *NotificationService) SendBatch(ctx context.Context, notifications []*domain.Notification) ([]*domain.NotificationResult, error) {
 	results := make([]*domain.NotificationResult, 0, len(notifications))
 
+	// Enforce RBAC authorization for each notification
+	for _, notification := range notifications {
+		if err := s.checkAuthorization(ctx, notification); err != nil {
+			return nil, fmt.Errorf("authorization denied for notification type=%s account=%s: %w", notification.Type, notification.Account, err)
+		}
+	}
+
 	// Store all notifications
 	for _, notification := range notifications {
 		s.storeNotification(notification)
@@ -439,12 +456,7 @@ func (s *NotificationService) GetStats(ctx context.Context) (*domain.Notificatio
 // GetNotifiers returns information about available notifiers, filtered by authorization if auth context is provided
 func (s *NotificationService) GetNotifiers(ctx context.Context) (*domain.NotifiersResponse, error) {
 	// Extract auth context from request context if available
-	var authCtx *auth.AuthContext
+	authCtx, _ := auth.GetAuthContext(ctx)
-	if authVal := ctx.Value("auth"); authVal != nil {
-		if ac, ok := authVal.(*auth.AuthContext); ok {
-			authCtx = ac
-		}
-	}
 
 	supportedTypes := s.factory.SupportedTypes()
 	notifiers := make([]domain.NotifierInfo, 0, len(supportedTypes))
@@ -510,6 +522,30 @@ func (s *NotificationService) updateNotification(notification *domain.Notificati
 	s.notifications[notification.ID] = notification
 }
 
+// checkAuthorization verifies that the caller is authorized to send to the given notifier/account.
+// Returns nil if authorized or if RBAC is not configured.
+func (s *NotificationService) checkAuthorization(ctx context.Context, notification *domain.Notification) error {
+	if s.authz == nil || !s.authz.HasRules() {
+		return nil // RBAC not configured
+	}
+
+	authCtx, ok := auth.GetAuthContext(ctx)
+	if !ok {
+		return nil // No auth context (auth may be disabled)
+	}
+
+	account := notification.Account
+	if account == "" && s.accountResolver != nil {
+		account = s.accountResolver.GetDefaultAccount(notification.Type)
+	}
+
+	if !s.authz.IsAuthorized(authCtx, notification.Type, account) {
+		return fmt.Errorf("not authorized to send %s notifications to account %s", notification.Type, account)
+	}
+
+	return nil
+}
+
 // matchesFilter checks if a notification matches the filter
 func (s *NotificationService) matchesFilter(notification *domain.Notification, filter *domain.NotificationFilter) bool {
 	if filter == nil {

k8s/deployment.yaml

@@ -50,6 +50,8 @@ spec:
           readOnly: true
         - name: queue-storage
           mountPath: /var/lib/notifier
+        - name: tmp
+          mountPath: /tmp
         resources:
           requests:
             cpu: 100m
@@ -77,7 +79,7 @@ spec:
           runAsNonRoot: true
           runAsUser: 1000
           allowPrivilegeEscalation: false
-          readOnlyRootFilesystem: false
+          readOnlyRootFilesystem: true
           capabilities:
             drop:
             - ALL
@@ -87,6 +89,8 @@ spec:
           name: notifier-config
       - name: queue-storage
         emptyDir: {}
+      - name: tmp
+        emptyDir: {}
       restartPolicy: Always
 ---
 apiVersion: v1