From 86bbe65445b46235dde33ff370ca7c7c6a0345d5 Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Tue, 24 Mar 2026 11:33:03 -0400 Subject: [PATCH 1/7] =?UTF-8?q?feat(poam):=20BCH-1186=20=E2=80=94=20promot?= =?UTF-8?q?e=20risk=20to=20POAM=20item?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- internal/api/handler/api.go | 2 +- internal/api/handler/poam_items.go | 12 + internal/api/handler/risks.go | 132 +++++++- .../risks_promote_to_poam_integration_test.go | 305 ++++++++++++++++++ internal/service/relational/poam/models.go | 4 + internal/service/relational/poam/service.go | 61 ++++ internal/service/relational/risks/events.go | 6 + .../relational/risks/promote_to_poam.go | 175 ++++++++++ 8 files changed, 695 insertions(+), 2 deletions(-) create mode 100644 internal/api/handler/risks_promote_to_poam_integration_test.go create mode 100644 internal/service/relational/risks/promote_to_poam.go diff --git a/internal/api/handler/api.go b/internal/api/handler/api.go index 6562527e..8ea560d6 100644 --- a/internal/api/handler/api.go +++ b/internal/api/handler/api.go @@ -61,7 +61,7 @@ func RegisterHandlers(server *api.Server, logger *zap.SugaredLogger, db *gorm.DB sspPoamGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) poamHandler.RegisterSSPScoped(sspPoamGroup) - riskHandler := NewRiskHandler(logger, db) + riskHandler := NewRiskHandler(logger, db, poamService) riskGroup := server.API().Group("/risks") riskGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) riskHandler.Register(riskGroup) diff --git a/internal/api/handler/poam_items.go b/internal/api/handler/poam_items.go index ae551a05..d6af7857 100644 --- a/internal/api/handler/poam_items.go +++ b/internal/api/handler/poam_items.go @@ -83,6 +83,9 @@ type createPoamItemRequest struct { PlannedCompletionDate *time.Time `json:"plannedCompletionDate"` CreatedFromRiskID *string `json:"createdFromRiskId"` AcceptanceRationale *string `json:"acceptanceRationale"` + PocName *string `json:"pocName"` + PocEmail *string `json:"pocEmail"` + ResourceRequired *string `json:"resourceRequired"` RiskIDs []string `json:"riskIds"` EvidenceIDs []string `json:"evidenceIds"` ControlRefs []poamControlRefRequest `json:"controlRefs"` @@ -178,6 +181,9 @@ type poamItemResponse struct { CompletedAt *time.Time `json:"completedAt,omitempty"` CreatedFromRiskID *uuid.UUID `json:"createdFromRiskId,omitempty"` AcceptanceRationale *string `json:"acceptanceRationale,omitempty"` + PocName *string `json:"pocName,omitempty"` + PocEmail *string `json:"pocEmail,omitempty"` + ResourceRequired *string `json:"resourceRequired,omitempty"` LastStatusChangeAt time.Time `json:"lastStatusChangeAt"` CreatedAt time.Time `json:"createdAt"` UpdatedAt time.Time `json:"updatedAt"` @@ -216,6 +222,9 @@ func toPoamItemResponse(item *poamsvc.PoamItem) poamItemResponse { CompletedAt: item.CompletedAt, CreatedFromRiskID: item.CreatedFromRiskID, AcceptanceRationale: item.AcceptanceRationale, + PocName: item.PocName, + PocEmail: item.PocEmail, + ResourceRequired: item.ResourceRequired, LastStatusChangeAt: item.LastStatusChangeAt, CreatedAt: item.CreatedAt, UpdatedAt: item.UpdatedAt, @@ -369,6 +378,9 @@ func (h *PoamItemsHandler) Create(c echo.Context) error { SourceType: in.SourceType, PlannedCompletionDate: in.PlannedCompletionDate, AcceptanceRationale: in.AcceptanceRationale, + PocName: in.PocName, + PocEmail: in.PocEmail, + ResourceRequired: in.ResourceRequired, } if in.PrimaryOwnerUserID != nil { diff --git a/internal/api/handler/risks.go b/internal/api/handler/risks.go index b49e6889..a628a61d 100644 --- a/internal/api/handler/risks.go +++ b/internal/api/handler/risks.go @@ -14,6 +14,7 @@ import ( "github.com/compliance-framework/api/internal/api" "github.com/compliance-framework/api/internal/authn" svc "github.com/compliance-framework/api/internal/service" + poamsvc "github.com/compliance-framework/api/internal/service/relational/poam" riskrel "github.com/compliance-framework/api/internal/service/relational/risks" "github.com/google/uuid" "github.com/labstack/echo/v4" @@ -23,6 +24,7 @@ import ( type RiskHandler struct { riskService *riskrel.RiskService + poamService *poamsvc.PoamService sugar *zap.SugaredLogger pagination *svc.PaginationConfig } @@ -32,9 +34,10 @@ const ( maxRiskDescriptionLength = 1000 ) -func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB) *RiskHandler { +func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB, poamSvc *poamsvc.PoamService) *RiskHandler { return &RiskHandler{ riskService: riskrel.NewRiskService(db), + poamService: poamSvc, sugar: sugar, pagination: svc.NewPaginationConfig(), } @@ -47,6 +50,7 @@ func (h *RiskHandler) Register(api *echo.Group) { api.PUT("/:id", h.Update) api.POST("/:id/accept", h.Accept) api.POST("/:id/review", h.Review) + api.POST("/:id/promote-to-poam", h.PromoteToPoam) api.DELETE("/:id", h.Delete) api.GET("/:id/events", h.GetEvents) api.GET("/:id/reviews", h.GetReviews) @@ -83,6 +87,7 @@ func (h *RiskHandler) RegisterSSPScoped(api *echo.Group) { api.PUT("/:id", h.UpdateForSSP) api.POST("/:id/accept", h.AcceptForSSP) api.POST("/:id/review", h.ReviewForSSP) + api.POST("/:id/promote-to-poam", h.PromoteToPoamForSSP) api.DELETE("/:id", h.DeleteForSSP) api.GET("/:id/events", h.GetEventsForSSP) api.GET("/:id/reviews", h.GetReviewsForSSP) @@ -2237,3 +2242,128 @@ func (h *RiskHandler) mapRiskToResponseWithAssociations(risk *riskrel.Risk, asso return response } + +// promoteToPoamRequest is the request body for POST /risks/:id/promote-to-poam. +type promoteToPoamRequest struct { + // Title overrides the risk's title as the POAM item title. + // If omitted, the risk's own title is used. + Title *string `json:"title"` + // Deadline maps to PoamItem.PlannedCompletionDate. + Deadline *time.Time `json:"deadline"` + // ResourceRequired is a free-text description of resources needed. + ResourceRequired *string `json:"resourceRequired"` + // PocName is the point-of-contact name. + PocName *string `json:"pocName"` + // PocEmail is the point-of-contact email. + PocEmail *string `json:"pocEmail"` + // Milestones are additional milestones to append after any copied from the + // risk's RemediationTemplate. + Milestones []createMilestoneRequest `json:"milestones"` +} + +// PromoteToPoam godoc +// +// @Summary Promote risk to POAM item +// @Description Promotes a risk-accepted risk to a POAM item. The risk must be in risk-accepted status. The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional. +// @Tags Risks +// @Accept json +// @Produce json +// @Param id path string true "Risk ID" +// @Param body body promoteToPoamRequest false "Promotion payload" +// @Success 201 {object} GenericDataResponse[poamItemResponse] +// @Failure 400 {object} api.Error +// @Failure 404 {object} api.Error +// @Failure 422 {object} api.Error +// @Failure 500 {object} api.Error +// @Security OAuth2Password +// @Router /risks/{id}/promote-to-poam [post] +func (h *RiskHandler) PromoteToPoam(ctx echo.Context) error { + riskID, err := parsePathUUID(ctx, "id") + if err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + + var req promoteToPoamRequest + if err := ctx.Bind(&req); err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + + // Map request milestones to service params. + var milestones []poamsvc.CreateMilestoneParams + for i, m := range req.Milestones { + orderIdx := 0 + if m.OrderIndex != nil { + orderIdx = *m.OrderIndex + } else { + orderIdx = i + } + milestones = append(milestones, poamsvc.CreateMilestoneParams{ + Title: m.Title, + Description: m.Description, + Status: m.Status, + PlannedCompletionDate: m.PlannedCompletionDate, + ResponsibleParty: m.ResponsibleParty, + Remarks: m.Remarks, + OrderIndex: orderIdx, + }) + } + + return h.withActorUserID(ctx, func(actorID *uuid.UUID) error { + poamItem, err := h.riskService.PromoteToPoam(h.poamService, riskrel.PromoteToPoamParams{ + RiskID: riskID, + ActorUserID: actorID, + Title: req.Title, + Deadline: req.Deadline, + ResourceRequired: req.ResourceRequired, + PocName: req.PocName, + PocEmail: req.PocEmail, + ExtraMilestones: milestones, + }) + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("risk not found"))) + } + if riskrel.IsValidationError(err) { + return ctx.JSON(http.StatusUnprocessableEntity, api.NewError(err)) + } + return h.internalServerError(ctx, "failed to promote risk to POAM item", err) + } + + return ctx.JSON(http.StatusCreated, GenericDataResponse[poamItemResponse]{Data: toPoamItemResponse(poamItem)}) + }) +} + +// PromoteToPoamForSSP godoc +// +// @Summary Promote risk to POAM item (SSP-scoped) +// @Description Promotes a risk-accepted risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in risk-accepted status. +// @Tags Risks +// @Accept json +// @Produce json +// @Param sspId path string true "SSP ID" +// @Param id path string true "Risk ID" +// @Param body body promoteToPoamRequest false "Promotion payload" +// @Success 201 {object} GenericDataResponse[poamItemResponse] +// @Failure 400 {object} api.Error +// @Failure 404 {object} api.Error +// @Failure 422 {object} api.Error +// @Failure 500 {object} api.Error +// @Security OAuth2Password +// @Router /oscal/system-security-plans/{sspId}/risks/{id}/promote-to-poam [post] +func (h *RiskHandler) PromoteToPoamForSSP(ctx echo.Context) error { + sspID, err := parsePathUUID(ctx, "sspId") + if err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + riskID, err := parsePathUUID(ctx, "id") + if err != nil { + return ctx.JSON(http.StatusBadRequest, api.NewError(err)) + } + if err := h.ensureRiskBelongsToSSP(riskID, sspID); err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("risk not found"))) + } + return h.internalServerError(ctx, "failed to validate scoped risk", err) + } + return h.PromoteToPoam(ctx) +} diff --git a/internal/api/handler/risks_promote_to_poam_integration_test.go b/internal/api/handler/risks_promote_to_poam_integration_test.go new file mode 100644 index 00000000..8ac7f8e6 --- /dev/null +++ b/internal/api/handler/risks_promote_to_poam_integration_test.go @@ -0,0 +1,305 @@ +//go:build integration + +package handler + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + "time" + + riskrel "github.com/compliance-framework/api/internal/service/relational/risks" + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +// --------------------------------------------------------------------------- +// BCH-1186: POST /risks/:id/promote-to-poam integration tests +// --------------------------------------------------------------------------- + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { + sspID := suite.newSSPID() + + // Create a risk in investigating status, then accept it. + created := suite.createRisk(map[string]any{ + "title": "Unencrypted data at rest", + "description": "Sensitive data stored without encryption", + "ssp-id": sspID, + "status": "investigating", + "likelihood": "high", + "impact": "critical", + }) + + acceptDeadline := time.Now().Add(30 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ + "justification": "accepted pending encryption rollout", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + + // Promote to POAM with full payload. + deadline := time.Now().Add(90 * 24 * time.Hour).UTC().Truncate(time.Second) + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{ + "title": "Encrypt data at rest — remediation plan", + "deadline": deadline.Format(time.RFC3339), + "resourceRequired": "3 engineer days", + "pocName": "Jane Smith", + "pocEmail": "jane@example.com", + "milestones": []map[string]any{ + {"title": "Identify all unencrypted data stores", "orderIndex": 0}, + {"title": "Apply AES-256 encryption to all stores", "orderIndex": 1}, + }, + }) + suite.server.E().ServeHTTP(promoteRec, promoteReq) + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code, "promote-to-poam should return 201") + + var poamResp GenericDataResponse[poamItemResponse] + require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) + + // Verify POAM item fields. + require.Equal(suite.T(), "Encrypt data at rest — remediation plan", poamResp.Data.Title) + require.Equal(suite.T(), "Sensitive data stored without encryption", poamResp.Data.Description) + require.Equal(suite.T(), "open", poamResp.Data.Status) + require.Equal(suite.T(), "risk-promotion", poamResp.Data.SourceType) + require.NotNil(suite.T(), poamResp.Data.CreatedFromRiskID) + require.Equal(suite.T(), created.ID.String(), poamResp.Data.CreatedFromRiskID.String()) + require.NotNil(suite.T(), poamResp.Data.PlannedCompletionDate) + require.WithinDuration(suite.T(), deadline, *poamResp.Data.PlannedCompletionDate, time.Second) + require.NotNil(suite.T(), poamResp.Data.PocName) + require.Equal(suite.T(), "Jane Smith", *poamResp.Data.PocName) + require.NotNil(suite.T(), poamResp.Data.PocEmail) + require.Equal(suite.T(), "jane@example.com", *poamResp.Data.PocEmail) + require.NotNil(suite.T(), poamResp.Data.ResourceRequired) + require.Equal(suite.T(), "3 engineer days", *poamResp.Data.ResourceRequired) + + // Verify milestones. + require.Len(suite.T(), poamResp.Data.Milestones, 2) + require.Equal(suite.T(), "Identify all unencrypted data stores", poamResp.Data.Milestones[0].Title) + require.Equal(suite.T(), "Apply AES-256 encryption to all stores", poamResp.Data.Milestones[1].Title) + + // Verify risk link was created. + require.Len(suite.T(), poamResp.Data.RiskLinks, 1) + require.Equal(suite.T(), created.ID.String(), poamResp.Data.RiskLinks[0].RiskID.String()) + + // Verify risk_event(poam_promoted) was emitted. + var promotedEvents int64 + require.NoError(suite.T(), suite.DB.Model(&riskrel.RiskEvent{}). + Where("risk_id = ? AND event_type = ?", created.ID, string(riskrel.RiskEventTypePoamPromoted)). + Count(&promotedEvents).Error) + require.Equal(suite.T(), int64(1), promotedEvents) +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_DefaultsFromRisk() { + sspID := suite.newSSPID() + + created := suite.createRisk(map[string]any{ + "title": "Weak password policy", + "description": "Password complexity rules not enforced", + "ssp-id": sspID, + "status": "investigating", + }) + + acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ + "justification": "accepted pending policy update", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + + // Promote with empty body — title and description should default from risk. + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) + suite.server.E().ServeHTTP(promoteRec, promoteReq) + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code) + + var poamResp GenericDataResponse[poamItemResponse] + require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) + + // Title and description should default to the risk's values. + require.Equal(suite.T(), "Weak password policy", poamResp.Data.Title) + require.Equal(suite.T(), "Password complexity rules not enforced", poamResp.Data.Description) + require.Equal(suite.T(), "open", poamResp.Data.Status) + require.Equal(suite.T(), "risk-promotion", poamResp.Data.SourceType) +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNonAcceptedRisk() { + sspID := suite.newSSPID() + + // Risk in "open" status — not risk-accepted. + created := suite.createRisk(map[string]any{ + "title": "Open risk", + "description": "Not yet accepted", + "ssp-id": sspID, + "status": "open", + }) + + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) + suite.server.E().ServeHTTP(promoteRec, promoteReq) + require.Equal(suite.T(), http.StatusUnprocessableEntity, promoteRec.Code, "should return 422 for non-accepted risk") + + // Risk in "investigating" status — also not risk-accepted. + investigating := suite.createRisk(map[string]any{ + "title": "Investigating risk", + "description": "Still being investigated", + "ssp-id": sspID, + "status": "investigating", + }) + + promoteRec2, promoteReq2 := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", investigating.ID), map[string]any{}) + suite.server.E().ServeHTTP(promoteRec2, promoteReq2) + require.Equal(suite.T(), http.StatusUnprocessableEntity, promoteRec2.Code, "should return 422 for investigating risk") +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsActivePoamAlreadyLinked() { + sspID := suite.newSSPID() + + created := suite.createRisk(map[string]any{ + "title": "Duplicate promotion risk", + "description": "Testing re-promotion guard", + "ssp-id": sspID, + "status": "investigating", + }) + + acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ + "justification": "accepted", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + + // First promotion should succeed. + firstRec, firstReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) + suite.server.E().ServeHTTP(firstRec, firstReq) + require.Equal(suite.T(), http.StatusCreated, firstRec.Code) + + // Second promotion should fail — active POAM item already linked. + secondRec, secondReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) + suite.server.E().ServeHTTP(secondRec, secondReq) + require.Equal(suite.T(), http.StatusUnprocessableEntity, secondRec.Code, "should return 422 when active POAM already linked") +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNotFound() { + nonExistentID := uuid.New() + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", nonExistentID), map[string]any{}) + suite.server.E().ServeHTTP(promoteRec, promoteReq) + require.Equal(suite.T(), http.StatusNotFound, promoteRec.Code) +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { + sspID := suite.newSSPID() + + created := suite.createRisk(map[string]any{ + "title": "SSP-scoped promotion risk", + "description": "Testing SSP-scoped promote endpoint", + "ssp-id": sspID, + "status": "investigating", + }) + + acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ + "justification": "accepted for SSP-scoped test", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + + // Promote via SSP-scoped endpoint. + sspPromoteRec, sspPromoteReq := suite.authedRequest( + http.MethodPost, + fmt.Sprintf("/api/oscal/system-security-plans/%s/risks/%s/promote-to-poam", sspID, created.ID), + map[string]any{"pocName": "SSP Owner"}, + ) + suite.server.E().ServeHTTP(sspPromoteRec, sspPromoteReq) + require.Equal(suite.T(), http.StatusCreated, sspPromoteRec.Code) + + var poamResp GenericDataResponse[poamItemResponse] + require.NoError(suite.T(), json.Unmarshal(sspPromoteRec.Body.Bytes(), &poamResp)) + require.Equal(suite.T(), "SSP-scoped promotion risk", poamResp.Data.Title) + require.NotNil(suite.T(), poamResp.Data.PocName) + require.Equal(suite.T(), "SSP Owner", *poamResp.Data.PocName) +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_RejectsWrongSSP() { + sspID := suite.newSSPID() + wrongSSPID := suite.newSSPID() + + created := suite.createRisk(map[string]any{ + "title": "Wrong SSP risk", + "description": "Risk belongs to a different SSP", + "ssp-id": sspID, + "status": "investigating", + }) + + acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ + "justification": "accepted", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + + // Attempt to promote via wrong SSP — should return 404. + wrongSSPRec, wrongSSPReq := suite.authedRequest( + http.MethodPost, + fmt.Sprintf("/api/oscal/system-security-plans/%s/risks/%s/promote-to-poam", wrongSSPID, created.ID), + map[string]any{}, + ) + suite.server.E().ServeHTTP(wrongSSPRec, wrongSSPReq) + require.Equal(suite.T(), http.StatusNotFound, wrongSSPRec.Code) +} + +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_WithRemediationTemplate() { + sspID := suite.newSSPID() + + created := suite.createRisk(map[string]any{ + "title": "Risk with remediation template", + "description": "Has a remediation template with tasks", + "ssp-id": sspID, + "status": "investigating", + }) + + // Create a remediation template with 2 tasks. + // Note: the remediationTaskRequest uses "order-index" (kebab-case) as its JSON tag. + remRec, remReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/remediation-template", created.ID), map[string]any{ + "title": "Standard remediation plan", + "tasks": []map[string]any{ + {"title": "Template task 1", "order-index": 0}, + {"title": "Template task 2", "order-index": 1}, + }, + }) + suite.server.E().ServeHTTP(remRec, remReq) + require.Equal(suite.T(), http.StatusCreated, remRec.Code) + + // Accept the risk. + acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ + "justification": "accepted", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + + // Promote with 1 extra milestone — should have 3 total (2 template + 1 extra). + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{ + "milestones": []map[string]any{ + {"title": "Extra milestone from request", "orderIndex": 2}, + }, + }) + suite.server.E().ServeHTTP(promoteRec, promoteReq) + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code) + + var poamResp GenericDataResponse[poamItemResponse] + require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) + + // Should have 3 milestones: 2 from template + 1 from request. + require.Len(suite.T(), poamResp.Data.Milestones, 3) + require.Equal(suite.T(), "Template task 1", poamResp.Data.Milestones[0].Title) + require.Equal(suite.T(), "Template task 2", poamResp.Data.Milestones[1].Title) + require.Equal(suite.T(), "Extra milestone from request", poamResp.Data.Milestones[2].Title) +} + +// Ensure the testing import is used. +var _ = (*testing.T)(nil) diff --git a/internal/service/relational/poam/models.go b/internal/service/relational/poam/models.go index 2bb0aed9..8df43261 100644 --- a/internal/service/relational/poam/models.go +++ b/internal/service/relational/poam/models.go @@ -78,6 +78,10 @@ type PoamItem struct { CompletedAt *time.Time ` json:"completedAt,omitempty"` CreatedFromRiskID *uuid.UUID `gorm:"type:uuid" json:"createdFromRiskId,omitempty"` AcceptanceRationale *string ` json:"acceptanceRationale,omitempty"` + // Point-of-contact and resource fields — populated on risk-promotion or manual entry. + PocName *string `gorm:"type:text" json:"pocName,omitempty"` + PocEmail *string `gorm:"type:text" json:"pocEmail,omitempty"` + ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` LastStatusChangeAt time.Time `gorm:"not null" json:"lastStatusChangeAt"` CreatedAt time.Time ` json:"createdAt"` UpdatedAt time.Time ` json:"updatedAt"` diff --git a/internal/service/relational/poam/service.go b/internal/service/relational/poam/service.go index 9e052c94..321369c0 100644 --- a/internal/service/relational/poam/service.go +++ b/internal/service/relational/poam/service.go @@ -38,6 +38,9 @@ type CreatePoamItemParams struct { PlannedCompletionDate *time.Time CreatedFromRiskID *uuid.UUID AcceptanceRationale *string + PocName *string + PocEmail *string + ResourceRequired *string RiskIDs []uuid.UUID EvidenceIDs []uuid.UUID ControlRefs []ControlRef @@ -116,6 +119,9 @@ func (s *PoamService) Create(params CreatePoamItemParams) (*PoamItem, error) { PlannedCompletionDate: params.PlannedCompletionDate, CreatedFromRiskID: params.CreatedFromRiskID, AcceptanceRationale: params.AcceptanceRationale, + PocName: params.PocName, + PocEmail: params.PocEmail, + ResourceRequired: params.ResourceRequired, } tx, err := beginTx(s.db) @@ -688,6 +694,61 @@ func (s *PoamService) DeleteFindingLink(poamItemID, findingID uuid.UUID) error { return nil } +// CreateWithTx inserts a new POAM item and its milestones/links within an +// externally-managed *gorm.DB transaction. The caller is responsible for +// committing or rolling back the transaction. This is used by cross-context +// operations such as RiskService.PromoteToPoam that need atomicity across +// multiple bounded contexts. +func (s *PoamService) CreateWithTx(tx *gorm.DB, params CreatePoamItemParams) (*PoamItem, error) { + item := PoamItem{ + SspID: params.SspID, + Title: params.Title, + Description: params.Description, + Status: params.Status, + SourceType: params.SourceType, + PrimaryOwnerUserID: params.PrimaryOwnerUserID, + PlannedCompletionDate: params.PlannedCompletionDate, + CreatedFromRiskID: params.CreatedFromRiskID, + AcceptanceRationale: params.AcceptanceRationale, + PocName: params.PocName, + PocEmail: params.PocEmail, + ResourceRequired: params.ResourceRequired, + } + + if err := tx.Create(&item).Error; err != nil { + return nil, err + } + + for i, mp := range params.Milestones { + orderIdx := mp.OrderIndex + if orderIdx == 0 { + orderIdx = i + } + ms := PoamItemMilestone{ + PoamItemID: item.ID, + Title: mp.Title, + Description: mp.Description, + Status: mp.Status, + PlannedCompletionDate: mp.PlannedCompletionDate, + ResponsibleParty: mp.ResponsibleParty, + Remarks: mp.Remarks, + OrderIndex: orderIdx, + } + if err := tx.Create(&ms).Error; err != nil { + return nil, err + } + } + + for _, riskID := range params.RiskIDs { + link := PoamItemRiskLink{PoamItemID: item.ID, RiskID: riskID} + if err := tx.Clauses(clause.OnConflict{DoNothing: true}).Create(&link).Error; err != nil { + return nil, err + } + } + + return &item, nil +} + // --------------------------------------------------------------------------- // Transaction helpers // --------------------------------------------------------------------------- diff --git a/internal/service/relational/risks/events.go b/internal/service/relational/risks/events.go index 379ea4d4..9e9a4d9c 100644 --- a/internal/service/relational/risks/events.go +++ b/internal/service/relational/risks/events.go @@ -34,6 +34,7 @@ const ( RiskEventTypeRemediationUpdated RiskEventType = "remediation_updated" RiskEventTypeRemediationDeleted RiskEventType = "remediation_deleted" RiskEventTypeEvidenceRecovered RiskEventType = "evidence_recovered" + RiskEventTypePoamPromoted RiskEventType = "poam_promoted" ) type RiskEvent struct { @@ -187,6 +188,11 @@ func BuildRiskEventDetails(eventType string, payload datatypes.JSONMap, occurred return fmt.Sprintf("Evidence %s recovered; risk is accepted so no automatic status change was applied.", evidenceID) } return "Evidence recovered; risk is accepted so no automatic status change was applied." + case string(RiskEventTypePoamPromoted): + if poamItemID := payloadString(payload, "poamItemId"); poamItemID != "" { + return fmt.Sprintf("Risk was promoted to POAM item %s.", poamItemID) + } + return "Risk was promoted to a POAM item." default: return fmt.Sprintf("Risk event recorded: %s.", eventType) } diff --git a/internal/service/relational/risks/promote_to_poam.go b/internal/service/relational/risks/promote_to_poam.go new file mode 100644 index 00000000..6d5f4f8c --- /dev/null +++ b/internal/service/relational/risks/promote_to_poam.go @@ -0,0 +1,175 @@ +package risks + +import ( + "errors" + "time" + + poamsvc "github.com/compliance-framework/api/internal/service/relational/poam" + "github.com/google/uuid" + "gorm.io/datatypes" + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +// PromoteToPoamParams carries all inputs required to promote a risk-accepted +// risk to a POAM item. +type PromoteToPoamParams struct { + // RiskID is the UUID of the risk to promote. The risk must be in + // risk-accepted status; any other status returns a 422 ValidationError. + RiskID uuid.UUID + // ActorUserID is the authenticated user performing the promotion. + ActorUserID *uuid.UUID + // Title overrides the risk's title as the POAM item title. + // If nil, the risk's own title is used. + Title *string + // Deadline maps to PoamItem.PlannedCompletionDate. + Deadline *time.Time + // ResourceRequired is a free-text field describing resources needed. + ResourceRequired *string + // PocName is the point-of-contact name. + PocName *string + // PocEmail is the point-of-contact email. + PocEmail *string + // ExtraMilestones are additional milestones supplied in the request body. + // They are appended after any milestones copied from the risk's + // RemediationTemplate, with order_index offset accordingly. + ExtraMilestones []poamsvc.CreateMilestoneParams +} + +// PromoteToPoam promotes a risk-accepted risk to a POAM item. The entire +// operation — POAM item creation, milestone creation, risk link creation, and +// risk event emission — is executed inside a single database transaction so +// that no partial state is persisted on failure. +// +// Re-promotion is allowed only if all previously linked POAM items are in +// completed status. If an active (non-completed) POAM item already exists for +// this risk, a ValidationError is returned. +func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params PromoteToPoamParams) (*poamsvc.PoamItem, error) { + tx, err := beginTx(s.db) + if err != nil { + return nil, err + } + defer rollbackTxOnPanic(tx) + + // 1. Load and lock the risk row. + var risk Risk + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). + Preload("OwnerAssignments"). + First(&risk, "id = ?", params.RiskID).Error; err != nil { + tx.Rollback() + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, err + } + return nil, err + } + + // 2. Guard: risk must be in risk-accepted status. + if risk.Status != string(RiskStatusRiskAccepted) { + tx.Rollback() + return nil, newValidationError("only risks in status risk-accepted can be promoted to a POAM item") + } + + // 3. Re-promotion guard: reject if an active (non-completed) POAM item + // is already linked to this risk. + type linkRow struct { + PoamItemID uuid.UUID + Status string + } + var activeLinks []linkRow + if err := tx.Raw(` + SELECT l.poam_item_id, p.status + FROM ccf_poam_item_risk_links l + JOIN ccf_poam_items p ON p.id = l.poam_item_id + WHERE l.risk_id = ? AND p.status != 'completed' + `, params.RiskID).Scan(&activeLinks).Error; err != nil { + tx.Rollback() + return nil, err + } + if len(activeLinks) > 0 { + tx.Rollback() + return nil, newValidationError("an active POAM item is already linked to this risk; complete it before re-promoting") + } + + // 4. Load the risk's RemediationTemplate (if any) to copy tasks as + // initial milestones. + var templateMilestones []poamsvc.CreateMilestoneParams + var remediationTemplate RiskRemediationTemplate + err = tx. + Where("risk_id = ?", params.RiskID). + Preload("Tasks", func(db *gorm.DB) *gorm.DB { return db.Order("order_index ASC") }). + First(&remediationTemplate).Error + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { + tx.Rollback() + return nil, err + } + if err == nil { + for _, task := range remediationTemplate.Tasks { + templateMilestones = append(templateMilestones, poamsvc.CreateMilestoneParams{ + Title: task.Title, + OrderIndex: task.OrderIndex, + }) + } + } + + // 5. Merge template milestones with extra milestones from the request. + // Extra milestones are appended after template tasks, with order_index + // offset by the number of template tasks. + offset := len(templateMilestones) + for i, extra := range params.ExtraMilestones { + if extra.OrderIndex == 0 { + extra.OrderIndex = offset + i + } + templateMilestones = append(templateMilestones, extra) + } + + // 6. Resolve the POAM item title — default to risk title if not overridden. + title := risk.Title + if params.Title != nil && *params.Title != "" { + title = *params.Title + } + + // 7. Build the POAM item creation params. + riskID := params.RiskID + createParams := poamsvc.CreatePoamItemParams{ + SspID: risk.SSPID, + Title: title, + Description: risk.Description, + Status: string(poamsvc.PoamItemStatusOpen), + SourceType: string(poamsvc.PoamItemSourceTypeRiskPromotion), + PlannedCompletionDate: params.Deadline, + CreatedFromRiskID: &riskID, + PocName: params.PocName, + PocEmail: params.PocEmail, + ResourceRequired: params.ResourceRequired, + RiskIDs: []uuid.UUID{params.RiskID}, + Milestones: templateMilestones, + } + + // 8. Create the POAM item within the shared transaction. + poamItem, err := poamSvc.CreateWithTx(tx, createParams) + if err != nil { + tx.Rollback() + return nil, err + } + + // 9. Emit the risk event. + riskSnapshot, err := s.getRiskSnapshot(tx, params.RiskID) + if err != nil { + tx.Rollback() + return nil, err + } + if err := s.logRiskEventWithSnapshot(tx, params.RiskID, RiskEventTypePoamPromoted, params.ActorUserID, datatypes.JSONMap{ + "poamItemId": poamItem.ID.String(), + }, riskSnapshot); err != nil { + tx.Rollback() + return nil, err + } + + // 10. Commit the transaction. + if err := tx.Commit().Error; err != nil { + return nil, err + } + + // 11. Return the fully-loaded POAM item (with milestones and links). + return poamSvc.GetByID(poamItem.ID) +} From 1099a495bd999548aa9e6bd81e1d9f35dc0fb0e2 Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Tue, 24 Mar 2026 12:18:42 -0400 Subject: [PATCH 2/7] =?UTF-8?q?fix(poam):=20BCH-1186=20CI=20fixes=20?= =?UTF-8?q?=E2=80=94=20regenerate=20swagger=20docs=20and=20fix=20test=20or?= =?UTF-8?q?der-index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- docs/docs.go | 195 ++++++++++++++++++ docs/swagger.json | 195 ++++++++++++++++++ docs/swagger.yaml | 135 ++++++++++++ .../risks_promote_to_poam_integration_test.go | 4 +- internal/service/relational/poam/models.go | 12 +- 5 files changed, 533 insertions(+), 8 deletions(-) diff --git a/docs/docs.go b/docs/docs.go index 55c577f3..67f9e4b6 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -18666,6 +18666,82 @@ const docTemplate = `{ ] } }, + "/oscal/system-security-plans/{sspId}/risks/{id}/promote-to-poam": { + "post": { + "description": "Promotes a risk-accepted risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in risk-accepted status.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Risks" + ], + "summary": "Promote risk to POAM item (SSP-scoped)", + "parameters": [ + { + "type": "string", + "description": "SSP ID", + "name": "sspId", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Risk ID", + "name": "id", + "in": "path", + "required": true + }, + { + "description": "Promotion payload", + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/handler.promoteToPoamRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_poamItemResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "422": { + "description": "Unprocessable Entity", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, "/oscal/system-security-plans/{sspId}/risks/{id}/remediation-template": { "get": { "description": "Gets the remediation template linked to a risk scoped to an SSP.", @@ -21594,6 +21670,75 @@ const docTemplate = `{ ] } }, + "/risks/{id}/promote-to-poam": { + "post": { + "description": "Promotes a risk-accepted risk to a POAM item. The risk must be in risk-accepted status. The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Risks" + ], + "summary": "Promote risk to POAM item", + "parameters": [ + { + "type": "string", + "description": "Risk ID", + "name": "id", + "in": "path", + "required": true + }, + { + "description": "Promotion payload", + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/handler.promoteToPoamRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_poamItemResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "422": { + "description": "Unprocessable Entity", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, "/risks/{id}/remediation-template": { "get": { "description": "Gets the remediation template linked to a risk.", @@ -28174,9 +28319,18 @@ const docTemplate = `{ "plannedCompletionDate": { "type": "string" }, + "pocEmail": { + "type": "string" + }, + "pocName": { + "type": "string" + }, "primaryOwnerUserId": { "type": "string" }, + "resourceRequired": { + "type": "string" + }, "riskIds": { "type": "array", "items": { @@ -28385,9 +28539,18 @@ const docTemplate = `{ "plannedCompletionDate": { "type": "string" }, + "pocEmail": { + "type": "string" + }, + "pocName": { + "type": "string" + }, "primaryOwnerUserId": { "type": "string" }, + "resourceRequired": { + "type": "string" + }, "riskLinks": { "type": "array", "items": { @@ -28411,6 +28574,38 @@ const docTemplate = `{ } } }, + "handler.promoteToPoamRequest": { + "type": "object", + "properties": { + "deadline": { + "description": "Deadline maps to PoamItem.PlannedCompletionDate.", + "type": "string" + }, + "milestones": { + "description": "Milestones are additional milestones to append after any copied from the\nrisk's RemediationTemplate.", + "type": "array", + "items": { + "$ref": "#/definitions/handler.createMilestoneRequest" + } + }, + "pocEmail": { + "description": "PocEmail is the point-of-contact email.", + "type": "string" + }, + "pocName": { + "description": "PocName is the point-of-contact name.", + "type": "string" + }, + "resourceRequired": { + "description": "ResourceRequired is a free-text description of resources needed.", + "type": "string" + }, + "title": { + "description": "Title overrides the risk's title as the POAM item title.\nIf omitted, the risk's own title is used.", + "type": "string" + } + } + }, "handler.publicUserResponse": { "type": "object", "properties": { diff --git a/docs/swagger.json b/docs/swagger.json index 813a94a5..c1e42e4f 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -18660,6 +18660,82 @@ ] } }, + "/oscal/system-security-plans/{sspId}/risks/{id}/promote-to-poam": { + "post": { + "description": "Promotes a risk-accepted risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in risk-accepted status.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Risks" + ], + "summary": "Promote risk to POAM item (SSP-scoped)", + "parameters": [ + { + "type": "string", + "description": "SSP ID", + "name": "sspId", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Risk ID", + "name": "id", + "in": "path", + "required": true + }, + { + "description": "Promotion payload", + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/handler.promoteToPoamRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_poamItemResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "422": { + "description": "Unprocessable Entity", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, "/oscal/system-security-plans/{sspId}/risks/{id}/remediation-template": { "get": { "description": "Gets the remediation template linked to a risk scoped to an SSP.", @@ -21588,6 +21664,75 @@ ] } }, + "/risks/{id}/promote-to-poam": { + "post": { + "description": "Promotes a risk-accepted risk to a POAM item. The risk must be in risk-accepted status. The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Risks" + ], + "summary": "Promote risk to POAM item", + "parameters": [ + { + "type": "string", + "description": "Risk ID", + "name": "id", + "in": "path", + "required": true + }, + { + "description": "Promotion payload", + "name": "body", + "in": "body", + "schema": { + "$ref": "#/definitions/handler.promoteToPoamRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/handler.GenericDataResponse-handler_poamItemResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "422": { + "description": "Unprocessable Entity", + "schema": { + "$ref": "#/definitions/api.Error" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/api.Error" + } + } + }, + "security": [ + { + "OAuth2Password": [] + } + ] + } + }, "/risks/{id}/remediation-template": { "get": { "description": "Gets the remediation template linked to a risk.", @@ -28168,9 +28313,18 @@ "plannedCompletionDate": { "type": "string" }, + "pocEmail": { + "type": "string" + }, + "pocName": { + "type": "string" + }, "primaryOwnerUserId": { "type": "string" }, + "resourceRequired": { + "type": "string" + }, "riskIds": { "type": "array", "items": { @@ -28379,9 +28533,18 @@ "plannedCompletionDate": { "type": "string" }, + "pocEmail": { + "type": "string" + }, + "pocName": { + "type": "string" + }, "primaryOwnerUserId": { "type": "string" }, + "resourceRequired": { + "type": "string" + }, "riskLinks": { "type": "array", "items": { @@ -28405,6 +28568,38 @@ } } }, + "handler.promoteToPoamRequest": { + "type": "object", + "properties": { + "deadline": { + "description": "Deadline maps to PoamItem.PlannedCompletionDate.", + "type": "string" + }, + "milestones": { + "description": "Milestones are additional milestones to append after any copied from the\nrisk's RemediationTemplate.", + "type": "array", + "items": { + "$ref": "#/definitions/handler.createMilestoneRequest" + } + }, + "pocEmail": { + "description": "PocEmail is the point-of-contact email.", + "type": "string" + }, + "pocName": { + "description": "PocName is the point-of-contact name.", + "type": "string" + }, + "resourceRequired": { + "description": "ResourceRequired is a free-text description of resources needed.", + "type": "string" + }, + "title": { + "description": "Title overrides the risk's title as the POAM item title.\nIf omitted, the risk's own title is used.", + "type": "string" + } + } + }, "handler.publicUserResponse": { "type": "object", "properties": { diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 1b76704a..72c591c9 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1609,8 +1609,14 @@ definitions: type: array plannedCompletionDate: type: string + pocEmail: + type: string + pocName: + type: string primaryOwnerUserId: type: string + resourceRequired: + type: string riskIds: items: type: string @@ -1751,8 +1757,14 @@ definitions: type: array plannedCompletionDate: type: string + pocEmail: + type: string + pocName: + type: string primaryOwnerUserId: type: string + resourceRequired: + type: string riskLinks: items: $ref: '#/definitions/handler.riskLinkResponse' @@ -1768,6 +1780,33 @@ definitions: updatedAt: type: string type: object + handler.promoteToPoamRequest: + properties: + deadline: + description: Deadline maps to PoamItem.PlannedCompletionDate. + type: string + milestones: + description: |- + Milestones are additional milestones to append after any copied from the + risk's RemediationTemplate. + items: + $ref: '#/definitions/handler.createMilestoneRequest' + type: array + pocEmail: + description: PocEmail is the point-of-contact email. + type: string + pocName: + description: PocName is the point-of-contact name. + type: string + resourceRequired: + description: ResourceRequired is a free-text description of resources needed. + type: string + title: + description: |- + Title overrides the risk's title as the POAM item title. + If omitted, the risk's own title is used. + type: string + type: object handler.publicUserResponse: properties: id: @@ -20939,6 +20978,56 @@ paths: summary: Delete risk evidence link for SSP tags: - Risks + /oscal/system-security-plans/{sspId}/risks/{id}/promote-to-poam: + post: + consumes: + - application/json + description: Promotes a risk-accepted risk to a POAM item, scoped to a specific + SSP. The risk must belong to the given SSP and be in risk-accepted status. + parameters: + - description: SSP ID + in: path + name: sspId + required: true + type: string + - description: Risk ID + in: path + name: id + required: true + type: string + - description: Promotion payload + in: body + name: body + schema: + $ref: '#/definitions/handler.promoteToPoamRequest' + produces: + - application/json + responses: + "201": + description: Created + schema: + $ref: '#/definitions/handler.GenericDataResponse-handler_poamItemResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/api.Error' + "404": + description: Not Found + schema: + $ref: '#/definitions/api.Error' + "422": + description: Unprocessable Entity + schema: + $ref: '#/definitions/api.Error' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/api.Error' + security: + - OAuth2Password: [] + summary: Promote risk to POAM item (SSP-scoped) + tags: + - Risks /oscal/system-security-plans/{sspId}/risks/{id}/remediation-template: delete: description: Deletes the remediation template linked to a risk scoped to an @@ -22825,6 +22914,52 @@ paths: summary: Delete risk evidence link tags: - Risks + /risks/{id}/promote-to-poam: + post: + consumes: + - application/json + description: Promotes a risk-accepted risk to a POAM item. The risk must be + in risk-accepted status. The POAM item is pre-populated from the risk's data + and any RemediationTemplate tasks. The entire operation is transactional. + parameters: + - description: Risk ID + in: path + name: id + required: true + type: string + - description: Promotion payload + in: body + name: body + schema: + $ref: '#/definitions/handler.promoteToPoamRequest' + produces: + - application/json + responses: + "201": + description: Created + schema: + $ref: '#/definitions/handler.GenericDataResponse-handler_poamItemResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/api.Error' + "404": + description: Not Found + schema: + $ref: '#/definitions/api.Error' + "422": + description: Unprocessable Entity + schema: + $ref: '#/definitions/api.Error' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/api.Error' + security: + - OAuth2Password: [] + summary: Promote risk to POAM item + tags: + - Risks /risks/{id}/remediation-template: delete: description: Deletes the remediation template linked to a risk. diff --git a/internal/api/handler/risks_promote_to_poam_integration_test.go b/internal/api/handler/risks_promote_to_poam_integration_test.go index 8ac7f8e6..c8c18d75 100644 --- a/internal/api/handler/risks_promote_to_poam_integration_test.go +++ b/internal/api/handler/risks_promote_to_poam_integration_test.go @@ -266,8 +266,8 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_WithRemediationTemplate( remRec, remReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/remediation-template", created.ID), map[string]any{ "title": "Standard remediation plan", "tasks": []map[string]any{ - {"title": "Template task 1", "order-index": 0}, - {"title": "Template task 2", "order-index": 1}, + {"title": "Template task 1", "order-index": 1}, + {"title": "Template task 2", "order-index": 2}, }, }) suite.server.E().ServeHTTP(remRec, remReq) diff --git a/internal/service/relational/poam/models.go b/internal/service/relational/poam/models.go index 8df43261..bc8a810a 100644 --- a/internal/service/relational/poam/models.go +++ b/internal/service/relational/poam/models.go @@ -79,12 +79,12 @@ type PoamItem struct { CreatedFromRiskID *uuid.UUID `gorm:"type:uuid" json:"createdFromRiskId,omitempty"` AcceptanceRationale *string ` json:"acceptanceRationale,omitempty"` // Point-of-contact and resource fields — populated on risk-promotion or manual entry. - PocName *string `gorm:"type:text" json:"pocName,omitempty"` - PocEmail *string `gorm:"type:text" json:"pocEmail,omitempty"` - ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` - LastStatusChangeAt time.Time `gorm:"not null" json:"lastStatusChangeAt"` - CreatedAt time.Time ` json:"createdAt"` - UpdatedAt time.Time ` json:"updatedAt"` + PocName *string `gorm:"type:text" json:"pocName,omitempty"` + PocEmail *string `gorm:"type:text" json:"pocEmail,omitempty"` + ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` + LastStatusChangeAt time.Time `gorm:"not null" json:"lastStatusChangeAt"` + CreatedAt time.Time ` json:"createdAt"` + UpdatedAt time.Time ` json:"updatedAt"` // Associations — loaded on demand via Preload. Milestones []PoamItemMilestone `gorm:"foreignKey:PoamItemID;constraint:OnDelete:CASCADE" json:"milestones,omitempty"` From dd67d969d2ce65d04f4ef0028523defc330b8d91 Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Wed, 25 Mar 2026 02:26:04 -0400 Subject: [PATCH 3/7] =?UTF-8?q?fix(poam):=20BCH-1186=20address=20review=20?= =?UTF-8?q?comments=20=E2=80=94=20investigating=20status=20+=20lifecycle?= =?UTF-8?q?=20transitions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- docs/docs.go | 4 +- docs/swagger.json | 4 +- docs/swagger.yaml | 13 +- internal/api/handler/api.go | 6 +- internal/api/handler/poam_items.go | 37 +++- internal/api/handler/risks.go | 17 +- .../risks_promote_to_poam_integration_test.go | 183 +++++++++++------- internal/service/relational/risks/events.go | 10 +- .../relational/risks/poam_completion.go | 93 +++++++++ .../relational/risks/promote_to_poam.go | 40 +++- 10 files changed, 305 insertions(+), 102 deletions(-) create mode 100644 internal/service/relational/risks/poam_completion.go diff --git a/docs/docs.go b/docs/docs.go index 67f9e4b6..1293670c 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -18668,7 +18668,7 @@ const docTemplate = `{ }, "/oscal/system-security-plans/{sspId}/risks/{id}/promote-to-poam": { "post": { - "description": "Promotes a risk-accepted risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in risk-accepted status.", + "description": "Promotes an investigating risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in investigating status. On success, the risk transitions to mitigating-planned.", "consumes": [ "application/json" ], @@ -21672,7 +21672,7 @@ const docTemplate = `{ }, "/risks/{id}/promote-to-poam": { "post": { - "description": "Promotes a risk-accepted risk to a POAM item. The risk must be in risk-accepted status. The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional.", + "description": "Promotes an investigating risk to a POAM item and transitions the risk to mitigating-planned. The risk must be in investigating status (risk-accepted risks cannot be promoted — they have been formally accepted as tolerable). The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional.", "consumes": [ "application/json" ], diff --git a/docs/swagger.json b/docs/swagger.json index c1e42e4f..24e01df2 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -18662,7 +18662,7 @@ }, "/oscal/system-security-plans/{sspId}/risks/{id}/promote-to-poam": { "post": { - "description": "Promotes a risk-accepted risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in risk-accepted status.", + "description": "Promotes an investigating risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in investigating status. On success, the risk transitions to mitigating-planned.", "consumes": [ "application/json" ], @@ -21666,7 +21666,7 @@ }, "/risks/{id}/promote-to-poam": { "post": { - "description": "Promotes a risk-accepted risk to a POAM item. The risk must be in risk-accepted status. The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional.", + "description": "Promotes an investigating risk to a POAM item and transitions the risk to mitigating-planned. The risk must be in investigating status (risk-accepted risks cannot be promoted — they have been formally accepted as tolerable). The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional.", "consumes": [ "application/json" ], diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 72c591c9..365ebded 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -20982,8 +20982,9 @@ paths: post: consumes: - application/json - description: Promotes a risk-accepted risk to a POAM item, scoped to a specific - SSP. The risk must belong to the given SSP and be in risk-accepted status. + description: Promotes an investigating risk to a POAM item, scoped to a specific + SSP. The risk must belong to the given SSP and be in investigating status. + On success, the risk transitions to mitigating-planned. parameters: - description: SSP ID in: path @@ -22918,9 +22919,11 @@ paths: post: consumes: - application/json - description: Promotes a risk-accepted risk to a POAM item. The risk must be - in risk-accepted status. The POAM item is pre-populated from the risk's data - and any RemediationTemplate tasks. The entire operation is transactional. + description: Promotes an investigating risk to a POAM item and transitions the + risk to mitigating-planned. The risk must be in investigating status (risk-accepted + risks cannot be promoted — they have been formally accepted as tolerable). + The POAM item is pre-populated from the risk's data and any RemediationTemplate + tasks. The entire operation is transactional. parameters: - description: Risk ID in: path diff --git a/internal/api/handler/api.go b/internal/api/handler/api.go index 8ea560d6..afbc7d21 100644 --- a/internal/api/handler/api.go +++ b/internal/api/handler/api.go @@ -11,6 +11,7 @@ import ( "github.com/compliance-framework/api/internal/service/digest" evidencesvc "github.com/compliance-framework/api/internal/service/relational/evidence" poamsvc "github.com/compliance-framework/api/internal/service/relational/poam" + riskrel "github.com/compliance-framework/api/internal/service/relational/risks" workflowsvc "github.com/compliance-framework/api/internal/service/relational/workflows" "github.com/compliance-framework/api/internal/workflow" "github.com/labstack/echo/v4" @@ -50,7 +51,8 @@ func RegisterHandlers(server *api.Server, logger *zap.SugaredLogger, db *gorm.DB evidenceHandler.Register(server.API().Group("/evidence")) poamService := poamsvc.NewPoamService(db) - poamHandler := NewPoamItemsHandler(poamService, logger) + riskService := riskrel.NewRiskService(db) + poamHandler := NewPoamItemsHandler(poamService, riskService, logger) // Flat route: /api/poam-items (supports ?sspId= query filter) poamGroup := server.API().Group("/poam-items") poamGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) @@ -61,7 +63,7 @@ func RegisterHandlers(server *api.Server, logger *zap.SugaredLogger, db *gorm.DB sspPoamGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) poamHandler.RegisterSSPScoped(sspPoamGroup) - riskHandler := NewRiskHandler(logger, db, poamService) + riskHandler := NewRiskHandler(logger, db, poamService, riskService) riskGroup := server.API().Group("/risks") riskGroup.Use(middleware.JWTMiddleware(config.JWTPublicKey)) riskHandler.Register(riskGroup) diff --git a/internal/api/handler/poam_items.go b/internal/api/handler/poam_items.go index d6af7857..6f61a3a3 100644 --- a/internal/api/handler/poam_items.go +++ b/internal/api/handler/poam_items.go @@ -7,7 +7,9 @@ import ( "time" "github.com/compliance-framework/api/internal/api" + "github.com/compliance-framework/api/internal/authn" poamsvc "github.com/compliance-framework/api/internal/service/relational/poam" + riskrel "github.com/compliance-framework/api/internal/service/relational/risks" "github.com/google/uuid" "github.com/labstack/echo/v4" "go.uber.org/zap" @@ -19,12 +21,13 @@ import ( // imports gorm directly for data access. type PoamItemsHandler struct { poamService *poamsvc.PoamService + riskService *riskrel.RiskService sugar *zap.SugaredLogger } // NewPoamItemsHandler constructs a PoamItemsHandler. -func NewPoamItemsHandler(svc *poamsvc.PoamService, sugar *zap.SugaredLogger) *PoamItemsHandler { - return &PoamItemsHandler{poamService: svc, sugar: sugar} +func NewPoamItemsHandler(svc *poamsvc.PoamService, riskSvc *riskrel.RiskService, sugar *zap.SugaredLogger) *PoamItemsHandler { + return &PoamItemsHandler{poamService: svc, riskService: riskSvc, sugar: sugar} } // Register mounts all POAM routes onto the given Echo group. JWT middleware @@ -572,6 +575,16 @@ func (h *PoamItemsHandler) Update(c echo.Context) error { } params.RemoveFindingIDs = removeFindingIDs + // Capture the current status before the update to detect a completion transition. + currentItem, err := h.poamService.GetByID(id) + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return c.JSON(http.StatusNotFound, api.NewError(err)) + } + return h.internalError(c, "failed to fetch poam item", err) + } + wasCompleted := currentItem.Status == string(poamsvc.PoamItemStatusCompleted) + item, err := h.poamService.Update(id, params) if err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { @@ -579,6 +592,26 @@ func (h *PoamItemsHandler) Update(c echo.Context) error { } return h.internalError(c, "failed to update poam item", err) } + + // When a POAM item transitions to completed, advance all linked risks from + // mitigating-planned → mitigating-implemented. + nowCompleted := item.Status == string(poamsvc.PoamItemStatusCompleted) + if !wasCompleted && nowCompleted { + var actorUserID *uuid.UUID + if claims, ok := c.Get("user").(*authn.UserClaims); ok && claims != nil { + if uid, err := h.riskService.ResolveUserIDByEmail(claims.Subject); err == nil { + actorUserID = uid + } + } + if err := h.riskService.OnPoamItemCompleted(id, actorUserID); err != nil { + // Log but do not fail the POAM update — the item is already saved. + h.sugar.Warnw("failed to advance linked risk statuses on POAM completion", + "poamItemId", id, + "error", err, + ) + } + } + return c.JSON(http.StatusOK, GenericDataResponse[poamItemResponse]{Data: toPoamItemResponse(item)}) } diff --git a/internal/api/handler/risks.go b/internal/api/handler/risks.go index a628a61d..c13b76b1 100644 --- a/internal/api/handler/risks.go +++ b/internal/api/handler/risks.go @@ -34,9 +34,15 @@ const ( maxRiskDescriptionLength = 1000 ) -func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB, poamSvc *poamsvc.PoamService) *RiskHandler { +func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB, poamSvc *poamsvc.PoamService, riskSvc ...*riskrel.RiskService) *RiskHandler { + var rs *riskrel.RiskService + if len(riskSvc) > 0 && riskSvc[0] != nil { + rs = riskSvc[0] + } else { + rs = riskrel.NewRiskService(db) + } return &RiskHandler{ - riskService: riskrel.NewRiskService(db), + riskService: rs, poamService: poamSvc, sugar: sugar, pagination: svc.NewPaginationConfig(), @@ -1996,7 +2002,8 @@ func validateStatusTransition(oldStatus, newStatus string) error { string(riskrel.RiskStatusClosed): {}, }, string(riskrel.RiskStatusRiskAccepted): { - string(riskrel.RiskStatusClosed): {}, + string(riskrel.RiskStatusClosed): {}, + string(riskrel.RiskStatusMitigatingPlanned): {}, }, string(riskrel.RiskStatusRemediated): { string(riskrel.RiskStatusOpen): {}, @@ -2264,7 +2271,7 @@ type promoteToPoamRequest struct { // PromoteToPoam godoc // // @Summary Promote risk to POAM item -// @Description Promotes a risk-accepted risk to a POAM item. The risk must be in risk-accepted status. The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional. +// @Description Promotes an investigating risk to a POAM item and transitions the risk to mitigating-planned. The risk must be in investigating status (risk-accepted risks cannot be promoted — they have been formally accepted as tolerable). The POAM item is pre-populated from the risk's data and any RemediationTemplate tasks. The entire operation is transactional. // @Tags Risks // @Accept json // @Produce json @@ -2336,7 +2343,7 @@ func (h *RiskHandler) PromoteToPoam(ctx echo.Context) error { // PromoteToPoamForSSP godoc // // @Summary Promote risk to POAM item (SSP-scoped) -// @Description Promotes a risk-accepted risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in risk-accepted status. +// @Description Promotes an investigating risk to a POAM item, scoped to a specific SSP. The risk must belong to the given SSP and be in investigating status. On success, the risk transitions to mitigating-planned. // @Tags Risks // @Accept json // @Produce json diff --git a/internal/api/handler/risks_promote_to_poam_integration_test.go b/internal/api/handler/risks_promote_to_poam_integration_test.go index c8c18d75..d9f4f44e 100644 --- a/internal/api/handler/risks_promote_to_poam_integration_test.go +++ b/internal/api/handler/risks_promote_to_poam_integration_test.go @@ -18,10 +18,13 @@ import ( // BCH-1186: POST /risks/:id/promote-to-poam integration tests // --------------------------------------------------------------------------- +// TestPromoteToPoam_HappyPath verifies the full happy-path: a risk in +// investigating status is promoted to a POAM item, the POAM fields are +// populated correctly, and the risk status advances to mitigating-planned. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { sspID := suite.newSSPID() - // Create a risk in investigating status, then accept it. + // Create a risk directly in investigating status. created := suite.createRisk(map[string]any{ "title": "Unencrypted data at rest", "description": "Sensitive data stored without encryption", @@ -31,14 +34,6 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { "impact": "critical", }) - acceptDeadline := time.Now().Add(30 * 24 * time.Hour).UTC() - acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ - "justification": "accepted pending encryption rollout", - "review-deadline": acceptDeadline.Format(time.RFC3339), - }) - suite.server.E().ServeHTTP(acceptRec, acceptReq) - require.Equal(suite.T(), http.StatusOK, acceptRec.Code) - // Promote to POAM with full payload. deadline := time.Now().Add(90 * 24 * time.Hour).UTC().Truncate(time.Second) promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{ @@ -48,12 +43,12 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { "pocName": "Jane Smith", "pocEmail": "jane@example.com", "milestones": []map[string]any{ - {"title": "Identify all unencrypted data stores", "orderIndex": 0}, - {"title": "Apply AES-256 encryption to all stores", "orderIndex": 1}, + {"title": "Identify all unencrypted data stores", "orderIndex": 1}, + {"title": "Apply AES-256 encryption to all stores", "orderIndex": 2}, }, }) suite.server.E().ServeHTTP(promoteRec, promoteReq) - require.Equal(suite.T(), http.StatusCreated, promoteRec.Code, "promote-to-poam should return 201") + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code, "promote-to-poam should return 201: %s", promoteRec.Body.String()) var poamResp GenericDataResponse[poamItemResponse] require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) @@ -89,8 +84,18 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { Where("risk_id = ? AND event_type = ?", created.ID, string(riskrel.RiskEventTypePoamPromoted)). Count(&promotedEvents).Error) require.Equal(suite.T(), int64(1), promotedEvents) + + // Verify risk status was transitioned to mitigating-planned. + getRec, getReq := suite.authedRequest(http.MethodGet, fmt.Sprintf("/api/risks/%s", created.ID), nil) + suite.server.E().ServeHTTP(getRec, getReq) + require.Equal(suite.T(), http.StatusOK, getRec.Code) + var riskResp GenericDataResponse[riskResponse] + require.NoError(suite.T(), json.Unmarshal(getRec.Body.Bytes(), &riskResp)) + require.Equal(suite.T(), "mitigating-planned", riskResp.Data.Status, "risk status should be mitigating-planned after promotion") } +// TestPromoteToPoam_DefaultsFromRisk verifies that when no title/description +// are provided, the POAM item inherits them from the risk. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_DefaultsFromRisk() { sspID := suite.newSSPID() @@ -101,18 +106,10 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_DefaultsFromRisk() { "status": "investigating", }) - acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() - acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ - "justification": "accepted pending policy update", - "review-deadline": acceptDeadline.Format(time.RFC3339), - }) - suite.server.E().ServeHTTP(acceptRec, acceptReq) - require.Equal(suite.T(), http.StatusOK, acceptRec.Code) - // Promote with empty body — title and description should default from risk. promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) suite.server.E().ServeHTTP(promoteRec, promoteReq) - require.Equal(suite.T(), http.StatusCreated, promoteRec.Code) + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code, "promote-to-poam should return 201: %s", promoteRec.Body.String()) var poamResp GenericDataResponse[poamItemResponse] require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) @@ -124,34 +121,45 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_DefaultsFromRisk() { require.Equal(suite.T(), "risk-promotion", poamResp.Data.SourceType) } -func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNonAcceptedRisk() { +// TestPromoteToPoam_RejectsNonInvestigatingRisk verifies that only risks in +// investigating status can be promoted; open and risk-accepted are rejected. +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNonInvestigatingRisk() { sspID := suite.newSSPID() - // Risk in "open" status — not risk-accepted. - created := suite.createRisk(map[string]any{ + // Risk in "open" status — cannot be promoted. + openRisk := suite.createRisk(map[string]any{ "title": "Open risk", - "description": "Not yet accepted", + "description": "Not yet under investigation", "ssp-id": sspID, "status": "open", }) - promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", openRisk.ID), map[string]any{}) suite.server.E().ServeHTTP(promoteRec, promoteReq) - require.Equal(suite.T(), http.StatusUnprocessableEntity, promoteRec.Code, "should return 422 for non-accepted risk") + require.Equal(suite.T(), http.StatusUnprocessableEntity, promoteRec.Code, "should return 422 for open risk") - // Risk in "investigating" status — also not risk-accepted. - investigating := suite.createRisk(map[string]any{ - "title": "Investigating risk", - "description": "Still being investigated", + // Risk in "risk-accepted" status — accepted risks are not remediated. + acceptedRisk := suite.createRisk(map[string]any{ + "title": "Accepted risk", + "description": "Formally accepted, not being remediated", "ssp-id": sspID, "status": "investigating", }) + acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() + acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", acceptedRisk.ID), map[string]any{ + "justification": "accepted pending policy review", + "review-deadline": acceptDeadline.Format(time.RFC3339), + }) + suite.server.E().ServeHTTP(acceptRec, acceptReq) + require.Equal(suite.T(), http.StatusOK, acceptRec.Code) - promoteRec2, promoteReq2 := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", investigating.ID), map[string]any{}) + promoteRec2, promoteReq2 := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", acceptedRisk.ID), map[string]any{}) suite.server.E().ServeHTTP(promoteRec2, promoteReq2) - require.Equal(suite.T(), http.StatusUnprocessableEntity, promoteRec2.Code, "should return 422 for investigating risk") + require.Equal(suite.T(), http.StatusUnprocessableEntity, promoteRec2.Code, "should return 422 for risk-accepted risk") } +// TestPromoteToPoam_RejectsActivePoamAlreadyLinked verifies that a second +// promotion is rejected when an active (non-completed) POAM item already exists. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsActivePoamAlreadyLinked() { sspID := suite.newSSPID() @@ -162,25 +170,21 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsActivePoamAlready "status": "investigating", }) - acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() - acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ - "justification": "accepted", - "review-deadline": acceptDeadline.Format(time.RFC3339), - }) - suite.server.E().ServeHTTP(acceptRec, acceptReq) - require.Equal(suite.T(), http.StatusOK, acceptRec.Code) - // First promotion should succeed. firstRec, firstReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) suite.server.E().ServeHTTP(firstRec, firstReq) - require.Equal(suite.T(), http.StatusCreated, firstRec.Code) + require.Equal(suite.T(), http.StatusCreated, firstRec.Code, "first promotion should succeed: %s", firstRec.Body.String()) // Second promotion should fail — active POAM item already linked. + // Risk is now mitigating-planned, which is also not investigating, so we + // expect 422 from the status guard. secondRec, secondReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) suite.server.E().ServeHTTP(secondRec, secondReq) require.Equal(suite.T(), http.StatusUnprocessableEntity, secondRec.Code, "should return 422 when active POAM already linked") } +// TestPromoteToPoam_RejectsNotFound verifies that promoting a non-existent +// risk returns 404. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNotFound() { nonExistentID := uuid.New() promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", nonExistentID), map[string]any{}) @@ -188,6 +192,8 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNotFound() { require.Equal(suite.T(), http.StatusNotFound, promoteRec.Code) } +// TestPromoteToPoam_SSPScoped_HappyPath verifies the SSP-scoped endpoint +// returns 201 with correct data for a risk belonging to the given SSP. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { sspID := suite.newSSPID() @@ -198,14 +204,6 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { "status": "investigating", }) - acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() - acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ - "justification": "accepted for SSP-scoped test", - "review-deadline": acceptDeadline.Format(time.RFC3339), - }) - suite.server.E().ServeHTTP(acceptRec, acceptReq) - require.Equal(suite.T(), http.StatusOK, acceptRec.Code) - // Promote via SSP-scoped endpoint. sspPromoteRec, sspPromoteReq := suite.authedRequest( http.MethodPost, @@ -213,7 +211,7 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { map[string]any{"pocName": "SSP Owner"}, ) suite.server.E().ServeHTTP(sspPromoteRec, sspPromoteReq) - require.Equal(suite.T(), http.StatusCreated, sspPromoteRec.Code) + require.Equal(suite.T(), http.StatusCreated, sspPromoteRec.Code, "SSP-scoped promote should return 201: %s", sspPromoteRec.Body.String()) var poamResp GenericDataResponse[poamItemResponse] require.NoError(suite.T(), json.Unmarshal(sspPromoteRec.Body.Bytes(), &poamResp)) @@ -222,6 +220,8 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { require.Equal(suite.T(), "SSP Owner", *poamResp.Data.PocName) } +// TestPromoteToPoam_SSPScoped_RejectsWrongSSP verifies that promoting via a +// different SSP's scoped endpoint returns 404. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_RejectsWrongSSP() { sspID := suite.newSSPID() wrongSSPID := suite.newSSPID() @@ -233,14 +233,6 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_RejectsWrongSS "status": "investigating", }) - acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() - acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ - "justification": "accepted", - "review-deadline": acceptDeadline.Format(time.RFC3339), - }) - suite.server.E().ServeHTTP(acceptRec, acceptReq) - require.Equal(suite.T(), http.StatusOK, acceptRec.Code) - // Attempt to promote via wrong SSP — should return 404. wrongSSPRec, wrongSSPReq := suite.authedRequest( http.MethodPost, @@ -251,6 +243,9 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_RejectsWrongSS require.Equal(suite.T(), http.StatusNotFound, wrongSSPRec.Code) } +// TestPromoteToPoam_WithRemediationTemplate verifies that milestones from the +// risk's RemediationTemplate are copied first, followed by any extra milestones +// from the request body. func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_WithRemediationTemplate() { sspID := suite.newSSPID() @@ -262,7 +257,7 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_WithRemediationTemplate( }) // Create a remediation template with 2 tasks. - // Note: the remediationTaskRequest uses "order-index" (kebab-case) as its JSON tag. + // Note: remediationTaskRequest uses "order-index" (kebab-case) as its JSON tag. remRec, remReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/remediation-template", created.ID), map[string]any{ "title": "Standard remediation plan", "tasks": []map[string]any{ @@ -271,25 +266,16 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_WithRemediationTemplate( }, }) suite.server.E().ServeHTTP(remRec, remReq) - require.Equal(suite.T(), http.StatusCreated, remRec.Code) - - // Accept the risk. - acceptDeadline := time.Now().Add(14 * 24 * time.Hour).UTC() - acceptRec, acceptReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/accept", created.ID), map[string]any{ - "justification": "accepted", - "review-deadline": acceptDeadline.Format(time.RFC3339), - }) - suite.server.E().ServeHTTP(acceptRec, acceptReq) - require.Equal(suite.T(), http.StatusOK, acceptRec.Code) + require.Equal(suite.T(), http.StatusCreated, remRec.Code, "remediation template creation should return 201: %s", remRec.Body.String()) // Promote with 1 extra milestone — should have 3 total (2 template + 1 extra). promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{ "milestones": []map[string]any{ - {"title": "Extra milestone from request", "orderIndex": 2}, + {"title": "Extra milestone from request", "orderIndex": 3}, }, }) suite.server.E().ServeHTTP(promoteRec, promoteReq) - require.Equal(suite.T(), http.StatusCreated, promoteRec.Code) + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code, "promote-to-poam should return 201: %s", promoteRec.Body.String()) var poamResp GenericDataResponse[poamItemResponse] require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) @@ -301,5 +287,58 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_WithRemediationTemplate( require.Equal(suite.T(), "Extra milestone from request", poamResp.Data.Milestones[2].Title) } +// TestPromoteToPoam_CompletionAdvancesRiskStatus verifies the full lifecycle: +// promote (investigating → mitigating-planned), then complete the POAM item +// (mitigating-planned → mitigating-implemented). +func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_CompletionAdvancesRiskStatus() { + sspID := suite.newSSPID() + + created := suite.createRisk(map[string]any{ + "title": "Lifecycle risk", + "description": "Testing full POAM lifecycle", + "ssp-id": sspID, + "status": "investigating", + }) + + // Promote to POAM. + promoteRec, promoteReq := suite.authedRequest(http.MethodPost, fmt.Sprintf("/api/risks/%s/promote-to-poam", created.ID), map[string]any{}) + suite.server.E().ServeHTTP(promoteRec, promoteReq) + require.Equal(suite.T(), http.StatusCreated, promoteRec.Code, "promote should succeed: %s", promoteRec.Body.String()) + + var poamResp GenericDataResponse[poamItemResponse] + require.NoError(suite.T(), json.Unmarshal(promoteRec.Body.Bytes(), &poamResp)) + poamID := poamResp.Data.ID + + // Verify risk is now mitigating-planned. + getRec, getReq := suite.authedRequest(http.MethodGet, fmt.Sprintf("/api/risks/%s", created.ID), nil) + suite.server.E().ServeHTTP(getRec, getReq) + require.Equal(suite.T(), http.StatusOK, getRec.Code) + var riskResp GenericDataResponse[riskResponse] + require.NoError(suite.T(), json.Unmarshal(getRec.Body.Bytes(), &riskResp)) + require.Equal(suite.T(), "mitigating-planned", riskResp.Data.Status) + + // Complete the POAM item. + completeRec, completeReq := suite.authedRequest(http.MethodPut, fmt.Sprintf("/api/poam-items/%s", poamID), map[string]any{ + "status": "completed", + }) + suite.server.E().ServeHTTP(completeRec, completeReq) + require.Equal(suite.T(), http.StatusOK, completeRec.Code, "POAM completion should succeed: %s", completeRec.Body.String()) + + // Verify risk has advanced to mitigating-implemented. + getRec2, getReq2 := suite.authedRequest(http.MethodGet, fmt.Sprintf("/api/risks/%s", created.ID), nil) + suite.server.E().ServeHTTP(getRec2, getReq2) + require.Equal(suite.T(), http.StatusOK, getRec2.Code) + var riskResp2 GenericDataResponse[riskResponse] + require.NoError(suite.T(), json.Unmarshal(getRec2.Body.Bytes(), &riskResp2)) + require.Equal(suite.T(), "mitigating-implemented", riskResp2.Data.Status, "risk should advance to mitigating-implemented after POAM completion") + + // Verify poam_completed event was emitted on the risk. + var completedEvents int64 + require.NoError(suite.T(), suite.DB.Model(&riskrel.RiskEvent{}). + Where("risk_id = ? AND event_type = ?", created.ID, string(riskrel.RiskEventTypePoamCompleted)). + Count(&completedEvents).Error) + require.Equal(suite.T(), int64(1), completedEvents) +} + // Ensure the testing import is used. var _ = (*testing.T)(nil) diff --git a/internal/service/relational/risks/events.go b/internal/service/relational/risks/events.go index 9e9a4d9c..b5dfd981 100644 --- a/internal/service/relational/risks/events.go +++ b/internal/service/relational/risks/events.go @@ -35,6 +35,7 @@ const ( RiskEventTypeRemediationDeleted RiskEventType = "remediation_deleted" RiskEventTypeEvidenceRecovered RiskEventType = "evidence_recovered" RiskEventTypePoamPromoted RiskEventType = "poam_promoted" + RiskEventTypePoamCompleted RiskEventType = "poam_completed" ) type RiskEvent struct { @@ -190,9 +191,14 @@ func BuildRiskEventDetails(eventType string, payload datatypes.JSONMap, occurred return "Evidence recovered; risk is accepted so no automatic status change was applied." case string(RiskEventTypePoamPromoted): if poamItemID := payloadString(payload, "poamItemId"); poamItemID != "" { - return fmt.Sprintf("Risk was promoted to POAM item %s.", poamItemID) + return fmt.Sprintf("Risk was promoted to POAM item %s; status transitioned to mitigating-planned.", poamItemID) } - return "Risk was promoted to a POAM item." + return "Risk was promoted to a POAM item; status transitioned to mitigating-planned." + case string(RiskEventTypePoamCompleted): + if poamItemID := payloadString(payload, "poamItemId"); poamItemID != "" { + return fmt.Sprintf("Linked POAM item %s was completed; risk status advanced to mitigating-implemented.", poamItemID) + } + return "A linked POAM item was completed; risk status advanced to mitigating-implemented." default: return fmt.Sprintf("Risk event recorded: %s.", eventType) } diff --git a/internal/service/relational/risks/poam_completion.go b/internal/service/relational/risks/poam_completion.go new file mode 100644 index 00000000..51dd081a --- /dev/null +++ b/internal/service/relational/risks/poam_completion.go @@ -0,0 +1,93 @@ +package risks + +import ( + "github.com/google/uuid" + "gorm.io/datatypes" + "gorm.io/gorm/clause" +) + +// OnPoamItemCompleted is called by the POAM handler when a POAM item +// transitions to the "completed" status. It advances every linked risk that is +// currently in mitigating-planned status to mitigating-implemented, emitting a +// status_changed event and a poam_completed event for each one. +// +// Only risks in mitigating-planned are advanced; risks in any other status are +// left untouched (they may have been manually moved or re-accepted). +func (s *RiskService) OnPoamItemCompleted(poamItemID uuid.UUID, actorUserID *uuid.UUID) error { + // Find all risk IDs linked to this POAM item. + type linkRow struct { + RiskID uuid.UUID + } + var links []linkRow + if err := s.db.Raw(` + SELECT risk_id FROM ccf_poam_item_risk_links WHERE poam_item_id = ? + `, poamItemID).Scan(&links).Error; err != nil { + return err + } + if len(links) == 0 { + return nil + } + + for _, link := range links { + if err := s.advanceRiskToMitigatingImplemented(link.RiskID, poamItemID, actorUserID); err != nil { + return err + } + } + return nil +} + +// advanceRiskToMitigatingImplemented transitions a single risk from +// mitigating-planned → mitigating-implemented inside its own transaction. +// If the risk is not in mitigating-planned, it is silently skipped. +func (s *RiskService) advanceRiskToMitigatingImplemented(riskID, poamItemID uuid.UUID, actorUserID *uuid.UUID) error { + tx, err := beginTx(s.db) + if err != nil { + return err + } + defer rollbackTxOnPanic(tx) + + var risk Risk + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). + First(&risk, "id = ?", riskID).Error; err != nil { + tx.Rollback() + return err + } + + // Only advance risks that are in mitigating-planned. + if risk.Status != string(RiskStatusMitigatingPlanned) { + tx.Rollback() + return nil + } + + oldStatus := risk.Status + risk.Status = string(RiskStatusMitigatingImplemented) + if err := tx.Save(&risk).Error; err != nil { + tx.Rollback() + return err + } + + riskSnapshot, err := s.getRiskSnapshot(tx, riskID) + if err != nil { + tx.Rollback() + return err + } + + // Emit status_changed event. + if err := s.logRiskEventWithSnapshot(tx, riskID, RiskEventTypeStatusChange, actorUserID, datatypes.JSONMap{ + "from": oldStatus, + "to": risk.Status, + }, riskSnapshot); err != nil { + tx.Rollback() + return err + } + + // Emit poam_completed event. + if err := s.logRiskEventWithSnapshot(tx, riskID, RiskEventTypePoamCompleted, actorUserID, datatypes.JSONMap{ + "poamItemId": poamItemID.String(), + }, riskSnapshot); err != nil { + tx.Rollback() + return err + } + + return tx.Commit().Error +} diff --git a/internal/service/relational/risks/promote_to_poam.go b/internal/service/relational/risks/promote_to_poam.go index 6d5f4f8c..7349b28d 100644 --- a/internal/service/relational/risks/promote_to_poam.go +++ b/internal/service/relational/risks/promote_to_poam.go @@ -11,11 +11,11 @@ import ( "gorm.io/gorm/clause" ) -// PromoteToPoamParams carries all inputs required to promote a risk-accepted +// PromoteToPoamParams carries all inputs required to promote an investigating // risk to a POAM item. type PromoteToPoamParams struct { // RiskID is the UUID of the risk to promote. The risk must be in - // risk-accepted status; any other status returns a 422 ValidationError. + // investigating status; any other status returns a 422 ValidationError. RiskID uuid.UUID // ActorUserID is the authenticated user performing the promotion. ActorUserID *uuid.UUID @@ -36,8 +36,9 @@ type PromoteToPoamParams struct { ExtraMilestones []poamsvc.CreateMilestoneParams } -// PromoteToPoam promotes a risk-accepted risk to a POAM item. The entire -// operation — POAM item creation, milestone creation, risk link creation, and +// PromoteToPoam promotes an investigating risk to a POAM item and transitions +// the risk status to mitigating-planned. The entire operation — POAM item +// creation, milestone creation, risk link creation, risk status transition, and // risk event emission — is executed inside a single database transaction so // that no partial state is persisted on failure. // @@ -63,10 +64,13 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote return nil, err } - // 2. Guard: risk must be in risk-accepted status. - if risk.Status != string(RiskStatusRiskAccepted) { + // 2. Guard: risk must be in investigating status. + // A risk that is accepted (risk-accepted) has been formally accepted as + // tolerable — it should not receive a remediation plan. POAM promotion + // is only valid while the risk is actively being investigated. + if risk.Status != string(RiskStatusInvestigating) { tx.Rollback() - return nil, newValidationError("only risks in status risk-accepted can be promoted to a POAM item") + return nil, newValidationError("only risks in status investigating can be promoted to a POAM item") } // 3. Re-promotion guard: reject if an active (non-completed) POAM item @@ -152,12 +156,28 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote return nil, err } - // 9. Emit the risk event. + // 9. Transition the risk status: investigating → mitigating-planned. + // This records that a formal remediation plan now exists for the risk. + oldStatus := risk.Status + risk.Status = string(RiskStatusMitigatingPlanned) + if err := tx.Save(&risk).Error; err != nil { + tx.Rollback() + return nil, err + } + + // 10. Emit risk events: status_changed + poam_promoted. riskSnapshot, err := s.getRiskSnapshot(tx, params.RiskID) if err != nil { tx.Rollback() return nil, err } + if err := s.logRiskEventWithSnapshot(tx, params.RiskID, RiskEventTypeStatusChange, params.ActorUserID, datatypes.JSONMap{ + "from": oldStatus, + "to": risk.Status, + }, riskSnapshot); err != nil { + tx.Rollback() + return nil, err + } if err := s.logRiskEventWithSnapshot(tx, params.RiskID, RiskEventTypePoamPromoted, params.ActorUserID, datatypes.JSONMap{ "poamItemId": poamItem.ID.String(), }, riskSnapshot); err != nil { @@ -165,11 +185,11 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote return nil, err } - // 10. Commit the transaction. + // 11. Commit the transaction. if err := tx.Commit().Error; err != nil { return nil, err } - // 11. Return the fully-loaded POAM item (with milestones and links). + // 12. Return the fully-loaded POAM item (with milestones and links). return poamSvc.GetByID(poamItem.ID) } From e54081cb0fd7ce32df5fe3fc49a2a1890ca7b583 Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Wed, 25 Mar 2026 06:57:39 -0400 Subject: [PATCH 4/7] 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 --- docs/docs.go | 22 +++-------------- docs/swagger.json | 22 +++-------------- docs/swagger.yaml | 20 +++++----------- internal/api/handler/poam_items.go | 12 ++-------- internal/api/handler/risks.go | 24 +++++++++---------- .../risks_promote_to_poam_integration_test.go | 14 ++++------- internal/service/relational/poam/models.go | 8 +++---- internal/service/relational/poam/service.go | 8 +------ .../relational/risks/promote_to_poam.go | 24 +++++++++++++------ 9 files changed, 52 insertions(+), 102 deletions(-) diff --git a/docs/docs.go b/docs/docs.go index 1293670c..91d02388 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -28319,12 +28319,6 @@ const docTemplate = `{ "plannedCompletionDate": { "type": "string" }, - "pocEmail": { - "type": "string" - }, - "pocName": { - "type": "string" - }, "primaryOwnerUserId": { "type": "string" }, @@ -28539,12 +28533,6 @@ const docTemplate = `{ "plannedCompletionDate": { "type": "string" }, - "pocEmail": { - "type": "string" - }, - "pocName": { - "type": "string" - }, "primaryOwnerUserId": { "type": "string" }, @@ -28588,16 +28576,12 @@ const docTemplate = `{ "$ref": "#/definitions/handler.createMilestoneRequest" } }, - "pocEmail": { - "description": "PocEmail is the point-of-contact email.", - "type": "string" - }, - "pocName": { - "description": "PocName is the point-of-contact name.", + "primaryOwnerUserId": { + "description": "PrimaryOwnerUserID optionally overrides the POAM item owner.\nIf omitted, the risk's own PrimaryOwnerUserID is inherited automatically.", "type": "string" }, "resourceRequired": { - "description": "ResourceRequired is a free-text description of resources needed.", + "description": "ResourceRequired is a free-text planning field describing effort or budget needed.", "type": "string" }, "title": { diff --git a/docs/swagger.json b/docs/swagger.json index 24e01df2..2bb80169 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -28313,12 +28313,6 @@ "plannedCompletionDate": { "type": "string" }, - "pocEmail": { - "type": "string" - }, - "pocName": { - "type": "string" - }, "primaryOwnerUserId": { "type": "string" }, @@ -28533,12 +28527,6 @@ "plannedCompletionDate": { "type": "string" }, - "pocEmail": { - "type": "string" - }, - "pocName": { - "type": "string" - }, "primaryOwnerUserId": { "type": "string" }, @@ -28582,16 +28570,12 @@ "$ref": "#/definitions/handler.createMilestoneRequest" } }, - "pocEmail": { - "description": "PocEmail is the point-of-contact email.", - "type": "string" - }, - "pocName": { - "description": "PocName is the point-of-contact name.", + "primaryOwnerUserId": { + "description": "PrimaryOwnerUserID optionally overrides the POAM item owner.\nIf omitted, the risk's own PrimaryOwnerUserID is inherited automatically.", "type": "string" }, "resourceRequired": { - "description": "ResourceRequired is a free-text description of resources needed.", + "description": "ResourceRequired is a free-text planning field describing effort or budget needed.", "type": "string" }, "title": { diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 365ebded..3eb2729f 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1609,10 +1609,6 @@ definitions: type: array plannedCompletionDate: type: string - pocEmail: - type: string - pocName: - type: string primaryOwnerUserId: type: string resourceRequired: @@ -1757,10 +1753,6 @@ definitions: type: array plannedCompletionDate: type: string - pocEmail: - type: string - pocName: - type: string primaryOwnerUserId: type: string resourceRequired: @@ -1792,14 +1784,14 @@ definitions: items: $ref: '#/definitions/handler.createMilestoneRequest' type: array - pocEmail: - description: PocEmail is the point-of-contact email. - type: string - pocName: - description: PocName is the point-of-contact name. + primaryOwnerUserId: + description: |- + PrimaryOwnerUserID optionally overrides the POAM item owner. + If omitted, the risk's own PrimaryOwnerUserID is inherited automatically. type: string resourceRequired: - description: ResourceRequired is a free-text description of resources needed. + description: ResourceRequired is a free-text planning field describing effort + or budget needed. type: string title: description: |- diff --git a/internal/api/handler/poam_items.go b/internal/api/handler/poam_items.go index 6f61a3a3..17df91d1 100644 --- a/internal/api/handler/poam_items.go +++ b/internal/api/handler/poam_items.go @@ -86,9 +86,7 @@ type createPoamItemRequest struct { PlannedCompletionDate *time.Time `json:"plannedCompletionDate"` CreatedFromRiskID *string `json:"createdFromRiskId"` AcceptanceRationale *string `json:"acceptanceRationale"` - PocName *string `json:"pocName"` - PocEmail *string `json:"pocEmail"` - ResourceRequired *string `json:"resourceRequired"` + ResourceRequired *string `json:"resourceRequired"` RiskIDs []string `json:"riskIds"` EvidenceIDs []string `json:"evidenceIds"` ControlRefs []poamControlRefRequest `json:"controlRefs"` @@ -184,9 +182,7 @@ type poamItemResponse struct { CompletedAt *time.Time `json:"completedAt,omitempty"` CreatedFromRiskID *uuid.UUID `json:"createdFromRiskId,omitempty"` AcceptanceRationale *string `json:"acceptanceRationale,omitempty"` - PocName *string `json:"pocName,omitempty"` - PocEmail *string `json:"pocEmail,omitempty"` - ResourceRequired *string `json:"resourceRequired,omitempty"` + ResourceRequired *string `json:"resourceRequired,omitempty"` LastStatusChangeAt time.Time `json:"lastStatusChangeAt"` CreatedAt time.Time `json:"createdAt"` UpdatedAt time.Time `json:"updatedAt"` @@ -225,8 +221,6 @@ func toPoamItemResponse(item *poamsvc.PoamItem) poamItemResponse { CompletedAt: item.CompletedAt, CreatedFromRiskID: item.CreatedFromRiskID, AcceptanceRationale: item.AcceptanceRationale, - PocName: item.PocName, - PocEmail: item.PocEmail, ResourceRequired: item.ResourceRequired, LastStatusChangeAt: item.LastStatusChangeAt, CreatedAt: item.CreatedAt, @@ -381,8 +375,6 @@ func (h *PoamItemsHandler) Create(c echo.Context) error { SourceType: in.SourceType, PlannedCompletionDate: in.PlannedCompletionDate, AcceptanceRationale: in.AcceptanceRationale, - PocName: in.PocName, - PocEmail: in.PocEmail, ResourceRequired: in.ResourceRequired, } diff --git a/internal/api/handler/risks.go b/internal/api/handler/risks.go index c13b76b1..a258a334 100644 --- a/internal/api/handler/risks.go +++ b/internal/api/handler/risks.go @@ -2257,12 +2257,11 @@ type promoteToPoamRequest struct { Title *string `json:"title"` // Deadline maps to PoamItem.PlannedCompletionDate. Deadline *time.Time `json:"deadline"` - // ResourceRequired is a free-text description of resources needed. + // ResourceRequired is a free-text planning field describing effort or budget needed. ResourceRequired *string `json:"resourceRequired"` - // PocName is the point-of-contact name. - PocName *string `json:"pocName"` - // PocEmail is the point-of-contact email. - PocEmail *string `json:"pocEmail"` + // PrimaryOwnerUserID optionally overrides the POAM item owner. + // If omitted, the risk's own PrimaryOwnerUserID is inherited automatically. + PrimaryOwnerUserID *uuid.UUID `json:"primaryOwnerUserId"` // Milestones are additional milestones to append after any copied from the // risk's RemediationTemplate. Milestones []createMilestoneRequest `json:"milestones"` @@ -2317,14 +2316,13 @@ func (h *RiskHandler) PromoteToPoam(ctx echo.Context) error { return h.withActorUserID(ctx, func(actorID *uuid.UUID) error { poamItem, err := h.riskService.PromoteToPoam(h.poamService, riskrel.PromoteToPoamParams{ - RiskID: riskID, - ActorUserID: actorID, - Title: req.Title, - Deadline: req.Deadline, - ResourceRequired: req.ResourceRequired, - PocName: req.PocName, - PocEmail: req.PocEmail, - ExtraMilestones: milestones, + RiskID: riskID, + ActorUserID: actorID, + Title: req.Title, + Deadline: req.Deadline, + ResourceRequired: req.ResourceRequired, + PrimaryOwnerUserID: req.PrimaryOwnerUserID, + ExtraMilestones: milestones, }) if err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { diff --git a/internal/api/handler/risks_promote_to_poam_integration_test.go b/internal/api/handler/risks_promote_to_poam_integration_test.go index d9f4f44e..d6b8a491 100644 --- a/internal/api/handler/risks_promote_to_poam_integration_test.go +++ b/internal/api/handler/risks_promote_to_poam_integration_test.go @@ -40,8 +40,6 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { "title": "Encrypt data at rest — remediation plan", "deadline": deadline.Format(time.RFC3339), "resourceRequired": "3 engineer days", - "pocName": "Jane Smith", - "pocEmail": "jane@example.com", "milestones": []map[string]any{ {"title": "Identify all unencrypted data stores", "orderIndex": 1}, {"title": "Apply AES-256 encryption to all stores", "orderIndex": 2}, @@ -62,10 +60,8 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { require.Equal(suite.T(), created.ID.String(), poamResp.Data.CreatedFromRiskID.String()) require.NotNil(suite.T(), poamResp.Data.PlannedCompletionDate) require.WithinDuration(suite.T(), deadline, *poamResp.Data.PlannedCompletionDate, time.Second) - require.NotNil(suite.T(), poamResp.Data.PocName) - require.Equal(suite.T(), "Jane Smith", *poamResp.Data.PocName) - require.NotNil(suite.T(), poamResp.Data.PocEmail) - require.Equal(suite.T(), "jane@example.com", *poamResp.Data.PocEmail) + // PrimaryOwnerUserID should be inherited from the risk's owner (the authenticated actor). + require.NotNil(suite.T(), poamResp.Data.PrimaryOwnerUserID) require.NotNil(suite.T(), poamResp.Data.ResourceRequired) require.Equal(suite.T(), "3 engineer days", *poamResp.Data.ResourceRequired) @@ -208,7 +204,7 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { sspPromoteRec, sspPromoteReq := suite.authedRequest( http.MethodPost, fmt.Sprintf("/api/oscal/system-security-plans/%s/risks/%s/promote-to-poam", sspID, created.ID), - map[string]any{"pocName": "SSP Owner"}, + map[string]any{}, ) suite.server.E().ServeHTTP(sspPromoteRec, sspPromoteReq) require.Equal(suite.T(), http.StatusCreated, sspPromoteRec.Code, "SSP-scoped promote should return 201: %s", sspPromoteRec.Body.String()) @@ -216,8 +212,8 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { var poamResp GenericDataResponse[poamItemResponse] require.NoError(suite.T(), json.Unmarshal(sspPromoteRec.Body.Bytes(), &poamResp)) require.Equal(suite.T(), "SSP-scoped promotion risk", poamResp.Data.Title) - require.NotNil(suite.T(), poamResp.Data.PocName) - require.Equal(suite.T(), "SSP Owner", *poamResp.Data.PocName) + // PrimaryOwnerUserID should be inherited from the risk's owner. + require.NotNil(suite.T(), poamResp.Data.PrimaryOwnerUserID) } // TestPromoteToPoam_SSPScoped_RejectsWrongSSP verifies that promoting via a diff --git a/internal/service/relational/poam/models.go b/internal/service/relational/poam/models.go index bc8a810a..b591efff 100644 --- a/internal/service/relational/poam/models.go +++ b/internal/service/relational/poam/models.go @@ -78,10 +78,10 @@ type PoamItem struct { CompletedAt *time.Time ` json:"completedAt,omitempty"` CreatedFromRiskID *uuid.UUID `gorm:"type:uuid" json:"createdFromRiskId,omitempty"` AcceptanceRationale *string ` json:"acceptanceRationale,omitempty"` - // Point-of-contact and resource fields — populated on risk-promotion or manual entry. - PocName *string `gorm:"type:text" json:"pocName,omitempty"` - PocEmail *string `gorm:"type:text" json:"pocEmail,omitempty"` - ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` + // ResourceRequired is a free-text planning field describing effort or budget needed. + // Point-of-contact identity is expressed via PrimaryOwnerUserID (a FK to the users table) + // rather than free-text poc_name/poc_email fields. + ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` LastStatusChangeAt time.Time `gorm:"not null" json:"lastStatusChangeAt"` CreatedAt time.Time ` json:"createdAt"` UpdatedAt time.Time ` json:"updatedAt"` diff --git a/internal/service/relational/poam/service.go b/internal/service/relational/poam/service.go index 321369c0..d2066ba6 100644 --- a/internal/service/relational/poam/service.go +++ b/internal/service/relational/poam/service.go @@ -38,9 +38,7 @@ type CreatePoamItemParams struct { PlannedCompletionDate *time.Time CreatedFromRiskID *uuid.UUID AcceptanceRationale *string - PocName *string - PocEmail *string - ResourceRequired *string + ResourceRequired *string RiskIDs []uuid.UUID EvidenceIDs []uuid.UUID ControlRefs []ControlRef @@ -119,8 +117,6 @@ func (s *PoamService) Create(params CreatePoamItemParams) (*PoamItem, error) { PlannedCompletionDate: params.PlannedCompletionDate, CreatedFromRiskID: params.CreatedFromRiskID, AcceptanceRationale: params.AcceptanceRationale, - PocName: params.PocName, - PocEmail: params.PocEmail, ResourceRequired: params.ResourceRequired, } @@ -710,8 +706,6 @@ func (s *PoamService) CreateWithTx(tx *gorm.DB, params CreatePoamItemParams) (*P PlannedCompletionDate: params.PlannedCompletionDate, CreatedFromRiskID: params.CreatedFromRiskID, AcceptanceRationale: params.AcceptanceRationale, - PocName: params.PocName, - PocEmail: params.PocEmail, ResourceRequired: params.ResourceRequired, } diff --git a/internal/service/relational/risks/promote_to_poam.go b/internal/service/relational/risks/promote_to_poam.go index 7349b28d..f5c75d3b 100644 --- a/internal/service/relational/risks/promote_to_poam.go +++ b/internal/service/relational/risks/promote_to_poam.go @@ -24,12 +24,11 @@ type PromoteToPoamParams struct { Title *string // Deadline maps to PoamItem.PlannedCompletionDate. Deadline *time.Time - // ResourceRequired is a free-text field describing resources needed. + // ResourceRequired is a free-text planning field describing effort or budget needed. ResourceRequired *string - // PocName is the point-of-contact name. - PocName *string - // PocEmail is the point-of-contact email. - PocEmail *string + // PrimaryOwnerUserID optionally overrides the POAM item owner. + // If nil, the risk's own PrimaryOwnerUserID is inherited automatically. + PrimaryOwnerUserID *uuid.UUID // ExtraMilestones are additional milestones supplied in the request body. // They are appended after any milestones copied from the risk's // RemediationTemplate, with order_index offset accordingly. @@ -140,10 +139,9 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote Description: risk.Description, Status: string(poamsvc.PoamItemStatusOpen), SourceType: string(poamsvc.PoamItemSourceTypeRiskPromotion), + PrimaryOwnerUserID: coalesceUUID(params.PrimaryOwnerUserID, risk.PrimaryOwnerUserID), PlannedCompletionDate: params.Deadline, CreatedFromRiskID: &riskID, - PocName: params.PocName, - PocEmail: params.PocEmail, ResourceRequired: params.ResourceRequired, RiskIDs: []uuid.UUID{params.RiskID}, Milestones: templateMilestones, @@ -193,3 +191,15 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote // 12. Return the fully-loaded POAM item (with milestones and links). return poamSvc.GetByID(poamItem.ID) } + +// coalesceUUID returns the first non-nil UUID pointer from the provided +// arguments, mirroring the SQL COALESCE semantics. Used to allow an optional +// caller-supplied override to fall back to a model-derived default. +func coalesceUUID(vals ...*uuid.UUID) *uuid.UUID { + for _, v := range vals { + if v != nil { + return v + } + } + return nil +} From c61da239ad411614ce3e2abf3190bf0af21706c1 Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Wed, 25 Mar 2026 11:02:53 -0400 Subject: [PATCH 5/7] =?UTF-8?q?fix(poam):=20BCH-1186=20CI=20fixes=20?= =?UTF-8?q?=E2=80=94=20swag=20fmt=20formatting=20and=20test=20owner=20inhe?= =?UTF-8?q?ritance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- internal/api/handler/poam_items.go | 4 +-- .../risks_promote_to_poam_integration_test.go | 36 ++++++++++++------- internal/service/relational/poam/models.go | 2 +- internal/service/relational/poam/service.go | 2 +- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/internal/api/handler/poam_items.go b/internal/api/handler/poam_items.go index 17df91d1..589f39c6 100644 --- a/internal/api/handler/poam_items.go +++ b/internal/api/handler/poam_items.go @@ -86,7 +86,7 @@ type createPoamItemRequest struct { PlannedCompletionDate *time.Time `json:"plannedCompletionDate"` CreatedFromRiskID *string `json:"createdFromRiskId"` AcceptanceRationale *string `json:"acceptanceRationale"` - ResourceRequired *string `json:"resourceRequired"` + ResourceRequired *string `json:"resourceRequired"` RiskIDs []string `json:"riskIds"` EvidenceIDs []string `json:"evidenceIds"` ControlRefs []poamControlRefRequest `json:"controlRefs"` @@ -182,7 +182,7 @@ type poamItemResponse struct { CompletedAt *time.Time `json:"completedAt,omitempty"` CreatedFromRiskID *uuid.UUID `json:"createdFromRiskId,omitempty"` AcceptanceRationale *string `json:"acceptanceRationale,omitempty"` - ResourceRequired *string `json:"resourceRequired,omitempty"` + ResourceRequired *string `json:"resourceRequired,omitempty"` LastStatusChangeAt time.Time `json:"lastStatusChangeAt"` CreatedAt time.Time `json:"createdAt"` UpdatedAt time.Time `json:"updatedAt"` diff --git a/internal/api/handler/risks_promote_to_poam_integration_test.go b/internal/api/handler/risks_promote_to_poam_integration_test.go index d6b8a491..da607449 100644 --- a/internal/api/handler/risks_promote_to_poam_integration_test.go +++ b/internal/api/handler/risks_promote_to_poam_integration_test.go @@ -24,14 +24,18 @@ import ( func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { sspID := suite.newSSPID() - // Create a risk directly in investigating status. + // Use a deterministic owner UUID so we can assert inheritance. + ownerID := uuid.New() + + // Create a risk directly in investigating status with an explicit owner. created := suite.createRisk(map[string]any{ - "title": "Unencrypted data at rest", - "description": "Sensitive data stored without encryption", - "ssp-id": sspID, - "status": "investigating", - "likelihood": "high", - "impact": "critical", + "title": "Unencrypted data at rest", + "description": "Sensitive data stored without encryption", + "ssp-id": sspID, + "status": "investigating", + "likelihood": "high", + "impact": "critical", + "primary-owner-user-id": ownerID.String(), }) // Promote to POAM with full payload. @@ -60,8 +64,9 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_HappyPath() { require.Equal(suite.T(), created.ID.String(), poamResp.Data.CreatedFromRiskID.String()) require.NotNil(suite.T(), poamResp.Data.PlannedCompletionDate) require.WithinDuration(suite.T(), deadline, *poamResp.Data.PlannedCompletionDate, time.Second) - // PrimaryOwnerUserID should be inherited from the risk's owner (the authenticated actor). + // PrimaryOwnerUserID should be inherited from the risk's explicit owner. require.NotNil(suite.T(), poamResp.Data.PrimaryOwnerUserID) + require.Equal(suite.T(), ownerID, *poamResp.Data.PrimaryOwnerUserID) require.NotNil(suite.T(), poamResp.Data.ResourceRequired) require.Equal(suite.T(), "3 engineer days", *poamResp.Data.ResourceRequired) @@ -193,11 +198,15 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_RejectsNotFound() { func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { sspID := suite.newSSPID() + // Use a deterministic owner UUID so we can assert inheritance. + sspOwnerID := uuid.New() + created := suite.createRisk(map[string]any{ - "title": "SSP-scoped promotion risk", - "description": "Testing SSP-scoped promote endpoint", - "ssp-id": sspID, - "status": "investigating", + "title": "SSP-scoped promotion risk", + "description": "Testing SSP-scoped promote endpoint", + "ssp-id": sspID, + "status": "investigating", + "primary-owner-user-id": sspOwnerID.String(), }) // Promote via SSP-scoped endpoint. @@ -212,8 +221,9 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_SSPScoped_HappyPath() { var poamResp GenericDataResponse[poamItemResponse] require.NoError(suite.T(), json.Unmarshal(sspPromoteRec.Body.Bytes(), &poamResp)) require.Equal(suite.T(), "SSP-scoped promotion risk", poamResp.Data.Title) - // PrimaryOwnerUserID should be inherited from the risk's owner. + // PrimaryOwnerUserID should be inherited from the risk's explicit owner. require.NotNil(suite.T(), poamResp.Data.PrimaryOwnerUserID) + require.Equal(suite.T(), sspOwnerID, *poamResp.Data.PrimaryOwnerUserID) } // TestPromoteToPoam_SSPScoped_RejectsWrongSSP verifies that promoting via a diff --git a/internal/service/relational/poam/models.go b/internal/service/relational/poam/models.go index b591efff..4ee2770a 100644 --- a/internal/service/relational/poam/models.go +++ b/internal/service/relational/poam/models.go @@ -81,7 +81,7 @@ type PoamItem struct { // ResourceRequired is a free-text planning field describing effort or budget needed. // Point-of-contact identity is expressed via PrimaryOwnerUserID (a FK to the users table) // rather than free-text poc_name/poc_email fields. - ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` + ResourceRequired *string `gorm:"type:text" json:"resourceRequired,omitempty"` LastStatusChangeAt time.Time `gorm:"not null" json:"lastStatusChangeAt"` CreatedAt time.Time ` json:"createdAt"` UpdatedAt time.Time ` json:"updatedAt"` diff --git a/internal/service/relational/poam/service.go b/internal/service/relational/poam/service.go index d2066ba6..7150180b 100644 --- a/internal/service/relational/poam/service.go +++ b/internal/service/relational/poam/service.go @@ -38,7 +38,7 @@ type CreatePoamItemParams struct { PlannedCompletionDate *time.Time CreatedFromRiskID *uuid.UUID AcceptanceRationale *string - ResourceRequired *string + ResourceRequired *string RiskIDs []uuid.UUID EvidenceIDs []uuid.UUID ControlRefs []ControlRef From acc2fb6d50979d28fad088261a0d128a9c4deee4 Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Thu, 26 Mar 2026 08:54:05 -0400 Subject: [PATCH 6/7] fix(poam): BCH-1186 address PR #362 review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- internal/api/handler/poam_items.go | 17 +++-------- internal/api/handler/risks.go | 29 +++++++------------ .../risks_promote_to_poam_integration_test.go | 3 -- internal/service/relational/poam/service.go | 28 ++++++++++++++---- .../relational/risks/promote_to_poam.go | 12 ++++---- 5 files changed, 43 insertions(+), 46 deletions(-) diff --git a/internal/api/handler/poam_items.go b/internal/api/handler/poam_items.go index 589f39c6..f6536fa7 100644 --- a/internal/api/handler/poam_items.go +++ b/internal/api/handler/poam_items.go @@ -417,19 +417,14 @@ func (h *PoamItemsHandler) Create(c echo.Context) error { } params.ControlRefs = controlRefs - for i, mr := range in.Milestones { + for _, mr := range in.Milestones { if mr.Title == "" { return c.JSON(http.StatusBadRequest, api.NewError(fmt.Errorf("milestone title is required"))) } if mr.Status != "" && !poamsvc.MilestoneStatus(mr.Status).IsValid() { return c.JSON(http.StatusBadRequest, api.NewError(fmt.Errorf("invalid milestone status: %s", mr.Status))) } - // When orderIndex is omitted (nil), fall back to the slice position so - // ordering is still deterministic without requiring the client to set it. - msOrderIdx := i - if mr.OrderIndex != nil { - msOrderIdx = *mr.OrderIndex - } + // Pass OrderIndex as a pointer; nil means auto-assign from slice position. params.Milestones = append(params.Milestones, poamsvc.CreateMilestoneParams{ Title: mr.Title, Description: mr.Description, @@ -437,7 +432,7 @@ func (h *PoamItemsHandler) Create(c echo.Context) error { PlannedCompletionDate: mr.PlannedCompletionDate, ResponsibleParty: mr.ResponsibleParty, Remarks: mr.Remarks, - OrderIndex: msOrderIdx, + OrderIndex: mr.OrderIndex, }) } @@ -705,10 +700,6 @@ func (h *PoamItemsHandler) AddMilestone(c echo.Context) error { if in.Status != "" && !poamsvc.MilestoneStatus(in.Status).IsValid() { return c.JSON(http.StatusBadRequest, api.NewError(fmt.Errorf("invalid milestone status: %s", in.Status))) } - var orderIdx int - if in.OrderIndex != nil { - orderIdx = *in.OrderIndex - } m, err := h.poamService.AddMilestone(id, poamsvc.CreateMilestoneParams{ Title: in.Title, Description: in.Description, @@ -716,7 +707,7 @@ func (h *PoamItemsHandler) AddMilestone(c echo.Context) error { PlannedCompletionDate: in.PlannedCompletionDate, ResponsibleParty: in.ResponsibleParty, Remarks: in.Remarks, - OrderIndex: orderIdx, + OrderIndex: in.OrderIndex, }) if err != nil { return h.internalError(c, "failed to add milestone", err) diff --git a/internal/api/handler/risks.go b/internal/api/handler/risks.go index a258a334..30d2ab42 100644 --- a/internal/api/handler/risks.go +++ b/internal/api/handler/risks.go @@ -34,15 +34,9 @@ const ( maxRiskDescriptionLength = 1000 ) -func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB, poamSvc *poamsvc.PoamService, riskSvc ...*riskrel.RiskService) *RiskHandler { - var rs *riskrel.RiskService - if len(riskSvc) > 0 && riskSvc[0] != nil { - rs = riskSvc[0] - } else { - rs = riskrel.NewRiskService(db) - } +func NewRiskHandler(sugar *zap.SugaredLogger, db *gorm.DB, poamSvc *poamsvc.PoamService, riskSvc *riskrel.RiskService) *RiskHandler { return &RiskHandler{ - riskService: rs, + riskService: riskSvc, poamService: poamSvc, sugar: sugar, pagination: svc.NewPaginationConfig(), @@ -1997,13 +1991,15 @@ func validateStatusTransition(oldStatus, newStatus string) error { }, string(riskrel.RiskStatusMitigatingPlanned): { string(riskrel.RiskStatusMitigatingImplemented): {}, + string(riskrel.RiskStatusInvestigating): {}, // mitigation can fail; risk returns to investigation }, string(riskrel.RiskStatusMitigatingImplemented): { - string(riskrel.RiskStatusClosed): {}, + string(riskrel.RiskStatusClosed): {}, + string(riskrel.RiskStatusRemediated): {}, // evidence fully green → remediated before close }, string(riskrel.RiskStatusRiskAccepted): { - string(riskrel.RiskStatusClosed): {}, - string(riskrel.RiskStatusMitigatingPlanned): {}, + string(riskrel.RiskStatusClosed): {}, + string(riskrel.RiskStatusInvestigating): {}, // re-open accepted risk for investigation }, string(riskrel.RiskStatusRemediated): { string(riskrel.RiskStatusOpen): {}, @@ -2295,14 +2291,9 @@ func (h *RiskHandler) PromoteToPoam(ctx echo.Context) error { } // Map request milestones to service params. + // OrderIndex is passed as a pointer; nil means auto-assign from slice position. var milestones []poamsvc.CreateMilestoneParams - for i, m := range req.Milestones { - orderIdx := 0 - if m.OrderIndex != nil { - orderIdx = *m.OrderIndex - } else { - orderIdx = i - } + for _, m := range req.Milestones { milestones = append(milestones, poamsvc.CreateMilestoneParams{ Title: m.Title, Description: m.Description, @@ -2310,7 +2301,7 @@ func (h *RiskHandler) PromoteToPoam(ctx echo.Context) error { PlannedCompletionDate: m.PlannedCompletionDate, ResponsibleParty: m.ResponsibleParty, Remarks: m.Remarks, - OrderIndex: orderIdx, + OrderIndex: m.OrderIndex, }) } diff --git a/internal/api/handler/risks_promote_to_poam_integration_test.go b/internal/api/handler/risks_promote_to_poam_integration_test.go index da607449..60a58cf0 100644 --- a/internal/api/handler/risks_promote_to_poam_integration_test.go +++ b/internal/api/handler/risks_promote_to_poam_integration_test.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "net/http" - "testing" "time" riskrel "github.com/compliance-framework/api/internal/service/relational/risks" @@ -346,5 +345,3 @@ func (suite *RiskApiIntegrationSuite) TestPromoteToPoam_CompletionAdvancesRiskSt require.Equal(suite.T(), int64(1), completedEvents) } -// Ensure the testing import is used. -var _ = (*testing.T)(nil) diff --git a/internal/service/relational/poam/service.go b/internal/service/relational/poam/service.go index 7150180b..b05938a6 100644 --- a/internal/service/relational/poam/service.go +++ b/internal/service/relational/poam/service.go @@ -68,6 +68,8 @@ type UpdatePoamItemParams struct { } // CreateMilestoneParams carries all data required to create a single milestone. +// OrderIndex is a pointer so that callers can distinguish "not provided" (nil, +// auto-assign) from "explicitly set to 0" (valid 0-based position). type CreateMilestoneParams struct { Title string Description string @@ -75,7 +77,7 @@ type CreateMilestoneParams struct { PlannedCompletionDate *time.Time ResponsibleParty *string Remarks *string - OrderIndex int + OrderIndex *int } // UpdateMilestoneParams carries the fields that may be patched on an existing @@ -132,8 +134,10 @@ func (s *PoamService) Create(params CreatePoamItemParams) (*PoamItem, error) { } for i, mp := range params.Milestones { - orderIdx := mp.OrderIndex - if orderIdx == 0 { + var orderIdx int + if mp.OrderIndex != nil { + orderIdx = *mp.OrderIndex + } else { orderIdx = i } ms := PoamItemMilestone{ @@ -436,6 +440,10 @@ func (s *PoamService) ListMilestones(poamItemID uuid.UUID) ([]PoamItemMilestone, // AddMilestone inserts a new milestone for the given POAM item. func (s *PoamService) AddMilestone(poamItemID uuid.UUID, params CreateMilestoneParams) (*PoamItemMilestone, error) { + var orderIdx int + if params.OrderIndex != nil { + orderIdx = *params.OrderIndex + } m := PoamItemMilestone{ PoamItemID: poamItemID, Title: params.Title, @@ -444,7 +452,7 @@ func (s *PoamService) AddMilestone(poamItemID uuid.UUID, params CreateMilestoneP PlannedCompletionDate: params.PlannedCompletionDate, ResponsibleParty: params.ResponsibleParty, Remarks: params.Remarks, - OrderIndex: params.OrderIndex, + OrderIndex: orderIdx, } if err := s.db.Create(&m).Error; err != nil { return nil, err @@ -695,6 +703,12 @@ func (s *PoamService) DeleteFindingLink(poamItemID, findingID uuid.UUID) error { // committing or rolling back the transaction. This is used by cross-context // operations such as RiskService.PromoteToPoam that need atomicity across // multiple bounded contexts. +// +// NOTE: This method only processes Milestones and RiskIDs from +// CreatePoamItemParams. EvidenceIDs, ControlRefs, and FindingIDs present in +// the params struct are intentionally ignored — this method is scoped to the +// risk-promotion use case. Use the full Create method for general POAM item +// creation with all link types. func (s *PoamService) CreateWithTx(tx *gorm.DB, params CreatePoamItemParams) (*PoamItem, error) { item := PoamItem{ SspID: params.SspID, @@ -714,8 +728,10 @@ func (s *PoamService) CreateWithTx(tx *gorm.DB, params CreatePoamItemParams) (*P } for i, mp := range params.Milestones { - orderIdx := mp.OrderIndex - if orderIdx == 0 { + var orderIdx int + if mp.OrderIndex != nil { + orderIdx = *mp.OrderIndex + } else { orderIdx = i } ms := PoamItemMilestone{ diff --git a/internal/service/relational/risks/promote_to_poam.go b/internal/service/relational/risks/promote_to_poam.go index f5c75d3b..dd94c029 100644 --- a/internal/service/relational/risks/promote_to_poam.go +++ b/internal/service/relational/risks/promote_to_poam.go @@ -54,7 +54,6 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote // 1. Load and lock the risk row. var risk Risk if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). - Preload("OwnerAssignments"). First(&risk, "id = ?", params.RiskID).Error; err != nil { tx.Rollback() if errors.Is(err, gorm.ErrRecordNotFound) { @@ -107,20 +106,23 @@ func (s *RiskService) PromoteToPoam(poamSvc *poamsvc.PoamService, params Promote } if err == nil { for _, task := range remediationTemplate.Tasks { + idx := task.OrderIndex templateMilestones = append(templateMilestones, poamsvc.CreateMilestoneParams{ Title: task.Title, - OrderIndex: task.OrderIndex, + OrderIndex: &idx, }) } } // 5. Merge template milestones with extra milestones from the request. // Extra milestones are appended after template tasks, with order_index - // offset by the number of template tasks. + // offset by the number of template tasks. A nil OrderIndex means + // "auto-assign"; a non-nil pointer (including 0) is used as-is. offset := len(templateMilestones) for i, extra := range params.ExtraMilestones { - if extra.OrderIndex == 0 { - extra.OrderIndex = offset + i + if extra.OrderIndex == nil { + idx := offset + i + extra.OrderIndex = &idx } templateMilestones = append(templateMilestones, extra) } From ece92018a5f1cc5f6d68057ee4465788be45523f Mon Sep 17 00:00:00 2001 From: AKAbdulHanif Date: Thu, 26 Mar 2026 09:35:50 -0400 Subject: [PATCH 7/7] fix(poam): normalise swag fmt whitespace in validateStatusTransition map --- internal/api/handler/risks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/handler/risks.go b/internal/api/handler/risks.go index 30d2ab42..2cb3ada7 100644 --- a/internal/api/handler/risks.go +++ b/internal/api/handler/risks.go @@ -1994,7 +1994,7 @@ func validateStatusTransition(oldStatus, newStatus string) error { string(riskrel.RiskStatusInvestigating): {}, // mitigation can fail; risk returns to investigation }, string(riskrel.RiskStatusMitigatingImplemented): { - string(riskrel.RiskStatusClosed): {}, + string(riskrel.RiskStatusClosed): {}, string(riskrel.RiskStatusRemediated): {}, // evidence fully green → remediated before close }, string(riskrel.RiskStatusRiskAccepted): {