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

The rule only applied to "input" schemas, which are more important to check for
the attributes, but the API Handbook recommends the attributes for repsonse
schemas as well. This commit expands the rule to check for those scenarios.

Signed-off-by: Dustin Popp <dpopp07@gmail.com>

docs/ibm-cloud-rules.md

@@ -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>

packages/ruleset/src/functions/string-attributes.js

@@ -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;
@@ -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.
@@ -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({
@@ -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;
-}

packages/ruleset/test/string-attributes.test.js

@@ -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 = {
@@ -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);
+ }
+ });
  });
 });

packages/ruleset/test/utils/root-document.js

@@ -248,6 +248,7 @@ module.exports = {
  type: 'string',
  minLength: 0,
  maxLength: 512,
+ pattern: '^[a-zA-Z0-9 ]+$',
  },
  },
  },
@@ -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,
  },
  },
  },
@@ -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',
@@ -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,
  },
  },
  },

packages/validator/test/cli-validator/mock-files/oas3/clean-with-tabs.yml

@@ -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:
@@ -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'
@@ -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:

packages/validator/test/cli-validator/mock-files/oas3/clean.yml

@@ -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:
@@ -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'
@@ -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:

packages/validator/test/cli-validator/mock-files/oas31/clean.yml

@@ -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:
@@ -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'
@@ -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: