refactor: audit log record (#1654)

Author: yottahmd

internal/persis/fileaudit/cleaner.go

@@ -1,28 +1,38 @@
 package fileaudit
 
 import (
+	"context"
+	"encoding/json"
 	"log/slog"
 	"os"
 	"path/filepath"
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/dagu-org/dagu/internal/service/audit"
 )
 
+// appendFn is a function that appends an audit entry to the store.
+type appendFn func(ctx context.Context, entry *audit.Entry) error
+
 // cleaner handles periodic cleanup of expired audit log files.
 type cleaner struct {
 	baseDir       string
 	retentionDays int
+	appendFn      appendFn
 	stopCh        chan struct{}
 	stopOnce      sync.Once
 }
 
 // newCleaner creates and starts a cleaner that purges expired audit log files.
 // It runs purgeExpiredFiles immediately, then every 24 hours.
-func newCleaner(baseDir string, retentionDays int) *cleaner {
+// The appendFn is called to record cleanup events in the audit log.
+func newCleaner(baseDir string, retentionDays int, fn appendFn) *cleaner {
 	c := &cleaner{
 		baseDir:       baseDir,
 		retentionDays: retentionDays,
+		appendFn:      fn,
 		stopCh:        make(chan struct{}),
 	}
 	go c.run()
@@ -75,7 +85,7 @@ func (c *cleaner) purgeExpiredFiles() {
 	cutoff := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, time.UTC).
 		AddDate(0, 0, -c.retentionDays)
 
-	removed := 0
+	var purgedDates []string
 	for _, entry := range entries {
 		if entry.IsDir() {
 			continue
@@ -100,13 +110,33 @@ func (c *cleaner) purgeExpiredFiles() {
 					slog.String("error", err.Error()))
 				continue
 			}
-			removed++
+			purgedDates = append(purgedDates, datePart)
 		}
 	}
 
-	if removed > 0 {
+	if len(purgedDates) > 0 {
 		slog.Info("fileaudit: purged expired audit log files",
-			slog.Int("removed", removed),
+			slog.Int("removed", len(purgedDates)),
 			slog.Int("retentionDays", c.retentionDays))
+
+		if c.appendFn != nil {
+			details, err := json.Marshal(map[string]any{
+				"purged_from":    purgedDates[0],
+				"purged_to":      purgedDates[len(purgedDates)-1],
+				"files_removed":  len(purgedDates),
+				"retention_days": c.retentionDays,
+			})
+			if err != nil {
+				slog.Warn("fileaudit: failed to marshal cleanup audit details",
+					slog.String("error", err.Error()))
+				details = []byte("{}")
+			}
+			entry := audit.NewEntry(audit.CategorySystem, "audit_cleanup", "", "system").
+				WithDetails(string(details))
+			if err := c.appendFn(context.Background(), entry); err != nil {
+				slog.Warn("fileaudit: failed to log cleanup audit entry",
+					slog.String("error", err.Error()))
+			}
+		}
 	}
 }

internal/persis/fileaudit/cleaner_test.go

@@ -1,11 +1,14 @@
 package fileaudit
 
 import (
+	"context"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/dagu-org/dagu/internal/service/audit"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -147,9 +150,7 @@ func TestPurgeExpiredFiles_ZeroRetentionSkipsCleanup(t *testing.T) {
 }
 
 func TestCleaner_StopIsIdempotent(t *testing.T) {
-	dir := t.TempDir()
-
-	c := newCleaner(dir, 7)
+	c := newTestCleaner(t.TempDir(), 7)
 
 	// Calling stop multiple times should not panic
 	assert.NotPanics(t, func() {
@@ -168,6 +169,57 @@ func TestPurgeExpiredFiles_NonexistentDirectory(t *testing.T) {
 	})
 }
 
+func TestPurgeExpiredFiles_LogsCleanupAuditEntry(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create two expired files with known dates
+	oldDate := time.Now().UTC().AddDate(0, 0, -30).Format(dateFormat)
+	olderDate := time.Now().UTC().AddDate(0, 0, -31).Format(dateFormat)
+	createTestFile(t, dir, oldDate+auditFileExtension)
+	createTestFile(t, dir, olderDate+auditFileExtension)
+
+	var mu sync.Mutex
+	var entries []*audit.Entry
+	fn := func(_ context.Context, entry *audit.Entry) error {
+		mu.Lock()
+		defer mu.Unlock()
+		entries = append(entries, entry)
+		return nil
+	}
+
+	c := &cleaner{baseDir: dir, retentionDays: 7, appendFn: fn, stopCh: make(chan struct{})}
+	c.purgeExpiredFiles()
+
+	require.Len(t, entries, 1)
+	assert.Equal(t, audit.CategorySystem, entries[0].Category)
+	assert.Equal(t, "audit_cleanup", entries[0].Action)
+	assert.Equal(t, "", entries[0].UserID)
+	assert.Equal(t, "system", entries[0].Username)
+	assert.Contains(t, entries[0].Details, `"files_removed":2`)
+	assert.Contains(t, entries[0].Details, `"retention_days":7`)
+	assert.Contains(t, entries[0].Details, `"purged_from"`)
+	assert.Contains(t, entries[0].Details, `"purged_to"`)
+}
+
+func TestPurgeExpiredFiles_NoEntryWhenNothingPurged(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create only a recent file — nothing to purge
+	recentDate := time.Now().UTC().AddDate(0, 0, -1).Format(dateFormat)
+	createTestFile(t, dir, recentDate+auditFileExtension)
+
+	called := false
+	fn := func(_ context.Context, _ *audit.Entry) error {
+		called = true
+		return nil
+	}
+
+	c := &cleaner{baseDir: dir, retentionDays: 7, appendFn: fn, stopCh: make(chan struct{})}
+	c.purgeExpiredFiles()
+
+	assert.False(t, called, "appendFn should not be called when no files are purged")
+}
+
 func TestPurgeExpiredFiles_BoundaryDate(t *testing.T) {
 	dir := t.TempDir()
 

internal/persis/fileaudit/store.go

@@ -54,7 +54,7 @@ func New(baseDir string, retentionDays int) (*Store, error) {
 	s := &Store{baseDir: baseDir}
 
 	if retentionDays > 0 {
-		s.cleaner = newCleaner(baseDir, retentionDays)
+		s.cleaner = newCleaner(baseDir, retentionDays, s.Append)
 	}
 
 	return s, nil

internal/service/audit/entry.go

@@ -19,6 +19,7 @@ const (
 	CategoryWebhook  Category = "webhook"
 	CategoryGitSync  Category = "git_sync"
 	CategoryAgent    Category = "agent"
+	CategorySystem   Category = "system"
 )
 
 // Entry represents a single audit log entry.

internal/service/frontend/api/v1/agent_config.go

@@ -86,7 +86,7 @@ func (a *API) UpdateAgentConfig(ctx context.Context, request api.UpdateAgentConf
 		return nil, errFailedToSaveAgentConfig
 	}
 
-	a.logAuditEntry(ctx, audit.CategoryAgent, auditActionAgentConfigUpdate, buildAgentConfigChanges(request.Body))
+	a.logAudit(ctx, audit.CategoryAgent, auditActionAgentConfigUpdate, buildAgentConfigChanges(request.Body))
 
 	return api.UpdateAgentConfig200JSONResponse(toAgentConfigResponse(cfg)), nil
 }

internal/service/frontend/api/v1/agent_models.go

@@ -148,7 +148,7 @@ func (a *API) CreateAgentModel(ctx context.Context, request api.CreateAgentModel
 	// If this is the first model, auto-set as default
 	a.autoSetDefaultModel(ctx, id)
 
-	a.logAuditEntry(ctx, audit.CategoryAgent, auditActionModelCreate, map[string]any{
+	a.logAudit(ctx, audit.CategoryAgent, auditActionModelCreate, map[string]any{
 		"model_id": id,
 		"name":     body.Name,
 		"provider": string(body.Provider),
@@ -196,7 +196,7 @@ func (a *API) UpdateAgentModel(ctx context.Context, request api.UpdateAgentModel
 		return nil, &Error{Code: api.ErrorCodeInternalError, Message: "Failed to update model", HTTPStatus: http.StatusInternalServerError}
 	}
 
-	a.logAuditEntry(ctx, audit.CategoryAgent, auditActionModelUpdate, map[string]any{
+	a.logAudit(ctx, audit.CategoryAgent, auditActionModelUpdate, map[string]any{
 		"model_id": request.ModelId,
 	})
 
@@ -223,7 +223,7 @@ func (a *API) DeleteAgentModel(ctx context.Context, request api.DeleteAgentModel
 	// If deleted model was default, reset to first remaining
 	a.resetDefaultIfNeeded(ctx, request.ModelId)
 
-	a.logAuditEntry(ctx, audit.CategoryAgent, auditActionModelDelete, map[string]any{
+	a.logAudit(ctx, audit.CategoryAgent, auditActionModelDelete, map[string]any{
 		"model_id": request.ModelId,
 	})
 
@@ -262,7 +262,7 @@ func (a *API) SetDefaultAgentModel(ctx context.Context, request api.SetDefaultAg
 		return nil, errFailedToSaveAgentConfig
 	}
 
-	a.logAuditEntry(ctx, audit.CategoryAgent, auditActionModelSetDef, map[string]any{
+	a.logAudit(ctx, audit.CategoryAgent, auditActionModelSetDef, map[string]any{
 		"model_id": modelID,
 	})
 

internal/service/frontend/api/v1/api.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"path"
 	"reflect"
@@ -469,22 +470,44 @@ func (a *API) requireUserManagement() error {
 	return nil
 }
 
-// logAuditEntry logs an audit entry with the specified category and action details.
-// It silently returns if the audit service is not configured or if no user is present in the context.
-func (a *API) logAuditEntry(ctx context.Context, category audit.Category, action string, details any) {
-	user, ok := auth.UserFromContext(ctx)
-	if a.auditService == nil || !ok {
+// logAudit logs an audit entry with the specified category, action, and details.
+// It silently returns if the audit service is not configured.
+// User and IP are extracted from context; missing user is allowed (recorded as empty).
+func (a *API) logAudit(ctx context.Context, category audit.Category, action string, details any) {
+	if a.auditService == nil {
 		return
 	}
 
-	detailsJSON, _ := json.Marshal(details)
+	var userID, username string
+	if user, ok := auth.UserFromContext(ctx); ok && user != nil {
+		userID = user.ID
+		username = user.Username
+	}
+
 	clientIP, _ := auth.ClientIPFromContext(ctx)
 
-	entry := audit.NewEntry(category, action, user.ID, user.Username).
-		WithDetails(string(detailsJSON)).
+	var detailsStr string
+	if details != nil {
+		detailsJSON, err := json.Marshal(details)
+		if err != nil {
+			logger.Warn(ctx, "Failed to marshal audit details", tag.Error(err))
+			detailsStr = "{}"
+		} else {
+			detailsStr = string(detailsJSON)
+		}
+	}
+
+	entry := audit.NewEntry(category, action, userID, username).
+		WithDetails(detailsStr).
 		WithIPAddress(clientIP)
 
-	_ = a.auditService.Log(ctx, entry)
+	if err := a.auditService.Log(ctx, entry); err != nil {
+		logger.Warn(ctx, "Failed to write audit log",
+			tag.Error(err),
+			slog.String("action", action),
+			slog.String("category", string(category)),
+		)
+	}
 }
 
 // ptrOf returns a pointer to v, or nil if v is the zero value for its type.