Skip to main content

AgentExchange Security Review false positives: what reviewers accept

· 9 min read
Vulkro
Security research

Every package that goes through the AgentExchange (formerly AppExchange) Security Review arrives with scanner output, and almost all scanner output contains false positives. A CRUD finding on a custom setting. A sharing finding on a class that declares inherited sharing. A SOQL injection finding on a query where every value is a bind. The security review false positive document is where you explain each of those, and it decides whether a reviewer spends their time on your real risks or on questions about your noise.

Salesforce treats the document as part of the submission. Its packaging guide has a page on how to document your responses to false positives, offers a template, and asks for the code location and the reason the code is not vulnerable for each flagged issue. If the explanation will not hold up, the guidance is to change the code instead.

That last point is the one teams skip. A false positive document is not a place to argue. It is a list of checkable claims. This post covers what a claim needs, what reviewers push back on, and how to produce the evidence without writing every justification from scratch.

What is a false positive in the security review?​

A scanner reports a pattern. A false positive is a report where the pattern is present but the risk is not, because something the scanner did not account for prevents it. The word that matters is "because". "This is a false positive" is an opinion. "This query runs WITH USER_MODE on line 42, so the platform enforces object and field access" is a fact a reviewer can open the file and confirm.

Three things are not false positives, and listing them as such costs credibility for the rest of the document:

  • Code that is safe only because of how the org is configured. Your package is installed into orgs you do not control.
  • Code that is safe only because the UI never calls it that way. An @AuraEnabled or @RestResource method can be called directly, with any arguments.
  • Code that is risky but intended. A without sharing class that genuinely needs system access is a design decision. Document it as one, with the reason and the mitigations, rather than calling it a false positive.

How do you write the security review false positive document?​

Write one entry per finding, not one per rule. A reviewer checks entries one at a time, and a blanket line such as "all CRUD findings are false positives because we check access" invites them to find the one place you did not.

Each entry needs five parts:

  1. The finding as reported. Scanner, rule name, file and line, exactly as they appear in the report, so the reviewer can match it.
  2. The verdict. False positive, or intended behaviour with justification.
  3. The mechanism. The specific control that prevents the risk: the bind variable, the WITH USER_MODE clause, the Security.stripInaccessible call, the custom metadata type that holds no user data.
  4. The proof line. Where that control is, as file:line, close enough to the finding that the reviewer can see both on one screen.
  5. What would make it wrong. The condition under which the justification would stop holding, if there is one. Reviewers trust a team that names its own limits.

A good entry reads like this:

Finding: ApexCRUDViolation, classes/SettingsService.cls:31
Verdict: False positive
Mechanism: The DML targets Integration_Settings__c, a hierarchy custom
setting that stores configuration only. It holds no customer
record data.
Proof: objects/Integration_Settings__c/Integration_Settings__c.object-meta.xml
declares <customSettingsType>Hierarchy</customSettingsType>.
Limits: Would not hold if the setting began storing user data.

A weak entry reads like this:

Finding: ApexCRUDViolation, SettingsService.cls
Verdict: False positive. We follow Salesforce best practice.

The second one has no line, no mechanism and no proof. It will come back as a question, and the review waits while you answer it.

What evidence do reviewers accept?​

Evidence a reviewer can check without trusting you. In practice:

  • Code they can read. The guarding statement, cited by line.
  • Metadata they can read. A custom setting or custom metadata type definition, a class API version, a sharing declaration.
  • Platform behaviour with a source. For example, that a bind variable in static SOQL is not concatenated into the query string. Cite Salesforce's documentation for the behaviour rather than asserting it.
  • Test code. A finding inside an @IsTest class does not ship as runtime behaviour, and saying so is a complete answer.

What does not work: screenshots of a clean org, statements about how customers use the product, and "the scanner is known to be noisy". The reviewer already knows scanners are noisy. That is why they ask for the document.

