From 410cbbcdeb675f0f00e7730199ce1da6bf3915c7 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 07:40:51 -0300 Subject: [PATCH 01/16] feat: risk notification jobs --- .env.example | 3 +- docs/docs.go | 10 + docs/swagger.json | 10 + docs/swagger.yaml | 8 + internal/api/handler/users.go | 8 + .../api/handler/users_integration_test.go | 11 +- internal/config/config.go | 12 + internal/config/risk.go | 114 +++ internal/config/risk_test.go | 146 ++++ .../service/email/templates/service_test.go | 29 + .../templates/risk-review-due-reminder.html | 14 + .../templates/risk-review-due-reminder.txt | 10 + .../risk-review-overdue-escalation.html | 14 + .../risk-review-overdue-escalation.txt | 10 + .../templates/risk-stale-open-reminder.html | 14 + .../templates/risk-stale-open-reminder.txt | 10 + internal/service/relational/ccf_internal.go | 3 + internal/service/worker/jobs.go | 16 + internal/service/worker/risk_job_types.go | 112 +++ internal/service/worker/risk_workers.go | 679 ++++++++++++++++++ internal/service/worker/risk_workers_test.go | 342 +++++++++ internal/service/worker/service.go | 120 ++++ internal/service/worker/service_test.go | 19 + internal/service/worker/user_repository.go | 1 + risk.yaml | 14 + 25 files changed, 1726 insertions(+), 3 deletions(-) create mode 100644 internal/config/risk.go create mode 100644 internal/config/risk_test.go create mode 100644 internal/service/email/templates/templates/risk-review-due-reminder.html create mode 100644 internal/service/email/templates/templates/risk-review-due-reminder.txt create mode 100644 internal/service/email/templates/templates/risk-review-overdue-escalation.html create mode 100644 internal/service/email/templates/templates/risk-review-overdue-escalation.txt create mode 100644 internal/service/email/templates/templates/risk-stale-open-reminder.html create mode 100644 internal/service/email/templates/templates/risk-stale-open-reminder.txt create mode 100644 internal/service/worker/risk_job_types.go create mode 100644 internal/service/worker/risk_workers.go create mode 100644 internal/service/worker/risk_workers_test.go create mode 100644 risk.yaml diff --git a/.env.example b/.env.example index b625355f..c3872c83 100644 --- a/.env.example +++ b/.env.example @@ -8,8 +8,9 @@ CCF_JWT_PRIVATE_KEY=private.pem CCF_JWT_PUBLIC_KEY=public.pem CCF_API_ALLOWED_ORIGINS="http://localhost:3000,http://localhost:8000" +CCF_RISK_CONFIG="risk.yaml" CCF_ENVIRONMENT="" # Defaults to production. ## This configuration disables cookie setting to allow testing on Safari ## It is insecure so use it with caution -#CCF_ENVIRONMENT="local" \ No newline at end of file +#CCF_ENVIRONMENT="local" diff --git a/docs/docs.go b/docs/docs.go index 8c3b170c..706d84e0 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -23548,6 +23548,9 @@ const docTemplate = `{ "handler.SubscriptionsResponse": { "type": "object", "properties": { + "riskNotificationsSubscribed": { + "type": "boolean" + }, "subscribed": { "type": "boolean" }, @@ -23562,6 +23565,9 @@ const docTemplate = `{ "handler.UpdateSubscriptionsRequest": { "type": "object", "properties": { + "riskNotificationsSubscribed": { + "type": "boolean" + }, "subscribed": { "type": "boolean" }, @@ -31532,6 +31538,10 @@ const docTemplate = `{ "lastName": { "type": "string" }, + "riskNotificationsSubscribed": { + "description": "RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications", + "type": "boolean" + }, "taskAvailableEmailSubscribed": { "description": "TaskAvailableEmailSubscribed indicates if the user wants an email when tasks become available", "type": "boolean" diff --git a/docs/swagger.json b/docs/swagger.json index b278c91c..9a13d4e3 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -23542,6 +23542,9 @@ "handler.SubscriptionsResponse": { "type": "object", "properties": { + "riskNotificationsSubscribed": { + "type": "boolean" + }, "subscribed": { "type": "boolean" }, @@ -23556,6 +23559,9 @@ "handler.UpdateSubscriptionsRequest": { "type": "object", "properties": { + "riskNotificationsSubscribed": { + "type": "boolean" + }, "subscribed": { "type": "boolean" }, @@ -31526,6 +31532,10 @@ "lastName": { "type": "string" }, + "riskNotificationsSubscribed": { + "description": "RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications", + "type": "boolean" + }, "taskAvailableEmailSubscribed": { "description": "TaskAvailableEmailSubscribed indicates if the user wants an email when tasks become available", "type": "boolean" diff --git a/docs/swagger.yaml b/docs/swagger.yaml index ff9e2c56..05061ae3 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1344,6 +1344,8 @@ definitions: type: object handler.SubscriptionsResponse: properties: + riskNotificationsSubscribed: + type: boolean subscribed: type: boolean taskAvailableEmailSubscribed: @@ -1353,6 +1355,8 @@ definitions: type: object handler.UpdateSubscriptionsRequest: properties: + riskNotificationsSubscribed: + type: boolean subscribed: type: boolean taskAvailableEmailSubscribed: @@ -6624,6 +6628,10 @@ definitions: type: string lastName: type: string + riskNotificationsSubscribed: + description: RiskNotificationsSubscribed indicates if the user wants to receive + risk lifecycle notifications + type: boolean taskAvailableEmailSubscribed: description: TaskAvailableEmailSubscribed indicates if the user wants an email when tasks become available diff --git a/internal/api/handler/users.go b/internal/api/handler/users.go index f95098c8..dd23e790 100644 --- a/internal/api/handler/users.go +++ b/internal/api/handler/users.go @@ -27,12 +27,14 @@ type SubscriptionsResponse struct { Subscribed bool `json:"subscribed"` TaskAvailableEmailSubscribed bool `json:"taskAvailableEmailSubscribed"` TaskDailyDigestSubscribed bool `json:"taskDailyDigestSubscribed"` + RiskNotificationsSubscribed bool `json:"riskNotificationsSubscribed"` } type UpdateSubscriptionsRequest struct { Subscribed *bool `json:"subscribed"` TaskAvailableEmailSubscribed *bool `json:"taskAvailableEmailSubscribed"` TaskDailyDigestSubscribed *bool `json:"taskDailyDigestSubscribed"` + RiskNotificationsSubscribed *bool `json:"riskNotificationsSubscribed"` } func NewUserHandler(sugar *zap.SugaredLogger, db *gorm.DB) *UserHandler { @@ -437,6 +439,7 @@ func (h *UserHandler) GetSubscriptions(ctx echo.Context) error { Subscribed: user.DigestSubscribed, TaskAvailableEmailSubscribed: user.TaskAvailableEmailSubscribed, TaskDailyDigestSubscribed: user.TaskDailyDigestSubscribed, + RiskNotificationsSubscribed: user.RiskNotificationsSubscribed, }, }) } @@ -484,6 +487,9 @@ func (h *UserHandler) UpdateSubscriptions(ctx echo.Context) error { if req.TaskDailyDigestSubscribed != nil { user.TaskDailyDigestSubscribed = *req.TaskDailyDigestSubscribed } + if req.RiskNotificationsSubscribed != nil { + user.RiskNotificationsSubscribed = *req.RiskNotificationsSubscribed + } if err := h.db.Save(&user).Error; err != nil { h.sugar.Errorw("Failed to update user subscriptions", "error", err) @@ -496,6 +502,7 @@ func (h *UserHandler) UpdateSubscriptions(ctx echo.Context) error { "subscribed", user.DigestSubscribed, "taskAvailableEmailSubscribed", user.TaskAvailableEmailSubscribed, "taskDailyDigestSubscribed", user.TaskDailyDigestSubscribed, + "riskNotificationsSubscribed", user.RiskNotificationsSubscribed, ) return ctx.JSON(200, GenericDataResponse[SubscriptionsResponse]{ @@ -503,6 +510,7 @@ func (h *UserHandler) UpdateSubscriptions(ctx echo.Context) error { Subscribed: user.DigestSubscribed, TaskAvailableEmailSubscribed: user.TaskAvailableEmailSubscribed, TaskDailyDigestSubscribed: user.TaskDailyDigestSubscribed, + RiskNotificationsSubscribed: user.RiskNotificationsSubscribed, }, }) } diff --git a/internal/api/handler/users_integration_test.go b/internal/api/handler/users_integration_test.go index 5d4a85d8..33d65ca9 100644 --- a/internal/api/handler/users_integration_test.go +++ b/internal/api/handler/users_integration_test.go @@ -403,6 +403,7 @@ func (suite *UserApiIntegrationSuite) TestSubscriptions() { Subscribed bool `json:"subscribed"` TaskAvailableEmailSubscribed bool `json:"taskAvailableEmailSubscribed"` TaskDailyDigestSubscribed bool `json:"taskDailyDigestSubscribed"` + RiskNotificationsSubscribed bool `json:"riskNotificationsSubscribed"` } `json:"data"` } err = json.Unmarshal(rec.Body.Bytes(), &response) @@ -412,6 +413,7 @@ func (suite *UserApiIntegrationSuite) TestSubscriptions() { suite.False(response.Data.Subscribed, "Expected default digest subscription to be false") suite.False(response.Data.TaskAvailableEmailSubscribed, "Expected task available email subscription to default to false") suite.False(response.Data.TaskDailyDigestSubscribed, "Expected task daily digest subscription to default to false") + suite.True(response.Data.RiskNotificationsSubscribed, "Expected risk notifications subscription to default to true") }) suite.Run("UpdateSubscriptions", func() { @@ -420,6 +422,7 @@ func (suite *UserApiIntegrationSuite) TestSubscriptions() { "subscribed": true, "taskAvailableEmailSubscribed": true, "taskDailyDigestSubscribed": true, + "riskNotificationsSubscribed": false, } payloadJSON, err := json.Marshal(payload) suite.Require().NoError(err, "Failed to marshal update subscriptions request") @@ -437,6 +440,7 @@ func (suite *UserApiIntegrationSuite) TestSubscriptions() { Subscribed bool `json:"subscribed"` TaskAvailableEmailSubscribed bool `json:"taskAvailableEmailSubscribed"` TaskDailyDigestSubscribed bool `json:"taskDailyDigestSubscribed"` + RiskNotificationsSubscribed bool `json:"riskNotificationsSubscribed"` } `json:"data"` } err = json.Unmarshal(rec.Body.Bytes(), &response) @@ -445,11 +449,13 @@ func (suite *UserApiIntegrationSuite) TestSubscriptions() { suite.True(response.Data.Subscribed, "Expected digest subscription to be updated to true") suite.True(response.Data.TaskAvailableEmailSubscribed, "Expected task available email subscription to be updated to true") suite.True(response.Data.TaskDailyDigestSubscribed, "Expected task daily digest subscription to be updated to true") + suite.False(response.Data.RiskNotificationsSubscribed, "Expected risk notifications subscription to be updated to false") // Test unsubscribing from digest payload = map[string]interface{}{ - "subscribed": false, - "taskDailyDigestSubscribed": false, + "subscribed": false, + "taskDailyDigestSubscribed": false, + "riskNotificationsSubscribed": true, } payloadJSON, err = json.Marshal(payload) suite.Require().NoError(err, "Failed to marshal unsubscribe request") @@ -468,6 +474,7 @@ func (suite *UserApiIntegrationSuite) TestSubscriptions() { suite.False(response.Data.Subscribed, "Expected digest subscription to be updated to false") suite.True(response.Data.TaskAvailableEmailSubscribed, "Expected task available email subscription to remain unchanged when omitted") suite.False(response.Data.TaskDailyDigestSubscribed, "Expected task daily digest subscription to be updated to false") + suite.True(response.Data.RiskNotificationsSubscribed, "Expected risk notifications subscription to be updated to true") }) suite.Run("UpdateSubscriptionsInvalidPayload", func() { diff --git a/internal/config/config.go b/internal/config/config.go index c0ed9fb4..a71f55c5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -38,6 +38,7 @@ type Config struct { DigestEnabled bool // Enable or disable the digest scheduler DigestSchedule string // Cron schedule for digest emails Workflow *WorkflowConfig + Risk *RiskConfig } func NewConfig(logger *zap.SugaredLogger) *Config { @@ -174,6 +175,16 @@ func NewConfig(logger *zap.SugaredLogger) *Config { workflowConfig = &WorkflowConfig{SchedulerEnabled: false} } + riskConfigPath := viper.GetString("risk_config") + if riskConfigPath == "" { + riskConfigPath = "risk.yaml" + } + riskConfig, err := LoadRiskConfig(riskConfigPath) + if err != nil { + logger.Warnw("Failed to load risk config, risk jobs will be disabled", "error", err, "path", riskConfigPath) + riskConfig = DefaultRiskConfig() + } + // Worker configuration workerConfig := DefaultWorkerConfig() if viper.IsSet("worker_enabled") { @@ -206,6 +217,7 @@ func NewConfig(logger *zap.SugaredLogger) *Config { DigestEnabled: digestEnabled, DigestSchedule: digestSchedule, Workflow: workflowConfig, + Risk: riskConfig, } } diff --git a/internal/config/risk.go b/internal/config/risk.go new file mode 100644 index 00000000..e95e6203 --- /dev/null +++ b/internal/config/risk.go @@ -0,0 +1,114 @@ +package config + +import ( + "errors" + "fmt" + "strings" + + "github.com/robfig/cron/v3" + "github.com/spf13/viper" +) + +// RiskConfig contains configuration for risk-related periodic workers. +type RiskConfig struct { + ReviewDeadlineReminderEnabled bool `mapstructure:"review_deadline_reminder_enabled" yaml:"review_deadline_reminder_enabled" json:"reviewDeadlineReminderEnabled"` + ReviewDeadlineReminderSchedule string `mapstructure:"review_deadline_reminder_schedule" yaml:"review_deadline_reminder_schedule" json:"reviewDeadlineReminderSchedule"` + + ReviewOverdueEscalationEnabled bool `mapstructure:"review_overdue_escalation_enabled" yaml:"review_overdue_escalation_enabled" json:"reviewOverdueEscalationEnabled"` + ReviewOverdueEscalationSchedule string `mapstructure:"review_overdue_escalation_schedule" yaml:"review_overdue_escalation_schedule" json:"reviewOverdueEscalationSchedule"` + + StaleRiskScannerEnabled bool `mapstructure:"stale_risk_scanner_enabled" yaml:"stale_risk_scanner_enabled" json:"staleRiskScannerEnabled"` + StaleRiskScannerSchedule string `mapstructure:"stale_risk_scanner_schedule" yaml:"stale_risk_scanner_schedule" json:"staleRiskScannerSchedule"` + + EvidenceReconciliationEnabled bool `mapstructure:"evidence_reconciliation_enabled" yaml:"evidence_reconciliation_enabled" json:"evidenceReconciliationEnabled"` + EvidenceReconciliationSchedule string `mapstructure:"evidence_reconciliation_schedule" yaml:"evidence_reconciliation_schedule" json:"evidenceReconciliationSchedule"` + + AutoReopenEnabled bool `mapstructure:"auto_reopen_enabled" yaml:"auto_reopen_enabled" json:"autoReopenEnabled"` + AutoReopenThresholdDays int `mapstructure:"auto_reopen_threshold_days" yaml:"auto_reopen_threshold_days" json:"autoReopenThresholdDays"` +} + +func DefaultRiskConfig() *RiskConfig { + return &RiskConfig{ + ReviewDeadlineReminderEnabled: false, + ReviewDeadlineReminderSchedule: "0 0 8 * * *", + ReviewOverdueEscalationEnabled: false, + ReviewOverdueEscalationSchedule: "0 0 9 * * *", + StaleRiskScannerEnabled: false, + StaleRiskScannerSchedule: "0 0 10 * * 1", + EvidenceReconciliationEnabled: false, + EvidenceReconciliationSchedule: "0 30 10 * * *", + AutoReopenEnabled: false, + AutoReopenThresholdDays: 30, + } +} + +func LoadRiskConfig(path string) (*RiskConfig, error) { + v := viper.NewWithOptions(viper.KeyDelimiter("::")) + + def := DefaultRiskConfig() + v.SetDefault("review_deadline_reminder_enabled", def.ReviewDeadlineReminderEnabled) + v.SetDefault("review_deadline_reminder_schedule", def.ReviewDeadlineReminderSchedule) + v.SetDefault("review_overdue_escalation_enabled", def.ReviewOverdueEscalationEnabled) + v.SetDefault("review_overdue_escalation_schedule", def.ReviewOverdueEscalationSchedule) + v.SetDefault("stale_risk_scanner_enabled", def.StaleRiskScannerEnabled) + v.SetDefault("stale_risk_scanner_schedule", def.StaleRiskScannerSchedule) + v.SetDefault("evidence_reconciliation_enabled", def.EvidenceReconciliationEnabled) + v.SetDefault("evidence_reconciliation_schedule", def.EvidenceReconciliationSchedule) + v.SetDefault("auto_reopen_enabled", def.AutoReopenEnabled) + v.SetDefault("auto_reopen_threshold_days", def.AutoReopenThresholdDays) + + v.SetEnvPrefix("CCF_RISK") + v.SetEnvKeyReplacer(strings.NewReplacer("::", "_", ".", "_", "-", "_")) + v.AutomaticEnv() + + if path != "" { + v.SetConfigFile(path) + v.SetConfigType("yaml") + if err := v.ReadInConfig(); err != nil { + var notFound viper.ConfigFileNotFoundError + if !errors.As(err, ¬Found) { + return nil, fmt.Errorf("failed to read risk config file: %w", err) + } + } + } + + var cfg RiskConfig + if err := v.Unmarshal(&cfg); err != nil { + return nil, fmt.Errorf("failed to parse risk config: %w", err) + } + if err := cfg.Validate(); err != nil { + return nil, err + } + + return &cfg, nil +} + +func (c *RiskConfig) Validate() error { + parser := cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor) + + if c.ReviewDeadlineReminderEnabled { + if _, err := parser.Parse(c.ReviewDeadlineReminderSchedule); err != nil { + return fmt.Errorf("invalid review_deadline_reminder_schedule: %w", err) + } + } + if c.ReviewOverdueEscalationEnabled { + if _, err := parser.Parse(c.ReviewOverdueEscalationSchedule); err != nil { + return fmt.Errorf("invalid review_overdue_escalation_schedule: %w", err) + } + } + if c.StaleRiskScannerEnabled { + if _, err := parser.Parse(c.StaleRiskScannerSchedule); err != nil { + return fmt.Errorf("invalid stale_risk_scanner_schedule: %w", err) + } + } + if c.EvidenceReconciliationEnabled { + if _, err := parser.Parse(c.EvidenceReconciliationSchedule); err != nil { + return fmt.Errorf("invalid evidence_reconciliation_schedule: %w", err) + } + } + if c.AutoReopenThresholdDays < 0 { + return fmt.Errorf("risk auto reopen threshold days must be non-negative") + } + + return nil +} diff --git a/internal/config/risk_test.go b/internal/config/risk_test.go new file mode 100644 index 00000000..856a0e82 --- /dev/null +++ b/internal/config/risk_test.go @@ -0,0 +1,146 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDefaultRiskConfig(t *testing.T) { + cfg := DefaultRiskConfig() + assert.False(t, cfg.ReviewDeadlineReminderEnabled) + assert.Equal(t, "0 0 8 * * *", cfg.ReviewDeadlineReminderSchedule) + assert.False(t, cfg.ReviewOverdueEscalationEnabled) + assert.Equal(t, "0 0 9 * * *", cfg.ReviewOverdueEscalationSchedule) + assert.False(t, cfg.StaleRiskScannerEnabled) + assert.Equal(t, "0 0 10 * * 1", cfg.StaleRiskScannerSchedule) + assert.False(t, cfg.EvidenceReconciliationEnabled) + assert.Equal(t, "0 30 10 * * *", cfg.EvidenceReconciliationSchedule) + assert.False(t, cfg.AutoReopenEnabled) + assert.Equal(t, 30, cfg.AutoReopenThresholdDays) +} + +func TestLoadRiskConfigDefaults(t *testing.T) { + require.NoError(t, os.Unsetenv("CCF_RISK_REVIEW_DEADLINE_REMINDER_ENABLED")) + require.NoError(t, os.Unsetenv("CCF_RISK_REVIEW_DEADLINE_REMINDER_SCHEDULE")) + require.NoError(t, os.Unsetenv("CCF_RISK_REVIEW_OVERDUE_ESCALATION_ENABLED")) + require.NoError(t, os.Unsetenv("CCF_RISK_REVIEW_OVERDUE_ESCALATION_SCHEDULE")) + require.NoError(t, os.Unsetenv("CCF_RISK_STALE_RISK_SCANNER_ENABLED")) + require.NoError(t, os.Unsetenv("CCF_RISK_STALE_RISK_SCANNER_SCHEDULE")) + require.NoError(t, os.Unsetenv("CCF_RISK_EVIDENCE_RECONCILIATION_ENABLED")) + require.NoError(t, os.Unsetenv("CCF_RISK_EVIDENCE_RECONCILIATION_SCHEDULE")) + require.NoError(t, os.Unsetenv("CCF_RISK_AUTO_REOPEN_ENABLED")) + require.NoError(t, os.Unsetenv("CCF_RISK_AUTO_REOPEN_THRESHOLD_DAYS")) + + cfg, err := LoadRiskConfig("") + require.NoError(t, err) + assert.False(t, cfg.ReviewDeadlineReminderEnabled) + assert.False(t, cfg.ReviewOverdueEscalationEnabled) + assert.False(t, cfg.StaleRiskScannerEnabled) + assert.False(t, cfg.EvidenceReconciliationEnabled) + assert.False(t, cfg.AutoReopenEnabled) + assert.Equal(t, 30, cfg.AutoReopenThresholdDays) +} + +func TestLoadRiskConfigFromEnv(t *testing.T) { + require.NoError(t, os.Setenv("CCF_RISK_REVIEW_DEADLINE_REMINDER_ENABLED", "true")) + require.NoError(t, os.Setenv("CCF_RISK_REVIEW_DEADLINE_REMINDER_SCHEDULE", "0 15 8 * * *")) + require.NoError(t, os.Setenv("CCF_RISK_AUTO_REOPEN_ENABLED", "true")) + require.NoError(t, os.Setenv("CCF_RISK_AUTO_REOPEN_THRESHOLD_DAYS", "45")) + defer func() { + _ = os.Unsetenv("CCF_RISK_REVIEW_DEADLINE_REMINDER_ENABLED") + _ = os.Unsetenv("CCF_RISK_REVIEW_DEADLINE_REMINDER_SCHEDULE") + _ = os.Unsetenv("CCF_RISK_AUTO_REOPEN_ENABLED") + _ = os.Unsetenv("CCF_RISK_AUTO_REOPEN_THRESHOLD_DAYS") + }() + + cfg, err := LoadRiskConfig("") + require.NoError(t, err) + assert.True(t, cfg.ReviewDeadlineReminderEnabled) + assert.Equal(t, "0 15 8 * * *", cfg.ReviewDeadlineReminderSchedule) + assert.True(t, cfg.AutoReopenEnabled) + assert.Equal(t, 45, cfg.AutoReopenThresholdDays) +} + +func TestLoadRiskConfigFromFile(t *testing.T) { + content := ` +review_deadline_reminder_enabled: true +review_deadline_reminder_schedule: "0 0 8 * * *" +review_overdue_escalation_enabled: true +review_overdue_escalation_schedule: "0 0 9 * * *" +stale_risk_scanner_enabled: true +stale_risk_scanner_schedule: "0 0 10 * * 1" +evidence_reconciliation_enabled: true +evidence_reconciliation_schedule: "0 30 10 * * *" +auto_reopen_enabled: true +auto_reopen_threshold_days: 60 +` + tmpDir := t.TempDir() + cfgPath := filepath.Join(tmpDir, "risk.yaml") + require.NoError(t, os.WriteFile(cfgPath, []byte(content), 0644)) + + cfg, err := LoadRiskConfig(cfgPath) + require.NoError(t, err) + assert.True(t, cfg.ReviewDeadlineReminderEnabled) + assert.True(t, cfg.ReviewOverdueEscalationEnabled) + assert.True(t, cfg.StaleRiskScannerEnabled) + assert.True(t, cfg.EvidenceReconciliationEnabled) + assert.True(t, cfg.AutoReopenEnabled) + assert.Equal(t, 60, cfg.AutoReopenThresholdDays) +} + +func TestRiskConfigValidate(t *testing.T) { + tests := []struct { + name string + cfg *RiskConfig + wantErr bool + }{ + { + name: "valid config", + cfg: &RiskConfig{ + ReviewDeadlineReminderEnabled: true, + ReviewDeadlineReminderSchedule: "0 0 8 * * *", + AutoReopenThresholdDays: 30, + }, + wantErr: false, + }, + { + name: "invalid reminder schedule", + cfg: &RiskConfig{ + ReviewDeadlineReminderEnabled: true, + ReviewDeadlineReminderSchedule: "not-cron", + }, + wantErr: true, + }, + { + name: "negative threshold", + cfg: &RiskConfig{ + AutoReopenThresholdDays: -1, + }, + wantErr: true, + }, + { + name: "disabled schedule may be invalid", + cfg: &RiskConfig{ + ReviewDeadlineReminderEnabled: false, + ReviewDeadlineReminderSchedule: "not-cron", + AutoReopenThresholdDays: 0, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.cfg.Validate() + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} diff --git a/internal/service/email/templates/service_test.go b/internal/service/email/templates/service_test.go index baf9f814..216d07de 100644 --- a/internal/service/email/templates/service_test.go +++ b/internal/service/email/templates/service_test.go @@ -250,6 +250,35 @@ func TestTemplateService_WorkflowTaskDigest_EmptyTasks(t *testing.T) { require.Contains(t, html, "Bob") } +func TestTemplateService_RiskTemplates(t *testing.T) { + service, err := NewTemplateService() + require.NoError(t, err) + + data := TemplateData{ + "OwnerName": "Alice Smith", + "RiskTitle": "Weak MFA Controls", + "SSPName": "GoodRead SSP", + "RiskStatus": "risk-accepted", + "ReviewDeadline": "01/mar/2026", + "LastSeenAt": "15/feb/2026", + "RiskURL": "https://app.example.com/risks/123", + } + + for _, name := range []string{ + "risk-review-due-reminder", + "risk-review-overdue-escalation", + "risk-stale-open-reminder", + } { + html, text, err := service.Use(name, data) + require.NoError(t, err) + require.NotEmpty(t, html) + require.NotEmpty(t, text) + require.Contains(t, html, "Alice Smith") + require.Contains(t, text, "Weak MFA Controls") + require.Contains(t, text, "https://app.example.com/risks/123") + } +} + func TestTemplateService_ListTemplates(t *testing.T) { service, err := NewTemplateService() require.NoError(t, err, "Failed to create template service") diff --git a/internal/service/email/templates/templates/risk-review-due-reminder.html b/internal/service/email/templates/templates/risk-review-due-reminder.html new file mode 100644 index 00000000..2e0088fd --- /dev/null +++ b/internal/service/email/templates/templates/risk-review-due-reminder.html @@ -0,0 +1,14 @@ + + + +

Hello {{.OwnerName}},

+

A risk review deadline is approaching.

+ +

Open risk

+ + diff --git a/internal/service/email/templates/templates/risk-review-due-reminder.txt b/internal/service/email/templates/templates/risk-review-due-reminder.txt new file mode 100644 index 00000000..50e7c0e7 --- /dev/null +++ b/internal/service/email/templates/templates/risk-review-due-reminder.txt @@ -0,0 +1,10 @@ +Hello {{.OwnerName}}, + +A risk review deadline is approaching. + +Risk: {{.RiskTitle}} +SSP: {{.SSPName}} +Status: {{.RiskStatus}} +Review deadline: {{.ReviewDeadline}} + +Open risk: {{.RiskURL}} diff --git a/internal/service/email/templates/templates/risk-review-overdue-escalation.html b/internal/service/email/templates/templates/risk-review-overdue-escalation.html new file mode 100644 index 00000000..18b2f902 --- /dev/null +++ b/internal/service/email/templates/templates/risk-review-overdue-escalation.html @@ -0,0 +1,14 @@ + + + +

Hello {{.OwnerName}},

+

A risk review is overdue and requires attention.

+ +

Open risk

+ + diff --git a/internal/service/email/templates/templates/risk-review-overdue-escalation.txt b/internal/service/email/templates/templates/risk-review-overdue-escalation.txt new file mode 100644 index 00000000..7768e75f --- /dev/null +++ b/internal/service/email/templates/templates/risk-review-overdue-escalation.txt @@ -0,0 +1,10 @@ +Hello {{.OwnerName}}, + +A risk review is overdue and requires attention. + +Risk: {{.RiskTitle}} +SSP: {{.SSPName}} +Status: {{.RiskStatus}} +Review deadline: {{.ReviewDeadline}} + +Open risk: {{.RiskURL}} diff --git a/internal/service/email/templates/templates/risk-stale-open-reminder.html b/internal/service/email/templates/templates/risk-stale-open-reminder.html new file mode 100644 index 00000000..98fd8353 --- /dev/null +++ b/internal/service/email/templates/templates/risk-stale-open-reminder.html @@ -0,0 +1,14 @@ + + + +

Hello {{.OwnerName}},

+

A risk appears stale and should be reviewed.

+ +

Open risk

