Skip to content

Commit

Permalink
fix(*-attributes): fill gaps in (array|property|string)-attributes ru…
Browse files Browse the repository at this point in the history
…les (#670)

Issues fixed:
- Not all of the keywords were considered invalid for non-scoped schemas
- min/max comparison didn't take 0 as a falsy value into account
- Only input schemas were considered for keyword checks on non-strings

Signed-off-by: Dustin Popp <[email protected]>
  • Loading branch information
dpopp07 authored Jun 21, 2024
1 parent 7c6484e commit 30e2de7
Show file tree
Hide file tree
Showing 9 changed files with 452 additions and 55 deletions.
4 changes: 2 additions & 2 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"files": "package-lock.json|^.secrets.baseline$",
"lines": null
},
"generated_at": "2024-05-20T16:10:39Z",
"generated_at": "2024-06-21T19:45:54Z",
"plugins_used": [
{
"name": "AWSKeyDetector"
Expand Down Expand Up @@ -106,7 +106,7 @@
}
]
},
"version": "0.13.1+ibm.62.dss",
"version": "0.13.1+ibm.56.dss",
"word_list": {
"file": null,
"hash": null
Expand Down
8 changes: 7 additions & 1 deletion docs/ibm-cloud-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ paths:
</tr>
<tr>
<td valign=top><b>Description:</b></td>
<td>Array schemas must define the <code>items</code> field, and should define the <code>minItems</code> and <code>maxItems</code> fields.
<td>Array schemas must define the <code>items</code> field, and should define the <code>minItems</code> and <code>maxItems</code> fields. Non-arrays must not define array keywords.
[<a href="https://cloud.ibm.com/docs/api-handbook?topic=api-handbook-types#array">1</a>].</td>
</tr>
<tr>
Expand Down Expand Up @@ -4911,6 +4911,9 @@ paths:
<li><code>minimum</code> must not be greater than <code>maximum</code>.</li>
<li><code>minimum</code> must not be defined for a schema type other than <code>integer</code> or <code>number</code>.</li>
<li><code>maximum</code> must not be defined for a schema type other than <code>integer</code> or <code>number</code>.</li>
<li><code>multipleOf</code> must not be defined for a schema type other than <code>integer</code> or <code>number</code>.</li>
<li><code>exclusiveMaximum</code> must not be defined for a schema type other than <code>integer</code> or <code>number</code>.</li>
<li><code>exclusiveMinimum</code> must not be defined for a schema type other than <code>integer</code> or <code>number</code>.</li>
</ul>
</dd>
<dt>Object schemas (type=object):</dt>
Expand All @@ -4919,6 +4922,9 @@ paths:
<li><code>minProperties</code> must not be greater than <code>maxProperties</code>.</li>
<li><code>minProperties</code> must not be defined for a schema type other than <code>object</code>.</li>
<li><code>maxProperties</code> must not be defined for a schema type other than <code>object</code>.</li>
<li><code>additionalProperties</code> must not be defined for a schema type other than <code>object</code>.</li>
<li><code>properties</code> must not be defined for a schema type other than <code>object</code>.</li>
<li><code>required</code> must not be defined for a schema type other than <code>object</code>.</li>
</ul>
</dd>
</dl>
Expand Down
20 changes: 18 additions & 2 deletions packages/ruleset/src/functions/array-attributes.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

Expand Down Expand Up @@ -64,6 +64,16 @@ function arrayAttributeErrors(schema, path) {
});
}

// Is enum defined? Shouldn't be
const enm = getCompositeSchemaAttribute(schema, 'enum');
if (isDefined(enm)) {
logger.debug('enum field is present!');
errors.push({
message: `Array schemas should not define an 'enum' field`,
path: [...path, 'enum'],
});
}

