Fixed dependabot alerts are not reflected in action-cve checks

https://github.com/Andrew5194/graphql-testing

Seems like fixed dependabot alerts are not reflected in the slack notifications?

For example, I have the following dependabot alerts fixed:

But my slack notifications still seems to show the vulnerabilities.

Looking into this further, it would appear as though the graphQL query at https://github.com/kunalnagarco/action-cve/blob/main/src/fetch-alerts.ts#L15-L60 fetches the alerts indiscriminantly, for example:

    query {
      repository(owner:"${repositoryOwner}" name:"${repositoryName}") {
        vulnerabilityAlerts(last: ${count}) {
          edges {
            node {
              id
              repository {
                name
                owner {
                  login
                }
              }
              securityAdvisory {
                id
                description
                cvss {
                  score
                  vectorString
                }
                permalink
                severity
                summary
              }
              securityVulnerability {
                firstPatchedVersion {
                  identifier
                }
                package {
                  ecosystem
                  name
                }
                vulnerableVersionRange
                advisory {
                  cvss {
                    score
                    vectorString
                  }
                  summary
                }
              }
            }
          }
        }
      }
    }
  `)

Which returns something like the following for my repo owner Andrew5194 and repo name graphql-testing (with the already fixed dependabot alerts).

{
  "data": {
    "repository": {
      "vulnerabilityAlerts": {
        "edges": [
          {
            "node": {
              "id": "RVA_kwDOHjlOLs6cWPR8",
              "repository": {
                "name": "graphql-testing",
                "owner": {
                  "login": "Andrew5194"
                }
              },
              "securityAdvisory": {
                "id": "GSA_kwCzR0hTQS03NGZqLTJqMmgtYzQycc0hVA",
                "description": "follow-redirects is vulnerable to Exposure of Private Personal Information to an Unauthorized Actor",
                "cvss": {
                  "score": 8,
                  "vectorString": "CVSS:3.0/AV:N/AC:L/PR:L/UI:R/S:U/C:H/I:H/A:H"
                },
                "permalink": "https://github.com/advisories/GHSA-74fj-2j2h-c42q",
                "severity": "HIGH",
                "summary": "Exposure of sensitive information in follow-redirects"
              },
              "securityVulnerability": {
                "firstPatchedVersion": {
                  "identifier": "1.14.7"
                },
                "package": {
                  "ecosystem": "NPM",
                  "name": "follow-redirects"
                },
                "vulnerableVersionRange": "< 1.14.7",
                "advisory": {
                  "cvss": {
                    "score": 8,
                    "vectorString": "CVSS:3.0/AV:N/AC:L/PR:L/UI:R/S:U/C:H/I:H/A:H"
                  },
                  "summary": "Exposure of sensitive information in follow-redirects"
                }
              }
            }
          },
          {
            "node": {
              "id": "RVA_kwDOHjlOLs6cWPSF",
              "repository": {
                "name": "graphql-testing",
                "owner": {
                  "login": "Andrew5194"
                }
              },
              "securityAdvisory": {
                "id": "GSA_kwCzR0hTQS1wdzJyLXZxNnYtaHI4Y80qJw",
                "description": "Exposure of Sensitive Information to an Unauthorized Actor in NPM follow-redirects prior to 1.14.8.",
                "cvss": {
                  "score": 5.9,
                  "vectorString": "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
                },
                "permalink": "https://github.com/advisories/GHSA-pw2r-vq6v-hr8c",
                "severity": "MODERATE",
                "summary": "Exposure of Sensitive Information to an Unauthorized Actor in follow-redirects"
              },
              "securityVulnerability": {
                "firstPatchedVersion": {
                  "identifier": "1.14.8"
                },
                "package": {
                  "ecosystem": "NPM",
                  "name": "follow-redirects"
                },
                "vulnerableVersionRange": "< 1.14.8",
                "advisory": {
                  "cvss": {
                    "score": 5.9,
                    "vectorString": "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
                  },
                  "summary": "Exposure of Sensitive Information to an Unauthorized Actor in follow-redirects"
                }
              }
            }
          },
          {
            "node": {
              "id": "RVA_kwDOHjlOLs6cWPSO",
              "repository": {
                "name": "graphql-testing",
                "owner": {
                  "login": "Andrew5194"
                }
              },
              "securityAdvisory": {
                "id": "GSA_kwCzR0hTQS14dmNoLTVndjQtOTg0aM0z6A",
                "description": "Minimist <=1.2.5 is vulnerable to Prototype Pollution via file index.js, function setKey() (lines 69-95).",
                "cvss": {
                  "score": 9.8,
                  "vectorString": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"
                },
                "permalink": "https://github.com/advisories/GHSA-xvch-5gv4-984h",
                "severity": "CRITICAL",
                "summary": "Prototype Pollution in minimist"
              },
              "securityVulnerability": {
                "firstPatchedVersion": {
                  "identifier": "1.2.6"
                },
                "package": {
                  "ecosystem": "NPM",
                  "name": "minimist"
                },
                "vulnerableVersionRange": "< 1.2.6",
                "advisory": {
                  "cvss": {
                    "score": 9.8,
                    "vectorString": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"
                  },
                  "summary": "Prototype Pollution in minimist"
                }
              }
            }
          }
        ]
      }
    }
  }
}

I would expect that the slack notifications would tell me that I no longer have any vulnerabilities. Perhaps I'm doing something wrong? But if not, it would probably be best to implement some client side filtering for only those vulnerabilities that are considered "open" or "not (currently) fixed": https://github.com/kunalnagarco/action-cve/blob/main/src/fetch-alerts.ts#L61-L71

hey @Andrew5194 - thanks for the issue! I'll spend some time investigating this and get back to you.

Ideally, GitHub shouldn't send any alerts about vulnerabilities that have been marked as fixed 🤷

But if they are, then like you said, I'll definitely have to do some filtering in this action. Thanks again for bringing this up 🙂

@Andrew5194 - firstly, thanks for sharing the graphql-testing repo - when i run the GraphQL query for fetching alerts on that repo, i get the following result:

Maybe, GitHub takes some time to update their results after the alerts have been dismissed? Could you try running the action now and see if you are still getting the alerts in Slack? Thanks!

@kunalnagar I believe that you need to be the owner of the repo that you're looking to query in order to access the data. For example, I got the same output above when I tried to execute a GraphQL query for any repo that I didn't personally own (either someone else's repo or a repo that I forked).

But to be clear, when I add a fixedAt field according to https://docs.github.com/en/graphql/reference/objects#repositoryvulnerabilityalert, it shows that the query output returns the same dependabot vulnerabilities that have already been fixed. Thus, unless you're doing some kind of client-side filtering, these same vulnerabilities will probably be sent via the slack notification, which I seem to be seeing on my side (i.e. I received 3 alerts via slack for my dependabot vulnerabilities, I fixed them, but when I run the action again, I still get 3 alerts via slack).

In any case, I think it would be best to have your action-cve workflow only send notifications for "active" or "not fixed" dependabot vulnerabilities, otherwise if it's sending notifications for the same vulnerabilities regardless of whether or not they've been fixed, there isn't much value in using the action. Though of course, perhaps this is user error and I'm mistaken!

In terms of implementation suggestions, probably something along the lines of "if fixedAt is null for each node, then add the specified node to the alert list?" https://github.com/kunalnagarco/action-cve/blob/main/src/fetch-alerts.ts#L61-L71

query {
      repository(owner:"Andrew5194" name:"graphql-testing") {
        vulnerabilityAlerts(last: 10) {
          edges {
            node {
              fixedAt
              id
              repository {
                name
                owner {
                  login
                }
              }
              securityAdvisory {
                id
                description
                cvss {
                  score
                  vectorString
                }
                permalink
                severity
                summary
              }
              securityVulnerability {
                firstPatchedVersion {
                  identifier
                }
                package {
                  ecosystem
                  name
                }
                vulnerableVersionRange
                advisory {
                  cvss {
                    score
                    vectorString
                  }
                  summary
                }
              }
            }
          }
        }
      }
    }

{
  "data": {
    "repository": {
      "vulnerabilityAlerts": {
        "edges": [
          {
            "node": {
              "fixedAt": "2022-06-24T18:44:59Z",
              "id": "RVA_kwDOHjlOLs6cWPR8",
              "repository": {
                "name": "graphql-testing",
                "owner": {
                  "login": "Andrew5194"
                }
              },
              "securityAdvisory": {
                "id": "GSA_kwCzR0hTQS03NGZqLTJqMmgtYzQycc0hVA",
                "description": "follow-redirects is vulnerable to Exposure of Private Personal Information to an Unauthorized Actor",
                "cvss": {
                  "score": 8,
                  "vectorString": "CVSS:3.0/AV:N/AC:L/PR:L/UI:R/S:U/C:H/I:H/A:H"
                },
                "permalink": "https://github.com/advisories/GHSA-74fj-2j2h-c42q",
                "severity": "HIGH",
                "summary": "Exposure of sensitive information in follow-redirects"
              },
              "securityVulnerability": {
                "firstPatchedVersion": {
                  "identifier": "1.14.7"
                },
                "package": {
                  "ecosystem": "NPM",
                  "name": "follow-redirects"
                },
                "vulnerableVersionRange": "< 1.14.7",
                "advisory": {
                  "cvss": {
                    "score": 8,
                    "vectorString": "CVSS:3.0/AV:N/AC:L/PR:L/UI:R/S:U/C:H/I:H/A:H"
                  },
                  "summary": "Exposure of sensitive information in follow-redirects"
                }
              }
            }
          },
          {
            "node": {
              "fixedAt": "2022-06-24T18:44:59Z",
              "id": "RVA_kwDOHjlOLs6cWPSF",
              "repository": {
                "name": "graphql-testing",
                "owner": {
                  "login": "Andrew5194"
                }
              },
              "securityAdvisory": {
                "id": "GSA_kwCzR0hTQS1wdzJyLXZxNnYtaHI4Y80qJw",
                "description": "Exposure of Sensitive Information to an Unauthorized Actor in NPM follow-redirects prior to 1.14.8.",
                "cvss": {
                  "score": 5.9,
                  "vectorString": "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
                },
                "permalink": "https://github.com/advisories/GHSA-pw2r-vq6v-hr8c",
                "severity": "MODERATE",
                "summary": "Exposure of Sensitive Information to an Unauthorized Actor in follow-redirects"
              },
              "securityVulnerability": {
                "firstPatchedVersion": {
                  "identifier": "1.14.8"
                },
                "package": {
                  "ecosystem": "NPM",
                  "name": "follow-redirects"
                },
                "vulnerableVersionRange": "< 1.14.8",
                "advisory": {
                  "cvss": {
                    "score": 5.9,
                    "vectorString": "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
                  },
                  "summary": "Exposure of Sensitive Information to an Unauthorized Actor in follow-redirects"
                }
              }
            }
          },
          {
            "node": {
              "fixedAt": "2022-06-24T18:44:59Z",
              "id": "RVA_kwDOHjlOLs6cWPSO",
              "repository": {
                "name": "graphql-testing",
                "owner": {
                  "login": "Andrew5194"
                }
              },
              "securityAdvisory": {
                "id": "GSA_kwCzR0hTQS14dmNoLTVndjQtOTg0aM0z6A",
                "description": "Minimist <=1.2.5 is vulnerable to Prototype Pollution via file index.js, function setKey() (lines 69-95).",
                "cvss": {
                  "score": 9.8,
                  "vectorString": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"
                },
                "permalink": "https://github.com/advisories/GHSA-xvch-5gv4-984h",
                "severity": "CRITICAL",
                "summary": "Prototype Pollution in minimist"
              },
              "securityVulnerability": {
                "firstPatchedVersion": {
                  "identifier": "1.2.6"
                },
                "package": {
                  "ecosystem": "NPM",
                  "name": "minimist"
                },
                "vulnerableVersionRange": "< 1.2.6",
                "advisory": {
                  "cvss": {
                    "score": 9.8,
                    "vectorString": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"
                  },
                  "summary": "Prototype Pollution in minimist"
                }
              }
            }
          }
        ]
      }
    }
  }
}

@Andrew5194 - this is PERFECT.

i also found another field called dismissedAt that we can use to filter out the alerts that have been manually dismissed through the GUI.

so basically, the client side filtering will be done based on two keys:

fixedAt !== null
dismissedAt !== null

i'll try to get a fix out soon, thanks for your help in debugging this 🙂

Just to clarify, I think you may want to specify the following as your conditionals:

fixedAt == null OR
dismissedAt == null

as the records returned would then be those alerts that have NOT been fixed yet OR already manually dismissed (i.e. the alerts that I as a developer actually care about when being notified).

P.S. Thanks for being open to making the change!

@Andrew5194 - i think instead of OR, we should use AND because we only want to include an alert in the list if both fixedAt AND dismissedAt are null.

I'll have a PR up for this soon so you can take a look and provide a review 🙂

Edit - The PR is up for review if you wanna take a look! For some reason, I can't add you to the list of reviewers 🤷

This looks great @kunalnagar! I’ll test the new changes tomorrow and report back. 🙏

@kunalnagar reporting back as promised, this is now working as expected! I did an ABAB test just to make sure (i.e. I re-added the vulnerable dependencies, kicked off your action and got the notifications, then fixed the vulnerable dependencies, and kicked off your action and did not receive any notifications. Thank you so much! I appreciate the prompt response!!

@Andrew5194 - this is awesome! Thanks so much for bringing this up and helping debug. Really appreciate it 🙂

kunalnagarco / action-cve

Fixed dependabot alerts are not reflected in action-cve checks #92