Pull Requests are an Essential Part of Security
Separation of Duties
Before I became a software engineer, I studied accounting and even worked as an auditor for a brief period. Not much accounting knowledge remains, but a few nuggets stayed with me, particularly the internal control known as “Separation of Duties” (not to be confused with the design principle “Separation of Concerns”). To summarize, no one person should have complete control over an entire transaction. For example, the person collecting cash from a customer for a bill should not be the one to also record the transaction and deposit the cash. Otherwise, that person could collect funds, credit the customer’s account, and take a little off the top before making the bank deposit. Having another person in this process does not eliminate the risk, but colluding with someone else is harder to hide. You know what they say about the only way for two people keep a secret.
What does this have to with code and pull requests? Pull requests serve a similar security purpose. Not only do pull requests provide the nice to haves of allowing for asynchronous collaboration, a second set of eyes, etc, they are also a very important auditing artifact and should be considered an essential component of your delivery process.
Code controls business logic, often at a pace magnitudes faster than what any human can keep up with. And because of the complex nature of writing code, it poses a huge security vulnerability where malicious behavior can easily be obfuscated. For example a fork bomb looks like an inconspicuous line of code, but can actually crash your entire system. Because of which, every line of code that makes its way into any system of importance should be reviewed by someone else. Pull requests not only provide the mechanism to do this review, but also create the audit trail for every change.
Components of a Pull Request
Let’s examine the components of a pull requests and how they contribute to security.
First and foremost a pull request aggregates proposed changes via its commits. By using git blame
you are able to find the latest commit attributed to every line in the codebase. With this you are able to follow the history of any line change. If you ever wondered why a change was made, following the commit history is a great start. With GitHub, you can even see which pull request was the one that brought that commit into the current branch.
Not only can you see the history of changes, but the commits attribute the changes to the person that introduced it. Commit history great, but also being able to talk to the person that made the change can help provide more context that’s not in the code.
So far these are just things you get from a commit history alone. Pull requests show their value in the additional layer of tracking they provide. With a pull request, we can see the reviewer that approved the merge. Now you have at least two people involved in the process of getting code into production (provided that you have configured your version control to not allow self-approvals).
The pull request itself also provides an area to have a discussion - on the features, the code decisions, tech debt introduction. By doing this in the pull request, this discussion is linked very closely to the code itself. You could have the discussion happen in some other document, but several months down the line that document may get lost to some obscure folder in your company’s google drive.
For extra credit, you could also tag pull requests with the work item associated to the pull request, so you know why a change was made.
Security Benefits and Beyond
With all of these ingredients together, you can trace from a single line change who made the change, approved the change, approved the item to be worked on at all, and any context capture in any comments for the PR! And with all theses duties separated by person, you’ve mitigated a lot of the risk of any one person trying to act maliciously within an organization.
As an auditor, this kind of paper trail is gold. In fact, I’ve been involved in security audits that went super smoothly because this process was in place. In addition to security benefits, it was so useful to be able to see the context of something far into the future. Have you ever looked at code that YOU wrote and wondered why you took some odd approach? With this trail you can find the context of the original change, hopefully with an explanation of why.
Constructive Review vs. Fodder for Punishment
I want to emphasize that this internal control is not about placing blame. Accountability is important, but if this process is wielded as a threat, your team will be reluctant to to adhere to it. Should something happen, a bug, security breach, or even just a missed feature, this process should be used to trace the source of the problem. Afterwards a constructive, blameless discussion can happen where processes can improved. What happened in the past happened - the only thing to do is move on and learn from it. If leadership uses this process to punish contributors for their mistakes, the engineers will find ways to not be associated to the changes. Like any tool, its value to the organization highly depends on the culture those using it.
There are not many things that I consider essential for an engineering org, but pull requests are something that is a must have for me, and possibly a deal breaker for me if an organization refused to use them without a much better alternative. Even when I was the only working on a project professionally, I STILL used pull requests so that I could have the benefits of the audit trail, both for my future self or anyone else who would have to inherit my work after I left.
Good morning, and in case I don't see ya: Good afternoon, good evening, and good night!
Throughout my career I have joined and left several organizations, all for different reasons, and I have always felt confident in my choice to move on. For the first time, however, I am as equally excited in my next role as I am saddened to leave my previous one. Later this month, I will be joining the team at Twitch, leaving my current role at Salesforce.
I cannot express enough how lucky I was to be able to work with the people that I did at Salesforce. These were some of the most capable individuals I have ever worked with, and I found myself continually impressed by everyone. The way I was able to thrive, grow, and learn was phenomenal. Not just in the technical skills that I have gained, but also the numerous lessons I have learned in what we typically call “soft” skills. Lessons on how to lead and be a part of phenomenal teams, how to work productively and, more importantly, sustainably. What it means to work together with compassion and empathy. And what it truly means to work in a way that benefits not just the company, but myself.
There are too many people to thank in the organization. I am a different person, changed for the better, and for good, because of everyone I had the pleasure to interact with, and I hope our friendships will continue.
In particular I wanted to thank a few individuals in leadership that were instrumental in my growth. To Joysorlyn Dixon, Nina Patel, and Alex Peralta, the leaders that I was fortunate enough to work with closely on a daily basis, thank you for your mentorship, your guidance, and your friendship. I have never felt so valued and supported - even when I decided to move on to a new role you still put my wellbeing and my growth first. Whether I cried ugly tears on camera, vented my frustration at a tricky code base or situation, or just needed some advice on how to lead, your support was key to my success and ability to keep moving forward.
The attitude of servant leadership is something that is rare and a treasure - to not only help guide the success of the product, but also prioritize the growth of the individual is so admirable. I hope I can take those lessons with me and pass them on.
I am so lucky to have had this opportunity for the last few years to work with such great people. To my leadership, to my teammates on my scrum teams, and to those I was not fortunate enough to collaborate with more extensively - the words “thank you” are not enough to express my gratitude for all of your patience, encouragement, and support.
And so onwards to new adventures! I am so excited and grateful for this next opportunity and only hope to be even a fraction as impactful to those I work with in the future as everyone I worked with here at Salesforce has been for me.
Toxic Positivity - It's OK to Not Be OK
A friend recently introduced me to the idea of “toxic positivity”, which can be summarized as the “everything is awesome!” approach to life. I can’t speak to other industries, but I see this attitude all too often in tech. While I believe we should encourage some level of positive thinking, when we don’t allow ourselves the space for our pain, for our grief, we tip the balance into toxicity.
It’s not me, it’s you
If you work in tech, you have probably run into one of the following mantras at some point:
- Celebrate your failures
- Don’t come to me with problems, come to me solutions
- Happiness is a choice
When something bad happens, it’s actually good! When you have a problem, it’s not worth talking to me about it unless you know how to fix it. You can be happy too - just be happy.
There are plenty more, but it all boils down to the idea that if you are not satisfied with a situation, you have the power to fix it. In the book “America the Anxious” by Ruth Whippman, she summarizes the problem with this mentality beautifully:
But it doesn’t take long to realize that the flip side of this logic is that, if I am not happy, then it is All My Own Fault
Let’s be honest here - failure sucks and I do not want to dwell on it, much less celebrate it. Sometimes I have a problem, I have no idea what to do, and I cannot face it alone. Sometimes I am sad, angry, disgusted, scared, and happiness won’t help me right now, because I need to process this feeling.
In my career, especially in the last year, I cannot stress enough how lucky I am to have worked in environments (some more equipped than others) where I have been given the space to have these feelings, to step away, to reset. While this is a practice I want to encourage, it still implies that the underlying problem is within the individual. People may be given the time to get away, but what happens when the problem is rooted in the way we work?
Everything is awesome!
I think retros are the most important thing you can do as team. Taking the time to reflect on the work you just completed, to celebrate the successes, to re-examine the challenges, is necessary for the continual improvement of the team. Too often, however, I’ve seen retro devolve into just being a “good vibes sesh”, a full hour of teammates just giving each other kudos. Recognizing and celebrating each other is important part of bonding the team, but it can’t be the only point of the discussion.
If retro doesn’t allow people to recognize the things that are not working, when will the team ever get the chance to fix it? Even worse, people may be actively discouraged from bringing up anything that’s going wrong because no one wants to be the person that “kills the vibes.” I have attended retros for sprints that completely burned me out, just seething in silence as the team celebrated all the “success.” I don’t care if the release went out on time, should we really be patting ourselves on the back for the all nighter we just pulled to make it so? Shouldn’t we instead be examining the circumstances that led to this situation in the first place? But I hesitated to speak up because I didn’t want to rain on the parade.
Spinning things as a positive does not magically make things positive. Instead, it isolates each individual who is struggling with the status quo, who actually may not be alone in their thinking! It gaslights them into thinking that maybe the process is fine and that they are problem. After all, no one has said anything, so it must be fine, right? And so what happens? They leave.
Bringing an attitude of balance
I am by no means encouraging complain-a-thons. The key here is balance. Here are a few ways you could help bring that balance to your team:
- During retros, time-box each part so that there is space to discuss both the successes and the challenges
- Be the person that brings up challenges. Try to do so calmly, without attacking, and using neutral words to keep things constructive. You are probably not the only person that is feeling that way.
- If you’re a leader, recognize when things are going great and when things are not going well. And provide the space for the team to try things to remedy the problem. Yes, it might slow things down, but I can promise you that things will come to a screeching halt when your team decides to leave instead.
We have to recognize that in order to improve, we have to come to terms that the things we feel can’t just be ignored, that the things that aren’t going well need to be addressed. Recognizing that things can be improved doesn’t mean that everything is currently broken. But how will things get better if we don’t see them for how they truly are?