ci: fix gosec image tag (drop v prefix)#25
Merged
Merged
Conversation
defer r.Body.Close() was registered after the early-return error check, so a read failure leaked the body. Move defer before io.ReadAll.
LimitReader silently truncated bodies at maxBodySize without any error or indication to the caller. Use the maxBodySize+1 probe (same approach as multiplex/sendRequest) to detect truncation and return 413. Also correct the misleading 'Payload too large' message on genuine I/O errors, which now returns 400 via writeJSONError.
echoHandler default mode and healthCheck wrote JSON bodies without the corresponding Content-Type header, inconsistent with every other JSON-producing handler in the service.
Both counters were registered but never incremented, making them dead metrics. retriesTotal is now recorded at each retry point labelled by cause (network_error or server_error). retrySuccess is recorded after a successful response that required at least one retry, labelled by the total attempt count.
securego/gosec tags ≥2.10 are published without the 'v' prefix on Docker Hub, so the existing pin `securego/gosec:v2.22.4` resolves to `manifest unknown` and fails the gosec step with exit 125. Use `2.22.4`.
The -healthcheck CLI flag's Close() return value was unchecked; gosec G104 flagged it. We immediately os.Exit after this point so the close error is genuinely irrelevant — explicit `_ =` makes the intent clear and satisfies the scan.
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub Actions workflows to use the correct securego/gosec Docker tag format (dropping the v prefix) so CI security scanning no longer fails with manifest unknown. It also includes additional, unrelated runtime changes in fanout.go (echo/health handlers and retry metrics).
Changes:
- Update
securego/gosecimage tag fromv2.22.4→2.22.4in CI workflows. - Adjust echo mode request body reading to properly detect “payload too large” via
maxBodySize+1, and standardize JSON responses/content-type. - Add Prometheus retry counters for network/server retry events and a “retry success” counter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
fanout.go |
Updates echo/health response behavior and adds retry-related Prometheus metrics instrumentation. |
.github/workflows/docker-image.yml |
Fixes securego/gosec Docker tag so the security scan step can run. |
.github/workflows/binary-release.yml |
Applies the same gosec tag fix to release workflow security scanning. |
Comment on lines
372
to
+382
| func echoHandler(w http.ResponseWriter, r *http.Request) { | ||
| bodyBytes, err := io.ReadAll(io.LimitReader(r.Body, maxBodySize)) | ||
| defer r.Body.Close() | ||
| bodyBytes, err := io.ReadAll(io.LimitReader(r.Body, maxBodySize+1)) | ||
| if err != nil { | ||
| logError("Error reading body: %v", err) | ||
| http.Error(w, "Payload too large", http.StatusRequestEntityTooLarge) | ||
| writeJSONError(w, "Failed to read request body", http.StatusBadRequest) | ||
| return | ||
| } | ||
| if int64(len(bodyBytes)) > maxBodySize { | ||
| logError("Request body size exceeds limit (%d bytes read)", len(bodyBytes)) | ||
| writeJSONError(w, "Payload too large", http.StatusRequestEntityTooLarge) |
| resp.Body = string(respBody) | ||
| resp.Attempts = attempts + 1 | ||
|
|
||
| if metricsEnabled && attempts > 0 { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
securego/gosectags ≥2.10 are published on Docker Hub without thevprefixsecurego/gosec:v2.22.4returnsmanifest unknown→ exit 125v. Tag2.22.4exists and matches the originally intended pinTest plan
Security scan (gosec)step indocker-image.ymlbinary-release.ymlwill pick up the same fix on next tag push