6

For the Force.com Security Source Scanner, is there a mechanism of the same nature as eslint-disable-line for inhibiting known false-positive reports? With the false positives excluded, future scans could focus on the new issues flagged not on a repeated analysis of the false positives.

What approach have others taken to this problem?

PS

An example where the test code has been written to avoid duplication and make the tests more readable:

@IsTest
private class MyTest {

    private class Builder {

        ...

        void assertTotals(...) {
            System.assert(...);
        }
    }

    @IsTest
    static void case1() {
        new Builder().addAbc(...).addDef(...).assertTotals(...);
    }

    @IsTest
    static void case2() {
        new Builder().addAbc(...).addDef(...).assertTotals(...);
    }

    ...
}

results in these false positive reports for each @IsTest method:

Test Methods With No Assert

4

You really only have two choices: fix the code so it no longer generates the false positive, or live with it. The first almost always results in suboptimal code changes, such as purposefully calling String.escapeSingleQuotes on a String that's been previously filtered by a regular expression to never include quotes. I believe there are configuration options you could use in the on-premise version of checkmarx, but the online scanner is pre-configured based on recommendations from the security review team. They want to see the false positives in the report, even though they amount to just so much clutter. Having annotations or comments you could use to suppress these warnings would defeat the purpose of these scans. In the meantime, you can submit bugs to checkmarx, and they may get around to fixing it in a future release. I'm not sure what their policy is on this, so you might want to look at their site.

  • 1
    I'd be happy with two categories of warnings in the report: "flagged as known ones" (perhaps with some cross reference to an explanation in the marker) and "not flagged as known ones". If Salesforce want to review the former ones to see if they are bogus then that is up to them. Personally I'd like to be able to focus on the latter ones. – Keith C Mar 29 '18 at 17:04
  • @KeithC Yes, that would be a nice idea. Maybe you should post one on Salesforce Ideas? – sfdcfox Mar 29 '18 at 17:16
  • Personally don't have much faith in that channel for dev oriented stuff... – Keith C Mar 29 '18 at 19:06
  • @KeithC True enough :( Maybe you could find the head of the security team and ask them? Someone is ultimately responsible for the flags and integration, so I presume there's someone that would be appropriate to ask. I'm not sure who that might be, though. – sfdcfox Mar 29 '18 at 19:10
1

My present thought is to comment the code for our internal benefit. Using the example from the question:

// checkmarx false positive type 7
@IsTest
static void case1() {
    new Builder().addAbc(...).addDef(...).assertTotals(...);
}

or maybe this so the comment carries through into the Checkmarx report:

@IsTest
static void case1() {   /* checkmarx 7 */
    new Builder().addAbc(...).addDef(...).assertTotals(...);
}

where the false positive numbers relate to sections of the explanatory document submitted as part of the security review. This should make it easier the next time to explain the false positives.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.