From 621a665692d2b7ac16162fef8cd6b1b870c18fe8 Mon Sep 17 00:00:00 2001 From: James Salt Date: Thu, 28 May 2026 09:10:45 +0100 Subject: [PATCH 1/2] BCH-1150: Validate evidence file type against evidence type on backend Reject screenshot evidence submissions where the file media type is not an image (png/jpg/jpeg/gif/webp). Adds screenshotAllowedMediaTypes and validates in validateEvidenceRequirements before checking required types. Co-Authored-By: Claude Sonnet 4.6 --- internal/workflow/step_transition.go | 18 ++++++++ internal/workflow/step_transition_test.go | 56 +++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/internal/workflow/step_transition.go b/internal/workflow/step_transition.go index fbca8bd3..20a352d4 100644 --- a/internal/workflow/step_transition.go +++ b/internal/workflow/step_transition.go @@ -33,6 +33,14 @@ type StepTransitionService struct { var ErrInvalidStepTransition = errors.New("invalid step transition") var errTransitionForbidden = errors.New("forbidden") +var screenshotAllowedMediaTypes = map[string]bool{ + "image/png": true, + "image/jpeg": true, + "image/jpg": true, + "image/gif": true, + "image/webp": true, +} + // WorkflowDefinitionServiceInterface defines the interface for workflow definition operations type WorkflowDefinitionServiceInterface interface { GetByID(id *uuid.UUID) (*workflows.WorkflowDefinition, error) @@ -288,8 +296,18 @@ func (s *StepTransitionService) validateTransition(currentStatus, newStatus stri } // validateEvidenceRequirements validates that all required evidence has been submitted +// and that file media types are compatible with their declared evidence type. func (s *StepTransitionService) validateEvidenceRequirements(stepDef *workflows.WorkflowStepDefinition, submittedEvidence []EvidenceSubmission) error { + // Validate file type compatibility per submission. + for _, evidence := range submittedEvidence { + if evidence.EvidenceType == "screenshot" && evidence.MediaType != "" { + if !screenshotAllowedMediaTypes[evidence.MediaType] { + return fmt.Errorf("evidence type 'screenshot' requires an image file (png/jpg/jpeg/gif/webp), got %q", evidence.MediaType) + } + } + } + // Build a map of submitted evidence types submittedTypes := make(map[string]int) for _, evidence := range submittedEvidence { diff --git a/internal/workflow/step_transition_test.go b/internal/workflow/step_transition_test.go index 05c00b77..98e6f28f 100644 --- a/internal/workflow/step_transition_test.go +++ b/internal/workflow/step_transition_test.go @@ -271,6 +271,62 @@ func TestTransitionStepStatus_UnexpectedPermissionLookupErrorBubbles(t *testing. mockRole.AssertExpectations(t) } +// BCH-1150: screenshot evidence should reject non-image file types. +// Observed: validateEvidenceRequirements accepts PDF/DOC for screenshot type. +// Expected: returns error when media type is not an image for screenshot evidence. +func TestValidateEvidenceRequirements_ScreenshotRejectsNonImageMediaType(t *testing.T) { + svc := NewStepTransitionService(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + + stepDef := &workflows.WorkflowStepDefinition{ + EvidenceRequired: []workflows.EvidenceRequirement{ + {Type: "screenshot", Required: true}, + }, + } + + err := svc.validateEvidenceRequirements(stepDef, []EvidenceSubmission{ + {EvidenceType: "screenshot", MediaType: "application/pdf"}, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "screenshot") +} + +// BCH-1150: screenshot evidence should accept image file types. +func TestValidateEvidenceRequirements_ScreenshotAcceptsImageMediaType(t *testing.T) { + svc := NewStepTransitionService(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + + stepDef := &workflows.WorkflowStepDefinition{ + EvidenceRequired: []workflows.EvidenceRequirement{ + {Type: "screenshot", Required: true}, + }, + } + + for _, imageType := range []string{"image/png", "image/jpeg", "image/gif", "image/webp"} { + err := svc.validateEvidenceRequirements(stepDef, []EvidenceSubmission{ + {EvidenceType: "screenshot", MediaType: imageType}, + }) + assert.NoError(t, err, "expected %s to be accepted for screenshot", imageType) + } +} + +// BCH-1150: document evidence should accept a broader set of media types including PDF and images. +func TestValidateEvidenceRequirements_DocumentAcceptsBroadMediaTypes(t *testing.T) { + svc := NewStepTransitionService(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + + stepDef := &workflows.WorkflowStepDefinition{ + EvidenceRequired: []workflows.EvidenceRequirement{ + {Type: "document", Required: true}, + }, + } + + for _, mediaType := range []string{"application/pdf", "application/msword", "image/png"} { + err := svc.validateEvidenceRequirements(stepDef, []EvidenceSubmission{ + {EvidenceType: "document", MediaType: mediaType}, + }) + assert.NoError(t, err, "expected %s to be accepted for document", mediaType) + } +} + type mockStepAssignmentService struct { called bool } From c26f95f8ee66215c315af7af807128624ec10b0f Mon Sep 17 00:00:00 2001 From: James Salt Date: Thu, 28 May 2026 09:33:56 +0100 Subject: [PATCH 2/2] BCH-1150: Address PR review comments - normalize media type and align test coverage Add image/jpg to acceptance test loop to match the allowlist. Normalize media type (lowercase, strip parameters) before allowlist lookup so that values like Image/PNG or image/png; charset=binary are accepted. Co-Authored-By: Claude Sonnet 4.6 --- internal/workflow/step_transition.go | 4 +++- internal/workflow/step_transition_test.go | 21 ++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/internal/workflow/step_transition.go b/internal/workflow/step_transition.go index 20a352d4..be8a5356 100644 --- a/internal/workflow/step_transition.go +++ b/internal/workflow/step_transition.go @@ -302,7 +302,9 @@ func (s *StepTransitionService) validateEvidenceRequirements(stepDef *workflows. // Validate file type compatibility per submission. for _, evidence := range submittedEvidence { if evidence.EvidenceType == "screenshot" && evidence.MediaType != "" { - if !screenshotAllowedMediaTypes[evidence.MediaType] { + // Normalize: lowercase and strip parameters (e.g. "Image/PNG; charset=binary" → "image/png") + normalized := strings.ToLower(strings.TrimSpace(strings.SplitN(evidence.MediaType, ";", 2)[0])) + if !screenshotAllowedMediaTypes[normalized] { return fmt.Errorf("evidence type 'screenshot' requires an image file (png/jpg/jpeg/gif/webp), got %q", evidence.MediaType) } } diff --git a/internal/workflow/step_transition_test.go b/internal/workflow/step_transition_test.go index 98e6f28f..c7da08d2 100644 --- a/internal/workflow/step_transition_test.go +++ b/internal/workflow/step_transition_test.go @@ -301,7 +301,7 @@ func TestValidateEvidenceRequirements_ScreenshotAcceptsImageMediaType(t *testing }, } - for _, imageType := range []string{"image/png", "image/jpeg", "image/gif", "image/webp"} { + for _, imageType := range []string{"image/png", "image/jpeg", "image/jpg", "image/gif", "image/webp"} { err := svc.validateEvidenceRequirements(stepDef, []EvidenceSubmission{ {EvidenceType: "screenshot", MediaType: imageType}, }) @@ -309,6 +309,25 @@ func TestValidateEvidenceRequirements_ScreenshotAcceptsImageMediaType(t *testing } } +// BCH-1150: media type matching must be case-insensitive and strip parameters so that +// values like "Image/PNG" or "image/png; charset=binary" are accepted for screenshot evidence. +func TestValidateEvidenceRequirements_ScreenshotAcceptsNormalizedMediaTypes(t *testing.T) { + svc := NewStepTransitionService(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + + stepDef := &workflows.WorkflowStepDefinition{ + EvidenceRequired: []workflows.EvidenceRequirement{ + {Type: "screenshot", Required: true}, + }, + } + + for _, imageType := range []string{"Image/PNG", "IMAGE/JPEG", "image/png; charset=binary"} { + err := svc.validateEvidenceRequirements(stepDef, []EvidenceSubmission{ + {EvidenceType: "screenshot", MediaType: imageType}, + }) + assert.NoError(t, err, "expected normalized %q to be accepted for screenshot", imageType) + } +} + // BCH-1150: document evidence should accept a broader set of media types including PDF and images. func TestValidateEvidenceRequirements_DocumentAcceptsBroadMediaTypes(t *testing.T) { svc := NewStepTransitionService(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)