What should a false positive document never claim?​

  • Never claim a fix you have not made. If you change code in response to a finding, say so and point to the new line. Do not list it as a false positive.
  • Never rely on client-side checks. A check in LWC or Aura JavaScript does not protect an Apex method. The Aura developer guide tells developers to assume an attacker can call any @AuraEnabled method with any parameters.
  • Never justify by obscurity. "The endpoint is not documented" is not access control.
  • Never paste a generated paragraph you have not verified. If a tool drafted the text, someone on your team should have opened every cited line.

How Vulkro helps produce the document​

The slow part of a false positive document is not the writing. It is finding, for each of dozens of scanner rows, the line that proves the row wrong, or discovering that it is not wrong at all. Vulkro for Salesforce does that lookup from its own analysis.

vulkro-sf appexchange-report imports a Salesforce Code Analyzer report (JSON, SARIF or CSV from sf code-analyzer run --output-file) and gives each row one of three verdicts:

  • Vulkro agrees. Vulkro has its own finding of the same class in the same method. This row is never drafted as a false positive, whatever else is true. Fix it.
  • Draft false positive, marked certain or likely. One kind of evidence holds, with a file:line proof: the operation targets a custom setting or custom metadata type; the statement runs in user mode or its records pass through Security.stripInaccessible; every value in the query is a bind or a typed value; the class declares inherited sharing; the row sits in an @IsTest class; or a "credential" is a placeholder or a Named Credential merge field.
  • Needs rationale. Nothing decisive. The row lists what Vulkro does know, and a person writes the entry.

Drafts come from fixed templates filled with Vulkro's own evidence, not from a language model, and every draft carries an evidence line naming the scanner build so the text can be traced. Only the custom setting and test code cases are marked certain. The others match guards in the source, so someone should still read the cited line.

sf code-analyzer run --output-file ca-results.json
vulkro-sf appexchange-report ./force-app \
--import-code-analyzer ca-results.json \
--fpw-out FPW.json \
-o readiness.html

The HTML report carries the submission packet: the review-blocking findings ordered by estimated fix effort, then the imported rows with their verdicts and drafts. --fpw-out writes a False Positives Whitelist file where every entry is marked accepted or draft. A draft is never promoted to accepted by the tool. The packet does not replace the Code Analyzer report the review requires.

Decisions your team has already made feed the same file. A finding marked as a false positive or an accepted risk in .vulkro/triage.toml, with its reason, is reviewed in pull requests like code and carried into the whitelist as accepted. So the document grows as you build, rather than the week before you submit.

False positive document checklist​

  • One entry per finding, matched to the report by rule, file and line.
  • A verdict for each: false positive, intended with justification, or fixed.
  • A specific mechanism and a file:line proof for every false positive.
  • No reliance on org configuration, UI flow or client-side checks.
  • Intended without sharing and system-mode code documented as design decisions.
  • Every drafted entry read by a person before upload.

Where to see it​

The AgentExchange Security Review use case shows the whole flow from first scan to submission. What fails the AgentExchange Security Review covers the findings you should fix rather than justify, and the readiness report docs list every flag. Findings in Lightning components are covered in LWC security.

To run it on your package, get started.

Frequently asked questions​

What should a security review false positive document include?​

For each flagged issue: the finding as reported (rule, file, line), your verdict, the specific control that prevents the risk, and the line where that control is. Salesforce's guidance asks for the code location and the reason the code is not vulnerable, and recommends using its template.

Can I mark a whole rule as a false positive?​

It is better not to. Reviewers check findings one at a time, and a blanket statement invites them to find the exception. Write one entry per finding, even when many share the same reason.

Is a without sharing class a false positive?​

Usually not. If a class needs system access, that is a design decision: document why it needs it and what limits its use, such as the callers it serves and the fields it touches. Calling it a false positive suggests you have not looked.

Does Vulkro write the false positive document for me?​

It drafts justifications for imported Code Analyzer rows where its own analysis finds evidence, marks each one as a draft with a confidence, and leaves the rest as needing a rationale. A person still reads every draft and decides what to submit.