-
Notifications
You must be signed in to change notification settings - Fork 690
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
SONARJAVA-5413 Update rules metadata #5060
Conversation
leonardo-pilastri-sonarsource
commented
Mar 20, 2025
•
edited by dorian-burihabwa-sonarsource
Loading
edited by dorian-burihabwa-sonarsource
|
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.
Let's check if we need to change the metadata for S5542
@@ -127,10 +127,11 @@ <h3>Standards</h3> | |||
Exposure</a> </li> | |||
<li> OWASP - <a href="https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration">Top 10 2017 Category A6 - Security | |||
Misconfiguration</a> </li> | |||
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/327">CWE-327 - Use of a Broken or Risky Cryptographic Algorithm</a> </li> |
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.
Weirdly enough, the CWE 327 is removed from the docs but not from the metadata JSON file. Should we do something about it?
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.
See comment on file above that should modify line 28
@@ -38,7 +38,7 @@ <h4>Compliant solution</h4> | |||
public class AsyncNotificationProcessor implements NotificationProcessor { | |||
|
|||
@Resource | |||
private AsyncNotificationProcessor | |||
private AsyncNotificationProcessor asyncNotificationProcessor; |
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.
Just to clarify, this change should have come as part of SONARJAVA-5401, right?
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.
There was not ticket for this PR, right?
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.
No, I think I counted on the GitHub automation to create it, but it seems like on rspec for some projects (including ours) the automation can't find an appropriate project key to create a new ticket.
public class RomeBioparc implements Zoo { | ||
@Override | ||
public Animal getMostPopularAnimal() { | ||
return new Pantegana(); |
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.
TIL what a pantegana is 🐀
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.
No ticket for this PR, right?
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.
Same as S7180
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.
The CWE discrepancy was checked and is OK for now. Let's merge for now and see if we need to merge something before release
Includes changes from