Working with the Salesforce CLI Scanner

Tweet

Introduction

A couple of weeks ago I had my first exposure to the Salesforce CLI Scanner and it's fair to say I quite liked it - two blog posts in two days is above my usual cadence. The first thing I wanted to do was find out if I could tune it to my specific requirements, which  I was pleased to find out I could by creating custom XPath rules and installing them into my CLI instance.

The next job was to introduce them into my workflow and take action when issues were flagged up, which I've been working on in what time I could carve out.

Customising the Default Rulesets

Some of the default rules from the CLI Scanner weren't applicable to all of my projects - an example being the CRUD/FLS enforcement. If I'm not targeting the app exchange, a lot of the time my code needs to be able to carry out actions on specific objects and fields regardless of the user's profile - I'm relying on running in system mode. I can't see the point in letting this be flagged up as warnings and then hiding or ignoring them, so I wanted to exclude this specific rule. At the time of writing (October 2020) the CLI Scanner doesn't allow rules to be excluded or included at run time, so I needed a way to remove them from the ruleset.

After a small amount of digging around, I found the default ruleset configurations in my ~/.sfdx-scanner directory, so I copied them and removed the CRUD rules. Executing the scanner on my sample codebase showed this had no effect, so I checked the file to make sure I'd removed all instances. I found a couple, removed them and executed the scanner again with the same result. Checking the files again showed the CRUD rules were still in place, and this time I knew that I'd definitely removed all references, so I realised that these configuration files were generated at run time so any changes I made were overwritten. Back to the drawing board.

Cloning the Default Rulesets

As PMD is open source, the rulesets are available in the Github repo, so creating a copy was fairly straightforward. I edited the rulesets to remove the rules that I wasn't interested in, and to change the name - I didn't want to replace the defaults as there are times when I want all the rules applied. 

Something else I learned since my original post is that if you are relying on existing Java rule classes, you don't need to build a JAR file containing the ruleset XML, you can just install the XML file itself, so that saved me a little time. Once I had the custom rulesets in place I was good to go.

Adding to the Workflow

The CLI scanner is now part of my overnight Continuous Integration processing on our newly launched BrightFORUM event management accelerator.  This doesn't have a vast amount of custom code and we've had a very clear set of standards and rules that are applied at code review time, so I wasn't expecting many issues, and I was pleased to find that was the case. All the violations were level 3 or lower as well, and I'd have been pretty annoyed with myself if there were any higher than that. 

Satisfying the Scanner

Clearing the violations was an interesting exercise.  Some were very simple, like unused local variables. Most of the time this was overzealous retaining a reference that wasn't used later, but sometimes it was genuine leftovers from a previous approach. As these had been through code review, I can't see that we'd have picked them up manually in the future unless we'd made further changes.

Others required more effort, like cyclomatic or cognitive complexity, which typically means methods that are trying to do too much at once. An automated approach helps here as it's not down to individual developers deciding that a method is a bit long or a bit complicated, and convince others that is the case. A tool flagging this up takes all the emotion out of it. Fixing these required some refactoring, which I had no problem undertaking as we have excellent test coverage to confirm I didn't break anything inadvertently.

Suppressing Violations

I didn't change the code in every case though - for example, I had a potential SOQL injection flagged up, as the fields in the query were dynamically generated. The string being used in the query had been generated from the built-in schema classes, a couple of lines earlier, so it was guaranteed to contain only valid field names and there was no chance of unescaped user input or the like being included. I could bow to the scanner and escape the string anyway, but that would suggest to anyone reading the code that it was coming from an untrusted source, which wasn't the case. This gave my a chance to try out an Apex annotation that two weeks ago I had no idea existed - @SuppressWarnings.

From the Apex docs : 

This annotation does nothing in Apex but can be used to provide information to third party tools.

To turn off the violation, I added the following annotation to the method that contained the SOQL query:

    @SuppressWarnings('PMD.ApexSOQLInjection')

