Fix npm vulnerabilities #8091

pajakd · 2025-12-05T11:30:54Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

In our internal mirror of Kueue repository, any build time dependency installation (npm install ${dep}) is flagged as insecure -- there are two such lines in kueueviz frontend. This PR is an attempt to fix those.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

netlify · 2025-12-05T11:30:59Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`6ffc744`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6932ea60d830950008f9d555

k8s-ci-robot · 2025-12-05T11:31:00Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pajakd
Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mimowo · 2025-12-05T11:41:59Z

In our internal mirror of Kueue repository, any build time dependency installation (npm install ${dep}) is flagged as insecure -- there are two such lines in kueueviz frontend. This PR is an attempt to fix those.

I see, so IIUC the scanner scans the dependencies which are put locally in the node_modules, and by this trick you avoid them being scanned. Is this correct, or more nuanced?

Assuming this is correct the alternatives I see:

disable the folder in the configuration of the internal scanner
open issues for the libraries and wait for resolutions

(2.) might be tedious on our side if we are not using KueueViz internally yet.
(1.) might be preferred over the proposal here, because if other companies have scanners working similarly, and they are using KueueVIz, then it might be beneficial for them to see the scanner reports, and they could proceed with (2.).

cc @kannon92 @tenzen-y

mbobrovskyi · 2025-12-05T11:42:55Z

cmd/kueueviz/frontend/node_packages/package.json

Why not add this package to the base package.json?

The reason is that it is imported with the -g flag -- so it should be available system-wide.

The -g flag installs it globally, so you can use serve (or /usr/local/bin/serve) as a binary. But it doesn’t matter where you run it.

pajakd · 2025-12-05T11:51:57Z

I see, so IIUC the scanner scans the dependencies which are put locally in the node_modules, and by this trick you avoid them being scanned. Is this correct, or more nuanced?

@mimowo To clarify -- there is no issue with any of the libraries. The thing is that each installed library must be present in the package.json and the package-lock.json

So, what is flagged as insecure is any explicit npm install some_package in any Dockerfile or script

mimowo · 2025-12-05T11:55:52Z

I see, so rather than calling npm install some_package can we just declare some_package in the dependencies?

For example here https://github.com/kubernetes-sigs/kueue/pull/8091/files#diff-afa357502d22dc9bd50e3995c9034a3524903909ba97e5c28e7d86488c5459daL41 we could declare in the package.json cypress as a devDependency and then the npm install would install it (iiuc).

mbobrovskyi · 2025-12-05T12:00:12Z

cmd/kueueviz/frontend/Dockerfile

+COPY ./node_packages/package-lock.json /usr/local/node_packages/
+WORKDIR /usr/local/node_packages/
+RUN npm install
+ENV PATH="/usr/local/node_packages/node_modules/.bin:${PATH}"


I think we can add it on scripts and run it like npm run serve.

tenzen-y · 2025-12-05T12:28:37Z

I see, so rather than calling npm install some_package can we just declare some_package in the dependencies?

For example here https://github.com/kubernetes-sigs/kueue/pull/8091/files#diff-afa357502d22dc9bd50e3995c9034a3524903909ba97e5c28e7d86488c5459daL41 we could declare in the package.json cypress as a devDependency and then the npm install would install it (iiuc).

+1 Agree

pajakd · 2025-12-05T12:30:51Z

/test all

mbobrovskyi · 2025-12-05T12:30:59Z

cmd/kueueviz/frontend/package.json

    "preview": "vite preview",
-    "check-unused": "depcheck"
+    "check-unused": "depcheck",
+    "serve": "npm run serve"


Probably you also need to update start.sh.

Or another alternative. You can copy binary from node_modules from previous stage.

Is it working without start.sh changes?

The e2e tests are passing. But do they use the start.sh script? Probably not, so I have to test on the real cluster, right?

Yeah, please 🙏

@pajakd @mbobrovskyi it seems this is the last pending thing to do, is it clear what / how to test?

@mimowo yes, sorry for the delay. I'll test it tomorrow.

EDIT: or some time this week

mbobrovskyi · 2025-12-05T12:31:16Z

cmd/kueueviz/frontend/package.json

    "depcheck": "^1.4.7",
-    "vite": "^7.2.4"
+    "vite": "^7.2.4",
+    "serve": "^14.2.5"


Serve should be dependency.

pajakd · 2025-12-05T14:05:26Z

I see, so rather than calling npm install some_package can we just declare some_package in the dependencies?
For example here https://github.com/kubernetes-sigs/kueue/pull/8091/files#diff-afa357502d22dc9bd50e3995c9034a3524903909ba97e5c28e7d86488c5459daL41 we could declare in the package.json cypress as a devDependency and then the npm install would install it (iiuc).

+1 Agree

Actually, its already there so I claim that this npm install is unnecessary.

mimowo · 2025-12-05T14:10:50Z

sgtm

mbobrovskyi · 2025-12-05T14:14:05Z

cmd/kueueviz/frontend/package-lock.json

Please sync this file with cmd/kueueviz/frontend/package.json.

Fix vulns

a9194d8

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. release-note-none Denotes a PR that doesn't merit a release note. labels Dec 5, 2025

k8s-ci-robot requested review from akram and gabesaba December 5, 2025 11:30

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 5, 2025

mbobrovskyi reviewed Dec 5, 2025

View reviewed changes

pajakd marked this pull request as draft December 5, 2025 12:03

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2025

Address comments

cfb9e83

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 5, 2025

mbobrovskyi reviewed Dec 5, 2025

View reviewed changes

Address comments

fc76e15

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 5, 2025

pajakd marked this pull request as ready for review December 5, 2025 12:34

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2025

k8s-ci-robot requested a review from tenzen-y December 5, 2025 12:34

Lock update

937c08f

Update lockfile

d9529d1

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 5, 2025

mbobrovskyi reviewed Dec 5, 2025

View reviewed changes

Update lock files

6ffc744

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 5, 2025

Fix npm vulnerabilities #8091

Are you sure you want to change the base?

Fix npm vulnerabilities #8091

Conversation

pajakd commented Dec 5, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

netlify bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Uh oh!

k8s-ci-robot commented Dec 5, 2025

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pajakd commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mimowo commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tenzen-y commented Dec 5, 2025

Uh oh!

pajakd commented Dec 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbobrovskyi Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pajakd Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pajakd commented Dec 5, 2025

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

mbobrovskyi Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

netlify bot commented Dec 5, 2025 •

edited

Loading

pajakd commented Dec 5, 2025 •

edited

Loading

mimowo commented Dec 5, 2025 •

edited

Loading

mbobrovskyi Dec 5, 2025 •

edited

Loading

pajakd Dec 8, 2025 •

edited

Loading

mbobrovskyi Dec 5, 2025 •

edited

Loading