DEV Community

Cover image for Code Security: Manual Code Reviews Ain't Enough
Dwayne McDaniel for GitGuardian

Posted on

Code Security: Manual Code Reviews Ain't Enough

Performing manual code reviews means a human is looking through proposed code changes and checking for issues before the code is merged and pushed toward production. While this approach is really a good idea overall, the reality is that in modern DevOps organizations, only doing reviews by hand is not going to give you the results you need. What is needed is scalable and performant tools that you can automate, and tools developers can use to catch issues before any pull requests are created, shifting testing as far to the left as feasible.

Manual code reviews are far better than doing no code reviews. No matter how good your coding skills are, bugs and security issues have an uncanny way of sometimes making it into the code.

Before we go on to why you should supplement manual code reviews with automation and developer tooling, let's quickly remind ourselves about the good of this approach.

Why you should do manual code reviews

Education opportunities through peer review

Having someone else on your team review your work, looking for bugs, logic errors, and possible security issues, is commonly referred to as a peer review. Peer reviews typically will rely on more senior members of the team who are very familiar with the code itself and the story of how it evolved. This can be an excellent opportunity to mentor or teach newer developers more efficient coding patterns or help stop previously discovered antipatterns from making their way into production. Manual code reviews provide effective teaching opportunities, helping everyone improve while making the code better.

Identifying business logic issues

Just because something technically functions does not mean it is the best way to accomplish the business goals of the organization. A great advantage of the manual code review process is being able to step back and think about the larger picture. It is very easy for anyone developing code to fall down rabbit holes and focus on a particular technical issue; having another set of eyes can help everyone focus on what is important: the customer.

Security is a team sport

While in a perfect world, every developer would be up to date on the latest security vulnerability reports and new guidelines, the reality is the landscape is constantly changing, and it is easy to overlook security best practices. Having another human being do a gut check about any new changes, especially when any third-party components are being added, can be very effective in reducing risk. Review steps that require sign-off from team leads help share the burden of needing to get security right by yourself all the time.

Why manual code reviews are not enough

Given the above section, it is pretty clear we are fans of manual code reviews, but we need to acknowledge that they do have some limitations and downsides.

Humans are error-prone

All manual code review processes have the common element of a human performing the review. As the old saying goes: "To err is human." Expecting perfection from every member of your team in all areas is unrealistic and leads to burnout and turnover.

Manual reviews are slow (and costly)

Reading code is unlike almost every other kind of reading. It requires a lot of concentration to maintain a mental model of what the code is doing while also looking for best practices and security issues. The larger the set of proposed changes, the slower the process becomes.

Developer time is certainly not free, but aside from the direct cost of the time for performing a code review, there are indirect opportunity costs. Every second spent reviewing code is time that same more seasoned developer is removed from creating new features. In the worst situations, the team can get caught in a loop of revisions which interrupt development flow and slow the entire software development lifecycle down.

The same defects often show up more than once

While there are always going to be one-off issues, it is more than likely that if an issue pops up in one place, it is going to show up repeatedly throughout the code. For example, if you mistype a variable name once, your IDE might use that spelling in the auto-complete suggestions throughout the rest of the project. Worse yet, in the case of typosquatting attacks, this can mean introducing a malicious dependency over and over again. Finding every single flaw by hand is very impractical.

Security issues have higher stakes

While manual code reviews are a great time to share better security practices with team members, the consequences of missing a security issue can be catastrophic. Relying solely on manual security checks in the review process inside an organization that is deploying several times a day is inviting trouble. With the constant change in the threat landscape, it is not a matter of if security issues will show up but a matter of when.

Manual code reviews rarely look at the history of the project, just the top-level changes. This can mean things you don't want out in public being exposed in your git log. For example, if a developer has added a hardcoded credential in one commit but then removed it from the following commit, the secret will still be in the shared history, visible to anyone who gains access to the code base. This can be easily overlooked while focusing on the latest changes and how they will affect the app in production.

Code review tools and automation

There are multiple drivers for an organization to adopt tools and automation, the biggest ones being consistency and scalability. Pursuing these goals is at the heart of 'shifting left.' Only doing reviews at the 'testing and review' stages of the software development lifecycle slows things means blocking flow.

A Kanban board with a backlog column filled up with blocked work due to failed tests

Adopting tools and automation means that the review process can shift much earlier in the code development process, even before the code is initially committed, by the developer making the changes. This early 'self-review' process can shave off hours or days of review time and give more consistent results.

But what kind of tools should an organization adopt? While there are a lot of tools on the market, here are the main categories we think you should consider as you are planning your path forward.

SAST

Static Analysis Security Testing, SAST, tools focus on source code scanning. While they can be run at any stage of the SDLC, the real advantage is they can be run locally to find issues before the code makes it into your source control history. These types of scanners are extremely fast since they don't require build steps or even testing environments to deploy. SAST tools ensure protection from security issues such as saving a password in clear text or sending data over an unencrypted connection.

We are very proud to partner with a SAST tool maker Snyk to strengthen developer security. Their tools offer unparalleled visibility into issues from code to cloud. They are helping organizations of any size keep their whole software supply chain safe, starting with their locally produced lines of code.

DAST

Dynamic Analysis Security Testing, DAST, tools test your code from the outside, assuming no knowledge of application's the inner workings. Sometimes referred to as 'black box' testing this approach tries to find flaws in the application much the same way an attacker would. In fact, one of the most widely used open-source DAST tools is called Zed Attack Proxy, ZED from OWASP.

DAST can catch things that SAST won't catch, but is important to realize they also accomplish different things. DAST does not provide code insight, just outcomes of how the application performs. DAST is also much slower and dependent on having a running application, meaning you have to complete the full build steps and have a working environment before you can perform this testing.

SCA

Software Composition Analysis Software, SCA, is a way to track and analyze components brought into a project. SCA is used to scan your dependencies for security vulnerabilities. Given that 70%-90% of modern code bases contain third-party software, mostly open-source components, it is critical to understand what is going into your application and any vulnerabilities those components bring. Unlike SAST, which looks at what your written code is doing, SCA focuses on which parts of the codebase are brought in from elsewhere.

SCA can be run as early as the first draft of your application. More importantly, it should be run during maintenance updates to ensure your application's components or dependencies are fully patched and secure. Again we are grateful to our partners at Snyk for making SCA easier to accomplish at scale.

Secrets Detection

If you are a fan of the GitGuardian blog, then this is a topic you will already be familiar with. As we see in the State of Secrets Sprawl report, the issue of hardcoded credentials leaking out to the world is a growing problem. The path to a more secure organization is improving your Secrets Management Maturity. Maturity here focuses on 2 main areas:

  • Consistent management of secrets through tools like Vault by HashiCorp or Doppler.
  • Actively monitoring for things like API keys, usernames/passwords, or certificates to catch them entering your code perimeter.

Vaults or secret management solutions are essential to deliver a consistent way to store and access your secrets, keep them encrypted throughout their lifecycle, and give you access controls, complete with logs. However, vault solutions alone are not enough in the fast-moving world of modern DevOps organizations. Developers often will test new keys or credentials to ensure the connection works while debugging something or implementing a new service locally, all with the full intent of handling those secrets correctly

Trying to actively monitor for new secrets entering the code base by hand is slow and very error-prone. This is exactly the right job for the GitGuardian Secret Detection platform. You can easily set up detection on your entire code perimeter to alert you instantly when a credential ends up in the code. Developers can even leverage the power of GitGuardian scanning using ggshield, our open source CLI, to automatically check for any plaintext secrets before any commit is made locally.

But what about legacy code or codebases you have acquired? Beyond just scanning the most recent changes, GitGuardian will perform historical scans throughout your codebase, uncovering any hardcoded secrets that were added in previous commits or in non-production branches. We have helped a lot of companies greatly improve their secrets management posture, and we are confident we can help you as well.

Manual + automated reviews for the win

Before we end, we want to reiterate that we believe manual code reviews provide a lot of value and educational opportunities that help improve both team members and the code base itself. However, they are slow, error-prone, and don't scale. Worse yet, missed security issues might lead to a catastrophic breach in the worst case. That's why you need to leverage the right tools and automated testing, applying technology that can relieve much of the toil of searching for individual errors throughout the code.  

When combined, you get the best possible outcomes. Use tools to find issues faster, at scale, and earlier in the process. Use automation to apply those tools repeatedly and reliably and free your team from the monotonous and repetitive work of searching for issues. Help your team focus on doing what humans are uniquely great at, providing actionable feedback and driving business value.

Top comments (0)