Skip to content
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

Rule template for GCC custom rules not working due to key validation: - sign not allowed #2586

Open
jenspopp opened this issue Oct 27, 2023 · 15 comments
Labels

Comments

@jenspopp
Copy link

Describe the bug
When analyzing gcc compiler warnings I wanted to add a new one for -Wstringop-truncation

When trying to enter this key, I get a validation error, that key is only allowed to be upper and lower case letters, digits and "_" , so "-" is not allowed. Since this is used as id from the regex, I don't know how to use this key...

Also backup, manual edit of xml and restore does not work

To Reproduce
Try to create a new custom rule from gcc rule template

Expected behavior

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. Linux]
  • SonarQube version: 9.9LTS
  • cxx plugin version: 2.11
  • sonar-scanner version: 5.0.1.3006
@jenspopp
Copy link
Author

I found a solution with DB update, but only for admins not for users...
e.g., if the key in UI was given as '_wstringop_truncation'

update rules set plugin_rule_key='-Wstringop-truncation' where plugin_rule_key='_wstringop_truncation';

@guwirth
Copy link
Collaborator

guwirth commented Oct 28, 2023

Hi @jenspopp,

thanks for the feedback. What is exactly the error message you get? Like to verify if this message / check is from the cxx plugin or the SQ framework.

Regards,

@guwirth guwirth changed the title Rule template for GCC custom rules not working due to key validation Oct 28, 2023
@jenspopp
Copy link
Author

jenspopp commented Oct 28, 2023

Hi @guwirth,

The error ist

The rule key "-Wsomething-something" is invalid, it should only contain: a-z, 0-9, "_"

As mentioned, when I create the rule with some dummy key, I can update it in postgres. After that it works as advertised.

On a related topic, some users ask to catch all warnings without rules (so basically new compiler warnings). I'm new to plugins in Sonar, but I'm able to get all gcc rules via the rules web api. I also understand the basic workings of the cxx sensor. But I have found no docu on how to access the rules/web api from the sensor? I would be happy to contribute, if I have a starting point.

@guwirth
Copy link
Collaborator

guwirth commented Oct 28, 2023

Hi @jenspopp,

On a related topic, some users ask to catch all warnings without rules (so basically new compiler warnings)

In case you wanna create rules / issues on the fly it’s better to use
https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/importing-external-issues/generic-issue-import-format/

Regards,

@guwirth
Copy link
Collaborator

guwirth commented Oct 28, 2023

Hi @jenspopp,

The rule key "-Wsomething-something" is invalid, it should only contain: a-z, 0-9, "_"

The warning is from the SQ core
https://github.com/SonarSource/sonarqube/blob/dbed19dc8cf4cd6fc2a2112d0e07454b41c105ff/server/sonar-webserver-webapi/src/main/java/org/sonar/server/rule/RuleCreator.java#L179

Means we can’t work around it in the cxx plugin. Maybe you can ask in the SQ forum why this limitation exists https://community.sonarsource.com/

Regards,

@guwirth
Copy link
Collaborator

guwirth commented Oct 28, 2023

Hi @jenspopp,

I would be happy to contribute, if I have a starting point.

The cxx plugin is using a XML file to define the supported gcc rules:
https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-sensors/src/main/resources/compiler-gcc.xml

To extend (add new) rules we have to extend the file and provide a new version of the plugin.

Regards,

@jenspopp
Copy link
Author

jenspopp commented Nov 6, 2023

Hi @guwirth,

sorry for the late reply, had a week off. My idea was to load at the initialization of the scanner al list of all known issues per tool (gcc, cppcheck, ...). When an issue is created I would like to check if the issue (e.g. compiler warning) is a handled issue or an unknown issue. If it is unknown I would change the id to something like UNHANDLED_GCC_WARNING and create a rule for that. We could than decide if we want to activate the rule or not.

Reason is, that I have beside the "productive" build on RHEL and SLES a build on latest tumbleweed with the newest gcc to catch any new warnings. It would be cumbersome to check every time, if there is a new warning. The build is on docker against the latest and changes much more often then our sonar instance...

As for sonar, I have a workaround with the sql update... a bit cumbersome but workable. My assumption is, that they want to avoid special characters that might cause attack scenarios/warnings...

@guwirth
Copy link
Collaborator

guwirth commented Nov 6, 2023

Hi @jenspopp,

My idea was to load at the initialization of the scanner al list of all known issues per tool

