Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed https://npmjs.com/advisories/1589 #121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JimmyBjorklund
Copy link

No description provided.

package.json Outdated Show resolved Hide resolved
@ariross
Copy link

ariross commented Dec 29, 2020

Could this be merged in? @dominictarr if you have a chance?

@jmz527
Copy link

jmz527 commented Jan 7, 2021

Let's try them again. Hey @dominictarr! You around? This PR needs approval.

@pavoltravnik
Copy link

pavoltravnik commented Jan 11, 2021

This change of version of ini is really essential to pass security checks. It seems, that @dominictarr is inactive on internet for a
few months. How we can deal with this? Someone forks this package and release it on npm as a substitute?

@blankstar85
Copy link

Bringing this to light again

@goatandsheep
Copy link

goatandsheep commented Feb 2, 2021

@TrySound can you please help?
fixes #120

@TrySound
Copy link
Contributor

TrySound commented Feb 2, 2021

🤷‍♂️

@Snipx
Copy link

Snipx commented Feb 4, 2021

Hey @dominictarr, apparently after some years the community really needs your help :) Do you think you can merge this PR and release a new version?

@dominictarr
Copy link
Owner

hey everyone! sorry to have bad news but I'm not gonna merge this. I'm gonna use this issue to point out the bigger problems with the way we do open source, for burnt out maintainers everywhere.

  1. I'm burnt out and I havn't even written any code in months.
  2. this is not actually a problem with my code, it's a dep. ini could backport the fix. then the reported thing would go away without me doing anything.
  3. I don't care about this.
  4. this is a false positive. It will be nearly impossible to actually turn this into an actual attack. this is a configuration loading library. If an attacker can write new configuration files into user space you've probably been owned anyway. But there is some tool like npm audit that's saying this is a problem, but it's not the real problem.
  5. this is not an isolated issue. I am sure there are many other cases where another ex-maintainer has a module that needs a trivial update. npm should have a way to override the dependencies of sub deps. I think that's the real solution that needs to happen here.

If you still really want me to merge this. I'll do it for $300 usd. You should be able to find me by email on transferwise. My email is in the package.json of this module. I am hoping that this will create a viral shit storm. probably loads of open source consumers will be outraged, good. I know that actually open maintainers will back me. ps. going away for the weekend and I won't look at this issue until monday. when I see a transfer into my account I'll merge it.

@dominictarr
Copy link
Owner

and yes, I could have merged it in less time than it took me to write that response. But, I'd rather do that, because I'd rather fix the underlying problem. The thing that I love about open source is that individuals actually get the agency to make fix the problems that affect them. right now I can do that better by not merging it!

haha, so tell your manager that this issue is still open because the maintainer is not merging it as a political statement

@goatandsheep
Copy link

goatandsheep commented Feb 5, 2021

Regarding becoming an ex-maintainer, I've worked with the NPM team before on issues like this. The solution is someone else comes along with a repo where it is solved and the ownership of the library gets passed along. You shouldn't have to deal with it any longer if you don't want to. There shouldn't be single point of failure. This happened with react-native-dotenv, where I took it over and renovated it. Make sure you have people you trust as collaborators on the project so it's maintained or transfer it to someone who cares. Is this a bad system? Probably. What do y'all propose?

On the flip side, you're right that updating a constant stream of dependencies is a job in itself and is something that needs to be fixed.

Github used to have auto-merge but now it's been removed from dependabot

@JimmyBjorklund
Copy link
Author

Development groups tend to solve the issue, e.g share the workload. I guess there are a few devs that could help if you let them. It's like normal work, if you sitt on all the power and don't delegate then you also have too do all the work.

@goatandsheep
Copy link

@dominictarr I have a strategy to fund open source development:

@goatandsheep
Copy link

goatandsheep commented Feb 12, 2021

I've published

this package is protected from crypto attacks and stuff by snyk

@lev-kuznetsov
Copy link

I thought kiwis are supposed to be nice. What a dick.

@goatandsheep
Copy link

@lev-kuznetsov he's not entitled to your time. Open source is hard, thankless work. No need to call him names

@lev-kuznetsov
Copy link

If you have no time for this just send github emails to spam. If you genuinely want to leave the world a little bit better ask if anybody wants to take over npm publish rights. He spent time to figure out what's going on and then spent more time coming up with fallacious arguments why he can't be bothered to make it better and then he spent more time typing it all up.

Life hack: if anyone says they're doing anything to make a political statement they're a dick regardless of actual gender.

@beenotung
Copy link

As stieban as mentioned in igorshubovych/markdownlint-cli#146 (comment), you should be able to resolve the npm audit issue by updating the dependency without any update in this package.

e.g. run pnpm update or npm update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet