Added new operation 4xx response rule

Checks for at least a single request error defined for each operation.
Had to update the burgershop spec, as it fell out of favor. added a silly 4xx code for fun.

Signed-off-by: Dave Shanley <dave@quobix.com>

functions/functions.go

@@ -96,6 +96,7 @@ func MapBuiltinFunctions() Functions {
 		funcs["oasDocumentSchema"] = openapi_functions.OASSchema{}
 		funcs["oasAPIServers"] = openapi_functions.APIServers{}
 		funcs["noAmbiguousPaths"] = openapi_functions.AmbiguousPaths{}
+		funcs["oasOpErrorResponse"] = openapi_functions.Operation4xResponse{}
 
 	})
 

functions/functions_test.go

@@ -7,5 +7,5 @@ import (
 
 func TestMapBuiltinFunctions(t *testing.T) {
 	funcs := MapBuiltinFunctions()
-	assert.Len(t, funcs.GetAllFunctions(), 39)
+	assert.Len(t, funcs.GetAllFunctions(), 40)
 }

functions/openapi/operation_4x_response.go

@@ -0,0 +1,75 @@
+// Copyright 2020-2021 Dave Shanley / Quobix
+// SPDX-License-Identifier: MIT
+
+package openapi
+
+import (
+	"fmt"
+	"github.com/daveshanley/vacuum/model"
+	"github.com/daveshanley/vacuum/utils"
+	"gopkg.in/yaml.v3"
+	"strconv"
+)
+
+// Operation4xResponse is a rule that checks if an operation returns a 4xx (user error) code.
+type Operation4xResponse struct {
+}
+
+// GetSchema returns a model.RuleFunctionSchema defining the schema of the SuccessResponse rule.
+func (or Operation4xResponse) GetSchema() model.RuleFunctionSchema {
+	return model.RuleFunctionSchema{Name: "operation_4xx_response"}
+}
+
+// RunRule will execute the Operation4xResponse rule, based on supplied context and a supplied []*yaml.Node slice.
+func (or Operation4xResponse) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) []model.RuleFunctionResult {
+
+	if len(nodes) <= 0 {
+		return nil
+	}
+
+	var results []model.RuleFunctionResult
+
+	if context.Index.GetPathsNode() == nil {
+		return results
+	}
+	ops := context.Index.GetPathsNode().Content
+
+	var opPath, opMethod string
+	for i, op := range ops {
+		if i%2 == 0 {
+			opPath = op.Value
+			continue
+		}
+		for m, method := range op.Content {
+			if m%2 == 0 {
+				opMethod = method.Value
+				continue
+			}
+			basePath := fmt.Sprintf("$.paths.%s.%s", opPath, opMethod)
+			_, responsesNode := utils.FindKeyNode("responses", method.Content)
+
+			if responsesNode != nil {
+				seen := false
+				for k, response := range responsesNode.Content {
+					if k%2 != 0 {
+						continue
+					}
+					responseCode, _ := strconv.Atoi(response.Value)
+					if responseCode >= 400 && responseCode <= 499 {
+						seen = true
+					}
+				}
+				if !seen {
+					results = append(results, model.RuleFunctionResult{
+						Message:   "Operation must define at least one 4xx error response",
+						StartNode: method,
+						EndNode:   utils.FindLastChildNode(method),
+						Path:      basePath,
+						Rule:      context.Rule,
+					})
+				}
+			}
+		}
+	}
+	return results
+}

functions/openapi/operation_4x_response_test.go

