Bumping deps to avoid CVE (21/09/2025) #9224
Conversation
WalkthroughBumped numerous runtime and dev dependencies in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
package.json (1)
1-147: Add engines and CI CVE gate to make the bump defensibleTo lock runtime expectations and enforce CVE intent, consider:
- Declaring supported Node versions via "engines".
- Adding a lightweight CVE gate in CI (npm audit --omit=dev && osv-scanner).
Proposed edits:
{ "name": "noobaa-core", "version": "5.21.0", + "engines": { + "node": ">=20.10 <23" + }, "license": "SEE LICENSE IN LICENSE",Add to CI a job step:
npm ci npm audit --omit=dev --audit-level=high || exit 1 # Optional if available in your pipeline: # osv-scanner -r .Tune the node range to your actual prod baseline.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Noobaa Image
- GitHub Check: run-jest-unit-tests
- GitHub Check: run-package-lock-validation
🔇 Additional comments (5)
package.json (5)
80-83: Azure SDK bumps look fine; verify auth flows in CI/prodIdentity/Monitor/Blob minor bumps are usually safe, but token cache and retry policy defaults occasionally shift.
Please run smoke tests that cover:
- ManagedIdentityCredential in your deployment env
- Blob uploads with customer-provided headers
93-93: express-http-proxy patch bump — safeNo API changes expected; good to take for CVE hygiene.
129-145: Align @types/node, Jest 30, and node-gyp with your runtime & CI
- package.json shows @types/node 22.18.6 (also Jest 30.1.3 and node-gyp 11.4.2); automated search did not find an "engines" field, any setup-node node-version/matrix in .github/workflows, or a Dockerfile FROM node: — confirm the actual runtime Node version (e.g., 18/20/22) and either set engines or pin @types/node to that major.
- Jest 30 changes defaults — verify your jest.config (transforms, ESM interop) and run tests locally/CI before merge.
- node-gyp 11 may require newer Python/build toolchain in CI — ensure CI images/install steps (python, make/build-essentials) are updated and that native builds pass.
113-113: node-rdkafka 3.5.0 — ensure CI has librdkafka headers + node-gyp toolchain or run npm install in the builder image
- Checked: .nvmrc = 22.11.0; workflows use Node 22 (.github/workflows/Validate-package-lock.yaml, .github/workflows/releaser.yaml) — Node version is consistent.
- Builder image installs python3.9 and "Development Tools" (gcc/make) in src/deploy/NVA_build/builder.Dockerfile; Base.Dockerfile runs npm install in that image and strips node-rdkafka/deps afterward — image builds have the toolchain.
- Gap: Validate-package-lock job runs npm install on ubuntu-latest but does not install librdkafka-dev or explicit build deps — npm install there can fail when node-rdkafka needs native build. (See: .github/workflows/Validate-package-lock.yaml).
- Action (pick one): either (A) add OS build deps to Validate-package-lock before npm install (Ubuntu example: apt-get update && apt-get install -y build-essential python3 pkg-config libssl-dev librdkafka-dev) or (B) run the npm install step inside the builder image that already installs Development Tools (src/deploy/NVA_build/builder.Dockerfile / src/deploy/NVA_build/Base.Dockerfile).
101-101: ldapts 8.0.9 — OK: CJS require works, Node >=20 satisfied, API compatible
- src/util/ldap_client.js uses require('ldapts') and calls new Client(...), bind/unbind and search — ldapts v8 provides both ESM and a CJS
requireentry and the Client API (Client constructor, bind/unbind, search -> { searchEntries }) matches these calls. (npmjs.com)- npm view (ran in the repo) shows engines = { node: '>=20' } and repo .nvmrc = 22.11.0 — Node requirement is satisfied; ensure CI/runtime use Node >=20.
package.json
Outdated
| "@google-cloud/storage": "7.17.1", | ||
| "@smithy/node-http-handler": "3.3.3", |
There was a problem hiding this comment.
🧩 Analysis chain
Potential ESM-only library upgrade (@google-cloud/storage v7) — confirm import style and Node version
v7 of @google-cloud/storage is ESM-first. If your codebase is CommonJS, require() will fail. Ensure all imports use ESM (import { Storage } from '@google-cloud/storage') or dynamic import in CJS, and that runtime Node version supports it.
Quick checks:
If still on CJS at runtime, migrate the import sites or gate this bump.
🏁 Script executed:
#!/bin/bash
# Find CJS-style requires that will break
rg -nP "require\\(['\"]@google-cloud/storage['\"]\\)"
# Find Node version hints
rg -nP "\bFROM node:|setup-node|\.nvmrc|\.tool-versions|\"engines\"\\s*:" -C2 .github workflows Dockerfile* **/.nvmrc **/.tool-versions package.jsonLength of output: 1627
Blocking: @google-cloud/storage v7 is ESM-first — CommonJS requires found; update imports or pin v6
Found CJS requires that will break if v7 is ESM-only:
- src/util/google_storage_wrap.js:6 — const { Storage } = require('@google-cloud/storage');
- src/tools/diagnostics/analyze_resource_gcp.js:5 — const { Storage } = require('@google-cloud/storage');
CI uses Node 22 (.github/workflows/Validate-package-lock.yaml); verify runtime/deploy Node version. Fixes: migrate these files to ESM imports (set "type":"module" or use .mjs), switch to dynamic import() and adapt async flow, or pin @google-cloud/storage to v6 until migration is complete.
🤖 Prompt for AI Agents
In package.json around lines 83-84 and the two files
src/util/google_storage_wrap.js and
src/tools/diagnostics/analyze_resource_gcp.js, the project currently uses a
CommonJS require for @google-cloud/storage but v7 of that package is ESM-first;
either pin the package to the latest v6 release in package.json (set
"@google-cloud/storage": "6.x" and run npm install) OR migrate the two files to
ESM by renaming to .mjs or setting "type":"module" in package.json and replacing
the require with an ESM import (import { Storage } from '@google-cloud/storage')
or, if you must stay CJS per-file, use a dynamic import (const { Storage } =
await import('@google-cloud/storage')) and update surrounding code to handle the
async import; choose one approach, apply it consistently, and run CI to verify
Node runtime compatibility.
liranmauda
force-pushed
the
liran-bump-deps
branch
from
d37d475 to
7aa584e
Compare
- Bumping deps to avoid CVE (21/09/2025) Signed-off-by: liranmauda <liran.mauda@gmail.com>
liranmauda
force-pushed
the
liran-bump-deps
branch
from
7aa584e to
8313d8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
93-93: express-http-proxy upgrade — confirm behavior and CVE target.src/server/web_server.js:129–131 — express-http-proxy is used with no options (defaults applied).
- Streaming: proxy requests/responses are piped/streamed by default; streaming is only disabled when using userResDecorator/userResHeaderDecorator/maybeSkipToNext. (github.com)
- Body parsing/buffering: parseReqBody defaults to true (request bodies are parsed/buffered); set parseReqBody:false to avoid buffering large uploads. (github.com)
- Timeouts: no timeout is set by default — use the timeout option to enforce one (otherwise the proxy may wait indefinitely). (github.com)
- Header rewriting: header changes are opt‑in via proxyReqOptDecorator/userResHeaderDecorator/preserveHostHdr and are not used in these routes. (github.com)
- CVE: no direct CVE recorded for express-http-proxy; npm shows v2.1.2 and Snyk reports no direct vulnerabilities. (npmjs.com)
Recommendation: confirm metrics endpoints are fine with the defaults, or set explicit options on the three proxy routes (e.g. { preserveHostHdr: true, timeout: , parseReqBody: false }).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ceph-s3-tests / Ceph S3 Tests
🔇 Additional comments (6)
package.json (6)
84-84: Smithy handler skew resolved.Previously flagged mismatch is fixed by bumping to 4.2.1. LGTM.
113-113: Confirm CI/images install compatible librdkafka & native build toolchain for node-rdkafka 3.5.0Couldn't find Dockerfile(s) or .github/workflows in this checkout (rg returned "No such file or directory") and
npm lsreturned an empty tree. Verify your CI/Docker images install a compatible librdkafka + headers and the native build toolchain (gcc/clang, make, python for node-gyp, pkg-config, node-gyp), and confirmnpm rebuild node-rdkafka --build-from-source(or full CI build) succeeds on your project's Node version. Check Dockerfile(s) at repo root, .github/workflows/*, and package.json (node-rdkafka: "3.5.0").
101-101: ldapts v8 upgrade — verify API & runtime compatibilityrequire('ldapts') is used in src/util/ldap_client.js; CI/local Node is v24.3.0. Smoke-test bind + search flows and confirm ldapts v8 breaking changes (Node engine, TLS/auth behavior, client init/bind/search signatures, error/result handling); update src/util/ldap_client.js if needed.
75-79: AWS SDK v3 versions aligned — good.Verification incomplete:
npm lsreturned an empty dependency tree in this environment. Run locally (after installing deps) and paste the outputs of:
npm ci
npm ls @aws-sdk/client-s3 @aws-sdk/client-sts @aws-sdk/credential-providers @aws-sdk/lib-storage @aws-sdk/s3-request-presigner
npm ls @smithy/node-http-handler @smithy/protocol-http @smithy/types
83-83: Blocking: @google-cloud/storage v7 is ESM-first — CJS require() call sites will breakFound CJS requires for @google-cloud/storage in:
- src/util/google_storage_wrap.js (const { Storage } = require('@google-cloud/storage'))
- src/tools/diagnostics/analyze_resource/analyze_resource_gcp.js (const { Storage } = require('@google-cloud/storage'))
package.json has no "type": "module". Either pin to 6.x in this PR or convert those files to ESM/dynamic import.
Suggested safe pin for this PR:
- "@google-cloud/storage": "7.17.1", + "@google-cloud/storage": "6.x",
129-146: Sanity-check Jest 30, Node version, and native buildsQuick findings: package-lock contains @jest/@30; package.json devDeps include jest@30 and node-gyp@11; .github/workflows/Validate-package-lock.yaml sets node-version: 22; releaser/run-pr workflows reference .nvmrc but a quick scan did not find .nvmrc or jest.config. in the branch.
Actionable checks:
- Verify jest config/loaders (jest.config.* or package.json "jest") are compatible with Jest 30 and your ESM/CJS setup.
- Ensure CI/test/build Node version matches expectations (Validate-package-lock uses Node 22) or add/commit .nvmrc; run tests under the Node versions you claim to support.
- Confirm native addons/node-gyp builds succeed on the target Node; add a Node-version test matrix if you need older Node support.
Locations: package.json (devDeps ~lines 129–146), package-lock.json (multiple @jest/*@30 entries), .github/workflows/Validate-package-lock.yaml, .github/workflows/releaser.yaml, .github/workflows/run-pr-tests.yaml.
2 participants
Explain the Changes
Summary by CodeRabbit