fix: remediate plural-cli image vulnerability findings#779
fix: remediate plural-cli image vulnerability findings#779plural-copilot[bot] wants to merge 1 commit into
Conversation
plural-copilot
Bot
requested review from
michaeljguarino and
zreigz
as code owners
![plural-copilot[bot]](https://avatars.githubusercontent.com/in/916369?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR was generated by the codex Plural Agent Runtime. Here's some useful information you might want to know to evaluate the ai's perfomance:
| Name | Details |
|---|---|
| 💬 Prompt | Create a PR in https://github.com/pluralsh/plural-cli to remediate vulnerabilities associated with the ghcr.io/pluralsh/console:sha-3e7dd03 scan, based on the known environment mapping that this image's Go vulnerabilities map to this repo.... |
| 🔗 Run history | View run history |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Greptile SummaryThis PR remediates vulnerability findings by removing the
Confidence Score: 4/5Safe to merge; the core vulnerability remediation is sound and all functional changes are well-scoped The new ChartMuseum HTTP client correctly reproduces the bearer-token behavior of the removed library, the Dockerfile final-stage shrink is a net positive, and the Go toolchain bump is straightforward. The only actionable issues are the misplaced t.Helper() call in the test function and the use of http.DefaultClient without a timeout, both of which are non-blocking quality concerns. pkg/helm/chartmuseum_test.go (t.Helper() misuse) and pkg/helm/chartmuseum.go (no HTTP timeout)
|
| Filename | Overview |
|---|---|
| pkg/helm/chartmuseum.go | Replaces chartmuseum/helm-push dependency with a custom HTTP download implementation; logic is correct but uses http.DefaultClient with no timeout |
| pkg/helm/chartmuseum_test.go | New tests for the HTTP-based ChartMuseum client; t.Helper() is incorrectly called inside a Test* function on line 17 |
| Dockerfile | Builder upgraded to golang:1.26.5; final stage switched from golang:1.26.4-alpine3.22 to alpine:3.22 (good attack-surface reduction), but the binary is still placed in /go/bin/ which is a non-standard path for Alpine |
| go.mod | Go toolchain bumped to 1.26.5; github.com/chartmuseum/helm-push removed; several indirect dependencies promoted to direct to reflect actual usage |
| test.Dockerfile | Test image bumped from golang:1.26.4-bookworm to golang:1.26.5-bookworm; no functional changes |
| go.sum | Checksums updated to reflect the dependency changes in go.mod; no issues |
Comments Outside Diff (1)
-
Dockerfile, line 44-45 (link)/go/bin/is a Go-image convention, not an Alpine pathThe final stage is now
alpine:3.22, which has no/godirectory. Docker'sCOPYwill create it, so the image builds correctly, but it is an unusual path on a non-Go base image. The conventional location for user-installed binaries on Alpine is/usr/local/bin/; moving there would make the image layout consistent with Alpine conventions and with thePATHthat Alpine sets by default.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "fix: remediate plural-cli image dependen..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func TestChartMuseumGetDownloadsChart(t *testing.T) { | ||
| t.Helper() |
There was a problem hiding this comment.
t.Helper() called inside a Test* function
t.Helper() is designed for shared helper functions (e.g., setup, assertFoo) to skip their frame in failure stack traces. Calling it at the top of TestChartMuseumGetDownloadsChart marks the test function itself as a helper, meaning any t.Fatal/t.Errorf inside it will report the failure location as the caller — which is the testing framework, not your test code. This produces confusing "unknown" or misleading failure output rather than the exact line that failed.
| req.Header.Set("Authorization", "Bearer "+conf.Token) | ||
| } | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
http.DefaultClient has no timeout
http.DefaultClient has zero timeout, so a slow or unresponsive ChartMuseum server will hang the CLI process indefinitely. A simple http.Client{Timeout: 30 * time.Second} (or a package-level client with a configurable timeout) would prevent indefinite hangs in production use.
Summary
github.com/chartmuseum/helm-pushdependency path by replacing the narrow ChartMuseum download usage with an internal HTTP implementationPlural Service: a1748282-ce8b-48ab-ae7e-326e74fce04e/console
Image / build path mapping
The scan was reported against
ghcr.io/pluralsh/console:sha-3e7dd03, but environment guidance maps this image's Go vulnerabilities to this repo because the affected contents are the plural CLI binary.In this repo, the corresponding image build path is:
Dockerfile.github/workflows/ci.yaml→build-image/publish-imageghcr.io/pluralsh/plural-cliExact version changes
go.mod: Go toolchain1.26.4->1.26.5Dockerfilebuilder image:golang:1.26.4-alpine3.22->golang:1.26.5Dockerfilefinal image/tooling stage path:golang:1.26.4-alpine3.22->alpine:3.22with the built binary copied from the Go 1.26.5 buildertest.Dockerfile:golang:1.26.4-bookworm->golang:1.26.5-bookwormgithub.com/chartmuseum/helm-push v0.11.1removed from the build path entirelyVulnerability remediation notes
oras.land/oras-go/v2remediation: the vulnerable chain came fromgithub.com/chartmuseum/helm-push. This PR removes that dependency from the build by replacing the narrowpkg/helm/chartmuseum.goclient usage with an internal HTTP download implementation that preserves bearer-token behavior.golang.org/x/crypto: I investigated direct usage and found only SSH consumers (pkg/scm/keys.go,pkg/utils/git/auth.go);openpgpis not directly used in this repo. I attempted to move the module, but the current module graph/tidy flow reintroducesgolang.org/x/crypto v0.53.0, so I am calling that out as a residual limitation instead of making a broad dependency graph refresh in this surgical PR.Verification
docker build -f test.Dockerfile .docker run --rm -v "$PWD":/workspace -w /workspace --entrypoint /usr/local/go/bin/go golang:1.26.5-bookworm test ./pkg/helm -run TestChartMuseum -count=1docker run --rm -v "$PWD":/workspace -w /workspace --entrypoint /usr/local/go/bin/go golang:1.26.5-bookworm build -buildvcs=false ./cmd/pluraldocker build -f Dockerfile .go mod tidy-driven indirect dependency churn as part of this surgical remediation.Residual limitation
golang.org/x/crypto v0.53.0remains in the resolved graph because the repo still actively uses thesshpackage and the current transitive dependency set retains that module version. This PR removes theoras-gofinding cleanly and upgrades the toolchain/base images, but a fullx/cryptoremediation likely needs a broader dependency refresh outside this minimal change set.