SQ is working different. During the start of the server all plugins are loaded. The rules are a static list inside of every plugin. It’s not possible to create or change the rules afterwards. Only exception is
https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/importing-external-issues/generic-issue-import-format/

Regards,

@jenspopp
Copy link
Author

jenspopp commented Nov 7, 2023

Hello @guwirth,

I don't want to create a new rule ;-) The idea would be to have a fixed rule for each tool (gcc, cppcheck...) with the ruleId = NEW_VIOLATION.
The scanner would load at the beginning the existing rules (all rules, not only the activated) via the rules api of sonarqube. Than I would build in a hook e.g. in CxxIssuesReportSensor#saveUniqueViolation to check in the loaded data based on the getRuleRepositoryKey, if the violation is known or not.
Only if it is not known, I would change the rule key to "NEW_VIOALATION" and add the given key (e.g. -Wnew-warning) to the description...

My problem is, I don't know, how to access the rule api from within the sensor in a proper way. I already did the needed Rest query via postman...

Best regards
Jens

@guwirth
Copy link
Collaborator

guwirth commented Nov 8, 2023

Hi @guwirth,

The idea would be to have a fixed rule for each tool (gcc, cppcheck...) with the ruleId = NEW_VIOLATION.

That should be possible we discussed this in the past already, can’t remember the result?
Can you verify what happens if you add an unknown rule-id. Is there already a warning in the LOG file with debug info on?
https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Get-Debug-Information

Regards,

@jenspopp
Copy link
Author

jenspopp commented Nov 8, 2023

Hi,

I did some test runs, where I created an artificial -Wtest-scanner compiler warning. What I see is in the scanner log is no real info:

12:28:05.566 INFO: Sensor CXX GCC compiler report import [cxx]
12:28:05.567 DEBUG: Searching 'sonar.cxx.gcc.reportPaths' files with Ant pattern '[compiler.log]'
12:28:05.568 DEBUG: Search files(s) in path(s): '.../compiler.log'
12:28:05.569 DEBUG: Found '1' file(s)
12:28:05.571 INFO: Processing report '...\compiler.log'
12:28:05.573 DEBUG: Processing 'GCC' report '...\compiler.log', Encoding='UTF-8', Pattern='(?[^:]+):(?\d{1,5}):\d{1,5}:\x20warning:\x20(?.?)(\x20[(?.)])?\s*$'
12:28:05.661 DEBUG: named-capturing group 'column' is not used in regex.
12:28:06.235 INFO: Processing successful, saved new issues=162

After that is only the upload. On the server side I see in the proxy logs multiple lines:
"GET /api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt,updatedAt,deprecatedKeys&activation=true&qprofile=AYtbq66YuxtNuJxvsmcP&ps=500&p=1 HTTP/1.1" 200 53236 "-" "ScannerCLI/5.0.1.3006" "192.168.89.5:9000"

Where it downloads the activated rules (&activation=true). In the real processing in sonar I see only some lines:

sonarqube | 2023.11.08 12:45:04 DEBUG ce[][org.apache.http.wire] http-outgoing-0 >> "{"isNewCodeReference":false,"line":388,"project":"AYuuv_isn_AYwlZCmvoP","effort":5,"language":"cxx","sansTop25":[],"type":"CODE_SMELL","resolution":null,"branch":"AYuuv_isn_AYwlZCmvoP","issueUpdatedAt":"2023-11-08T11:44:39.000Z","modulePath":".AYuuv_isn_AYwlZCmvoP.","severityValue":3,"ruleUuid":"AYtbqkCsTZfCf1ai8-E5","owaspAsvs-4.0":[],"scope":"MAIN","dirPath":"src","key":"AYuuwAGtRP1B5X1jRf29","issueCreatedAt":"2023-11-08T11:44:39.000Z","severity":"CRITICAL","vulnerabilityProbability":1,"owaspTop10":[],"filePath":"src/wcs_rv6_receive.c","authorLogin":null,"join_issues":{"name":"issue","parent":"auth_AYuuv_isn_AYwlZCmvoP"},"issueClosedAt":null,"tags":["compiler-gcc"],"cwe":["unknown"],"sonarsourceSecurity":"others","component":"AYuuv_x0RP1B5X1jRf03","indexType":"issue","pciDss-4.0":[],"pciDss-3.2":[],"assignee":null,"owaspTop10-2021":[],"isMainBranch":true,"status":"OPEN"}[\n]"

