Skip to content

Commit bec3b87

Browse files
fredbifredbi
authored
Merge pull request #290 from fredbi/security-req-rule
Security req rule
2 parents 2f0ae12 + b677c6f commit bec3b87

11 files changed

Lines changed: 1192 additions & 30 deletions

‎.golangci.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ linters:
1010
- gomodguard
1111
- gomodguard_v2
1212
- exhaustruct
13+
- exhaustruct_v5
1314
- ireturn
1415
- nlreturn
1516
- nestif

‎collection_format.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package validate
5+
6+
import (
7+
"fmt"
8+
"strconv"
9+
10+
"github.com/go-openapi/spec"
11+
)
12+
13+
// validateCollectionFormats warns about a collectionFormat that has no effect.
14+
//
15+
// collectionFormat says how to join the members of an array into one value on the wire — csv, ssv,
16+
// tsv, pipes, or multi for a parameter repeated once per member:
17+
//
18+
// parameters:
19+
// - name: tags
20+
// in: query
21+
// type: array
22+
// items: { type: string }
23+
// collectionFormat: pipes
24+
//
25+
// There is nothing to join when the type is not array, so a collectionFormat written on a string or
26+
// an integer does nothing. Swagger 2.0 says the member "determines the format of the array if type
27+
// array is used" and stops there — it never forbids writing it elsewhere, so this is a warning and
28+
// the specification stays valid.
29+
//
30+
// The meta-schema already covers the rest of the collectionFormat rules, and covers them at every
31+
// location: the value must be one of csv, ssv, tsv or pipes, widened with multi for a query or
32+
// formData parameter, where repeating the parameter is possible. A body parameter cannot carry the
33+
// member at all. None of that needs a rule here.
34+
//
35+
// Only a parameter, a header and an items carry a collectionFormat — [spec.SimpleSchema] holds it,
36+
// and [spec.Schema] has no such member. So this walks what the operations of the document declare,
37+
// the way [SpecValidator.validateItems] does, and never looks at a schema. A JSON schema validated
38+
// on its own is untouched by this rule.
39+
func (s *SpecValidator) validateCollectionFormats() *Result {
40+
res := validatorPools.results.Borrow()
41+
42+
operations := s.analyzer.Operations()
43+
for _, method := range sortedKeys(operations) {
44+
byPath := operations[method]
45+
for _, path := range sortedKeys(byPath) {
46+
op := byPath[path]
47+
48+
for _, param := range paramHelp.safeExpandedParamsFor(path, method, op.ID, res, s) {
49+
if param.In == swaggerBody {
50+
// a body parameter describes itself with a schema, and the meta-schema
51+
// forbids it a collectionFormat outright
52+
continue
53+
}
54+
55+
at := s.parameterPath(path, method, param.In, param.Name)
56+
in := fmt.Sprintf("parameter %q", param.Name)
57+
checkCollectionFormat(at, param.CollectionFormat, param.Type, in, res)
58+
checkItemsCollectionFormats(at, param.Items, in, res)
59+
}
60+
61+
for _, response := range responsesOf(op) {
62+
at := responsePath(path, method, response.code)
63+
for _, name := range sortedKeys(response.resp.Headers) {
64+
header := response.resp.Headers[name]
65+
headerAt := at.children(swaggerHeaders, name)
66+
in := fmt.Sprintf("header %q", name)
67+
checkCollectionFormat(headerAt, header.CollectionFormat, header.Type, in, res)
68+
checkItemsCollectionFormats(headerAt, header.Items, in, res)
69+
}
70+
}
71+
}
72+
}
73+
74+
return res
75+
}
76+
77+
// codedResponse is a response together with the code the operation files it under, "default"
78+
// included.
79+
type codedResponse struct {
80+
code string
81+
resp spec.Response
82+
}
83+
84+
// responsesOf lists the responses an operation declares, in a settled order: the default response
85+
// first, then the status codes in ascending order.
86+
func responsesOf(op *spec.Operation) []codedResponse {
87+
if op == nil || op.Responses == nil {
88+
return nil
89+
}
90+
91+
var responses []codedResponse
92+
if op.Responses.Default != nil {
93+
responses = append(responses, codedResponse{code: jsonDefault, resp: *op.Responses.Default})
94+
}
95+
96+
for _, code := range sortedKeys(op.Responses.StatusCodeResponses) {
97+
responses = append(responses, codedResponse{
98+
code: strconv.Itoa(code),
99+
resp: op.Responses.StatusCodeResponses[code],
100+
})
101+
}
102+
103+
return responses
104+
}
105+
106+
// checkItemsCollectionFormats checks the items of a parameter or header, then the items of those
107+
// items, as deep as the document nests them.
108+
//
109+
// Every level is named the same way in a message: an array of arrays that writes a pointless
110+
// collectionFormat twice is one thing to fix, and the deeper location is reported only when the
111+
// shallower one is sound.
112+
func checkItemsCollectionFormats(at pathSegments, items *spec.Items, in string, res *Result) {
113+
for items != nil {
114+
at = at.child(jsonItems)
115+
checkCollectionFormat(at, items.CollectionFormat, items.Type, "items of "+in, res)
116+
items = items.Items
117+
}
118+
}
119+
120+
// checkCollectionFormat warns when a collectionFormat is written on something that is not an array.
121+
//
122+
// A missing collectionFormat has nothing to answer for, and neither has a missing type: a document
123+
// that leaves the type out is already reported by the meta-schema, and guessing what it meant here
124+
// would only add noise.
125+
func checkCollectionFormat(at pathSegments, collectionFormat, typ, in string, res *Result) {
126+
if collectionFormat == "" || typ == "" || typ == arrayType {
127+
return
128+
}
129+
130+
res.addWarningsAt(at.child(swaggerCollectionFormat), collectionFormatIgnoredMsg(collectionFormat, in, typ))
131+
}

‎collection_format_test.go‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package validate
5+
6+
import (
7+
"encoding/json"
8+
"fmt"
9+
"testing"
10+
11+
"github.com/go-openapi/analysis"
12+
"github.com/go-openapi/loads"
13+
"github.com/go-openapi/spec"
14+
"github.com/go-openapi/strfmt"
15+
"github.com/go-openapi/testify/v2/assert"
16+
"github.com/go-openapi/testify/v2/require"
17+
)
18+
19+
// collectionFormatSpec builds a specification whose only operation declares the given parameters
20+
// and the given headers on its 200 response.
21+
func collectionFormatSpec(parameters, headers string) string {
22+
return fmt.Sprintf(`{
23+
"swagger": "2.0",
24+
"info": {"title": "t", "version": "1"},
25+
"paths": {
26+
"/x": {
27+
"get": {
28+
"parameters": [%s],
29+
"responses": {"200": {"description": "ok", "headers": {%s}}}
30+
}
31+
}
32+
}
33+
}`, parameters, headers)
34+
}
35+
36+
// collectionFormatValidatorFromJSON builds a SpecValidator wired the way Validate does, up to the
37+
// point validateCollectionFormats needs.
38+
func collectionFormatValidatorFromJSON(t *testing.T, doc string) *SpecValidator {
39+
t.Helper()
40+
41+
d, err := loads.Analyzed(json.RawMessage(doc), "")
42+
require.NoError(t, err)
43+
44+
s := NewSpecValidator(d.Schema(), strfmt.Default)
45+
s.spec = d
46+
s.analyzer = analysis.New(d.Spec())
47+
s.paramLocations = newParamLocations(d.Spec())
48+
49+
return s
50+
}
51+
52+
func TestValidateCollectionFormats(t *testing.T) {
53+
t.Parallel()
54+
55+
tests := []struct {
56+
name string
57+
parameters string
58+
headers string
59+
wantWarnings []string
60+
}{
61+
{
62+
name: "collectionFormat on an array",
63+
parameters: `{"name":"q","in":"query","type":"array","items":{"type":"string"},"collectionFormat":"pipes"}`,
64+
},
65+
{
66+
name: "array without a collectionFormat",
67+
parameters: `{"name":"q","in":"query","type":"array","items":{"type":"string"}}`,
68+
},
69+
{
70+
name: "no collectionFormat anywhere",
71+
parameters: `{"name":"q","in":"query","type":"string"}`,
72+
},
73+
{
74+
name: "collectionFormat on a string parameter",
75+
parameters: `{"name":"q","in":"query","type":"string","collectionFormat":"csv"}`,
76+
wantWarnings: []string{
77+
`collectionFormat "csv" is ignored in parameter "q": it joins the members of an array, and the type is "string"`,
78+
},
79+
},
80+
{
81+
name: "collectionFormat on an integer parameter",
82+
parameters: `{"name":"q","in":"query","type":"integer","collectionFormat":"pipes"}`,
83+
wantWarnings: []string{
84+
`collectionFormat "pipes" is ignored in parameter "q": it joins the members of an array, and the type is "integer"`,
85+
},
86+
},
87+
{
88+
name: "collectionFormat on a file parameter",
89+
parameters: `{"name":"f","in":"formData","type":"file","collectionFormat":"csv"}`,
90+
wantWarnings: []string{
91+
`collectionFormat "csv" is ignored in parameter "f": it joins the members of an array, and the type is "file"`,
92+
},
93+
},
94+
{
95+
name: "collectionFormat on the items of an array",
96+
parameters: `{"name":"q","in":"query","type":"array","items":{"type":"string","collectionFormat":"ssv"}}`,
97+
wantWarnings: []string{
98+
`collectionFormat "ssv" is ignored in items of parameter "q": it joins the members of an array, and the type is "string"`,
99+
},
100+
},
101+
{
102+
name: "collectionFormat on the items of an array of arrays",
103+
parameters: `{"name":"q","in":"query","type":"array","items":{"type":"array","items":{"type":"string"},"collectionFormat":"tsv"}}`,
104+
},
105+
{
106+
name: "collectionFormat on a string response header",
107+
headers: `"X":{"type":"string","collectionFormat":"csv"}`,
108+
wantWarnings: []string{
109+
`collectionFormat "csv" is ignored in header "X": it joins the members of an array, and the type is "string"`,
110+
},
111+
},
112+
{
113+
name: "collectionFormat on an array response header",
114+
headers: `"X":{"type":"array","items":{"type":"string"},"collectionFormat":"csv"}`,
115+
},
116+
{
117+
name: "collectionFormat on the items of a response header",
118+
headers: `"X":{"type":"array","items":{"type":"integer","collectionFormat":"pipes"}}`,
119+
wantWarnings: []string{
120+
`collectionFormat "pipes" is ignored in items of header "X": it joins the members of an array, and the type is "integer"`,
121+
},
122+
},
123+
}
124+
125+
for _, tc := range tests {
126+
t.Run(tc.name, func(t *testing.T) {
127+
t.Parallel()
128+
129+
doc := collectionFormatSpec(tc.parameters, tc.headers)
130+
res := collectionFormatValidatorFromJSON(t, doc).validateCollectionFormats()
131+
132+
assert.Equal(t, tc.wantWarnings, nonEmpty(warningMessages(res)))
133+
assert.Empty(t, errorMessages(res), "the rule reports warnings only")
134+
})
135+
}
136+
}
137+
138+
// TestCollectionFormatKeepsSpecValid holds the rule to a warning: Swagger 2.0 says where
139+
// collectionFormat applies without forbidding it elsewhere, so a specification that writes one on a
140+
// string stays valid.
141+
func TestCollectionFormatKeepsSpecValid(t *testing.T) {
142+
t.Parallel()
143+
144+
doc := collectionFormatSpec(`{"name":"q","in":"query","type":"string","collectionFormat":"csv"}`, "")
145+
146+
d, err := loads.Analyzed(json.RawMessage(doc), "")
147+
require.NoError(t, err)
148+
149+
require.NoError(t, Spec(d, strfmt.Default))
150+
151+
_, warns := NewSpecValidator(d.Schema(), strfmt.Default).Validate(d)
152+
assert.SliceContainsT(t, errorMessages(warns),
153+
`collectionFormat "csv" is ignored in parameter "q": it joins the members of an array, and the type is "string"`)
154+
}
155+
156+
// TestCollectionFormatLocations pins down where the warning says it happened.
157+
func TestCollectionFormatLocations(t *testing.T) {
158+
t.Parallel()
159+
160+
doc := collectionFormatSpec(
161+
`{"name":"q","in":"query","type":"array","items":{"type":"string","collectionFormat":"ssv"}}`,
162+
`"X":{"type":"string","collectionFormat":"csv"}`,
163+
)
164+
165+
d, err := loads.Analyzed(json.RawMessage(doc), "")
166+
require.NoError(t, err)
167+
168+
_, warns := NewSpecValidator(d.Schema(), strfmt.Default).Validate(d)
169+
170+
pointers := pointersOf(warns.LocatedErrors())
171+
assert.SliceContainsT(t, pointers, "/paths/~1x/get/parameters/0/items/collectionFormat")
172+
assert.SliceContainsT(t, pointers, "/paths/~1x/get/responses/200/headers/X/collectionFormat")
173+
}
174+
175+
// TestCollectionFormatThroughRef covers a parameter reached through a $ref: the walk sees it
176+
// expanded, so the warning is reported just as it is for one written in place.
177+
func TestCollectionFormatThroughRef(t *testing.T) {
178+
t.Parallel()
179+
180+
doc := `{
181+
"swagger": "2.0",
182+
"info": {"title": "t", "version": "1"},
183+
"parameters": {
184+
"Q": {"name": "q", "in": "query", "type": "string", "collectionFormat": "csv"}
185+
},
186+
"paths": {
187+
"/x": {
188+
"get": {
189+
"parameters": [{"$ref": "#/parameters/Q"}],
190+
"responses": {"200": {"description": "ok"}}
191+
}
192+
}
193+
}
194+
}`
195+
196+
res := collectionFormatValidatorFromJSON(t, doc).validateCollectionFormats()
197+
assert.Equal(t, []string{
198+
`collectionFormat "csv" is ignored in parameter "q": it joins the members of an array, and the type is "string"`,
199+
}, warningMessages(res))
200+
}
201+
202+
// TestCollectionFormatSkipsBodyParameters guards the one location the meta-schema handles on its
203+
// own: a body parameter may not carry a collectionFormat at all, and that is an error rather than
204+
// this warning.
205+
func TestCollectionFormatSkipsBodyParameters(t *testing.T) {
206+
t.Parallel()
207+
208+
doc := collectionFormatSpec(`{"name":"b","in":"body","schema":{"type":"string"}}`, "")
209+
210+
res := collectionFormatValidatorFromJSON(t, doc).validateCollectionFormats()
211+
assert.Empty(t, warningMessages(res))
212+
}
213+
214+
// TestCollectionFormatIsNotASchemaKeyword holds the rule to what it walks. collectionFormat is a
215+
// member of [spec.SimpleSchema], which a parameter, a header and an items carry; [spec.Schema] has
216+
// none, so validating an ordinary JSON schema can never reach this rule.
217+
//
218+
// The guard is on the type, not on the walk: a compile-time field access is what would break if a
219+
// later version of go-openapi/spec moved the member onto Schema.
220+
func TestCollectionFormatIsNotASchemaKeyword(t *testing.T) {
221+
t.Parallel()
222+
223+
var (
224+
_ = spec.Items{}.CollectionFormat
225+
_ = spec.Header{}.CollectionFormat
226+
_ = spec.Parameter{}.CollectionFormat
227+
)
228+
229+
raw, err := json.Marshal(spec.Schema{
230+
SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{stringType}},
231+
})
232+
require.NoError(t, err)
233+
234+
var members map[string]any
235+
require.NoError(t, json.Unmarshal(raw, &members))
236+
assert.MapNotContainsT(t, members, swaggerCollectionFormat)
237+
238+
// and validating data against a schema reports nothing of the sort
239+
schema := new(spec.Schema)
240+
require.NoError(t, json.Unmarshal([]byte(`{"type": "string"}`), schema))
241+
require.NoError(t, AgainstSchema(schema, "a,b,c", strfmt.Default))
242+
}

0 commit comments

Comments
 (0)