Replace replace-in-file package to resolve CWE-772 in inflight

[joserodriguez16]

replace-in-file uses glob ^8.1.0, which uses inflight which is a vulnerable package that doesn’t get updates. An issue on the replace-in-file repo has been open since March 11 of this year but the vulnerability has not been addressed. Therefore, it should be replaced. Inflight vulnerability details:

CWE-772
https://cwe.mitre.org/data/definitions/772.html

Snyk analysis
https://security.snyk.io/package/npm/inflight/1.0.6

[shaharkazaz]

@joserodriguez16 Why was this marked as completed?

[PeterHewat]

PR #693 removed unused dependencies (including replace-in-file)

[joserodriguez16]

@joserodriguez16 Why was this marked as completed?

Sorry I should've left a comment. replace-in-file was recently updated to v8 and the vulnerability was addressed. I was going to open another issue proposing an update to the dependency

[shaharkazaz]

@PeterHewat These dependencies are used by the schematics.

[skatterwe]

Is there anything that can be done to help out here? i suppose all that is needed is a version bump to v8 right?

[shaharkazaz]

@skatterwe The package is now ESM which means all the schematics need an upgrade to use it.
You are welcome to open a PR for it 🙌

[skatterwe]

@shaharkazaz if you guide me a bit on what i need to take care of in your pull requests etc i can surely have a look (commit message formates, branch name policies etc)

[skatterwe]

Ah found some hints in the contributing file. i'll see what i can do :)

[skatterwe]

@shaharkazaz as what would i commit this? i initially did as feature. but that somehow feels a bit weird. so before i'll raise a PR can you please let me know as what I should commit? Do you consider a version bump a refactor or a chore change or should I stick with feature?

[shaharkazaz]

@skatterwe hm... we can make it a fix I guess as this isn't actually adding a feature.
We need to understand if this is somehow a breaking change it might be sue to the minimum node version needed to run the schematics no?

[skatterwe]

@shaharkazaz yeah I was also wondering about the question with the breaking change. I tried linking the dist from my fork within some of my projects and it worked all fine (without any changes) but I remember too well the flat update and how that caused us to update the tests later on 😅

but is fix with a breaking change something you consider ok commit wise? feels a bit strange. just let me know what you think best, then I'll create a new branch with the commit settings you would like :)

[shaharkazaz]

@skatterwe My suspect is the minimum node version required.
Currently, the minimum Angular version is 16 which supports node v16, I vaguely remember that you need node 18 something for the ESM support but I'm not sure. If there is a breaking change that would be it.

Regarding the commit type, let's go with feature.

[skatterwe]

@shaharkazaz sorry was away. Just looking into it again. Reading from this: https://nodejs.medium.com/announcing-core-node-js-support-for-ecmascript-modules-c5d6dc29b663 the node version itself should be able to support ESM. I've tried running your tests with node 16 (to be precise: v16.20.2). That fails (not just on my branch with the replace-in-file update but also on master) because NX is using some internal functions that are not available in node 16.

       NX   (0 , node_os_1.availableParallelism) is not a function

any other suggestions how to double check that node 16 will work?

[abdallahbedir2]

Please stop using inflight

npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.

[stefanello57]

Any updates on this?

[skatterwe]

I need more information how to test if its a breaking change. just switching the node version in that project somewhat didn't reveal much. other than that there is a branch ready to raise a pr

@shaharkazaz can you please advise me on how to best check if there is a breaking change. #768 (comment) this was my initial try

[shaharkazaz]

@skatterwe Transloco is requiring Angular v16 which support node versions: ^16.14.0 || ^18.10.0. if we can't guarantee that node 16.14 works this is a breaking change as we need to set the minimum node version higher than what Angular supports by default.

Sorry for the delayed replies, I generally haven't been much around in Tranlsoco due to my reserve duty service.

[skatterwe]

@shaharkazaz yeah I understood that. I was wondering if you have any other idea on how to verify if it works with node 16.14.0.

from the references found it should not be breaking, but i would like to verify and failed to do so so far.

[shaharkazaz]

@skatterwe The easiest way is to create a new Angular workspace and use node v16.14 to create it. Personally I use nvm to manage my active node version which makes it pretty easy to switch back and forward.