From c8a439ede2c0c3601356d43cab636bcfdd4fadaa Mon Sep 17 00:00:00 2001 From: James Salt Date: Thu, 28 May 2026 09:57:41 +0100 Subject: [PATCH 1/2] BCH-1145: Remove definition-level evidence-required from API contract Evidence requirements are modelled exclusively at step level (EvidenceRequirement[] per step). The definition-level field was duplicated and never used by step transition validation. - Remove EvidenceRequired from Create/Update request structs - Remove handler logic that copied the field onto the definition - Tag model field json:"-" to exclude from responses (DB column kept for migration safety) - Add integration tests asserting evidence_required is absent from Create and Update responses Co-Authored-By: Claude Sonnet 4.6 --- docs/docs.go | 10 ---- docs/swagger.json | 10 ---- docs/swagger.yaml | 7 --- .../handler/workflows/workflow_definition.go | 6 -- .../workflow_definition_integration_test.go | 59 ++++++++++++++++++- .../workflows/workflow_definition.go | 6 +- 6 files changed, 61 insertions(+), 37 deletions(-) diff --git a/docs/docs.go b/docs/docs.go index 01a7ef74..32c38181 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -40386,9 +40386,6 @@ const docTemplate = `{ "description": { "type": "string" }, - "evidence-required": { - "type": "string" - }, "grace-period-days": { "type": "integer" }, @@ -40973,9 +40970,6 @@ const docTemplate = `{ "description": { "type": "string" }, - "evidence-required": { - "type": "string" - }, "grace-period-days": { "type": "integer" }, @@ -41064,10 +41058,6 @@ const docTemplate = `{ "description": { "type": "string" }, - "evidence_required": { - "description": "JSON array of required evidence types", - "type": "string" - }, "grace-period-days": { "description": "Override global default if set", "type": "integer" diff --git a/docs/swagger.json b/docs/swagger.json index 4eed8ef6..a77c8650 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -40380,9 +40380,6 @@ "description": { "type": "string" }, - "evidence-required": { - "type": "string" - }, "grace-period-days": { "type": "integer" }, @@ -40967,9 +40964,6 @@ "description": { "type": "string" }, - "evidence-required": { - "type": "string" - }, "grace-period-days": { "type": "integer" }, @@ -41058,10 +41052,6 @@ "description": { "type": "string" }, - "evidence_required": { - "description": "JSON array of required evidence types", - "type": "string" - }, "grace-period-days": { "description": "Override global default if set", "type": "integer" diff --git a/docs/swagger.yaml b/docs/swagger.yaml index c740ac6d..ec1aded6 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -8800,8 +8800,6 @@ definitions: properties: description: type: string - evidence-required: - type: string grace-period-days: type: integer name: @@ -9190,8 +9188,6 @@ definitions: properties: description: type: string - evidence-required: - type: string grace-period-days: type: integer name: @@ -9250,9 +9246,6 @@ definitions: $ref: '#/definitions/gorm.DeletedAt' description: type: string - evidence_required: - description: JSON array of required evidence types - type: string grace-period-days: description: Override global default if set type: integer diff --git a/internal/api/handler/workflows/workflow_definition.go b/internal/api/handler/workflows/workflow_definition.go index b0711dba..3a911a7f 100644 --- a/internal/api/handler/workflows/workflow_definition.go +++ b/internal/api/handler/workflows/workflow_definition.go @@ -32,7 +32,6 @@ type CreateWorkflowDefinitionRequest struct { Description string `json:"description"` Version string `json:"version"` SuggestedCadence string `json:"suggested-cadence"` - EvidenceRequired string `json:"evidence-required"` GracePeriodDays *int `json:"grace-period-days"` } @@ -41,7 +40,6 @@ type UpdateWorkflowDefinitionRequest struct { Description *string `json:"description"` Version *string `json:"version"` SuggestedCadence *string `json:"suggested-cadence"` - EvidenceRequired *string `json:"evidence-required"` GracePeriodDays *int `json:"grace-period-days"` } @@ -78,7 +76,6 @@ func (h *WorkflowDefinitionHandler) Create(ctx echo.Context) error { Description: req.Description, Version: req.Version, SuggestedCadence: req.SuggestedCadence, - EvidenceRequired: req.EvidenceRequired, GracePeriodDays: req.GracePeriodDays, } @@ -184,9 +181,6 @@ func (h *WorkflowDefinitionHandler) Update(ctx echo.Context) error { if req.SuggestedCadence != nil { definition.SuggestedCadence = *req.SuggestedCadence } - if req.EvidenceRequired != nil { - definition.EvidenceRequired = *req.EvidenceRequired - } if req.GracePeriodDays != nil { definition.GracePeriodDays = req.GracePeriodDays } diff --git a/internal/api/handler/workflows/workflow_definition_integration_test.go b/internal/api/handler/workflows/workflow_definition_integration_test.go index ac65a323..f8f3a390 100644 --- a/internal/api/handler/workflows/workflow_definition_integration_test.go +++ b/internal/api/handler/workflows/workflow_definition_integration_test.go @@ -37,7 +37,6 @@ func TestWorkflowDefinitionHandler_Create(t *testing.T) { Description: "Quarterly security assessment process", Version: "1.0", SuggestedCadence: "quarterly", - EvidenceRequired: `["vulnerability_scan", "penetration_test"]`, GracePeriodDays: intPtr(10), } @@ -66,6 +65,35 @@ func TestWorkflowDefinitionHandler_Create(t *testing.T) { assert.Equal(t, 10, *response.Data.GracePeriodDays) }) + // BCH-1145: definition-level evidence-required duplicates step-level requirements. + // Observed: POST /workflows/definitions accepts and returns evidence-required. + // Expected: the field must not be part of the API contract. + t.Run("EvidenceRequired_NotInResponse", func(t *testing.T) { + reqBody := map[string]interface{}{ + "name": "Evidence Test Workflow", + "evidence-required": `["document"]`, + } + body, err := json.Marshal(reqBody) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/workflows/definitions", bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + err = handler.Create(c) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, rec.Code) + + var rawResponse map[string]interface{} + err = json.Unmarshal(rec.Body.Bytes(), &rawResponse) + require.NoError(t, err) + data, ok := rawResponse["data"].(map[string]interface{}) + require.True(t, ok, "response must have a data object") + _, hasField := data["evidence_required"] + assert.False(t, hasField, "evidence_required must not appear in the workflow definition response") + }) + t.Run("ValidationError_MissingName", func(t *testing.T) { reqBody := CreateWorkflowDefinitionRequest{ Description: "Missing name", @@ -236,6 +264,35 @@ func TestWorkflowDefinitionHandler_Update(t *testing.T) { assert.Equal(t, 14, *response.Data.GracePeriodDays) }) + // BCH-1145: evidence-required must not appear in update responses either. + t.Run("EvidenceRequired_NotInUpdateResponse", func(t *testing.T) { + reqBody := map[string]interface{}{ + "name": "Updated Name", + "evidence-required": `["screenshot"]`, + } + body, err := json.Marshal(reqBody) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPut, "/workflows/definitions/"+definition.ID.String(), bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + c.SetParamNames("id") + c.SetParamValues(definition.ID.String()) + + err = handler.Update(c) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, rec.Code) + + var rawResponse map[string]interface{} + err = json.Unmarshal(rec.Body.Bytes(), &rawResponse) + require.NoError(t, err) + data, ok := rawResponse["data"].(map[string]interface{}) + require.True(t, ok, "response must have a data object") + _, hasField := data["evidence_required"] + assert.False(t, hasField, "evidence_required must not appear in the workflow definition update response") + }) + t.Run("PartialUpdate", func(t *testing.T) { newVersion := "2.0" reqBody := UpdateWorkflowDefinitionRequest{ diff --git a/internal/service/relational/workflows/workflow_definition.go b/internal/service/relational/workflows/workflow_definition.go index 9346f458..d9f00cea 100644 --- a/internal/service/relational/workflows/workflow_definition.go +++ b/internal/service/relational/workflows/workflow_definition.go @@ -22,9 +22,9 @@ type WorkflowDefinition struct { Version string `gorm:"size:50" json:"version"` // Workflow Configuration - SuggestedCadence string `gorm:"size:50" json:"suggested_cadence"` // daily, weekly, monthly, quarterly, annually - EvidenceRequired string `gorm:"type:text" json:"evidence_required"` // JSON array of required evidence types - GracePeriodDays *int `json:"grace-period-days,omitempty"` // Override global default if set + SuggestedCadence string `gorm:"size:50" json:"suggested_cadence"` // daily, weekly, monthly, quarterly, annually + EvidenceRequired string `gorm:"type:text" json:"-"` // kept in DB for migration safety; excluded from API contract (BCH-1145) + GracePeriodDays *int `json:"grace-period-days,omitempty"` // Override global default if set // Audit Fields CreatedByID *uuid.UUID `gorm:"index" json:"created_by_id,omitempty"` From 0dec5e2540400ae8a594e7f0f12f275c0fb082c9 Mon Sep 17 00:00:00 2001 From: James Salt Date: Thu, 28 May 2026 10:20:14 +0100 Subject: [PATCH 2/2] BCH-1145: Assert both evidence-required key variants absent from responses Add hyphen-form (evidence-required) assertions alongside the existing underscore-form checks in the Create and Update integration tests to fully enforce contract removal of the definition-level field. Co-Authored-By: Claude Sonnet 4.6 --- .../workflow_definition_integration_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/api/handler/workflows/workflow_definition_integration_test.go b/internal/api/handler/workflows/workflow_definition_integration_test.go index f8f3a390..c791e127 100644 --- a/internal/api/handler/workflows/workflow_definition_integration_test.go +++ b/internal/api/handler/workflows/workflow_definition_integration_test.go @@ -90,8 +90,10 @@ func TestWorkflowDefinitionHandler_Create(t *testing.T) { require.NoError(t, err) data, ok := rawResponse["data"].(map[string]interface{}) require.True(t, ok, "response must have a data object") - _, hasField := data["evidence_required"] - assert.False(t, hasField, "evidence_required must not appear in the workflow definition response") + _, hasUnderscore := data["evidence_required"] + assert.False(t, hasUnderscore, "evidence_required must not appear in the workflow definition response") + _, hasHyphen := data["evidence-required"] + assert.False(t, hasHyphen, "evidence-required must not appear in the workflow definition response") }) t.Run("ValidationError_MissingName", func(t *testing.T) { @@ -289,8 +291,10 @@ func TestWorkflowDefinitionHandler_Update(t *testing.T) { require.NoError(t, err) data, ok := rawResponse["data"].(map[string]interface{}) require.True(t, ok, "response must have a data object") - _, hasField := data["evidence_required"] - assert.False(t, hasField, "evidence_required must not appear in the workflow definition update response") + _, hasUnderscore := data["evidence_required"] + assert.False(t, hasUnderscore, "evidence_required must not appear in the workflow definition update response") + _, hasHyphen := data["evidence-required"] + assert.False(t, hasHyphen, "evidence-required must not appear in the workflow definition update response") }) t.Run("PartialUpdate", func(t *testing.T) {