Bitwarden strives to build a safe and secure password manager that everyone can use¹. In our previous posts we discussed this vision and how such a vision translates itself to a software architecture. This week we will discuss the quality and evolution the Bitwarden software has been through. As a password manager security is a key component of the Bitwarden product. For such a product clean code is not only a system to keep the project maintainable and readable for developers. Clean code ensures bugs have a tougher time forming and code smells that cause security issues can be identified and solved.

That being said, Bitwarden does not offer explicit coding guidelines. Although every repository includes a CONTRIBUTING.md these rarely go in-depth into exact software quality processes. The CONTRIBUTING.md of the Bitwarden/help repository ² has a larger style guide specifying hyperlink and abbreviation usage then the Bitwarden/server repository has on code styling. Even if there is a lack of explicit guidelines for software quality the code in the project is clearly written but that will be discussed further in the rest of this post.

In the following sections we will discuss the continuous integration, test processes, hotspot components and general code quality in the Bitwarden projects. Finally we will look at the technical debt that could be present in the system.

Key elements continuous integration

As the Bitwarden software system consists of multiple applications the continuous integration processes happen in multiple places. As described in the previous essay about the Bitwarden system architecture the system consists of five applications with which the user directly interacts and one backend application which makes sure everything is safely stored.

Before new code is added to one of the codebases of the application a contributor needs to make a pull request. Bitwarden uses a few integrations from GitHub in their process. After making the pull request these integrations check whenever the contributor has signed the contributor agreement and if the pull request has been reviewed by a trusted contributor.³ These trusted contributors often work at Bitwarden. Each application also has its own specific set of pipeline jobs that are done when a pull request is made. Because almost all applications with which the user directly interacts are multiply platform applications the pipelines for the continuous integration processes are complex. For example, the jobs for the build pipeline of the desktop application include a cloc, linux, windows and mac os job. The cloc job is used in multiple repositories. Cloc is used to determine ”blank lines, comment lines, and physical lines of source code” ⁴. These statistics are used to help with determining the condition of the quality of the source code.

If we look at the continuous integration of the backend server application. The order of jobs is: Cloc job, ubuntu and finally the windows job. In the ubuntu step, the application build script⁵ is executed and the docker images are made. These images are then pushed to the docker hub of Bitwarden. The latest images of all Bitwarden repositories can be found in their respective docker hubs ^{6 7}. After this step the windows step is done. The application is built and the tests are executed.

The few snippets of code below are from the build.yml of the server repository⁵.

- name: Push latest images
        if: github.event_name == 'release'
        run: ./build.sh push latest
        env:
          DOCKER_CONTENT_TRUST: 1
          DOCKER_CONTENT_TRUST_REPOSITORY_PASSPHRASE: ${{ secrets.DOCKER_CONTENT_TRUST_REPOSITORY_PASSPHRASE }}

Pushing the latest version of the server application to the docker hub.

- name: Test solution
        run: dotnet test .\test\Core.Test\Core.Test.csproj --configuration Debug --no-build
        shell: pwsh

Executing tests on the server application.

There is no automatic deploy script for the Bitwarden applications. This part is manually done by the people at Bitwarden or by the companies that self-host the product. The browser extension, desktop, mobile and cli applications of Bitwarden are downloaded and installed by the users.

Testing

In this section we will look at the rigor of test processes, and the role of test coverage. Bitwarden currently has 11 repositories on GitHub, of which we will look at the testing situation of the 7 most important ones. To get an idea of the importance of tests at Bitwarden, lets look at how many tests there are in each of these repo’s. In this list, we looked at how many tests are being run by the CI/CD pipeline, as well as the pipeline performs static-analysis.

• server: 162 tests (of which 11 are skipped)
• web: 0 tests
• browser: 0 tests
• mobile: 49 tests
• desktop: 0 tests
• CLI (command line interface): 0 tests
• directory-connector: 0 tests

It is notable that Bitwarden only has tests for two of its seven applications. Furthermore, the repo’s that do have tests do not report test coverage. Apparently, they do not think it is worth the effort to create tests. Part of this reason might be that most of their untested repositories are different front-end applications, which can often be harder to test than back-end applications, due to them being visually oriented. However, this does not mean that front-ends cannot be tested at all, so we were a bit surprised to find zero tests in so many repositories. Despite the lack of tests in some repositories, all repositories use static-analysis/linting in their pipeline.

In order to narrow the scope of this post, we will look in depth to the back-end of Bitwarden, the Bitwarden/server repository. This repository has a tests folder, in which tests are split into two sub folders: core tests and API tests. Only the tests in the core test folder are actually being run by the CI/CD pipeline. The pipeline is setup to fail once a test does not pass, however, test coverage is not measured or reported.

Hotspot components from the past and future

We have analysed the Bitwarden repositories using a tool called CodeScene ⁸. On the Bitwarden system level the server component is marked as the largest past hotspot by far with the frequency of changes being twice as high as the second place component. In second is the Mobile component and in third the Desktop, Browser and Web. A visualization of this hierarchy is given in the figure ‘Overview of Bitwarden hotspots’. This is expected as the server is the single central component that connects the Bitwarden system and the mobile is a large platform with only one front end thus all mobile users are dependent on this one interface. That the Desktop, Browser and Web components essentially share hotspot third place is logical as they all provide access to the same features through a different interface for the same platforms.

