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(ormake 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(ormake 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 secureas 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.