G307: Readonly files also a problem? #579
Labels
Comments
|
This is a good point. gosec should handle this case. |
gregtyler
added a commit
to ministryofjustice/opg-sirius-user-management
that referenced
this issue
Mar 29, 2021
This flags as an error in gosec because we're not handling any errors when the file closes. But that isn't a concern for us because we're not writing to it See also: securego/gosec#579
gregtyler
added a commit
to ministryofjustice/opg-sirius-user-management
that referenced
this issue
Mar 30, 2021
* Create gosec.yml * Check filepath before opening file To ensure that we don't end up opening a file outside of the base pacts directory * Ignore G307 error This flags as an error in gosec because we're not handling any errors when the file closes. But that isn't a concern for us because we're not writing to it See also: securego/gosec#579 * Wrap open in `filepath.Clean` This removes any `..` from the filepath, so it can't break out of the pacts directory
Nicholassully
pushed a commit
to ministryofjustice/opg-sirius-user-management
that referenced
this issue
Jul 26, 2021
* Create gosec.yml * Check filepath before opening file To ensure that we don't end up opening a file outside of the base pacts directory * Ignore G307 error This flags as an error in gosec because we're not handling any errors when the file closes. But that isn't a concern for us because we're not writing to it See also: securego/gosec#579 * Wrap open in `filepath.Clean` This removes any `..` from the filepath, so it can't break out of the pacts directory
|
Just tripped over this one with code that calls For the time being I've added an exclusion for this in issues:
exclude:
- Deferring unsafe method "Close" on type "io\.ReadCloser" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I understand that G307 is a valid concern on writable files.
But what about files that are opened ReadOnly? E.g.
os.Openopens files in readonly mode. UsingWriteon it will error immediately withbad file descriptor. So the arguments mentioned in https://www.joeshaw.org/dont-defer-close-on-writable-files/ won't hold for readonly file descriptors.Shouldn't
gosecmake that distinction as well?Especially when using
deferonClose, the open file function is usually in the same function as theClose. Then it should be possible to discern that.The text was updated successfully, but these errors were encountered: