AppExchange Security Review: What I Learned the Hard Way

The AppExchange Security Review is one of those things that sounds straightforward until you’re three weeks into your second re-submission, rewriting false positive documentation at midnight. After going through this process with our 2GP managed package at Tucario, I want to share what actually matters — and what the documentation doesn’t tell you.

What the Security Review actually checks

The review has three layers, and understanding each one saves you weeks of back-and-forth.

Checkmarx runs an automated static code scan. It looks for patterns: missing CRUD/FLS, without sharing without justification, SOQL without USER_MODE, CSRF, DML in loops. You can trigger a Checkmarx scan yourself before submission through the Salesforce Code Scanner — do this early and often.

Manual penetration testing is where a reviewer actually uses your app. They check for clickjacking, sharing violations, runtime CRUD/FLS enforcement. This is the part that catches issues automated tools miss.

Salesforce Code Analyzer v5 is your responsibility. You run it locally, attach the HTML report to your submission, and document every Critical/High finding you’re not fixing.

The timeline: expect 2-4 weeks for initial feedback, 1-2 weeks for re-submissions. Plan accordingly.

CRUD/FLS in 2GP — the part nobody warns you about

This is where I spent the most time, and where most ISVs get stuck on their first submission.

The mistake is thinking CRUD checks are enough. Adding isCreateable() before an insert and calling it done. Reviewers will reject this. They require field-level security enforcement, not just object-level CRUD.

In a second-generation managed package, the standard FLS approaches simply do not work:

The solution is System.AccessLevel.USER_MODE — with the System. prefix. This is critical:

// Works in 2GP
Database.insert(record, true, System.AccessLevel.USER_MODE);
Database.update(records, true, System.AccessLevel.USER_MODE);

// Does NOT compile in 2GP
Database.insert(record, true, AccessLevel.USER_MODE);

That System. prefix is the difference between a clean build and a compiler error that makes no sense until you understand namespace resolution in managed packages.

When USER_MODE will break your app

System.AccessLevel.USER_MODE enforces sharing rules in addition to CRUD/FLS. That sounds great until it silently breaks business logic in specific contexts.

Do not use it in without sharing classes — USER_MODE overrides without sharing, defeating the purpose. Do not use it in trigger handlers — these operate in system context, and user validation belongs earlier in the call stack. Do not use it with Share objects — they have a fixed, platform-defined schema with no configurable FLS. And do not use it in @InvocableMethod methods called from Flow — any user can trigger a Flow, and they may not have the required permission set.

In these cases, use explicit CRUD checks and document them as false positives. Trying to force USER_MODE everywhere will break your app for users who don’t have the full permission set assigned.

The without sharing architecture pattern

Every managed package needs without sharing somewhere. The reviewer knows this. What they want to see is a controlled, documented security architecture.

On our package, we settled on this pattern:

LWC Component
  -> @AuraEnabled Controller (with sharing)
    -> Authorization check: isCurrentUserManager()
    -> CRUD/FLS check
    -> private without sharing inner class
      -> CRUD check (again, in every method)
      -> DML operation

The key compensating controls that get without sharing approved:

1. private visibility on the inner class — it cannot be called from outside the controller
2. CRUD checks in every method — isAccessible(), isUpdateable(), isDeletable()
3. Authorization gate — isCurrentUserManager() or equivalent before reaching the elevated class
4. with sharing outer class — the calling class always respects sharing
5. Full documentation in the false positive DOCX

One rule that saves you a Checkmarx finding: always declare with sharing or without sharing explicitly. Classes without a declaration are flagged as a sharing violation, even if the default behavior is correct.

Share objects are a special case

If your package works with share records (AccountShare, ObjectTeamMember__Share, custom __Share objects), you need to understand that these have a fixed, platform-defined schema. Fields like ParentId, UserOrGroupId, AccessLevel, and RowCause are system fields with no configurable FLS.