@@ -0,0 +1,72 @@
+package openapi
+
+import (
+	"github.com/daveshanley/vacuum/model"
+	"github.com/daveshanley/vacuum/utils"
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v3"
+	"io/ioutil"
+	"testing"
+)
+
+func TestOperation4xResponse_GetSchema(t *testing.T) {
+	def := Operation4xResponse{}
+	assert.Equal(t, "operation_4xx_response", def.GetSchema().Name)
+}
+
+func TestOperation4xResponse_RunRule(t *testing.T) {
+	def := Operation4xResponse{}
+	res := def.RunRule(nil, model.RuleFunctionContext{})
+	assert.Len(t, res, 0)
+}
+
+func TestOperation4xResponse_RunRule_Success(t *testing.T) {
+
+	sampleYaml, _ := ioutil.ReadFile("../../model/test_files/burgershop.openapi.yaml")
+
+	var rootNode yaml.Node
+	yaml.Unmarshal(sampleYaml, &rootNode)
+	nodes, _ := utils.FindNodes(sampleYaml, "$")
+	rule := buildOpenApiTestRuleAction(GetAllOperationsJSONPath(), "xor", "responses", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	ctx.Index = model.NewSpecIndex(&rootNode)
+
+	def := Operation4xResponse{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 0)
+}
+
+func TestOperation4xResponse_RunRule_ExitEarly(t *testing.T) {
+
+	sampleYaml := []byte("hi: there")
+
+	var rootNode yaml.Node
+	yaml.Unmarshal(sampleYaml, &rootNode)
+	nodes, _ := utils.FindNodes(sampleYaml, "$")
+	rule := buildOpenApiTestRuleAction(GetAllOperationsJSONPath(), "xor", "responses", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	ctx.Index = model.NewSpecIndex(&rootNode)
+
+	def := Operation4xResponse{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 0)
+}
+
+func TestOperation4xResponse_RunRule_Fail(t *testing.T) {
+
+	sampleYaml, _ := ioutil.ReadFile("../../model/test_files/stripe.yaml")
+
+	var rootNode yaml.Node
+	yaml.Unmarshal(sampleYaml, &rootNode)
+	nodes, _ := utils.FindNodes(sampleYaml, "$")
+	rule := buildOpenApiTestRuleAction(GetAllOperationsJSONPath(), "xor", "responses", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	ctx.Index = model.NewSpecIndex(&rootNode)
+
+	def := Operation4xResponse{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 402)
+}

model/spec_index_test.go

@@ -180,7 +180,7 @@ func TestSpecIndex_BurgerShop(t *testing.T) {
 
 	assert.Equal(t, 5, len(index.GetAllSchemas()))
 
-	assert.Equal(t, 17, len(index.GetAllSequencedReferences()))
+	assert.Equal(t, 18, len(index.GetAllSequencedReferences()))
 	assert.NotNil(t, index.GetSchemasNode())
 	assert.Nil(t, index.GetParametersNode())
 

model/test_files/burgershop.openapi.yaml

@@ -259,6 +259,14 @@ paths:
                   $ref: '#/components/schemas/Dressing'
               example:
                 - name: Burger Sauce
+        "418":
+          description: I am a teapot.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+              example:
+                message: It's teapot time.
         "500":
           description: Something went wrong with getting dressings.
           content:

motor/rule_tests/openapi_rule_test.go

@@ -68,7 +68,7 @@ func Test_Default_OpenAPIRuleSet_FireABunchOfIssues(t *testing.T) {
 	results, err := motor.ApplyRules(rs.GenerateOpenAPIDefaultRuleSet(), []byte(badDoc))
 	assert.NoError(t, err)
 	assert.NotNil(t, results)
-	assert.Len(t, results, 28)
+	assert.Len(t, results, 29)
 
 	for n := 0; n < len(results); n++ {
 		assert.NotNil(t, results[n].Path)

rulesets/rule_fixes.go

@@ -142,4 +142,8 @@ const (
 	ambiguousPathsFix string = "Paths must all resolve unambiguously, they can't be confused with one another (/{id}/ambiguous and /ambiguous/{id} " +
 		"are the same thing. Make sure every path and the variables used are unique and do conflict with one another. Check the ordering of variables " +
 		"and the naming of path segments."
+
+	operationsErrorResponseFix string = "Make sure each operation defines at least one 4xx error response. 4xx Errors are " +
+		"used to inform clients they are using the API incorrectly, with bad input, or malformed requests. An API with no errors" +
+		"defined is really hard to navigate."
 )

rulesets/ruleset_functions.go

@@ -813,7 +813,7 @@ func GetOAS2SchemaRule() *model.Rule {
 		Description:  "OpenAPI 2 specification is invalid",
 		Given:        "$",
 		Resolved:     false,
-		Recommended:  false,
+		Recommended:  true,
 		RuleCategory: model.RuleCategories[model.CategoryValidation],
 		Type:         validation,
 		Severity:     err,
@@ -833,8 +833,8 @@ func GetOAS3SchemaRule() *model.Rule {
 		Description:  "OpenAPI 3 specification is invalid",
 		Given:        "$",
 		Resolved:     false,
-		Recommended:  false,
-		RuleCategory: model.RuleCategories[model.CategoryValidation],
+		Recommended:  true,
+		RuleCategory: model.RuleCategories[model.CategorySchemas],
 		Type:         validation,
 		Severity:     err,
 		Then: model.RuleAction{
@@ -1149,3 +1149,23 @@ func NoAmbiguousPaths() *model.Rule {
 		HowToFix: ambiguousPathsFix,
 	}
 }
+
+// GetOperationErrorResponseRule will return the rule for checking for a 4xx response defined in operations.
+func GetOperationErrorResponseRule() *model.Rule {
+	return &model.Rule{
+		Name:         "Operations must return at least 4xx user error response",
+		Id:           operationErrorResponse,
+		Formats:      model.AllFormats,
+		Description:  "Make sure operations return at least one 4xx error response to help with bad requests",
+		Given:        "$.paths",
+		Resolved:     true,
+		Recommended:  true,
+		RuleCategory: model.RuleCategories[model.CategoryOperations],
+		Type:         validation,
+		Severity:     warn,
+		Then: model.RuleAction{
+			Function: "oasOpErrorResponse",
+		},
+		HowToFix: operationsErrorResponseFix,
+	}
+}

rulesets/rulesets.go

@@ -30,6 +30,7 @@ const (
 	allOperations = "[?(@.get || @.post || @.put || @.patch || @.delete || @.trace || @.options || @.head)]"
 
 	noAmbiguousPaths                  = "no-ambiguous-paths"
+	operationErrorResponse            = "operation-4xx-response"
 	operationSuccessResponse          = "operation-success-response"
 	operationOperationIdUnique        = "operation-operationId-unique"
 	operationOperationId              = "operation-operationId"
@@ -299,9 +300,8 @@ func generateDefaultOpenAPIRuleSet() *RuleSet {
 	rules[oas3ValidSchemaExample] = GetOAS3ExamplesRule()
 	rules[oas2ValidSchemaExample] = GetOAS2ExamplesRule()
 	rules[noAmbiguousPaths] = NoAmbiguousPaths()
-
-	// TODO: enable for a different ruleset.
-	//rules[oas2Schema] = GetOAS2SchemaRule()
+	rules[operationErrorResponse] = GetOperationErrorResponseRule()
+	rules[oas2Schema] = GetOAS2SchemaRule()
 	rules[oas3Schema] = GetOAS3SchemaRule()
 
 	set := &RuleSet{

rulesets/rulesets_test.go

@@ -9,7 +9,7 @@ func TestBuildDefaultRuleSets(t *testing.T) {
 
 	rs := BuildDefaultRuleSets()
 	assert.NotNil(t, rs.GenerateOpenAPIDefaultRuleSet())
-	assert.Len(t, rs.GenerateOpenAPIDefaultRuleSet().Rules, 49)
+	assert.Len(t, rs.GenerateOpenAPIDefaultRuleSet().Rules, 51)
 
 }
 
@@ -121,10 +121,10 @@ func TestRuleSet_GetConfiguredRules_All(t *testing.T) {
 	// read spec and parse to dashboard.
 	rs := BuildDefaultRuleSets()
 	ruleSet := rs.GenerateOpenAPIDefaultRuleSet()
-	assert.Len(t, ruleSet.Rules, 49)
+	assert.Len(t, ruleSet.Rules, 51)
 
 	ruleSet = rs.GenerateOpenAPIRecommendedRuleSet()
-	assert.Len(t, ruleSet.Rules, 38)
+	assert.Len(t, ruleSet.Rules, 41)
 
 }
 
@@ -140,7 +140,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 38)
+	assert.Len(t, override.Rules, 41)
 	assert.Len(t, override.RuleDefinitions, 1)
 }
 
@@ -173,7 +173,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 49)
+	assert.Len(t, override.Rules, 51)
 	assert.Len(t, override.RuleDefinitions, 1)
 }
 
@@ -189,7 +189,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 37)
+	assert.Len(t, override.Rules, 40)
 	assert.Len(t, override.RuleDefinitions, 1)
 }
 
@@ -205,7 +205,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 38)
+	assert.Len(t, override.Rules, 41)
 	assert.Equal(t, "hint", override.Rules["operation-success-response"].Severity)
 }
 
@@ -265,7 +265,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	newrs := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, newrs.Rules, 50)
+	assert.Len(t, newrs.Rules, 52)
 	assert.Equal(t, true, newrs.Rules["fish-cakes"].Recommended)
 	assert.Equal(t, "yummy sea food", newrs.Rules["fish-cakes"].Description)
 
@@ -290,7 +290,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	repl := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, repl.Rules, 49)
+	assert.Len(t, repl.Rules, 51)
 	assert.Equal(t, true, repl.Rules["info-contact"].Recommended)
 	assert.Equal(t, "yummy sea food", repl.Rules["info-contact"].Description)
 
@@ -315,7 +315,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	repl := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, repl.Rules, 49)
+	assert.Len(t, repl.Rules, 51)
 	assert.Equal(t, true, repl.Rules["info-contact"].Recommended)
 	assert.Equal(t, "yummy sea food", repl.Rules["info-contact"].Description)
 

vacuum-report/vacuum_report_test.go

@@ -85,7 +85,7 @@ func TestCheckFileForVacuumReport_CompressedJSON(t *testing.T) {
 	vr, err := CheckFileForVacuumReport(j)
 	assert.NoError(t, err)
 	assert.NotNil(t, vr)
-	assert.Len(t, *vr.SpecInfo.SpecBytes, 11489)
+	assert.Len(t, *vr.SpecInfo.SpecBytes, 11730)
 }
 
 func TestCheckFileForVacuumReport_UncompressedJSON(t *testing.T) {
@@ -94,7 +94,7 @@ func TestCheckFileForVacuumReport_UncompressedJSON(t *testing.T) {
 	vr, err := CheckFileForVacuumReport(j)
 	assert.NoError(t, err)
 	assert.NotNil(t, vr)
-	assert.Len(t, *vr.SpecInfo.SpecBytes, 11489)
+	assert.Len(t, *vr.SpecInfo.SpecBytes, 11730)
 }
 
 func TestCheckFileForVacuumReport_BadJSON_Uncompressed(t *testing.T) {