Where "ruleUuid":"AYtbqkCsTZfCf1ai8-E5" is in the db: -Wunused-but-set-variable. But that seems to be only the elastic search entry...

So I don't know, where the rule is filtered... if it is already done on the scanner (with the download of the activated rules probable...) or on the server.

I have one entry on the server:

sonarqube | 2023.11.08 12:45:03 INFO ce[AYuuv_lun_AYwlZCmvoY][o.s.c.t.s.ComputationStepExecutor] Persist issues | cacheSize=52.5 kB | inserts=154 | updates=0 | merged=0 | status=SUCCESS | time=57ms

Where the inserts=154 is less then the saved new issues=162 from the scanner. The compiler log has in total 162 warnings.

(Please don't look at the time stamps, is from different runs, but same data and fresh projects)

Best regards
Jens

@jenspopp
Copy link
Author

Hello @guwirth,

I had a longer discussion with the sonarsource developers, final answer:
"Since we don’t support writing rules for C-Family languages and since you’re basing this on Cxx, I’m going to have to refer you to the Cxx folks at this point."

So in the end I would need the following query executed from the scanner side:

https://myserver:9001/api/rules/search?languages=cxx&tags=compiler-gcc&f=internalKey

That would return all internal keys and I could compare them to the extracted warning. Any idea how to achieve that from the scanner api? Is there any point, to retrieve the logged in http connection or better the rules api itself?

Best regards
Jens

@guwirth
Copy link
Collaborator

guwirth commented Nov 17, 2023

Hi,

I’m going to have to refer you to the Cxx folks at this point

not really surprised about that answer. They are not really fans of the cxx plugin …

Any idea how to achieve that from the scanner api?

The sensor for gcc is here:
https://github.com/SonarOpenCommunity/sonar-cxx/tree/master/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/gcc

Here we add the issue from the report:

public void saveUniqueViolation(CxxReportIssue issue) {

Here should be a check for unknown rule id and replace it by „unknown“. Not sure how to verify if unknown? Also not sure if this would not result in a performance penalty.

Read the rules on server side:

public class CxxCompilerGccRuleRepository extends RulesDefinitionXml {

Found this to find active rules on scanner side:
https://javadocs.sonarsource.org/latest/org/sonar/api/batch/rule/ActiveRules.html

Found no API for all rules:
https://javadocs.sonarsource.org/latest/allclasses.html

Regards,

@jenspopp
Copy link
Author

Hello,

thanks for the answer ;-) Found this too. I did also a deeper dig in Sonar Core source. The issue is, that they run the scanner in Spring, but the plugins seem to be in a separate classloader. So I have no easy access to the @Autowired to inject the correct initialized classes. All you get is the context, which only contains already instrumented/initialized interfaces.

My only idea right now is to build an own connection to the server and call the API (That would be a lot of code duplication from the sonar source). I still have hope to access the initialized wsClient via reflection, but have not found a way in yet. (all classes only seem to have already filled maps, no connection objects)

I basically had already the complete code, if AutoWired would have worked :-( It would be only minimal change in the existing code, when you save the rule.

I'm still very interested in this topic and will continue, but probably only over the next holidays...

Perhaps it helps, if you vote this one up ;-)

https://stackoverflow.com/questions/77493510/sonarqube-access-api-from-sensor

Best regards
Jens

@jenspopp
Copy link
Author

jenspopp commented Jan 3, 2024

Hello @guwirth,

good news, I finally had a bit of time to create a solution. I tested it with gcc, cppcheck and clangtidy.

I used the java httpclient to download the all rule ids for the sensors and check whether the rule is already existing. Unknown rules are mapped to the key "unknown" (customizable with a property - see readme change).

There is also a small fix in the default regex for gcc

Open points:

  1. Default unknown rule is right now only for the tested frameworks in the xml definitions
  2. We could use protobuf instead of json (but at least for my local use case it was so fast, that I didn't see a reason to invest time here)
  3. In the long run we might need to replace all "-" with "_" in the rule ids.... (due to sonar community changes)
  4. I'm not sure, I covered all possible exceptions (e.g. with login/authentication); but they should occur already before the cxx plugin
  5. Right now only login with token is supported

The default fallback is on the existing behavior with a warning printed.

Please find attached the git diff; let me know, if I should fork and create a pull request..

Best regards
Jens

unknownId-changes.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants