Cherry-pick #47872: Fix high-severity CVEs flagged by code scanning#47916
Open
georgekarrv wants to merge 2 commits into
Open
Cherry-pick #47872: Fix high-severity CVEs flagged by code scanning#47916georgekarrv wants to merge 2 commits into
georgekarrv wants to merge 2 commits into
Conversation
## Summary Addresses all 12 high-severity code-scanning alerts open on `rc-minor-fleet-v4.87.0`: - **form-data (CVE-2026-12143)** — bumped to 2.5.6 / 4.0.6 across all 5 lock files (root `yarn.lock`, `tools/fleet-slackbot`, `tools/fleetctl-npm`, `website`, `.github/actions/eng-metrics`). Added an `overrides` block in `website/package.json` to force `form-data@2.5.6` inside `@sailshq/request`, which exact-pins the vulnerable 2.5.5. - **ws (CVE-2026-48779)** — bumped to 8.21.0 in `website` and `tools/fleet-slackbot`. Added a `ws@~8.20.1 -> 8.21.0` override so the transitive `engine.io` pin doesn't pull the vulnerable version back in. - **hono (CVE-2026-54290)** — bumped to 4.12.25 in `tools/fleet-slackbot`. - **docker/docker (CVE-2026-34040, -41567, -42306)** — migrated `test/upgrade/fleet_test.go` from `github.com/docker/docker` to `github.com/moby/moby/client v0.5.0`. The three CVEs are in the Docker daemon (`dockerd`), which Fleet does not compile in — only the client SDK is used in upgrade tests. The new `moby/moby/client` is the maintained split-out client package after the v29 module restructure (where `github.com/docker/docker` is no longer the canonical path). `docker/docker` remains as an indirect transitive dep (via `go.elastic.co/apm`'s `go-sysinfo`), bumped from v28.0.0 to v28.5.2 — the latest in that line. Its removal would require changes outside this branch's scope. The `website/package-lock.json` diff is large because `npm install --package-lock-only` cleaned up `"extraneous": true` entries under `sails-hook-grunt` that were already dead in the resolved tree. No functional packages were removed. ## Test plan - [ ] CI passes - [ ] Re-run code scanning on the merged commit to confirm the 12 high-severity alerts are resolved (or marked not-applicable for the daemon CVEs since we no longer directly require docker/docker) - [ ] `yarn install --frozen-lockfile` succeeds in `tools/fleet-slackbot` and root - [ ] `npm ci` succeeds in `website/` and `.github/actions/eng-metrics/` - [ ] `go test -count=0 ./test/upgrade/...` compiles cleanly (verified locally)
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
WalkthroughThe pull request migrates the upgrade test harness from 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Cherry-picks the security dependency updates from #47872 onto main to resolve high-severity code scanning CVEs across the website, tooling, and upgrade tests.
Changes:
- Added npm
overridesto force patchedform-dataandwsversions in the website dependency graph. - Updated lockfiles (npm + yarn) to pull in patched dependency versions and remove stale
extraneousentries. - Migrated upgrade tests from
github.com/docker/dockerclient usage togithub.com/moby/moby/clientand updated Go module dependencies accordingly, plus added Trivy ignore notes for daemon-only Docker CVEs still present transitively.
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| website/package.json | Adds overrides to force patched form-data and ws versions for transitive deps. |
| website/package-lock.json | Updates resolved versions for form-data and ws and prunes extraneous entries. |
| tools/fleet-slackbot/yarn.lock | Bumps form-data and ws to patched versions in slackbot’s dependency tree. |
| test/upgrade/fleet_test.go | Switches Docker SDK usage to github.com/moby/moby/client APIs for upgrade tests. |
| security/code/.trivyignore | Documents/ignores daemon-only Docker CVEs that remain via indirect dependency. |
| go.mod | Removes direct github.com/docker/docker, adds github.com/moby/moby/client, and bumps indirect Docker-related deps. |
| go.sum | Updates checksums consistent with the Go module dependency changes. |
| .github/actions/eng-metrics/package-lock.json | Updates form-data (and related transitive deps) to patched versions for the action. |
Files not reviewed (2)
- .github/actions/eng-metrics/package-lock.json: Generated file
- website/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #47916 +/- ##
==========================================
+ Coverage 67.25% 67.31% +0.05%
==========================================
Files 3641 3655 +14
Lines 230237 231251 +1014
Branches 11988 12220 +232
==========================================
+ Hits 154856 155661 +805
- Misses 61463 61624 +161
- Partials 13918 13966 +48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cdcme
previously approved these changes
Jun 19, 2026
- website/package.json: add override forcing request → form-data 2.5.6 (the deprecated request@2.88.0 was still pulling form-data 2.3.3, an open dependabot alert) - website/package-lock.json: regenerate via `npm install` so CI's check-doc-gen step doesn't dirty the lockfile. The original cherry-pick used `--package-lock-only` which deletes extraneous entries; this diverges from what CI's `npm install` produces. - .github/dependency-review-config.yml: allow-list the 5 docker/docker GHSAs flagged by the dependency-review action. All are Docker daemon (dockerd) vulnerabilities; Fleet doesn't compile or ship dockerd. The matching CVEs are already in security/code/.trivyignore. - .github/workflows/dependency-review.yml: wire up the config file. - security/code/.trivyignore: add CVE-2026-33997 and CVE-2026-41568 to match the two additional docker daemon advisories. - package.json: drop the **/wait-on/axios resolution that was forcing axios down to 0.28.1 (vulnerable to GHSA-p92q-9vqr-4j8v and GHSA-j5f8-grm9-p9fc, both proxy-auth credential leaks). wait-on natively wants axios ^1.6.1, which is well above the vulnerable range. - yarn.lock: regenerate after removing the resolution.
georgekarrv
requested review from
getvictor,
lucasmrod,
lukeheath and
sharon-fdm
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/upgrade/fleet_test.go`:
- Around line 207-214: The code at the port assignment (where
result.Items[0].Ports[0].PublicPort is accessed) does not check if the Ports
slice contains any elements before indexing. Add a bounds check after confirming
the container exists to verify that len(result.Items[0].Ports) is greater than
zero before accessing Ports[0], and return an appropriate error if no ports are
available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19070473-a51e-4684-9fcc-60b7ea1fdfbf
⛔ Files ignored due to path filters (5)
.github/actions/eng-metrics/package-lock.jsonis excluded by!**/package-lock.jsongo.sumis excluded by!**/*.sumtools/fleet-slackbot/yarn.lockis excluded by!**/yarn.lock,!**/*.lockwebsite/package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.github/dependency-review-config.yml.github/workflows/dependency-review.ymlgo.modpackage.jsonsecurity/code/.trivyignoretest/upgrade/fleet_test.gowebsite/package.json
💤 Files with no reviewable changes (1)
- package.json
Comment on lines
+207
to
+214
| result, err := f.dockerClient.ContainerList(context.TODO(), dockerclient.ContainerListOptions{Filters: make(dockerclient.Filters).Add("name", containerName), All: true}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if len(containers) == 0 { | ||
| if len(result.Items) == 0 { | ||
| return errors.New("no fleet container found") | ||
| } | ||
| port := containers[0].Ports[0].PublicPort | ||
| port := result.Items[0].Ports[0].PublicPort |
Contributor
There was a problem hiding this comment.
Guard Ports[0] access before indexing to avoid panics.
At Line 214, result.Items[0].Ports[0] assumes at least one mapped port. If Docker returns the container before port bindings are populated, this panics and makes the upgrade test flaky.
Suggested fix
if len(result.Items) == 0 {
return errors.New("no fleet container found")
}
+ if len(result.Items[0].Ports) == 0 {
+ return errors.New("fleet container has no published ports yet")
+ }
port := result.Items[0].Ports[0].PublicPort📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| result, err := f.dockerClient.ContainerList(context.TODO(), dockerclient.ContainerListOptions{Filters: make(dockerclient.Filters).Add("name", containerName), All: true}) | |
| if err != nil { | |
| return err | |
| } | |
| if len(containers) == 0 { | |
| if len(result.Items) == 0 { | |
| return errors.New("no fleet container found") | |
| } | |
| port := containers[0].Ports[0].PublicPort | |
| port := result.Items[0].Ports[0].PublicPort | |
| result, err := f.dockerClient.ContainerList(context.TODO(), dockerclient.ContainerListOptions{Filters: make(dockerclient.Filters).Add("name", containerName), All: true}) | |
| if err != nil { | |
| return err | |
| } | |
| if len(result.Items) == 0 { | |
| return errors.New("no fleet container found") | |
| } | |
| if len(result.Items[0].Ports) == 0 { | |
| return errors.New("fleet container has no published ports yet") | |
| } | |
| port := result.Items[0].Ports[0].PublicPort |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/upgrade/fleet_test.go` around lines 207 - 214, The code at the port
assignment (where result.Items[0].Ports[0].PublicPort is accessed) does not
check if the Ports slice contains any elements before indexing. Add a bounds
check after confirming the container exists to verify that
len(result.Items[0].Ports) is greater than zero before accessing Ports[0], and
return an appropriate error if no ports are available.
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
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.
Related issue: Cherry-pick of #47872 into
main.Originally merged into
rc-minor-fleet-v4.87.0. This brings the same CVE fixes ontomainso they're not lost when the RC is cut over.Checklist for submitter
For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
Chores
Tests