Skip to content

fix: auto-create diagram groupings on first diagram POST #415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gusfcarvalho merged 1 commit into from
Jun 11, 2026
Merged

fix: auto-create diagram groupings on first diagram POST #415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/docs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16631,7 +16631,7 @@ const docTemplate = `{
},
"/oscal/system-security-plans/{id}/system-characteristics/authorization-boundary/diagrams": {
"post": {
"description": "Creates a new Diagram under the Authorization Boundary of a System Security Plan.",
"description": "Creates a new Diagram under the Authorization Boundary of a System Security Plan. Creates the Authorization Boundary grouping if it does not exist yet.",
"consumes": [
"application/json"
],
Expand Down Expand Up @@ -16896,7 +16896,7 @@ const docTemplate = `{
},
"/oscal/system-security-plans/{id}/system-characteristics/data-flow/diagrams": {
"post": {
"description": "Creates a new Diagram under the Data Flow of a System Security Plan.",
"description": "Creates a new Diagram under the Data Flow of a System Security Plan. Creates the Data Flow grouping if it does not exist yet.",
"consumes": [
"application/json"
],
Expand Down Expand Up @@ -17161,7 +17161,7 @@ const docTemplate = `{
},
"/oscal/system-security-plans/{id}/system-characteristics/network-architecture/diagrams": {
"post": {
"description": "Creates a new Diagram under the Network Architecture of a System Security Plan.",
"description": "Creates a new Diagram under the Network Architecture of a System Security Plan. Creates the Network Architecture grouping if it does not exist yet.",
"consumes": [
"application/json"
],
Expand Down
6 changes: 3 additions & 3 deletions docs/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16625,7 +16625,7 @@
},
"/oscal/system-security-plans/{id}/system-characteristics/authorization-boundary/diagrams": {
"post": {
"description": "Creates a new Diagram under the Authorization Boundary of a System Security Plan.",
"description": "Creates a new Diagram under the Authorization Boundary of a System Security Plan. Creates the Authorization Boundary grouping if it does not exist yet.",

@coderabbitai coderabbitai Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify consistency of auto-create descriptions across all three generated swagger artifacts.

echo "=== Checking swagger.json ==="
rg -n "Creates the Authorization Boundary grouping if it does not exist yet" docs/swagger.json
rg -n "Creates the Data Flow grouping if it does not exist yet" docs/swagger.json
rg -n "Creates the Network Architecture grouping if it does not exist yet" docs/swagger.json

echo -e "\n=== Checking swagger.yaml ==="
rg -n "Creates the Authorization Boundary grouping if it does not exist yet" docs/swagger.yaml
rg -n "Creates the Data Flow grouping if it does not exist yet" docs/swagger.yaml
rg -n "Creates the Network Architecture grouping if it does not exist yet" docs/swagger.yaml

echo -e "\n=== Checking docs.go ==="
rg -n "Creates the Authorization Boundary grouping if it does not exist yet" docs/docs.go
rg -n "Creates the Data Flow grouping if it does not exist yet" docs/docs.go
rg -n "Creates the Network Architecture grouping if it does not exist yet" docs/docs.go

Repository: compliance-framework/api

Length of output: 1324

Fix Swagger YAML description inconsistency across generated artifacts

  • docs/swagger.json and docs/docs.go contain the updated descriptions for Authorization Boundary (16628), Data Flow (16893), and Network Architecture (17158).
  • docs/swagger.yaml contains only the updated Data Flow description (but is missing the Authorization Boundary and Network Architecture “grouping if it does not exist yet” descriptions).
  • Re-run the swagger generation (e.g., swag init) and ensure docs/swagger.yaml is regenerated to match the other artifacts.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/swagger.json` at line 16628, The swagger YAML is out of sync:
docs/swagger.yaml is missing the updated descriptions for Authorization Boundary
and Network Architecture that appear in docs/swagger.json and docs/docs.go
(e.g., the “grouping if it does not exist yet” text around the Authorization
Boundary description at the Authorization Boundary, Data Flow, and Network
Architecture endpoints). Re-run your Swagger generator (e.g., run swag init) to
regenerate docs/swagger.yaml so its description strings match docs/swagger.json
and docs/docs.go, then verify the Authorization Boundary, Data Flow, and Network
Architecture operation descriptions all include the “grouping if it does not
exist yet” phrasing.

Source: Learnings

"consumes": [
"application/json"
],
Expand Down Expand Up @@ -16890,7 +16890,7 @@
},
"/oscal/system-security-plans/{id}/system-characteristics/data-flow/diagrams": {
"post": {
"description": "Creates a new Diagram under the Data Flow of a System Security Plan.",
"description": "Creates a new Diagram under the Data Flow of a System Security Plan. Creates the Data Flow grouping if it does not exist yet.",
"consumes": [
"application/json"
],
Expand Down Expand Up @@ -17155,7 +17155,7 @@
},
"/oscal/system-security-plans/{id}/system-characteristics/network-architecture/diagrams": {
"post": {
"description": "Creates a new Diagram under the Network Architecture of a System Security Plan.",
"description": "Creates a new Diagram under the Network Architecture of a System Security Plan. Creates the Network Architecture grouping if it does not exist yet.",
"consumes": [
"application/json"
],
Expand Down
8 changes: 5 additions & 3 deletions docs/swagger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20433,7 +20433,8 @@ paths:
consumes:
- application/json
description: Creates a new Diagram under the Authorization Boundary of a System
Security Plan.
Security Plan. Creates the Authorization Boundary grouping if it does not
exist yet.
parameters:
- description: System Security Plan ID
in: path
Expand Down Expand Up @@ -20607,7 +20608,7 @@ paths:
consumes:
- application/json
description: Creates a new Diagram under the Data Flow of a System Security
Plan.
Plan. Creates the Data Flow grouping if it does not exist yet.
parameters:
- description: System Security Plan ID
in: path
Expand Down Expand Up @@ -20782,7 +20783,8 @@ paths:
consumes:
- application/json
description: Creates a new Diagram under the Network Architecture of a System
Security Plan.
Security Plan. Creates the Network Architecture grouping if it does not exist
yet.
parameters:
- description: System Security Plan ID
in: path
Expand Down
33 changes: 27 additions & 6 deletions internal/api/handler/oscal/system_security_plans.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ func (h *SystemSecurityPlanHandler) GetCharacteristicsNetworkArchitecture(ctx ec
// CreateCharacteristicsNetworkArchitectureDiagram godoc
//
// @Summary Create a Network Architecture Diagram
// @Description Creates a new Diagram under the Network Architecture of a System Security Plan.
// @Description Creates a new Diagram under the Network Architecture of a System Security Plan. Creates the Network Architecture grouping if it does not exist yet.
// @Tags System Security Plans
// @Accept json
// @Produce json
Expand Down Expand Up @@ -600,7 +600,14 @@ func (h *SystemSecurityPlanHandler) CreateCharacteristicsNetworkArchitectureDiag

na := ssp.SystemCharacteristics.NetworkArchitecture
if na == nil || na.ID == nil {
return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("no network architecture for system security plan %s", idParam)))
if ssp.SystemCharacteristics.ID == nil {
return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("no system characteristics for system security plan %s", idParam)))
}
na = &relational.NetworkArchitecture{SystemCharacteristicsId: *ssp.SystemCharacteristics.ID}
if err := h.db.Create(na).Error; err != nil {
h.sugar.Errorf("Failed to create network architecture: %v", err)
return ctx.JSON(http.StatusInternalServerError, api.NewError(err))
}
Comment on lines +603 to +610

@coderabbitai coderabbitai Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make grouping+diagram creation atomic and validate input before mutating DB.

All three handlers can persist a new grouping before ctx.Bind/UUID validation runs, so malformed requests can return 400 after mutating state. They also perform grouping create and diagram create as separate writes, so concurrent “first diagram” calls can race into duplicate-group or unique-constraint 500 paths.

Wrap “ensure grouping + create diagram” in one transaction, and move request bind/validation before any write.

Suggested pattern (apply to all three handlers) 
- // ensure/create grouping first
- if group == nil || group.ID == nil {
-   if ssp.SystemCharacteristics.ID == nil { ...404... }
-   group = &relational.<Grouping>{SystemCharacteristicsId: *ssp.SystemCharacteristics.ID}
-   if err := h.db.Create(group).Error; err != nil { ...500... }
- }
-
- // then bind/validate diagram
+ // bind/validate first (no side effects on 400)
+ var oscalDiag oscalTypes_1_1_3.Diagram
+ if err := ctx.Bind(&oscalDiag); err != nil { ...400... }
+ if oscalDiag.UUID == "" { ...400... }
+ if _, err := uuid.Parse(oscalDiag.UUID); err != nil { ...400... }
+
+ err = h.db.Transaction(func(tx *gorm.DB) error {
+   // idempotent ensure-grouping (conflict-safe)
+   // then create diagram under resolved grouping ID
+   // return error to rollback both on any failure
+   return nil
+ })
+ if err != nil { ...500... }

Also applies to: 848-855, 1089-1096

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/api/handler/oscal/system_security_plans.go` around lines 603 - 610,
The code is creating relational.NetworkArchitecture (na) before validating the
incoming request and is doing grouping + diagram creation as separate writes;
move the request binding and UUID validation (ctx.Bind and any ID parsing) to
occur before any DB mutation, then wrap the "ensure grouping + create diagram"
sequence in a single DB transaction using h.db.Transaction so both the grouping
create and the na create use the transactional tx (e.g., replace h.db.Create
calls with tx.Create inside the transaction closure), check for existing
grouping within the transaction and only create if missing to avoid
race/duplicate errors, and convert DB errors into appropriate HTTP responses
(handle unique-constraint conflicts vs internal errors); apply the same change
to the other two handlers referenced (around the other blocks at 848-855 and
1089-1096).

}

// Bind incoming diagram
Expand Down Expand Up @@ -803,7 +810,7 @@ func (h *SystemSecurityPlanHandler) GetCharacteristicsDataFlow(ctx echo.Context)
// CreateCharacteristicsDataFlowDiagram godoc
//
// @Summary Create a Data Flow Diagram
// @Description Creates a new Diagram under the Data Flow of a System Security Plan.
// @Description Creates a new Diagram under the Data Flow of a System Security Plan. Creates the Data Flow grouping if it does not exist yet.
// @Tags System Security Plans
// @Accept json
// @Produce json
Expand Down Expand Up @@ -838,7 +845,14 @@ func (h *SystemSecurityPlanHandler) CreateCharacteristicsDataFlowDiagram(ctx ech

df := ssp.SystemCharacteristics.DataFlow
if df == nil || df.ID == nil {
return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("no data flow for system security plan %s", idParam)))
if ssp.SystemCharacteristics.ID == nil {
return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("no system characteristics for system security plan %s", idParam)))
}
df = &relational.DataFlow{SystemCharacteristicsId: *ssp.SystemCharacteristics.ID}
if err := h.db.Create(df).Error; err != nil {
h.sugar.Errorf("Failed to create data flow: %v", err)
return ctx.JSON(http.StatusInternalServerError, api.NewError(err))
}
}

var oscalDiag oscalTypes_1_1_3.Diagram
Expand Down Expand Up @@ -1037,7 +1051,7 @@ func (h *SystemSecurityPlanHandler) GetCharacteristicsAuthorizationBoundary(ctx
// CreateCharacteristicsAuthorizationBoundaryDiagram godoc
//
// @Summary Create an Authorization Boundary Diagram
// @Description Creates a new Diagram under the Authorization Boundary of a System Security Plan.
// @Description Creates a new Diagram under the Authorization Boundary of a System Security Plan. Creates the Authorization Boundary grouping if it does not exist yet.
// @Tags System Security Plans
// @Accept json
// @Produce json
Expand Down Expand Up @@ -1072,7 +1086,14 @@ func (h *SystemSecurityPlanHandler) CreateCharacteristicsAuthorizationBoundaryDi

ab := ssp.SystemCharacteristics.AuthorizationBoundary
if ab == nil || ab.ID == nil {
return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("no authorization boundary for system security plan %s", idParam)))
if ssp.SystemCharacteristics.ID == nil {
return ctx.JSON(http.StatusNotFound, api.NewError(fmt.Errorf("no system characteristics for system security plan %s", idParam)))
}
ab = &relational.AuthorizationBoundary{SystemCharacteristicsId: *ssp.SystemCharacteristics.ID}
if err := h.db.Create(ab).Error; err != nil {
h.sugar.Errorf("Failed to create authorization boundary: %v", err)
return ctx.JSON(http.StatusInternalServerError, api.NewError(err))
}
}