Security.stripInaccessible() does not apply. USER_MODE does not apply. CRUD checks (isCreateable(), isDeletable()) are required. Operations on share objects always require without sharing or elevated context.

This is a legitimate false positive for every finding related to FLS on share objects. Document it clearly and cite the platform behavior.

Surviving Checkmarx

A Checkmarx report will never be clean. This is not a failure — it’s by design. The scanner flags patterns, and many of those patterns are intentional in a managed package.

Findings that will always appear as false positives:

• SOQL SOSL User Mode Missing — without sharing classes, trigger handlers, setup objects, dynamic SOQL
• Sharing — every without sharing class, even with compensating controls
• CRUD Delete — trigger handlers, Queueable, Batch, even with isDeletable() checks
• FLS Create — trigger handlers, PostInstallHandler, system context
• Apex CSRF In Aura LWC — LWC has built-in CSRF protection, always a false positive
• DML Statements Inside Loops — bounded by design with distinct object types or batch sizes

What Checkmarx does not recognize: isDeletable() as CRUD mitigation (it expects WITH USER_MODE), private visibility as a compensating control, authorization gates like isCurrentUserManager(), and @SuppressWarnings annotations.

The lesson: stop trying to make the report green. Focus on writing thorough false positive documentation.

Salesforce Code Analyzer v5

Run this before every package build:

sf code-analyzer run \
  --workspace force-app \
  --output-file code-analyzer-report.html \
  --output-file code-analyzer-report.csv

Focus on Critical and High findings — fix them or document them. Moderate findings are code quality issues (complexity, SLDS patterns) that reviewers typically don’t block on.

One gotcha: @SuppressWarnings('PMD.ApexCRUDViolation') cannot be combined with @InvocableMethod. Apex throws “The only annotation that can be used with InvocableMethod is Deprecated.” These must be documented as false positives — there is no code-level fix.

False positive documentation that works

The quality of your false positive documentation determines whether you pass on the next round or get sent back again.

For each finding, the reviewer needs to see:

1. The exact vulnerability name from the scan
2. Which scanner found it — Checkmarx and Code Analyzer get separate documents
3. Why it’s a false positive — not “because we need it,” but the specific technical reason USER_MODE cannot be used
4. The data flow — LWC -> controller (with sharing) -> auth check -> CRUD -> elevated class
5. Line numbers, method names, class names — be specific

The most important thing I learned: explain the business case. Don’t just say “because without sharing.” Explain that the object has Private OWD, that users need to see team members across record ownership, that Flow context prevents USER_MODE, that share objects have no FLS. Reviewers are security engineers — they understand nuance when you provide it.

The package build checklist

Before every submission:

# Run Code Analyzer first — fix Critical/High before building
sf code-analyzer run \
  --workspace force-app \
  --output-file code-analyzer-report.html \
  --output-file code-analyzer-report.csv

# Build with code coverage
sf package version create -p MyPackage -w 60 \
  --code-coverage --target-dev-hub DevHUB --installation-key-bypass

# Verify coverage (minimum 75%)
sf package version list --target-dev-hub DevHUB \
  --packages MyPackage --order-by CreatedDate

# Promote — this is irreversible
sf package version promote --package 04t... --target-dev-hub DevHUB

Do not promote before fixes — a promoted version is immutable. Do not build a new package version if there are no code changes. Run Code Analyzer before building, not after. And remember: Checkmarx scans the promoted version, so promoted must equal final.

The takeaway

The AppExchange Security Review is not a scan-and-fix exercise. It’s a conversation between your architecture and a security engineer. The code needs to be secure, but the documentation needs to tell the story of why your architecture is sound.

Build your security model in layers: UI, controller with sharing, authorization gate, CRUD/FLS check, elevated inner class with CRUD checks again. Document every layer. Explain every without sharing. Justify every false positive with technical specifics, not hand-waving.

A clean Checkmarx report is a myth. A well-documented security architecture is what gets you through.