Repository navigation
fix: address website dependency vulnerabilities - #19814
Conversation
|
The failing |
There was a problem hiding this comment.
Pull request overview
Updates the documentation website’s npm dependency graph to remediate Dependabot-reported vulnerabilities by pinning select transitive packages via npm overrides and regenerating website/package-lock.json accordingly. This aligns the committed lockfile with a patched set of transitive versions without affecting Druid runtime or the web-console.
Changes:
- Add npm
overridesinwebsite/package.jsonto force patched transitive versions (brace-expansion,serialize-javascript,sockjs→uuid,tmp). - Refresh
website/package-lock.jsonto the updated resolved dependency graph. - Update website CI install step to use
npm ciin.github/scripts/web-checks.sh.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
website/package.json |
Adds targeted npm overrides to pin vulnerable transitive packages to patched versions. |
website/package-lock.json |
Regenerates the lockfile to reflect the overridden/patched transitive dependency graph. |
.github/scripts/web-checks.sh |
Switches website install step to npm ci for reproducible CI installs. |
Files not reviewed (1)
- website/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- website/package-lock.json: Generated file
Comments suppressed due to low confidence (1)
.github/scripts/web-checks.sh:30
npx --yes npm@10.8.2 ciinstalls the lockfile with the pinned npm version, but the subsequentnpm run ...commands will use whatevernpmis onPATH(currentlyweb-console's frontend-maven-plugin installs npm 10.9.0). For reproducibility and to keep the website build checks aligned with the lockfile generator, run the build/link-lint/spellcheck steps via the same pinned npm as well.
(cd website && npx --yes npm@10.8.2 ci)
cd website
npm run build
npm run link-lint
npm run spellcheck
FrankChen021
left a comment
There was a problem hiding this comment.
No actionable findings. Reviewed 3 of 3 changed files (.github/scripts/web-checks.sh, website/package.json, and website/package-lock.json) plus surrounding CI/dependency context; npm@10.8.2 CI dry-run passed and GitHub checks were green.
This is an automated review by Codex GPT-5.6-Sol
GWphua
left a comment
There was a problem hiding this comment.
LGTM with non-blocking comments
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 3 of 3 changed files. The global security override introduces a verified CommonJS API incompatibility with installed minimatch 3 consumers.
This is an automated review by Codex GPT-5.6-Sol
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- website/package-lock.json: Generated file
Suppressed comments (1)
.github/scripts/web-checks.sh:26
- Using
npx npm@10.8.2forces a network fetch of npm at runtime, which can make CI slower and introduce registry/network flakiness compared to invoking a pre-installed/pinned npm. Consider installing/pinning npm earlier in the job (or via the existing Node/npm provisioning step) and runningnpm cidirectly, or add a version assertion (fail fast if npm isn’t 10.8.2) to keep determinism without requiringnpxdownloads during the checks.
(cd website && npx --yes npm@10.8.2 ci)
50fbee4 to
8603c2b
Compare
8603c2b to
3779afe
Compare
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 3 of 3 changed files. The prior baseline was unavailable locally, so this was a full current-diff static review.
This is an automated review by Codex GPT-5.6-Luna(max)
FrankChen021
left a comment
There was a problem hiding this comment.
🟢 Approval recommended
No actionable issues found in this updated-head review. The incremental patch is primarily the PR branch's merge-from-master history; the current merge-base diff contains only the three website dependency/CI files. The prior brace-expansion/minimatch compatibility finding is resolved: the current lockfile keeps brace-expansion@1.1.18 for minimatch@3.1.5, while the remaining targeted overrides resolve to compatible versions.
Reviewed 3 of 3 changed files.
Validation: git diff --check e5208b3fd284d9d6c75236e73d28191e423c5873 77f4f3be953c0ce859c2726dea4c784bfc1d5b26 passed. A read-only JSON and lockfile dependency-consistency check also passed. No builds, tests, dependency installs, formatters, or broad scans were run.
This is an automated review by Codex GPT-5.6-Luna(max)
FrankChen021
left a comment
There was a problem hiding this comment.
🟢 Approval recommended
No actionable findings in this updated-head review. The incremental patch only removes the temporary npx --yes npm@10.8.2 invocation; the final head's .github/scripts/web-checks.sh matches the merge base, so it is not part of the current net PR diff. The current merge-base diff contains only the website dependency manifest and lockfile.
The prior [P2] brace-expansion/minimatch compatibility finding remains resolved: the current manifest has no global brace-expansion v5 override, and the lockfile keeps brace-expansion@1.1.18 for minimatch@3.1.5. The serialize-javascript@7.0.5, sockjs-scoped uuid@11.1.1, and tmp@0.2.7 overrides are reflected consistently in the current lockfile.
Reviewed 2 of 2 current changed files (website/package.json and website/package-lock.json), plus the incremental CI edit, surrounding CI usage, and the frontend Maven plugin configuration. Validation: git diff --check passed for both the full merge-base-to-head diff and the incremental previous-review-to-head diff. No builds, tests, dependency installs, formatters, or broad scans were run.
This is an automated review by Codex GPT-5.6-Luna(max)
Dependency
Depends on #19788 and must be merged after #19788.
This is a stacked PR based on #19788 head commit
65630f6ab7100e1e82f160054d90dd502a2d1b7b. Its website dependency changes assume the Docusaurus and Node.js updates from that PR.Changes
brace-expansion5.0.8serialize-javascript7.0.5sockjs'suuid11.1.1tmp0.2.7website/package-lock.jsonwith the repository-pinned npm 10.8.2.This covers all 50 currently open Dependabot alerts for
website/package-lock.json, spanning 27 packages. A range-by-range check against the GitHub alert metadata found zero installed versions within any reported vulnerable range.The change only affects the documentation website dependency graph. It does not change Druid runtime or web-console dependencies.
Validation
npx --yes npm@10.8.2 cinpm ls --all --omit=optionalnpm run buildnpm run link-lintnpm run spellcheck(264 files)The production build succeeds with the existing broken-anchor warnings.
Attribution
This pull request was created by GPT-5.6-Sol.