-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
feat(rulesets): validate API security in oas-operation-security-defined #2046
Conversation
11b4420
to
e792e63
Compare
@@ -50,6 +53,31 @@ export default createRulesetFunction<{ paths: Record<string, unknown> }, Options | |||
const schemes = _get(targetVal, schemesPath); | |||
const allDefs = isObject(schemes) ? Object.keys(schemes) : []; | |||
|
|||
// Check global security requirements | |||
|
|||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this block? It doesn't seem like security
is redeclared within the current scope. It is a later on at
const { security } = value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amended PR to remove unnecessary block.
e792e63
to
536800a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks a lot.
# [@stoplight/spectral-rulesets-v1.6.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-rulesets-v1.5.2...@stoplight/spectral-rulesets-v1.6.0) (2022-03-03) ### Features * **rulesets:** validate API security in oas-operation-security-defined ([#2046](#2046)) ([5540250](5540250))
🎉 This PR is included in version @stoplight/spectral-rulesets-v1.6.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
This PR adds checking in the function for oas[23]-operation-security-defined to also check the API-level security requirement to ensure it also only references defined security schemes.
I admit that it is stretching the intent of these rules a bit, to check the API-level definition in a rule that by its name would only work on the operation-level security requirement. But it seemed like overkill to define an entirely new rule to perform the same checking as this one, just at the global level. And in some sense the API level security is the operation-level security for any operation that does not override it, so it didn't seem too unreasonable to include these checks here.
UPDATE: I've updated this PR to add checking for all the fields in a security requirements object -- not just the first one.
Checklist
Does this PR introduce a breaking change?