+ + diff --git a/internal/service/email/templates/templates/risk-stale-open-reminder.txt b/internal/service/email/templates/templates/risk-stale-open-reminder.txt new file mode 100644 index 00000000..484ba6f5 --- /dev/null +++ b/internal/service/email/templates/templates/risk-stale-open-reminder.txt @@ -0,0 +1,10 @@ +Hello {{.OwnerName}}, + +A risk appears stale and should be reviewed. + +Risk: {{.RiskTitle}} +SSP: {{.SSPName}} +Status: {{.RiskStatus}} +Last seen: {{.LastSeenAt}} + +Open risk: {{.RiskURL}} diff --git a/internal/service/relational/ccf_internal.go b/internal/service/relational/ccf_internal.go index 4ee7dbf5..bfdfa2a9 100644 --- a/internal/service/relational/ccf_internal.go +++ b/internal/service/relational/ccf_internal.go @@ -35,6 +35,9 @@ type User struct { TaskAvailableEmailSubscribed bool `json:"taskAvailableEmailSubscribed" gorm:"default:false"` // TaskDailyDigestSubscribed indicates if the user wants to receive a daily task digest email TaskDailyDigestSubscribed bool `json:"taskDailyDigestSubscribed" gorm:"default:false"` + + // RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications + RiskNotificationsSubscribed bool `json:"riskNotificationsSubscribed" gorm:"default:true"` } func (User) TableName() string { diff --git a/internal/service/worker/jobs.go b/internal/service/worker/jobs.go index a8c9628a..91aeb51c 100644 --- a/internal/service/worker/jobs.go +++ b/internal/service/worker/jobs.go @@ -173,6 +173,7 @@ type NotificationUser struct { LastName string TaskAvailableEmailSubscribed bool TaskDailyDigestSubscribed bool + RiskNotificationsSubscribed bool } func (u NotificationUser) FullName() string { @@ -660,6 +661,12 @@ func Workers(emailService EmailService, digestService DigestService, userRepo Us if db != nil { riskEvidenceWorker := NewRiskEvidenceWorker(db, logger) river.AddWorker(workers, river.WorkFunc(riskEvidenceWorker.Work)) + + riskReconcileDuplicatesWorker := NewRiskReconcileDuplicatesWorker(db, logger) + river.AddWorker(workers, river.WorkFunc(riskReconcileDuplicatesWorker.Work)) + + riskReviewOverdueReopenWorker := NewRiskReviewOverdueReopenWorker(db, logger) + river.AddWorker(workers, river.WorkFunc(riskReviewOverdueReopenWorker.Work)) } // Register workflow notification workers if dependencies are available @@ -676,6 +683,15 @@ func Workers(emailService EmailService, digestService DigestService, userRepo Us workflowExecutionFailedWorker := NewWorkflowExecutionFailedWorker(db, emailService, userRepo, webBaseURL, logger) river.AddWorker(workers, river.WorkFunc(workflowExecutionFailedWorker.Work)) + + riskReviewDueReminderWorker := NewRiskReviewDueReminderWorker(db, emailService, userRepo, webBaseURL, logger) + river.AddWorker(workers, river.WorkFunc(riskReviewDueReminderWorker.Work)) + + riskReviewOverdueEscalationWorker := NewRiskReviewOverdueEscalationWorker(db, emailService, userRepo, webBaseURL, logger) + river.AddWorker(workers, river.WorkFunc(riskReviewOverdueEscalationWorker.Work)) + + riskStaleOpenReminderWorker := NewRiskStaleOpenReminderWorker(db, emailService, userRepo, webBaseURL, logger) + river.AddWorker(workers, river.WorkFunc(riskStaleOpenReminderWorker.Work)) } } diff --git a/internal/service/worker/risk_job_types.go b/internal/service/worker/risk_job_types.go new file mode 100644 index 00000000..c8ee6ebb --- /dev/null +++ b/internal/service/worker/risk_job_types.go @@ -0,0 +1,112 @@ +package worker + +import ( + "time" + + "github.com/google/uuid" + "github.com/riverqueue/river" +) + +const ( + JobTypeRiskReviewDeadlineReminderScanner = "risk_review_deadline_reminder_scanner" + JobTypeRiskReviewOverdueEscalationScanner = "risk_review_overdue_escalation_scanner" + JobTypeRiskStaleRiskScanner = "risk_stale_risk_scanner" + JobTypeRiskEvidenceReconciliationScanner = "risk_evidence_reconciliation_scanner" + + JobTypeRiskReviewDueReminder = "risk_review_due_reminder" + JobTypeRiskReviewOverdueEscalation = "risk_review_overdue_escalation" + JobTypeRiskStaleOpenReminder = "risk_stale_open_reminder" + JobTypeRiskReconcileDuplicates = "risk_reconcile_duplicates" + JobTypeRiskReviewOverdueReopen = "risk_review_overdue_reopen" +) + +type RiskReviewDeadlineReminderScannerArgs struct{} +type RiskReviewOverdueEscalationScannerArgs struct{} +type RiskStaleRiskScannerArgs struct{} +type RiskEvidenceReconciliationScannerArgs struct{} + +type RiskReviewDueReminderArgs struct { + RiskID uuid.UUID `json:"risk_id"` + OwnerUserID uuid.UUID `json:"owner_user_id"` + ReviewDeadline string `json:"review_deadline"` + ReminderWindow string `json:"reminder_window"` +} + +type RiskReviewOverdueEscalationArgs struct { + RiskID uuid.UUID `json:"risk_id"` + OwnerUserID uuid.UUID `json:"owner_user_id"` + ReviewDeadline string `json:"review_deadline"` + OverdueWindow string `json:"overdue_window"` +} + +type RiskStaleOpenReminderArgs struct { + RiskID uuid.UUID `json:"risk_id"` + OwnerUserID uuid.UUID `json:"owner_user_id"` + LastSeenAt string `json:"last_seen_at"` + StaleBucketDate string `json:"stale_bucket_date"` +} + +type RiskReconcileDuplicatesArgs struct { + DedupeKey string `json:"dedupe_key"` +} + +type RiskReviewOverdueReopenArgs struct { + RiskID uuid.UUID `json:"risk_id"` + ReviewDeadline string `json:"review_deadline"` + ThresholdDays int `json:"threshold_days"` +} + +func (RiskReviewDeadlineReminderScannerArgs) Kind() string { + return JobTypeRiskReviewDeadlineReminderScanner +} +func (RiskReviewOverdueEscalationScannerArgs) Kind() string { + return JobTypeRiskReviewOverdueEscalationScanner +} +func (RiskStaleRiskScannerArgs) Kind() string { return JobTypeRiskStaleRiskScanner } +func (RiskEvidenceReconciliationScannerArgs) Kind() string { + return JobTypeRiskEvidenceReconciliationScanner +} +func (RiskReviewDueReminderArgs) Kind() string { return JobTypeRiskReviewDueReminder } +func (RiskReviewOverdueEscalationArgs) Kind() string { return JobTypeRiskReviewOverdueEscalation } +func (RiskStaleOpenReminderArgs) Kind() string { return JobTypeRiskStaleOpenReminder } +func (RiskReconcileDuplicatesArgs) Kind() string { return JobTypeRiskReconcileDuplicates } +func (RiskReviewOverdueReopenArgs) Kind() string { return JobTypeRiskReviewOverdueReopen } + +func (RiskReviewDeadlineReminderScannerArgs) Timeout() time.Duration { return 5 * time.Minute } +func (RiskReviewOverdueEscalationScannerArgs) Timeout() time.Duration { return 5 * time.Minute } +func (RiskStaleRiskScannerArgs) Timeout() time.Duration { return 5 * time.Minute } +func (RiskEvidenceReconciliationScannerArgs) Timeout() time.Duration { return 5 * time.Minute } +func (RiskReviewDueReminderArgs) Timeout() time.Duration { return 30 * time.Second } +func (RiskReviewOverdueEscalationArgs) Timeout() time.Duration { return 30 * time.Second } +func (RiskStaleOpenReminderArgs) Timeout() time.Duration { return 30 * time.Second } +func (RiskReconcileDuplicatesArgs) Timeout() time.Duration { return 2 * time.Minute } +func (RiskReviewOverdueReopenArgs) Timeout() time.Duration { return 30 * time.Second } + +func JobInsertOptionsForRiskNotification(byPeriod time.Duration) *river.InsertOpts { + return &river.InsertOpts{ + Queue: "email", + MaxAttempts: 3, + UniqueOpts: river.UniqueOpts{ + ByArgs: true, + ByPeriod: byPeriod, + }, + } +} + +func JobInsertOptionsForRiskWorker() *river.InsertOpts { + return &river.InsertOpts{ + Queue: "risk", + MaxAttempts: 3, + } +} + +func JobInsertOptionsForRiskWorkerUnique(byPeriod time.Duration) *river.InsertOpts { + return &river.InsertOpts{ + Queue: "risk", + MaxAttempts: 3, + UniqueOpts: river.UniqueOpts{ + ByArgs: true, + ByPeriod: byPeriod, + }, + } +} diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go new file mode 100644 index 00000000..c1039e39 --- /dev/null +++ b/internal/service/worker/risk_workers.go @@ -0,0 +1,679 @@ +package worker + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + "time" + + "github.com/compliance-framework/api/internal/service/email/types" + "github.com/compliance-framework/api/internal/service/relational" + riskrel "github.com/compliance-framework/api/internal/service/relational/risks" + "github.com/compliance-framework/api/internal/workflow" + "github.com/google/uuid" + "github.com/riverqueue/river" + "go.uber.org/zap" + "gorm.io/gorm" +) + +type RiskReviewDeadlineReminderScannerWorker struct { + db *gorm.DB + client workflow.RiverClient + logger *zap.SugaredLogger +} + +func NewRiskReviewDeadlineReminderScannerWorker(db *gorm.DB, client workflow.RiverClient, logger *zap.SugaredLogger) *RiskReviewDeadlineReminderScannerWorker { + return &RiskReviewDeadlineReminderScannerWorker{db: db, client: client, logger: logger} +} + +func (w *RiskReviewDeadlineReminderScannerWorker) Work(ctx context.Context, _ *river.Job[RiskReviewDeadlineReminderScannerArgs]) error { + now := time.Now().UTC() + windowEnd := now.Add(30 * 24 * time.Hour) + + var risks []riskrel.Risk + if err := w.db.WithContext(ctx). + Where("status = ? AND review_deadline IS NOT NULL AND review_deadline > ? AND review_deadline <= ?", + string(riskrel.RiskStatusRiskAccepted), now, windowEnd). + Find(&risks).Error; err != nil { + return fmt.Errorf("risk deadline reminder scanner: query failed: %w", err) + } + + params := make([]river.InsertManyParams, 0, len(risks)) + for i := range risks { + risk := &risks[i] + if risk.ReviewDeadline == nil { + continue + } + ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) + if err != nil { + return fmt.Errorf("risk deadline reminder scanner: resolve owners for risk %s: %w", risk.ID.String(), err) + } + for _, ownerID := range ownerIDs { + params = append(params, river.InsertManyParams{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), + ReminderWindow: "30d", + }, + InsertOpts: JobInsertOptionsForRiskNotification(24 * time.Hour), + }) + } + } + + if len(params) == 0 { + w.logger.Infow("RiskReviewDeadlineReminderScannerWorker: no reminders to enqueue") + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk deadline reminder scanner: enqueue failed: %w", err) + } + + w.logger.Infow("RiskReviewDeadlineReminderScannerWorker: enqueued reminders", "count", len(params)) + return nil +} + +type RiskReviewOverdueEscalationScannerWorker struct { + db *gorm.DB + client workflow.RiverClient + logger *zap.SugaredLogger + autoReopenEnabled bool + autoReopenThresholdDays int +} + +func NewRiskReviewOverdueEscalationScannerWorker( + db *gorm.DB, + client workflow.RiverClient, + logger *zap.SugaredLogger, + autoReopenEnabled bool, + autoReopenThresholdDays int, +) *RiskReviewOverdueEscalationScannerWorker { + return &RiskReviewOverdueEscalationScannerWorker{ + db: db, + client: client, + logger: logger, + autoReopenEnabled: autoReopenEnabled, + autoReopenThresholdDays: autoReopenThresholdDays, + } +} + +func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ *river.Job[RiskReviewOverdueEscalationScannerArgs]) error { + now := time.Now().UTC() + + var risks []riskrel.Risk + if err := w.db.WithContext(ctx). + Where("status = ? AND review_deadline IS NOT NULL AND review_deadline < ?", + string(riskrel.RiskStatusRiskAccepted), now). + Find(&risks).Error; err != nil { + return fmt.Errorf("risk overdue escalation scanner: query failed: %w", err) + } + + params := make([]river.InsertManyParams, 0, len(risks)) + reopenByRiskID := make(map[uuid.UUID]RiskReviewOverdueReopenArgs, len(risks)) + for i := range risks { + risk := &risks[i] + if risk.ReviewDeadline == nil { + continue + } + + ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) + if err != nil { + return fmt.Errorf("risk overdue escalation scanner: resolve owners for risk %s: %w", risk.ID.String(), err) + } + overdueWindow := now.Format("2006-01-02") + for _, ownerID := range ownerIDs { + params = append(params, river.InsertManyParams{ + Args: RiskReviewOverdueEscalationArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), + OverdueWindow: overdueWindow, + }, + InsertOpts: JobInsertOptionsForRiskNotification(24 * time.Hour), + }) + } + + if w.autoReopenEnabled { + overdueFor := now.Sub(risk.ReviewDeadline.UTC()) + threshold := time.Duration(w.autoReopenThresholdDays) * 24 * time.Hour + if overdueFor >= threshold { + reopenByRiskID[*risk.ID] = RiskReviewOverdueReopenArgs{ + RiskID: *risk.ID, + ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), + ThresholdDays: w.autoReopenThresholdDays, + } + } + } + } + + for _, args := range reopenByRiskID { + params = append(params, river.InsertManyParams{ + Args: args, + InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + }) + } + + if len(params) == 0 { + w.logger.Infow("RiskReviewOverdueEscalationScannerWorker: no escalations to enqueue") + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk overdue escalation scanner: enqueue failed: %w", err) + } + + w.logger.Infow("RiskReviewOverdueEscalationScannerWorker: enqueued jobs", "count", len(params), "reopen_count", len(reopenByRiskID)) + return nil +} + +type RiskStaleRiskScannerWorker struct { + db *gorm.DB + client workflow.RiverClient + logger *zap.SugaredLogger +} + +func NewRiskStaleRiskScannerWorker(db *gorm.DB, client workflow.RiverClient, logger *zap.SugaredLogger) *RiskStaleRiskScannerWorker { + return &RiskStaleRiskScannerWorker{db: db, client: client, logger: logger} +} + +func (w *RiskStaleRiskScannerWorker) Work(ctx context.Context, _ *river.Job[RiskStaleRiskScannerArgs]) error { + now := time.Now().UTC() + cutoff := now.Add(-30 * 24 * time.Hour) + staleBucketDate := startOfISOWeekUTC(now).Format("2006-01-02") + + var risks []riskrel.Risk + if err := w.db.WithContext(ctx). + Where("status IN ? AND last_seen_at <= ?", + []string{ + string(riskrel.RiskStatusOpen), + string(riskrel.RiskStatusInvestigating), + string(riskrel.RiskStatusMitigatingPlanned), + }, + cutoff). + Find(&risks).Error; err != nil { + return fmt.Errorf("risk stale scanner: query failed: %w", err) + } + + params := make([]river.InsertManyParams, 0, len(risks)) + for i := range risks { + risk := &risks[i] + ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) + if err != nil { + return fmt.Errorf("risk stale scanner: resolve owners for risk %s: %w", risk.ID.String(), err) + } + for _, ownerID := range ownerIDs { + params = append(params, river.InsertManyParams{ + Args: RiskStaleOpenReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + LastSeenAt: risk.LastSeenAt.UTC().Format(time.RFC3339), + StaleBucketDate: staleBucketDate, + }, + InsertOpts: JobInsertOptionsForRiskNotification(7 * 24 * time.Hour), + }) + } + } + + if len(params) == 0 { + w.logger.Infow("RiskStaleRiskScannerWorker: no stale reminders to enqueue") + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk stale scanner: enqueue failed: %w", err) + } + + w.logger.Infow("RiskStaleRiskScannerWorker: enqueued stale reminders", "count", len(params)) + return nil +} + +type RiskEvidenceReconciliationScannerWorker struct { + db *gorm.DB + client workflow.RiverClient + logger *zap.SugaredLogger +} + +func NewRiskEvidenceReconciliationScannerWorker(db *gorm.DB, client workflow.RiverClient, logger *zap.SugaredLogger) *RiskEvidenceReconciliationScannerWorker { + return &RiskEvidenceReconciliationScannerWorker{db: db, client: client, logger: logger} +} + +func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *river.Job[RiskEvidenceReconciliationScannerArgs]) error { + var params []river.InsertManyParams + + // 1) Evidence failures without linked risks. + var orphanEvidence []relational.Evidence + if err := w.db.WithContext(ctx). + Model(&relational.Evidence{}). + Where("(status->>'state' = ? OR json_extract(status, '$.state') = ? OR CAST(status as TEXT) LIKE ?)", + relational.EvidenceStatusNotSatisfied, relational.EvidenceStatusNotSatisfied, "%\"state\":\"not-satisfied\"%"). + Where("NOT EXISTS (SELECT 1 FROM risk_evidence_links rel WHERE rel.evidence_id = evidences.id)"). + Find(&orphanEvidence).Error; err != nil { + return fmt.Errorf("risk reconciliation scanner: query orphan evidence failed: %w", err) + } + + for i := range orphanEvidence { + e := &orphanEvidence[i] + state := e.Status.Data().State + if state == "" { + state = relational.EvidenceStatusNotSatisfied + } + params = append(params, river.InsertManyParams{ + Args: RiskProcessEvidenceFailureArgs{ + EvidenceID: *e.ID, + EvidenceEnd: e.End.UTC().Format(time.RFC3339), + Status: state, + }, + InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + }) + } + + // 2) Evidence-linked risks with missing subject links. + var riskIDs []uuid.UUID + if err := w.db.WithContext(ctx). + Model(&riskrel.Risk{}). + Select("risk_register_risks.id"). + Where("risk_register_risks.source_type = ? AND risk_register_risks.status <> ?", + string(riskrel.RiskSourceTypeEvidenceAuto), string(riskrel.RiskStatusClosed)). + Where("EXISTS (SELECT 1 FROM risk_evidence_links rel WHERE rel.risk_id = risk_register_risks.id)"). + Where("NOT EXISTS (SELECT 1 FROM risk_subject_links rsl WHERE rsl.risk_id = risk_register_risks.id)"). + Pluck("risk_register_risks.id", &riskIDs).Error; err != nil { + return fmt.Errorf("risk reconciliation scanner: query missing subject links failed: %w", err) + } + + for _, riskID := range riskIDs { + var evidence relational.Evidence + err := w.db.WithContext(ctx). + Model(&relational.Evidence{}). + Joins("JOIN risk_evidence_links rel ON rel.evidence_id = evidences.id"). + Where("rel.risk_id = ?", riskID). + Order("evidences.end DESC"). + First(&evidence).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + continue + } + return fmt.Errorf("risk reconciliation scanner: load evidence for risk %s failed: %w", riskID.String(), err) + } + + var subjectCount int64 + if err := w.db.WithContext(ctx). + Table("evidence_subjects"). + Where("evidence_id = ?", *evidence.ID). + Count(&subjectCount).Error; err != nil { + return fmt.Errorf("risk reconciliation scanner: count evidence subjects failed: %w", err) + } + if subjectCount == 0 { + continue + } + + state := evidence.Status.Data().State + if state == "" { + state = relational.EvidenceStatusNotSatisfied + } + params = append(params, river.InsertManyParams{ + Args: RiskProcessEvidenceFailureArgs{ + EvidenceID: *evidence.ID, + EvidenceEnd: evidence.End.UTC().Format(time.RFC3339), + Status: state, + }, + InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + }) + } + + // 3) Duplicate active risks with same dedupe key. + var duplicateKeys []string + if err := w.db.WithContext(ctx). + Model(&riskrel.Risk{}). + Select("dedupe_key"). + Where("status <> ? AND dedupe_key <> ''", string(riskrel.RiskStatusClosed)). + Group("dedupe_key"). + Having("COUNT(*) > 1"). + Pluck("dedupe_key", &duplicateKeys).Error; err != nil { + return fmt.Errorf("risk reconciliation scanner: query duplicate dedupe keys failed: %w", err) + } + for _, key := range duplicateKeys { + params = append(params, river.InsertManyParams{ + Args: RiskReconcileDuplicatesArgs{DedupeKey: key}, + InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + }) + } + + if len(params) == 0 { + w.logger.Infow("RiskEvidenceReconciliationScannerWorker: no reconciliation jobs to enqueue") + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk reconciliation scanner: enqueue failed: %w", err) + } + + w.logger.Infow("RiskEvidenceReconciliationScannerWorker: enqueued reconciliation jobs", "count", len(params)) + return nil +} + +type RiskReviewDueReminderWorker struct { + db *gorm.DB + emailService EmailService + userRepo UserRepository + webBaseURL string + logger *zap.SugaredLogger +} + +func NewRiskReviewDueReminderWorker(db *gorm.DB, emailService EmailService, userRepo UserRepository, webBaseURL string, logger *zap.SugaredLogger) *RiskReviewDueReminderWorker { + return &RiskReviewDueReminderWorker{db: db, emailService: emailService, userRepo: userRepo, webBaseURL: webBaseURL, logger: logger} +} + +func (w *RiskReviewDueReminderWorker) Work(ctx context.Context, job *river.Job[RiskReviewDueReminderArgs]) error { + return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-review-due-reminder", + fmt.Sprintf("Risk review due soon: %s", w.safeRiskTitle(ctx, job.Args.RiskID))) +} + +type RiskReviewOverdueEscalationWorker struct { + db *gorm.DB + emailService EmailService + userRepo UserRepository + webBaseURL string + logger *zap.SugaredLogger +} + +func NewRiskReviewOverdueEscalationWorker(db *gorm.DB, emailService EmailService, userRepo UserRepository, webBaseURL string, logger *zap.SugaredLogger) *RiskReviewOverdueEscalationWorker { + return &RiskReviewOverdueEscalationWorker{db: db, emailService: emailService, userRepo: userRepo, webBaseURL: webBaseURL, logger: logger} +} + +func (w *RiskReviewOverdueEscalationWorker) Work(ctx context.Context, job *river.Job[RiskReviewOverdueEscalationArgs]) error { + return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-review-overdue-escalation", + fmt.Sprintf("Risk review overdue: %s", w.safeRiskTitle(ctx, job.Args.RiskID))) +} + +type RiskStaleOpenReminderWorker struct { + db *gorm.DB + emailService EmailService + userRepo UserRepository + webBaseURL string + logger *zap.SugaredLogger +} + +func NewRiskStaleOpenReminderWorker(db *gorm.DB, emailService EmailService, userRepo UserRepository, webBaseURL string, logger *zap.SugaredLogger) *RiskStaleOpenReminderWorker { + return &RiskStaleOpenReminderWorker{db: db, emailService: emailService, userRepo: userRepo, webBaseURL: webBaseURL, logger: logger} +} + +func (w *RiskStaleOpenReminderWorker) Work(ctx context.Context, job *river.Job[RiskStaleOpenReminderArgs]) error { + return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-stale-open-reminder", + fmt.Sprintf("Stale risk reminder: %s", w.safeRiskTitle(ctx, job.Args.RiskID))) +} + +func (w *RiskReviewDueReminderWorker) safeRiskTitle(ctx context.Context, riskID uuid.UUID) string { + return safeRiskTitle(ctx, w.db, riskID) +} + +func (w *RiskReviewOverdueEscalationWorker) safeRiskTitle(ctx context.Context, riskID uuid.UUID) string { + return safeRiskTitle(ctx, w.db, riskID) +} + +func (w *RiskStaleOpenReminderWorker) safeRiskTitle(ctx context.Context, riskID uuid.UUID) string { + return safeRiskTitle(ctx, w.db, riskID) +} + +func safeRiskTitle(ctx context.Context, db *gorm.DB, riskID uuid.UUID) string { + var risk riskrel.Risk + if err := db.WithContext(ctx).Select("title").First(&risk, "id = ?", riskID).Error; err != nil { + return riskID.String() + } + if strings.TrimSpace(risk.Title) == "" { + return riskID.String() + } + return risk.Title +} + +func (w *RiskReviewDueReminderWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subject string) error { + return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subject) +} + +func (w *RiskReviewOverdueEscalationWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subject string) error { + return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subject) +} + +func (w *RiskStaleOpenReminderWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subject string) error { + return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subject) +} + +func sendRiskNotification( + ctx context.Context, + db *gorm.DB, + emailService EmailService, + userRepo UserRepository, + webBaseURL string, + logger *zap.SugaredLogger, + riskID, ownerUserID uuid.UUID, + templateName, subject string, +) error { + var risk riskrel.Risk + if err := db.WithContext(ctx).First(&risk, "id = ?", riskID).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + logger.Warnw("Risk notification worker: risk not found, skipping", "risk_id", riskID) + return nil + } + return fmt.Errorf("risk notification worker: load risk failed: %w", err) + } + + user, err := userRepo.FindUserByID(ctx, ownerUserID.String()) + if err != nil { + logger.Warnw("Risk notification worker: owner user not found, skipping", "risk_id", riskID, "owner_user_id", ownerUserID, "error", err) + return nil + } + if !user.RiskNotificationsSubscribed { + logger.Debugw("Risk notification worker: owner unsubscribed, skipping", "risk_id", riskID, "owner_user_id", ownerUserID) + return nil + } + + sspName := resolveSSPDisplayName(ctx, db, risk.SSPID) + reviewDeadline := "" + if risk.ReviewDeadline != nil { + reviewDeadline = formatDate(*risk.ReviewDeadline) + } + + templateData := map[string]interface{}{ + "OwnerName": user.FullName(), + "RiskTitle": risk.Title, + "SSPName": sspName, + "RiskStatus": risk.Status, + "ReviewDeadline": reviewDeadline, + "LastSeenAt": formatDate(risk.LastSeenAt), + "RiskURL": resolveRiskURL(webBaseURL, riskID), + } + + htmlBody, textBody, err := emailService.UseTemplate(templateName, templateData) + if err != nil { + return fmt.Errorf("risk notification worker: render template %q failed: %w", templateName, err) + } + + message := &types.Message{ + From: emailService.GetDefaultFromAddress(), + To: []string{user.Email}, + Subject: subject, + HTMLBody: htmlBody, + TextBody: textBody, + } + result, err := emailService.Send(ctx, message) + if err != nil { + return fmt.Errorf("risk notification worker: send email failed: %w", err) + } + if !result.Success { + return fmt.Errorf("risk notification worker: email send failed: %s", result.Error) + } + + logger.Infow("Risk notification worker: email sent", + "risk_id", riskID, + "owner_user_id", ownerUserID, + "template", templateName, + "message_id", result.MessageID, + ) + return nil +} + +type RiskReconcileDuplicatesWorker struct { + db *gorm.DB + logger *zap.SugaredLogger +} + +func NewRiskReconcileDuplicatesWorker(db *gorm.DB, logger *zap.SugaredLogger) *RiskReconcileDuplicatesWorker { + return &RiskReconcileDuplicatesWorker{db: db, logger: logger} +} + +func (w *RiskReconcileDuplicatesWorker) Work(ctx context.Context, job *river.Job[RiskReconcileDuplicatesArgs]) error { + if strings.TrimSpace(job.Args.DedupeKey) == "" { + return nil + } + + var duplicates []riskrel.Risk + if err := w.db.WithContext(ctx). + Where("dedupe_key = ? AND status <> ?", job.Args.DedupeKey, string(riskrel.RiskStatusClosed)). + Order("created_at ASC, id ASC"). + Find(&duplicates).Error; err != nil { + return fmt.Errorf("risk reconcile duplicates: load duplicates failed: %w", err) + } + if len(duplicates) <= 1 { + return nil + } + + riskSvc := riskrel.NewRiskService(w.db) + keeper := duplicates[0] + for i := 1; i < len(duplicates); i++ { + risk := duplicates[i] + oldStatus := risk.Status + risk.Status = string(riskrel.RiskStatusClosed) + if _, err := riskSvc.Update(riskrel.UpdateRiskParams{ + Risk: &risk, + OldStatus: oldStatus, + StatusChanged: oldStatus != risk.Status, + }); err != nil { + return fmt.Errorf("risk reconcile duplicates: close duplicate %s failed: %w", risk.ID.String(), err) + } + } + + w.logger.Infow("RiskReconcileDuplicatesWorker: reconciled duplicates", + "dedupe_key", job.Args.DedupeKey, + "kept_risk_id", keeper.ID.String(), + "closed_count", len(duplicates)-1, + ) + return nil +} + +type RiskReviewOverdueReopenWorker struct { + db *gorm.DB + logger *zap.SugaredLogger +} + +func NewRiskReviewOverdueReopenWorker(db *gorm.DB, logger *zap.SugaredLogger) *RiskReviewOverdueReopenWorker { + return &RiskReviewOverdueReopenWorker{db: db, logger: logger} +} + +func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job[RiskReviewOverdueReopenArgs]) error { + riskSvc := riskrel.NewRiskService(w.db) + risk, err := riskSvc.GetByID(job.Args.RiskID) + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + w.logger.Warnw("RiskReviewOverdueReopenWorker: risk not found, skipping", "risk_id", job.Args.RiskID) + return nil + } + return fmt.Errorf("risk overdue reopen: load risk failed: %w", err) + } + if risk.Status != string(riskrel.RiskStatusRiskAccepted) || risk.ReviewDeadline == nil { + return nil + } + + now := time.Now().UTC() + threshold := time.Duration(job.Args.ThresholdDays) * 24 * time.Hour + if now.Sub(risk.ReviewDeadline.UTC()) < threshold { + return nil + } + + oldStatus := risk.Status + risk.Status = string(riskrel.RiskStatusInvestigating) + risk.ReviewDeadline = nil + if _, err := riskSvc.Update(riskrel.UpdateRiskParams{ + Risk: risk, + OldStatus: oldStatus, + StatusChanged: true, + }); err != nil { + return fmt.Errorf("risk overdue reopen: update risk failed: %w", err) + } + + w.logger.Infow("RiskReviewOverdueReopenWorker: reopened overdue accepted risk", + "risk_id", job.Args.RiskID, + "threshold_days", job.Args.ThresholdDays, + ) + return nil +} + +func resolveRiskOwnerUserIDs(ctx context.Context, db *gorm.DB, risk *riskrel.Risk) ([]uuid.UUID, error) { + if risk == nil { + return nil, nil + } + + ownerSet := make(map[uuid.UUID]struct{}, 4) + if risk.PrimaryOwnerUserID != nil { + ownerSet[*risk.PrimaryOwnerUserID] = struct{}{} + } + + var assignments []riskrel.RiskOwnerAssignment + if err := db.WithContext(ctx). + Where("risk_id = ? AND owner_kind = ?", *risk.ID, "user"). + Find(&assignments).Error; err != nil { + return nil, err + } + for _, assignment := range assignments { + parsed, err := uuid.Parse(assignment.OwnerRef) + if err != nil { + continue + } + ownerSet[parsed] = struct{}{} + } + + owners := make([]uuid.UUID, 0, len(ownerSet)) + for ownerID := range ownerSet { + owners = append(owners, ownerID) + } + sort.Slice(owners, func(i, j int) bool { return owners[i].String() < owners[j].String() }) + return owners, nil +} + +func resolveSSPDisplayName(ctx context.Context, db *gorm.DB, sspID uuid.UUID) string { + if !db.WithContext(ctx).Migrator().HasTable(&relational.SystemCharacteristics{}) { + return sspID.String() + } + + var sc relational.SystemCharacteristics + if err := db.WithContext(ctx). + Select("system_name_short", "system_name"). + First(&sc, "system_security_plan_id = ?", sspID).Error; err != nil { + return sspID.String() + } + + name := strings.TrimSpace(sc.SystemNameShort) + if name != "" { + return name + } + name = strings.TrimSpace(sc.SystemName) + if name != "" { + return name + } + return sspID.String() +} + +func resolveRiskURL(webBaseURL string, riskID uuid.UUID) string { + base := strings.TrimRight(webBaseURL, "/") + return base + "/risks/" + riskID.String() +} + +func startOfISOWeekUTC(t time.Time) time.Time { + d := time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, time.UTC) + wd := int(d.Weekday()) + if wd == 0 { + wd = 7 + } + return d.AddDate(0, 0, -(wd - 1)) +} diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go new file mode 100644 index 00000000..3ad33550 --- /dev/null +++ b/internal/service/worker/risk_workers_test.go @@ -0,0 +1,342 @@ +package worker + +import ( + "context" + "testing" + "time" + + "github.com/compliance-framework/api/internal/service/email/types" + "github.com/compliance-framework/api/internal/service/relational" + riskrel "github.com/compliance-framework/api/internal/service/relational/risks" + oscalTypes_1_1_3 "github.com/defenseunicorns/go-oscal/src/types/oscal-1-1-3" + "github.com/google/uuid" + "github.com/riverqueue/river" + "github.com/riverqueue/river/rivertype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "gorm.io/datatypes" + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +type stubRiverClient struct { + params []river.InsertManyParams + err error +} + +func (s *stubRiverClient) InsertMany(_ context.Context, params []river.InsertManyParams) ([]*rivertype.JobInsertResult, error) { + s.params = append(s.params, params...) + if s.err != nil { + return nil, s.err + } + return []*rivertype.JobInsertResult{}, nil +} + +func newRiskWorkersTestDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate( + &relational.User{}, + &relational.SystemSecurityPlan{}, + &relational.SystemCharacteristics{}, + &relational.Evidence{}, + &relational.Labels{}, + &relational.AssessmentSubject{}, + &relational.SystemComponent{}, + &relational.InventoryItem{}, + &riskrel.Risk{}, + &riskrel.RiskOwnerAssignment{}, + &riskrel.RiskEvidenceLink{}, + &riskrel.RiskSubjectLink{}, + &riskrel.RiskEvent{}, + )) + return db +} + +func createTestRiskWithOwner(t *testing.T, db *gorm.DB, status riskrel.RiskStatus, reviewDeadline *time.Time, lastSeenAt time.Time) (riskrel.Risk, uuid.UUID) { + t.Helper() + sspID := uuid.New() + require.NoError(t, db.Create(&relational.SystemSecurityPlan{ + UUIDModel: relational.UUIDModel{ID: &sspID}, + }).Error) + + ownerID := uuid.New() + riskID := uuid.New() + r := riskrel.Risk{ + UUIDModel: relational.UUIDModel{ID: &riskID}, + Title: "Test Risk", + Description: "Test Risk Description", + Status: string(status), + SSPID: sspID, + SourceType: string(riskrel.RiskSourceTypeManual), + ReviewDeadline: reviewDeadline, + FirstSeenAt: time.Now().UTC().Add(-24 * time.Hour), + LastSeenAt: lastSeenAt, + } + require.NoError(t, db.Create(&r).Error) + require.NoError(t, db.Create(&riskrel.RiskOwnerAssignment{ + RiskID: riskID, + OwnerKind: "user", + OwnerRef: ownerID.String(), + IsPrimary: true, + }).Error) + return r, ownerID +} + +func createTestUser(t *testing.T, db *gorm.DB, id uuid.UUID, subscribed bool) { + t.Helper() + require.NoError(t, db.Create(&relational.User{ + UUIDModel: relational.UUIDModel{ID: &id}, + Email: id.String() + "@example.com", + FirstName: "Risk", + LastName: "Owner", + AuthMethod: "password", + RiskNotificationsSubscribed: subscribed, + }).Error) + require.NoError(t, db.Model(&relational.User{}). + Where("id = ?", id). + Update("risk_notifications_subscribed", subscribed).Error) +} + +func TestRiskReviewDeadlineReminderScannerWorker_EnqueuesPerUserOwner(t *testing.T) { + db := newRiskWorkersTestDB(t) + client := &stubRiverClient{} + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(29 * 24 * time.Hour) + _, _ = createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + + w := NewRiskReviewDeadlineReminderScannerWorker(db, client, logger) + err := w.Work(context.Background(), &river.Job[RiskReviewDeadlineReminderScannerArgs]{}) + require.NoError(t, err) + require.Len(t, client.params, 1) + + args, ok := client.params[0].Args.(RiskReviewDueReminderArgs) + require.True(t, ok) + assert.Equal(t, "30d", args.ReminderWindow) + assert.Equal(t, 24*time.Hour, client.params[0].InsertOpts.UniqueOpts.ByPeriod) +} + +func TestRiskReviewOverdueEscalationScannerWorker_EnqueuesEscalationAndReopen(t *testing.T) { + db := newRiskWorkersTestDB(t) + client := &stubRiverClient{} + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(-31 * 24 * time.Hour) + _, _ = createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now.Add(-40*24*time.Hour)) + + w := NewRiskReviewOverdueEscalationScannerWorker(db, client, logger, true, 30) + err := w.Work(context.Background(), &river.Job[RiskReviewOverdueEscalationScannerArgs]{}) + require.NoError(t, err) + + var escalationCount, reopenCount int + for _, p := range client.params { + switch p.Args.(type) { + case RiskReviewOverdueEscalationArgs: + escalationCount++ + case RiskReviewOverdueReopenArgs: + reopenCount++ + } + } + assert.Equal(t, 1, escalationCount) + assert.Equal(t, 1, reopenCount) +} + +func TestRiskStaleRiskScannerWorker_EnqueuesWeeklyReminder(t *testing.T) { + db := newRiskWorkersTestDB(t) + client := &stubRiverClient{} + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + _, _ = createTestRiskWithOwner(t, db, riskrel.RiskStatusOpen, nil, now.Add(-31*24*time.Hour)) + + w := NewRiskStaleRiskScannerWorker(db, client, logger) + err := w.Work(context.Background(), &river.Job[RiskStaleRiskScannerArgs]{}) + require.NoError(t, err) + require.Len(t, client.params, 1) + + _, ok := client.params[0].Args.(RiskStaleOpenReminderArgs) + require.True(t, ok) + assert.Equal(t, 7*24*time.Hour, client.params[0].InsertOpts.UniqueOpts.ByPeriod) +} + +func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T) { + db := newRiskWorkersTestDB(t) + client := &stubRiverClient{} + logger := zap.NewNop().Sugar() + + // Evidence failure without linked risk + evidenceID := uuid.New() + require.NoError(t, db.Create(&relational.Evidence{ + UUIDModel: relational.UUIDModel{ID: &evidenceID}, + UUID: uuid.New(), + Title: "failing evidence", + Start: time.Now().UTC().Add(-time.Hour), + End: time.Now().UTC(), + Status: datatypes.NewJSONType(oscalTypes_1_1_3.ObjectiveStatus{State: relational.EvidenceStatusNotSatisfied}), + }).Error) + + // Duplicate active risks by dedupe key + dupSSP := uuid.New() + require.NoError(t, db.Create(&relational.SystemSecurityPlan{UUIDModel: relational.UUIDModel{ID: &dupSSP}}).Error) + r1ID := uuid.New() + r2ID := uuid.New() + require.NoError(t, db.Create(&riskrel.Risk{ + UUIDModel: relational.UUIDModel{ID: &r1ID}, + Title: "dup1", + Description: "dup1", + Status: string(riskrel.RiskStatusOpen), + SSPID: dupSSP, + SourceType: string(riskrel.RiskSourceTypeManual), + DedupeKey: "dup-key", + FirstSeenAt: time.Now().UTC(), + LastSeenAt: time.Now().UTC(), + }).Error) + require.NoError(t, db.Create(&riskrel.Risk{ + UUIDModel: relational.UUIDModel{ID: &r2ID}, + Title: "dup2", + Description: "dup2", + Status: string(riskrel.RiskStatusInvestigating), + SSPID: dupSSP, + SourceType: string(riskrel.RiskSourceTypeManual), + DedupeKey: "dup-key", + FirstSeenAt: time.Now().UTC(), + LastSeenAt: time.Now().UTC(), + }).Error) + + w := NewRiskEvidenceReconciliationScannerWorker(db, client, logger) + err := w.Work(context.Background(), &river.Job[RiskEvidenceReconciliationScannerArgs]{}) + require.NoError(t, err) + + var sawEvidenceRepair, sawDuplicateRepair bool + for _, p := range client.params { + switch args := p.Args.(type) { + case RiskProcessEvidenceFailureArgs: + if args.EvidenceID == evidenceID { + sawEvidenceRepair = true + } + case RiskReconcileDuplicatesArgs: + if args.DedupeKey == "dup-key" { + sawDuplicateRepair = true + } + } + } + assert.True(t, sawEvidenceRepair) + assert.True(t, sawDuplicateRepair) +} + +func TestRiskReconcileDuplicatesWorker_ClosesNewerRisk(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + sspID := uuid.New() + require.NoError(t, db.Create(&relational.SystemSecurityPlan{UUIDModel: relational.UUIDModel{ID: &sspID}}).Error) + + oldID := uuid.New() + newID := uuid.New() + require.NoError(t, db.Create(&riskrel.Risk{ + UUIDModel: relational.UUIDModel{ID: &oldID}, + Title: "old", + Description: "old", + Status: string(riskrel.RiskStatusOpen), + SSPID: sspID, + SourceType: string(riskrel.RiskSourceTypeManual), + DedupeKey: "dup-close", + FirstSeenAt: time.Now().UTC(), + LastSeenAt: time.Now().UTC(), + CreatedAt: time.Now().UTC().Add(-time.Hour), + }).Error) + require.NoError(t, db.Create(&riskrel.Risk{ + UUIDModel: relational.UUIDModel{ID: &newID}, + Title: "new", + Description: "new", + Status: string(riskrel.RiskStatusInvestigating), + SSPID: sspID, + SourceType: string(riskrel.RiskSourceTypeManual), + DedupeKey: "dup-close", + FirstSeenAt: time.Now().UTC(), + LastSeenAt: time.Now().UTC(), + CreatedAt: time.Now().UTC(), + }).Error) + + w := NewRiskReconcileDuplicatesWorker(db, logger) + err := w.Work(context.Background(), &river.Job[RiskReconcileDuplicatesArgs]{Args: RiskReconcileDuplicatesArgs{DedupeKey: "dup-close"}}) + require.NoError(t, err) + + var oldRisk, newRisk riskrel.Risk + require.NoError(t, db.First(&oldRisk, "id = ?", oldID).Error) + require.NoError(t, db.First(&newRisk, "id = ?", newID).Error) + assert.Equal(t, string(riskrel.RiskStatusOpen), oldRisk.Status) + assert.Equal(t, string(riskrel.RiskStatusClosed), newRisk.Status) +} + +func TestRiskReviewOverdueReopenWorker_ReopensAcceptedRisk(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + reviewDeadline := time.Now().UTC().Add(-40 * 24 * time.Hour) + risk, _ := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, time.Now().UTC()) + + w := NewRiskReviewOverdueReopenWorker(db, logger) + err := w.Work(context.Background(), &river.Job[RiskReviewOverdueReopenArgs]{ + Args: RiskReviewOverdueReopenArgs{ + RiskID: *risk.ID, + ThresholdDays: 30, + }, + }) + require.NoError(t, err) + + var updated riskrel.Risk + require.NoError(t, db.First(&updated, "id = ?", risk.ID).Error) + assert.Equal(t, string(riskrel.RiskStatusInvestigating), updated.Status) + assert.Nil(t, updated.ReviewDeadline) +} + +func TestRiskReviewDueReminderWorker_RespectsRiskSubscription(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(7 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + createTestUser(t, db, ownerID, false) + + mockEmail := &MockEmailService{} + userRepo := NewGORMUserRepository(db) + worker := NewRiskReviewDueReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + + err := worker.Work(context.Background(), &river.Job[RiskReviewDueReminderArgs]{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertNotCalled(t, "Send") +} + +func TestRiskReviewDueReminderWorker_SendsWhenSubscribed(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(7 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + createTestUser(t, db, ownerID, true) + + mockEmail := &MockEmailService{} + mockEmail.On("UseTemplate", "risk-review-due-reminder", mock.Anything).Return("ok", "ok", nil) + mockEmail.On("GetDefaultFromAddress").Return("noreply@example.com") + mockEmail.On("Send", mock.Anything, mock.MatchedBy(func(msg *types.Message) bool { + return len(msg.To) == 1 && msg.To[0] == ownerID.String()+"@example.com" + })).Return(&types.SendResult{Success: true, MessageID: "risk-msg"}, nil) + + userRepo := NewGORMUserRepository(db) + worker := NewRiskReviewDueReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewDueReminderArgs]{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertExpectations(t) +} diff --git a/internal/service/worker/service.go b/internal/service/worker/service.go index 93e3faa6..7f9d1dbc 100644 --- a/internal/service/worker/service.go +++ b/internal/service/worker/service.go @@ -224,6 +224,30 @@ func NewServiceWithDigest( digestCheckerWorker := NewWorkflowTaskDigestCheckerWorker(db, clientProxy, logger) river.AddWorker(workers, river.WorkFunc(digestCheckerWorker.Work)) + // Add risk scanner workers + riskCfg := config.DefaultRiskConfig() + if digestCfg != nil && digestCfg.Risk != nil { + riskCfg = digestCfg.Risk + } + + riskReminderScannerWorker := NewRiskReviewDeadlineReminderScannerWorker(db, clientProxy, logger) + river.AddWorker(workers, river.WorkFunc(riskReminderScannerWorker.Work)) + + riskOverdueScannerWorker := NewRiskReviewOverdueEscalationScannerWorker( + db, + clientProxy, + logger, + riskCfg.AutoReopenEnabled, + riskCfg.AutoReopenThresholdDays, + ) + river.AddWorker(workers, river.WorkFunc(riskOverdueScannerWorker.Work)) + + riskStaleScannerWorker := NewRiskStaleRiskScannerWorker(db, clientProxy, logger) + river.AddWorker(workers, river.WorkFunc(riskStaleScannerWorker.Work)) + + riskReconciliationScannerWorker := NewRiskEvidenceReconciliationScannerWorker(db, clientProxy, logger) + river.AddWorker(workers, river.WorkFunc(riskReconciliationScannerWorker.Work)) + // Configure periodic jobs periodicJobs := periodicJobsFromConfig(digestCfg, logger) @@ -443,6 +467,90 @@ func NewWorkflowTaskDigestPeriodicJob(schedule string, logger *zap.SugaredLogger ) } +func NewRiskReviewDeadlineReminderPeriodicJob(schedule string, logger *zap.SugaredLogger) *river.PeriodicJob { + sched := parseCronScheduleWithFallback(schedule, "0 0 8 * * *", "risk review deadline reminder scanner", logger) + + return river.NewPeriodicJob( + sched, + func() (river.JobArgs, *river.InsertOpts) { + return &RiskReviewDeadlineReminderScannerArgs{}, &river.InsertOpts{ + Queue: "risk", + MaxAttempts: 3, + UniqueOpts: river.UniqueOpts{ + ByArgs: true, + ByPeriod: 24 * time.Hour, + }, + } + }, + &river.PeriodicJobOpts{ + RunOnStart: false, + }, + ) +} + +func NewRiskReviewOverdueEscalationPeriodicJob(schedule string, logger *zap.SugaredLogger) *river.PeriodicJob { + sched := parseCronScheduleWithFallback(schedule, "0 0 9 * * *", "risk review overdue escalation scanner", logger) + + return river.NewPeriodicJob( + sched, + func() (river.JobArgs, *river.InsertOpts) { + return &RiskReviewOverdueEscalationScannerArgs{}, &river.InsertOpts{ + Queue: "risk", + MaxAttempts: 3, + UniqueOpts: river.UniqueOpts{ + ByArgs: true, + ByPeriod: 24 * time.Hour, + }, + } + }, + &river.PeriodicJobOpts{ + RunOnStart: false, + }, + ) +} + +func NewRiskStaleScannerPeriodicJob(schedule string, logger *zap.SugaredLogger) *river.PeriodicJob { + sched := parseCronScheduleWithFallback(schedule, "0 0 10 * * 1", "risk stale scanner", logger) + + return river.NewPeriodicJob( + sched, + func() (river.JobArgs, *river.InsertOpts) { + return &RiskStaleRiskScannerArgs{}, &river.InsertOpts{ + Queue: "risk", + MaxAttempts: 3, + UniqueOpts: river.UniqueOpts{ + ByArgs: true, + ByPeriod: 7 * 24 * time.Hour, + }, + } + }, + &river.PeriodicJobOpts{ + RunOnStart: false, + }, + ) +} + +func NewRiskEvidenceReconciliationPeriodicJob(schedule string, logger *zap.SugaredLogger) *river.PeriodicJob { + sched := parseCronScheduleWithFallback(schedule, "0 30 10 * * *", "risk evidence reconciliation scanner", logger) + + return river.NewPeriodicJob( + sched, + func() (river.JobArgs, *river.InsertOpts) { + return &RiskEvidenceReconciliationScannerArgs{}, &river.InsertOpts{ + Queue: "risk", + MaxAttempts: 3, + UniqueOpts: river.UniqueOpts{ + ByArgs: true, + ByPeriod: 24 * time.Hour, + }, + } + }, + &river.PeriodicJobOpts{ + RunOnStart: false, + }, + ) +} + func periodicJobsFromConfig(cfg *config.Config, logger *zap.SugaredLogger) []*river.PeriodicJob { var periodicJobs []*river.PeriodicJob if cfg == nil { @@ -460,6 +568,18 @@ func periodicJobsFromConfig(cfg *config.Config, logger *zap.SugaredLogger) []*ri if cfg.Workflow != nil && cfg.Workflow.TaskDigestEnabled { periodicJobs = append(periodicJobs, NewWorkflowTaskDigestPeriodicJob(cfg.Workflow.TaskDigestSchedule, logger)) } + if cfg.Risk != nil && cfg.Risk.ReviewDeadlineReminderEnabled { + periodicJobs = append(periodicJobs, NewRiskReviewDeadlineReminderPeriodicJob(cfg.Risk.ReviewDeadlineReminderSchedule, logger)) + } + if cfg.Risk != nil && cfg.Risk.ReviewOverdueEscalationEnabled { + periodicJobs = append(periodicJobs, NewRiskReviewOverdueEscalationPeriodicJob(cfg.Risk.ReviewOverdueEscalationSchedule, logger)) + } + if cfg.Risk != nil && cfg.Risk.StaleRiskScannerEnabled { + periodicJobs = append(periodicJobs, NewRiskStaleScannerPeriodicJob(cfg.Risk.StaleRiskScannerSchedule, logger)) + } + if cfg.Risk != nil && cfg.Risk.EvidenceReconciliationEnabled { + periodicJobs = append(periodicJobs, NewRiskEvidenceReconciliationPeriodicJob(cfg.Risk.EvidenceReconciliationSchedule, logger)) + } return periodicJobs } diff --git a/internal/service/worker/service_test.go b/internal/service/worker/service_test.go index 1a6c9d47..b19f278b 100644 --- a/internal/service/worker/service_test.go +++ b/internal/service/worker/service_test.go @@ -369,6 +369,25 @@ func TestPeriodicJobsFromConfig_WorkflowSchedulerEnabledGuard(t *testing.T) { assert.Len(t, jobs, 3) } +func TestPeriodicJobsFromConfig_RiskJobsFromDedicatedConfig(t *testing.T) { + logger := zap.NewNop().Sugar() + + jobs := periodicJobsFromConfig(&config.Config{ + Risk: &config.RiskConfig{ + ReviewDeadlineReminderEnabled: true, + ReviewDeadlineReminderSchedule: "0 0 8 * * *", + ReviewOverdueEscalationEnabled: true, + ReviewOverdueEscalationSchedule: "0 0 9 * * *", + StaleRiskScannerEnabled: true, + StaleRiskScannerSchedule: "0 0 10 * * 1", + EvidenceReconciliationEnabled: true, + EvidenceReconciliationSchedule: "0 30 10 * * *", + }, + }, logger) + + assert.Len(t, jobs, 4) +} + func TestWorkflowSchedulerPeriodicJobConstructor_InsertOpts(t *testing.T) { args, opts := workflowSchedulerPeriodicJobConstructor() assert.NotNil(t, args) diff --git a/internal/service/worker/user_repository.go b/internal/service/worker/user_repository.go index 538bca88..c30558e2 100644 --- a/internal/service/worker/user_repository.go +++ b/internal/service/worker/user_repository.go @@ -42,5 +42,6 @@ func (r *GORMUserRepository) FindUserByID(ctx context.Context, userID string) (N LastName: user.LastName, TaskAvailableEmailSubscribed: user.TaskAvailableEmailSubscribed, TaskDailyDigestSubscribed: user.TaskDailyDigestSubscribed, + RiskNotificationsSubscribed: user.RiskNotificationsSubscribed, }, nil } diff --git a/risk.yaml b/risk.yaml new file mode 100644 index 00000000..559d08d1 --- /dev/null +++ b/risk.yaml @@ -0,0 +1,14 @@ +review_deadline_reminder_enabled: false +review_deadline_reminder_schedule: "0 0 8 * * *" + +review_overdue_escalation_enabled: false +review_overdue_escalation_schedule: "0 0 9 * * *" + +stale_risk_scanner_enabled: false +stale_risk_scanner_schedule: "0 0 10 * * 1" + +evidence_reconciliation_enabled: false +evidence_reconciliation_schedule: "0 30 10 * * *" + +auto_reopen_enabled: false +auto_reopen_threshold_days: 30 From 656a2aeee6197f98488bdf89b7b0d6c182252e89 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 08:45:05 -0300 Subject: [PATCH 02/16] fix: copilot issues --- cmd/root.go | 1 + internal/service/worker/risk_job_types.go | 7 --- internal/service/worker/risk_workers.go | 61 ++++++-------------- internal/service/worker/risk_workers_test.go | 17 +++--- 4 files changed, 25 insertions(+), 61 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 9d164bc0..891c6017 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -46,6 +46,7 @@ func bindEnvironmentVariables() { viper.MustBindEnv("sso_config") viper.MustBindEnv("email_config") viper.MustBindEnv("workflow_config") + viper.MustBindEnv("risk_config") viper.MustBindEnv("metrics_enabled") viper.MustBindEnv("metrics_port") viper.MustBindEnv("use_dev_logger") diff --git a/internal/service/worker/risk_job_types.go b/internal/service/worker/risk_job_types.go index c8ee6ebb..23a0a421 100644 --- a/internal/service/worker/risk_job_types.go +++ b/internal/service/worker/risk_job_types.go @@ -93,13 +93,6 @@ func JobInsertOptionsForRiskNotification(byPeriod time.Duration) *river.InsertOp } } -func JobInsertOptionsForRiskWorker() *river.InsertOpts { - return &river.InsertOpts{ - Queue: "risk", - MaxAttempts: 3, - } -} - func JobInsertOptionsForRiskWorkerUnique(byPeriod time.Duration) *river.InsertOpts { return &river.InsertOpts{ Queue: "risk", diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index c1039e39..3c89a25f 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -43,9 +43,6 @@ func (w *RiskReviewDeadlineReminderScannerWorker) Work(ctx context.Context, _ *r params := make([]river.InsertManyParams, 0, len(risks)) for i := range risks { risk := &risks[i] - if risk.ReviewDeadline == nil { - continue - } ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) if err != nil { return fmt.Errorf("risk deadline reminder scanner: resolve owners for risk %s: %w", risk.ID.String(), err) @@ -115,9 +112,6 @@ func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ * reopenByRiskID := make(map[uuid.UUID]RiskReviewOverdueReopenArgs, len(risks)) for i := range risks { risk := &risks[i] - if risk.ReviewDeadline == nil { - continue - } ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) if err != nil { @@ -367,8 +361,7 @@ func NewRiskReviewDueReminderWorker(db *gorm.DB, emailService EmailService, user } func (w *RiskReviewDueReminderWorker) Work(ctx context.Context, job *river.Job[RiskReviewDueReminderArgs]) error { - return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-review-due-reminder", - fmt.Sprintf("Risk review due soon: %s", w.safeRiskTitle(ctx, job.Args.RiskID))) + return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-review-due-reminder", "Risk review due soon") } type RiskReviewOverdueEscalationWorker struct { @@ -384,8 +377,7 @@ func NewRiskReviewOverdueEscalationWorker(db *gorm.DB, emailService EmailService } func (w *RiskReviewOverdueEscalationWorker) Work(ctx context.Context, job *river.Job[RiskReviewOverdueEscalationArgs]) error { - return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-review-overdue-escalation", - fmt.Sprintf("Risk review overdue: %s", w.safeRiskTitle(ctx, job.Args.RiskID))) + return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-review-overdue-escalation", "Risk review overdue") } type RiskStaleOpenReminderWorker struct { @@ -401,43 +393,19 @@ func NewRiskStaleOpenReminderWorker(db *gorm.DB, emailService EmailService, user } func (w *RiskStaleOpenReminderWorker) Work(ctx context.Context, job *river.Job[RiskStaleOpenReminderArgs]) error { - return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-stale-open-reminder", - fmt.Sprintf("Stale risk reminder: %s", w.safeRiskTitle(ctx, job.Args.RiskID))) + return w.sendRiskNotification(ctx, job.Args.RiskID, job.Args.OwnerUserID, "risk-stale-open-reminder", "Stale risk reminder") } -func (w *RiskReviewDueReminderWorker) safeRiskTitle(ctx context.Context, riskID uuid.UUID) string { - return safeRiskTitle(ctx, w.db, riskID) +func (w *RiskReviewDueReminderWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subjectPrefix string) error { + return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subjectPrefix) } -func (w *RiskReviewOverdueEscalationWorker) safeRiskTitle(ctx context.Context, riskID uuid.UUID) string { - return safeRiskTitle(ctx, w.db, riskID) +func (w *RiskReviewOverdueEscalationWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subjectPrefix string) error { + return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subjectPrefix) } -func (w *RiskStaleOpenReminderWorker) safeRiskTitle(ctx context.Context, riskID uuid.UUID) string { - return safeRiskTitle(ctx, w.db, riskID) -} - -func safeRiskTitle(ctx context.Context, db *gorm.DB, riskID uuid.UUID) string { - var risk riskrel.Risk - if err := db.WithContext(ctx).Select("title").First(&risk, "id = ?", riskID).Error; err != nil { - return riskID.String() - } - if strings.TrimSpace(risk.Title) == "" { - return riskID.String() - } - return risk.Title -} - -func (w *RiskReviewDueReminderWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subject string) error { - return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subject) -} - -func (w *RiskReviewOverdueEscalationWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subject string) error { - return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subject) -} - -func (w *RiskStaleOpenReminderWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subject string) error { - return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subject) +func (w *RiskStaleOpenReminderWorker) sendRiskNotification(ctx context.Context, riskID, ownerUserID uuid.UUID, templateName, subjectPrefix string) error { + return sendRiskNotification(ctx, w.db, w.emailService, w.userRepo, w.webBaseURL, w.logger, riskID, ownerUserID, templateName, subjectPrefix) } func sendRiskNotification( @@ -448,7 +416,7 @@ func sendRiskNotification( webBaseURL string, logger *zap.SugaredLogger, riskID, ownerUserID uuid.UUID, - templateName, subject string, + templateName, subjectPrefix string, ) error { var risk riskrel.Risk if err := db.WithContext(ctx).First(&risk, "id = ?", riskID).Error; err != nil { @@ -469,6 +437,11 @@ func sendRiskNotification( return nil } + riskTitle := strings.TrimSpace(risk.Title) + if riskTitle == "" { + riskTitle = riskID.String() + } + sspName := resolveSSPDisplayName(ctx, db, risk.SSPID) reviewDeadline := "" if risk.ReviewDeadline != nil { @@ -477,7 +450,7 @@ func sendRiskNotification( templateData := map[string]interface{}{ "OwnerName": user.FullName(), - "RiskTitle": risk.Title, + "RiskTitle": riskTitle, "SSPName": sspName, "RiskStatus": risk.Status, "ReviewDeadline": reviewDeadline, @@ -493,7 +466,7 @@ func sendRiskNotification( message := &types.Message{ From: emailService.GetDefaultFromAddress(), To: []string{user.Email}, - Subject: subject, + Subject: fmt.Sprintf("%s: %s", subjectPrefix, riskTitle), HTMLBody: htmlBody, TextBody: textBody, } diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 3ad33550..0d34b43b 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -88,17 +88,14 @@ func createTestRiskWithOwner(t *testing.T, db *gorm.DB, status riskrel.RiskStatu func createTestUser(t *testing.T, db *gorm.DB, id uuid.UUID, subscribed bool) { t.Helper() - require.NoError(t, db.Create(&relational.User{ - UUIDModel: relational.UUIDModel{ID: &id}, - Email: id.String() + "@example.com", - FirstName: "Risk", - LastName: "Owner", - AuthMethod: "password", - RiskNotificationsSubscribed: subscribed, + require.NoError(t, db.Model(&relational.User{}).Create(map[string]interface{}{ + "id": id, + "email": id.String() + "@example.com", + "first_name": "Risk", + "last_name": "Owner", + "auth_method": "password", + "risk_notifications_subscribed": subscribed, }).Error) - require.NoError(t, db.Model(&relational.User{}). - Where("id = ?", id). - Update("risk_notifications_subscribed", subscribed).Error) } func TestRiskReviewDeadlineReminderScannerWorker_EnqueuesPerUserOwner(t *testing.T) { From 2fea42933f962c5bf5a3fb48f3d8d35a4b5c9090 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 09:09:32 -0300 Subject: [PATCH 03/16] fix: copilot issues --- docs/docs.go | 2 +- docs/swagger.json | 2 +- docs/swagger.yaml | 5 +- internal/config/risk.go | 3 +- internal/config/risk_test.go | 10 ++ internal/config/workflow.go | 3 +- internal/config/workflow_test.go | 10 ++ internal/service/relational/ccf_internal.go | 3 +- internal/service/worker/risk_workers.go | 117 +++++++++++++++----- 9 files changed, 119 insertions(+), 36 deletions(-) diff --git a/docs/docs.go b/docs/docs.go index 706d84e0..c89a5d15 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -31539,7 +31539,7 @@ const docTemplate = `{ "type": "string" }, "riskNotificationsSubscribed": { - "description": "RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications", + "description": "RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications.\nThe DB default is intentionally true so existing users are opted in when the column is introduced.", "type": "boolean" }, "taskAvailableEmailSubscribed": { diff --git a/docs/swagger.json b/docs/swagger.json index 9a13d4e3..57cc9746 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -31533,7 +31533,7 @@ "type": "string" }, "riskNotificationsSubscribed": { - "description": "RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications", + "description": "RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications.\nThe DB default is intentionally true so existing users are opted in when the column is introduced.", "type": "boolean" }, "taskAvailableEmailSubscribed": { diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 05061ae3..f7313566 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -6629,8 +6629,9 @@ definitions: lastName: type: string riskNotificationsSubscribed: - description: RiskNotificationsSubscribed indicates if the user wants to receive - risk lifecycle notifications + description: |- + RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications. + The DB default is intentionally true so existing users are opted in when the column is introduced. type: boolean taskAvailableEmailSubscribed: description: TaskAvailableEmailSubscribed indicates if the user wants an email diff --git a/internal/config/risk.go b/internal/config/risk.go index e95e6203..c29f2000 100644 --- a/internal/config/risk.go +++ b/internal/config/risk.go @@ -3,6 +3,7 @@ package config import ( "errors" "fmt" + "os" "strings" "github.com/robfig/cron/v3" @@ -66,7 +67,7 @@ func LoadRiskConfig(path string) (*RiskConfig, error) { v.SetConfigType("yaml") if err := v.ReadInConfig(); err != nil { var notFound viper.ConfigFileNotFoundError - if !errors.As(err, ¬Found) { + if !errors.As(err, ¬Found) && !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("failed to read risk config file: %w", err) } } diff --git a/internal/config/risk_test.go b/internal/config/risk_test.go index 856a0e82..8b4b7e3b 100644 --- a/internal/config/risk_test.go +++ b/internal/config/risk_test.go @@ -92,6 +92,16 @@ auto_reopen_threshold_days: 60 assert.Equal(t, 60, cfg.AutoReopenThresholdDays) } +func TestLoadRiskConfigMissingFileUsesDefaults(t *testing.T) { + cfg, err := LoadRiskConfig(filepath.Join(t.TempDir(), "does-not-exist.yaml")) + require.NoError(t, err) + + def := DefaultRiskConfig() + assert.Equal(t, def.ReviewDeadlineReminderEnabled, cfg.ReviewDeadlineReminderEnabled) + assert.Equal(t, def.ReviewDeadlineReminderSchedule, cfg.ReviewDeadlineReminderSchedule) + assert.Equal(t, def.AutoReopenThresholdDays, cfg.AutoReopenThresholdDays) +} + func TestRiskConfigValidate(t *testing.T) { tests := []struct { name string diff --git a/internal/config/workflow.go b/internal/config/workflow.go index 4c2b4953..333e3a38 100644 --- a/internal/config/workflow.go +++ b/internal/config/workflow.go @@ -3,6 +3,7 @@ package config import ( "errors" "fmt" + "os" "strings" "github.com/robfig/cron/v3" @@ -75,7 +76,7 @@ func LoadWorkflowConfig(path string) (*WorkflowConfig, error) { v.SetConfigType("yaml") if err := v.ReadInConfig(); err != nil { var notFound viper.ConfigFileNotFoundError - if !errors.As(err, ¬Found) { + if !errors.As(err, ¬Found) && !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("failed to read workflow config file: %w", err) } // If file not found, we continue with defaults and env vars diff --git a/internal/config/workflow_test.go b/internal/config/workflow_test.go index 6b06d101..7cf7564c 100644 --- a/internal/config/workflow_test.go +++ b/internal/config/workflow_test.go @@ -108,6 +108,16 @@ overdue_check_enabled: false assert.False(t, config.OverdueCheckEnabled) } +func TestLoadWorkflowConfig_MissingFileUsesDefaults(t *testing.T) { + config, err := LoadWorkflowConfig(filepath.Join(t.TempDir(), "does-not-exist.yaml")) + require.NoError(t, err) + + def := DefaultWorkflowConfig() + assert.Equal(t, def.SchedulerEnabled, config.SchedulerEnabled) + assert.Equal(t, def.Schedule, config.Schedule) + assert.Equal(t, def.GracePeriodDays, config.GracePeriodDays) +} + func TestWorkflowConfig_Validate(t *testing.T) { tests := []struct { name string diff --git a/internal/service/relational/ccf_internal.go b/internal/service/relational/ccf_internal.go index bfdfa2a9..c3b233f1 100644 --- a/internal/service/relational/ccf_internal.go +++ b/internal/service/relational/ccf_internal.go @@ -36,7 +36,8 @@ type User struct { // TaskDailyDigestSubscribed indicates if the user wants to receive a daily task digest email TaskDailyDigestSubscribed bool `json:"taskDailyDigestSubscribed" gorm:"default:false"` - // RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications + // RiskNotificationsSubscribed indicates if the user wants to receive risk lifecycle notifications. + // The DB default is intentionally true so existing users are opted in when the column is introduced. RiskNotificationsSubscribed bool `json:"riskNotificationsSubscribed" gorm:"default:true"` } diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 3c89a25f..392c77a7 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -6,6 +6,7 @@ import ( "fmt" "sort" "strings" + "sync" "time" "github.com/compliance-framework/api/internal/service/email/types" @@ -40,13 +41,18 @@ func (w *RiskReviewDeadlineReminderScannerWorker) Work(ctx context.Context, _ *r return fmt.Errorf("risk deadline reminder scanner: query failed: %w", err) } + ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) + if err != nil { + return fmt.Errorf("risk deadline reminder scanner: resolve owners failed: %w", err) + } + params := make([]river.InsertManyParams, 0, len(risks)) for i := range risks { risk := &risks[i] - ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) - if err != nil { - return fmt.Errorf("risk deadline reminder scanner: resolve owners for risk %s: %w", risk.ID.String(), err) + if risk.ID == nil { + continue } + ownerIDs := ownersByRiskID[*risk.ID] for _, ownerID := range ownerIDs { params = append(params, river.InsertManyParams{ Args: RiskReviewDueReminderArgs{ @@ -108,15 +114,19 @@ func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ * return fmt.Errorf("risk overdue escalation scanner: query failed: %w", err) } + ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) + if err != nil { + return fmt.Errorf("risk overdue escalation scanner: resolve owners failed: %w", err) + } + params := make([]river.InsertManyParams, 0, len(risks)) reopenByRiskID := make(map[uuid.UUID]RiskReviewOverdueReopenArgs, len(risks)) for i := range risks { risk := &risks[i] - - ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) - if err != nil { - return fmt.Errorf("risk overdue escalation scanner: resolve owners for risk %s: %w", risk.ID.String(), err) + if risk.ID == nil { + continue } + ownerIDs := ownersByRiskID[*risk.ID] overdueWindow := now.Format("2006-01-02") for _, ownerID := range ownerIDs { params = append(params, river.InsertManyParams{ @@ -191,13 +201,18 @@ func (w *RiskStaleRiskScannerWorker) Work(ctx context.Context, _ *river.Job[Risk return fmt.Errorf("risk stale scanner: query failed: %w", err) } + ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) + if err != nil { + return fmt.Errorf("risk stale scanner: resolve owners failed: %w", err) + } + params := make([]river.InsertManyParams, 0, len(risks)) for i := range risks { risk := &risks[i] - ownerIDs, err := resolveRiskOwnerUserIDs(ctx, w.db, risk) - if err != nil { - return fmt.Errorf("risk stale scanner: resolve owners for risk %s: %w", risk.ID.String(), err) + if risk.ID == nil { + continue } + ownerIDs := ownersByRiskID[*risk.ID] for _, ownerID := range ownerIDs { params = append(params, river.InsertManyParams{ Args: RiskStaleOpenReminderArgs{ @@ -239,10 +254,16 @@ func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *r // 1) Evidence failures without linked risks. var orphanEvidence []relational.Evidence - if err := w.db.WithContext(ctx). - Model(&relational.Evidence{}). - Where("(status->>'state' = ? OR json_extract(status, '$.state') = ? OR CAST(status as TEXT) LIKE ?)", - relational.EvidenceStatusNotSatisfied, relational.EvidenceStatusNotSatisfied, "%\"state\":\"not-satisfied\"%"). + orphanQuery := w.db.WithContext(ctx).Model(&relational.Evidence{}) + switch w.db.Name() { + case "postgres": + orphanQuery = orphanQuery.Where("status->>'state' = ?", relational.EvidenceStatusNotSatisfied) + case "sqlite": + orphanQuery = orphanQuery.Where("json_extract(status, '$.state') = ?", relational.EvidenceStatusNotSatisfied) + default: + orphanQuery = orphanQuery.Where("status->>'state' = ?", relational.EvidenceStatusNotSatisfied) + } + if err := orphanQuery. Where("NOT EXISTS (SELECT 1 FROM risk_evidence_links rel WHERE rel.evidence_id = evidences.id)"). Find(&orphanEvidence).Error; err != nil { return fmt.Errorf("risk reconciliation scanner: query orphan evidence failed: %w", err) @@ -582,40 +603,78 @@ func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job return nil } -func resolveRiskOwnerUserIDs(ctx context.Context, db *gorm.DB, risk *riskrel.Risk) ([]uuid.UUID, error) { - if risk == nil { - return nil, nil +func resolveRiskOwnerUserIDsBatch(ctx context.Context, db *gorm.DB, risks []riskrel.Risk) (map[uuid.UUID][]uuid.UUID, error) { + ownerSetByRiskID := make(map[uuid.UUID]map[uuid.UUID]struct{}, len(risks)) + riskIDs := make([]uuid.UUID, 0, len(risks)) + for i := range risks { + risk := &risks[i] + if risk == nil || risk.ID == nil { + continue + } + riskID := *risk.ID + riskIDs = append(riskIDs, riskID) + if _, ok := ownerSetByRiskID[riskID]; !ok { + ownerSetByRiskID[riskID] = make(map[uuid.UUID]struct{}, 4) + } + if risk.PrimaryOwnerUserID != nil { + ownerSetByRiskID[riskID][*risk.PrimaryOwnerUserID] = struct{}{} + } } - - ownerSet := make(map[uuid.UUID]struct{}, 4) - if risk.PrimaryOwnerUserID != nil { - ownerSet[*risk.PrimaryOwnerUserID] = struct{}{} + if len(riskIDs) == 0 { + return map[uuid.UUID][]uuid.UUID{}, nil } var assignments []riskrel.RiskOwnerAssignment if err := db.WithContext(ctx). - Where("risk_id = ? AND owner_kind = ?", *risk.ID, "user"). + Where("risk_id IN ? AND owner_kind = ?", riskIDs, "user"). Find(&assignments).Error; err != nil { return nil, err } + for _, assignment := range assignments { parsed, err := uuid.Parse(assignment.OwnerRef) if err != nil { continue } - ownerSet[parsed] = struct{}{} + set, ok := ownerSetByRiskID[assignment.RiskID] + if !ok { + set = make(map[uuid.UUID]struct{}, 1) + ownerSetByRiskID[assignment.RiskID] = set + } + set[parsed] = struct{}{} } - owners := make([]uuid.UUID, 0, len(ownerSet)) - for ownerID := range ownerSet { - owners = append(owners, ownerID) + ownersByRiskID := make(map[uuid.UUID][]uuid.UUID, len(ownerSetByRiskID)) + for riskID, ownerSet := range ownerSetByRiskID { + owners := make([]uuid.UUID, 0, len(ownerSet)) + for ownerID := range ownerSet { + owners = append(owners, ownerID) + } + sort.Slice(owners, func(i, j int) bool { return owners[i].String() < owners[j].String() }) + ownersByRiskID[riskID] = owners } - sort.Slice(owners, func(i, j int) bool { return owners[i].String() < owners[j].String() }) - return owners, nil + + return ownersByRiskID, nil +} + +var systemCharacteristicsTableExistsCache sync.Map + +func hasSystemCharacteristicsTable(ctx context.Context, db *gorm.DB) bool { + sqlDB, err := db.DB() + if err != nil { + return false + } + if cached, ok := systemCharacteristicsTableExistsCache.Load(sqlDB); ok { + return cached.(bool) + } + + exists := db.WithContext(ctx).Migrator().HasTable(&relational.SystemCharacteristics{}) + systemCharacteristicsTableExistsCache.Store(sqlDB, exists) + return exists } func resolveSSPDisplayName(ctx context.Context, db *gorm.DB, sspID uuid.UUID) string { - if !db.WithContext(ctx).Migrator().HasTable(&relational.SystemCharacteristics{}) { + if !hasSystemCharacteristicsTable(ctx, db) { return sspID.String() } From 5209828eaf7d0f6b2dd297f9cdbf9cfdcdef3478 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 09:31:57 -0300 Subject: [PATCH 04/16] fix: copilot issues --- internal/service/worker/risk_workers.go | 6 +++--- internal/service/worker/risk_workers_test.go | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 392c77a7..9ed5e882 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -156,7 +156,7 @@ func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ * for _, args := range reopenByRiskID { params = append(params, river.InsertManyParams{ Args: args, - InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), }) } @@ -281,7 +281,7 @@ func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *r EvidenceEnd: e.End.UTC().Format(time.RFC3339), Status: state, }, - InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), }) } @@ -334,7 +334,7 @@ func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *r EvidenceEnd: evidence.End.UTC().Format(time.RFC3339), Status: state, }, - InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), }) } diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 0d34b43b..4a584b23 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -207,12 +207,15 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T err := w.Work(context.Background(), &river.Job[RiskEvidenceReconciliationScannerArgs]{}) require.NoError(t, err) - var sawEvidenceRepair, sawDuplicateRepair bool + var sawEvidenceRepair, sawEvidenceRepairWithRetries, sawDuplicateRepair bool for _, p := range client.params { switch args := p.Args.(type) { case RiskProcessEvidenceFailureArgs: if args.EvidenceID == evidenceID { sawEvidenceRepair = true + if p.InsertOpts != nil && p.InsertOpts.MaxAttempts == 5 { + sawEvidenceRepairWithRetries = true + } } case RiskReconcileDuplicatesArgs: if args.DedupeKey == "dup-key" { @@ -221,6 +224,7 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T } } assert.True(t, sawEvidenceRepair) + assert.True(t, sawEvidenceRepairWithRetries) assert.True(t, sawDuplicateRepair) } From 86a0ec443c4329b1f711869ea58d05032489b613 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 09:53:48 -0300 Subject: [PATCH 05/16] fix: copilot issues --- internal/service/worker/risk_workers.go | 91 +++++++++++++------- internal/service/worker/risk_workers_test.go | 5 ++ 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 9ed5e882..82044f83 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -156,7 +156,7 @@ func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ * for _, args := range reopenByRiskID { params = append(params, river.InsertManyParams{ Args: args, - InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), + InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), }) } @@ -298,44 +298,71 @@ func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *r return fmt.Errorf("risk reconciliation scanner: query missing subject links failed: %w", err) } - for _, riskID := range riskIDs { - var evidence relational.Evidence - err := w.db.WithContext(ctx). - Model(&relational.Evidence{}). - Joins("JOIN risk_evidence_links rel ON rel.evidence_id = evidences.id"). - Where("rel.risk_id = ?", riskID). - Order("evidences.end DESC"). - First(&evidence).Error - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { + if len(riskIDs) > 0 { + type latestRiskEvidence struct { + RiskID uuid.UUID `gorm:"column:risk_id"` + EvidenceID uuid.UUID `gorm:"column:evidence_id"` + } + var latestEvidence []latestRiskEvidence + if err := w.db.WithContext(ctx). + Table("risk_evidence_links rel"). + Select("rel.risk_id, rel.evidence_id"). + Joins("JOIN evidences e ON e.id = rel.evidence_id"). + Where("rel.risk_id IN ?", riskIDs). + Where("EXISTS (SELECT 1 FROM evidence_subjects es WHERE es.evidence_id = rel.evidence_id)"). + Where(`NOT EXISTS ( + SELECT 1 + FROM risk_evidence_links rel2 + JOIN evidences e2 ON e2.id = rel2.evidence_id + WHERE rel2.risk_id = rel.risk_id + AND (e2.end > e.end OR (e2.end = e.end AND rel2.evidence_id > rel.evidence_id)) + )`). + Scan(&latestEvidence).Error; err != nil { + return fmt.Errorf("risk reconciliation scanner: query latest evidence for missing subject links failed: %w", err) + } + + evidenceIDs := make([]uuid.UUID, 0, len(latestEvidence)) + seenEvidenceIDs := make(map[uuid.UUID]struct{}, len(latestEvidence)) + for _, item := range latestEvidence { + if _, seen := seenEvidenceIDs[item.EvidenceID]; seen { continue } - return fmt.Errorf("risk reconciliation scanner: load evidence for risk %s failed: %w", riskID.String(), err) + seenEvidenceIDs[item.EvidenceID] = struct{}{} + evidenceIDs = append(evidenceIDs, item.EvidenceID) } - var subjectCount int64 - if err := w.db.WithContext(ctx). - Table("evidence_subjects"). - Where("evidence_id = ?", *evidence.ID). - Count(&subjectCount).Error; err != nil { - return fmt.Errorf("risk reconciliation scanner: count evidence subjects failed: %w", err) - } - if subjectCount == 0 { - continue + evidenceByID := make(map[uuid.UUID]relational.Evidence, len(evidenceIDs)) + if len(evidenceIDs) > 0 { + var evidences []relational.Evidence + if err := w.db.WithContext(ctx).Where("id IN ?", evidenceIDs).Find(&evidences).Error; err != nil { + return fmt.Errorf("risk reconciliation scanner: load latest evidence records failed: %w", err) + } + for i := range evidences { + if evidences[i].ID == nil { + continue + } + evidenceByID[*evidences[i].ID] = evidences[i] + } } - state := evidence.Status.Data().State - if state == "" { - state = relational.EvidenceStatusNotSatisfied + for _, item := range latestEvidence { + evidence, found := evidenceByID[item.EvidenceID] + if !found { + continue + } + state := evidence.Status.Data().State + if state == "" { + state = relational.EvidenceStatusNotSatisfied + } + params = append(params, river.InsertManyParams{ + Args: RiskProcessEvidenceFailureArgs{ + EvidenceID: item.EvidenceID, + EvidenceEnd: evidence.End.UTC().Format(time.RFC3339), + Status: state, + }, + InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), + }) } - params = append(params, river.InsertManyParams{ - Args: RiskProcessEvidenceFailureArgs{ - EvidenceID: *evidence.ID, - EvidenceEnd: evidence.End.UTC().Format(time.RFC3339), - Status: state, - }, - InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), - }) } // 3) Duplicate active risks with same dedupe key. diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 4a584b23..6e652872 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -130,16 +130,21 @@ func TestRiskReviewOverdueEscalationScannerWorker_EnqueuesEscalationAndReopen(t require.NoError(t, err) var escalationCount, reopenCount int + var reopenInsertOpts *river.InsertOpts for _, p := range client.params { switch p.Args.(type) { case RiskReviewOverdueEscalationArgs: escalationCount++ case RiskReviewOverdueReopenArgs: reopenCount++ + reopenInsertOpts = p.InsertOpts } } assert.Equal(t, 1, escalationCount) assert.Equal(t, 1, reopenCount) + require.NotNil(t, reopenInsertOpts) + assert.Equal(t, 24*time.Hour, reopenInsertOpts.UniqueOpts.ByPeriod) + assert.Equal(t, 3, reopenInsertOpts.MaxAttempts) } func TestRiskStaleRiskScannerWorker_EnqueuesWeeklyReminder(t *testing.T) { From 46a9204cf6628851c8e4912cfee5b86f4853f748 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 10:10:00 -0300 Subject: [PATCH 06/16] Reject zero auto reopen threshold --- internal/config/risk.go | 3 +++ internal/config/risk_test.go | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/internal/config/risk.go b/internal/config/risk.go index c29f2000..9ce2a6af 100644 --- a/internal/config/risk.go +++ b/internal/config/risk.go @@ -110,6 +110,9 @@ func (c *RiskConfig) Validate() error { if c.AutoReopenThresholdDays < 0 { return fmt.Errorf("risk auto reopen threshold days must be non-negative") } + if c.AutoReopenEnabled && c.AutoReopenThresholdDays <= 0 { + return fmt.Errorf("risk auto reopen threshold days must be greater than zero when auto reopen is enabled") + } return nil } diff --git a/internal/config/risk_test.go b/internal/config/risk_test.go index 8b4b7e3b..d9d1d722 100644 --- a/internal/config/risk_test.go +++ b/internal/config/risk_test.go @@ -132,6 +132,14 @@ func TestRiskConfigValidate(t *testing.T) { }, wantErr: true, }, + { + name: "zero threshold invalid when auto reopen enabled", + cfg: &RiskConfig{ + AutoReopenEnabled: true, + AutoReopenThresholdDays: 0, + }, + wantErr: true, + }, { name: "disabled schedule may be invalid", cfg: &RiskConfig{ From 944b53f309c9ad7c5197183fb02b2217420b1471 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 10:26:23 -0300 Subject: [PATCH 07/16] Fix resolveSSP display name fallback --- internal/service/worker/risk_workers.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 82044f83..ed4d9eb3 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -6,7 +6,6 @@ import ( "fmt" "sort" "strings" - "sync" "time" "github.com/compliance-framework/api/internal/service/email/types" @@ -684,27 +683,7 @@ func resolveRiskOwnerUserIDsBatch(ctx context.Context, db *gorm.DB, risks []risk return ownersByRiskID, nil } -var systemCharacteristicsTableExistsCache sync.Map - -func hasSystemCharacteristicsTable(ctx context.Context, db *gorm.DB) bool { - sqlDB, err := db.DB() - if err != nil { - return false - } - if cached, ok := systemCharacteristicsTableExistsCache.Load(sqlDB); ok { - return cached.(bool) - } - - exists := db.WithContext(ctx).Migrator().HasTable(&relational.SystemCharacteristics{}) - systemCharacteristicsTableExistsCache.Store(sqlDB, exists) - return exists -} - func resolveSSPDisplayName(ctx context.Context, db *gorm.DB, sspID uuid.UUID) string { - if !hasSystemCharacteristicsTable(ctx, db) { - return sspID.String() - } - var sc relational.SystemCharacteristics if err := db.WithContext(ctx). Select("system_name_short", "system_name"). From 8b65e7d2bd3cfd5f378dcdf90a3705e9cef8450e Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 10:51:26 -0300 Subject: [PATCH 08/16] Limit risk evidence orphan query --- internal/service/worker/risk_workers.go | 5 ++ internal/service/worker/risk_workers_test.go | 60 +++++++++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index ed4d9eb3..a5276a05 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -18,6 +18,8 @@ import ( "gorm.io/gorm" ) +const riskEvidenceReconciliationOrphanLimit = 500 + type RiskReviewDeadlineReminderScannerWorker struct { db *gorm.DB client workflow.RiverClient @@ -264,6 +266,9 @@ func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *r } if err := orphanQuery. Where("NOT EXISTS (SELECT 1 FROM risk_evidence_links rel WHERE rel.evidence_id = evidences.id)"). + Order("evidences.end DESC"). + Order("evidences.id DESC"). + Limit(riskEvidenceReconciliationOrphanLimit). Find(&orphanEvidence).Error; err != nil { return fmt.Errorf("risk reconciliation scanner: query orphan evidence failed: %w", err) } diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 6e652872..f8974ed5 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -168,6 +168,7 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T db := newRiskWorkersTestDB(t) client := &stubRiverClient{} logger := zap.NewNop().Sugar() + now := time.Now().UTC() // Evidence failure without linked risk evidenceID := uuid.New() @@ -175,11 +176,49 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T UUIDModel: relational.UUIDModel{ID: &evidenceID}, UUID: uuid.New(), Title: "failing evidence", - Start: time.Now().UTC().Add(-time.Hour), - End: time.Now().UTC(), + Start: now.Add(-time.Hour), + End: now, + Status: datatypes.NewJSONType(oscalTypes_1_1_3.ObjectiveStatus{State: relational.EvidenceStatusNotSatisfied}), + }).Error) + + // Evidence-linked risk with missing risk_subject_links + path2SSP := uuid.New() + require.NoError(t, db.Create(&relational.SystemSecurityPlan{UUIDModel: relational.UUIDModel{ID: &path2SSP}}).Error) + path2RiskID := uuid.New() + require.NoError(t, db.Create(&riskrel.Risk{ + UUIDModel: relational.UUIDModel{ID: &path2RiskID}, + Title: "auto risk missing subjects", + Description: "auto risk missing subjects", + Status: string(riskrel.RiskStatusInvestigating), + SSPID: path2SSP, + SourceType: string(riskrel.RiskSourceTypeEvidenceAuto), + FirstSeenAt: now.Add(-2 * time.Hour), + LastSeenAt: now, + }).Error) + + path2EvidenceID := uuid.New() + path2Evidence := relational.Evidence{ + UUIDModel: relational.UUIDModel{ID: &path2EvidenceID}, + UUID: uuid.New(), + Title: "linked failing evidence", + Start: now.Add(-2 * time.Hour), + End: now.Add(-30 * time.Minute), Status: datatypes.NewJSONType(oscalTypes_1_1_3.ObjectiveStatus{State: relational.EvidenceStatusNotSatisfied}), + } + require.NoError(t, db.Create(&path2Evidence).Error) + require.NoError(t, db.Create(&riskrel.RiskEvidenceLink{ + RiskID: path2RiskID, + EvidenceID: path2EvidenceID, }).Error) + subjectID := uuid.New() + subject := relational.AssessmentSubject{ + UUIDModel: relational.UUIDModel{ID: &subjectID}, + Type: "component", + } + require.NoError(t, db.Create(&subject).Error) + require.NoError(t, db.Model(&path2Evidence).Association("Subjects").Append(&subject)) + // Duplicate active risks by dedupe key dupSSP := uuid.New() require.NoError(t, db.Create(&relational.SystemSecurityPlan{UUIDModel: relational.UUIDModel{ID: &dupSSP}}).Error) @@ -193,8 +232,8 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T SSPID: dupSSP, SourceType: string(riskrel.RiskSourceTypeManual), DedupeKey: "dup-key", - FirstSeenAt: time.Now().UTC(), - LastSeenAt: time.Now().UTC(), + FirstSeenAt: now, + LastSeenAt: now, }).Error) require.NoError(t, db.Create(&riskrel.Risk{ UUIDModel: relational.UUIDModel{ID: &r2ID}, @@ -204,15 +243,15 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T SSPID: dupSSP, SourceType: string(riskrel.RiskSourceTypeManual), DedupeKey: "dup-key", - FirstSeenAt: time.Now().UTC(), - LastSeenAt: time.Now().UTC(), + FirstSeenAt: now, + LastSeenAt: now, }).Error) w := NewRiskEvidenceReconciliationScannerWorker(db, client, logger) err := w.Work(context.Background(), &river.Job[RiskEvidenceReconciliationScannerArgs]{}) require.NoError(t, err) - var sawEvidenceRepair, sawEvidenceRepairWithRetries, sawDuplicateRepair bool + var sawEvidenceRepair, sawMissingSubjectRepair, sawEvidenceRepairWithRetries, sawDuplicateRepair bool for _, p := range client.params { switch args := p.Args.(type) { case RiskProcessEvidenceFailureArgs: @@ -222,6 +261,12 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T sawEvidenceRepairWithRetries = true } } + if args.EvidenceID == path2EvidenceID { + sawMissingSubjectRepair = true + if p.InsertOpts != nil && p.InsertOpts.MaxAttempts == 5 { + sawEvidenceRepairWithRetries = true + } + } case RiskReconcileDuplicatesArgs: if args.DedupeKey == "dup-key" { sawDuplicateRepair = true @@ -229,6 +274,7 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T } } assert.True(t, sawEvidenceRepair) + assert.True(t, sawMissingSubjectRepair) assert.True(t, sawEvidenceRepairWithRetries) assert.True(t, sawDuplicateRepair) } From 9699b54fcd55b88036ef8c3c1953cd38e73b9b37 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 11:12:29 -0300 Subject: [PATCH 09/16] Clear acceptance justification --- internal/service/worker/risk_workers.go | 1 + internal/service/worker/risk_workers_test.go | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index a5276a05..b9817c21 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -619,6 +619,7 @@ func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job oldStatus := risk.Status risk.Status = string(riskrel.RiskStatusInvestigating) risk.ReviewDeadline = nil + risk.AcceptanceJustification = nil if _, err := riskSvc.Update(riskrel.UpdateRiskParams{ Risk: risk, OldStatus: oldStatus, diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index f8974ed5..ce4d2b8d 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -328,6 +328,10 @@ func TestRiskReviewOverdueReopenWorker_ReopensAcceptedRisk(t *testing.T) { logger := zap.NewNop().Sugar() reviewDeadline := time.Now().UTC().Add(-40 * 24 * time.Hour) risk, _ := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, time.Now().UTC()) + justification := "temporarily accepted due to compensating controls" + require.NoError(t, db.Model(&riskrel.Risk{}). + Where("id = ?", risk.ID). + Update("acceptance_justification", justification).Error) w := NewRiskReviewOverdueReopenWorker(db, logger) err := w.Work(context.Background(), &river.Job[RiskReviewOverdueReopenArgs]{ @@ -342,6 +346,7 @@ func TestRiskReviewOverdueReopenWorker_ReopensAcceptedRisk(t *testing.T) { require.NoError(t, db.First(&updated, "id = ?", risk.ID).Error) assert.Equal(t, string(riskrel.RiskStatusInvestigating), updated.Status) assert.Nil(t, updated.ReviewDeadline) + assert.Nil(t, updated.AcceptanceJustification) } func TestRiskReviewDueReminderWorker_RespectsRiskSubscription(t *testing.T) { From cc9c5c506748429f28c0d159c0010fe88cadb44f Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 11:41:00 -0300 Subject: [PATCH 10/16] fix: remove uneeded self-heal behavior --- internal/service/worker/risk_workers.go | 163 ++----------------- internal/service/worker/risk_workers_test.go | 123 ++++++-------- 2 files changed, 72 insertions(+), 214 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index b9817c21..d100eb56 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -18,8 +18,6 @@ import ( "gorm.io/gorm" ) -const riskEvidenceReconciliationOrphanLimit = 500 - type RiskReviewDeadlineReminderScannerWorker struct { db *gorm.DB client workflow.RiverClient @@ -253,123 +251,7 @@ func NewRiskEvidenceReconciliationScannerWorker(db *gorm.DB, client workflow.Riv func (w *RiskEvidenceReconciliationScannerWorker) Work(ctx context.Context, _ *river.Job[RiskEvidenceReconciliationScannerArgs]) error { var params []river.InsertManyParams - // 1) Evidence failures without linked risks. - var orphanEvidence []relational.Evidence - orphanQuery := w.db.WithContext(ctx).Model(&relational.Evidence{}) - switch w.db.Name() { - case "postgres": - orphanQuery = orphanQuery.Where("status->>'state' = ?", relational.EvidenceStatusNotSatisfied) - case "sqlite": - orphanQuery = orphanQuery.Where("json_extract(status, '$.state') = ?", relational.EvidenceStatusNotSatisfied) - default: - orphanQuery = orphanQuery.Where("status->>'state' = ?", relational.EvidenceStatusNotSatisfied) - } - if err := orphanQuery. - Where("NOT EXISTS (SELECT 1 FROM risk_evidence_links rel WHERE rel.evidence_id = evidences.id)"). - Order("evidences.end DESC"). - Order("evidences.id DESC"). - Limit(riskEvidenceReconciliationOrphanLimit). - Find(&orphanEvidence).Error; err != nil { - return fmt.Errorf("risk reconciliation scanner: query orphan evidence failed: %w", err) - } - - for i := range orphanEvidence { - e := &orphanEvidence[i] - state := e.Status.Data().State - if state == "" { - state = relational.EvidenceStatusNotSatisfied - } - params = append(params, river.InsertManyParams{ - Args: RiskProcessEvidenceFailureArgs{ - EvidenceID: *e.ID, - EvidenceEnd: e.End.UTC().Format(time.RFC3339), - Status: state, - }, - InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), - }) - } - - // 2) Evidence-linked risks with missing subject links. - var riskIDs []uuid.UUID - if err := w.db.WithContext(ctx). - Model(&riskrel.Risk{}). - Select("risk_register_risks.id"). - Where("risk_register_risks.source_type = ? AND risk_register_risks.status <> ?", - string(riskrel.RiskSourceTypeEvidenceAuto), string(riskrel.RiskStatusClosed)). - Where("EXISTS (SELECT 1 FROM risk_evidence_links rel WHERE rel.risk_id = risk_register_risks.id)"). - Where("NOT EXISTS (SELECT 1 FROM risk_subject_links rsl WHERE rsl.risk_id = risk_register_risks.id)"). - Pluck("risk_register_risks.id", &riskIDs).Error; err != nil { - return fmt.Errorf("risk reconciliation scanner: query missing subject links failed: %w", err) - } - - if len(riskIDs) > 0 { - type latestRiskEvidence struct { - RiskID uuid.UUID `gorm:"column:risk_id"` - EvidenceID uuid.UUID `gorm:"column:evidence_id"` - } - var latestEvidence []latestRiskEvidence - if err := w.db.WithContext(ctx). - Table("risk_evidence_links rel"). - Select("rel.risk_id, rel.evidence_id"). - Joins("JOIN evidences e ON e.id = rel.evidence_id"). - Where("rel.risk_id IN ?", riskIDs). - Where("EXISTS (SELECT 1 FROM evidence_subjects es WHERE es.evidence_id = rel.evidence_id)"). - Where(`NOT EXISTS ( - SELECT 1 - FROM risk_evidence_links rel2 - JOIN evidences e2 ON e2.id = rel2.evidence_id - WHERE rel2.risk_id = rel.risk_id - AND (e2.end > e.end OR (e2.end = e.end AND rel2.evidence_id > rel.evidence_id)) - )`). - Scan(&latestEvidence).Error; err != nil { - return fmt.Errorf("risk reconciliation scanner: query latest evidence for missing subject links failed: %w", err) - } - - evidenceIDs := make([]uuid.UUID, 0, len(latestEvidence)) - seenEvidenceIDs := make(map[uuid.UUID]struct{}, len(latestEvidence)) - for _, item := range latestEvidence { - if _, seen := seenEvidenceIDs[item.EvidenceID]; seen { - continue - } - seenEvidenceIDs[item.EvidenceID] = struct{}{} - evidenceIDs = append(evidenceIDs, item.EvidenceID) - } - - evidenceByID := make(map[uuid.UUID]relational.Evidence, len(evidenceIDs)) - if len(evidenceIDs) > 0 { - var evidences []relational.Evidence - if err := w.db.WithContext(ctx).Where("id IN ?", evidenceIDs).Find(&evidences).Error; err != nil { - return fmt.Errorf("risk reconciliation scanner: load latest evidence records failed: %w", err) - } - for i := range evidences { - if evidences[i].ID == nil { - continue - } - evidenceByID[*evidences[i].ID] = evidences[i] - } - } - - for _, item := range latestEvidence { - evidence, found := evidenceByID[item.EvidenceID] - if !found { - continue - } - state := evidence.Status.Data().State - if state == "" { - state = relational.EvidenceStatusNotSatisfied - } - params = append(params, river.InsertManyParams{ - Args: RiskProcessEvidenceFailureArgs{ - EvidenceID: item.EvidenceID, - EvidenceEnd: evidence.End.UTC().Format(time.RFC3339), - Status: state, - }, - InsertOpts: JobInsertOptionsForRiskProcessEvidenceFailure(), - }) - } - } - - // 3) Duplicate active risks with same dedupe key. + // Duplicate active risks with same dedupe key. var duplicateKeys []string if err := w.db.WithContext(ctx). Model(&riskrel.Risk{}). @@ -597,35 +479,26 @@ func NewRiskReviewOverdueReopenWorker(db *gorm.DB, logger *zap.SugaredLogger) *R } func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job[RiskReviewOverdueReopenArgs]) error { - riskSvc := riskrel.NewRiskService(w.db) - risk, err := riskSvc.GetByID(job.Args.RiskID) - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { - w.logger.Warnw("RiskReviewOverdueReopenWorker: risk not found, skipping", "risk_id", job.Args.RiskID) - return nil - } - return fmt.Errorf("risk overdue reopen: load risk failed: %w", err) - } - if risk.Status != string(riskrel.RiskStatusRiskAccepted) || risk.ReviewDeadline == nil { - return nil - } - now := time.Now().UTC() threshold := time.Duration(job.Args.ThresholdDays) * 24 * time.Hour - if now.Sub(risk.ReviewDeadline.UTC()) < threshold { - return nil + cutoff := now.Add(-threshold) + updateResult := w.db.WithContext(ctx). + Model(&riskrel.Risk{}). + Where("id = ? AND status = ? AND review_deadline IS NOT NULL AND review_deadline <= ?", + job.Args.RiskID, + string(riskrel.RiskStatusRiskAccepted), + cutoff, + ). + Updates(map[string]interface{}{ + "status": string(riskrel.RiskStatusInvestigating), + "review_deadline": nil, + "acceptance_justification": nil, + }) + if updateResult.Error != nil { + return fmt.Errorf("risk overdue reopen: update risk failed: %w", updateResult.Error) } - - oldStatus := risk.Status - risk.Status = string(riskrel.RiskStatusInvestigating) - risk.ReviewDeadline = nil - risk.AcceptanceJustification = nil - if _, err := riskSvc.Update(riskrel.UpdateRiskParams{ - Risk: risk, - OldStatus: oldStatus, - StatusChanged: true, - }); err != nil { - return fmt.Errorf("risk overdue reopen: update risk failed: %w", err) + if updateResult.RowsAffected == 0 { + return nil } w.logger.Infow("RiskReviewOverdueReopenWorker: reopened overdue accepted risk", diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index ce4d2b8d..346bf6f4 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -2,13 +2,13 @@ package worker import ( "context" + "errors" "testing" "time" "github.com/compliance-framework/api/internal/service/email/types" "github.com/compliance-framework/api/internal/service/relational" riskrel "github.com/compliance-framework/api/internal/service/relational/risks" - oscalTypes_1_1_3 "github.com/defenseunicorns/go-oscal/src/types/oscal-1-1-3" "github.com/google/uuid" "github.com/riverqueue/river" "github.com/riverqueue/river/rivertype" @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap" - "gorm.io/datatypes" "gorm.io/driver/sqlite" "gorm.io/gorm" ) @@ -164,61 +163,12 @@ func TestRiskStaleRiskScannerWorker_EnqueuesWeeklyReminder(t *testing.T) { assert.Equal(t, 7*24*time.Hour, client.params[0].InsertOpts.UniqueOpts.ByPeriod) } -func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T) { +func TestRiskEvidenceReconciliationScannerWorker_EnqueuesDuplicateRepairJobs(t *testing.T) { db := newRiskWorkersTestDB(t) client := &stubRiverClient{} logger := zap.NewNop().Sugar() now := time.Now().UTC() - // Evidence failure without linked risk - evidenceID := uuid.New() - require.NoError(t, db.Create(&relational.Evidence{ - UUIDModel: relational.UUIDModel{ID: &evidenceID}, - UUID: uuid.New(), - Title: "failing evidence", - Start: now.Add(-time.Hour), - End: now, - Status: datatypes.NewJSONType(oscalTypes_1_1_3.ObjectiveStatus{State: relational.EvidenceStatusNotSatisfied}), - }).Error) - - // Evidence-linked risk with missing risk_subject_links - path2SSP := uuid.New() - require.NoError(t, db.Create(&relational.SystemSecurityPlan{UUIDModel: relational.UUIDModel{ID: &path2SSP}}).Error) - path2RiskID := uuid.New() - require.NoError(t, db.Create(&riskrel.Risk{ - UUIDModel: relational.UUIDModel{ID: &path2RiskID}, - Title: "auto risk missing subjects", - Description: "auto risk missing subjects", - Status: string(riskrel.RiskStatusInvestigating), - SSPID: path2SSP, - SourceType: string(riskrel.RiskSourceTypeEvidenceAuto), - FirstSeenAt: now.Add(-2 * time.Hour), - LastSeenAt: now, - }).Error) - - path2EvidenceID := uuid.New() - path2Evidence := relational.Evidence{ - UUIDModel: relational.UUIDModel{ID: &path2EvidenceID}, - UUID: uuid.New(), - Title: "linked failing evidence", - Start: now.Add(-2 * time.Hour), - End: now.Add(-30 * time.Minute), - Status: datatypes.NewJSONType(oscalTypes_1_1_3.ObjectiveStatus{State: relational.EvidenceStatusNotSatisfied}), - } - require.NoError(t, db.Create(&path2Evidence).Error) - require.NoError(t, db.Create(&riskrel.RiskEvidenceLink{ - RiskID: path2RiskID, - EvidenceID: path2EvidenceID, - }).Error) - - subjectID := uuid.New() - subject := relational.AssessmentSubject{ - UUIDModel: relational.UUIDModel{ID: &subjectID}, - Type: "component", - } - require.NoError(t, db.Create(&subject).Error) - require.NoError(t, db.Model(&path2Evidence).Association("Subjects").Append(&subject)) - // Duplicate active risks by dedupe key dupSSP := uuid.New() require.NoError(t, db.Create(&relational.SystemSecurityPlan{UUIDModel: relational.UUIDModel{ID: &dupSSP}}).Error) @@ -251,31 +201,15 @@ func TestRiskEvidenceReconciliationScannerWorker_EnqueuesRepairJobs(t *testing.T err := w.Work(context.Background(), &river.Job[RiskEvidenceReconciliationScannerArgs]{}) require.NoError(t, err) - var sawEvidenceRepair, sawMissingSubjectRepair, sawEvidenceRepairWithRetries, sawDuplicateRepair bool + var sawDuplicateRepair bool for _, p := range client.params { switch args := p.Args.(type) { - case RiskProcessEvidenceFailureArgs: - if args.EvidenceID == evidenceID { - sawEvidenceRepair = true - if p.InsertOpts != nil && p.InsertOpts.MaxAttempts == 5 { - sawEvidenceRepairWithRetries = true - } - } - if args.EvidenceID == path2EvidenceID { - sawMissingSubjectRepair = true - if p.InsertOpts != nil && p.InsertOpts.MaxAttempts == 5 { - sawEvidenceRepairWithRetries = true - } - } case RiskReconcileDuplicatesArgs: if args.DedupeKey == "dup-key" { sawDuplicateRepair = true } } } - assert.True(t, sawEvidenceRepair) - assert.True(t, sawMissingSubjectRepair) - assert.True(t, sawEvidenceRepairWithRetries) assert.True(t, sawDuplicateRepair) } @@ -397,3 +331,54 @@ func TestRiskReviewDueReminderWorker_SendsWhenSubscribed(t *testing.T) { require.NoError(t, err) mockEmail.AssertExpectations(t) } + +func TestRiskReviewDueReminderWorker_TemplateError_ReturnsError(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(7 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + createTestUser(t, db, ownerID, true) + + mockEmail := &MockEmailService{} + mockEmail.On("UseTemplate", "risk-review-due-reminder", mock.Anything).Return("", "", errors.New("template boom")) + + userRepo := NewGORMUserRepository(db) + worker := NewRiskReviewDueReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewDueReminderArgs]{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, "render template") + require.ErrorContains(t, err, "template boom") + mockEmail.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) +} + +func TestRiskReviewDueReminderWorker_SendUnsuccessful_ReturnsError(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(7 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + createTestUser(t, db, ownerID, true) + + mockEmail := &MockEmailService{} + mockEmail.On("UseTemplate", "risk-review-due-reminder", mock.Anything).Return("ok", "ok", nil) + mockEmail.On("GetDefaultFromAddress").Return("noreply@example.com") + mockEmail.On("Send", mock.Anything, mock.Anything). + Return(&types.SendResult{Success: false, Error: "provider refused"}, nil) + + userRepo := NewGORMUserRepository(db) + worker := NewRiskReviewDueReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewDueReminderArgs]{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, "email send failed: provider refused") +} From de991608939d265f1de30782140deddce37336aa Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 12:39:48 -0300 Subject: [PATCH 11/16] Use risk service for overdue reopen --- internal/service/worker/risk_workers.go | 38 ++++++++++++-------- internal/service/worker/risk_workers_test.go | 6 ++++ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index d100eb56..75df6615 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -482,25 +482,33 @@ func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job now := time.Now().UTC() threshold := time.Duration(job.Args.ThresholdDays) * 24 * time.Hour cutoff := now.Add(-threshold) - updateResult := w.db.WithContext(ctx). - Model(&riskrel.Risk{}). - Where("id = ? AND status = ? AND review_deadline IS NOT NULL AND review_deadline <= ?", - job.Args.RiskID, - string(riskrel.RiskStatusRiskAccepted), - cutoff, - ). - Updates(map[string]interface{}{ - "status": string(riskrel.RiskStatusInvestigating), - "review_deadline": nil, - "acceptance_justification": nil, - }) - if updateResult.Error != nil { - return fmt.Errorf("risk overdue reopen: update risk failed: %w", updateResult.Error) + + riskSvc := riskrel.NewRiskService(w.db.WithContext(ctx)) + risk, err := riskSvc.GetByID(job.Args.RiskID) + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil + } + return fmt.Errorf("risk overdue reopen: load risk failed: %w", err) } - if updateResult.RowsAffected == 0 { + + if risk.Status != string(riskrel.RiskStatusRiskAccepted) || risk.ReviewDeadline == nil || risk.ReviewDeadline.UTC().After(cutoff) { return nil } + oldStatus := risk.Status + risk.Status = string(riskrel.RiskStatusInvestigating) + risk.ReviewDeadline = nil + risk.AcceptanceJustification = nil + + if _, err := riskSvc.Update(riskrel.UpdateRiskParams{ + Risk: risk, + OldStatus: oldStatus, + StatusChanged: oldStatus != risk.Status, + }); err != nil { + return fmt.Errorf("risk overdue reopen: update risk failed: %w", err) + } + w.logger.Infow("RiskReviewOverdueReopenWorker: reopened overdue accepted risk", "risk_id", job.Args.RiskID, "threshold_days", job.Args.ThresholdDays, diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 346bf6f4..bab374fe 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -281,6 +281,12 @@ func TestRiskReviewOverdueReopenWorker_ReopensAcceptedRisk(t *testing.T) { assert.Equal(t, string(riskrel.RiskStatusInvestigating), updated.Status) assert.Nil(t, updated.ReviewDeadline) assert.Nil(t, updated.AcceptanceJustification) + + var statusChangeEvents int64 + require.NoError(t, db.Model(&riskrel.RiskEvent{}). + Where("risk_id = ? AND event_type = ?", risk.ID, string(riskrel.RiskEventTypeStatusChange)). + Count(&statusChangeEvents).Error) + assert.Equal(t, int64(1), statusChangeEvents) } func TestRiskReviewDueReminderWorker_RespectsRiskSubscription(t *testing.T) { From 4a321a9dc49064cf4db8d91acf08354a8f7aa961 Mon Sep 17 00:00:00 2001 From: Gustavo Fernandes de Carvalho <17139678+gusfcarvalho@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:52:15 -0300 Subject: [PATCH 12/16] fix: use context on db Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/service/worker/risk_workers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 75df6615..d3aae527 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -446,7 +446,7 @@ func (w *RiskReconcileDuplicatesWorker) Work(ctx context.Context, job *river.Job return nil } - riskSvc := riskrel.NewRiskService(w.db) + riskSvc := riskrel.NewRiskService(w.db.WithContext(ctx)) keeper := duplicates[0] for i := 1; i < len(duplicates); i++ { risk := duplicates[i] From 93b60515511bbcfe953aa25d394e473de0f66eb8 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 13:48:40 -0300 Subject: [PATCH 13/16] Update risk scanner batching --- internal/service/worker/risk_workers.go | 270 +++++++++++-------- internal/service/worker/risk_workers_test.go | 89 ++++++ internal/service/worker/user_repository.go | 2 +- 3 files changed, 244 insertions(+), 117 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index d3aae527..085a64aa 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -18,6 +18,8 @@ import ( "gorm.io/gorm" ) +const riskScannerBatchSize = 1000 + type RiskReviewDeadlineReminderScannerWorker struct { db *gorm.DB client workflow.RiverClient @@ -32,49 +34,58 @@ func (w *RiskReviewDeadlineReminderScannerWorker) Work(ctx context.Context, _ *r now := time.Now().UTC() windowEnd := now.Add(30 * 24 * time.Hour) - var risks []riskrel.Risk + var ( + risks []riskrel.Risk + totalEnqueued int + ) if err := w.db.WithContext(ctx). Where("status = ? AND review_deadline IS NOT NULL AND review_deadline > ? AND review_deadline <= ?", string(riskrel.RiskStatusRiskAccepted), now, windowEnd). - Find(&risks).Error; err != nil { - return fmt.Errorf("risk deadline reminder scanner: query failed: %w", err) - } + FindInBatches(&risks, riskScannerBatchSize, func(_ *gorm.DB, _ int) error { + ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) + if err != nil { + return fmt.Errorf("risk deadline reminder scanner: resolve owners failed: %w", err) + } - ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) - if err != nil { - return fmt.Errorf("risk deadline reminder scanner: resolve owners failed: %w", err) - } + params := make([]river.InsertManyParams, 0, len(risks)) + for i := range risks { + risk := &risks[i] + if risk.ID == nil { + continue + } + ownerIDs := ownersByRiskID[*risk.ID] + for _, ownerID := range ownerIDs { + params = append(params, river.InsertManyParams{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), + ReminderWindow: "30d", + }, + InsertOpts: JobInsertOptionsForRiskNotification(24 * time.Hour), + }) + } + } - params := make([]river.InsertManyParams, 0, len(risks)) - for i := range risks { - risk := &risks[i] - if risk.ID == nil { - continue - } - ownerIDs := ownersByRiskID[*risk.ID] - for _, ownerID := range ownerIDs { - params = append(params, river.InsertManyParams{ - Args: RiskReviewDueReminderArgs{ - RiskID: *risk.ID, - OwnerUserID: ownerID, - ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), - ReminderWindow: "30d", - }, - InsertOpts: JobInsertOptionsForRiskNotification(24 * time.Hour), - }) - } + if len(params) == 0 { + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk deadline reminder scanner: enqueue failed: %w", err) + } + totalEnqueued += len(params) + return nil + }).Error; err != nil { + return fmt.Errorf("risk deadline reminder scanner: query failed: %w", err) } - if len(params) == 0 { + if totalEnqueued == 0 { w.logger.Infow("RiskReviewDeadlineReminderScannerWorker: no reminders to enqueue") return nil } - if _, err := w.client.InsertMany(ctx, params); err != nil { - return fmt.Errorf("risk deadline reminder scanner: enqueue failed: %w", err) - } - - w.logger.Infow("RiskReviewDeadlineReminderScannerWorker: enqueued reminders", "count", len(params)) + w.logger.Infow("RiskReviewDeadlineReminderScannerWorker: enqueued reminders", "count", totalEnqueued) return nil } @@ -104,71 +115,77 @@ func NewRiskReviewOverdueEscalationScannerWorker( func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ *river.Job[RiskReviewOverdueEscalationScannerArgs]) error { now := time.Now().UTC() + threshold := time.Duration(w.autoReopenThresholdDays) * 24 * time.Hour - var risks []riskrel.Risk + var ( + risks []riskrel.Risk + totalEnqueued int + totalReopenCount int + ) if err := w.db.WithContext(ctx). Where("status = ? AND review_deadline IS NOT NULL AND review_deadline < ?", string(riskrel.RiskStatusRiskAccepted), now). - Find(&risks).Error; err != nil { - return fmt.Errorf("risk overdue escalation scanner: query failed: %w", err) - } - - ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) - if err != nil { - return fmt.Errorf("risk overdue escalation scanner: resolve owners failed: %w", err) - } + FindInBatches(&risks, riskScannerBatchSize, func(_ *gorm.DB, _ int) error { + ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) + if err != nil { + return fmt.Errorf("risk overdue escalation scanner: resolve owners failed: %w", err) + } - params := make([]river.InsertManyParams, 0, len(risks)) - reopenByRiskID := make(map[uuid.UUID]RiskReviewOverdueReopenArgs, len(risks)) - for i := range risks { - risk := &risks[i] - if risk.ID == nil { - continue - } - ownerIDs := ownersByRiskID[*risk.ID] - overdueWindow := now.Format("2006-01-02") - for _, ownerID := range ownerIDs { - params = append(params, river.InsertManyParams{ - Args: RiskReviewOverdueEscalationArgs{ - RiskID: *risk.ID, - OwnerUserID: ownerID, - ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), - OverdueWindow: overdueWindow, - }, - InsertOpts: JobInsertOptionsForRiskNotification(24 * time.Hour), - }) - } + params := make([]river.InsertManyParams, 0, len(risks)) + for i := range risks { + risk := &risks[i] + if risk.ID == nil { + continue + } + ownerIDs := ownersByRiskID[*risk.ID] + overdueWindow := now.Format("2006-01-02") + for _, ownerID := range ownerIDs { + params = append(params, river.InsertManyParams{ + Args: RiskReviewOverdueEscalationArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), + OverdueWindow: overdueWindow, + }, + InsertOpts: JobInsertOptionsForRiskNotification(24 * time.Hour), + }) + } - if w.autoReopenEnabled { - overdueFor := now.Sub(risk.ReviewDeadline.UTC()) - threshold := time.Duration(w.autoReopenThresholdDays) * 24 * time.Hour - if overdueFor >= threshold { - reopenByRiskID[*risk.ID] = RiskReviewOverdueReopenArgs{ - RiskID: *risk.ID, - ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), - ThresholdDays: w.autoReopenThresholdDays, + if w.autoReopenEnabled && w.autoReopenThresholdDays > 0 { + overdueFor := now.Sub(risk.ReviewDeadline.UTC()) + if overdueFor >= threshold { + params = append(params, river.InsertManyParams{ + Args: RiskReviewOverdueReopenArgs{ + RiskID: *risk.ID, + ReviewDeadline: risk.ReviewDeadline.UTC().Format(time.RFC3339), + ThresholdDays: w.autoReopenThresholdDays, + }, + InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), + }) + totalReopenCount++ + } } } - } - } - for _, args := range reopenByRiskID { - params = append(params, river.InsertManyParams{ - Args: args, - InsertOpts: JobInsertOptionsForRiskWorkerUnique(24 * time.Hour), - }) + if len(params) == 0 { + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk overdue escalation scanner: enqueue failed: %w", err) + } + totalEnqueued += len(params) + return nil + }).Error; err != nil { + return fmt.Errorf("risk overdue escalation scanner: query failed: %w", err) } - if len(params) == 0 { + if totalEnqueued == 0 { w.logger.Infow("RiskReviewOverdueEscalationScannerWorker: no escalations to enqueue") return nil } - if _, err := w.client.InsertMany(ctx, params); err != nil { - return fmt.Errorf("risk overdue escalation scanner: enqueue failed: %w", err) - } - - w.logger.Infow("RiskReviewOverdueEscalationScannerWorker: enqueued jobs", "count", len(params), "reopen_count", len(reopenByRiskID)) + w.logger.Infow("RiskReviewOverdueEscalationScannerWorker: enqueued jobs", "count", totalEnqueued, "reopen_count", totalReopenCount) return nil } @@ -187,54 +204,64 @@ func (w *RiskStaleRiskScannerWorker) Work(ctx context.Context, _ *river.Job[Risk cutoff := now.Add(-30 * 24 * time.Hour) staleBucketDate := startOfISOWeekUTC(now).Format("2006-01-02") - var risks []riskrel.Risk + var ( + risks []riskrel.Risk + totalEnqueued int + ) if err := w.db.WithContext(ctx). Where("status IN ? AND last_seen_at <= ?", []string{ string(riskrel.RiskStatusOpen), string(riskrel.RiskStatusInvestigating), string(riskrel.RiskStatusMitigatingPlanned), + string(riskrel.RiskStatusMitigatingImplemented), }, cutoff). - Find(&risks).Error; err != nil { - return fmt.Errorf("risk stale scanner: query failed: %w", err) - } + FindInBatches(&risks, riskScannerBatchSize, func(_ *gorm.DB, _ int) error { + ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) + if err != nil { + return fmt.Errorf("risk stale scanner: resolve owners failed: %w", err) + } - ownersByRiskID, err := resolveRiskOwnerUserIDsBatch(ctx, w.db, risks) - if err != nil { - return fmt.Errorf("risk stale scanner: resolve owners failed: %w", err) - } + params := make([]river.InsertManyParams, 0, len(risks)) + for i := range risks { + risk := &risks[i] + if risk.ID == nil { + continue + } + ownerIDs := ownersByRiskID[*risk.ID] + for _, ownerID := range ownerIDs { + params = append(params, river.InsertManyParams{ + Args: RiskStaleOpenReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + LastSeenAt: risk.LastSeenAt.UTC().Format(time.RFC3339), + StaleBucketDate: staleBucketDate, + }, + InsertOpts: JobInsertOptionsForRiskNotification(7 * 24 * time.Hour), + }) + } + } - params := make([]river.InsertManyParams, 0, len(risks)) - for i := range risks { - risk := &risks[i] - if risk.ID == nil { - continue - } - ownerIDs := ownersByRiskID[*risk.ID] - for _, ownerID := range ownerIDs { - params = append(params, river.InsertManyParams{ - Args: RiskStaleOpenReminderArgs{ - RiskID: *risk.ID, - OwnerUserID: ownerID, - LastSeenAt: risk.LastSeenAt.UTC().Format(time.RFC3339), - StaleBucketDate: staleBucketDate, - }, - InsertOpts: JobInsertOptionsForRiskNotification(7 * 24 * time.Hour), - }) - } + if len(params) == 0 { + return nil + } + + if _, err := w.client.InsertMany(ctx, params); err != nil { + return fmt.Errorf("risk stale scanner: enqueue failed: %w", err) + } + totalEnqueued += len(params) + return nil + }).Error; err != nil { + return fmt.Errorf("risk stale scanner: query failed: %w", err) } - if len(params) == 0 { + if totalEnqueued == 0 { w.logger.Infow("RiskStaleRiskScannerWorker: no stale reminders to enqueue") return nil } - if _, err := w.client.InsertMany(ctx, params); err != nil { - return fmt.Errorf("risk stale scanner: enqueue failed: %w", err) - } - - w.logger.Infow("RiskStaleRiskScannerWorker: enqueued stale reminders", "count", len(params)) + w.logger.Infow("RiskStaleRiskScannerWorker: enqueued stale reminders", "count", totalEnqueued) return nil } @@ -363,8 +390,11 @@ func sendRiskNotification( user, err := userRepo.FindUserByID(ctx, ownerUserID.String()) if err != nil { - logger.Warnw("Risk notification worker: owner user not found, skipping", "risk_id", riskID, "owner_user_id", ownerUserID, "error", err) - return nil + if errors.Is(err, gorm.ErrRecordNotFound) { + logger.Warnw("Risk notification worker: owner user not found, skipping", "risk_id", riskID, "owner_user_id", ownerUserID, "error", err) + return nil + } + return fmt.Errorf("risk notification worker: load owner user failed: %w", err) } if !user.RiskNotificationsSubscribed { logger.Debugw("Risk notification worker: owner unsubscribed, skipping", "risk_id", riskID, "owner_user_id", ownerUserID) @@ -479,6 +509,14 @@ func NewRiskReviewOverdueReopenWorker(db *gorm.DB, logger *zap.SugaredLogger) *R } func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job[RiskReviewOverdueReopenArgs]) error { + if job.Args.ThresholdDays <= 0 { + w.logger.Infow("RiskReviewOverdueReopenWorker: skipping reopen due to non-positive threshold_days", + "risk_id", job.Args.RiskID, + "threshold_days", job.Args.ThresholdDays, + ) + return nil + } + now := time.Now().UTC() threshold := time.Duration(job.Args.ThresholdDays) * 24 * time.Hour cutoff := now.Add(-threshold) diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index bab374fe..60d47f22 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -33,6 +33,18 @@ func (s *stubRiverClient) InsertMany(_ context.Context, params []river.InsertMan return []*rivertype.JobInsertResult{}, nil } +type stubUserRepository struct { + user NotificationUser + err error +} + +func (s *stubUserRepository) FindUserByID(_ context.Context, _ string) (NotificationUser, error) { + if s.err != nil { + return NotificationUser{}, s.err + } + return s.user, nil +} + func newRiskWorkersTestDB(t *testing.T) *gorm.DB { t.Helper() db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) @@ -163,6 +175,22 @@ func TestRiskStaleRiskScannerWorker_EnqueuesWeeklyReminder(t *testing.T) { assert.Equal(t, 7*24*time.Hour, client.params[0].InsertOpts.UniqueOpts.ByPeriod) } +func TestRiskStaleRiskScannerWorker_EnqueuesMitigatingImplemented(t *testing.T) { + db := newRiskWorkersTestDB(t) + client := &stubRiverClient{} + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + _, _ = createTestRiskWithOwner(t, db, riskrel.RiskStatusMitigatingImplemented, nil, now.Add(-31*24*time.Hour)) + + w := NewRiskStaleRiskScannerWorker(db, client, logger) + err := w.Work(context.Background(), &river.Job[RiskStaleRiskScannerArgs]{}) + require.NoError(t, err) + require.Len(t, client.params, 1) + + _, ok := client.params[0].Args.(RiskStaleOpenReminderArgs) + require.True(t, ok) +} + func TestRiskEvidenceReconciliationScannerWorker_EnqueuesDuplicateRepairJobs(t *testing.T) { db := newRiskWorkersTestDB(t) client := &stubRiverClient{} @@ -289,6 +317,26 @@ func TestRiskReviewOverdueReopenWorker_ReopensAcceptedRisk(t *testing.T) { assert.Equal(t, int64(1), statusChangeEvents) } +func TestRiskReviewOverdueReopenWorker_SkipsNonPositiveThreshold(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + reviewDeadline := time.Now().UTC().Add(-40 * 24 * time.Hour) + risk, _ := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, time.Now().UTC()) + + w := NewRiskReviewOverdueReopenWorker(db, logger) + err := w.Work(context.Background(), &river.Job[RiskReviewOverdueReopenArgs]{ + Args: RiskReviewOverdueReopenArgs{ + RiskID: *risk.ID, + ThresholdDays: 0, + }, + }) + require.NoError(t, err) + + var unchanged riskrel.Risk + require.NoError(t, db.First(&unchanged, "id = ?", risk.ID).Error) + assert.Equal(t, string(riskrel.RiskStatusRiskAccepted), unchanged.Status) +} + func TestRiskReviewDueReminderWorker_RespectsRiskSubscription(t *testing.T) { db := newRiskWorkersTestDB(t) logger := zap.NewNop().Sugar() @@ -388,3 +436,44 @@ func TestRiskReviewDueReminderWorker_SendUnsuccessful_ReturnsError(t *testing.T) require.Error(t, err) require.ErrorContains(t, err, "email send failed: provider refused") } + +func TestRiskReviewDueReminderWorker_UserNotFound_Skips(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(7 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + + mockEmail := &MockEmailService{} + userRepo := &stubUserRepository{err: gorm.ErrRecordNotFound} + worker := NewRiskReviewDueReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewDueReminderArgs]{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) +} + +func TestRiskReviewDueReminderWorker_UserLookupError_ReturnsError(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(7 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + + mockEmail := &MockEmailService{} + userRepo := &stubUserRepository{err: errors.New("database unavailable")} + worker := NewRiskReviewDueReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewDueReminderArgs]{ + Args: RiskReviewDueReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, "load owner user failed") + mockEmail.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) +} diff --git a/internal/service/worker/user_repository.go b/internal/service/worker/user_repository.go index c30558e2..f00855c5 100644 --- a/internal/service/worker/user_repository.go +++ b/internal/service/worker/user_repository.go @@ -30,7 +30,7 @@ func (r *GORMUserRepository) FindUserByID(ctx context.Context, userID string) (N var user relational.User if err := r.db.WithContext(ctx).First(&user, parsed).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { - return NotificationUser{}, fmt.Errorf("user %s not found", userID) + return NotificationUser{}, fmt.Errorf("user %s not found: %w", userID, gorm.ErrRecordNotFound) } return NotificationUser{}, fmt.Errorf("failed to fetch user %s: %w", userID, err) } From bae41e21d91df9eddca97796202cbb0d08d22c75 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 14:13:16 -0300 Subject: [PATCH 14/16] Apply copilot comments and defaultv2 --- internal/service/worker/risk_workers_test.go | 98 ++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 60d47f22..253ecb46 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -477,3 +477,101 @@ func TestRiskReviewDueReminderWorker_UserLookupError_ReturnsError(t *testing.T) require.ErrorContains(t, err, "load owner user failed") mockEmail.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) } + +func TestRiskReviewOverdueEscalationWorker_SendsWhenSubscribed(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(-2 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + createTestUser(t, db, ownerID, true) + + mockEmail := &MockEmailService{} + mockEmail.On("UseTemplate", "risk-review-overdue-escalation", mock.Anything).Return("ok", "ok", nil) + mockEmail.On("GetDefaultFromAddress").Return("noreply@example.com") + mockEmail.On("Send", mock.Anything, mock.MatchedBy(func(msg *types.Message) bool { + return len(msg.To) == 1 && + msg.To[0] == ownerID.String()+"@example.com" && + msg.Subject == "Risk review overdue: Test Risk" + })).Return(&types.SendResult{Success: true, MessageID: "risk-overdue-msg"}, nil) + + userRepo := NewGORMUserRepository(db) + worker := NewRiskReviewOverdueEscalationWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewOverdueEscalationArgs]{ + Args: RiskReviewOverdueEscalationArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertExpectations(t) +} + +func TestRiskReviewOverdueEscalationWorker_RespectsRiskSubscription(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + reviewDeadline := now.Add(-2 * 24 * time.Hour) + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, now) + createTestUser(t, db, ownerID, false) + + mockEmail := &MockEmailService{} + userRepo := NewGORMUserRepository(db) + worker := NewRiskReviewOverdueEscalationWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskReviewOverdueEscalationArgs]{ + Args: RiskReviewOverdueEscalationArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) +} + +func TestRiskStaleOpenReminderWorker_SendsWhenSubscribed(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusOpen, nil, now.Add(-35*24*time.Hour)) + createTestUser(t, db, ownerID, true) + + mockEmail := &MockEmailService{} + mockEmail.On("UseTemplate", "risk-stale-open-reminder", mock.Anything).Return("ok", "ok", nil) + mockEmail.On("GetDefaultFromAddress").Return("noreply@example.com") + mockEmail.On("Send", mock.Anything, mock.MatchedBy(func(msg *types.Message) bool { + return len(msg.To) == 1 && + msg.To[0] == ownerID.String()+"@example.com" && + msg.Subject == "Stale risk reminder: Test Risk" + })).Return(&types.SendResult{Success: true, MessageID: "risk-stale-msg"}, nil) + + userRepo := NewGORMUserRepository(db) + worker := NewRiskStaleOpenReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskStaleOpenReminderArgs]{ + Args: RiskStaleOpenReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertExpectations(t) +} + +func TestRiskStaleOpenReminderWorker_RespectsRiskSubscription(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + now := time.Now().UTC() + risk, ownerID := createTestRiskWithOwner(t, db, riskrel.RiskStatusOpen, nil, now.Add(-35*24*time.Hour)) + createTestUser(t, db, ownerID, false) + + mockEmail := &MockEmailService{} + userRepo := NewGORMUserRepository(db) + worker := NewRiskStaleOpenReminderWorker(db, mockEmail, userRepo, "https://app.example.com", logger) + err := worker.Work(context.Background(), &river.Job[RiskStaleOpenReminderArgs]{ + Args: RiskStaleOpenReminderArgs{ + RiskID: *risk.ID, + OwnerUserID: ownerID, + }, + }) + require.NoError(t, err) + mockEmail.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) +} From 23658454b2808f8ba7703d5f1b56d476f0d74aeb Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 14:23:33 -0300 Subject: [PATCH 15/16] Lock risk when reopening review --- internal/service/relational/risks/service.go | 10 +++++++ internal/service/worker/risk_workers.go | 29 ++++++-------------- internal/service/worker/risk_workers_test.go | 28 +++++++++++++++++-- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/internal/service/relational/risks/service.go b/internal/service/relational/risks/service.go index 97b12f46..227c1dce 100644 --- a/internal/service/relational/risks/service.go +++ b/internal/service/relational/risks/service.go @@ -63,6 +63,9 @@ type ReviewRiskParams struct { Decision RiskReviewDecision Notes *string NextReviewDeadline *time.Time + // RequireCurrentReviewDeadlineBefore enforces, under lock, that the current review deadline + // is set and no later than this timestamp before applying the decision. + RequireCurrentReviewDeadlineBefore *time.Time } type Associations struct { @@ -339,6 +342,13 @@ func (s *RiskService) ReviewRisk(params ReviewRiskParams) (*Risk, error) { tx.Rollback() return nil, newValidationError("only risks in status risk-accepted can be reviewed") } + if params.RequireCurrentReviewDeadlineBefore != nil { + cutoff := params.RequireCurrentReviewDeadlineBefore.UTC() + if risk.ReviewDeadline == nil || risk.ReviewDeadline.UTC().After(cutoff) { + tx.Rollback() + return nil, newValidationError("risk review deadline no longer eligible for requested decision") + } + } reviewedAt := time.Now().UTC() if params.ReviewedAt != nil { diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 085a64aa..7c9c83e3 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -522,29 +522,16 @@ func (w *RiskReviewOverdueReopenWorker) Work(ctx context.Context, job *river.Job cutoff := now.Add(-threshold) riskSvc := riskrel.NewRiskService(w.db.WithContext(ctx)) - risk, err := riskSvc.GetByID(job.Args.RiskID) - if err != nil { - if errors.Is(err, gorm.ErrRecordNotFound) { + if _, err := riskSvc.ReviewRisk(riskrel.ReviewRiskParams{ + RiskID: job.Args.RiskID, + Decision: riskrel.RiskReviewDecisionReopen, + ReviewedAt: &now, + RequireCurrentReviewDeadlineBefore: &cutoff, + }); err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) || riskrel.IsValidationError(err) { return nil } - return fmt.Errorf("risk overdue reopen: load risk failed: %w", err) - } - - if risk.Status != string(riskrel.RiskStatusRiskAccepted) || risk.ReviewDeadline == nil || risk.ReviewDeadline.UTC().After(cutoff) { - return nil - } - - oldStatus := risk.Status - risk.Status = string(riskrel.RiskStatusInvestigating) - risk.ReviewDeadline = nil - risk.AcceptanceJustification = nil - - if _, err := riskSvc.Update(riskrel.UpdateRiskParams{ - Risk: risk, - OldStatus: oldStatus, - StatusChanged: oldStatus != risk.Status, - }); err != nil { - return fmt.Errorf("risk overdue reopen: update risk failed: %w", err) + return fmt.Errorf("risk overdue reopen: reopen review failed: %w", err) } w.logger.Infow("RiskReviewOverdueReopenWorker: reopened overdue accepted risk", diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index 253ecb46..e62f082a 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -61,9 +61,10 @@ func newRiskWorkersTestDB(t *testing.T) *gorm.DB { &riskrel.Risk{}, &riskrel.RiskOwnerAssignment{}, &riskrel.RiskEvidenceLink{}, - &riskrel.RiskSubjectLink{}, - &riskrel.RiskEvent{}, - )) + &riskrel.RiskSubjectLink{}, + &riskrel.RiskEvent{}, + &riskrel.RiskReview{}, + )) return db } @@ -337,6 +338,27 @@ func TestRiskReviewOverdueReopenWorker_SkipsNonPositiveThreshold(t *testing.T) { assert.Equal(t, string(riskrel.RiskStatusRiskAccepted), unchanged.Status) } +func TestRiskReviewOverdueReopenWorker_SkipsWhenNotYetOverdueForThreshold(t *testing.T) { + db := newRiskWorkersTestDB(t) + logger := zap.NewNop().Sugar() + reviewDeadline := time.Now().UTC().Add(-5 * 24 * time.Hour) + risk, _ := createTestRiskWithOwner(t, db, riskrel.RiskStatusRiskAccepted, &reviewDeadline, time.Now().UTC()) + + w := NewRiskReviewOverdueReopenWorker(db, logger) + err := w.Work(context.Background(), &river.Job[RiskReviewOverdueReopenArgs]{ + Args: RiskReviewOverdueReopenArgs{ + RiskID: *risk.ID, + ThresholdDays: 30, + }, + }) + require.NoError(t, err) + + var unchanged riskrel.Risk + require.NoError(t, db.First(&unchanged, "id = ?", risk.ID).Error) + assert.Equal(t, string(riskrel.RiskStatusRiskAccepted), unchanged.Status) + assert.NotNil(t, unchanged.ReviewDeadline) +} + func TestRiskReviewDueReminderWorker_RespectsRiskSubscription(t *testing.T) { db := newRiskWorkersTestDB(t) logger := zap.NewNop().Sugar() From f8513518e17ee5d7fb2234642d58d6e346782ccb Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 10 Mar 2026 16:22:35 -0300 Subject: [PATCH 16/16] Address risk worker comments --- internal/service/worker/risk_workers.go | 4 ++-- internal/service/worker/risk_workers_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/service/worker/risk_workers.go b/internal/service/worker/risk_workers.go index 7c9c83e3..a2c5e831 100644 --- a/internal/service/worker/risk_workers.go +++ b/internal/service/worker/risk_workers.go @@ -132,13 +132,13 @@ func (w *RiskReviewOverdueEscalationScannerWorker) Work(ctx context.Context, _ * } params := make([]river.InsertManyParams, 0, len(risks)) + overdueWindow := now.Format("2006-01-02") for i := range risks { risk := &risks[i] if risk.ID == nil { continue } ownerIDs := ownersByRiskID[*risk.ID] - overdueWindow := now.Format("2006-01-02") for _, ownerID := range ownerIDs { params = append(params, river.InsertManyParams{ Args: RiskReviewOverdueEscalationArgs{ @@ -546,7 +546,7 @@ func resolveRiskOwnerUserIDsBatch(ctx context.Context, db *gorm.DB, risks []risk riskIDs := make([]uuid.UUID, 0, len(risks)) for i := range risks { risk := &risks[i] - if risk == nil || risk.ID == nil { + if risk.ID == nil { continue } riskID := *risk.ID diff --git a/internal/service/worker/risk_workers_test.go b/internal/service/worker/risk_workers_test.go index e62f082a..edb3e2f4 100644 --- a/internal/service/worker/risk_workers_test.go +++ b/internal/service/worker/risk_workers_test.go @@ -61,10 +61,10 @@ func newRiskWorkersTestDB(t *testing.T) *gorm.DB { &riskrel.Risk{}, &riskrel.RiskOwnerAssignment{}, &riskrel.RiskEvidenceLink{}, - &riskrel.RiskSubjectLink{}, - &riskrel.RiskEvent{}, - &riskrel.RiskReview{}, - )) + &riskrel.RiskSubjectLink{}, + &riskrel.RiskEvent{}, + &riskrel.RiskReview{}, + )) return db }