Skip to content
This repository has been archived by the owner on Feb 15, 2024. It is now read-only.

internal/fileutils/contains.go:40:12: G304: Potential file inclusion via variable (gosec) #106

Closed
atc0005 opened this issue Jul 21, 2020 · 3 comments · Fixed by #114
Closed

internal/fileutils/contains.go:40:12: G304: Potential file inclusion via variable (gosec) #106

atc0005 opened this issue Jul 21, 2020 · 3 comments · Fixed by #114
Assignees
@atc0005
Labels
bug Something isn't working linting
Milestone
v0.4.1

Comments

@atc0005
Copy link
Owner

atc0005 commented Jul 21, 2020

Found this after using --exclude-use-default=false with version v1.29.0.

$ golangci-lint run -v --exclude-use-default=false
# ...
internal/fileutils/contains.go:40:12: G304: Potential file inclusion via variable (gosec)
        f, err := os.Open(filename)
                  ^
@atc0005 atc0005 added bug Something isn't working linting labels Jul 21, 2020
@atc0005 atc0005 self-assigned this Jul 21, 2020
@atc0005 atc0005 added this to the Next Release milestone Jul 21, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Jul 22, 2020

https://securego.io/docs/rules/g304.html

@atc0005
Copy link
Owner Author

atc0005 commented Jul 22, 2020

https://golang.org/pkg/path/filepath/#Clean

@atc0005
Copy link
Owner Author

atc0005 commented Jul 22, 2020

I tried to set a cleanFilename variable some number of lines earlier and use that, but it is still a variable and still fails the check.

Instead, I have to use an explicit filepath.Clean(filename) call at the point where the linting error was being raised. Since I'm logging the original and the sanitized version of the filename it feels redundant to call filepath.Clean twice, but it's a small price to pay to build better habits.

atc0005 added a commit that referenced this issue Jul 22, 2020
@atc0005
Linting (G304) | Explicitly use filepath.Clean
59c3731 
Instead of using the original value as-is or attempting to
create a localized "cleanFilename" var, use filepath.Clean
directly at the os.Open call. This resolves the original
linting issue and (presumably) makes our intent clearer.

refs GH-106
@atc0005 atc0005 mentioned this issue Jul 22, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@atc0005 atc0005

Labels
bug Something isn't working linting
Projects
None yet
Milestone
v0.4.1
Development

Successfully merging a pull request may close this issue.

Linting (G304) | Explicitly use filepath.Clean atc0005/brick
1 participant
@atc0005