Feat/poam promote-to-poam#366
Merged
Merged
Conversation
Implements POST /risks/:id/promote-to-poam and the SSP-scoped variant POST /oscal/system-security-plans/:sspId/risks/:id/promote-to-poam. Changes: - poam/models.go: add PocName, PocEmail, ResourceRequired fields to PoamItem - poam/service.go: extend CreatePoamItemParams with poc/resource fields; add CreateWithTx for use within an external transaction - risks/events.go: add RiskEventTypePoamPromoted constant + BuildRiskEventDetails case - risks/promote_to_poam.go: new PromoteToPoam service method — validates risk-accepted status, guards against re-promotion of active POAM, copies RemediationTemplate tasks as milestones, merges extra milestones from request, creates POAM item + risk link + risk event atomically - handler/poam_items.go: expose PocName, PocEmail, ResourceRequired in createPoamItemRequest and poamItemResponse - handler/risks.go: add promoteToPoamRequest, PromoteToPoam and PromoteToPoamForSSP handlers; inject PoamService into RiskHandler - handler/api.go: pass poamService to NewRiskHandler - risks_promote_to_poam_integration_test.go: 7 integration tests covering happy path, defaults, 422 rejection cases, SSP-scoped, and remediation template milestone merging Depends-on: BCH-1185 (POAM Item CRUD), BCH-1182 (Risk Accept + Review)
…rder-index - docs/: regenerate swagger docs to include new promote-to-poam endpoints (fixes check-diff CI failure — docs were stale after adding handler annotations) - risks_promote_to_poam_integration_test.go: fix remediation template task order-index values from 0-based to 1-based (validation requires > 0) (fixes TestPromoteToPoam_WithRemediationTemplate integration test failure) - poam/models.go: include GORM column tags for PocName, PocEmail, ResourceRequired to ensure AutoMigrate generates correct column names
…lifecycle transitions - Change PromoteToPoam guard: risk must be in 'investigating' status (not 'risk-accepted'). A formally accepted risk has been deemed tolerable and should not receive a remediation plan; POAM promotion is only valid while the risk is actively being investigated. - Transition risk to 'mitigating-planned' on promotion: when a POAM item is created, the risk advances from investigating → mitigating-planned inside the same atomic transaction, emitting status_changed + poam_promoted events. - Add POAM completion lifecycle callback (poam_completion.go): when a POAM item transitions to 'completed', OnPoamItemCompleted advances all linked risks from mitigating-planned → mitigating-implemented, emitting status_changed + poam_completed events per risk. - Add RiskEventTypePoamCompleted constant and event details case. - Update validateStatusTransition: allow risk-accepted → mitigating-planned for manual transitions. - Wire riskService into PoamItemsHandler for the completion callback; share a single RiskService instance across both handlers via api.go. - Update all integration tests: remove accept step (risks are promoted from investigating, not risk-accepted); add assertions for mitigating-planned after promotion; add TestPromoteToPoam_CompletionAdvancesRiskStatus for the full investigating → mitigating-planned → mitigating-implemented path. - Regenerate Swagger docs.
…Item Remove free-text PocName and PocEmail fields from the PoamItem model, CreatePoamItemParams, and all handler request/response structs. Replace with the existing PrimaryOwnerUserID FK pattern already used on the Risk model, which links to the users table for structured ownership. ResourceRequired is retained as a free-text operational field. On promotion, the POAM item automatically inherits the risk's PrimaryOwnerUserID. Callers may override it via primaryOwnerUserId in the request body. Addresses review comment from gusfcarvalho on models.go line 85. - Remove PocName, PocEmail from PoamItem model (GORM + JSON tags) - Remove from CreatePoamItemParams and service Create/CreateWithTx - Remove from PromoteToPoamParams; inherit risk.PrimaryOwnerUserID - Remove from promoteToPoamRequest handler struct - Remove from createPoamItemRequest, poamItemResponse, toPoamItemResponse - Update integration tests: assert PrimaryOwnerUserID instead of PocName/PocEmail - Regenerate Swagger docs
…eritance - Run swag fmt to fix check-diff: poam_items.go, poam/models.go, poam/service.go annotations were not formatted to match swag fmt output on CI - Fix TestPromoteToPoam_HappyPath and TestPromoteToPoam_SSPScoped_HappyPath: create risk with explicit primary-owner-user-id so PrimaryOwnerUserID inheritance assertion is testable (dummy@example.com is not persisted to DB so actor-based resolution returns nil)
- fix(handler): make riskSvc a required parameter in NewRiskHandler, removing the variadic fallback pattern (gusfcarvalho review) - fix(workflow): add mitigating-planned → investigating transition so a failed mitigation can return to investigation (gusfcarvalho review) - fix(workflow): add mitigating-implemented → remediated transition so fully-evidenced risks can be marked remediated before close (gusfcarvalho review) - fix(workflow): remove risk-accepted → mitigating-planned from validateStatusTransition to align with PromoteToPoam which only allows promotion from investigating (Copilot review) - fix(workflow): replace risk-accepted → mitigating-planned with risk-accepted → investigating to allow re-opening accepted risks - fix(service): remove unnecessary Preload(OwnerAssignments) from the SELECT FOR UPDATE lock query in PromoteToPoam; PrimaryOwnerUserID is read directly from the Risk struct (Copilot + gusfcarvalho review) - fix(service): change CreateMilestoneParams.OrderIndex from int to *int so callers can explicitly set 0 without it being treated as unset; update all call sites in Create, CreateWithTx, AddMilestone, and the PromoteToPoam handler (Copilot review) - fix(service): add NOTE godoc to CreateWithTx documenting that EvidenceIDs, ControlRefs, and FindingIDs are intentionally not processed (gusfcarvalho: documentation only is sufficient) - fix(test): remove dummy testing import and var _ = (*testing.T)(nil) sentinel from risks_promote_to_poam_integration_test.go (Copilot review)
Add three scheduled River background jobs that mirror the Risk Review
Deadline Scanner pattern for POAM lifecycle management:
1. POAM Deadline Reminder (daily, 0 8 * * *)
- Scans open|in-progress items where deadline - now <= 30 days AND deadline > now
- Enqueues poam_deadline_reminder per item per owner
- Idempotency key: PoamItemID + ReminderWindowBucket (calendar day)
2. POAM Overdue Transition (daily, 0 9 * * *)
- Scans open|in-progress items where deadline < now
- Transitions status to overdue in DB (with updated_at)
- Enqueues poam_overdue_notification per item per owner
- Idempotency key: PoamItemID + OverdueWindow (calendar day)
- Already-overdue items are excluded (idempotent)
3. Incomplete Milestone Scanner (weekly, 0 8 * * 1)
- Scans planned milestones where planned_completion_date < now
AND parent poam_item.status != completed
- Enqueues poam_milestone_overdue_reminder per milestone per owner
- Idempotency key: MilestoneID + WeeklyBucket (ISO week)
New files:
- poam_job_types.go — args structs, Kind/Timeout/InsertOpts helpers
- poam_helpers.go — shared resolvePoamRecipients, resolvePoamSSPDisplayName,
resolvePoamURL, GORMUserRepository helpers
- poam_deadline_reminder_worker.go — scanner + notification worker
- poam_overdue_transition_worker.go — scanner + DB transition + notification worker
- poam_milestone_overdue_worker.go — scanner + notification worker
- poam_periodic_jobs.go — PeriodicJob constructors for River scheduler
- internal/config/poam.go — PoamConfig (DeadlineReminderSchedule,
OverdueTransitionSchedule, MilestoneOverdueSchedule)
Updated files:
- jobs.go — register 3 notification workers in Workers()
- service.go — register 3 scanner workers + poam queue in buildRiverConfig,
add 3 PeriodicJobs in periodicJobsFromConfig
- config.go — add Poam *PoamConfig to Config struct
Tests:
- 12 unit tests (SQLite in-memory) covering all query boundary conditions:
window inclusion/exclusion, status filtering, idempotency
- 7 integration tests (Postgres via IntegrationTestSuite) covering:
full scanner→DB mutation→notification worker pipeline,
idempotency bucket key consistency, completed-parent exclusion
All unit tests pass. Integration tests compile cleanly (require Postgres).
Implements the periodic POAM open digest as specified in BCH-1186 Phase 3. Workers added: - PoamOpenDigestSchedulerWorker: periodic River job (daily/weekly, configurable) that resolves recipients from active POAM item owners and enqueues one PoamOpenDigestArgs job per recipient. - PoamOpenDigestWorker: per-recipient worker that loads POAM items, classifies them into five buckets, and sends the grouped HTML+text digest email. Digest buckets: - NewSinceLastDigest — items created within the current digest window - Overdue — items with status: overdue - ApproachingDeadline — items with deadline within 30 days (open/in-progress) - MilestonesDueSoon — planned milestones with due_date within 14 days - Stale — open items with no update in >30 days River config: - Queue: poam | MaxAttempts: 3 - UniqueOpts.ByArgs: true | UniqueOpts.ByPeriod: 24h (daily) or 7d (weekly) - Timeout: 30s per recipient digest Bug fixes: - jobs.go: removed incorrect NewPoamDigestSchedulerWorker call (scheduler is registered in service.go via clientProxy, not in jobs.go Workers()) - poam_digest_worker.go: replaced PostgreSQL-specific ::text cast with CAST(... AS TEXT) for SQLite compatibility in unit tests - poam_digest_worker.go: corrected table name to ccf_poam_items Files added: - internal/service/worker/poam_digest_worker.go - internal/service/worker/poam_digest_worker_test.go (35 unit tests, all pass) - internal/service/email/templates/templates/poam-open-digest.html - internal/service/email/templates/templates/poam-open-digest.txt Files modified: - internal/config/poam.go — OpenDigestWindow field + validation - internal/service/worker/poam_job_types.go — PoamOpenDigestSchedulerArgs/PoamOpenDigestArgs - internal/service/worker/poam_periodic_jobs.go — NewPoamOpenDigestPeriodicJob - internal/service/worker/service.go — scheduler worker registration - internal/service/worker/jobs.go — per-recipient digest worker registration - internal/service/worker/poam_workers_integration_test.go — 3 digest integration tests
…Worker
Structural consistency fixes following audit against Risk digest (BCH-1184)
canonical pattern:
1. Add PoamItemID field to PoamDigestEmailItem for test assertions
- Mirrors how RiskDigestEmailItem carries RiskID for integration test
assertions; field is not rendered in the email template
2. Add RiskNotificationsSubscribed subscription check to PoamOpenDigestWorker
- Risk digest skips unsubscribed users; POAM must do the same
- Reuses existing RiskNotificationsSubscribed field until a dedicated
PoamNotificationsSubscribed column is added
3. Add nil-db guard in PoamOpenDigestWorker.Work
- Mirrors the equivalent guard in RiskOpenDigestWorker
4. Align templateData keys: use user.FullName() instead of manual
FirstName+LastName concatenation; add Has* boolean flags
(HasNewSinceLast, HasOverdue, HasApproachingDeadline, HasMilestonesDueSoon,
HasStale) to match the Risk digest template data contract
5. Check result.Success and log result.MessageID after emailService.Send
POAM digest now does the same
6. Fix integration test: replace undefined poamDigestEmailItem (lowercase)
with exported PoamDigestEmailItem; add risk_notifications_subscribed=true
to seedUser so the digest worker does not skip the test recipient
All 35 unit tests and go vet pass.
Lint (golangci-lint unused): - Remove unused resolveUserEmail helper from poam_helpers.go (function was added speculatively but never called; also removes the now-orphaned 'errors' import) check-diff (swag fmt / swag init): - Apply swag fmt alignment fixes across four files that CI regenerates and checks for dirtiness: * internal/config/poam.go — align struct tag spacing * internal/service/worker/poam_deadline_reminder_worker.go — sort imports * internal/service/worker/poam_job_types.go — align const/field spacing * internal/service/worker/poam_periodic_jobs.go — remove blank line - Regenerate docs/ from swag init so check-diff sees no diff
gusfcarvalho
reviewed
Apr 1, 2026
Root cause: RiskEvidenceReconciliationScannerWorker only reconciled duplicate dedupe keys. Open auto-generated risks whose SSP profile was swapped or unbound were never cleaned up because handleEvidenceResolution only runs on evidence arrival. Fix: Extend the reconciliation scanner with a second pass (Pass 2) that queries all open auto-generated risks with at least one control link and enqueues a RiskOrphanedControlsArgs job per risk. New RiskOrphanedControlsWorker: - Loads the risk's control links (risk_control_links.catalog_id + control_id) - Loads the SSP's current profile controls via profile_controls JOIN ssp - If zero intersection (or SSP has no profile): transitions risk to remediated with an 'orphaned_profile_change' audit event - Skips manually created risks (risk_template_id IS NULL) - Skips already terminal risks (closed, remediated) - No-ops if risk was deleted between scan and execution Handles both cases: - Single-profile swap: old profile's controls absent from new profile - Multi-profile detach: SSP profile_id set to NULL Tests added: - TestRiskOrphanedControlsWorker_ProfileUnbound - TestRiskOrphanedControlsWorker_ControlStillInProfile - TestRiskOrphanedControlsWorker_ControlRemovedFromProfile - TestRiskOrphanedControlsWorker_SkipsManualRisk - TestRiskOrphanedControlsWorker_SkipsAlreadyTerminal - TestRiskOrphanedControlsWorker_DeletedRiskIsNoop - TestReconciliationScannerPass2_EnqueuesOrphanJobs - TestReconciliationScannerPass2_NoOrphans Also: added RiskControlLink to newRiskWorkersTestDB migration list so the existing TestRiskEvidenceReconciliationScannerWorker test continues to pass with the new Pass 2 query.
…ty_user_id FK Reviewer (gusfcarvalho) noted that milestone overdue notifications are sent to the POAM item owner rather than the milestone's responsible party. The current ccf_poam_item_milestones.responsible_party column is a free-text string with no FK to the users table, so we cannot resolve an email address from it. Added a TODO comment at the notification dispatch site explaining the constraint and the required follow-on schema migration (add responsible_party_user_id uuid FK to ccf_poam_item_milestones). Closes reviewer comment on PR #366.
Three workers referenced email templates that did not exist as embedded files, causing the email service to fail at runtime with a template-not- found error: - poam-overdue-notification (PoamOverdueNotificationWorker) - poam-deadline-reminder (PoamDeadlineReminderWorker) - poam-milestone-overdue-reminder (MilestoneOverdueReminderWorker) Added HTML and plain-text variants for all three templates, following the style and variable conventions established by the existing poam-open-digest and risk-* templates. Also added TestTemplateService_Poam* unit tests to the template service test suite so that missing templates are caught at test time rather than at runtime.
…1186)" This reverts commit b1ea739.
gusfcarvalho
reviewed
Apr 9, 2026
gusfcarvalho
reviewed
Apr 9, 2026
gusfcarvalho
reviewed
Apr 9, 2026
gusfcarvalho
approved these changes
Apr 9, 2026
Contributor
|
just doing a copilot pass to see if something weird comes up -- otherwise tested it locally and LGTM :) |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds POAM background processing to the worker service: periodic scanners for approaching deadlines / overdue transitions / overdue milestones, plus a scheduled “open POAM digest” that groups active POAM items by recipient and emails a summary.
Changes:
- Introduces POAM River job types, periodic job constructors, and worker implementations (scanners + notifications + digest scheduler + per-recipient digest).
- Adds email templates (HTML + text) for POAM deadline reminder, POAM overdue notification, milestone overdue reminder, and POAM open digest.
- Adds unit + integration tests for POAM workers and template rendering; wires POAM config into the main config.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/service/worker/service.go | Registers POAM scanner + digest scheduler workers; adds POAM periodic jobs and queue config. |
| internal/service/worker/jobs.go | Registers POAM notification workers and per-recipient digest worker. |
| internal/service/worker/poam_job_types.go | Defines POAM job kinds/args/timeouts and insert option helpers. |
| internal/service/worker/poam_periodic_jobs.go | Adds periodic job constructors for POAM scanners and digest scheduler. |
| internal/service/worker/poam_helpers.go | Adds helper functions for recipients, SSP display names, and URL building. |
| internal/service/worker/poam_deadline_reminder_worker.go | Implements deadline reminder scanner + notification email worker. |
| internal/service/worker/poam_overdue_transition_worker.go | Implements overdue transition scanner + overdue notification email worker. |
| internal/service/worker/poam_milestone_overdue_worker.go | Implements milestone overdue scanner + reminder email worker. |
| internal/service/worker/poam_digest_worker.go | Implements POAM digest windowing, classification, scheduler worker, and digest worker. |
| internal/service/worker/poam_workers_test.go | Unit tests for POAM scanner workers using SQLite. |
| internal/service/worker/poam_workers_integration_test.go | Integration tests for scanner pipelines and POAM digest grouping behavior. |
| internal/service/worker/poam_digest_worker_test.go | Unit tests for digest predicates/windowing/classification and DB-level scheduler tests. |
| internal/service/email/templates/templates/poam-deadline-reminder.html | HTML template for approaching-deadline reminder email. |
| internal/service/email/templates/templates/poam-deadline-reminder.txt | Text template for approaching-deadline reminder email. |
| internal/service/email/templates/templates/poam-overdue-notification.html | HTML template for overdue POAM item notification email. |
| internal/service/email/templates/templates/poam-overdue-notification.txt | Text template for overdue POAM item notification email. |
| internal/service/email/templates/templates/poam-milestone-overdue-reminder.html | HTML template for milestone overdue reminder email. |
| internal/service/email/templates/templates/poam-milestone-overdue-reminder.txt | Text template for milestone overdue reminder email. |
| internal/service/email/templates/templates/poam-open-digest.html | HTML template for grouped POAM digest email. |
| internal/service/email/templates/templates/poam-open-digest.txt | Text template for grouped POAM digest email. |
| internal/service/email/templates/service_test.go | Adds template render tests for the new POAM email templates. |
| internal/config/poam.go | Adds POAM worker configuration loader/defaults/validation. |
| internal/config/config.go | Loads PoamConfig and adds it to the top-level Config struct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove WebBaseURL from PoamConfig; workers use top-level Config.WebBaseURL - Wire ReminderWindowDays from PoamConfig into PoamDeadlineReminderScannerWorker replacing the hard-coded 30-day constant; constructor now accepts a reminderWindow time.Duration parameter with a safe positive-value guard - Simplify service.go POAM block to resolve poamCfg once and reuse it for both the scanner window and the digest window kind - Call cfg.Validate() in LoadPoamConfig after unmarshal so invalid cron schedules and misconfigured fields fail fast at startup, consistent with RiskConfig and WorkflowConfig loading patterns - Extend PoamConfig.Validate() to check open_digest_window is 'daily' or 'weekly' and that ReminderWindowDays is a positive integer - Fix 5-field cron comment examples in poam_job_types.go to 6-field format (0 0 8 * * * / 0 0 9 * * *) consistent with the rest of the codebase - Update unit and integration test call sites for the new constructor signature
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the periodic POAM open digest job that aggregates open/overdue POAM items per stakeholder and sends a grouped summary email. Prevents POAM items from being silently ignored after creation.
This is a backend-only change — no UI changes are required. The digest runs as a scheduled River background job and delivers email notifications directly to POAM item owners.
Changes - New workers
PoamOpenDigestSchedulerWorker — periodic River job (registered via service.go) that resolves the distinct set of active POAM item owners and enqueues one PoamOpenDigestArgs job per recipient.
PoamOpenDigestWorker — per-recipient worker that loads all active POAM items for the recipient, classifies them into five buckets, and sends the grouped HTML + plain-text digest email.
Digest buckets (per recipient)
status = overdueopenorin-progressdue_datewithin 14 days, not completedstatus = open, no update in ≥ 30 daysConfiguration
CCF_POAM_OPEN_DIGEST_ENABLEDfalseCCF_POAM_OPEN_DIGEST_SCHEDULE0 0 7 * * *CCF_POAM_OPEN_DIGEST_WINDOWdailydailyorweeklyFiles changed
internal/service/worker/poam_digest_worker.gointernal/service/worker/poam_digest_worker_test.gointernal/service/email/templates/templates/poam-open-digest.htmlinternal/service/email/templates/templates/poam-open-digest.txtinternal/config/poam.goOpenDigestWindowfield + validationinternal/service/worker/poam_job_types.goPoamOpenDigestSchedulerArgsandPoamOpenDigestArgsinternal/service/worker/poam_periodic_jobs.goNewPoamOpenDigestPeriodicJobinternal/service/worker/service.goPoamOpenDigestSchedulerWorkerinternal/service/worker/jobs.goPoamOpenDigestWorkerinternal/service/worker/poam_workers_integration_test.goDefinition of Done