Update security scanning

Labels: lifecycle/rotten

Timeline

Steve Moyer (smoyer64) opened (edited)

As discussed in PR #883, scanning the project for security vulnerabilities when a PR is "checked" breaks the zen flow of PR approval process.

The vulnerability found by govulncheck (via the Github workflow via make secure) is in fact real (for the small percentage of git-bug users who are serving the webui to the public). Additionally, this error was NOT found by the Github security tools.

Additionally, the vulnerabilities found by govulncheck can come to be outside the project's build process - when a vulnerability is discovered, there might be an arbitrary amount of time until the check is run against a PR. For these reasons, this issue proposes that the govulncheck be run as a timed action and that it be integrated into Github security disclosures. (So the `make secure-vulnerabilities' recipe would be excluded.)

The security problems reported by GoKart (as make secure-practices) on the other hand, are introduced by developers and therefore should remain part of the PR's "checks" and should break the build if the developer introduces a security issue. The problems reported by GoKart generally involve processing data that hasn't been properly cleaned as described by #3 and #8 in the OWASP Top 10 Web Application Security Risks.

To summarize the above, the following two tasks should be completed to resolve this issue:

  • Run govulncheck (or make secure-vulnerabilities) via a periodically triggered Github workflow and report via the Github Security process (preferred) or by creating a stand-alone issue to resolve the vulnerability.
  • Run GoKart (or make secure-practices) as a check that occurs when a PR is created or when a commit is added to an existing PR.
  • Continue to provide make secure as a build recipe so that developers can check the project thoroughly before committing code.

Since govulncheck is the official tool for verifying that known Go vulnerabilities are not included in code, it's likely that Github will ultimately include this functionality as part of either the security scanner or dependabot. If this occurs, we can eliminate the automated workflow for the scanner.

Michael Muré (MichaelMure) commented

Make sense to me. My only concern is if those tools are mature enough to give a good signal (which I don't know).

Steve Moyer (smoyer64) commented

I've been using GoKart for years. The govulncheck tool is new so I don't know either but I'll assume you meant signal-to-noise above. So far, the only vulnerability it reported was real (but that's not a statistically valid n).

Michael Muré (MichaelMure) commented

Let's give it a try then.

github-actions (github-actions) commented

This bot triages untriaged issues and PRs according to the following rules:

  • After 90 days of inactivity, the lifecycle/stale label is applied
  • After 30 days of inactivity since lifecycle/stale was applied, the issue is closed

To remove the stale status, you can:

  • Remove the lifecycle/stale label
  • Comment on this issue

github-actions (github-actions) added label lifecycle/stale

github-actions (github-actions) commented

This bot triages issues in order to help the maintainers identify what needs attention, according to the following lifecycle rules:

  • After 90 days of inactivity, lifecycle/stale is applied
  • After 90 days of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

This bot will not automatically close stale issues.

To remove the stale status, you can:

  • Remove the stale label from this issue
  • Comment on this issue
  • Close this issue
  • Offer to help out with triaging

To avoid automatic lifecycle management of this issue, add lifecycle/frozen.

github-actions (github-actions) added label lifecycle/rotten

github-actions (github-actions) removed label lifecycle/stale

sudoforge removed label lifecycle/dormant