IBM/node-sdk-core

move some dependencies to devdependencies

huineng opened this issue · 6 comments

huineng commented

Although this is not a big deal , there are a few packages currently reported as dependency, that rather should be dev dependencies as they are mostly used during build phase and not at run time

"dependencies": {
    "@types/debug": "^4.1.12",
    "@types/file-type": "~5.2.1",
    "@types/isstream": "^0.1.0",
    "@types/node": "~10.14.19",
    "@types/tough-cookie": "^4.0.0",
    "expect": "^26.1.0"
  },

thanks

dpopp07 commented

Thanks for the issue @huineng

expect should not be included here because it is part of the exposed API that provides helpers for unit tests of depending SDKs.

For the others, we'll look at moving them.

  • 👍1
dpopp07 commented

After further investigation, our public API includes types from "@types/debug", "@types/node", and "@types/tough-cookie". In order to best support TS users, I think we want to keep them in "dependencies" so that those types are available.

The "@types/file-type" and "@types/isstream" packages will be moved to "devDependencies". Thanks again.

mpawlow commented

@dpopp07

  • Question: Is there any update on removing expect from the runtime dependencies?
  • Problem: Our Security tools are picking up and flagging micromatch and braces with a vulnerability (see details below) that needs to be remediated
  • Assumption: devDependencies are excluded from the OSS scans
  `-- @ibm-cloud/cloudant@0.9.1
    `-- ibm-cloud-sdk-core@4.3.0
      `-- expect@27.5.1
        `-- jest-message-util@27.5.1
          `-- micromatch@4.0.5

 `-- @ibm-cloud/cloudant@0.9.1
   `-- ibm-cloud-sdk-core@4.3.0
     `-- expect@27.5.1
       `-- jest-message-util@27.5.1
         `-- micromatch@4.0.5
           `-- braces@3.0.2

[PSIRT][PVR0511742] micromatch-4.0.5.tgz (Publicly disclosed vulnerability found by Mend)

Filename: micromatch-4.0.5.tgz
Artifact ID: micromatch-4.0.5.tgz
Group ID: micromatch
Name: micromatch
Version: 4.0.5
Description: Glob matching for javascript/node.js. A replacement and faster alternative to minimatch and multimatch.
Type: NODE_PACKAGED_MODULE

Mend References (at the time of ADV creation):
https://www.mend.io/vulnerability-database/CVE-2024-4067
https://nvd.nist.gov/vuln/detail/CVE-2024-4067

CVE Details

CVEID: CVE-2024-4067
Description: Node.js micromatch module is vulnerable to a denial of service; caused by a regular expression denial of service (ReDoS) flaw in micromatch.braces() in index.js. By sending a specially crafted payload; a remote attacker could exploit this vulnerability to increase the consumption time until the application hangs or slows down.
CVSS Base Score: 7.5
CVSS Temporal Score: https://exchange.xforce.ibmcloud.com/vulnerabilities/290676 for more information
CVSS Vector: (CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H)

[PSIRT][PVR0511484] braces-3.0.2.tgz (Publicly disclosed vulnerability found by Mend)

Filename: braces-3.0.2.tgz
Artifact ID: braces-3.0.2.tgz
Group ID: braces
Name: braces
Version: 3.0.2
Description: Bash-like brace expansion, implemented in JavaScript. Safer than other brace expansion libs, with complete support for the Bash 4.3 braces specification, without sacrificing speed.
Type: NODE_PACKAGED_MODULE

Mend References (at the time of ADV creation):
https://www.mend.io/vulnerability-database/CVE-2024-4068
https://nvd.nist.gov/vuln/detail/CVE-2024-4068

CVE Details

CVEID: CVE-2024-4068
Description: Node.js braces module is vulnerable to a denial of service; caused by the failure to limit the number of characters it can handle. leading to a memory exhaustion in lib/parse.js. By sending imbalanced braces as input; the parsing will enter a loop causing the JavaScript heap limit to be reached; and the program will crash.
CVSS Base Score: 7.5
CVSS Temporal Score: https://exchange.xforce.ibmcloud.com/vulnerabilities/290675 for more information
CVSS Vector: (CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H)
Affected Platforms:

ricellis commented

@dpopp07 I think it would be better to encourage the downstream SDKs to pick up test dependencies like expect through their own devDependencies block (i.e. add it to the template repo) rather than including them as dependencies of the core. I guess that might be a breaking change, but it would seem to be a lot cleaner than pulling in a bunch of test dependencies for all the SDK installs.

Whilst it would likely be cleaner to package the test helpers separately as proposed in #174 I think it is possible and potentially less disruptive (at least as a first step) to continue to package them with the core. I think this is possible with the dependencies "missing" since they are only referenced by the test code. As such the code paths that need those deps are never used at runtime. It would only cause build/test time problems for downstream SDKs that didn't update their devDependencies to include expect (and friends?).

FWIW I tried this out locally with our SDK and a modified core that has expect as a devDependency. The generated tests ran fine even without me adding expect to our devDependencies because it is also inlcuded by jest (which is already in the SDK template anyway).

dpopp07 commented

@ricellis thanks for the comment - that is some valuable insight. Sounds like we might be able to get away with removing this production level dependency without a breaking change due to the nature of the development dependencies in the template, etc.

I'll be investigating that soon.

ibm-devx-sdk commented

🎉 This issue has been resolved in version 4.3.2 🎉

The release is available on:

Your semantic-release bot 📦🚀