Peer-dependency type package causing node engine error

[TarikGul]

Currently the @edgeware/node-types package is causing issues when trying to download @polkadot/apps-config when using a node version above 14.0.0. PR to fix it.

Rel: #601.

Steps to reproduce:

First make sure your node version is above v14.0.0 (I use nvm ex: nvm use 16.4.0)

$ mkdir new_empty_dir && cd new_empty_dir
$ yarn add @polkadot/apps-config

I would also love to open up some dialog in terms of third-party packages, and would love to contribute a PR or even maybe some docs that could help mitigate future issues that come through third party type packages. To further explain (it might be better to bring this conversation in a new issue but anyways...), recently there was a patch to update the @darwinia/types which introduced some breaking changes to their imports, and also pushed the version out of alpha. That day there seemed to be some issue with npm (I am still not totally sure what the issue was, I am waiting for a response from npm) where the registry was resolving @darwinia/types@1.1.0-alpha.22 to a stable release @darwinia/types@1.1.2. The scary thing is it did this for every release we had from when the types were introduced into apps-config in February to now. That meant every version of substrate-api-sidecar would successfully build but fail to launch locally because of the breaking changes introduced in @darwinia/types (even though it wasnt yet applied in sidecar or polkadot-js).

What really worried me about this, was within that 24hr period, @darwinia/types could have published any malicious code into their package, revert the breaking changes, and make a new release and every single @polkadot/apps-config that was pulled from the registry that day would have had the injected malicious code and no one would have known.

After that 24hr period the bug went away, and seemed to be patched by npm internally but I wasnt able to find any information about it. This also makes it terribly hard/impossible to reproduce. I did have my teammates also reproduce the bug when it was reproducible, and each of them had the same issue show up.

To sum it all up, I know it's very hard to audit all the third party type packages, and if you have any thoughts or guidelines you would like to further deploy I would love to gladly help out. Anything to increase our chances of being more secure, but also to introduce less peer-dependency bugs that may happen.

[jacogr]

I actually do check all the 3rd party packages, by hand, e.g. reading the code as published, each time I update their references to a newer version. Basically once a week when I update them in the apps UI, or when the maintainers bump the packages as well. (Sometimes a week becomes 2...)

But yes, in the above case, that would have helped the apps UI, but not the actual people using @polkadot/apps-config.

Maybe for the above it would help to remove any ^ or ~ in apps-config (doesn't make a difference in the upgrades to the deps, it is done via yarn upgrade-interactive)

[TarikGul]

Thanks for the reply.

I actually do check all the 3rd party packages, by hand, e.g. reading the code as published,

Ahh nice, yea i assumed you probably check each one pretty thoroughly. That's quite a lot of work, so thanks for that!

That being said, after doing some more digging into the edge case with NPM above, it seems to me that it was an incredibly rare and unpredictable situation that caused that issue, but also an issue directly related to npm-registry internal logic which is unfortunately out of our hands.

The more I think about it as well, it seems like this is also an issue outside of the scope of polkadot-js in terms of vulnerabilities. The obvious quick fix I think off in my head would be to not allow beta dependencies to make sure the npm-registry cant resolve the version up or down by mistake. But then I think that's also a bad solution to a rare problem.

Maybe for the above it would help to remove any ^ or ~ in apps-config (doesn't make a difference in the upgrades to the deps, it is done via yarn upgrade-interactive)

Makes sense. We are using yarn upgrade-interactive as well, so it doesnt make a difference for sure.

[jacogr]

I'll play with removing all the ^ this weekend and then updating - if that works perfectly, I do think it is a much safer approach to actually follow here anyway.

[polkadot-js-bot]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.