From efea0e5f2c75ab6f854a4e860955d1648ab06a63 Mon Sep 17 00:00:00 2001 From: Phil Adams Date: Fri, 9 Sep 2022 15:52:10 -0500 Subject: [PATCH] feat(path-param-not-crn): add new spectral-style rule This commit introduces the new spectral-style 'path-param-not-crn' validation rule which will verify that there are no path parameters that are defined as a CRN (Cloud Resource Name) value. Signed-off-by: Phil Adams --- docs/ibm-cloud-rules.md | 97 ++++++++++++ packages/ruleset/src/functions/index.js | 1 + .../src/functions/path-param-not-crn.js | 140 +++++++++++++++++ packages/ruleset/src/ibm-oas.js | 1 + packages/ruleset/src/rules/index.js | 1 + .../ruleset/src/rules/path-param-not-crn.js | 18 +++ .../ruleset/test/path-param-not-crn.test.js | 146 ++++++++++++++++++ 7 files changed, 404 insertions(+) create mode 100644 packages/ruleset/src/functions/path-param-not-crn.js create mode 100644 packages/ruleset/src/rules/path-param-not-crn.js create mode 100644 packages/ruleset/test/path-param-not-crn.test.js diff --git a/docs/ibm-cloud-rules.md b/docs/ibm-cloud-rules.md index dbe3a2797..935b0c92a 100644 --- a/docs/ibm-cloud-rules.md +++ b/docs/ibm-cloud-rules.md @@ -68,6 +68,7 @@ which is delivered in the `@ibm-cloud/openapi-ruleset` NPM package. * [Rule: parameter-order](#rule-parameter-order) * [Rule: parameter-schema-or-content](#rule-parameter-schema-or-content) * [Rule: patch-request-content-type](#rule-patch-request-content-type) + * [Rule: path-param-not-crn](#rule-path-param-not-crn) * [Rule: path-segment-case-convention](#rule-path-segment-case-convention) * [Rule: precondition-header](#rule-precondition-header) * [Rule: prohibit-summary-sentence-style](#rule-prohibit-summary-sentence-style) @@ -366,6 +367,12 @@ or application/merge-patch+json. oas3 +path-param-not-crn +warn +Verifies that path parameters are not defined as CRN (Cloud Resource Name) values +oas2, oas3 + + path-segment-case-convention error Path segments must follow a specific case convention @@ -3349,6 +3356,96 @@ paths: +### Rule: path-param-not-crn + + + + + + + + + + + + + + + + + + + + + + + + +
Rule id:path-param-not-crn
Description:This rule checks to make sure that there are no path parameters that are defined as a CRN (Cloud Resource Name) value. +

In order to determine whether or not a path parameter is considered to be defined as a CRN value, this validation rule +will perform the following checks: +

    +
  • The parameter's name field contains "crn" (e.g. "resource_crn")
    • +
  • The parameter's schema is defined with type=string, format=crn
    • +
  • The parameter's schema is defined with a pattern field that starts with either "crn" or "^crn" (e.g. 'crn:[-0-9A-Fa-f]+')
    • +
  • The parameter's example field contains a CRN-like value (e.g. "crn:0afd-0138-2636")
    • +
  • The parameter's examples field contains an entry containing a CRN-like value, as in this example: +
    +
    +components: + parameters: + ThingIdParam: + name: thing_id + description: The id of the Thing instance + in: path + required: true + schema: + type: string + examples: + crn_example: + value: 'crn:0afd-0138-2636' +
    • +
  • The parameter schema's example field contains a CRN-like value (e.g. "crn:0afd-0138-2636")
    • +
  • The parameter's description field contains either "CRN" or "Cloud Resource Name"
    • +
  • The parameter schema's description field contains either "CRN" or "Cloud Resource Name"
    • +
+These checks are logically OR'd together, so that if any one or more of these checks +are true for a particular parameter, then a warning is raised for that parameter. +
Severity:warn
OAS Versions:oas2, oas3
Non-compliant example: +
+components:
+  parameters:
+    ThingCrnParam:
+      name: thing_crn
+      description: The CRN associated with the Thing instance
+      in: path
+      required: true
+      schema:
+        type: string
+        format: crn
+        pattern: '^crn:[-0-9A-Fa-f]+$'
+        minLength: 5
+        maxLength: 32
+
+
Compliant example: +
+components:
+  parameters:
+    ThingIdParam:
+      name: thing_id
+      description: The id associated with the Thing instance
+      in: path
+      required: true
+      schema:
+        type: string
+        format: identifier
+        pattern: '^id:[-0-9A-Fa-f]+$'
+        minLength: 5
+        maxLength: 32
+
+
+ + ### Rule: path-segment-case-convention diff --git a/packages/ruleset/src/functions/index.js b/packages/ruleset/src/functions/index.js index 8594cf854..cf789e668 100644 --- a/packages/ruleset/src/functions/index.js +++ b/packages/ruleset/src/functions/index.js @@ -28,6 +28,7 @@ module.exports = { parameterDescription: require('./parameter-description'), parameterOrder: require('./parameter-order'), patchRequestContentType: require('./patch-request-content-type'), + pathParamNotCRN: require('./path-param-not-crn'), pathSegmentCaseConvention: require('./path-segment-case-convention'), preconditionHeader: require('./precondition-header'), propertyAttributes: require('./property-attributes'), diff --git a/packages/ruleset/src/functions/path-param-not-crn.js b/packages/ruleset/src/functions/path-param-not-crn.js new file mode 100644 index 000000000..f963053ba --- /dev/null +++ b/packages/ruleset/src/functions/path-param-not-crn.js @@ -0,0 +1,140 @@ +const { + isStringSchema, + checkCompositeSchemaForConstraint +} = require('../utils'); + +module.exports = function(pathParam, _opts, { path }) { + return pathParamNotCRN(pathParam, path); +}; + +/** + * This function will check "pathParam" (assumed to be a path parameter object) + * to make sure that it is not defined as a "CRN" (Cloud Resource Name) value. + * @param {*} pathParam the path parameter object to check + * @param {*} path the jsonpath location of "pathParam" within the API definition + * @returns an array containing the violations found or [] if no violations + */ +function pathParamNotCRN(pathParam, path) { + const subPath = isCRNParameter(pathParam); + if (subPath && subPath.length > 0) { + return [ + { + message: 'Path parameter should not be defined as a CRN value', + path: [...path, ...subPath] + } + ]; + } + + return []; +} + +/** + * This function checks to see if the specified parameter object "p" is defined as a CRN-like value. + * If "p" does not appear to be defined as a CRN-like value, then [] is returned. + * If "p" does appear to be defined as a CRN-like value, then the return value will be a + * list of one or more jsonpath segments that represent the relative location of the violation + * (relative to "p"'s location within the API definition) + * @param {} p the parameter object to check + * @returns a list of zero or more jsonpath segments to indicate that a violation + * was found at that relative location + */ +function isCRNParameter(p) { + // Check if the parameter name contains "crn". + if ( + p.name && + typeof p.name === 'string' && + /crn/.test(p.name.toLowerCase()) + ) { + return ['name']; + } + + // Grab the parameter's schema object. + let schema = p.schema; + if (!schema) { + // If not set directly on the parameter, grab the first schema within the content map instead. + if (p.content && typeof p.content === 'object' && p.content.size > 0) { + schema = p.content.values()[0]; + } + } + + // Check if the schema is defined as type=string/format=crn. + if ( + schema && + isStringSchema(schema) && + checkCompositeSchemaForConstraint(schema, s => s.format === 'crn') + ) { + return ['schema', 'format']; + } + + // Check if the schema defines a pattern field that starts with "crn" or "^crn". + if ( + schema && + checkCompositeSchemaForConstraint( + schema, + s => + s.pattern && + typeof s.pattern === 'string' && + /^\^?crn.*/.test(s.pattern.trim().toLowerCase()) + ) + ) { + return ['schema', 'pattern']; + } + + // Check if the parameter's "example" field is a CRN-like value. + if (p.example && isCRNValue(p.example)) { + return ['example']; + } + + // Check if the parameter's "examples" field contains an entry with a CRN-like value. + if (p.examples && typeof p.examples === 'object') { + for (const [name, obj] of Object.entries(p.examples)) { + if (obj && obj.value && isCRNValue(obj.value)) { + return ['examples', name, 'value']; + } + } + } + + // Check if the parameter schema's "example" field is a CRN-like value. + if (schema && schema.example && isCRNValue(schema.example)) { + return ['schema', 'example']; + } + + // Check if the parameter's description contains "CRN" or "Cloud Resource Name". + if (isCRNInDescription(p)) { + return ['description']; + } + + // Check if the parameter schema's description contains "CRN" or "Cloud Resource Name". + if (isCRNInDescription(schema)) { + return ['schema', 'description']; + } + + // If we made it through the gauntlet unscathed, then return an empty list to + // indicate no violations. + return []; +} + +/** + * Returns true iff the specified example value appears to be an example of + * a CRN value. + * @param {} value the value to check + * @return boolean + */ +function isCRNValue(value) { + return value && typeof value === 'string' && /^crn:.*/.test(value.trim()); +} + +/** + * Returns true iff "obj" is an object with a "description" field that contains + * "CRN" or "Cloud Resource Name". + * @param {*} obj the objet whose "description" field should be checked + * @return boolean + */ +function isCRNInDescription(obj) { + return ( + obj && + obj.description && + typeof obj.description === 'string' && + /^.*((CRN)|(Cloud\s*Resource\s*Name)).*$/.test(obj.description) + ); +} diff --git a/packages/ruleset/src/ibm-oas.js b/packages/ruleset/src/ibm-oas.js index 615c56077..b71730060 100644 --- a/packages/ruleset/src/ibm-oas.js +++ b/packages/ruleset/src/ibm-oas.js @@ -130,6 +130,7 @@ module.exports = { 'parameter-order': ibmRules.parameterOrder, 'parameter-schema-or-content': ibmRules.parameterSchemaOrContent, 'patch-request-content-type': ibmRules.patchRequestContentType, + 'path-param-not-crn': ibmRules.pathParamNotCRN, 'path-segment-case-convention': ibmRules.pathSegmentCaseConvention, 'precondition-header': ibmRules.preconditionHeader, 'prohibit-summary-sentence-style': ibmRules.prohibitSummarySentenceStyle, diff --git a/packages/ruleset/src/rules/index.js b/packages/ruleset/src/rules/index.js index f1e04d819..d38937033 100644 --- a/packages/ruleset/src/rules/index.js +++ b/packages/ruleset/src/rules/index.js @@ -39,6 +39,7 @@ module.exports = { parameterOrder: require('./parameter-order'), parameterSchemaOrContent: require('./parameter-schema-or-content'), patchRequestContentType: require('./patch-request-content-type'), + pathParamNotCRN: require('./path-param-not-crn'), pathSegmentCaseConvention: require('./path-segment-case-convention'), preconditionHeader: require('./precondition-header'), prohibitSummarySentenceStyle: require('./prohibit-summary-sentence-style'), diff --git a/packages/ruleset/src/rules/path-param-not-crn.js b/packages/ruleset/src/rules/path-param-not-crn.js new file mode 100644 index 000000000..237903917 --- /dev/null +++ b/packages/ruleset/src/rules/path-param-not-crn.js @@ -0,0 +1,18 @@ +const { oas2, oas3 } = require('@stoplight/spectral-formats'); +const { pathParamNotCRN } = require('../functions'); + +module.exports = { + description: + 'Path parameter should not be defined as a CRN (Cloud Resource Name) value', + message: '{{error}}', + formats: [oas2, oas3], + given: [ + '$.paths[*].parameters[?(@.in === "path")]', + '$.paths[*][get,put,post,delete,options,head,patch,trace].parameters[?(@.in === "path")]' + ], + severity: 'warn', + resolved: true, + then: { + function: pathParamNotCRN + } +}; diff --git a/packages/ruleset/test/path-param-not-crn.test.js b/packages/ruleset/test/path-param-not-crn.test.js new file mode 100644 index 000000000..0be83ad87 --- /dev/null +++ b/packages/ruleset/test/path-param-not-crn.test.js @@ -0,0 +1,146 @@ +const { pathParamNotCRN } = require('../src/rules'); +const { makeCopy, rootDocument, testRule, severityCodes } = require('./utils'); + +const rule = pathParamNotCRN; +const ruleId = 'path-param-not-crn'; +const expectedSeverity = severityCodes.warning; +const expectedMsg = 'Path parameter should not be defined as a CRN value'; + +describe('Spectral rule: path-param-not-crn', () => { + describe('Should not yield errors', () => { + it('Clean spec', async () => { + const results = await testRule(ruleId, rule, rootDocument); + expect(results).toHaveLength(0); + }); + + it('Query param w/ CRN in description', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.VerboseParam.description = + 'this is a CRN value'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + }); + + describe('Should yield errors', () => { + it('Path param w/ crn in name', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.DrinkIdParam.name = 'drink_crn'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/drinks/{drink_id}.parameters.0.name' + ); + }); + + it('Path param w/ format=crn', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.MovieIdParam.schema.format = 'crn'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.parameters.0.schema.format' + ); + }); + + it('Path param w/ pattern="crn..."', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.MovieIdParam.schema.pattern = + 'crn:[0-9A-F]+'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.parameters.0.schema.pattern' + ); + }); + + it('Path param w/ pattern="^crn..."', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.MovieIdParam.schema.pattern = + '^crn:[0-9A-F]+$'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.parameters.0.schema.pattern' + ); + }); + + it('Path param w/ crn-like example value', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.MovieIdParam.example = + 'crn:88adez-01abdfe'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.parameters.0.example' + ); + }); + + it('Path param w/ crn-like value in "examples" field', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.MovieIdParam.examples = { + good: { + description: 'A good example', + value: '88adez-01abdfe' + }, + bad: { + description: 'A bad example', + value: 'crn:88adez-01abdfe' + } + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.parameters.0.examples.bad.value' + ); + }); + + it('Path param w/ crn-like example value in schema', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.parameters.MovieIdParam.schema.example = + 'crn:88adez-01abdfe'; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + expect(results[0].code).toBe(ruleId); + expect(results[0].message).toBe(expectedMsg); + expect(results[0].severity).toBe(expectedSeverity); + expect(results[0].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.parameters.0.schema.example' + ); + }); + }); +});