Skip to content

Use a http.Server with a timeout - #3139

Merged
parametalol merged 1 commit into
masterfrom
michael/gosec-G114-fix-webhookserver
Sep 26, 2022
Merged

Use a http.Server with a timeout#3139
parametalol merged 1 commit into
masterfrom
michael/gosec-G114-fix-webhookserver

Conversation

@parametalol

@parametalol parametalol commented Sep 20, 2022

Copy link
Copy Markdown
Contributor

Description

gosec reports G114: Use of net/http serve function that has no support for setting timeouts.

(Some versions of gosec would actually crash trying to report that.)

Checklist

  • Investigated and inspected CI test results
  • Unit test and regression tests added
  • Evaluated and added CHANGELOG entry if required
  • Determined and documented upgrade steps
  • Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)

Testing Performed

$ gosec webhookserver
[gosec] 2022/09/20 10:32:51 Including rules: default
[gosec] 2022/09/20 10:32:51 Excluding rules: default
[gosec] 2022/09/20 10:32:51 Import directory: /home/mipetrov/go/src/github.com/stackrox/stackrox/webhookserver
[gosec] 2022/09/20 10:32:51 Checking package: main
[gosec] 2022/09/20 10:32:51 Checking file: /home/mipetrov/go/src/github.com/stackrox/stackrox/webhookserver/server.go
Results:


Summary:
  Gosec  : dev
  Files  : 1
  Lines  : 99
  Nosec  : 0
  Issues : 0

@parametalol
parametalol force-pushed the michael/gosec-G114-fix-webhookserver branch from 1cdb598 to 299f111 Compare September 20, 2022 09:10
@ghost

ghost commented Sep 20, 2022

Copy link
Copy Markdown

Images are ready for the commit at 299f111.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.72.x-35-g299f1119f4.

@misberner

misberner commented Sep 20, 2022

Copy link
Copy Markdown
Contributor

I guess this change doesn't hurt, but just wanted to remark the webhookserver is test-only software (run during e2e tests) and thus probably not super interesting from a security standpoint.

@parametalol

parametalol commented Sep 20, 2022

Copy link
Copy Markdown
Contributor Author

I guess this change doesn't hurt, but just wanted to remark the webhookserver is test-only software (run during e2e tests) and thus probably not super interesting from a security standpoint.

Right, thank you Malte. This fix will remove the only appearance of G114 from gosec report, which is just nice to get rid of.

@parametalol

parametalol commented Sep 21, 2022

Copy link
Copy Markdown
Contributor Author

/retest

@parametalol
parametalol merged commit 0daca41 into master Sep 26, 2022
@parametalol
parametalol deleted the michael/gosec-G114-fix-webhookserver branch September 26, 2022 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vikin91 vikin91 vikin91 approved these changes
@tommartensen tommartensen Awaiting requested review from tommartensen

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@parametalol @misberner @vikin91