Code Quality & Maintenance Key Elements

Practices and internal processes set up to maintain high-quality code, ensuring the stability of the product through regular updates and bug fixes.

Risk: High (unintentional vulnerabilities or possible injection of malicious code)

This check determines whether the project requires human code review before pull requests (merge requests) are merged.

Reviews detect various unintentional problems, including vulnerabilities that can be fixed immediately before they are merged, which improves the quality of the code. Reviews may also detect or deter an attacker trying to insert malicious code (either as a malicious contributor or as an attacker who has subverted a contributor's account) because a reviewer might detect the subversion.

The check determines whether the most recent changes (over the last ~30 commits) have approval on GitHub or if the merger is different from the committer (implicit review). It also performs a similar check for reviews using Prow (labels "lgtm" or "approved") and Gerrit ("Reviewed-on" and "Reviewed-by"). If recent changes are solely bot activity (e.g. Dependabot, Renovate bot, or custom bots), the check returns inconclusively.

Scoring is leveled instead of proportional to make the check more predictable. If any bot-originated changes are unreviewed, 3 points are deducted. If any human changes are unreviewed, 7 points are deducted if a single change is unreviewed, and another 3 are deducted if multiple changes are unreviewed.

Review by bots, including bots powered by artificial intelligence/machine learning (AI/ML), do not count as code review. Such reviews do not provide confidence that there will be a second person who understands the code change (e.g. if the originator suddenly becomes unavailable). However, analysis by bots may be able to meet (at least in part) the SAST criterion.

Note: Requiring reviews for all changes is infeasible for some projects, such as those with only one active participant. Even a project with multiple active contributors may not have enough active participation to be able to require a review of all proposed changes. Projects with a small number of active participants sometimes aim for a review of a percentage of proposals (e.g., "at least half of all proposed changes are reviewed").

Requiring review does not eliminate all risks. The other reviewers might fail to notice unintentional vulnerabilities or malicious code, be colluding with a malicious developer, or even be the same person (using a "sock puppet" account).

Remediation steps

  • If the project has only one contributor or does not have enough reviewers to practically require that all contributions be reviewed, try to recruit more maintainers to the project who will be willing to review others' work. Ideally, at least some of these people will be from different organizations (see Contributors). If the project has very limited utility, consider expanding its intended utility so more people will be interested in improving it, and make that larger scope clear to potential contributors.

  • Follow security best practices by performing strict code reviews for every new pull request/merge request.

  • Make "code reviews" mandatory in your repository configuration. (Instructions for GitHub.)

  • Enforce the rule for administrators/code owners as well. (Instructions for GitHub.)

Risk: High (possibly unpatched vulnerabilities)

This check determines whether the project is actively maintained. If the project is archived, it receives the lowest score. If there is at least one commitment per week during the previous 90 days, the project receives the highest score. If there is activity on issues from users who are collaborators, members, or owners of the project, the project receives a partial score.

A project that is not active might not be patched, have its dependencies patched, or be actively tested and used. However, a lack of active maintenance is not necessarily always a problem. Some software, especially smaller utility functions, does not normally need to be maintained. For example, a library that determines if an integer is even would not normally need maintenance unless an underlying implementation language definition changed. A lack of active maintenance should signal that potential users should investigate further to judge the situation.

This check will only succeed if a Github project is >90 days old. Projects that are younger than this are too new to assess whether they are maintained or not, and users should inspect the contents of those projects to ensure they are as expected.

Remediation steps

  • There is no remediation work needed from projects with a low score; this check simply provides insight into the project activity and maintenance commitment. External users should determine whether the software is the type that would not normally need active maintenance.

Risk: High (vulnerable to malicious code additions)

This check determines whether the project's automated workflows tokens follow the principle of least privilege. This is important because attackers may use a compromised token with write access to, for example, push malicious code into the project.

It is currently limited to repositories hosted on GitHub, and does not support other source hosting repositories (i.e., Forges).

The highest score is awarded when the permissions definitions in each workflow's yaml file are set as read-only at the top level and the required write permissions are declared at the run-level. One point is reduced from the score if all jobs have their permissions defined but the top level permissions are not defined. This configuration is secure, but there is a chance that when a new job is added to the workflow, its job permissions could be left undefined because of human error.

Though a project's score won't be penalized, the check's details will include warnings for more sensitive run-level permissions, listed below:

  • actions - This may allow an attacker to steal GitHub secrets by approving to run an action that needs approval.

  • checks - This may allow an attacker to remove pre-submit checks and introduce a bug.

  • contents - Allows an attacker to commit unreviewed code. However, points are not reduced if the job utilizes a recognized packaging action or command.

  • deployments - This may allow an attacker to charge the repo owner by triggering VM runs, and tiny chance an attacker can trigger a remote service with code they own if the server accepts code/location variables unsanitized.

  • packages - Allows an attacker to publish packages. However, points are not reduced if the job utilizes a recognized packaging action or command.

  • security-events - May allow an attacker to read vulnerability reports before a patch is available. However, points are not reduced if the job utilizes a recognized action for uploading SARIF results.

  • statuses - This may allow an attacker to change the result of pre-submit checks and get a PR merged.

This compromise makes it clear the maintainer has done what's possible to use those permissions safely but allows users to identify that the permissions are used.

The check cannot detect if the "read-only" GitHub permission setting is enabled, as there is no API available.

Remediation steps

  • Set top-level permissions as read-all or contents: read as described in GitHub's documentation.

  • Set any required write permissions at the job level. Only set the permissions required for that job; do not set permissions: write-all at the job level.

  • To help determine the permissions needed for your workflows, you may use StepSecurity's online tool by ticking the "Restrict permissions for GITHUB_TOKEN". You may also tick the "Pin actions to a full-length commit SHA" to fix issues found by the Pinned-dependencies check.

Risk: High (known vulnerabilities)

This check determines whether the project has open, unfixed vulnerabilities in its own codebase or its dependencies using the OSV (Open Source Vulnerabilities) service. An open vulnerability is readily exploited by attackers and should be fixed as soon as possible.

Remediation steps

  • Fix the vulnerabilities in your own code base. The details of each vulnerability can be found on https://osv.dev.

  • If the vulnerability is in a dependency, update the dependency to a non-vulnerable version. If no update is available, consider whether to remove the dependency.

  • If you believe the vulnerability does not affect your project, the vulnerability can be ignored. To ignore, create an osv-scanner.toml file next to the dependency manifest (e.g. package-lock.json) and specify the ID to ignore and reason. Details on the structure of osv-scanner.toml can be found on the OSV-Scanner repository.

Last updated