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

chore: Enable G103 rule for gosec #13038

Merged
merged 8 commits into from
Apr 12, 2023
Merged

chore: Enable G103 rule for gosec #13038

merged 8 commits into from
Apr 12, 2023

Conversation

zak-pawel
Copy link
Collaborator

resolves #12891

Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

In addition to Thomas' questions, I had one about how we document this.

plugins/inputs/procstat/win_service_windows.go Outdated Show resolved Hide resolved
@powersj powersj self-assigned this Apr 5, 2023
@powersj powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Apr 5, 2023
@powersj powersj removed their assignment Apr 5, 2023
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks @zak-pawel for looking at this and also for fixing some of the locations. Maybe I'm just misreading the comments, but to me

//nolint:gosec // G103: Use of unsafe calls should be audited

reads like "we will check this later". If this is the actual meaning I want to veto this as nobody will ever look for those again once silenced. If the meaning is "I checked and it is a valid use" we should state that explicitly IMO.

plugins/inputs/procstat/win_service_windows.go Outdated Show resolved Hide resolved
plugins/inputs/win_eventlog/util.go Outdated Show resolved Hide resolved
plugins/inputs/win_eventlog/zsyscall_windows.go Outdated Show resolved Hide resolved
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for making the comment more clear @zak-pawel!

@powersj are you OK with the new wording?

@srebhan srebhan requested a review from powersj April 11, 2023 09:06
@srebhan srebhan assigned powersj and unassigned srebhan Apr 11, 2023
@powersj powersj merged commit 55e4bb6 into influxdata:master Apr 12, 2023
powersj pushed a commit that referenced this pull request Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore linter ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linter: gosec, Rule: G103 - Audit the use of unsafe block. Should we enable it?
4 participants