Skip to content

Commit b1b84bc

Browse files
myasnikovdaniilmyasnikovdaniil
committed
test(api): cover nested-in-additionalProperties and anyOf/oneOf/not in sanitizer regression
Address review feedback from lexfrei and coderabbitai on pkg/cmd/server/openapi_test.go: TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties only exercised the top-level, properties, items, and allOf positions, leaving the one remaining type-checker-reachable sanitizer branch untested: a boolean additionalProperties nested inside an object-form additionalProperties. Deleting that recursion line kept the whole suite green while the input panicked SchemaDeclType again (#2863). Add the nested-under-additionalproperties case (the reachable blocker, verified red->green by mutation) plus anyOf/oneOf/not cases (defense-in-depth: those branches are recursed by the sanitizer but never traversed by SchemaDeclType, so the hasBooleanAdditionalProperties assertion guards them). Rewrite the test docstring to distinguish the type-checker-reachable cases from the defense-in-depth ones, replacing the inaccurate "each case panics SchemaDeclType" note. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 244de9d commit b1b84bc

1 file changed

Lines changed: 22 additions & 10 deletions

File tree

‎pkg/cmd/server/openapi_test.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,18 +213,30 @@ func hasBooleanAdditionalProperties(s *spec.Schema) bool {
213213
// regression test for the review finding on #2867: ApplicationDefinition
214214
// schemas are untrusted input, and a user-supplied boolean additionalProperties
215215
// — declared explicitly at the top level, or nested anywhere under
216-
// properties/items/allOf — reintroduces the exact #2863 KCM crash. patchSpec
217-
// must neutralize every such node (both the true and false forms, since both
218-
// carry a nil inner schema), at any depth, so the published schema can never
219-
// crash the VAP type-checker. Each case below panics SchemaDeclType before this
220-
// sanitizer and passes after it.
216+
// properties/items/additionalProperties/allOf/anyOf/oneOf/not — reintroduces the
217+
// exact #2863 KCM crash. patchSpec must neutralize every such node (both the true
218+
// and false forms, since both carry a nil inner schema), at any depth, so the
219+
// published schema can never crash the VAP type-checker.
220+
//
221+
// Two assertions guard each case. SchemaDeclType only descends into properties,
222+
// items, and additionalProperties.Schema, so the top-level / nested-under-
223+
// properties / nested-under-items / nested-under-additionalproperties cases are
224+
// type-checker-reachable: they nil-deref (panic) SchemaDeclType on the
225+
// pre-sanitizer code. The allOf/anyOf/oneOf/not cases are defense-in-depth —
226+
// SchemaDeclType never traverses those branches, so it is the
227+
// hasBooleanAdditionalProperties assertion, not the SchemaDeclType check, that
228+
// keeps their recursion lines from regressing silently.
221229
func TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties(t *testing.T) {
222230
cases := map[string]string{
223-
"top-level-boolean-true": `{"type":"object","additionalProperties":true}`,
224-
"top-level-boolean-false": `{"type":"object","additionalProperties":false}`,
225-
"nested-under-properties": `{"type":"object","properties":{"foo":{"type":"object","additionalProperties":true}}}`,
226-
"nested-under-items": `{"type":"object","properties":{"list":{"type":"array","items":{"type":"object","additionalProperties":true}}}}`,
227-
"nested-under-allof": `{"type":"object","allOf":[{"type":"object","additionalProperties":false}]}`,
231+
"top-level-boolean-true": `{"type":"object","additionalProperties":true}`,
232+
"top-level-boolean-false": `{"type":"object","additionalProperties":false}`,
233+
"nested-under-properties": `{"type":"object","properties":{"foo":{"type":"object","additionalProperties":true}}}`,
234+
"nested-under-items": `{"type":"object","properties":{"list":{"type":"array","items":{"type":"object","additionalProperties":true}}}}`,
235+
"nested-under-additionalproperties": `{"type":"object","additionalProperties":{"type":"object","additionalProperties":true}}`,
236+
"nested-under-allof": `{"type":"object","allOf":[{"type":"object","additionalProperties":false}]}`,
237+
"nested-under-anyof": `{"type":"object","anyOf":[{"type":"object","additionalProperties":true}]}`,
238+
"nested-under-oneof": `{"type":"object","oneOf":[{"type":"object","additionalProperties":false}]}`,
239+
"nested-under-not": `{"type":"object","not":{"type":"object","additionalProperties":true}}`,
228240
}
229241

230242
for name, raw := range cases {

0 commit comments

Comments
 (0)