// Is items defined?
const items = getCompositeSchemaAttribute(schema, 'items');
if (!isDefined(items) || !isPlainObject(items)) {
Expand All @@ -76,7 +86,7 @@ function arrayAttributeErrors(schema, path) {
];
}
} else {
// minItems/maxItems should not be defined for a non-array schema
// minItems/maxItems/items should not be defined for a non-array schema
if (schema.minItems) {
errors.push({
message: `'minItems' should not be defined for a non-array schema`,
Expand All @@ -89,6 +99,12 @@ function arrayAttributeErrors(schema, path) {
path: [...path, 'maxItems'],
});
}
if (schema.items) {
errors.push({
message: `'items' should not be defined for a non-array schema`,
path: [...path, 'items'],
});
}
}

return errors;
Expand Down
96 changes: 75 additions & 21 deletions packages/ruleset/src/functions/property-attributes.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
validateSubschemas,
isBooleanSchema,
isNumberSchema,
isIntegerSchema,
isFloatSchema,
isDoubleSchema,
isObjectSchema,
} = require('@ibm-cloud/openapi-ruleset-utilities');
const { LoggerFactory } = require('../utils');
Expand All @@ -26,11 +25,11 @@ module.exports = function (schema, _opts, context) {
/**
* This rule performs the following checks on each schema (and schema property)
* found in the API definition:
* 1) minimum/maximum should not be defined for a non-numeric (number, integer) schema
* 1) Number-scope keywords should not be defined for a non-numeric (number, integer) schema
* 2) minimum <= maximum
* 3) minItems/maxItems should not be defined for a non-array schema
* 4) minProperties/maxProperties should not be defined for a non-object schema
* 4) Object-scope keywords should not be defined for a non-object schema
* 5) minProperties <= maxProperties
* 6) enum field should not be present for object or boolean schemas
*
* @param {*} schema the schema to check
* @param {*} path the array of path segments indicating the "location" of the schema within the API definition
Expand All @@ -47,66 +46,121 @@ function checkPropertyAttributes(schema, path) {
logger.debug('schema is numeric');

// 2) minimum <= maximum
if (schema.minimum && schema.maximum && schema.minimum > schema.maximum) {
if (
'minimum' in schema &&
'maximum' in schema &&
schema.minimum > schema.maximum
) {
errors.push({
message: `'minimum' cannot be greater than 'maximum'`,
path: [...path, 'minimum'],
});
}
} else {
// 1) minimum/maximum should not be defined for a non-numeric (number, integer) schema
if (schema.minimum) {
// 1) minimum/maximum/multipleOf/exclusiveMaximum/exclusiveMinimum
// should not be defined for a non-numeric (number, integer) schema
if ('minimum' in schema) {
errors.push({
message: `'minimum' should not be defined for non-numeric schemas`,
path: [...path, 'minimum'],
});
}
if (schema.maximum) {
if ('maximum' in schema) {
errors.push({
message: `'maximum' should not be defined for non-numeric schemas`,
path: [...path, 'maximum'],
});
}
if ('multipleOf' in schema) {
errors.push({
message: `'multipleOf' should not be defined for non-numeric schemas`,
path: [...path, 'multipleOf'],
});
}
if ('exclusiveMaximum' in schema) {
errors.push({
message: `'exclusiveMaximum' should not be defined for non-numeric schemas`,
path: [...path, 'exclusiveMaximum'],
});
}
if ('exclusiveMinimum' in schema) {
errors.push({
message: `'exclusiveMinimum' should not be defined for non-numeric schemas`,
path: [...path, 'exclusiveMinimum'],
});
}
}

if (isObjectSchema(schema)) {
logger.debug('schema is an object schema');

// 5) minProperties <= maxProperties
if (
schema.minProperties &&
schema.maxProperties &&
'minProperties' in schema &&
'maxProperties' in schema &&
schema.minProperties > schema.maxProperties
) {
errors.push({
message: `'minProperties' cannot be greater than 'maxProperties'`,
path: [...path, 'minProperties'],
});
}

// 6) enum should not be present
if ('enum' in schema) {
errors.push({
message: `'enum' should not be defined for object schemas`,
path: [...path, 'enum'],
});
}
} else {
// 4) minProperties/maxProperties should not be defined for a non-object schema
if (schema.minProperties) {
// 4) minProperties/maxProperties/additionalProperties/properties/required
// should not be defined for a non-object schema
if ('minProperties' in schema) {
errors.push({
message: `'minProperties' should not be defined for non-object schemas`,
path: [...path, 'minProperties'],
});
}
if (schema.maxProperties) {
if ('maxProperties' in schema) {
errors.push({
message: `'maxProperties' should not be defined for non-object schemas`,
path: [...path, 'maxProperties'],
});
}
if ('additionalProperties' in schema) {
errors.push({
message: `'additionalProperties' should not be defined for non-object schemas`,
path: [...path, 'additionalProperties'],
});
}
if ('properties' in schema) {
errors.push({
message: `'properties' should not be defined for non-object schemas`,
path: [...path, 'properties'],
});
}
if ('required' in schema) {
errors.push({
message: `'required' should not be defined for non-object schemas`,
path: [...path, 'required'],
});
}
}

if (isBooleanSchema(schema)) {
// 6) enum should not be present
if ('enum' in schema) {
errors.push({
message: `'enum' should not be defined for boolean schemas`,
path: [...path, 'enum'],
});
}
}

return errors;
}

function isNumericSchema(s) {
return (
isNumberSchema(s) ||
isIntegerSchema(s) ||
isFloatSchema(s) ||
isDoubleSchema(s)
);
return isNumberSchema(s) || isIntegerSchema(s);
}
29 changes: 25 additions & 4 deletions packages/ruleset/src/functions/string-attributes.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

Expand All @@ -9,7 +9,11 @@ const {
validateNestedSchemas,
} = require('@ibm-cloud/openapi-ruleset-utilities');

const { getCompositeSchemaAttribute, LoggerFactory } = require('../utils');
const {
getCompositeSchemaAttribute,
LoggerFactory,
pathMatchesRegexp,
} = require('../utils');

let ruleId;
let logger;
Expand Down Expand Up @@ -49,7 +53,9 @@ function stringBoundaryErrors(schema, path) {

const errors = [];

if (isStringSchema(schema)) {
// Only check for the presence of validation keywords on input schemas
// (i.e. those used for parameters and request bodies).
if (isStringSchema(schema) && isInputSchema(path)) {
logger.debug('schema is a string schema');

// Perform these checks only if enum is not defined.
Expand Down Expand Up @@ -89,7 +95,10 @@ function stringBoundaryErrors(schema, path) {
});
}
}
} else {
}

// Make sure string attributes aren't used for non-strings.
if (!isStringSchema(schema)) {
// Make sure that string-related fields are not present in a non-string schema.
if (schemaContainsAttribute(schema, 'pattern')) {
errors.push({
Expand All @@ -110,6 +119,7 @@ function stringBoundaryErrors(schema, path) {
});
}
}

return errors;
}

Expand All @@ -125,3 +135,14 @@ function schemaContainsAttribute(schema, attrName) {
s => attrName in s && isDefined(s[attrName])
);
}

function isInputSchema(path) {
// Output schemas are much simpler to check for with regex.
// Use the inverse of that to determine input schemas.
const isOutputSchema = pathMatchesRegexp(
path,
/^paths,.*,responses,.+,(content|headers),.+,schema/
);

return !isOutputSchema;
}
13 changes: 5 additions & 8 deletions packages/ruleset/src/rules/string-attributes.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/**
* Copyright 2017 - 2023 IBM Corporation.
* Copyright 2017 - 2024 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
schemas,
} = require('@ibm-cloud/openapi-ruleset-utilities/src/collections');
const { oas3 } = require('@stoplight/spectral-formats');
const { stringAttributes } = require('../functions');

Expand All @@ -12,13 +15,7 @@ module.exports = {
severity: 'warn',
formats: [oas3],
resolved: true,
given: [
'$.paths[*][parameters][*].schema',
'$.paths[*][parameters][*].content[*].schema',
'$.paths[*][*][parameters][*].schema',
'$.paths[*][*][parameters][*].content[*].schema',
'$.paths[*][*].requestBody.content[*].schema',
],
given: schemas,
then: {
function: stringAttributes,
},
Expand Down
Loading

0 comments on commit 30e2de7

Please sign in to comment.