var oscalDiag oscalTypes_1_1_3.Diagram
Expand Down
44 changes: 37 additions & 7 deletions internal/api/handler/oscal/system_security_plans_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2319,7 +2319,7 @@ func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateAuthorizationBound
suite.True(found, "created diagram should be present in authorization boundary")
}

// Negative: Network Architecture missing, invalid IDs and invalid body
// Network Architecture diagram creation: invalid IDs, missing grouping auto-create, invalid body
func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateNetworkArchitectureDiagram_Negative() {
logConf := zap.NewDevelopmentConfig()
logConf.Level = zap.NewAtomicLevelAt(zap.ErrorLevel)
Expand All @@ -2340,21 +2340,36 @@ func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateNetworkArchitectur
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusBadRequest, resp.Code)

// 2) Missing Network Architecture -> 404
// 2) Missing Network Architecture -> grouping is auto-created on first diagram POST
ssp := suite.createBasicSSP()
ssp.SystemCharacteristics.NetworkArchitecture = nil
req = suite.createRequest(http.MethodPost, "/api/oscal/system-security-plans", ssp)
resp = httptest.NewRecorder()
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusCreated, resp.Code)

// Attempt to create diagram when NA is missing
// Create diagram when NA is missing
diagram := oscalTypes_1_1_3.Diagram{UUID: uuid.New().String(), Description: "NA diag"}
req = suite.createRequest(http.MethodPost,
fmt.Sprintf("/api/oscal/system-security-plans/%s/system-characteristics/network-architecture/diagrams", ssp.UUID), diagram)
resp = httptest.NewRecorder()
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusNotFound, resp.Code)
suite.Equal(http.StatusCreated, resp.Code)

// The grouping should now exist and contain the diagram
req = suite.createRequest(http.MethodGet,
fmt.Sprintf("/api/oscal/system-security-plans/%s/system-characteristics/network-architecture", ssp.UUID), nil)
resp = httptest.NewRecorder()
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusOK, resp.Code)

var naResponse handler.GenericDataResponse[*oscalTypes_1_1_3.NetworkArchitecture]
err = json.Unmarshal(resp.Body.Bytes(), &naResponse)
suite.Require().NoError(err)
suite.Require().NotNil(naResponse.Data)
suite.Require().NotNil(naResponse.Data.Diagrams)
suite.Require().Len(*naResponse.Data.Diagrams, 1)
suite.Equal(diagram.UUID, (*naResponse.Data.Diagrams)[0].UUID)

// 3) Invalid/Missing diagram UUID -> 400
// Recreate SSP with NA present
Expand Down Expand Up @@ -2385,7 +2400,7 @@ func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateNetworkArchitectur
suite.Equal(http.StatusBadRequest, resp.Code)
}

// Negative: Data Flow missing, invalid IDs and invalid body
// Data Flow diagram creation: invalid IDs, missing grouping auto-create, invalid body
func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateDataFlowDiagram_Negative() {
logConf := zap.NewDevelopmentConfig()
logConf.Level = zap.NewAtomicLevelAt(zap.ErrorLevel)
Expand All @@ -2406,7 +2421,7 @@ func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateDataFlowDiagram_Ne
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusBadRequest, resp.Code)

