Skip to content

Conversation

@liranmauda
Copy link
Contributor

@liranmauda liranmauda commented Dec 10, 2025

Explain the Changes

  • Bumping deps to avoid CVE (10/12/2025)

Summary by CodeRabbit

  • Chores
    • Routine dependency updates across cloud SDKs, storage clients, web framework, authentication libraries, LDAP tooling, database utilities, native bindings, messaging client, YAML parser, and TypeScript typings. No changes to public APIs, features, or project structure; no user-facing behavior altered.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Walkthrough

This PR updates dependency versions in package.json, bumping multiple AWS SDK v3 packages and several other libraries (Google Cloud Storage, aws-sdk v2, express, jsonwebtoken, ldapts, nan, node-rdkafka, yaml, and @types/node). No source-code or structural changes.

Changes

Cohort / File(s) Change Summary
Dependency version bumps
package.json
Updated dependency and devDependency versions (minor/patch): @aws-sdk/* (client-s3, client-sts, credential-providers, lib-storage, s3-request-presigner, client-iam), @google-cloud/storage, aws-sdk (v2), express, jsonwebtoken, ldapts, mongo-query-to-postgres-jsonb, nan, node-rdkafka, yaml, @types/node, plus various runtime/dev helpers (ping, prom-client, ps-node, seedrandom, setimmediate, @types/pg, test tooling). No code or structural changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify consistency across @aws-sdk/* version ranges.
  • Spot-check changelogs for ldapts, express, node-rdkafka, and nan for any behavior-important minor changes.
  • Ensure dev tooling version updates (types, test libs) don't break CI.

Possibly related PRs

Suggested reviewers

  • nimrod-becker
  • jackyalbo
  • dannyzaken

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly indicates the main change: dependency updates to address security vulnerabilities (CVE). It matches the raw summary showing multiple package.json dependency updates and aligns with the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e0d37a and ddabbe5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json
⏰ 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: run-jest-unit-tests
  • GitHub Check: Build Noobaa Image
  • GitHub Check: run-package-lock-validation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b676c6 and 1d5fad2.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Neon-White
Repo: noobaa/noobaa-core PR: 9229
File: .github/workflows/ibm-nightly-provision-dispatcher.yaml:13-13
Timestamp: 2025-09-30T08:56:55.478Z
Learning: In the noobaa-core repository, PR #9229 (nightly IBM VM provision dispatcher) has a dependency on `.github/ibm-warp-runner-config.yaml` which is provided in PR #9230, requiring PR #9230 to be merged first.
📚 Learning: 2025-09-30T10:27:57.996Z
Learnt from: Neon-White
Repo: noobaa/noobaa-core PR: 9230
File: tools/ibm_runner_helpers/slack_notifier.js:4-4
Timestamp: 2025-09-30T10:27:57.996Z
Learning: In the noobaa-core repository, the `tools/ibm_runner_helpers/slack_notifier.js` script depends on `slack/webhook`, which is installed globally via `npm -g install 'slack/webhook'` in `.github/ibm-warp-runner-config.yaml` (cloud-init configuration) rather than being declared in package.json, since the script runs on IBM Cloud VMs provisioned by cloud-init.

Applied to files:

  • package.json
⏰ 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-package-lock-validation
  • GitHub Check: run-jest-unit-tests
🔇 Additional comments (3)
package.json (3)

111-113: Verify native addon build compatibility.

The update to [email protected] and [email protected] (native modules requiring compilation) should be tested to ensure:

  1. The npm run build:native process completes successfully
  2. No ABI incompatibilities are introduced with the Node.js version in use
  3. node-rdkafka's system-level Kafka dependencies remain compatible

74-146: Ensure package-lock.json is updated.

When updating dependencies in package.json, package-lock.json must be regenerated to reflect the new versions and any transitive dependency changes. Please confirm:

  1. That npm ci or npm install has been run to update the lockfile
  2. That the lockfile changes only reflect the intended updates (no unexpected transitive dependency changes)
  3. That CI/CD pipelines have validated the updated dependencies work together

75-135: Verify which CVEs motivated these dependency updates and confirm breaking change compatibility.

This PR bumps 15 dependencies without specifying which CVEs are being patched or which packages are affected by them. Based on available CVE data:

  • AWS SDK v3 (3.948.0) shows no public security-fix entry; the Java SDK has CVE-2025-24970 (Netty), not the JavaScript SDK
  • jsonwebtoken 9.0.3 addresses CVEs from 2022, not recent ones
  • Express 4.22.1 reverted a disputed CVE-2024-51999 that was rejected (no actual vulnerability)
  • yaml 2.8.2 and ldapts 8.0.18 have no known public CVEs

Before merging, please clarify:

  1. Which specific CVE(s) triggered these bumps and which packages are affected?
  2. Have you tested for breaking changes, especially in Express 4.22.1 and AWS SDK v3 compatibility?
  3. Are all version bumps security-driven, or are some routine maintenance updates?

@liranmauda liranmauda force-pushed the liran-bump-deps branch 5 times, most recently from 4d4fcb7 to 7e0d37a Compare December 11, 2025 10:12
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
package.json (1)

92-92: Express 4.22.1 compatibility already verified in prior review.

The bumping of Express to 4.22.1 (line 92) was previously flagged and verified. Per the past review comment (lines 92–92), Express 4.22.1 reverts a breaking change from v4.22.0 related to the extended query parser. The concern has been addressed in commits 906b072 to ca2563c. No further action required.

🧹 Nitpick comments (1)
package.json (1)

75-79: Verify AWS SDK v3 3.948.0 compatibility with existing codebase.

Multiple AWS SDK v3 packages are synchronized to 3.948.0 (clients, credential-providers, lib-storage, s3-request-presigner), a jump of ~11–12 patch versions from 3.936.x/3.937.x. Verify that:

  1. No breaking changes in the AWS SDK changelog between current and target version.
  2. Integration tests pass with the new SDK version, especially those exercising S3, STS, and IAM operations.

Also applies to: 131-131

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8343689 and 7e0d37a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T10:27:57.996Z
Learnt from: Neon-White
Repo: noobaa/noobaa-core PR: 9230
File: tools/ibm_runner_helpers/slack_notifier.js:4-4
Timestamp: 2025-09-30T10:27:57.996Z
Learning: In the noobaa-core repository, the `tools/ibm_runner_helpers/slack_notifier.js` script depends on `slack/webhook`, which is installed globally via `npm -g install 'slack/webhook'` in `.github/ibm-warp-runner-config.yaml` (cloud-init configuration) rather than being declared in package.json, since the script runs on IBM Cloud VMs provisioned by cloud-init.

Applied to files:

  • package.json
⏰ 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-package-lock-validation
  • GitHub Check: run-jest-unit-tests
🔇 Additional comments (3)
package.json (3)

137-137: Clarify @types/pg addition and its relationship to the CVE fix.

The entry "@types/pg": "8.16.0" appears to be newly added to devDependencies (line 137). Verify:

  1. Whether this addition is intentional and part of the CVE fix or a separate improvement.
  2. Whether pg version 8.16.3 (line 114) requires corresponding type definitions.
  3. No conflicts with other TypeScript type definitions in the project.

75-137: Provide CVE details and confirm all tests pass before merging.

The PR title states "Bumping deps to avoid CVE (10/12/2025)" but the PR description and context provide no specific CVE identifier (e.g., CVE-2025-XXXXX) or advisory link. Additionally, no test results are shown. Before merging, ensure:

  1. CVE Details: Link to the specific CVE advisory or security bulletin that these version bumps address, confirming that the chosen versions actually fix the vulnerability.
  2. Test Coverage: Run and verify all test suites pass:
    • Unit tests (npm run mocha)
    • Integration tests with HTTP/S3/LDAP/PostgreSQL operations
    • Jest tests (npm run jest)
    • TypeScript compilation (npm run ts)
    • Native build (npm run build:native)
  3. CI Pipeline: Ensure the PR branch passes all CI checks (linting, tests, build).

101-101: ldapts version 8.0.19 does not exist—pin to 8.0.18 or verify the intended version.

ldapts 8.0.19 is not published on npm or any public registry. The latest available version is 8.0.18. Update the dependency to a valid, published version.

Likely an incorrect or invalid review comment.

- Bumping deps to avoid CVE (11/12/2025)

Signed-off-by: liranmauda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants