department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
99 stars 69 forks source link

S83 Dependabot Risks #13498

Closed productmike closed 1 year ago

productmike commented 1 year ago

Description

There are currently 11 security alerts identified by Dependabot. Four (4) are thought to be straightforward; the other 7 require more investigation time.

Acceptance Criteria

ndouglas commented 1 year ago

Dependabot Alerts Policy

Background

Dependabot creates alerts in response to security vulnerabilities detected within project dependencies. In the case of the CMS, these dependencies can be any of several different languages, installed through several package managers, span multiple subprojects and concerns within the greater CMS project, and be easily forgotten.

This policy's intent is to systematize how we handle these on an ongoing basis.

Recommendations

  1. We should consider establishing a second, private repository (e.g. va.gov-cms-sensitive) for tracking issues related to security. Any details about these packages and their vulnerabilities (e.g. name, version, risks, example code) should not be placed in the public repository.

    It's true that someone can simply clone our repo and run npm audit to gain a list of our current vulnerabilities, but it's not great to publish this information publicly. Dependabot alerts are themselves restricted to repository admins.

  2. Determine who will handle these issues. This might be an existing team (cms-devops-engineers, cms-qa-engineers), etc, or simply any engineer on the CMS team. This is toil, so it should be rotated as broadly as possible. This should not be the responsibility of a single person.

  3. Carefully evaluate and consider ignoring vulnerabilities in devDependencies. Dev dependencies are not used in our production environments, so we don't need to worry about them being remotely exploited at runtime. We control the build and test processes during which these dependencies are used, so we do not need to worry about them being exploited then.

    If a bad actor gains control over our build or test processes, they are not going to waste their time exploiting these vulnerabilities when they have far more tempting targets. They could just inject code into any file and have that executed whenever and wherever they wish.

    There is a definite risk of a compromised developer dependency injecting malicious code somewhere, so this should not be taken as a suggestion to ignore all alerts on developer dependencies, or to do so by default. But we should evaluate the vulnerability in terms of the risk it poses and be prepared to dismiss it if that risk is negligible.

  4. Triage and create issues for Dependabot alerts during DevSecOps Refinement meetings. We don't need a lot of boilerplate here, and these things are almost impossible to size. But we can at least say "Try to knock out the stack overflow vulnerability in padLeft" or whatever.

    Issues should be opened in the private repo mentioned above rather than in the CMS repo itself.

    If multiple issues are present within a single project, we face the possibility of merge conflicts and additional toil on top of the toil inherent to these issues; we should therefore try to solve multiple issues in a single project in one fell swoop where possible, allowing for the possibility that one or more of those issues may be thorny enough, after investigation, to warrant a followup ticket.

  5. Assign issues as part of sprint planning. As mentioned above, these should be rotated. We should acknowledge the possibility that any given issue might becoming incredibly complex and frustrating to manage, however simple it might seem at first.

ndouglas commented 1 year ago

Status of S83 Dependabot Risks (originally 11):

ndouglas commented 1 year ago

Screenshot 2023-05-01 at 3 34 06 PM

ndouglas commented 1 year ago

I'm reconsidering my above suggestion of adding a second repository to handle security issues. I think the better thing to do would be to maintain them completely in the open (given that literally anyone can see and run composer outdated or npm audit on our codebase). Especially given the nature of these vulnerabilities and how infrequently they are remotely exploitable, I think it'd better to minimize the overhead of managing these issues and the resuiting work.

That said, I would like to note in the policy that if something is remotely exploitable or otherwise problematic, we should handle discussions within a more secure context.

Evaluate alerts based on the following... does addressing this alert:

If not, we should not do the thing.

ndouglas commented 1 year ago

I opened a PR here for comments: https://github.com/department-of-veterans-affairs/va.gov-cms/pull/13622