and the violation is gone. You can suppress multiple warnings by comma separating them inside the single quotes, and you can also apply this at the class level to suppress all warnings of the specified type(s).

This is something that should definitely be used with care - once I've suppressed SOQL injection warnings for a method, any code added that introduces a genuine SOQL injection vulnerability won't get picked up. I'm typically adding a comment to explain why I've suppressed the warning and to say that the method shouldn't be changed without taking this into consideration. I'm also putting regular time into my calendar to revisit the suppress warnings annotations and confirm that I don't have a better approach.

I wouldn't use this annotation to exclude a class from all rules - that could be done, but it would be a big list of suppressions and it would be simpler to exclude the class from those processed. I'd also add a ticket that there is technical debt to be addressed in that class, so that it doesn't get forgotten about and embarrass me in the future.

Takeaways

The key takeaway from my last couple of weeks experience is that automated code scanning leads to cleaner code - things that aren't needed are gone, things that were hard to understand are now easier, and I have some custom rules that ensure that concerns are appropriately separated. Including it in my CI ensures that I pick up any issues as soon as they are added to the codebase, and I also plan to have the development team run the scanner before pushing their work. Aside from the initial effort to set this up, it's pretty much zero effort for a lot of benefit.

Static code analysis shouldn't replace manual code reviews though - an automated tool is configured to look for specific code/markup that indicates problems. A developer carrying out a code review understands the purpose of the code and can determine if it looks like it does what it is supposed to. Code reviews are also an excellent way to share knowledge in a development team. By adding automation you free up code reviewers from having to check whether every variable is used and figure out what makes a method too complex. By using both you'll catch more problems earlier, so what's not to like?

Resources

• Salesforce CLI Scanner Custom XPath Rules - Part 2
• Salesforce CLI Scanner Custom XPath Rules - Part 1
• PMD Site
• PMD Apex Rules
• Salesforce Developers Blog post introducing the scanner
• CLI Scanner Documentation

Follow @bob_buzzard

Automation with Platform Events

Tweet

Introduction

During the Trailblazers Innovate live session on 13th October, Wade Wegner made a comment about modern development being event driven rather than trigger driven, which is an approach I've been following for a while now, as in my view it maximises flexibility. Most of us are experiencing event driven development on the client side as we are working more and more with JavaScript, where user interactions generate events. It also applies server side, where platform events can be used to drive automation.

(As an aside, if you google trigger driven development with Salesforce, one of the top hits is an April Fool's Day post of mine about moving all of your code into triggers and faking record changes to make it run. Don't do that!)

Trigger Driven

