Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions internal/workflow/step_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -288,8 +296,20 @@ 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 != "" {
// 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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
}

// Build a map of submitted evidence types
submittedTypes := make(map[string]int)
for _, evidence := range submittedEvidence {
Expand Down
75 changes: 75 additions & 0 deletions internal/workflow/step_transition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,81 @@ 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/jpg", "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: 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)

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
}
Expand Down
Loading