Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/static code security scanner #74

Merged
merged 14 commits into from
Feb 20, 2020
Merged

Feature/static code security scanner #74

merged 14 commits into from
Feb 20, 2020

Conversation

joshmarsh
Copy link
Contributor

@joshmarsh joshmarsh commented Feb 18, 2020

Proposed changes

gosec is a static code analysis tool that scans for known security vulnerabilities. This PR adds gosec to the pipeline and fixes some issues raised by the tool.

Example of gosec working in our pipeline

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (changes to code, which do not change application behavior)

Checklist

  • I have filled out this PR template
  • I have read the CONTRIBUTING doc
  • I have added automated tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (README.md, inline comments, etc.)
  • I have updated the CHANGELOG.md under a ## next release, with a short summary of my changes

Relevant Links

Further comments

#fi
#echo "done."

#echo -n "Linting golang code... "
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to enable formatting/linting in a separate PR since it will likely be a large diff that doesn't change any functionality.

@joshmarsh
Copy link
Contributor Author

@@ -70,6 +70,9 @@ type DeleteLeasesIDOK struct {
}

func (o *DeleteLeasesIDOK) Error() string {
/*
#nosec: CWE-89 false positive. No sql here.
Copy link
Contributor Author

@joshmarsh joshmarsh Feb 18, 2020

Choose a reason for hiding this comment

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

#nosec is the syntax for annotating known gosec issues so that they don't cause the tool to fail.

@joshmarsh joshmarsh changed the title [WIP] Feature/static code security scanner Feature/static code security scanner Feb 18, 2020
@joshmarsh joshmarsh mentioned this pull request Feb 19, 2020
9 tasks
@@ -40,6 +43,9 @@ func (u *AWSUtil) UploadDirectoryToS3(localPath string, bucket string, prefix st
if err != nil {
log.Fatalln("Unable to get relative path:", path, err)
}
/*
#nosec CWE-22: added disclaimer to function docs
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshmarsh can you explain how this works / what is does?

Copy link
Contributor Author

@joshmarsh joshmarsh Feb 19, 2020

Choose a reason for hiding this comment

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

Sorry, I though I added a comment explaining nosec, but I’m not seeing it anymore.

Adding a //#nosec comment causes gosec to skip the code that you commented.

Here are the docs: https://github.com/securego/gosec/blob/master/README.md#annotating-code

@joshmarsh joshmarsh merged commit ccc8ce8 into Optum:master Feb 20, 2020
@joshmarsh joshmarsh deleted the feature/static-code-security-scanner branch February 20, 2020 01:39
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