The trigger approach (and I'm using this term to cover pro/low/no code mechanisms including Apex triggers, process builders, flows, workflow rules) typically means that you (via the Salesforce platform) monitor records and react to specific changes involving those records. 

An example of this, loosely based on some of the code behind my toolbox site where I keep details of really useful sites, tools, documents etc, would be to notify me if the number of clicks on an Entry goes up.  I can add automation that fires when an Entry record is updated and the clicks field is updated, and sends me the an email to that effect. This works a treat and I'd imagine there are about a billion automations similar to this out in the wild.

The downside to this is the automation is tightly coupled to the data model. If I decided for that I wanted to move the dynamic information out of the Entry and into another object, let's call it EntryInteraction, my automation has to reflect the change to the data model. I need something that monitors EntryInteraction records and emails me when the clicks field on that changes. 

Event Driven

In the event approach, rather than monitor records, my automation subscribes to a platform event that is fired when the clicks are updated. It neither knows nor cares where the clicks are stored - it could be off platform - it just knows that when it receives this event, the clicks changed somewhere, so it should send the email. The code that updates the clicks fires this platform event, and if the location of the clicks changes, that code changes. But regardless of what the code does, the event that it fires stays the same.

The automation is now decoupled from the data model and knows nothing of the internals of how the application works. As the application author, I don't need to worry about dependencies on automation, and I can change my data model as I want to reflect my changing needs. A much better situation, I'm sure you will agree.

Resources

• Platform Events Developer Guide
• Platform Events Basics Trailhead Badge

Follow @bob_buzzard

Salesforce CLI Scanner Custom XPath Rules - Part 2

Tweet

Introduction

In Part 1 of this post I showed how to create an XPath expression that looks for SOQL queries outside of test method or accessor classes via the PMD designer. This in itself was good fun, and there's a definite rush when you finally come up with the appropriate runes, but is of limited value. In this post I'll embed the expression in a custom rule and add it to the Salesforce CLI Scanner.

Rule

A rule needs to be inside a ruleset XML file. The PMD site has excellent documentation on this, so I won't reproduce it here for the sake of padding my post out.

A couple of key aspects of my rule is that it is an XPath expression to evaluate against Apex coded, so I need to add that detail to the definition so that the appropriate class can be instantiated to handle it:

<rule name="SoqlOnlyInTestAndAccessorClasses" 
      language="apex" 
      message="SOQL queries can only appear in test methods and accessor classes"
      class="net.sourceforge.pmd.lang.apex.rule.ApexXPathRule">

and the XPath expression itself appears in the rule properties, along with the XPath version:

<properties>
  <property name="version" value="2.0"/>
  <property name="xpath">
    <value>
      <![CDATA[
//UserClass[not(ends-with(@Image, 'Accessor'))]/Method/ModifierNode[@Test=false()]/..//
(SoqlExpression | MethodCallExpression[lower-case(@FullMethodName)='database.query'])
]]>
    </value>
  </property>
</properties>

Packaging the Rule

The ruleset xml needs to be packaged as a JAR file (Java ARchive) using the tool of the same name. If I'd written a custom Java rule, I'd need to compile the class and include that in the JAR, but as I'm relying on a pre-existing class (net.sourceforge.pmd.lang.apex.rule.ApexXPathRule) I can just package the ruleset xml file.

Per the official Authoring Custom Rules documentation, my ruleset file lives at:

    ./category/apex/bobbuzzard.xml

so to create my JAR I execute the following command, which gives me plenty of information about what is being added:

$ jar -cvf bobbuzzard.jar ./category

added manifest
adding: category/(in = 0) (out= 0)(stored 0%)
adding: category/apex/(in = 0) (out= 0)(stored 0%)
adding: category/apex/bobbuzzard.xml(in = 1105) (out= 545)(deflated 50%)

Installing the Rule

Once I've packaged the rule, I can install it using the scanner:rule:add Salesforce CLI command, specifying the language as Apex and the JAR I just created:

$ sfdx scanner:rule:add -l apex -p bobbuzzard.jar 

Successfully added rules for apex.
1 Path(s) added: /Users/kbowden/PMDRULE/bobbuzzard.jar

Per the official docs, I list the commands to ensure it went in correctly (when I was figuring all this out but didn't have my ruleset.xml file set up right, the command would succeed but the rule wouldn't be added, so verifying via the list command is definitely a step worth taking). So that I don't have to wade through a tone of output, I pipe it through grep to exclude any lines that don't contain the characters Buzzard :

$ sfdx scanner:rule:list | grep Buzzard

SoqlOnlyInTestAndAccessorClasses          apex         Bob Buzzard     pmd

so all is well - my rule is all present and correct.

Running the Rule

All that is left is to run my rule against some sample Apex classes. The classes, and the ruleset and JAR re available at the Github repository.

I have:

• ScannerExample.cls - this has a SOQL expression and a Database.query method call, so I expect it to be flagged twice.
• ScannerExampleAccessor.cls - as above, but as it's a class name ending in Accessor I don't expect it to be flagged.
• ScannerExampleTest.cls - a test class with one SOQL expression in a test method and one in a regular method. I expect it to be flagged once regarding the regular method

When running the scanner, I set the category to "Bob Buzzard" as I only want the code evaluated against my rule:

$ sfdx scanner:run --target './**/*.cls' -c "Bob Buzzard"

LOCATION                                                  DESCRIPTION                                          CATEGORY     U R L
────────────────────────────────────────────────────────  ───────────────────────────────────────────────────  ───────────  ─────
force-app/main/default/classes/ScannerExample.cls:5         SOQL queries can only appear in test methods and   Bob Buzzard
                                                            accessor classes
force-app/main/default/classes/ScannerExample.cls:10        SOQL queries can only appear in test methods and   Bob Buzzard
                                                            accessor classes
force-app/main/default/classes/ScannerExampleTest.cls:19    SOQL queries can only appear in test methods and   Bob Buzzard
                                                            accessor classes

and there we have it - the lines that break my rule have been identified.

Resources

• Github repository with the sample classes, rule and JAR
• Salesforce CLI Scanner Custom XPath Rules - Part 1
• PMD Site
• PMD Apex Rules
• Salesforce Developers Blog post introducing the scanner
• CLI Scanner Documentation

Follow @bob_buzzard

Salesforce CLI Scanner Custom XPath Rules - Part 1

Tweet

Introduction

Earlier this week (6th October 2020) a tweet popped up in my feed about the Salesforce CLI Scanner, which obviously caught my attention as I seem to have some kind of tunnel vision on the CLI and plug-ins right now. I'd seen something about this before, but at the time it felt like it was aimed more at ISVs to help them pass the security review. This wasn't something I had any immediate need for, so it went on my never ending mental list of things to look at when I have time. 

The tweet linked to a Salesforce Developers Blog post and quick skim read of that suggested there was plenty more to it, so I set aside some time to read it properly and take a look at the code, which was well worth the investment. The Plug-in docs cover installation and running, so I won't go into them here.

Static Code Analysis

Simply put, a static code code analysis tool examines source code before the code is run, evaluating it against a set of rules. Assuming you've crafted your rules correctly, it will find every occurrence of code that breaks a rule. Essentially a code review carried out by a Terminator:

    "It can’t be bargained with. It can’t be reasoned with. It doesn’t feel pity, or remorse, or fear."

The CLI Scanner combines a bunch of static code analysis tools into a single plug-in. The power of the CLI is that this can be installed in a single command using the CLI itself. As the CLI is pretty ubiquitous, it's straightforward for most of us to install the scanner and start using it immediately. 

Apex

While the scanner supports other languages, I'm going to stick to Apex for this post, as it's where most of my code lives right now.

The scanner evaluates Apex using the Apex rules for PMD originally created by Robert Sösemann, part of the open-source PMD source code analyser. I'd looked at PMD several years ago and got the basics working, but things weren't quite as slick back then so I didn't proceed with it. I was also under the impression that I had to write any custom rules in Java, which I wasn't particularly interested in at the time.

Once I had the scanner working I decided to check out the custom rules again, as I was definitely going to need some to apply the BrightGen house rules. The documentation again suggests that Java is the way to go, so I went to the PMD site to learn more about it. Reading through the documentation I was delighted to stumble on the following:

XML rule definition

New rules must be declared in a ruleset before they’re referenced. This is the case for both XPath and Java rules. To do this, the rule element is used, but instead of mentioning the ref attribute, it mentions the class attribute, with the implementation class of your rule.

• For Java rules: this is the class extending AbstractRule (transitively)
• For XPath rules: this is net.sourceforge.pmd.lang.rule.XPathRule

My old friend XPath - the mechanism I've used many times in the past with Selenium automated testing. As long ago as March 2019 I was telling people to learn about this, as evidenced by Don Robin's tweet from my London's Calling session:

It now felt like I had a fighting chance of coming up with a custom rule in fairly short order, although obviously in PMD I'm not working with HTML. Instead the source code is parsed into an Abstract Syntax Tree, which I can navigate in a similar fashion.

One of the major helpers for Selenium is being able to run XPath expressions in the Chrome inspector and gradually build up a complex rule from simple steps. Reading more on the PMD documentation I was delighted to find the designer - a GUI where I could write some Apex code, execute an XPath expression against that code and see the matches live. It also has an excellent interactive reference where I can click on one of the tree elements and see its properties.  Well worth spending some time with:

Sample Rule

My sample rule was to check that SOQL statements are only present inside test methods or accessor classes, where accessor classes have the naming convention <item>Accessor.

The rule is somewhat lengthy - here it is in it's entirety before I break it down into sections:

//UserClass[not(ends-with(@Image, 'Accessor'))]/Method/ModifierNode[@Test=false()]/..//(SoqlExpression | MethodCallExpression[lower-case(@FullMethodName)='database.query'])

The first part checks that the class name doesn't end with Accessor - if it is, I am in an accessor class and we can stop now:

//UserClass[not(ends-with(@Image, 'Accessor'))]

if I'm not in an accessor, I look for any method that isn't a test method:

/Method/ModifierNode[@Test=false()]

The @Test property is part of the magic of PMD - the method modifier tree element knows whether a method is a test or not, so I don't have to worry about annotations or keywords.

I then back up from the ModifierNode to the method and consider all the child nodes of all non-test methods:

/..//

Then comes the meat of the rule - I look for the union of nodes that are SOQL expressions or have a database.query method call - note that I convert the FullMethodName property to lower case so that I don't have to care about capitalisation:

(SoqlExpression | MethodCallExpression[lower-case(@FullMethodName)='database.query'])

Executing this rule against an Apex class snippet:

class MyClass
{
   void SoqlQuery()
   {
       List<Contact> contacts=[select id from Contact];
   }

   void SoqlQuery2()
   {
        List<Contact> contacts=database.query('select id from Contact');
   }
}

matched as expected against the SOQL query on line 5 and the Database.query method call on line 10:

That's as far as I'm going for part 1 - in part 2 I'll look at how to create a ruleset for this rule and install it into the CLI Scanner. You won't have to wait too long as I'm hoping to write it in the next day or so while it's fresh in my mind.

Resources

• PMD Site
• PMD Apex Rules
• Salesforce Developers Blog post introducing the scanner
• CLI Scanner Documentation

Follow @bob_buzzard

Working remotely (and locally)

Tweet

Work has changed

Work is no longer somewhere you go - it’s something you do

It's something you do from pretty much anywhere, if you can and your team/employer can manage the timezones. The COVID-19 outbreak has compressed 5-10 years worth of gradual working practice changes into a couple of quarters. It’s been 6 months for us in the UK and bar, a short government push to get people back to the office, there’s been very little sign of a return to how things were. The last round of advice suggested we are looking at another 6 months, which feels like a number chosen because it’s the biggest we will quietly accept. 

It probably won't change back

If this is the shape of things to come, rather than a one off pandemic, it would be a brave company that maintains a huge office portfolio for on-again off-again occupation. It’s like the world’s worst Hokey Cokey at the moment – right team in, right team out, left team in, left team out, but with no idea of when you’ll be able to put your whole team back in for any length of time.

Working remotely affects other workers

The lack of commuters to cities is also having an impact on the service industry – coffee, sandwiches, food trucks, restaurants and bars. I used to work out of our London office one day a week and typically spent £10–20 each time on sundries (and maybe a pint after work). I know many who spent this amount every day! I haven’t been for 6 months now and that is not looking likely to change any time soon. 

That money won’t entirely go away as people continue to work from home, although it seems like people are still being cautious with spending, but a fair bit will likely end up being spent locally. This isn’t a bad thing – instead of someone making a 70 minute commute to serve me a coffee after my 70 minute commute, I’ll walk to my local café and be served by someone who also walked there. (Or I would if there were any cafés that I could walk to in under an hour without taking my life in my hands, but you get the idea). Of course there’s some irony that in the great enabler of remote working, the coffee shop, the last thing they now want is people working there all day. 

Many of the local outlets are suffering too, but for those that can survive, the changes to our habits forced on us by the pandemic might just give them a brighter future. And the cities will have to focus on the people that live there, rather than those that are transported in and out each day. 

Follow @bob_buzzard