diff --git a/functions/functions.go b/functions/functions.go index 67c0eb67..9bceef19 100644 --- a/functions/functions.go +++ b/functions/functions.go @@ -104,6 +104,7 @@ func MapBuiltinFunctions() Functions { funcs["infoLicenseUrl"] = openapi_functions.InfoLicenseURL{} funcs["infoLicenseURLSPDX"] = openapi_functions.InfoLicenseURLSPDX{} funcs["infoContactProperties"] = openapi_functions.InfoContactProperties{} + funcs["noRequestBody"] = openapi_functions.NoRequestBody{} // add owasp functions used by the owasp rules funcs["owaspHeaderDefinition"] = owasp.HeaderDefinition{} diff --git a/functions/functions_test.go b/functions/functions_test.go index 1ab49fff..74984faa 100644 --- a/functions/functions_test.go +++ b/functions/functions_test.go @@ -8,5 +8,5 @@ import ( func TestMapBuiltinFunctions(t *testing.T) { funcs := MapBuiltinFunctions() - assert.Len(t, funcs.GetAllFunctions(), 73) + assert.Len(t, funcs.GetAllFunctions(), 74) } diff --git a/functions/openapi/no_request_body.go b/functions/openapi/no_request_body.go new file mode 100644 index 00000000..bbac2b39 --- /dev/null +++ b/functions/openapi/no_request_body.go @@ -0,0 +1,72 @@ +// Copyright 2025 Dave Shanley / Quobix +// SPDX-License-Identifier: MIT + +package openapi + +import ( + "fmt" + "github.com/daveshanley/vacuum/model" + vacuumUtils "github.com/daveshanley/vacuum/utils" + "github.com/pb33f/doctor/model/high/base" + "gopkg.in/yaml.v3" + "strings" +) + +// NoRequestBody is a rule that checks operations are using tags and they are not empty. +type NoRequestBody struct { +} + +// GetSchema returns a model.RuleFunctionSchema defining the schema of the NoRequestBody rule. +func (r NoRequestBody) GetSchema() model.RuleFunctionSchema { + return model.RuleFunctionSchema{ + Name: "noRequestBody", + } +} + +// GetCategory returns the category of the TagDefined rule. +func (r NoRequestBody) GetCategory() string { + return model.FunctionCategoryOpenAPI +} + +// RunRule will execute the NoRequestBody rule, based on supplied context and a supplied []*yaml.Node slice. +func (r NoRequestBody) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) []model.RuleFunctionResult { + + var results []model.RuleFunctionResult + + if context.DrDocument == nil { + return results + } + + paths := context.DrDocument.V3Document.Paths + if paths != nil { + for pathItemPairs := paths.PathItems.First(); pathItemPairs != nil; pathItemPairs = pathItemPairs.Next() { + path := pathItemPairs.Key() + v := pathItemPairs.Value() + + for opPairs := v.GetOperations().First(); opPairs != nil; opPairs = opPairs.Next() { + method := opPairs.Key() + op := opPairs.Value() + + for _, checkedMethods := range []string{"GET", "DELETE"} { + if strings.EqualFold(method, checkedMethods) { + if op.RequestBody != nil { + + res := model.RuleFunctionResult{ + Message: vacuumUtils.SuppliedOrDefault(context.Rule.Message, fmt.Sprintf("`%s` operation should not have a requestBody defined", + strings.ToUpper(method))), + StartNode: op.Value.GoLow().KeyNode, + EndNode: vacuumUtils.BuildEndNode(op.Value.GoLow().KeyNode), + Path: fmt.Sprintf("$.paths['%s'].%s", path, method), + Rule: context.Rule, + } + results = append(results, res) + op.AddRuleFunctionResult(base.ConvertRuleResult(&res)) + } + } + } + } + } + } + return results + +} diff --git a/functions/openapi/no_request_body_test.go b/functions/openapi/no_request_body_test.go new file mode 100644 index 00000000..e4ff7b9c --- /dev/null +++ b/functions/openapi/no_request_body_test.go @@ -0,0 +1,147 @@ +package openapi + +import ( + "fmt" + "github.com/daveshanley/vacuum/model" + drModel "github.com/pb33f/doctor/model" + "github.com/pb33f/libopenapi" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestNoRequestBody_GetSchema(t *testing.T) { + def := NoRequestBody{} + assert.Equal(t, "noRequestBody", def.GetSchema().Name) +} + +func TestNoRequestBody_RunRule(t *testing.T) { + def := NoRequestBody{} + res := def.RunRule(nil, model.RuleFunctionContext{}) + assert.Len(t, res, 0) +} + +func TestNoRequestBody_RunRule_Fail(t *testing.T) { + + yml := `openapi: 3.0.1 +paths: + /melody: + post: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string + /maddox: + get: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string + delete: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string + /ember: + get: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string +` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + + m, _ := document.BuildV3Model() + path := "$" + + drDocument := drModel.NewDrDocument(m) + + rule := buildOpenApiTestRuleAction(path, "responses", "", nil) + ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule + + def := NoRequestBody{} + res := def.RunRule(nil, ctx) + assert.Len(t, res, 3) +} + +func TestNoRequestBody_RunRule_Success(t *testing.T) { + + yml := `openapi: 3.0.1 +paths: + /melody: + post: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string + /maddox: + post: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string + /ember: + patch: + requestBody: + description: "the body of the request" + content: + application/json: + schema: + properties: + id: + type: string +` + + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + + m, _ := document.BuildV3Model() + path := "$" + + drDocument := drModel.NewDrDocument(m) + + rule := buildOpenApiTestRuleAction(path, "responses", "", nil) + ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule + + def := NoRequestBody{} + res := def.RunRule(nil, ctx) + + assert.Len(t, res, 0) + +} diff --git a/model/rules.go b/model/rules.go index c4cf59f1..e2723e50 100644 --- a/model/rules.go +++ b/model/rules.go @@ -68,6 +68,7 @@ type RuleFunctionResult struct { ModelContext any `json:"-" yaml:"-"` } +// IgnoredItems is a map of the rule ID to an array of violation paths type IgnoredItems map[string][]string // RuleResultSet contains all the results found during a linting run, and all the methods required to diff --git a/rulesets/rule_fixes.go b/rulesets/rule_fixes.go index e48ed959..47862294 100644 --- a/rulesets/rule_fixes.go +++ b/rulesets/rule_fixes.go @@ -162,6 +162,8 @@ const ( schemaTypeFix string = "Make sure each schema has a value type defined. Without a type, the schema is useless" postSuccessResponseFix string = "Make sure your POST operations return a 'success' response via 2xx or 3xx response code. " + + noRequestBodyResponseFix string = "Remove 'requestBody' from HTTP GET and DELETE methods" ) const ( diff --git a/rulesets/ruleset_functions.go b/rulesets/ruleset_functions.go index 20a8f6e5..a660f7e8 100644 --- a/rulesets/ruleset_functions.go +++ b/rulesets/ruleset_functions.go @@ -1307,6 +1307,26 @@ func GetPostSuccessResponseRule() *model.Rule { Function: "postResponseSuccess", FunctionOptions: opts, }, - HowToFix: oas3HostNotExampleFix, + HowToFix: postSuccessResponseFix, + } +} + +// GetNoRequestBodyRule will check that HTTP GET and DELETE do not accept request bodies +func GetNoRequestBodyRule() *model.Rule { + return &model.Rule{ + Name: "Check GET and DELETE methods do not accept request bodies", + Id: NoRequestBody, + Formats: model.OAS3AllFormat, + Description: "HTTP GET and DELETE should not accept request bodies", + Given: "$", + Resolved: false, + RuleCategory: model.RuleCategories[model.CategoryOperations], + Recommended: true, + Type: Style, + Severity: model.SeverityWarn, + Then: model.RuleAction{ + Function: "noRequestBody", + }, + HowToFix: noRequestBodyResponseFix, } } diff --git a/rulesets/rulesets.go b/rulesets/rulesets.go index fc95bd2b..aeb31798 100644 --- a/rulesets/rulesets.go +++ b/rulesets/rulesets.go @@ -115,6 +115,7 @@ const ( OwaspConstrainedAdditionalProperties = "owasp-constrained-additionalProperties" OwaspSecurityHostsHttpsOAS3 = "owasp-security-hosts-https-oas3" PostResponseSuccess = "post-response-success" + NoRequestBody = "no-request-body" SpectralOpenAPI = "spectral:oas" SpectralOwasp = "spectral:owasp" VacuumOwasp = "vacuum:owasp" @@ -439,6 +440,7 @@ func GetAllBuiltInRules() map[string]*model.Rule { rules[Oas3ExampleExternalCheck] = GetOAS3ExamplesExternalCheck() rules[OasSchemaCheck] = GetSchemaTypeCheckRule() rules[PostResponseSuccess] = GetPostSuccessResponseRule() + rules[NoRequestBody] = GetNoRequestBodyRule() // dead. //rules[Oas2ValidSchemaExample] = GetOAS2ExamplesRule() diff --git a/rulesets/rulesets_test.go b/rulesets/rulesets_test.go index 47c26740..824ab587 100644 --- a/rulesets/rulesets_test.go +++ b/rulesets/rulesets_test.go @@ -14,9 +14,9 @@ import ( "time" ) -var totalRules = 58 +var totalRules = 59 var totalOwaspRules = 23 -var totalRecommendedRules = 46 +var totalRecommendedRules = 47 func TestBuildDefaultRuleSets(t *testing.T) { @@ -545,7 +545,7 @@ rules: rs, err := CreateRuleSetFromData([]byte(yamlA)) assert.NoError(t, err) override := def.GenerateRuleSetFromSuppliedRuleSet(rs) - assert.Len(t, override.Rules, 48) + assert.Len(t, override.Rules, 49) assert.Len(t, override.RuleDefinitions, 2) assert.NotNil(t, rs.Rules["ding"]) assert.NotNil(t, rs.Rules["dong"]) @@ -734,7 +734,7 @@ func TestRuleSet_GetExtendsLocalSpec_Multi_Chain(t *testing.T) { rs, err := CreateRuleSetFromData([]byte(yaml)) assert.NoError(t, err) override := def.GenerateRuleSetFromSuppliedRuleSet(rs) - assert.Len(t, override.Rules, 59) + assert.Len(t, override.Rules, 60) assert.Len(t, override.RuleDefinitions, 1) }