Skip to content

Fix gosec security issues#942

Open
jfantinhardesty wants to merge 3 commits into
mainfrom
bugfix/fix-gosec-issues
Open

Fix gosec security issues#942
jfantinhardesty wants to merge 3 commits into
mainfrom
bugfix/fix-gosec-issues

Conversation

@jfantinhardesty

Copy link
Copy Markdown
Contributor

What type of Pull Request is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Describe your changes in brief

Add gosec as a linter which finds go security issues and fixed identified issues.

Checklist

  • Tested locally
  • Added new dependencies
  • Updated documentation
  • Added tests

Related Issues

  • Related Issue #
  • Closes #

@foodprocessor foodprocessor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should check for overflow in cases where it will never happen. To fix the vulnerability, we can and should normalize incoming values to make overflow impossible. I'm grateful to gosec for highlighting the issue, but I don't think the gosec integer overflow checker is mature enough to be included in our CI.
So, I think we should add G115 to the excludes list, and this PR should pivot to addressing the problem in the best way without affecting readability (normalize incoming values and change variable types to remove the need for conversions, where possible).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants