Automated static analysis catches a lot. It doesn’t catch everything. Here’s the checklist I use for security-focused Apex code reviews - focused on the things that require human judgment.
Access Control
- Every class has an explicit
with sharing,without sharing, orinherited sharingdeclaration. No implicit sharing. - Classes declared
without sharinghave a documented justification. “It was easier” is not a justification. -
Schema.describeSObjectResultand field-level describe calls used before accessing data in dynamic queries. - REST and SOAP endpoints verify the calling user’s permissions before executing - don’t rely solely on profile settings.
- Invocable methods and
@AuraEnabledmethods checkFeatureManagement.checkPermission()where applicable.
Injection
- No
Database.query()calls using string concatenation. All dynamic queries use bind variables orString.escapeSingleQuotes()for legacy code. -
SOSLqueries use the same bind variable pattern. - No
Url.getOrgDomainUrl()or similar calls whose output is used unsanitised in redirect responses. - No
String.format()used to construct SOQL - ever.
// This will fail code review
String q = 'SELECT Id FROM Account WHERE Name = \'' + input + '\'';
// This passes
List<Account> results = [SELECT Id FROM Account WHERE Name = :input];
Cryptography
- No sensitive data (passwords, tokens, SSNs, payment data) stored in custom settings, custom metadata, or standard text fields.
-
Crypto.generateAesKey()used for symmetric encryption - notMath.random()or timestamp-based “keys.” - HMAC used for message signing, not MD5 or SHA-1 alone.
- OAuth tokens stored in Named Credentials - not in custom objects.
- No Base64 encoding presented to users as “encryption.”
Data Exposure
-
@AuraEnabledmethods return only the fields required by the component - not entire SObject records with all fields populated. - Error messages don’t expose internal implementation details (Salesforce IDs, field names, exception stack traces) to the end user.
- Debug logs don’t log sensitive field values. Use
Logger.mask()patterns or avoid logging PII entirely. - Visualforce pages don’t use
<apex:outputText escape="false">with any user-controlled content.
External Callouts
- Endpoint URLs are validated against an allowlist before any
HttpRequest.setEndpoint()call. - Response bodies are validated before processing - don’t assume the external service returns what you expect.
- Timeouts are set explicitly. No callout with
setReadTimeout(0)or default timeouts. - Callout responses are not logged in their entirety - response bodies may contain tokens or PII.
// Always set explicit timeouts
HttpRequest req = new HttpRequest();
req.setTimeout(30000); // 30 seconds max
req.setEndpoint('callout:My_Named_Credential/api/endpoint');
Platform Events and Triggers
- Trigger handlers check
TriggerOperationcontext - anafter inserthandler shouldn’t execute onbefore delete. - Platform Event publishers don’t include sensitive data in the event payload - events are visible to all subscribers with the appropriate object permission.
- Asynchronous operations (future methods, Queueable, Batch) that process sensitive data don’t pass that data as parameters - query it fresh within the async context.
Test Code
- No
@isTest(SeeAllData=true)anywhere in code destined for a managed package or production deployment. - Test classes test failure scenarios and access control - not just happy-path functionality.
-
System.runAs()used to test different user contexts. - Test data created by the test itself - no dependency on org data.
The Non-Obvious Ones
Sharing recalculation - Code that uses Database.update() with a sharing-relevant field change may not recalculate sharing rules immediately. Test this explicitly.
CSRF in Visualforce - Pages that perform DML should include {!$Page.name} in the form action, use apex:form, or implement a custom CSRF token for non-standard form submissions.
Governor limit as a security control - Never use “it’ll hit a governor limit before it becomes a problem” as a security argument. Governor limits change between releases and can be raised by Salesforce support.
Order of operations in trigger chains - A trigger that disables another trigger to “avoid recursion” is hiding business logic and potentially disabling security checks. Understand what you’re disabling before disabling it.
This checklist doesn’t replace automated scanning - it supplements it. Run PMD and Code Analyzer first, then apply human judgment to what they can’t catch.