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

[False Result] - Assigning to a property called "key" #1372

Open
mattlacey opened this issue Mar 1, 2024 · 2 comments
Open

[False Result] - Assigning to a property called "key" #1372

mattlacey opened this issue Mar 1, 2024 · 2 comments
Labels
PMD-AppExchange Issues related to the pmd-appexchange engine rules

Comments

@mattlacey
Copy link

Salesforce Code Analyzer False Results Template

Description:
Scanner reporting use of hard-coded credentials

Documentation:

AvoidHardcodedCredentials Remove hard-coded credentials from source code. https://github.com/forcedotcom/sfdx-scanner/blob/dev/pmd-appexchange/docs/AvoidHardcodedCredentials.md

False Results Report:
The line of code triggering this is:

        this.key = '';

It's simply an Apex class property where key is being used as an identiier

Steps To Reproduce:

Expected Behavior:
Scanner should have no issues with the line of code in question

Screenshots:

Desktop:

Sonoma 14.3.1
Analyzer 3.21.0
@salesforce/cli/2.30.8 darwin-arm64 node-v20.11.1

@nwcm
Copy link

nwcm commented Apr 10, 2024

We see this as well. I don't think the scanner will be able to skip this however. It reports anything with string containing key, token, session or similar key words and cannot tell if it is legit or not. That's why you must include this in the false positive report with justification of what it is actually used for.

For example Jewellery_Accessories__c this field will flag because it has access in it...

Best way is to rename to something which doesn't contain a string they are looking for.

@stephen-carter-at-sf stephen-carter-at-sf added the PMD-AppExchange Issues related to the pmd-appexchange engine rules label May 23, 2024
@rrajaram-salesforce
Copy link
Collaborator

I don't want to remove the variable "key" from being discovered;
I realize that leads to false +ves; but I'd rather report false +ves than miss a true +ve.
Also, we have to look for variable names that include "key" or "token" etc,.

An updated rule which should be available soon which is a bit more nuanced.

  • Empty string variables will not be reported
    for ex: String apikey=''; // will not reported as an issue

  • String literals assigned to "interesting" variables will not reported if the string belongs to a pre-determined list
    for ex: String token = 'Basic ' + base64EncodedVar; //will not reported
    Both the above put together should reduce the false +ve count for this rule.

  • But the below might still get reported
    ex: String certified='something'; //matches key word "cert"

For those reported issues that you believe are false +ves ; please add //NOPMD comment to the line so that future scans will not report it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PMD-AppExchange Issues related to the pmd-appexchange engine rules
Projects
None yet
Development

No branches or pull requests

4 participants