// 2) Missing Data Flow -> 404
// 2) Missing Data Flow -> grouping is auto-created on first diagram POST
ssp := suite.createBasicSSP()
ssp.SystemCharacteristics.DataFlow = nil
req = suite.createRequest(http.MethodPost, "/api/oscal/system-security-plans", ssp)
Expand All @@ -2419,7 +2434,22 @@ func (suite *SystemSecurityPlanApiIntegrationSuite) TestCreateDataFlowDiagram_Ne
fmt.Sprintf("/api/oscal/system-security-plans/%s/system-characteristics/data-flow/diagrams", ssp.UUID), diagram)
resp = httptest.NewRecorder()
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusNotFound, resp.Code)
suite.Equal(http.StatusCreated, resp.Code)

// The grouping should now exist and contain the diagram
req = suite.createRequest(http.MethodGet,
fmt.Sprintf("/api/oscal/system-security-plans/%s/system-characteristics/data-flow", ssp.UUID), nil)
resp = httptest.NewRecorder()
server.E().ServeHTTP(resp, req)
suite.Equal(http.StatusOK, resp.Code)

var dfResponse handler.GenericDataResponse[*oscalTypes_1_1_3.DataFlow]
err = json.Unmarshal(resp.Body.Bytes(), &dfResponse)
suite.Require().NoError(err)
suite.Require().NotNil(dfResponse.Data)
suite.Require().NotNil(dfResponse.Data.Diagrams)
suite.Require().Len(*dfResponse.Data.Diagrams, 1)
suite.Equal(diagram.UUID, (*dfResponse.Data.Diagrams)[0].UUID)

// 3) Invalid/Missing diagram UUID -> 400
// Recreate SSP with DF present
Expand Down
Loading