ci: advance floating minor tag (vX.Y) on real releases

Publishes an additional floating vX.Y tag alongside the immutable vX.Y.Z
so `docker pull ...:vX.Y` fetches the newest patch. Skipped on --rebuild
of an older version so the float never rolls backward. Deploys still pin
vX.Y.Z; the cleanup reaper only targets three-part semver, so the float
is never eligible for deletion.

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

.gitea/workflows/build-and-publish.yml

@@ -0,0 +1,180 @@
+name: Build and Publish Container
+
+# Builds and publishes a multi-arch image on every push to main, minting the
+# next patch version from git tags (vX.Y.Z) and pushing the tag back. Real
+# releases also advance a floating minor tag (vX.Y -> newest patch) as a
+# pull convenience; deploys still pin the immutable vX.Y.Z.
+#
+# NOTE (public repo): unlike private app repos, this workflow deliberately has
+# NO step that pushes to the deployment (GitOps) repository and holds no
+# credentials for it. Deployment repos are expected to poll the registry and
+# pull new tags themselves.
+
+on:
+  push:
+    branches:
+      - main
+  workflow_dispatch:
+    inputs:
+      ref:
+        description: 'Git tag (e.g. v0.1.5) or commit SHA to rebuild. Leave empty to build latest.'
+        required: false
+        default: ''
+
+env:
+  REGISTRY: gitea.ivangodwin.com
+  IMAGE_NAME: ${{ gitea.repository }}
+
+jobs:
+  build-and-publish:
+    runs-on: docker
+
+    permissions:
+      contents: write
+      packages: write
+
+    steps:
+      - name: Checkout code
+        run: |
+          if [ -n "${{ inputs.ref }}" ]; then
+            git clone https://${{ gitea.actor }}:${{ gitea.token }}@gitea.ivangodwin.com/${{ gitea.repository }}.git .
+            git fetch --tags
+            git checkout "${{ inputs.ref }}"
+          else
+            git clone --depth 1 https://${{ gitea.actor }}:${{ gitea.token }}@gitea.ivangodwin.com/${{ gitea.repository }}.git .
+            git checkout ${{ gitea.sha }}
+          fi
+
+      - name: Determine next version
+        run: |
+          REF="${{ inputs.ref }}"
+          if [ -n "$REF" ] && echo "$REF" | grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+$'; then
+            VERSION="$REF"
+            echo "REBUILD=true" >> $GITHUB_ENV
+          else
+            git fetch --tags
+            LATEST=$(git tag --sort=-v:refname | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | head -1 || true)
+            if [ -z "$LATEST" ]; then
+              VERSION="v0.1.0"
+            else
+              MAJOR=$(echo "$LATEST" | cut -d. -f1 | tr -d 'v')
+              MINOR=$(echo "$LATEST" | cut -d. -f2)
+              PATCH=$(echo "$LATEST" | cut -d. -f3)
+              VERSION="v${MAJOR}.${MINOR}.$((PATCH + 1))"
+            fi
+            echo "REBUILD=false" >> $GITHUB_ENV
+          fi
+          echo "VERSION=${VERSION}" >> $GITHUB_ENV
+
+      - name: Log in to Gitea Container Registry
+        run: |
+          echo "${{ secrets.CI_TOKEN }}" | docker login -u "${{ secrets.CI_USER }}" --password-stdin ${{ env.REGISTRY }}
+
+      - name: Register QEMU emulators
+        run: |
+          docker run --rm --privileged tonistiigi/binfmt:latest --install all
+
+      - name: Set up Docker Buildx
+        run: |
+          docker buildx inspect multiarch >/dev/null 2>&1 \
+            || docker buildx create --name multiarch --driver docker-container
+          docker buildx use multiarch
+          docker buildx inspect --bootstrap
+
+      - name: Build and push multi-arch image
+        run: |
+          TAGS="-t ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ env.VERSION }}"
+          # On real releases, also advance the floating minor tag (vX.Y) to this
+          # build so `docker pull ...:vX.Y` fetches the newest patch. Skipped on
+          # a --rebuild of an older version, which must not clobber the float.
+          if [ "${{ env.REBUILD }}" != "true" ]; then
+            MINOR_TAG=$(echo "${{ env.VERSION }}" | grep -oE '^v[0-9]+\.[0-9]+')
+            TAGS="${TAGS} -t ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${MINOR_TAG}"
+          fi
+          docker buildx build \
+            --platform linux/amd64,linux/arm64 \
+            --build-arg VERSION="${{ env.VERSION }}" \
+            --build-arg GIT_COMMIT="$(git rev-parse --short HEAD)" \
+            --build-arg BUILD_TIME="$(date -u '+%Y-%m-%d_%H:%M:%S_UTC')" \
+            --no-cache \
+            --provenance=false \
+            ${TAGS} \
+            --push \
+            .
+
+      - name: Tag release
+        if: env.REBUILD != 'true'
+        run: |
+          git tag "${{ env.VERSION }}"
+          git push https://${{ gitea.actor }}:${{ gitea.token }}@gitea.ivangodwin.com/${{ gitea.repository }}.git "${{ env.VERSION }}"
+
+      - name: Clean up old container images
+        continue-on-error: true
+        timeout-minutes: 5
+        env:
+          CI_TOKEN: ${{ secrets.CI_TOKEN }}
+        run: |
+          # Best-effort cleanup of old container image versions; must never
+          # fail or stall the pipeline, so the whole body runs under a hard
+          # timeout and the step always exits 0 itself.
+          cat > /tmp/cleanup.sh <<'CLEAN'
+          #!/bin/sh
+          set -u
+
+          if ! apk add -q --no-cache curl; then
+            echo "curl unavailable; skipping cleanup."
+            exit 0
+          fi
+
+          API="https://gitea.ivangodwin.com/api/v1"
+          OWNER="igodwin"
+          NAME="notifier"
+          KEEP=5
+
+          BODY=$(curl -s --connect-timeout 15 --max-time 30 \
+            -H "Authorization: token ${CI_TOKEN}" \
+            "${API}/packages/${OWNER}?type=container&q=${NAME}&limit=200" || true)
+
+          if [ -z "$BODY" ] || [ "$(printf '%s' "$BODY" | tr -d ' \t\r\n')" = "[]" ]; then
+            echo "No packages found."
+            exit 0
+          fi
+
+          # Only three-part semver (vX.Y.Z) is eligible for deletion; the
+          # floating minor tag (vX.Y) never matches, so it is never reaped.
+          VERSIONS=$(printf '%s' "$BODY" \
+            | grep -oE '"version":"v[0-9]+\.[0-9]+\.[0-9]+"' \
+            | grep -oE 'v[0-9]+\.[0-9]+\.[0-9]+' \
+            | sort -u)
+          MINORS=$(printf '%s' "$VERSIONS" | grep -oE '^v[0-9]+\.[0-9]+' | sort -u)
+
+          for MINOR in $MINORS; do
+            PATCHES=$(printf '%s' "$VERSIONS" | grep "^${MINOR}\." | sort -t. -k3,3n)
+            TOTAL=$(printf '%s\n' "$PATCHES" | grep -c .)
+
+            if [ "$TOTAL" -le "$KEEP" ]; then
+              echo "${MINOR}: ${TOTAL} versions, nothing to delete"
+              continue
+            fi
+
+            TO_DELETE=$((TOTAL - KEEP))
+            echo "${MINOR}: ${TOTAL} versions, keeping ${KEEP}, deleting ${TO_DELETE}"
+
+            printf '%s\n' "$PATCHES" | head -n "$TO_DELETE" | while read -r VER; do
+              STATUS=$(curl -s --connect-timeout 15 --max-time 60 -o /dev/null -w "%{http_code}" \
+                -X DELETE -H "Authorization: token ${CI_TOKEN}" \
+                "${API}/packages/${OWNER}/container/${NAME}/${VER}" || echo 000)
+              case "$STATUS" in
+                200|202|204) echo "  deleted ${NAME}:${VER} (HTTP ${STATUS})" ;;
+                *)           echo "  WARN: could not delete ${NAME}:${VER} (HTTP ${STATUS})" ;;
+              esac
+            done
+          done
+          CLEAN
+
+          timeout 240 sh /tmp/cleanup.sh
+          rc=$?
+          if [ "$rc" -ne 0 ]; then
+            echo "cleanup did not finish cleanly (rc=${rc}); ignoring and exiting green."
+          fi
+          exit 0

.gitea/workflows/bump-version.yml

@@ -0,0 +1,47 @@
+name: Bump Version
+
+on:
+  workflow_dispatch:
+    inputs:
+      component:
+        description: 'Version component to bump'
+        required: true
+        type: choice
+        options:
+          - minor
+          - major
+
+jobs:
+  bump:
+    runs-on: docker
+
+    permissions:
+      contents: write
+
+    steps:
+      - name: Checkout code
+        run: |
+          git clone --depth 1 https://${{ gitea.actor }}:${{ gitea.token }}@gitea.ivangodwin.com/${{ gitea.repository }}.git .
+
+      - name: Compute and push new version tag
+        run: |
+          git fetch --tags
+          LATEST=$(git tag --sort=-v:refname | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | head -1 || true)
+          if [ -z "$LATEST" ]; then
+            if [ "${{ inputs.component }}" = "major" ]; then
+              VERSION="v1.0.0"
+            else
+              VERSION="v0.1.0"
+            fi
+          else
+            MAJOR=$(echo "$LATEST" | cut -d. -f1 | tr -d 'v')
+            MINOR=$(echo "$LATEST" | cut -d. -f2)
+            if [ "${{ inputs.component }}" = "major" ]; then
+              VERSION="v$((MAJOR + 1)).0.0"
+            else
+              VERSION="v${MAJOR}.$((MINOR + 1)).0"
+            fi
+          fi
+          echo "Bumping to ${VERSION}"
+          git tag "$VERSION"
+          git push https://${{ gitea.actor }}:${{ gitea.token }}@gitea.ivangodwin.com/${{ gitea.repository }}.git "$VERSION"

.gitea/workflows/ci.yml

