How to pass Salesforce security review for 2nd generation managed package without using "with sharing" while defining class?

Question

We have developed a 2nd generation managed package and submitted our package for security review. But after reviewed by Salesforce, they suggested us some suggestion. One of the suggestions received is regarding the use of "with sharing" while defining classes in which DML operations are performed.

However, based on our specific requirements, we must utilize 'without sharing' for certain classes. We have implemented rigorous field-level security checks before executing DML statements within these classes to ensure a secure application. Can anyone suggest how can we pass security review with class defined as "without sharing" and DML statement is used in same class?

We need guidance on how we can address this concern and successfully pass the security review. Are there specific considerations or documentation requirements that we should use, such as including details in a False Positive document?

Answer 1

To add to David Reed's succinct and appropriate answer, I just wanted to say that there are ways to reduce the burden on collating and documenting your false positives.

The approach we have successfully applied over the last 7 years is to have a short false positive document that explains how we have covered each false positive by annotating the code with formatted comments, on each line where a false positive exists.

The format of comments we use is consistent, but the content varies based on the type of deviation from "best practice", such as Sharing, CRUD/FLS, XSS etc.

Since, historically, Salesforce has used Checkmarx to perform the static analysis and Checkmarx includes source code lines that include deviations in the report, we have added these (single line) comments on the end of the first line of the statement with the deviation. That way the Checkmarx report actually includes the false positive explanation.

We are expecting to change the approach applied slightly as Salesforce moves from using Checkmarx to applying PMD and other scanning technology, but the principle remains the same:

Ensure that each deviation is commented or annotated in a way that clearly explains why the deviation exists.

Our comments are formatted as:

<some code> // <type> False Positive: <explanation>

In the case of using without sharing we apply a "<type>" of "Sharing". An example of one such comment, with an explanation, is:

public without sharing class ErrorObjectLogging { // Sharing False Positive: This is a utility class to log errors, from data processing, which must be ignorant of sharing rules and user permissions (errors must always be logged)

Since the entire comment is on the first line of the statement (here a class declaration) with the deviation, Checkmarx will include it in the report. This allows the security reviewer to understand why we have the deviation without us having to capture every single one explicitly in a document - indeed, trying to do so would mean we had a document with 3600+ false positives which would be almost impossible to maintain outside of the source code.

Answer 2

Are there specific considerations or documentation requirements that we should use, such as including details in a False Positive document?

Yes, that. You need to provide Security Review with a detailed explanation of why you need to use without sharing in the specific locations you’re using it.

That’s it. You’re diverging from best practices and guidelines. You need to provide a convincing explanation as to why, and why it’s not an actual security risk.

I am a Salesforce employee but do not work on Security Reviews