Winston users: please update to 3.3.4

[DABH]

Winston users: Malignant commits were pushed to a dependency of [email protected] (and probably affecting some lower versions too). This has been fixed in 3.3.4. This is the first release in quite some time, so if you are on 3.x but for some reason weren’t on 3.3.3, please take this opportunity to update to 3.3.4 (you should notice no incompatibilities, I think…). Thanks for understanding…

[DABH]

@wbt @fearphage @maverick1872 or others -- I'm seeing some test failures on the v3.3.4 commit that I am now able to reproduce on prior commits like 65ab472 . Any thoughts? I do rm -rf node_modules && npm i && npm test. Also happening on earlier commits like 0f8cf59 so I don't think it's something with new dependency versions. Any help figuring this out would be appreciated.

It seems like if I run the one failing test in isolation, it passes... 😕

[m1keil]

@DABH first of all, thank you very much for the prompt response to this manner. As many other I also stumbled upon this bug today. Luckily I did remember the the issue being mentioned on HN and got to the root cause pretty quickly (our dependency on winston logger and colors.js indirectly).

As someone who doesn't write JS everyday, any chance you can maybe explain to me the rational behind not pinning down external dependencies in projects like winston? While I can pin down winston, it seems like I don't have much control over the dependencies of winston itself.

This is not a critic of your decisions to not pin it, I'm genuinely asking.

Thank you again.

[DABH]

In the early days of Node (let’s say, the past decade or so), the attitude was “the community is working together, we are obeying semver, we should try to prevent dependency rot and use the latest versions of deps that we can.” So using things like ^1.4.0 in package.json was fine (particularly for repos like colors.js, which I was a maintainer on until I was removed as part of this incident) — it was an easy way to not have to update package.json just to get the latest bug fixes etc.

However, there have been several incidents like this in the past 1-3 years, and while those incidents are isolated, it’s painful enough to maybe change developers’ default stance to “only use exact/pinned versions.” It’s a little disappointing if that’s where we’re at in the Node ecosystem, but maybe that is the better approach going forwards (at least for packages outside the org).

[wuteng606]

here are some memory leak error

[wbt]

@m1keil I spent all day Thursday working to resolve some npm audit failures in one particular project, where vulnerabilities had been introduced through sub-sub-sub dependencies. In all cases, the root project which had the vulnerability had been fixed, generally even before the publication of the security advisory, and the issue only remained open because of some project higher up in a dependency chain that pinned their dependent version tightly enough that the patched version without the vulnerability was excluded. In most cases, it's hard to try to get a hold of the folks who maintain those packages to be able to issue an update. Even in this project, being able to get comments & action from multiple maintainers so quickly after this issue going public is somewhat of a minor miracle.

In my tracing through dependencies last week (preparing for a release), I found and described a specific example of how not pinning even this specific dependency led to faster, better security outcomes than pinning would have, in Winston. I also identified a case in the showdown library where updating & unpinning would resolve an open transitive vulnerability, but with the developers seeking to wait until some number of months in the future for a major-version release, that issue seems likely to remain open.

In short, most open-source developers have good intentions and it's far more likely that a patch update will intentionally resolve a vulnerability than intentionally introduce an issue. Because of this demonstration from this developer, I think it's now worth pinning anything from that developer to a specific version, but probably still better from a security perspective to not pin content from developers who have only demonstrated good faith engagement. Hopefully that helps answer the question!

[DABH]

NPM has removed the vandalized versions of colors.js, so I am going to close this issue for now. Winston users should be safe even on versions prior to 3.3.4, although I'd encourage upgrading to 3.4.0 for other fixes including this.

[msalimbene]

here are some memory leak error

@wuteng606 were you able to fix the memory leak issue by updating Winston?