@@ -0,0 +1,112 @@
+name: CI
+
+# Runner-native workflow: the self-hosted act_runner's job containers have no
+# Node.js, so JavaScript actions (actions/checkout, actions/setup-go, ...)
+# fail with "Cannot find: node in PATH". Every step here is a plain shell
+# run-step inside a golang container instead.
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+# Cancel superseded runs for the same ref to save runner capacity.
+concurrency:
+  group: ci-${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  lint:
+    name: Lint
+    runs-on: docker
+    container:
+      image: golang:1.25-alpine
+    steps:
+      - name: Checkout
+        run: |
+          apk add -q --no-cache git make protobuf protobuf-dev curl
+          git init -q .
+          git remote add origin https://gitea.ivangodwin.com/${{ gitea.repository }}.git
+          git fetch -q --depth 1 origin ${{ gitea.sha }}
+          git checkout -q FETCH_HEAD
+
+      # api/grpc/pb/ is gitignored and generated at build time, so anything
+      # that compiles this module - including the linter, which type-checks
+      # packages - needs the generated code in place first.
+      - name: Generate protobuf code
+        run: |
+          go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.10
+          go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.5.1
+          export PATH="$PATH:$(go env GOPATH)/bin"
+          make proto-gen
+
+      # Installed via `go install` (module proxy + sumdb verification): the
+      # official install.sh tarball download hit checksum mismatches on this
+      # runner.
+      - name: Run golangci-lint
+        run: |
+          go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
+          "$(go env GOPATH)/bin/golangci-lint" run ./...
+
+  test:
+    name: Test
+    runs-on: docker
+    container:
+      image: golang:1.25-alpine
+    steps:
+      - name: Checkout
+        run: |
+          apk add -q --no-cache git make protobuf protobuf-dev gcc musl-dev
+          git init -q .
+          git remote add origin https://gitea.ivangodwin.com/${{ gitea.repository }}.git
+          git fetch -q --depth 1 origin ${{ gitea.sha }}
+          git checkout -q FETCH_HEAD
+
+      - name: Generate protobuf code
+        run: |
+          go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.10
+          go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.5.1
+          export PATH="$PATH:$(go env GOPATH)/bin"
+          make proto-gen
+
+      # -race needs cgo, hence gcc/musl-dev above. tests/e2e uses
+      # testcontainers-go (needs a Docker daemon) and is excluded; run it
+      # locally with: go test -race ./tests/e2e/...
+      # coverage.out is left in the workspace; artifact upload is omitted
+      # until the instance's artifact storage is confirmed working.
+      - name: Run tests (excluding e2e)
+        run: |
+          go test -race -covermode=atomic -coverprofile=coverage.out \
+            $(go list ./... | grep -v '/tests/e2e')
+
+  vuln:
+    name: Vulnerability scan
+    runs-on: docker
+    container:
+      image: golang:1.25-alpine
+    # Advisory: govulncheck also reports Go-stdlib findings that are only
+    # fixable by toolchain updates; flip to blocking once triaged.
+    continue-on-error: true
+    steps:
+      - name: Checkout
+        run: |
+          apk add -q --no-cache git make protobuf protobuf-dev
+          git init -q .
+          git remote add origin https://gitea.ivangodwin.com/${{ gitea.repository }}.git
+          git fetch -q --depth 1 origin ${{ gitea.sha }}
+          git checkout -q FETCH_HEAD
+
+      # govulncheck also loads and type-checks the module's packages, so the
+      # generated protobuf code has to exist first.
+      - name: Generate protobuf code
+        run: |
+          go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.10
+          go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.5.1
+          export PATH="$PATH:$(go env GOPATH)/bin"
+          make proto-gen
+
+      - name: Run govulncheck
+        run: |
+          go install golang.org/x/vuln/cmd/govulncheck@latest
+          "$(go env GOPATH)/bin/govulncheck" ./...

.golangci.yml

@@ -0,0 +1,46 @@
+# golangci-lint configuration.
+#
+# Schema: golangci-lint v2 (this machine had no golangci-lint installed at
+# authoring time, so this targets the latest stable v2 config schema -
+# https://golangci-lint.run/usage/configuration/ - as of v2.12.x). If your
+# CI/local installs a v1 binary, upgrade it rather than downgrading this file;
+# v1 binaries do not understand `version: "2"` configs.
+version: "2"
+
+run:
+  timeout: 5m
+
+linters:
+  # Start from nothing and opt in explicitly, rather than `standard`/`all`,
+  # so the enabled set below is the complete, intentional list.
+  default: none
+
+  enable:
+    - govet      # suspicious constructs (vet)
+    - staticcheck # bugs, deprecated APIs, simplifications (includes old `gosimple`/`stylecheck` checks)
+    - errcheck   # unchecked error return values
+    - ineffassign # assignments that are never used
+    - unused     # unused constants, variables, functions, types
+    - misspell   # common English misspellings in comments/strings
+    - gosec      # security-focused static analysis
+    - revive     # style/lint rules (golint replacement)
+
+  settings:
+    gosec:
+      # G104 (unchecked errors) is already covered by errcheck above and is
+      # noisy/redundant when both linters are enabled together.
+      excludes:
+        - G104
+
+  exclusions:
+    # Generated protobuf code should never be linted or hand-edited. This is
+    # a plain path match against whatever is on disk at lint time, so it
+    # excludes api/grpc/pb/ whether or not the directory happens to exist -
+    # it's gitignored and regenerated by `make proto-gen` before CI lints
+    # (see .gitea/actions/generate-proto), not committed to the repo.
+    paths:
+      - api/grpc/pb/
+
+    # Keep the generated-code detector on too, in case other generated files
+    # show up elsewhere in the tree in the future.
+    generated: strict

Makefile

@@ -1,4 +1,4 @@
-.PHONY: proto proto-gen proto-clean deps build build-dev run run-grpc run-rest test lint fmt vet check docker-build docker-build-dev docker-buildx-setup docker-run clean help
+.PHONY: proto proto-gen proto-clean deps build build-dev run run-grpc run-rest test lint vuln fmt vet check docker-build docker-build-dev docker-buildx-setup docker-run clean help
 
 # Variables
 REGISTRY ?=
@@ -137,8 +137,15 @@ lint:
 	golangci-lint run ./...
 	@echo "Linting passed"
 
+# Run vulnerability scan (requires govulncheck)
+vuln:
+	@echo "Running vulnerability scan..."
+	@which govulncheck > /dev/null || (echo "govulncheck not installed. Run: go install golang.org/x/vuln/cmd/govulncheck@latest" && exit 1)
+	govulncheck ./...
+	@echo "Vulnerability scan passed"
+
 # Run all quality checks
-qa: fmt vet lint test
+qa: fmt vet lint vuln test
 	@echo "All quality checks passed!"
 
 # Build Docker image (production - optimized)

api/grpc/handler.go

@@ -1,8 +1,12 @@
+// Package grpc implements the gRPC transport for the notifier service,
+// translating between the generated protobuf types and internal domain
+// types.
 package grpc
 
 import (
 	"context"
 	"fmt"
+	"math"
 
 	"github.com/google/uuid"
 	pb "github.com/igodwin/notifier/api/grpc/pb"
@@ -29,7 +33,7 @@ func NewNotifierHandler(svc domain.NotificationService, logger *logging.Logger)
 }
 
 // HealthCheck verifies the service is operational
-func (h *NotifierHandler) HealthCheck(ctx context.Context, req *pb.HealthCheckRequest) (*pb.HealthCheckResponse, error) {
+func (h *NotifierHandler) HealthCheck(_ context.Context, _ *pb.HealthCheckRequest) (*pb.HealthCheckResponse, error) {
 	// TODO: Implement proper health check logic
 	return &pb.HealthCheckResponse{
 		Healthy: true,
@@ -56,7 +60,7 @@ func (h *NotifierHandler) SendNotification(ctx context.Context, req *pb.SendNoti
 	}
 
 	// Convert content type, defaulting to text
-	contentType := convertProtoContentTypeToDomain(req.ContentType)
+	contentType := convertProtoContentTypeToDomain(req.ContentType) //nolint:staticcheck // deprecated content_type field still honored for backward compatibility
 
 	// Build notification
 	notification := &domain.Notification{
@@ -205,7 +209,7 @@ func (h *NotifierHandler) RetryNotification(ctx context.Context, req *pb.RetryNo
 }
 
 // GetStats returns notification statistics
-func (h *NotifierHandler) GetStats(ctx context.Context, req *pb.GetStatsRequest) (*pb.GetStatsResponse, error) {
+func (h *NotifierHandler) GetStats(ctx context.Context, _ *pb.GetStatsRequest) (*pb.GetStatsResponse, error) {
 	stats, err := h.service.GetStats(ctx)
 	if err != nil {
 		return nil, err
@@ -222,7 +226,7 @@ func (h *NotifierHandler) GetStats(ctx context.Context, req *pb.GetStatsRequest)
 }
 
 // GetNotifiers returns information about available notifiers
-func (h *NotifierHandler) GetNotifiers(ctx context.Context, req *pb.GetNotifiersRequest) (*pb.GetNotifiersResponse, error) {
+func (h *NotifierHandler) GetNotifiers(ctx context.Context, _ *pb.GetNotifiersRequest) (*pb.GetNotifiersResponse, error) {
 	h.logger.Infof("gRPC: Received request for available notifiers")
 
 	notifiers, err := h.service.GetNotifiers(ctx)
@@ -248,6 +252,18 @@ func (h *NotifierHandler) GetNotifiers(ctx context.Context, req *pb.GetNotifiers
 
 // Helper functions to convert between proto and domain types
 
+// clampInt32 narrows an int to int32, saturating at the int32 bounds
+// instead of silently wrapping when the domain value is out of range.
+func clampInt32(v int) int32 {
+	if v > math.MaxInt32 {
+		return math.MaxInt32
+	}
+	if v < math.MinInt32 {
+		return math.MinInt32
+	}
+	return int32(v)
+}
+
 // convertStringMapToInterface converts proto's map[string]string to domain's map[string]interface{}
 func convertStringMapToInterface(m map[string]string) map[string]interface{} {
 	if m == nil {
@@ -315,17 +331,6 @@ func convertProtoContentTypeToDomain(protoType pb.ContentType) domain.ContentTyp
 	}
 }
 
-func convertDomainContentTypeToProto(domainType domain.ContentType) pb.ContentType {
-	switch domainType {
-	case domain.ContentTypeHTML:
-		return pb.ContentType_CONTENT_TYPE_HTML
-	case domain.ContentTypeText:
-		return pb.ContentType_CONTENT_TYPE_TEXT
-	default:
-		return pb.ContentType_CONTENT_TYPE_TEXT
-	}
-}
-
 func convertDomainToProtoType(domainType domain.NotificationType) pb.NotificationType {
 	switch domainType {
 	case domain.TypeEmail:
@@ -365,7 +370,7 @@ func convertDomainToProtoNotification(notif *domain.Notification) *pb.Notificati
 		Id:         notif.ID,
 		Type:       convertDomainToProtoType(notif.Type),
 		Account:    notif.Account,
-		Priority:   pb.Priority(notif.Priority),
+		Priority:   pb.Priority(clampInt32(int(notif.Priority))),
 		Status:     convertDomainToProtoStatus(notif.Status),
 		Subject:    notif.Subject,
 		Body:       notif.Body,
@@ -373,8 +378,8 @@ func convertDomainToProtoNotification(notif *domain.Notification) *pb.Notificati
 		Recipients: notif.Recipients,
 		Metadata:   convertInterfaceMapToString(notif.Metadata),
 		CreatedAt:  timestamppb.New(notif.CreatedAt),
-		RetryCount: int32(notif.RetryCount),
-		MaxRetries: int32(notif.MaxRetries),
+		RetryCount: clampInt32(notif.RetryCount),
+		MaxRetries: clampInt32(notif.MaxRetries),
 		LastError:  notif.LastError,
 	}
 

api/rest/cors_test.go

@@ -17,9 +17,9 @@ func TestCORSMiddleware_AllowedOrigin(t *testing.T) {
 	}
 
 	middleware := newCORSMiddleware(config)
-	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte("OK"))
+		_, _ = w.Write([]byte("OK"))
 	}))
 
 	tests := []struct {
@@ -94,9 +94,9 @@ func TestCORSMiddleware_BlockedOrigin(t *testing.T) {
 	}
 
 	middleware := newCORSMiddleware(config)
-	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte("OK"))
+		_, _ = w.Write([]byte("OK"))
 	}))
 
 	tests := []struct {
@@ -158,7 +158,7 @@ func TestCORSMiddleware_PreflightRequest(t *testing.T) {
 	}
 
 	middleware := newCORSMiddleware(config)
-	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := middleware(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
 		t.Error("Handler should not be called for OPTIONS request")
 	}))
 
@@ -171,13 +171,13 @@ func TestCORSMiddleware_PreflightRequest(t *testing.T) {
 		{
 			name:          "preflight from allowed origin",
 			origin:        "https://example.com",
-			expectStatus:  http.StatusOK,
+			expectStatus:  http.StatusNoContent,
 			expectHeaders: true,
 		},
 		{
 			name:          "preflight from blocked origin",
 			origin:        "https://malicious.com",
-			expectStatus:  http.StatusOK,
+			expectStatus:  http.StatusForbidden,
 			expectHeaders: false,
 		},
 	}
@@ -191,7 +191,8 @@ func TestCORSMiddleware_PreflightRequest(t *testing.T) {
 			rec := httptest.NewRecorder()
 			handler.ServeHTTP(rec, req)
 
-			// Preflight should always return 200 OK
+			// Allowed preflights succeed with 204; blocked ones get 403
+			// with no CORS headers so the browser rejects the request.
 			if rec.Code != tt.expectStatus {
 				t.Errorf("status = %v, want %v", rec.Code, tt.expectStatus)
 			}
@@ -216,7 +217,7 @@ func TestCORSMiddleware_Credentials(t *testing.T) {
 	}
 
 	middleware := newCORSMiddleware(config)
-	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
@@ -242,7 +243,7 @@ func TestCORSMiddleware_NoWildcard(t *testing.T) {
 	}
 
 	middleware := newCORSMiddleware(config)
-	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
@@ -272,7 +273,7 @@ func TestCORSMiddleware_EmptyConfig(t *testing.T) {
 	}
 
 	middleware := newCORSMiddleware(config)
-	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
@@ -354,7 +355,7 @@ func TestCORSMiddleware_MaxAge(t *testing.T) {
 			}
 
 			middleware := newCORSMiddleware(config)
-			handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			handler := middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 				w.WriteHeader(http.StatusOK)
 			}))
 

api/rest/handlers.go

@@ -2,6 +2,7 @@ package rest
 
 import (
 	"encoding/json"
+	"errors"
 	"net/http"
 	"strconv"
 	"time"
@@ -160,7 +161,7 @@ func (h *Handler) CancelNotification(w http.ResponseWriter, r *http.Request) {
 	id := vars["id"]
 
 	if err := h.service.CancelNotification(r.Context(), id); err != nil {
-		respondError(w, http.StatusInternalServerError, "failed to cancel notification", err)
+		respondError(w, statusForServiceError(err), "failed to cancel notification", err)
 		return
 	}
 
@@ -177,7 +178,7 @@ func (h *Handler) RetryNotification(w http.ResponseWriter, r *http.Request) {
 
 	result, err := h.service.RetryNotification(r.Context(), id)
 	if err != nil {
-		respondError(w, http.StatusInternalServerError, "failed to retry notification", err)
+		respondError(w, statusForServiceError(err), "failed to retry notification", err)
 		return
 	}
 
@@ -186,6 +187,19 @@ func (h *Handler) RetryNotification(w http.ResponseWriter, r *http.Request) {
 	})
 }
 
+// statusForServiceError maps service-layer sentinel errors to HTTP status
+// codes; anything unrecognized is an internal error.
+func statusForServiceError(err error) int {
+	switch {
+	case errors.Is(err, domain.ErrNotificationNotFound):
+		return http.StatusNotFound
+	case errors.Is(err, domain.ErrNotificationAlreadySent):
+		return http.StatusConflict
+	default:
+		return http.StatusInternalServerError
+	}
+}
+
 // GetStats handles GET /api/v1/stats
 func (h *Handler) GetStats(w http.ResponseWriter, r *http.Request) {
 	stats, err := h.service.GetStats(r.Context())
@@ -212,7 +226,7 @@ func (h *Handler) GetNotifiers(w http.ResponseWriter, r *http.Request) {
 }
 
 // HealthCheck handles GET /health
-func (h *Handler) HealthCheck(w http.ResponseWriter, r *http.Request) {
+func (h *Handler) HealthCheck(w http.ResponseWriter, _ *http.Request) {
 	respondJSON(w, http.StatusOK, map[string]interface{}{
 		"status":  "healthy",
 		"service": "notifier",
@@ -270,10 +284,12 @@ func respondJSON(w http.ResponseWriter, status int, data interface{}) {
 	}
 }
 
-// respondError sends an error response
+// respondError sends an error response. Client errors (4xx) include the
+// underlying detail to help callers fix their request; server errors (5xx)
+// deliberately do not echo internals — those belong in the server log.
 func respondError(w http.ResponseWriter, status int, message string, err error) {
 	errMsg := message
-	if err != nil {
+	if err != nil && status < http.StatusInternalServerError {
 		errMsg = message + ": " + err.Error()
 	}
 

api/rest/keys.go

@@ -2,10 +2,10 @@ package rest
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/gorilla/mux"
@@ -120,7 +120,7 @@ func (h *KeyManagementHandler) CreateKey(w http.ResponseWriter, r *http.Request)
 	apiKey, err := h.keyStore.CreateKey(ctx, req.ClientID, req.Roles, req.RateLimit, expiresInDuration, authCtx.ClientID)
 	if err != nil {
 		h.logger.Errorf("Failed to create API key: %v", err)
-		h.respondError(w, http.StatusInternalServerError, "Failed to create API key", err.Error())
+		h.respondError(w, http.StatusInternalServerError, "Failed to create API key", "")
 		return
 	}
 
@@ -165,7 +165,7 @@ func (h *KeyManagementHandler) ListKeys(w http.ResponseWriter, r *http.Request)
 	keys, err := h.keyStore.ListKeys(ctx, clientID)
 	if err != nil {
 		h.logger.Errorf("Failed to list API keys: %v", err)
-		h.respondError(w, http.StatusInternalServerError, "Failed to list API keys", err.Error())
+		h.respondError(w, http.StatusInternalServerError, "Failed to list API keys", "")
 		return
 	}
 
@@ -173,7 +173,7 @@ func (h *KeyManagementHandler) ListKeys(w http.ResponseWriter, r *http.Request)
 	keyInfos := make([]*KeyInfo, len(keys))
 	for i, key := range keys {
 		keyInfos[i] = &KeyInfo{
-			Key:        "nk_" + key.Key[len(key.Key)-4:], // Show only last 4 chars
+			Key:        key.KeyPreview, // non-sensitive preview, e.g. "nk_…ab12"
 			Name:       key.Name,
 			ClientID:   key.ClientID,
 			Roles:      key.Roles,
@@ -214,11 +214,11 @@ func (h *KeyManagementHandler) RevokeKey(w http.ResponseWriter, r *http.Request)
 
 	err := h.keyStore.DeactivateKeyByName(ctx, keyName, authCtx.ClientID)
 	if err != nil {
-		if strings.Contains(err.Error(), "not found") {
+		if errors.Is(err, auth.ErrKeyNotFound) {
 			h.respondError(w, http.StatusNotFound, "Key not found", "")
 		} else {
 			h.logger.Errorf("Failed to revoke API key: %v", err)
-			h.respondError(w, http.StatusInternalServerError, "Failed to revoke API key", err.Error())
+			h.respondError(w, http.StatusInternalServerError, "Failed to revoke API key", "")
 		}
 		return
 	}
@@ -291,7 +291,7 @@ func (h *KeyManagementHandler) GetAuditLog(w http.ResponseWriter, r *http.Reques
 	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())
+		h.respondError(w, http.StatusInternalServerError, "Failed to get audit log", "")
 		return
 	}
 
@@ -306,7 +306,7 @@ func (h *KeyManagementHandler) GetAuditLog(w http.ResponseWriter, r *http.Reques
 // Helper methods
 
 // hasRole checks if the auth context has a specific role
-func (h *KeyManagementHandler) hasRole(authCtx *auth.AuthContext, role string) bool {
+func (h *KeyManagementHandler) hasRole(authCtx *auth.Context, role string) bool {
 	for _, r := range authCtx.Roles {
 		if r == role {
 			return true
@@ -319,16 +319,24 @@ func (h *KeyManagementHandler) hasRole(authCtx *auth.AuthContext, role string) b
 func (h *KeyManagementHandler) respondJSON(w http.ResponseWriter, statusCode int, data interface{}) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(statusCode)
-	json.NewEncoder(w).Encode(data)
+	// Headers are already written at this point, so there's nothing left to
+	// do but log an encode failure.
+	if err := json.NewEncoder(w).Encode(data); err != nil {
+		h.logger.Errorf("Failed to encode JSON response: %v", err)
+	}
 }
 
 // respondError writes an error JSON response
-func (h *KeyManagementHandler) respondError(w http.ResponseWriter, statusCode int, error string, message string) {
+func (h *KeyManagementHandler) respondError(w http.ResponseWriter, statusCode int, errMsg string, message string) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(statusCode)
 	resp := ErrorResponse{
-		Error:   error,
+		Error:   errMsg,
 		Message: message,
 	}
-	json.NewEncoder(w).Encode(resp)
+	// Headers are already written at this point, so there's nothing left to
+	// do but log an encode failure.
+	if err := json.NewEncoder(w).Encode(resp); err != nil {
+		h.logger.Errorf("Failed to encode JSON error response: %v", err)
+	}
 }

api/rest/router.go

@@ -1,9 +1,15 @@
+// Package rest implements the HTTP/JSON transport for the notifier
+// service: request routing, handlers, authentication and CORS middleware,
+// and API key management endpoints.
 package rest
 
 import (
+	"context"
+	"encoding/json"
 	"net/http"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/gorilla/mux"
 	"github.com/igodwin/notifier/internal/auth"
@@ -43,27 +49,50 @@ func DefaultCORSConfig() *CORSConfig {
 	}
 }
 
-// NewRouter creates a new HTTP router with all routes configured
-func NewRouter(service domain.NotificationService, logger *logging.Logger) *mux.Router {
-	return NewRouterWithAuth(service, logger, nil)
+// ReadinessCheck reports whether a named dependency is ready. Implementations
+// should be cheap; they run on every /readyz request.
+type ReadinessCheck func(ctx context.Context) error
+
+// RouterOptions configures the REST router.
+type RouterOptions struct {
+	Service   domain.NotificationService
+	Logger    *logging.Logger
+	AuthStore *auth.APIKeyStore   // nil disables authentication
+	KeyStore  *auth.HybridKeyStore // nil disables key-management routes
+	CORS      *CORSConfig          // nil disables CORS headers entirely
+	// Readiness maps a component name (e.g. "queue", "database") to its check.
+	Readiness map[string]ReadinessCheck
+	// Instrument, when set, wraps the router for request instrumentation
+	// (e.g. Prometheus HTTP metrics).
+	Instrument func(http.Handler) http.Handler
 }
 
-// NewRouterWithAuth creates a new HTTP router with optional authentication and CORS configuration
+// NewRouter creates a new HTTP router with all routes configured
+func NewRouter(service domain.NotificationService, logger *logging.Logger) *mux.Router {
+	return NewRouterWithOptions(RouterOptions{Service: service, Logger: logger})
+}
+
+// NewRouterWithAuth creates a new HTTP router with optional authentication
 func NewRouterWithAuth(service domain.NotificationService, logger *logging.Logger, authStore *auth.APIKeyStore) *mux.Router {
-	return NewRouterWithAuthAndKeyStore(service, logger, authStore, nil)
+	return NewRouterWithOptions(RouterOptions{Service: service, Logger: logger, AuthStore: authStore})
 }
 
 // NewRouterWithAuthAndKeyStore creates a new HTTP router with authentication and key management
 func NewRouterWithAuthAndKeyStore(service domain.NotificationService, logger *logging.Logger, authStore *auth.APIKeyStore, keyStore *auth.HybridKeyStore) *mux.Router {
-	handler := NewHandler(service, logger)
+	return NewRouterWithOptions(RouterOptions{Service: service, Logger: logger, AuthStore: authStore, KeyStore: keyStore})
+}
+
+// NewRouterWithOptions creates the HTTP router from RouterOptions.
+func NewRouterWithOptions(opts RouterOptions) *mux.Router {
+	handler := NewHandler(opts.Service, opts.Logger)
 	router := mux.NewRouter()
 
 	// API v1 routes
 	v1 := router.PathPrefix("/api/v1").Subrouter()
 
 	// Apply authentication middleware if auth store is provided
-	if authStore != nil {
-		authMiddleware := auth.NewRESTAuthMiddleware(authStore, logger)
+	if opts.AuthStore != nil {
+		authMiddleware := auth.NewRESTAuthMiddleware(opts.AuthStore, opts.Logger)
 		v1.Use(authMiddleware.Middleware)
 	}
 
@@ -82,8 +111,8 @@ func NewRouterWithAuthAndKeyStore(service domain.NotificationService, logger *lo
 	v1.HandleFunc("/notifiers", handler.GetNotifiers).Methods(http.MethodGet)
 
 	// Key management routes (requires auth and keystore)
-	if authStore != nil && keyStore != nil {
-		keyHandler := NewKeyManagementHandler(keyStore, logger)
+	if opts.AuthStore != nil && opts.KeyStore != nil {
+		keyHandler := NewKeyManagementHandler(opts.KeyStore, opts.Logger)
 		v1.HandleFunc("/admin/keys", keyHandler.CreateKey).Methods(http.MethodPost)
 		v1.HandleFunc("/admin/keys", keyHandler.ListKeys).Methods(http.MethodGet)
 		v1.HandleFunc("/admin/keys/{name}", keyHandler.RevokeKey).Methods(http.MethodDelete)
@@ -91,16 +120,73 @@ func NewRouterWithAuthAndKeyStore(service domain.NotificationService, logger *lo
 		v1.HandleFunc("/admin/keys/{name}/audit", keyHandler.GetAuditLog).Methods(http.MethodGet)
 	}
 
-	// Health check route (no auth required)
+	// Liveness and readiness routes (no auth required). /health stays a pure
+	// liveness signal; /readyz fails when a dependency is unavailable.
 	router.HandleFunc("/health", handler.HealthCheck).Methods(http.MethodGet)
+	router.HandleFunc("/readyz", readinessHandler(opts.Readiness)).Methods(http.MethodGet)
 
-	// Middleware - logging, request size limit, and CORS
+	// Middleware - CORS (when configured), logging, and request size limits
+	if opts.CORS != nil && len(opts.CORS.AllowedOrigins) > 0 {
+		router.Use(newCORSMiddleware(opts.CORS))
+	}
+	if opts.Instrument != nil {
+		router.Use(mux.MiddlewareFunc(opts.Instrument))
+	}
 	router.Use(loggingMiddleware)
 	v1.Use(maxBodySizeMiddleware(1 << 20)) // 1 MB limit on API request bodies
 
 	return router
 }
 
+// LivenessHandler returns a minimal liveness handler for dedicated health
+// listeners (the REST router serves the same signal at /health).
+func LivenessHandler() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		// Headers are already written, so an encode error can only be
+		// dropped on the floor here.
+		_ = json.NewEncoder(w).Encode(map[string]interface{}{
+			"status":  "healthy",
+			"service": "notifier",
+			"time":    time.Now().UTC(),
+		})
+	})
+}
+
+// ReadinessHandler exposes the readiness checks for dedicated health listeners.
+func ReadinessHandler(checks map[string]ReadinessCheck) http.Handler {
+	return readinessHandler(checks)
+}
+
+// readinessHandler runs each dependency check and reports 503 if any fail.
+func readinessHandler(checks map[string]ReadinessCheck) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second)
+		defer cancel()
+
+		status := http.StatusOK
+		components := make(map[string]string, len(checks))
+		for name, check := range checks {
+			if err := check(ctx); err != nil {
+				status = http.StatusServiceUnavailable
+				components[name] = "unavailable"
+			} else {
+				components[name] = "ok"
+			}
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(status)
+		ready := status == http.StatusOK
+		// Headers are already written, so an encode error can only be
+		// dropped on the floor here.
+		_ = json.NewEncoder(w).Encode(map[string]interface{}{
+			"ready":      ready,
+			"components": components,
+		})
+	}
+}
+
 // 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 {
@@ -162,10 +248,15 @@ func newCORSMiddleware(config *CORSConfig) func(http.Handler) http.Handler {
 				}
 			}
 
-			// Handle preflight OPTIONS requests
-			if r.Method == http.MethodOptions {
-				// Return 200 OK for preflight requests
-				w.WriteHeader(http.StatusOK)
+			// Handle preflight OPTIONS requests: succeed only for allowed
+			// origins; disallowed cross-origin preflights get 403 with no
+			// CORS headers so browsers block the actual request.
+			if r.Method == http.MethodOptions && origin != "" {
+				if allowed {
+					w.WriteHeader(http.StatusNoContent)
+				} else {
+					w.WriteHeader(http.StatusForbidden)
+				}
 				return
 			}
 

cmd/client/main.go

@@ -1,3 +1,5 @@
+// Command client is a CLI for sending and managing notifications through
+// the notifier service's REST API.
 package main
 
 import (
@@ -107,7 +109,7 @@ Options:
 	account := fs.String("account", "", "")
 	recipients := fs.String("recipients", "", "")
 
-	fs.Parse(args)
+	_ = fs.Parse(args) // flag.ExitOnError means Parse never returns a non-nil error
 
 	if *notifType == "" || *body == "" {
 		fmt.Fprintf(os.Stderr, "Error: --type and --body are required\n")
@@ -118,7 +120,7 @@ Options:
 	ctx, cancel := context.WithTimeout(context.Background(), *timeout)
 	defer cancel()
 
-	cfg := client.ClientConfig{
+	cfg := client.Config{
 		BaseURL:     *baseURL,
 		APIKey:      *apiKey,
 		Timeout:     *timeout,
@@ -174,7 +176,7 @@ Options:
 	timeout := fs.Duration("timeout", 30*time.Second, "")
 	id := fs.String("id", "", "")
 
-	fs.Parse(args)
+	_ = fs.Parse(args) // flag.ExitOnError means Parse never returns a non-nil error
 
 	if *id == "" {
 		fmt.Fprintf(os.Stderr, "Error: --id is required\n")
@@ -185,7 +187,7 @@ Options:
 	ctx, cancel := context.WithTimeout(context.Background(), *timeout)
 	defer cancel()
 
-	cfg := client.ClientConfig{
+	cfg := client.Config{
 		BaseURL:     *baseURL,
 		APIKey:      *apiKey,
 		Timeout:     *timeout,
@@ -231,12 +233,12 @@ Options:
 	limit := fs.Int("limit", 10, "")
 	offset := fs.Int("offset", 0, "")
 
-	fs.Parse(args)
+	_ = fs.Parse(args) // flag.ExitOnError means Parse never returns a non-nil error
 
 	ctx, cancel := context.WithTimeout(context.Background(), *timeout)
 	defer cancel()
 
-	cfg := client.ClientConfig{
+	cfg := client.Config{
 		BaseURL:     *baseURL,
 		APIKey:      *apiKey,
 		Timeout:     *timeout,
@@ -290,12 +292,12 @@ Options:
 	apiKey := fs.String("key", "", "")
 	timeout := fs.Duration("timeout", 30*time.Second, "")
 
-	fs.Parse(args)
+	_ = fs.Parse(args) // flag.ExitOnError means Parse never returns a non-nil error
 
 	ctx, cancel := context.WithTimeout(context.Background(), *timeout)
 	defer cancel()
 
-	cfg := client.ClientConfig{
+	cfg := client.Config{
 		BaseURL:     *baseURL,
 		APIKey:      *apiKey,
 		Timeout:     *timeout,
@@ -333,12 +335,12 @@ Options:
 	apiKey := fs.String("key", "", "")
 	timeout := fs.Duration("timeout", 30*time.Second, "")
 
-	fs.Parse(args)
+	_ = fs.Parse(args) // flag.ExitOnError means Parse never returns a non-nil error
 
 	ctx, cancel := context.WithTimeout(context.Background(), *timeout)
 	defer cancel()
 
-	cfg := client.ClientConfig{
+	cfg := client.Config{
 		BaseURL:     *baseURL,
 		APIKey:      *apiKey,
 		Timeout:     *timeout,
@@ -374,12 +376,12 @@ Options:
 	baseURL := fs.String("url", "http://localhost:8080", "")
 	timeout := fs.Duration("timeout", 30*time.Second, "")
 
-	fs.Parse(args)
+	_ = fs.Parse(args) // flag.ExitOnError means Parse never returns a non-nil error
 
 	ctx, cancel := context.WithTimeout(context.Background(), *timeout)
 	defer cancel()
 
-	cfg := client.ClientConfig{
+	cfg := client.Config{
 		BaseURL:     *baseURL,
 		Timeout:     *timeout,
 		TLSInsecure: false,
@@ -396,8 +398,7 @@ Options:
 	if healthy {
 		fmt.Println("Service is healthy")
 		os.Exit(0)
-	} else {
-		fmt.Println("Service is unhealthy")
-		os.Exit(1)
 	}
+	fmt.Println("Service is unhealthy")
+	os.Exit(1)
 }

cmd/server/main.go

@@ -1,3 +1,5 @@
+// Command server runs the notifier service, exposing its REST and gRPC
+// APIs and wiring up configuration, queueing, auth, and metrics.
 package main
 
 import (
@@ -12,7 +14,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/gorilla/mux"
 	grpcapi "github.com/igodwin/notifier/api/grpc"
 	pb "github.com/igodwin/notifier/api/grpc/pb"
 	"github.com/igodwin/notifier/api/rest"
@@ -20,10 +21,14 @@ import (
 	"github.com/igodwin/notifier/internal/config"
 	"github.com/igodwin/notifier/internal/domain"
 	"github.com/igodwin/notifier/internal/logging"
+	"github.com/igodwin/notifier/internal/metrics"
 	"github.com/igodwin/notifier/internal/notifier"
 	"github.com/igodwin/notifier/internal/queue"
 	"github.com/igodwin/notifier/internal/service"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/health"
+	healthgrpc "google.golang.org/grpc/health/grpc_health_v1"
 	"google.golang.org/grpc/reflection"
 )
 
@@ -54,10 +59,10 @@ func main() {
 	}
 
 	// Create logger from config
-	logger, err := logging.NewFromConfig(cfg.Logging.Level, cfg.Logging.OutputPath)
+	logger, err := logging.NewFromOptions(cfg.Logging.Level, cfg.Logging.Format, cfg.Logging.OutputPath)
 	if err != nil {
 		// Fallback to stdout if log file can't be opened
-		logger, _ = logging.NewFromConfig(cfg.Logging.Level, "stdout")
+		logger, _ = logging.NewFromOptions(cfg.Logging.Level, cfg.Logging.Format, "stdout")
 		logger.Warnf("Failed to open log file, using stdout: %v", err)
 	}
 
@@ -91,15 +96,15 @@ func main() {
 	var authStore *auth.APIKeyStore
 	var hybridKeyStore *auth.HybridKeyStore
 	var authz *auth.NotifierAuthz
+	var dbStore *auth.KeyStoreDB
 	if cfg.Auth.Enabled {
 		authStore = auth.NewAPIKeyStore()
 		authz = auth.NewNotifierAuthz()
 		logger.Info("API authentication enabled")
 
 		// Create database backend if configured
-		var dbStore *auth.KeyStoreDB
 		if cfg.Auth.Database.URL != "" {
-			dbStore, err = auth.NewKeyStoreDB(cfg.Auth.Database.URL)
+			dbStore, err = auth.NewKeyStoreDB(cfg.Auth.Database.URL, logger)
 			if err != nil {
 				logger.Fatalf("Failed to create database key store: %v", err)
 			}
@@ -112,6 +117,16 @@ func main() {
 		hybridKeyStore = auth.NewHybridKeyStore(authStore, dbStore)
 		logger.Debugf("Initialized hybrid key store for API key management")
 
+		// Load persisted keys into the cache so previously issued keys
+		// keep authenticating across restarts.
+		if hybridKeyStore.HasDatabase() {
+			if loaded, err := hybridKeyStore.InitializeFromDatabase(ctx); err != nil {
+				logger.Errorf("Failed to load API keys from database: %v", err)
+			} else {
+				logger.Infof("Loaded %d API key(s) from database into cache", loaded)
+			}
+		}
+
 		// Bootstrap admin key if configured
 		if cfg.Auth.Bootstrap.Enabled {
 			bootstrapCfg := &auth.BootstrapConfig{
@@ -133,12 +148,23 @@ func main() {
 
 			// If we have an existing key, use it
 			if existingKey != "" {
-				if _, err := auth.RegisterAdminKeyInMemory(authStore, existingKey, logger); err != nil {
+				apiKey, err := auth.RegisterAdminKeyInMemory(authStore, existingKey, logger)
+				if err != nil {
 					logger.Warnf("Failed to register existing admin key: %v", err)
+				} else if err := hybridKeyStore.EnsurePersisted(ctx, apiKey, "bootstrap"); err != nil {
+					logger.Warnf("Failed to persist existing admin key: %v", err)
 				}
 			} else {
-				// Generate new key
-				if apiKey, err := auth.BootstrapAdminKeyInMemory(authStore, bootstrapCfg, logger); err != nil {
+				// Generate a new key — through the hybrid store when a
+				// database is configured so the admin key survives restarts.
+				var apiKey *auth.APIKey
+				var err error
+				if hybridKeyStore.HasDatabase() {
+					apiKey, err = auth.BootstrapAdminKey(ctx, hybridKeyStore, bootstrapCfg, logger)
+				} else {
+					apiKey, err = auth.BootstrapAdminKeyInMemory(authStore, bootstrapCfg, logger)
+				}
+				if err != nil {
 					logger.Warnf("Bootstrap admin key creation failed: %v", err)
 				} else if apiKey != nil {
 					// Store in Kubernetes secret if configured
@@ -176,6 +202,7 @@ func main() {
 
 	// Create notification service (pass config as account resolver and authz for RBAC)
 	svc := service.NewNotificationService(factory, q, cfg.Queue.WorkerCount, cfg, authz, logger)
+	svc.WithRetryBackoff(cfg.Queue.RetryBackoff)
 
 	// Configure notification retention if enabled
 	if err := svc.WithRetentionConfig(cfg.Retention); err != nil {
@@ -193,7 +220,24 @@ func main() {
 	}
 	logger.Infof("Started %d worker(s)", cfg.Queue.WorkerCount)
 
-	// Wait group for both servers
+	// Readiness checks shared by the REST /readyz route and the dedicated
+	// health listener: the queue must be open and, when configured, the
+	// auth database reachable.
+	readiness := map[string]rest.ReadinessCheck{
+		"queue": q.HealthCheck,
+	}
+	if dbStore != nil {
+		readiness["database"] = dbStore.Ping
+	}
+
+	// Metrics collector + endpoint
+	var collector *metrics.Collector
+	if cfg.Metrics.Enabled {
+		collector = metrics.NewCollector(svc, q, logger)
+		go collector.Run(ctx, 10*time.Second)
+	}
+
+	// Wait group for all servers
 	var wg sync.WaitGroup
 
 	// Start gRPC server if enabled
@@ -207,7 +251,22 @@ func main() {
 	var restServer *http.Server
 	if cfg.Server.Mode == "both" || cfg.Server.Mode == "rest" {
 		wg.Add(1)
-		restServer = startRESTServer(ctx, &wg, cfg, svc, logger, authStore, hybridKeyStore)
+		restServer = startRESTServer(ctx, &wg, cfg, svc, logger, authStore, hybridKeyStore, readiness, collector)
+	}
+
+	// Start metrics server if enabled
+	var metricsServer *http.Server
+	if collector != nil {
+		wg.Add(1)
+		metricsServer = startMetricsServer(&wg, cfg, collector, logger)
+	}
+
+	// Start dedicated health listener if enabled (needed for probes when
+	// running in grpc-only mode; harmless duplication otherwise)
+	var healthServer *http.Server
+	if cfg.HealthCheck.Enabled {
+		wg.Add(1)
+		healthServer = startHealthServer(&wg, cfg, readiness, logger)
 	}
 
 	// Wait for interrupt signal
@@ -221,10 +280,12 @@ func main() {
 	shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer shutdownCancel()
 
-	// Stop REST server
-	if restServer != nil {
-		if err := restServer.Shutdown(shutdownCtx); err != nil {
-			logger.Errorf("Error during REST server shutdown: %v", err)
+	// Stop HTTP servers
+	for _, server := range []*http.Server{restServer, metricsServer, healthServer} {
+		if server != nil {
+			if err := server.Shutdown(shutdownCtx); err != nil {
+				logger.Errorf("Error during HTTP server shutdown (%s): %v", server.Addr, err)
+			}
 		}
 	}
 
@@ -305,7 +366,7 @@ func registerNotifiers(cfg *config.Config, factory *notifier.Factory, logger *lo
 	}
 }
 
-func startGRPCServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config, svc domain.NotificationService, logger *logging.Logger, authStore *auth.APIKeyStore) *grpc.Server {
+func startGRPCServer(_ context.Context, wg *sync.WaitGroup, cfg *config.Config, svc domain.NotificationService, logger *logging.Logger, authStore *auth.APIKeyStore) *grpc.Server {
 	addr := fmt.Sprintf("%s:%d", cfg.Server.Host, cfg.Server.GRPCPort)
 
 	lis, err := net.Listen("tcp", addr)
@@ -316,6 +377,18 @@ func startGRPCServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config
 	// Create gRPC server options
 	var serverOpts []grpc.ServerOption
 
+	// TLS credentials if configured
+	if cfg.Server.TLS.Enabled {
+		creds, err := credentials.NewServerTLSFromFile(cfg.Server.TLS.CertFile, cfg.Server.TLS.KeyFile)
+		if err != nil {
+			logger.Fatalf("Failed to load gRPC TLS credentials: %v", err)
+		}
+		serverOpts = append(serverOpts, grpc.Creds(creds))
+	}
+
+	// Bound message sizes to match the REST body limit
+	serverOpts = append(serverOpts, grpc.MaxRecvMsgSize(1<<20))
+
 	// Add authentication interceptors if enabled
 	if authStore != nil {
 		authMiddleware := auth.NewGRPCAuthMiddleware(authStore, logger)
@@ -331,6 +404,13 @@ func startGRPCServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config
 	grpcHandler := grpcapi.NewNotifierHandler(svc, logger)
 	pb.RegisterNotifierServiceServer(grpcServer, grpcHandler)
 
+	// Standard gRPC health service (grpc.health.v1) for Kubernetes gRPC
+	// probes and load balancers.
+	healthSvc := health.NewServer()
+	healthSvc.SetServingStatus("", healthgrpc.HealthCheckResponse_SERVING)
+	healthSvc.SetServingStatus(pb.NotifierService_ServiceDesc.ServiceName, healthgrpc.HealthCheckResponse_SERVING)
+	healthgrpc.RegisterHealthServer(grpcServer, healthSvc)
+
 	// Enable reflection for tools like grpcurl
 	reflection.Register(grpcServer)
 
@@ -347,16 +427,33 @@ func startGRPCServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config
 	return grpcServer
 }
 
-func startRESTServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config, svc domain.NotificationService, logger *logging.Logger, authStore *auth.APIKeyStore, hybridKeyStore *auth.HybridKeyStore) *http.Server {
-	var router *mux.Router
-	if authStore != nil && hybridKeyStore != nil {
-		router = rest.NewRouterWithAuthAndKeyStore(svc, logger, authStore, hybridKeyStore)
-	} else if authStore != nil {
-		router = rest.NewRouterWithAuth(svc, logger, authStore)
-	} else {
-		router = rest.NewRouter(svc, logger)
+func startRESTServer(_ context.Context, wg *sync.WaitGroup, cfg *config.Config, svc domain.NotificationService, logger *logging.Logger, authStore *auth.APIKeyStore, hybridKeyStore *auth.HybridKeyStore, readiness map[string]rest.ReadinessCheck, collector *metrics.Collector) *http.Server {
+	opts := rest.RouterOptions{
+		Service:   svc,
+		Logger:    logger,
+		AuthStore: authStore,
+		KeyStore:  hybridKeyStore,
+		Readiness: readiness,
 	}
 
+	// Wire CORS from config (validated at load time; empty origins = disabled)
+	if len(cfg.CORS.AllowedOrigins) > 0 {
+		opts.CORS = &rest.CORSConfig{
+			AllowedOrigins:   cfg.CORS.AllowedOrigins,
+			AllowedMethods:   cfg.CORS.AllowedMethods,
+			AllowedHeaders:   cfg.CORS.AllowedHeaders,
+			AllowCredentials: cfg.CORS.AllowCredentials,
+			MaxAge:           cfg.CORS.MaxAge,
+		}
+		logger.Infof("CORS enabled for %d origin(s)", len(cfg.CORS.AllowedOrigins))
+	}
+
+	if collector != nil {
+		opts.Instrument = collector.InstrumentHTTP
+	}
+
+	router := rest.NewRouterWithOptions(opts)
+
 	addr := fmt.Sprintf("%s:%d", cfg.Server.Host, cfg.Server.RESTPort)
 	server := &http.Server{
 		Addr:         addr,
@@ -368,8 +465,15 @@ func startRESTServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config
 
 	go func() {
 		defer wg.Done()
-		logger.Infof("REST server listening on %s", addr)
-		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+		var err error
+		if cfg.Server.TLS.Enabled {
+			logger.Infof("REST server listening on %s (TLS)", addr)
+			err = server.ListenAndServeTLS(cfg.Server.TLS.CertFile, cfg.Server.TLS.KeyFile)
+		} else {
+			logger.Infof("REST server listening on %s", addr)
+			err = server.ListenAndServe()
+		}
+		if err != nil && err != http.ErrServerClosed {
 			logger.Fatalf("Failed to start REST server: %v", err)
 		}
 	}()
@@ -377,6 +481,60 @@ func startRESTServer(ctx context.Context, wg *sync.WaitGroup, cfg *config.Config
 	return server
 }
 
+// startMetricsServer serves Prometheus metrics on the configured port.
+func startMetricsServer(wg *sync.WaitGroup, cfg *config.Config, collector *metrics.Collector, logger *logging.Logger) *http.Server {
+	path := cfg.Metrics.Path
+	if path == "" {
+		path = "/metrics"
+	}
+	mux := http.NewServeMux()
+	mux.Handle(path, collector.Handler())
+
+	addr := fmt.Sprintf("%s:%d", cfg.Server.Host, cfg.Metrics.Port)
+	server := &http.Server{
+		Addr:         addr,
+		Handler:      mux,
+		ReadTimeout:  10 * time.Second,
+		WriteTimeout: 10 * time.Second,
+	}
+
+	go func() {
+		defer wg.Done()
+		logger.Infof("Metrics server listening on %s%s", addr, path)
+		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+			logger.Errorf("Metrics server stopped: %v", err)
+		}
+	}()
+
+	return server
+}
+
+// startHealthServer serves liveness (/health) and readiness (/readyz) on a
+// dedicated port so probes work even in grpc-only mode.
+func startHealthServer(wg *sync.WaitGroup, cfg *config.Config, readiness map[string]rest.ReadinessCheck, logger *logging.Logger) *http.Server {
+	mux := http.NewServeMux()
+	mux.Handle("/health", rest.LivenessHandler())
+	mux.Handle("/readyz", rest.ReadinessHandler(readiness))
+
+	addr := fmt.Sprintf("%s:%d", cfg.Server.Host, cfg.HealthCheck.Port)
+	server := &http.Server{
+		Addr:         addr,
+		Handler:      mux,
+		ReadTimeout:  10 * time.Second,
+		WriteTimeout: 10 * time.Second,
+	}
+
+	go func() {
+		defer wg.Done()
+		logger.Infof("Health server listening on %s", addr)
+		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+			logger.Errorf("Health server stopped: %v", err)
+		}
+	}()
+
+	return server
+}
+
 func registerAuthorizationRules(cfg *config.Config, authz *auth.NotifierAuthz, logger *logging.Logger) {
 	// Register SMTP authorization rules
 	for accountName, smtpConfig := range cfg.Notifiers.SMTP {

config.yaml

@@ -5,6 +5,12 @@ server:
   rest_port: 8080
   host: "0.0.0.0"
   mode: "both" # Options: both, grpc, rest
+  # Optional TLS for both listeners. Leave disabled when a TLS-terminating
+  # gateway or service mesh fronts the service.
+  tls:
+    enabled: false
+    # cert_file: "/etc/notifier/tls/tls.crt"
+    # key_file: "/etc/notifier/tls/tls.key"
 
 queue:
   type: "local" # Options: local, kafka

go.mod

@@ -1,17 +1,16 @@
 module github.com/igodwin/notifier
 
-go 1.24.0
-
-toolchain go1.24.6
+go 1.25.0
 
 require (
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/mux v1.8.1
 	github.com/lib/pq v1.10.9
+	github.com/prometheus/client_golang v1.23.2
 	github.com/spf13/viper v1.19.0
 	github.com/testcontainers/testcontainers-go v0.39.0
-	google.golang.org/grpc v1.76.0
-	google.golang.org/protobuf v1.36.10
+	google.golang.org/grpc v1.82.1
+	google.golang.org/protobuf v1.36.11
 	k8s.io/api v0.34.1
 	k8s.io/apimachinery v0.34.1
 	k8s.io/client-go v0.34.1
@@ -21,7 +20,9 @@ require (
 	dario.cat/mergo v1.0.2 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
 	github.com/Microsoft/go-winio v0.6.2 // indirect
+	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.2.1 // indirect
+	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/containerd/errdefs v1.0.0 // indirect
 	github.com/containerd/errdefs/pkg v0.3.0 // indirect
 	github.com/containerd/log v0.1.0 // indirect
@@ -70,6 +71,9 @@ require (
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
+	github.com/prometheus/client_model v0.6.2 // indirect
+	github.com/prometheus/common v0.66.1 // indirect
+	github.com/prometheus/procfs v0.16.1 // indirect
 	github.com/sagikazarmark/locafero v0.4.0 // indirect
 	github.com/sagikazarmark/slog-shim v0.1.0 // indirect
 	github.com/shirou/gopsutil/v4 v4.25.6 // indirect
@@ -84,27 +88,26 @@ require (
 	github.com/tklauser/numcpus v0.6.1 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
 	github.com/yusufpapurcu/wmi v1.2.4 // indirect
-	go.opentelemetry.io/auto/sdk v1.1.0 // indirect
+	go.opentelemetry.io/auto/sdk v1.2.1 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect
-	go.opentelemetry.io/otel v1.38.0 // indirect
+	go.opentelemetry.io/otel v1.43.0 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.38.0 // indirect
-	go.opentelemetry.io/otel/metric v1.38.0 // indirect
-	go.opentelemetry.io/otel/sdk/metric v1.38.0 // indirect
-	go.opentelemetry.io/otel/trace v1.38.0 // indirect
+	go.opentelemetry.io/otel/metric v1.43.0 // indirect
+	go.opentelemetry.io/otel/trace v1.43.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.8.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.9.0 // indirect
 	go.yaml.in/yaml/v2 v2.4.2 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
-	golang.org/x/crypto v0.43.0 // indirect
+	golang.org/x/crypto v0.54.0 // indirect
 	golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect
-	golang.org/x/net v0.46.0 // indirect
-	golang.org/x/oauth2 v0.30.0 // indirect
-	golang.org/x/sys v0.37.0 // indirect
-	golang.org/x/term v0.36.0 // indirect
-	golang.org/x/text v0.30.0 // indirect
+	golang.org/x/net v0.57.0 // indirect
+	golang.org/x/oauth2 v0.36.0 // indirect
+	golang.org/x/sys v0.47.0 // indirect
+	golang.org/x/term v0.45.0 // indirect
+	golang.org/x/text v0.40.0 // indirect
 	golang.org/x/time v0.9.0 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20251014184007-4626949a642f // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect

go.sum

@@ -6,8 +6,12 @@ github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOEl
 github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
 github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
+github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
+github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=
 github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
+github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
+github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI=
 github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M=
 github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151Xdx3ZPPE=
@@ -97,6 +101,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
+github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
 github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4=
@@ -150,8 +156,16 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRI
 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c h1:ncq/mPwQF4JjgDlrVEn3C11VoGHZN7m8qihwgMEtzYw=
 github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE=
-github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII=
-github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o=
+github.com/prometheus/client_golang v1.23.2 h1:Je96obch5RDVy3FDMndoUsjAhG5Edi49h0RJWRi/o0o=
+github.com/prometheus/client_golang v1.23.2/go.mod h1:Tb1a6LWHB3/SPIzCoaDXI4I8UHKeFTEQ1YCr+0Gyqmg=
+github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk=
+github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE=
+github.com/prometheus/common v0.66.1 h1:h5E0h5/Y8niHc5DlaLlWLArTQI7tMrsfQjHV+d9ZoGs=
+github.com/prometheus/common v0.66.1/go.mod h1:gcaUsgf3KfRSwHY4dIMXLPV0K/Wg1oZ8+SbZk/HH/dA=
+github.com/prometheus/procfs v0.16.1 h1:hZ15bTNuirocR6u0JZ6BAHHmwS1p8B4P6MRqxtzMyRg=
+github.com/prometheus/procfs v0.16.1/go.mod h1:teAbpZRB1iIAJYREa1LsoWUXykVXA1KlTmWl8x/U+Is=
+github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
+github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc=
 github.com/sagikazarmark/locafero v0.4.0 h1:HApY1R9zGo4DBgr7dqsTH/JJxLTTsOt7u6keLGt6kNQ=
 github.com/sagikazarmark/locafero v0.4.0/go.mod h1:Pe1W6UlPYUk/+wc/6KFhbORCfqzgYEpgQ3O5fPuL3H4=
 github.com/sagikazarmark/slog-shim v0.1.0 h1:diDBnUNK9N/354PgrxMywXnAwEr1QZcOr6gto+ugjYE=
@@ -198,28 +212,30 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0=
 github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
-go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA=
-go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A=
+go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64=
+go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y=
 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 h1:jq9TW8u3so/bN+JPT166wjOI6/vQPF6Xe7nMNIltagk=
 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0/go.mod h1:p8pYQP+m5XfbZm9fxtSKAbM6oIllS7s2AfxrChvc7iw=
-go.opentelemetry.io/otel v1.38.0 h1:RkfdswUDRimDg0m2Az18RKOsnI8UDzppJAtj01/Ymk8=
-go.opentelemetry.io/otel v1.38.0/go.mod h1:zcmtmQ1+YmQM9wrNsTGV/q/uyusom3P8RxwExxkZhjM=
+go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I=
+go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0=
 go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.38.0 h1:GqRJVj7UmLjCVyVJ3ZFLdPRmhDUp2zFmQe3RHIOsw24=
 go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.38.0/go.mod h1:ri3aaHSmCTVYu2AWv44YMauwAQc0aqI9gHKIcSbI1pU=
 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 h1:IeMeyr1aBvBiPVYihXIaeIZba6b8E1bYp7lbdxK8CQg=
 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0/go.mod h1:oVdCUtjq9MK9BlS7TtucsQwUcXcymNiEDjgDD2jMtZU=
-go.opentelemetry.io/otel/metric v1.38.0 h1:Kl6lzIYGAh5M159u9NgiRkmoMKjvbsKtYRwgfrA6WpA=
-go.opentelemetry.io/otel/metric v1.38.0/go.mod h1:kB5n/QoRM8YwmUahxvI3bO34eVtQf2i4utNVLr9gEmI=
-go.opentelemetry.io/otel/sdk v1.38.0 h1:l48sr5YbNf2hpCUj/FoGhW9yDkl+Ma+LrVl8qaM5b+E=
-go.opentelemetry.io/otel/sdk v1.38.0/go.mod h1:ghmNdGlVemJI3+ZB5iDEuk4bWA3GkTpW+DOoZMYBVVg=
-go.opentelemetry.io/otel/sdk/metric v1.38.0 h1:aSH66iL0aZqo//xXzQLYozmWrXxyFkBJ6qT5wthqPoM=
-go.opentelemetry.io/otel/sdk/metric v1.38.0/go.mod h1:dg9PBnW9XdQ1Hd6ZnRz689CbtrUp0wMMs9iPcgT9EZA=
-go.opentelemetry.io/otel/trace v1.38.0 h1:Fxk5bKrDZJUH+AMyyIXGcFAPah0oRcT+LuNtJrmcNLE=
-go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs=
+go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM=
+go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY=
+go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg=
+go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg=
+go.opentelemetry.io/otel/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw=
+go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A=
+go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A=
+go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0=
 go.opentelemetry.io/proto/otlp v1.8.0 h1:fRAZQDcAFHySxpJ1TwlA1cJ4tvcrw7nXl9xWWC8N5CE=
 go.opentelemetry.io/proto/otlp v1.8.0/go.mod h1:tIeYOeNBU4cvmPqpaji1P+KbB4Oloai8wN4rWzRrFF0=
 go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
 go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
+go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
+go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
 go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI=
 go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ=
 go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
@@ -229,8 +245,8 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
-golang.org/x/crypto v0.43.0 h1:dduJYIi3A3KOfdGOHX8AVZ/jGiyPa3IbBozJ5kNuE04=
-golang.org/x/crypto v0.43.0/go.mod h1:BFbav4mRNlXJL4wNeejLpWxB7wMbc79PdRGhWKncxR0=
+golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw=
+golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk=
 golang.org/x/exp v0.0.0-20230905200255-921286631fa9 h1:GoHiUyI/Tp2nVkLI2mCxVkOjsbSXD66ic0XW0js0R9g=
 golang.org/x/exp v0.0.0-20230905200255-921286631fa9/go.mod h1:S2oDrQGGwySpoQPVqRShND87VCbxmc6bL1Yd2oYrm6k=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
@@ -239,10 +255,10 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
-golang.org/x/net v0.46.0 h1:giFlY12I07fugqwPuWJi68oOnpfqFnJIJzaIIm2JVV4=
-golang.org/x/net v0.46.0/go.mod h1:Q9BGdFy1y4nkUwiLvT5qtyhAnEHgnQ/zd8PfU6nc210=
-golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI=
-golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU=
+golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE=
+golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU=
+golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs=
+golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -255,37 +271,37 @@ golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ=
-golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
-golang.org/x/term v0.36.0 h1:zMPR+aF8gfksFprF/Nc/rd1wRS1EI6nDBGyWAvDzx2Q=
-golang.org/x/term v0.36.0/go.mod h1:Qu394IJq6V6dCBRgwqshf3mPF85AqzYEzofzRdZkWss=
+golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
+golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
+golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0=
+golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
-golang.org/x/text v0.30.0 h1:yznKA/E9zq54KzlzBEAWn1NXSQ8DIp/NYMy88xJjl4k=
-golang.org/x/text v0.30.0/go.mod h1:yDdHFIX9t+tORqspjENWgzaCVXgk0yYnYuSZ8UzzBVM=
+golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs=
+golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY=
 golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY=
 golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.37.0 h1:DVSRzp7FwePZW356yEAChSdNcQo6Nsp+fex1SUW09lE=
-golang.org/x/tools v0.37.0/go.mod h1:MBN5QPQtLMHVdvsbtarmTNukZDdgwdwlO5qGacAzF0w=
+golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q=
+golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk=
-gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E=
+gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4=
+gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E=
 google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9 h1:9+tzLLstTlPTRyJTh+ah5wIMsBW5c4tQwGTN3thOW9Y=
-google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 h1:BIRfGDEjiHRrk0QKZe3Xv2ieMhtgRGeLcZQ0mIVn4EY=
-google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5/go.mod h1:j3QtIyytwqGr1JUDtYXwtMXWPKsEa5LtzIFN1Wn5WvE=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20251014184007-4626949a642f h1:1FTH6cpXFsENbPR5Bu8NQddPSaUUE6NA2XdZdDSAJK4=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20251014184007-4626949a642f/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk=
-google.golang.org/grpc v1.76.0 h1:UnVkv1+uMLYXoIz6o7chp59WfQUYA2ex/BXQ9rHZu7A=
-google.golang.org/grpc v1.76.0/go.mod h1:Ju12QI8M6iQJtbcsV+awF5a4hfJMLi4X0JLo94ULZ6c=
-google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE=
-google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco=
+google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 h1:yQugLulqltosq0B/f8l4w9VryjV+N/5gcW0jQ3N8Qec=
+google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478/go.mod h1:C6ADNqOxbgdUUeRTU+LCHDPB9ttAMCTff6auwCVa4uc=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 h1:RmoJA1ujG+/lRGNfUnOMfhCy5EipVMyvUE+KNbPbTlw=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8=
+google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE=
+google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA=
+google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE=
+google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=

internal/auth/auth.go

@@ -1,24 +1,44 @@
+// Package auth provides API key authentication and authorization for the
+// notifier service, including key storage backends (in-memory, database,
+// and a hybrid cache-plus-database store) and RBAC-style notifier
+// authorization.
 package auth
 
 import (
 	"context"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"sync"
 	"time"
 )
 
-// APIKeyStore manages API keys with rate limiting
+// Sentinel errors for key validation and lookup. Match with errors.Is.
+var (
+	ErrInvalidKey  = errors.New("invalid API key")
+	ErrKeyInactive = errors.New("API key is inactive")
+	ErrKeyExpired  = errors.New("API key has expired")
+	ErrKeyNotFound = errors.New("API key not found")
+)
+
+// APIKeyStore manages API keys with rate limiting.
+// Keys are stored indexed by SHA-256 digest; the raw key is never retained
+// after creation.
 type APIKeyStore struct {
 	mu         sync.RWMutex
-	keys       map[string]*APIKey
+	keys       map[string]*APIKey // keyed by KeyHash
 	rateLimits map[string]*RateLimiter
 }
 
-// APIKey represents an API key with metadata
+// APIKey represents an API key with metadata.
+// Key holds the raw secret only on the value returned from key creation;
+// stored and persisted records carry only KeyHash and KeyPreview.
 type APIKey struct {
-	Key        string     `json:"key"`
+	Key        string     `json:"key,omitempty"`
+	KeyHash    string     `json:"-"`
+	KeyPreview string     `json:"key_preview,omitempty"` // e.g. "nk_…ab12"
 	Name       string     `json:"name"`
 	ClientID   string     `json:"client_id"`
 	Roles      []string   `json:"roles"`
@@ -38,13 +58,60 @@ type RateLimiter struct {
 	mu          sync.Mutex
 }
 
-// AuthContext holds auth information attached to request context
-type AuthContext struct {
+// Context holds auth information attached to request context
+type Context struct {
 	APIKey   *APIKey
 	ClientID string
 	Roles    []string
 }
 
+// HashKey returns the hex-encoded SHA-256 digest of a raw API key.
+// All storage and lookups are keyed by this digest so a leaked store or
+// database dump does not expose usable credentials.
+func HashKey(rawKey string) string {
+	sum := sha256.Sum256([]byte(rawKey))
+	return hex.EncodeToString(sum[:])
+}
+
+// keyPreview returns a non-sensitive display form of a raw key ("nk_…ab12").
+func keyPreview(rawKey string) string {
+	if len(rawKey) < 4 {
+		return ""
+	}
+	return "nk_…" + rawKey[len(rawKey)-4:]
+}
+
+// generateAPIKey creates a new APIKey with a random secret without touching
+// any store. The returned value carries the raw secret in Key; callers decide
+// where (and whether) to register it.
+func generateAPIKey(clientID string, roles []string, rateLimit int, expiresIn *time.Duration) (*APIKey, error) {
+	keyBytes := make([]byte, 32)
+	if _, err := rand.Read(keyBytes); err != nil {
+		return nil, fmt.Errorf("failed to generate key: %w", err)
+	}
+	key := "nk_" + hex.EncodeToString(keyBytes)
+
+	now := time.Now().UTC()
+	apiKey := &APIKey{
+		Key:        key,
+		KeyHash:    HashKey(key),
+		KeyPreview: keyPreview(key),
+		ClientID:   clientID,
+		Roles:      roles,
+		CreatedAt:  now,
+		IsActive:   true,
+		RateLimit:  rateLimit,
+		Name:       fmt.Sprintf("%s-%d", clientID, now.Unix()),
+	}
+
+	if expiresIn != nil {
+		expiresAt := now.Add(*expiresIn)
+		apiKey.ExpiresAt = &expiresAt
+	}
+
+	return apiKey, nil
+}
+
 // NewAPIKeyStore creates a new API key store
 func NewAPIKeyStore() *APIKeyStore {
 	return &APIKeyStore{
@@ -53,61 +120,64 @@ func NewAPIKeyStore() *APIKeyStore {
 	}
 }
 
-// CreateKey generates a new API key
+// CreateKey generates a new API key and registers it in the store.
+// The returned APIKey carries the raw secret; the stored copy does not.
 func (s *APIKeyStore) CreateKey(clientID string, roles []string, rateLimit int, expiresIn *time.Duration) (*APIKey, error) {
+	apiKey, err := generateAPIKey(clientID, roles, rateLimit, expiresIn)
+	if err != nil {
+		return nil, err
+	}
+	s.RegisterKey(apiKey)
+	return apiKey, nil
+}
+
+// RegisterKey adds a key to the store, indexed by digest. The stored copy has
+// the raw secret stripped. If KeyHash is unset it is computed from Key.
+func (s *APIKeyStore) RegisterKey(apiKey *APIKey) {
+	stored := *apiKey
+	if stored.KeyHash == "" && stored.Key != "" {
+		stored.KeyHash = HashKey(stored.Key)
+	}
+	if stored.KeyPreview == "" && stored.Key != "" {
+		stored.KeyPreview = keyPreview(stored.Key)
+	}
+	stored.Key = "" // never retain the raw secret
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
-
-	// Generate random key
-	keyBytes := make([]byte, 32)
-	if _, err := rand.Read(keyBytes); err != nil {
-		return nil, fmt.Errorf("failed to generate key: %w", err)
-	}
-	key := "nk_" + hex.EncodeToString(keyBytes)
-
-	now := time.Now().UTC()
-	apiKey := &APIKey{
-		Key:       key,
-		ClientID:  clientID,
-		Roles:     roles,
-		CreatedAt: now,
-		IsActive:  true,
-		RateLimit: rateLimit,
-		Name:      fmt.Sprintf("%s-%d", clientID, now.Unix()),
-	}
-
-	if expiresIn != nil {
-		expiresAt := now.Add(*expiresIn)
-		apiKey.ExpiresAt = &expiresAt
-	}
-
-	s.keys[key] = apiKey
-	s.rateLimits[key] = &RateLimiter{
-		maxRequests: rateLimit,
+	s.keys[stored.KeyHash] = &stored
+	s.rateLimits[stored.KeyHash] = &RateLimiter{
+		maxRequests: stored.RateLimit,
 		window:      time.Minute,
 		resetTime:   time.Now().Add(time.Minute),
 		count:       0,
 	}
-
-	return apiKey, nil
 }
 
-// ValidateKey checks if an API key is valid and returns the key metadata
+// RemoveKeyByHash deletes a key and its rate limiter from the store.
+func (s *APIKeyStore) RemoveKeyByHash(keyHash string) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	delete(s.keys, keyHash)
+	delete(s.rateLimits, keyHash)
+}
+
+// ValidateKey checks if a raw API key is valid and returns the key metadata
 func (s *APIKeyStore) ValidateKey(keyStr string) (*APIKey, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 
-	key, exists := s.keys[keyStr]
+	key, exists := s.keys[HashKey(keyStr)]
 	if !exists {
-		return nil, fmt.Errorf("invalid API key")
+		return nil, ErrInvalidKey
 	}
 
 	if !key.IsActive {
-		return nil, fmt.Errorf("API key is inactive")
+		return nil, ErrKeyInactive
 	}
 
 	if key.ExpiresAt != nil && time.Now().After(*key.ExpiresAt) {
-		return nil, fmt.Errorf("API key has expired")
+		return nil, ErrKeyExpired
 	}
 
 	return key, nil
@@ -115,13 +185,15 @@ 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) {
+	keyHash := HashKey(keyStr)
+
 	// Look up key and limiter under the store lock, then release it
 	// before acquiring the per-key limiter lock to avoid nested locking.
 	s.mu.RLock()
-	key, exists := s.keys[keyStr]
+	key, exists := s.keys[keyHash]
 	if !exists {
 		s.mu.RUnlock()
-		return false, fmt.Errorf("invalid API key")
+		return false, ErrInvalidKey
 	}
 
 	// Unlimited rate limit
@@ -130,7 +202,7 @@ func (s *APIKeyStore) CheckRateLimit(keyStr string) (bool, error) {
 		return true, nil
 	}
 
-	limiter, exists := s.rateLimits[keyStr]
+	limiter, exists := s.rateLimits[keyHash]
 	if !exists {
 		s.mu.RUnlock()
 		return false, fmt.Errorf("rate limiter not found")
@@ -160,9 +232,9 @@ func (s *APIKeyStore) UpdateLastUsed(keyStr string) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	key, exists := s.keys[keyStr]
+	key, exists := s.keys[HashKey(keyStr)]
 	if !exists {
-		return fmt.Errorf("invalid API key")
+		return ErrInvalidKey
 	}
 
 	now := time.Now().UTC()
@@ -170,28 +242,28 @@ func (s *APIKeyStore) UpdateLastUsed(keyStr string) error {
 	return nil
 }
 
-// DeactivateKey deactivates an API key
+// DeactivateKey deactivates an API key by its raw value
 func (s *APIKeyStore) DeactivateKey(keyStr string) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	key, exists := s.keys[keyStr]
+	key, exists := s.keys[HashKey(keyStr)]
 	if !exists {
-		return fmt.Errorf("invalid API key")
+		return ErrInvalidKey
 	}
 
 	key.IsActive = false
 	return nil
 }
 
-// GetKey retrieves key metadata (for management purposes)
+// GetKey retrieves key metadata by raw key (for management purposes)
 func (s *APIKeyStore) GetKey(keyStr string) (*APIKey, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 
-	key, exists := s.keys[keyStr]
+	key, exists := s.keys[HashKey(keyStr)]
 	if !exists {
-		return nil, fmt.Errorf("key not found")
+		return nil, ErrKeyNotFound
 	}
 
 	return key, nil
@@ -215,12 +287,12 @@ func (s *APIKeyStore) ListKeys(clientID string) []*APIKey {
 type authContextKey struct{}
 
 // ContextWithAuth adds auth context to a request context
-func ContextWithAuth(ctx context.Context, auth *AuthContext) context.Context {
+func ContextWithAuth(ctx context.Context, auth *Context) context.Context {
 	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(authContextKey{}).(*AuthContext)
+func GetAuthContext(ctx context.Context) (*Context, bool) {
+	auth, ok := ctx.Value(authContextKey{}).(*Context)
 	return auth, ok
 }

internal/auth/authz.go

@@ -26,7 +26,7 @@ func (a *NotifierAuthz) RegisterRule(notificationType domain.NotificationType, a
 }
 
 // IsAuthorized checks if an auth context is authorized to use a specific notifier
-func (a *NotifierAuthz) IsAuthorized(auth *AuthContext, notificationType domain.NotificationType, account string) bool {
+func (a *NotifierAuthz) IsAuthorized(auth *Context, notificationType domain.NotificationType, account string) bool {
 	if auth == nil || len(auth.Roles) == 0 {
 		return false
 	}

internal/auth/bootstrap.go

@@ -101,26 +101,18 @@ func RegisterAdminKeyInMemory(keyStore *APIKeyStore, adminKey string, logger *lo
 	adminRoles := []string{"admin", "notify-email", "notify-slack", "notify-ntfy"}
 	now := time.Now().UTC()
 	apiKey := &APIKey{
-		Key:       adminKey,
-		ClientID:  "admin-bootstrap",
-		Roles:     adminRoles,
-		CreatedAt: now,
-		IsActive:  true,
-		RateLimit: 0, // Unlimited
-		Name:      fmt.Sprintf("admin-bootstrap-%d", now.Unix()),
+		Key:        adminKey,
+		KeyHash:    HashKey(adminKey),
+		KeyPreview: keyPreview(adminKey),
+		ClientID:   "admin-bootstrap",
+		Roles:      adminRoles,
+		CreatedAt:  now,
+		IsActive:   true,
+		RateLimit:  0, // Unlimited
+		Name:       fmt.Sprintf("admin-bootstrap-%d", now.Unix()),
 	}
 
-	// Add to keystore
-	keyStore.mu.Lock()
-	defer keyStore.mu.Unlock()
-
-	keyStore.keys[adminKey] = apiKey
-	keyStore.rateLimits[adminKey] = &RateLimiter{
-		maxRequests: 0, // Unlimited
-		window:      time.Minute,
-		resetTime:   time.Now().Add(time.Minute),
-		count:       0,
-	}
+	keyStore.RegisterKey(apiKey)
 
 	logger.Infof("Registered existing admin key from Kubernetes secret")
 	return apiKey, nil
@@ -191,7 +183,7 @@ func BootstrapAdminKey(ctx context.Context, keyStore *HybridKeyStore, cfg *Boots
 
 // LoadBootstrapKeyFromEnv checks if a bootstrap key was provided via environment variable
 // This allows injecting a pre-generated key via CI/CD
-func LoadBootstrapKeyFromEnv(ctx context.Context, keyStore *HybridKeyStore, logger *logging.Logger) error {
+func LoadBootstrapKeyFromEnv(_ context.Context, _ *HybridKeyStore, logger *logging.Logger) error {
 	bootstrapKey := os.Getenv("NOTIFIER_BOOTSTRAP_ADMIN_KEY")
 	if bootstrapKey == "" {
 		return nil // Not set, skip

internal/auth/grpc_middleware.go

@@ -55,7 +55,7 @@ func (m *GRPCAuthMiddleware) UnaryInterceptor() grpc.UnaryServerInterceptor {
 		}
 
 		// Create auth context and attach to request
-		authCtx := &AuthContext{
+		authCtx := &Context{
 			APIKey:   key,
 			ClientID: key.ClientID,
 			Roles:    key.Roles,
@@ -99,7 +99,7 @@ func (m *GRPCAuthMiddleware) StreamInterceptor() grpc.StreamServerInterceptor {
 		}
 
 		// Create auth context and attach to request
-		authCtx := &AuthContext{
+		authCtx := &Context{
 			APIKey:   key,
 			ClientID: key.ClientID,
 			Roles:    key.Roles,

internal/auth/keystore_db.go

@@ -3,34 +3,41 @@ package auth
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"fmt"
 	"time"
 
+	"github.com/igodwin/notifier/internal/logging"
 	"github.com/lib/pq"
-	_ "github.com/lib/pq" // PostgreSQL driver
 )
 
-// KeyStoreDB provides persistent storage for API keys using PostgreSQL
-// It acts as the backend for the in-memory cache
+// KeyStoreDB provides persistent storage for API keys using PostgreSQL.
+// It acts as the backend for the in-memory cache. Only SHA-256 digests of
+// keys are persisted — the raw secret never reaches the database.
 type KeyStoreDB struct {
-	db *sql.DB
+	db     *sql.DB
+	logger *logging.Logger
 }
 
 // NewKeyStoreDB creates a new database-backed key store
-func NewKeyStoreDB(dbURL string) (*KeyStoreDB, error) {
+func NewKeyStoreDB(dbURL string, logger *logging.Logger) (*KeyStoreDB, error) {
 	db, err := sql.Open("postgres", dbURL)
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to database: %w", err)
 	}
 
-	// Test connection
-	if err := db.Ping(); err != nil {
+	db.SetMaxOpenConns(10)
+	db.SetMaxIdleConns(5)
+	db.SetConnMaxLifetime(30 * time.Minute)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	if err := db.PingContext(ctx); err != nil {
 		return nil, fmt.Errorf("failed to ping database: %w", err)
 	}
 
-	ks := &KeyStoreDB{db: db}
+	ks := &KeyStoreDB{db: db, logger: logger}
 
-	// Initialize schema
 	if err := ks.initializeSchema(); err != nil {
 		return nil, fmt.Errorf("failed to initialize schema: %w", err)
 	}
@@ -38,14 +45,15 @@ func NewKeyStoreDB(dbURL string) (*KeyStoreDB, error) {
 	return ks, nil
 }
 
-// initializeSchema creates the necessary tables and indexes if they don't exist
+// initializeSchema creates the necessary tables and indexes if they don't
+// exist, and migrates legacy plaintext-key rows to hashed storage.
 func (ks *KeyStoreDB) initializeSchema() error {
-	// Create tables
 	tableSchema := `
-	-- API Keys table
+	-- API Keys table (key_hash is the SHA-256 digest of the raw key)
 	CREATE TABLE IF NOT EXISTS api_keys (
 		id SERIAL PRIMARY KEY,
-		key VARCHAR(255) UNIQUE NOT NULL,
+		key_hash VARCHAR(64) UNIQUE NOT NULL,
+		key_preview VARCHAR(16) NOT NULL DEFAULT '',
 		name VARCHAR(255) NOT NULL,
 		client_id VARCHAR(255) NOT NULL,
 		roles TEXT[] DEFAULT '{}',
@@ -73,9 +81,11 @@ func (ks *KeyStoreDB) initializeSchema() error {
 		return fmt.Errorf("failed to create tables: %w", err)
 	}
 
-	// Create indexes separately (PostgreSQL syntax)
+	if err := ks.migrateLegacyPlaintextKeys(); err != nil {
+		return fmt.Errorf("failed to migrate legacy plaintext keys: %w", err)
+	}
+
 	indexSchema := `
-	CREATE INDEX IF NOT EXISTS idx_api_keys_key ON api_keys(key);
 	CREATE INDEX IF NOT EXISTS idx_api_keys_client_id ON api_keys(client_id);
 	CREATE INDEX IF NOT EXISTS idx_api_keys_active ON api_keys(is_active);
 	CREATE INDEX IF NOT EXISTS idx_api_keys_expires ON api_keys(expires_at);
@@ -90,12 +100,87 @@ func (ks *KeyStoreDB) initializeSchema() error {
 	return nil
 }
 
-// SaveKey persists an API key to the database
+// migrateLegacyPlaintextKeys upgrades tables created by earlier versions that
+// stored the raw key in a "key" column: it adds the hash columns, hashes each
+// plaintext key in place, then drops the plaintext column entirely.
+func (ks *KeyStoreDB) migrateLegacyPlaintextKeys() error {
+	var hasLegacyColumn bool
+	err := ks.db.QueryRow(`
+		SELECT EXISTS (
+			SELECT 1 FROM information_schema.columns
+			WHERE table_name = 'api_keys' AND column_name = 'key'
+		)`).Scan(&hasLegacyColumn)
+	if err != nil {
+		return fmt.Errorf("failed to inspect schema: %w", err)
+	}
+	if !hasLegacyColumn {
+		return nil
+	}
+
+	if _, err := ks.db.Exec(`
+		ALTER TABLE api_keys ADD COLUMN IF NOT EXISTS key_hash VARCHAR(64);
+		ALTER TABLE api_keys ADD COLUMN IF NOT EXISTS key_preview VARCHAR(16) NOT NULL DEFAULT '';
+	`); err != nil {
+		return fmt.Errorf("failed to add hash columns: %w", err)
+	}
+
+	rows, err := ks.db.Query(`SELECT id, key FROM api_keys WHERE key_hash IS NULL AND key IS NOT NULL`)
+	if err != nil {
+		return fmt.Errorf("failed to read legacy keys: %w", err)
+	}
+	defer func() { _ = rows.Close() }()
+
+	type legacyRow struct {
+		id  int
+		key string
+	}
+	var legacy []legacyRow
+	for rows.Next() {
+		var r legacyRow
+		if err := rows.Scan(&r.id, &r.key); err != nil {
+			return fmt.Errorf("failed to scan legacy key: %w", err)
+		}
+		legacy = append(legacy, r)
+	}
+	if err := rows.Err(); err != nil {
+		return err
+	}
+
+	for _, r := range legacy {
+		if _, err := ks.db.Exec(
+			`UPDATE api_keys SET key_hash = $1, key_preview = $2 WHERE id = $3`,
+			HashKey(r.key), keyPreview(r.key), r.id,
+		); err != nil {
+			return fmt.Errorf("failed to hash legacy key id=%d: %w", r.id, err)
+		}
+	}
+
+	// Drop the plaintext column and enforce uniqueness on the digest.
+	if _, err := ks.db.Exec(`
+		ALTER TABLE api_keys DROP COLUMN key;
+		ALTER TABLE api_keys ALTER COLUMN key_hash SET NOT NULL;
+		CREATE UNIQUE INDEX IF NOT EXISTS idx_api_keys_key_hash ON api_keys(key_hash);
+	`); err != nil {
+		return fmt.Errorf("failed to finalize hash migration: %w", err)
+	}
+
+	if ks.logger != nil {
+		ks.logger.Infof("Migrated %d legacy plaintext API key(s) to hashed storage", len(legacy))
+	}
+	return nil
+}
+
+// SaveKey persists an API key to the database. Only the digest and preview
+// are stored; apiKey.Key (the raw secret) is never written.
 func (ks *KeyStoreDB) SaveKey(ctx context.Context, key *APIKey, createdBy string) error {
+	if key.KeyHash == "" {
+		return fmt.Errorf("refusing to save key without digest")
+	}
+
 	query := `
-	INSERT INTO api_keys (key, name, client_id, roles, created_at, last_used_at, expires_at, is_active, rate_limit, created_by)
-	VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)
-	ON CONFLICT (key) DO UPDATE SET
+	INSERT INTO api_keys (key_hash, key_preview, name, client_id, roles, created_at, last_used_at, expires_at, is_active, rate_limit, created_by)
+	VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
+	ON CONFLICT (key_hash) DO UPDATE SET
 		name = EXCLUDED.name,
 		roles = EXCLUDED.roles,
 		is_active = EXCLUDED.is_active,
@@ -104,10 +189,11 @@ func (ks *KeyStoreDB) SaveKey(ctx context.Context, key *APIKey, createdBy string
 	`
 
 	_, err := ks.db.ExecContext(ctx, query,
-		key.Key,
+		key.KeyHash,
+		key.KeyPreview,
 		key.Name,
 		key.ClientID,
-		pq.Array(key.Roles), // Convert Go slice to PostgreSQL array
+		pq.Array(key.Roles),
 		key.CreatedAt,
 		key.LastUsedAt,
 		key.ExpiresAt,
@@ -120,8 +206,7 @@ func (ks *KeyStoreDB) SaveKey(ctx context.Context, key *APIKey, createdBy string
 		return fmt.Errorf("failed to save key: %w", err)
 	}
 
-	// Log to audit trail
-	ks.logAudit(ctx, key.Key, "created", createdBy, map[string]interface{}{
+	ks.logAudit(ctx, key.KeyHash, "created", createdBy, map[string]interface{}{
 		"client_id": key.ClientID,
 		"roles":     key.Roles,
 	})
@@ -129,87 +214,75 @@ func (ks *KeyStoreDB) SaveKey(ctx context.Context, key *APIKey, createdBy string
 	return nil
 }
 
-// GetKey retrieves an API key from the database
-func (ks *KeyStoreDB) GetKey(ctx context.Context, keyStr 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 key = $1
-	`
-
+// scanKey scans a single api_keys row into an APIKey.
+func scanKey(scanner interface{ Scan(...interface{}) error }) (*APIKey, error) {
 	var key APIKey
 	var roles []string
 
-	err := ks.db.QueryRowContext(ctx, query, keyStr).Scan(
-		&key.Key,
+	err := scanner.Scan(
+		&key.KeyHash,
+		&key.KeyPreview,
 		&key.Name,
 		&key.ClientID,
-		&roles,
+		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: %w", err)
+		return nil, err
 	}
 
 	key.Roles = roles
 	return &key, nil
 }
 
-// ListKeys retrieves all API keys for a client
-func (ks *KeyStoreDB) ListKeys(ctx context.Context, clientID 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 client_id = $1 AND is_active = true
-	ORDER BY created_at DESC
-	`
+const keyColumns = `key_hash, key_preview, name, client_id, roles, created_at, last_used_at, expires_at, is_active, rate_limit`
 
-	rows, err := ks.db.QueryContext(ctx, query, clientID)
+// GetKeyByHash retrieves an API key from the database by its digest
+func (ks *KeyStoreDB) GetKeyByHash(ctx context.Context, keyHash string) (*APIKey, error) {
+	row := ks.db.QueryRowContext(ctx,
+		`SELECT `+keyColumns+` FROM api_keys WHERE key_hash = $1`, keyHash)
+
+	key, err := scanKey(row)
+	if err == sql.ErrNoRows {
+		return nil, ErrKeyNotFound
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to get key: %w", err)
+	}
+	return key, nil
+}
+
+// ListKeys retrieves all active API keys for a client
+func (ks *KeyStoreDB) ListKeys(ctx context.Context, clientID string) ([]*APIKey, error) {
+	rows, err := ks.db.QueryContext(ctx,
+		`SELECT `+keyColumns+` FROM api_keys
+		 WHERE client_id = $1 AND is_active = true
+		 ORDER BY created_at DESC`, clientID)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list keys: %w", err)
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	var keys []*APIKey
 	for rows.Next() {
-		var key APIKey
-		var roles []string
-
-		err := rows.Scan(
-			&key.Key,
-			&key.Name,
-			&key.ClientID,
-			&roles,
-			&key.CreatedAt,
-			&key.LastUsedAt,
-			&key.ExpiresAt,
-			&key.IsActive,
-			&key.RateLimit,
-		)
+		key, err := scanKey(rows)
 		if err != nil {
 			return nil, fmt.Errorf("failed to scan key: %w", err)
 		}
-
-		key.Roles = roles
-		keys = append(keys, &key)
+		keys = append(keys, key)
 	}
 
 	return keys, rows.Err()
 }
 
-// DeactivateKey disables an API key
-func (ks *KeyStoreDB) DeactivateKey(ctx context.Context, keyStr string, deactivatedBy string) error {
-	query := `UPDATE api_keys SET is_active = false WHERE key = $1`
-
-	result, err := ks.db.ExecContext(ctx, query, keyStr)
+// DeactivateKeyByHash disables an API key identified by its digest
+func (ks *KeyStoreDB) DeactivateKeyByHash(ctx context.Context, keyHash string, deactivatedBy string) error {
+	result, err := ks.db.ExecContext(ctx,
+		`UPDATE api_keys SET is_active = false WHERE key_hash = $1`, keyHash)
 	if err != nil {
 		return fmt.Errorf("failed to deactivate key: %w", err)
 	}
@@ -223,85 +296,78 @@ func (ks *KeyStoreDB) DeactivateKey(ctx context.Context, keyStr string, deactiva
 		return ErrKeyNotFound
 	}
 
-	ks.logAudit(ctx, keyStr, "deactivated", deactivatedBy, nil)
+	ks.logAudit(ctx, keyHash, "deactivated", deactivatedBy, nil)
 	return nil
 }
 
-// UpdateLastUsed updates the last_used_at timestamp
-func (ks *KeyStoreDB) UpdateLastUsed(ctx context.Context, keyStr string) error {
-	query := `UPDATE api_keys SET last_used_at = CURRENT_TIMESTAMP WHERE key = $1`
-
-	_, err := ks.db.ExecContext(ctx, query, keyStr)
+// UpdateLastUsed updates the last_used_at timestamp for a key digest
+func (ks *KeyStoreDB) UpdateLastUsed(ctx context.Context, keyHash string) error {
+	_, err := ks.db.ExecContext(ctx,
+		`UPDATE api_keys SET last_used_at = CURRENT_TIMESTAMP WHERE key_hash = $1`, keyHash)
 	if err != nil {
 		return fmt.Errorf("failed to update last used: %w", err)
 	}
-
 	return nil
 }
 
 // LoadAllKeys loads all active keys into memory for caching
 func (ks *KeyStoreDB) LoadAllKeys(ctx context.Context) ([]*APIKey, error) {
-	query := `
-	SELECT key, name, client_id, roles, created_at, last_used_at, expires_at, is_active, rate_limit
-	FROM api_keys
-	WHERE is_active = true AND (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)
-	`
-
-	rows, err := ks.db.QueryContext(ctx, query)
+	rows, err := ks.db.QueryContext(ctx,
+		`SELECT `+keyColumns+` FROM api_keys
+		 WHERE is_active = true AND (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)`)
 	if err != nil {
 		return nil, fmt.Errorf("failed to load keys: %w", err)
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	var keys []*APIKey
 	for rows.Next() {
-		var key APIKey
-		var roles []string
-
-		err := rows.Scan(
-			&key.Key,
-			&key.Name,
-			&key.ClientID,
-			&roles,
-			&key.CreatedAt,
-			&key.LastUsedAt,
-			&key.ExpiresAt,
-			&key.IsActive,
-			&key.RateLimit,
-		)
+		key, err := scanKey(rows)
 		if err != nil {
 			return nil, fmt.Errorf("failed to scan key: %w", err)
 		}
-
-		key.Roles = roles
-		keys = append(keys, &key)
+		keys = append(keys, key)
 	}
 
 	return keys, rows.Err()
 }
 
-// logAudit logs a key operation to the audit trail
-func (ks *KeyStoreDB) logAudit(ctx context.Context, keyStr string, action string, performedBy string, details map[string]interface{}) {
-	// Get key ID
+// logAudit logs a key operation to the audit trail. Failures don't abort the
+// calling operation but are logged rather than silently dropped.
+func (ks *KeyStoreDB) logAudit(ctx context.Context, keyHash string, action string, performedBy string, details map[string]interface{}) {
 	var keyID int
-	err := ks.db.QueryRowContext(ctx, "SELECT id FROM api_keys WHERE key = $1", keyStr).Scan(&keyID)
-	if err != nil {
-		return // Silently fail audit logging
+	if err := ks.db.QueryRowContext(ctx,
+		"SELECT id FROM api_keys WHERE key_hash = $1", keyHash).Scan(&keyID); err != nil {
+		ks.warnf("audit log: failed to resolve key for action=%s: %v", action, err)
+		return
 	}
 
-	// Log the action
-	detailsJSON := "{}"
+	detailsJSON := []byte("{}")
 	if len(details) > 0 {
-		// Simple JSON encoding (could use jsonb package for robustness)
-		detailsJSON = fmt.Sprintf(`{"event": "%s"}`, action)
+		encoded, err := json.Marshal(details)
+		if err != nil {
+			ks.warnf("audit log: failed to encode details for action=%s: %v", action, err)
+		} else {
+			detailsJSON = encoded
+		}
 	}
 
-	query := `
-	INSERT INTO api_key_audit_log (key_id, action, performed_by, details)
-	VALUES ($1, $2, $3, $4)
-	`
+	if _, err := ks.db.ExecContext(ctx,
+		`INSERT INTO api_key_audit_log (key_id, action, performed_by, details) VALUES ($1, $2, $3, $4)`,
+		keyID, action, performedBy, detailsJSON); err != nil {
+		ks.warnf("audit log: failed to record action=%s: %v", action, err)
+	}
+}
 
-	ks.db.ExecContext(ctx, query, keyID, action, performedBy, detailsJSON)
+func (ks *KeyStoreDB) warnf(format string, args ...interface{}) {
+	if ks.logger != nil {
+		ks.logger.Warnf(format, args...)
+	}
+}
+
+// Ping verifies database connectivity (used by readiness checks).
+func (ks *KeyStoreDB) Ping(ctx context.Context) error {
+	return ks.db.PingContext(ctx)
 }
 
 // Close closes the database connection
@@ -309,113 +375,66 @@ func (ks *KeyStoreDB) Close() error {
 	return ks.db.Close()
 }
 
-// GetAuditLog retrieves audit log entries for a key
-func (ks *KeyStoreDB) GetAuditLog(ctx context.Context, keyStr 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.key = $1
-	ORDER BY al.performed_at DESC
-	LIMIT $2
-	`
-
-	rows, err := ks.db.QueryContext(ctx, query, keyStr, 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()
+// GetAuditLog retrieves audit log entries for a key digest
+func (ks *KeyStoreDB) GetAuditLog(ctx context.Context, keyHash string, limit int) ([]map[string]interface{}, error) {
+	return ks.auditLogQuery(ctx,
+		`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.key_hash = $1
+		 ORDER BY al.performed_at DESC
+		 LIMIT $2`, keyHash, limit)
 }
 
 // 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,
-	)
+	row := ks.db.QueryRowContext(ctx,
+		`SELECT `+keyColumns+` FROM api_keys WHERE name = $1`, name)
 
+	key, err := scanKey(row)
 	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
+	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)
+	return ks.DeactivateKeyByHash(ctx, key.KeyHash, 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
-	`
+	return ks.auditLogQuery(ctx,
+		`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`, name, limit)
+}
 
-	rows, err := ks.db.QueryContext(ctx, query, name, limit)
+func (ks *KeyStoreDB) auditLogQuery(ctx context.Context, query string, ident string, limit int) ([]map[string]interface{}, error) {
+	rows, err := ks.db.QueryContext(ctx, query, ident, limit)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get audit log: %w", err)
 	}
-	defer rows.Close()
+	defer func() { _ = 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 {
+		if err := rows.Scan(&action, &performedBy, &performedAt, &details); err != nil {
 			return nil, err
 		}
 
@@ -429,8 +448,3 @@ func (ks *KeyStoreDB) GetAuditLogByName(ctx context.Context, name string, limit
 
 	return logs, rows.Err()
 }
-
-// Custom errors
-var (
-	ErrKeyNotFound = fmt.Errorf("API key not found")
-)

internal/auth/keystore_hybrid.go

@@ -2,128 +2,180 @@ package auth
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"time"
 )
 
-// HybridKeyStore combines in-memory cache with persistent database backend
-// Write-through strategy: writes go to DB first, then cache is updated
-// This ensures consistency: if DB write fails, cache is not updated
+// keyDatabase is the persistence surface HybridKeyStore needs. *KeyStoreDB
+// implements it; tests substitute a fake.
+type keyDatabase interface {
+	SaveKey(ctx context.Context, key *APIKey, createdBy string) error
+	GetKeyByHash(ctx context.Context, keyHash string) (*APIKey, error)
+	GetKeyByName(ctx context.Context, name string) (*APIKey, error)
+	ListKeys(ctx context.Context, clientID string) ([]*APIKey, error)
+	DeactivateKeyByHash(ctx context.Context, keyHash string, deactivatedBy string) error
+	UpdateLastUsed(ctx context.Context, keyHash string) error
+	LoadAllKeys(ctx context.Context) ([]*APIKey, error)
+	GetAuditLog(ctx context.Context, keyHash string, limit int) ([]map[string]interface{}, error)
+	GetAuditLogByName(ctx context.Context, name string, limit int) ([]map[string]interface{}, error)
+	Close() error
+}
+
+// HybridKeyStore combines an in-memory cache with an optional persistent
+// database backend. Write-through strategy: writes go to the DB first and the
+// cache is only updated after the DB write succeeds. Without a database the
+// store degrades to in-memory-only operation.
 type HybridKeyStore struct {
-	cache *APIKeyStore // In-memory cache for fast lookups
-	db    *KeyStoreDB  // Database backend for persistence
+	cache *APIKeyStore
+	db    keyDatabase // nil when no database is configured
 	mu    sync.RWMutex
 }
 
-// NewHybridKeyStore creates a new hybrid key store
+// NewHybridKeyStore creates a new hybrid key store. db may be nil, in which
+// case all operations are served from the in-memory cache only.
 func NewHybridKeyStore(cache *APIKeyStore, db *KeyStoreDB) *HybridKeyStore {
-	return &HybridKeyStore{
-		cache: cache,
-		db:    db,
+	h := &HybridKeyStore{cache: cache}
+	if db != nil {
+		h.db = db
 	}
+	return h
 }
 
-// InitializeFromDatabase loads all keys from database into cache at startup
-func (h *HybridKeyStore) InitializeFromDatabase(ctx context.Context) error {
+// HasDatabase reports whether a persistent backend is configured.
+func (h *HybridKeyStore) HasDatabase() bool {
+	return h.db != nil
+}
+
+// InitializeFromDatabase loads all active keys from the database into the
+// cache. Call once at startup so previously issued keys survive restarts.
+// Returns the number of keys loaded.
+func (h *HybridKeyStore) InitializeFromDatabase(ctx context.Context) (int, error) {
+	if h.db == nil {
+		return 0, nil
+	}
+
 	keys, err := h.db.LoadAllKeys(ctx)
 	if err != nil {
-		return fmt.Errorf("failed to load keys from database: %w", err)
+		return 0, fmt.Errorf("failed to load keys from database: %w", err)
 	}
 
 	for _, key := range keys {
-		h.cache.keys[key.Key] = key
-		rateLimit := key.RateLimit
-		if rateLimit <= 0 {
-			rateLimit = 100 // Default rate limit
-		}
-		h.cache.rateLimits[key.Key] = &RateLimiter{
-			maxRequests: rateLimit,
-			window:      time.Minute,
-			resetTime:   time.Now().Add(time.Minute),
-			count:       0,
-		}
+		h.cache.RegisterKey(key)
 	}
 
-	return nil
+	return len(keys), nil
 }
 
-// CreateKey generates a new API key and persists it
-// Returns error if database write fails
+// CreateKey generates a new API key, persists it (when a database is
+// configured), and only then registers it in the cache. If the database write
+// fails the key is not usable anywhere.
 func (h *HybridKeyStore) CreateKey(ctx context.Context, clientID string, roles []string, rateLimit int, expiresIn *time.Duration, createdBy string) (*APIKey, error) {
 	h.mu.Lock()
 	defer h.mu.Unlock()
 
-	// Generate random key in memory
-	apiKey, err := h.generateKey(clientID, roles, rateLimit, expiresIn)
+	apiKey, err := generateAPIKey(clientID, roles, rateLimit, expiresIn)
 	if err != nil {
 		return nil, err
 	}
 
-	// Write to database first (consistency)
-	if err := h.db.SaveKey(ctx, apiKey, createdBy); err != nil {
-		return nil, err
-	}
-
-	// Update cache after successful DB write
-	h.cache.keys[apiKey.Key] = apiKey
-	h.cache.rateLimits[apiKey.Key] = &RateLimiter{
-		maxRequests: rateLimit,
-		window:      time.Minute,
-		resetTime:   time.Now().Add(time.Minute),
-		count:       0,
+	if h.db != nil {
+		if err := h.db.SaveKey(ctx, apiKey, createdBy); err != nil {
+			return nil, err
+		}
 	}
 
+	h.cache.RegisterKey(apiKey)
 	return apiKey, nil
 }
 
-// ValidateKey checks if a key is valid
-// Checks cache first for performance, falls back to database if cache miss
-func (h *HybridKeyStore) ValidateKey(keyStr string) (*APIKey, error) {
-	// Check cache first (fast path)
-	h.cache.mu.RLock()
-	key, exists := h.cache.keys[keyStr]
-	h.cache.mu.RUnlock()
-
-	if exists {
-		if h.isKeyValid(key) {
-			return key, nil
-		}
-		return nil, fmt.Errorf("key is inactive or expired")
+// EnsurePersisted upserts an externally created key (e.g. a bootstrap admin
+// key loaded from a Kubernetes secret) into the database, if one is configured.
+func (h *HybridKeyStore) EnsurePersisted(ctx context.Context, apiKey *APIKey, createdBy string) error {
+	if h.db == nil {
+		return nil
 	}
-
-	// Cache miss - this is normal in distributed deployments
-	// Could implement database fallback here if needed:
-	// key, err := h.db.GetKey(context.Background(), keyStr)
-	// But for now, rely on cache being populated at startup
-
-	return nil, fmt.Errorf("API key not found")
+	return h.db.SaveKey(ctx, apiKey, createdBy)
 }
 
-// ListKeys returns all active keys for a client
+// ValidateKey checks if a raw key is valid. The cache is consulted first; on
+// a miss the database is checked and the cache repopulated, so keys created
+// by another instance (or before a restart) still authenticate.
+func (h *HybridKeyStore) ValidateKey(ctx context.Context, keyStr string) (*APIKey, error) {
+	if key, err := h.cache.ValidateKey(keyStr); err == nil {
+		return key, nil
+	} else if !errors.Is(err, ErrInvalidKey) {
+		// Present in cache but inactive/expired — no point hitting the DB.
+		return nil, err
+	}
+
+	if h.db == nil {
+		return nil, ErrKeyNotFound
+	}
+
+	key, err := h.db.GetKeyByHash(ctx, HashKey(keyStr))
+	if err != nil {
+		return nil, err
+	}
+
+	h.cache.RegisterKey(key)
+	return h.cache.ValidateKey(keyStr)
+}
+
+// ListKeys returns all active keys for a client.
 func (h *HybridKeyStore) ListKeys(ctx context.Context, clientID string) ([]*APIKey, error) {
+	if h.db == nil {
+		return h.cache.ListKeys(clientID), nil
+	}
 	return h.db.ListKeys(ctx, clientID)
 }
 
-// DeactivateKey deactivates a key in both cache and database
-func (h *HybridKeyStore) DeactivateKey(ctx context.Context, keyStr string, deactivatedBy string) error {
+// DeactivateKeyByName deactivates a key identified by name in both the
+// database and the cache.
+func (h *HybridKeyStore) DeactivateKeyByName(ctx context.Context, name string, deactivatedBy string) error {
 	h.mu.Lock()
 	defer h.mu.Unlock()
 
-	// Remove from cache first
-	h.cache.mu.Lock()
-	delete(h.cache.keys, keyStr)
-	delete(h.cache.rateLimits, keyStr)
-	h.cache.mu.Unlock()
+	if h.db == nil {
+		return h.deactivateCachedByName(name)
+	}
 
-	// Update database
-	return h.db.DeactivateKey(ctx, keyStr, deactivatedBy)
+	key, err := h.db.GetKeyByName(ctx, name)
+	if err != nil {
+		return err
+	}
+
+	if err := h.db.DeactivateKeyByHash(ctx, key.KeyHash, deactivatedBy); err != nil {
+		return err
+	}
+
+	h.cache.RemoveKeyByHash(key.KeyHash)
+	return nil
 }
 
-// UpdateLastUsed updates the last used timestamp in database
-// Cache is not updated to avoid contention
+// deactivateCachedByName handles revocation when running without a database.
+func (h *HybridKeyStore) deactivateCachedByName(name string) error {
+	h.cache.mu.Lock()
+	defer h.cache.mu.Unlock()
+
+	for hash, key := range h.cache.keys {
+		if key.Name == name {
+			delete(h.cache.keys, hash)
+			delete(h.cache.rateLimits, hash)
+			return nil
+		}
+	}
+	return ErrKeyNotFound
+}
+
+// UpdateLastUsed updates the last used timestamp in the database.
+// Cache is not updated to avoid contention.
 func (h *HybridKeyStore) UpdateLastUsed(ctx context.Context, keyStr string) error {
-	return h.db.UpdateLastUsed(ctx, keyStr)
+	if h.db == nil {
+		return h.cache.UpdateLastUsed(keyStr)
+	}
+	return h.db.UpdateLastUsed(ctx, HashKey(keyStr))
 }
 
 // CheckRateLimit checks if a key has exceeded its rate limit
@@ -131,69 +183,37 @@ func (h *HybridKeyStore) CheckRateLimit(keyStr string) (bool, error) {
 	return h.cache.CheckRateLimit(keyStr)
 }
 
-// GetAuditLog retrieves audit log for a key
+// GetAuditLog retrieves audit log for a raw key
 func (h *HybridKeyStore) GetAuditLog(ctx context.Context, keyStr string, limit int) ([]map[string]interface{}, error) {
-	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
+	if h.db == nil {
+		return nil, fmt.Errorf("audit log requires a database backend")
 	}
-
-	// 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)
+	return h.db.GetAuditLog(ctx, HashKey(keyStr), limit)
 }
 
 // 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) {
+	if h.db == nil {
+		return nil, fmt.Errorf("audit log requires a database backend")
+	}
 	return h.db.GetAuditLogByName(ctx, name, limit)
 }
 
-// Close closes the database connection
+// Close closes the database connection, if any.
 func (h *HybridKeyStore) Close() error {
+	if h.db == nil {
+		return nil
+	}
 	return h.db.Close()
 }
 
-// Helper functions
-
-// generateKey creates an APIKey with cryptographic random bytes
-func (h *HybridKeyStore) generateKey(clientID string, roles []string, rateLimit int, expiresIn *time.Duration) (*APIKey, error) {
-	apiKey, err := h.cache.CreateKey(clientID, roles, rateLimit, expiresIn)
-	if err != nil {
-		return nil, err
-	}
-	return apiKey, nil
-}
-
-// isKeyValid checks if a key is currently valid
-func (h *HybridKeyStore) isKeyValid(key *APIKey) bool {
-	if !key.IsActive {
-		return false
-	}
-
-	if key.ExpiresAt != nil && time.Now().After(*key.ExpiresAt) {
-		return false
-	}
-
-	return true
-}
-
-// SyncCache performs a full cache refresh from database
-// Useful for multi-instance deployments where keys may be created elsewhere
+// SyncCache performs a full cache refresh from the database. Useful for
+// multi-instance deployments where keys may be created or revoked elsewhere.
 func (h *HybridKeyStore) SyncCache(ctx context.Context) error {
+	if h.db == nil {
+		return nil
+	}
+
 	h.mu.Lock()
 	defer h.mu.Unlock()
 
@@ -202,26 +222,14 @@ func (h *HybridKeyStore) SyncCache(ctx context.Context) error {
 		return fmt.Errorf("failed to sync cache: %w", err)
 	}
 
-	// Clear cache
 	h.cache.mu.Lock()
 	h.cache.keys = make(map[string]*APIKey)
 	h.cache.rateLimits = make(map[string]*RateLimiter)
-
-	// Repopulate cache
-	for _, key := range keys {
-		h.cache.keys[key.Key] = key
-		rateLimit := key.RateLimit
-		if rateLimit <= 0 {
-			rateLimit = 100
-		}
-		h.cache.rateLimits[key.Key] = &RateLimiter{
-			maxRequests: rateLimit,
-			window:      time.Minute,
-			resetTime:   time.Now().Add(time.Minute),
-			count:       0,
-		}
-	}
 	h.cache.mu.Unlock()
 
+	for _, key := range keys {
+		h.cache.RegisterKey(key)
+	}
+
 	return nil
 }

internal/auth/keystore_test.go

@@ -0,0 +1,320 @@
+package auth
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/igodwin/notifier/internal/logging"
+)
+
+// fakeKeyDB is an in-memory keyDatabase for exercising HybridKeyStore.
+type fakeKeyDB struct {
+	byHash   map[string]*APIKey
+	saveErr  error
+	saveCnt  int
+	closeCnt int
+}
+
+func newFakeKeyDB() *fakeKeyDB {
+	return &fakeKeyDB{byHash: make(map[string]*APIKey)}
+}
+
+func (f *fakeKeyDB) SaveKey(_ context.Context, key *APIKey, _ string) error {
+	f.saveCnt++
+	if f.saveErr != nil {
+		return f.saveErr
+	}
+	stored := *key
+	stored.Key = ""
+	f.byHash[key.KeyHash] = &stored
+	return nil
+}
+
+func (f *fakeKeyDB) GetKeyByHash(_ context.Context, keyHash string) (*APIKey, error) {
+	key, ok := f.byHash[keyHash]
+	if !ok {
+		return nil, ErrKeyNotFound
+	}
+	return key, nil
+}
+
+func (f *fakeKeyDB) GetKeyByName(_ context.Context, name string) (*APIKey, error) {
+	for _, key := range f.byHash {
+		if key.Name == name {
+			return key, nil
+		}
+	}
+	return nil, ErrKeyNotFound
+}
+
+func (f *fakeKeyDB) ListKeys(_ context.Context, clientID string) ([]*APIKey, error) {
+	var keys []*APIKey
+	for _, key := range f.byHash {
+		if key.ClientID == clientID && key.IsActive {
+			keys = append(keys, key)
+		}
+	}
+	return keys, nil
+}
+
+func (f *fakeKeyDB) DeactivateKeyByHash(_ context.Context, keyHash string, _ string) error {
+	key, ok := f.byHash[keyHash]
+	if !ok {
+		return ErrKeyNotFound
+	}
+	key.IsActive = false
+	return nil
+}
+
+func (f *fakeKeyDB) UpdateLastUsed(_ context.Context, _ string) error { return nil }
+
+func (f *fakeKeyDB) LoadAllKeys(_ context.Context) ([]*APIKey, error) {
+	var keys []*APIKey
+	for _, key := range f.byHash {
+		if key.IsActive {
+			keys = append(keys, key)
+		}
+	}
+	return keys, nil
+}
+
+func (f *fakeKeyDB) GetAuditLog(_ context.Context, _ string, _ int) ([]map[string]interface{}, error) {
+	return nil, nil
+}
+
+func (f *fakeKeyDB) GetAuditLogByName(_ context.Context, _ string, _ int) ([]map[string]interface{}, error) {
+	return nil, nil
+}
+
+func (f *fakeKeyDB) Close() error {
+	f.closeCnt++
+	return nil
+}
+
+func newHybridWithFake(db keyDatabase) (*HybridKeyStore, *APIKeyStore) {
+	cache := NewAPIKeyStore()
+	h := &HybridKeyStore{cache: cache, db: db}
+	return h, cache
+}
+
+func TestAPIKeyStoreDoesNotRetainRawKey(t *testing.T) {
+	store := NewAPIKeyStore()
+	apiKey, err := store.CreateKey("client-a", []string{"notify-email"}, 10, nil)
+	if err != nil {
+		t.Fatalf("CreateKey: %v", err)
+	}
+	if apiKey.Key == "" {
+		t.Fatal("creation response must include the raw key")
+	}
+
+	// The store must be indexed by digest, not raw key, and stored copies
+	// must not carry the raw secret.
+	store.mu.RLock()
+	defer store.mu.RUnlock()
+	if _, ok := store.keys[apiKey.Key]; ok {
+		t.Error("store is keyed by raw key; expected digest")
+	}
+	stored, ok := store.keys[apiKey.KeyHash]
+	if !ok {
+		t.Fatal("store missing entry under key digest")
+	}
+	if stored.Key != "" {
+		t.Error("stored record retains raw key")
+	}
+	if stored.KeyPreview == "" {
+		t.Error("stored record missing preview")
+	}
+}
+
+func TestValidateKeyByRawValue(t *testing.T) {
+	store := NewAPIKeyStore()
+	apiKey, err := store.CreateKey("client-a", []string{"admin"}, 0, nil)
+	if err != nil {
+		t.Fatalf("CreateKey: %v", err)
+	}
+
+	got, err := store.ValidateKey(apiKey.Key)
+	if err != nil {
+		t.Fatalf("ValidateKey with raw key: %v", err)
+	}
+	if got.ClientID != "client-a" {
+		t.Errorf("ClientID = %q, want client-a", got.ClientID)
+	}
+
+	if _, err := store.ValidateKey("nk_bogus"); !errors.Is(err, ErrInvalidKey) {
+		t.Errorf("bogus key error = %v, want ErrInvalidKey", err)
+	}
+
+	if err := store.DeactivateKey(apiKey.Key); err != nil {
+		t.Fatalf("DeactivateKey: %v", err)
+	}
+	if _, err := store.ValidateKey(apiKey.Key); !errors.Is(err, ErrKeyInactive) {
+		t.Errorf("inactive key error = %v, want ErrKeyInactive", err)
+	}
+}
+
+func TestValidateKeyExpiry(t *testing.T) {
+	store := NewAPIKeyStore()
+	expires := -time.Minute // already expired
+	apiKey, err := store.CreateKey("client-a", []string{"admin"}, 0, &expires)
+	if err != nil {
+		t.Fatalf("CreateKey: %v", err)
+	}
+	if _, err := store.ValidateKey(apiKey.Key); !errors.Is(err, ErrKeyExpired) {
+		t.Errorf("expired key error = %v, want ErrKeyExpired", err)
+	}
+}
+
+func TestHybridCreateKeyIsWriteThrough(t *testing.T) {
+	db := newFakeKeyDB()
+	db.saveErr = errors.New("db down")
+	h, cache := newHybridWithFake(db)
+
+	_, err := h.CreateKey(context.Background(), "client-a", []string{"admin"}, 0, nil, "tester")
+	if err == nil {
+		t.Fatal("expected error when DB write fails")
+	}
+
+	// The failed key must not be usable from the cache.
+	cache.mu.RLock()
+	n := len(cache.keys)
+	cache.mu.RUnlock()
+	if n != 0 {
+		t.Errorf("cache has %d key(s) after failed DB write, want 0", n)
+	}
+}
+
+func TestHybridCreateKeySucceedsAndCaches(t *testing.T) {
+	db := newFakeKeyDB()
+	h, _ := newHybridWithFake(db)
+
+	apiKey, err := h.CreateKey(context.Background(), "client-a", []string{"admin"}, 0, nil, "tester")
+	if err != nil {
+		t.Fatalf("CreateKey: %v", err)
+	}
+	if db.saveCnt != 1 {
+		t.Errorf("saveCnt = %d, want 1", db.saveCnt)
+	}
+	if _, err := h.ValidateKey(context.Background(), apiKey.Key); err != nil {
+		t.Errorf("ValidateKey after create: %v", err)
+	}
+}
+
+func TestHybridValidateKeyFallsBackToDatabase(t *testing.T) {
+	db := newFakeKeyDB()
+	h, cache := newHybridWithFake(db)
+
+	// Simulate a key created before a restart: present in DB, absent in cache.
+	apiKey, err := generateAPIKey("client-a", []string{"admin"}, 5, nil)
+	if err != nil {
+		t.Fatalf("generateAPIKey: %v", err)
+	}
+	if err := db.SaveKey(context.Background(), apiKey, "tester"); err != nil {
+		t.Fatalf("SaveKey: %v", err)
+	}
+
+	got, err := h.ValidateKey(context.Background(), apiKey.Key)
+	if err != nil {
+		t.Fatalf("ValidateKey via DB fallback: %v", err)
+	}
+	if got.ClientID != "client-a" {
+		t.Errorf("ClientID = %q, want client-a", got.ClientID)
+	}
+
+	// The fallback must repopulate the cache (including a rate limiter).
+	if _, err := cache.ValidateKey(apiKey.Key); err != nil {
+		t.Errorf("cache not repopulated after fallback: %v", err)
+	}
+	if ok, err := h.CheckRateLimit(apiKey.Key); err != nil || !ok {
+		t.Errorf("CheckRateLimit after fallback: ok=%v err=%v", ok, err)
+	}
+}
+
+func TestHybridInitializeFromDatabase(t *testing.T) {
+	db := newFakeKeyDB()
+	h, cache := newHybridWithFake(db)
+
+	for i := 0; i < 3; i++ {
+		apiKey, err := generateAPIKey("client-a", []string{"admin"}, 0, nil)
+		if err != nil {
+			t.Fatalf("generateAPIKey: %v", err)
+		}
+		if err := db.SaveKey(context.Background(), apiKey, "tester"); err != nil {
+			t.Fatalf("SaveKey: %v", err)
+		}
+	}
+
+	loaded, err := h.InitializeFromDatabase(context.Background())
+	if err != nil {
+		t.Fatalf("InitializeFromDatabase: %v", err)
+	}
+	if loaded != 3 {
+		t.Errorf("loaded = %d, want 3", loaded)
+	}
+	cache.mu.RLock()
+	n := len(cache.keys)
+	cache.mu.RUnlock()
+	if n != 3 {
+		t.Errorf("cache has %d key(s), want 3", n)
+	}
+}
+
+func TestHybridWithoutDatabase(t *testing.T) {
+	h := NewHybridKeyStore(NewAPIKeyStore(), nil)
+
+	if h.HasDatabase() {
+		t.Fatal("HasDatabase() = true with nil db")
+	}
+
+	// All of these must work (or fail cleanly) without panicking.
+	apiKey, err := h.CreateKey(context.Background(), "client-a", []string{"admin"}, 0, nil, "tester")
+	if err != nil {
+		t.Fatalf("CreateKey without db: %v", err)
+	}
+	if _, err := h.ValidateKey(context.Background(), apiKey.Key); err != nil {
+		t.Errorf("ValidateKey without db: %v", err)
+	}
+	if keys, err := h.ListKeys(context.Background(), "client-a"); err != nil || len(keys) != 1 {
+		t.Errorf("ListKeys without db: keys=%d err=%v", len(keys), err)
+	}
+	if err := h.UpdateLastUsed(context.Background(), apiKey.Key); err != nil {
+		t.Errorf("UpdateLastUsed without db: %v", err)
+	}
+	if err := h.DeactivateKeyByName(context.Background(), apiKey.Name, "tester"); err != nil {
+		t.Errorf("DeactivateKeyByName without db: %v", err)
+	}
+	if _, err := h.ValidateKey(context.Background(), apiKey.Key); err == nil {
+		t.Error("key still valid after revocation")
+	}
+	if _, err := h.GetAuditLogByName(context.Background(), apiKey.Name, 10); err == nil {
+		t.Error("expected audit log error without db")
+	}
+	if err := h.Close(); err != nil {
+		t.Errorf("Close without db: %v", err)
+	}
+}
+
+func TestRegisterAdminKeyInMemoryHashesKey(t *testing.T) {
+	store := NewAPIKeyStore()
+	logger, _ := logging.NewFromConfig("error", "stdout")
+
+	raw := "nk_test_admin_key_value"
+	apiKey, err := RegisterAdminKeyInMemory(store, raw, logger)
+	if err != nil {
+		t.Fatalf("RegisterAdminKeyInMemory: %v", err)
+	}
+	if apiKey.KeyHash != HashKey(raw) {
+		t.Error("registered key missing correct digest")
+	}
+	if _, err := store.ValidateKey(raw); err != nil {
+		t.Errorf("ValidateKey after admin registration: %v", err)
+	}
+
+	store.mu.RLock()
+	defer store.mu.RUnlock()
+	if _, ok := store.keys[raw]; ok {
+		t.Error("admin key stored under raw value; expected digest")
+	}
+}

internal/auth/rest_middleware.go

@@ -55,7 +55,7 @@ func (m *RESTAuthMiddleware) Middleware(next http.Handler) http.Handler {
 		}
 
 		// Create auth context and attach to request
-		authCtx := &AuthContext{
+		authCtx := &Context{
 			APIKey:   key,
 			ClientID: key.ClientID,
 			Roles:    key.Roles,

internal/config/config.go

@@ -1,3 +1,6 @@
+// Package config loads and validates the notifier service's configuration
+// (notifiers, queue, auth, retention, and related settings) from files,
+// environment variables, and defaults via viper.
 package config
 
 import (
@@ -27,10 +30,20 @@ type Config struct {
 
 // ServerConfig contains server configuration
 type ServerConfig struct {
-	GRPCPort int    `mapstructure:"grpc_port"`
-	RESTPort int    `mapstructure:"rest_port"`
-	Host     string `mapstructure:"host"`
-	Mode     string `mapstructure:"mode"` // "both", "grpc", "rest"
+	GRPCPort int       `mapstructure:"grpc_port"`
+	RESTPort int       `mapstructure:"rest_port"`
+	Host     string    `mapstructure:"host"`
+	Mode     string    `mapstructure:"mode"` // "both", "grpc", "rest"
+	TLS      TLSConfig `mapstructure:"tls"`
+}
+
+// TLSConfig enables TLS on the REST and gRPC listeners. When disabled the
+// servers speak plaintext, which is only appropriate behind a TLS-terminating
+// gateway or service mesh.
+type TLSConfig struct {
+	Enabled  bool   `mapstructure:"enabled"`
+	CertFile string `mapstructure:"cert_file"`
+	KeyFile  string `mapstructure:"key_file"`
 }
 
 // NotifiersConfig contains configuration for all notifier types
@@ -258,6 +271,12 @@ func (c *Config) Validate() error {
 		return fmt.Errorf("invalid server mode: %s (must be both, grpc, or rest)", c.Server.Mode)
 	}
 
+	if c.Server.TLS.Enabled {
+		if c.Server.TLS.CertFile == "" || c.Server.TLS.KeyFile == "" {
+			return fmt.Errorf("server.tls.enabled requires both cert_file and key_file")
+		}
+	}
+
 	// Validate queue config
 	validQueueTypes := map[string]bool{"local": true, "kafka": true}
 	if !validQueueTypes[c.Queue.Type] {
@@ -265,7 +284,7 @@ func (c *Config) Validate() error {
 	}
 
 	if c.Queue.Type == "kafka" && c.Queue.Kafka == nil {
-		return fmt.Errorf("Kafka queue type selected but no Kafka configuration provided")
+		return fmt.Errorf("kafka queue type selected but no kafka configuration provided")
 	}
 
 	// Validate at least one notifier is configured

internal/config/config_test.go

@@ -10,7 +10,7 @@ func TestSanitizeDatabaseURL(t *testing.T) {
 		input    string
 		expected string
 	}{
-		{
+		{ //nolint:gosec // test fixture URL, not a real credential
 			name:     "PostgreSQL with password",
 			input:    "postgresql://user:password@localhost:5432/dbname",
 			expected: "postgresql://user:***REDACTED***@localhost:5432/dbname",
@@ -20,7 +20,7 @@ func TestSanitizeDatabaseURL(t *testing.T) {
 			input:    "postgresql://user@localhost:5432/dbname",
 			expected: "postgresql://user@localhost:5432/dbname",
 		},
-		{
+		{ //nolint:gosec // test fixture URL, not a real credential
 			name:     "MySQL with special characters in password",
 			input:    "mysql://root:SuperSecret123!@db.example.com:3306/mydb",
 			expected: "mysql://root:***REDACTED***@db.example.com:3306/mydb",
@@ -40,12 +40,12 @@ func TestSanitizeDatabaseURL(t *testing.T) {
 			input:    "postgresql://localhost:5432/dbname",
 			expected: "postgresql://localhost:5432/dbname",
 		},
-		{
+		{ //nolint:gosec // test fixture URL, not a real credential
 			name:     "PostgreSQL with password containing colons",
 			input:    "postgresql://user:pass:word@localhost:5432/dbname",
 			expected: "postgresql://user:***REDACTED***@localhost:5432/dbname",
 		},
-		{
+		{ //nolint:gosec // test fixture URL, not a real credential
 			name:     "PostgreSQL with complex hostname and port",
 			input:    "postgresql://admin:p@ssw0rd!@db-prod.example.com:5432/production",
 			expected: "postgresql://admin:***REDACTED***@db-prod.example.com:5432/production",

internal/domain/notification.go

@@ -1,12 +1,24 @@
+// Package domain contains the core types shared across the notifier
+// service - notifications, queueing primitives, and the notifier
+// interfaces that provider implementations satisfy.
 package domain
 
 import (
+	"errors"
 	"time"
 )
 
+// Sentinel errors for notification lookup and state transitions.
+// Match with errors.Is.
+var (
+	ErrNotificationNotFound    = errors.New("notification not found")
+	ErrNotificationAlreadySent = errors.New("notification already sent")
+)
+
 // Priority defines the urgency level of a notification
 type Priority int
 
+// Priority levels, in increasing order of urgency.
 const (
 	PriorityLow Priority = iota
 	PriorityNormal
@@ -17,6 +29,7 @@ const (
 // NotificationType defines the channel through which to send the notification
 type NotificationType string
 
+// Supported notification channels.
 const (
 	TypeEmail  NotificationType = "email"
 	TypeSlack  NotificationType = "slack"
@@ -27,6 +40,7 @@ const (
 // ContentType defines the format of the notification body
 type ContentType string
 
+// Supported body content types.
 const (
 	ContentTypeText ContentType = "text"
 	ContentTypeHTML ContentType = "html"
@@ -35,6 +49,7 @@ const (
 // NotificationStatus represents the current state of a notification
 type NotificationStatus string
 
+// Notification lifecycle states.
 const (
 	StatusPending    NotificationStatus = "pending"
 	StatusQueued     NotificationStatus = "queued"
@@ -107,6 +122,49 @@ type Notification struct {
 
 	// LastError stores the most recent error message if failed
 	LastError string `json:"last_error,omitempty"`
+
+	// ClientID identifies the API client that submitted the notification, used
+	// to scope visibility of notifications to the tenant that created them.
+	// Empty when auth is disabled.
+	ClientID string `json:"client_id,omitempty"`
+}
+
+// Clone returns a copy of the notification that shares no mutable state with
+// the original. The service layer stores and hands out clones so that
+// concurrent readers (REST/gRPC handlers) and writers (queue workers) never
+// observe or race on the same underlying slices, maps, or pointer fields.
+func (n *Notification) Clone() *Notification {
+	if n == nil {
+		return nil
+	}
+
+	clone := *n
+
+	if n.Recipients != nil {
+		clone.Recipients = append([]string(nil), n.Recipients...)
+	}
+	if n.CC != nil {
+		clone.CC = append([]string(nil), n.CC...)
+	}
+	if n.BCC != nil {
+		clone.BCC = append([]string(nil), n.BCC...)
+	}
+	if n.Metadata != nil {
+		clone.Metadata = make(map[string]interface{}, len(n.Metadata))
+		for k, v := range n.Metadata {
+			clone.Metadata[k] = v
+		}
+	}
+	if n.ScheduledFor != nil {
+		scheduledFor := *n.ScheduledFor
+		clone.ScheduledFor = &scheduledFor
+	}
+	if n.SentAt != nil {
+		sentAt := *n.SentAt
+		clone.SentAt = &sentAt
+	}
+
+	return &clone
 }
 
 // NotificationResult represents the outcome of sending a notification

internal/logging/logger.go

@@ -1,22 +1,29 @@
+// Package logging provides structured logging backed by log/slog, with UTC
+// RFC3339 timestamps and a level-gated API compatible with the previous
+// *log.Logger-based implementation.
 package logging
 
 import (
 	"fmt"
 	"io"
-	"log"
+	"log/slog"
 	"os"
 	"time"
 )
 
-// Logger provides structured logging with ISO 8601 timestamps
+// Logger provides structured logging backed by log/slog, with UTC RFC3339
+// timestamps and a level-gated API compatible with the previous
+// *log.Logger-based implementation.
 type Logger struct {
-	*log.Logger
-	level LogLevel
+	slogger *slog.Logger
+	level   LogLevel
 }
 
 // LogLevel represents the logging level
 type LogLevel int
 
+// Logging levels, in increasing order of severity. DebugLevel is the most
+// verbose and ErrorLevel the least.
 const (
 	DebugLevel LogLevel = iota
 	InfoLevel
@@ -24,20 +31,73 @@ const (
 	ErrorLevel
 )
 
-// New creates a new logger with ISO 8601 timestamp format
+// toSlogLevel maps our LogLevel to the equivalent slog.Level.
+func (l LogLevel) toSlogLevel() slog.Level {
+	switch l {
+	case DebugLevel:
+		return slog.LevelDebug
+	case InfoLevel:
+		return slog.LevelInfo
+	case WarnLevel:
+		return slog.LevelWarn
+	case ErrorLevel:
+		return slog.LevelError
+	default:
+		return slog.LevelInfo
+	}
+}
+
+// replaceAttr normalizes the slog time attribute to a UTC RFC3339 timestamp
+// so log output has a stable, predictable format regardless of handler.
+func replaceAttr(_ []string, a slog.Attr) slog.Attr {
+	if a.Key == slog.TimeKey {
+		if t, ok := a.Value.Any().(time.Time); ok {
+			a.Value = slog.StringValue(t.UTC().Format(time.RFC3339))
+		}
+	}
+	return a
+}
+
+// newHandler builds a slog.Handler for the given format ("json" or "text",
+// with "json" as the default), level, and output writer.
+func newHandler(format string, level LogLevel, output io.Writer) slog.Handler {
+	opts := &slog.HandlerOptions{
+		Level:       level.toSlogLevel(),
+		ReplaceAttr: replaceAttr,
+	}
+
+	switch format {
+	case "text":
+		return slog.NewTextHandler(output, opts)
+	default:
+		return slog.NewJSONHandler(output, opts)
+	}
+}
+
+// New creates a new logger using the text format with the given level and
+// output writer.
 func New(level LogLevel, output io.Writer) *Logger {
 	if output == nil {
 		output = os.Stdout
 	}
 
 	return &Logger{
-		Logger: log.New(output, "", 0), // No flags, we'll format ourselves
-		level:  level,
+		slogger: slog.New(newHandler("text", level, output)),
+		level:   level,
 	}
 }
 
-// NewFromConfig creates a logger from configuration
+// NewFromConfig creates a logger from configuration using the json format,
+// matching the documented default for logging.format.
 func NewFromConfig(levelStr string, outputPath string) (*Logger, error) {
+	return NewFromOptions(levelStr, "json", outputPath)
+}
+
+// NewFromOptions creates a logger from configuration with an explicit
+// format ("json", "text", or "" which defaults to "json"). outputPath may be
+// "stdout", "stderr", "" (defaults to stdout), or a file path, which is
+// opened for append (creating it if necessary).
+func NewFromOptions(levelStr string, format string, outputPath string) (*Logger, error) {
 	level := parseLevel(levelStr)
 
 	var output io.Writer
@@ -47,92 +107,73 @@ func NewFromConfig(levelStr string, outputPath string) (*Logger, error) {
 	case "stderr":
 		output = os.Stderr
 	default:
-		file, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
+		file, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) //nolint:gosec // outputPath is operator-configured (logging.output), not user-controlled input
 		if err != nil {
 			return nil, err
 		}
 		output = file
 	}
 
-	return New(level, output), nil
+	return &Logger{
+		slogger: slog.New(newHandler(format, level, output)),
+		level:   level,
+	}, nil
 }
 
-// formatMessage formats a log message with ISO 8601 timestamp
-func (l *Logger) formatMessage(level string, msg string) string {
-	timestamp := time.Now().UTC().Format(time.RFC3339)
-	return timestamp + " [" + level + "] " + msg
+// Slog exposes the underlying *slog.Logger for structured call sites.
+func (l *Logger) Slog() *slog.Logger {
+	return l.slogger
 }
 
 // Debug logs a debug message
 func (l *Logger) Debug(msg string) {
-	if l.level <= DebugLevel {
-		l.Logger.Println(l.formatMessage("DEBUG", msg))
-	}
+	l.slogger.Debug(msg)
 }
 
 // Debugf logs a formatted debug message
 func (l *Logger) Debugf(format string, args ...interface{}) {
-	if l.level <= DebugLevel {
-		msg := sprintf(format, args...)
-		l.Logger.Println(l.formatMessage("DEBUG", msg))
-	}
+	l.slogger.Debug(fmt.Sprintf(format, args...))
 }
 
 // Info logs an info message
 func (l *Logger) Info(msg string) {
-	if l.level <= InfoLevel {
-		l.Logger.Println(l.formatMessage("INFO", msg))
-	}
+	l.slogger.Info(msg)
 }
 
 // Infof logs a formatted info message
 func (l *Logger) Infof(format string, args ...interface{}) {
-	if l.level <= InfoLevel {
-		msg := sprintf(format, args...)
-		l.Logger.Println(l.formatMessage("INFO", msg))
-	}
+	l.slogger.Info(fmt.Sprintf(format, args...))
 }
 
 // Warn logs a warning message
 func (l *Logger) Warn(msg string) {
-	if l.level <= WarnLevel {
-		l.Logger.Println(l.formatMessage("WARN", msg))
-	}
+	l.slogger.Warn(msg)
 }
 
 // Warnf logs a formatted warning message
 func (l *Logger) Warnf(format string, args ...interface{}) {
-	if l.level <= WarnLevel {
-		msg := sprintf(format, args...)
-		l.Logger.Println(l.formatMessage("WARN", msg))
-	}
+	l.slogger.Warn(fmt.Sprintf(format, args...))
 }
 
 // Error logs an error message
 func (l *Logger) Error(msg string) {
-	if l.level <= ErrorLevel {
-		l.Logger.Println(l.formatMessage("ERROR", msg))
-	}
+	l.slogger.Error(msg)
 }
 
 // Errorf logs a formatted error message
 func (l *Logger) Errorf(format string, args ...interface{}) {
-	if l.level <= ErrorLevel {
-		msg := sprintf(format, args...)
-		l.Logger.Println(l.formatMessage("ERROR", msg))
-	}
+	l.slogger.Error(fmt.Sprintf(format, args...))
 }
 
-// Fatal logs a fatal message and exits
+// Fatal logs a fatal message at error level and exits
 func (l *Logger) Fatal(msg string) {
-	l.Logger.Println(l.formatMessage("FATAL", msg))
+	l.slogger.Error(msg)
 	os.Exit(1)
 }
 
-// Fatalf logs a formatted fatal message and exits
+// Fatalf logs a formatted fatal message at error level and exits
 func (l *Logger) Fatalf(format string, args ...interface{}) {
-	msg := sprintf(format, args...)
-	l.Logger.Println(l.formatMessage("FATAL", msg))
+	l.slogger.Error(fmt.Sprintf(format, args...))
 	os.Exit(1)
 }
 
@@ -151,8 +192,3 @@ func parseLevel(levelStr string) LogLevel {
 		return InfoLevel
 	}
 }
-
-// sprintf is a helper using fmt
-func sprintf(format string, args ...interface{}) string {
-	return fmt.Sprintf(format, args...)
-}

internal/logging/logger_test.go

@@ -0,0 +1,221 @@
+package logging
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestNew_TextFormat(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "text.log")
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("unexpected error opening file: %v", err)
+	}
+	defer func() { _ = file.Close() }()
+
+	logger := New(InfoLevel, file)
+	logger.Info("hello world")
+
+	data, err := os.ReadFile(path) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read log file: %v", err)
+	}
+	out := string(data)
+
+	if !strings.Contains(out, "hello world") {
+		t.Fatalf("expected output to contain message, got: %q", out)
+	}
+	if !strings.Contains(out, "level=INFO") {
+		t.Fatalf("expected text output to contain level=INFO, got: %q", out)
+	}
+	if json.Valid(data) {
+		t.Fatalf("expected non-JSON text output, got JSON: %q", out)
+	}
+}
+
+func TestNewFromConfig_JSONOutput(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "json.log")
+
+	logger, err := NewFromConfig("info", path)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	logger.Info("structured message")
+
+	data, err := os.ReadFile(path) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read log file: %v", err)
+	}
+
+	var payload map[string]interface{}
+	if err := json.Unmarshal(data, &payload); err != nil {
+		t.Fatalf("expected valid JSON output, got error %v; output: %q", err, string(data))
+	}
+
+	if payload["msg"] != "structured message" {
+		t.Fatalf("expected msg field to equal message, got: %v", payload["msg"])
+	}
+	if payload["level"] != "INFO" {
+		t.Fatalf("expected level field INFO, got: %v", payload["level"])
+	}
+	timeVal, ok := payload["time"].(string)
+	if !ok {
+		t.Fatalf("expected time field to be a string, got: %v", payload["time"])
+	}
+	if _, err := time.Parse(time.RFC3339, timeVal); err != nil {
+		t.Fatalf("expected time field to be RFC3339, got %q: %v", timeVal, err)
+	}
+}
+
+func TestLevelFiltering_DebugSuppressedAtInfo(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "level.log")
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("unexpected error opening file: %v", err)
+	}
+	defer func() { _ = file.Close() }()
+
+	logger := New(InfoLevel, file)
+	logger.Debug("should not appear")
+	logger.Info("should appear")
+
+	data, err := os.ReadFile(path) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read log file: %v", err)
+	}
+	out := string(data)
+
+	if strings.Contains(out, "should not appear") {
+		t.Fatalf("expected debug message to be suppressed, got: %q", out)
+	}
+	if !strings.Contains(out, "should appear") {
+		t.Fatalf("expected info message to be present, got: %q", out)
+	}
+}
+
+func TestNewFromOptions_FileOutput(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "out.log")
+
+	logger, err := NewFromOptions("info", "json", path)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	logger.Info("file message")
+
+	data, err := os.ReadFile(path) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read log file: %v", err)
+	}
+
+	var payload map[string]interface{}
+	if err := json.Unmarshal(data, &payload); err != nil {
+		t.Fatalf("expected valid JSON in file, got error %v; contents: %q", err, string(data))
+	}
+	if payload["msg"] != "file message" {
+		t.Fatalf("expected msg field, got: %v", payload["msg"])
+	}
+}
+
+func TestNewFromOptions_FormatSelection(t *testing.T) {
+	dir := t.TempDir()
+
+	jsonPath := filepath.Join(dir, "json.log")
+	jsonLogger, err := NewFromOptions("info", "json", jsonPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	jsonLogger.Info("json message")
+
+	jsonData, err := os.ReadFile(jsonPath) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read json log file: %v", err)
+	}
+	if !json.Valid(jsonData) {
+		t.Fatalf("expected valid JSON, got: %q", string(jsonData))
+	}
+
+	textPath := filepath.Join(dir, "text.log")
+	textLogger, err := NewFromOptions("info", "text", textPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	textLogger.Info("text message")
+
+	textData, err := os.ReadFile(textPath) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read text log file: %v", err)
+	}
+	if json.Valid(textData) {
+		t.Fatalf("expected non-JSON text output, got: %q", string(textData))
+	}
+	if !strings.Contains(string(textData), `msg="text message"`) {
+		t.Fatalf("expected text output to contain msg attribute, got: %q", string(textData))
+	}
+
+	// Default format ("" -> json) via NewFromOptions.
+	defaultPath := filepath.Join(dir, "default.log")
+	defaultLogger, err := NewFromOptions("info", "", defaultPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	defaultLogger.Info("default message")
+
+	defaultData, err := os.ReadFile(defaultPath) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read default log file: %v", err)
+	}
+	if !json.Valid(defaultData) {
+		t.Fatalf("expected empty format to default to JSON, got: %q", string(defaultData))
+	}
+
+	// NewFromConfig should also default to JSON.
+	configPath := filepath.Join(dir, "config.log")
+	configLogger, err := NewFromConfig("info", configPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	configLogger.Info("config message")
+
+	configData, err := os.ReadFile(configPath) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read config log file: %v", err)
+	}
+	if !json.Valid(configData) {
+		t.Fatalf("expected NewFromConfig to default to JSON format, got: %q", string(configData))
+	}
+}
+
+func TestSlogAccessor(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "slog.log")
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("unexpected error opening file: %v", err)
+	}
+	defer func() { _ = file.Close() }()
+
+	logger := New(InfoLevel, file)
+	if logger.Slog() == nil {
+		t.Fatal("expected Slog() to return a non-nil *slog.Logger")
+	}
+
+	logger.Slog().Info("via slog accessor")
+
+	data, err := os.ReadFile(path) //nolint:gosec // path is from t.TempDir()
+	if err != nil {
+		t.Fatalf("failed to read log file: %v", err)
+	}
+	if !strings.Contains(string(data), "via slog accessor") {
+		t.Fatalf("expected message logged via Slog() accessor, got: %q", string(data))
+	}
+}

internal/metrics/metrics.go

@@ -0,0 +1,160 @@
+// Package metrics exposes Prometheus metrics for the notifier service.
+//
+// Notification totals are sampled from the service's own stats rather than
+// instrumented inline, so the service layer stays metrics-agnostic; the
+// sampling interval bounds staleness at a few seconds, which is fine for
+// counters scraped every 15-60s.
+package metrics
+
+import (
+	"context"
+	"net/http"
+	"strconv"
+	"time"
+
+	"github.com/igodwin/notifier/internal/domain"
+	"github.com/igodwin/notifier/internal/logging"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/collectors"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
+)
+
+// Collector owns the notifier metrics and the sampling loop.
+type Collector struct {
+	registry *prometheus.Registry
+
+	notificationsByStatus *prometheus.GaugeVec
+	notificationsByType   *prometheus.GaugeVec
+	queueDepth            prometheus.Gauge
+
+	httpRequests *prometheus.CounterVec
+	httpDuration *prometheus.HistogramVec
+
+	service domain.NotificationService
+	queue   domain.Queue
+	logger  *logging.Logger
+}
+
+// NewCollector creates and registers the notifier metrics.
+func NewCollector(service domain.NotificationService, queue domain.Queue, logger *logging.Logger) *Collector {
+	registry := prometheus.NewRegistry()
+
+	c := &Collector{
+		registry: registry,
+		service:  service,
+		queue:    queue,
+		logger:   logger,
+		notificationsByStatus: prometheus.NewGaugeVec(prometheus.GaugeOpts{
+			Name: "notifier_notifications",
+			Help: "Number of tracked notifications by status.",
+		}, []string{"status"}),
+		notificationsByType: prometheus.NewGaugeVec(prometheus.GaugeOpts{
+			Name: "notifier_notifications_by_type",
+			Help: "Number of tracked notifications by notification type.",
+		}, []string{"type"}),
+		queueDepth: prometheus.NewGauge(prometheus.GaugeOpts{
+			Name: "notifier_queue_depth",
+			Help: "Number of messages currently waiting in the queue.",
+		}),
+		httpRequests: prometheus.NewCounterVec(prometheus.CounterOpts{
+			Name: "notifier_http_requests_total",
+			Help: "REST API requests by method, path pattern, and status code.",
+		}, []string{"method", "path", "code"}),
+		httpDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{
+			Name:    "notifier_http_request_duration_seconds",
+			Help:    "REST API request duration.",
+			Buckets: prometheus.DefBuckets,
+		}, []string{"method", "path"}),
+	}
+
+	registry.MustRegister(
+		c.notificationsByStatus,
+		c.notificationsByType,
+		c.queueDepth,
+		c.httpRequests,
+		c.httpDuration,
+		collectors.NewGoCollector(),
+		collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}),
+	)
+
+	return c
+}
+
+// Handler returns the /metrics HTTP handler.
+func (c *Collector) Handler() http.Handler {
+	return promhttp.HandlerFor(c.registry, promhttp.HandlerOpts{})
+}
+
+// Run samples service stats until ctx is cancelled.
+func (c *Collector) Run(ctx context.Context, interval time.Duration) {
+	if interval <= 0 {
+		interval = 10 * time.Second
+	}
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-ticker.C:
+			c.sample(ctx)
+		}
+	}
+}
+
+func (c *Collector) sample(ctx context.Context) {
+	sampleCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+
+	if stats, err := c.service.GetStats(sampleCtx); err == nil {
+		c.notificationsByStatus.Reset()
+		for status, count := range stats.ByStatus {
+			c.notificationsByStatus.WithLabelValues(status).Set(float64(count))
+		}
+		c.notificationsByType.Reset()
+		for typ, count := range stats.ByType {
+			c.notificationsByType.WithLabelValues(typ).Set(float64(count))
+		}
+	} else if c.logger != nil {
+		c.logger.Debugf("metrics: failed to sample service stats: %v", err)
+	}
+
+	if size, err := c.queue.Size(sampleCtx); err == nil {
+		c.queueDepth.Set(float64(size))
+	}
+}
+
+// InstrumentHTTP wraps an HTTP handler with request count and duration
+// metrics. The path label uses the route pattern when available (via
+// gorilla/mux CurrentRoute) to keep cardinality bounded.
+func (c *Collector) InstrumentHTTP(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		start := time.Now()
+		sw := &statusWriter{ResponseWriter: w, status: http.StatusOK}
+		next.ServeHTTP(sw, r)
+
+		path := routePattern(r)
+		c.httpRequests.WithLabelValues(r.Method, path, strconv.Itoa(sw.status)).Inc()
+		c.httpDuration.WithLabelValues(r.Method, path).Observe(time.Since(start).Seconds())
+	})
+}
+
+type statusWriter struct {
+	http.ResponseWriter
+	status int
+}
+
+func (w *statusWriter) WriteHeader(status int) {
+	w.status = status
+	w.ResponseWriter.WriteHeader(status)
+}
+
+// routePattern extracts the mux route template (e.g. /api/v1/notifications/{id})
+// so metrics don't explode into one series per notification ID.
+func routePattern(r *http.Request) string {
+	if route := currentRoute(r); route != "" {
+		return route
+	}
+	return "unmatched"
+}

internal/metrics/mux.go

@@ -0,0 +1,20 @@
+package metrics
+
+import (
+	"net/http"
+
+	"github.com/gorilla/mux"
+)
+
+// currentRoute returns the gorilla/mux path template for the request, if the
+// request was matched by a mux router.
+func currentRoute(r *http.Request) string {
+	route := mux.CurrentRoute(r)
+	if route == nil {
+		return ""
+	}
+	if tmpl, err := route.GetPathTemplate(); err == nil {
+		return tmpl
+	}
+	return ""
+}

internal/notifier/notifier.go

@@ -1,3 +1,6 @@
+// Package notifier defines the notifier provider interfaces and a factory
+// for constructing and looking up configured notifier instances by type
+// and account.
 package notifier
 
 import (

internal/notifier/ntfy.go

@@ -127,7 +127,7 @@ func validateCACertPath(caCertPath string) error {
 	}
 
 	// Try to read and parse the certificate
-	certData, err := os.ReadFile(caCertPath)
+	certData, err := os.ReadFile(caCertPath) //nolint:gosec // caCertPath is operator-configured (ntfy CA cert path), not user-controlled input
 	if err != nil {
 		return fmt.Errorf("failed to read CA certificate file: %w", err)
 	}
@@ -270,8 +270,8 @@ func (n *NtfyNotifier) Send(ctx context.Context, notification *domain.Notificati
 					if body, ok := actionMap["body"].(string); ok {
 						ntfyAct.Body = body
 					}
-					if clear, ok := actionMap["clear"].(bool); ok {
-						ntfyAct.Clear = clear
+					if clearAction, ok := actionMap["clear"].(bool); ok {
+						ntfyAct.Clear = clearAction
 					}
 					req.Actions = append(req.Actions, ntfyAct)
 				}
@@ -302,7 +302,7 @@ func (n *NtfyNotifier) Send(ctx context.Context, notification *domain.Notificati
 
 // sendToTopic sends a notification to a specific ntfy topic
 func (n *NtfyNotifier) sendToTopic(ctx context.Context, req *ntfyRequest) error {
-	url := fmt.Sprintf("%s", n.config.ServerURL)
+	url := n.config.ServerURL
 
 	jsonData, err := json.Marshal(req)
 	if err != nil {
@@ -327,7 +327,7 @@ func (n *NtfyNotifier) sendToTopic(ctx context.Context, req *ntfyRequest) error
 	if err != nil {
 		return fmt.Errorf("failed to send ntfy notification: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		return fmt.Errorf("ntfy server returned status: %d", resp.StatusCode)

internal/notifier/ntfy_tls_test.go

@@ -48,7 +48,7 @@ func TestNewNtfyNotifierWithDefaultCA(t *testing.T) {
 func TestNewNtfyNotifierWithCustomCA(t *testing.T) {
 	// Create a temporary CA certificate file
 	certPath := createTempCACert(t)
-	defer os.Remove(certPath)
+	defer func() { _ = os.Remove(certPath) }()
 
 	config := &NtfyConfig{
 		ServerURL:    "https://self-signed.example.com",
@@ -95,13 +95,13 @@ func TestValidateCACertPathInvalidFormat(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to create temp file: %v", err)
 	}
-	defer os.Remove(tmpFile.Name())
+	defer func() { _ = os.Remove(tmpFile.Name()) }()
 
 	// Write invalid content (not PEM format)
 	if _, err := tmpFile.WriteString("This is not a valid certificate"); err != nil {
 		t.Fatalf("Failed to write to temp file: %v", err)
 	}
-	tmpFile.Close()
+	_ = tmpFile.Close()
 
 	config := &NtfyConfig{
 		ServerURL:    "https://ntfy.sh",
@@ -128,7 +128,7 @@ func TestValidateCACertPathIsDirectory(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to create temp directory: %v", err)
 	}
-	defer os.RemoveAll(tmpDir)
+	defer func() { _ = os.RemoveAll(tmpDir) }()
 
 	config := &NtfyConfig{
 		ServerURL:    "https://ntfy.sh",
@@ -210,7 +210,7 @@ func TestTLSConfigNeverSkipsVerification(t *testing.T) {
 func TestCustomCACertLoading(t *testing.T) {
 	// Create a temporary CA certificate
 	certPath := createTempCACert(t)
-	defer os.Remove(certPath)
+	defer func() { _ = os.Remove(certPath) }()
 
 	config := &NtfyConfig{
 		ServerURL:    "https://self-signed.example.com",
@@ -257,8 +257,8 @@ func TestEmptyCertFileError(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to create temp file: %v", err)
 	}
-	defer os.Remove(tmpFile.Name())
-	tmpFile.Close()
+	defer func() { _ = os.Remove(tmpFile.Name()) }()
+	_ = tmpFile.Close()
 
 	err = validateCACertPath(tmpFile.Name())
 	if err == nil {
@@ -274,7 +274,7 @@ func createTempCACert(t *testing.T) string {
 	if err != nil {
 		t.Fatalf("Failed to create temp file: %v", err)
 	}
-	defer tmpFile.Close()
+	defer func() { _ = tmpFile.Close() }()
 
 	// Generate a self-signed certificate for testing
 	certPEM := generateSelfSignedCert(t)

internal/notifier/slack.go

@@ -55,12 +55,12 @@ type slackTextBlock struct {
 // NewSlackNotifier creates a new Slack notifier
 func NewSlackNotifier(config *SlackConfig) (*SlackNotifier, error) {
 	if config == nil {
-		return nil, fmt.Errorf("Slack config is required")
+		return nil, fmt.Errorf("slack config is required")
 	}
 
 	// Either webhook URL or token is required
 	if config.WebhookURL == "" && config.Token == "" && len(config.Webhooks) == 0 {
-		return nil, fmt.Errorf("Slack webhook URL, token, or channel webhooks are required")
+		return nil, fmt.Errorf("slack webhook URL, token, or channel webhooks are required")
 	}
 
 	return &SlackNotifier{
@@ -201,10 +201,10 @@ func (s *SlackNotifier) sendToSlack(ctx context.Context, webhookURL string, msg
 	if err != nil {
 		return fmt.Errorf("failed to send Slack notification: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return fmt.Errorf("Slack API returned status: %d", resp.StatusCode)
+		return fmt.Errorf("slack API returned status: %d", resp.StatusCode)
 	}
 
 	return nil

internal/notifier/smtp.go

@@ -3,9 +3,12 @@ package notifier
 import (
 	"context"
 	"crypto/rand"
+	"crypto/tls"
 	"encoding/hex"
 	"fmt"
 	"html"
+	"mime"
+	"net/mail"
 	"net/smtp"
 	"regexp"
 	"strings"
@@ -75,15 +78,16 @@ func (s *SMTPNotifier) Send(ctx context.Context, notification *domain.Notificati
 	allRecipients = append(allRecipients, notification.CC...)
 	allRecipients = append(allRecipients, notification.BCC...)
 
-	// Validate email recipients
+	// Validate email recipients: reject header-injection attempts (CR/LF) outright and
+	// otherwise require a syntactically valid RFC 5322 address.
 	for _, recipient := range allRecipients {
-		if !strings.Contains(recipient, "@") {
+		if err := validateRecipient(recipient); err != nil {
 			return &domain.NotificationResult{
 				NotificationID: notification.ID,
 				Success:        false,
-				Error:          fmt.Sprintf("invalid email address: %s", recipient),
+				Error:          err.Error(),
 				SentAt:         time.Now(),
-			}, fmt.Errorf("invalid email address: %s", recipient)
+			}, fmt.Errorf("invalid recipient: %w", err)
 		}
 	}
 
@@ -95,7 +99,7 @@ func (s *SMTPNotifier) Send(ctx context.Context, notification *domain.Notificati
 	auth := smtp.PlainAuth("", s.config.Username, s.config.Password, s.config.Host)
 
 	// smtp.SendMail needs all recipients (To, CC, BCC) for actual delivery
-	err := smtp.SendMail(addr, auth, s.config.From, allRecipients, []byte(message))
+	err := s.sendMail(addr, auth, s.config.From, allRecipients, []byte(message))
 	if err != nil {
 		return &domain.NotificationResult{
 			NotificationID: notification.ID,
@@ -118,31 +122,125 @@ func (s *SMTPNotifier) Send(ctx context.Context, notification *domain.Notificati
 	}, nil
 }
 
+// sendMail dispatches the message using the transport appropriate for the configured port.
+// When UseTLS is enabled and the port is the implicit-TLS SMTPS port (465), the connection is
+// wrapped in TLS from the very first byte. Otherwise smtp.SendMail is used, which opportunistically
+// upgrades the plaintext connection to STARTTLS if the server advertises support for it, but
+// will silently fall back to a plaintext session if it does not.
+func (s *SMTPNotifier) sendMail(addr string, auth smtp.Auth, from string, recipients []string, msg []byte) error {
+	if s.config.UseTLS && s.config.Port == 465 {
+		return sendMailImplicitTLS(addr, s.config.Host, auth, from, recipients, msg)
+	}
+
+	return smtp.SendMail(addr, auth, from, recipients, msg)
+}
+
+// sendMailImplicitTLS sends an email over an implicit TLS (SMTPS) connection, verifying the
+// server certificate against serverName. The certificate is always verified (no
+// InsecureSkipVerify escape hatch is provided).
+func sendMailImplicitTLS(addr, serverName string, auth smtp.Auth, from string, recipients []string, msg []byte) error {
+	tlsConfig := &tls.Config{
+		ServerName: serverName,
+		MinVersion: tls.VersionTLS12,
+	}
+
+	conn, err := tls.Dial("tcp", addr, tlsConfig)
+	if err != nil {
+		return fmt.Errorf("failed to establish TLS connection to %s: %w", addr, err)
+	}
+	defer func() { _ = conn.Close() }()
+
+	client, err := smtp.NewClient(conn, serverName)
+	if err != nil {
+		return fmt.Errorf("failed to create SMTP client: %w", err)
+	}
+	defer func() { _ = client.Close() }()
+
+	if auth != nil {
+		if ok, _ := client.Extension("AUTH"); ok {
+			if err := client.Auth(auth); err != nil {
+				return fmt.Errorf("SMTP authentication failed: %w", err)
+			}
+		}
+	}
+
+	if err := client.Mail(from); err != nil {
+		return fmt.Errorf("failed to set sender %q: %w", from, err)
+	}
+
+	for _, recipient := range recipients {
+		if err := client.Rcpt(recipient); err != nil {
+			return fmt.Errorf("failed to add recipient %q: %w", recipient, err)
+		}
+	}
+
+	writer, err := client.Data()
+	if err != nil {
+		return fmt.Errorf("failed to open message data stream: %w", err)
+	}
+
+	if _, err := writer.Write(msg); err != nil {
+		return fmt.Errorf("failed to write message body: %w", err)
+	}
+
+	if err := writer.Close(); err != nil {
+		return fmt.Errorf("failed to finalize message: %w", err)
+	}
+
+	return client.Quit()
+}
+
+// validateRecipient ensures a recipient address cannot be used to inject additional SMTP
+// headers and is a syntactically valid RFC 5322 address. CR/LF are rejected outright (rather
+// than relying on mail.ParseAddress to catch them) so the failure reason is unambiguous.
+func validateRecipient(recipient string) error {
+	if strings.ContainsAny(recipient, "\r\n") {
+		return fmt.Errorf("recipient %q contains illegal CR/LF characters", recipient)
+	}
+
+	if _, err := mail.ParseAddress(recipient); err != nil {
+		return fmt.Errorf("recipient %q is not a valid email address: %w", recipient, err)
+	}
+
+	return nil
+}
+
+// encodeHeaderValue produces a header-safe representation of an untrusted value such as a
+// Subject or display name. mime.QEncoding.Encode leaves plain ASCII untouched but RFC
+// 2047-encodes anything containing control characters (including bare CR/LF) or non-ASCII
+// runes, so injected header/line breaks cannot survive into the raw message.
+func encodeHeaderValue(value string) string {
+	return mime.QEncoding.Encode("utf-8", value)
+}
+
 // buildMessage constructs the email message with headers
 func (s *SMTPNotifier) buildMessage(notification *domain.Notification) string {
 	var builder strings.Builder
 
-	// Format From header with optional display name
+	// Format From header with optional display name. The display name is untrusted
+	// configuration input, so it's run through the same header-encoding as Subject.
 	fromHeader := s.config.From
 	if s.config.FromName != "" {
-		fromHeader = fmt.Sprintf("%s <%s>", s.config.FromName, s.config.From)
+		fromHeader = fmt.Sprintf("%s <%s>", encodeHeaderValue(s.config.FromName), s.config.From)
 	}
 
-	builder.WriteString(fmt.Sprintf("From: %s\r\n", fromHeader))
+	fmt.Fprintf(&builder, "From: %s\r\n", fromHeader)
 
 	// Add To header (optional if only BCC is specified)
 	if len(notification.Recipients) > 0 {
-		builder.WriteString(fmt.Sprintf("To: %s\r\n", strings.Join(notification.Recipients, ", ")))
+		fmt.Fprintf(&builder, "To: %s\r\n", strings.Join(notification.Recipients, ", "))
 	}
 
 	// Add CC header (optional)
 	if len(notification.CC) > 0 {
-		builder.WriteString(fmt.Sprintf("Cc: %s\r\n", strings.Join(notification.CC, ", ")))
+		fmt.Fprintf(&builder, "Cc: %s\r\n", strings.Join(notification.CC, ", "))
 	}
 
 	// Note: BCC is intentionally NOT included in headers (that's the point of BCC!)
 
-	builder.WriteString(fmt.Sprintf("Subject: %s\r\n", notification.Subject))
+	// Subject is fully attacker-controlled, so it is always run through RFC 2047 encoding.
+	// This neutralizes embedded CR/LF (and non-ASCII) instead of interpolating it raw.
+	fmt.Fprintf(&builder, "Subject: %s\r\n", encodeHeaderValue(notification.Subject))
 	builder.WriteString("MIME-Version: 1.0\r\n")
 
 	switch {
@@ -177,24 +275,24 @@ func isHTMLContent(notification *domain.Notification) bool {
 func (s *SMTPNotifier) buildMultipartMessage(builder *strings.Builder, plainText, htmlBody string) {
 	boundary := generateBoundary()
 
-	builder.WriteString(fmt.Sprintf("Content-Type: multipart/alternative; boundary=\"%s\"\r\n", boundary))
+	fmt.Fprintf(builder, "Content-Type: multipart/alternative; boundary=\"%s\"\r\n", boundary)
 	builder.WriteString("\r\n")
 
-	builder.WriteString(fmt.Sprintf("--%s\r\n", boundary))
+	fmt.Fprintf(builder, "--%s\r\n", boundary)
 	builder.WriteString("Content-Type: text/plain; charset=UTF-8\r\n")
 	builder.WriteString("Content-Transfer-Encoding: 7bit\r\n")
 	builder.WriteString("\r\n")
 	builder.WriteString(plainText)
 	builder.WriteString("\r\n\r\n")
 
-	builder.WriteString(fmt.Sprintf("--%s\r\n", boundary))
+	fmt.Fprintf(builder, "--%s\r\n", boundary)
 	builder.WriteString("Content-Type: text/html; charset=UTF-8\r\n")
 	builder.WriteString("Content-Transfer-Encoding: 7bit\r\n")
 	builder.WriteString("\r\n")
 	builder.WriteString(htmlBody)
 	builder.WriteString("\r\n\r\n")
 
-	builder.WriteString(fmt.Sprintf("--%s--\r\n", boundary))
+	fmt.Fprintf(builder, "--%s--\r\n", boundary)
 }
 
 // detectContentType auto-detects if the body is HTML

internal/notifier/smtp_test.go

@@ -0,0 +1,292 @@
+package notifier
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/igodwin/notifier/internal/domain"
+)
+
+// TestValidateRecipient covers the recipient validation helper used by Send() to reject
+// header-injection attempts and syntactically invalid addresses before a message is built
+// or a network connection is opened.
+func TestValidateRecipient(t *testing.T) {
+	tests := []struct {
+		name      string
+		recipient string
+		wantErr   bool
+	}{
+		{
+			name:      "valid simple address",
+			recipient: "user@example.com",
+			wantErr:   false,
+		},
+		{
+			name:      "valid address with display name",
+			recipient: "Jane Doe <jane@example.com>",
+			wantErr:   false,
+		},
+		{
+			name:      "missing at sign",
+			recipient: "not-an-email",
+			wantErr:   true,
+		},
+		{
+			name:      "empty string",
+			recipient: "",
+			wantErr:   true,
+		},
+		{
+			name:      "CRLF header injection attempt",
+			recipient: "user@example.com\r\nBcc: attacker@evil.com",
+			wantErr:   true,
+		},
+		{
+			name:      "bare LF header injection attempt",
+			recipient: "user@example.com\nX-Injected: true",
+			wantErr:   true,
+		},
+		{
+			name:      "bare CR header injection attempt",
+			recipient: "user@example.com\rX-Injected: true",
+			wantErr:   true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateRecipient(tt.recipient)
+			if tt.wantErr && err == nil {
+				t.Fatalf("validateRecipient(%q) = nil, want error", tt.recipient)
+			}
+			if !tt.wantErr && err != nil {
+				t.Fatalf("validateRecipient(%q) = %v, want nil", tt.recipient, err)
+			}
+		})
+	}
+}
+
+// TestSendRejectsInvalidRecipientsWithoutNetworkAccess verifies that Send() rejects invalid
+// or CRLF-laden recipients during validation, before ever attempting to dial the SMTP server.
+// The configured host is deliberately non-routable so the test would hang or fail on a real
+// dial attempt if validation didn't short-circuit first.
+func TestSendRejectsInvalidRecipientsWithoutNetworkAccess(t *testing.T) {
+	tests := []struct {
+		name       string
+		recipients []string
+		cc         []string
+		bcc        []string
+	}{
+		{
+			name:       "invalid To address",
+			recipients: []string{"not-an-email"},
+		},
+		{
+			name:       "CRLF injection in To address",
+			recipients: []string{"user@example.com\r\nBcc: attacker@evil.com"},
+		},
+		{
+			name:       "CRLF injection in CC address",
+			recipients: []string{"user@example.com"},
+			cc:         []string{"cc@example.com\r\nX-Injected: true"},
+		},
+		{
+			name:       "CRLF injection in BCC address",
+			recipients: []string{"user@example.com"},
+			bcc:        []string{"bcc@example.com\r\nX-Injected: true"},
+		},
+	}
+
+	notifier, err := NewSMTPNotifier(&SMTPConfig{
+		Host: "invalid.invalid", // non-routable placeholder; must never be dialed
+		Port: 587,
+		From: "sender@example.com",
+	})
+	if err != nil {
+		t.Fatalf("NewSMTPNotifier() error = %v", err)
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			notification := &domain.Notification{
+				ID:         "test-id",
+				Type:       domain.TypeEmail,
+				Subject:    "Test Subject",
+				Body:       "Test Body",
+				Recipients: tt.recipients,
+				CC:         tt.cc,
+				BCC:        tt.bcc,
+			}
+
+			result, err := notifier.Send(t.Context(), notification)
+			if err == nil {
+				t.Fatalf("Send() error = nil, want validation error")
+			}
+			if result == nil || result.Success {
+				t.Fatalf("Send() result = %+v, want Success=false", result)
+			}
+		})
+	}
+}
+
+// TestBuildMessageNeutralizesSubjectCRLF ensures a CRLF-laden Subject cannot smuggle a new
+// header into the raw message: the injected header line must not appear verbatim.
+func TestBuildMessageNeutralizesSubjectCRLF(t *testing.T) {
+	notifier, err := NewSMTPNotifier(&SMTPConfig{
+		Host: "smtp.example.com",
+		Port: 587,
+		From: "sender@example.com",
+	})
+	if err != nil {
+		t.Fatalf("NewSMTPNotifier() error = %v", err)
+	}
+
+	notification := &domain.Notification{
+		ID:         "test-id",
+		Type:       domain.TypeEmail,
+		Subject:    "Hello\r\nX-Injected: evil",
+		Body:       "Test Body",
+		Recipients: []string{"user@example.com"},
+	}
+
+	message := notifier.buildMessage(notification)
+
+	if strings.Contains(message, "\r\nX-Injected:") {
+		t.Fatalf("built message contains injected header line:\n%s", message)
+	}
+	if strings.Contains(message, "X-Injected: evil") {
+		t.Fatalf("built message contains raw injected header value:\n%s", message)
+	}
+
+	// The Subject header line must still be present, just RFC 2047 encoded.
+	if !strings.Contains(message, "Subject: =?utf-8?q?") {
+		t.Fatalf("expected RFC 2047 encoded Subject header, got message:\n%s", message)
+	}
+}
+
+// TestBuildMessageNeutralizesFromNameCRLF ensures a CRLF-laden FromName config value cannot
+// inject an extra header into the From line.
+func TestBuildMessageNeutralizesFromNameCRLF(t *testing.T) {
+	notifier, err := NewSMTPNotifier(&SMTPConfig{
+		Host:     "smtp.example.com",
+		Port:     587,
+		From:     "sender@example.com",
+		FromName: "Evil\r\nX-Injected: true",
+	})
+	if err != nil {
+		t.Fatalf("NewSMTPNotifier() error = %v", err)
+	}
+
+	notification := &domain.Notification{
+		ID:         "test-id",
+		Type:       domain.TypeEmail,
+		Subject:    "Hello",
+		Body:       "Test Body",
+		Recipients: []string{"user@example.com"},
+	}
+
+	message := notifier.buildMessage(notification)
+
+	if strings.Contains(message, "\r\nX-Injected:") {
+		t.Fatalf("built message contains injected header line from FromName:\n%s", message)
+	}
+	if strings.Contains(message, "X-Injected: true") {
+		t.Fatalf("built message contains raw injected FromName value:\n%s", message)
+	}
+
+	if !strings.Contains(message, "From: =?utf-8?q?") {
+		t.Fatalf("expected RFC 2047 encoded From display name, got message:\n%s", message)
+	}
+}
+
+// TestBuildMessagePlainSubjectUnchanged verifies that a benign ASCII subject is left
+// unencoded (mime.QEncoding.Encode is a no-op for plain ASCII), preserving existing behavior.
+func TestBuildMessagePlainSubjectUnchanged(t *testing.T) {
+	notifier, err := NewSMTPNotifier(&SMTPConfig{
+		Host: "smtp.example.com",
+		Port: 587,
+		From: "sender@example.com",
+	})
+	if err != nil {
+		t.Fatalf("NewSMTPNotifier() error = %v", err)
+	}
+
+	notification := &domain.Notification{
+		ID:         "test-id",
+		Type:       domain.TypeEmail,
+		Subject:    "Plain Subject Line",
+		Body:       "Test Body",
+		Recipients: []string{"user@example.com"},
+	}
+
+	message := notifier.buildMessage(notification)
+
+	if !strings.Contains(message, "Subject: Plain Subject Line\r\n") {
+		t.Fatalf("expected plain ASCII subject to be left unencoded, got message:\n%s", message)
+	}
+}
+
+// TestBuildMessageMultipartWithHTMLBody verifies that supplying HTMLBody still produces a
+// correct multipart/alternative message with both text/plain and text/html parts.
+func TestBuildMessageMultipartWithHTMLBody(t *testing.T) {
+	notifier, err := NewSMTPNotifier(&SMTPConfig{
+		Host: "smtp.example.com",
+		Port: 587,
+		From: "sender@example.com",
+	})
+	if err != nil {
+		t.Fatalf("NewSMTPNotifier() error = %v", err)
+	}
+
+	notification := &domain.Notification{
+		ID:         "test-id",
+		Type:       domain.TypeEmail,
+		Subject:    "Multipart Test",
+		Body:       "Plain text body",
+		HTMLBody:   "<p>HTML body</p>",
+		Recipients: []string{"user@example.com"},
+	}
+
+	message := notifier.buildMessage(notification)
+
+	if !strings.Contains(message, "Content-Type: multipart/alternative; boundary=") {
+		t.Fatalf("expected multipart/alternative content type, got message:\n%s", message)
+	}
+	if !strings.Contains(message, "Content-Type: text/plain; charset=UTF-8") {
+		t.Fatalf("expected text/plain part, got message:\n%s", message)
+	}
+	if !strings.Contains(message, "Content-Type: text/html; charset=UTF-8") {
+		t.Fatalf("expected text/html part, got message:\n%s", message)
+	}
+	if !strings.Contains(message, "Plain text body") {
+		t.Fatalf("expected plain text body verbatim, got message:\n%s", message)
+	}
+	if !strings.Contains(message, "<p>HTML body</p>") {
+		t.Fatalf("expected HTML body verbatim, got message:\n%s", message)
+	}
+
+	// Ensure the message ends with a proper closing boundary.
+	if !strings.Contains(message, "--\r\n") {
+		t.Fatalf("expected closing MIME boundary, got message:\n%s", message)
+	}
+}
+
+// TestNewSMTPNotifierDefaultsAndUseTLS is a sanity check that UseTLS is stored on the config
+// and that port defaulting still works as before, since Send() now branches on both.
+func TestNewSMTPNotifierDefaultsAndUseTLS(t *testing.T) {
+	notifier, err := NewSMTPNotifier(&SMTPConfig{
+		Host:   "smtp.example.com",
+		From:   "sender@example.com",
+		UseTLS: true,
+	})
+	if err != nil {
+		t.Fatalf("NewSMTPNotifier() error = %v", err)
+	}
+
+	if notifier.config.Port != 587 {
+		t.Fatalf("expected default port 587, got %d", notifier.config.Port)
+	}
+	if !notifier.config.UseTLS {
+		t.Fatalf("expected UseTLS to be preserved as true")
+	}
+}

internal/queue/local.go

@@ -1,3 +1,6 @@
+// Package queue provides domain.Queue implementations used to buffer
+// notifications between submission and delivery, including an in-memory
+// LocalQueue with optional disk persistence.
 package queue
 
 import (
@@ -52,13 +55,14 @@ func NewLocalQueue(config *domain.LocalQueueConfig) (*LocalQueue, error) {
 	return lq, nil
 }
 
-// Enqueue adds a notification to the queue
+// Enqueue adds a notification to the queue.
+//
+// The mutex must NOT be held while sending on the channel: when the buffer is
+// full the send blocks, and workers need the same mutex (Dequeue bookkeeping,
+// Ack) to drain the channel — holding it here deadlocks the whole pool.
 func (lq *LocalQueue) Enqueue(ctx context.Context, notification *domain.Notification) error {
-	lq.mu.Lock()
-	defer lq.mu.Unlock()
-
-	if lq.closed {
-		return fmt.Errorf("queue is closed")
+	if err := lq.checkOpen(); err != nil {
+		return err
 	}
 
 	msg := &domain.QueueMessage{
@@ -70,51 +74,40 @@ func (lq *LocalQueue) Enqueue(ctx context.Context, notification *domain.Notifica
 
 	select {
 	case lq.queue <- msg:
-		lq.messages[msg.ID] = msg
-		notification.Status = domain.StatusQueued
-
-		if lq.persistToDisk {
-			return lq.persistToDiskSync()
-		}
-		return nil
 	case <-ctx.Done():
 		return ctx.Err()
 	case <-lq.closeChan:
 		return fmt.Errorf("queue is closed")
 	}
+
+	lq.mu.Lock()
+	defer lq.mu.Unlock()
+	lq.messages[msg.ID] = msg
+	notification.Status = domain.StatusQueued
+
+	if lq.persistToDisk {
+		return lq.persistToDiskSync()
+	}
+	return nil
 }
 
 // EnqueueBatch adds multiple notifications to the queue
 func (lq *LocalQueue) EnqueueBatch(ctx context.Context, notifications []*domain.Notification) error {
-	lq.mu.Lock()
-	defer lq.mu.Unlock()
+	for _, notification := range notifications {
+		if err := lq.Enqueue(ctx, notification); err != nil {
+			return err
+		}
+	}
+	return nil
+}
 
+// checkOpen reports an error if the queue has been closed.
+func (lq *LocalQueue) checkOpen() error {
+	lq.mu.RLock()
+	defer lq.mu.RUnlock()
 	if lq.closed {
 		return fmt.Errorf("queue is closed")
 	}
-
-	for _, notification := range notifications {
-		msg := &domain.QueueMessage{
-			ID:           uuid.New().String(),
-			Notification: notification,
-			Attempt:      0,
-			EnqueuedAt:   time.Now().Unix(),
-		}
-
-		select {
-		case lq.queue <- msg:
-			lq.messages[msg.ID] = msg
-			notification.Status = domain.StatusQueued
-		case <-ctx.Done():
-			return ctx.Err()
-		case <-lq.closeChan:
-			return fmt.Errorf("queue is closed")
-		}
-	}
-
-	if lq.persistToDisk {
-		return lq.persistToDiskSync()
-	}
 	return nil
 }
 
@@ -139,7 +132,7 @@ func (lq *LocalQueue) Dequeue(ctx context.Context) (*domain.QueueMessage, error)
 }
 
 // Ack acknowledges successful processing of a message
-func (lq *LocalQueue) Ack(ctx context.Context, messageID string) error {
+func (lq *LocalQueue) Ack(_ context.Context, messageID string) error {
 	lq.mu.Lock()
 	defer lq.mu.Unlock()
 
@@ -155,50 +148,57 @@ func (lq *LocalQueue) Ack(ctx context.Context, messageID string) error {
 	return nil
 }
 
-// Nack indicates processing failure and may requeue the message
+// Nack indicates processing failure and may requeue the message.
+// Like Enqueue, the requeue send happens without holding the mutex to avoid
+// deadlocking against workers draining the channel.
 func (lq *LocalQueue) Nack(ctx context.Context, messageID string, requeue bool) error {
 	lq.mu.Lock()
-	defer lq.mu.Unlock()
-
 	msg, exists := lq.messages[messageID]
 	if !exists {
+		lq.mu.Unlock()
 		return fmt.Errorf("message not found: %s", messageID)
 	}
 
-	if requeue {
-		msg.Notification.Status = domain.StatusRetrying
-		select {
-		case lq.queue <- msg:
-			if lq.persistToDisk {
-				return lq.persistToDiskSync()
-			}
-			return nil
-		case <-ctx.Done():
-			return ctx.Err()
-		case <-lq.closeChan:
-			return fmt.Errorf("queue is closed")
-		}
-	} else {
+	if !requeue {
 		msg.Notification.Status = domain.StatusFailed
 		delete(lq.messages, messageID)
 
+		var err error
 		if lq.persistToDisk {
-			return lq.persistToDiskSync()
+			err = lq.persistToDiskSync()
 		}
+		lq.mu.Unlock()
+		return err
 	}
 
+	msg.Notification.Status = domain.StatusRetrying
+	lq.mu.Unlock()
+
+	select {
+	case lq.queue <- msg:
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-lq.closeChan:
+		return fmt.Errorf("queue is closed")
+	}
+
+	lq.mu.Lock()
+	defer lq.mu.Unlock()
+	if lq.persistToDisk {
+		return lq.persistToDiskSync()
+	}
 	return nil
 }
 
 // Size returns the current number of messages in the queue
-func (lq *LocalQueue) Size(ctx context.Context) (int64, error) {
+func (lq *LocalQueue) Size(_ context.Context) (int64, error) {
 	lq.mu.RLock()
 	defer lq.mu.RUnlock()
 	return int64(len(lq.queue)), nil
 }
 
 // Purge removes all messages from the queue
-func (lq *LocalQueue) Purge(ctx context.Context) error {
+func (lq *LocalQueue) Purge(_ context.Context) error {
 	lq.mu.Lock()
 	defer lq.mu.Unlock()
 
@@ -234,12 +234,14 @@ func (lq *LocalQueue) Close() error {
 		}
 	}
 
-	close(lq.queue)
+	// The queue channel is intentionally not closed: senders no longer hold
+	// the mutex while sending, so a concurrent close could panic. closeChan
+	// unblocks all pending senders and receivers instead.
 	return nil
 }
 
 // HealthCheck verifies the queue is operational
-func (lq *LocalQueue) HealthCheck(ctx context.Context) error {
+func (lq *LocalQueue) HealthCheck(_ context.Context) error {
 	lq.mu.RLock()
 	defer lq.mu.RUnlock()
 
@@ -261,7 +263,7 @@ func (lq *LocalQueue) persistToDiskSync() error {
 		return fmt.Errorf("failed to marshal queue state: %w", err)
 	}
 
-	if err := os.WriteFile(lq.persistPath, data, 0644); err != nil {
+	if err := os.WriteFile(lq.persistPath, data, 0600); err != nil {
 		return fmt.Errorf("failed to write queue state: %w", err)
 	}
 

internal/service/service.go

@@ -1,3 +1,7 @@
+// Package service implements the core notification service: queueing,
+// worker-pool delivery with retry/backoff, in-memory notification tracking,
+// retention cleanup, and multi-tenant access control on top of the
+// domain and auth packages.
 package service
 
 import (
@@ -17,6 +21,21 @@ type AccountResolver interface {
 	GetDefaultAccount(notifierType domain.NotificationType) string
 }
 
+const (
+	// defaultRetryBaseDelay is the base delay used for exponential retry
+	// backoff: delay = defaultRetryBaseDelay * 2^(RetryCount-1).
+	defaultRetryBaseDelay = time.Second
+
+	// maxRetryBackoffDelay caps the exponential backoff delay so a
+	// persistently failing notification doesn't wait arbitrarily long between
+	// attempts.
+	maxRetryBackoffDelay = 30 * time.Second
+
+	// adminRole grants access to all tenants' notifications regardless of
+	// ClientID.
+	adminRole = "admin"
+)
+
 // NotificationService implements the domain.NotificationService interface
 type NotificationService struct {
 	factory                domain.NotifierFactory
@@ -27,12 +46,15 @@ type NotificationService struct {
 	mu                     sync.RWMutex
 	workerCount            int
 	stopChan               chan struct{}
+	runCancel              context.CancelFunc
 	wg                     sync.WaitGroup
 	logger                 *logging.Logger
 	retentionConfig        config.NotificationRetentionConfig
 	cleanupStopChan        chan struct{}
 	ttlDuration            time.Duration
 	checkFrequencyDuration time.Duration
+	retryBackoff           string
+	retryBaseDelay         time.Duration
 }
 
 // NewNotificationService creates a new notification service
@@ -51,9 +73,19 @@ func NewNotificationService(factory domain.NotifierFactory, queue domain.Queue,
 		stopChan:        make(chan struct{}),
 		logger:          logger,
 		cleanupStopChan: make(chan struct{}),
+		retryBaseDelay:  defaultRetryBaseDelay,
 	}
 }
 
+// WithRetryBackoff sets the retry backoff strategy used when requeueing a
+// notification after a retryable send failure. Recognized values are
+// "exponential" (delay = 1s * 2^(RetryCount-1), capped at 30s) and "none"
+// (requeue immediately). An empty string is treated as "exponential", matching
+// the documented default for queue.retry_backoff.
+func (s *NotificationService) WithRetryBackoff(mode string) {
+	s.retryBackoff = mode
+}
+
 // WithRetentionConfig sets the notification retention configuration
 func (s *NotificationService) WithRetentionConfig(cfg config.NotificationRetentionConfig) error {
 	s.retentionConfig = cfg
@@ -77,15 +109,20 @@ func (s *NotificationService) WithRetentionConfig(cfg config.NotificationRetenti
 
 // Start starts the worker pool and cleanup goroutine
 func (s *NotificationService) Start(ctx context.Context) error {
+	// Derive a service-lifetime context so Stop() can interrupt workers
+	// blocked in Dequeue instead of waiting out their poll timeout.
+	runCtx, cancel := context.WithCancel(ctx)
+	s.runCancel = cancel
+
 	for i := 0; i < s.workerCount; i++ {
 		s.wg.Add(1)
-		go s.worker(ctx, i)
+		go s.worker(runCtx, i)
 	}
 
 	// Start cleanup goroutine if retention is enabled
 	if s.retentionConfig.Enabled && s.checkFrequencyDuration > 0 {
 		s.wg.Add(1)
-		go s.cleanupLoop(ctx)
+		go s.cleanupLoop(runCtx)
 	}
 
 	return nil
@@ -95,6 +132,9 @@ func (s *NotificationService) Start(ctx context.Context) error {
 func (s *NotificationService) Stop() error {
 	close(s.stopChan)
 	close(s.cleanupStopChan)
+	if s.runCancel != nil {
+		s.runCancel()
+	}
 	s.wg.Wait()
 	return s.queue.Close()
 }
@@ -130,14 +170,12 @@ func (s *NotificationService) performCleanup() {
 
 	// Track which notifications to delete
 	var toDelete []string
-	var allNotifications []*domain.Notification
 
-	// First pass: identify expired notifications and collect all for sorting
+	// First pass: identify expired notifications
 	for id, notification := range s.notifications {
 		if notification.CreatedAt.Before(expiredBefore) {
 			toDelete = append(toDelete, id)
 		}
-		allNotifications = append(allNotifications, notification)
 	}
 
 	// Delete expired notifications
@@ -182,7 +220,7 @@ func (s *NotificationService) performCleanup() {
 }
 
 // worker processes notifications from the queue
-func (s *NotificationService) worker(ctx context.Context, id int) {
+func (s *NotificationService) worker(ctx context.Context, _ int) {
 	defer s.wg.Done()
 
 	for {
@@ -218,7 +256,19 @@ func (s *NotificationService) worker(ctx context.Context, id int) {
 
 // processNotification sends a notification and handles the result
 func (s *NotificationService) processNotification(ctx context.Context, msg *domain.QueueMessage) {
-	notification := msg.Notification
+	// Work on our own clone of the queued notification. msg.Notification is
+	// owned by the queue (which mutates its Status for its own bookkeeping);
+	// cloning here means our mutations below never race with the queue's or
+	// with clones already handed out by GetNotification/ListNotifications.
+	notification := msg.Notification.Clone()
+
+	// Derive retry progress from msg.Attempt (incremented by the queue on
+	// every dequeue): the queue's copy never sees the counter on our clone,
+	// so the message itself is the source of truth across requeues.
+	notification.RetryCount = msg.Attempt - 1
+	if notification.RetryCount < 0 {
+		notification.RetryCount = 0
+	}
 
 	s.logger.Debugf("Processing notification - id=%s, type=%s, recipients=%d",
 		notification.ID, notification.Type, len(notification.Recipients))
@@ -236,7 +286,9 @@ func (s *NotificationService) processNotification(ctx context.Context, msg *doma
 			notification.ID, notification.Type, account, err)
 		notification.Status = domain.StatusFailed
 		notification.LastError = fmt.Sprintf("failed to create notifier: %v", err)
-		s.queue.Nack(ctx, msg.ID, false)
+		if nackErr := s.queue.Nack(ctx, msg.ID, false); nackErr != nil {
+			s.logger.Warnf("failed to nack message id=%s: %v", msg.ID, nackErr)
+		}
 		s.updateNotification(notification)
 		return
 	}
@@ -257,18 +309,25 @@ func (s *NotificationService) processNotification(ctx context.Context, msg *doma
 			notification.Status = domain.StatusRetrying
 			s.logger.Warnf("Notification send failed, will retry - id=%s, type=%s, account=%s, attempt=%d/%d, error=%s",
 				notification.ID, notification.Type, account, notification.RetryCount, notification.MaxRetries, notification.LastError)
-			s.queue.Nack(ctx, msg.ID, true) // Requeue
+			// Hand the retry goroutine its own clone: the worker still
+			// publishes this notification via updateNotification below, and
+			// a shutdown-time abandonRetry must not write to the same object.
+			s.scheduleRetry(ctx, msg, notification.Clone())
 		} else {
 			notification.Status = domain.StatusFailed
 			s.logger.Errorf("Notification send failed permanently - id=%s, type=%s, account=%s, recipients=%v, attempts=%d, error=%s",
 				notification.ID, notification.Type, account, notification.Recipients, notification.RetryCount, notification.LastError)
-			s.queue.Nack(ctx, msg.ID, false) // Don't requeue
+			if nackErr := s.queue.Nack(ctx, msg.ID, false); nackErr != nil { // Don't requeue
+				s.logger.Warnf("failed to nack message id=%s: %v", msg.ID, nackErr)
+			}
 		}
 	} else {
 		notification.Status = domain.StatusSent
 		now := time.Now()
 		notification.SentAt = &now
-		s.queue.Ack(ctx, msg.ID)
+		if ackErr := s.queue.Ack(ctx, msg.ID); ackErr != nil {
+			s.logger.Warnf("failed to ack message id=%s: %v", msg.ID, ackErr)
+		}
 		s.logger.Infof("Notification sent successfully - id=%s, type=%s, account=%s, recipients=%v",
 			notification.ID, notification.Type, account, notification.Recipients)
 	}
@@ -276,6 +335,82 @@ func (s *NotificationService) processNotification(ctx context.Context, msg *doma
 	s.updateNotification(notification)
 }
 
+// scheduleRetry requeues a failed notification, optionally delaying the
+// requeue according to the configured backoff strategy. When a delay applies,
+// the wait happens on a goroutine tracked by the service WaitGroup so Stop()
+// blocks until it finishes - guaranteeing the queue is never closed while a
+// requeue for it is still pending.
+func (s *NotificationService) scheduleRetry(ctx context.Context, msg *domain.QueueMessage, notification *domain.Notification) {
+	delay := s.retryDelay(notification.RetryCount)
+	if delay <= 0 {
+		if err := s.queue.Nack(ctx, msg.ID, true); err != nil { // Requeue immediately
+			s.logger.Warnf("failed to nack message id=%s: %v", msg.ID, err)
+		}
+		return
+	}
+
+	s.wg.Add(1)
+	go func() {
+		defer s.wg.Done()
+
+		timer := time.NewTimer(delay)
+		defer timer.Stop()
+
+		select {
+		case <-timer.C:
+			if err := s.queue.Nack(ctx, msg.ID, true); err != nil { // Requeue after backoff
+				s.logger.Warnf("failed to nack message id=%s: %v", msg.ID, err)
+			}
+		case <-ctx.Done():
+			s.abandonRetry(msg, notification)
+		case <-s.stopChan:
+			s.abandonRetry(msg, notification)
+		}
+	}()
+}
+
+// abandonRetry marks a notification as permanently failed without requeueing
+// it. It is used when the service shuts down while a backoff delay for a
+// retry is still pending, so the notification isn't left stuck in "retrying"
+// forever and no goroutine lingers past shutdown.
+func (s *NotificationService) abandonRetry(msg *domain.QueueMessage, notification *domain.Notification) {
+	if err := s.queue.Nack(context.Background(), msg.ID, false); err != nil { // Don't requeue
+		s.logger.Warnf("failed to nack message id=%s: %v", msg.ID, err)
+	}
+	notification.Status = domain.StatusFailed
+	s.updateNotification(notification)
+}
+
+// retryDelay computes how long to wait before requeueing a notification that
+// failed on attempt retryCount, per the configured backoff strategy. "none"
+// means requeue immediately (delay of 0); anything else - including the
+// empty string, the documented default - uses exponential backoff: base 1s *
+// 2^(retryCount-1), capped at maxRetryBackoffDelay.
+func (s *NotificationService) retryDelay(retryCount int) time.Duration {
+	if s.retryBackoff == "none" {
+		return 0
+	}
+
+	base := s.retryBaseDelay
+	if base <= 0 {
+		base = defaultRetryBaseDelay
+	}
+
+	if retryCount < 1 {
+		retryCount = 1
+	}
+	shift := retryCount - 1
+	if shift > 20 { // guard against overflow for pathological retry counts
+		shift = 20
+	}
+
+	delay := base * time.Duration(int64(1)<<uint(shift))
+	if delay <= 0 || delay > maxRetryBackoffDelay {
+		delay = maxRetryBackoffDelay
+	}
+	return delay
+}
+
 // Send queues a notification for delivery
 func (s *NotificationService) Send(ctx context.Context, notification *domain.Notification) (*domain.NotificationResult, error) {
 	// Enforce RBAC authorization if configured
@@ -288,15 +423,30 @@ func (s *NotificationService) Send(ctx context.Context, notification *domain.Not
 		}, err
 	}
 
-	// Store the notification
+	// Stamp tenant ownership from the auth context, if present. Absent auth
+	// context (auth disabled) leaves ClientID empty, preserving current
+	// behavior.
+	if authCtx, ok := auth.GetAuthContext(ctx); ok && authCtx != nil {
+		notification.ClientID = authCtx.ClientID
+	}
+
+	// Mark queued and store BEFORE enqueueing: a worker can pick the message
+	// up immediately, and a post-enqueue status write here would overwrite
+	// the worker's Sent/Failed transition.
+	notification.Status = domain.StatusQueued
 	s.storeNotification(notification)
 
-	// Enqueue for processing
-	if err := s.queue.Enqueue(ctx, notification); err != nil {
+	// Enqueue a clone: the queue mutates Status on its copy for its own
+	// bookkeeping and workers clone again on dequeue, so no notification
+	// object is ever shared between the queue, the store, and callers.
+	if err := s.queue.Enqueue(ctx, notification.Clone()); err != nil {
+		notification.Status = domain.StatusFailed
+		notification.LastError = fmt.Sprintf("failed to enqueue: %v", err)
+		s.updateNotification(notification)
 		return &domain.NotificationResult{
 			NotificationID: notification.ID,
 			Success:        false,
-			Error:          fmt.Sprintf("failed to enqueue: %v", err),
+			Error:          notification.LastError,
 			SentAt:         time.Now(),
 		}, err
 	}
@@ -320,13 +470,23 @@ func (s *NotificationService) SendBatch(ctx context.Context, notifications []*do
 		}
 	}
 
-	// Store all notifications
-	for _, notification := range notifications {
-		s.storeNotification(notification)
+	// Stamp tenant ownership from the auth context, if present.
+	if authCtx, ok := auth.GetAuthContext(ctx); ok && authCtx != nil {
+		for _, notification := range notifications {
+			notification.ClientID = authCtx.ClientID
+		}
 	}
 
-	// Enqueue batch
-	if err := s.queue.EnqueueBatch(ctx, notifications); err != nil {
+	// Mark queued and store BEFORE enqueueing (see Send), then hand the
+	// queue clones so it never shares notification objects with the store
+	// or callers.
+	queued := make([]*domain.Notification, 0, len(notifications))
+	for _, notification := range notifications {
+		notification.Status = domain.StatusQueued
+		s.storeNotification(notification)
+		queued = append(queued, notification.Clone())
+	}
+	if err := s.queue.EnqueueBatch(ctx, queued); err != nil {
 		return nil, fmt.Errorf("failed to enqueue batch: %w", err)
 	}
 
@@ -343,20 +503,25 @@ func (s *NotificationService) SendBatch(ctx context.Context, notifications []*do
 	return results, nil
 }
 
-// GetNotification retrieves a notification by ID
+// GetNotification retrieves a notification by ID. If an auth context is
+// present and the caller lacks the admin role, a notification belonging to a
+// different tenant is reported as not found rather than leaking its
+// existence.
 func (s *NotificationService) GetNotification(ctx context.Context, id string) (*domain.Notification, error) {
 	s.mu.RLock()
-	defer s.mu.RUnlock()
-
 	notification, exists := s.notifications[id]
-	if !exists {
-		return nil, fmt.Errorf("notification not found: %s", id)
+	s.mu.RUnlock()
+
+	if !exists || !s.tenantCanAccess(ctx, notification) {
+		return nil, fmt.Errorf("%w: %s", domain.ErrNotificationNotFound, id)
 	}
 
-	return notification, nil
+	return notification.Clone(), nil
 }
 
-// ListNotifications retrieves notifications matching the filter
+// ListNotifications retrieves notifications matching the filter, scoped to
+// the caller's tenant unless they have the admin role or no auth context is
+// present.
 func (s *NotificationService) ListNotifications(ctx context.Context, filter *domain.NotificationFilter) ([]*domain.Notification, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
@@ -365,8 +530,11 @@ func (s *NotificationService) ListNotifications(ctx context.Context, filter *dom
 	var results []*domain.Notification
 
 	for _, notification := range s.notifications {
+		if !s.tenantCanAccess(ctx, notification) {
+			continue
+		}
 		if s.matchesFilter(notification, filter) {
-			results = append(results, notification)
+			results = append(results, notification.Clone())
 		}
 	}
 
@@ -382,27 +550,32 @@ func (s *NotificationService) ListNotifications(ctx context.Context, filter *dom
 	return results, nil
 }
 
-// CancelNotification cancels a pending notification
+// CancelNotification cancels a pending notification. A notification belonging
+// to a different tenant is reported as not found, matching GetNotification.
 func (s *NotificationService) CancelNotification(ctx context.Context, id string) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
 	notification, exists := s.notifications[id]
-	if !exists {
-		return fmt.Errorf("notification not found: %s", id)
+	if !exists || !s.tenantCanAccess(ctx, notification) {
+		return fmt.Errorf("%w: %s", domain.ErrNotificationNotFound, id)
 	}
 
 	if notification.Status == domain.StatusSent {
-		return fmt.Errorf("notification already sent")
+		return domain.ErrNotificationAlreadySent
 	}
 
-	notification.Status = domain.StatusFailed
-	notification.LastError = "cancelled by user"
+	updated := notification.Clone()
+	updated.Status = domain.StatusFailed
+	updated.LastError = "cancelled by user"
+	s.notifications[id] = updated
 
 	return nil
 }
 
-// RetryNotification retries a failed notification
+// RetryNotification retries a failed notification. Cross-tenant access is
+// rejected the same way as GetNotification (via the same not-found error),
+// since RetryNotification is built on top of it.
 func (s *NotificationService) RetryNotification(ctx context.Context, id string) (*domain.NotificationResult, error) {
 	notification, err := s.GetNotification(ctx, id)
 	if err != nil {
@@ -415,7 +588,7 @@ func (s *NotificationService) RetryNotification(ctx context.Context, id string)
 			Success:        false,
 			Error:          "notification already sent",
 			SentAt:         time.Now(),
-		}, fmt.Errorf("notification already sent")
+		}, domain.ErrNotificationAlreadySent
 	}
 
 	// Reset retry count and status
@@ -426,7 +599,8 @@ func (s *NotificationService) RetryNotification(ctx context.Context, id string)
 	return s.Send(ctx, notification)
 }
 
-// GetStats returns notification statistics
+// GetStats returns notification statistics, scoped to the caller's tenant
+// unless they have the admin role or no auth context is present.
 func (s *NotificationService) GetStats(ctx context.Context) (*domain.NotificationStats, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
@@ -437,6 +611,10 @@ func (s *NotificationService) GetStats(ctx context.Context) (*domain.Notificatio
 	}
 
 	for _, notification := range s.notifications {
+		if !s.tenantCanAccess(ctx, notification) {
+			continue
+		}
+
 		switch notification.Status {
 		case domain.StatusSent:
 			stats.TotalSent++
@@ -510,18 +688,55 @@ func (s *NotificationService) GetNotifiers(ctx context.Context) (*domain.Notifie
 	}, nil
 }
 
-// storeNotification stores a notification in memory
+// storeNotification stores a clone of the notification in memory. Storing a
+// clone (rather than the caller's pointer) ensures the map never aliases a
+// notification that the caller, the queue, or a worker may still mutate.
 func (s *NotificationService) storeNotification(notification *domain.Notification) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.notifications[notification.ID] = notification
+	s.notifications[notification.ID] = notification.Clone()
 }
 
-// updateNotification updates a notification in memory
+// updateNotification updates a notification in memory with a clone of the
+// given notification, for the same reason as storeNotification.
 func (s *NotificationService) updateNotification(notification *domain.Notification) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.notifications[notification.ID] = notification
+	s.notifications[notification.ID] = notification.Clone()
+}
+
+// tenantCanAccess reports whether the caller identified by ctx is allowed to
+// see the given notification. Behavior:
+//   - No auth context present (auth disabled): always allowed, preserving
+//     pre-multi-tenant behavior.
+//   - Auth context present with the admin role: always allowed.
+//   - Auth context present without the admin role: allowed only if the
+//     notification's ClientID matches the caller's.
+func (s *NotificationService) tenantCanAccess(ctx context.Context, notification *domain.Notification) bool {
+	if notification == nil {
+		return false
+	}
+
+	authCtx, ok := auth.GetAuthContext(ctx)
+	if !ok || authCtx == nil {
+		return true
+	}
+
+	if hasRole(authCtx.Roles, adminRole) {
+		return true
+	}
+
+	return notification.ClientID == authCtx.ClientID
+}
+
+// hasRole reports whether role is present in roles.
+func hasRole(roles []string, role string) bool {
+	for _, r := range roles {
+		if r == role {
+			return true
+		}
+	}
+	return false
 }
 
 // checkAuthorization verifies that the caller is authorized to send to the given notifier/account.

internal/service/service_race_test.go

@@ -0,0 +1,141 @@
+package service
+
+import (
+	"context"
+	"fmt"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"github.com/igodwin/notifier/internal/domain"
+)
+
+// TestConcurrentSendGetListNoRace hammers Send, GetNotification, and
+// ListNotifications concurrently while worker goroutines process notifications
+// pulled from the queue in the background. It is meant to be run with
+// `go test -race`: if the service ever stored or returned a raw pointer that a
+// worker also mutates (the bug this test guards against), the race detector
+// flags a data race here.
+func TestConcurrentSendGetListNoRace(t *testing.T) {
+	svc := createTestService(t)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
+	defer cancel()
+
+	if err := svc.Start(ctx); err != nil {
+		t.Fatalf("Failed to start service: %v", err)
+	}
+	defer func() { _ = svc.Stop() }()
+
+	const numSenders = 8
+	const sendsPerSender = 25
+
+	var idsMu sync.Mutex
+	var ids []string
+
+	var sendersWg sync.WaitGroup
+	for s := 0; s < numSenders; s++ {
+		sendersWg.Add(1)
+		go func(sender int) {
+			defer sendersWg.Done()
+			for i := 0; i < sendsPerSender; i++ {
+				notification := &domain.Notification{
+					ID:         uuid.New().String(),
+					Type:       domain.TypeStdout,
+					Status:     domain.StatusPending,
+					Subject:    fmt.Sprintf("race-test-%d-%d", sender, i),
+					Body:       "race test body",
+					Recipients: []string{"race@example.com"},
+					CC:         []string{"cc@example.com"},
+					Metadata:   map[string]interface{}{"sender": sender},
+					CreatedAt:  time.Now(),
+					MaxRetries: 1,
+				}
+
+				if _, err := svc.Send(ctx, notification); err != nil {
+					t.Errorf("Send failed: %v", err)
+					continue
+				}
+
+				idsMu.Lock()
+				ids = append(ids, notification.ID)
+				idsMu.Unlock()
+			}
+		}(s)
+	}
+
+	// Readers race against the senders and against the worker pool mutating
+	// notifications as they're processed.
+	stopReaders := make(chan struct{})
+	var readersWg sync.WaitGroup
+	for r := 0; r < 4; r++ {
+		readersWg.Add(1)
+		go func() {
+			defer readersWg.Done()
+			for {
+				select {
+				case <-stopReaders:
+					return
+				default:
+				}
+
+				idsMu.Lock()
+				n := len(ids)
+				var id string
+				if n > 0 {
+					id = ids[n-1]
+				}
+				idsMu.Unlock()
+
+				if id != "" {
+					if notif, err := svc.GetNotification(ctx, id); err == nil {
+						// Touch the returned notification's reference fields;
+						// if it were aliased with the stored/queued copy a
+						// concurrent worker mutation would trip the race
+						// detector right here.
+						_ = notif.Status
+						_ = append([]string(nil), notif.Recipients...)
+					}
+				}
+
+				list, err := svc.ListNotifications(ctx, &domain.NotificationFilter{})
+				if err != nil {
+					t.Errorf("ListNotifications failed: %v", err)
+					continue
+				}
+				for _, notif := range list {
+					_ = notif.Status
+					_ = append([]string(nil), notif.Recipients...)
+				}
+			}
+		}()
+	}
+
+	sendersWg.Wait()
+	close(stopReaders)
+	readersWg.Wait()
+
+	// Give the worker pool a moment to drain the queue, then sanity check the
+	// service is still consistent.
+	deadline := time.Now().Add(5 * time.Second)
+	for time.Now().Before(deadline) {
+		stats, err := svc.GetStats(ctx)
+		if err != nil {
+			t.Fatalf("GetStats failed: %v", err)
+		}
+		if stats.TotalSent == numSenders*sendsPerSender {
+			return
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+
+	stats, err := svc.GetStats(ctx)
+	if err != nil {
+		t.Fatalf("GetStats failed: %v", err)
+	}
+	t.Logf("final stats: sent=%d failed=%d queued=%d pending=%d", stats.TotalSent, stats.TotalFailed, stats.TotalQueued, stats.TotalPending)
+	if stats.TotalSent != numSenders*sendsPerSender {
+		t.Errorf("expected all %d notifications to reach Sent, got %d", numSenders*sendsPerSender, stats.TotalSent)
+	}
+}

internal/service/service_retention_test.go

@@ -58,7 +58,7 @@ func TestTTLBasedCleanup(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	// Create old notification (created 2 seconds ago)
 	oldTime := time.Now().Add(-2 * time.Second)
@@ -128,7 +128,7 @@ func TestMaxSizeEnforcement(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	// Create 10 notifications
 	for i := 0; i < 10; i++ {
@@ -179,7 +179,7 @@ func TestCleanupRemovesOldestFirst(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	// Create notifications with distinct times
 	baseTime := time.Now()
@@ -234,7 +234,7 @@ func TestCleanupDisabled(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	// Create old notification
 	oldTime := time.Now().Add(-2 * time.Second)
@@ -278,7 +278,7 @@ func TestCleanupConcurrency(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	// Create some initial notifications
 	for i := 0; i < 10; i++ {
@@ -404,10 +404,11 @@ func TestCleanupGracefulShutdown(t *testing.T) {
 		t.Errorf("Stop failed: %v", stopErr)
 	}
 
-	// Verify notifications are still intact after graceful shutdown
-	stats, err := svc.GetStats(context.Background())
-	if err == nil && stats.TotalSent > 0 {
-		// This is expected - notifications should persist through shutdown
+	// Verify notifications are still intact after graceful shutdown - it's
+	// expected that notifications persist through shutdown, so there's
+	// nothing further to assert beyond GetStats succeeding.
+	if stats, err := svc.GetStats(context.Background()); err == nil {
+		t.Logf("stats after graceful shutdown: sent=%d", stats.TotalSent)
 	}
 }
 
@@ -432,7 +433,7 @@ func TestCleanupWithMixedNotificationStatuses(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	oldTime := time.Now().Add(-2 * time.Second)
 
@@ -499,7 +500,7 @@ func TestCleanupPerformance(t *testing.T) {
 	if err := svc.Start(ctx); err != nil {
 		t.Fatalf("Failed to start service: %v", err)
 	}
-	defer svc.Stop()
+	defer func() { _ = svc.Stop() }()
 
 	// Create 5000 old notifications
 	startTime := time.Now()

internal/service/service_retry_test.go

@@ -0,0 +1,247 @@
+package service
+
+import (
+	"context"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/igodwin/notifier/internal/domain"
+	"github.com/igodwin/notifier/internal/logging"
+	"github.com/igodwin/notifier/internal/notifier"
+	"github.com/igodwin/notifier/internal/queue"
+)
+
+// alwaysFailNotifier is a fake domain.Notifier that always reports failure
+// while recording the wall-clock time of each Send call, so tests can assert
+// on the spacing between retry attempts.
+type alwaysFailNotifier struct {
+	mu    sync.Mutex
+	calls []time.Time
+}
+
+func (n *alwaysFailNotifier) Send(_ context.Context, notification *domain.Notification) (*domain.NotificationResult, error) {
+	n.mu.Lock()
+	n.calls = append(n.calls, time.Now())
+	n.mu.Unlock()
+
+	return &domain.NotificationResult{
+		NotificationID: notification.ID,
+		Success:        false,
+		Error:          "simulated failure",
+		SentAt:         time.Now(),
+	}, nil
+}
+
+func (n *alwaysFailNotifier) Type() domain.NotificationType { return domain.TypeStdout }
+
+func (n *alwaysFailNotifier) Validate(_ *domain.Notification) error { return nil }
+
+func (n *alwaysFailNotifier) Close() error { return nil }
+
+func (n *alwaysFailNotifier) callTimes() []time.Time {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+	return append([]time.Time(nil), n.calls...)
+}
+
+// createFailingTestService builds a NotificationService wired to the given
+// (always failing) notifier instead of the stdout notifier used by
+// createTestService.
+func createFailingTestService(t *testing.T, fail domain.Notifier) *NotificationService {
+	t.Helper()
+
+	factory := notifier.NewFactory()
+	if err := factory.RegisterNotifier(domain.TypeStdout, "", fail); err != nil {
+		t.Fatalf("Failed to register notifier: %v", err)
+	}
+
+	q, err := queue.NewLocalQueue(&domain.LocalQueueConfig{BufferSize: 100})
+	if err != nil {
+		t.Fatalf("Failed to create queue: %v", err)
+	}
+
+	logger, err := logging.NewFromConfig("error", "stdout")
+	if err != nil {
+		t.Fatalf("Failed to create logger: %v", err)
+	}
+
+	return NewNotificationService(factory, q, 2, nil, nil, logger)
+}
+
+// waitForStatus polls GetNotification until it observes the notification in
+// the given status, or fails the test after timeout.
+func waitForStatus(ctx context.Context, t *testing.T, svc *NotificationService, id string, status domain.NotificationStatus, timeout time.Duration) *domain.Notification {
+	t.Helper()
+
+	deadline := time.Now().Add(timeout)
+	for time.Now().Before(deadline) {
+		n, err := svc.GetNotification(ctx, id)
+		if err == nil && n.Status == status {
+			return n
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	t.Fatalf("notification %s did not reach status %s within %v", id, status, timeout)
+	return nil
+}
+
+// TestRetryBackoffExponentialDelaysRequeue verifies that, with the default
+// (exponential) backoff mode, the delay between retry attempts roughly
+// doubles each time rather than hammering the failing notifier in a tight
+// loop.
+func TestRetryBackoffExponentialDelaysRequeue(t *testing.T) {
+	fail := &alwaysFailNotifier{}
+	svc := createFailingTestService(t, fail)
+	svc.retryBaseDelay = 40 * time.Millisecond // tiny base delay for a fast test
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	if err := svc.Start(ctx); err != nil {
+		t.Fatalf("Failed to start service: %v", err)
+	}
+	defer func() { _ = svc.Stop() }()
+
+	notification := &domain.Notification{
+		ID:         "backoff-exponential-1",
+		Type:       domain.TypeStdout,
+		Status:     domain.StatusPending,
+		Recipients: []string{"test@example.com"},
+		MaxRetries: 3,
+	}
+
+	if _, err := svc.Send(ctx, notification); err != nil {
+		t.Fatalf("Send failed: %v", err)
+	}
+
+	waitForStatus(ctx, t, svc, notification.ID, domain.StatusFailed, 5*time.Second)
+
+	calls := fail.callTimes()
+	if len(calls) != 3 {
+		t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
+	}
+
+	gap1 := calls[1].Sub(calls[0])
+	gap2 := calls[2].Sub(calls[1])
+
+	// Expected gaps are ~40ms then ~80ms; use generous tolerances to absorb
+	// scheduler jitter while still proving a real, growing delay was applied.
+	if gap1 < 25*time.Millisecond {
+		t.Errorf("expected delay before 2nd attempt >= ~40ms, got %v", gap1)
+	}
+	if gap2 < gap1 {
+		t.Errorf("expected delay before 3rd attempt (%v) to be larger than before 2nd (%v)", gap2, gap1)
+	}
+
+	t.Logf("attempt gaps: %v, %v", gap1, gap2)
+}
+
+// TestRetryBackoffNoneIsImmediate verifies that retryBackoff "none" requeues
+// immediately, ignoring the configured base delay entirely.
+func TestRetryBackoffNoneIsImmediate(t *testing.T) {
+	fail := &alwaysFailNotifier{}
+	svc := createFailingTestService(t, fail)
+	svc.WithRetryBackoff("none")
+	svc.retryBaseDelay = 5 * time.Second // large, to prove "none" ignores it
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	if err := svc.Start(ctx); err != nil {
+		t.Fatalf("Failed to start service: %v", err)
+	}
+	defer func() { _ = svc.Stop() }()
+
+	notification := &domain.Notification{
+		ID:         "backoff-none-1",
+		Type:       domain.TypeStdout,
+		Status:     domain.StatusPending,
+		Recipients: []string{"test@example.com"},
+		MaxRetries: 3,
+	}
+
+	start := time.Now()
+	if _, err := svc.Send(ctx, notification); err != nil {
+		t.Fatalf("Send failed: %v", err)
+	}
+
+	waitForStatus(ctx, t, svc, notification.ID, domain.StatusFailed, 2*time.Second)
+	elapsed := time.Since(start)
+
+	if elapsed > 1*time.Second {
+		t.Errorf("expected immediate retries with backoff=none, took %v", elapsed)
+	}
+
+	calls := fail.callTimes()
+	if len(calls) != 3 {
+		t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
+	}
+}
+
+// TestRetryBackoffShutdownAbandonsCleanly verifies that Stop() does not block
+// on (or leak) a goroutine that is waiting out a backoff delay: it should
+// abandon the pending retry and mark the notification Failed instead.
+func TestRetryBackoffShutdownAbandonsCleanly(t *testing.T) {
+	fail := &alwaysFailNotifier{}
+	svc := createFailingTestService(t, fail)
+	svc.retryBaseDelay = 5 * time.Second // long enough that shutdown lands mid-wait
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	if err := svc.Start(ctx); err != nil {
+		t.Fatalf("Failed to start service: %v", err)
+	}
+
+	notification := &domain.Notification{
+		ID:         "backoff-shutdown-1",
+		Type:       domain.TypeStdout,
+		Status:     domain.StatusPending,
+		Recipients: []string{"test@example.com"},
+		MaxRetries: 3,
+	}
+
+	if _, err := svc.Send(ctx, notification); err != nil {
+		t.Fatalf("Send failed: %v", err)
+	}
+
+	// Give the worker time to make its first (failing) attempt and enter the
+	// backoff wait.
+	deadline := time.Now().Add(2 * time.Second)
+	for time.Now().Before(deadline) && len(fail.callTimes()) == 0 {
+		time.Sleep(10 * time.Millisecond)
+	}
+	if len(fail.callTimes()) == 0 {
+		t.Fatal("notifier was never called; cannot test shutdown mid-backoff")
+	}
+
+	stopped := make(chan error, 1)
+	go func() {
+		stopped <- svc.Stop()
+	}()
+
+	select {
+	case err := <-stopped:
+		if err != nil {
+			t.Errorf("Stop returned error: %v", err)
+		}
+	case <-time.After(3 * time.Second):
+		t.Fatal("Stop did not return promptly - possible goroutine leak in retry backoff")
+	}
+
+	n, err := svc.GetNotification(context.Background(), notification.ID)
+	if err != nil {
+		t.Fatalf("GetNotification failed after shutdown: %v", err)
+	}
+	if n.Status != domain.StatusFailed {
+		t.Errorf("expected notification to be marked Failed after shutdown abandon, got %s", n.Status)
+	}
+
+	// Only the one attempt made before shutdown should have happened; the
+	// pending retry must not have fired.
+	if calls := len(fail.callTimes()); calls != 1 {
+		t.Errorf("expected exactly 1 send attempt before shutdown abandoned the retry, got %d", calls)
+	}
+}

internal/service/service_tenant_test.go

@@ -0,0 +1,194 @@
+package service
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/igodwin/notifier/internal/auth"
+	"github.com/igodwin/notifier/internal/domain"
+)
+
+// ctxForClient builds a context carrying an auth.Context for the given
+// client and roles, as REST/gRPC middleware would attach after authenticating
+// a request.
+func ctxForClient(clientID string, roles ...string) context.Context {
+	return auth.ContextWithAuth(context.Background(), &auth.Context{
+		ClientID: clientID,
+		Roles:    roles,
+	})
+}
+
+// TestTenantScopingNonAdminSeesOnlyOwnNotifications verifies that a
+// non-admin caller only sees notifications stamped with their own ClientID.
+func TestTenantScopingNonAdminSeesOnlyOwnNotifications(t *testing.T) {
+	svc := createTestService(t)
+
+	tenantA := &domain.Notification{
+		ID:         uuid.New().String(),
+		Type:       domain.TypeStdout,
+		Status:     domain.StatusSent,
+		Recipients: []string{"a@example.com"},
+		ClientID:   "tenant-a",
+	}
+	tenantB := &domain.Notification{
+		ID:         uuid.New().String(),
+		Type:       domain.TypeStdout,
+		Status:     domain.StatusSent,
+		Recipients: []string{"b@example.com"},
+		ClientID:   "tenant-b",
+	}
+
+	svc.storeNotification(tenantA)
+	svc.storeNotification(tenantB)
+
+	ctxA := ctxForClient("tenant-a", "user")
+
+	list, err := svc.ListNotifications(ctxA, &domain.NotificationFilter{})
+	if err != nil {
+		t.Fatalf("ListNotifications failed: %v", err)
+	}
+	if len(list) != 1 || list[0].ID != tenantA.ID {
+		t.Fatalf("expected tenant-a to see only its own notification, got %d results", len(list))
+	}
+
+	if _, err := svc.GetNotification(ctxA, tenantA.ID); err != nil {
+		t.Errorf("tenant-a should be able to get its own notification: %v", err)
+	}
+
+	if _, err := svc.GetNotification(ctxA, tenantB.ID); err == nil {
+		t.Error("tenant-a should not be able to get tenant-b's notification")
+	}
+
+	stats, err := svc.GetStats(ctxA)
+	if err != nil {
+		t.Fatalf("GetStats failed: %v", err)
+	}
+	if stats.TotalSent != 1 {
+		t.Errorf("expected tenant-a stats to count only its own notification, got %d", stats.TotalSent)
+	}
+}
+
+// TestTenantScopingAdminSeesAll verifies that a caller with the admin role
+// can see notifications belonging to any tenant.
+func TestTenantScopingAdminSeesAll(t *testing.T) {
+	svc := createTestService(t)
+
+	tenantA := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"a@example.com"}, ClientID: "tenant-a"}
+	tenantB := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"b@example.com"}, ClientID: "tenant-b"}
+
+	svc.storeNotification(tenantA)
+	svc.storeNotification(tenantB)
+
+	adminCtx := ctxForClient("admin-client", "admin")
+
+	list, err := svc.ListNotifications(adminCtx, &domain.NotificationFilter{})
+	if err != nil {
+		t.Fatalf("ListNotifications failed: %v", err)
+	}
+	if len(list) != 2 {
+		t.Fatalf("expected admin to see both notifications, got %d", len(list))
+	}
+
+	if _, err := svc.GetNotification(adminCtx, tenantA.ID); err != nil {
+		t.Errorf("admin should be able to get tenant-a's notification: %v", err)
+	}
+	if _, err := svc.GetNotification(adminCtx, tenantB.ID); err != nil {
+		t.Errorf("admin should be able to get tenant-b's notification: %v", err)
+	}
+
+	stats, err := svc.GetStats(adminCtx)
+	if err != nil {
+		t.Fatalf("GetStats failed: %v", err)
+	}
+	if stats.TotalSent != 2 {
+		t.Errorf("expected admin stats to count both notifications, got %d", stats.TotalSent)
+	}
+}
+
+// TestTenantScopingAuthDisabledUnchanged verifies that when no auth context
+// is present (auth disabled), behavior is unchanged: every notification is
+// visible regardless of ClientID.
+func TestTenantScopingAuthDisabledUnchanged(t *testing.T) {
+	svc := createTestService(t)
+
+	tenantA := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"a@example.com"}, ClientID: "tenant-a"}
+	tenantB := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"b@example.com"}, ClientID: ""}
+
+	svc.storeNotification(tenantA)
+	svc.storeNotification(tenantB)
+
+	ctx := context.Background() // no auth context attached
+
+	list, err := svc.ListNotifications(ctx, &domain.NotificationFilter{})
+	if err != nil {
+		t.Fatalf("ListNotifications failed: %v", err)
+	}
+	if len(list) != 2 {
+		t.Fatalf("expected both notifications visible when auth is disabled, got %d", len(list))
+	}
+
+	if _, err := svc.GetNotification(ctx, tenantA.ID); err != nil {
+		t.Errorf("expected to get tenant-a's notification with auth disabled: %v", err)
+	}
+	if _, err := svc.GetNotification(ctx, tenantB.ID); err != nil {
+		t.Errorf("expected to get tenant-b's notification with auth disabled: %v", err)
+	}
+}
+
+// TestTenantScopingCrossTenantAccessReturnsNotFound verifies that
+// Get/Cancel/Retry on another tenant's notification return the exact same
+// not-found error as a genuinely missing ID, so existence isn't leaked.
+func TestTenantScopingCrossTenantAccessReturnsNotFound(t *testing.T) {
+	svc := createTestService(t)
+
+	tenantA := &domain.Notification{
+		ID:         uuid.New().String(),
+		Type:       domain.TypeStdout,
+		Status:     domain.StatusPending,
+		Recipients: []string{"a@example.com"},
+		MaxRetries: 3,
+		ClientID:   "tenant-a",
+	}
+	svc.storeNotification(tenantA)
+
+	ctxB := ctxForClient("tenant-b", "user")
+
+	wantMsg := fmt.Sprintf("notification not found: %s", tenantA.ID)
+
+	_, getErr := svc.GetNotification(ctxB, tenantA.ID)
+	if getErr == nil {
+		t.Fatal("expected not-found error for cross-tenant Get")
+	}
+	if getErr.Error() != wantMsg {
+		t.Errorf("expected cross-tenant Get error %q, got %q", wantMsg, getErr.Error())
+	}
+
+	_, missingErr := svc.GetNotification(ctxB, "does-not-exist")
+	if missingErr == nil {
+		t.Fatal("expected not-found error for missing ID")
+	}
+
+	cancelErr := svc.CancelNotification(ctxB, tenantA.ID)
+	if cancelErr == nil {
+		t.Fatal("expected not-found error for cross-tenant Cancel")
+	}
+	if cancelErr.Error() != wantMsg {
+		t.Errorf("expected cross-tenant Cancel error %q, got %q", wantMsg, cancelErr.Error())
+	}
+
+	_, retryErr := svc.RetryNotification(ctxB, tenantA.ID)
+	if retryErr == nil {
+		t.Fatal("expected not-found error for cross-tenant Retry")
+	}
+	if retryErr.Error() != wantMsg {
+		t.Errorf("expected cross-tenant Retry error %q, got %q", wantMsg, retryErr.Error())
+	}
+
+	// The owning tenant should still be able to see and act on it.
+	ctxA := ctxForClient("tenant-a", "user")
+	if _, err := svc.GetNotification(ctxA, tenantA.ID); err != nil {
+		t.Errorf("tenant-a should still be able to get its own notification: %v", err)
+	}
+}

pkg/client/rest.go

@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
+	"strconv"
 	"time"
 )
 
@@ -22,7 +24,7 @@ type RESTClient struct {
 }
 
 // NewRESTClient creates a new REST client with the given config
-func NewRESTClient(cfg ClientConfig) *RESTClient {
+func NewRESTClient(cfg Config) *RESTClient {
 	if cfg.Timeout == 0 {
 		cfg.Timeout = 30 * time.Second
 	}
@@ -34,7 +36,7 @@ func NewRESTClient(cfg ClientConfig) *RESTClient {
 	}
 
 	tlsConfig := &tls.Config{
-		InsecureSkipVerify: cfg.TLSInsecure,
+		InsecureSkipVerify: cfg.TLSInsecure, // #nosec G402 -- explicit user opt-in (TLSInsecure) for self-signed test endpoints
 	}
 
 	httpClient := &http.Client{
@@ -134,9 +136,33 @@ func (c *RESTClient) GetNotification(ctx context.Context, id string) (*Notificat
 	return &notif, nil
 }
 
-// ListNotifications lists notifications with filters
+// ListNotifications lists notifications with filters. Filter fields are
+// encoded as query parameters matching the server's parseNotificationFilter
+// (limit, offset, repeated type/status/recipient).
 func (c *RESTClient) ListNotifications(ctx context.Context, filter ListNotificationsRequest) (*ListNotificationsResponse, error) {
-	respBody, statusCode, err := c.doRequest(ctx, "GET", "/api/v1/notifications", nil)
+	query := url.Values{}
+	if filter.Limit > 0 {
+		query.Set("limit", strconv.Itoa(filter.Limit))
+	}
+	if filter.Offset > 0 {
+		query.Set("offset", strconv.Itoa(filter.Offset))
+	}
+	for _, t := range filter.Types {
+		query.Add("type", t)
+	}
+	for _, s := range filter.Statuses {
+		query.Add("status", string(s))
+	}
+	for _, r := range filter.Recipients {
+		query.Add("recipient", r)
+	}
+
+	path := "/api/v1/notifications"
+	if encoded := query.Encode(); encoded != "" {
+		path += "?" + encoded
+	}
+
+	respBody, statusCode, err := c.doRequest(ctx, "GET", path, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -238,7 +264,7 @@ func (c *RESTClient) HealthCheck(ctx context.Context) (bool, error) {
 	if err != nil {
 		return false, fmt.Errorf("health check failed: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	return resp.StatusCode == http.StatusOK, nil
 }
@@ -281,7 +307,7 @@ func (c *RESTClient) doRequest(ctx context.Context, method, path string, body []
 		}
 
 		respBody, err := io.ReadAll(resp.Body)
-		resp.Body.Close()
+		_ = resp.Body.Close()
 
 		if err != nil {
 			lastErr = fmt.Errorf("failed to read response: %w", err)

pkg/client/types.go

@@ -1,3 +1,5 @@
+// Package client provides a Go client library and types for interacting
+// with the notifier service's REST API.
 package client
 
 import "time"
@@ -24,6 +26,7 @@ type NotificationResponse struct {
 // NotificationStatus represents the status of a notification
 type NotificationStatus string
 
+// Notification status values returned by the notifier service.
 const (
 	StatusPending  NotificationStatus = "pending"
 	StatusQueued   NotificationStatus = "queued"
@@ -89,8 +92,8 @@ type NotifiersResponse struct {
 	Notifiers []NotifierInfo `json:"notifiers"`
 }
 
-// ClientConfig contains configuration for the client
-type ClientConfig struct {
+// Config contains configuration for the client
+type Config struct {
 	BaseURL      string        // Base URL for REST API (e.g., "http://localhost:8080")
 	APIKey       string        // Optional API key for authentication
 	Timeout      time.Duration // Request timeout (default: 30s)
@@ -100,3 +103,7 @@ type ClientConfig struct {
 	// NEVER set this to true in production. Use proper certificates or provide custom CA certificates instead.
 	TLSInsecure bool
 }
+
+// ClientConfig is a backward-compatible alias for Config.
+// Deprecated: use Config.
+type ClientConfig = Config //nolint:revive // kept for API compatibility

tests/e2e/critical_1_test.go

@@ -94,7 +94,6 @@ func TestCRITICAL1_MaxSizeEnforcement(t *testing.T) {
 
 	// Send more notifications than max_size
 	notificationCount := 10
-	notificationIDs := make([]string, 0, notificationCount)
 
 	for i := 0; i < notificationCount; i++ {
 		req := client.NotificationRequest{
@@ -104,11 +103,10 @@ func TestCRITICAL1_MaxSizeEnforcement(t *testing.T) {
 			Recipients: []string{"test@example.com"},
 		}
 
-		resp, err := suite.Client.Send(ctx, req)
+		_, err := suite.Client.Send(ctx, req)
 		if err != nil {
 			t.Fatalf("Failed to send notification %d: %v", i, err)
 		}
-		notificationIDs = append(notificationIDs, resp.NotificationID)
 	}
 
 	t.Logf("Sent %d notifications", notificationCount)
@@ -328,7 +326,7 @@ func TestCRITICAL1_MemoryBounded(t *testing.T) {
 			req := client.NotificationRequest{
 				Type:       "stdout",
 				Subject:    fmt.Sprintf("Batch %d Notif %d", batch, i),
-				Body:       fmt.Sprintf("Test data for notification"),
+				Body:       "Test data for notification",
 				Recipients: []string{"test@example.com"},
 			}
 
@@ -382,7 +380,7 @@ func TestCRITICAL1_ServiceHealthy(t *testing.T) {
 			req := client.NotificationRequest{
 				Type:       "stdout",
 				Subject:    fmt.Sprintf("Health %d", i),
-				Body:       fmt.Sprintf("Test"),
+				Body:       "Test",
 				Recipients: []string{"test@example.com"},
 			}
 

tests/e2e/suite_test.go

@@ -94,13 +94,17 @@ func SetupSuite(t *testing.T, retention ...string) *TestSuite {
 	// Get container port
 	host, err := container.Host(ctx)
 	if err != nil {
-		container.Terminate(ctx)
+		if termErr := container.Terminate(ctx); termErr != nil {
+			t.Logf("Failed to terminate container during cleanup: %v", termErr)
+		}
 		t.Fatalf("Failed to get container host: %v", err)
 	}
 
 	port, err := container.MappedPort(ctx, "8080")
 	if err != nil {
-		container.Terminate(ctx)
+		if termErr := container.Terminate(ctx); termErr != nil {
+			t.Logf("Failed to terminate container during cleanup: %v", termErr)
+		}
 		t.Fatalf("Failed to get container port: %v", err)
 	}
 
@@ -118,7 +122,9 @@ func SetupSuite(t *testing.T, retention ...string) *TestSuite {
 	deadline := time.Now().Add(30 * time.Second)
 	for {
 		if time.Now().After(deadline) {
-			container.Terminate(ctx)
+			if termErr := container.Terminate(ctx); termErr != nil {
+				t.Logf("Failed to terminate container during cleanup: %v", termErr)
+			}
 			t.Fatalf("Service failed to become ready")
 		}
 
@@ -141,7 +147,13 @@ func SetupSuite(t *testing.T, retention ...string) *TestSuite {
 // TeardownSuite stops and removes the container
 func (s *TestSuite) TeardownSuite(ctx context.Context) {
 	if s.Container != nil {
-		s.Container.Terminate(ctx)
+		if err := s.Container.Terminate(ctx); err != nil {
+			if s.T != nil {
+				s.T.Logf("Failed to terminate container during cleanup: %v", err)
+			} else {
+				fmt.Printf("e2e: failed to terminate container during cleanup: %v\n", err)
+			}
+		}
 	}
 }
 
@@ -172,7 +184,7 @@ func (s *TestSuite) GetLogs(ctx context.Context) string {
 	if err != nil {
 		return fmt.Sprintf("error reading logs: %v", err)
 	}
-	defer reader.Close()
+	defer func() { _ = reader.Close() }()
 
 	logs, err := io.ReadAll(reader)
 	if err != nil {
@@ -181,31 +193,3 @@ func (s *TestSuite) GetLogs(ctx context.Context) string {
 
 	return string(logs)
 }
-
-// buildTestImage builds the docker image for testing
-func buildTestImage(t *testing.T) {
-	// Get the project root
-	wd, err := os.Getwd()
-	if err != nil {
-		t.Fatalf("Failed to get working directory: %v", err)
-	}
-
-	// Find the project root by looking for go.mod
-	for {
-		if _, err := os.Stat(filepath.Join(wd, "go.mod")); err == nil {
-			break
-		}
-		parent := filepath.Dir(wd)
-		if parent == wd {
-			t.Fatalf("Could not find project root")
-		}
-		wd = parent
-	}
-
-	// Check if Dockerfile exists
-	dockerfile := filepath.Join(wd, "Dockerfile")
-	if _, err := os.Stat(dockerfile); err != nil {
-		t.Logf("Warning: Dockerfile not found at %s, using generic build", dockerfile)
-		// The container will be built from the binary
-	}
-}