Repository: Redocly/redocly-cli
Pull request #879 (2022-09-19 16:21:44)

fix: change security-defined rule reporting
## What/Why/How?
The [security-defined](https://redocly.com/docs/cli/rules/security-defined/) has a bad reporting.

## Reference
Some context can be found [here](https://redoc-ly.slack.com/archives/C02UHE7EL3E/p1663345423549179).

## Testing
Tested locally.

## Screenshots (optional)
<img width="620" alt="image" src="https://user-images.githubusercontent.com/54616703/191061797-9f67243e-d830-4743-9610-82d5bccf5cd4.png">

## Check yourself

- [x] Code is linted
- [ ] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :--------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 72.99%     |    3246/4447    |
|                                                                           🟡                                                                           | Branches   | 64.64%     |    1766/2732    |
|                                                                           🟡                                                                           | Functions  | 65.08%     |     533/819     |
|                                                                           🟡                                                                           | Lines      | 72.96%     |    3020/4139    |


<details><summary>Show files with reduced coverage 🔻</summary>
<br/>

| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | File                                                                                     | Statements | Branches                                                       | Functions | Lines  |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------------------------------------------------------------------------------------- | :--------- | :------------------------------------------------------------- | :-------- | :----- |
|                                                                           🟢                                                                           | <div title="core/src/rules/common/security-defined.ts">`...` / security-defined.ts</div> | 96.15%     | <div title="Base coverage is: 91.67%">85.71% (-5.95% 🔻)</div> | 100%      | 95.83% |

</details>

## Test suite run success
494 tests passing in 85 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from 26e996967a7e32614388f07a8b1a5cbb51b584ed</p>


Patch:
diff --git a/packages/core/src/rules/common/security-defined.ts b/packages/core/src/rules/common/security-defined.ts
index c1d98bd1..fd2ec55d 100644
--- a/packages/core/src/rules/common/security-defined.ts
+++ b/packages/core/src/rules/common/security-defined.ts
@@ -13,10 +13,11 @@ export const SecurityDefined: Oas3Rule | Oas2Rule = () => {
     }
   >();
 
+  const operationsWithoutSecurity: Location[] = [];
   let eachOperationHasSecurity: boolean = true;
 
   return {
-    DefinitionRoot: {
+    Root: {
       leave(root: Oas2Definition | Oas3Definition, { report }: UserContext) {
         for (const [name, scheme] of referencedSchemes.entries()) {
           if (scheme.defined) continue;
@@ -31,9 +32,12 @@ export const SecurityDefined: Oas3Rule | Oas2Rule = () => {
         if (root.security || eachOperationHasSecurity) {
           return;
         } else {
-          report({
-            message: `Every API should have security defined on the root level or for each operation.`,
-          });
+          for (const operationLocation of operationsWithoutSecurity) {
+            report({
+              message: `Every operation should have security defined on it or on the root level.`,
+              location: operationLocation.key(),
+            });
+          }
         }
       },
     },
@@ -51,9 +55,10 @@ export const SecurityDefined: Oas3Rule | Oas2Rule = () => {
         }
       }
     },
-    Operation(operation: Oas2Operation | Oas3Operation) {
+    Operation(operation: Oas2Operation | Oas3Operation, { location }: UserContext) {
       if (!operation?.security) {
         eachOperationHasSecurity = false;
+        operationsWithoutSecurity.push(location);
       }
     },
   };


Test patch:
diff --git a/__tests__/lint/assertions-custom-function-options/snapshot.js b/__tests__/lint/assertions-custom-function-options/snapshot.js
index 5ba63d60..ab21c346 100644
--- a/__tests__/lint/assertions-custom-function-options/snapshot.js
+++ b/__tests__/lint/assertions-custom-function-options/snapshot.js
@@ -80,25 +80,37 @@ operation-summary-length assertion failed because the Operation summary didn't m
 Error was generated by the operation-summary-length assertion rule.
 
 
-[5] openapi.yaml:1:1 at #/
+[5] openapi.yaml:18:5 at #/paths/~1pet~1findByStatus/get
 
-Every API should have security defined on the root level or for each operation.
+Every operation should have security defined on it or on the root level.
 
- 1 | openapi: 3.1.0
-   | ^^^^^^^^^^^^^^
- 2 | servers:
-   | ^^^^^^^^
- … | < 46 more lines >
-49 |           description: example description
-   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-50 |
+16 | paths:
+17 |   /pet/findByStatus:
+18 |     get:
+   |     ^^^
+19 |       operationId: example
+20 |       summary: List of pets
+
+Error was generated by the security-defined rule.
+
+
+[6] openapi.yaml:34:5 at #/paths/~1pet~1findByStatus/post
+
+Every operation should have security defined on it or on the root level.
+
+32 |     '404':
+33 |       description: example description
+34 | post:
+   | ^^^^
+35 |   operationId: example-post
+36 |   summary: summary looooong
 
 Error was generated by the security-defined rule.
 
 
 /openapi.yaml: validated in <test>ms
 
-❌ Validation failed with 5 errors.
+❌ Validation failed with 6 errors.
 run \`openapi lint --generate-ignore-file\` to add all problems to the ignore file.
 
 
diff --git a/__tests__/lint/operation-security-scheme-oas3-error/snapshot.js b/__tests__/lint/operation-security-scheme-oas3-error/snapshot.js
index f2e2c650..6c736d61 100644
--- a/__tests__/lint/operation-security-scheme-oas3-error/snapshot.js
+++ b/__tests__/lint/operation-security-scheme-oas3-error/snapshot.js
@@ -100,18 +100,16 @@ Property \`authorizationUrl\` is not expected here.
 Error was generated by the spec rule.
 
 
-[8] openapi.yaml:1:1 at #/
+[8] openapi.yaml:18:5 at #/paths/~1pet~1findByStatus/get
 
-Every API should have security defined on the root level or for each operation.
+Every operation should have security defined on it or on the root level.
 
- 1 | openapi: 3.0.0
-   | ^^^^^^^^^^^^^^
- 2 | servers:
-   | ^^^^^^^^
- … | < 55 more lines >
-58 |       authorizationUrl: 'http://petstore.swagger.io/api/oauth/dialog'
-   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-59 |
+16 | paths:
+17 |   /pet/findByStatus:
+18 |     get:
+   |     ^^^
+19 |       operationId: example
+20 |       summary: summary example
 
 Error was generated by the security-defined rule.
 
diff --git a/__tests__/lint/spec-error-if-minimum-not-correct/snapshot.js b/__tests__/lint/spec-error-if-minimum-not-correct/snapshot.js
index 06373c56..621eea72 100644
--- a/__tests__/lint/spec-error-if-minimum-not-correct/snapshot.js
+++ b/__tests__/lint/spec-error-if-minimum-not-correct/snapshot.js
@@ -104,18 +104,16 @@ The value of the minProperties field must be greater than or equal to 0
 Error was generated by the spec rule.
 
 
-[8] openapi.yaml:1:1 at #/
-
-Every API should have security defined on the root level or for each operation.
-
- 1 | openapi: 3.0.0
-   | ^^^^^^^^^^^^^^
- 2 | servers:
-   | ^^^^^^^^
- … | < 70 more lines >
-73 |             - packSize
-   |             ^^^^^^^^^^
-74 |
+[8] openapi.yaml:15:5 at #/paths/~1pet~1findByStatus/get
+
+Every operation should have security defined on it or on the root level.
+
+13 | paths:
+14 |   /pet/findByStatus:
+15 |     get:
+   |     ^^^
+16 |       tags:
+17 |         - pet
 
 Error was generated by the security-defined rule.
 
diff --git a/packages/core/src/rules/common/__tests__/security-defined.test.ts b/packages/core/src/rules/common/__tests__/security-defined.test.ts
index fd9443b9..89b29818 100644
--- a/packages/core/src/rules/common/__tests__/security-defined.test.ts
+++ b/packages/core/src/rules/common/__tests__/security-defined.test.ts
@@ -81,12 +81,12 @@ describe('Oas3 security-defined', () => {
         Object {
           "location": Array [
             Object {
-              "pointer": "#/",
-              "reportOnKey": false,
+              "pointer": "#/paths/~1pets/get",
+              "reportOnKey": true,
               "source": "foobar.yaml",
             },
           ],
-          "message": "Every API should have security defined on the root level or for each operation.",
+          "message": "Every operation should have security defined on it or on the root level.",
           "ruleId": "security-defined",
           "severity": "error",
           "suggest": Array [],
@@ -133,12 +133,12 @@ describe('Oas3 security-defined', () => {
         Object {
           "location": Array [
             Object {
-              "pointer": "#/",
-              "reportOnKey": false,
+              "pointer": "#/paths/~1cats/get",
+              "reportOnKey": true,
               "source": "foobar.yaml",
             },
           ],
-          "message": "Every API should have security defined on the root level or for each operation.",
+          "message": "Every operation should have security defined on it or on the root level.",
           "ruleId": "security-defined",
           "severity": "error",
           "suggest": Array [],