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

enable gosec #969

Merged
merged 4 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,20 @@ jobs:
run: |
go fmt -x ./...
git diff --exit-code || { echo 'Go sources need to be formatted. Execute "go fmt -x ./..." locally in the 'generator' folder and commit changes to fix an issue'; exit 1; }

- name: Run Gosec Security Scanner
run: |
export PATH=$PATH:$(go env GOPATH)/bin
go install github.com/securego/gosec/v2/cmd/gosec@latest
./run_gosec.sh
if [[ $? != 0 ]]
then
echo "gosec scanner failed to run "
exit 1
fi

- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v2
with:
# Path to SARIF file relative to the root of the repository
sarif_file: gosec.sarif
4 changes: 4 additions & 0 deletions pkg/apis/workspaces/v1alpha1/component_plugin_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func convertPluginComponentTo_v1alpha2(srcComponent *Component, destComponent *v
destComponent.Name = pluginKey

for _, srcCommand := range src.Commands {
srcCommand := srcCommand
Copy link
Contributor

Choose a reason for hiding this comment

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

This line appears to be a no-op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it ensures the address is not reused. It's one of the approaches you can use to fix this issue (the other is to use an index) according to this discussion: https://stackoverflow.com/questions/62446118/implicit-memory-aliasing-in-for-loop.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah makes sense, I didn't realize we used &srcCommand below.

if srcCommand.Custom != nil {
// v1alpha2 does not support Plugin Custom commands, so we have to drop them here
continue
Expand All @@ -40,6 +41,7 @@ func convertPluginComponentTo_v1alpha2(srcComponent *Component, destComponent *v
}

for _, srcComponent := range src.Components {
srcComponent := srcComponent
destComponent := v1alpha2.ComponentPluginOverride{}
err := convertPluginComponentSubComponentTo_v1alpha2(&srcComponent, &destComponent)
if err != nil {
Expand Down Expand Up @@ -101,6 +103,7 @@ func convertPluginComponentFrom_v1alpha2(srcComponent *v1alpha2.Component, destC
destComponent.Plugin.Name = srcComponent.Name

for _, srcCommand := range src.Commands {
srcCommand := srcCommand
destCommand := Command{}
err := convertPluginComponentCommandFrom_v1alpha2(&srcCommand, &destCommand)
if err != nil {
Expand All @@ -110,6 +113,7 @@ func convertPluginComponentFrom_v1alpha2(srcComponent *v1alpha2.Component, destC
}

for _, srcComponent := range src.Components {
srcComponent := srcComponent
destComponent := PluginComponentsOverride{}
err := convertPluginComponentSubComponentFrom_v1alpha2(&srcComponent, &destComponent)
if err != nil {
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/workspaces/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func convertDevWorkspaceTemplateSpecTo_v1alpha2(src *DevWorkspaceTemplateSpec, d
}
}
for _, srcComponent := range src.Components {
srcComponent := srcComponent
destComponent := v1alpha2.Component{}
err := convertComponentTo_v1alpha2(&srcComponent, &destComponent)
if err != nil {
Expand All @@ -63,6 +64,7 @@ func convertDevWorkspaceTemplateSpecTo_v1alpha2(src *DevWorkspaceTemplateSpec, d
dest.Components = append(dest.Components, destComponent)
}
for _, srcProject := range src.Projects {
srcProject := srcProject
destProject := v1alpha2.Project{}
err := convertProjectTo_v1alpha2(&srcProject, &destProject)
if err != nil {
Expand All @@ -71,6 +73,7 @@ func convertDevWorkspaceTemplateSpecTo_v1alpha2(src *DevWorkspaceTemplateSpec, d
dest.Projects = append(dest.Projects, destProject)
}
for _, srcStarterProject := range src.StarterProjects {
srcStarterProject := srcStarterProject
destStarterProject := v1alpha2.StarterProject{}
err := convertStarterProjectTo_v1alpha2(&srcStarterProject, &destStarterProject)
if err != nil {
Expand All @@ -79,6 +82,7 @@ func convertDevWorkspaceTemplateSpecTo_v1alpha2(src *DevWorkspaceTemplateSpec, d
dest.StarterProjects = append(dest.StarterProjects, destStarterProject)
}
for _, srcCommand := range src.Commands {
srcCommand := srcCommand
destCommand := v1alpha2.Command{}
err := convertCommandTo_v1alpha2(&srcCommand, &destCommand)
if err != nil {
Expand All @@ -105,6 +109,7 @@ func convertDevWorkspaceTemplateSpecFrom_v1alpha2(src *v1alpha2.DevWorkspaceTemp
}
}
for _, srcComponent := range src.Components {
srcComponent := srcComponent
destComponent := Component{}
err := convertComponentFrom_v1alpha2(&srcComponent, &destComponent)
if err != nil {
Expand All @@ -113,6 +118,7 @@ func convertDevWorkspaceTemplateSpecFrom_v1alpha2(src *v1alpha2.DevWorkspaceTemp
dest.Components = append(dest.Components, destComponent)
}
for _, srcProject := range src.Projects {
srcProject := srcProject
destProject := Project{}
err := convertProjectFrom_v1alpha2(&srcProject, &destProject)
if err != nil {
Expand All @@ -121,6 +127,7 @@ func convertDevWorkspaceTemplateSpecFrom_v1alpha2(src *v1alpha2.DevWorkspaceTemp
dest.Projects = append(dest.Projects, destProject)
}
for _, srcStarterProject := range src.StarterProjects {
srcStarterProject := srcStarterProject
destStarterProject := StarterProject{}
err := convertStarterProjectFrom_v1alpha2(&srcStarterProject, &destStarterProject)
if err != nil {
Expand All @@ -129,6 +136,7 @@ func convertDevWorkspaceTemplateSpecFrom_v1alpha2(src *v1alpha2.DevWorkspaceTemp
dest.StarterProjects = append(dest.StarterProjects, destStarterProject)
}
for _, srcCommand := range src.Commands {
srcCommand := srcCommand
destCommand := Command{}
err := convertCommandFrom_v1alpha2(&srcCommand, &destCommand)
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/workspaces/v1alpha1/parent_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func convertParentTo_v1alpha2(src *Parent, dest *v1alpha2.Parent) error {
}

for _, srcCommand := range src.Commands {
srcCommand := srcCommand
if srcCommand.Custom != nil {
// v1alpha2 does not support Parent Custom commands, so we have to drop them here
continue
Expand All @@ -30,6 +31,7 @@ func convertParentTo_v1alpha2(src *Parent, dest *v1alpha2.Parent) error {
}

for _, srcComponent := range src.Components {
srcComponent := srcComponent
if srcComponent.Custom != nil {
// v1alpha2 does not support Parent Custom Components, so we have to drop them here
continue
Expand All @@ -43,6 +45,7 @@ func convertParentTo_v1alpha2(src *Parent, dest *v1alpha2.Parent) error {
}

for _, srcProject := range src.Projects {
srcProject := srcProject
destProject := v1alpha2.Project{}
err := convertProjectTo_v1alpha2(&srcProject, &destProject)
if err != nil {
Expand All @@ -61,6 +64,7 @@ func convertParentTo_v1alpha2(src *Parent, dest *v1alpha2.Parent) error {
}

for _, srcProject := range src.StarterProjects {
srcProject := srcProject
destProject := v1alpha2.StarterProject{}
err := convertStarterProjectTo_v1alpha2(&srcProject, &destProject)
if err != nil {
Expand Down Expand Up @@ -146,6 +150,7 @@ func convertParentFrom_v1alpha2(src *v1alpha2.Parent, dest *Parent) error {
dest.Kubernetes = &kube
}
for _, srcCommand := range src.Commands {
srcCommand := srcCommand
destCommand := Command{}
err := convertParentCommandFrom_v1alpha2(&srcCommand, &destCommand)
if err != nil {
Expand All @@ -155,6 +160,7 @@ func convertParentFrom_v1alpha2(src *v1alpha2.Parent, dest *Parent) error {
}

for _, srcComponent := range src.Components {
srcComponent := srcComponent
destComponent := Component{}
err := convertParentComponentFrom_v1alpha2(&srcComponent, &destComponent)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/workspaces/v1alpha1/union_implementation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func visitUnion(union interface{}, visitor interface{}) (err error) {
}

func simplifyUnion(union Union, visitorType reflect.Type) {
normalizeUnion(union, visitorType)
_ = normalizeUnion(union, visitorType)
*union.discriminator() = ""
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/workspaces/v1alpha2/union_implementation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func visitUnion(union interface{}, visitor interface{}) (err error) {
}

func simplifyUnion(union Union, visitorType reflect.Type) {
normalizeUnion(union, visitorType)
_ = normalizeUnion(union, visitorType)
*union.discriminator() = ""
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/unions/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (n *normalizer) Struct(s reflect.Value) error {
if addr.CanInterface() {
i := addr.Interface()
if u, ok := i.(dw.Union); ok {
u.Normalize()
_ = u.Normalize()
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions run_gosec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash
# This script runs the gosec scanner locally

if ! command -v gosec 2> /dev/null
then
echo "error gosec must be installed with this command: go install github.com/securego/gosec/v2/cmd/gosec@latest" && exit 1
fi

gosec -no-fail -fmt=sarif -out=gosec.sarif -exclude-dir test -exclude-dir generator ./...
Copy link
Contributor

Choose a reason for hiding this comment

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

Trailling newline missing.