When zooming in to file level hotspots, we see that there are several files scattered over all repositories that are highlighted as hotspots in the ‘Overview of Bitwarden hotspots’ figure. The most notable ones are AccessibilityHelpers.cs from the mobile component, OrganizationService.cs from the server component, and main.background.ts from the browser component. These single files have over 30 commits each, are 600+ lines long, and are all flagged by CodeScene as having the poorest code health of all hotspot files, a warning level. This is significant as other hotspot files that are shorter files of less than 200 lines, display healthy, almost perfect code health scores. For example the startup.cs file in the server component or the nativeMessaging.service.ts file from the desktop component. An example comparison can be seen in the figure ‘Comparison of hotspot code health’

In the future server will still be the major hotspot due to it inherently being the center of Bitwarden. New front ends have recently been introduced like the CLI and we expect that they will catch up to older components over time. In their roadmap Bitwarden plans to expand its feature set over all front ends and continue presenting one unified product, so the hotspots will likely remain the same.

Code quality, focus on hotspot components

Instead of discussing the code quality for the multitude of projects under the Bitwarden corporation we want to focus our discussion on the code quality in the Bitwarden server repository. This repository is subject to the highest amount of changes and used by many other services in Bitwarden. We ran the open source code analysis tool SonarLint⁹ and generated a report on all files. To our amazement not a single C# file had an issue. Some minor issues were raised in regards to styling and variable naming. However some major issues were raised by SonarLint in regards to outdated function calls or missing meta data in .cshtml files.

Even though SonarLint lists these issues as major we do not see any reason for concern. Most of these major issues were in regards to readability of the web pages in the server repository. However the server repository is only accessed by users through another application, the browser extension or mobile app for instance. Therefore we do not expect these issues to be relevant to end users of the Bitwarden project.

Certain files were identified as hotspots by the CodeScene tool. These were files with a large number of changes, certain functions had more changes then others as well as having a high cyclomatic complexity. This is a recipe for bugs or code smells to sift through the cracks and into the main branch so we decided to investigate. Our investigations showed that the cyclomatic complexity was due to complicated reasoning trees when generating relevant feedback messages for users or developers. Please note the snippet has been shortened to improve readability.

public async Task<string> AdjustSeatsAsync(Guid organizationId, int seatAdjustment)
{
    var organization = await GetOrgById(organizationId);
    if (organization == null)
    {
        throw new NotFoundException();
    }

    if (string.IsNullOrWhiteSpace(organization.GatewayCustomerId))
    {
        throw new BadRequestException("No payment method found.");
    }

    if (string.IsNullOrWhiteSpace(organization.GatewaySubscriptionId))
    {
        throw new BadRequestException("No subscription found.");
    }
    ...
    return paymentIntentClientSecret;
}

When the code is structured neatly a high cyclomatic complexity is not a concern of us. Above all the high cyclomatic complexity can be justified in this specific case. We see that even though this file (and function specifically) has been subject to many changes the Bitwarden team manage to ensure the code is readable and clear.

Quality culture

The quality culture at Bitwarden can be derived from the many issues and pull requests from the several repositories of Bitwarden as well as the community forum. At these places, contributors and Bitwarden users continuously create new discussions on feature adjustments, bug fixes or new features. At multiple pull requests of different repositories, we noticed that the contributors of Bitwarden paid close attention to the unnecessary adjustments or spacing of the code, for example here^1011121314. So even though there are no explicit code format guidelines this shows that the contributors value clean code which has the same format.

Besides code styling the contributors with write access lay a focus on the completeness of the Bitwarden project, shown here[community-lock-vault]. When working on a feature in a system as large and loosely coupled as Bitwarden many changes affect multiple repositories. Users and developers are asked to keep track of these changes and link them together. But this is enforced by these contributors through pull requests and forum posts.

In previous posts we have discussed the shareholders of the Bitwarden project. The main one being the Bitwarden company. These are the main actors in guaranteeing the quality of the Bitwarden product. In the code evaluation as well as feature request message boards you see the Bitwarden company discuss changes and approaches offline, internally. This sometimes leads to whole new features¹⁰ or outside developers waiting on internal reactions¹⁵.

Technical debt

Bitwarden has its fair share of technical debt. For an application of this size, it is not unprecedented. The way that this is handled shows inspires confidence and does not raise concern with us. Only a small amount of issues is labeled as bug, however, this varies with different repositories. Most of these bugs are not system wide bugs, but platform specific bugs. Luckily there is a sizable community behind Bitwarden that is pro active in reporting and solving issues.

To prevent the accumulation of technical debt and code smells, Bitwarden has set-up static-analysis for all of its repo’s in the CI/CD pipeline through the use of linter software, such as TSlint ¹⁶. This filters out easy to spot code style mistakes, and other violated best practices. However, they could still improve their technical debt prevention by adding tests to the CI/CD pipeline, which is currently only done for 2 of their 7 application repositories as can be read in the section on testing. Additionally, Bitwarden could start using test coverage metrics for the repo’s that have tests, to get a measurement of how much code is actually being tested.

References