Fix SonarCloud HTTPS-redirect findings in download-latest.sh - #124
Merged
LarsLaskowski merged 1 commit intoAug 22, 2026
Merged
Conversation
SonarCloud flagged both curl/wget calls in the new release-download script (rule shell:S6506): a redirect could silently downgrade the transfer to plain HTTP, allowing a MITM to serve a tampered release. - The GitHub API request now runs through a shared curl invocation with --proto '=https' --proto-redir '=https', which rejects both an insecure initial URL and an insecure redirect target. - The release-archive download switches from wget to the same curl invocation, since wget (tested against a local redirecting server) has no equivalent way to reject an HTTP redirect target for a non-recursive download; --https-only only restricts recursive link following, not redirects. This also drops wget from the script's required-commands check.
9 tasks
LarsLaskowski
added a commit
that referenced
this pull request
Aug 22, 2026
* Add release download script Automating release retrieval avoids requiring users to manually resolve the latest tag and select a package architecture. * Delete packaging/download_latest_test.go * Fix SonarCloud HTTPS-redirect findings in download-latest.sh (#124) Enforce HTTPS-only redirects in download-latest.sh SonarCloud flagged both curl/wget calls in the new release-download script (rule shell:S6506): a redirect could silently downgrade the transfer to plain HTTP, allowing a MITM to serve a tampered release. - The GitHub API request now runs through a shared curl invocation with --proto '=https' --proto-redir '=https', which rejects both an insecure initial URL and an insecure redirect target. - The release-archive download switches from wget to the same curl invocation, since wget (tested against a local redirecting server) has no equivalent way to reject an HTTP redirect target for a non-recursive download; --https-only only restricts recursive link following, not redirects. This also drops wget from the script's required-commands check.
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.
📖 Description
Targets
#123's branch directly (opened againstcodex/add-release-download-script, notmain) so it can be merged straight into that PR.SonarCloud's analysis of PR #123 flagged two
shell:S6506("Not enforcing HTTPS / not disabling redirects might allow for redirections to insecure websites") vulnerabilities in the newpackaging/download-latest.sh:curlcall fetching the latest-release metadata from the GitHub API followed redirects without restricting their protocol.wget --no-clobber "$url"call downloading the release archive had the same issue.A malicious or compromised server in the redirect chain could downgrade either request from HTTPS to plain HTTP, letting a network-level attacker tamper with the release metadata or the downloaded archive.
Fix: both calls now go through a shared
curlinvocation with--proto '=https' --proto-redir '=https', which rejects an insecure URL and an insecure redirect target. The archive download switches fromwgetto this samecurlcall —wgetwas tested locally against a redirecting server and has no equivalent guard for a non-recursive download (--https-onlyonly restricts recursive link-following, not a redirect on the initial request), so it couldn't satisfy the rule for that line. This also dropswgetfrom the script's required-commands check and, as a side effect, replaceswget --no-clobber's silent reuse of a stale/partial leftover archive with a normal overwrite-on-every-run download.🎫 Issues
Addresses SonarCloud findings on #123 (rule
shell:S6506, both instances).👩💻 Reviewer Notes
Minimal, mechanical fix — no behavior change beyond the protocol enforcement and the
wget→curlswap for the archive download. Verified--proto/--proto-redirare accepted by the installed curl, and confirmed empirically (via a local HTTP server issuing a redirect) thatwget --https-onlydoes not block a downgrade redirect outside of-r/recursive mode, which is whywgetwas dropped rather than patched in place.📑 Test Plan
bash -n packaging/download-latest.sh(syntax check)curl --proto '=https' --proto-redir '=https'rejects anhttp://URL/redirect and is accepted by the curl build used in CI.go build ./.../go vet ./...(unaffected by this change, run for safety).install.sh, the only prior shell script, likewise has no test coverage), so no automated test accompanies this fix.✅ Checklist
General
go test ./... -race -coverpasses locally). — N/A, see Test Plan above.go vet ./...andgolangci-lint runare clean.ARCHITECTURE.mdif this changes a documented design decision.REST API / configuration / packaging
docs/API.mdto reflect a REST API change./api/v1/...response shapes, or a new API version (/api/v2/...) was introduced instead.README.md/packaging/pimonitor.example.yamlto reflect a new or changed configuration option.packaging/install.shor the systemd units if this changes installation/packaging, and kept the unprivileged/privileged service split intact (seeSECURITY.md).⏭ Next Steps
None.