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

Please fix security issues. #914

Closed
Mister-Hope opened this issue Mar 22, 2022 · 34 comments
Closed

Please fix security issues. #914

Mister-Hope opened this issue Mar 22, 2022 · 34 comments

Comments

@Mister-Hope
Copy link

image
image

@Mister-Hope
Copy link
Author

Also, is there any strong reson why commitizen choose to lock minimist version? No other packages in my project is doing that.

@kilahm
Copy link

kilahm commented Mar 23, 2022

See #913 - looks like the auto-merge of the dependency update is waiting on the stabilityDays setting of RenovateBot. I tried to find the value, but I was unable to trace it beyond https://github.com/commitizen/commitizen-renovate-config/blob/master/default.json

@Mister-Hope
Copy link
Author

I am just curious why this dep's version is fixed. If we do not pin it with an exact version, this can be fixed by updating deps myself.

@Mister-Hope
Copy link
Author

any update about this one?

@LinusU
Copy link
Contributor

LinusU commented Mar 28, 2022

I very much prefer non-pinned dependencies myself, but it was made this way before I started contributing. These days I'm not personally using this tool, and I believe that the publishing workflow doesn't work anymore (#897). Because of this I don't have much bandwidth to fix things here...

If it sounds like a good way forward, I would be open to merging a pull request that changes the dependencies to be specified with ^ (it would need to update the renovate config as well), and then doing a manual release from my computer 🚀

@urecio
Copy link

urecio commented Mar 29, 2022

Hi, any update on this? Looks like only publishing is needed now

@zSakuraEvilz
Copy link

Hi, any update on this? I need to use it but still have security issues. So my company does not allow the use.

@Mister-Hope
Copy link
Author

Half a month bro 🤡, any schedule? Or shall I fork and publish a temp version first?

@Mister-Hope
Copy link
Author

Just noticed that the Severity is lifted to Critical. And I am sure that this will bother many people due to their company policity.

@Mister-Hope
Copy link
Author

@jimthedev Help needed here

@techmunk
Copy link

techmunk commented Apr 5, 2022

We worked around this using the overrides package.json property added in npm v8. Something like the below:

  "overrides": {
    "commitizen": {
      "inquirer": "8.2.0",
      "minimist": "1.2.6"
    }
  }

Not ideal, but might help some people move forward. I had to remove node modules and possibly package-lock.json as well before doing npm install to get the changes to take effect.

Commitizen seems to be working fine with these changes, and npm audit is happy now too.

@caleb-mabry
Copy link

If the above solution doesn't work for you, we used https://www.npmjs.com/package/npm-force-resolutions to resolve the vulnerability for the time being.

@noahnu
Copy link

noahnu commented Apr 10, 2022

If using Yarn modern, you can set this resolution in your project's package.json (equivalent of npm overrides):

"[email protected]": "npm:1.2.6"

@enaukkarinen
Copy link

Any update on this? I don't see a workaround for this for people using NPM 7 :(

@hasghari
Copy link

The master branch has this fix. Is there going to be a release soon?

@yvesnrb
Copy link

yvesnrb commented Apr 18, 2022

Anyone know of an alternative program for generating git commits following the cz-conventional-changelog format?

@techmunk
Copy link

@yvesnrb best I know of is https://commitizen-tools.github.io/commitizen/, which uses python.

@machadodev
Copy link

Almost a month now. Still no release date?

@maximelafarie
Copy link

Any roadmap to fix this 💩? It seems there is a moment this issue has been reported.

# npm audit report

minimist  <1.2.6
Severity: critical
Prototype Pollution in minimist - https://github.com/advisories/GHSA-xvch-5gv4-984h
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/minimist
  commitizen  >=1.0.5
  Depends on vulnerable versions of cz-conventional-changelog
  Depends on vulnerable versions of minimist
  node_modules/commitizen
    cz-conventional-changelog  >=3.0.2
    Depends on vulnerable versions of commitizen
    node_modules/commitizen/node_modules/cz-conventional-changelog
    node_modules/cz-conventional-changelog

3 critical severity vulnerabilities

@Manokii
Copy link

Manokii commented May 10, 2022

Looks like this was fixed in #913 but maintainers need to make a new release

@pumano
Copy link

pumano commented May 11, 2022

@jimthedev please help with it. Many critical vulnerabilities here in package, need updated dependencies and release.

@Narrenschyff
Copy link

also need this, pls thanks

@spock123
Copy link

This project appears dead. Need to find alternatives

@david-heward-unmind
Copy link

can we get a release please maintainers?

@phil-gratifypay
Copy link

phil-gratifypay commented May 18, 2022

Please update minimist to 1.2.6, thank you!

@ryansonshine
Copy link

ryansonshine commented May 19, 2022

The fix has already been merged into master, the problem is that it appears the maintainers do not have access to publish
or have abandoned the project*, and have not published a new version in over a year now.

I couldn't wait any longer for this due to the number of tickets I've had opened because of this so I've created a fork with an automated publishing lifecycle, which I'm now using in all of my projects.

If anyone is looking for a solution that doesn't involve using overrides feel free to use it; it will continue to get security updates from this repository going forward since it'll be supporting enterprise projects.

Migrate

Run the following and commit the result in your project to complete the migration. (assuming you're using cz-conventional-changelog)

npm

npm uninstall commitizen cz-conventional-changelog
npx @ryansonshine/commitizen init @ryansonshine/cz-conventional-changelog --includeCommitizen --force

yarn

yarn remove commitizen cz-conventional-changelog
yarn add @ryansonshine/commitizen
npx @ryansonshine/commitizen init @ryansonshine/cz-conventional-changelog --force --yarn

@Mister-Hope
Copy link
Author

Mister-Hope commented May 19, 2022

The fix has already been merged into master, the problem is that it appears the maintainers do not have access to publish to npm, and have not had access for over a year now.

I don’t think so, the 1st contributor was publishing release in the past years ➡️@jimthedev And he still has activity in the past week on GitHub. As the project creator and 1st contributor, I think he should be still in charged with this package, and I am not supposing he just “lost access”

I would like to prefer to believe that he dropped this package maintenance and prefer to deprecate it (but without any announcement and still pin this package in his profile)

This is open source, yet he should not be blamed, but I still think that his behavior does not fit the community environment and does have negative infect in the npm ecosystem. At least he fails other contributor who still working on this project.

@ryansonshine
Copy link

ryansonshine commented May 19, 2022

Whether they don't have access or are no longer maintaining the publish cycle still ends in the same result: we're going on 60 days since this issue was opened so I took action for myself and the projects I'm involved in. Mine will continue to get security updates for my projects, if anyone else can benefit from that then great; if not well that's fine too :)

@Mister-Hope
Copy link
Author

Mister-Hope commented May 19, 2022

I am just expressing the following point:

  1. He published the first version of package, and he managed to switch to a bot account to continue publishing, I DO NOT believe he lost access. If he stop using github account or I only opened this issue for a week, I don't feel strange, but after 2 months with github activity, we can definitely tell that he choosed "ignore" from "pickup this package and give some active contributor rights to maintain".

  2. He is acting as he quit this project (or stop maintaining this project), and I think that's pretty ok. But I think as an open source project with > 20 contributor, he should try to keep this project running and give rights to other active contibutor instead of failing them, yet we are still meeting no one publishing this package. I agree that opensource authors have rights to deal with their proejcts, as I have open projects too. But I think that it's necessary to keep basic polite to do either:

    • archieve the repo and make annoucement
    • give rights to active contibuotr to keep running it

    specially when this project as >10k stars and a high download number

I just feel deep disappointed metting this situation, specially finding out this is still the first pin repo on his profile.😑

@ryansonshine
Copy link

ryansonshine commented May 19, 2022

Ah my mistake, I misread your initial comment.

I agree with both of your statements. I've updated my comment accordingly.

In an ideal world the ownership would be handed off to someone active, but considering we're coming up on 2 months for this issue I wouldn't get my hopes up.

@Manokii
Copy link

Manokii commented May 20, 2022

The fix has already been merged into master, the problem is that it appears the maintainers do not have access to publish or have abandoned the project*, and have not published a new version in over a year now.

I couldn't wait any longer for this due to the number of tickets I've had opened because of this so I've created a fork with an automated publishing lifecycle, which I'm now using in all of my projects.

If anyone is looking for a solution that doesn't involve using overrides feel free to use it; it will continue to get security updates from this repository going forward since it'll be supporting enterprise projects.

Migrate

Run the following and commit the result in your project to complete the migration. (assuming you're using cz-conventional-changelog)

npm

npm uninstall commitizen cz-conventional-changelog
npx @ryansonshine/commitizen init @ryansonshine/cz-conventional-changelog --includeCommitizen --force

yarn

npm uninstall commitizen cz-conventional-changelog
npx @ryansonshine/commitizen init @ryansonshine/cz-conventional-changelog --includeCommitizen --force --yarn

yarn migration doesn't seem to work on a monorepo, wasn't able to submit an issue in the forked repo so im adding it here.

Migration for yarn workspaces/monorepo

  1. Replace commitizen
yarn remove commitizen cz-conventional-changelog -W
yarn add -D -W @ryansonshine/commitizen @ryansonshine/cz-conventional-changelog
  1. Update commitizen path in package.json to:
// ...
  "config": {
    "commitizen": {
      "path": "./node_modules/@ryansonshine/cz-conventional-changelog"
    }
  }

@ryansonshine
Copy link

ryansonshine commented May 21, 2022

Thanks for the feedback @Manokii !

I've went ahead an enabled issues on the repository as well as updated the installation instructions for yarn on my initial comment.

github-actions bot pushed a commit to sb-mig/sb-mig that referenced this issue May 22, 2022
## [4.0.11](v4.0.10...v4.0.11) (2022-05-22)

### Bug Fixes

* **commitizen:** once again some updates for commitizen, added resolution for minimist ([d0aa816](d0aa816)), closes [/github.com/commitizen/cz-cli/issues/914#issuecomment-1094335756](https://github.com//github.com/commitizen/cz-cli/issues/914/issues/issuecomment-1094335756)
sinanbekar added a commit to sinanbekar/browser-extension-react-typescript-starter that referenced this issue May 28, 2022
sinanbekar added a commit to sinanbekar/browser-extension-react-typescript-starter that referenced this issue May 28, 2022
sinanbekar added a commit to sinanbekar/browser-extension-react-typescript-starter that referenced this issue May 28, 2022
fix(commitizen): move from unmaintained commitizen repo to forked repo commitizen/cz-cli#914
@Zhengqbbb
Copy link
Contributor

@yvesnrb best I know of is https://commitizen-tools.github.io/commitizen/, which uses python.

Try run command npx czg in your any project

@kdmcguire
Copy link

I believe this is fixed in the new release, 4.2.5.

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

No branches or pull requests