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

…682)

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 <[email protected]>

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: