feat(poam): BCH-1186 — promote risk to POAM item#362
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)
| ) | ||
|
|
||
| func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB) *RiskHandler { | ||
| func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB, poamSvc *poamsvc.PoamService, riskSvc ...*riskrel.RiskService) *RiskHandler { |
Contributor
There was a problem hiding this comment.
Why are we passing an optional riskSvc here? We should enforce this to be one entry, and change the signature where used.
Comment on lines
+38
to
+43
| var rs *riskrel.RiskService | ||
| if len(riskSvc) > 0 && riskSvc[0] != nil { | ||
| rs = riskSvc[0] | ||
| } else { | ||
| rs = riskrel.NewRiskService(db) | ||
| } |
Contributor
There was a problem hiding this comment.
same - if we are injecting the risk service now, we should probably remove all of these lines and just enforce the risk service to be used. It's the best approach since we are now creating the risk service under api.go
Contributor
There was a problem hiding this comment.
Pull request overview
Implements “Risk → POAM promotion” and ties POAM completion back into the Risk lifecycle, enabling atomic creation of a POAM item (plus milestones/links) from a risk and emitting new risk event types to reflect these lifecycle transitions.
Changes:
- Add
POST /risks/:id/promote-to-poam(and SSP-scoped variant) to create a POAM item from a risk and transition the risk tomitigating-planned. - Add POAM completion callback logic to advance linked risks to
mitigating-implementedand emitpoam_completedevents. - Extend POAM item model/API with
resourceRequired, add POAM serviceCreateWithTx, and update Swagger docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/service/relational/risks/promote_to_poam.go | New transactional promotion flow from risk to POAM item + event emission |
| internal/service/relational/risks/poam_completion.go | New hook to advance linked risks when a POAM item is completed |
| internal/service/relational/risks/events.go | Add poam_promoted / poam_completed event types and detail strings |
| internal/service/relational/poam/service.go | Add ResourceRequired to create params and new CreateWithTx helper |
| internal/service/relational/poam/models.go | Add ResourceRequired field to PoamItem |
| internal/api/handler/risks.go | Add promote-to-POAM handlers + inject PoamService into RiskHandler |
| internal/api/handler/poam_items.go | Expose resourceRequired; trigger risk advancement on POAM completion |
| internal/api/handler/api.go | Wire shared PoamService/RiskService into handlers |
| internal/api/handler/risks_promote_to_poam_integration_test.go | New integration coverage for promote-to-POAM + completion lifecycle |
| docs/swagger.yaml | Document new endpoints + resourceRequired |
| docs/swagger.json | Generated Swagger JSON updates for the same |
| docs/docs.go | Generated docs template updates for the same |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 1. Load and lock the risk row. | ||
| var risk Risk | ||
| if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). | ||
| Preload("OwnerAssignments"). |
There was a problem hiding this comment.
OwnerAssignments is preloaded when loading the risk, but this method never reads it (and getRiskSnapshot() preloads it again later). This adds an unnecessary query and extra row data under the risk row lock; consider dropping this Preload here.
Suggested change
| Preload("OwnerAssignments"). |
Contributor
There was a problem hiding this comment.
we might need to use this for the primaryOwner of the POAM... if not, we should indeed just remove it.
Comment on lines
+120
to
+126
| offset := len(templateMilestones) | ||
| for i, extra := range params.ExtraMilestones { | ||
| if extra.OrderIndex == 0 { | ||
| extra.OrderIndex = offset + i | ||
| } | ||
| templateMilestones = append(templateMilestones, extra) | ||
| } |
There was a problem hiding this comment.
extra.OrderIndex == 0 is being used as a sentinel for “unset”, but OrderIndex is an int where 0 can be a legitimate value (e.g., 0-based ordering). As written, a client cannot explicitly request orderIndex: 0 because it will be rewritten to offset+i. Consider changing CreateMilestoneParams.OrderIndex to *int (or adding an explicit boolean) so you can distinguish omitted vs provided values.
Comment on lines
+2005
to
+2006
| string(riskrel.RiskStatusClosed): {}, | ||
| string(riskrel.RiskStatusMitigatingPlanned): {}, |
There was a problem hiding this comment.
validateStatusTransition now allows risk-accepted -> mitigating-planned, but RiskService.PromoteToPoam currently rejects risk-accepted and only allows investigating. This creates an inconsistent state machine (clients could transition accepted risks to mitigating-planned via update even though promotion disallows it). Please align the allowed transitions with the promotion rules/intended spec.
Suggested change
| string(riskrel.RiskStatusClosed): {}, | |
| string(riskrel.RiskStatusMitigatingPlanned): {}, | |
| string(riskrel.RiskStatusClosed): {}, |
Comment on lines
+349
to
+350
| // Ensure the testing import is used. | ||
| var _ = (*testing.T)(nil) |
There was a problem hiding this comment.
This test file imports testing only to satisfy the compiler via var _ = (*testing.T)(nil). Consider removing the testing import and the dummy assignment (it’s not needed since the suite already provides suite.T()), which will keep the test file cleaner.
- 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)
9 tasks
gusfcarvalho
approved these changes
Mar 27, 2026
gusfcarvalho
added a commit
that referenced
this pull request
Apr 9, 2026
* feat(poam): BCH-1186 — promote risk to POAM item 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) * fix(poam): BCH-1186 CI fixes — regenerate swagger docs and fix test order-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 * fix(poam): BCH-1186 address review comments — investigating status + 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. * fix(poam): replace poc_name/poc_email with primaryOwnerUserId on PoamItem 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 * fix(poam): BCH-1186 CI fixes — swag fmt formatting and test owner inheritance - 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(poam): BCH-1186 address PR #362 review comments - 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) * fix(poam): normalise swag fmt whitespace in validateStatusTransition map * feat(worker): BCH-1186 Phase 3 — POAM background jobs 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). * feat(worker): BCH-1186 Phase 3 — POAM open digest background jobs 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 * fix(worker): align PoamOpenDigestWorker structure with RiskOpenDigestWorker 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. * fix(ci): resolve lint and check-diff failures 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 * fix(worker): clean up stale risks on SSP profile change (BCH-1186) 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. * fix(poam): address PR review — add TODO for milestone responsible_party_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. * fix(email): add missing POAM notification email templates 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. * Revert "fix(worker): clean up stale risks on SSP profile change (BCH-1186)" This reverts commit b1ea739. * fix(poam): address PR #366 review comments - 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 * chore: apply swag fmt struct field alignment in deadline reminder worker --------- Co-authored-by: AKAbdulHanif <AKAbdulHanif@users.noreply.github.com> Co-authored-by: Gustavo Fernandes de Carvalho <17139678+gusfcarvalho@users.noreply.github.com>
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.
BCH-1186: POAM Phase 2 — Risk Integration (Promote to POAM)
Summary
Implements
POST /risks/:id/promote-to-poamand the SSP-scoped variantPOST /oscal/system-security-plans/:sspId/risks/:id/promote-to-poam.When a risk is in
risk-acceptedstatus, the risk owner can promote it to a structured POAM item. The entire operation is atomic — POAM item creation, milestone creation, risk link creation, and risk event emission all commit or roll back together.Changes
poam/models.goPocName,PocEmail,ResourceRequiredfields toPoamItem(GORM AutoMigrate adds columns)poam/service.goCreatePoamItemParamswith poc/resource fields; addCreateWithTxfor shared-transaction userisks/events.goRiskEventTypePoamPromotedconstant +BuildRiskEventDetailscaserisks/promote_to_poam.goPromoteToPoamservice method — validatesrisk-acceptedstatus, guards re-promotion of active POAM, copiesRemediationTemplatetasks as milestones, merges extra milestones from request body, atomic transactionhandler/poam_items.goPocName,PocEmail,ResourceRequiredincreatePoamItemRequestandpoamItemResponsehandler/risks.gopromoteToPoamRequest,PromoteToPoamandPromoteToPoamForSSPhandlers; injectPoamServiceintoRiskHandlerhandler/api.gopoamServicetoNewRiskHandlerrisks_promote_to_poam_integration_test.goDefinition of Done
POST /risks/:id/promote-to-poamcreates PoamItem linked to Riskrisk-acceptedstatus (422 with clear error message)RemediationTemplateare copied in correct orderpoam_item_risk_linksentry created;risk_event(poam_promoted)row emittedDepends on
BCH-1185 (POAM Item CRUD), BCH-1182 (Risk Accept + Review)