Skip to content

Fix SonarCloud HTTPS-redirect findings in download-latest.sh - #124

Merged
LarsLaskowski merged 1 commit into
codex/add-release-download-scriptfrom
claude/review-pr-123-elfyny
Aug 22, 2026
Merged

Fix SonarCloud HTTPS-redirect findings in download-latest.sh#124
LarsLaskowski merged 1 commit into
codex/add-release-download-scriptfrom
claude/review-pr-123-elfyny

Conversation

@LarsLaskowski

@LarsLaskowski LarsLaskowski commented Aug 22, 2026

Copy link
Copy Markdown
Owner

📖 Description

Targets #123's branch directly (opened against codex/add-release-download-script, not main) 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 new packaging/download-latest.sh:

  • Line 45 — the curl call fetching the latest-release metadata from the GitHub API followed redirects without restricting their protocol.
  • Line 64 — the 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 curl invocation with --proto '=https' --proto-redir '=https', which rejects an insecure URL and an insecure redirect target. The archive download switches from wget to this same curl call — wget was tested locally against a redirecting server and has no equivalent guard for a non-recursive download (--https-only only restricts recursive link-following, not a redirect on the initial request), so it couldn't satisfy the rule for that line. This also drops wget from the script's required-commands check and, as a side effect, replaces wget --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 wgetcurl swap for the archive download. Verified --proto/--proto-redir are accepted by the installed curl, and confirmed empirically (via a local HTTP server issuing a redirect) that wget --https-only does not block a downgrade redirect outside of -r/recursive mode, which is why wget was dropped rather than patched in place.

📑 Test Plan

  • bash -n packaging/download-latest.sh (syntax check)
  • Manually confirmed curl --proto '=https' --proto-redir '=https' rejects an http:// URL/redirect and is accepted by the curl build used in CI.
  • go build ./... / go vet ./... (unaffected by this change, run for safety).
  • No Go test suite applies to this shell script; the repository has no shellcheck/shell-test job in CI (install.sh, the only prior shell script, likewise has no test coverage), so no automated test accompanies this fix.

✅ Checklist

General

  • I have added/updated tests for my changes (go test ./... -race -cover passes locally). — N/A, see Test Plan above.
  • go vet ./... and golangci-lint run are clean.
  • I have tested my changes.
  • I have read the CONTRIBUTING documentation and followed the project's code style guidelines.
  • I have updated ARCHITECTURE.md if this changes a documented design decision.

REST API / configuration / packaging

  • I have updated docs/API.md to reflect a REST API change.
  • No breaking change to /api/v1/... response shapes, or a new API version (/api/v2/...) was introduced instead.
  • I have updated README.md / packaging/pimonitor.example.yaml to reflect a new or changed configuration option.
  • I have updated packaging/install.sh or the systemd units if this changes installation/packaging, and kept the unprivileged/privileged service split intact (see SECURITY.md).

⏭ Next Steps

None.

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.
@LarsLaskowski LarsLaskowski mentioned this pull request Aug 22, 2026
9 tasks
@LarsLaskowski
LarsLaskowski merged commit 56bc44e into codex/add-release-download-script Aug 22, 2026
@LarsLaskowski
LarsLaskowski deleted the claude/review-pr-123-elfyny branch August 22, 2026 11:46
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants