Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Apex and VisualForce rules #3085

Merged
merged 24 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
18f0d4d
Add apex and VF security rules
garretpatten-ncino Aug 30, 2023
cb00d04
Remove best practice rule
garretpatten-ncino Aug 30, 2023
28c0b82
Clean up
garretpatten-ncino Aug 30, 2023
56b7ccd
Add performance and best practice rules
garretpatten-ncino Aug 30, 2023
d5a27e6
Rename AbsoluteUrls rule to use Pascal Case
garretpatten-ncino Aug 30, 2023
fa6a1d7
Merge branch 'develop' into develop
garretpatten-ncino Aug 31, 2023
7412e16
Update technology to Salesforce
garretpatten-ncino Sep 19, 2023
630adf2
Merge branch 'develop' into develop
garretpatten-ncino Sep 19, 2023
bb14e74
Merge branch 'develop' into develop
garretpatten-ncino Sep 27, 2023
86bf953
Update ids and metadata
p4p3r Oct 4, 2023
3be9d8e
Use standard folder structure
p4p3r Oct 5, 2023
c980c40
Update ids and metadata for VF rules
p4p3r Oct 5, 2023
a0d5fd4
Fix lints
p4p3r Oct 5, 2023
93e61c6
Move best practice rule
p4p3r Oct 5, 2023
1a57109
Merge pull request #1 from garretpatten-ncino/claudio-semgrep/review
garretpatten-ncino Oct 6, 2023
dd65caf
Merge branch 'develop' into develop
garretpatten-ncino Oct 6, 2023
f65cc80
Fix testsuite
p4p3r Oct 9, 2023
0b2b53a
Merge pull request #2 from garretpatten-ncino/claudio-semgrep/fix-tests
p4p3r Oct 9, 2023
212cbf8
Merge branch 'develop' into develop
garretpatten-ncino Oct 16, 2023
7d9a4c1
Update min-version
p4p3r Oct 17, 2023
66b88f3
Merge pull request #3 from garretpatten-ncino/claudio/min-version
p4p3r Oct 17, 2023
71c0813
Merge remote-tracking branch 'upstream/develop' into claudio/sync-ups…
p4p3r Oct 18, 2023
cd99d0f
Merge remote-tracking branch 'upstream/develop' into claudio/sync-ups…
p4p3r Oct 18, 2023
ffa3eb2
Merge pull request #4 from garretpatten-ncino/claudio/sync-upstream
p4p3r Oct 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* global javadoc
* javadoc with global in it
* javadoc global
*/
// ruleid: global-access-modifiers
global without sharing class GlobalAccessModifiers {
// ruleid: global-access-modifiers
global String globalInstanceVariable = 'globalInstanceVariable';

// ok: global-access-modifiers
public String publicInstanceVariable = 'publicInstanceVariable';

// ok: global-access-modifiers
private String privateInstanceVariable = 'privateInstanceVariable';

// ok: global-access-modifiers
// This is a test comment that has the word global in it
// ruleid: global-access-modifiers
global static void myGlobalMethod() { }

// ok: global-access-modifiers
public static void myPublicMethod() { }

// ok: global-access-modifiers
private static void myPrivateMethod() { }

// ok: global-access-modifiers
// This is another test comment with global
// ruleid: global-access-modifiers
global with sharing class TestGlobalClass { }

// ok: global-access-modifiers
public with sharing class TestPublicClass { }

// ok: global-access-modifiers
private without sharing class SystemMode { }

// ok: global-access-modifiers
// Global test comment - last one
// ruleid: global-access-modifiers
global static String globalStaticVariable = 'globalStaticVariable';

// ok: global-access-modifiers
public static String publicStaticVariable = 'publicStaticVariable';

// ok: global-access-modifiers
private static String privateStaticVariable = 'privateStaticVariable';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
rules:
- id: global-access-modifiers
min-version: 1.44.0
severity: WARNING
languages:
- apex
metadata:
cwe:
- 'CWE-284: Improper Access Control'
category: best-practice
technology:
- salesforce
references:
- https://cwe.mitre.org/data/definitions/284.html
message: >-
Global classes, methods, and variables should be avoided (especially in managed packages) as they can
never be deleted or changed in signature. Always check twice if something needs to be global.
patterns:
- pattern-regex: global [A-Za-z0-9_]{3,}
- pattern-not-regex: //(\s+([a-zA-Z]+\s+)+)[a-zA-Z]+
- pattern-not-regex: '[*](\s+([a-zA-Z]+\s+)+)[a-zA-Z]+'
paths:
exclude:
- "*Test*"
- "*test*"
13 changes: 13 additions & 0 deletions apex/lang/best-practice/ncino/tests/UseAssertClass.cls
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
public class UseAssertClass {
// ruleid: use-assert-class
System.assert(true);

// ruleid: use-assert-class
System.assertEquals(1,1);

// ruleid: use-assert-class
System.assertNotEquals(1,1);

// ok: use-assert-class
Assert.areEqual(1,1);
}
18 changes: 18 additions & 0 deletions apex/lang/best-practice/ncino/tests/UseAssertClass.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
rules:
- id: use-assert-class
min-version: 1.44.0
severity: WARNING
languages:
- generic
metadata:
category: best-practice
technology:
- salesforce
message: >-
Assert methods in the System class have been replaced with the Assert class:
https://developer.salesforce.com/docs/atlas.en-us.apexref.meta/apexref/apex_class_System_Assert.htm
pattern-regex: System\.assert
paths:
include:
- "*.cls"
- "UseAssertClass.cls"
51 changes: 51 additions & 0 deletions apex/lang/best-practice/ncino/urls/AbsoluteUrls.cls
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Test Controller with Absolute URLs
*/
public with sharing class AbsoluteUrls {

public PageReference absoluteSalesforceUrlExample() {
// ruleid: absolute-urls
String strUrl = 'https://na8.salesforce.com/TestVFPage?AccountId=999';
PageReference newUrl = new PageReference(strUrl);
newURL.setRedirect(true);

return newURL;
}

public PageReference absoluteNonSalesforceUrlExample() {
// ok: absolute-urls
String strUrl = 'https://www.website.com';
PageReference newUrl = new PageReference(strUrl);
newURL.setRedirect(true);

return newURL;
}

public PageReference nonAbsoluteSalesforceUrlExample() {
// ok: absolute-urls
String strUrl = URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AccountId=999';
PageReference newUrl = new PageReference(strUrl);
newURL.setRedirect(true);

return newURL;
}

// Absolute Salesforce URL comment example
// ruleid: absolute-urls
// https://na8.salesforce.com/TestVFPage?AccountId=999

// Absolute non-Salesforce URL comment example
// ok: absolute-urls
// https://www.website.com

// Non-absolute Salesforce URL comment example
// ok: absolute-urls
// URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AccountId=999

// ruleid: absolute-urls
private static final String ABSOLUTE_SF_URL_CONSTANT = 'https://na8.salesforce.com/TestVFPage?AccountId=999';
// ok: absolute-urls
private static final String ABSOLUTE_NON_SF_URL_CONSTANT = 'https://www.website.com';
// ok: absolute-urls
private static final String NON_ASBOLUTE_SF_URL_CONSTANT = URL.getOrgDomainUrl().toExternalForm() + '/TestVFPage?AccountId=999';
}
21 changes: 21 additions & 0 deletions apex/lang/best-practice/ncino/urls/AbsoluteUrls.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
rules:
- id: absolute-urls
min-version: 1.44.0
severity: WARNING
languages:
- apex
metadata:
category: best-practice
technology:
- salesforce
message: >-
Using absolute URLs to Salesforce Pages is bug prone. Different sandboxes and production
environments will have different instance names (like "na10", "na15" etc.). Code using
absolute URLs will only work when it runs in the corresponding salesforce instances. It
will break as soon as it is deployed in another one. Thus only relative URLs, i.e. without
the domain and subdomain names, should be used when pointing to a salesforce page.
pattern-regex: (http|https)://.*(salesforce|force|visualforce)\.com\.*
paths:
exclude:
- "*Test*"
- "*test*"
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
public class AvoidNativeDmlInLoops {
public void insertInsideLoop() {
for (Integer i = 0; i < 151; i++) {
// ruleid: avoid-native-dml-in-loops
insert account;
// ruleid: avoid-native-dml-in-loops
Database.insert(a);
}
}

public void insertInsideLoop2() {
for (Account a : accounts) {
// ruleid: avoid-native-dml-in-loops
insert account;
// ruleid: avoid-native-dml-in-loops
Database.insert(a);
}
}

public void insertInsideLoop3() {
while (someCondition) {
// ruleid: avoid-native-dml-in-loops
insert account;
// ruleid: avoid-native-dml-in-loops
Database.insert(a);
}
}

public void insertInsideLoop4() {
do {
// ruleid: avoid-native-dml-in-loops
insert account;
// ruleid: avoid-native-dml-in-loops
Database.insert(a);
} while (someCondition);
}

public void updateInsideLoop() {
for (Integer i = 0; i < 151; i++) {
// ruleid: avoid-native-dml-in-loops
update account;
// ruleid: avoid-native-dml-in-loops
Database.update(a);
}
}

public void updateInsideLoop2() {
for (Account a : accounts) {
// ruleid: avoid-native-dml-in-loops
update account;
// ruleid: avoid-native-dml-in-loops
Database.update(a);
}
}

public void updateInsideLoop3() {
while (someCondition) {
// ruleid: avoid-native-dml-in-loops
update account;
// ruleid: avoid-native-dml-in-loops
Database.update(a);
}
}

public void updateInsideLoop4() {
do {
// ruleid: avoid-native-dml-in-loops
update account;
// ruleid: avoid-native-dml-in-loops
Database.update(a);
} while (someCondition);
}

public void upsertInsideLoop() {
for (Integer i = 0; i < 151; i++) {
// ruleid: avoid-native-dml-in-loops
upsert account;
// ruleid: avoid-native-dml-in-loops
Database.upsert(a);
}
}

public void upsertInsideLoop2() {
for (Account a : accounts) {
// ruleid: avoid-native-dml-in-loops
upsert account;
// ruleid: avoid-native-dml-in-loops
Database.upsert(a);
}
}

public void upsertInsideLoop3() {
while (someCondition) {
// ruleid: avoid-native-dml-in-loops
upsert account;
// ruleid: avoid-native-dml-in-loops
Database.upsert(a);
}
}

public void upsertInsideLoop4() {
do {
// ruleid: avoid-native-dml-in-loops
upsert account;
// ruleid: avoid-native-dml-in-loops
Database.upsert(a);
} while (someCondition);
}

public void deleteInsideLoop() {
for (Integer i = 0; i < 151; i++) {
// ruleid: avoid-native-dml-in-loops
delete account;
// ruleid: avoid-native-dml-in-loops
Database.delete(a);
}
}

public void deleteInsideLoop2() {
for (Account a : accounts) {
// ruleid: avoid-native-dml-in-loops
delete account;
// ruleid: avoid-native-dml-in-loops
Database.delete(a);
}
}

public void deleteInsideLoop3() {
while (someCondition) {
// ruleid: avoid-native-dml-in-loops
delete account;
// ruleid: avoid-native-dml-in-loops
Database.delete(a);
}
}

public void deleteInsideLoop4() {
do {
// ruleid: avoid-native-dml-in-loops
delete account;
// ruleid: avoid-native-dml-in-loops
Database.delete(a);
} while (someCondition);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
rules:
- id: avoid-native-dml-in-loops
min-version: 1.44.0
severity: ERROR
languages:
- generic
metadata:
category: performance
technology:
- salesforce
message: >-
Avoid DML statements inside loops to avoid hitting the DML governor limit.
Instead, try to batch up the data into a list and invoke your DML once on
that list of data outside the loop.
patterns:
- pattern-either:
- pattern-inside: |
for (...) {
...
}
- pattern-inside: |
while (...) {
...
}
- pattern-inside: |
do {
...
} while (...);
- pattern-either:
- pattern: |
insert $DATA;
- pattern: |
update $DATA;
- pattern: |
upsert $DATA;
- pattern: |
delete $DATA;
- pattern: |
Database.insert($DATA);
- pattern: |
Database.update($DATA);
- pattern: |
Database.upsert($DATA);
- pattern: |
Database.delete($DATA);
Loading
Loading