Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ibm-string-attributes): expand rule to apply to response schemas #682

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/ibm-cloud-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -6522,7 +6522,6 @@ fields in order to clearly define the set of valid values for the property.
[<a href="https://cloud.ibm.com/docs/api-handbook?topic=api-handbook-types#string">1</a>].
<p>Note that these checks are bypassed for the following scenarios:
<ul>
<li>All checks are bypassed for string schemas that are used only within an operation response.
<li>All checks are bypassed for string schemas that contain an <code>enum</code> field.</li>
<li>The check for the <code>pattern</code> field is bypassed if <code>format</code> is set to
<code>binary</code>, <code>byte</code>, <code>date</code>, <code>date-time</code>, or <code>url</code>.</li>
Expand Down
24 changes: 3 additions & 21 deletions packages/ruleset/src/functions/string-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ const {
validateNestedSchemas,
} = require('@ibm-cloud/openapi-ruleset-utilities');

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

let ruleId;
let logger;
Expand Down Expand Up @@ -55,7 +51,7 @@ function stringBoundaryErrors(schema, path) {

// 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)) {
if (isStringSchema(schema)) {
logger.debug('schema is a string schema');

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

// Make sure string attributes aren't used for non-strings.
if (!isStringSchema(schema)) {
} else {
// Make sure that string-related fields are not present in a non-string schema.
if (schemaContainsAttribute(schema, 'pattern')) {
errors.push({
Expand Down Expand Up @@ -135,14 +128,3 @@ 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;
}
57 changes: 36 additions & 21 deletions packages/ruleset/test/string-attributes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,27 +212,6 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(0);
});

it('Response body string schema has no keywords', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.paths['/v1/movies/{movie_id}'].get.responses['200'] = {
content: {
'application/json': {
schema: {
properties: {
name: {
type: 'string',
description: 'no validation',
},
},
},
},
},
};

const results = await testRule(ruleId, rule, testDocument);
expect(results).toHaveLength(0);
});

it('Response header string schema has no keywords', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.paths['/v1/movies/{movie_id}'].get.responses.headers = {
Expand Down Expand Up @@ -615,5 +594,41 @@ describe(`Spectral rule: ${ruleId}`, () => {
);
expect(validation.severity).toBe(expectedSeverity);
});

it('Response body string schema has no keywords', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.paths['/v1/movies/{movie_id}'].get.responses['200'] = {
content: {
'application/json': {
schema: {
properties: {
name: {
type: 'string',
description: 'no validation',
},
},
},
},
},
};

const results = await testRule(ruleId, rule, testDocument);
expect(results).toHaveLength(3);

const expectedPath =
'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.properties.name';
const expectedMessages = [
`String schemas should define property 'pattern'`,
`String schemas should define property 'minLength'`,
`String schemas should define property 'maxLength'`,
];

for (let i = 0; i < results.length; i++) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toBe(expectedMessages[i]);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPath);
}
});
});
});
14 changes: 13 additions & 1 deletion packages/ruleset/test/utils/root-document.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ module.exports = {
type: 'string',
minLength: 0,
maxLength: 512,
pattern: '^[a-zA-Z0-9 ]+$',
},
},
},
Expand Down Expand Up @@ -1039,7 +1040,9 @@ module.exports = {
trace: {
description: 'The error trace information.',
type: 'string',
format: 'uuid',
format: 'identifier',
pattern: '^[a-zA-Z0-9 ]+$',
maxLength: 30,
},
},
},
Expand All @@ -1055,10 +1058,16 @@ module.exports = {
message: {
description: 'The error message.',
type: 'string',
pattern: '^[a-zA-Z0-9 ]+$',
minLength: 1,
maxLength: 128,
},
more_info: {
description: 'Additional info about the error.',
type: 'string',
pattern: '^[a-zA-Z0-9 ]+$',
minLength: 1,
maxLength: 128,
},
target: {
$ref: '#/components/schemas/ErrorTarget',
Expand All @@ -1078,6 +1087,9 @@ module.exports = {
description:
'The name of the field/header/query parameter associated with the error.',
type: 'string',
pattern: '^[a-zA-Z0-9 ]+$',
minLength: 1,
maxLength: 30,
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ paths:
schema:
type: string
description: A link to the next page of responses
pattern: '^[a-zA-Z0-9]+$'
minLength: 1
maxLength: 128
content:
application/json:
schema:
Expand Down Expand Up @@ -159,6 +162,9 @@ components:
next_token:
type: string
description: next token
pattern: '^[a-zA-Z0-9_]+$'
minLength: 1
maxLength: 128
first:
allOf:
- $ref: '#/components/schemas/PageLink'
Expand Down Expand Up @@ -190,6 +196,9 @@ components:
href:
description: Request URL string
type: string
pattern: '^[a-zA-Z0-9]+$'
minLength: 1
maxLength: 128
Error:
type: object
description:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ paths:
schema:
type: string
description: A link to the next page of responses
pattern: '^[a-zA-Z0-9]+$'
minLength: 1
maxLength: 128
content:
application/json:
schema:
Expand Down Expand Up @@ -159,6 +162,9 @@ components:
next_token:
type: string
description: next token
pattern: '^[a-zA-Z0-9_]+$'
minLength: 1
maxLength: 128
first:
allOf:
- $ref: '#/components/schemas/PageLink'
Expand Down Expand Up @@ -190,6 +196,9 @@ components:
href:
description: Request URL string
type: string
pattern: '^[a-zA-Z0-9]+$'
minLength: 1
maxLength: 128
Error:
type: object
description:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ paths:
schema:
type: string
description: A link to the next page of responses
pattern: '^[a-zA-Z0-9]+$'
minLength: 1
maxLength: 128
content:
application/json:
schema:
Expand Down Expand Up @@ -159,6 +162,9 @@ components:
next_token:
type: string
description: next token
pattern: '^[a-zA-Z0-9_]+$'
minLength: 1
maxLength: 128
first:
allOf:
- $ref: '#/components/schemas/PageLink'
Expand Down Expand Up @@ -190,6 +196,9 @@ components:
href:
description: Request URL string
type: string
pattern: '^[a-zA-Z0-9]+$'
minLength: 1
maxLength: 128
Error:
type: object
description:
Expand Down