senecajs / seneca

A microservices toolkit for Node.js.
http://senecajs.org
MIT License
3.95k stars 314 forks source link

security issue #939

Open wzrdtales opened 2 months ago

wzrdtales commented 2 months ago

The following security issue just got published: https://github.com/advisories/GHSA-4wm9-3qmv-gvxj

This seems to have been reported actually already month ago. https://github.com/jsonicjs/jsonic/issues/31

@rjrodger Do you need help maintaining these things?

rjrodger commented 1 month ago

There seems to be an uptick in these pretty spurious reports recently. This report is against internal undocumented utility functions.

rjrodger commented 1 month ago

to wit: https://gist.github.com/mestrtee

@mestrtee please stop annoying open source maintainers with this nonsense.

wzrdtales commented 1 month ago

It recently hit our security pipelines and stopped production for us. I don't blame github for accepting, since the repro does indeed work.

My guess is, this is a bot, b/c it is only searching for prototype pollution. Which is quite easy to find with the right tools and kinda automatable.

rjrodger commented 1 month ago

https://x.com/matteocollina/status/1791137534996586808

rjrodger commented 1 month ago

@wzrdtales sorry to hear that - I reckon you'll keep getting hit by this at random for any deps you have - it is driving a lot of people crazy

What workaround did you use?

wzrdtales commented 1 month ago

Easy quick fix was https://pnpm.io/package_json#pnpmauditconfigignorecves . pnpm is just in every regard the better npm

But we have been working on replacing seneca, which we in light of this event pushed through the door.

rjrodger commented 1 month ago

Can't fault you for that @wzrdtales. Thanks for the support over the years and I hope it was some use to you to get you to this point!

PS. I am doing a retrospective talk on seneca soon - do you have any notes or advice?

wzrdtales commented 1 month ago

It was of tremendous value yes. Our reasons are strategical, financial and technical actually. We already hard-forked seneca some time ago, but the code was not maintainable for us, so it was rather sticking with that status quo than taking care of it. We went for efficiency and simplicity, to a) save quite some money (the ms landscape we rolled it out to yesterday went done from 2000 nodes to 860), roughly ~60% (we can push about 105k rps per node process without additional logic now, this made a bigger impact than we actually expected to see) and at least a ~40% improvement on general latencies across the board, and b) keep the code maintainable.

Notes:

Advice:

Only one. Don't design your services too tiny, at least if latency is something of need, rather go for "macro" services, than microservices. Probably don't need to tell you that though.

There is quite some code that lives in the heart of the new backbone, we did not consider writing everything on our own completely, so we still use parts of seneca and still operate in the same way and with the same interfaces. We really just made this a drop in ( for our use case ).

https://github.com/WizardTales/MicroWizard

rjrodger commented 1 month ago

Wow - that's really really useful - thank you for these notes! A lot to think about - in particular the balance of features vs performance.

Best of luck @wzrdtales !!!

wzrdtales commented 1 month ago

Thanks @rjrodger will definitely still follow around.

And Github just removed it: https://github.com/github/advisory-database/pull/4603#issuecomment-2226204501 I took the freedom to report this false accusation of criticality.