IBM / node-sdk-core

The node-sdk-core repository contains core functionality required by code generated by the IBM OpenAPI SDK Generator.
Apache License 2.0
20 stars 26 forks source link

move some dependencies to devdependencies #266

Open huineng opened 4 months ago

huineng commented 4 months ago

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 4 months ago

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.

dpopp07 commented 4 months ago

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 2 weeks ago

@dpopp07

  `-- @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 2 weeks ago

@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 2 days ago

@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.